Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Feb 16, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions packages/@aws-cdk/aws-s3/test/bucket.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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\'/);


});
});
4 changes: 2 additions & 2 deletions packages/@aws-cdk/core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 28 additions & 5 deletions packages/@aws-cdk/core/lib/resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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();
Expand All @@ -227,3 +228,25 @@ 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 (!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));
}
43 changes: 34 additions & 9 deletions packages/@aws-cdk/core/test/cross-environment-token.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,27 +244,52 @@ 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);
});

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', {
type: 'My::Resource',
properties: {
resourceName: this.physicalName,
},
});
}
}