Skip to content

Commit

Permalink
Draft: feat(rds):use unique resource name for Database Proxy (under f…
Browse files Browse the repository at this point in the history
…eature flag)

fixes aws#18578
  • Loading branch information
Yamazaki Hiroki committed Jan 14, 2023
1 parent 2dfaaf4 commit cfbe31a
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 3 deletions.
9 changes: 7 additions & 2 deletions packages/@aws-cdk/aws-rds/lib/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as ec2 from '@aws-cdk/aws-ec2';
import * as iam from '@aws-cdk/aws-iam';
import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
import * as cdk from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import { Construct } from 'constructs';
import { IDatabaseCluster } from './cluster-ref';
import { IEngine } from './engine';
Expand Down Expand Up @@ -414,7 +415,11 @@ export class DatabaseProxy extends DatabaseProxyBase
private readonly resource: CfnDBProxy;

constructor(scope: Construct, id: string, props: DatabaseProxyProps) {
super(scope, id, { physicalName: props.dbProxyName || id });
super(scope, id);

const physicalName = props.dbProxyName || (
cdk.FeatureFlags.of(this).isEnabled(cxapi.DATABASE_PROXY_UNIQUE_RESOURCE_NAME) ? cdk.Names.uniqueResourceName(this, {}) : id
);

const role = props.role || new iam.Role(this, 'IAMRole', {
assumedBy: new iam.ServicePrincipal('rds.amazonaws.com'),
Expand Down Expand Up @@ -447,7 +452,7 @@ export class DatabaseProxy extends DatabaseProxyBase
secretArn: _.secretArn,
};
}),
dbProxyName: this.physicalName,
dbProxyName: physicalName,
debugLogging: props.debugLogging,
engineFamily: bindResult.engineFamily,
idleClientTimeout: props.idleClientTimeout?.toSeconds(),
Expand Down
50 changes: 50 additions & 0 deletions packages/@aws-cdk/aws-rds/test/proxy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,56 @@ describe('proxy', () => {
});
});

test('create a DB proxy from an instance with a unique resource name when flag enabled', () => {
// GIVEN
stack = new cdk.Stack();
stack.node.setContext('@aws-cdk/aws-rds:databaseProxyUniqueResourceName', true);
vpc = new ec2.Vpc(stack, 'VPC');
const instance = new rds.DatabaseInstance(stack, 'Instance', {
engine: rds.DatabaseInstanceEngine.mysql({
version: rds.MysqlEngineVersion.VER_5_7,
}),
vpc,
});

// WHEN
new rds.DatabaseProxy(stack, 'Proxy', {
proxyTarget: rds.ProxyTarget.fromInstance(instance),
secrets: [instance.secret!],
vpc,
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::RDS::DBProxy', {
Auth: [
{
AuthScheme: 'SECRETS',
IAMAuth: 'DISABLED',
SecretArn: {
Ref: 'InstanceSecretAttachment83BEE581',
},
},
],
DBProxyName: 'Proxy',
EngineFamily: 'MYSQL',
RequireTLS: true,
RoleArn: {
'Fn::GetAtt': [
'ProxyIAMRole2FE8AB0F',
'Arn',
],
},
VpcSubnetIds: [
{
Ref: 'VPCPrivateSubnet1Subnet8BCA10E0',
},
{
Ref: 'VPCPrivateSubnet2SubnetCFCDAA7A',
},
],
});
});

test('create a DB proxy from a cluster', () => {
// GIVEN
const cluster = new rds.DatabaseCluster(stack, 'Database', {
Expand Down
22 changes: 21 additions & 1 deletion packages/@aws-cdk/cx-api/FEATURE_FLAGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ Flags come in three types:
| [@aws-cdk/aws-iam:standardizedServicePrincipals](#aws-cdkaws-iamstandardizedserviceprincipals) | Use standardized (global) service principals everywhere | 2.51.0 | (fix) |
| [@aws-cdk/aws-s3:serverAccessLogsUseBucketPolicy](#aws-cdkaws-s3serveraccesslogsusebucketpolicy) | Use S3 Bucket Policy instead of ACLs for Server Access Logging | 2.59.0 | (fix) |
| [@aws-cdk/aws-iam:importedRoleStackSafeDefaultPolicyName](#aws-cdkaws-iamimportedrolestacksafedefaultpolicyname) | Enable this feature to by default create default policy names for imported roles that depend on the stack the role is in. | V2NEXT | (fix) |
| [@aws-cdk/aws-rds:databaseProxyUniqueResourceName](#aws-cdkaws-rdsdatabaseproxyuniqueresourcename) | Use unique resource name for Database Proxy | V2NEXT | (fix) |

<!-- END table -->

Expand Down Expand Up @@ -72,7 +73,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou
"@aws-cdk/aws-iam:standardizedServicePrincipals": true,
"@aws-cdk/aws-ecs:disableExplicitDeploymentControllerForCircuitBreaker": true,
"@aws-cdk/aws-iam:importedRoleStackSafeDefaultPolicyName": true,
"@aws-cdk/aws-s3:serverAccessLogsUseBucketPolicy": true
"@aws-cdk/aws-s3:serverAccessLogsUseBucketPolicy": true,
"@aws-cdk/aws-rds:databaseProxyUniqueResourceName": true
}
}
```
Expand Down Expand Up @@ -736,4 +738,22 @@ This new implementation creates default policy names based on the constructs nod
| V2NEXT | `false` | `true` |


### @aws-cdk/aws-rds:databaseProxyUniqueResourceName

*Use unique resource name for Database Proxy* (fix)

If this flag is not set, the default behavior for `DatabaseProxy` is to use `id` of the constructor for `dbProxyName` when it's not specified in the argument.
In this case, users can't deploy `DatabaseProxy`s that have the same `id` in the same region.

If this flag is set, the default behavior is to use unique resource names for each `DatabaseProxy`.

This is a feature flag as the old behavior was technically incorrect, but users may have come to depend on it.


| Since | Default | Recommended |
| ----- | ----- | ----- |
| (not in v1) | | |
| V2NEXT | `false` | `true` |


<!-- END details -->
16 changes: 16 additions & 0 deletions packages/@aws-cdk/cx-api/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,3 +139,19 @@ https://docs.aws.amazon.com/AmazonS3/latest/userguide/enable-server-access-loggi
}
}
```

* `@aws-cdk/aws-rds:databaseProxyUniqueResourceName`

Enable this feature flag to use unique resource names for each `DatabaseProxy`.

Previously, the default behavior for `DatabaseProxy` was to use `id` of the constructor for `dbProxyName` . In this case, users couldn't deploy `DatabaseProxy`s that have the same `id` in the same region.

This is a feature flag as the old behavior was technically incorrect, but users may have come to depend on it.

```json
{
"context": {
"@aws-cdk/aws-rds:databaseProxyUniqueResourceName": true
}
}
```
18 changes: 18 additions & 0 deletions packages/@aws-cdk/cx-api/lib/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export const EVENTS_TARGET_QUEUE_SAME_ACCOUNT = '@aws-cdk/aws-events:eventsTarge
export const IAM_STANDARDIZED_SERVICE_PRINCIPALS = '@aws-cdk/aws-iam:standardizedServicePrincipals';
export const ECS_DISABLE_EXPLICIT_DEPLOYMENT_CONTROLLER_FOR_CIRCUIT_BREAKER = '@aws-cdk/aws-ecs:disableExplicitDeploymentControllerForCircuitBreaker';
export const S3_SERVER_ACCESS_LOGS_USE_BUCKET_POLICY = '@aws-cdk/aws-s3:serverAccessLogsUseBucketPolicy';
export const DATABASE_PROXY_UNIQUE_RESOURCE_NAME = '@aws-cdk/aws-rds:databaseProxyUniqueResourceName';

export const FLAGS: Record<string, FlagInfo> = {
//////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -593,6 +594,23 @@ export const FLAGS: Record<string, FlagInfo> = {
introducedIn: { v2: '2.59.0' },
recommendedValue: true,
},

//////////////////////////////////////////////////////////////////////
[DATABASE_PROXY_UNIQUE_RESOURCE_NAME]: {
type: FlagType.BugFix,
summary: 'Use unique resource name for Database Proxy',
detailsMd: `
If this flag is not set, the default behavior for \`DatabaseProxy\` is
to use \`id\` of the constructor for \`dbProxyName\` when it's not specified in the argument..
In this case, users can't deploy \`DatabaseProxy\`s that have the same \`id\` in the same region.
If this flag is set, the default behavior is to use unique resource names for each \`DatabaseProxy\`.
This is a feature flag as the old behavior was technically incorrect, but users may have come to depend on it.
`,
introducedIn: { v2: 'V2NEXT' },
recommendedValue: true,
},
};

const CURRENT_MV = 'v2';
Expand Down

0 comments on commit cfbe31a

Please sign in to comment.