Skip to content

Commit

Permalink
fix(codepipeline-actions): cross stack reference causes stack cycle i…
Browse files Browse the repository at this point in the history
…n sources that use CloudWatch Events (#20149)

When using a newly-created, CDK-managed resource, such as an S3 Bucket or a CodeCommit Repository,
as the source of a CodePipeline, when it's in a different Stack than the pipeline
(but in the same environment as it),
and we use CloudWatch Events to trigger the pipeline from the source,
that would result in a cycle:

1. Because the Event Rule is created in the source Stack, that Stack needs the name of the pipeline to trigger from the Rule, and also the Role to use for the trigger, which is created in the target (in this case, the pipeline) Stack. So, the source Stack depends on the pipeline Stack.
2. The pipeline Stack needs the name of the source resource. So, the pipeline Stack depends on the source Stack.

The only way to break this cycle is to move the Event Rule to the target Stack.
This PR adds an API to the Events module to make it possible for event-creating constructs to make that choice,
and uses that capability in the CodePipeline `CodeCommitSourceAction` and `S3SourceAction`.

Fixes #3087
Fixes #8042
Fixes #10896

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
skinny85 authored Aug 9, 2022
1 parent 5408aad commit adf4022
Show file tree
Hide file tree
Showing 12 changed files with 2,453 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ export class CodeCommitSourceAction extends Action {
eventRole: this.props.eventRole,
}),
branches: [this.branch],
crossStackScope: stage.pipeline as unknown as Construct,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ export class S3SourceAction extends Action {
this.props.bucket.onCloudTrailWriteObject(id, {
target: new targets.CodePipeline(stage.pipeline),
paths: [this.props.bucketKey],
crossStackScope: stage.pipeline as unknown as Construct,
});
}

Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-codepipeline-actions/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
"@aws-cdk/aws-s3-assets": "0.0.0",
"@aws-cdk/cdk-build-tools": "0.0.0",
"@aws-cdk/integ-runner": "0.0.0",
"@aws-cdk/integ-tests": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"@aws-cdk/pkglint": "0.0.0",
"@types/jest": "^27.5.2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ describe('CodeCommit Source Action', () => {
});

Template.fromStack(stack).resourceCountIs('AWS::Events::Rule', 1);


});

test('cross-account CodeCommit Repository Source does not use target role in source stack', () => {
Expand Down Expand Up @@ -118,8 +116,6 @@ describe('CodeCommit Source Action', () => {
},
],
});


});

test('does not poll for source changes and uses Events for CodeCommitTrigger.EVENTS', () => {
Expand All @@ -143,8 +139,6 @@ describe('CodeCommit Source Action', () => {
});

Template.fromStack(stack).resourceCountIs('AWS::Events::Rule', 1);


});

test('polls for source changes and does not use Events for CodeCommitTrigger.POLL', () => {
Expand All @@ -168,8 +162,6 @@ describe('CodeCommit Source Action', () => {
});

Template.fromStack(stack).resourceCountIs('AWS::Events::Rule', 0);


});

test('does not poll for source changes and does not use Events for CodeCommitTrigger.NONE', () => {
Expand All @@ -193,8 +185,6 @@ describe('CodeCommit Source Action', () => {
});

Template.fromStack(stack).resourceCountIs('AWS::Events::Rule', 0);


});

test('cannot be created with an empty branch', () => {
Expand All @@ -211,8 +201,6 @@ describe('CodeCommit Source Action', () => {
branch: '',
});
}).toThrow(/'branch' parameter cannot be an empty string/);


});

test('allows using the same repository multiple times with different branches when trigger=EVENTS', () => {
Expand Down Expand Up @@ -253,8 +241,6 @@ describe('CodeCommit Source Action', () => {
},
],
});


});

test('exposes variables for other actions to consume', () => {
Expand Down Expand Up @@ -308,8 +294,6 @@ describe('CodeCommit Source Action', () => {
},
],
});


});

test('allows using a Token for the branch name', () => {
Expand Down Expand Up @@ -351,8 +335,6 @@ describe('CodeCommit Source Action', () => {
},
},
});


});

test('allows to enable full clone', () => {
Expand Down Expand Up @@ -456,8 +438,6 @@ describe('CodeCommit Source Action', () => {
]),
},
});


});

test('uses the role when passed', () => {
Expand Down Expand Up @@ -512,8 +492,6 @@ describe('CodeCommit Source Action', () => {
},
],
});


});

test('grants explicit s3:PutObjectAcl permissions when the Actions is cross-account', () => {
Expand Down Expand Up @@ -569,8 +547,49 @@ describe('CodeCommit Source Action', () => {
}]),
},
});
});

test('allows using a new Repository from another Stack as a source of the Pipeline, with Events', () => {
const app = new App();

const repoStack = new Stack(app, 'RepositoryStack');
const repo = new codecommit.Repository(repoStack, 'Repository', {
repositoryName: 'my-repo',
});

const pipelineStack = new Stack(app, 'PipelineStack');
const sourceOutput = new codepipeline.Artifact();
new codepipeline.Pipeline(pipelineStack, 'Pipeline', {
stages: [
{
stageName: 'Source',
actions: [
new cpactions.CodeCommitSourceAction({
actionName: 'Source',
repository: repo,
output: sourceOutput,
}),
],
},
{
stageName: 'Build',
actions: [
new cpactions.CodeBuildAction({
actionName: 'Build',
project: codebuild.Project.fromProjectName(pipelineStack, 'Project', 'my-project'),
input: sourceOutput,
}),
],
},
],
});

// If the Event Rule was created in the repo's Stack,
// we would have a cycle
// (the repo's Stack would need the name of the CodePipeline to trigger through the Rule,
// while the pipeline's Stack would need the name of the Repository to use as a Source).
// By moving the Rule to pipeline's Stack, we get rid of the cycle.
Template.fromStack(pipelineStack).resourceCountIs('AWS::Events::Rule', 1);
});
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/// !cdk-integ PipelineStack

import * as codebuild from '@aws-cdk/aws-codebuild';
import * as codepipeline from '@aws-cdk/aws-codepipeline';
import * as s3 from '@aws-cdk/aws-s3';
import { App, RemovalPolicy, Stack } from '@aws-cdk/core';
import * as integ from '@aws-cdk/integ-tests';
import * as cpactions from '../../lib';

const app = new App();
const bucketStack = new Stack(app, 'BucketStack');
const bucket = new s3.Bucket(bucketStack, 'Bucket', {
removalPolicy: RemovalPolicy.DESTROY,
});

const pipelineStack = new Stack(app, 'PipelineStack');
const sourceOutput = new codepipeline.Artifact();
new codepipeline.Pipeline(pipelineStack, 'Pipeline', {
stages: [
{
stageName: 'Source',
actions: [
new cpactions.S3SourceAction({
actionName: 'Source',
bucket,
trigger: cpactions.S3Trigger.EVENTS,
bucketKey: 'file.zip',
output: sourceOutput,
}),
],
},
{
stageName: 'Build',
actions: [
new cpactions.CodeBuildAction({
actionName: 'Build',
project: new codebuild.PipelineProject(pipelineStack, 'Project'),
input: sourceOutput,
}),
],
},
],
});

new integ.IntegTest(app, 'CodePipelineS3SourceTest', {
testCases: [pipelineStack],
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"Resources": {
"Bucket83908E77": {
"Type": "AWS::S3::Bucket",
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete"
}
},
"Outputs": {
"ExportsOutputRefBucket83908E7781C90AC0": {
"Value": {
"Ref": "Bucket83908E77"
},
"Export": {
"Name": "BucketStack:ExportsOutputRefBucket83908E7781C90AC0"
}
},
"ExportsOutputFnGetAttBucket83908E77Arn063C8555": {
"Value": {
"Fn::GetAtt": [
"Bucket83908E77",
"Arn"
]
},
"Export": {
"Name": "BucketStack:ExportsOutputFnGetAttBucket83908E77Arn063C8555"
}
}
}
}
Loading

0 comments on commit adf4022

Please sign in to comment.