Skip to content
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
27 changes: 24 additions & 3 deletions samtranslator/plugins/globals/globals.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,8 +306,12 @@ def _parse(self, globals_dict): # type: ignore[no-untyped-def]
f"Must be one of the following values - {supported_displayed}",
)

override_properties = []
if resource_type == SamResourceType.Function.value:
override_properties = ["Architectures"]

# Store all Global properties in a map with key being the AWS::Serverless::* resource type
_globals[resource_type] = GlobalProperties(properties)
_globals[resource_type] = GlobalProperties(properties, override_properties)

return _globals

Expand Down Expand Up @@ -433,18 +437,35 @@ class GlobalProperties:
```
(in other words, Deployments will be turned off for the Function)

**Overridable Properties**
Some top-level resource properties must replace the global value instead of following the default merge behavior.
For example, Function Architectures is a list, but Lambda accepts only one architecture value. If both global and
local Architectures are set, the local value must override the global value instead of concatenating lists.

"""

def __init__(self, global_properties) -> None: # type: ignore[no-untyped-def]
def __init__(self, global_properties, override_properties=None) -> None: # type: ignore[no-untyped-def]
self.global_properties = global_properties
self.override_properties = override_properties or []

def merge(self, local_properties): # type: ignore[no-untyped-def]
"""
Merge Global & local level properties according to the above rules

:return local_properties: Dictionary of local properties
"""
return self._do_merge(self.global_properties, local_properties) # type: ignore[no-untyped-call]
global_properties = self._drop_overridden_globals(self.global_properties, local_properties)
return self._do_merge(global_properties, local_properties) # type: ignore[no-untyped-call]

def _drop_overridden_globals(self, global_properties, local_properties): # type: ignore[no-untyped-def]
if not self.override_properties or not isinstance(global_properties, dict) or not isinstance(local_properties, dict):
return global_properties

global_properties = global_properties.copy()
for key in self.override_properties:
if key in global_properties and key in local_properties:
del global_properties[key]
return global_properties
Comment on lines +457 to +468

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this approach makes sense and would work, but I my first instinct is that we could get rid of the loop/copy by checking the override_properties in _merge_dict. I left another comment in the place where that might work, but I could be missing something, let me know what you think.


def _do_merge(self, global_value, local_value): # type: ignore[no-untyped-def]
"""
Expand Down
30 changes: 29 additions & 1 deletion tests/plugins/globals/test_globals.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,34 @@ class GlobalPropertiesTestCases:

mixed_type_inputs_must_be_handled = {"global": {"a": "b"}, "local": [1, 2, 3], "expected_output": [1, 2, 3]}

architectures_in_local_must_override_global_instead_of_concatenating = {
"global": {"Architectures": ["x86_64"]},
"local": {"Architectures": ["arm64"]},
"override_properties": ["Architectures"],
"expected_output": {"Architectures": ["arm64"]},
}

architectures_in_global_must_be_used_when_local_does_not_set_it = {
"global": {"Architectures": ["arm64"], "Runtime": "python3.12"},
"local": {"Runtime": "nodejs20.x"},
"override_properties": ["Architectures"],
"expected_output": {"Architectures": ["arm64"], "Runtime": "nodejs20.x"},
}

override_property_in_local_only_must_not_error = {
"global": {"Runtime": "python3.12"},
"local": {"Architectures": ["arm64"]},
"override_properties": ["Architectures"],
"expected_output": {"Runtime": "python3.12", "Architectures": ["arm64"]},
}

override_properties_only_apply_at_root = {
"global": {"Nested": {"Architectures": ["x86_64"]}},
"local": {"Nested": {"Architectures": ["arm64"]}},
"override_properties": ["Architectures"],
"expected_output": {"Nested": {"Architectures": ["x86_64", "arm64"]}},
}


class TestGlobalPropertiesMerge(TestCase):
# Get all attributes of the test case object which is not a built-in method like __str__
Expand All @@ -180,7 +208,7 @@ def test_global_properties_merge(self, testcase):
if not configuration:
raise Exception("Invalid configuration for test case " + testcase)

global_properties = GlobalProperties(configuration["global"])
global_properties = GlobalProperties(configuration["global"], configuration.get("override_properties"))
actual = global_properties.merge(configuration["local"])

self.assertEqual(actual, configuration["expected_output"])
Expand Down
2 changes: 2 additions & 0 deletions tests/translator/input/globals_for_function.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ Resources:
PermissionsBoundary: arn:aws:1234:iam:boundary/OverridePermissionsBoundary
Layers:
- !Sub arn:${AWS::Partition}:lambda:${AWS::Region}:${AWS::AccountId}:layer:MyLayer2:2
Architectures:
- arm64
SnapStart:
ApplyOn: None
ReservedConcurrentExecutions: 100
Expand Down
2 changes: 1 addition & 1 deletion tests/translator/output/aws-cn/globals_for_function.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"FunctionWithOverrides": {
"Properties": {
"Architectures": [
"x86_64"
"arm64"
],
"Code": {
"S3Bucket": "sam-demo-bucket",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"FunctionWithOverrides": {
"Properties": {
"Architectures": [
"x86_64"
"arm64"
],
"Code": {
"S3Bucket": "sam-demo-bucket",
Expand Down
2 changes: 1 addition & 1 deletion tests/translator/output/globals_for_function.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"FunctionWithOverrides": {
"Properties": {
"Architectures": [
"x86_64"
"arm64"
],
"Code": {
"S3Bucket": "sam-demo-bucket",
Expand Down
Loading