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(aws-codepipeline): #2572 allow IAM Role to be passed to Pipeline #2573

Closed
wants to merge 5 commits into from
Closed

feat(aws-codepipeline): #2572 allow IAM Role to be passed to Pipeline #2573

wants to merge 5 commits into from

Conversation

AlexChesters
Copy link
Contributor

@AlexChesters AlexChesters commented May 17, 2019

Pull Request Checklist

  • [N/A] Testing
    • As this feature is very similar to the existing pattern of artifacts bucket, which isn't unit tested as far as I can see, I didn't add, modify or remove any existing unit tests.
  • Docs
    • jsdocs: All public APIs documented
    • README: README and/or documentation topic updated
    • Design: For significant features, design document added to design folder
  • Title and Description
    • Change type: title prefixed with fix, feat and module name in parens, which will appear in changelog
    • Title: use lower-case and doesn't end with a period
    • Breaking?: last paragraph: "BREAKING CHANGE: <describe what changed + link for details>"
    • Issues: Indicate issues fixed via: "Fixes #xxx" or "Closes #xxx"
  • [N/A] Sensitive Modules (requires 2 PR approvers)
    • IAM Policy Document (in @aws-cdk/aws-iam)
    • EC2 Security Groups and ACLs (in @aws-cdk/aws-ec2)
    • Grant APIs (only if not based on official documentation with a reference)

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

@AlexChesters AlexChesters changed the title feat(aws-codepipeline): #2572 allow IAM Role to be passed to CodePipe… feat(aws-codepipeline): #2572 allow IAM Role to be passed to Pipeline May 17, 2019
Copy link
Contributor

@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.

Thanks for your contribution @AlexChesters ! 2 small comments before I merge this in.

assumedBy: new iam.ServicePrincipal('codepipeline.amazonaws.com')
});
}
this.role = propsRole;
Copy link
Contributor

Choose a reason for hiding this comment

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

We usually write it in one line:

this.role = props.role || new iam.Role(this, 'Role', {
  assumedBy: new iam.ServicePrincipal('codepipeline.amazonaws.com')
});

@@ -68,6 +68,12 @@ export interface PipelineProps {
*/
readonly artifactBucket?: s3.IBucket;

/**
* The IAM role to be assumed by this Pipeline.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we be a little more precise here? "The IAM role used for executing this Pipeline. If not specified, a new IAM role, trusting the CodePipeline service principal, will be created".

Sander Knape and others added 4 commits May 20, 2019 10:37
Implemented an awslint rule to help identify all "export" methods and remove them.

Align all AWS resource constructs (besides `events.EventRule`) to their canonical name.

Fixes #2577
Fixes #2578
Fixes #2458
Fixes #2419
Fixes #2579
Fixes #2313

Related #2551

BREAKING CHANGE: All `export` methods from all AWS resources have been removed. CloudFormation Exports are now automatically created when attributes are referenced across stacks within the same app. To export resources manually, you can explicitly define a `CfnOutput`.
* `kms.EncryptionKey` renamed to `kms.Key`
* `ec2.VpcNetwork` renamed to `ec2.Vpc`
* `ec2.VpcSubnet` renamed to `ec2.Subnet`
* `cloudtrail.CloudTrail` renamed `to `cloudtrail.Trail`
* Deleted a few `XxxAttribute` and `XxxImportProps` interfaces which were no longer in used after their corresponding `export` method was deleted and there was no use for them in imports.
* `ecs.ClusterAttributes` now accepts `IVpc` and `ISecurityGroup` instead of attributes. You can use their
corresponding `fromXxx` methods to import them as needed.
* `servicediscovery.CnameInstance.instanceCname` renamed to `cname`.
* `glue.IDatabase.locationUrl` is now only in `glue.Database` (not on the interface)
* `ec2.TcpPortFromAttribute` and `UdpPortFromAttribute` removed. Use `TcpPort` and `UdpPort` with `new Token(x).toNumber` instead.
* `ec2.VpcNetwork.importFromContext` renamed to `ec2.Vpc.fromLookup`
* `iam.IRole.roleId` has been removed from the interface, but `Role.roleId` is still available for owned resources.
@AlexChesters
Copy link
Contributor Author

Oops, I messed up the history on this branch :| I'll create a new PR..

@AlexChesters
Copy link
Contributor Author

Opened new PR in #2587 (cc: @skinny85)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants