add rule E3017 to check when a prop is required based on a value

src/cfnlint/data/AdditionalSpecs/RequiredBasedOnValue.json

@@ -0,0 +1,23 @@
+{
+    "PropertyTypes": {
+    },
+    "ResourceTypes": {
+        "AWS::Lambda::EventSourceMapping": {
+            "EventSourceArn": [
+                {
+                    "Regex": "arn:aws*:(kinesis|kafka|dynamodb):*",
+                    "GetAtt": {
+                        "AWS::DynamoDB::Table": "StreamArn",
+                        "AWS::Kinesis::Stream": "Arn"
+                    },
+                    "Ref": [
+                        "AWS::MSK::Cluster"
+                    ],
+                    "RequiredProperties": [
+                        "StartingPosition"
+                    ]
+                }
+            ]
+        }
+    }
+}

src/cfnlint/rules/resources/properties/RequiredBasedOnValue.py

@@ -0,0 +1,118 @@
+"""
+Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+SPDX-License-Identifier: MIT-0
+"""
+import re
+import six
+from cfnlint.rules import CloudFormationLintRule
+from cfnlint.rules import RuleMatch
+import cfnlint.helpers
+from cfnlint.data import AdditionalSpecs
+
+
+class RequiredBasedOnValue(CloudFormationLintRule):
+    """Check Required Properties are supplied when another property has a certain value"""
+    id = 'E3017'
+    shortdesc = 'Property is required based on another properties value'
+    description = 'When certain properties have a certain value it results in other properties being required. ' \
+        'This rule will validate those required properties are specified when those values are supplied'
+    tags = ['resources']
+
+    def __init__(self):
+        """Init"""
+        super(RequiredBasedOnValue, self).__init__()
+        self.requiredbasedonvalue = cfnlint.helpers.load_resource(
+            AdditionalSpecs, 'RequiredBasedOnValue.json')
+        self.resource_types_specs = self.requiredbasedonvalue['ResourceTypes']
+        self.property_types_specs = self.requiredbasedonvalue['PropertyTypes']
+        for resource_type_spec in self.resource_types_specs:
+            self.resource_property_types.append(resource_type_spec)
+        for property_type_spec in self.property_types_specs:
+            self.resource_sub_property_types.append(property_type_spec)
+
+    def _check_value(self, value, spec, cfn):
+        """ Checks a value to see if it fits in the spec """
+        if isinstance(value, six.string_types):
+            regex = spec.get('Regex')
+            if regex:
+                if re.match(spec.get('Regex'), value):
+                    return True
+        if isinstance(value, dict):
+            if len(value) == 1:
+                for k, v in value.items():
+                    ref = spec.get('Ref')
+                    if ref:
+                        if k == 'Ref':
+                            if isinstance(v, six.string_types):
+                                return cfn.template.get('Resources').get(v, {}).get('Type') in ref
+
+                    getatt = spec.get('GetAtt')
+                    if getatt:
+                        if k == 'Fn::GetAtt':
+                            if isinstance(v, list):
+                                restype = cfn.template.get('Resources').get(v[0]).get('Type')
+                                if restype in getatt:
+                                    return getatt.get(restype) == v[1]
+
+        return False
+
+    def _check_obj(self, properties, specs, path, cfn):
+        matches = []
+        for k, v in specs.items():
+            for s in v:
+                required_props = [k] + s.get('RequiredProperties')
+                scenarios = cfn.get_object_without_conditions(
+                    properties, property_names=required_props)
+                for scenario in scenarios:
+                    if self._check_value(scenario.get('Object').get(k), s, cfn):
+                        for required_prop in s.get('RequiredProperties'):
+                            if required_prop not in scenario.get('Object'):
+                                if scenario['Scenario'] is None:
+                                    message = 'When property \'{0}\' has its current value property \'{1}\' should be specified at {2}'
+                                    matches.append(RuleMatch(
+                                        path,
+                                        message.format(k, required_prop, '/'.join(map(str, path)))
+                                    ))
+                                else:
+                                    scenario_text = ' and '.join(['when condition "%s" is %s' % (
+                                        k, v) for (k, v) in scenario['Scenario'].items()])
+                                    message = 'When property \'{0}\' has its current value property \'{1}\' when {2}'
+                                    matches.append(RuleMatch(
+                                        path,
+                                        message.format(k, required_prop, scenario_text)))
+
+        return matches
+
+    def match_resource_properties(self, properties, resource_type, path, cfn):
+        """Check CloudFormation Properties"""
+        matches = []
+
+        if properties is None:
+            # covered under rule E3001.  If there are required properties properties is required first
+            return matches
+
+        # Need to get this spec
+        specs = self.requiredbasedonvalue.get('ResourceTypes').get(resource_type)
+
+        matches.extend(
+            self._check_obj(properties, specs, path, cfn)
+        )
+
+        return matches
+
+    def match_resource_sub_properties(self, properties, property_type, path, cfn):
+        """Check CloudFormation Properties"""
+        matches = []
+
+        if properties is None:
+            # covered under rule E3001.  If there are required properties properties is required first
+            return matches
+
+        # Need to get this spec
+        specs = self.requiredbasedonvalue.get('PropertyTypes').get(property_type)
+
+        matches.extend(
+            self._check_obj(properties, specs, path, cfn)
+        )
+
+        return matches

test/fixtures/schemas/RequiredBasedOnValue.json

@@ -0,0 +1,80 @@
+{
+    "$schema": "http://json-schema.org/draft-04/schema#",
+    "title": "Product",
+    "description": "Required Based on Value",
+    "type": "object",
+    "additionalProperties": false,
+    "properties": {
+        "ResourceTypes": { "$ref": "#/definitions/resourcetypes" },
+        "PropertyTypes": { "$ref": "#/definitions/propertytypes" }
+    },
+    "definitions": {
+        "propertytypes": {
+            "patternProperties": {
+                "^[a-zA-Z0-9]+::[a-zA-Z0-9]+::[a-zA-Z0-9]+\\.[a-zA-Z0-9]+$": {
+                    "type": "object",
+                    "items": {
+                        "$ref": "#/definitions/property"
+                    }
+                }
+            },
+            "additionalProperties": false
+        },
+        "resourcetypes": {
+            "patternProperties": {
+                "^[a-zA-Z0-9]+::[a-zA-Z0-9]+::[a-zA-Z0-9]+$": {
+                    "type": "object",
+                    "items": {
+                        "$ref": "#/definitions/property"
+                    }
+                }
+            },
+            "additionalProperties": false
+        },
+        "property": {
+            "patternProperties": {
+                "^.*$": {
+                    "type": "array",
+                    "items": {
+                        "$ref": "#/definitions/constraint"
+                    }
+                }
+            }
+        },
+        "getatt": {
+            "patternProperties": {
+                "^.*$": {
+                    "type": "string"
+                }
+            }
+        },
+        "constraint": {
+            "properties": {
+                "Regex": {
+                    "description": "The regex of the value we are looking for",
+                    "type": "string"
+                },
+                "GetAtt": {
+                    "description": "The type of supported GetAtt resource type and attribute",
+                    "type": "object",
+                    "items": {
+                        "$ref": "#/definitions/getatt"
+                    }
+                },
+                "Ref": {
+                    "description": "The type of Resource Types that can be REFed",
+                    "type": "array",
+                    "items": {
+                        "type": "string"
+                    },
+                    "uniqueItems": true
+                }
+            },
+            "additionalProperties": false
+        }
+    },
+    "required": [
+        "ResourceTypes",
+        "PropertyTypes"
+    ]
+}

test/fixtures/schemas/__init__.py

@@ -0,0 +1,4 @@
+"""
+Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+SPDX-License-Identifier: MIT-0
+"""

test/fixtures/templates/bad/resources/properties/required_based_on_value.yaml

@@ -0,0 +1,44 @@
+Parameters:
+  Kinesis:
+    Type: String
+    Default: ""
+Conditions:
+  isKinesis: !Not [!Equals [!Ref Kinesis, ""]]
+
+Resources:
+  EventSourceValue:
+    Type: AWS::Lambda::EventSourceMapping
+    Properties:
+      FunctionName: Test
+      EventSourceArn: "arn:aws:kinesis:test"
+  EventSourceValueWithCondition:
+    Type: AWS::Lambda::EventSourceMapping
+    Properties:
+      FunctionName: Test
+      EventSourceArn:
+        !If [isKinesis, "arn:aws:kinesis:test", "arn:aws:sqs:test"]
+      StartingPosition: !If [isKinesis, !Ref "AWS::NoValue", ""]
+  KmsCluster:
+    Type: AWS::MSK::Cluster
+    Properties:
+      ClusterName: Test
+      KafkaVersion: Test
+      BrokerNodeGroupInfo:
+        ClientSubnets:
+          - subnet-123456
+        InstanceType: t2.medium
+      NumberOfBrokerNodes: 3
+  EventSourceRef:
+    Type: AWS::Lambda::EventSourceMapping
+    Properties:
+      FunctionName: Test
+      EventSourceArn: !Ref KmsCluster
+  KinesisStream:
+    Type: AWS::Kinesis::Stream
+    Properties:
+      ShardCount: 1
+  EventSourceGetAtt:
+    Type: AWS::Lambda::EventSourceMapping
+    Properties:
+      FunctionName: Test
+      EventSourceArn: !GetAtt KinesisStream.Arn

test/fixtures/templates/good/resources/properties/required_based_on_value.yaml

@@ -0,0 +1,33 @@
+Resources:
+  EventSourceValue:
+    Type: AWS::Lambda::EventSourceMapping
+    Properties:
+      FunctionName: Test
+      EventSourceArn: 'arn:aws:kinesis:test'
+      StartingPosition: "0"
+  KmsCluster:
+    Type: AWS::MSK::Cluster
+    Properties:
+      ClusterName: Test
+      KafkaVersion: Test
+      BrokerNodeGroupInfo:
+        ClientSubnets:
+          - subnet-123456
+        InstanceType: t2.medium
+      NumberOfBrokerNodes: 3
+  EventSourceRef:
+    Type: AWS::Lambda::EventSourceMapping
+    Properties:
+      FunctionName: Test
+      EventSourceArn: !Ref KmsCluster
+      StartingPosition: "0"
+  KinesisStream:
+    Type: AWS::Kinesis::Stream
+    Properties:
+      ShardCount: 1
+  EventSourceGetAtt:
+    Type: AWS::Lambda::EventSourceMapping
+    Properties:
+      FunctionName: Test
+      EventSourceArn: !GetAtt KinesisStream.Arn
+      StartingPosition: "0"

test/unit/rules/resources/properties/test_required_based_on_value.py

@@ -0,0 +1,27 @@
+"""
+Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+SPDX-License-Identifier: MIT-0
+"""
+from test.unit.rules import BaseRuleTestCase
+from cfnlint.rules.resources.properties.RequiredBasedOnValue import RequiredBasedOnValue  # pylint: disable=E0401
+
+
+class TestRequiredBasedOnValue(BaseRuleTestCase):
+    """Test OnlyOne Property Configuration"""
+
+    def setUp(self):
+        """Setup"""
+        super(TestRequiredBasedOnValue, self).setUp()
+        self.collection.register(RequiredBasedOnValue())
+        self.success_templates = [
+            'test/fixtures/templates/good/resources/properties/required_based_on_value.yaml'
+        ]
+
+    def test_file_positive(self):
+        """Test Positive"""
+        self.helper_file_positive()
+
+    def test_file_negative(self):
+        """Test failure"""
+        self.helper_file_negative(
+            'test/fixtures/templates/bad/resources/properties/required_based_on_value.yaml', 4)

test/unit/specs/__init__.py

@@ -0,0 +1,4 @@
+"""
+Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+SPDX-License-Identifier: MIT-0
+"""

test/unit/specs/test_required_based_on_value.py

@@ -0,0 +1,25 @@
+"""
+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
+import cfnlint.helpers
+
+
+LOGGER = logging.getLogger('cfnlint.maintenance')
+LOGGER.addHandler(logging.NullHandler())
+
+
+class TestRequiredBasedOnValueSpec(BaseTestCase):
+    """Used for Testing Rules"""
+
+
+    def test_success_sbd_domain_removed(self):
+        """Success removal of SBD Domain form unsupported regions"""
+        spec = cfnlint.helpers.load_resource(cfnlint.data.AdditionalSpecs, 'RequiredBasedOnValue.json')
+        schema = cfnlint.helpers.load_resource(test.fixtures.schemas, 'RequiredBasedOnValue.json')
+        validate(spec, schema)
+