feat(events-targets): target systems manager run command and automation with event rules #22636
Conversation
|
|
github-actions
bot
added
beginning-contributor
aws-cdk-automation
left a comment
aws-cdk-automation
left a comment
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.
NathanTCz
force-pushed
the
feat/ssm-events-targets
branch
from
338d5f7 to
cb51bae
Compare
TheRealAmazonKendra
changed the title
TheRealAmazonKendra
left a comment
TheRealAmazonKendra
left a comment
There was a problem hiding this comment.
I only got part of the way though this review because I don't think this can be implemented at this time given that it will create a breaking change once Document is implemented as a resource for ssm and also because union types don't work with jsii so any further review of this PR will require extensive reworking or another PR implementing Document.
| /** | ||
| * Can be an instance of `ssm.CfnDocument` or a share/managed document ARN. | ||
| */ | ||
| public readonly document: ssm.CfnDocument | string, |
There was a problem hiding this comment.
Union types don't work with jsii.
There was a problem hiding this comment.
In fact, I don't think we want to use the Cfn type of anything here. When Document gets implemented, this will then create a breaking change. This should use an IDocument as the type. I realize that this isn't implemented yet but I think that it needs to be before we can accept this change.
There was a problem hiding this comment.
I had the exact same thought and started down that road.. but figured I would get your feedback first. Is there any existing or in-progress work already on implementing L2 constructs for Document? Is it possible to just wrap CfnDocument to avoid the breaking change down the road?
There was a problem hiding this comment.
I'd be willing to start the implementation of Document. Is there any documentation, standards, or requirements I can follow or reference for that implementation? Other than looking at other similar L2 implementations.
There was a problem hiding this comment.
One more thought.. once Document is implemented, there would be no need for the union type as theoretically (and hopefully) there would be some sort of .fromLookup() method on Document for shared documents.
There was a problem hiding this comment.
See this package: https://github.com/aws/aws-cdk/tree/main/packages/%40aws-cdk/example-construct-library
Specifically, the example resource, as that would be what the Document would be. Please also open that in a separate PR.
From the documentation it looks like this should be a pretty simple implementation.
mergify
bot
dismissed
TheRealAmazonKendra’s stale review
Pull request has been modified.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Blocked by #22911 |
|
This PR has been in the BUILD FAILING state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
|
Waiting on #22911 |
|
This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error. |
aws-cdk-automation
added
the
closed-for-staleness
3 participants
Added new Event Rule Targets:
SsmRunCommandSsmAutomationThis provides the ability to target Amazon EC2 Systems Manager Run Command and Automation Documents in Event Rules.
closes #7710
All Submissions:
New Features
yarn integto deploy the infrastructure and generate the snapshot (i.e.yarn integwithout--dry-run)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license