Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
PreprocResource,
)
from localstack.services.cloudformation.v2.entities import ChangeSet
from localstack.utils.numbers import is_number

CHANGESET_KNOWN_AFTER_APPLY: Final[str] = "{{changeSet:KNOWN_AFTER_APPLY}}"

Expand Down Expand Up @@ -96,6 +97,19 @@ def _resolve_attribute(self, arguments: str | list[str], select_before: bool) ->

return value

def visit_node_intrinsic_function(self, node_intrinsic_function: NodeIntrinsicFunction):
"""
Intrinsic function results are always strings when referring to the describe output
"""
# TODO: what about other places?
# TODO: should this be put in the preproc?
delta = super().visit_node_intrinsic_function(node_intrinsic_function)
if is_number(delta.before):
delta.before = str(delta.before)
if is_number(delta.after):
delta.after = str(delta.after)
return delta

def visit_node_intrinsic_function_fn_join(
self, node_intrinsic_function: NodeIntrinsicFunction
) -> PreprocEntityDelta:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
from localstack.services.cloudformation.v2.entities import ChangeSet
from localstack.services.cloudformation.v2.types import ResolvedResource
from localstack.utils.aws.arns import get_partition
from localstack.utils.numbers import to_number
from localstack.utils.objects import get_value_from_path
from localstack.utils.run import to_str
from localstack.utils.strings import to_bytes
Expand Down Expand Up @@ -1029,6 +1030,9 @@ def _resolve_parameter_type(value: str, type_: str) -> Any:
match type_:
case "List<String>" | "CommaDelimitedList":
return [item.strip() for item in value.split(",")]
case "Number":
# TODO: validate the parameter type at template parse time (or whatever is in parity with AWS) so we know this cannot fail
return to_number(value)
return value

if not is_nothing(after):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,10 @@
StackInstance,
StackSet,
)
from localstack.services.cloudformation.v2.types import EngineParameter
from localstack.services.cloudformation.v2.types import EngineParameter, engine_parameter_value
from localstack.services.plugins import ServiceLifecycleHook
from localstack.utils.collections import select_attributes
from localstack.utils.numbers import is_number
from localstack.utils.strings import short_uid
from localstack.utils.threads import start_worker_thread

Expand Down Expand Up @@ -262,6 +263,12 @@ def _resolve_parameters(
no_echo=parameter.get("NoEcho"),
)

# validate the type
if parameter["Type"] == "Number" and not is_number(
engine_parameter_value(resolved_parameter)
):
raise ValidationError(f"Parameter '{name}' must be a number.")

# TODO: support other parameter types
if match := SSM_PARAMETER_TYPE_RE.match(parameter["Type"]):
inner_type = match.group("innertype")
Expand Down
7 changes: 7 additions & 0 deletions localstack-core/localstack/utils/numbers.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ def format_number(number: float, decimals: int = 2):


def is_number(s: Any) -> bool:
# booleans inherit from int
#
# >>> a.__class__.__mro__
# (<class 'bool'>, <class 'int'>, <class 'object'>)
if s is False or s is True:
return False
Comment on lines +18 to +19
Copy link
Member

Choose a reason for hiding this comment

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

Praise: Nice consideration.


try:
float(s) # for int, long and float
return True
Expand Down
28 changes: 28 additions & 0 deletions tests/aws/services/cloudformation/resources/test_logs.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import os.path

import pytest
from tests.aws.services.cloudformation.conftest import skip_if_legacy_engine

from localstack.testing.pytest import markers
from localstack.testing.pytest.fixtures import StackDeployError
from localstack.utils.strings import short_uid


@markers.aws.validated
Expand Down Expand Up @@ -47,3 +52,26 @@ def test_cfn_handle_log_group_resource(deploy_cfn_template, aws_client, snapshot
stack.destroy()
response = aws_client.logs.describe_log_groups(logGroupNamePrefix=log_group_prefix)
assert len(response["logGroups"]) == 0


@markers.aws.validated
@skip_if_legacy_engine()
def test_handle_existing_log_group(deploy_cfn_template, aws_client, snapshot, cleanups):
snapshot.add_transformer(snapshot.transform.cloudformation_api())
snapshot.add_transformer(snapshot.transform.key_value("ParameterValue"))

log_group_name = f"logs-{short_uid()}"

# create the log group
aws_client.logs.create_log_group(logGroupName=log_group_name)
cleanups.append(lambda: aws_client.logs.delete_log_group(logGroupName=log_group_name))

with pytest.raises(StackDeployError) as exc_info:
deploy_cfn_template(
template_path=os.path.join(
os.path.dirname(__file__), "../../../templates/logs_group.yml"
),
parameters={"LogGroupName": log_group_name},
)

snapshot.match("failed-stack-describe", exc_info.value.describe_result)
Copy link
Member

Choose a reason for hiding this comment

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

Nit: It could be useful to have the status reason if there was any.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry I'm not sure I understand. There is no StatusReason in the snapshot so do you mean we should set it even though it's not in parity?

Copy link
Member

Choose a reason for hiding this comment

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

No. I'm talking about the StatusReason that should be in the CREATE_FAILED event of the Log Group.

Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,37 @@
}
}
}
},
"tests/aws/services/cloudformation/resources/test_logs.py::test_handle_existing_log_group": {
"recorded-date": "06-10-2025, 16:24:57",
"recorded-content": {
"failed-stack-describe": {
"Capabilities": [
"CAPABILITY_AUTO_EXPAND",
"CAPABILITY_IAM",
"CAPABILITY_NAMED_IAM"
],
"CreationTime": "datetime",
"DeletionTime": "datetime",
"DisableRollback": false,
"DriftInformation": {
"StackDriftStatus": "NOT_CHECKED"
},
"EnableTerminationProtection": false,
"LastUpdatedTime": "datetime",
"NotificationARNs": [],
"Parameters": [
{
"ParameterKey": "LogGroupName",
"ParameterValue": "<parameter-value:1>"
}
],
"RollbackConfiguration": {},
"StackId": "arn:<partition>:cloudformation:<region>:111111111111:stack/<stack-name:1>/<resource:1>",
"StackName": "<stack-name:1>",
"StackStatus": "ROLLBACK_COMPLETE",
"Tags": []
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@
"tests/aws/services/cloudformation/resources/test_logs.py::test_cfn_handle_log_group_resource": {
"last_validated_date": "2024-06-20T16:15:47+00:00"
},
"tests/aws/services/cloudformation/resources/test_logs.py::test_handle_existing_log_group": {
"last_validated_date": "2025-10-06T16:24:57+00:00",
"durations_in_seconds": {
"setup": 0.81,
"call": 10.58,
"teardown": 0.22,
"total": 11.61
}
},
"tests/aws/services/cloudformation/resources/test_logs.py::test_logstream": {
"last_validated_date": "2022-07-29T11:22:53+00:00"
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import copy
import json

import pytest
from botocore.exceptions import ClientError
from localstack_snapshot.snapshots.transformer import RegexTransformer
from tests.aws.services.cloudformation.conftest import skip_if_legacy_engine

from localstack.testing.pytest import markers
from localstack.utils.strings import long_uid
from localstack.utils.strings import long_uid, short_uid


@skip_if_legacy_engine()
Expand Down Expand Up @@ -123,3 +128,64 @@ def test_update_parameter_default_value_with_dynamic_overrides(
capture_update_process(
snapshot, template_1, template_2, p1={"TopicName": name1}, p2={"TopicName": name2}
)

@markers.aws.validated
def test_parameter_type_change(self, snapshot, capture_update_process):
snapshot.add_transformer(snapshot.transform.key_value("PhysicalResourceId"))

template1 = {
"Parameters": {
"Value": {
"Type": "Number",
},
},
"Resources": {
"MyParameter": {
"Type": "AWS::SSM::Parameter",
"Properties": {
"Type": "String",
"Value": {"Ref": "Value"},
},
},
},
}
template2 = copy.deepcopy(template1)
template2["Parameters"]["Value"]["Type"] = "String"

capture_update_process(
snapshot, template1, template2, p1={"Value": "123"}, p2={"Value": "456"}
)

@markers.aws.validated
def test_invalid_parameter_type(self, snapshot, aws_client):
template = {
"Parameters": {
"Value": {
"Type": "Number",
},
},
"Resources": {
"MyParameter": {
"Type": "AWS::SSM::Parameter",
"Properties": {
"Type": "String",
"Value": short_uid(),
},
},
},
}

stack_name = f"stack-{short_uid()}"
cs_name = f"cs-{short_uid()}"
with pytest.raises(ClientError) as exc_info:
aws_client.cloudformation.create_change_set(
ChangeSetName=cs_name,
StackName=stack_name,
ChangeSetType="CREATE",
TemplateBody=json.dumps(template),
Parameters=[
{"ParameterKey": "Value", "ParameterValue": "not-a-number"},
],
)

snapshot.match("error", exc_info.value.response)
Loading
Loading