Skip to content

Commit 2f5c502

Browse files
Connor Robertsonssenchenko
Connor Robertson
and
ssenchenko
authored
fix: LayerVersion cannot be the target of DependsOn due to being hashed during transformation (#3396)
Co-authored-by: Slava Senchenko <[email protected]>
1 parent 5f70f7f commit 2f5c502

File tree

7 files changed

+283
-0
lines changed

7 files changed

+283
-0
lines changed

‎samtranslator/translator/translator.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
from samtranslator.sdk.parameter import SamParameterValues
3838
from samtranslator.translator.arn_generator import ArnGenerator
3939
from samtranslator.translator.verify_logical_id import verify_unique_logical_id
40+
from samtranslator.utils.actions import ResolveDependsOn
41+
from samtranslator.utils.traverse import traverse
4042
from samtranslator.validator.value_validator import sam_expect
4143

4244

@@ -238,6 +240,8 @@ def translate( # noqa: PLR0912, PLR0915
238240
del template["Transform"]
239241

240242
if len(self.document_errors) == 0:
243+
resolveDependsOn = ResolveDependsOn(resolution_data=changed_logical_ids) # Initializes ResolveDependsOn
244+
template = traverse(template, [resolveDependsOn])
241245
template = intrinsics_resolver.resolve_sam_resource_id_refs(template, changed_logical_ids)
242246
return intrinsics_resolver.resolve_sam_resource_refs(template, supported_resource_refs)
243247
raise InvalidDocumentException(self.document_errors)

‎samtranslator/utils/actions.py

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
from abc import ABC, abstractmethod
2+
from typing import Any, Dict
3+
4+
5+
class Action(ABC):
6+
"""
7+
Base class for Resolver function actions. Each Resolver function must subclass this,
8+
override the , and provide a execute() method
9+
"""
10+
11+
@abstractmethod
12+
def execute(self, template: Dict[str, Any]) -> Dict[str, Any]:
13+
pass
14+
15+
16+
class ResolveDependsOn(Action):
17+
DependsOn = "DependsOn"
18+
19+
def __init__(self, resolution_data: Dict[str, str]):
20+
"""
21+
Initializes ResolveDependsOn. Where data necessary to resolve execute can be provided.
22+
23+
:param resolution_data: Extra data necessary to resolve execute properly.
24+
"""
25+
self.resolution_data = resolution_data
26+
27+
def execute(self, template: Dict[str, Any]) -> Dict[str, Any]:
28+
"""
29+
Resolve DependsOn when logical ids get changed when transforming (ex: AWS::Serverless::LayerVersion)
30+
31+
:param input_dict: Chunk of the template that is attempting to be resolved
32+
:param resolution_data: Dictionary of the original and changed logical ids
33+
:return: Modified dictionary with values resolved
34+
"""
35+
# Checks if input dict is resolvable
36+
if template is None or not self._can_handle_depends_on(input_dict=template):
37+
return template
38+
# Checks if DependsOn is valid
39+
if not (isinstance(template[self.DependsOn], (list, str))):
40+
return template
41+
# Check if DependsOn matches the original value of a changed_logical_id key
42+
for old_logical_id, changed_logical_id in self.resolution_data.items():
43+
# Done like this as there is no other way to know if this is a DependsOn vs some value named the
44+
# same as the old logical id. (ex LayerName is commonly the old_logical_id)
45+
if isinstance(template[self.DependsOn], list):
46+
for index, value in enumerate(template[self.DependsOn]):
47+
if value == old_logical_id:
48+
template[self.DependsOn][index] = changed_logical_id
49+
elif template[self.DependsOn] == old_logical_id:
50+
template[self.DependsOn] = changed_logical_id
51+
return template
52+
53+
def _can_handle_depends_on(self, input_dict: Dict[str, Any]) -> bool:
54+
"""
55+
Checks if the input dictionary is of length one and contains "DependsOn"
56+
57+
:param input_dict: the Dictionary that is attempting to be resolved
58+
:return boolean value of validation attempt
59+
"""
60+
return isinstance(input_dict, dict) and self.DependsOn in input_dict

‎samtranslator/utils/traverse.py

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
from typing import Any, Dict, List
2+
3+
from samtranslator.utils.actions import Action
4+
5+
6+
def traverse(
7+
input_value: Any,
8+
actions: List[Action],
9+
) -> Any:
10+
"""
11+
Driver method that performs the actual traversal of input and calls the execute method of the provided actions.
12+
13+
Traversal Algorithm:
14+
15+
Imagine the input dictionary/list as a tree. We are doing a Pre-Order tree traversal here where we first
16+
process the root node before going to its children. Dict and Lists are the only two iterable nodes.
17+
Everything else is a leaf node.
18+
19+
:param input_value: Any primitive type (dict, array, string etc) whose value might contain a changed value
20+
:param actions: Method that will be called to actually resolve the function.
21+
:return: Modified `input` with values resolved
22+
"""
23+
24+
for action in actions:
25+
action.execute(input_value)
26+
27+
if isinstance(input_value, dict):
28+
return _traverse_dict(input_value, actions)
29+
if isinstance(input_value, list):
30+
return _traverse_list(input_value, actions)
31+
# We can iterate only over dict or list types. Primitive types are terminals
32+
33+
return input_value
34+
35+
36+
def _traverse_dict(
37+
input_dict: Dict[str, Any],
38+
actions: List[Action],
39+
) -> Any:
40+
"""
41+
Traverse a dictionary to resolves changed values on every value
42+
43+
:param input_dict: Input dictionary to traverse
44+
:param actions: This is just to pass it to the template partition
45+
:return: Modified dictionary with values resolved
46+
"""
47+
for key, value in input_dict.items():
48+
input_dict[key] = traverse(value, actions)
49+
50+
return input_dict
51+
52+
53+
def _traverse_list(
54+
input_list: List[Any],
55+
actions: List[Action],
56+
) -> Any:
57+
"""
58+
Traverse a list to resolve changed values on every element
59+
60+
:param input_list: List of input
61+
:param actions: This is just to pass it to the template partition
62+
:return: Modified list with values functions resolved
63+
"""
64+
for index, value in enumerate(input_list):
65+
input_list[index] = traverse(value, actions)
66+
67+
return input_list

‎tests/translator/input/layer_version_depends_on.yaml

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
Transform: AWS::Serverless-2016-10-31
2+
Resources:
3+
Layer1:
4+
Type: AWS::Serverless::LayerVersion
5+
Properties:
6+
ContentUri:
7+
Bucket: test
8+
Key: test.zip
9+
10+
Layer2:
11+
Type: AWS::Serverless::LayerVersion
12+
DependsOn: Layer1
13+
Properties:
14+
ContentUri:
15+
Bucket: test
16+
Key: test.zip
17+
18+
Layer3:
19+
Type: AWS::Serverless::LayerVersion
20+
DependsOn:
21+
- Layer1
22+
- Layer2
23+
Properties:
24+
ContentUri:
25+
Bucket: test
26+
Key: test.zip

‎tests/translator/output/aws-cn/layer_version_depends_on.json

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
{
2+
"Resources": {
3+
"Layer1d45b36fd2d": {
4+
"DeletionPolicy": "Retain",
5+
"Properties": {
6+
"Content": {
7+
"S3Bucket": "test",
8+
"S3Key": "test.zip"
9+
},
10+
"LayerName": "Layer1"
11+
},
12+
"Type": "AWS::Lambda::LayerVersion"
13+
},
14+
"Layer25093239808": {
15+
"DeletionPolicy": "Retain",
16+
"DependsOn": "Layer1d45b36fd2d",
17+
"Properties": {
18+
"Content": {
19+
"S3Bucket": "test",
20+
"S3Key": "test.zip"
21+
},
22+
"LayerName": "Layer2"
23+
},
24+
"Type": "AWS::Lambda::LayerVersion"
25+
},
26+
"Layer34d7f81220c": {
27+
"DeletionPolicy": "Retain",
28+
"DependsOn": [
29+
"Layer1d45b36fd2d",
30+
"Layer25093239808"
31+
],
32+
"Properties": {
33+
"Content": {
34+
"S3Bucket": "test",
35+
"S3Key": "test.zip"
36+
},
37+
"LayerName": "Layer3"
38+
},
39+
"Type": "AWS::Lambda::LayerVersion"
40+
}
41+
}
42+
}

‎tests/translator/output/aws-us-gov/layer_version_depends_on.json

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
{
2+
"Resources": {
3+
"Layer1d45b36fd2d": {
4+
"DeletionPolicy": "Retain",
5+
"Properties": {
6+
"Content": {
7+
"S3Bucket": "test",
8+
"S3Key": "test.zip"
9+
},
10+
"LayerName": "Layer1"
11+
},
12+
"Type": "AWS::Lambda::LayerVersion"
13+
},
14+
"Layer25093239808": {
15+
"DeletionPolicy": "Retain",
16+
"DependsOn": "Layer1d45b36fd2d",
17+
"Properties": {
18+
"Content": {
19+
"S3Bucket": "test",
20+
"S3Key": "test.zip"
21+
},
22+
"LayerName": "Layer2"
23+
},
24+
"Type": "AWS::Lambda::LayerVersion"
25+
},
26+
"Layer34d7f81220c": {
27+
"DeletionPolicy": "Retain",
28+
"DependsOn": [
29+
"Layer1d45b36fd2d",
30+
"Layer25093239808"
31+
],
32+
"Properties": {
33+
"Content": {
34+
"S3Bucket": "test",
35+
"S3Key": "test.zip"
36+
},
37+
"LayerName": "Layer3"
38+
},
39+
"Type": "AWS::Lambda::LayerVersion"
40+
}
41+
}
42+
}

‎tests/translator/output/layer_version_depends_on.json

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
{
2+
"Resources": {
3+
"Layer1d45b36fd2d": {
4+
"DeletionPolicy": "Retain",
5+
"Properties": {
6+
"Content": {
7+
"S3Bucket": "test",
8+
"S3Key": "test.zip"
9+
},
10+
"LayerName": "Layer1"
11+
},
12+
"Type": "AWS::Lambda::LayerVersion"
13+
},
14+
"Layer25093239808": {
15+
"DeletionPolicy": "Retain",
16+
"DependsOn": "Layer1d45b36fd2d",
17+
"Properties": {
18+
"Content": {
19+
"S3Bucket": "test",
20+
"S3Key": "test.zip"
21+
},
22+
"LayerName": "Layer2"
23+
},
24+
"Type": "AWS::Lambda::LayerVersion"
25+
},
26+
"Layer34d7f81220c": {
27+
"DeletionPolicy": "Retain",
28+
"DependsOn": [
29+
"Layer1d45b36fd2d",
30+
"Layer25093239808"
31+
],
32+
"Properties": {
33+
"Content": {
34+
"S3Bucket": "test",
35+
"S3Key": "test.zip"
36+
},
37+
"LayerName": "Layer3"
38+
},
39+
"Type": "AWS::Lambda::LayerVersion"
40+
}
41+
}
42+
}

0 commit comments

Comments
 (0)