fix(pipelines): can't use exports from very long stack names

packages/@aws-cdk/pipelines/lib/codepipeline/_codebuild-factory.ts

@@ -1,4 +1,3 @@
-import * as crypto from 'crypto';
 import * as fs from 'fs';
 import * as path from 'path';
 import * as codebuild from '@aws-cdk/aws-codebuild';
@@ -8,9 +7,10 @@ import * as ec2 from '@aws-cdk/aws-ec2';
 import * as iam from '@aws-cdk/aws-iam';
 import { IDependable, Stack } from '@aws-cdk/core';
 import { Construct, Node } from 'constructs';
-import { FileSetLocation, ShellStep, StackDeployment, StackOutputReference } from '../blueprint';
+import { FileSetLocation, ShellStep, StackOutputReference } from '../blueprint';
 import { PipelineQueries } from '../helpers-internal/pipeline-queries';
 import { cloudAssemblyBuildSpecDir, obtainScope } from '../private/construct-internals';
+import { hash, stackVariableNamespace } from '../private/identifiers';
 import { mapValues, mkdict, noEmptyObject, noUndefined, partition } from '../private/javascript';
 import { ArtifactMap } from './artifact-map';
 import { CodeBuildStep } from './codebuild-step';
@@ -426,12 +426,6 @@ function isDefined<A>(x: A | undefined): x is NonNullable<A> {
   return x !== undefined;
 }
 
-function hash<A>(obj: A) {
-  const d = crypto.createHash('sha256');
-  d.update(JSON.stringify(obj));
-  return d.digest('hex');
-}
-
 /**
  * Serialize a build environment to data (get rid of constructs & objects), so we can JSON.stringify it
  */
@@ -447,10 +441,6 @@ function serializeBuildEnvironment(env: codebuild.BuildEnvironment) {
   };
 }
 
-export function stackVariableNamespace(stack: StackDeployment) {
-  return stack.stackArtifactId;
-}
-
 /**
  * Whether the given string contains a reference to a CodePipeline variable
  */

packages/@aws-cdk/pipelines/lib/codepipeline/codepipeline.ts

@@ -9,14 +9,15 @@ import * as cxapi from '@aws-cdk/cx-api';
 import { Construct, Node } from 'constructs';
 import { AssetType, FileSet, IFileSetProducer, ManualApprovalStep, ShellStep, StackAsset, StackDeployment, Step } from '../blueprint';
 import { DockerCredential, dockerCredentialsInstallCommands, DockerCredentialUsage } from '../docker-credentials';
-import { GraphNode, GraphNodeCollection, isGraph, AGraphNode, PipelineGraph } from '../helpers-internal';
+import { GraphNodeCollection, isGraph, AGraphNode, PipelineGraph } from '../helpers-internal';
 import { PipelineBase } from '../main';
 import { AssetSingletonRole } from '../private/asset-singleton-role';
 import { appOf, assemblyBuilderOf, embeddedAsmPath, obtainScope } from '../private/construct-internals';
 import { toPosixPath } from '../private/fs';
+import { actionName, stackVariableNamespace } from '../private/identifiers';
 import { enumerate, flatten, maybeSuffix, noUndefined } from '../private/javascript';
 import { writeTemplateConfiguration } from '../private/template-configuration';
-import { CodeBuildFactory, mergeCodeBuildOptions, stackVariableNamespace } from './_codebuild-factory';
+import { CodeBuildFactory, mergeCodeBuildOptions } from './_codebuild-factory';
 import { ArtifactMap } from './artifact-map';
 import { CodeBuildStep } from './codebuild-step';
 import { CodePipelineActionFactoryResult, ICodePipelineActionFactory } from './codepipeline-action-factory';
@@ -839,15 +840,6 @@ enum CodeBuildProjectType {
   STEP = 'STEP',
 }
 
-function actionName<A>(node: GraphNode<A>, parent: GraphNode<A>) {
-  const names = node.ancestorPath(parent).map(n => n.id);
-  return names.map(sanitizeName).join('.').substr(0, 100); // Cannot exceed 100 chars
-}
-
-function sanitizeName(x: string): string {
-  return x.replace(/[^A-Za-z0-9.@\-_]/g, '_');
-}
-
 /**
  * Take a set of tranches and split them up into groups so
  * that no set of tranches has more than n items total

packages/@aws-cdk/pipelines/lib/private/identifiers.ts

@@ -0,0 +1,61 @@
+import * as crypto from 'crypto';
+import { StackDeployment } from '../blueprint/stack-deployment';
+import { GraphNode } from '../helpers-internal/graph';
+
+export function hash<A>(obj: A) {
+  const d = crypto.createHash('sha256');
+  d.update(JSON.stringify(obj));
+  return d.digest('hex');
+}
+
+export function actionName<A>(node: GraphNode<A>, parent: GraphNode<A>) {
+  const names = node.ancestorPath(parent).map(n => n.id).map(sanitizeName);
+
+  // Something slightly complicated here:
+  //
+  // Because we'll have structured action names like:
+  //
+  //     'VeryLongStackName.Prepare', 'VeryLongStackName.Deploy'
+  //
+  // it would be shitty if cut and hashed them differently:
+  //
+  //     'VeryLongSAAAAA.Prepare', 'VeryLonBBBBBme.Deploy'
+  //
+  // wouldn't sort and comprehend nicely. We will therefore trim each component individually.
+  const totalMax = 100; // Max length of everything
+
+  // No need to do anything
+  if (names.join('.').length <= totalMax) {
+    return names.join('.');
+  }
+
+  const componentMin = 15; // Can't really go shorter than this, becomes unreadable
+  const dots = names.length - 1;
+  const maxLength = Math.max(componentMin, Math.floor((totalMax - dots) / names.length));
+  const trimmedNames = names.map(name => limitIdentifierLength(name, maxLength));
+
+  return limitIdentifierLength(trimmedNames.join('.'), totalMax); // Final trim in case we couldn't make it
+}
+
+export function stackVariableNamespace(stack: StackDeployment) {
+  return limitIdentifierLength(stack.stackArtifactId, 100);
+}
+
+function sanitizeName(x: string): string {
+  return x.replace(/[^A-Za-z0-9.@\-_]/g, '_');
+}
+
+
+/**
+ * Makes sure the given identifier length does not exceed N characters
+ *
+ * Replaces characters in the middle (to leave the start and end identifiable) and replaces
+ * them with a hash to prevent collissions.
+ */
+export function limitIdentifierLength(s: string, n: number): string {
+  if (s.length <= n) { return s; }
+  const h = hash(s).substr(0, 8);
+  const mid = Math.floor((n - h.length) / 2);
+
+  return s.substr(0, mid) + h + s.substr(s.length - mid);
+}

packages/@aws-cdk/pipelines/test/codepipeline/codebuild-step.test.ts

@@ -1,7 +1,7 @@
 import { Template, Match } from '@aws-cdk/assertions';
 import { Stack } from '@aws-cdk/core';
 import * as cdkp from '../../lib';
-import { PIPELINE_ENV, TestApp } from '../testhelpers';
+import { PIPELINE_ENV, TestApp, ModernTestGitHubNpmPipeline, AppWithOutput } from '../testhelpers';
 
 let app: TestApp;
 let pipelineStack: Stack;
@@ -41,4 +41,25 @@ test('additionalinputs creates the right commands', () => {
       })),
     },
   });
+});
+
+test('envFromOutputs works even with very long stage and stack names', () => {
+  const pipeline = new ModernTestGitHubNpmPipeline(pipelineStack, 'Cdk');
+
+  const myApp = new AppWithOutput(app, 'Alpha'.repeat(20), {
+    stackId: 'Stack'.repeat(20),
+  });
+
+  pipeline.addStage(myApp, {
+    post: [
+      new cdkp.ShellStep('Approve', {
+        commands: ['/bin/true'],
+        envFromCfnOutputs: {
+          THE_OUTPUT: myApp.theOutput,
+        },
+      }),
+    ],
+  });
+
+  // THEN - did not throw an error about identifier lengths
 });

packages/@aws-cdk/pipelines/test/private/identifiers.test.ts

@@ -0,0 +1,36 @@
+import { Graph } from '../../lib/helpers-internal';
+import { actionName } from '../../lib/private/identifiers';
+import { mkGraph } from '../blueprint/helpers-internal/util';
+
+test('actionName trims subcomponents the same way', () => {
+  const long1 = 'ExtremelyLong'.repeat(10);
+  const long2 = 'AlsoLong'.repeat(10);
+
+  const g = mkGraph('MyGraph', G => {
+    G.graph(long1, [], G1 => {
+      G1.graph(long2, [], G2 => {
+        G2.node('Prepare');
+        G2.node('Deploy');
+      });
+    });
+  });
+
+  const G2 = ((g.tryGetChild(long1) as Graph<any>)?.tryGetChild(long2) as Graph<any>);
+  expect(G2).toBeDefined();
+
+  const prep = G2.tryGetChild('Prepare');
+  const deploy = G2.tryGetChild('Deploy');
+
+  expect(prep).toBeDefined();
+  expect(deploy).toBeDefined();
+
+  // ActionNames have the same prefix
+  const prepParts = actionName(prep!, g).split('.');
+  const deployParts = actionName(deploy!, g).split('.');
+
+  // Final parts are unchanged
+  expect(prepParts.pop()).toEqual('Prepare');
+  expect(deployParts.pop()).toEqual('Deploy');
+  // Prefixes are the same
+  expect(prepParts).toEqual(deployParts);
+});

packages/@aws-cdk/pipelines/test/testhelpers/test-app.ts

@@ -62,13 +62,17 @@ export class OneStackApp extends Stage {
   }
 }
 
+export interface AppWithOutputProps extends StageProps {
+  readonly stackId?: string;
+}
+
 export class AppWithOutput extends Stage {
   public readonly theOutput: CfnOutput;
 
-  constructor(scope: Construct, id: string, props?: StageProps) {
+  constructor(scope: Construct, id: string, props: AppWithOutputProps = {}) {
     super(scope, id, props);
 
-    const stack = new BucketStack(this, 'Stack');
+    const stack = new BucketStack(this, props.stackId ?? 'Stack');
     this.theOutput = new CfnOutput(stack, 'MyOutput', { value: stack.bucket.bucketName });
   }
 }