Skip to content
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

feat(core): configure Stack SNS notification ARNs on the Stack construct #31107

Merged
merged 69 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
48f5b63
working basic implementation
comcalvi Jun 11, 2024
f05b908
Merge branch 'main' of github.com:aws/aws-cdk into comcalvi/notificat…
comcalvi Jun 12, 2024
868cb1f
Merge branch 'main' of github.com:aws/aws-cdk into comcalvi/notificat…
comcalvi Jun 12, 2024
f5c930b
fix
comcalvi Jun 12, 2024
662697d
don't skip deploy if stack notifications arnrns have changed
comcalvi Jun 12, 2024
bc0afea
more changes
comcalvi Jun 12, 2024
b3a5716
cleanup
comcalvi Jun 12, 2024
4b88b03
most tests working
comcalvi Jun 13, 2024
af40c5e
toolkit tests
comcalvi Jun 13, 2024
ffaf14e
docs
comcalvi Jun 13, 2024
055d496
remove the bad cast, it is evil
comcalvi Jun 13, 2024
fc184cc
cleanup
comcalvi Jun 13, 2024
fac1245
Merge branch 'main' of github.com:aws/aws-cdk into comcalvi/notificat…
comcalvi Jun 13, 2024
5e606af
silly licenses
comcalvi Jun 13, 2024
180066b
newline\n...
comcalvi Jun 13, 2024
2bc9320
foo
comcalvi Jun 13, 2024
e9f9fa4
small change to trigger test pipeline
comcalvi Jun 14, 2024
48b8e49
destroy fix hopefully
comcalvi Jun 17, 2024
effc510
jest......
comcalvi Jun 17, 2024
8e7fc27
tests...jest....
comcalvi Jun 17, 2024
e3f2a5c
ok, fine then...
comcalvi Jun 17, 2024
8cd8f6e
logging for this at this point
comcalvi Jun 18, 2024
f605e1d
throw the error then
comcalvi Jun 18, 2024
8848d0a
type error
comcalvi Jun 18, 2024
c5201ed
man this is painful
comcalvi Jun 18, 2024
6c6616e
woowoowoowoo
comcalvi Jun 18, 2024
d5b9a80
selector
comcalvi Jun 19, 2024
c718b20
jest
comcalvi Jun 19, 2024
335d6dd
jest
comcalvi Jun 19, 2024
cbde013
jest
comcalvi Jun 19, 2024
e9b0ddf
jest
comcalvi Jun 19, 2024
8bd64ea
cloud assem
comcalvi Jun 20, 2024
83cc722
wowzers
comcalvi Jun 20, 2024
2c9884f
asm
comcalvi Jun 20, 2024
2539c9c
asm
comcalvi Jun 20, 2024
ad71122
asm
comcalvi Jun 20, 2024
45357e1
asm
comcalvi Jun 20, 2024
abd6dc4
finally fixed it......
comcalvi Jun 21, 2024
178e712
fixed
comcalvi Jun 21, 2024
4fbdaa4
done
comcalvi Jun 21, 2024
d1bf1a0
code teleports to a different line in the file
comcalvi Jun 21, 2024
67188db
toolkit test
comcalvi Jun 21, 2024
f00ed25
toolkit
comcalvi Jun 21, 2024
a20f0e7
Merge branch 'main' of github.com:aws/aws-cdk into comcalvi/notificat…
comcalvi Jun 24, 2024
c885502
Merge branch 'main' of github.com:aws/aws-cdk into comcalvi/notificat…
comcalvi Jul 2, 2024
c38e7a4
wowowo cli integ test
comcalvi Jul 2, 2024
f79368f
integ-fix
comcalvi Jul 2, 2024
b03209d
fix deploy all test
comcalvi Jul 2, 2024
22bb7e8
wowowo
comcalvi Jul 2, 2024
bf5deb9
fixing the cli test framework
comcalvi Jul 10, 2024
95e8dac
fix
comcalvi Jul 10, 2024
4331c91
fix the busted test
comcalvi Jul 12, 2024
f4c65a2
fix
comcalvi Jul 12, 2024
f704997
Merge branch 'main' of github.com:aws/aws-cdk into comcalvi/notificat…
comcalvi Jul 15, 2024
b1c116e
typo
comcalvi Aug 12, 2024
0bc224f
Merge branch 'main' of github.com:aws/aws-cdk into comcalvi/notificat…
comcalvi Aug 12, 2024
79b2e7d
changes:
comcalvi Aug 12, 2024
e933382
fix
comcalvi Aug 13, 2024
6b447a1
schema
comcalvi Aug 14, 2024
a428809
cli integ test conflicts
comcalvi Sep 10, 2024
817c959
build
comcalvi Sep 10, 2024
a40985c
conflicts
comcalvi Sep 18, 2024
f5aafad
deps + test
comcalvi Sep 18, 2024
8dd5ce9
bump cdk-assets
comcalvi Sep 18, 2024
b376437
Merge branch 'main' into comcalvi/notification-arns
comcalvi Sep 18, 2024
f45f4f6
Merge branch 'main' of github.com:aws/aws-cdk into comcalvi/notificat…
comcalvi Sep 19, 2024
8323e18
Merge branch 'main' of github.com:aws/aws-cdk into comcalvi/notificat…
comcalvi Sep 19, 2024
b8ad5f3
more version fun
comcalvi Sep 19, 2024
4977203
Merge branch 'main' into comcalvi/notification-arns
mergify[bot] Sep 23, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,12 @@ class BuiltinLambdaStack extends cdk.Stack {
}
}

class NotificationArnPropStack extends cdk.Stack {
constructor(parent, id, props) {
super(parent, id, props);
new sns.Topic(this, 'topic');
}
}
class AppSyncHotswapStack extends cdk.Stack {
constructor(parent, id, props) {
super(parent, id, props);
Expand Down Expand Up @@ -708,6 +714,10 @@ switch (stackSet) {
new DockerStack(app, `${stackPrefix}-docker`);
new DockerStackWithCustomFile(app, `${stackPrefix}-docker-with-custom-file`);

new NotificationArnPropStack(app, `${stackPrefix}-notification-arn-prop`, {
notificationArns: [`arn:aws:sns:${defaultEnv.region}:${defaultEnv.account}:${stackPrefix}-test-topic-prop`],
});

// SSO stacks
new SsoInstanceAccessControlConfig(app, `${stackPrefix}-sso-access-control`);
new SsoAssignment(app, `${stackPrefix}-sso-assignment`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
withCDKMigrateFixture,
withExtendedTimeoutFixture,
randomString,
withoutBootstrap,
} from '../../lib';

jest.setTimeout(2 * 60 * 60_000); // Includes the time to acquire locks, worst-case single-threaded runtime
Expand Down Expand Up @@ -276,9 +277,12 @@ integTest(
}),
);

// bootstrapping also performs synthesis. As it turns out, bootstrap-stage synthesis still causes the lookups to be cached, meaning that the lookup never
// happens when we actually call `cdk synth --no-lookups`. This results in the error never being thrown, because it never tries to lookup anything.
// Fix this by not trying to bootstrap; there's no need to bootstrap anyway, since the test never tries to deploy anything.
integTest(
'context in stage propagates to top',
withDefaultFixture(async (fixture) => {
withoutBootstrap(async (fixture) => {
await expect(
fixture.cdkSynth({
// This will make it error to prove that the context bubbles up, and also that we can fail on command
Expand Down Expand Up @@ -613,12 +617,13 @@ integTest(
);

integTest(
'deploy with notification ARN',
'deploy with notification ARN as flag',
withDefaultFixture(async (fixture) => {
const topicName = `${fixture.stackNamePrefix}-test-topic`;
const topicName = `${fixture.stackNamePrefix}-test-topic-flag`;

const response = await fixture.aws.sns.send(new CreateTopicCommand({ Name: topicName }));
const topicArn = response.TopicArn!;

try {
await fixture.cdkDeploy('test-2', {
options: ['--notification-arns', topicArn],
Expand All @@ -641,6 +646,31 @@ integTest(
}),
);

integTest('deploy with notification ARN as prop', withDefaultFixture(async (fixture) => {
const topicName = `${fixture.stackNamePrefix}-test-topic-prop`;

const response = await fixture.aws.sns.send(new CreateTopicCommand({ Name: topicName }));
const topicArn = response.TopicArn!;

try {
await fixture.cdkDeploy('notification-arn-prop');

// verify that the stack we deployed has our notification ARN
const describeResponse = await fixture.aws.cloudFormation.send(
new DescribeStacksCommand({
StackName: fixture.fullStackName('notification-arn-prop'),
}),
);
expect(describeResponse.Stacks?.[0].NotificationARNs).toEqual([topicArn]);
} finally {
await fixture.aws.sns.send(
new DeleteTopicCommand({
TopicArn: topicArn,
}),
);
}
}));

// NOTE: this doesn't currently work with modern-style synthesis, as the bootstrap
// role by default will not have permission to iam:PassRole the created role.
integTest(
Expand Down
22 changes: 21 additions & 1 deletion packages/@aws-cdk/cx-api/FEATURE_FLAGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ Flags come in three types:
| [@aws-cdk/custom-resources:logApiResponseDataPropertyTrueDefault](#aws-cdkcustom-resourceslogapiresponsedatapropertytruedefault) | When enabled, the custom resource used for `AwsCustomResource` will configure the `logApiResponseData` property as true by default | 2.145.0 | (fix) |
| [@aws-cdk/aws-s3:keepNotificationInImportedBucket](#aws-cdkaws-s3keepnotificationinimportedbucket) | When enabled, Adding notifications to a bucket in the current stack will not remove notification from imported stack. | 2.155.0 | (fix) |
| [@aws-cdk/aws-stepfunctions-tasks:useNewS3UriParametersForBedrockInvokeModelTask](#aws-cdkaws-stepfunctions-tasksusenews3uriparametersforbedrockinvokemodeltask) | When enabled, use new props for S3 URI field in task definition of state machine for bedrock invoke model. | 2.156.0 | (fix) |
| [@aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions](#aws-cdkaws-ecsreduceec2fargatecloudwatchpermissions) | When enabled, we will only grant the necessary permissions when users specify cloudwatch log group through logConfiguration | 2.159.0 | (fix) |

<!-- END table -->

Expand Down Expand Up @@ -134,7 +135,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou
"@aws-cdk/aws-ec2:ebsDefaultGp3Volume": true,
"@aws-cdk/aws-ecs:removeDefaultDeploymentAlarm": true,
"@aws-cdk/custom-resources:logApiResponseDataPropertyTrueDefault": false,
"@aws-cdk/aws-s3:keepNotificationInImportedBucket": false
"@aws-cdk/aws-s3:keepNotificationInImportedBucket": false,
"@aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions": true
}
}
```
Expand Down Expand Up @@ -1378,4 +1380,22 @@ When this feature flag is enabled, specify newly introduced props 's3InputUri' a
**Compatibility with old behavior:** Disable the feature flag to use input and output path fields for s3 URI


### @aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions

*When enabled, we will only grant the necessary permissions when users specify cloudwatch log group through logConfiguration* (fix)

Currently, we automatically add a number of cloudwatch permissions to the task role when no cloudwatch log group is
specified as logConfiguration and it will grant 'Resources': ['*'] to the task role.

When this feature flag is enabled, we will only grant the necessary permissions when users specify cloudwatch log group.


| Since | Default | Recommended |
| ----- | ----- | ----- |
| (not in v1) | | |
| 2.159.0 | `false` | `true` |

**Compatibility with old behavior:** Disable the feature flag to continue grant permissions to log group when no log group is specified


<!-- END details -->
4 changes: 2 additions & 2 deletions packages/@aws-cdk/cx-api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,12 @@
"semver": "^7.6.3"
},
"peerDependencies": {
"@aws-cdk/cloud-assembly-schema": "^36.0.5"
"@aws-cdk/cloud-assembly-schema": "^38.0.0"
},
"license": "Apache-2.0",
"devDependencies": {
"@aws-cdk/cdk-build-tools": "0.0.0",
"@aws-cdk/cloud-assembly-schema": "^36.0.24",
"@aws-cdk/cloud-assembly-schema": "^38.0.0",
"@aws-cdk/pkglint": "0.0.0",
"@types/jest": "^29.5.12",
"@types/mock-fs": "^4.13.4",
Expand Down
5 changes: 3 additions & 2 deletions packages/@aws-cdk/integ-runner/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,12 @@
},
"dependencies": {
"chokidar": "^3.6.0",
"@aws-cdk/cloud-assembly-schema": "^36.0.24",
"@aws-cdk/cloud-assembly-schema": "^38.0.0",
"@aws-cdk/cloudformation-diff": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"cdk-assets": "^2.154.0",
"@aws-cdk/aws-service-spec": "^0.1.24",
"cdk-assets": "^2.151.29",

"@aws-cdk/cdk-cli-wrapper": "0.0.0",
"aws-cdk": "0.0.0",
"chalk": "^4",
Expand Down
12 changes: 12 additions & 0 deletions packages/aws-cdk-lib/core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1242,6 +1242,18 @@ const stack = new Stack(app, 'StackName', {
});
```

### Receiving CloudFormation Stack Events

You can add one or more SNS Topic ARNs to any Stack:

```ts
const stack = new Stack(app, 'StackName', {
notificationArns: ['arn:aws:sns:us-east-1:23456789012:Topic'],
});
```

Stack events will be sent to any SNS Topics in this list.

### CfnJson

`CfnJson` allows you to postpone the resolution of a JSON blob from
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export function addStackArtifactToAssembly(
terminationProtection: stack.terminationProtection,
tags: nonEmptyDict(stack.tags.tagValues()),
validateOnSynth: session.validateOnSynth,
notificationArns: stack._notificationArns,
...stackProps,
...stackNameProperty,
};
Expand Down
22 changes: 22 additions & 0 deletions packages/aws-cdk-lib/core/lib/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,13 @@ export interface StackProps {
*/
readonly tags?: { [key: string]: string };

/**
* SNS Topic ARNs that will receive stack events.
*
* @default - no notfication arns.
*/
readonly notificationArns?: string[];

/**
* Synthesis method to use while deploying this stack
*
Expand Down Expand Up @@ -364,6 +371,13 @@ export class Stack extends Construct implements ITaggable {
*/
public readonly _crossRegionReferences: boolean;

/**
* SNS Notification ARNs to receive stack events.
*
* @internal
*/
public readonly _notificationArns: string[];

/**
* Logical ID generation strategy
*/
Expand Down Expand Up @@ -451,6 +465,14 @@ export class Stack extends Construct implements ITaggable {
}
this.tags = new TagManager(TagType.KEY_VALUE, 'aws:cdk:stack', props.tags);

for (const notificationArn of props.notificationArns ?? []) {
if (Token.isUnresolved(notificationArn)) {
throw new Error(`Stack '${id}' includes one or more tokens in its notification ARNs: ${props.notificationArns}`);
}
}

this._notificationArns = props.notificationArns ?? [];
comcalvi marked this conversation as resolved.
Show resolved Hide resolved

if (!VALID_STACK_NAME_REGEX.test(this.stackName)) {
throw new Error(`Stack name must match the regular expression: ${VALID_STACK_NAME_REGEX.toString()}, got '${this.stackName}'`);
}
Expand Down
26 changes: 26 additions & 0 deletions packages/aws-cdk-lib/core/test/stack.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2075,6 +2075,32 @@ describe('stack', () => {
expect(asm.getStackArtifact(stack2.artifactId).tags).toEqual(expected);
});

test('stack notification arns are reflected in the stack artifact properties', () => {
// GIVEN
const NOTIFICATION_ARNS = ['arn:aws:sns:bermuda-triangle-1337:123456789012:MyTopic'];
const app = new App({ stackTraces: false });
const stack1 = new Stack(app, 'stack1', {
notificationArns: NOTIFICATION_ARNS,
});

// WHEN
const asm = app.synth();

// THEN
expect(asm.getStackArtifact(stack1.artifactId).notificationArns).toEqual(NOTIFICATION_ARNS);
});

test('throws if stack notification arns contain tokens', () => {
// GIVEN
const NOTIFICATION_ARNS = ['arn:aws:sns:bermuda-triangle-1337:123456789012:MyTopic'];
const app = new App({ stackTraces: false });

// THEN
expect(() => new Stack(app, 'stack1', {
notificationArns: [...NOTIFICATION_ARNS, Aws.URL_SUFFIX],
})).toThrow('includes one or more tokens in its notification ARNs');
});

test('Termination Protection is reflected in Cloud Assembly artifact', () => {
// if the root is an app, invoke "synth" to avoid double synthesis
const app = new App();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ export class CloudFormationStackArtifact extends CloudArtifact {
*/
public readonly tags: { [id: string]: string };

/**
* SNS Topics that will receive stack events.
*/
public readonly notificationArns: string[];

/**
* The physical name of this stack.
*/
Expand Down Expand Up @@ -158,6 +163,7 @@ export class CloudFormationStackArtifact extends CloudArtifact {
// We get the tags from 'properties' if available (cloud assembly format >= 6.0.0), otherwise
// from the stack metadata
this.tags = properties.tags ?? this.tagsFromMetadata();
this.notificationArns = properties.notificationArns ?? [];
this.assumeRoleArn = properties.assumeRoleArn;
this.assumeRoleExternalId = properties.assumeRoleExternalId;
this.cloudFormationExecutionRoleArn = properties.cloudFormationExecutionRoleArn;
Expand Down
18 changes: 18 additions & 0 deletions packages/aws-cdk-lib/cx-api/test/stack-artifact.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,24 @@ afterEach(() => {
rimraf(builder.outdir);
});

test('read notification arns from artifact properties', () => {
// GIVEN
const NOTIFICATION_ARNS = ['arn:aws:sns:bermuda-triangle-1337:123456789012:MyTopic'];
builder.addArtifact('Stack', {
...stackBase,
properties: {
...stackBase.properties,
notificationArns: NOTIFICATION_ARNS,
},
});

// WHEN
const assembly = builder.buildAssembly();

// THEN
expect(assembly.getStackByName('Stack').notificationArns).toEqual(NOTIFICATION_ARNS);
});

test('read tags from artifact properties', () => {
// GIVEN
builder.addArtifact('Stack', {
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@
"@aws-cdk/asset-awscli-v1": "^2.2.202",
"@aws-cdk/asset-kubectl-v20": "^2.1.2",
"@aws-cdk/asset-node-proxy-agent-v6": "^2.1.0",
"@aws-cdk/cloud-assembly-schema": "^36.0.24",
"@aws-cdk/cloud-assembly-schema": "^38.0.0",
"@balena/dockerignore": "^1.0.2",
"case": "1.6.3",
"fs-extra": "^11.2.0",
Expand Down
10 changes: 10 additions & 0 deletions packages/aws-cdk/lib/api/deploy-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,12 @@ async function canSkipDeploy(
return false;
}

// Notification arns have changed
if (!arrayEquals(cloudFormationStack.notificationArns, deployStackOptions.notificationArns ?? [])) {
debug(`${deployName}: notification arns have changed`);
return false;
}

// Termination protection has been updated
if (!!deployStackOptions.stack.terminationProtection !== !!cloudFormationStack.terminationProtection) {
debug(`${deployName}: termination protection has been updated`);
Expand Down Expand Up @@ -694,3 +700,7 @@ function suffixWithErrors(msg: string, errors?: string[]) {
? `${msg}: ${errors.join(', ')}`
: msg;
}

function arrayEquals(a: any[], b: any[]): boolean {
return a.every(item => b.includes(item)) && b.every(item => a.includes(item));
}
11 changes: 10 additions & 1 deletion packages/aws-cdk/lib/api/util/cloudformation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,21 @@ export class CloudFormationStack {
/**
* The stack's current tags
*
* Empty list of the stack does not exist
* Empty list if the stack does not exist
*/
public get tags(): CloudFormation.Tags {
return this.stack?.Tags || [];
}

/**
* SNS Topic ARNs that will receive stack events.
*
* Empty list if the stack does not exist
*/
public get notificationArns(): CloudFormation.NotificationARNs {
return this.stack?.NotificationARNs ?? [];
}

/**
* Return the names of all current parameters to the stack
*
Expand Down
Loading
Loading