fix(core): Prevent volatile physical name generation (#2984)

The PhysicalName generation strategy for cross-account/region use-cases
could generate names that are subject to collisions/sniping when the
account and region were not specified; and those names would also have
changed if a stack went from re-locatable to account/region specific,
even if the target environment would not have changed.

The generator will now throw errors in case the account ID or region is
blank or a Token.

packages/@aws-cdk/aws-codepipeline-actions/test/test.pipeline.ts

@@ -756,8 +756,9 @@ export = {
       const app = new App();
 
       const buildAccount = '901234567890';
+      const buildRegion = 'bermuda-triangle-1';
       const buildStack = new Stack(app, 'BuildStack', {
-        env: { account: buildAccount },
+        env: { account: buildAccount, region: buildRegion },
       });
       const rolePhysicalName = 'ProjectRolePhysicalName';
       const projectRole = new iam.Role(buildStack, 'ProjectRole', {
@@ -771,7 +772,7 @@ export = {
       });
 
       const pipelineStack = new Stack(app, 'PipelineStack', {
-        env: { account: '123456789012' },
+        env: { account: '123456789012', region: 'bermuda-triangle-42' },
       });
       const sourceBucket = new s3.Bucket(pipelineStack, 'ArtifactBucket', {
         bucketName: 'source-bucket',
@@ -826,7 +827,7 @@ export = {
                       {
                         "Ref": "AWS::Partition",
                       },
-                      `:iam::${buildAccount}:role/buildstackebuildactionrole166c75d145cdaa010350`,
+                      `:iam::${buildAccount}:role/buildstackebuildactionrole166c75d1d8be701b1ad8`,
                     ],
                   ],
                 },
@@ -861,7 +862,7 @@ export = {
                       {
                         "Ref": "AWS::Partition",
                       },
-                      `:s3:::pipelinestackeartifactsbucket5409dc8418216ab8debe`,
+                      ':s3:::pipelinestackeartifactsbucket5409dc84ec8d21c5e28c',
                     ],
                   ],
                 },
@@ -873,7 +874,7 @@ export = {
                       {
                         "Ref": "AWS::Partition",
                       },
-                      `:s3:::pipelinestackeartifactsbucket5409dc8418216ab8debe/*`,
+                      ':s3:::pipelinestackeartifactsbucket5409dc84ec8d21c5e28c/*',
                     ],
                   ],
                 },

packages/@aws-cdk/core/lib/private/physical-name-generator.ts

@@ -8,14 +8,14 @@ export function generatePhysicalName(resource: IResource): string {
   const stackPart = new PrefixNamePart(stack.stackName, 25);
   const idPart = new SuffixNamePart(resource.node.uniqueId, 24);
 
-  let region: string | undefined = stack.region;
-  if (Token.isUnresolved(region)) {
-    region = undefined;
+  const region: string = stack.region;
+  if (Token.isUnresolved(region) || !region) {
+    throw new Error(`Cannot generate a physical name for ${resource.node.path}, because the region is un-resolved or missing`);
   }
 
-  let account: string | undefined = stack.account;
-  if (Token.isUnresolved(account)) {
-    account = undefined;
+  const account: string = stack.account;
+  if (Token.isUnresolved(account) || !account) {
+    throw new Error(`Cannot generate a physical name for ${resource.node.path}, because the account is un-resolved or missing`);
   }
 
   const parts = [stackPart, idPart]
@@ -25,8 +25,8 @@ export function generatePhysicalName(resource: IResource): string {
   const sha256 = crypto.createHash('sha256')
     .update(stackPart.bareStr)
     .update(idPart.bareStr)
-    .update(region || '')
-    .update(account || '');
+    .update(region)
+    .update(account);
   const hash = sha256.digest('hex').slice(0, hashLength);
 
   const ret = [...parts, hash].join('');

packages/@aws-cdk/core/test/private/test.physical-name-generator.ts

@@ -0,0 +1,92 @@
+import nodeunit = require('nodeunit');
+import { App, Aws, Resource, Stack } from '../../lib';
+import { generatePhysicalName } from '../../lib/private/physical-name-generator';
+
+export = nodeunit.testCase({
+  'generates correct physical names'(test: nodeunit.Test) {
+    const app = new App();
+    const stack = new Stack(app, 'TestStack', { env: { account: '012345678912', region: 'bermuda-triangle-1' } });
+
+    const testResourceA = new TestResource(stack, 'A');
+    const testResourceB = new TestResource(testResourceA, 'B');
+
+    test.equal(generatePhysicalName(testResourceA), 'teststackteststackaa164c141d59b37c1b663');
+    test.equal(generatePhysicalName(testResourceB), 'teststackteststackab27595cd34d8188283a1f');
+
+    test.done();
+  },
+
+  'generates different names in different accounts'(test: nodeunit.Test) {
+    const appA = new App();
+    const stackA = new Stack(appA, 'TestStack', { env: { account: '012345678912', region: 'bermuda-triangle-1' } });
+    const resourceA = new TestResource(stackA, 'Resource');
+
+    const appB = new App();
+    const stackB = new Stack(appB, 'TestStack', { env: { account: '012345678913', region: 'bermuda-triangle-1' } });
+    const resourceB = new TestResource(stackB, 'Resource');
+
+    test.notEqual(generatePhysicalName(resourceA), generatePhysicalName(resourceB));
+
+    test.done();
+  },
+
+  'generates different names in different regions'(test: nodeunit.Test) {
+    const appA = new App();
+    const stackA = new Stack(appA, 'TestStack', { env: { account: '012345678912', region: 'bermuda-triangle-1' } });
+    const resourceA = new TestResource(stackA, 'Resource');
+
+    const appB = new App();
+    const stackB = new Stack(appB, 'TestStack', { env: { account: '012345678912', region: 'bermuda-triangle-2' } });
+    const resourceB = new TestResource(stackB, 'Resource');
+
+    test.notEqual(generatePhysicalName(resourceA), generatePhysicalName(resourceB));
+
+    test.done();
+  },
+
+  'fails when the region is an unresolved token'(test: nodeunit.Test) {
+    const app = new App();
+    const stack = new Stack(app, 'TestStack', { env: { account: '012345678912', region: Aws.REGION } });
+    const testResource = new TestResource(stack, 'A');
+
+    test.throws(() => generatePhysicalName(testResource),
+      /Cannot generate a physical name for TestStack\/A, because the region is un-resolved or missing/);
+
+    test.done();
+  },
+
+  'fails when the region is not provided'(test: nodeunit.Test) {
+    const app = new App();
+    const stack = new Stack(app, 'TestStack', { env: { account: '012345678912' } });
+    const testResource = new TestResource(stack, 'A');
+
+    test.throws(() => generatePhysicalName(testResource),
+      /Cannot generate a physical name for TestStack\/A, because the region is un-resolved or missing/);
+
+    test.done();
+  },
+
+  'fails when the account is an unresolved token'(test: nodeunit.Test) {
+    const app = new App();
+    const stack = new Stack(app, 'TestStack', { env: { account: Aws.ACCOUNT_ID, region: 'bermuda-triangle-1' } });
+    const testResource = new TestResource(stack, 'A');
+
+    test.throws(() => generatePhysicalName(testResource),
+      /Cannot generate a physical name for TestStack\/A, because the account is un-resolved or missing/);
+
+    test.done();
+  },
+
+  'fails when the account is not provided'(test: nodeunit.Test) {
+    const app = new App();
+    const stack = new Stack(app, 'TestStack', { env: { region: 'bermuda-triangle-1' } });
+    const testResource = new TestResource(stack, 'A');
+
+    test.throws(() => generatePhysicalName(testResource),
+      /Cannot generate a physical name for TestStack\/A, because the account is un-resolved or missing/);
+
+    test.done();
+  },
+});
+
+class TestResource extends Resource {}

packages/@aws-cdk/core/test/test.cross-environment-token.ts

@@ -12,13 +12,15 @@ export = {
       const stack1 = new Stack(app, 'Stack1', {
         env: {
           account: '123456789012',
+          region: 'bermuda-triangle-1337',
         },
       });
       const myResource = new MyResource(stack1, 'MyResource', 'PhysicalName');
 
       const stack2 = new Stack(app, 'Stack2', {
         env: {
           account: '234567890123',
+          region: 'bermuda-triangle-42',
         },
       });
 
@@ -56,11 +58,13 @@ export = {
       const stack1 = new Stack(app, 'Stack1', {
         env: {
           account: '123456789012',
+          region: 'bermuda-triangle-1337',
         },
       });
       const stack2 = new Stack(app, 'Stack2', {
         env: {
           account: '234567890123',
+          region: 'bermuda-triangle-42',
         },
       });
 
@@ -88,13 +92,15 @@ export = {
       const stack1 = new Stack(app, 'Stack1', {
         env: {
           account: '123456789012',
+          region: 'bermuda-triangle-1337',
         },
       });
       const myResource = new MyResource(stack1, 'MyResource', PhysicalName.GENERATE_IF_NEEDED);
 
       const stack2 = new Stack(app, 'Stack2', {
         env: {
           account: '234567890123',
+          region: 'bermuda-triangle-42',
         },
       });
 
@@ -115,7 +121,7 @@ export = {
                   {
                     Ref: 'AWS::Partition',
                   },
-                  ':myservice:::my-resource/stack1stack1myresourcec54ced43dab875fcfa49',
+                  ':myservice:::my-resource/stack1stack1myresourcec54ced43683ebf9a3c4c',
                 ],
               ],
             },
@@ -132,11 +138,13 @@ export = {
       const stack1 = new Stack(app, 'Stack1', {
         env: {
           account: '123456789012',
+          region: 'bermuda-triangle-1337',
         },
       });
       const stack2 = new Stack(app, 'Stack2', {
         env: {
           account: '234567890123',
+          region: 'bermuda-triangle-42',
         },
       });
 
@@ -150,7 +158,7 @@ export = {
       test.deepEqual(toCloudFormation(stack2), {
         Outputs: {
           Output: {
-            Value: 'stack1stack1myresourcec54ced43dab875fcfa49',
+            Value: 'stack1stack1myresourcec54ced43683ebf9a3c4c',
           },
         },
       });
@@ -165,11 +173,13 @@ export = {
     const stack1 = new Stack(app, 'Stack1', {
       env: {
         account: '123456789012',
+        region: 'bermuda-triangle-1337',
       },
     });
     const stack2 = new Stack(app, 'Stack2', {
       env: {
         account: '234567890123',
+        region: 'bermuda-triangle-42',
       },
     });