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

The variable namespace identifier in CodePipeline allows a maximum of
100 characters. If we ever come up with an identifier that would be
too long, trim it down.

While writing tests for this, discovered that actions exhibited the same
length issues, and did the same there too.

Fixes #17436.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: rix0rrr

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 });
   }
 }