aws-cdk icon indicating copy to clipboard operation
aws-cdk copied to clipboard

aws-rds: Support SQL Server for DatabaseProxies

Open jeastham1993 opened this issue 3 years ago • 2 comments

Describe the bug

When trying to deploy a CDK application that uses a DatabaseProxy for SQL Server the synth returns an error stating Proxies are not available for SQL Server version...

'Engine 'sqlserver-ex-15.00' does not support proxies'

Expected Behavior

RDS Proxy now supports SQL Server, so this should be allowed.

https://aws.amazon.com/about-aws/whats-new/2022/09/amazon-rds-proxy-rds-sql-server/

Current Behavior

CDK errors on Synth

Reproduction Steps

Create a new SQL Server based RDS database instance and add a proxy. Then run cdk deploy

var database = new DatabaseInstance(scope, "products-db", new DatabaseInstanceProps()
            {
                Vpc = props.Vpc,
                VpcSubnets = new SubnetSelection()
                {
                    SubnetType = SubnetType.PRIVATE_WITH_EGRESS
                },
                Engine = DatabaseInstanceEngine.SqlServerEx(new SqlServerExInstanceEngineProps()
                {
                    Version = SqlServerEngineVersion.VER_15
                }),
                InstanceType = InstanceType.Of(InstanceClass.BURSTABLE3, InstanceSize.SMALL),
                Credentials = Credentials.FromGeneratedSecret("admin"),
                MultiAz = false,
                AllocatedStorage = 100,
                MaxAllocatedStorage = 105,
                DeleteAutomatedBackups = true,
                DeletionProtection = false,
                PubliclyAccessible = true,
            });

            database.Connections.AllowFromAnyIpv4(Port.Tcp(1433));

            // CDK does not yet support DatabaseProxy for RDS SQL Server
            var proxy = new DatabaseProxy(this, "SqlProxy", new DatabaseProxyProps()
            {
                ProxyTarget = ProxyTarget.FromInstance(database),
                Secrets = new[] {database.Secret},
                Vpc = props.Vpc
            });
            
            var role = new Role(this, "SqlProxyRole", new RoleProps()
            {
                AssumedBy = new AccountPrincipal(Stack.Of(this).Account)
            });
            proxy.GrantConnect(role, "admin");

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.42.1

Framework Version

No response

Node.js Version

14.18

OS

Windows 10

Language

.NET

Language Version

.NET 6

Other information

Full stack trace: Unhandled exception. Amazon.JSII.Runtime.JsiiException: Engine 'sqlserver-ex-15.00' does not support proxies at Amazon.JSII.Runtime.Services.Client.TryDeserialize[TResponse](String responseJson) at Amazon.JSII.Runtime.Services.Client.ReceiveResponseTResponse at Amazon.JSII.Runtime.Services.Client.Send[TRequest,TResponse](TRequest requestObject) at Amazon.JSII.Runtime.Services.Client.Create(CreateRequest request) at Amazon.JSII.Runtime.Services.Client.Create(String fullyQualifiedName, Object[] arguments, Override[] overrides, String[] interfaces) at Amazon.JSII.Runtime.Deputy.DeputyBase..ctor(DeputyProps props) at Constructs.Construct..ctor(DeputyProps props) at Amazon.CDK.Resource..ctor(DeputyProps props)

jeastham1993 avatar Sep 21 '22 08:09 jeastham1993

In the current documentation of CloudFormation for the RDS Proxy it seems that the SQL Server engine family is not yet added. See here https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-rds-dbproxy.html.

When this is added, the CDK code can be adjusted so that the SqlServerInstanceEngineBase can add the engineFamily property in the constructor. See here: https://github.com/aws/aws-cdk/blob/ce59b6aaa7a19ec074547824c3641822ab853213/packages/%40aws-cdk/aws-rds/lib/instance-engine.ts#L1605-L1629

This would lead to not throw the error described in the issue because this check would pass: https://github.com/aws/aws-cdk/blob/ce59b6aaa7a19ec074547824c3641822ab853213/packages/%40aws-cdk/aws-rds/lib/proxy.ts#L82-L85

I think this issue should be tagged with needs-cfn.

daschaa avatar Sep 21 '22 10:09 daschaa

Josh from RDS here. Thank you for reporting this issue. We'll work on a fix.

jtoberon avatar Sep 22 '22 03:09 jtoberon

Thanks both, I look forward to the updates :) let me know if there's anything I can do to support,

jeastham1993 avatar Sep 24 '22 19:09 jeastham1993

@daschaa thanks for the details!

This issue has been classified as p2. That means a workaround is available or it is deemed a nice-to-have feature. Given the amount of work there is to do and the relative priority of this issue, the CDK team is unlikely to address it. That does not mean the issue will never be fixed! If someone from the community submits a PR to fix this issue, and the PR is small and straightforward enough, and meets the quality bars to be reviewed and merged with little effort we will accept that PR. PRs that do not build or need complex or multiple rounds of reviews are unlikely to be merged and will be closed to keep our backlog manageable.

In the mean time, remember that you can always use the escape hatch (https://docs.aws.amazon.com/cdk/v2/guide/cfn_layer.html) mechanism to have fine control over the CloudFormation output you want. We will keep the issue open for discoverability, to collect upvotes, and to facilitate discussion around this topic.

We use +1s on this issue to help prioritize our work, and are happy to re-evaluate the prioritization of this issue based on community feedback. You can reach out to the cdk.dev community on Slack (https://cdk.dev/) to solicit support for reprioritization.

corymhall avatar Sep 26 '22 14:09 corymhall

When the CloudFormation fix is out, you'll see it reflected here: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-rds-dbproxy.html.

jtoberon avatar Oct 12 '22 20:10 jtoberon

CloudFormation is fixed.

I created a PR with a fix, but closed since it will require a bit more changes than I imagined. I will leave it for people with more experience on cdk to design and work on the changes.

The PR got a comment suggesting to receive the input via props and I think that is a better approach. Another thing that should be fixed with this is the handling of iamAuth.

https://github.com/aws/aws-cdk/blob/74318c7d22bfc00de9e005f68a0a6aaa58c7db39/packages/%40aws-cdk/aws-rds/lib/proxy.ts#L206-L211

CDK is currently using a boolean to represent it but the valid values are ENABLED | DISABLED | REQUIRED

From doc https://docs.aws.amazon.com/cli/latest/reference/rds/create-db-proxy.html

IAMAuth -> (string) Whether to require or disallow Amazon Web Services Identity and Access Management (IAM) authentication for connections to the proxy. The ENABLED value is valid only for proxies with RDS for Microsoft SQL Server.

motirajaws avatar Oct 14 '22 20:10 motirajaws

https://github.com/aws/aws-cdk/blob/74318c7d22bfc00de9e005f68a0a6aaa58c7db39/packages/%40aws-cdk/aws-rds/lib/proxy.ts#L446 We're setting it based off the boolean, but we use DISABLED | REQUIRED in this context.

TheRealAmazonKendra avatar Oct 16 '22 20:10 TheRealAmazonKendra

https://github.com/aws/aws-cdk/blob/74318c7d22bfc00de9e005f68a0a6aaa58c7db39/packages/%40aws-cdk/aws-rds/lib/proxy.ts#L446 We're setting it based off the boolean, but we use DISABLED | REQUIRED in this context.

Yes, but it should be migrated out of boolean since there is a third option ENABLED that cannot be currently used with CDK. ENABLED is a hybrid option that is available for SQLServer proxies.

motirajaws avatar Oct 31 '22 22:10 motirajaws

Hi guys!

CDK v2.62.2 is still reporting:

RuntimeError: Engine 'sqlserver-ee-14.00' does not support proxies

When will this be fixed?

tvb avatar Jan 31 '23 09:01 tvb

@tvb I have opened a pull request for fixing this issue :)

daschaa avatar Mar 11 '23 13:03 daschaa

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

github-actions[bot] avatar Apr 18 '23 13:04 github-actions[bot]