RDS: add support for creating Proxy endpoints

aalam-tu commented 3 years ago

I have created a rds cluster:

I am trying to create a rds proxy with both reader and read/write endpoints using AWS CDK

Reproduction Steps

I have created a rds proxy using:

    this.rdsCluster = new DatabaseCluster(this, clusterName, {
      clusterIdentifier: clusterName,
      engine: clusterEngine,
      credentials: {
        username: ResourcePropsConstants.RDS_MASTER_USER,
        excludeCharacters: ';',
      },
      instanceProps: {
        instanceType: new InstanceType(RdsUtils.determineInstanceSize(envName)),
        vpc: this.iVpc,
        vpcSubnets: {
          subnetType: SubnetType.PRIVATE,
        },
        securityGroups: [this.iRdsSG],
      },
      storageEncrypted: true,
      backup: {
        retention: Duration.days(RdsUtils.determineRetentionPeriod(envName)),
      },
      parameterGroup: this.parameterGroup,
      deletionProtection: isHigherEnv,
    });

    this.rdsProxy = this.rdsCluster.addProxy(dbProxyName, {
      dbProxyName,
      borrowTimeout: Duration.minutes(3),
      maxConnectionsPercent: 85,
      maxIdleConnectionsPercent: 10,
      idleClientTimeout: Duration.minutes(15),
      requireTLS: true,
      secrets: [this.rdsSecret],
      securityGroups: [this.iRdsProxySG],
      vpcSubnets: {
        subnetType: SubnetType.PRIVATE,
      },
      vpc: this.iVpc,
    });

What did you expect to happen?

I am expecting two endpoints to be created in the proxy - one with Read/Write and the other with Read target role. When I manually create a proxy using AWS Console a proxy is created with two endpoints

What actually happened?

Only one endpoint was created with Read/Write target role

Environment

**CDK CLI Version :1.66
Framework Version:
Node.js Version:
OS :
Language (Version): -- Typescript

Other

This is :bug: Bug Report

skinny85 commented 3 years ago

Thanks for opening the issue @aalam-tu. I don't think it's a bug. What you probably want is to have a Proxy TargetGroup that points to both instances of the Cluster.

We don't support that in the ProxyTarget class yet, so I'm changing this to a feature request. You could probably use the low-level CfnDBProxyTargetGroup class to achieve that today, you just have to take out the Instances from a Cluster. Perhaps something like this?

this.rdsCluster.node.children.forEach(child => {
  if (child instanceof rds.CfnDBInstance) {
    // do something with `child`

Hope this helps!

Thanks, Adam

aalam-tu commented 3 years ago

Thanks for the reply @skinny85.

Also, Initially I tried using this bit of code to generate the proxy and it created only one endpoint.

    this.rdsProxy = new DatabaseProxy(this, dbProxyName, {
      proxyTarget: ProxyTarget.fromCluster(dbCluster),
      dbProxyName,
      maxConnectionsPercent: 85,
      maxIdleConnectionsPercent: 1,
      borrowTimeout: Duration.minutes(3),
      secret: this.rdsCluster.secret,
      iamAuth: false,
      securityGroups: [this.iRdsProxySG],
      vpcSubnets: {
        subnetType: SubnetType.PRIVATE,
      },
      vpc: this.iVpc,
      idleClientTimeout: Duration.minutes(15),
      requireTLS: true,
    });

    // https://github.com/aws/aws-cdk/issues/8885
    const targetGroup = this.rdsProxy.node.findChild('ProxyTargetGroup') as CfnDBProxyTargetGroup;
    targetGroup.addOverride('Properties.TargetGroupName', 'default');
    targetGroup.addOverride('Properties.DBClusterIdentifiers', [this.rdsCluster.clusterIdentifier]);
    targetGroup.addOverride('Properties.DBInstanceIdentifiers', []);

I tried to override the DBInstanceIdentifiers :

    const targetGroup = this.rdsProxy.node.findChild('ProxyTargetGroup') as CfnDBProxyTargetGroup;
    targetGroup.addOverride('Properties.DBInstanceIdentifiers', [`${this.rdsCluster.clusterIdentifier}instance1`,
      `${this.rdsCluster.clusterIdentifier}instance2`]);

But, I got an exception: Resource handler returned message: "Only one DB instance is supported but 2 instances are provided (Service: AmazonRDS; Status Code: 400; Error Code: InvalidParameterValue; Request ID: ad46eb41-0dfe-4333-8030-3dafa5739d83; Proxy: null)" (RequestToken: 7b497dd9-e910-4b14-904d-1dbd76f02d2e, HandlerErrorCode: GeneralServiceException)

skinny85 commented 3 years ago

Ok... So, given that limitation, I wonder what does the setup in the Console that you mentioned do. Does it create two separate Proxies? One Proxy, but two ProxyTargetGroups? Or something else?

aalam-tu commented 3 years ago

When I created a proxy via aws console referencing the rds cluster, it creates one proxy with two endpoints - Read and Read/Write

In the prompt, I get asked if I want a reader endpoint. I believe in the .addProxy() we should get a boolean attribute like 'includeReaderEndpoint' to cater for this

Once the proxy is created it generates two endpoints

skinny85 commented 3 years ago

Ok. Looks like there's a resource called AWS::RDS::DBProxyEndpoint. This is probably what's missing from the DatabaseProxy class today.

You can create them today by using the CfnDBProxyEndpoint from the CDK. I'll keep this issue open as a feature request to add support for Proxy endpoints to our DatabaseProxy class.

yeti-g commented 2 years ago

@ericzbeard @skinny85 It's been over a year and this issue is still unassigned. 😞 Can you add the read-only endpoint creation as a boolean in DatabaseProxyProps to match the console functionality?

skinny85 commented 2 years ago

Sorry @yeti-g, we haven't prioritized this issue yet. Feel free to +1 it (we use +1s to help us prioritize).

We also encourage community contributions, so if you'd like to open us a Pull Request adding this feature, that would be fantastic! Our "Contributing" guide: https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md.

Thanks, Adam

ekeyser commented 2 years ago

@skinny85 You should know that whenever someone from AWS suggests that people freely submit PRs to a company with tens of billions of dollars a year in net profits and has a market cap approaching 2 trillion dollars it sounds a bit obtuse. Just so you're aware. Most of us either personally and out of our own pocket spend thousands of dollars a year on AWS services or we work for organizations that spend orders of magnitude more. This is not an isolated incident and quite frankly the tone of some of the other devs in this project have been very contemptuous borderline rude. If I were you I would perhaps hold back on that suggestion. We, the ones that use aws-cdk, are the ones that pay your salary either directly or indirectly. Just a little bit of feedback.

skinny85 commented 2 years ago

That's fair @ekeyser. I just want to underscore that we're a relatively small team, so we will always lag behind all AWS features in one area or another. So, given that, if a particular feature is very valuable to your project, the quickest way to get it to CDK is often contributing a PR for it. But of course, that is completely voluntary, and there is no expectation on anyone in the community about opening a PR.