diff --git a/README.md b/README.md index 123c3fe210..4e35bd0e35 100644 --- a/README.md +++ b/README.md @@ -93,6 +93,18 @@ Lint all `yaml` files in `path` and all subdirectories (recursive): *Note*: Glob in Python 3.5 supports recursive searching `**/*.yaml`. If you are using an earlier version of Python you will have to handle this manually (`folder1/*.yaml`, `folder2/*.yaml`, etc). +##### Exit Codes +`cfn-lint` will return a non zero exit if there are any issues with your template. The value is dependent on the sevirity of the issues found. For each level of discovered error `cfn-lint` will use bitwise OR to determine the final exit code. This will result in these possibilities. + +- 0 is no issue was found +- 2 is an error +- 4 is a warning +- 6 is an error and a warning +- 8 is an informational +- 10 is an error and informational +- 12 is an warning and informational +- 14 is an error and a warning and an informational + ##### Specifying the template as an input stream The template to be linted can also be passed using standard input: @@ -138,7 +150,7 @@ Optional parameters: | ------------- | ------------- | ------------- | ------------- | | -h, --help | | | Get description of cfn-lint | | -t, --template | | filename | Alternative way to specify Template file path to the file that needs to be tested by cfn-lint | -| -f, --format | format | quiet, parseable, json, junit | Output format | +| -f, --format | format | quiet, parseable, json, junit, pretty | Output format | | -l, --list-rules | | | List all the rules | | -r, --regions | regions | [REGIONS [REGIONS ...]], ALL_REGIONS | Test the template against many regions. [Supported regions](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/cfn-resource-specification.html) | | -b, --ignore-bad-template | ignore_bad_template | | Ignores bad template errors | diff --git a/scripts/update_specs_from_pricing.py b/scripts/update_specs_from_pricing.py index f69bd2e7cf..17c1182f8b 100755 --- a/scripts/update_specs_from_pricing.py +++ b/scripts/update_specs_from_pricing.py @@ -41,16 +41,9 @@ 'South America (Sao Paulo)': 'sa-east-1', 'US East (N. Virginia)': 'us-east-1', 'US East (Ohio)': 'us-east-2', - 'US East (Verizon) - Atlanta': 'us-east-1', - 'US East (Verizon) - Boston': 'us-east-1', - 'US East (Verizon) - New York': 'us-east-1', - 'US East (Verizon) - Washington DC': 'us-east-1', 'US West (N. California)': 'us-west-1', 'US West (Oregon)': 'us-west-2', 'US West (Los Angeles)': 'us-west-2', - 'US West (Verizon) - San Francisco Bay Area': 'us-west-2', - 'US East (Verizon) - Miami': 'us-east-1', - 'US East (Verizon) - Dallas': 'us-east-1', } session = boto3.session.Session() @@ -206,7 +199,7 @@ def get_results(service, product_families): products = json.loads(price_item) product = products.get('product', {}) if product: - if product.get('productFamily') in product_families: + if product.get('productFamily') in product_families and 'Verizon' not in product.get('attributes').get('location'): if not results.get(region_map[product.get('attributes').get('location')]): results[region_map[product.get('attributes').get('location')]] = set() results[region_map[product.get('attributes').get('location')]].add( diff --git a/src/cfnlint/__main__.py b/src/cfnlint/__main__.py index bcb6cd0cba..0af2fb38d7 100644 --- a/src/cfnlint/__main__.py +++ b/src/cfnlint/__main__.py @@ -38,7 +38,7 @@ def main(): matches.extend(errors) LOGGER.debug('Completed linting of file: %s', str(filename)) - matches_output = formatter.print_matches(matches, rules) + matches_output = formatter.print_matches(matches, rules, filenames) if matches_output: if args.output_file: diff --git a/src/cfnlint/config.py b/src/cfnlint/config.py index 95daf8cff0..8ec5602eac 100644 --- a/src/cfnlint/config.py +++ b/src/cfnlint/config.py @@ -354,7 +354,7 @@ def __call__(self, parser, namespace, values, option_string=None): '-I', '--info', help='Enable information logging', action='store_true' ) standard.add_argument( - '-f', '--format', help='Output Format', choices=['quiet', 'parseable', 'json', 'junit'] + '-f', '--format', help='Output Format', choices=['quiet', 'parseable', 'json', 'junit', 'pretty'] ) standard.add_argument( diff --git a/src/cfnlint/core.py b/src/cfnlint/core.py index 3120dda584..02d752b428 100644 --- a/src/cfnlint/core.py +++ b/src/cfnlint/core.py @@ -56,11 +56,11 @@ def get_exit_code(matches): """ Determine exit code """ exit_code = 0 for match in matches: - if match.rule.id[0] == 'I': + if match.rule.severity == 'informational': exit_code = exit_code | 8 - elif match.rule.id[0] == 'W': + elif match.rule.severity == 'warning': exit_code = exit_code | 4 - elif match.rule.id[0] == 'E': + elif match.rule.severity == 'error': exit_code = exit_code | 2 return exit_code @@ -79,6 +79,8 @@ def get_formatter(fmt): formatter = cfnlint.formatters.JsonFormatter() elif fmt == 'junit': formatter = cfnlint.formatters.JUnitFormatter() + elif fmt == 'pretty': + formatter = cfnlint.formatters.PrettyFormatter() else: formatter = cfnlint.formatters.Formatter() diff --git a/src/cfnlint/data/AdditionalSpecs/SubNeededExcludes.json b/src/cfnlint/data/AdditionalSpecs/SubNeededExcludes.json new file mode 100644 index 0000000000..97f5a8fda9 --- /dev/null +++ b/src/cfnlint/data/AdditionalSpecs/SubNeededExcludes.json @@ -0,0 +1,173 @@ +{ + "ResourceTypes": { + "AWS::ApiGateway::Method": { + "ExcludeRegex": "stageVariables\\..*" + }, + "AWS::AppSync::FunctionConfiguration": { + "Properties": { + "RequestMappingTemplate": { + "ExcludeAll": true + }, + "ResponseMappingTemplate": { + "ExcludeAll": true + } + } + }, + "AWS::AppSync::Resolver": { + "Properties": { + "RequestMappingTemplate": { + "ExcludeAll": true + }, + "ResponseMappingTemplate": { + "ExcludeAll": true + } + } + }, + "AWS::AutoScaling::LaunchConfiguration": { + "Properties": { + "UserData": { + "ExcludeAll": true + } + } + }, + "AWS::CloudFormation::StackSet": { + "Properties": { + "TemplateBody": { + "ExcludeAll": true + } + } + }, + "AWS::CodeBuild::Project": { + "Properties": { + "BuildSpec": { + "ExcludeAll": true + } + } + }, + "AWS::EC2::FlowLog": { + "Properties": { + "LogFormat": { + "ExcludeAll": true + } + } + }, + "AWS::EC2::Instance": { + "Metadata": { + "AWS::CloudFormation::Init": { + "ExcludeAll": true + } + }, + "Properties": { + "UserData": { + "ExcludeAll": true + } + } + }, + "AWS::EMR::InstanceGroupConfig": { + "Properties": { + "AutoScalingPolicy": { + "Properties": { + "ScalingRule": { + "Properties": { + "ScalingTrigger": { + "Properties": { + "CloudWatchAlarmDefinition": { + "ExcludeAll": true + } + } + } + } + } + } + } + } + }, + "AWS::IAM::Policy": { + "Properties": { + "PolicyDocument": { + "ExcludeValues": [ + "aws:CurrentTime", + "aws:EpochTime", + "aws:TokenIssueTime", + "aws:principaltype", + "aws:SecureTransport", + "aws:SourceIp", + "aws:UserAgent", + "aws:userid", + "aws:username", + "ec2:SourceInstanceARN", + "iot:Connection.Thing.ThingName", + "iot:Connection.Thing.ThingTypeName", + "iot:Connection.Thing.IsAttached", + "iot:ClientId", + "transfer:HomeBucket", + "transfer:HomeDirectory", + "transfer:HomeFolder", + "transfer:UserName", + "redshift:DbUser", + "cognito-identity.amazonaws.com:aud", + "cognito-identity.amazonaws.com:sub", + "cognito-identity.amazonaws.com:amr" + ] + } + } + }, + "AWS::IoT::Policy": { + "Properties": { + "PolicyDocument": { + "ExcludeValues": [ + "aws:CurrentTime", + "aws:EpochTime", + "aws:TokenIssueTime", + "aws:principaltype", + "aws:SecureTransport", + "aws:SourceIp", + "aws:UserAgent", + "aws:userid", + "aws:username", + "ec2:SourceInstanceARN", + "iot:Connection.Thing.ThingName", + "iot:Connection.Thing.ThingTypeName", + "iot:Connection.Thing.IsAttached", + "iot:ClientId", + "transfer:HomeBucket", + "transfer:HomeDirectory", + "transfer:HomeFolder", + "transfer:UserName", + "redshift:DbUser", + "cognito-identity.amazonaws.com:aud", + "cognito-identity.amazonaws.com:sub", + "cognito-identity.amazonaws.com:amr" + ] + } + } + }, + "AWS::IoT::TopicRule": { + "Properties": { + "TopicRulePayload": { + "ExcludeAll": true + } + } + }, + "AWS::Lambda::Function": { + "Properties": { + "Code": { + "Properties": { + "ZipFile": { + "ExcludeAll": true + } + } + } + } + }, + "AWS::StepFunctions::StateMachine": { + "Properties": { + "DefinitionString": { + "ExcludeBasedOnProperties": [ + "AWS::StepFunctions::StateMachine.DefinitionSubstitutions" + ] + } + } + } + } +} \ No newline at end of file diff --git a/src/cfnlint/data/CloudSpecs/us-east-1.json b/src/cfnlint/data/CloudSpecs/us-east-1.json index f7aeb05188..620a418ad3 100644 --- a/src/cfnlint/data/CloudSpecs/us-east-1.json +++ b/src/cfnlint/data/CloudSpecs/us-east-1.json @@ -14316,19 +14316,6 @@ } } }, - "AWS::EC2::CarrierGateway.Tags": { - "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-carriergateway-tags.html", - "Properties": { - "Tags": { - "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-carriergateway-tags.html#cfn-ec2-carriergateway-tags-tags", - "DuplicatesAllowed": false, - "ItemType": "Tag", - "Required": false, - "Type": "List", - "UpdateType": "Mutable" - } - } - }, "AWS::EC2::ClientVpnEndpoint.CertificateAuthenticationRequest": { "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-clientvpnendpoint-certificateauthenticationrequest.html", "Properties": { @@ -54621,7 +54608,7 @@ "Tags": { "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ec2-carriergateway.html#cfn-ec2-carriergateway-tags", "Required": false, - "Type": "Tags", + "Type": "Tag", "UpdateType": "Mutable" }, "VpcId": { diff --git a/src/cfnlint/data/CloudSpecs/us-west-2.json b/src/cfnlint/data/CloudSpecs/us-west-2.json index a732b19bd1..d7cc074bf7 100644 --- a/src/cfnlint/data/CloudSpecs/us-west-2.json +++ b/src/cfnlint/data/CloudSpecs/us-west-2.json @@ -14147,19 +14147,6 @@ } } }, - "AWS::EC2::CarrierGateway.Tags": { - "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-carriergateway-tags.html", - "Properties": { - "Tags": { - "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-carriergateway-tags.html#cfn-ec2-carriergateway-tags-tags", - "DuplicatesAllowed": false, - "ItemType": "Tag", - "Required": false, - "Type": "List", - "UpdateType": "Mutable" - } - } - }, "AWS::EC2::ClientVpnEndpoint.CertificateAuthenticationRequest": { "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-clientvpnendpoint-certificateauthenticationrequest.html", "Properties": { @@ -54382,7 +54369,7 @@ "Tags": { "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ec2-carriergateway.html#cfn-ec2-carriergateway-tags", "Required": false, - "Type": "Tags", + "Type": "Tag", "UpdateType": "Mutable" }, "VpcId": { diff --git a/src/cfnlint/data/ExtendedSpecs/all/01_spec_patch.json b/src/cfnlint/data/ExtendedSpecs/all/01_spec_patch.json index dc98a1b9ca..b93bffe8cc 100644 --- a/src/cfnlint/data/ExtendedSpecs/all/01_spec_patch.json +++ b/src/cfnlint/data/ExtendedSpecs/all/01_spec_patch.json @@ -417,5 +417,14 @@ "op": "replace", "path": "/ResourceTypes/AWS::IoT::DomainConfiguration/Properties/Tags/Type", "value": "Tag" + }, + { + "op": "remove", + "path": "/PropertyTypes/AWS::EC2::CarrierGateway.Tags" + }, + { + "op": "replace", + "path": "/ResourceTypes/AWS::EC2::CarrierGateway/Properties/Tags/Type", + "value": "Tag" } ] \ No newline at end of file diff --git a/src/cfnlint/data/ExtendedSpecs/all/03_value_types.json b/src/cfnlint/data/ExtendedSpecs/all/03_value_types.json index 55a73ca7a1..1c4f43ede6 100644 --- a/src/cfnlint/data/ExtendedSpecs/all/03_value_types.json +++ b/src/cfnlint/data/ExtendedSpecs/all/03_value_types.json @@ -52,6 +52,46 @@ ] } }, + "AvailabilityZoneWithAll": { + "AllowedValues": [ + "all", "af-south-1a", "af-south-1b", "af-south-1c", + "ap-east-1a", "ap-east-1b", "ap-east-1c", + "ap-northeast-1a", "ap-northeast-1b", "ap-northeast-1c", "ap-northeast-1d", + "ap-northeast-2a", "ap-northeast-2b", "ap-northeast-2c", "ap-northeast-2d", + "ap-northeast-3a", + "ap-south-1a", "ap-south-1b", "ap-south-1c", + "ap-southeast-1a", "ap-southeast-1b", "ap-southeast-1c", + "ap-southeast-2a", "ap-southeast-2b", "ap-southeast-2c", + "ca-central-1a", "ca-central-1b", "ca-central-1d", + "cn-north-1a", "cn-north-1b", + "cn-northwest-1a", "cn-northwest-1b", "cn-northwest-1c", + "eu-central-1a", "eu-central-1b", "eu-central-1c", + "eu-north-1a", "eu-north-1b", "eu-north-1c", + "eu-south-1a", "eu-south-1b", "eu-south-1c", + "eu-west-1a", "eu-west-1b", "eu-west-1c", + "eu-west-2a", "eu-west-2b", "eu-west-2c", + "eu-west-3a", "eu-west-3b", "eu-west-3c", + "me-south-1a", "me-south-1b", "me-south-1c", + "sa-east-1a", "sa-east-1b", "sa-east-1c", + "us-east-1a", "us-east-1b", "us-east-1c", "us-east-1d", "us-east-1e", "us-east-1f", + "us-east-2a", "us-east-2b", "us-east-2c", + "us-gov-east-1a", "us-gov-east-1b", "us-gov-east-1c", + "us-gov-west-1a", "us-gov-west-1b", "us-gov-west-1c", + "us-west-1a", "us-west-1b", "us-west-1c", + "us-west-2a", "us-west-2b", "us-west-2c", "us-west-2d", "us-west-2-lax-1a", "us-west-2-lax-1b" + ], + "GetAtt": { + "AWS::EC2::Instance": "AvailabilityZone", + "AWS::EC2::Subnet": "AvailabilityZone", + "AWS::OpsWorks::Instance": "AvailabilityZone" + }, + "Ref": { + "Parameters": [ + "String", + "AvailabilityZone" + ] + } + }, "CertificateValidationMethod": { "botocore": "acm/2015-12-08/ValidationMethod" }, diff --git a/src/cfnlint/data/ExtendedSpecs/all/04_property_values.json b/src/cfnlint/data/ExtendedSpecs/all/04_property_values.json index 49439b9520..9e41b9fbe1 100644 --- a/src/cfnlint/data/ExtendedSpecs/all/04_property_values.json +++ b/src/cfnlint/data/ExtendedSpecs/all/04_property_values.json @@ -906,7 +906,7 @@ "op": "add", "path": "/PropertyTypes/AWS::ElasticLoadBalancingV2::TargetGroup.TargetDescription/Properties/AvailabilityZone/Value", "value": { - "ValueType": "AvailabilityZone" + "ValueType": "AvailabilityZoneWithAll" } }, { diff --git a/src/cfnlint/formatters/__init__.py b/src/cfnlint/formatters/__init__.py index dbc9f00141..c15f69c821 100644 --- a/src/cfnlint/formatters/__init__.py +++ b/src/cfnlint/formatters/__init__.py @@ -2,21 +2,44 @@ Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. SPDX-License-Identifier: MIT-0 """ +import itertools import json +import operator +import sys from junit_xml import TestSuite, TestCase, to_xml_report_string from cfnlint.rules import Match +class color(object): + error = '\033[31m' + warning = '\033[33m' + informational = '\033[34m' + unknown = '\033[37m' + green = '\033[32m' + reset = '\033[0m' + bold_reset = '\033[1:0m' + underline_reset = '\033[4m' + + +def colored(s, c): + """ Takes in string s and outputs it with color """ + if sys.stdout.isatty(): + return '{}{}{}'.format(c, s, color.reset) + + return s + + class BaseFormatter(object): """Base Formatter class""" def _format(self, match): """Format the specific match""" - def print_matches(self, matches, rules=None): + def print_matches(self, matches, rules=None, filenames=None): """Output all the matches""" # Unused argument http://pylint-messages.wikidot.com/messages:w0613 del rules + del filenames if not matches: return None @@ -57,7 +80,7 @@ def _failure_format(self, match): match.columnnumber ) - def print_matches(self, matches, rules=None): + def print_matches(self, matches, rules=None, filenames=None): """Output all the matches""" if not rules: @@ -103,13 +126,6 @@ class CustomEncoder(json.JSONEncoder): def default(self, o): if isinstance(o, Match): - if o.rule.id[0] == 'W': - level = 'Warning' - elif o.rule.id[0] == 'I': - level = 'Informational' - else: - level = 'Error' - return { 'Rule': { 'Id': o.rule.id, @@ -128,13 +144,13 @@ def default(self, o): }, 'Path': getattr(o, 'path', None), }, - 'Level': level, + 'Level': o.rule.severity.capitalize(), 'Message': o.message, 'Filename': o.filename, } return {'__{}__'.format(o.__class__.__name__): o.__dict__} - def print_matches(self, matches, rules=None): + def print_matches(self, matches, rules=None, filenames=None): # JSON formatter outputs a single JSON object # Unused argument http://pylint-messages.wikidot.com/messages:w0613 del rules @@ -172,3 +188,60 @@ def _format(self, match): match.rule.id, match.message ) + + +class PrettyFormatter(BaseFormatter): + """Generic Formatter""" + + def _format(self, match): + """Format output""" + formatstr = '{0}{1}{2}' + pos = '{0}:{1}:'.format(match.linenumber, match.columnnumber) + return formatstr.format( + colored('{:20}'.format(pos), color.reset), + colored('{:10}'.format(match.rule.id), getattr(color, match.rule.severity.lower())), + match.message, + ) + + def print_matches(self, matches, rules=None, filenames=None): + results = self._format_matches(matches) + + results.append('Cfn-lint scanned {} templates against {} rules and found {} errors, {} warnings, and {} informational violations'.format( + colored(len(filenames), color.bold_reset), + colored(len(rules), color.bold_reset), + colored(len([i for i in matches if i.rule.severity.lower() == 'error']), color.error), + colored(len([i for i in matches if i.rule.severity.lower() == 'warning']), color.warning), + colored(len([i for i in matches if i.rule.severity.lower() + == 'informational']), color.informational), + )) + return '\n'.join(results) + + def _format_matches(self, matches): + """Output all the matches""" + output = [] + + # This better be sorted + for filename, file_matches in itertools.groupby( + matches, + key=operator.attrgetter('filename') + ): + levels = { + 'error': [], + 'warning': [], + 'informational': [], + 'unknown': [] + } + + output.append(colored(filename, color.underline_reset)) + for match in file_matches: + level = match.rule.severity.lower() + if level not in ['error', 'warning', 'informational']: + level = 'unknown' + levels[level].append(match) + for _, all_matches in levels.items(): + for match in all_matches: + output.extend([self._format(match)]) + + output.append('') # Newline after each group + + return output diff --git a/src/cfnlint/rules/__init__.py b/src/cfnlint/rules/__init__.py index 89769c63eb..550da549a9 100644 --- a/src/cfnlint/rules/__init__.py +++ b/src/cfnlint/rules/__init__.py @@ -16,7 +16,6 @@ class CloudFormationLintRule(object): """CloudFormation linter rules""" - id = '' shortdesc = '' description = '' @@ -35,6 +34,16 @@ def __init__(self): def __repr__(self): return '%s: %s' % (self.id, self.shortdesc) + @property + def severity(self): + """Severity level""" + levels = { + 'I': 'informational', + 'E': 'error', + 'W': 'warning', + } + return levels.get(self.id[0].upper(), 'unknown') + def verbose(self): """Verbose output""" return '%s: %s\n%s' % (self.id, self.shortdesc, self.description) diff --git a/src/cfnlint/rules/functions/SubNeeded.py b/src/cfnlint/rules/functions/SubNeeded.py index 132a5f135c..1909789460 100644 --- a/src/cfnlint/rules/functions/SubNeeded.py +++ b/src/cfnlint/rules/functions/SubNeeded.py @@ -2,57 +2,36 @@ Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. SPDX-License-Identifier: MIT-0 """ -from functools import reduce # pylint: disable=redefined-builtin import re import six +import cfnlint.helpers from cfnlint.rules import CloudFormationLintRule from cfnlint.rules import RuleMatch +from cfnlint.data import AdditionalSpecs class SubNeeded(CloudFormationLintRule): - """Check if a substitution string exists without a substitution function""" + """ + Check if a substitution string exists without a substitution function + + Found a false-positive or false-negative? All configuration for this rule + is contained in `src/cfnlint/data/AdditionalSpecs/SubNeededExcludes.json` + so you can add new entries to fix false-positives or amend existing + entries for false-negatives. + """ id = 'E1029' shortdesc = 'Sub is required if a variable is used in a string' description = 'If a substitution variable exists in a string but isn\'t wrapped with the Fn::Sub function the deployment will fail.' source_url = 'https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-sub.html' tags = ['functions', 'sub'] - # Free-form text properties to exclude from this rule - excludes = ['UserData', 'ZipFile', 'Condition', 'AWS::CloudFormation::Init', - 'CloudWatchAlarmDefinition', 'TopicRulePayload', 'BuildSpec', - 'RequestMappingTemplate', 'LogFormat', 'TemplateBody', 'ResponseMappingTemplate'] - api_excludes = ['Uri', 'Body', 'ConnectionId'] - - - # IAM Policy has special variables that don't require !Sub, Check for these - # https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_variables.html - # https://docs.aws.amazon.com/iot/latest/developerguide/basic-policy-variables.html - # https://docs.aws.amazon.com/iot/latest/developerguide/thing-policy-variables.html - # https://docs.aws.amazon.com/transfer/latest/userguide/users.html#users-policies-scope-down - # https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_iam-condition-keys.html - resource_excludes = ['${aws:CurrentTime}', '${aws:EpochTime}', - '${aws:TokenIssueTime}', '${aws:principaltype}', - '${aws:SecureTransport}', '${aws:SourceIp}', - '${aws:UserAgent}', '${aws:userid}', - '${aws:username}', '${ec2:SourceInstanceARN}', - '${iot:Connection.Thing.ThingName}', - '${iot:Connection.Thing.ThingTypeName}', - '${iot:Connection.Thing.IsAttached}', - '${iot:ClientId}', '${transfer:HomeBucket}', - '${transfer:HomeDirectory}', '${transfer:HomeFolder}', - '${transfer:UserName}', '${redshift:DbUser}', - '${cognito-identity.amazonaws.com:aud}', - '${cognito-identity.amazonaws.com:sub}', - '${cognito-identity.amazonaws.com:amr}'] - - # https://docs.aws.amazon.com/redshift/latest/mgmt/redshift-iam-access-control-identity-based.html - condition_excludes = [ - '${redshift:DbUser}', - ] - def __init__(self): """Init""" super(SubNeeded, self).__init__() + excludes_config = cfnlint.helpers.load_resource(AdditionalSpecs, 'SubNeededExcludes.json') + + self.excludes = excludes_config + self.config_definition = { 'custom_excludes': { 'default': '', @@ -62,19 +41,20 @@ def __init__(self): self.configure() self.subParameterRegex = re.compile(r'(\$\{[A-Za-z0-9_:\.]+\})') - def _match_values(self, cfnelem, path): + + def _match_values_recursive(self, cfnelem, path): """Recursively search for values matching the searchRegex""" values = [] if isinstance(cfnelem, dict): for key in cfnelem: pathprop = path[:] pathprop.append(key) - values.extend(self._match_values(cfnelem[key], pathprop)) + values.extend(self._match_values_recursive(cfnelem[key], pathprop)) elif isinstance(cfnelem, list): for index, item in enumerate(cfnelem): pathprop = path[:] pathprop.append(index) - values.extend(self._match_values(item, pathprop)) + values.extend(self._match_values_recursive(item, pathprop)) else: # Leaf node if isinstance(cfnelem, six.string_types): # and re.match(searchRegex, cfnelem): @@ -83,20 +63,17 @@ def _match_values(self, cfnelem, path): return values - def match_values(self, cfn): + + def _match_values(self, cfn): """ Search for values in all parts of the templates that match the searchRegex """ results = [] - results.extend(self._match_values(cfn.template, [])) + results.extend(self._match_values_recursive(cfn.template, [])) # Globals are removed during a transform. They need to be checked manually - results.extend(self._match_values(cfn.template.get('Globals', {}), [])) + results.extend(self._match_values_recursive(cfn.template.get('Globals', {}), [])) return results - def _api_exceptions(self, value): - """ Key value exceptions """ - parameter_search = re.compile(r'^\$\{stageVariables\..*\}$') - return re.match(parameter_search, value) def _variable_custom_excluded(self, value): """ User-defined exceptions for variables, anywhere in the file """ @@ -106,56 +83,125 @@ def _variable_custom_excluded(self, value): return re.match(custom_search, value) return False + + def _excluded_by_additional_resource_specs(self, variable, path, cfn): + """ + Should the variable be excluded due to the resource + configuration in the AdditionalSpecs file + 'SubNeededExcludes.json'? + """ + resource_name = path[0] + resource_type = cfn.template.get('Resources').get(resource_name).get('Type') + + if resource_type in self.excludes['ResourceTypes']: + return self._excluded_by_additional_resource_specs_recursive(variable, path, path[2:], self.excludes['ResourceTypes'][resource_type], cfn) + + return False + + + def _excluded_by_criteria(self, variable, full_path, exclusions, cfn): + """ + Run the exclusion logic, and because pylint too-many-return-statements complains + """ + # Have we excluded all variables for this resource? + if 'ExcludeAll' in exclusions and exclusions['ExcludeAll']: + return True + + # Have we excluded the specific variable + if 'ExcludeValues' in exclusions: + if variable in exclusions['ExcludeValues']: + return True + + if 'ExcludeRegex' in exclusions: + exclude_regex = re.compile(r'^{}$'.format(exclusions['ExcludeRegex'])) + if re.match(exclude_regex, variable): + return True + + if 'ExcludeBasedOnProperties' in exclusions: + for exclude_based_on_property in exclusions['ExcludeBasedOnProperties']: + exclude_based_on_property_path = exclude_based_on_property.split('.')[1:] + + current_property = cfn.template.get('Resources').get(full_path[0]).get('Properties') + + while exclude_based_on_property_path and current_property: + current_property = current_property.get(exclude_based_on_property_path[0]) + exclude_based_on_property_path = exclude_based_on_property_path[1:] + + if current_property and variable in current_property: + return True + + return False + + + def _excluded_by_additional_resource_specs_recursive(self, variable, full_path, path, exclusions, cfn): + """ + Should the variable be excluded due to the resource + configuration in the AdditionalSpecs file + 'SubNeededExcludes.json'? + """ + # Make sure the path is well-defined + if not path: + return False + + # Should we exclude the variable based on the current criteria defined in the specs? + if self._excluded_by_criteria(variable, full_path, exclusions, cfn): + return True + + # Recursively check the exclusion critera based on the property tree + sub_fields = ['Metadata', 'Properties'] + + for sub_field in sub_fields: + if sub_field in exclusions: + + # Traverse over any arrays, path should never end with an integer + while isinstance(path[0], int): + path = path[1:] + + # Recurse + if path[0] in exclusions[sub_field]: + if self._excluded_by_additional_resource_specs_recursive(variable, full_path, path[1:], exclusions[sub_field][path[0]], cfn): + return True + + return False + + + def _excluded_by_additional_specs(self, variable, path, cfn): + """ + Should the variable be excluded due to the configuration + in the AdditionalSpecs file 'SubNeededExcludes.json'? + """ + if path[0] == 'Resources': + # Check exclusion at the resource level first + return self._excluded_by_additional_resource_specs(variable[2:-1], path[1:], cfn) + + return False + + def match(self, cfn): matches = [] # Get a list of paths to every leaf node string containing at least one ${parameter} - parameter_string_paths = self.match_values(cfn) + parameter_string_paths = self._match_values(cfn) # We want to search all of the paths to check if each one contains an 'Fn::Sub' for parameter_string_path in parameter_string_paths: if parameter_string_path[0] in ['Parameters']: continue - # Exclude the special IAM variables - variable = parameter_string_path[-1] - if 'Resource' in parameter_string_path: - if variable in self.resource_excludes: - continue - if 'NotResource' in parameter_string_path: - if variable in self.resource_excludes: - continue - if 'Condition' in parameter_string_path: - if variable in self.condition_excludes: - continue - - # Step Function State Machine has a Definition Substitution that allows usage of special variables outside of a !Sub - # https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-stepfunctions-statemachine-definitionsubstitutions.html - - if 'DefinitionString' in parameter_string_path: - modified_parameter_string_path = parameter_string_path - index = parameter_string_path.index('DefinitionString') - modified_parameter_string_path[index] = 'DefinitionSubstitutions' - modified_parameter_string_path = modified_parameter_string_path[:index+1] - modified_parameter_string_path.append(variable[2:-1]) - if reduce(lambda c, k: c.get(k, {}), modified_parameter_string_path, cfn.template): - continue + # The variable to be substituted + variable = parameter_string_path[-1] # Exclude variables that match custom exclude filters, if configured # (for third-party tools that pre-process templates before uploading them to AWS) if self._variable_custom_excluded(variable): continue - # Exclude literals (https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-sub.html) - if variable.startswith('${!'): + if self._excluded_by_additional_specs(variable, parameter_string_path, cfn): continue found_sub = False # Does the path contain an 'Fn::Sub'? for step in parameter_string_path: - if step in self.api_excludes: - if self._api_exceptions(parameter_string_path[-1]): - found_sub = True - elif step == 'Fn::Sub' or step in self.excludes: + if step == 'Fn::Sub': found_sub = True # If we didn't find an 'Fn::Sub' it means a string containing a ${parameter} may not be evaluated correctly diff --git a/src/cfnlint/rules/resources/ServerlessTransform.py b/src/cfnlint/rules/resources/ServerlessTransform.py index 462e0231be..61cf76161a 100644 --- a/src/cfnlint/rules/resources/ServerlessTransform.py +++ b/src/cfnlint/rules/resources/ServerlessTransform.py @@ -2,6 +2,7 @@ Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. SPDX-License-Identifier: MIT-0 """ +import six from cfnlint.rules import CloudFormationLintRule from cfnlint.rules import RuleMatch @@ -31,10 +32,11 @@ def match(self, cfn): for resource_name, resource_values in cfn.get_resources().items(): resource_type = resource_values['Type'] - if resource_type.startswith('AWS::Serverless::'): - message = 'Serverless Transform required for Type {0} for resource {1}' - matches.append( - RuleMatch(['Transform'], message.format(resource_type, resource_name)) - ) - break + if isinstance(resource_type, six.string_types): + if resource_type.startswith('AWS::Serverless::'): + message = 'Serverless Transform required for Type {0} for resource {1}' + matches.append( + RuleMatch(['Transform'], message.format(resource_type, resource_name)) + ) + break return matches diff --git a/src/cfnlint/template.py b/src/cfnlint/template.py index 5166bc8eb4..dfdbe89149 100644 --- a/src/cfnlint/template.py +++ b/src/cfnlint/template.py @@ -166,20 +166,21 @@ def get_valid_getatts(self): for name, value in resources.items(): if 'Type' in value: valtype = value['Type'] - if valtype.startswith(astrik_string_types): - LOGGER.debug('Cant build an appropriate getatt list from %s', valtype) - results[name] = {'*': {'PrimitiveItemType': 'String'}} - elif valtype.startswith(astrik_unknown_types): - LOGGER.debug('Cant build an appropriate getatt list from %s', valtype) - results[name] = {'*': {}} - else: - if value['Type'] in resourcetypes: - if 'Attributes' in resourcetypes[valtype]: - results[name] = {} - for attname, attvalue in resourcetypes[valtype]['Attributes'].items(): - element = {} - element.update(attvalue) - results[name][attname] = element + if isinstance(valtype, six.string_types): + if valtype.startswith(astrik_string_types): + LOGGER.debug('Cant build an appropriate getatt list from %s', valtype) + results[name] = {'*': {'PrimitiveItemType': 'String'}} + elif valtype.startswith(astrik_unknown_types): + LOGGER.debug('Cant build an appropriate getatt list from %s', valtype) + results[name] = {'*': {}} + else: + if value['Type'] in resourcetypes: + if 'Attributes' in resourcetypes[valtype]: + results[name] = {} + for attname, attvalue in resourcetypes[valtype]['Attributes'].items(): + element = {} + element.update(attvalue) + results[name][attname] = element return results diff --git a/test/fixtures/schemas/SubNeededExcludes.json b/test/fixtures/schemas/SubNeededExcludes.json new file mode 100644 index 0000000000..831aca2524 --- /dev/null +++ b/test/fixtures/schemas/SubNeededExcludes.json @@ -0,0 +1,66 @@ +{ + "$schema": "http://json-schema.org/draft-04/schema#", + "title": "Product", + "description": "Exclusion critera for the Fn::Sub Needed rule (E1029)", + "type": "object", + "additionalProperties": false, + "properties": { + "ResourceTypes": { + "$ref": "#/definitions/resourcetypes" + } + }, + "definitions": { + "resourcetypes": { + "patternProperties": { + "^[a-zA-Z0-9]+::[a-zA-Z0-9]+::[a-zA-Z0-9]+$": { + "$ref": "#/definitions/resource" + } + }, + "additionalProperties": false, + "uniqueItems": true + }, + "resource": { + "type": "object", + "properties": { + "ExcludeRegex": { + "description": "A regex to match variables to exclude from the rule", + "type": "string" + }, + "ExcludeAll": { + "description": "Exclude all variables from the rule", + "type": "boolean" + }, + "ExcludeValues": { + "description": "Exclude any variable matching these values from the rule", + "type": "array", + "items": { + "type": "string" + }, + "uniqueItems": true + }, + "ExcludeBasedOnProperties": { + "description": "Exclude any variables defined in these properties from the rule", + "type": "array", + "items": { + "type": "string" + }, + "uniqueItems": true + }, + "Metadata": { + "description": "Exclusion critera for metadata associated with a resource", + "$ref": "#/definitions/resource" + }, + "Properties": { + "description": "Exclusion critera for properties associated with a resource", + "$ref": "#/definitions/resource" + } + }, + "additionalProperties": { + "$ref": "#/definitions/resource" + } + } + }, + "required": [ + "ResourceTypes" + ] +} \ No newline at end of file diff --git a/test/fixtures/templates/bad/functions/sub_needed.yaml b/test/fixtures/templates/bad/functions/sub_needed.yaml index bed1b7a1cf..2297fb547f 100644 --- a/test/fixtures/templates/bad/functions/sub_needed.yaml +++ b/test/fixtures/templates/bad/functions/sub_needed.yaml @@ -31,6 +31,53 @@ Resources: Type: AWS::SNS::Topic Properties: TopicName: "${aws:username}-topic" + MultilineYAMLTestPolicy: + Type: AWS::IoT::Policy + Properties: + PolicyDocument: > + { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": "iot:Connect", + "Resource": "arn:aws:iot:eu-central-1:123456789012:${badstringsubstitution}/${iot:Connection.Thing.ThingName}" + } + ] + } + PolicyName: ConnectWithThingName + GreetingRequest: + Type: AWS::ApiGateway::Method + Properties: + AuthorizationType: NONE + HttpMethod: GET + Integration: + Type: AWS + IntegrationHttpMethod: POST + Uri: + Fn::Join: + - "" + - - "arn:aws:apigateway:" + - !Ref "AWS::Region" + - ":lambda:path/2015-03-31/functions/" + - !GetAtt GreetingLambda.Arn + - ":${notAstageVariable.LambdaAlias}" + - "/invocations" + IntegrationResponses: + - StatusCode: '200' + RequestTemplates: + application/json: + Fn::Join: + - "" + - - "{" + - " \"name\": \"$input.params('name')\"" + - "}" + RequestParameters: + method.request.querystring.name: false + ResourceId: !Ref GreetingResource + RestApiId: !Ref GreetingApi + MethodResponses: + - StatusCode: '200' TestBadStateMachine1: Type: AWS::StepFunctions::StateMachine diff --git a/test/fixtures/templates/good/functions/sub_needed.yaml b/test/fixtures/templates/good/functions/sub_needed.yaml index d084810c24..b8938a6fcb 100644 --- a/test/fixtures/templates/good/functions/sub_needed.yaml +++ b/test/fixtures/templates/good/functions/sub_needed.yaml @@ -134,6 +134,21 @@ Resources: - !Ref AWS::Region - !Ref AWS::AccountId - thing/${iot:Connection.Thing.ThingName}* + MultilineYAMLTestPolicy: + Type: AWS::IoT::Policy + Properties: + PolicyDocument: > + { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": "iot:Connect", + "Resource": "arn:aws:iot:eu-central-1:123456789012:client/${iot:Connection.Thing.ThingName}" + } + ] + } + PolicyName: ConnectWithThingName TestGoodStateMachine1: Type: AWS::StepFunctions::StateMachine Properties: diff --git a/test/unit/rules/functions/test_sub_needed.py b/test/unit/rules/functions/test_sub_needed.py index 21071b3210..a9db847367 100644 --- a/test/unit/rules/functions/test_sub_needed.py +++ b/test/unit/rules/functions/test_sub_needed.py @@ -36,4 +36,4 @@ def test_template_config(self): def test_file_negative(self): """Test failure""" - self.helper_file_negative('test/fixtures/templates/bad/functions/sub_needed.yaml', 8) + self.helper_file_negative('test/fixtures/templates/bad/functions/sub_needed.yaml', 10) diff --git a/test/unit/specs/test_required_based_on_value.py b/test/unit/specs/test_required_based_on_value.py index 35f02ad980..3be0b9a182 100644 --- a/test/unit/specs/test_required_based_on_value.py +++ b/test/unit/specs/test_required_based_on_value.py @@ -6,6 +6,7 @@ import test.fixtures.schemas from jsonschema import validate from test.testlib.testcase import BaseTestCase +from cfnlint.data import AdditionalSpecs import cfnlint.helpers diff --git a/test/unit/specs/test_sub_needed_excludes_schema.py b/test/unit/specs/test_sub_needed_excludes_schema.py new file mode 100644 index 0000000000..189f2a82db --- /dev/null +++ b/test/unit/specs/test_sub_needed_excludes_schema.py @@ -0,0 +1,23 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" +import logging +import test.fixtures.schemas +from jsonschema import validate +from test.testlib.testcase import BaseTestCase +from cfnlint.data import AdditionalSpecs +import cfnlint.helpers + + +LOGGER = logging.getLogger('cfnlint.maintenance') +LOGGER.addHandler(logging.NullHandler()) + + +class TestSubNeededExcludesSchema(BaseTestCase): + + + def test_validate_additional_specs_schema(self): + spec = cfnlint.helpers.load_resource(cfnlint.data.AdditionalSpecs, 'SubNeededExcludes.json') + schema = cfnlint.helpers.load_resource(test.fixtures.schemas, 'SubNeededExcludes.json') + validate(instance=spec, schema=schema)