feat(rds): Kerberos authentication support in Aurora Database Clusters by badmintoncryer · Pull Request #28559 · aws/aws-cdk

badmintoncryer · 2024-01-03T13:46:15Z

I have added the arguments domain and domainRole to support Kerberos authentication for the Aurora Database cluster. The specifications for these arguments are the same as the existing domain and domainRole in the Instance.

declare const vpc: ec2.Vpc
declare const iamRole: iam.IRole
new rds.DatabaseCluster(this, 'Database', {
  engine: rds.DatabaseClusterEngine.auroraMysql({ version: rds.AuroraMysqlEngineVersion.VER_3_05_1 }),
  writer: rds.ClusterInstance.provisioned('Instance', {
    instanceType: ec2.InstanceType.of(ec2.InstanceClass.BURSTABLE3, ec2.InstanceSize.MEDIUM),
  }),
  vpc,
  domain: 'd-????????', // added
  domainRole: iamRole, // added
});

Closes #28050.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

badmintoncryer · 2024-01-06T19:08:32Z

packages/aws-cdk-lib/aws-rds/lib/cluster.ts

+      this.domainRole = props.domainRole ?? new iam.Role(this, 'RDSClusterDirectoryServiceRole', {
+        assumedBy: new iam.CompositePrincipal(
+          new iam.ServicePrincipal('rds.amazonaws.com'),
+          new iam.ServicePrincipal('directoryservice.rds.amazonaws.com'),


In the implementation of the instance, directoryservice.rds.amazonaws.com is not set as a principal, which is a bug and causes the deployment to fail.

I will address this in a separate issue(#28600).

Nice catch!

go-to-k

Thanks for your contribution. I made some comments about README.

go-to-k · 2024-01-21T09:38:56Z

packages/aws-cdk-lib/aws-rds/README.md

+    new iam.ServicePrincipal('directoryservice.rds.amazonaws.com'),
+  ),
+  managedPolicies: [
+    iam.ManagedPolicy.fromManagedPolicyArn(this, 'RdsRole', 'arn:aws:iam::aws:policy/service-role/AmazonRDSDirectoryServiceAccess'),


Why not use fromAwsManagedPolicyName?

Why was I using the fromManagerdPolicyArn? 😂

Let's fix the same in an integ test and an unit test so that contributors who see these codes will not misunderstand!

https://github.com/aws/aws-cdk/pull/28559/files#diff-5faaee1dfd608a160551274db73518aeab8d44bd3351879b34941a052a0135f9R3991

https://github.com/aws/aws-cdk/pull/28559/files#diff-14e95b4529a923153b85040624674c00a266dbd44e28599755901a0ae7d89b6bR19

I was using it elsewhere too.. Thanks for pointing that out. I've made the correction!

packages/aws-cdk-lib/aws-rds/README.md

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

badmintoncryer · 2024-01-21T14:24:34Z

@go-to-k Thank you for your review! I've addressed your comments.

go-to-k · 2024-01-22T04:19:50Z

Could you update snapshots for the integ test?

badmintoncryer · 2024-01-22T05:20:51Z

@go-to-k
Sorry, I added updated integ test files.

go-to-k

Looks good.

paulhcsun

Left some comments

packages/aws-cdk-lib/aws-rds/README.md

packages/aws-cdk-lib/aws-rds/lib/cluster.ts

paulhcsun · 2024-01-23T02:18:11Z

packages/aws-cdk-lib/aws-rds/lib/cluster.ts

+  /**
+   * The Active Directory directory ID, required for setting up Kerberos authentication, to create the DB cluster in.
+   *
+   * @default - Do not join domain


This feels a bit vague to me but it could just be that I'm missing some context. Is this saying that the cluster will be created but just not within a domain (or within the Active Directory)?

I understand, you're right; the cluster will be created but not associated with any domain. I've revised the comment section for clarity. Could you please check it again?
(I have limited English skills, so I would appreciate it if you could appropriately adjust the text for me.)

The new description is a lot clearer to me now! And no worries at all, I really appreciate that you do your best to address the description changes!

paulhcsun · 2024-01-23T02:20:31Z

packages/aws-cdk-lib/aws-rds/lib/cluster.ts

+   * The IAM role to be used when making API calls to the Directory Service. The role needs the AWS-managed policy
+   * AmazonRDSDirectoryServiceAccess or equivalent.
+   *
+   * @default - The role will be created for you if `DatabaseClusterBaseProps#domain` is specified


Would be nice to know what permissions are granted with the default role. Also is DatabaseClusterBaseProps#domain supposed to be DatabaseClusterBaseProps.domain?

I've updated the sentence. Please confirm it.

packages/aws-cdk-lib/aws-rds/lib/cluster.ts

paulhcsun · 2024-01-23T02:22:59Z

packages/aws-cdk-lib/aws-rds/lib/cluster.ts

+      this.domainRole = props.domainRole ?? new iam.Role(this, 'RDSClusterDirectoryServiceRole', {
+        assumedBy: new iam.CompositePrincipal(
+          new iam.ServicePrincipal('rds.amazonaws.com'),
+          new iam.ServicePrincipal('directoryservice.rds.amazonaws.com'),


Nice catch!

Co-authored-by: paulhcsun <47882901+paulhcsun@users.noreply.github.com>

Pull request has been modified.

Co-authored-by: paulhcsun <47882901+paulhcsun@users.noreply.github.com>

paulhcsun

Thank you for updating the descriptions! They're a lot clearer to me now.

packages/aws-cdk-lib/aws-rds/lib/cluster.ts

paulhcsun

Great work on this @badmintoncryer!

And thank you for reviewing @go-to-k!

aws-cdk-automation · 2024-01-24T18:06:52Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: dbc4093
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2024-01-24T18:07:11Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation requested a review from a team January 3, 2024 13:46

github-actions bot added valued-contributor [Pilot] contributed between 6-12 PRs to the CDK effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 labels Jan 3, 2024

aws-cdk-automation previously requested changes Jan 3, 2024

View reviewed changes

badmintoncryer changed the title ~~Support for kerberos authentication for Aurora Database Cluster~~ feat(rds): Kerberos authentication support in Aurora Database Clusters. Jan 3, 2024

badmintoncryer force-pushed the 28050-supportKerberos branch from 4087725 to 2d83f35 Compare January 5, 2024 00:41

feat: support for kerberos

e2173ef

badmintoncryer force-pushed the 28050-supportKerberos branch from 2d83f35 to 681d1d4 Compare January 5, 2024 00:41

test: unit and integ test

bd37ee1

badmintoncryer force-pushed the 28050-supportKerberos branch from 681d1d4 to bd37ee1 Compare January 5, 2024 01:30

badmintoncryer changed the title ~~feat(rds): Kerberos authentication support in Aurora Database Clusters.~~ feat(rds): Kerberos authentication support in Aurora Database Clusters Jan 5, 2024

badmintoncryer added 2 commits January 7, 2024 01:27

fix: assume principal

87c31d0

fix: unit test

9e10cc9

badmintoncryer force-pushed the 28050-supportKerberos branch from da04a78 to 9e10cc9 Compare January 6, 2024 16:44

test: integ test

77a62d8

badmintoncryer marked this pull request as ready for review January 6, 2024 18:43

badmintoncryer commented Jan 6, 2024

View reviewed changes

fix: readme

1f3876a

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jan 6, 2024

badmintoncryer mentioned this pull request Jan 9, 2024

rds: Failed to create a Database Instance with Kerberos authentication configured #28600

Closed

badmintoncryer added 2 commits January 14, 2024 04:40

Merge branch 'main' into 28050-supportKerberos

8d5d058

Merge branch 'main' into 28050-supportKerberos

5e203a3

go-to-k suggested changes Jan 21, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jan 21, 2024

badmintoncryer and others added 3 commits January 21, 2024 23:19

Update packages/aws-cdk-lib/aws-rds/README.md

70910c9

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-rds/README.md

77f1424

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

refactor: use FromAwsMangedPolicyName

0180201

badmintoncryer added 2 commits January 21, 2024 23:24

Merge branch 'main' into 28050-supportKerberos

dc227d5

refactor: use fromAwsManagedPolicyName()

55b0e88

update: integ test

f93e70c

go-to-k approved these changes Jan 22, 2024

View reviewed changes

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 22, 2024

paulhcsun previously requested changes Jan 23, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 23, 2024

badmintoncryer and others added 2 commits January 23, 2024 14:46

README.md を更新

7bba7b3

Co-authored-by: paulhcsun <47882901+paulhcsun@users.noreply.github.com>

cluster.ts を更新

1133d74

Co-authored-by: paulhcsun <47882901+paulhcsun@users.noreply.github.com>

cluster.ts を更新

fc45065

Co-authored-by: paulhcsun <47882901+paulhcsun@users.noreply.github.com>

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 23, 2024

badmintoncryer and others added 3 commits January 24, 2024 05:01

chore: comment

d53de8f

chore: comment

3ffe433

Merge branch 'main' into 28050-supportKerberos

fe003f9

paulhcsun reviewed Jan 24, 2024

View reviewed changes

packages/aws-cdk-lib/aws-rds/lib/cluster.ts Outdated Show resolved Hide resolved

paulhcsun added 2 commits January 24, 2024 09:39

wording change

48d62e5

Merge branch 'main' into 28050-supportKerberos

dbc4093

paulhcsun approved these changes Jan 24, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 24, 2024

mergify bot merged commit bdf4285 into aws:main Jan 24, 2024

badmintoncryer deleted the 28050-supportKerberos branch January 24, 2024 19:08

github-actions bot mentioned this pull request Feb 1, 2024

Monthly PR metrics report - Jan 2024 #28952

Closed

Conversation

badmintoncryer commented Jan 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Uh oh!

badmintoncryer Jan 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

go-to-k left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

badmintoncryer commented Jan 21, 2024

Uh oh!

go-to-k commented Jan 22, 2024

Uh oh!

badmintoncryer commented Jan 22, 2024

Uh oh!

go-to-k left a comment

Choose a reason for hiding this comment

Uh oh!

paulhcsun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paulhcsun Jan 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paulhcsun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

paulhcsun left a comment

Choose a reason for hiding this comment

Uh oh!

aws-cdk-automation commented Jan 24, 2024

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Jan 24, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

badmintoncryer commented Jan 3, 2024 •

edited

Loading

badmintoncryer Jan 6, 2024 •

edited

Loading

paulhcsun Jan 24, 2024 •

edited

Loading