Repository navigation
fix(sns): clear subscription RedrivePolicy and FilterPolicy like AWS - #350
Conversation
…ed value Real AWS rejects SetSubscriptionAttributes with RedrivePolicy set to an empty string; the policy is removed by leaving AttributeValue out. fauxqs accepted the empty string and stored it, so code that worked against fauxqs failed on AWS.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 39 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (8)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughSNS subscription attribute updates now resolve omitted and empty policy values by attribute name. Undefined values remove stored attributes. Empty ChangesRedrive policy updates
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to This change aligns SNS subscription attribute clearing with the AWS behavior it targets. No merge-blocking risk was identified in the supplied changes. Architecture SummaryArchitecture risk: 🟡 Medium · up to The change affects 2 systems. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
Reliability and maintainability
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
AWS drops FilterPolicy from GetSubscriptionAttributes after it is set to an empty string; fauxqs kept returning "". Delivery already treated the empty value as no filter.
kibertoad
changed the title
…moval Pin the parse error in the empty RedrivePolicy tests, add a Subscribe case for the empty value, and document in the README that the subscription policy is removed by omitting AttributeValue.
…ibuteName - Subscribe validation and normalization moved into SnsStore.subscribe, so server.subscribe() and init config reject an empty or malformed RedrivePolicy like the Subscribe API does. - Subscribe with FilterPolicy "" stores no policy, matching SetSubscriptionAttributes, so a later Subscribe without attributes is idempotent instead of failing with "different attributes". - SetSubscriptionAttributes without AttributeName now fails with InvalidParameter, as on AWS, instead of returning success. - Empty FilterPolicy and RedrivePolicy values persisted by earlier versions are dropped on load and the row is rewritten.
zizmor 1.30 adds the self-repository audit, which fails on uses: ./... for in-repo reusable workflows. The $/ form is resolved by GitHub rather than from the runner's checkout.
2 participants
fauxqs clears subscription policies differently from real AWS. It accepts
SetSubscriptionAttributeswithRedrivePolicy: "", which AWS rejects withInvalidParameter. It also keeps returningFilterPolicy: ""after that value is set, while AWS drops the attribute. This PR makes both follow AWS.Evidence
putSubscriptionAttribute: "The AWS API requires a non-empty string value or nil for the RedrivePolicy attribute, else throws an InvalidParameter error." Its acceptance testTestAccSNSTopicSubscription_redrivePolicyremoves the policy against real AWS by sending no value.AttributeValueas optional.FilterPolicy, LocalStack'stest_filter_policyis markedaws.validated, meaning it was also run against real AWS. It setsAttributeValue: ""and waits untilFilterPolicyis gone fromGetSubscriptionAttributes.This came up in message-queue-toolkit#618, where clearing with
""passed on fauxqs and would have failed on AWS.Changes
validateSubscriptionRedrivePolicyno longer allows"", so it fails with the existing "unable to parse RedrivePolicy as JSON" error. This also applies to aRedrivePolicy: ""passed toSubscribe.SetSubscriptionAttributesforRedrivePolicywithout anAttributeValuedeletes the attribute, soGetSubscriptionAttributesno longer returns it. Before, the missing value was turned into""and stored.SetSubscriptionAttributeswith an empty or omittedFilterPolicydeletes the attribute instead of storing"". Delivery is unchanged, as an empty value already meant no filter.Not covered
GetSubscriptionAttributesafter aRedrivePolicyremoval (absent or"") is not documented. Absent matches how a subscription that never had a policy looks.FilterPolicy: "{}"is still stored as given. The SNS docs use it to remove a filter policy, but whatGetSubscriptionAttributesreturns afterwards is not documented.SetQueueAttributeswith an emptyRedrivePolicyis unchanged. That one is valid on AWS.Summary by CodeRabbit
RedrivePolicy; settingFilterPolicyto an empty string also removes it.RedrivePolicyvalue is now rejected as invalid.