From 309b9b4cf554474c87fe3d833a5205498e200ecf Mon Sep 17 00:00:00 2001
From: Rico Huijbers <rix0rrr@gmail.com>
Date: Tue, 20 Jul 2021 18:55:02 +0200
Subject: [PATCH] fix(pipelines): new pipeline stages aren't validated (#15665)

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*
---
 .../lib/blueprint/stage-deployment.ts         |  3 +-
 .../@aws-cdk/pipelines/lib/legacy/stage.ts    |  3 +-
 .../lib/private/construct-internals.ts        |  4 ++
 .../pipelines/test/compliance/synths.test.ts  | 46 ++++++++++++++++++-
 4 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/packages/@aws-cdk/pipelines/lib/blueprint/stage-deployment.ts b/packages/@aws-cdk/pipelines/lib/blueprint/stage-deployment.ts
index 499d324dfb25f..b8e3f0ea77536 100644
--- a/packages/@aws-cdk/pipelines/lib/blueprint/stage-deployment.ts
+++ b/packages/@aws-cdk/pipelines/lib/blueprint/stage-deployment.ts
@@ -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';
 
@@ -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.
diff --git a/packages/@aws-cdk/pipelines/lib/legacy/stage.ts b/packages/@aws-cdk/pipelines/lib/legacy/stage.ts
index 8f1d0e205eefc..3b4140fdba5ce 100644
--- a/packages/@aws-cdk/pipelines/lib/legacy/stage.ts
+++ b/packages/@aws-cdk/pipelines/lib/legacy/stage.ts
@@ -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
@@ -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) {
diff --git a/packages/@aws-cdk/pipelines/lib/private/construct-internals.ts b/packages/@aws-cdk/pipelines/lib/private/construct-internals.ts
index fe2ddf1953f64..34d12b84e1d8c 100644
--- a/packages/@aws-cdk/pipelines/lib/private/construct-internals.ts
+++ b/packages/@aws-cdk/pipelines/lib/private/construct-internals.ts
@@ -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
  */
diff --git a/packages/@aws-cdk/pipelines/test/compliance/synths.test.ts b/packages/@aws-cdk/pipelines/test/compliance/synths.test.ts
index 70e012aad8f1d..7c47600b66155 100644
--- a/packages/@aws-cdk/pipelines/test/compliance/synths.test.ts
+++ b/packages/@aws-cdk/pipelines/test/compliance/synths.test.ts
@@ -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;
@@ -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);
+    }
+  }
 });
\ No newline at end of file