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.

Sign up for GitHub

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): Pipeline now accepts existing IAM role #2587

Merged
merged 4 commits into from
May 21, 2019
Merged

feat(aws-codepipeline): Pipeline now accepts existing IAM role #2587

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d8b5963
feat(aws-codepipeline): #2572 allow IAM Role to be passed to Pipeline
AlexChesters May 20, 2019
37b5a8c
use interface over concerete class
AlexChesters May 20, 2019
63939c9
docs(aws-codepipeline): add default declaration
AlexChesters May 20, 2019
5ba879a
add test
AlexChesters May 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ export interface PipelineProps {
*/
readonly artifactBucket?: s3.IBucket;

/**
* The IAM role to be assumed by this Pipeline.
* If not specified, a new IAM role will be created.
Copy link
Contributor

Choose a reason for hiding this comment

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

Use a @default marker in the docs to indicate the default behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rix0rrr do you have an example of the @default format that's expected? I'm not familiar with it, and I can't see where this is used anywhere else in this file

Copy link
Contributor

Choose a reason for hiding this comment

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

Here is an example:

https://github.com/awslabs/aws-cdk/blob/master/packages/@aws-cdk/aws-lambda/lib/function.ts#L115

I would not copy the second line "Both supplied and generated...". That feature is a pattern all over the library that can be assumed and doesn't have to be documented in every location.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha, done that. How does it look?

*/
readonly role?: iam.Role;
Copy link
Contributor

Choose a reason for hiding this comment

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

IRole


/**
* Indicates whether to rerun the AWS CodePipeline pipeline after you update it.
*/
Expand Down Expand Up @@ -233,7 +239,8 @@ export class Pipeline extends PipelineBase {
}
this.artifactBucket = propsBucket;

this.role = new iam.Role(this, 'Role', {
// If a role has been provided, use it - otherwise, create a role.
this.role = props.role || new iam.Role(this, 'Role', {
assumedBy: new iam.ServicePrincipal('codepipeline.amazonaws.com')
});

Expand Down