Skip to content

Commit 33d8b1e

Browse files
rafael747rafael747
committed
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.
1 parent f59ddab commit 33d8b1e

File tree

5 files changed

+115
-3
lines changed

5 files changed

+115
-3
lines changed

‎samcli/lib/samlib/resource_metadata_normalizer.py

+5-1
Original file line numberDiff line numberDiff line change
@@ -253,10 +253,14 @@ def get_resource_id(resource_properties, logical_id):
253253
# Design doc of CDK path: https://github.com/aws/aws-cdk/blob/master/design/construct-tree.md
254254
cdk_path_partitions = resource_cdk_path.split("/")
255255
min_cdk_path_partitions_length = 2
256+
max_cdk_path_partitions_length = 3
256257

257258
LOG.debug("CDK Path for resource %s is %s", logical_id, cdk_path_partitions)
258259

259-
if len(cdk_path_partitions) < min_cdk_path_partitions_length:
260+
if (
261+
len(cdk_path_partitions) < min_cdk_path_partitions_length
262+
or len(cdk_path_partitions) > max_cdk_path_partitions_length
263+
):
260264
LOG.warning(
261265
"Cannot detect function id from aws:cdk:path metadata '%s', using default logical id", resource_cdk_path
262266
)

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

+16
Original file line numberDiff line numberDiff line change
@@ -748,6 +748,12 @@ def setUp(self):
748748
"aws:cdk:path": "Stack/CDKResource1-x/Resource",
749749
},
750750
},
751+
"CDKResource2": {
752+
"Properties": {"Body"},
753+
"Metadata": {
754+
"aws:cdk:path": "CDKResource2-x",
755+
},
756+
},
751757
"CFNResource1": {
752758
"Properties": {"Body"},
753759
},
@@ -767,6 +773,11 @@ def setUp(self):
767773
"aws:cdk:path": "Stack/CDKResourceInChild1-x/Resource",
768774
},
769775
},
776+
"CDKResourceInChild2": {
777+
"Metadata": {
778+
"aws:cdk:path": "Stack/CDKResourceInChild2-x/Lambda/Resource",
779+
},
780+
},
770781
"CFNResourceInChild1": {
771782
"Properties": {"Body"},
772783
},
@@ -780,16 +791,21 @@ def setUp(self):
780791
(ResourceIdentifier("CFNResource1"), "CFNResource1"),
781792
(ResourceIdentifier("CDKResource1"), "CDKResource1-x"),
782793
(ResourceIdentifier("CDKResource1-x"), "CDKResource1-x"),
794+
(ResourceIdentifier("CDKResource2"), "CDKResource2"),
783795
(ResourceIdentifier("CFNResourceInChild1"), "childStack/CFNResourceInChild1"),
784796
(ResourceIdentifier("childStack/CFNResourceInChild1"), "childStack/CFNResourceInChild1"),
785797
(ResourceIdentifier("CDKResourceInChild1"), "childStack/CDKResourceInChild1-x"),
786798
(ResourceIdentifier("CDKResourceInChild1-x"), "childStack/CDKResourceInChild1-x"),
799+
(ResourceIdentifier("CDKResourceInChild2"), "childStack/CDKResourceInChild2"),
787800
(ResourceIdentifier("childStack/CDKResourceInChild1-x"), "childStack/CDKResourceInChild1-x"),
801+
(ResourceIdentifier("childStack/CDKResourceInChild2"), "childStack/CDKResourceInChild2"),
788802
(ResourceIdentifier("InvalidResourceId"), None),
789803
(ResourceIdentifier("InvalidStackId/CFNResourceInChild1"), None),
790804
# we should use iac_resource_id to define full path, could not use resource logical id in full path although
791805
# cdk id is there
792806
(ResourceIdentifier("childStack/CDKResourceInChild1"), None),
807+
(ResourceIdentifier("CDKResource2-x"), None),
808+
(ResourceIdentifier("childStack/CDKResourceInChild2-x"), None),
793809
]
794810
)
795811
def test_get_resource_full_path_by_id(self, resource_id, expected_full_path):

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

+47
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,19 @@ class TestSamFunctionProviderEndToEnd(TestCase):
212212
"aws:asset:property": "Code",
213213
},
214214
},
215+
"LambdaCDKFuncWithInvalidCDKPath": {
216+
"Type": "AWS::Lambda::Function",
217+
"Properties": {
218+
"Code": {"Bucket": "bucket", "Key": "key"},
219+
"Runtime": "nodejs4.3",
220+
"Handler": "index.handler",
221+
},
222+
"Metadata": {
223+
"aws:cdk:path": "Stack/LambdaCFKFunction-x/Lambda/Resource",
224+
"aws:asset:path": "/usr/foo/bar",
225+
"aws:asset:property": "Code",
226+
},
227+
},
215228
"OtherResource": {
216229
"Type": "AWS::Serverless::Api",
217230
"Properties": {"StageName": "prod", "DefinitionUri": "s3://bucket/key"},
@@ -929,6 +942,39 @@ def setUp(self):
929942
function_build_info=FunctionBuildInfo.BuildableZip,
930943
),
931944
),
945+
(
946+
"LambdaCDKFuncWithInvalidCDKPath",
947+
Function(
948+
function_id="LambdaCDKFuncWithInvalidCDKPath",
949+
name="LambdaCDKFuncWithInvalidCDKPath",
950+
functionname="LambdaCDKFuncWithInvalidCDKPath",
951+
runtime="nodejs4.3",
952+
handler="index.handler",
953+
codeuri="/usr/foo/bar",
954+
memory=None,
955+
timeout=None,
956+
environment=None,
957+
rolearn=None,
958+
layers=[],
959+
events=None,
960+
metadata={
961+
"aws:cdk:path": "Stack/LambdaCFKFunction-x/Lambda/Resource",
962+
"aws:asset:path": "/usr/foo/bar",
963+
"aws:asset:property": "Code",
964+
"SamNormalized": True,
965+
"SamResourceId": "LambdaCDKFuncWithInvalidCDKPath",
966+
},
967+
inlinecode=None,
968+
imageuri=None,
969+
imageconfig=None,
970+
packagetype=ZIP,
971+
codesign_config_arn=None,
972+
architectures=None,
973+
function_url_config=None,
974+
stack_path="",
975+
function_build_info=FunctionBuildInfo.BuildableZip,
976+
),
977+
),
932978
(
933979
"LambdaCDKFuncInChild-x",
934980
Function(
@@ -1059,6 +1105,7 @@ def test_get_all_must_return_all_functions(self):
10591105
"LambdaFuncWithCodeSignConfig",
10601106
"LambdaFunctionCustomId-x",
10611107
"LambdaCFKFunction-x",
1108+
"LambdaCDKFuncWithInvalidCDKPath",
10621109
posixpath.join("ChildStack", "SamFunctionsInChild"),
10631110
posixpath.join("ChildStack", "SamFunctionsInChildAbsPath"),
10641111
posixpath.join("ChildStack", "SamImageFunctionsInChild"),

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

+35
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,23 @@ class TestSamLayerProvider(TestCase):
5656
"aws:asset:property": "Content",
5757
},
5858
},
59+
"CDKLambdaLayerWithInvalidCDKPath": {
60+
"Type": "AWS::Lambda::LayerVersion",
61+
"Properties": {
62+
"LayerName": "Layer1",
63+
"Content": {
64+
"S3Bucket": "bucket",
65+
"S3Key": "key",
66+
},
67+
"CompatibleRuntimes": ["python3.9"],
68+
},
69+
"Metadata": {
70+
"BuildMethod": "python3.9",
71+
"aws:cdk:path": "stack/CDKLambdaLayer-x/Lambda/Resource",
72+
"aws:asset:path": "PyLayer/",
73+
"aws:asset:property": "Content",
74+
},
75+
},
5976
"ServerlessLayerNoBuild": {
6077
"Type": "AWS::Serverless::LayerVersion",
6178
"Properties": {
@@ -256,6 +273,23 @@ def setUp(self):
256273
stack_path="",
257274
),
258275
),
276+
(
277+
"CDKLambdaLayerWithInvalidCDKPath",
278+
LayerVersion(
279+
"CDKLambdaLayerWithInvalidCDKPath",
280+
"PyLayer",
281+
["python3.9"],
282+
{
283+
"BuildMethod": "python3.9",
284+
"aws:cdk:path": "stack/CDKLambdaLayer-x/Lambda/Resource",
285+
"aws:asset:path": "PyLayer/",
286+
"aws:asset:property": "Content",
287+
"SamNormalized": True,
288+
"SamResourceId": "CDKLambdaLayerWithInvalidCDKPath",
289+
},
290+
stack_path="",
291+
),
292+
),
259293
(
260294
"CDKLambdaLayerInChild-x",
261295
LayerVersion(
@@ -326,6 +360,7 @@ def test_get_all_must_return_all_layers(self):
326360
"LambdaLayer",
327361
"LambdaLayerWithCustomId-x",
328362
"CDKLambdaLayer-x",
363+
"CDKLambdaLayerWithInvalidCDKPath",
329364
"ServerlessLayerNoBuild",
330365
"LambdaLayerNoBuild",
331366
posixpath.join("ChildStack", "SamLayerInChild"),

‎tests/unit/lib/samlib/test_resource_metadata_normalizer.py

+12-2
Original file line numberDiff line numberDiff line change
@@ -521,9 +521,19 @@ def test_use_cdk_id_as_resource_id(self, cdk_path, expected_resource_id):
521521

522522
self.assertEqual(expected_resource_id, resource_id)
523523

524-
def test_use_logical_id_as_resource_id_incase_of_invalid_cdk_path(self):
524+
@parameterized.expand(
525+
[
526+
("func_cdk_id"),
527+
("stack_id/construct_id/lambda/Resource"),
528+
]
529+
)
530+
def test_use_logical_id_as_resource_id_incase_of_invalid_cdk_path(self, cdk_path):
525531
resource_id = ResourceMetadataNormalizer.get_resource_id(
526-
{"Type": "any:value", "Properties": {"key": "value"}, "Metadata": {"aws:cdk:path": "func_cdk_id"}},
532+
{
533+
"Type": "any:value",
534+
"Properties": {"key": "value"},
535+
"Metadata": {"aws:cdk:path": cdk_path},
536+
},
527537
"logical_id",
528538
)
529539

0 commit comments

Comments
 (0)