Consider output conditions in E6101 (#3414)

aws-cloudformation · Jun 26, 2024 · 31ac633 · 31ac633
1 parent 36d79c6
commit 31ac633
Show file tree

Hide file tree

Showing 3 changed files with 183 additions and 13 deletions.
diff --git a/src/cfnlint/rules/outputs/Value.py b/src/cfnlint/rules/outputs/Value.py
@@ -23,14 +23,25 @@ class Value(CfnLintJsonSchema):
 
     def __init__(self):
         super().__init__(
-            keywords=["Outputs/*/Value"],
+            keywords=["Outputs/*"],
             all_matches=True,
         )
 
     def validate(self, validator: Validator, _: Any, instance: Any, schema: Any):
+        value = instance.get("Value")
+        if not value:
+            return
+
+        conditions = {}
+        condition = instance.get("Condition")
+        if condition:
+            conditions = {condition: True}
         validator = validator.evolve(
             context=validator.context.evolve(
                 functions=list(FUNCTIONS),
+                conditions=validator.context.conditions.evolve(
+                    conditions,
+                ),
             ),
             schema={
                 "type": ["array", "string"],
@@ -40,4 +51,6 @@ def validate(self, validator: Validator, _: Any, instance: Any, schema: Any):
             },
         )
 
-        yield from self._iter_errors(validator, instance)
+        for err in self._iter_errors(validator, value):
+            err.path.appendleft("Value")
+            yield err
diff --git a/test/integration/test_schema_files.py b/test/integration/test_schema_files.py
@@ -34,6 +34,7 @@ class TestSchemaFiles(TestCase):
         "Metadata/AWS::CloudFormation::Interface",
         "Metadata/cfn-lint",
         "Outputs",
+        "Outputs/*",
         "Outputs/*/Condition",
         "Outputs/*/Export/Name",
         "Outputs/*/Value",

diff --git a/test/unit/rules/outputs/test_value.py b/test/unit/rules/outputs/test_value.py
@@ -3,27 +3,183 @@
 SPDX-License-Identifier: MIT-0
 """
 
+from collections import deque
+
 import pytest
 
-from cfnlint.jsonschema import CfnTemplateValidator
+from cfnlint.jsonschema import CfnTemplateValidator, ValidationError
+from cfnlint.rules.functions.Cidr import Cidr
+from cfnlint.rules.functions.Join import Join
+from cfnlint.rules.functions.Ref import Ref
 from cfnlint.rules.outputs.Value import Value  # pylint: disable=E0401
 
 
+@pytest.fixture
+def template():
+    return {
+        "Parameters": {
+            "additionalVpcCidr": {
+                "Type": "String",
+                "Default": "",
+            },
+            "badAdditionalVpcCidr": {
+                "Type": "String",
+                "Default": "1",
+            },
+        },
+        "Conditions": {
+            "isAdditionalVpc": {
+                "Fn::Not": [
+                    {
+                        "Fn::Equals": [
+                            {"Ref": "additionalVpcCidr"},
+                            "",
+                        ]
+                    }
+                ]
+            },
+            "isBadAdditionalVpc": {
+                "Fn::Not": [
+                    {
+                        "Fn::Equals": [
+                            {"Ref": "badAdditionalVpcCidr"},
+                            "",
+                        ]
+                    }
+                ]
+            },
+        },
+    }
+
+
+@pytest.fixture
+def validator(cfn):
+    yield CfnTemplateValidator(schema={}).extend(
+        validators={
+            "fn_join": Join().fn_join,
+            "ref": Ref().ref,
+            "fn_cidr": Cidr().fn_cidr,
+        }
+    )(
+        schema={},
+        cfn=cfn,
+    )
+
+
 @pytest.mark.parametrize(
     "input,expected",
     [
-        ("foo", 0),
-        (1.0, 1),
-        (1, 1),
-        (True, 1),
-        ([{}], 1),
-        ({"foo": "bar"}, 1),
+        (
+            {
+                "Value": "foo",
+            },
+            [],
+        ),
+        (
+            {
+                "Value": 1.0,
+            },
+            [
+                ValidationError(
+                    "1.0 is not of type 'array', 'string'",
+                    validator="type",
+                    schema_path=deque(["type"]),
+                    path=deque(["Value"]),
+                    rule=Value(),
+                )
+            ],
+        ),
+        (
+            {
+                "Value": 1,
+            },
+            [
+                ValidationError(
+                    "1 is not of type 'array', 'string'",
+                    validator="type",
+                    schema_path=deque(["type"]),
+                    path=deque(["Value"]),
+                    rule=Value(),
+                )
+            ],
+        ),
+        (
+            {"Value": True},
+            [
+                ValidationError(
+                    "True is not of type 'array', 'string'",
+                    validator="type",
+                    schema_path=deque(["type"]),
+                    path=deque(["Value"]),
+                    rule=Value(),
+                )
+            ],
+        ),
+        (
+            {"Value": [{}]},
+            [
+                ValidationError(
+                    "{} is not of type 'string'",
+                    validator="type",
+                    schema_path=deque(["items", "type"]),
+                    path=deque(["Value", 0]),
+                    rule=Value(),
+                )
+            ],
+        ),
+        (
+            {"Value": {"foo": "bar"}},
+            [
+                ValidationError(
+                    "{'foo': 'bar'} is not of type 'array', 'string'",
+                    validator="type",
+                    schema_path=deque(["type"]),
+                    path=deque(["Value"]),
+                    rule=Value(),
+                )
+            ],
+        ),
+        (
+            {
+                "Condition": "isAdditionalVpc",
+                "Value": {
+                    "Fn::Join": [
+                        ",",
+                        {"Fn::Cidr": [{"Ref": "additionalVpcCidr"}, 3, 8]},
+                    ]
+                },
+            },
+            [],
+        ),
+        (
+            {
+                "Condition": "isBadAdditionalVpc",
+                "Value": {
+                    "Fn::Join": [
+                        ",",
+                        {"Fn::Cidr": [{"Ref": "badAdditionalVpcCidr"}, 3, 8]},
+                    ]
+                },
+            },
+            [
+                ValidationError(
+                    (
+                        "{'Ref': 'badAdditionalVpcCidr'} does not match "
+                        "'^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\\\\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])(\\\\/([0-9]|[1-2][0-9]|3[0-2]))$'"
+                        " when 'Ref' is resolved"
+                    ),
+                    validator="ref",
+                    schema_path=deque(
+                        ["fn_join", "fn_items", "fn_cidr", "fn_items", "ref", "pattern"]
+                    ),
+                    path=deque(["Value", "Fn::Join", 1, "Fn::Cidr", 0, "Ref"]),
+                )
+            ],
+        ),
     ],
 )
-def test_output_value(input, expected):
+def test_output_value(input, expected, validator):
     rule = Value()
-    validator = CfnTemplateValidator()
-
     results = list(rule.validate(validator, {}, input, {}))
 
-    assert len(results) == expected, f"Expected {expected} results, got {results}"
+    assert results == expected, f"Expected {expected} results, got {results}"