Sns:v2 publish #13399

baermat · 2025-11-19T14:05:02Z

Motivation

This PR enables publishing and batch-publishing for the SNS v2 provider. It enables the first skipped tests that relate to publish.
relates to PNX-75
It also adds the code for the raw http certificate response and the publisher.

Changes

add publish and publish_batch
enables the following test classes:
- TestSNSPublishCrud
- TestSNSSubscriptionSQS
adds publisher.py to the v2 folder with slight adjustments due to the new models/store
add http routes for the certificate

Tests

Test Results - Preflight, Unit

22 332 tests ±0 20 578 ✅ ±0 6m 15s ⏱️ +12s
1 suites ±0 1 754 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit e8005bf. ± Comparison against base commit fedfb84.

♻️ This comment has been updated with latest results.

github-actions · 2025-11-19T14:42:03Z

Test Results - Alternative Providers

198 tests 110 ✅ 36s ⏱️
1 suites 88 💤
1 files 0 ❌

Results for commit e8005bf.

♻️ This comment has been updated with latest results.

github-actions · 2025-11-19T14:43:58Z

Test Results (amd64) - Acceptance

7 tests ±0 5 ✅ ±0 3m 24s ⏱️ +12s
1 suites ±0 2 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit e8005bf. ± Comparison against base commit fedfb84.

♻️ This comment has been updated with latest results.

github-actions · 2025-11-19T15:00:43Z

Test Results (amd64) - Integration, Bootstrap

5 files 5 suites 1h 28m 44s ⏱️
2 980 tests 2 828 ✅ 152 💤 0 ❌
2 986 runs 2 828 ✅ 158 💤 0 ❌

Results for commit e8005bf.

♻️ This comment has been updated with latest results.

github-actions · 2025-11-19T15:04:38Z

LocalStack Community integration with Pro

2 files ± 0 2 suites ±0 1h 0m 40s ⏱️ - 1h 0m 26s
2 956 tests - 1 993 2 801 ✅ - 1 770 155 💤 - 223 0 ❌ ±0
2 958 runs - 1 993 2 801 ✅ - 1 770 157 💤 - 223 0 ❌ ±0

Results for commit e8005bf. ± Comparison against base commit fedfb84.

This pull request removes 2004 and adds 11 tests. Note that renamed tests count towards both.

tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiDocumentationPart ‑ test_import_documentation_parts_bad_file
tests.aws.services.apigateway.test_apigateway_import.TestApiGatewayImportRestApi ‑ test_import_api_bad_file
tests.aws.services.s3.test_s3_list_operations.TestS3ListBuckets ‑ test_list_buckets_region_validation
tests.aws.services.s3.test_s3_list_operations.TestS3ListBuckets ‑ test_list_buckets_region_validation_disabled
tests.aws.services.s3.test_s3_preconditions.TestS3CopySourcePreconditions ‑ test_s3_copy_object_if_source_modified_since_versioned
tests.aws.services.s3.test_s3_preconditions.TestS3CopySourcePreconditions ‑ test_s3_copy_object_if_source_unmodified_since_versioned
tests.aws.services.s3.test_s3_preconditions.TestS3CopySourcePreconditions ‑ test_s3_copy_object_preconditions
tests.aws.services.stepfunctions.v2.test_sfn_api_validation.TestSfnApiValidation ‑ test_validate_state_machine_definition_type_express[INVALID_DOWNGRADE_QUERY_LANGUAGE]
tests.aws.services.stepfunctions.v2.test_sfn_api_validation.TestSfnApiValidation ‑ test_validate_state_machine_definition_type_express[VALID_QUERY_LANGUAGE_PASS]
tests.aws.services.stepfunctions.v2.test_sfn_api_validation.TestSfnApiValidation ‑ test_validate_state_machine_definition_type_standard[INVALID_DOWNGRADE_QUERY_LANGUAGE]
…

♻️ This comment has been updated with latest results.

bentsku

Nice work! This looks great, loving the changes in the provider 👍 Looks like it was only a limited set of changes.

Are there still some tests failing? Do we have an idea of what it will take to have everything working?

I only have one big comment around the full duplication of publisher.py and of the maintenance burden it could be (looking at the double set of DynamoDB providers and the pain it is, I'm trying to avoid it as much as possible now). I believe it is possible to make the current implementation working with both providers without big trouble, but correct me if I'm wrong! I might be missing something, I tried to diff the files locally to see the difference between the new version here and the current version.

I believe once this is addressed, we're good to go!

bentsku · 2025-11-19T18:31:31Z

localstack-core/localstack/services/sns/v2/provider.py

        if topic_arn not in store.topics:
            raise NotFoundException("Topic does not exist")

        topic_subscriptions = store.topics[topic_arn]["subscriptions"]


nit: as we are calling self._get_topic a bit under, I think we could save the topic here:

Suggested change

if not (topic := store.topics.get(topic_arn)):

raise NotFoundException("Topic does not exist")

topic_subscriptions = topic["subscriptions"]

So that we could reuse the topic at line 393 (to get the attributes)

by calling _get_topic we actually already do that check already, so it eliminates even more code, thanks!

bentsku · 2025-11-19T18:45:50Z

localstack-core/localstack/services/sns/v2/publisher.py

I've looked at the diff between the regular publisher.py and this one, and it's only 5 lines differences. I'm not sure it is worth fully copy pasting the file to be honest.

I think the code could be adapted in such a way that it would be compatible with both providers, as the difference is only about the utility to fetch the subscriptions of a topic:

the models are almost identical and don't have a change in behavior, so they are fine to use which ever version between v1 and v2

one minor change for the SignatureVersion casing, we could try to get both values I think

the new utility to fetch the topic subscriptions could be adapted to also work for the old store, that way you could use that one everywhere (you might have type hints issue but I think it is alright):

def get_topic_subscriptions(store: SnsStore, topic_arn: str) -> list[SnsSubscription]: # TODO: delete this once the legacy implementation has been removed if hasattr(store, "topic_subscriptions"): sub_arns = store.topic_subscriptions.get(topic_arn, []) else: sub_arns: list[str] = store.topics[topic_arn].get("subscriptions", []) subscriptions = [store.subscriptions[k] for k in sub_arns if k in store.subscriptions] return subscriptions

What do you think? I think it would reduce the scope of changes, and make it easier to see the diff of changes, and if one day we need a fix in the publisher while we still have 2 providers, we can fix it in one place only.

The reason why I did this was that if we allow both casings, and then retire the v1 provider at some point, this has high potential to stay. At that point, it is unnecessary at best and a potential bug at worst. Then again, it probably won't have much impact and doesn't compare against 1k line copy pasted. The big question is "how much do we need to change ultimately". If it is 20 lines of codes, it's probably alright to fill the publisher with some legacy code. If it's 100, it's probably better to copy paste it. I will mark that spot with a comment, so if somebody at some point looks for "v1" in the code, they can at least identify the spots easily.
tl;dr: not duplicating the publisher and marking all those spots with comments is probably a good middle ground

bentsku

Sorry I missed something else, it seems we're still trying to access an attribute of the topic via .content_based_deduplication which will fail. Would be good to fix too 👍

bentsku · 2025-11-19T18:51:29Z

localstack-core/localstack/services/sns/v2/provider.py

+        # for compatibility reasons, AWS allows users to use either TargetArn or TopicArn for publishing to a topic
+        # use any of them for topic validation
+        topic_or_target_arn = topic_arn or target_arn
+        topic_model = None


nit: I believe we could call this topic here as model was heavily tied with moto, and now it is only a TypedDict

bentsku · 2025-11-19T18:55:23Z

localstack-core/localstack/services/sns/v2/provider.py

+                    "Invalid parameter: The MessageGroupId parameter is required for FIFO topics",
+                )
+            topic_model = self._get_topic(topic_or_target_arn, context)
+            if topic_model.content_based_deduplication == "false":


I believe this is a leftover from moto: topic_model is a typed dict and won't have an attribute named content_based_deduplication and it will trigger an AttributeError, sorry for missing it earlier, it's hard with no diff to know what's new to not 😄

nice catch, thanks!

baermat

@bentsku I worked in the suggestions and also realised I failed to actually submit my comments 🤦 please take a look (but it can easily be tackled in a follow up PR)

localstack-core/localstack/services/sns/v2/provider.py

baermat · 2025-11-19T14:22:00Z

localstack-core/localstack/services/sns/v2/provider.py

-    # TODO: reintroduce multi-region parameter (latest before final migration from v1)
    @staticmethod
-    def _get_topic(arn: str, context: RequestContext) -> Topic:
+    def _get_topic(arn: str, context: RequestContext, multi_region: bool = False) -> Topic:
        """
        :param arn: the Topic ARN
        :param context: the RequestContext of the request
        :return: the model Topic
        """
        arn_data = parse_and_validate_topic_arn(arn)
-        if context.region != arn_data["region"]:
+        if not multi_region and context.region != arn_data["region"]:
            raise InvalidParameterException("Invalid parameter: TopicArn")


I don't think there is any usage for this with multi_region = True, but @bentsku you disagreed last time, and since I was tackling a failing test that was testing multi region, I tackled this. I don't think the access to the store makes sense, and the current v1 provider runs without it as well. Can you elaborate on where do you think we need to allow multi_region access?

You're right, I think it was actually an oversight and a mistake I had done to add it. As you said it's not in the current provider anymore and I'm not sure why I disagreed last time 😅 sorry for that, I think it's safe to remove!

baermat · 2025-11-20T14:21:23Z

localstack-core/localstack/services/sns/v2/provider.py

        if topic_arn not in store.topics:
            raise NotFoundException("Topic does not exist")

        topic_subscriptions = store.topics[topic_arn]["subscriptions"]


by calling _get_topic we actually already do that check already, so it eliminates even more code, thanks!

baermat · 2025-11-20T14:22:13Z

localstack-core/localstack/services/sns/v2/provider.py

+                    "Invalid parameter: The MessageGroupId parameter is required for FIFO topics",
+                )
+            topic_model = self._get_topic(topic_or_target_arn, context)
+            if topic_model.content_based_deduplication == "false":


nice catch, thanks!

baermat · 2025-11-20T14:34:14Z

localstack-core/localstack/services/sns/v2/publisher.py

The reason why I did this was that if we allow both casings, and then retire the v1 provider at some point, this has high potential to stay. At that point, it is unnecessary at best and a potential bug at worst. Then again, it probably won't have much impact and doesn't compare against 1k line copy pasted. The big question is "how much do we need to change ultimately". If it is 20 lines of codes, it's probably alright to fill the publisher with some legacy code. If it's 100, it's probably better to copy paste it. I will mark that spot with a comment, so if somebody at some point looks for "v1" in the code, they can at least identify the spots easily.
tl;dr: not duplicating the publisher and marking all those spots with comments is probably a good middle ground

bentsku

LGTM! Thanks a lot for addressing the comment, and good catch on the multi_region parameter, I don't know why I insisted on that 😅 it can be addressed in a follow up too 👍

This looks great! Congrats! 🚀

bentsku · 2025-11-20T17:24:12Z

localstack-core/localstack/services/sns/v2/provider.py

-    # TODO: reintroduce multi-region parameter (latest before final migration from v1)
    @staticmethod
-    def _get_topic(arn: str, context: RequestContext) -> Topic:
+    def _get_topic(arn: str, context: RequestContext, multi_region: bool = False) -> Topic:
        """
        :param arn: the Topic ARN
        :param context: the RequestContext of the request
        :return: the model Topic
        """
        arn_data = parse_and_validate_topic_arn(arn)
-        if context.region != arn_data["region"]:
+        if not multi_region and context.region != arn_data["region"]:
            raise InvalidParameterException("Invalid parameter: TopicArn")


You're right, I think it was actually an oversight and a mistake I had done to add it. As you said it's not in the current provider anymore and I'm not sure why I disagreed last time 😅 sorry for that, I think it's safe to remove!

bentsku · 2025-11-20T17:27:08Z

localstack-core/localstack/services/sns/publisher.py

thanks a lot for keeping it as one 🙏 I agree this legacy code might stay, we should do somewhat of a checklist of what to delete once we remove the legacy provider to keep track of all clean up we'll need to do (removing the test job, removing this in publisher.py, etc)

baermat added 4 commits November 19, 2025 09:39

port publish

a1542c2

first working test

5145c7c

publish-crud tests working

22837e6

TestSNSSubscriptionSQS tests working

f69b1d3

baermat added semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes labels Nov 19, 2025

fix comment

2274061

add comment

2e1e02b

baermat marked this pull request as ready for review November 19, 2025 15:28

baermat requested a review from bentsku as a code owner November 19, 2025 15:28

bentsku reviewed Nov 19, 2025

View reviewed changes

baermat added 2 commits November 20, 2025 15:22

work in small suggestions

cec12dc

remove duplicate publisher

81b1b12

baermat commented Nov 20, 2025

View reviewed changes

baermat requested a review from bentsku November 20, 2025 14:41

fix test

e8005bf

bentsku approved these changes Nov 20, 2025

View reviewed changes

baermat merged commit 3eb61b9 into main Nov 20, 2025
43 checks passed

baermat deleted the sns/v2-publish branch November 20, 2025 17:48

+if not (topic := store.topics.get(topic_arn)):
+    raise NotFoundException("Topic does not exist")
+topic_subscriptions = topic["subscriptions"]

Uh oh!

Sns:v2 publish #13399

Sns:v2 publish #13399

Uh oh!

Conversation

baermat commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

Tests

Related

Uh oh!

github-actions bot commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results - Preflight, Unit

Uh oh!

github-actions bot commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results - Alternative Providers

Uh oh!

github-actions bot commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results (amd64) - Acceptance

Uh oh!

github-actions bot commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results (amd64) - Integration, Bootstrap

Uh oh!

github-actions bot commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LocalStack Community integration with Pro

Uh oh!

bentsku left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bentsku left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

baermat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bentsku left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

baermat commented Nov 19, 2025 •

edited

Loading

github-actions bot commented Nov 19, 2025 •

edited

Loading

github-actions bot commented Nov 19, 2025 •

edited

Loading

github-actions bot commented Nov 19, 2025 •

edited

Loading

github-actions bot commented Nov 19, 2025 •

edited

Loading

github-actions bot commented Nov 19, 2025 •

edited

Loading