Skip to content

feat(aws-cloudwatch-actions): add ssm incidents as alarm action#21167

Merged
mergify[bot] merged 8 commits intoaws:mainfrom
callumthomson:master
Aug 10, 2022
Merged

feat(aws-cloudwatch-actions): add ssm incidents as alarm action#21167
mergify[bot] merged 8 commits intoaws:mainfrom
callumthomson:master

Conversation

@callumthomson
Copy link
Copy Markdown
Contributor

@callumthomson callumthomson commented Jul 15, 2022 •
edited
Loading

Finishes work originally contributed by @dosatos by addressing changes requested by @comcalvi in #20553

closes #20552

aws-byeldos and others added 2 commits May 31, 2022 13:59
@aws-byeldos
(aws-cloudwatch-actions): Add the possibility to have SSM Incident Ac…
a85da25 
…tion

This small PR will add SSM Incident action to cloudwatch alarm.
The arn format was taken from the UI console (under Incident Manager Response Plan)
@callumthomson
feat(aws-cloudwatch-actions): add ssm incidents as alarm action
e002868 
Finishes work originally contributed by @dosatos by addressing changes requested by @comcalvi in aws#20553
closes aws#20552
@gitpod-io
Copy link
Copy Markdown

gitpod-io bot commented Jul 15, 2022

@aws-cdk-automation aws-cdk-automation requested a review from a team July 15, 2022 15:00
@github-actions github-actions bot added the p2 label Jul 15, 2022
TheRealAmazonKendra
TheRealAmazonKendra previously requested changes Jul 15, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@TheRealAmazonKendra TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks great! I think we'll need an integration test for this, though. Could you please add one?

@dosatos dosatos mentioned this pull request Jul 19, 2022
Closed
@mergify mergify bot dismissed TheRealAmazonKendra’s stale review July 24, 2022 17:18

Pull request has been modified.

@callumthomson
Copy link
Copy Markdown
Contributor Author

callumthomson commented Jul 26, 2022

I've added an integration test using IntegTest, but I think more is needed. Could you please let me know if this is a step in the right direction, @TheRealAmazonKendra ?

TheRealAmazonKendra
TheRealAmazonKendra previously requested changes Jul 27, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@TheRealAmazonKendra TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@callumthomson Your integ test file is what we're looking for. We just need you to generate the expected template and assets. See our integration test guide.

I've also added a few comments inline regarding the specific errors in the build.

@mergify mergify bot dismissed TheRealAmazonKendra’s stale review August 9, 2022 19:55

Pull request has been modified.

@callumthomson
Copy link
Copy Markdown
Contributor Author

callumthomson commented Aug 9, 2022

@TheRealAmazonKendra I've added in the requested changes. Please let me know any further desired amendments.

TheRealAmazonKendra
TheRealAmazonKendra previously requested changes Aug 9, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@TheRealAmazonKendra TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment inline but it also looks like your test can use SSM Incident as alarm action isn't working because the partition is a reference in the actual output, but you've used a literal in your expected output. We just need to get this test working so the build passes. If you have any questions about this, please let me know.

@mergify mergify bot dismissed TheRealAmazonKendra’s stale review August 10, 2022 09:46

Pull request has been modified.

@callumthomson
Copy link
Copy Markdown
Contributor Author

callumthomson commented Aug 10, 2022

@TheRealAmazonKendra that seems to be working now, but there appears to have been an error in aws-appsync:

@aws-cdk/aws-appsync: Verifying integration test snapshots...
@aws-cdk/aws-appsync: /codebuild/output/src184008709/src/github.com/aws/aws-cdk/packages/@aws-cdk/aws-appsync/lib/graphqlapi.js:371
@aws-cdk/aws-appsync:             throw Error('API key expiration must be between 1 and 365 days.');
@aws-cdk/aws-appsync:             ^
@aws-cdk/aws-appsync: Error: API key expiration must be between 1 and 365 days.
@aws-cdk/aws-appsync:     at GraphqlApi.createAPIKey (/codebuild/output/src184008709/src/github.com/aws/aws-cdk/packages/@aws-cdk/aws-appsync/lib/graphqlapi.js:371:19)
@aws-cdk/aws-appsync:     at new GraphqlApi (/codebuild/output/src184008709/src/github.com/aws/aws-cdk/packages/@aws-cdk/aws-appsync/lib/graphqlapi.js:188:40)
@aws-cdk/aws-appsync:     at Object.<anonymous> (/codebuild/output/src184008709/src/github.com/aws/aws-cdk/packages/@aws-cdk/aws-appsync/test/integ.auth-apikey.js:23:13)
@aws-cdk/aws-appsync:     at Module._compile (internal/modules/cjs/loader.js:1085:14)
@aws-cdk/aws-appsync:     at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
@aws-cdk/aws-appsync:     at Module.load (internal/modules/cjs/loader.js:950:32)
@aws-cdk/aws-appsync:     at Function.Module._load (internal/modules/cjs/loader.js:790:12)
@aws-cdk/aws-appsync:     at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:75:12)
@aws-cdk/aws-appsync:     at internal/main/run_main_module.js:17:47

@TheRealAmazonKendra
Copy link
Copy Markdown
Contributor

TheRealAmazonKendra commented Aug 10, 2022

@Mergifyio update

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Aug 10, 2022

update

✅ Branch has been successfully updated

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Aug 10, 2022

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

aws-cdk-automation commented Aug 10, 2022

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: ee7e8b8
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit 471511e into aws:main Aug 10, 2022
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Aug 10, 2022

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TheRealAmazonKendra TheRealAmazonKendra TheRealAmazonKendra approved these changes

Assignees

No one assigned

Labels

p2

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

(aws-cloudwatch-actions): Add the possibility to have SSM Incident Action

4 participants

@callumthomson @TheRealAmazonKendra @aws-cdk-automation @aws-byeldos