From d88e915c45378cac6a1c7eb31b015391e74f6503 Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Mon, 19 Apr 2021 03:52:18 -0700 Subject: [PATCH] fix(codepipeline): detect the account of the Action from its backing resource's account, not its Stack's account (#14224) In CodePipeline, Actions can have a resource backing it (like an S3 Bucket, or an ECS Service). In the CodePipeline construct however, the account a given action was in was incorrectly determined by checking the Stack of the backing resource, instead of the resource itself. This value can be different for imported resources. Fixes #14165 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../@aws-cdk/aws-codepipeline/lib/pipeline.ts | 60 ++++++++++------ .../aws-codepipeline/test/cross-env.test.ts | 72 ++++++++++++++++++- 2 files changed, 111 insertions(+), 21 deletions(-) diff --git a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts index 8ef8cc3dede8e..b0967bcf53648 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts @@ -662,48 +662,68 @@ export class Pipeline extends PipelineBase { * @param action the Action to return the Stack for */ private getOtherStackIfActionIsCrossAccount(action: IAction): Stack | undefined { - const pipelineStack = Stack.of(this); + const targetAccount = action.actionProperties.resource + ? action.actionProperties.resource.env.account + : action.actionProperties.account; - if (action.actionProperties.resource) { - const resourceStack = Stack.of(action.actionProperties.resource); - // check if resource is from a different account - if (pipelineStack.account === resourceStack.account) { - return undefined; - } else { - this._crossAccountSupport[resourceStack.account] = resourceStack; - return resourceStack; - } - } - - if (!action.actionProperties.account) { + if (targetAccount === undefined) { + // if the account of the Action is not specified, + // then it defaults to the same account the pipeline itself is in return undefined; } - const targetAccount = action.actionProperties.account; - // check whether the account is a static string + // check whether the action's account is a static string if (Token.isUnresolved(targetAccount)) { - throw new Error(`The 'account' property must be a concrete value (action: '${action.actionProperties.actionName}')`); + if (Token.isUnresolved(this.env.account)) { + // the pipeline is also env-agnostic, so that's fine + return undefined; + } else { + throw new Error(`The 'account' property must be a concrete value (action: '${action.actionProperties.actionName}')`); + } } - // check whether the pipeline account is a static string - if (Token.isUnresolved(pipelineStack.account)) { + + // At this point, we know that the action's account is a static string. + // In this case, the pipeline's account must also be a static string. + if (Token.isUnresolved(this.env.account)) { throw new Error('Pipeline stack which uses cross-environment actions must have an explicitly set account'); } - if (pipelineStack.account === targetAccount) { + // at this point, we know that both the Pipeline's account, + // and the action-backing resource's account are static strings + + // if they are identical - nothing to do (the action is not cross-account) + if (this.env.account === targetAccount) { return undefined; } + // at this point, we know that the action is certainly cross-account, + // so we need to return a Stack in its account to create the helper Role in + + const candidateActionResourceStack = action.actionProperties.resource + ? Stack.of(action.actionProperties.resource) + : undefined; + if (candidateActionResourceStack?.account === targetAccount) { + // we always use the "latest" action-backing resource's Stack for this account, + // even if a different one was used earlier + this._crossAccountSupport[targetAccount] = candidateActionResourceStack; + return candidateActionResourceStack; + } + let targetAccountStack: Stack | undefined = this._crossAccountSupport[targetAccount]; if (!targetAccountStack) { const stackId = `cross-account-support-stack-${targetAccount}`; const app = this.requireApp(); targetAccountStack = app.node.tryFindChild(stackId) as Stack; if (!targetAccountStack) { + const actionRegion = action.actionProperties.resource + ? action.actionProperties.resource.env.region + : action.actionProperties.region; + const pipelineStack = Stack.of(this); targetAccountStack = new Stack(app, stackId, { stackName: `${pipelineStack.stackName}-support-${targetAccount}`, env: { account: targetAccount, - region: action.actionProperties.region ? action.actionProperties.region : pipelineStack.region, + region: actionRegion ?? pipelineStack.region, }, }); } diff --git a/packages/@aws-cdk/aws-codepipeline/test/cross-env.test.ts b/packages/@aws-cdk/aws-codepipeline/test/cross-env.test.ts index 5a0d9a5e3b78b..f22c80e82cba1 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/cross-env.test.ts +++ b/packages/@aws-cdk/aws-codepipeline/test/cross-env.test.ts @@ -138,4 +138,74 @@ describe.each(['legacy', 'modern'])('with %s synthesis', (synthesisStyle: string }); }); }); -}); \ No newline at end of file +}); + +describe('cross-environment CodePipeline', function () { + test('correctly detects that an Action is cross-account from the account of the resource backing the Action', () => { + const app = new App(); + + const pipelineStack = new Stack(app, 'PipelineStack', { + env: { account: '123', region: 'my-region' }, + }); + const sourceOutput = new codepipeline.Artifact(); + const pipeline = new codepipeline.Pipeline(pipelineStack, 'Pipeline', { + stages: [ + { + stageName: 'Source', + actions: [ + new FakeSourceAction({ + actionName: 'Source', + output: sourceOutput, + }), + ], + }, + ], + }); + + // Import a resource backing the FakeBuildAction into the pipeline's Stack, + // but specify a different account for it during the import. + // This should be correctly detected by the CodePipeline construct, + // and a correct support Stack should be created. + const deployBucket = s3.Bucket.fromBucketAttributes(pipelineStack, 'DeployBucket', { + bucketName: 'my-bucket', + account: '456', + }); + pipeline.addStage({ + stageName: 'Build', + actions: [ + new FakeBuildAction({ + actionName: 'Build', + input: sourceOutput, + resource: deployBucket, + }), + ], + }); + + const asm = app.synth(); + const supportStack = asm.getStackByName(`${pipelineStack.stackName}-support-456`); + expect(supportStack).toHaveResourceLike('AWS::IAM::Role', { + RoleName: 'pipelinestack-support-456dbuildactionrole91c6f1a469fd11d52dfe', + }); + + expect(pipelineStack).toHaveResourceLike('AWS::CodePipeline::Pipeline', { + Stages: [ + { Name: 'Source' }, + { + Name: 'Build', + Actions: [ + { + Name: 'Build', + RoleArn: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':iam::456:role/pipelinestack-support-456dbuildactionrole91c6f1a469fd11d52dfe', + ]], + }, + }, + ], + }, + ], + }); + }); +});