Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rds: use DataAPI for Aurora cluster without invoking secret.grantRead() #29362

Closed
1 of 2 tasks
badmintoncryer opened this issue Mar 5, 2024 · 2 comments · Fixed by #29399 · May be fixed by NOUIY/aws-solutions-constructs#98, NOUIY/aws-solutions-constructs#99 or NOUIY/aws-solutions-constructs#101
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@badmintoncryer
Copy link
Contributor

badmintoncryer commented Mar 5, 2024

Describe the feature

To use DataAPI for an Aurora cluster without invoking secret.grantRead().

Use Case

DataAPI for an Aurora cluster is supported (#29338).

It is necessary to invoke secret.grantRead() for DatabaseCluster but it is unnecessary for ServelessCluster.
This inconsistency is user-unfriendly and should be addressed.

// Create a serverless V1 cluster
const serverlessV1Cluster = new rds.ServerlessCluster(this, 'AnotherCluster', {
  engine: rds.DatabaseClusterEngine.AURORA_MYSQL,
  vpc,
  enableDataApi: true,
});
serverlessV1Cluster.grantDataApiAccess(fn);

// Create an Aurora cluster
const cluster = new rds.DatabaseCluster(this, 'Cluster', {
  engine: rds.DatabaseClusterEngine.AURORA_MYSQL,
  vpc,
  enableDataApi: true,
});
cluster.grantDataApiAccess(fn);
// It is necessary to grant the function access to the secret associated with the cluster for `DatabaseCluster`.
cluster.secret!.grantRead(fn);

Proposed Solution

Move cluster.secret from DatabaseClusterNew to DatabaseClusterBase and invoke secret.grantRead() in cluster.grantDataApiAccess()

#29338 (comment)

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.131.0

Environment details (OS name and version, etc.)

irrelevant

@badmintoncryer badmintoncryer added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Mar 5, 2024
@github-actions github-actions bot added the @aws-cdk/aws-rds Related to Amazon Relational Database label Mar 5, 2024
@badmintoncryer badmintoncryer changed the title rds: use DataAPI for Aurora cluster without calling secret.grantRead() rds: use DataAPI for Aurora cluster without invoking secret.grantRead() Mar 5, 2024
@tim-finnigan tim-finnigan self-assigned this Mar 6, 2024
@tim-finnigan tim-finnigan added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Mar 6, 2024
@tim-finnigan
Copy link

Thanks for the feature request - the response to the comment you linked seems to suggest that this is a valid request, if you are interested in submitting another PR for consideration.

@tim-finnigan tim-finnigan added p2 effort/medium Medium work item – several days of effort and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Mar 6, 2024
@tim-finnigan tim-finnigan removed their assignment Mar 6, 2024
@mergify mergify bot closed this as completed in #29399 Mar 14, 2024
mergify bot pushed a commit that referenced this issue Mar 14, 2024
…okes when using DataAPI with Aurora cluster (#29399)

### Issue # (if applicable)

Closes #29362.

### Reason for this change

As discussed [there](#29338 (comment)), we should invoke `secret.grantRead()` explicitly when using DataAPI with Aurora cluster.
Because it's inconvenient for users, I made  `secret.grantRead()` be invoked within `cluster.grantDataApiAccess()`.

### Description of changes

- move `cluster.secret` from `DatabaseClusterNew` to `DatabaseClusterBase` to use it within `DatabaseClusterBase.grantDataApiAccess()`
- add `secret.grantRead()` in `cluster.grantDataApiAccess()`
- add `secret` property to `DatabaseClusterAttributes`

#### Points of concern

`DatabaseClusterBase` class is extended by `ImportedDatabaseCluster` class. Therefore, it is necessary to define `ImportedDatabaseCluster.secret`.

I simply added `secret` props to `DatabaseClusterAttributes` but I cannot believe this is the best way.
Other ways are..
- add `secretArn` to `DatabaseClusterAttributes`
- don't add secret info and `ImportedDatabaseCluster.secret` becomes always undefined

### Description of how you validated changes



### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

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

⚠️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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
2 participants