-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
CFn executor v2: provide previous payload correctly #12511
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
Conversation
e23ed84
to
b279404
Compare
LocalStack Community integration with Pro 2 files ± 0 2 suites ±0 20m 32s ⏱️ - 1h 31m 29s Results for commit 67d8ad8. ± Comparison against base commit 2ec3574. This pull request removes 3897 and adds 1 tests. Note that renamed tests count towards both.
This pull request removes 211 skipped tests and adds 1 skipped test. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
S3 Image Test Results (AMD64 / ARM64) 2 files 2 suites 8m 33s ⏱️ Results for commit 3a6780b. |
3a6780b
to
d072285
Compare
05fbb4f
to
e32c9d6
Compare
# Conflicts: # tests/aws/services/cloudformation/api/test_changesets.py diff --git c/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py i/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py index c341c76..503ae06 100644 --- c/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py +++ i/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py @@ -60,6 +60,8 @@ class ChangeSetModelExecutor(ChangeSetModelPreproc): return delta def _reduce_intrinsic_function_ref_value(self, preproc_value: Any) -> Any: + if preproc_value is None: + return None resource = self.resources.get(preproc_value.name) if resource is None: raise NotImplementedError(f"No resource '{preproc_value.name}' found") diff --git c/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py i/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py index bc3e6ce..5addbe4 100644 --- c/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py +++ i/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py @@ -173,20 +173,20 @@ class ChangeSetModelPreproc(ChangeSetModelVisitor): return condition return None - def _resolve_reference(self, logica_id: str) -> PreprocEntityDelta: - node_condition = self._get_node_condition_if_exists(condition_name=logica_id) + def _resolve_reference(self, logical_id: str) -> PreprocEntityDelta: + node_condition = self._get_node_condition_if_exists(condition_name=logical_id) if isinstance(node_condition, NodeCondition): condition_delta = self.visit(node_condition) return condition_delta - node_parameter = self._get_node_parameter_if_exists(parameter_name=logica_id) + node_parameter = self._get_node_parameter_if_exists(parameter_name=logical_id) if isinstance(node_parameter, NodeParameter): parameter_delta = self.visit(node_parameter) return parameter_delta # TODO: check for KNOWN AFTER APPLY values for logical ids coming from intrinsic functions as arguments. node_resource = self._get_node_resource_for( - resource_name=logica_id, node_template=self._node_template + resource_name=logical_id, node_template=self._node_template ) resource_delta = self.visit(node_resource) before = resource_delta.before @@ -210,11 +210,11 @@ class ChangeSetModelPreproc(ChangeSetModelVisitor): ) -> PreprocEntityDelta: before = None if before_logical_id is not None: - before_delta = self._resolve_reference(logica_id=before_logical_id) + before_delta = self._resolve_reference(logical_id=before_logical_id) before = before_delta.before after = None if after_logical_id is not None: - after_delta = self._resolve_reference(logica_id=after_logical_id) + after_delta = self._resolve_reference(logical_id=after_logical_id) after = after_delta.after return PreprocEntityDelta(before=before, after=after) @@ -335,7 +335,7 @@ class ChangeSetModelPreproc(ChangeSetModelVisitor): def _compute_delta_for_if_statement(args: list[Any]) -> PreprocEntityDelta: condition_name = args[0] - boolean_expression_delta = self._resolve_reference(logica_id=condition_name) + boolean_expression_delta = self._resolve_reference(logical_id=condition_name) return PreprocEntityDelta( before=args[1] if boolean_expression_delta.before else args[2], after=args[1] if boolean_expression_delta.after else args[2], @@ -420,16 +420,20 @@ class ChangeSetModelPreproc(ChangeSetModelVisitor): before_logical_id = arguments_delta.before before = None if before_logical_id is not None: - before_delta = self._resolve_reference(logica_id=before_logical_id) + before_delta = self._resolve_reference(logical_id=before_logical_id) before_value = before_delta.before before = self._reduce_intrinsic_function_ref_value(before_value) after_logical_id = arguments_delta.after after = None if after_logical_id is not None: - after_delta = self._resolve_reference(logica_id=after_logical_id) + after_delta = self._resolve_reference(logical_id=after_logical_id) after_value = after_delta.after - after = self._reduce_intrinsic_function_ref_value(after_value) + # TODO: swap isinstance to be a structured type check + if isinstance(after_value, str): + after = after_value + else: + after = self._reduce_intrinsic_function_ref_value(after_value) return PreprocEntityDelta(before=before, after=after) diff --git c/tests/aws/services/cloudformation/api/test_changesets.py i/tests/aws/services/cloudformation/api/test_changesets.py index 6ad8608..a2f002f 100644 --- c/tests/aws/services/cloudformation/api/test_changesets.py +++ i/tests/aws/services/cloudformation/api/test_changesets.py @@ -105,7 +105,6 @@ class TestUpdates: res.destroy() @markers.aws.needs_fixing - @pytest.mark.skip(reason="WIP") def test_deleting_resource(self, aws_client: ServiceLevelClientFactory, deploy_cfn_template): parameter_name = "my-parameter" value1 = "foo"
e32c9d6
to
67d8ad8
Compare
Motivation
There are a few limitations with the POC executor as it stands. It does not correctly propagate the previous state of the resource to the resource provider. This means that the resource provider is not aware of the previous state, even on update which is incorrect.
In addition the executor does not currently understand or propagate the template parameter values (as specified alongside the template) so the describer and executor cannot correctly determine if resources need to be updated because of parameter changes.
Unfortunately the scope of this PR expanded quite a bit to include provider cleanup (for v2) and simplification of the internal representation of stacks and change sets. This is because the old code was not designed to be used in a provider context and was very tightly coupled to the old executor. I have also added more robustness to the resource provider deploy process, since I was finding it difficult to debug deployment failures without it.
The bottom line is:
tests/aws/services/cloudformation/api/test_changesets.py::TestUpdates::test_deleting_resource
) fails with the old provider, but passes with the new 🎉Changes
StackMetadata
=>CreateChangeSetInput
)get_resource_type
since it's not overridden in pro any moreStack
andChangeSet
types_reduce_intrinsic_function_ref_value
therefore supportingRef
lookups from resourcesstacks_v2
property in the store