core: cdk deploy wiping stack notification ARNs property

wilhen01 commented 3 hours ago

Describe the bug

A change was introduced in v2.160.0 which appears to be wiping the notificationARNs property on a CloudFormation stack when cdk deploy is run against an existing stack with that property set. This represents a regression on previous behaviour.

Regression Issue

[X] Select this option if this issue appears to be a regression.

Last Known Working CDK Version

2.159.1

Expected Behavior

cdk deploy does not modify the notificationARNs property on existing deployed stacks

Current Behavior

cdk deploy wipes the notificationARNs property where it is set on existing deployed stacks

Reproduction Steps

Deploy a stack. Modify the notificationARNs property outside of CDK. Then deploy a subsequent update via cdk deploy

Possible Solution

No response

Additional Information/Context

We have an internal tooling platform which stacks can be registered with in order to provide deployment updates. In order to do this, the tool sets the notificationARNs property on the stack to listen to stack updates via SNS. Previously updating these stacks via cdk deploy did not conflict with that, but it does in recent versions.

CDK CLI Version

v2.167.1

Framework Version

No response

Node.js Version

18

OS

Mac OS

Language

TypeScript

Language Version

5.x

Other information

No response

pahud commented 2 hours ago

Hi

Looks like the PR you mentioned essentially added a new notificationArns attribute which didn't exist prior to that PR.

Did you mean you actually had notificationArns attribute in 2.159.1 but it was wiped out in 2.160.0 ?

pahud commented 2 hours ago

trying to create a minimal sample to verify it now.

mrgrain commented 2 hours ago

If I read this correctly, the scenario is:

deploy stack with v2.159.1, do not set notificationArns, do not use --notification-arns
manually add Notification ARNs via the CFN console
deploy stack with v2.160.0, still do not set notificationArns, still do not use --notification-arns
see how the manually added Notification ARNs are removed

Or in other words, with v2.159.1 and before you could "tell" the CDK to just not care about Notification ARNs at all. But since v2.160.0, the CDK got greedy and wants to always own Notification ARNs even if they are empty.

If this is confirmed, the code bug is probably the difference between not setting a parameter (old) and passing an empty array to the parameter (new).

pahud commented 1 hour ago

HI @wilhen01

I am assuming your code

export class MyStack extends cdk.Stack {
  constructor(scope: cdk.App, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    // Cfnout notificationArns
    new cdk.CfnOutput(this, 'NotificationARNs', {
      value: cdk.Fn.select(0, this.notificationArns),
    });
  }
}

in 2.159.1

you probably deploy like this

npx cdk deploy --notification-arns arn:aws:sns:us-east-1:123456789012:NotifyMe

And you see this

dummy-stack2.NotificationARNs = arn:aws:sns:us-east-1:123456789012:NotifyMe

Now, when you upgrade to 2.160.0 without changing any code.

From my test if you continue deploy with that, you should still see that output. The behavior won't change.

Can you tell us how did you deploy that with the notificationArn in 2.159.1? From what I can see the --notification-arns should be the only way you can specify that in CDK 2.159.1.

wilhen01 commented 1 hour ago

@mrgrain's summary is accurate @pahud - the stacks we're experiencing this problem with don't specify noticationArns at all. Basically the flow is:

Deploy stack via cdk deploy with no specification of NotificationARNs
Register stack with my organisation's internal tooling - this populates NotificationARNs in order to listen to stack updates
Deploy updates to stack via cdk deploy, again with no specification of NotificationARNs

prior to 2.160.0, the notification ARNs added by the internal tooling platform would be maintained across subsequent stack updates via cdk deploy. From 2.160.0 onwards, that is no longer the case.

aws / aws-cdk