From 5f138fd50e106f13c4332265f5a86296eccb212e Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Mon, 25 Feb 2019 16:13:24 -0800 Subject: [PATCH] feat(codepipeline): re-factor the CodePipeline Action `bind` method to take a Role separately from the Pipeline. --- .../alexa-ask/lib/pipeline-actions.ts | 2 +- .../test/test.pipeline-deploy-stack-action.ts | 2 +- .../lib/pipeline-actions.ts | 35 +++++----- .../test/test.pipeline-actions.ts | 7 +- .../aws-codebuild/lib/pipeline-actions.ts | 19 +++--- .../aws-codecommit/lib/pipeline-action.ts | 9 ++- packages/@aws-cdk/aws-codecommit/package.json | 3 +- .../aws-codedeploy/lib/pipeline-action.ts | 13 ++-- .../aws-codepipeline-api/lib/action.ts | 67 +++++++++++++------ .../lib/github-source-action.ts | 12 ++-- .../aws-codepipeline/lib/jenkins-actions.ts | 5 +- .../lib/manual-approval-action.ts | 14 ++-- .../@aws-cdk/aws-codepipeline/lib/pipeline.ts | 32 +++++---- .../@aws-cdk/aws-codepipeline/lib/stage.ts | 9 +-- .../aws-codepipeline/test/test.action.ts | 32 ++++----- .../@aws-cdk/aws-ecr/lib/pipeline-action.ts | 9 ++- .../aws-lambda/lib/pipeline-action.ts | 7 +- .../@aws-cdk/aws-s3/lib/pipeline-actions.ts | 13 ++-- 18 files changed, 159 insertions(+), 131 deletions(-) diff --git a/packages/@aws-cdk/alexa-ask/lib/pipeline-actions.ts b/packages/@aws-cdk/alexa-ask/lib/pipeline-actions.ts index fe2705ef3d9b0..c41e41df77cb2 100644 --- a/packages/@aws-cdk/alexa-ask/lib/pipeline-actions.ts +++ b/packages/@aws-cdk/alexa-ask/lib/pipeline-actions.ts @@ -64,7 +64,7 @@ export class AlexaSkillDeployAction extends codepipeline.DeployAction { } } - protected bind(_stage: codepipeline.IStage, _scope: cdk.Construct): void { + protected bind(_info: codepipeline.ActionBind): void { // nothing to do } } diff --git a/packages/@aws-cdk/app-delivery/test/test.pipeline-deploy-stack-action.ts b/packages/@aws-cdk/app-delivery/test/test.pipeline-deploy-stack-action.ts index b72c64234feb2..48c040e522213 100644 --- a/packages/@aws-cdk/app-delivery/test/test.pipeline-deploy-stack-action.ts +++ b/packages/@aws-cdk/app-delivery/test/test.pipeline-deploy-stack-action.ts @@ -313,7 +313,7 @@ class FakeAction extends api.Action { this.outputArtifact = new api.Artifact('OutputArtifact'); } - protected bind(_stage: api.IStage, _scope: cdk.Construct): void { + protected bind(_info: api.ActionBind): void { // do nothing } } diff --git a/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts b/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts index d1c2e41683cb1..d717fe04baa76 100644 --- a/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts +++ b/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts @@ -116,9 +116,8 @@ export class PipelineExecuteChangeSetAction extends PipelineCloudFormationAction this.props = props; } - protected bind(stage: codepipeline.IStage, _scope: cdk.Construct): void { - SingletonPolicy.forRole(stage.pipeline.role) - .grantExecuteChangeSet(this.props); + protected bind(info: codepipeline.ActionBind): void { + SingletonPolicy.forRole(info.role).grantExecuteChangeSet(this.props); } } @@ -257,11 +256,11 @@ export abstract class PipelineCloudFormationDeployAction extends PipelineCloudFo return this.getDeploymentRole('property role()'); } - protected bind(stage: codepipeline.IStage, scope: cdk.Construct): void { + protected bind(info: codepipeline.ActionBind): void { if (this.props.deploymentRole) { this._deploymentRole = this.props.deploymentRole; } else { - this._deploymentRole = new iam.Role(scope, 'Role', { + this._deploymentRole = new iam.Role(info.scope, 'Role', { assumedBy: new iam.ServicePrincipal('cloudformation.amazonaws.com') }); @@ -270,7 +269,7 @@ export abstract class PipelineCloudFormationDeployAction extends PipelineCloudFo } } - SingletonPolicy.forRole(stage.pipeline.role).grantPassRole(this._deploymentRole); + SingletonPolicy.forRole(info.role).grantPassRole(this._deploymentRole); } private getDeploymentRole(member: string): iam.IRole { @@ -321,10 +320,10 @@ export class PipelineCreateReplaceChangeSetAction extends PipelineCloudFormation this.props2 = props; } - protected bind(stage: codepipeline.IStage, scope: cdk.Construct): void { - super.bind(stage, scope); + protected bind(info: codepipeline.ActionBind): void { + super.bind(info); - SingletonPolicy.forRole(stage.pipeline.role).grantCreateReplaceChangeSet(this.props2); + SingletonPolicy.forRole(info.role).grantCreateReplaceChangeSet(this.props2); } } @@ -384,10 +383,10 @@ export class PipelineCreateUpdateStackAction extends PipelineCloudFormationDeplo this.props2 = props; } - protected bind(stage: codepipeline.IStage, scope: cdk.Construct): void { - super.bind(stage, scope); + protected bind(info: codepipeline.ActionBind): void { + super.bind(info); - SingletonPolicy.forRole(stage.pipeline.role).grantCreateUpdateStack(this.props2); + SingletonPolicy.forRole(info.role).grantCreateUpdateStack(this.props2); } } @@ -415,10 +414,10 @@ export class PipelineDeleteStackAction extends PipelineCloudFormationDeployActio this.props2 = props; } - protected bind(stage: codepipeline.IStage, scope: cdk.Construct): void { - super.bind(stage, scope); + protected bind(info: codepipeline.ActionBind): void { + super.bind(info); - SingletonPolicy.forRole(stage.pipeline.role).grantDeleteStack(this.props2); + SingletonPolicy.forRole(info.role).grantDeleteStack(this.props2); } } @@ -469,7 +468,7 @@ class SingletonPolicy extends cdk.Construct { * @param role the Role this policy is bound to. * @returns the SingletonPolicy for this role. */ - public static forRole(role: iam.Role): SingletonPolicy { + public static forRole(role: iam.IRole): SingletonPolicy { const found = role.node.tryFindChild(SingletonPolicy.UUID); return (found as SingletonPolicy) || new SingletonPolicy(role); } @@ -478,8 +477,8 @@ class SingletonPolicy extends cdk.Construct { private statements: { [key: string]: iam.PolicyStatement } = {}; - private constructor(private readonly role: iam.Role) { - super(role, SingletonPolicy.UUID); + private constructor(private readonly role: iam.IRole) { + super(role as unknown as cdk.Construct, SingletonPolicy.UUID); } public grantExecuteChangeSet(props: { stackName: string, changeSetName: string, region?: string }): void { diff --git a/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts b/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts index 9f661804bd5cc..35f07ac053b2a 100644 --- a/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts +++ b/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts @@ -342,7 +342,12 @@ class StageDouble implements cpapi.IStage { const stageParent = new cdk.Construct(pipeline, this.stageName); for (const action of actions) { const actionParent = new cdk.Construct(stageParent, action.actionName); - (action as any)._attachActionToPipeline(this, actionParent); + (action as any)._actionAttachedToPipeline({ + pipeline, + stage: this, + scope: actionParent, + role: pipeline.role, + }); } this.actions = actions; } diff --git a/packages/@aws-cdk/aws-codebuild/lib/pipeline-actions.ts b/packages/@aws-cdk/aws-codebuild/lib/pipeline-actions.ts index ecb3c84e42da9..9d0728f6b7176 100644 --- a/packages/@aws-cdk/aws-codebuild/lib/pipeline-actions.ts +++ b/packages/@aws-cdk/aws-codebuild/lib/pipeline-actions.ts @@ -1,6 +1,5 @@ import codepipeline = require('@aws-cdk/aws-codepipeline-api'); import iam = require('@aws-cdk/aws-iam'); -import cdk = require('@aws-cdk/cdk'); import { IProject } from './project'; /** @@ -103,8 +102,8 @@ export class PipelineBuildAction extends codepipeline.BuildAction { return findOutputArtifact(this.additionalOutputArtifacts(), name); } - protected bind(stage: codepipeline.IStage, _scope: cdk.Construct): void { - setCodeBuildNeededPermissions(stage, this.props.project, true); + protected bind(info: codepipeline.ActionBind): void { + setCodeBuildNeededPermissions(info, this.props.project, true); } } @@ -192,16 +191,16 @@ export class PipelineTestAction extends codepipeline.TestAction { return findOutputArtifact(this.additionalOutputArtifacts(), name); } - protected bind(stage: codepipeline.IStage, _scope: cdk.Construct): void { + protected bind(info: codepipeline.ActionBind): void { // TODO: revert "as any" when we centralize pipeline actions - setCodeBuildNeededPermissions(stage, this.props.project, (this as any)._outputArtifacts.length > 0); + setCodeBuildNeededPermissions(info, this.props.project, (this as any)._outputArtifacts.length > 0); } } -function setCodeBuildNeededPermissions(stage: codepipeline.IStage, project: IProject, - needsPipelineBucketWrite: boolean) { +function setCodeBuildNeededPermissions(info: codepipeline.ActionBind, project: IProject, + needsPipelineBucketWrite: boolean): void { // grant the Pipeline role the required permissions to this Project - stage.pipeline.role.addToPolicy(new iam.PolicyStatement() + info.role.addToPolicy(new iam.PolicyStatement() .addResource(project.projectArn) .addActions( 'codebuild:BatchGetBuilds', @@ -211,9 +210,9 @@ function setCodeBuildNeededPermissions(stage: codepipeline.IStage, project: IPro // allow the Project access to the Pipline's artifact Bucket if (needsPipelineBucketWrite) { - stage.pipeline.grantBucketReadWrite(project.role); + info.pipeline.grantBucketReadWrite(project.role); } else { - stage.pipeline.grantBucketRead(project.role); + info.pipeline.grantBucketRead(project.role); } } diff --git a/packages/@aws-cdk/aws-codecommit/lib/pipeline-action.ts b/packages/@aws-cdk/aws-codecommit/lib/pipeline-action.ts index 4f263150c4581..ce3332ecca275 100644 --- a/packages/@aws-cdk/aws-codecommit/lib/pipeline-action.ts +++ b/packages/@aws-cdk/aws-codecommit/lib/pipeline-action.ts @@ -1,6 +1,5 @@ import codepipeline = require('@aws-cdk/aws-codepipeline-api'); import iam = require('@aws-cdk/aws-iam'); -import cdk = require('@aws-cdk/cdk'); import { IRepository } from './repository'; /** @@ -62,10 +61,10 @@ export class PipelineSourceAction extends codepipeline.SourceAction { this.props = props; } - protected bind(stage: codepipeline.IStage, _scope: cdk.Construct): void { + protected bind(info: codepipeline.ActionBind): void { if (!this.props.pollForSourceChanges) { - this.props.repository.onCommit(stage.pipeline.node.uniqueId + 'EventRule', - stage.pipeline, this.props.branch || 'master'); + this.props.repository.onCommit(info.pipeline.node.uniqueId + 'EventRule', + info.pipeline, this.props.branch || 'master'); } // https://docs.aws.amazon.com/codecommit/latest/userguide/auth-and-access-control-permissions-reference.html#aa-acp @@ -77,7 +76,7 @@ export class PipelineSourceAction extends codepipeline.SourceAction { 'codecommit:CancelUploadArchive', ]; - stage.pipeline.role.addToPolicy(new iam.PolicyStatement() + info.role.addToPolicy(new iam.PolicyStatement() .addResource(this.props.repository.repositoryArn) .addActions(...actions)); } diff --git a/packages/@aws-cdk/aws-codecommit/package.json b/packages/@aws-cdk/aws-codecommit/package.json index 3448272f89c37..e16f0af4f3d7a 100644 --- a/packages/@aws-cdk/aws-codecommit/package.json +++ b/packages/@aws-cdk/aws-codecommit/package.json @@ -81,9 +81,10 @@ "peerDependencies": { "@aws-cdk/aws-codepipeline-api": "^0.27.0", "@aws-cdk/aws-events": "^0.27.0", + "@aws-cdk/aws-iam": "^0.27.0", "@aws-cdk/cdk": "^0.27.0" }, "engines": { "node": ">= 8.10.0" } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-codedeploy/lib/pipeline-action.ts b/packages/@aws-cdk/aws-codedeploy/lib/pipeline-action.ts index 44d8132509ff1..6f8eefe426881 100644 --- a/packages/@aws-cdk/aws-codedeploy/lib/pipeline-action.ts +++ b/packages/@aws-cdk/aws-codedeploy/lib/pipeline-action.ts @@ -1,6 +1,5 @@ import codepipeline = require('@aws-cdk/aws-codepipeline-api'); import iam = require('@aws-cdk/aws-iam'); -import cdk = require('@aws-cdk/cdk'); import { IServerDeploymentGroup } from './server'; /** @@ -43,33 +42,33 @@ export class PipelineDeployAction extends codepipeline.DeployAction { this.deploymentGroup = props.deploymentGroup; } - protected bind(stage: codepipeline.IStage, scope: cdk.Construct): void { + protected bind(info: codepipeline.ActionBind): void { // permissions, based on: // https://docs.aws.amazon.com/codedeploy/latest/userguide/auth-and-access-control-permissions-reference.html - stage.pipeline.role.addToPolicy(new iam.PolicyStatement() + info.role.addToPolicy(new iam.PolicyStatement() .addResource(this.deploymentGroup.application.applicationArn) .addActions( 'codedeploy:GetApplicationRevision', 'codedeploy:RegisterApplicationRevision', )); - stage.pipeline.role.addToPolicy(new iam.PolicyStatement() + info.role.addToPolicy(new iam.PolicyStatement() .addResource(this.deploymentGroup.deploymentGroupArn) .addActions( 'codedeploy:CreateDeployment', 'codedeploy:GetDeployment', )); - stage.pipeline.role.addToPolicy(new iam.PolicyStatement() - .addResource(this.deploymentGroup.deploymentConfig.deploymentConfigArn(scope)) + info.role.addToPolicy(new iam.PolicyStatement() + .addResource(this.deploymentGroup.deploymentConfig.deploymentConfigArn(info.scope)) .addActions( 'codedeploy:GetDeploymentConfig', )); // grant the ASG Role permissions to read from the Pipeline Bucket for (const asg of this.deploymentGroup.autoScalingGroups || []) { - stage.pipeline.grantBucketRead(asg.role); + info.pipeline.grantBucketRead(asg.role); } } } diff --git a/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts b/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts index 661cdbfd073a3..122489dfd831b 100644 --- a/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts +++ b/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts @@ -36,6 +36,32 @@ export function defaultBounds(): ActionArtifactBounds { }; } +/** + * The interface used in the {@link Action#bind()} callback. + */ +export interface ActionBind { + /** + * The pipeline this action has been added to. + */ + readonly pipeline: IPipeline; + + /** + * The stage this action has been added to. + */ + readonly stage: IStage; + + /** + * The scope construct for this action. + * Can be used by the action implementation to create any resources it needs to work correctly. + */ + readonly scope: cdk.Construct; + + /** + * The IAM Role to add the necessary permissions to. + */ + readonly role: iam.IRole; +} + /** * The abstract view of an AWS CodePipeline as required and used by Actions. * It extends {@link events.IEventRuleTarget}, @@ -52,11 +78,6 @@ export interface IPipeline extends cdk.IConstruct, events.IEventRuleTarget { */ readonly pipelineArn: string; - /** - * The service Role of the Pipeline. - */ - readonly role: iam.Role; - /** * Grants read permissions to the Pipeline's S3 Bucket to the given Identity. * @@ -81,11 +102,6 @@ export interface IStage { */ readonly stageName: string; - /** - * The Pipeline this Stage belongs to. - */ - readonly pipeline: IPipeline; - addAction(action: Action): void; onStateChange(name: string, target?: events.IEventRuleTarget, options?: events.EventRuleProps): events.EventRule; @@ -202,6 +218,7 @@ export abstract class Action { private readonly _actionOutputArtifacts = new Array(); private readonly artifactBounds: ActionArtifactBounds; + private _pipeline?: IPipeline; private _stage?: IStage; private _scope?: cdk.Construct; @@ -226,7 +243,7 @@ export abstract class Action { rule.addEventPattern({ detailType: [ 'CodePipeline Stage Execution State Change' ], source: [ 'aws.codepipeline' ], - resources: [ this.stage.pipeline.pipelineArn ], + resources: [ this.pipeline.pipelineArn ], detail: { stage: [ this.stage.stageName ], action: [ this.actionName ], @@ -303,24 +320,32 @@ export abstract class Action { * The method called when an Action is attached to a Pipeline. * This method is guaranteed to be called only once for each Action instance. * - * @param stage the stage this action has been added to - * (includes a reference to the pipeline as well) - * @param scope the scope construct for this action, - * can be used by the action implementation to create any resources it needs to work correctly + * @info an instance of the {@link ActionBind} class, + * that contains the necessary information for the Action + * to configure itself, like a reference to the Pipeline, Stage, Role, etc. */ - protected abstract bind(stage: IStage, scope: cdk.Construct): void; + protected abstract bind(info: ActionBind): void; - // ignore unused private method (it's actually used in Stage) + // ignore unused private method (it's actually used in Pipeline) // @ts-ignore - private _attachActionToPipeline(stage: IStage, scope: cdk.Construct): void { + private _actionAttachedToPipeline(info: ActionBind): void { if (this._stage) { throw new Error(`Action '${this.actionName}' has been added to a pipeline twice`); } - this._stage = stage; - this._scope = scope; + this._pipeline = info.pipeline; + this._stage = info.stage; + this._scope = info.scope; + + this.bind(info); + } - this.bind(stage, scope); + private get pipeline(): IPipeline { + if (this._pipeline) { + return this._pipeline; + } else { + throw new Error('Action must be added to a stage that is part of a pipeline before using onStateChange'); + } } private get stage(): IStage { diff --git a/packages/@aws-cdk/aws-codepipeline/lib/github-source-action.ts b/packages/@aws-cdk/aws-codepipeline/lib/github-source-action.ts index 71cc2681fe91a..294cfe9371bff 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/github-source-action.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/github-source-action.ts @@ -1,11 +1,11 @@ -import actions = require('@aws-cdk/aws-codepipeline-api'); +import cpapi = require('@aws-cdk/aws-codepipeline-api'); import cdk = require('@aws-cdk/cdk'); import { CfnWebhook } from './codepipeline.generated'; /** * Construction properties of the {@link GitHubSourceAction GitHub source action}. */ -export interface GitHubSourceActionProps extends actions.CommonActionProps { +export interface GitHubSourceActionProps extends cpapi.CommonActionProps { /** * The name of the source's output artifact. CfnOutput artifacts are used by CodePipeline as * inputs into other actions. @@ -52,7 +52,7 @@ export interface GitHubSourceActionProps extends actions.CommonActionProps { /** * Source that is provided by a GitHub repository. */ -export class GitHubSourceAction extends actions.SourceAction { +export class GitHubSourceAction extends cpapi.SourceAction { private readonly props: GitHubSourceActionProps; constructor(props: GitHubSourceActionProps) { @@ -73,9 +73,9 @@ export class GitHubSourceAction extends actions.SourceAction { this.props = props; } - protected bind(stage: actions.IStage, scope: cdk.Construct): void { + protected bind(info: cpapi.ActionBind): void { if (!this.props.pollForSourceChanges) { - new CfnWebhook(scope, 'WebhookResource', { + new CfnWebhook(info.scope, 'WebhookResource', { authentication: 'GITHUB_HMAC', authenticationConfiguration: { secretToken: this.props.oauthToken.toString(), @@ -87,7 +87,7 @@ export class GitHubSourceAction extends actions.SourceAction { }, ], targetAction: this.actionName, - targetPipeline: stage.pipeline.pipelineName, + targetPipeline: info.pipeline.pipelineName, targetPipelineVersion: 1, registerWithThirdParty: true, }); diff --git a/packages/@aws-cdk/aws-codepipeline/lib/jenkins-actions.ts b/packages/@aws-cdk/aws-codepipeline/lib/jenkins-actions.ts index cc81c44a18e84..d7569b5ddcb76 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/jenkins-actions.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/jenkins-actions.ts @@ -1,5 +1,4 @@ import cpapi = require('@aws-cdk/aws-codepipeline-api'); -import cdk = require('@aws-cdk/cdk'); import { IJenkinsProvider, jenkinsArtifactsBounds } from "./jenkins-provider"; /** @@ -68,7 +67,7 @@ export class JenkinsBuildAction extends cpapi.BuildAction { this.jenkinsProvider = props.jenkinsProvider; } - protected bind(_stage: cpapi.IStage, _scope: cdk.Construct): void { + protected bind(_info: cpapi.ActionBind): void { this.jenkinsProvider._registerBuildProvider(); } } @@ -123,7 +122,7 @@ export class JenkinsTestAction extends cpapi.TestAction { this.jenkinsProvider = props.jenkinsProvider; } - protected bind(_stage: cpapi.IStage, _scope: cdk.Construct): void { + protected bind(_info: cpapi.ActionBind): void { this.jenkinsProvider._registerTestProvider(); } } diff --git a/packages/@aws-cdk/aws-codepipeline/lib/manual-approval-action.ts b/packages/@aws-cdk/aws-codepipeline/lib/manual-approval-action.ts index 138dbd5bbfe8d..6214f7b408bc4 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/manual-approval-action.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/manual-approval-action.ts @@ -1,11 +1,11 @@ -import actions = require('@aws-cdk/aws-codepipeline-api'); +import cpapi = require('@aws-cdk/aws-codepipeline-api'); import sns = require('@aws-cdk/aws-sns'); import cdk = require('@aws-cdk/cdk'); /** * Construction properties of the {@link ManualApprovalAction}. */ -export interface ManualApprovalActionProps extends actions.CommonActionProps { +export interface ManualApprovalActionProps extends cpapi.CommonActionProps { /** * Optional SNS topic to send notifications to when an approval is pending. */ @@ -27,7 +27,7 @@ export interface ManualApprovalActionProps extends actions.CommonActionProps { /** * Manual approval action. */ -export class ManualApprovalAction extends actions.Action { +export class ManualApprovalAction extends cpapi.Action { /** * The SNS Topic passed when constructing the Action. * If no Topic was passed, but `notifyEmails` were provided, @@ -39,7 +39,7 @@ export class ManualApprovalAction extends actions.Action { constructor(props: ManualApprovalActionProps) { super({ ...props, - category: actions.ActionCategory.Approval, + category: cpapi.ActionCategory.Approval, provider: 'Manual', artifactBounds: { minInputs: 0, maxInputs: 0, minOutputs: 0, maxOutputs: 0 }, configuration: new cdk.Token(() => this.actionConfiguration()), @@ -52,15 +52,15 @@ export class ManualApprovalAction extends actions.Action { return this._notificationTopic; } - protected bind(stage: actions.IStage, scope: cdk.Construct): void { + protected bind(info: cpapi.ActionBind): void { if (this.props.notificationTopic) { this._notificationTopic = this.props.notificationTopic; } else if ((this.props.notifyEmails || []).length > 0) { - this._notificationTopic = new sns.Topic(scope, 'TopicResource'); + this._notificationTopic = new sns.Topic(info.scope, 'TopicResource'); } if (this._notificationTopic) { - this._notificationTopic.grantPublish(stage.pipeline.role); + this._notificationTopic.grantPublish(info.role); for (const notifyEmail of this.props.notifyEmails || []) { this._notificationTopic.subscribeEmail(`Subscription-${notifyEmail}`, notifyEmail); } diff --git a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts index ee25052a68e7a..d74b6af47b670 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts @@ -329,43 +329,51 @@ export class Pipeline extends cdk.Construct implements cpapi.IPipeline { // ignore unused private method (it's actually used in Stage) // @ts-ignore - private _attachActionToRegion(stage: Stage, action: cpapi.Action): void { - // handle cross-region Actions here - if (!action.region) { - return; + private _attachActionToPipeline(stage: Stage, action: cpapi.Action, actionScope: cdk.Construct): void { + if (action.region) { + // handle cross-region Actions here + this.ensureReplicationBucketExistsFor(action.region); } + (action as any)._actionAttachedToPipeline({ + pipeline: this, + stage, + scope: actionScope, + role: this.role, + }); + } + private ensureReplicationBucketExistsFor(region: string) { // get the region the Pipeline itself is in const pipelineRegion = this.node.stack.requireRegion( - "You need to specify an explicit region when using CodePipeline's cross-region support"); + "You need to specify an explicit region when using CodePipeline's cross-region support"); // if we already have an ArtifactStore generated for this region, or it's the Pipeline's region, nothing to do - if (this.artifactStores[action.region] || action.region === pipelineRegion) { + if (this.artifactStores[region] || region === pipelineRegion) { return; } - let replicationBucketName = this.crossRegionReplicationBuckets[action.region]; + let replicationBucketName = this.crossRegionReplicationBuckets[region]; if (!replicationBucketName) { const pipelineAccount = this.node.stack.requireAccountId( - "You need to specify an explicit account when using CodePipeline's cross-region support"); + "You need to specify an explicit account when using CodePipeline's cross-region support"); const app = this.node.stack.parentApp(); if (!app) { throw new Error(`Pipeline stack which uses cross region actions must be part of an application`); } const crossRegionScaffoldStack = new CrossRegionScaffoldStack(app, { - region: action.region, + region, account: pipelineAccount, }); - this._crossRegionScaffoldStacks[action.region] = crossRegionScaffoldStack; + this._crossRegionScaffoldStacks[region] = crossRegionScaffoldStack; replicationBucketName = crossRegionScaffoldStack.replicationBucketName; } - const replicationBucket = s3.Bucket.import(this, 'CrossRegionCodePipelineReplicationBucket-' + action.region, { + const replicationBucket = s3.Bucket.import(this, 'CrossRegionCodePipelineReplicationBucket-' + region, { bucketName: replicationBucketName, }); replicationBucket.grantReadWrite(this.role); - this.artifactStores[action.region] = { + this.artifactStores[region] = { Location: replicationBucket.bucketName, Type: 'S3', }; diff --git a/packages/@aws-cdk/aws-codepipeline/lib/stage.ts b/packages/@aws-cdk/aws-codepipeline/lib/stage.ts index 3d87749a899ac..79425f11a7786 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/stage.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/stage.ts @@ -79,12 +79,9 @@ export class Stage implements cpapi.IStage { } private attachActionToPipeline(action: cpapi.Action) { - const actionParent = new cdk.Construct(this.scope, action.actionName); - (action as any)._attachActionToPipeline(this, actionParent); - - // also notify the Pipeline of the new Action - // (useful for cross-region Actions, for example) - (this.pipeline as any)._attachActionToRegion(this, action); + // notify the Pipeline of the new Action + const actionScope = new cdk.Construct(this.scope, action.actionName); + (this.pipeline as any)._attachActionToPipeline(this, action, actionScope); } private renderAction(action: cpapi.Action): CfnPipeline.ActionDeclarationProperty { diff --git a/packages/@aws-cdk/aws-codepipeline/test/test.action.ts b/packages/@aws-cdk/aws-codepipeline/test/test.action.ts index 6d5b5614e0968..53f4577713ecd 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/test.action.ts +++ b/packages/@aws-cdk/aws-codepipeline/test/test.action.ts @@ -1,7 +1,7 @@ import { expect, haveResourceLike } from '@aws-cdk/assert'; import codebuild = require('@aws-cdk/aws-codebuild'); import codecommit = require('@aws-cdk/aws-codecommit'); -import actions = require('@aws-cdk/aws-codepipeline-api'); +import cpapi = require('@aws-cdk/aws-codepipeline-api'); import cdk = require('@aws-cdk/cdk'); import { Test } from 'nodeunit'; import codepipeline = require('../lib'); @@ -35,27 +35,27 @@ export = { 'action type validation': { 'must be source and is source'(test: Test) { - const result = actions.validateSourceAction(true, actions.ActionCategory.Source, 'test action', 'test stage'); + const result = cpapi.validateSourceAction(true, cpapi.ActionCategory.Source, 'test action', 'test stage'); test.deepEqual(result.length, 0); test.done(); }, 'must be source and is not source'(test: Test) { - const result = actions.validateSourceAction(true, actions.ActionCategory.Deploy, 'test action', 'test stage'); + const result = cpapi.validateSourceAction(true, cpapi.ActionCategory.Deploy, 'test action', 'test stage'); test.deepEqual(result.length, 1); test.ok(result[0].match(/may only contain Source actions/), 'the validation should have failed'); test.done(); }, 'cannot be source and is source'(test: Test) { - const result = actions.validateSourceAction(false, actions.ActionCategory.Source, 'test action', 'test stage'); + const result = cpapi.validateSourceAction(false, cpapi.ActionCategory.Source, 'test action', 'test stage'); test.deepEqual(result.length, 1); test.ok(result[0].match(/may only occur in first stage/), 'the validation should have failed'); test.done(); }, 'cannot be source and is not source'(test: Test) { - const result = actions.validateSourceAction(false, actions.ActionCategory.Deploy, 'test action', 'test stage'); + const result = cpapi.validateSourceAction(false, cpapi.ActionCategory.Deploy, 'test action', 'test stage'); test.deepEqual(result.length, 0); test.done(); }, @@ -152,7 +152,7 @@ export = { 'input Artifacts': { 'can be added multiple times to an Action safely'(test: Test) { - const artifact = new actions.Artifact('SomeArtifact'); + const artifact = new cpapi.Artifact('SomeArtifact'); const stack = new cdk.Stack(); const project = new codebuild.PipelineProject(stack, 'Project'); @@ -168,8 +168,8 @@ export = { }, 'cannot have duplicate names'(test: Test) { - const artifact1 = new actions.Artifact('SomeArtifact'); - const artifact2 = new actions.Artifact('SomeArtifact'); + const artifact1 = new cpapi.Artifact('SomeArtifact'); + const artifact2 = new cpapi.Artifact('SomeArtifact'); const stack = new cdk.Stack(); const project = new codebuild.PipelineProject(stack, 'Project'); @@ -188,7 +188,7 @@ export = { 'output Artifact names': { 'accept the same name multiple times safely'(test: Test) { - const artifact = new actions.Artifact('SomeArtifact'); + const artifact = new cpapi.Artifact('SomeArtifact'); const stack = new cdk.Stack(); const project = new codebuild.PipelineProject(stack, 'Project'); @@ -210,24 +210,24 @@ export = { }; function boundsValidationResult(numberOfArtifacts: number, min: number, max: number): string[] { - const artifacts: actions.Artifact[] = []; + const artifacts: cpapi.Artifact[] = []; for (let i = 0; i < numberOfArtifacts; i++) { - artifacts.push(new actions.Artifact(`TestArtifact${i}`)); + artifacts.push(new cpapi.Artifact(`TestArtifact${i}`)); } - return actions.validateArtifactBounds('output', artifacts, min, max, 'testCategory', 'testProvider'); + return cpapi.validateArtifactBounds('output', artifacts, min, max, 'testCategory', 'testProvider'); } -class FakeAction extends actions.Action { +class FakeAction extends cpapi.Action { constructor(actionName: string) { super({ actionName, - category: actions.ActionCategory.Source, + category: cpapi.ActionCategory.Source, provider: 'SomeService', - artifactBounds: actions.defaultBounds(), + artifactBounds: cpapi.defaultBounds(), }); } - protected bind(_stage: actions.IStage, _scope: cdk.Construct): void { + protected bind(_info: cpapi.ActionBind): void { // do nothing } } diff --git a/packages/@aws-cdk/aws-ecr/lib/pipeline-action.ts b/packages/@aws-cdk/aws-ecr/lib/pipeline-action.ts index 3d513c235a795..22d49211d23b5 100644 --- a/packages/@aws-cdk/aws-ecr/lib/pipeline-action.ts +++ b/packages/@aws-cdk/aws-ecr/lib/pipeline-action.ts @@ -1,6 +1,5 @@ import codepipeline = require('@aws-cdk/aws-codepipeline-api'); import iam = require('@aws-cdk/aws-iam'); -import cdk = require('@aws-cdk/cdk'); import { IRepository } from './repository-ref'; /** @@ -55,14 +54,14 @@ export class PipelineSourceAction extends codepipeline.SourceAction { this.props = props; } - protected bind(stage: codepipeline.IStage, _scope: cdk.Construct): void { - stage.pipeline.role.addToPolicy(new iam.PolicyStatement() + protected bind(info: codepipeline.ActionBind): void { + info.role.addToPolicy(new iam.PolicyStatement() .addActions( 'ecr:DescribeImages', ) .addResource(this.props.repository.repositoryArn)); - this.props.repository.onImagePushed(stage.pipeline.node.uniqueId + 'SourceEventRule', - stage.pipeline, this.props.imageTag); + this.props.repository.onImagePushed(info.pipeline.node.uniqueId + 'SourceEventRule', + info.pipeline, this.props.imageTag); } } diff --git a/packages/@aws-cdk/aws-lambda/lib/pipeline-action.ts b/packages/@aws-cdk/aws-lambda/lib/pipeline-action.ts index 017b044360049..80939280f7345 100644 --- a/packages/@aws-cdk/aws-lambda/lib/pipeline-action.ts +++ b/packages/@aws-cdk/aws-lambda/lib/pipeline-action.ts @@ -1,6 +1,5 @@ import codepipeline = require('@aws-cdk/aws-codepipeline-api'); import iam = require('@aws-cdk/aws-iam'); -import cdk = require('@aws-cdk/cdk'); import { IFunction } from './function-base'; /** @@ -122,14 +121,14 @@ export class PipelineInvokeAction extends codepipeline.Action { } } - protected bind(stage: codepipeline.IStage, _scope: cdk.Construct): void { + protected bind(info: codepipeline.ActionBind): void { // allow pipeline to list functions - stage.pipeline.role.addToPolicy(new iam.PolicyStatement() + info.role.addToPolicy(new iam.PolicyStatement() .addAction('lambda:ListFunctions') .addAllResources()); // allow pipeline to invoke this lambda functionn - stage.pipeline.role.addToPolicy(new iam.PolicyStatement() + info.role.addToPolicy(new iam.PolicyStatement() .addAction('lambda:InvokeFunction') .addResource(this.props.lambda.functionArn)); diff --git a/packages/@aws-cdk/aws-s3/lib/pipeline-actions.ts b/packages/@aws-cdk/aws-s3/lib/pipeline-actions.ts index 6c05dce21db64..b851e1b88221e 100644 --- a/packages/@aws-cdk/aws-s3/lib/pipeline-actions.ts +++ b/packages/@aws-cdk/aws-s3/lib/pipeline-actions.ts @@ -1,5 +1,4 @@ import codepipeline = require('@aws-cdk/aws-codepipeline-api'); -import cdk = require('@aws-cdk/cdk'); import { IBucket } from './bucket'; /** @@ -66,14 +65,14 @@ export class PipelineSourceAction extends codepipeline.SourceAction { this.props = props; } - protected bind(stage: codepipeline.IStage, _scope: cdk.Construct): void { + protected bind(info: codepipeline.ActionBind): void { if (this.props.pollForSourceChanges === false) { - this.props.bucket.onPutObject(stage.pipeline.node.uniqueId + 'SourceEventRule', - stage.pipeline, this.props.bucketKey); + this.props.bucket.onPutObject(info.pipeline.node.uniqueId + 'SourceEventRule', + info.pipeline, this.props.bucketKey); } // pipeline needs permissions to read from the S3 bucket - this.props.bucket.grantRead(stage.pipeline.role); + this.props.bucket.grantRead(info.role); } } @@ -137,8 +136,8 @@ export class PipelineDeployAction extends codepipeline.DeployAction { this.bucket = props.bucket; } - protected bind(stage: codepipeline.IStage, _scope: cdk.Construct): void { + protected bind(info: codepipeline.ActionBind): void { // pipeline needs permissions to write to the S3 bucket - this.bucket.grantWrite(stage.pipeline.role); + this.bucket.grantWrite(info.role); } }