From 74c7ffffb48fe5578a405b319cc0df973ceb9989 Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Thu, 15 Apr 2021 02:25:25 -0700 Subject: [PATCH] fix(rds): allow Instances to be referenced across environments (#13865) Do it be setting its `physicalName` property that it inherits from `Resource` that allows it to be referenced across environments. Make sure to lowercase the name if a customer provided one explicitly, as otherwise the ARN generated for the cross-environment access will be incorrect. Fixes #13832 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/aws-rds/lib/instance.ts | 42 ++++++++++------- .../@aws-cdk/aws-rds/test/instance.test.ts | 45 +++++++++++++++++++ 2 files changed, 72 insertions(+), 15 deletions(-) diff --git a/packages/@aws-cdk/aws-rds/lib/instance.ts b/packages/@aws-cdk/aws-rds/lib/instance.ts index 5ce8eca7bdf81..632e3cfec431b 100644 --- a/packages/@aws-cdk/aws-rds/lib/instance.ts +++ b/packages/@aws-cdk/aws-rds/lib/instance.ts @@ -5,16 +5,7 @@ import * as kms from '@aws-cdk/aws-kms'; import * as logs from '@aws-cdk/aws-logs'; import * as s3 from '@aws-cdk/aws-s3'; import * as secretsmanager from '@aws-cdk/aws-secretsmanager'; -import { - Duration, - FeatureFlags, - IResource, - Lazy, - RemovalPolicy, - Resource, - Stack, - Token, -} from '@aws-cdk/core'; +import { ArnComponents, Duration, FeatureFlags, IResource, Lazy, RemovalPolicy, Resource, Stack, Token } from '@aws-cdk/core'; import * as cxapi from '@aws-cdk/cx-api'; import { Construct } from 'constructs'; import { DatabaseSecret } from './database-secret'; @@ -196,12 +187,19 @@ export abstract class DatabaseInstanceBase extends Resource implements IDatabase * The instance arn. */ public get instanceArn(): string { - return Stack.of(this).formatArn({ + const commonAnComponents: ArnComponents = { service: 'rds', resource: 'db', sep: ':', + }; + const localArn = Stack.of(this).formatArn({ + ...commonAnComponents, resourceName: this.instanceIdentifier, }); + return this.getResourceArnAttribute(localArn, { + ...commonAnComponents, + resourceName: this.physicalName, + }); } /** @@ -640,7 +638,15 @@ abstract class DatabaseInstanceNew extends DatabaseInstanceBase implements IData protected enableIamAuthentication?: boolean; constructor(scope: Construct, id: string, props: DatabaseInstanceNewProps) { - super(scope, id); + // RDS always lower-cases the ID of the database, so use that for the physical name + // (which is the name used for cross-environment access, so it needs to be correct, + // regardless of the feature flag that changes it in the template for the L1) + const instancePhysicalName = Token.isUnresolved(props.instanceIdentifier) + ? props.instanceIdentifier + : props.instanceIdentifier?.toLowerCase(); + super(scope, id, { + physicalName: instancePhysicalName, + }); this.vpc = props.vpc; if (props.vpcSubnets && props.vpcPlacement) { @@ -697,7 +703,7 @@ abstract class DatabaseInstanceNew extends DatabaseInstanceBase implements IData }); } - const instanceIdentifier = FeatureFlags.of(this).isEnabled(cxapi.RDS_LOWERCASE_DB_IDENTIFIER) + const maybeLowercasedInstanceId = FeatureFlags.of(this).isEnabled(cxapi.RDS_LOWERCASE_DB_IDENTIFIER) ? props.instanceIdentifier?.toLowerCase() : props.instanceIdentifier; @@ -707,7 +713,13 @@ abstract class DatabaseInstanceNew extends DatabaseInstanceBase implements IData backupRetentionPeriod: props.backupRetention?.toDays(), copyTagsToSnapshot: props.copyTagsToSnapshot ?? true, dbInstanceClass: Lazy.string({ produce: () => `db.${this.instanceType}` }), - dbInstanceIdentifier: instanceIdentifier, + dbInstanceIdentifier: Token.isUnresolved(props.instanceIdentifier) + // if the passed identifier is a Token, + // we need to use the physicalName of the database + // (we cannot change its case anyway), + // as it might be used in a cross-environment fashion + ? this.physicalName + : maybeLowercasedInstanceId, dbSubnetGroupName: subnetGroup.subnetGroupName, deleteAutomatedBackups: props.deleteAutomatedBackups, deletionProtection: defaultDeletionProtection(props.deletionProtection, props.removalPolicy), @@ -982,7 +994,7 @@ export class DatabaseInstance extends DatabaseInstanceSource implements IDatabas storageEncrypted: props.storageEncryptionKey ? true : props.storageEncrypted, }); - this.instanceIdentifier = instance.ref; + this.instanceIdentifier = this.getResourceNameAttribute(instance.ref); this.dbInstanceEndpointAddress = instance.attrEndpointAddress; this.dbInstanceEndpointPort = instance.attrEndpointPort; diff --git a/packages/@aws-cdk/aws-rds/test/instance.test.ts b/packages/@aws-cdk/aws-rds/test/instance.test.ts index db227fa24c3a5..323db47326c13 100644 --- a/packages/@aws-cdk/aws-rds/test/instance.test.ts +++ b/packages/@aws-cdk/aws-rds/test/instance.test.ts @@ -1357,3 +1357,48 @@ test.each([ UpdateReplacePolicy: subnetValue, }, ResourcePart.CompleteDefinition); }); + +describe('cross-account instance', () => { + test.each([ + ['MyInstance', 'MyInstance', 'myinstance'], + ['PhysicalName.GENERATE_IF_NEEDED', cdk.PhysicalName.GENERATE_IF_NEEDED, 'instancestackncestackinstancec830ba83756a6dfc7154'], + ])("with database identifier '%s' can be referenced from a Stack in a different account", (_, providedInstanceId, expectedInstanceId) => { + const app = new cdk.App(); + const instanceStack = new cdk.Stack(app, 'InstanceStack', { + env: { account: '123', region: 'my-region' }, + }); + const instance = new rds.DatabaseInstance(instanceStack, 'Instance', { + vpc: new ec2.Vpc(instanceStack, 'Vpc'), + engine: rds.DatabaseInstanceEngine.mariaDb({ version: rds.MariaDbEngineVersion.VER_10_5 }), + // physical name set + instanceIdentifier: providedInstanceId, + }); + + const outputStack = new cdk.Stack(app, 'OutputStack', { + env: { account: '456', region: 'my-region' }, + }); + new cdk.CfnOutput(outputStack, 'DatabaseInstanceArn', { + value: instance.instanceArn, + }); + new cdk.CfnOutput(outputStack, 'DatabaseInstanceName', { + value: instance.instanceIdentifier, + }); + + expect(outputStack).toMatchTemplate({ + Outputs: { + DatabaseInstanceArn: { + Value: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + `:rds:my-region:123:db:${expectedInstanceId}`, + ]], + }, + }, + DatabaseInstanceName: { + Value: expectedInstanceId, + }, + }, + }); + }); +});