Feat add Aurora DSQL token generator #2929
Conversation
| { | ||
| "id": "96240bbb-6b53-41a0-a5e4-0055b0222be0", | ||
| "type": "release", | ||
| "description": "Add DSQL Auth Token Generator", |
There was a problem hiding this comment.
nit
| "description": "Add DSQL Auth Token Generator", | |
| "description": "Add Aurora DSQL Auth Token Generator", |
| ) | ||
|
|
||
| const ( | ||
| rdsClusterTokenID = "dsql" |
There was a problem hiding this comment.
This is actually vendor code
| rdsClusterTokenID = "dsql" | |
| vendorCode = "dsql" |
| func TestGenerateDbConnectAuthToken(t *testing.T) { | ||
| cases := map[string]dbTokenTestCase{ | ||
| "no region": { | ||
| endpoint: "https://prod-instance.us-east-1.rds.amazonaws.com:3306", |
There was a problem hiding this comment.
Aurora DSQLs endpoints dont need to have schema (https / http) and the port information. Also the endpoints are dual stack only. An example format is this.
| endpoint: "https://prod-instance.us-east-1.rds.amazonaws.com:3306", | |
| endpoint: "foo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws", |
Same goes for other examples below.
There was a problem hiding this comment.
Changed the endpoints to reflect this endpoint. I added them with scheme as well to make sure we handle this case as well and strip the scheme if passed by the developer
| // | ||
| // This is the admin user variant, see [GenerateDbConnectAuthToken] for the regular user variant | ||
| // | ||
| // * endpoint - Endpoint is the hostname and optional port to connect to the database |
There was a problem hiding this comment.
port is not needed
| // * endpoint - Endpoint is the hostname and optional port to connect to the database | |
| // * endpoint - Endpoint is the hostname to connect to the database |
| }, | ||
| "no endpoint": { | ||
| region: "us-west-2", | ||
| expectedError: "port", |
There was a problem hiding this comment.
do we validate port? For Aurora DSQL, port is not needed.
There was a problem hiding this comment.
We do not. Good part of these tests are based on what we have for RDS, which has a similar feature https://aws.github.io/aws-sdk-go-v2/docs/sdk-utilities/rds/
The tests are ported from this feature. There, we check the string for an exact match on the error string. Here, we check if an error was thrown, and we just print "expected to get an error X".
I'll change this to be a bit more explicit to say we don't check for port.
| expectedError: "endpoint is required", | ||
| }, | ||
| "endpoint with scheme": { | ||
| endpoint: "https://oo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws:3306", |
There was a problem hiding this comment.
| endpoint: "https://oo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws:3306", | |
| endpoint: "https://foo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws", |
| "endpoint with scheme": { | ||
| endpoint: "https://oo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws:3306", | ||
| region: "us-east-1", | ||
| expectedHost: "oo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws:3306", |
There was a problem hiding this comment.
| expectedHost: "oo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws:3306", | |
| expectedHost: "foo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws", |
| expectedQueryParams: []string{"Action=DbConnect"}, | ||
| }, | ||
| "endpoint without scheme": { | ||
| endpoint: "oo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws:3306", |
There was a problem hiding this comment.
| endpoint: "oo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws:3306", | |
| endpoint: "foo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws", |
| "endpoint without scheme": { | ||
| endpoint: "oo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws:3306", | ||
| region: "us-east-1", | ||
| expectedHost: "oo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws:3306", |
There was a problem hiding this comment.
| expectedHost: "oo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws:3306", | |
| expectedHost: "foo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws", |
| expectedQueryParams: []string{"Action=DbConnect"}, | ||
| }, | ||
| "endpoint without port": { | ||
| endpoint: "oo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws", |
There was a problem hiding this comment.
| endpoint: "oo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws", | |
| endpoint: "foo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws", |
| "endpoint without port": { | ||
| endpoint: "oo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws", | ||
| region: "us-east-1", | ||
| expectedHost: "oo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws", |
There was a problem hiding this comment.
| expectedHost: "oo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws", | |
| expectedHost: "foo0bar1baz2quux3quuux4.dsql.us-east-1.on.aws", |
feature/dsql/auth/CHANGELOG.md
Outdated
| @@ -0,0 +1,4 @@ | |||
| # v1.0.0 (2024-12-09) | |||
There was a problem hiding this comment.
We need to remove this. When make release runs, it'll slurp your .changelog entry and transform it into CHANGELOG.md content (which is expected) - but since you already have the message here it'll be duped:
1 # v1.0.0 (2024-12-13)
2
3 * **Release**: Add Aurora DSQL Auth Token Generator
4
5 # v1.0.0 (2024-12-09)
6
7 * **Release**: Add Aurora DSQL Auth Token Generator
8
Add Aurora DSQL token generator.
Followup from #2925