diff --git a/samtranslator/plugins/globals/globals.py b/samtranslator/plugins/globals/globals.py index bce449604e..a0d6cb6f0c 100644 --- a/samtranslator/plugins/globals/globals.py +++ b/samtranslator/plugins/globals/globals.py @@ -321,6 +321,11 @@ class GlobalProperties: Object holding the global properties of given type. It also contains methods to perform a merge between Global & resource-level properties. Here are the different cases during the merge and how we handle them: + **List Override Properties** + Some list properties only accept a single value (e.g. Architectures — Lambda supports exactly one + architecture). For these, a resource-level list replaces the global list entirely rather than being + concatenated with it. + **Primitive Type (String, Integer, Boolean etc)** If either global & local are of primitive types, then we the value at local will overwrite global. @@ -436,6 +441,9 @@ class GlobalProperties: """ + # List properties where the resource-level value replaces the global value instead of being concatenated. + _list_override_properties: frozenset[str] = frozenset({"Architectures"}) + def __init__(self, global_properties) -> None: # type: ignore[no-untyped-def] self.global_properties = global_properties @@ -500,8 +508,12 @@ def _merge_dict(self, global_dict, local_dict): # type: ignore[no-untyped-def] for key in local_dict: if key in global_dict: - # Both local & global contains the same key. Let's do a merge. - global_dict[key] = self._do_merge(global_dict[key], local_dict[key]) # type: ignore[no-untyped-call] + if key in self._list_override_properties: + # Resource-level value wins outright; do not concatenate. + global_dict[key] = local_dict[key] + else: + # Both local & global contains the same key. Let's do a merge. + global_dict[key] = self._do_merge(global_dict[key], local_dict[key]) # type: ignore[no-untyped-call] else: # Key is not in globals, just in local. Copy it over diff --git a/samtranslator/translator/verify_logical_id.py b/samtranslator/translator/verify_logical_id.py index 89177ea241..b035983e47 100644 --- a/samtranslator/translator/verify_logical_id.py +++ b/samtranslator/translator/verify_logical_id.py @@ -2,24 +2,25 @@ from samtranslator.model import Resource -do_not_verify = { +# Values are always lists so that `in` performs exact element matching, not substring matching. +do_not_verify: dict[str, list[str]] = { # type_after_transform: type_before_transform - "AWS::Lambda::Function": "AWS::Serverless::Function", - "AWS::Lambda::LayerVersion": "AWS::Serverless::LayerVersion", - "AWS::Lambda::CapacityProvider": "AWS::Serverless::CapacityProvider", - "AWS::ApiGateway::RestApi": "AWS::Serverless::Api", + "AWS::Lambda::Function": ["AWS::Serverless::Function"], + "AWS::Lambda::LayerVersion": ["AWS::Serverless::LayerVersion"], + "AWS::Lambda::CapacityProvider": ["AWS::Serverless::CapacityProvider"], + "AWS::ApiGateway::RestApi": ["AWS::Serverless::Api"], "AWS::ApiGatewayV2::Api": ["AWS::Serverless::HttpApi", "AWS::Serverless::WebSocketApi"], - "AWS::S3::Bucket": "AWS::S3::Bucket", - "AWS::SNS::Topic": "AWS::SNS::Topic", - "AWS::DynamoDB::Table": "AWS::Serverless::SimpleTable", - "AWS::CloudFormation::Stack": "AWS::Serverless::Application", - "AWS::Cognito::UserPool": "AWS::Cognito::UserPool", - "AWS::ApiGateway::DomainName": "AWS::ApiGateway::DomainName", - "AWS::ApiGateway::BasePathMapping": "AWS::ApiGateway::BasePathMapping", - "AWS::ApiGateway::DomainNameV2": "AWS::ApiGateway::DomainNameV2", - "AWS::ApiGateway::BasePathMappingV2": "AWS::ApiGateway::BasePathMappingV2", - "AWS::StepFunctions::StateMachine": "AWS::Serverless::StateMachine", - "AWS::AppSync::GraphQLApi": "AWS::Serverless::GraphQLApi", + "AWS::S3::Bucket": ["AWS::S3::Bucket"], + "AWS::SNS::Topic": ["AWS::SNS::Topic"], + "AWS::DynamoDB::Table": ["AWS::Serverless::SimpleTable"], + "AWS::CloudFormation::Stack": ["AWS::Serverless::Application"], + "AWS::Cognito::UserPool": ["AWS::Cognito::UserPool"], + "AWS::ApiGateway::DomainName": ["AWS::ApiGateway::DomainName"], + "AWS::ApiGateway::BasePathMapping": ["AWS::ApiGateway::BasePathMapping"], + "AWS::ApiGateway::DomainNameV2": ["AWS::ApiGateway::DomainNameV2"], + "AWS::ApiGateway::BasePathMappingV2": ["AWS::ApiGateway::BasePathMappingV2"], + "AWS::StepFunctions::StateMachine": ["AWS::Serverless::StateMachine"], + "AWS::AppSync::GraphQLApi": ["AWS::Serverless::GraphQLApi"], } diff --git a/tests/plugins/globals/test_globals.py b/tests/plugins/globals/test_globals.py index 9178244668..4e09b1088f 100644 --- a/tests/plugins/globals/test_globals.py +++ b/tests/plugins/globals/test_globals.py @@ -171,6 +171,19 @@ class GlobalPropertiesTestCases: mixed_type_inputs_must_be_handled = {"global": {"a": "b"}, "local": [1, 2, 3], "expected_output": [1, 2, 3]} + # Architectures must override, not concatenate — Lambda only supports one architecture value. + architectures_in_local_must_override_global = { + "global": {"Architectures": ["x86_64"], "Runtime": "python3.12"}, + "local": {"Architectures": ["arm64"]}, + "expected_output": {"Architectures": ["arm64"], "Runtime": "python3.12"}, + } + + architectures_only_in_global_must_be_inherited = { + "global": {"Architectures": ["x86_64"], "Runtime": "python3.12"}, + "local": {"Runtime": "python3.14"}, + "expected_output": {"Architectures": ["x86_64"], "Runtime": "python3.14"}, + } + class TestGlobalPropertiesMerge(TestCase): # Get all attributes of the test case object which is not a built-in method like __str__ diff --git a/tests/translator/test_verify_logical_id.py b/tests/translator/test_verify_logical_id.py new file mode 100644 index 0000000000..cc4f633336 --- /dev/null +++ b/tests/translator/test_verify_logical_id.py @@ -0,0 +1,68 @@ +from unittest import TestCase +from unittest.mock import MagicMock + +from samtranslator.translator.verify_logical_id import do_not_verify, verify_unique_logical_id + + +def _make_resource(logical_id, resource_type): + r = MagicMock() + r.logical_id = logical_id + r.resource_type = resource_type + return r + + +class TestDoNotVerifyDict(TestCase): + def test_all_values_are_lists(self): + for key, value in do_not_verify.items(): + self.assertIsInstance(value, list, f"do_not_verify['{key}'] must be a list, got {type(value)}") + + def test_no_substring_bypass(self): + # A crafted type that is a substring of a real allowed type must NOT match. + # Before the fix, "AWS::Serverless::Fun" in "AWS::Serverless::Function" was True. + resource = _make_resource("MyFunc", "AWS::Lambda::Function") + existing = {"MyFunc": {"Type": "AWS::Serverless::Fun"}} # substring, not exact + + result = verify_unique_logical_id(resource, existing) + self.assertFalse(result, "Substring of an allowed type must not bypass the uniqueness check") + + def test_no_superstring_bypass(self): + # A type that contains an allowed type as a substring must NOT match. + resource = _make_resource("MyFunc", "AWS::Lambda::Function") + existing = {"MyFunc": {"Type": "AWS::Serverless::FunctionExtra"}} + + result = verify_unique_logical_id(resource, existing) + self.assertFalse(result, "Superstring of an allowed type must not bypass the uniqueness check") + + +class TestVerifyUniqueLogicalId(TestCase): + def test_new_logical_id_is_unique(self): + resource = _make_resource("NewFunc", "AWS::Lambda::Function") + existing = {} + self.assertTrue(verify_unique_logical_id(resource, existing)) + + def test_none_logical_id_is_unique(self): + resource = _make_resource(None, "AWS::Lambda::Function") + self.assertTrue(verify_unique_logical_id(resource, {})) + + def test_allowed_transform_returns_true(self): + resource = _make_resource("MyFunc", "AWS::Lambda::Function") + existing = {"MyFunc": {"Type": "AWS::Serverless::Function"}} + self.assertTrue(verify_unique_logical_id(resource, existing)) + + def test_allowed_transform_multi_value_returns_true(self): + resource = _make_resource("MyApi", "AWS::ApiGatewayV2::Api") + existing = {"MyApi": {"Type": "AWS::Serverless::HttpApi"}} + self.assertTrue(verify_unique_logical_id(resource, existing)) + + existing2 = {"MyApi": {"Type": "AWS::Serverless::WebSocketApi"}} + self.assertTrue(verify_unique_logical_id(resource, existing2)) + + def test_disallowed_type_collision_returns_false(self): + resource = _make_resource("MyBucket", "AWS::Lambda::Function") + existing = {"MyBucket": {"Type": "AWS::S3::Bucket"}} + self.assertFalse(verify_unique_logical_id(resource, existing)) + + def test_unknown_resource_type_returns_false(self): + resource = _make_resource("SomeId", "AWS::Custom::Unknown") + existing = {"SomeId": {"Type": "AWS::Custom::Something"}} + self.assertFalse(verify_unique_logical_id(resource, existing))