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

CloudFormation Registry resource schema support (--registry-schemas) #1732

Merged
merged 26 commits into from
Dec 2, 2020
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
a706916
CloudFormation Registry resource schema support
PatMyron Oct 8, 2020
bef33b5
__init__.py
PatMyron Oct 8, 2020
a4bae45
remove print statement
PatMyron Oct 8, 2020
7e84e41
adding resource schema constraint for testing
PatMyron Oct 8, 2020
d980723
jsonschema validation
PatMyron Oct 9, 2020
44bc4c0
jsonschema.ValidationError -> RuleMatch format
PatMyron Oct 9, 2020
2d5a274
generalizing beyond one resource type
PatMyron Oct 10, 2020
60f0a9e
update E3001 Invalid or unsupported type to include Registry resource…
PatMyron Oct 10, 2020
8cb475a
handling dynamic references
PatMyron Oct 18, 2020
334b54a
handling psuedoparameters
PatMyron Oct 19, 2020
6c0b32b
handling intrinsic functions
PatMyron Oct 19, 2020
714d807
handling intrinsic functions without intrinsic function prefix
PatMyron Oct 19, 2020
f7d7ade
pylint
PatMyron Oct 19, 2020
5c82758
rule metadata
PatMyron Oct 19, 2020
6a3951a
removing resource schema used for testing
PatMyron Oct 19, 2020
1f6db95
renaming directory to ResourceSchemas
PatMyron Oct 19, 2020
e0a039d
adding Properties to RuleMatch path
PatMyron Nov 16, 2020
0258254
removing docstrings, typo
PatMyron Dec 1, 2020
3281ba6
renaming directory to RegistrySchemas
PatMyron Dec 1, 2020
90dc2ae
switching from reading from directory to reading from variable
PatMyron Dec 1, 2020
758dd5f
--registry-schemas CLI argument
PatMyron Dec 1, 2020
8504bf1
Merge branch 'master' into schemas
PatMyron Dec 1, 2020
3fea969
pylint
PatMyron Dec 1, 2020
82a5381
comment explaining CloudFormation template syntax being ignored
PatMyron Dec 1, 2020
2d65480
Merge branch 'master' into schemas
PatMyron Dec 1, 2020
64e4b96
Merge branch 'master' into schemas
PatMyron Dec 2, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ Optional parameters:
| -u, --update-specs | | | Update the [CloudFormation Resource Specifications](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/cfn-resource-specification.html). You may need sudo to run this. You will need internet access when running this command |
| -o, --override-spec | | filename | Spec-style file containing custom definitions. Can be used to override CloudFormation specifications. More info [here](#customize-specifications) |
| -g, --build-graph | | | Creates a file in the same directory as the template that models the template's resources in [DOT format](https://en.wikipedia.org/wiki/DOT_(graph_description_language)) |
| -s, --registry-schemas | | | one or more directories of [CloudFormation Registry](https://aws.amazon.com/blogs/aws/cloudformation-update-cli-third-party-resource-support-registry/) [Resource Schemas](https://github.com/aws-cloudformation/aws-cloudformation-resource-schema/)
PatMyron marked this conversation as resolved.
Show resolved Hide resolved
| -v, --version | | | Version of cfn-lint |

### Info Rules
Expand Down
3 changes: 1 addition & 2 deletions src/cfnlint/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@


def main():
"""Main function"""
if sys.version_info[:2] == (3, 4):
warnings.warn('Python 3.4 has reached end of life. '
'cfn-lint will end support for python 3.4 on July 1st, 2020.', Warning, stacklevel=3)
Expand All @@ -33,7 +32,7 @@ def main():
matches.extend(
cfnlint.core.run_cli(
filename, template, rules,
args.regions, args.override_spec, args.build_graph, args.mandatory_checks))
args.regions, args.override_spec, args.build_graph, args.registry_schemas, args.mandatory_checks))
else:
matches.extend(errors)
LOGGER.debug('Completed linting of file: %s', str(filename))
Expand Down
2 changes: 0 additions & 2 deletions src/cfnlint/conditions.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ def process_influenced_equal(self, equal):
self.Influenced_Equals[equal.Right.Function].add(equal.Left.String)

def process_condition(self, template, value):
""" process condition """
if isinstance(value, dict):
if len(value) == 1:
for func_name, func_value in value.items():
Expand Down Expand Up @@ -166,7 +165,6 @@ def process_condition(self, template, value):
raise ConditionParseError

def process_function(self, template, values):
""" Process Function """
results = []
for value in values:
if isinstance(value, dict):
Expand Down
40 changes: 8 additions & 32 deletions src/cfnlint/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@


def configure_logging(debug_logging, info_logging):
"""Setup Logging"""
ch = logging.StreamHandler()
ch.setLevel(logging.DEBUG)

Expand Down Expand Up @@ -333,7 +332,7 @@ def __call__(self, parser, namespace, values, option_string=None):
standard = parser.add_argument_group('Standard')
advanced = parser.add_argument_group('Advanced / Debugging')

# Alllow the template to be passes as an optional or a positional argument
# Allow the template to be passes as an optional or a positional argument
standard.add_argument(
'templates', metavar='TEMPLATE', nargs='*', help='The CloudFormation template to be linted')
standard.add_argument(
Expand All @@ -356,7 +355,6 @@ def __call__(self, parser, namespace, values, option_string=None):
standard.add_argument(
'-f', '--format', help='Output Format', choices=['quiet', 'parseable', 'json', 'junit', 'pretty']
)

standard.add_argument(
'-l', '--list-rules', dest='listrules', default=False,
action='store_true', help='list all the rules'
Expand Down Expand Up @@ -390,25 +388,23 @@ def __call__(self, parser, namespace, values, option_string=None):
standard.add_argument(
'-e', '--include-experimental', help='Include experimental rules', action='store_true'
)

standard.add_argument(
'-x', '--configure-rule', dest='configure_rules', nargs='+', default={},
action=RuleConfigurationAction,
help='Provide configuration for a rule. Format RuleId:key=value. Example: E3012:strict=false'
)

standard.add_argument('--config-file', dest='config_file',
help='Specify the cfnlintrc file to use')

advanced.add_argument(
'-o', '--override-spec', dest='override_spec',
help='A CloudFormation Spec override file that allows customization'
)

advanced.add_argument(
'-g', '--build-graph', help='Creates a file in the same directory as the template that models the template\'s resources in DOT format', action='store_true'
)

advanced.add_argument(
'-s', '--registry-schemas', help='one or more directories of CloudFormation Registry Schemas', action='extend', type=comma_separated_arg, nargs='+'
)
standard.add_argument(
'-v', '--version', help='Version of cfn-lint', action='version',
version='%(prog)s {version}'.format(version=__version__)
Expand All @@ -425,7 +421,6 @@ def __call__(self, parser, namespace, values, option_string=None):
'--update-iam-policies', help=argparse.SUPPRESS,
action='store_true'
)

standard.add_argument(
'--output-file', type=str, default=None,
help='Writes the output to the specified file, ideal for producing reports'
Expand All @@ -441,11 +436,9 @@ def __init__(self, template_args):
self.set_template_args(template_args)

def get_template_args(self):
""" Get Template Args"""
return self._template_args

def set_template_args(self, template):
""" Set Template Args"""
defaults = {}
if isinstance(template, dict):
configs = template.get('Metadata', {}).get('cfn-lint', {}).get('config', {})
Expand Down Expand Up @@ -491,7 +484,6 @@ def __init__(self, cli_args):
self, config_file=self._get_argument_value('config_file', False, False))

def _get_argument_value(self, arg_name, is_template, is_config_file):
""" Get Argument value """
cli_value = getattr(self.cli_args, arg_name)
template_value = self.template_args.get(arg_name)
file_value = self.file_args.get(arg_name)
Expand All @@ -505,28 +497,23 @@ def _get_argument_value(self, arg_name, is_template, is_config_file):

@property
def ignore_checks(self):
""" ignore_checks """
return self._get_argument_value('ignore_checks', True, True)

@property
def include_checks(self):
""" include_checks """
results = self._get_argument_value('include_checks', True, True)
return ['W', 'E'] + results

@property
def mandatory_checks(self):
""" mandatory_checks """
return self._get_argument_value('mandatory_checks', False, True)

@property
def include_experimental(self):
""" include_experimental """
return self._get_argument_value('include_experimental', True, True)

@property
def regions(self):
""" regions """
results = self._get_argument_value('regions', True, True)
if not results:
return ['us-east-1']
Expand All @@ -536,22 +523,18 @@ def regions(self):

@property
def ignore_bad_template(self):
""" ignore_bad_template """
return self._get_argument_value('ignore_bad_template', True, True)

@property
def debug(self):
""" debug """
return self._get_argument_value('debug', False, False)

@property
def format(self):
""" format """
return self._get_argument_value('format', False, True)

@property
def templates(self):
""" templates """
templates_args = self._get_argument_value('templates', False, True)
template_alt_args = self._get_argument_value('template_alt', False, False)
if template_alt_args:
Expand Down Expand Up @@ -588,7 +571,6 @@ def templates(self):
return sorted(all_filenames)

def _ignore_templates(self):
""" templates """
ignore_template_args = self._get_argument_value('ignore_templates', False, True)
if ignore_template_args:
filenames = ignore_template_args
Expand Down Expand Up @@ -619,50 +601,44 @@ def _ignore_templates(self):

@property
def append_rules(self):
""" append_rules """
return self._get_argument_value('append_rules', False, True)

@property
def override_spec(self):
""" override_spec """
return self._get_argument_value('override_spec', False, True)

@property
def update_specs(self):
""" update_specs """
return self._get_argument_value('update_specs', False, False)

@property
def update_documentation(self):
""" update_specs """
return self._get_argument_value('update_documentation', False, False)

@property
def update_iam_policies(self):
""" update_iam_policies """
return self._get_argument_value('update_iam_policies', False, False)

@property
def listrules(self):
""" listrules """
return self._get_argument_value('listrules', False, False)

@property
def configure_rules(self):
""" Configure rules """
return self._get_argument_value('configure_rules', True, True)

@property
def config_file(self):
""" Config file """
return self._get_argument_value('config_file', False, False)

@property
def build_graph(self):
""" build_graph """
return self._get_argument_value('build_graph', False, False)

@property
def output_file(self):
""" output_file """
return self._get_argument_value('output_file', False, True)

@property
def registry_schemas(self):
return self._get_argument_value('registry_schemas', False, True)
14 changes: 10 additions & 4 deletions src/cfnlint/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
SPDX-License-Identifier: MIT-0
"""
import json
import logging
import os
import sys
Expand All @@ -15,7 +16,7 @@
import cfnlint.formatters
import cfnlint.decode
import cfnlint.maintenance
from cfnlint.helpers import REGIONS
from cfnlint.helpers import REGIONS, REGISTRY_SCHEMAS

LOGGER = logging.getLogger('cfnlint')
DEFAULT_RULESDIR = os.path.join(os.path.dirname(__file__), 'rules')
Expand All @@ -39,7 +40,7 @@ class UnexpectedRuleException(CfnLintExitException):
"""When processing a rule fails in an unexpected way"""


def run_cli(filename, template, rules, regions, override_spec, build_graph, mandatory_rules=None):
def run_cli(filename, template, rules, regions, override_spec, build_graph, registry_schemas, mandatory_rules=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

outside packages shouldn't be using run_cli by any concerns with the ordering here? I've been adding parameters and setting a default value to keep some backwards compatibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

@PatMyron PatMyron Dec 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have generally been debating compatibility for reliance on undocumented internal functions/variables

"""Process args and run"""

if override_spec:
Expand All @@ -49,6 +50,13 @@ def run_cli(filename, template, rules, regions, override_spec, build_graph, mand
template_obj = Template(filename, template, regions)
template_obj.build_graph()

if registry_schemas:
for path in registry_schemas:
if path and os.path.isdir(os.path.expanduser(path)):
for f in os.listdir(path):
with open(os.path.join(path, f)) as schema:
REGISTRY_SCHEMAS.append(json.load(schema))

return run_checks(filename, template, rules, regions, mandatory_rules)


Expand All @@ -67,7 +75,6 @@ def get_exit_code(matches):


def get_formatter(fmt):
""" Get Formatter"""
formatter = {}
if fmt:
if fmt == 'quiet':
Expand All @@ -88,7 +95,6 @@ def get_formatter(fmt):


def get_rules(append_rules, ignore_rules, include_rules, configure_rules=None, include_experimental=False, mandatory_rules=None):
"""Get rules"""
rules = RulesCollection(ignore_rules, include_rules, configure_rules,
include_experimental, mandatory_rules)
rules_paths = [DEFAULT_RULESDIR] + append_rules
Expand Down
2 changes: 1 addition & 1 deletion src/cfnlint/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ def load_resource(package, filename='us-east-1.json'):


RESOURCE_SPECS = {}

REGISTRY_SCHEMAS = []

def merge_spec(source, destination):
""" Recursive merge spec dict """
Expand Down
4 changes: 0 additions & 4 deletions src/cfnlint/maintenance.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@


def update_resource_specs():
""" Update Resource Specs """

# Pool() uses cpu count if no number of processors is specified
# Pool() only implements the Context Manager protocol from Python3.3 onwards,
# so it will fail Python2.7 style linting, as well as throw AttributeError
Expand Down Expand Up @@ -87,8 +85,6 @@ def search_and_replace_botocore_types(obj):
json.dump(spec, f, indent=2, sort_keys=True, separators=(',', ': '))

def update_documentation(rules):
"""Generate documentation"""

# Update the overview of all rules in the linter
filename = 'docs/rules.md'

Expand Down
3 changes: 2 additions & 1 deletion src/cfnlint/rules/resources/Configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
SPDX-License-Identifier: MIT-0
"""
import six
from cfnlint.helpers import REGISTRY_SCHEMAS
from cfnlint.rules import CloudFormationLintRule
from cfnlint.rules import RuleMatch
import cfnlint.helpers
Expand Down Expand Up @@ -99,7 +100,7 @@ def _check_resource(self, cfn, resource_name, resource_values):
self.logger.debug('Check resource types by region...')
for region, specs in cfnlint.helpers.RESOURCE_SPECS.items():
if region in cfn.regions:
if resource_type not in specs['ResourceTypes']:
if resource_type not in specs['ResourceTypes'] and resource_type not in [s['typeName'] for s in REGISTRY_SCHEMAS]:
if not resource_type.startswith(('Custom::', 'AWS::Serverless::')) and not resource_type.endswith('::MODULE'):
message = 'Invalid or unsupported Type {0} for resource {1} in {2}'
matches.append(RuleMatch(
Expand Down
29 changes: 29 additions & 0 deletions src/cfnlint/rules/resources/ResourceSchema.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
"""
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
SPDX-License-Identifier: MIT-0
"""
import re
from jsonschema import validate, ValidationError
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the big problems we are going to have with using this package is handling Fn::If or other intrinsic functions. Either way we are going to have to handle it outside of the schema validation which we have logic to handle or we are going to have to rewrite these validation functions that this package provides to handle these things differently.

from cfnlint.helpers import REGEX_DYN_REF, PSEUDOPARAMS, FN_PREFIX, UNCONVERTED_SUFFIXES, REGISTRY_SCHEMAS
from cfnlint.rules import CloudFormationLintRule
from cfnlint.rules import RuleMatch

class ResourceSchema(CloudFormationLintRule):
id = 'E3000'
shortdesc = 'Resource schema'
description = 'CloudFormation Registry resource schema validation'
source_url = 'https://github.com/aws-cloudformation/aws-cloudformation-resource-schema/'
tags = ['resources']

def match(self, cfn):
matches = []
for schema in REGISTRY_SCHEMAS:
resource_type = schema['typeName']
for resource_name, resource_values in cfn.get_resources([resource_type]).items():
properties = resource_values.get('Properties', {})
if not re.match(REGEX_DYN_REF, str(properties)) and not any(x in str(properties) for x in PSEUDOPARAMS + UNCONVERTED_SUFFIXES) and FN_PREFIX not in str(properties):
Copy link
Contributor

@kddejong kddejong Nov 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this says ignore intrinsic function errors but doesn't handle more complex Fn::If scenarios.

ResourceName:
   Type: AWS::CloudFormation::StackSet
   Properties:
     Fn::If:
     - ConditionName
     - PropertiesScenarioTrue
     - PropertiesScenarioFalse

We basically don't get a failure even though there could be one in the provided scenario.

PatMyron marked this conversation as resolved.
Show resolved Hide resolved
try:
validate(properties, schema)
except ValidationError as e:
matches.append(RuleMatch(['Resources', resource_name, 'Properties'], e.message))
return matches
Loading