Skip to content

Commit 34049aa

Browse files
simonrwsimonrw
authored
CFNv2: minor parity assessment and triaging (#12871)
1 parent 1775743 commit 34049aa

File tree

22 files changed

+155
-137
lines changed

22 files changed

+155
-137
lines changed

‎.github/workflows/aws-tests.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ jobs:
394394
env:
395395
# add the GitHub API token to avoid rate limit issues
396396
GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }}
397-
PYTEST_ARGS: "${{ env.TINYBIRD_PYTEST_ARGS }}${{ env.TESTSELECTION_PYTEST_ARGS }} --splits 4 --group ${{ matrix.group }} --store-durations --clean-durations --ignore=tests/unit/ --ignore=tests/bootstrap"
397+
PYTEST_ARGS: "${{ env.TINYBIRD_PYTEST_ARGS }}${{ env.TESTSELECTION_PYTEST_ARGS }} --splits 4 --group ${{ matrix.group }} --store-durations --clean-durations --ignore=tests/unit/ --ignore=tests/bootstrap --ignore=tests/aws/services/cloudformation/v2"
398398
COVERAGE_FILE: "target/.coverage.integration-${{ env.PLATFORM }}-${{ matrix.group }}"
399399
JUNIT_REPORTS_FILE: "target/pytest-junit-integration-${{ env.PLATFORM }}-${{ matrix.group }}.xml"
400400
DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_PULL_USERNAME }}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,9 @@ def __init__(
190190
resources: NodeResources,
191191
outputs: NodeOutputs,
192192
):
193-
change_type = parent_change_type_of([transform, resources, outputs])
193+
change_type = parent_change_type_of(
194+
[transform, mappings, parameters, conditions, resources, outputs]
195+
)
194196
super().__init__(scope=scope, change_type=change_type)
195197
self.transform = transform
196198
self.mappings = mappings

‎localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
from localstack.services.cloudformation.stores import get_cloudformation_store
5151
from localstack.services.cloudformation.v2.entities import ChangeSet
5252
from localstack.utils.aws.arns import get_partition
53+
from localstack.utils.objects import get_value_from_path
5354
from localstack.utils.run import to_str
5455
from localstack.utils.strings import to_bytes
5556
from localstack.utils.urls import localstack_host
@@ -248,17 +249,22 @@ def _deployed_property_value_of(
248249
f"No deployed instances of resource '{resource_logical_id}' were found"
249250
)
250251
properties = resolved_resource.get("Properties", dict())
251-
property_value: Optional[Any] = properties.get(property_name)
252+
# support structured properties, e.g. NestedStack.Outputs.OutputName
253+
property_value: Optional[Any] = get_value_from_path(properties, property_name)
252254

253255
if property_value:
256+
if not isinstance(property_value, str):
257+
# TODO: is this correct? If there is a bug in the logic here, it's probably
258+
# better to know about it with a clear error message than to receive some form
259+
# of message about trying to use a dictionary in place of a string
260+
raise RuntimeError(
261+
f"Accessing property '{property_name}' from '{resource_logical_id}' resulted in a non-string value"
262+
)
254263
return property_value
255-
256264
elif config.CFN_IGNORE_UNSUPPORTED_RESOURCE_TYPES:
257265
return MOCKED_REFERENCE
258266

259-
raise RuntimeError(
260-
f"No '{property_name}' found for deployed resource '{resource_logical_id}' was found"
261-
)
267+
return property_value
262268

263269
def _before_deployed_property_value_of(
264270
self, resource_logical_id: str, property_name: str
@@ -949,8 +955,6 @@ def _resolve_parameter_type(value: str, type_: str) -> Any:
949955
return [item.strip() for item in value.split(",")]
950956
return value
951957

952-
if not is_nothing(before):
953-
before = _resolve_parameter_type(before, parameter_type.before)
954958
if not is_nothing(after):
955959
after = _resolve_parameter_type(after, parameter_type.after)
956960

‎localstack-core/localstack/services/cloudformation/v2/entities.py‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from typing import NotRequired, Optional, TypedDict
44

55
from localstack.aws.api.cloudformation import (
6+
Capability,
67
ChangeSetStatus,
78
ChangeSetType,
89
CreateChangeSetInput,
@@ -25,6 +26,7 @@
2526
StackIdentifier,
2627
)
2728
from localstack.services.cloudformation.engine.v2.change_set_model import (
29+
ChangeType,
2830
UpdateModel,
2931
)
3032
from localstack.utils.aws import arns
@@ -45,7 +47,9 @@ class Stack:
4547
stack_id: str
4648
creation_time: datetime
4749
deletion_time: datetime | None
48-
events = list[StackEvent]
50+
events: list[StackEvent]
51+
capabilities: list[Capability]
52+
enable_termination_protection: bool
4953
processed_template: dict | None
5054

5155
# state after deploy
@@ -61,18 +65,20 @@ def __init__(
6165
request_payload: CreateChangeSetInput | CreateStackInput,
6266
template: dict | None = None,
6367
template_body: str | None = None,
68+
initial_status: StackStatus = StackStatus.CREATE_IN_PROGRESS,
6469
):
6570
self.account_id = account_id
6671
self.region_name = region_name
6772
self.template = template
6873
self.template_original = copy.deepcopy(self.template)
6974
self.template_body = template_body
70-
self.status = StackStatus.CREATE_IN_PROGRESS
75+
self.status = initial_status
7176
self.status_reason = None
7277
self.change_set_ids = []
7378
self.creation_time = datetime.now(tz=timezone.utc)
7479
self.deletion_time = None
7580
self.change_set_id = None
81+
self.enable_termination_protection = False
7682
self.processed_template = None
7783

7884
self.stack_name = request_payload["StackName"]
@@ -85,6 +91,7 @@ def __init__(
8591
account_id=self.account_id,
8692
region_name=self.region_name,
8793
)
94+
self.capabilities = request_payload.get("Capabilities", []) or []
8895

8996
# TODO: only kept for v1 compatibility
9097
self.request_payload = request_payload
@@ -126,7 +133,10 @@ def set_resource_status(
126133
if not resource_status_reason:
127134
resource_description.pop("ResourceStatusReason")
128135

129-
self.resource_states[logical_resource_id] = resource_description
136+
if status == ResourceStatus.DELETE_COMPLETE:
137+
self.resource_states.pop(logical_resource_id)
138+
else:
139+
self.resource_states[logical_resource_id] = resource_description
130140
self._store_event(logical_resource_id, physical_resource_id, status, resource_status_reason)
131141

132142
def _store_event(
@@ -173,11 +183,15 @@ def describe_details(self) -> ApiStack:
173183
"DriftInformation": StackDriftInformation(
174184
StackDriftStatus=StackDriftStatus.NOT_CHECKED
175185
),
176-
"EnableTerminationProtection": False,
186+
"EnableTerminationProtection": self.enable_termination_protection,
177187
"LastUpdatedTime": self.creation_time,
178188
"RollbackConfiguration": {},
179189
"Tags": [],
190+
"NotificationARNs": [],
191+
"Capabilities": self.capabilities,
192+
"Parameters": self.parameters,
180193
}
194+
# TODO: confirm the logic for this
181195
if change_set_id := self.change_set_id:
182196
result["ChangeSetId"] = change_set_id
183197

@@ -210,6 +224,7 @@ class ChangeSet:
210224
change_set_type: ChangeSetType
211225
update_model: Optional[UpdateModel]
212226
status: ChangeSetStatus
227+
status_reason: str | None
213228
execution_status: ExecutionStatus
214229
creation_time: datetime
215230

@@ -222,6 +237,7 @@ def __init__(
222237
self.stack = stack
223238
self.template = template
224239
self.status = ChangeSetStatus.CREATE_IN_PROGRESS
240+
self.status_reason = None
225241
self.execution_status = ExecutionStatus.AVAILABLE
226242
self.update_model = None
227243
self.creation_time = datetime.now(tz=timezone.utc)
@@ -244,6 +260,9 @@ def set_change_set_status(self, status: ChangeSetStatus):
244260
def set_execution_status(self, execution_status: ExecutionStatus):
245261
self.execution_status = execution_status
246262

263+
def has_changes(self) -> bool:
264+
return self.update_model.node_template.change_type != ChangeType.UNCHANGED
265+
247266
@property
248267
def account_id(self) -> str:
249268
return self.stack.account_id

‎localstack-core/localstack/services/cloudformation/v2/provider.py‎

Lines changed: 52 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
DescribeStackResourcesOutput,
2525
DescribeStacksOutput,
2626
DisableRollback,
27+
EnableTerminationProtection,
2728
ExecuteChangeSetOutput,
2829
ExecutionStatus,
2930
GetTemplateOutput,
@@ -48,6 +49,7 @@
4849
TemplateStage,
4950
UpdateStackInput,
5051
UpdateStackOutput,
52+
UpdateTerminationProtectionOutput,
5153
)
5254
from localstack.services.cloudformation import api_utils
5355
from localstack.services.cloudformation.engine import template_preparer
@@ -90,14 +92,12 @@ def is_changeset_arn(change_set_name_or_id: str) -> bool:
9092
return ARN_CHANGESET_REGEX.match(change_set_name_or_id) is not None
9193

9294

93-
class StackWithNameNotFoundError(ValidationError):
94-
def __init__(self, stack_name: str):
95-
super().__init__(f"Stack [{stack_name}] does not exist")
96-
97-
98-
class StackWithIdNotFoundError(ValidationError):
99-
def __init__(self, stack_id: str):
100-
super().__init__("Stack with id <stack-name> does not exist")
95+
class StackNotFoundError(ValidationError):
96+
def __init__(self, stack_name_or_id: str):
97+
if is_stack_arn(stack_name_or_id):
98+
super().__init__(f"Stack with id {stack_name_or_id} does not exist")
99+
else:
100+
super().__init__(f"Stack [{stack_name_or_id}] does not exist")
101101

102102

103103
def find_stack_v2(state: CloudFormationStore, stack_name: str | None) -> Stack | None:
@@ -128,7 +128,7 @@ def find_change_set_v2(
128128
if stack_name is not None:
129129
stack = find_stack_v2(state, stack_name)
130130
if not stack:
131-
raise StackWithNameNotFoundError(stack_name)
131+
raise StackNotFoundError(stack_name)
132132

133133
for change_set_id in stack.change_set_ids:
134134
change_set_candidate = state.change_sets[change_set_id]
@@ -250,18 +250,14 @@ def create_change_set(
250250
request_payload=request,
251251
template=structured_template,
252252
template_body=template_body,
253+
initial_status=StackStatus.REVIEW_IN_PROGRESS,
253254
)
254255
state.stacks_v2[stack.stack_id] = stack
255256
else:
256257
if not active_stack_candidates:
257258
raise ValidationError(f"Stack '{stack_name}' does not exist.")
258259
stack = active_stack_candidates[0]
259260

260-
if stack.status in [StackStatus.CREATE_COMPLETE, StackStatus.UPDATE_COMPLETE]:
261-
stack.set_stack_status(StackStatus.UPDATE_IN_PROGRESS)
262-
else:
263-
stack.set_stack_status(StackStatus.REVIEW_IN_PROGRESS)
264-
265261
# TODO: test if rollback status is allowed as well
266262
if (
267263
change_set_type == ChangeSetType.CREATE
@@ -330,7 +326,19 @@ def create_change_set(
330326
previous_update_model=previous_update_model,
331327
)
332328

333-
change_set.set_change_set_status(ChangeSetStatus.CREATE_COMPLETE)
329+
# TODO: handle the empty change set case
330+
if not change_set.has_changes():
331+
change_set.set_change_set_status(ChangeSetStatus.FAILED)
332+
change_set.set_execution_status(ExecutionStatus.UNAVAILABLE)
333+
change_set.status_reason = "The submitted information didn't contain changes. Submit different information to create a change set."
334+
else:
335+
if stack.status in [StackStatus.CREATE_COMPLETE, StackStatus.UPDATE_COMPLETE]:
336+
stack.set_stack_status(StackStatus.UPDATE_IN_PROGRESS)
337+
else:
338+
stack.set_stack_status(StackStatus.REVIEW_IN_PROGRESS)
339+
340+
change_set.set_change_set_status(ChangeSetStatus.CREATE_COMPLETE)
341+
334342
stack.change_set_id = change_set.change_set_id
335343
stack.change_set_ids.append(change_set.change_set_id)
336344
state.change_sets[change_set.change_set_id] = change_set
@@ -438,6 +446,8 @@ def _describe_change_set(
438446
for (key, value) in change_set.stack.resolved_parameters.items()
439447
],
440448
Changes=changes,
449+
Capabilities=change_set.stack.capabilities,
450+
StatusReason=change_set.status_reason,
441451
)
442452
return result
443453

@@ -455,6 +465,7 @@ def describe_change_set(
455465
# only relevant if change_set_name isn't an ARN
456466
state = get_cloudformation_store(context.account_id, context.region)
457467
change_set = find_change_set_v2(state, change_set_name, stack_name)
468+
458469
if not change_set:
459470
raise ChangeSetNotFoundException(f"ChangeSet [{change_set_name}] does not exist")
460471
result = self._describe_change_set(
@@ -596,7 +607,8 @@ def describe_stacks(
596607
state = get_cloudformation_store(context.account_id, context.region)
597608
stack = find_stack_v2(state, stack_name)
598609
if not stack:
599-
raise StackWithIdNotFoundError(stack_name)
610+
raise StackNotFoundError(stack_name)
611+
# TODO: move describe_details method to provider
600612
return DescribeStacksOutput(Stacks=[stack.describe_details()])
601613

602614
@handler("ListStacks")
@@ -645,7 +657,7 @@ def describe_stack_resources(
645657
state = get_cloudformation_store(context.account_id, context.region)
646658
stack = find_stack_v2(state, stack_name)
647659
if not stack:
648-
raise StackWithIdNotFoundError(stack_name)
660+
raise StackNotFoundError(stack_name)
649661
# TODO: filter stack by PhysicalResourceId!
650662
statuses = []
651663
for resource_id, resource_status in stack.resource_states.items():
@@ -663,10 +675,14 @@ def describe_stack_events(
663675
next_token: NextToken = None,
664676
**kwargs,
665677
) -> DescribeStackEventsOutput:
678+
if not stack_name:
679+
raise ValidationError(
680+
"1 validation error detected: Value null at 'stackName' failed to satisfy constraint: Member must not be null"
681+
)
666682
state = get_cloudformation_store(context.account_id, context.region)
667683
stack = find_stack_v2(state, stack_name)
668684
if not stack:
669-
raise StackWithIdNotFoundError(stack_name)
685+
raise StackNotFoundError(stack_name)
670686
return DescribeStackEventsOutput(StackEvents=stack.events)
671687

672688
@handler("GetTemplate")
@@ -685,7 +701,7 @@ def get_template(
685701
elif stack_name:
686702
stack = find_stack_v2(state, stack_name)
687703
else:
688-
raise StackWithIdNotFoundError(stack_name)
704+
raise StackNotFoundError(stack_name)
689705

690706
if template_stage == TemplateStage.Processed and "Transform" in stack.template_body:
691707
template_body = json.dumps(stack.processed_template)
@@ -709,7 +725,7 @@ def get_template_summary(
709725
if stack_name:
710726
stack = find_stack_v2(state, stack_name)
711727
if not stack:
712-
raise StackWithIdNotFoundError(stack_name)
728+
raise StackNotFoundError(stack_name)
713729
template = stack.template
714730
else:
715731
template_body = request.get("TemplateBody")
@@ -758,6 +774,22 @@ def get_template_summary(
758774

759775
return result
760776

777+
@handler("UpdateTerminationProtection")
778+
def update_termination_protection(
779+
self,
780+
context: RequestContext,
781+
enable_termination_protection: EnableTerminationProtection,
782+
stack_name: StackNameOrId,
783+
**kwargs,
784+
) -> UpdateTerminationProtectionOutput:
785+
state = get_cloudformation_store(context.account_id, context.region)
786+
stack = find_stack_v2(state, stack_name)
787+
if not stack:
788+
raise StackNotFoundError(stack_name)
789+
790+
stack.enable_termination_protection = enable_termination_protection
791+
return UpdateTerminationProtectionOutput(StackId=stack.stack_id)
792+
761793
@handler("UpdateStack", expand=False)
762794
def update_stack(
763795
self,

‎localstack-core/localstack/services/kinesis/resource_providers/aws_kinesis_streamconsumer.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ def create(
6666
response = kinesis.register_stream_consumer(
6767
StreamARN=model["StreamARN"], ConsumerName=model["ConsumerName"]
6868
)
69-
model["ConsumerARN"] = response["Consumer"]["ConsumerARN"]
69+
model["Id"] = model["ConsumerARN"] = response["Consumer"]["ConsumerARN"]
7070
model["ConsumerStatus"] = response["Consumer"]["ConsumerStatus"]
7171
request.custom_context[REPEATED_INVOCATION] = True
7272
return ProgressEvent(

0 commit comments

Comments
 (0)