Merge branch 'master' into upparekh/support-for-elastic-inference-acc…

…elerators
aws · Apr 15, 2021 · 790a118 · 790a118
2 parents 3703898 + 74c7fff
commit 790a118
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 15 deletions.
diff --git 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
@@ -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,
+        },
+      },
+    });
+  });
+});