fix(sns): apply an SNS subscription attribute edit in place instead of replacing it (#4853)

[go-to-k]

Summary

Editing a mutable attribute of an AWS::SNS::Subscription (RawMessageDelivery, FilterPolicy, FilterPolicyScope, RedrivePolicy, DeliveryPolicy, ReplayPolicy, SubscriptionRoleArn) made cdkd deploy unsubscribe and subscribe again. The subscription got a new ARN, messages published in the gap were not delivered, and an email / http(s) endpoint went back to PendingConfirmation. CloudFormation applies these with SetSubscriptionAttributes, in place.

SNSSubscriptionProvider.update now:

• reads the subscription (GetSubscriptionAttributes) and plans against what AWS holds, never previousProperties. Three of the four update() callers (deploy, rollback revert, --revert-failed) pass the state record as the previous side, which can equal the desired bag while AWS drifted, and a cdkd drift --revert hands the record as both bags;
• sets only the attributes that differ, one SetSubscriptionAttributes call each. Policies compare parsed; an attribute AWS omits counts as its default, so an email subscription is never sent RawMessageDelivery (AWS refuses it on that protocol);
• resets an attribute dropped from the template to its default: RawMessageDelivery -> false, FilterPolicy / RedrivePolicy -> "", FilterPolicyScope -> MessageAttributes (after the policy). A scope is NOT reset on a subscription left with no policy: it filters nothing there, and SNS may refuse to set one without a policy, which would fail every later deploy;
• keeps the DELETE-first replacement, unchanged with its fix(provisioning): six more delete-result discarders outside the deploy engine — provider delegation plus four REPLACE paths that orphan or duplicate the old resource #1778 / fix(sns): a THROWN delete during a subscription replacement still creates, leaving two live subscriptions #1967 abort, for:
  • a create-only field (TopicArn / Protocol / Endpoint) differing from AWS. The engine routes such a template change to its own replacement, so this arm is a rollback revert or a record naming another subscription;
  • a subscription AWS reports PendingConfirmation: true (an unconfirmed email / http(s) endpoint), which loses nothing by being re-created;
  • the removal of DeliveryPolicy, ReplayPolicy or SubscriptionRoleArn, which have no documented reset;
  • a new (non-empty) FilterPolicy and a FilterPolicyScope change in the same plan, whatever the template declares (a scope dropped from the template resets too; security review S1). Subscribe applies them together, while two calls would evaluate the new policy against the old scope in between, and a failed second call would leave a pair neither side asked for. Neither order closes that gap, since a scope set first is refused against the old policy's shape;
  • the PendingConfirmation placeholder id, a subscription AWS no longer has (NotFound), and a readback with no attributes;
• fails the update, without replacing, on any other readback or SetSubscriptionAttributes failure. The AWS text is masked through context.maskSecrets (wrapMaskedAwsError through wrapMaskedAwsCallError, cause kept). The provider now declares UpdateContext, so it leaves the OMITS_UPDATE_CONTEXT worklist (scripts/check-provider-update-context.ts);
• on the replacement arm, two pre-existing gaps are closed (security review N2, N3). create() now takes a CreateContext and masks the Subscribe error text: AWS can quote a rejected FilterPolicy holding a resolved secret back, and replace() hands it the update's masker, never replayingState. replace() also hands its delete the record's expectedRegion, so a NotFound from another region is refused by assertRegionMatch rather than read as already gone. Every update error path is masked now, and earlier text in this PR body said so too soon: before this round, the replacement arm's Subscribe error was not masked.

The delete / replace safety logic (#1778, #1967) is not modified; it is now reached only by the arms above.

Tests

• New tests/unit/provisioning/sns-subscription-in-place-update-4853.test.ts (16 cases), red before the change:
  • in-place set for RawMessageDelivery, FilterPolicy and SubscriptionRoleArn;
  • a drift revert with equal bags converges against AWS;
  • a no-op when AWS already matches;
  • reset to default with ordering;
  • a default not reset when AWS omits it;
  • each replacement arm;
  • a readback failure and a set failure that do not replace, with the message masked;
  • a new policy meeting a dropped scope replaces (S1). The case resets a dropped FilterPolicyScope while a policy stays was flipped to that. The scope-alone reset and the empty-policy exemption are pinned too;
  • a secret in the replacement's Subscribe error is masked (N2), and a NotFound with the record in another region refuses the replacement (N3).
• Updated: sns-subscription-provider.test.ts, sns-subscription-provider-roundtrip.test.ts, replace-path-delete-skip-outcome.test.ts, sns-replace-abort-rollback-caller.test.ts, sns-subscription-thrown-delete.test.ts. The fix(provisioning): six more delete-result discarders outside the deploy engine — provider delegation plus four REPLACE paths that orphan or duplicate the old resource #1778 / fix(sns): a THROWN delete during a subscription replacement still creates, leaving two live subscriptions #1967 abort cases now reach the replacement through an explicit create-only difference in the readback (the reachable path), and the thrown-delete discriminator asserts the single readback. The round-trip cases assert the in-place calls.
• Mutation probes, one at a time with the tree restored byte-exact. Each of these turned a named case red:
  • always replace;
  • placeholder guard off;
  • NotFound not replaced;
  • a readback error replacing;
  • an empty readback treated as in-place;
  • the create-only check off;
  • comparing against previousProperties;
  • always setting;
  • parsed-JSON compare off;
  • "absent equals default" off;
  • no reset on removal;
  • an unresettable removal not replaced;
  • resetting at the default;
  • the FilterPolicy + scope guard off;
  • the scope ordered before the policy;
  • the set-error mask off;
  • wasReplaced: true;
  • the Arn attribute dropped;
  • the no-policy scope skip off;
  • the PendingConfirmation check off;
  • the S1 guard keyed back on the template, off, or without its empty-policy exemption;
  • create()'s mask off, or replace() not passing the masker (N2);
  • replace() not forwarding expectedRegion (N3);
  • the planned-policy check ignored.

Integration

Arm in the existing tests/integration/sns-subscription-update/ fixture. No open PR holds it; noecho-named-producer (#4850) is untouched.

• Phase 1: creates the subscription with RawMessageDelivery: false and FilterPolicy {kind:[a]}.
• Phase 2 (CDKD_TEST_UPDATE=raw-delivery,filter-b): RawMessageDelivery true and FilterPolicy {kind:[b]} reach AWS on the SAME subscription. The ARN, state physicalId, attributes.Arn and the Fn::GetAtt Output are unchanged, and the topic holds exactly one subscription. A binary without this change is red here: the ARN changes.
• Phase 2a (+body-scope): FilterPolicyScope MessageBody alone, applied in place, same ARN.
• Phase 2b (+drop-filter): FilterPolicy and its scope removed from the template. The policy is cleared on the live subscription and the scope is left, same ARN.
• Phase 3: a malformed recorded ARN. The in-place readback is refused (could not read its attributes, with SNS's Invalid parameter as an independent sentinel), and no Unsubscribe or Subscribe is issued. The fix(sns): a THROWN delete during a subscription replacement still creates, leaving two live subscriptions #1967 thrown-delete abort sits on the replacement path, which an attribute edit no longer takes; the unit tests pin it.
• Phase 4: repair and destroy, unchanged.

Not run against real AWS. src/provisioning/providers/** is in the integ-destroy marker scope, so a clean /run-integ is needed before merge. Suggested: sns-subscription-update, plus sns-subscription-filter, sns-inline-subscription, sns-pending-subscription, sns-sqs-event.

Closes #4853

[go-to-k]

Merged — thanks. Planning against the live readback rather than previousProperties was the right call for the revert callers, and the security round (plan-keyed policy+scope guard, masked replacement errors, region forwarded) closed every gap it raised; all five SNS fixtures passed and the pre-fix arm went red at Phase 2.