Skip to content

Commit 377b1a7

Browse files
MEPalmasimonrw
andauthored
CloudFormation: Update Graph Preprocessor (#12447)
Co-authored-by: Simon Walker <simon.walker@localstack.cloud>
1 parent 807b69d commit 377b1a7

24 files changed

+10783
-1720
lines changed

.github/workflows/marker-report.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ jobs:
6060
- name: Collect marker report
6161
if: ${{ !inputs.createIssue }}
6262
env:
63-
PYTEST_ADDOPTS: "-p no:localstack.testing.pytest.fixtures -p no:localstack_snapshot.pytest.snapshot -p no:localstack.testing.pytest.filters -p no:localstack.testing.pytest.fixture_conflicts -p no:tests.fixtures -p no:localstack.testing.pytest.stepfunctions.fixtures -s --co --disable-warnings --marker-report --marker-report-tinybird-upload"
63+
PYTEST_ADDOPTS: "-p no:localstack.testing.pytest.fixtures -p no:localstack_snapshot.pytest.snapshot -p no:localstack.testing.pytest.filters -p no:localstack.testing.pytest.fixture_conflicts -p no:tests.fixtures -p no:localstack.testing.pytest.stepfunctions.fixtures -p no:localstack.testing.pytest.cloudformation.fixtures -s --co --disable-warnings --marker-report --marker-report-tinybird-upload"
6464
MARKER_REPORT_PROJECT_NAME: localstack
6565
MARKER_REPORT_TINYBIRD_TOKEN: ${{ secrets.MARKER_REPORT_TINYBIRD_TOKEN }}
6666
MARKER_REPORT_COMMIT_SHA: ${{ github.sha }}
@@ -71,7 +71,7 @@ jobs:
7171
# makes use of the marker report plugin localstack.testing.pytest.marker_report
7272
- name: Generate marker report
7373
env:
74-
PYTEST_ADDOPTS: "-p no:localstack.testing.pytest.fixtures -p no:localstack_snapshot.pytest.snapshot -p no:localstack.testing.pytest.filters -p no:localstack.testing.pytest.fixture_conflicts -p no:tests.fixtures -p no:localstack.testing.pytest.stepfunctions.fixtures -s --co --disable-warnings --marker-report --marker-report-path './target'"
74+
PYTEST_ADDOPTS: "-p no:localstack.testing.pytest.fixtures -p no:localstack_snapshot.pytest.snapshot -p no:localstack.testing.pytest.filters -p no:localstack.testing.pytest.fixture_conflicts -p no:tests.fixtures -p no:localstack.testing.pytest.stepfunctions.fixtures -p no:localstack.testing.pytest.cloudformation.fixtures -p no: -s --co --disable-warnings --marker-report --marker-report-path './target'"
7575
MARKER_REPORT_PROJECT_NAME: localstack
7676
MARKER_REPORT_COMMIT_SHA: ${{ github.sha }}
7777
run: |

.github/workflows/tests-cli.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ jobs:
9898
pip install pytest pytest-tinybird
9999
- name: Run CLI tests
100100
env:
101-
PYTEST_ADDOPTS: "${{ env.TINYBIRD_PYTEST_ARGS }}-p no:localstack.testing.pytest.fixtures -p no:localstack_snapshot.pytest.snapshot -p no:localstack.testing.pytest.filters -p no:localstack.testing.pytest.fixture_conflicts -p no:localstack.testing.pytest.validation_tracking -p no:localstack.testing.pytest.path_filter -p no:tests.fixtures -p no:localstack.testing.pytest.stepfunctions.fixtures -s"
101+
PYTEST_ADDOPTS: "${{ env.TINYBIRD_PYTEST_ARGS }}-p no:localstack.testing.pytest.fixtures -p no:localstack_snapshot.pytest.snapshot -p no:localstack.testing.pytest.filters -p no:localstack.testing.pytest.fixture_conflicts -p no:localstack.testing.pytest.validation_tracking -p no:localstack.testing.pytest.path_filter -p no:tests.fixtures -p no:localstack.testing.pytest.stepfunctions.fixtures -p no:localstack.testing.pytest.cloudformation.fixtures -s"
102102
TEST_PATH: "tests/cli/"
103103
run: make test
104104

localstack-core/localstack/services/cloudformation/engine/entities.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -422,12 +422,17 @@ def changes(self):
422422
return result
423423

424424
# V2 only
425-
def populate_update_graph(self, before_template: dict | None, after_template: dict | None):
425+
def populate_update_graph(
426+
self,
427+
before_template: Optional[dict],
428+
after_template: Optional[dict],
429+
before_parameters: Optional[dict],
430+
after_parameters: Optional[dict],
431+
) -> None:
426432
change_set_model = ChangeSetModel(
427433
before_template=before_template,
428434
after_template=after_template,
429-
# TODO
430-
before_parameters=None,
431-
after_parameters=None,
435+
before_parameters=before_parameters,
436+
after_parameters=after_parameters,
432437
)
433438
self.update_graph = change_set_model.get_update_model()

localstack-core/localstack/services/cloudformation/engine/v2/change_set_model.py

Lines changed: 32 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
from typing_extensions import TypeVar
99

10-
from localstack.aws.api.cloudformation import ChangeAction
1110
from localstack.utils.strings import camel_to_snake_case
1211

1312
T = TypeVar("T")
@@ -67,15 +66,6 @@ class ChangeType(enum.Enum):
6766
def __str__(self):
6867
return self.value
6968

70-
def to_action(self) -> ChangeAction | None:
71-
match self:
72-
case self.CREATED:
73-
return ChangeAction.Add
74-
case self.MODIFIED:
75-
return ChangeAction.Modify
76-
case self.REMOVED:
77-
return ChangeAction.Remove
78-
7969
def for_child(self, child_change_type: ChangeType) -> ChangeType:
8070
if child_change_type == self:
8171
return self
@@ -525,7 +515,7 @@ def _resolve_intrinsic_function_ref(self, arguments: ChangeSetEntity) -> ChangeT
525515

526516
node_parameter = self._retrieve_parameter_if_exists(parameter_name=logical_id)
527517
if isinstance(node_parameter, NodeParameter):
528-
return node_parameter.dynamic_value.change_type
518+
return node_parameter.change_type
529519

530520
# TODO: this should check the replacement flag for a resource update.
531521
node_resource = self._retrieve_or_visit_resource(resource_name=logical_id)
@@ -584,19 +574,16 @@ def _resolve_intrinsic_function_fn_if(self, arguments: ChangeSetEntity) -> Chang
584574
def _visit_array(
585575
self, scope: Scope, before_array: Maybe[list], after_array: Maybe[list]
586576
) -> NodeArray:
587-
change_type = ChangeType.UNCHANGED
588577
array: list[ChangeSetEntity] = list()
589578
for index, (before_value, after_value) in enumerate(
590579
zip_longest(before_array, after_array, fillvalue=Nothing)
591580
):
592-
# TODO: should extract this scoping logic.
593581
value_scope = scope.open_index(index=index)
594582
value = self._visit_value(
595583
scope=value_scope, before_value=before_value, after_value=after_value
596584
)
597585
array.append(value)
598-
if value.change_type != ChangeType.UNCHANGED:
599-
change_type = ChangeType.MODIFIED
586+
change_type = self._change_type_for_parent_of([value.change_type for value in array])
600587
return NodeArray(scope=scope, change_type=change_type, array=array)
601588

602589
def _visit_object(
@@ -695,28 +682,12 @@ def _visit_property(
695682
node_property = self._visited_scopes.get(scope)
696683
if isinstance(node_property, NodeProperty):
697684
return node_property
698-
699685
value = self._visit_value(
700686
scope=scope, before_value=before_property, after_value=after_property
701687
)
702-
if self._is_created(before=before_property, after=after_property):
703-
node_property = NodeProperty(
704-
scope=scope,
705-
change_type=ChangeType.CREATED,
706-
name=property_name,
707-
value=value,
708-
)
709-
elif self._is_removed(before=before_property, after=after_property):
710-
node_property = NodeProperty(
711-
scope=scope,
712-
change_type=ChangeType.REMOVED,
713-
name=property_name,
714-
value=value,
715-
)
716-
else:
717-
node_property = NodeProperty(
718-
scope=scope, change_type=value.change_type, name=property_name, value=value
719-
)
688+
node_property = NodeProperty(
689+
scope=scope, change_type=value.change_type, name=property_name, value=value
690+
)
720691
self._visited_scopes[scope] = node_property
721692
return node_property
722693

@@ -748,6 +719,13 @@ def _visit_properties(
748719
self._visited_scopes[scope] = node_properties
749720
return node_properties
750721

722+
def _visit_type(self, scope: Scope, before_type: Any, after_type: Any) -> TerminalValue:
723+
value = self._visit_value(scope=scope, before_value=before_type, after_value=after_type)
724+
if not isinstance(value, TerminalValue):
725+
# TODO: decide where template schema validation should occur.
726+
raise RuntimeError()
727+
return value
728+
751729
def _visit_resource(
752730
self,
753731
scope: Scope,
@@ -766,8 +744,12 @@ def _visit_resource(
766744
else:
767745
change_type = ChangeType.UNCHANGED
768746

769-
# TODO: investigate behaviour with type changes, for now this is filler code.
770-
_, type_str = self._safe_access_in(scope, TypeKey, after_resource)
747+
scope_type, (before_type, after_type) = self._safe_access_in(
748+
scope, TypeKey, before_resource, after_resource
749+
)
750+
terminal_value_type = self._visit_type(
751+
scope=scope_type, before_type=before_type, after_type=after_type
752+
)
771753

772754
condition_reference = None
773755
scope_condition, (before_condition, after_condition) = self._safe_access_in(
@@ -792,7 +774,7 @@ def _visit_resource(
792774
scope=scope,
793775
change_type=change_type,
794776
name=resource_name,
795-
type_=TerminalValueUnchanged(scope=scope, value=type_str),
777+
type_=terminal_value_type,
796778
condition_reference=condition_reference,
797779
properties=properties,
798780
)
@@ -872,36 +854,19 @@ def _visit_parameter(
872854
return node_parameter
873855
# TODO: add logic to compute defaults already in the graph building process?
874856
dynamic_value = self._visit_dynamic_parameter(parameter_name=parameter_name)
875-
if self._is_created(before=before_parameter, after=after_parameter):
876-
node_parameter = NodeParameter(
877-
scope=scope,
878-
change_type=ChangeType.CREATED,
879-
name=parameter_name,
880-
value=TerminalValueCreated(scope=scope, value=after_parameter),
881-
dynamic_value=dynamic_value,
882-
)
883-
elif self._is_removed(before=before_parameter, after=after_parameter):
884-
node_parameter = NodeParameter(
885-
scope=scope,
886-
change_type=ChangeType.REMOVED,
887-
name=parameter_name,
888-
value=TerminalValueRemoved(scope=scope, value=before_parameter),
889-
dynamic_value=dynamic_value,
890-
)
891-
else:
892-
value = self._visit_value(
893-
scope=scope, before_value=before_parameter, after_value=after_parameter
894-
)
895-
change_type = self._change_type_for_parent_of(
896-
change_types=[dynamic_value.change_type, value.change_type]
897-
)
898-
node_parameter = NodeParameter(
899-
scope=scope,
900-
change_type=change_type,
901-
name=parameter_name,
902-
value=value,
903-
dynamic_value=dynamic_value,
904-
)
857+
value = self._visit_value(
858+
scope=scope, before_value=before_parameter, after_value=after_parameter
859+
)
860+
change_type = self._change_type_for_parent_of(
861+
change_types=[dynamic_value.change_type, value.change_type]
862+
)
863+
node_parameter = NodeParameter(
864+
scope=scope,
865+
change_type=change_type,
866+
name=parameter_name,
867+
value=value,
868+
dynamic_value=dynamic_value,
869+
)
905870
self._visited_scopes[scope] = node_parameter
906871
return node_parameter
907872

0 commit comments

Comments
 (0)