Sns topic update functionality by michalisfot · Pull Request #12777 · localstack/localstack · GitHub | Latest TMZ Celebrity News & Gossip | Watch TMZ Live
Skip to content

Sns topic update functionality #12777

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

michalisfot
Copy link

@michalisfot michalisfot commented Jun 19, 2025

Related PR: #12768

Motivation

This PR implements basic update support for the AWS::SNS::Topic CloudFormation resource, addressing the core requirement to enable updates for DisplayName, TopicName, and Tags properties. The implementation follows AWS CloudFormation's update semantics, including resource replacement when immutable properties change.

Changes

Enhanced Resource Provider (aws_sns_topic.py)

  • Implemented complete update() method with support for:

    • In-place updates: DisplayName attribute changes
    • Resource replacement: TopicName changes (recreates topic with new name)
    • Tag management: Add/remove/update tags with proper diffing
    • State preservation: Maintains subscriptions and existing tags during topic replacement
  • Key features:

    • Proper ARN handling and validation
    • Graceful error handling for subscription/tag preservation
    • Follows AWS update semantics (replacement vs. in-place)
    • Resource cleanup (deletes old topic after successful replacement)

Comprehensive Test Coverage (test_sns.py)

  • Created test_sns_topic_update_attributes() with in-place updates (no resource replacement):

    • DisplayName updates: Verifies DisplayName attribute changes are applied correctly
    • Tag updates: Tests adding, modifying, and removing tags
    • Same topic preservation: Ensures TopicArn remains unchanged during in-place updates
    • Snapshot validation: Captures before/after state for attribute and tag changes
  • Created test_sns_topic_update_name() with resource replacement:

    • Tag preservation validation: Verifies existing tags are preserved during replacement
    • Subscription preservation validation: Ensures subscriptions are migrated to new topic
    • ARN format validation: Builds and validates new ARN using proper AWS format
    • Resource replacement verification: Confirms old topic deletion and new topic creation
    • Comprehensive snapshots: Captures before/after state for all operations

Added new entries to the Snapshot File (test_sns.snapshot.json)

Testing

  • On AWS and ocally both test pass.

TODO

  • Test against AWS and find out why test_sns_topic_update_name test fails on AWS but not locally.
  • Find a proper way to validate the snapshot against the tags rather than manually checking each tag

Proposed Next Steps

1: Complete Core Properties

  1. Archive Policy Support
  2. Access Policy Support
  3. KMS Key Management
  4. FIFO Topic Properties
  5. ContentBasedDeduplication

2: Advanced Update Capabilities

  1. Subscription Management
  2. Data Protection Policy
  3. Delivery Policy Configuration

3: Robustness & Edge Cases

  1. Error Handling Enhancement - Improve rollback scenarios for failed updates
  2. Performance Optimizations

4: Testing

  1. Multi account and region testing
  2. Other types of testing as mentioned in the docs

@localstack-bot
Copy link
Collaborator

localstack-bot commented Jun 19, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@simonrw simonrw added the semver: patch Non-breaking changes which can be included in patch releases label Jun 19, 2025
@michalisfot
Copy link
Author

recheck

1 similar comment
@michalisfot
Copy link
Author

recheck

@michalisfot
Copy link
Author

I have read the CLA Document and I hereby sign the CLA.

@alexrashed
Copy link
Member

recheck

localstack-bot added a commit that referenced this pull request Jun 23, 2025
Copy link
Contributor

@simonrw simonrw left a comment

Choose a reason for hiding this comment

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

Thanks for posting the updates to this PR. I have left some follow ups on the previous PR and added some comments here. In general I am ok with this PR. I think we have captured a good part of the update implementation for SNS topics, including using CFn to manage the subscriptions.

I would be interested to capture the behaviour you had previously in a follow up where the subscriptions are created outside of CFn, however I think this use case is less common and therefore not as high a priority as the changes you've added here.

Thanks very much!

(I have requested changes again as I still have a couple of outstanding comments that have not been addressed. These are only minor though, I just would like your take on them before I approve).

Comment on lines +233 to +235
tag_dict = {tag["Key"]: tag["Value"] for tag in initial_tags["Tags"]}
assert tag_dict["Environment"] == "test"
assert tag_dict["Project"] == "localstack"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
tag_dict = {tag["Key"]: tag["Value"] for tag in initial_tags["Tags"]}
assert tag_dict["Environment"] == "test"
assert tag_dict["Project"] == "localstack"
snapshot.match("initial-tags", initial_tags)

We may need to add specific non-supported tags to the skip_snapshot_verify decorator at the top of the test function, as CFn adds its own tags to resources and we don't support that yet.

Copy link
Author

Choose a reason for hiding this comment

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

That was my initial plan. I couldn't make it work though because of how the Tags are formatted. I couldn't find a way to skip the verification based on the Key. Also, AWS creates the tags in a random order and this also affects the validation. My idea was to temporarily check the tags manually until we find a proper solution (which might be already there but I missed it). I checked other examples but the tags there were formatted differently e.g. in Lambda snapshots tags are all in a single dictionary (not a list of dicts like bellow).

E.g.:

        "Tags": [
          {
            "Key": "Project",
            "Value": "localstack"
          },
          {
            "Key": "aws:cloudformation:stack-name",
            "Value": "replaced-value"
          },
          {
            "Key": "Environment",
            "Value": "test"
          },
          {
            "Key": "aws:cloudformation:logical-id",
            "Value": "TestTopic"
          },
          {
            "Key": "aws:cloudformation:stack-id",
            "Value": "replaced-value"
          }
        ],

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 that's fair enough for now. Thanks for explaining.

Comment on lines +258 to +260
updated_tag_dict = {tag["Key"]: tag["Value"] for tag in updated_tags["Tags"]}
assert updated_tag_dict["Environment"] == "production"
assert updated_tag_dict["Project"] == "backend"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
updated_tag_dict = {tag["Key"]: tag["Value"] for tag in updated_tags["Tags"]}
assert updated_tag_dict["Environment"] == "production"
assert updated_tag_dict["Project"] == "backend"
snapshot.match("updated-tags", updated_tags)

Copy link
Author

Choose a reason for hiding this comment

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

@michalisfot michalisfot requested a review from simonrw July 3, 2025 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants

TMZ Celebrity News – Breaking Stories, Videos & Gossip

Looking for the latest TMZ celebrity news? You've come to the right place. From shocking Hollywood scandals to exclusive videos, TMZ delivers it all in real time.

Whether it’s a red carpet slip-up, a viral paparazzi moment, or a legal drama involving your favorite stars, TMZ news is always first to break the story. Stay in the loop with daily updates, insider tips, and jaw-dropping photos.

🎥 Watch TMZ Live

TMZ Live brings you daily celebrity news and interviews straight from the TMZ newsroom. Don’t miss a beat—watch now and see what’s trending in Hollywood.