aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.68k stars 3.93k forks source link

aws-redshift: Granting privileges to new tables does not work due to physical resource ID not being SQL table name #29231

Open ricky-escobar opened 8 months ago

ricky-escobar commented 8 months ago

Describe the bug

I updated from 2.31 to 2.127 and am now getting an error when deploying a new table. My previously existing tables in the same stack seem to be deploying fine (with no changes), but the deployment is failing when granting privileges to the new table.

Received response status [FAILED] from custom resource. Message returned: Statement status was FAILED: ERROR: syntax error at or near "-"

It appears that this fairly straightforward use case of granting privileges to a user on a table is fundamentally broken. I would guess this would affect a large number of applications using these components.

Expected Behavior

Deploying my stack succeeds, and user privileges are properly granted to the table.

Current Behavior

I get the following error when the stack is deploying the UserTablePrivileges resource.

Received response status [FAILED] from custom resource. Message returned: Statement status was FAILED: ERROR: syntax error at or near "-" Position: 28 Logs: /aws/lambda/KfbIronside-KfbVendorServ-QueryRedshiftDatabase3de-oAJNVqgF3XcN Position: 28 at waitForStatementComplete (/var/task/redshift-data.js:34:15) at process.processTicksAndRejections (node:internal/process/task_queues:95:5) at async executeStatement (/var/task/redshift-data.js:18:5) at async Promise.all (index 0) at async grantPrivileges (/var/task/privileges.js:35:5) at async handler (/var/task/privileges.js:11:9) (RequestId: 92da86ab-8906-4e16-80be-15c553d82763)

Reproduction Steps

const user = new User(this, "my_user", {
  adminUser: clusterAdminUserSecret,
  cluster: redshiftCluster,
  databaseName: databaseName,
  encryptionKey: kmsKey,
  username: "my_user",
});

const table = new Table(this, "my_table", {
  cluster: redshiftCluster,
  adminUser: clusterAdminUserSecret,
  databaseName: databaseName,
  tableName: "my_table",
  tableColumns: [{name: 'field', dataType: 'int'}],
});

table.grant(user, TableAction.SELECT, TableAction.INSERT);

Possible Solution

After digging through the code changes, it seems that this regression was introduced in https://github.com/aws/aws-cdk/pull/24308. This change updated the custom resource handler for Table, changing the physical resource ID from the simple table name to the above composite ID form. However, the Table construct assigns this physical resource ID to its tableName property. The privileges code uses this tableName property to construct the SQL statement, resulting in the syntax error during deployment.

So far, I am working around this by just not updating to the latest version. I don't see this issue on 2.114. I imagine that it should be possible to work around the issue by using Table.fromTableAttributes to create an ITable with the desired tableName rather than reusing the result of new Table(...) like normal. Though I think this would only work if not using an auto-generated table name.

Additional Information/Context

No response

CDK CLI Version

2.127.0 (build 6c90efc)

Framework Version

No response

Node.js Version

20.x

OS

Amazon Linux 2

Language

TypeScript

Language Version

No response

Other information

No response

pahud commented 8 months ago

Thank you for the report. This seems to be a bug of the tableName generation. I am making it a p1.

mike-eh2 commented 7 months ago

Any update? This is affecting my team. For now we will need to do the grants manually.

hshepherd commented 2 months ago

This is causing problems for us as well in version 2.152.0-alpha.0