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.

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

Make the Stage insertion API in CodePipeline more flexible #437

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
84 changes: 73 additions & 11 deletions packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import s3 = require('@aws-cdk/aws-s3');
import cdk = require('@aws-cdk/cdk');
import util = require('@aws-cdk/util');
import { cloudformation } from './codepipeline.generated';
import { Stage } from './stage';
import { Stage, StagePlacement, StageProps } from './stage';
import validation = require('./validation');

/**
Expand Down Expand Up @@ -86,7 +86,7 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget {
*/
public readonly artifactBucket: s3.BucketRef;

private readonly stages = new Array<Stage>();
private readonly _stages = new Array<Stage>();
private eventsRole?: iam.Role;

constructor(parent: cdk.Construct, name: string, props?: PipelineProps) {
Expand Down Expand Up @@ -131,6 +131,20 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget {
}));
}

/**
* Get a duplicate of this Pipeline's list of Stages.
*/
public get stages(): Stage[] {
return this._stages.slice();
}

/**
* Get the number of Stages in this Pipeline.
*/
public get stagesLength(): number {
return this._stages.length;
}

/**
* Adds a statement to the pipeline role.
*/
Expand Down Expand Up @@ -216,23 +230,71 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget {
* onChildAdded type hook.
* @override
*/
protected addChild(child: cdk.Construct, name: string) {
super.addChild(child, name);
protected addChild(child: cdk.Construct, name: string, props?: any) {
super.addChild(child, name, props);
if (child instanceof Stage) {
this.appendStage(child);
this.appendStage(child, props ? (props as StageProps).placed : undefined);
}
}

private appendStage(stage: Stage) {
if (this.stages.find(x => x.name === stage.name)) {
private appendStage(stage: Stage, placement?: StagePlacement) {
if (this._stages.find(x => x.name === stage.name)) {
throw new Error(`A stage with name '${stage.name}' already exists`);
}

this.stages.push(stage);
const index = placement
? this.calculateInsertIndexFromPlacement(placement)
: this.stagesLength;

this._stages.splice(index, 0, stage);
}

private calculateInsertIndexFromPlacement(placement: StagePlacement): number {
// check if at most one placement property was provided
const providedPlacementProps = ['rightBeforeStage', 'justAfterStage', 'atIndex']
.filter((prop) => (placement as any)[prop] !== undefined);
if (providedPlacementProps.length > 1) {
throw new Error("Error adding Stage to the Pipeline: " +
`you can only provide at most one placement property, ${providedPlacementProps} were given`);
}

if (placement.rightBeforeStage !== undefined) {
const targetIndex = this.findStageIndex(placement.rightBeforeStage);
if (targetIndex === -1) {
throw new Error("Error adding Stage to the Pipeline: " +
`the requested Stage to add it before, '${placement.rightBeforeStage.name}', was not found`);
}
return targetIndex;
}

if (placement.justAfterStage !== undefined) {
const targetIndex = this.findStageIndex(placement.justAfterStage);
if (targetIndex === -1) {
throw new Error("Error adding Stage to the Pipeline: " +
`the requested Stage to add it after, '${placement.justAfterStage.name}', was not found`);
}
return targetIndex + 1;
}

if (placement.atIndex !== undefined) {
const index = placement.atIndex;
if (index < 0 || index > this.stagesLength) {
throw new Error("Error adding Stage to the Pipeline: " +
`{ placed: atIndex } should be between 0 and the number of stages in the Pipeline (${this.stagesLength}), ` +
` got: ${index}`);
}
return index;
}

return this.stagesLength;
}

private findStageIndex(targetStage: Stage) {
return this._stages.findIndex((stage: Stage) => stage === targetStage);
}

private validateSourceActionLocations(): string[] {
return util.flatMap(this.stages, (stage, i) => {
return util.flatMap(this._stages, (stage, i) => {
const onlySourceActionsPermitted = i === 0;
return util.flatMap(stage.actions, (action, _) =>
validation.validateSourceAction(onlySourceActionsPermitted, action.category, action.name, stage.name)
Expand All @@ -241,7 +303,7 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget {
}

private validateHasStages(): string[] {
if (this.stages.length < 2) {
if (this._stages.length < 2) {
return ['Pipeline must have at least two stages'];
}
return [];
Expand Down Expand Up @@ -270,6 +332,6 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget {
}

private renderStages(): cloudformation.PipelineResource.StageDeclarationProperty[] {
return this.stages.map(stage => stage.render());
return this._stages.map(stage => stage.render());
}
}
50 changes: 48 additions & 2 deletions packages/@aws-cdk/aws-codepipeline/lib/stage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,52 @@ import { cloudformation } from './codepipeline.generated';
import { Pipeline } from './pipeline';
import validation = require('./validation');

/**
* The construction properties for {@link Stage}.
*/
export interface StageProps {
/**
* Allows specifying where should the newly created {@link Stage}
* be placed in the Pipeline.
*
* @default the stage is added to the end of the Pipeline
*/
readonly placed?: StagePlacement;
}

/**
* Allows you to control where to place a new Stage when it's added to the Pipeline.
* Note that you can provide only one of the below properties -
* specifying more than one will result in a validation error.
*
* @see #rightBeforeStage
* @see #justAfterStage
* @see #atIndex
*/
export interface StagePlacement {
/**
* Inserts the new Stage as a parent of the given Stage
* (changing its current parent Stage, if it had one).
*/
readonly rightBeforeStage?: Stage;

/**
* Inserts the new Stage as a child of the given Stage
* (changing its current child Stage, if it had one).
*/
readonly justAfterStage?: Stage;

/**
* Inserts the new Stage at the given index in the Pipeline,
* moving the Stage currently at that index,
* and any subsequent ones, one index down.
* Indexing starts at 0.
* The maximum allowed value is {@link Pipeline#stagesLength},
* which will insert the new Stage at the end of the Pipeline.
*/
readonly atIndex?: number;
}

/**
* A stage in a pipeline. Stages are added to a pipeline by constructing a Stage with
* the pipeline as the first argument to the constructor.
Expand All @@ -27,8 +73,8 @@ export class Stage extends cdk.Construct {
* always be attached to a pipeline. It's illogical to construct a Stage
* with any other parent.
*/
constructor(parent: Pipeline, name: string) {
super(parent, name);
constructor(parent: Pipeline, name: string, props?: StageProps) {
super(parent, name, props);
this.pipeline = parent;
validation.validateName('Stage', name);
}
Expand Down
141 changes: 141 additions & 0 deletions packages/@aws-cdk/aws-codepipeline/test/test.stages.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import cdk = require('@aws-cdk/cdk');
import { Test } from 'nodeunit';
import codepipeline = require('../lib');

// tslint:disable:object-literal-key-quotes

export = {
'Pipeline Stages': {
'can be inserted at index 0'(test: Test) {
const pipeline = pipelineForTesting();

const secondStage = new codepipeline.Stage(pipeline, 'SecondStage');
const firstStage = new codepipeline.Stage(pipeline, 'FirstStage', {
placed: {
atIndex: 0,
}
});

test.equal(pipeline.stages[0].name, firstStage.name);
test.equal(pipeline.stages[1].name, secondStage.name);

test.done();
},

'can be inserted before another Stage'(test: Test) {
const pipeline = pipelineForTesting();

const secondStage = new codepipeline.Stage(pipeline, 'SecondStage');
const firstStage = new codepipeline.Stage(pipeline, 'FirstStage', {
placed: {
rightBeforeStage: secondStage,
}
});

test.equal(pipeline.stages[0].name, firstStage.name);
test.equal(pipeline.stages[1].name, secondStage.name);

test.done();
},

'can be inserted after another Stage'(test: Test) {
const pipeline = pipelineForTesting();

const firstStage = new codepipeline.Stage(pipeline, 'FirstStage');
const thirdStage = new codepipeline.Stage(pipeline, 'ThirdStage');
const secondStage = new codepipeline.Stage(pipeline, 'SecondStage', {
placed: {
justAfterStage: firstStage,
}
});

test.equal(pipeline.stages[0].name, firstStage.name);
test.equal(pipeline.stages[1].name, secondStage.name);
test.equal(pipeline.stages[2].name, thirdStage.name);

test.done();
},

'attempting to insert a Stage at a negative index results in an error'(test: Test) {
const pipeline = pipelineForTesting();

test.throws(() => {
new codepipeline.Stage(pipeline, 'Stage', {
placed: {
atIndex: -1,
}
});
}, /atIndex/);

test.done();
},

'attempting to insert a Stage at an index larger than the current number of Stages results in an error'(test: Test) {
const pipeline = pipelineForTesting();

test.throws(() => {
new codepipeline.Stage(pipeline, 'Stage', {
placed: {
atIndex: 1,
}
});
}, /atIndex/);

test.done();
},

"attempting to insert a Stage before a Stage that doesn't exist results in an error"(test: Test) {
const pipeline = pipelineForTesting();
const stage = new codepipeline.Stage(pipeline, 'Stage');

const anotherPipeline = pipelineForTesting();
test.throws(() => {
new codepipeline.Stage(anotherPipeline, 'Stage', {
placed: {
rightBeforeStage: stage,
}
});
}, /before/i);

test.done();
},

"attempting to insert a Stage after a Stage that doesn't exist results in an error"(test: Test) {
const pipeline = pipelineForTesting();
const stage = new codepipeline.Stage(pipeline, 'Stage');

const anotherPipeline = pipelineForTesting();
test.throws(() => {
new codepipeline.Stage(anotherPipeline, 'Stage', {
placed: {
justAfterStage: stage,
}
});
}, /after/i);

test.done();
},

"providing more than one placement value results in an error"(test: Test) {
const pipeline = pipelineForTesting();
const stage = new codepipeline.Stage(pipeline, 'FirstStage');

test.throws(() => {
new codepipeline.Stage(pipeline, 'SecondStage', {
placed: {
rightBeforeStage: stage,
justAfterStage: stage,
}
});
});

test.done();
},
},
};

function pipelineForTesting(): codepipeline.Pipeline {
const stack = new cdk.Stack();
const pipeline = new codepipeline.Pipeline(stack, 'Pipeline');
return pipeline;
}
14 changes: 7 additions & 7 deletions packages/@aws-cdk/cdk/lib/core/construct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@ export class Construct {
* Creates a new construct node.
*
* @param parent The parent construct
* @param props Properties for this construct
* @param name The logical identfifier of the new Construct
* @param props Optional properties for this construct
*/
constructor(parent: Construct, name: string) {
constructor(parent: Construct, name: string, props?: any) {
this.name = name;
this.parent = parent;

Expand All @@ -49,7 +50,7 @@ export class Construct {
}

// Has side effect so must be very last thing in constructor
parent.addChild(this, this.name);
parent.addChild(this, this.name, props);
} else {
// This is a root construct.
this.name = name;
Expand Down Expand Up @@ -324,12 +325,11 @@ export class Construct {
* Adds a child construct to this node.
*
* @param child The child construct
* @param name The type name of the child construct.
* @returns The resolved path part name of the child
* @param childName The logical ID of the child construct
* @param _props The optional properties of the child construct
*/
protected addChild(child: Construct, childName: string) {
protected addChild(child: Construct, childName: string, _props?: any) {
if (this.locked) {

// special error if root is locked
if (!this.path) {
throw new Error('Cannot add children during synthesis');
Expand Down