"when 'Ref' is resolved" to a parameter

kddejong commented 1 week ago

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)

randybasrs commented 2 days ago

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 commented 2 days ago

@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 commented 1 day ago

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.

aws-cloudformation / cfn-lint