More advanced Offline Validation and Linting

[afronski]

Let's discuss here what we would love to see as a next step of validation / linting capabilities, our list so far:

• In many cases fails during CloudFormation run are related with particular name length:
  • E.g. maximum length is 64 characters for a particular value.
  • Problem: there is no specification for that, only way to determine that is trial and error. 😭
• Validating IP and CIDR.
  • Also helpful will be checking relations between:
    • E.g. Subnets inside VPC should be a subset of VPC CIDR and so on.
• Uniqueness / Missing exports for a project (we need a notion of project first).
• Check general state of infrastructure before run:
  • E.g. check before starting the CF stack that your subnet has expected number of IPs to assign EC2.
• Suggestions regarding programming constructs.
  • E.g. use Fn::Sub instead of Fn::Join and so on.
    • Or you can use ${AWS::Region} instead of your own variable.

[berespt]

• verification of parameters, which are not included in the schema, e.g. notification types for ASG. They can be obtained via API
• verification of IAM PolicyDocument actions:
  • verify if IAM permission exists (e.g. autoscaling:DescribeAutoScalingGroups)
  • verify if expression which uses wildcard, e.g. autoscaling:Describe* matches any existing permissions (and print the matching ones?)

[afronski]

We would love to have also:

• Watch mode that listens of filesystem changes from a certain directory and automatically validates the changed files.

[jlampar]

Tried fixing the fails concerning the values exceeding 64 characters. I've locally added the new function to validators package which checks if any string present within the template is longer than 64 characters. If so - it throws the warning but doesn't states that the template is invalid (there are just warnings with exact positions of their occurrence). It is scanning the whole Resources and lives inside validateResources function:

func validateResources(resources map[string]template.Resource, specification *specification.Specification, sink *logger.Logger, deadProp []string, deadRes []string) bool {
    var tooLong bool
    for resourceName, resourceValue := range resources {
        if deadResource := sliceContains(deadRes, resourceName); !deadResource {
            resourceValidation := sink.AddResourceForValidation(resourceName)

HERE ---->  tooLong = validators.IsValueTooLong(resourceValue, resourceValidation)
            if resourceSpecification, ok := specification.ResourceTypes[resourceValue.Type]; ok {
                for propertyName, propertyValue := range resourceSpecification.Properties {
                    if deadProperty := sliceContains(deadProp, propertyName); !deadProperty {
                        validateProperties(specification, resourceValue, propertyName, propertyValue, resourceValidation)
                    }
                }
            } else {
                resourceValidation.AddValidationError("Type needs to be specified")
            }
            if validator, ok := validatorsMap[resourceValue.Type]; ok {
                validator.(func(template.Resource, *logger.ResourceValidation) bool)(resourceValue, resourceValidation)
            }

        }
    }
    if tooLong {
        sink.Always("WARNING! Encountered strings longer than 64 characters. There is some probability that this could make this template rejected by the CloudFormation.")
    }
    return !sink.HasValidationErrors()
}

The sample template:

AWSTemplateFormatVersion: "2010-09-09"

Description: >
  Setting up IAM user for Packer. That will be used to spin up EC2 instance and EBS volume, also we are creating here
  IAM role and instance profile that will be associated to the machine in order to work properly with YUM
  repository stored on S3.

Parameters:

  ENV:
    Description: >
      This points out which region and stage should be used - we call it an environment.
    Type: String
    Default: eu-central-1.dev
    ConstraintDescription: >
      Must be one of the defined allowed values.

Mappings:

  EnvSettings:
    eu-central-1.dev:
      Stage: dev

Resources:

  PackerUser:
    Type: AWS::IAM::User
    Properties:
      UserName: 
        Fn::Sub:
        - "this-name-is-way-too-long-if-anyone-would-ask-me-packer-user-${AWS::Region}-${Stage}"
        - { Stage: !FindInMap [ EnvSettings, !Ref "ENV", Stage ] }

  PackerUserAccessKey:
    Type: AWS::IAM::AccessKey
    DependsOn:
      - PackerUser
    Properties:
      Serial: 0
      Status: Active
      UserName:
        Ref: "PackerUser"

  PackerUserPolicy:
    Type: AWS::IAM::Policy
    DependsOn:
      - PackerUserAccessKey
    Properties:
      CidrBlock: "0.0.0.0/24"
      Users:
        - !Ref "PackerUser"
      PolicyName: 
        Fn::Sub:
          - "this-name-surely-is-longer-than-64-characters-in-my-humble-opinion-packer-user-policy-${AWS::Region}-${Stage}"
          - { Stage: !FindInMap [ EnvSettings, !Ref "ENV", Stage ] }
      PolicyDocument:
        Statement:
          - Effect: Allow
            Action:
              - iam:PassRole
              - ec2:AttachVolume
              - ec2:AuthorizeSecurityGroupIngress
              - ec2:CopyImage
              - ec2:CreateImage
              - ec2:CreateKeypair
              - ec2:CreateSecurityGroup
              - ec2:CreateSnapshot
              - ec2:CreateTags
              - ec2:CreateVolume
              - ec2:DeleteKeypair
              - ec2:DeleteSecurityGroup
              - ec2:DeleteSnapshot
              - ec2:DeleteVolume
              - ec2:DeregisterImage
              - ec2:DescribeImageAttribute
              - ec2:DescribeImages
              - ec2:DescribeInstances
              - ec2:DescribeRegions
              - ec2:DescribeSecurityGroups
              - ec2:DescribeSnapsots
              - ec2:DescribeSubnets
              - ec2:DescribeTags
              - ec2:DescribeVolumes
              - ec2:DetachVolume
              - ec2:GetPasswordData
              - ec2:ModifyImageAttribute
              - ec2:ModifyInstanceAttribute
              - ec2:ModifySnapshotAttribute
              - ec2:RegisterImage
              - ec2:RunInstances
              - ec2:StopInstances
              - ec2:TerminateInstances
            Resource: "*"

  YumRepositoryAccessRole:
    Type: AWS::IAM::Role
    Properties:
      RoleName: 
        Fn::Sub:
        - "this-is-too-long-as-far-as-I-know-yum-access-packer-${AWS::Region}-${Stage}"
        - { Stage: !FindInMap [ EnvSettings, !Ref "ENV", Stage ] }
      AssumeRolePolicyDocument:
        Version: "2012-10-17"
        Statement:
          - Effect: Allow
            Principal:
              Service:
                - ec2.amazonaws.com
            Action:
              - sts:AssumeRole
      Policies:
      - PolicyName: 
          Fn::Sub:
            - "this-also-is-way-too-long-for-CloudFormation-purposes-yum-access-policy-packer-${AWS::Region}-${Stage}"
            - { Stage: !FindInMap [ EnvSettings, !Ref "ENV", Stage ] }
          PolicyDocument:
            Version: "2012-10-17"
            Statement:
              - Action:
                  - s3:ListAllMyBuckets
                Effect: Allow
                Resource:
                  - arn:aws:s3:::*
              - Action:
                  - s3:ListBucket
                  - s3:GetBucketLocation
                Effect: Allow
                Resource:
                  - Fn::Sub:
                      - "arn:aws:s3:::${Bucket}"
                      - { Bucket: "this-name-is-far-too-long-and-this-is-not-my-own-opinion-yum-repository-bucket-name" }
              - Action:
                  - s3:GetObject
                Effect: Allow
                Resource:
                  - Fn::Sub:
                      - "arn:aws:s3:::${Bucket}/*"
                      - { Bucket: "name-of-this-bucket-is-far-too-long-and-you-know-it-yum-repository-bucket-name" }

  ReadAccessToYumRepository:
    Type: AWS::IAM::InstanceProfile
    DependsOn:
      - YumRepositoryAccessRole
    Properties:
      Roles:
        - !Ref "YumRepositoryAccessRole"

Outputs:

  PackerUserAccessKeySecret:
    Description: >
      Secret access key for Packer user.
    Value: !GetAtt "PackerUserAccessKey.SecretAccessKey"

  PackerUserAccessKey:
    Description: >
      Access key for Packer user.
    Value: !Ref "PackerUserAccessKey"

  PackerInstanceProfileARN:
    Description: >
      Instance profile ARN used for Packer EC2 instances.
    Value: !GetAtt "ReadAccessToYumRepository.Arn"
    Export:
      Name: this-name-is-invalid-due-of-its-long-nature-packer-instance-profile-arn

Throws the following errors:

 INFO: Configuration file from the following location will be used: defaults/main.yaml
WARNING! PackerUserAccessKey: UserName <--- is nil.
WARNING! PackerUserPolicy: Users[0] <--- is nil.
WARNING! ReadAccessToYumRepository: Roles[0] <--- is nil.
WARNING! Encountered strings longer than 64 characters. There is some probability that this could make this template rejected by the CloudFormation.
ReadAccessToYumRepository  has no validation errors
YumRepositoryAccessRole
         Policies[0]: PolicyName value longer than 64 characters.
         RoleName value longer than 64 characters.
         Property PolicyDocument is required in Policy
PackerUser
         UserName value longer than 64 characters.
PackerUserAccessKey  has no validation errors
PackerUserPolicy
         PolicyName value longer than 64 characters.
ERROR: Template is invalid!

As we talked with @pfigwer, the best place for this particular validator is not in the validatorsMap as we would like to check every string in every Resource. This could lead to the future concept of having various validators which would check the values in the Resources for various aspects: do they include spaces, do they include only ASCII characters, do they match some naming convention... or various other analysis which would be handy.

[jlampar]

Meanwhile I've updated this concept to the one above-mentioned as the future concept. The function validators.IsValueTooLong is now exchanged with the one named validators.StringValidator which checks every string value inside the Resources and on that value it executes a small, checking function (e.g. validators.IsLonger64 which checks if the input is longer than 64 characters). The validators.StringValidator can take any simple, checking-one-string function as an argument and execute it on every string value inside the Resources. Then it returns the true or false and that result is taken by the result-printing function which provides the longer explanation of the encountered issue. This reduces the mess inside the validateResources function and the thrown errors have exactly the same form.
It also opens the possibility of checking the strings for whatever we desire, write those simple argument functions and plug them in to the validators.StringValidator.