Issue #95 - support for user-defined resource attribute values.

[RazzM13]

Added support for user-defined custom resource attribute values.

[RazzM13]

The reason for the Fn::GetAtt for an arbitrary attribute on a custom resource should return a mock result test failing is that cfn-lint does not anymore attempt to guess the custom resource attribute's value as it is expecting it to be provided on the command line.

One obvious course of action would be to copy the behaviour of the guess-parameters functionality and implement additional options, such as --guess-custom-resource-attributes, --no-guess-custom-resource-attributes and --only-guess-custom-resource-attributes.

What do you think @martysweet, @akdor1154 and anyone else?

[martysweet]

What happens if multiple custom resources are present in one template?

[RazzM13]

In hope that this is what you're referring to, let's assume the following template:

AWSTemplateFormatVersion: '2010-09-09'
Resources:
  AllSecurityGroups:
    Type: Custom::Split
    Properties:
      ServiceToken: "somefunctionarn"
  AllSecurityGroups2:
    Type: Custom::Split
    Properties:
      ServiceToken: "somefunctionarn2"

Outputs:
  AllSecurityGroups:
    Description: Security Groups that are associated with the EC2 instance
    Value:
      Fn::Join:
      - ", "
      - Fn::GetAtt:
        - AllSecurityGroups
        - Value
  AllSecurityGroups2:
    Description: Security Groups that are associated with the EC2 instance
    Value:
      Fn::Join:
      - ", "
      - Fn::GetAtt:
        - AllSecurityGroups2
        - Value

1. If we attempt to validate without any custom resource attribute defined, we get:

$ cfn-lint validate 2xcustom.yaml
0 infos
0 warn
4 crit
Resource: Outputs > AllSecurityGroups > Value
Message: Value for custom resource attribute was not provided
Documentation: http://docs.aws.amazon.com/search/doc-search.html?searchPath=documentation-guide&searchQuery=Fn::GetAtt&x=0&y=0&this_doc_product=AWS+CloudF
ormation&this_doc_guide=User+Guide&doc_locale=en_us#facet_doc_product=AWS%20CloudFormation&facet_doc_guide=User%20Guide

Resource: Outputs > AllSecurityGroups > Value
Message: Invalid parameters for Fn::Join
Documentation: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-join.html

Resource: Outputs > AllSecurityGroups2 > Value
Message: Value for custom resource attribute was not provided
Documentation: http://docs.aws.amazon.com/search/doc-search.html?searchPath=documentation-guide&searchQuery=Fn::GetAtt&x=0&y=0&this_doc_product=AWS+CloudF
ormation&this_doc_guide=User+Guide&doc_locale=en_us#facet_doc_product=AWS%20CloudFormation&facet_doc_guide=User%20Guide

Resource: Outputs > AllSecurityGroups2 > Value
Message: Invalid parameters for Fn::Join
Documentation: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-join.html

Template invalid!

2. If we attempt to validate but we specify a single custom resource attribute, we get:

$ cfn-lint validate 2xcustom.yaml -c AllSecurityGroups.Value=[1\\,2\\,3]
0 infos
0 warn
2 crit
Resource: Outputs > AllSecurityGroups2 > Value
Message: Value for custom resource attribute was not provided
Documentation: http://docs.aws.amazon.com/search/doc-search.html?searchPath=documentation-guide&searchQuery=Fn::GetAtt&x=0&y=0&this_doc_product=AWS+CloudF
ormation&this_doc_guide=User+Guide&doc_locale=en_us#facet_doc_product=AWS%20CloudFormation&facet_doc_guide=User%20Guide

Resource: Outputs > AllSecurityGroups2 > Value
Message: Invalid parameters for Fn::Join
Documentation: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-join.html

Template invalid!

3. If we attempt to validate and we do specify both custom resouce attribute values correctly:

$ cfn-lint validate 2xcustom.yaml -c AllSecurityGroups.Value=[1\\,2\\,3],AllSecurityGroups2.Value=[4\\,5\\,6]
0 infos
0 warn
0 crit
Template valid!

4. If we attempt to validate and we don't correctly specify one of the custom resource attribute values:

$ cfn-lint validate 2xcustom.yaml -c AllSecurityGroups.Value=[1\\,2\\,3],AllSecurityGroups2.Value=[4\\,5\\,6],AllSecurityGroups.Value
=gigi
0 infos
0 warn
1 crit
Resource: Outputs > AllSecurityGroups > Value
Message: Invalid parameters for Fn::Join
Documentation: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-join.html

Template invalid!

As you can see from the previous examples, the value of the custom-resource-attributes argument is used to build object representations of the specified custom resources and the properties of the aforementioned are updated based on the order of their occurence during processing therefore, AFAICT, there could be any number of individual custom resources and a multitude of attributes for each them and as long as they depict the contextually correct type of value and are not overriden with an incorrect one, by mistake, then everything should just-work(TM) 😈

PS: the above examples are with all the relevant PR's merged.

[RazzM13]

I've just managed to merge the current master into this branch but we still have the above mentioned test failling, @martysweet please advise 😁?

[akdor1154]

If I understand correctly (PR has no added tests and no docs, wink wink) this lets the user specify the attributes to mock by the logical resource name?
I would find it more useful to mock by the custom resource type. Not sure if this matches your expected usage.
Also maybe (not quite sure of Marty's normal git workflow) it would be better to rebase on master instead of merging master?

git checkout -b fn_getatt_custom_resource_saved # just in case

# update your master
git checkout master
git fetch upstream master
git reset --hard upstream/master

# rebase your branch
git checkout fn_getatt_custom_resource
git reset --hard c02edb5 # reset to before your merge commits
git rebase master # rebase on master
# you might need to fix conflicts here, git will walk you through it. Generally just
#  edit conflicting files
#  git add file
#  git rebase --continue

If something goes wrong your branch as now is saved as fn_getatt_custom_resource_saved.

[RazzM13]

Hi @akdor1154, thank you very much for the rebasing advice, I've adjusted the PR so that the changes are finally legible 😁

In regards, to the lack of documentation and tests, I've intentionally omitted them due to the fact that I wasn't really sure if my implementation was adequate and I was envisioning this PR to be more of a preliminary version / collaborative effort than a penultimatum.

Last but not least, you are correct, my vision for the -c command-line option is for it to accept lists of values and each value to be of the format: LogicalResourceName.PropertyName=SomeValue; I believe that this coincides with the current behaviour of the --parameters and --pseudo options and helps provide overall consistency.

Now, AFAICT and please correct me if I'm wrong, you are suggesting that the aforementioned value format should be something like: CustomResourceType.PropertyName=SomeValue. From my limited POV, this approach does seem more appealing because it allows a user to specifiy a mock value that could be applicable to a bulk of resources however, it does raises the dilemma of how to distinguish the CustomResourceType at runtime due to the fact that it can't be determined simply by checking the resource Type attribute's value, as it can have multiple forms such as AWS::CloudFormation::CustomResource and Custom::String (String can be any custom value), and probably there could also be a distinction between custom resources of the same type based on the ServiceToken attribute's value, we might be able to use something similar to CustomResourceType<ServiceToken>.PropertyName=SomeValue or ServiceToken.PropertyName=SomeValue, what are your thoughts on this?

[martysweet]

So we also have #149 "AWS::CloudFormation::Stack", which raises the need for other resources which might have variable attribute output - yay CFN Specification! This should no longer be focused on just Custom resource types. I'm sure there are other resources which probably have variable attribute output.

I feel overriding a whole type can be useful in some scenarios, but heavy-handed in others, so perhaps this calls for a dynamic way to give power users the flexibility they desire.

-c MyLambdaCustom.Attribute1="gg",MyCFNStack.VPCID="vpc-abcde"

-c AWS::CustomResource.Attribute1="abc"

In fact, we could allow both on a single cli parameter, as a logical name can never be the same as a resource name - a simple regex of allowed logical name characters can confirm this.

Then it would be

make map of -c parameters
   get all resource type overrides
   add to cfn specification object for this run, derive type from given values
   store a default value somewhere? maybe for reference at validation time?

At validation time for !GetAtt

  check if any custom attributes have been given to this resource directly use that value
  otherwise check the cfn spec - apply any default value if stored
  do magic

[RazzM13]

Could this be an augmented version of --pseudo parameters and perhaps replace it?

[akdor1154]

re types - yeah. I was actually thinking about the arbitrary part of the Type: Custom::TypeName (String in your example), but this only aligns with my usage, there is nothing forcing other cfn users to write their templates this way. I expect a fair amount of them do though. The service token of course would also be a way to do this.

Overriding per name would be also be a nice general purpose way for users to be able to do Stack Attributes though, maybe it's worth implementing both ways.

Not sure how useful it would be though, as it means all of your templates would need special attention for cfn-lint to work with them. Targeting by type means your could preconfigure cfn-lint in a script or something (I have it wrapped in something similar) to already know about all your custom resources.

[martysweet]

all of your templates would need special attention for cfn-lint to work with them

This is certainly something I want to avoid, I don't want any learning curve to using the tool, so assumptions (like we currently have) would probably be the best way to go.

Most users, unless they are aware of the advanced techniques they are using, will be returning strings from custom attributes, so this should be assumed if there is no additional CLI information to suggest otherwise.

[RazzM13]

AFAICT, everyone agrees that the way to move forward with this is by allowing user-defined attributes to be set at the resource-Type level (something similar to what @martysweet suggested above) and perhaps if it's not too difficult and doesn't impose any special attention on behalf of the user at the resource-Name level too. Hope I got that right if not then please advise.

PS: I am currently experimenting with the resource-Type version ... so stay tuned, more to follow 😁

[RazzM13]

I've just managed to write up the initial version for user-defined resource attribute values that are matched based on the resource type. How does this look?

[RazzM13]

In hopes that what I've just done is ok, a short summary of what remains:

• investigate and implement user-defined attributes for logical resource names, perhaps they may take precedence over resource-type based ones;
• infer a runtime value (potentially string) for custom resources that the user has not defined attributes for, which will fix the currently failling test;
• write tests and docs.

@akdor1154, @martysweet if you could both verify that I'm not missing something from this list and please advise if you would prefer a certain behaviour.

[martysweet]

The approach seems sane. It would be good if we can emit an info for usability when assuming values for backwards compatibility.

A logical resource attr should take precedence over a base resource type attr.

[RazzM13]

Hi @martysweet and @akdor1154, sorry for taking so long with this but I was working on something else that sparked my personal curiosity, which I'll be sharing with you shortly.
In regards to the previous commits, I've made a few changes such as dropping deep-assign as some weird issues were occuring and I've just noticed that it has been deprecated.
Please let me know if any changes need to be made.

[martysweet]

Just a couple of things. @akdor1154 Any thoughts?

[martysweet]

This doesn't format properly, is the bullet point is in the wrong place?

[martysweet]

There are a few others which don't format correctly. Leave this and I will clean it up afterwards

[RazzM13]

Ok.

[martysweet]

Need tests for this block

[RazzM13]

Tests added in b0c9a22. Please let me know if this new style is ok otherwise I'll change it to reflect the others.

[martysweet]

what does the cheeky ! do on the end of this?

[RazzM13]

Nice catch, the cheeky ! disables validation as Typescript believes that type can be undefined or at least non-existent in ResourceTypes however, this should not occur in this particular scenario. I actually borrowed it from the previous code.

[martysweet]

Can we use indexof() for this?

[RazzM13]

I don't see why not ...

[RazzM13]

Now I remember, by using indexOf we either have to use!!~ or == -1, which both seem kind of cheeky to me ;) but sure I'll change it now.

[martysweet]

https://xkcd.com/1171/ ;)

str.indexOf("::") > -1 ?

I dunno, @akdor1154 any suggestions on the neatest/cleanest way of simple substring checking in TS?

[RazzM13]

:)

[martysweet]

@RazzM13 I will be looking at merging this and publishing v1.7.0, probably on Sunday.

[martysweet]

Should this be a devdependancy?

[RazzM13]

Affirmative, thank you.

[RazzM13]

I believe I've finally managed to integrate master without losing history :)