fix(core): exportValue() does not work with resource names (#13052)

Most L2 resources employ the "PhysicalName" protocol, which checks usage of
resource names across environment borders, and can automatically turn
auto-named resources into physically-named resources if the situation calls for
it.

Unfortunately, this wrapped token is a generic IResolvable, not a Reference,
and so did not work with the `exportValue()` automatic reference detection.

Make the token returned by `getResourceNameAttribute()` etc. a `Reference`
that mimics the underlying `Reference` to make this work out.

Fixes #13002, fixes #12918.


----

*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/aws-s3/test/bucket.test.ts

@@ -2405,7 +2405,5 @@ describe('bucket', () => {
     expect(() => new s3.Bucket(stack, 'MyBucket', {
       autoDeleteObjects: true,
     })).toThrow(/Cannot use \'autoDeleteObjects\' property on a bucket without setting removal policy to \'DESTROY\'/);
-
-
   });
 });

packages/@aws-cdk/core/README.md

@@ -141,15 +141,15 @@ DEPLOYMENT 1: break the relationship
 - Make sure `stack2` no longer references `bucket.bucketName` (maybe the consumer
   stack now uses its own bucket, or it writes to an AWS DynamoDB table, or maybe you just
   remove the Lambda Function altogether).
-- In the `stack1` class, call `this.exportAttribute(this.bucket.bucketName)`. This
+- In the `stack1` class, call `this.exportValue(this.bucket.bucketName)`. This
   will make sure the CloudFormation Export continues to exist while the relationship
   between the two stacks is being broken.
 - Deploy (this will effectively only change the `stack2`, but it's safe to deploy both).
 
 DEPLOYMENT 2: remove the resource
 
 - You are now free to remove the `bucket` resource from `stack1`.
-- Don't forget to remove the `exportAttribute()` call as well.
+- Don't forget to remove the `exportValue()` call as well.
 - Deploy again (this time only the `stack1` will be changed -- the bucket will be deleted).
 
 ## Durations

packages/@aws-cdk/core/lib/resource.ts

@@ -4,11 +4,12 @@ import { IConstruct, Construct as CoreConstruct } from './construct-compat';
 
 import { Construct } from 'constructs';
 import { ArnComponents } from './arn';
-import { Lazy } from './lazy';
+import { IStringProducer, Lazy } from './lazy';
 import { generatePhysicalName, isGeneratedWhenNeededMarker } from './private/physical-name-generator';
 import { IResolveContext } from './resolvable';
 import { Stack } from './stack';
-import { Token } from './token';
+import { Token, Tokenization } from './token';
+import { Reference } from './reference';
 
 /**
  * Represents the environment a given resource lives in.
@@ -181,7 +182,7 @@ export abstract class Resource extends CoreConstruct implements IResource {
    * @experimental
    */
   protected getResourceNameAttribute(nameAttr: string) {
-    return Lazy.uncachedString({
+    return mimicReference(nameAttr, {
       produce: (context: IResolveContext) => {
         const consumingStack = Stack.of(context.scope);
 
@@ -214,8 +215,8 @@ export abstract class Resource extends CoreConstruct implements IResource {
    * @experimental
    */
   protected getResourceArnAttribute(arnAttr: string, arnComponents: ArnComponents) {
-    return Token.asString({
-      resolve: (context: IResolveContext) => {
+    return mimicReference(arnAttr, {
+      produce: (context: IResolveContext) => {
         const consumingStack = Stack.of(context.scope);
         if (this.stack.environment !== consumingStack.environment) {
           this._enableCrossEnvironment();
@@ -227,3 +228,28 @@ export abstract class Resource extends CoreConstruct implements IResource {
     });
   }
 }
+
+/**
+ * Produce a Lazy that is also a Reference (if the base value is a Reference).
+ *
+ * If the given value is a Reference (or resolves to a Reference), return a new
+ * Reference that mimics the same target and display name, but resolves using
+ * the logic of the passed lazy.
+ *
+ * If the given value is NOT a Reference, just return a simple Lazy.
+ */
+function mimicReference(refSource: any, producer: IStringProducer): string {
+  const reference = Tokenization.reverse(refSource, {
+    // If this is an ARN concatenation, just fail to extract a reference.
+    failConcat: false,
+  });
+  if (!Reference.isReference(reference)) {
+    return Lazy.uncachedString(producer);
+  }
+
+  return Token.asString(new class extends Reference {
+    resolve(context: IResolveContext) {
+      return producer.produce(context);
+    }
+  }(reference, reference.target, reference.displayName));
+}

packages/@aws-cdk/core/lib/token.ts

@@ -164,9 +164,16 @@ export class Tokenization {
    *
    * In case of a string, the string must not be a concatenation.
    */
-  public static reverse(x: any): IResolvable | undefined {
+  public static reverse(x: any, options: ReverseOptions = {}): IResolvable | undefined {
     if (Tokenization.isResolvable(x)) { return x; }
-    if (typeof x === 'string') { return Tokenization.reverseCompleteString(x); }
+    if (typeof x === 'string') {
+      if (options.failConcat === false) {
+        // Handle this specially because reverseCompleteString might fail
+        const fragments = Tokenization.reverseString(x);
+        return fragments.length === 1 ? fragments.firstToken : undefined;
+      }
+      return Tokenization.reverseCompleteString(x);
+    }
     if (Array.isArray(x)) { return Tokenization.reverseList(x); }
     if (typeof x === 'number') { return Tokenization.reverseNumber(x); }
     return undefined;
@@ -220,6 +227,20 @@ export class Tokenization {
   }
 }
 
+/**
+ * Options for the 'reverse()' operation
+ */
+export interface ReverseOptions {
+  /**
+   * Fail if the given string is a concatenation
+   *
+   * If `false`, just return `undefined`.
+   *
+   * @default true
+   */
+  readonly failConcat?: boolean;
+}
+
 /**
  * Options to the resolve() operation
  *

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

@@ -244,27 +244,91 @@ nodeunitShim({
   },
 });
 
+test.each([undefined, 'SomeName'])('stack.exportValue() on name attributes with PhysicalName=%s', physicalName => {
+  // Check that automatic exports and manual exports look the same
+  // GIVEN - auto
+  const appA = new App();
+  const producerA = new Stack(appA, 'Producer');
+  const resourceA = new MyResource(producerA, 'Resource', physicalName);
+
+  const consumerA = new Stack(appA, 'Consumer');
+  new CfnOutput(consumerA, 'ConsumeName', { value: resourceA.name });
+  new CfnOutput(consumerA, 'ConsumeArn', { value: resourceA.arn });
+
+  // WHEN - manual
+  const appM = new App();
+  const producerM = new Stack(appM, 'Producer');
+  const resourceM = new MyResource(producerM, 'Resource', physicalName);
+  producerM.exportValue(resourceM.name);
+  producerM.exportValue(resourceM.arn);
+
+  // THEN - producers are the same
+  const templateA = appA.synth().getStackByName(producerA.stackName).template;
+  const templateM = appM.synth().getStackByName(producerM.stackName).template;
+
+  expect(templateA).toEqual(templateM);
+});
+
+test('can instantiate resource with constructed physicalname ARN', () => {
+  const stack = new Stack();
+  new MyResourceWithConstructedArnAttribute(stack, 'Resource');
+});
+
 class MyResource extends Resource {
   public readonly arn: string;
   public readonly name: string;
 
   constructor(scope: Construct, id: string, physicalName?: string) {
     super(scope, id, { physicalName });
 
-    this.arn = this.getResourceArnAttribute('simple-arn', {
+    const res = new CfnResource(this, 'Resource', {
+      type: 'My::Resource',
+      properties: {
+        resourceName: this.physicalName,
+      },
+    });
+
+    this.name = this.getResourceNameAttribute(res.ref.toString());
+    this.arn = this.getResourceArnAttribute(res.getAtt('Arn').toString(), {
       region: '',
       account: '',
       resource: 'my-resource',
       resourceName: this.physicalName,
       service: 'myservice',
     });
-    this.name = this.getResourceNameAttribute('simple-name');
+  }
+}
 
-    new CfnResource(this, 'Resource', {
+/**
+ * Some resources build their own Arn attribute by constructing strings
+ *
+ * This will be because the L1 doesn't expose a `{ Fn::GetAtt: ['Arn'] }`.
+ *
+ * They won't be able to `exportValue()` it, but it shouldn't crash.
+ */
+class MyResourceWithConstructedArnAttribute extends Resource {
+  public readonly arn: string;
+  public readonly name: string;
+
+  constructor(scope: Construct, id: string, physicalName?: string) {
+    super(scope, id, { physicalName });
+
+    const res = new CfnResource(this, 'Resource', {
       type: 'My::Resource',
       properties: {
         resourceName: this.physicalName,
       },
     });
+
+    this.name = this.getResourceNameAttribute(res.ref.toString());
+    this.arn = this.getResourceArnAttribute(Stack.of(this).formatArn({
+      resource: 'my-resource',
+      resourceName: res.ref.toString(),
+      service: 'myservice',
+    }), {
+      resource: 'my-resource',
+      resourceName: this.physicalName,
+      service: 'myservice',
+    });
   }
 }