Skip to content

Commit

Permalink
fix(pipelines): new pipeline stages aren't validated (#15665)
Browse files Browse the repository at this point in the history
We do validation-as-part-of-synth for legacy pipelines;
also need to do the same for modern pipelines otherwise
failing context lookups are too hard to diagnose.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
rix0rrr authored Jul 20, 2021
1 parent 9532e3c commit 309b9b4
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as cdk from '@aws-cdk/core';
import { CloudFormationStackArtifact } from '@aws-cdk/cx-api';
import { isStackArtifact } from '../private/cloud-assembly-internals';
import { pipelineSynth } from '../private/construct-internals';
import { StackDeployment } from './stack-deployment';
import { Step } from './step';

Expand Down Expand Up @@ -44,7 +45,7 @@ export class StageDeployment {
* in dependency order.
*/
public static fromStage(stage: cdk.Stage, props: StageDeploymentProps = {}) {
const assembly = stage.synth();
const assembly = pipelineSynth(stage);
if (assembly.stacks.length === 0) {
// If we don't check here, a more puzzling "stage contains no actions"
// error will be thrown come deployment time.
Expand Down
3 changes: 2 additions & 1 deletion packages/@aws-cdk/pipelines/lib/legacy/stage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { CdkPipeline } from './pipeline';
// v2 - keep this import as a separate section to reduce merge conflict when forward merging with the v2 branch.
// eslint-disable-next-line
import { Construct as CoreConstruct } from '@aws-cdk/core';
import { pipelineSynth } from '../private/construct-internals';

/**
* Construction properties for a CdkStage
Expand Down Expand Up @@ -113,7 +114,7 @@ export class CdkStage extends CoreConstruct {
* publishing stage.
*/
public addApplication(appStage: Stage, options: AddStageOptions = {}) {
const asm = appStage.synth({ validateOnSynthesis: true });
const asm = pipelineSynth(appStage);
const extraRunOrderSpace = options.extraRunOrderSpace ?? 0;

if (options.confirmBroadeningPermissions ?? this.confirmBroadeningPermissions) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ export function assemblyBuilderOf(stage: Stage): cxapi.CloudAssemblyBuilder {
return (stage as any)._assemblyBuilder;
}

export function pipelineSynth(stage: Stage) {
return stage.synth({ validateOnSynthesis: true });
}

/**
* Return the relative path from the app assembly to the scope's (nested) assembly
*/
Expand Down
46 changes: 45 additions & 1 deletion packages/@aws-cdk/pipelines/test/compliance/synths.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import * as s3 from '@aws-cdk/aws-s3';
import { Stack } from '@aws-cdk/core';
import * as cdkp from '../../lib';
import { CodeBuildStep } from '../../lib';
import { behavior, PIPELINE_ENV, TestApp, LegacyTestGitHubNpmPipeline, ModernTestGitHubNpmPipeline, ModernTestGitHubNpmPipelineProps } from '../testhelpers';
import { behavior, PIPELINE_ENV, TestApp, LegacyTestGitHubNpmPipeline, ModernTestGitHubNpmPipeline, ModernTestGitHubNpmPipelineProps, OneStackApp } from '../testhelpers';

let app: TestApp;
let pipelineStack: Stack;
Expand Down Expand Up @@ -1104,4 +1104,48 @@ behavior('can provide custom BuildSpec that is merged with generated one', (suit
},
});
}
});

behavior('stacks synthesized for pipeline will be checked during synth', (suite) => {
let stage: OneStackApp;
beforeEach(() => {
stage = new OneStackApp(pipelineStack, 'MyApp');
});

suite.legacy(() => {
// WHEN
const pipeline = new LegacyTestGitHubNpmPipeline(pipelineStack, 'Cdk', {
sourceArtifact,
cloudAssemblyArtifact,
synthAction: new cdkp.SimpleSynthAction({
sourceArtifact,
cloudAssemblyArtifact,
installCommands: ['install1', 'install2'],
buildCommands: ['build1', 'build2'],
testCommands: ['test1', 'test2'],
synthCommand: 'cdk synth',
}),
});
pipeline.addApplicationStage(stage);

THEN();
});

suite.modern(() => {
const pipeline = new ModernTestGitHubNpmPipeline(pipelineStack, 'Cdk', {
installCommands: ['install1', 'install2'],
commands: ['build1', 'build2', 'test1', 'test2', 'cdk synth'],
});
pipeline.addStage(stage);

THEN();
});

function THEN() {
// All stacks in the ASM have been synthesized with 'validateOnSynth: true'
const asm = stage.synth();
for (const stack of asm.stacks) {
expect(stack.validateOnSynth).toEqual(true);
}
}
});

0 comments on commit 309b9b4

Please sign in to comment.