v1 - Using Resource Provider Schemas

[kddejong]

Description

The effort to go to v1 will be driven by the goal to convert from CloudFormation specs to CloudFormation resource provider schemas. This will be a large change for how cfn-lint works and will result in rules having to be updated and changed. This issue will also serve to communicate the migration efforts.

Details

The CloudFormation resource provider schema is based on JSON Schema draft-07 but has modifications to handle the CloudFormation service These schemas allow us to do more straight JSON schema validation against resource properties. There are modifications to the schema and how the JSON schema validators work to handle CloudFormation specific capabilities. We find it important to provide the functionality and features that cfn-lint has had in v0 including the ability to disable and configure rules. As a result we will integrate in JSON schema validation into cfn-lint and cfn-lint will provide the functionality to massage the schemas and handle CloudFormation specific capabilities. Additionally there are checks in cfn-lint (best practices, etc.) that cannot be written into JSON schema validation.

Rule changes

All rules that have been modified or where the logic will change:

• [x] E1027 - DynamicReferenceSecureString - Depended on match_resource_sub_properties. Was re-written to use match
• [x] E1010 - GetAtt - Used the specs for valid GetAtt values
• [x] E1022 - Join - Used the specs to determine the type of a GetAtt
• [x] E1019 - Sub - Used the specs to determine the type of a GetAtt
• [x] E1029 - SubNeeded - Used the specs to determine valid GetAtt values
• [x] E6003 - Value (Outputs) - Used the specs to determine the type of a GetAtt
• [x] W2031 - AllowedPattern (Parameters) - Replaced by JSON Schema validation logic
• [x] W2030 - AllowedValue (Parameters) - Replaced by JSON Schema validation logic
• [x] E3001 - Configuration (Resources) - Replaced by JSON Schema validation. Logic is outside of the registry
• [x] E3000 - JsonSchema (Resources) - Expanded to handle registry resource schemas. Parent rule for all JSON schema validation rules
• [x] E3031 - AllowedPattern (Resource/Properties) - Replaced by JSON Schema validation logic
• [x] E3030 - AllowedValue (Resource/Properties) - Replaced by JSON Schema validation logic
• [x] E2522 - AtLeastOne (Resource/Properties) - Deleted?
• [x] E3017 - CfnSchema (Resource/Properties) (New Rule) - New rule to extend JSON Schema validation to handle scenarios not covered by the registry schema
• [x] E2520 - Exclusive (Resource/Properties) - Deleted. Handled by if/then/else logic in JSON schema validation
• [x] E2521 - Inclusive (Resource/Properties) - Deleted. Handled by if/then/else logic in JSON schema validation
• [x] E3502 - JsonSize (Resource/Properties) - Deleted. Converted to string maxLength and minLength validation
• [x] E3037 - ListDuplicates (Resource/Properties) - Replaced by JSON Schema validation logic
• [x] I3037 - ListDuplicatesAllowed (Resource/Properties) - Replaced by JSON Schema validation logic
• [x] E3032 - ListSize (Resource/Properties) - Replaced by JSON Schema validation logic
• [x] E3034 - NumberSize (Resource/Properties) - Replaced by JSON Schema validation logic
• [x] E2523 - OnlyOne (Resource/Properties) - Replaced by oneOf logic in JSON schema
• [x] E3002 - Properties (Resource/Properties) - Replaced by properties and additionalProperties in JSON schema
• [x] E3003 - Required (Resource/Properties) - Replaced by required in JSON schema
• [x] E3017 - RequiredBasedOnValue (Resource/Properties) - Replaced by if/then/else logic using cfnSchema and JSON schema
• [x] E3033 - StringSize (Resource/Properties) - Replaced by minLength and maxLength in JSON schema. May still need to add exceptions for dynamic references
• [x] E3018 - UnwantedBasedOnValue (Resource/Properties) - Replaced by if/then/else logic using cfnSchema and JSON schema
• [x] E3012 - ValuePrimitiveType (Resource/Properties) - Replaced by type in JSON schema
• [ ] E3008 - ValueRefGetAtt (Resource/Properties) - WIP

Breaking changes

This will serve as a list of changes that will occur in the migration from v0 to v1

• match_resource_sub_properties will be deprecated. This function was based on the specs and is not easily converted into the resource provider schema approach

Issues

• [x] Some resource schemas have readOnlyProperties that are still write-able. Since we are removing them for validation we need to know the list of exceptions
• [x] Packaging doesn't like linked files so they are dropped. Need a different approach to handling marking resources as cached

Action Items

• [x] Convert custom or 3rd party registry schema validation to the new rule E3000
• [x] Don't run property checks multiple times if the specs across regions are cached
• [x] Convert enum values taken from boto
• [x] Convert other manually created allowedvalues, patterns, etc.
• [x] Write tests to validate JSON Schema docs and extensions
• [x] Add in AWS::CDK::Metadata schema

[PatMyron]

• match_resource_sub_properties will be deprecated

undocumented and no other public repos show up in sourcegraph search using that, which bodes well

[kddejong]

When dealing with oneOf, allOf, anyOf, if/then/else logic JSON schema will fail with a rolled up error and the context will include all the errors determined from the sub schemas. While this works it hides some of the information for the user to determine what actually caused their issues and how to fix it.

Do not raise any generic errors for these types allOf create multiple errors for all sub errors anyOf create multiple errors for all sub errors oneOf return the best error if/then/else return the best error

[whoDoneItAgain]

i saw the rc1 released yesterday. i ran it against one my pipelines CF stacks. It returned a many findings, though basically the same 3-5 findings repeatedly. Where would you like the feedback?

[kddejong]

I would love the feedback. I've been trying to find as many templates as I can to run against behind the scenes but I'm running out of additional issues to chase. Thanks!

[whoDoneItAgain]

E3001 and E3030 are the most seen errors. Below are the unique errors and a snippet example.

E3030 - {'Fn::FindInMap': ['StaticParams', 'S3EmptierLambda', 'LambdaOSArchitechture']} is not one of ['x86_64', 'arm64']

LambdaLogGroup: Type: AWS::Logs::LogGroup DeletionPolicy: Delete UpdateReplacePolicy: Delete Properties: LogGroupName: !Sub '/aws/lambda/${Region}-${VPCName}-${AppName}-${LambdaPurpose}' RetentionInDays: !FindInMap [GlobalSettings, GlobalSettings, LogRetention]`

E3001 - {'Fn::If': ['ConditionDev', 'Delete', 'Retain']} is not one of ['Delete', 'Retain', 'Snapshot']

E3001 - {'Fn::If': ['ConditionDev', 'Delete', 'Retain']} is not of type 'string'

ALBLogsBucket: Type: AWS::S3::Bucket DeletionPolicy: !If - ConditionDev - Delete - Retain UpdateReplacePolicy: !If - ConditionDev - Delete - Retain Properties:

E0002 - Unknown exception while processing rule E6003: Resource type 'Custom::R53EndpointIp' is not found in 'us-east-1'

E1010 - 'EndpointIds' is not one of ['FirewallId', 'ResourceArn'] (This is an interesting one. Erroring on a custom resource)

NetworkFirewallVpceAz1: Type: Custom::FirewallVpceFinder Condition: ConditionCreatePublicSubnets Properties: ServiceToken: Fn::ImportValue: !Sub '${Region}-${VpcName}-lambda-firewall-vpce-finder-fn-arn' EndpointIds: !GetAtt NetworkFirewall.EndpointIds AvailabilityZone: !GetAtt FirewallSubnet1.AvailabilityZone

E3032 - [] is too short

(Its barking about the Regions section)

ClientVpnAuthStackSet: Type: AWS::CloudFormation::StackSet Condition: ConditionCreateCrossRegionAuthRules Properties: ... StackInstancesGroup: - DeploymentTargets: Accounts: - !Ref AWS::AccountId Regions: - !If - ConditionCreateAuthorizationRulesUse1 - us-east-1 - !Ref AWS::NoValue - !If - ConditionCreateAuthorizationRulesUse2 - us-east-2 - !Ref AWS::NoValue - !If - ConditionCreateAuthorizationRulesUsw2 - us-west-2 - !Ref AWS::NoValue - !If - ConditionCreateAuthorizationRulesAps1 - ap-south-1 - !Ref AWS::NoValue

E3012 - Specify only 'SubnetMappings' or 'Subnets'

ApplicationLoadBalancer: Type: AWS::ElasticLoadBalancingV2::LoadBalancer Condition: ConditionCreateLambda Properties: Type: application Name: !Sub '${Region}-cross-vpc-testing-alb' Scheme: internal SecurityGroups: - !Ref ALBSecurityGroup Subnets: - Fn::ImportValue: !Sub '${Region}-${VPCName}-private-subnet-1-id' - Fn::ImportValue: !Sub '${Region}-${VPCName}-private-subnet-2-id' - !If - ConditionAzCount3 - Fn::ImportValue: !Sub '${Region}-${VPCName}-private-subnet-3-id' - !Ref AWS::NoValue

[kddejong]

Action items:

• [x] Language extension schema changes. This will handle E3001 conditions in the DeletionPolicy issue. Docs
• [x] Update enum, pattern, type to handle functions
• [x] Fix an issue with GetAtts where we dropped arrays that have a ref in the items portion of the schema
• [ ] E3012 - handle intrinsic functions in cfnSchema validation
• [x] Update conditions logic to include resource or output level Condition scenarios

I'll add more details as I investigate and figure out the solutions.

[kddejong]

for E3032 - [] is too short can you tell me what Condition: ConditionCreateCrossRegionAuthRules is configured like?

I am testing this and I'm not able to repeat this error. I used the following condition logic.

  ConditionCreateAuthorizationRulesUse1: !Equals [!Ref AWS::Region, "us-east-1"]
  ConditionCreateAuthorizationRulesUse2: !Equals [!Ref AWS::Region, "us-east-2"]
  ConditionCreateAuthorizationRulesUsw2: !Equals [!Ref AWS::Region, "us-west-2"]
  ConditionCreateAuthorizationRulesAps1: !Equals [!Ref AWS::Region, "ap-south-1"]
  ConditionCreateCrossRegionAuthRules: !Or [
    !Condition ConditionCreateAuthorizationRulesUse1,
    !Condition ConditionCreateAuthorizationRulesUse2,
    !Condition ConditionCreateAuthorizationRulesUsw2,
    !Condition ConditionCreateAuthorizationRulesAps1 ]

[whoDoneItAgain]

Mappings: ClientVpn: Fn::Transform: Name: AWS::Include Parameters: Location: !Sub 's3://${Region}-${Environment}-infra-pipeline-bucket-${AWS::AccountId}/resource-sync/transforms/cvpn/mapping-cvpn-${ClientVpnPurpose}.yaml'

Conditions: ConditionCreateCrossRegionAuthRules: !Or - !Condition ConditionCreateAuthorizationRulesUse1 - !Condition ConditionCreateAuthorizationRulesUse2 - !Condition ConditionCreateAuthorizationRulesUsw2 - !Condition ConditionCreateAuthorizationRulesAps1

ConditionCreateAuthorizationRulesUse1: !Equals [!FindInMap [ClientVpn, Enabled, use1], true]

ConditionCreateAuthorizationRulesUse2: !Equals [!FindInMap [ClientVpn, Enabled, use2], true]

ConditionCreateAuthorizationRulesUsw2: !Equals [!FindInMap [ClientVpn, Enabled, usw2], true]

ConditionCreateAuthorizationRulesAps1: !Equals [!FindInMap [ClientVpn, Enabled, aps1], true]

Conents of transform in mapping:

Acm: DomainName: domainName obscured ZoneId: zoneId obscured Enabled: aps1: false use1: true use2: false usw2: false Subnet: Assignment: 2

[kddejong]

Thank you. Let me look into it. I think this is a problem in v0 but there was an expanded check here in v1 that is highlighting the issue.

[whoDoneItAgain]

down to a single error code now. E1010

E1010 'EndpointIds' is not one of ['FirewallId', 'ResourceArn'] templates/common/vpc/vpc.yaml:854:7

  NetworkFirewallVpceAz3:
    Type: Custom::FirewallVpceFinder
    Condition: ConditionCreatePublicSubnetsAz3
    Properties:
      ServiceToken:
        Fn::ImportValue: !Sub '${Region}-${VpcName}-lambda-firewall-vpce-finder-fn-arn'
      EndpointIds: !GetAtt NetworkFirewall.EndpointIds
      AvailabilityZone: !GetAtt FirewallSubnet3.AvailabilityZone

  NetworkFirewall:
    Type: AWS::NetworkFirewall::Firewall
    Condition: ConditionCreatePublicSubnets
    Properties:
      FirewallName: !Sub '${Region}-${VpcName}-network-firewall'
      FirewallPolicyArn: !Ref NetworkFirewallPolicy
      VpcId: !Ref VPC
      SubnetMappings:
        - SubnetId: !Ref FirewallSubnet1
        - SubnetId: !Ref FirewallSubnet2
        - !If
          - ConditionAzCount3
          - SubnetId: !Ref FirewallSubnet3
          - !Ref AWS::NoValue
      Description: !Sub '${Region}-${VpcName}-network-firewall'
      Tags:
        - Key: Name
          Value: !Sub '${Region}-${VpcName}-network-firewall'

[kddejong]

got that one fixed now. Thanks for all the testing @whoDoneItAgain

[whoDoneItAgain]

yep. no problem. I'll run it against some of my other repos too and see what else I get

[whoDoneItAgain]

Do you want me to continue putting these here or open new issues for each?

E3510 datetime.date(2012, 10, 17) is not one of ['2008-10-17', '2012-10-17'] (easy enough to fix on my end by quoting the date but v0 doesnt error on this)

  GluePolicy:
    Metadata:
      cfn_nag:
        rules_to_suppress:
          - id: F4
            reason: Scoping will occur before promotion to Qa
    Type: AWS::IAM::Policy
    Condition: ConditionNotDev
    Properties:
      PolicyName: !Sub '${GluePurpose}-policy'
      PolicyDocument:
        Version: 2012-10-17
        Statement:
          - Effect: Allow
            Action:
              - glue:InvokeGlue

E1010 'Value' is not one of ['Id'] - Erroring on both FromPort and ToPort (I suspect this is fixed based on the last one i sent but since that was a custom resource I'm adding this)

  SourceSubnetEgressRule3:
    Type: AWS::EC2::SecurityGroupEgress
    Condition: ConditionDmsSgEgressRule3
    Properties:
      Description: !Sub "DMS Task '${TaskName}' - Source DB Access"
      IpProtocol: tcp
      FromPort: !If
        - ConditionImportedSource
        - !GetAtt ImportedSourceDatabasePortSSM.Value
        - !Ref LocalSourceDbPort
      ToPort: !If
        - ConditionImportedSource
        - !GetAtt ImportedSourceDatabasePortSSM.Value
        - !Ref LocalSourceDbPort
      CidrIp: !Ref SourceDbSubnet3
      GroupId:
        Fn::ImportValue: !Sub '${Region}-${BusinessUnit}-${Environment}-${AppName}-sg-dms-${DMSInstanceNumber}'

E3003 'NoncurrentDays' is a required property

          - Id: non-current-versions
            NoncurrentVersionExpiration:
              NoncurrentDays: !If
                - ConditionExpireNoncurrentVersions
                - !Ref NoncurrentVersionExpiration
                - !Ref AWS::NoValue
            NoncurrentVersionTransitions:
              - StorageClass: INTELLIGENT_TIERING
                TransitionInDays: 0
            Status: Enabled

E2523 Either CIDR Block or IPv4 IPAM Pool and IPv4 Netmask Length must be provided

Parameters:
  VPCCIDR:
    Description: VPC CIDR
    Type: String
    AllowedPattern: ^(?:10\.(19[2-9]|2[0-4]\d|25[0-5])\.([048]|(([1]?)([02468][048]|[13579][26]))|(([2]?)([024][048]|[13][26]))|252)\.0\/22)|(?:auto-22)$
    Default: auto-22

Conditions:
  ConditionAutoAssignVPCCidr: !Equals
    - !Select [0, !Split ['-', !Ref VPCCIDR]]
    - auto

Resources:
  VPC:
    Type: AWS::EC2::VPC
    Properties:
      EnableDnsSupport: true
      EnableDnsHostnames: true
      CidrBlock: !If
        - ConditionAutoAssignVPCCidr
        - !Ref AWS::NoValue
        - !Ref VPCCIDR
      Ipv4IpamPoolId: !If
        - ConditionAutoAssignVPCCidr
        - !FindInMap [IpamPoolsMap, !Ref 'AWS::Region', !Ref BusinessUnit]
        - !Ref AWS::NoValue
      Ipv4NetmaskLength: !If
        - ConditionAutoAssignVPCCidr
        - !Select [1, !Split ['-', !Ref VPCCIDR]]
        - !Ref AWS::NoValue
      Tags:
        - Key: Name
          Value: !Sub '${Region}-${BusinessUnit}-${Environment}-vpc'

[andrew-glenn]

I realize it's early - however will there be migration guidance for end-users whom have written custom rules ?

Edited to add: If that hasn't been really considered much yet, I'm happy to collaborate offline

[kddejong]

@whoDoneItAgain I think I got most of these items resolved but I did re-work some of how the json schema validation was handled so hopefully I didn't cause any more.

[kddejong]

@andrew-glenn absolutely. Not a lot has changed but there are a few things. There are probably some more clever ways to write rules though so should work on documenting that.

[kddejong]

https://aws.amazon.com/blogs/devops/aws-cloudformation-linter-v1/