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

SNS notifications on stack events #8581

Open
logemann opened this issue Jun 16, 2020 · 19 comments · Fixed by mannjaro/serverless-bedrock-proxy#2 · 4 remaining pull requests
Open

SNS notifications on stack events #8581

logemann opened this issue Jun 16, 2020 · 19 comments · Fixed by mannjaro/serverless-bedrock-proxy#2 · 4 remaining pull requests
Assignees
Labels
@aws-cdk/core Related to core CDK functionality effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1 package/tools Related to AWS CDK Tools or CLI

Comments

@logemann
Copy link

logemann commented Jun 16, 2020

❓ General Issue

I am playing around with different ways to hook into lifecycle events of Stacks / Components. Of course there is the AwsCustomResource or ProviderFramework way. While AwsCustomResource is only viable for a simple SDK calls, ProviderFramework offers a lot more but with more complexity involved.

Now i noticed notificationArns on Stack level. What i recall you can attach them via command line on deployment (not what i am looking for) or programatically via NestedStack Construct. I wonder why this requirement is in place? Why cant we add them on our main stacks? (i think you can only read them)

This way one could easily setup a topic and a consumer and one should be able to hook into those lifecycle events pretty easily.

proposed idea

Why not having a method like this on Stack construct:

let myStack = new MyStack(app,...);
myStack.addNotificationArns(...);

Not sure why this method is not available in the construct when at the end it will synth to AWS::CloudFormation::Stack

@logemann logemann added the needs-triage This issue or PR still needs to be triaged. label Jun 16, 2020
@SomayaB SomayaB added @aws-cdk/core Related to core CDK functionality guidance Question that needs advice or information. labels Jun 17, 2020
@eladb eladb changed the title why notificationArns programatically only available for nested stacks? SNS notifications on stack events Jun 22, 2020
@eladb eladb added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. and removed guidance Question that needs advice or information. labels Jun 22, 2020
@eladb
Copy link
Contributor

eladb commented Jun 22, 2020

Relabeled as feature. This is something we should be able to support.

@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Jun 22, 2020
@eladb eladb added package/tools Related to AWS CDK Tools or CLI p2 labels Aug 17, 2020
@eladb eladb assigned rix0rrr and unassigned eladb Aug 17, 2020
@yashghatti
Copy link

yashghatti commented Oct 9, 2020

@rix0rrr any update on this?

@AustinZhu
Copy link

+1

@domfie
Copy link

domfie commented Dec 22, 2020

AFAIK: Currently only possible through the cli:
cdk deploy stackID --notification-arns='arn:aws:sns:{region}:{aws-id}:{sns-name}'
(Replace the placeholders with your values)

On subsequent calls the arn can be omitted.

@logemann
Copy link
Author

logemann commented Jan 5, 2021

Thats why it would make up a good feature Request. Think should be part of the AWS CDK.

@rix0rrr rix0rrr removed their assignment Jun 3, 2021
@kornicameister
Copy link
Contributor

Wouldn't this be a bit of chicken & egg problem? I mean something gotta provide SNS topic?

@domfie
Copy link

domfie commented Sep 30, 2021

Wouldn't this be a bit of chicken & egg problem? I mean something gotta provide SNS topic?

Not really since you first need to deploy the sns with another stack or create it by hand. A check if this sns exists and a corresponding error message should not be the problem

@kornicameister
Copy link
Contributor

Given that it seems to be already supported on nested stack it shouldn't bee to much of a trouble to add this for top-level stacks, right?

@shubhamk1997
Copy link

Any updates on this ? I agree this should be a part of the Stack construct rather than the NestedStack construct.

@tedgonzalez
Copy link

+1

@driseley
Copy link

Now that this has been added to the Security Best Practices:

https://docs.aws.amazon.com/securityhub/latest/userguide/securityhub-standards-fsbp-controls.html#fsbp-cloudformation-1

it would be great to see better support for this.

As a simpler fix, could the notification arns be configurable in the cdk.json file, so that defaults can be configured without having to always supply them on the command line for each new stack:

https://docs.aws.amazon.com/cdk/v2/guide/cli.html#cli-config

@antonio-masotti
Copy link

Is there an update on this? Both the integration in the CDK or in the context file seem very good features for me.

It would be great, especially for projects where many devs are working on the same cdk app.

@github-actions github-actions bot added p1 and removed p2 labels Dec 18, 2022
@github-actions
Copy link

This issue has received a significant amount of attention so we are automatically upgrading its priority. A member of the community will see the re-prioritization and provide an update on the issue.

@RajasGujarathi
Copy link

This is definitely a needed feature.

@iamgabeortiz
Copy link

+1

@tim-g-provectusalgae
Copy link

Yes, this is being picked up by KICS as a MEDIUM severity issue and our pipelines are failing. We will have to outright ignore CloudFormation.1, which is a against AWS Security Best Practices.

@nickroberts
Copy link

Any word on this? Or a workaround?

@shazib-summar
Copy link

Its been 2 and a half years and still no update on this????

@markusl
Copy link
Contributor

markusl commented Apr 3, 2024

Hey! Is this on the roadmap? We'd like to use this in certain stacks which have a security impact

@comcalvi comcalvi self-assigned this Jun 12, 2024
mergify bot pushed a commit that referenced this issue Aug 13, 2024
… construct (#30551)

### Issue # (if applicable)

#8581.

### Reason for this change

It is easier and clearer to specify the SNS Topic ARNs on the stack construct itself instead of passing it as a command line argument.

### Description of changes

Added a new optional stack prop, `notificationArns`, that is written to the CloudAssembly and concatenated with the CLI option `--notification-arns`. 

When I added CLI integ tests, I discovered that the existing framework is unable to use your local code. It always retrieves the latest release, which is not what you want when running it locally. This fixes that. 

Don't forget to select stacks by hierarchical ID (currently display name, in our tests) when writing certain test code. Otherwise, the tests may not select the stack you expected. 

### Description of how you validated changes

Unit tests + CLI integ test.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
mergify bot pushed a commit that referenced this issue Sep 23, 2024
…uct (#31107)

### Issue # (if applicable)

#8581.

### Reason for this change

It is easier and clearer to specify the SNS Topic ARNs on the stack construct itself instead of passing it as a command line argument.

### Description of changes

Added a new optional stack prop, `notificationArns`, that is written to the CloudAssembly and concatenated with the CLI option `--notification-arns`. 

Don't forget to select stacks by hierarchical ID (currently display name, in our tests) when writing certain test code. Otherwise, the tests may not select the stack you expect.

Depends on: cdklabs/cdk-assets#87 and cdklabs/cloud-assembly-schema#58.

### Description of how you validated changes

Unit tests + CLI integ test. Framework integ tests not included because they would require an externally-created SNS Topic, which is not what we want in integ tests; besides, the case is covered by the CLI integ test. 

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
michelle-wangg pushed a commit to michelle-wangg/aws-cdk that referenced this issue Sep 23, 2024
…uct (aws#31107)

### Issue # (if applicable)

aws#8581.

### Reason for this change

It is easier and clearer to specify the SNS Topic ARNs on the stack construct itself instead of passing it as a command line argument.

### Description of changes

Added a new optional stack prop, `notificationArns`, that is written to the CloudAssembly and concatenated with the CLI option `--notification-arns`. 

Don't forget to select stacks by hierarchical ID (currently display name, in our tests) when writing certain test code. Otherwise, the tests may not select the stack you expect.

Depends on: cdklabs/cdk-assets#87 and cdklabs/cloud-assembly-schema#58.

### Description of how you validated changes

Unit tests + CLI integ test. Framework integ tests not included because they would require an externally-created SNS Topic, which is not what we want in integ tests; besides, the case is covered by the CLI integ test. 

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment