Skip to content

Commit

Permalink
fix(codepipeline): S3 source Action with trigger=Events fails for buc…
Browse files Browse the repository at this point in the history
…ketKey a Token (#9575)

We use bucketKey to differentiate between multiple source actions
that observe the same bucket using trigger=Events.
However, we can't do that if bucketKey is a lazy value,
as Tokens can't be used as parts of identifier for the created Event.
So, check for that case explicitly.

Fixes #9554

----

*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 13, 2020
1 parent cb6de0a commit 43214b4
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 7 deletions.
37 changes: 31 additions & 6 deletions packages/@aws-cdk/aws-codepipeline-actions/lib/s3/source-action.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as codepipeline from '@aws-cdk/aws-codepipeline';
import * as targets from '@aws-cdk/aws-events-targets';
import * as s3 from '@aws-cdk/aws-s3';
import { Construct } from '@aws-cdk/core';
import { Construct, Token } from '@aws-cdk/core';
import { Action } from '../action';
import { sourceArtifactBounds } from '../common';

Expand Down Expand Up @@ -110,11 +110,7 @@ export class S3SourceAction extends Action {
protected bound(_scope: Construct, stage: codepipeline.IStage, options: codepipeline.ActionBindOptions):
codepipeline.ActionConfig {
if (this.props.trigger === S3Trigger.EVENTS) {
const id = stage.pipeline.node.uniqueId + 'SourceEventRule' + this.props.bucketKey;
if (this.props.bucket.node.tryFindChild(id)) {
// this means a duplicate path for the same bucket - error out
throw new Error(`S3 source action with path '${this.props.bucketKey}' is already present in the pipeline for this source bucket`);
}
const id = this.generateEventId(stage);
this.props.bucket.onCloudTrailWriteObject(id, {
target: new targets.CodePipeline(stage.pipeline),
paths: [this.props.bucketKey],
Expand All @@ -135,4 +131,33 @@ export class S3SourceAction extends Action {
},
};
}

private generateEventId(stage: codepipeline.IStage): string {
let ret: string;
const baseId = stage.pipeline.node.uniqueId + 'SourceEventRule';

if (Token.isUnresolved(this.props.bucketKey)) {
// If bucketKey is a Token, don't include it in the ID.
// Instead, use numbers to differentiate if multiple actions observe the same bucket
let candidate = baseId;
let counter = 0;
while (this.props.bucket.node.tryFindChild(candidate) !== undefined) {
counter += 1;
candidate = baseId + counter;
}
ret = candidate;
} else {
// we can't use Tokens in construct IDs,
// however, if bucketKey is not a Token,
// we want it to differentiate between multiple actions
// observing the same Bucket with different keys
ret = baseId + this.props.bucketKey;
if (this.props.bucket.node.tryFindChild(ret)) {
// this means a duplicate path for the same bucket - error out
throw new Error(`S3 source action with path '${this.props.bucketKey}' is already present in the pipeline for this source bucket`);
}
}

return ret;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { countResources, expect, haveResourceLike, not } from '@aws-cdk/assert';
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 { Stack } from '@aws-cdk/core';
import { Lazy, Stack } from '@aws-cdk/core';
import { Test } from 'nodeunit';
import * as cpactions from '../../lib';

Expand Down Expand Up @@ -176,6 +176,45 @@ export = {
test.done();
},

'allows using a Token bucketKey with trigger = Events, multiple times'(test: Test) {
const stack = new Stack();

const bucket = new s3.Bucket(stack, 'MyBucket');
const sourceStage = minimalPipeline(stack, {
bucket,
bucketKey: Lazy.stringValue({ produce: () => 'my-bucket-key1' }),
trigger: cpactions.S3Trigger.EVENTS,
});
sourceStage.addAction(new cpactions.S3SourceAction({
actionName: 'Source2',
bucket,
bucketKey: Lazy.stringValue({ produce: () => 'my-bucket-key2' }),
trigger: cpactions.S3Trigger.EVENTS,
output: new codepipeline.Artifact(),
}));

expect(stack, /* skipValidation = */ true).to(haveResourceLike('AWS::CodePipeline::Pipeline', {
'Stages': [
{
'Actions': [
{
'Configuration': {
'S3ObjectKey': 'my-bucket-key1',
},
},
{
'Configuration': {
'S3ObjectKey': 'my-bucket-key2',
},
},
],
},
],
}));

test.done();
},

'exposes variables for other actions to consume'(test: Test) {
const stack = new Stack();

Expand Down

0 comments on commit 43214b4

Please sign in to comment.