Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"when 'Ref' is resolved" to a parameter #3378

Open
kddejong opened this issue Jun 22, 2024 · 3 comments
Open

"when 'Ref' is resolved" to a parameter #3378

kddejong opened this issue Jun 22, 2024 · 3 comments
Labels
question Further information is requested

Comments

@kddejong
Copy link
Contributor

cfn-lint Version

v1.3.0

cfn-lint will validate your template parameters against the resource provider schemas. To do this we use any values that are provided in the template including Default and AllowedValues. AllowedValues will be used if provided and if not we use the Default value.

The result can be confusing so I want to discuss how some of the expectations are and to use this issue to track this issue to see if it needs to be changed.

Basic example

To represent the issue we will use this basic template

Parameters:
  MyImageId:
    Type: String
    Default: ""
Resources:
  MyInstance:
    Type: AWS::EC2::Instance
    Properties:
      ImageId: !Ref MyImageId
      InstanceType: t2.micro

returns the below error because when we resolve the Default value we do not end up with a valid AMI ID

E1152 {'Ref': 'MyImageId'} is not a 'AWS::EC2::Image.Id' when 'Ref' is resolved

Resolutions

Conditions

Sometimes we use the default parameter to represent an optional parameter and we wrap it in a condition. The following template will be error free as cfn-lint can now determine the value will not be "" when ImageId is validated.

Parameters:
  MyImageId:
    Type: String
    Default: ""
Conditions:
  IsImageId: !Not [!Equals [!Ref MyImageId, ""]]
Resources:
  MyInstance:
    Condition: IsImageId
    Type: AWS::EC2::Instance
    Properties:
      ImageId: !Ref MyImageId
      InstanceType: t2.micro

No Default

If we require the template implementer to provide a valid value remove the Default property. If we remove Default we can use other parameter properties (AllowedPattern) to better validate the parameter value. We do this because we are expecting the template user to provide a value when they are deploying the template.

Parameters:
  MyImageId:
    Type: String
    AllowedPattern: "ami-.+"  # not meant to be perfect
Resources:
  MyInstance:
    Type: AWS::EC2::Instance
    Properties:
      ImageId: !Ref MyImageId
      InstanceType: t2.micro

"Valid" Default

For this we will provide a "valid" value as the Default value.

Parameters:
  MyImageId:
    Type: String
    Default: "ami-1234567890abcdef0"
Resources:
  MyInstance:
    Type: AWS::EC2::Instance
    Properties:
      ImageId: !Ref MyImageId
      InstanceType: t2.micro

You can also use Metadata to provide hints to the user that are using the console to deploy the template.

Metadata:
  AWS::CloudFormation::Interface:
    ParameterLabels:
      MyImageId:
        default: Provide a valid image ID (ami-1234567890abcdef0)
@kddejong kddejong added the question Further information is requested label Jun 22, 2024
@kddejong kddejong pinned this issue Jun 22, 2024
@randybasrs
Copy link

Thank you for being proactive about this. After the 1.0 update we have a large number of lint errors we're working through and this represents a decent portion of them, especially with our code modules. For those errors, I think removing the default for the parameters (which are blank) we expect from a user will be a great solution. I have not tested the module deployment or usage yet but the linting errors are gone after removing the blank defaults and I plan to add the proper validation to the schema so we can lint things like the ImageId and SecurityGroupIds list.

The documentation for figuring out how to use custom schemas, override specs, and custom rules could be better but I don't have any specific advice other than it could be more cohesive with, perhaps, a few more examples of using .cfnlintrc in conjuction with other external files.

@kddejong
Copy link
Contributor Author

kddejong commented Jul 2, 2024

@randybasrs If you are wrapping those Default: "" usages by a condition and we are still showing an error please let me know. We have put in fixes to account for condition logic meaning we shouldn't validate the "" against a pattern, enum, etc. if a condition wouldn't allow us to reach that part of the code.

Heard on the documentation let me see what we can do to improve that documentation. I'll start tackling that next week.

@randybasrs
Copy link

randybasrs commented Jul 3, 2024

To confirm, using Default: "" with some of our non required parameters did function properly as long as there was a condition for that particular property. Works like a champ!

Ex:

  UserData:
    !If
      - hasUserData
      - !Ref userData
      - !Ref "AWS::NoValue"
  PrivateIpAddress:
    !If
      - useDHCP
      - !Ref "AWS::NoValue"
      - !Ref staticIP

Both of these EC2 properties have a defulat of '' which expectedly allows a user to specify the static IP or userdata or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants