Skip to content

Commit

Permalink
Update cfn lint schema based rules for dynamic references (#3442)
Browse files Browse the repository at this point in the history
  • Loading branch information
kddejong authored Jun 28, 2024
1 parent 57a0070 commit 2e01052
Show file tree
Hide file tree
Showing 12 changed files with 115 additions and 8 deletions.
4 changes: 4 additions & 0 deletions scripts/update_snapshot_results.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#!/usr/bin/env bash

# integration/
cfn-lint test/fixtures/templates/integration/dynamic-references.yaml -e -c I --format json > test/fixtures/results/integration/dynamic-references.json
cfn-lint test/fixtures/templates/integration/resources-cloudformation-init.yaml -e -c I --format json > test/fixtures/results/integration/resources-cloudformation-init.json

# public/
cfn-lint test/fixtures/templates/public/lambda-poller.yaml -e -c I --format json > test/fixtures/results/public/lambda-poller.json
cfn-lint test/fixtures/templates/public/watchmaker.json -e -x E3012:strict=true -c I --format json > test/fixtures/results/public/watchmaker.json
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{
"additionalProperties": true,
"description": "When creating an aurora DBInstance don't specify 'AllocatedStorage', 'BackupRetentionPeriod', 'CopyTagsToSnapshot', 'DeletionProtection', 'EnableIAMDatabaseAuthentication', 'MasterUserPassword', or 'StorageEncrypted'",
"else": {},
"if": {
"properties": {
"Engine": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{
"additionalProperties": true,
"description": "When 'Type' is 'CLOUDWATCH_METRIC' you must specify 'AlarmIdentifier'",
"else": {},
"if": {
"properties": {
"Type": {
Expand Down
7 changes: 6 additions & 1 deletion src/cfnlint/jsonschema/_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ class FunctionFilter:
default=True,
)

validate_dynamic_references: bool = field(
init=True,
default=True,
)

def _filter_schemas(self, schema, validator: Validator) -> Tuple[Any, Any]:
"""
Filter the schemas to only include the ones that are required
Expand Down Expand Up @@ -104,7 +109,7 @@ def _filter_schemas(self, schema, validator: Validator) -> Tuple[Any, Any]:

def filter(self, validator: Any, instance: Any, schema: Any):
# Lets validate dynamic references when appropriate
if validator.is_type(instance, "string"):
if validator.is_type(instance, "string") and self.validate_dynamic_references:
if REGEX_DYN_REF.findall(instance):
# if we are in a function we can't validate
# dynamic references the same way
Expand Down
8 changes: 6 additions & 2 deletions src/cfnlint/rules/functions/DynamicReference.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,17 @@ def dynamicReference(
# SSM parameters can be used in Resources and Outputs and Parameters
# SSM secrets are only used in a small number of locations
# Secrets manager can be used only in resource properties
validator = validator.evolve(
function_filter=validator.function_filter.evolve(
add_cfn_lint_keyword=False,
),
)

for v in REGEX_DYN_REF.findall(instance):
parts = v.split(":")

evolved = validator.evolve(schema=_all)
found = False
evolved = validator.evolve(schema=_all)
for err in evolved.iter_errors(parts):
yield self._clean_errors(err)
found = True
Expand All @@ -126,4 +131,3 @@ def dynamicReference(

for err in evolved.iter_errors(parts):
yield self._clean_errors(err)
found = True
7 changes: 6 additions & 1 deletion src/cfnlint/rules/jsonschema/CfnLintJsonSchema.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,12 @@ def validate(
schema = self._schema

cfn_validator = self.extend_validator(
validator=validator,
validator=validator.evolve(
function_filter=validator.function_filter.evolve(
validate_dynamic_references=False,
add_cfn_lint_keyword=False,
)
),
schema=schema,
context=validator.context.evolve(
functions=[],
Expand Down
31 changes: 31 additions & 0 deletions test/fixtures/results/integration/dynamic-references.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
[
{
"Filename": "test/fixtures/templates/integration/dynamic-references.yaml",
"Id": "e019d69f-56db-cf2f-2bbb-f951c3bc9782",
"Level": "Error",
"Location": {
"End": {
"ColumnNumber": 21,
"LineNumber": 16
},
"Path": [
"Resources",
"SESEventSourceMappingBadDynamicReference",
"Properties",
"EventSourceArn"
],
"Start": {
"ColumnNumber": 7,
"LineNumber": 16
}
},
"Message": "'{{:ssm:/SQS_Queue/SQS_ARN}}' does not match 'arn:(aws[a-zA-Z0-9-]*):([a-zA-Z0-9\\\\-])+:([a-z]{2}(-gov)?(-iso([a-z])?)?-[a-z]+-\\\\d{1})?:(\\\\d{12})?:(.*)'",
"ParentId": null,
"Rule": {
"Description": "Check if properties have a valid value in case of a pattern (Regular Expression)",
"Id": "E3031",
"ShortDescription": "Check if property values adhere to a specific pattern",
"Source": "https://github.com/aws-cloudformation/cfn-lint/blob/main/docs/cfn-schema-specification.md#pattern"
}
}
]
31 changes: 31 additions & 0 deletions test/fixtures/templates/integration/dynamic-references.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@

AWSTemplateFormatVersion: '2010-09-09'
Resources:
SESEventSourceMapping:
Type: AWS::Lambda::EventSourceMapping
Properties:
BatchSize: 10
Enabled: true
EventSourceArn: '{{resolve:ssm:/SQS_Queue/SQS_ARN}}'
FunctionName: MyFunctionNameHere
SESEventSourceMappingBadDynamicReference:
Type: AWS::Lambda::EventSourceMapping
Properties:
BatchSize: 10
Enabled: true
EventSourceArn: '{{:ssm:/SQS_Queue/SQS_ARN}}'
FunctionName: MyFunctionNameHere
Broker:
Type: AWS::AmazonMQ::Broker
Properties:
Users:
- Username: test
Password: '{{resolve:secretsmanager:MyRDSSecret:SecretString:password}}'
DeploymentMode: SINGLE_INSTANCE
EngineType: RABBITMQ
EngineVersion: test
# testing dynamic reference against the regional schema validation
HostInstanceType: '{{resolve:ssm:/SQS_Queue/SQS_ARN}}'
AutoMinorVersionUpgrade: true
BrokerName: test
PubliclyAccessible: false
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,24 @@ class TestQuickStartTemplates(BaseCliTestCase):
scenarios = [
{
"filename": (
"test/fixtures/templates/integration/good"
"test/fixtures/templates/integration"
"/resources-cloudformation-init.yaml"
),
"results_filename": (
"test/fixtures/results/integration/"
"good/resources-cloudformation-init.json"
"resources-cloudformation-init.json"
),
"exit_code": 0,
},
{
"filename": (
"test/fixtures/templates/integration" "/dynamic-references.yaml"
),
"results_filename": (
"test/fixtures/results/integration/" "dynamic-references.json"
),
"exit_code": 2,
},
]

def test_templates(self):
Expand Down
20 changes: 20 additions & 0 deletions test/unit/rules/jsonschema/test_cfn_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
SPDX-License-Identifier: MIT-0
"""

from __future__ import annotations

from collections import deque

import pytest
Expand All @@ -18,6 +20,10 @@
"type": "string",
},
"b": {"type": "object", "properties": {"1": {"type": "string"}}},
"c": {
"type": "string",
"pattern": "^foo$",
},
},
}

Expand Down Expand Up @@ -78,6 +84,20 @@ def __init__(self) -> None:
),
],
),
(
"Validation error with dynamic reference",
_BestError(),
{"c": "{{resolve:ssm:/SQS_Queue/SQS_ARN}}"},
[
ValidationError(
message="Rule that returns the best error",
validator="pattern",
path=deque(["c"]),
rule=_BestError(),
schema_path=deque(["properties", "c", "pattern"]),
),
],
),
],
)
def test_cfn_schema(name, rule, instance, expected_errs):
Expand Down

0 comments on commit 2e01052

Please sign in to comment.