Skip to content

SNS: v2 permission operations #13516

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

Jump to bottom
Merged
baermat merged 1 commit into from
Dec 15, 2025
Merged

SNS: v2 permission operations #13516

baermat merged 1 commit into from
Dec 15, 2025

Conversation

@baermat
Copy link
Member

@baermat baermat commented Dec 12, 2025 •
edited
Loading

Motivation

This PR adds permission operations. The operations only add and remove in a CRUD manner, we do not actually act upon the policies (as we do not right now in the current implementation as far as I could tell).

closes PNX-73

Changes

  • add add-permisison, remove-permission
  • add tests to cover everything that was covered in moto

Tests

Related

@baermat 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 Dec 12, 2025
@baermat baermat marked this pull request as ready for review December 12, 2025 09:25
@baermat baermat requested a review from bentsku as a code owner December 12, 2025 09:25
@github-actions
Copy link

github-actions bot commented Dec 12, 2025

Test Results - Preflight, Unit

23 001 tests  ±0   21 158 ✅ ±0   6m 25s ⏱️ +7s
     1 suites ±0    1 843 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 4246fb5. ± Comparison against base commit d6882f9.

@github-actions
Copy link

github-actions bot commented Dec 12, 2025

Test Results - Alternative Providers

207 tests   163 ✅  2m 8s ⏱️
  1 suites   44 💤
  1 files      0 ❌

Results for commit 4246fb5.

@github-actions
Copy link

github-actions bot commented Dec 12, 2025

Test Results (amd64) - Acceptance

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

Results for commit 4246fb5. ± Comparison against base commit d6882f9.

@github-actions
Copy link

github-actions bot commented Dec 12, 2025

LocalStack Community integration with Pro

    2 files      2 suites   1h 1m 21s ⏱️
3 115 tests 2 943 ✅ 172 💤 0 ❌
3 121 runs  2 947 ✅ 174 💤 0 ❌

Results for commit 4246fb5.

@github-actions
Copy link

github-actions bot commented Dec 12, 2025

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   1h 30m 16s ⏱️
3 136 tests 2 967 ✅ 169 💤 0 ❌
3 148 runs  2 973 ✅ 175 💤 0 ❌

Results for commit 4246fb5.

bentsku
bentsku approved these changes Dec 12, 2025
View reviewed changes
Copy link
Contributor

@bentsku bentsku left a comment

Choose a reason for hiding this comment

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

LGTM! Nice and clean, with good test coverage 💯
Pretty cool interface to manage permissions tbh! Much easier thant to fully post it 😄

we do not actually act upon the policies (as we do not right now in the current implementation as far as I could tell).

This is managed by IAM directly via its plugins 👍 it calls get_topic_attributes(TopicArn=arn).get("Attributes", {}).get("Policy")

@baermat baermat merged commit bad8808 into main Dec 15, 2025
55 of 58 checks passed
@baermat baermat deleted the sns/v2-permission-operations branch December 15, 2025 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bentsku bentsku bentsku approved these changes

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@baermat @bentsku