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(events): ability to add cross-account targets #3323

Merged

Conversation

skinny85
Copy link
Contributor

This adds the ability to (correctly) reference targets in rules that are from a different account than the rule itself. Needed for things like cross-account CodePipeline source actions (which use events to detect changes).


Please read the contribution guidelines and follow the pull-request checklist.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@skinny85 skinny85 requested a review from eladb July 16, 2019 23:03
@skinny85 skinny85 requested review from RomainMuller and a team as code owners July 16, 2019 23:03
packages/@aws-cdk/aws-events/lib/rule.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-events/lib/rule.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-events/lib/rule.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-events/lib/rule.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-events/lib/rule.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-events/lib/rule.ts Outdated Show resolved Hide resolved
@skinny85 skinny85 force-pushed the feat/codecommit-pipeline-action-cross-account branch from 79356c0 to 856f451 Compare July 20, 2019 01:00
@skinny85
Copy link
Contributor Author

Updated with the latest comments & rebased to solve a conflict.

@skinny85 skinny85 self-assigned this Jul 22, 2019
@skinny85 skinny85 force-pushed the feat/codecommit-pipeline-action-cross-account branch from 398ea18 to 8a5402a Compare July 24, 2019 21:00
@skinny85
Copy link
Contributor Author

This PR is now ready. @rix0rrr @eladb please re-review!

Thanks,
Adam

Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

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

Looks great.

Update README please.

packages/@aws-cdk/aws-events/lib/rule.ts Show resolved Hide resolved
packages/@aws-cdk/aws-events/lib/rule.ts Show resolved Hide resolved
packages/@aws-cdk/aws-events/lib/target.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-events/lib/target.ts Outdated Show resolved Hide resolved
This adds the capability of adding a target to an event rule
that belongs to a different account than the rule itself.
@skinny85 skinny85 force-pushed the feat/codecommit-pipeline-action-cross-account branch from 8a5402a to 24a7f52 Compare August 1, 2019 01:02
Copy link
Contributor Author

@skinny85 skinny85 left a comment

Choose a reason for hiding this comment

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

Addressed all of the comments. @eladb can you do one final pass? (I understand you've already approved it, but this is quite an important feature, so I'd rather get one last confirmation that everything looks OK). Thanks!

packages/@aws-cdk/aws-events/lib/rule.ts Show resolved Hide resolved
packages/@aws-cdk/aws-events/lib/rule.ts Show resolved Hide resolved
packages/@aws-cdk/aws-events/lib/target.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-events/lib/target.ts Outdated Show resolved Hide resolved
@skinny85 skinny85 merged commit 3b794ea into aws:master Aug 1, 2019
@skinny85 skinny85 deleted the feat/codecommit-pipeline-action-cross-account branch August 1, 2019 19:59
eladb pushed a commit that referenced this pull request Aug 6, 2019
This adds the capability of adding a target to an event rule
that belongs to a different account than the rule itself.
Required for things like cross-account CodePipelines with source actions triggered by events.
mergify bot pushed a commit that referenced this pull request Aug 7, 2019
* chore: update package-lock.json

* feat(eks): define kubernetes resources

This change allows defining arbitrary Kubernetes resources within an EKS cluster.

* nice!

* update readme

* Update README.md

* feat(events): ability to add cross-account targets (#3323)

This adds the capability of adding a target to an event rule
that belongs to a different account than the rule itself.
Required for things like cross-account CodePipelines with source actions triggered by events.

* chore(ci): add mergify config file (#3502)

* chore: update jsii to 0.14.3 (#3513)

* fix(iam): correctly limit the default PolicyName to 128 characters (#3487)

Our logic for trimming the length of the default IAM policy name was not working,
as it wasn't updated when logicalId became a Token rather than a literate string,
and so it was never actually triggered
(it just checked that the display name of the Token was less than 128 characters,
which it always is).
The fix is to resolve the logical ID Token before applying the trimming logic.

Fixes #3402

* v1.3.0 (#3516)

See CHANGELOG

* fix: typo in restapi.ts (#3530)

* feat(ecs): container dependencies (#3032)

Add new addContainerDependencies method to allow for container dependencies
Fixes #2490

* feat(s3-deployment): CloudFront invalidation (#3213)

see #3106

* docs(core): findChild gets direct child only (#3512)

* doc(iam): update references to addManagedPolicy (#3511)

* fix(sqs): do not emit grants to the AWS-managed encryption key (#3169)

Grants on the `alias/aws/sqs` KMS key alias are not necessary since the
key will implicitly allow for it's intended usage to be fulfilled (as
opposed to how you have to manage grants yourself when using a
user-managed key instead).

This removes the statement that was generated using an invalid resource
entry.

Fixes #2794

* fix(lambda): allow ArnPrincipal in grantInvoke (#3501)

Fixes #3264 

I'm trying to allow a lambda function in another account to be able to invoke my CDK generated lambda function. This works through the CLI like so:

    aws lambda add-permission --function-name=myFunction --statement-id=ABoldStatement --action=lambda:InvokeFunction --principal=arn:aws:iam::{account_id}:role/a_lambda_execution_role

But CDK doesn't seem to allow me to add an ArnPrincipal doing something like this:

    myFunction.grantInvoke(new iam.ArnPrincipal(props.myARN))

With the error:

    Invalid principal type for Lambda permission statement: ArnPrincipal. Supported: AccountPrincipal, ServicePrincipal

This PR allows ArnPrincipal to be passed to lambda.grantInvoke.

There might be some additional validation required on the exact ARN as I believe only some ARNs are supported by lambda add-permission

* chore(contrib): remove API stabilization disclaimer

* fix(ssm): add GetParameters action to grantRead() (#3546)

* misc

* rename `KubernetesManifest` to `KubernetesResource` and `addResource`
* move AWS Auth APIs to `cluster.awsAuth` and expose `AwsAuth`
* remove the yaml library (we can just use a JSON stream)
* add support for adding accounts to aws-auth
* fix cluster deletion bug
* move kubctl app info to constants

* addManifest => addResource

* update test expectations

* add unit test for customresrouce.ref

* fix sample link
@NGL321 NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants