Skip to content

fix delete method of AWS::SNS::TopicPolicy CFn resource #12831

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 4, 2025
Merged

fix delete method of AWS::SNS::TopicPolicy CFn resource #12831

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 33 additions & 0 deletions localstack-core/localstack/services/sns/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
topicARN,
)
from localstack.services.stores import AccountRegionBundle, BaseStore, LocalAttribute
from localstack.utils.aws.arns import parse_arn
from localstack.utils.objects import singleton_factory
from localstack.utils.strings import long_uid

Expand All @@ -25,6 +26,38 @@
SnsMessageProtocols = Literal[SnsProtocols, SnsApplicationPlatforms]


def create_default_sns_topic_policy(topic_arn: str) -> dict:
Copy link
Contributor

Choose a reason for hiding this comment

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

nice addition! I'm always torn with this kind of utils in models.py, but it makes sense. It's going to nicely be used when we start creating topics in LocalStack directly 👀

Copy link
Member Author

@thrau thrau Jul 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

happy to put it elsewhere. what would be a good place? should we create a utils module, or should it just go into the provider? i could also move it to the resource provider directly

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is good for now! If we internalize, we’ll create a util folder and move things around! Sorry for the delay 😄

"""
Creates the default SNS topic policy for the given topic ARN.

:param topic_arn: The topic arn
:return: A policy document
"""
return {
"Version": "2008-10-17",
"Id": "__default_policy_ID",
"Statement": [
{
"Sid": "__default_statement_ID",
"Effect": "Allow",
"Principal": {"AWS": "*"},
"Action": [
"SNS:GetTopicAttributes",
"SNS:SetTopicAttributes",
"SNS:AddPermission",
"SNS:RemovePermission",
"SNS:DeleteTopic",
"SNS:Subscribe",
"SNS:ListSubscriptionsByTopic",
"SNS:Publish",
],
"Resource": topic_arn,
"Condition": {"StringEquals": {"AWS:SourceOwner": parse_arn(topic_arn)["account"]}},
}
],
}


@singleton_factory
def global_sns_message_sequence():
# creates a 20-digit number used as the start for the global sequence, adds 100 for it to be different from SQS's
Expand Down
8 changes: 6 additions & 2 deletions localstack-core/localstack/services/sns/resource_providers/aws_sns_topicpolicy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
ResourceProvider,
ResourceRequest,
)
from localstack.services.sns.models import create_default_sns_topic_policy


class SNSTopicPolicyProperties(TypedDict):
Expand Down Expand Up @@ -96,14 +97,17 @@ def delete(
for topic_arn in model["Topics"]:
try:
sns.set_topic_attributes(
TopicArn=topic_arn, AttributeName="Policy", AttributeValue=""
TopicArn=topic_arn,
AttributeName="Policy",
AttributeValue=json.dumps(create_default_sns_topic_policy(topic_arn)),
)

except ClientError as err:
if "NotFound" not in err.response["Error"]["Code"]:
raise

return ProgressEvent(
status=OperationStatus.IN_PROGRESS,
status=OperationStatus.SUCCESS,
Copy link
Contributor

Choose a reason for hiding this comment

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

question: I suppose this is the main fix, right? the Delete operation would never finish as it never returned SUCCESS?

Copy link
Member Author

Choose a reason for hiding this comment

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

this + the default policy, because sending an empty attribute value to moto raises and error here: https://github.com/getmoto/moto/blob/338cb256a4ab938f24b5b29ec079ed26a9d0c093/moto/sns/models.py#L137-L139

resource_model=model,
custom_context=request.custom_context,
)
Expand Down
56 changes: 56 additions & 0 deletions tests/aws/services/cloudformation/resources/test_sns.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from localstack.testing.aws.util import is_aws_cloud
from localstack.testing.pytest import markers
from localstack.utils.aws.arns import parse_arn
from localstack.utils.common import short_uid


Expand Down Expand Up @@ -152,6 +153,61 @@ def test_sns_topic_with_attributes(infrastructure_setup, aws_client, snapshot):
snapshot.match("topic-archive-policy", response["Attributes"]["ArchivePolicy"])


@markers.aws.validated
@markers.snapshot.skip_snapshot_verify(
paths=[
"$..Statement..Action", # TODO: see https://github.com/getmoto/moto/pull/9041
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks a lot for fixing this 😄 this is gonna remove a few snapshot skips!

]
)
def test_sns_topic_policy_resets_to_default(
sns_topic, infrastructure_setup, aws_client, snapshot, account_id
):
"""Tests the delete statement of a ``AWS::SNS::TopicPolicy`` resource, which should reset the topic's policy to the
default policy."""
# simulate a pre-existing topic
existing_topic_arn = sns_topic["Attributes"]["TopicArn"]
existing_topic_name = parse_arn(existing_topic_arn)["resource"]
snapshot.add_transformer(snapshot.transform.regex(existing_topic_name, "<topic-name>"))

# create the stack
stack_name = "SnsTopicPolicyStack"
infra = infrastructure_setup(namespace="SnsTests")
# persisting the stack means persisting the existing_topic_arn reference, but that changes every test run
infra.persist_output = False
stack = cdk.Stack(infra.cdk_app, stack_name=stack_name)

# get the existing topic
topic = cdk.aws_sns.Topic.from_topic_arn(stack, "Topic", existing_topic_arn)

# add the topic policy resource
topic_policy = cdk.aws_sns.TopicPolicy(stack, "CustomTopicPolicy", topics=[topic])
topic_policy.document.add_statements(
cdk.aws_iam.PolicyStatement(
effect=cdk.aws_iam.Effect.ALLOW,
principals=[cdk.aws_iam.AnyPrincipal()],
actions=["sns:Publish"],
resources=[topic.topic_arn],
conditions={"StringEquals": {"aws:SourceAccount": account_id}},
)
)

# snapshot its policy
default = aws_client.sns.get_topic_attributes(TopicArn=existing_topic_arn)
snapshot.match("default-topic-attributes", default["Attributes"]["Policy"])

# deploy the stack
cdk.CfnOutput(stack, "TopicArn", value=topic.topic_arn)
with infra.provisioner() as prov:
assert prov.get_stack_outputs(stack_name=stack_name)["TopicArn"] == existing_topic_arn

modified = aws_client.sns.get_topic_attributes(TopicArn=existing_topic_arn)
snapshot.match("modified-topic-attributes", modified["Attributes"]["Policy"])

# now that it's destroyed, get the topic attributes again
reverted = aws_client.sns.get_topic_attributes(TopicArn=existing_topic_arn)
snapshot.match("reverted-topic-attributes", reverted["Attributes"]["Policy"])


@markers.aws.validated
def test_sns_subscription_region(
snapshot,
Expand Down
82 changes: 82 additions & 0 deletions tests/aws/services/cloudformation/resources/test_sns.snapshot.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,5 +134,87 @@
}
}
}
},
"tests/aws/services/cloudformation/resources/test_sns.py::test_sns_topic_policy_resets_to_default": {
"recorded-date": "04-07-2025, 00:04:32",
"recorded-content": {
"default-topic-attributes": {
"Version": "2008-10-17",
"Id": "__default_policy_ID",
"Statement": [
{
"Sid": "__default_statement_ID",
"Effect": "Allow",
"Principal": {
"AWS": "*"
},
"Action": [
"SNS:GetTopicAttributes",
"SNS:SetTopicAttributes",
"SNS:AddPermission",
"SNS:RemovePermission",
"SNS:DeleteTopic",
"SNS:Subscribe",
"SNS:ListSubscriptionsByTopic",
"SNS:Publish"
],
"Resource": "arn:<partition>:sns:<region>:111111111111:<topic-name>",
"Condition": {
"StringEquals": {
"AWS:SourceOwner": "111111111111"
}
}
}
]
},
"modified-topic-attributes": {
"Version": "2012-10-17",
"Statement": [
{
"Sid": "0",
"Effect": "Allow",
"Principal": {
"AWS": "*"
},
"Action": "sns:Publish",
"Resource": "arn:<partition>:sns:<region>:111111111111:<topic-name>",
"Condition": {
"StringEquals": {
"aws:SourceAccount": "111111111111"
}
}
}
]
},
"reverted-topic-attributes": {
"Version": "2008-10-17",
"Id": "__default_policy_ID",
"Statement": [
{
"Sid": "__default_statement_ID",
"Effect": "Allow",
"Principal": {
"AWS": "*"
},
"Action": [
"SNS:GetTopicAttributes",
"SNS:SetTopicAttributes",
"SNS:AddPermission",
"SNS:RemovePermission",
"SNS:DeleteTopic",
"SNS:Subscribe",
"SNS:ListSubscriptionsByTopic",
"SNS:Publish"
],
"Resource": "arn:<partition>:sns:<region>:111111111111:<topic-name>",
"Condition": {
"StringEquals": {
"AWS:SourceOwner": "111111111111"
}
}
}
]
}
}
}
}
17 changes: 16 additions & 1 deletion tests/aws/services/cloudformation/resources/test_sns.validation.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,23 @@
"tests/aws/services/cloudformation/resources/test_sns.py::test_sns_topic_fifo_with_deduplication": {
"last_validated_date": "2023-11-27T20:27:29+00:00"
},
"tests/aws/services/cloudformation/resources/test_sns.py::test_sns_topic_policy_resets_to_default": {
"last_validated_date": "2025-07-04T00:04:32+00:00",
"durations_in_seconds": {
"setup": 1.08,
"call": 22.27,
"teardown": 0.09,
"total": 23.44
}
},
"tests/aws/services/cloudformation/resources/test_sns.py::test_sns_topic_with_attributes": {
"last_validated_date": "2024-08-16T15:44:50+00:00"
"last_validated_date": "2025-07-03T23:32:29+00:00",
"durations_in_seconds": {
"setup": 0.8,
"call": 21.33,
"teardown": 0.0,
"total": 22.13
}
},
"tests/aws/services/cloudformation/resources/test_sns.py::test_update_subscription": {
"last_validated_date": "2024-03-29T21:16:21+00:00"
Expand Down
Loading