Skip to content

fix: Prevent duplicated cdk-defined resource ids #7931

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion samcli/lib/samlib/resource_metadata_normalizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/commands/local/lib/test_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
Expand All @@ -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"},
},
Expand All @@ -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):
Expand Down
47 changes: 47 additions & 0 deletions tests/unit/commands/local/lib/test_sam_function_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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"),
Expand Down
35 changes: 35 additions & 0 deletions tests/unit/commands/local/lib/test_sam_layer_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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"),
Expand Down
14 changes: 12 additions & 2 deletions tests/unit/lib/samlib/test_resource_metadata_normalizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)

Expand Down