fix: Prevent duplicated cdk-defined resource ids

Limiting aws:cdk:path metadata partitions length in order to prevent
duplicated resource ids. Logical id will be used if it is not possible
to ensure that the resource id extracted from aws:cdk:path is unique.

Author: rafael747

samcli/lib/samlib/resource_metadata_normalizer.py

@@ -253,10 +253,14 @@ def get_resource_id(resource_properties, logical_id):
         # Design doc of CDK path: https://github.com/aws/aws-cdk/blob/master/design/construct-tree.md
         cdk_path_partitions = resource_cdk_path.split("/")
         min_cdk_path_partitions_length = 2
+        max_cdk_path_partitions_length = 3
 
         LOG.debug("CDK Path for resource %s is %s", logical_id, cdk_path_partitions)
 
-        if len(cdk_path_partitions) < min_cdk_path_partitions_length:
+        if (
+            len(cdk_path_partitions) < min_cdk_path_partitions_length
+            or len(cdk_path_partitions) > max_cdk_path_partitions_length
+        ):
             LOG.warning(
                 "Cannot detect function id from aws:cdk:path metadata '%s', using default logical id", resource_cdk_path
             )

tests/unit/commands/local/lib/test_provider.py

@@ -748,6 +748,12 @@ def setUp(self):
                                 "aws:cdk:path": "Stack/CDKResource1-x/Resource",
                             },
                         },
+                        "CDKResource2": {
+                            "Properties": {"Body"},
+                            "Metadata": {
+                                "aws:cdk:path": "CDKResource2-x",
+                            },
+                        },
                         "CFNResource1": {
                             "Properties": {"Body"},
                         },
@@ -767,6 +773,11 @@ def setUp(self):
                                 "aws:cdk:path": "Stack/CDKResourceInChild1-x/Resource",
                             },
                         },
+                        "CDKResourceInChild2": {
+                            "Metadata": {
+                                "aws:cdk:path": "Stack/CDKResourceInChild2-x/Lambda/Resource",
+                            },
+                        },
                         "CFNResourceInChild1": {
                             "Properties": {"Body"},
                         },
@@ -780,16 +791,21 @@ def setUp(self):
             (ResourceIdentifier("CFNResource1"), "CFNResource1"),
             (ResourceIdentifier("CDKResource1"), "CDKResource1-x"),
             (ResourceIdentifier("CDKResource1-x"), "CDKResource1-x"),
+            (ResourceIdentifier("CDKResource2"), "CDKResource2"),
             (ResourceIdentifier("CFNResourceInChild1"), "childStack/CFNResourceInChild1"),
             (ResourceIdentifier("childStack/CFNResourceInChild1"), "childStack/CFNResourceInChild1"),
             (ResourceIdentifier("CDKResourceInChild1"), "childStack/CDKResourceInChild1-x"),
             (ResourceIdentifier("CDKResourceInChild1-x"), "childStack/CDKResourceInChild1-x"),
+            (ResourceIdentifier("CDKResourceInChild2"), "childStack/CDKResourceInChild2"),
             (ResourceIdentifier("childStack/CDKResourceInChild1-x"), "childStack/CDKResourceInChild1-x"),
+            (ResourceIdentifier("childStack/CDKResourceInChild2"), "childStack/CDKResourceInChild2"),
             (ResourceIdentifier("InvalidResourceId"), None),
             (ResourceIdentifier("InvalidStackId/CFNResourceInChild1"), None),
             # we should use iac_resource_id to define full path, could not use resource logical id in full path although
             # cdk id is there
             (ResourceIdentifier("childStack/CDKResourceInChild1"), None),
+            (ResourceIdentifier("CDKResource2-x"), None),
+            (ResourceIdentifier("childStack/CDKResourceInChild2-x"), None),
         ]
     )
     def test_get_resource_full_path_by_id(self, resource_id, expected_full_path):

tests/unit/commands/local/lib/test_sam_function_provider.py

@@ -212,6 +212,19 @@ class TestSamFunctionProviderEndToEnd(TestCase):
                     "aws:asset:property": "Code",
                 },
             },
+            "LambdaCDKFuncWithInvalidCDKPath": {
+                "Type": "AWS::Lambda::Function",
+                "Properties": {
+                    "Code": {"Bucket": "bucket", "Key": "key"},
+                    "Runtime": "nodejs4.3",
+                    "Handler": "index.handler",
+                },
+                "Metadata": {
+                    "aws:cdk:path": "Stack/LambdaCFKFunction-x/Lambda/Resource",
+                    "aws:asset:path": "/usr/foo/bar",
+                    "aws:asset:property": "Code",
+                },
+            },
             "OtherResource": {
                 "Type": "AWS::Serverless::Api",
                 "Properties": {"StageName": "prod", "DefinitionUri": "s3://bucket/key"},
@@ -929,6 +942,39 @@ def setUp(self):
                     function_build_info=FunctionBuildInfo.BuildableZip,
                 ),
             ),
+            (
+                "LambdaCDKFuncWithInvalidCDKPath",
+                Function(
+                    function_id="LambdaCDKFuncWithInvalidCDKPath",
+                    name="LambdaCDKFuncWithInvalidCDKPath",
+                    functionname="LambdaCDKFuncWithInvalidCDKPath",
+                    runtime="nodejs4.3",
+                    handler="index.handler",
+                    codeuri="/usr/foo/bar",
+                    memory=None,
+                    timeout=None,
+                    environment=None,
+                    rolearn=None,
+                    layers=[],
+                    events=None,
+                    metadata={
+                        "aws:cdk:path": "Stack/LambdaCFKFunction-x/Lambda/Resource",
+                        "aws:asset:path": "/usr/foo/bar",
+                        "aws:asset:property": "Code",
+                        "SamNormalized": True,
+                        "SamResourceId": "LambdaCDKFuncWithInvalidCDKPath",
+                    },
+                    inlinecode=None,
+                    imageuri=None,
+                    imageconfig=None,
+                    packagetype=ZIP,
+                    codesign_config_arn=None,
+                    architectures=None,
+                    function_url_config=None,
+                    stack_path="",
+                    function_build_info=FunctionBuildInfo.BuildableZip,
+                ),
+            ),
             (
                 "LambdaCDKFuncInChild-x",
                 Function(
@@ -1059,6 +1105,7 @@ def test_get_all_must_return_all_functions(self):
             "LambdaFuncWithCodeSignConfig",
             "LambdaFunctionCustomId-x",
             "LambdaCFKFunction-x",
+            "LambdaCDKFuncWithInvalidCDKPath",
             posixpath.join("ChildStack", "SamFunctionsInChild"),
             posixpath.join("ChildStack", "SamFunctionsInChildAbsPath"),
             posixpath.join("ChildStack", "SamImageFunctionsInChild"),

tests/unit/commands/local/lib/test_sam_layer_provider.py

@@ -56,6 +56,23 @@ class TestSamLayerProvider(TestCase):
                     "aws:asset:property": "Content",
                 },
             },
+            "CDKLambdaLayerWithInvalidCDKPath": {
+                "Type": "AWS::Lambda::LayerVersion",
+                "Properties": {
+                    "LayerName": "Layer1",
+                    "Content": {
+                        "S3Bucket": "bucket",
+                        "S3Key": "key",
+                    },
+                    "CompatibleRuntimes": ["python3.9"],
+                },
+                "Metadata": {
+                    "BuildMethod": "python3.9",
+                    "aws:cdk:path": "stack/CDKLambdaLayer-x/Lambda/Resource",
+                    "aws:asset:path": "PyLayer/",
+                    "aws:asset:property": "Content",
+                },
+            },
             "ServerlessLayerNoBuild": {
                 "Type": "AWS::Serverless::LayerVersion",
                 "Properties": {
@@ -256,6 +273,23 @@ def setUp(self):
                     stack_path="",
                 ),
             ),
+            (
+                "CDKLambdaLayerWithInvalidCDKPath",
+                LayerVersion(
+                    "CDKLambdaLayerWithInvalidCDKPath",
+                    "PyLayer",
+                    ["python3.9"],
+                    {
+                        "BuildMethod": "python3.9",
+                        "aws:cdk:path": "stack/CDKLambdaLayer-x/Lambda/Resource",
+                        "aws:asset:path": "PyLayer/",
+                        "aws:asset:property": "Content",
+                        "SamNormalized": True,
+                        "SamResourceId": "CDKLambdaLayerWithInvalidCDKPath",
+                    },
+                    stack_path="",
+                ),
+            ),
             (
                 "CDKLambdaLayerInChild-x",
                 LayerVersion(
@@ -326,6 +360,7 @@ def test_get_all_must_return_all_layers(self):
             "LambdaLayer",
             "LambdaLayerWithCustomId-x",
             "CDKLambdaLayer-x",
+            "CDKLambdaLayerWithInvalidCDKPath",
             "ServerlessLayerNoBuild",
             "LambdaLayerNoBuild",
             posixpath.join("ChildStack", "SamLayerInChild"),

tests/unit/lib/samlib/test_resource_metadata_normalizer.py

@@ -521,9 +521,19 @@ def test_use_cdk_id_as_resource_id(self, cdk_path, expected_resource_id):
 
         self.assertEqual(expected_resource_id, resource_id)
 
-    def test_use_logical_id_as_resource_id_incase_of_invalid_cdk_path(self):
+    @parameterized.expand(
+        [
+            ("func_cdk_id"),
+            ("stack_id/construct_id/lambda/Resource"),
+        ]
+    )
+    def test_use_logical_id_as_resource_id_incase_of_invalid_cdk_path(self, cdk_path):
         resource_id = ResourceMetadataNormalizer.get_resource_id(
-            {"Type": "any:value", "Properties": {"key": "value"}, "Metadata": {"aws:cdk:path": "func_cdk_id"}},
+            {
+                "Type": "any:value",
+                "Properties": {"key": "value"},
+                "Metadata": {"aws:cdk:path": cdk_path},
+            },
             "logical_id",
         )