fix(redshift): UserTablePriviliges to track changes using table IDs'

packages/@aws-cdk/aws-redshift-alpha/lib/private/database-query-provider/privileges.ts

@@ -62,10 +62,28 @@ async function updatePrivileges(
   }
 
   const oldTablePrivileges = oldResourceProperties.tablePrivileges;
-  if (oldTablePrivileges !== tablePrivileges) {
-    await revokePrivileges(username, oldTablePrivileges, clusterProps);
-    await grantPrivileges(username, tablePrivileges, clusterProps);
-    return { replace: false };
+  const tablesToRevoke = oldTablePrivileges.filter(({ tableId, actions }) => {
+    const tableRemoved = !tablePrivileges.find(({ tableId: otherTableId }) => tableId === otherTableId);
+    const actionsRemoved = tablePrivileges.find(({ tableId: otherTableId, actions: otherActions }) => (
+      tableId === otherTableId && actions.some(action => !otherActions.includes(action))
+    ));
+    return tableRemoved || actionsRemoved;
+  });
+  if (tablesToRevoke.length > 0) {
+    await revokePrivileges(username, tablesToRevoke, clusterProps);
+  }
+
+  const tablesToGrant = tablePrivileges.filter(({ tableId, tableName, actions }) => {
+    const tableAdded = !oldTablePrivileges.find(({ tableId: otherTableId, tableName: otherTableName }) => (
+      tableId === otherTableId && tableName === otherTableName
+    ));
+    const actionsAdded = oldTablePrivileges.find(({ tableId: otherTableId, actions: otherActions }) => (
+      tableId === otherTableId && otherActions.some(action => !actions.includes(action))
+    ));
+    return tableAdded || actionsAdded;
+  });
+  if (tablesToGrant.length > 0) {
+    await grantPrivileges(username, tablesToGrant, clusterProps);
   }
 
   return { replace: false };

packages/@aws-cdk/aws-redshift-alpha/lib/private/handler-props.ts

@@ -25,6 +25,7 @@ export interface TableHandlerProps {
 }
 
 export interface TablePrivilege {
+  readonly tableId: string;
   readonly tableName: string;
   readonly actions: string[];
 }

packages/@aws-cdk/aws-redshift-alpha/lib/table.ts

@@ -236,6 +236,7 @@ export class Table extends TableBase {
   readonly tableColumns: Column[];
   readonly cluster: ICluster;
   readonly databaseName: string;
+  readonly id: string;
 
   private resource: DatabaseQuery<TableHandlerProps>;
 
@@ -253,6 +254,7 @@ export class Table extends TableBase {
     this.tableColumns = this.configureTableColumns(props.tableColumns);
     this.cluster = props.cluster;
     this.databaseName = props.databaseName;
+    this.id = id;
 
     const useColumnIds = !!cdk.FeatureFlags.of(this).isEnabled(REDSHIFT_COLUMN_ID);
 

packages/@aws-cdk/aws-redshift-alpha/test/database-query-provider/privileges.test.ts

@@ -3,7 +3,9 @@ import type * as AWSLambda from 'aws-lambda';
 
 const username = 'username';
 const tableName = 'tableName';
-const tablePrivileges = [{ tableName, actions: ['INSERT', 'SELECT'] }];
+const tableId = 'tableId';
+const actions = ['INSERT', 'SELECT'];
+const tablePrivileges = [{ tableId, tableName, actions }];
 const clusterName = 'clusterName';
 const adminUserArn = 'adminUserArn';
 const databaseName = 'databaseName';
@@ -147,9 +149,27 @@ describe('update', () => {
     }));
   });
 
-  test('does not replace when privileges change', async () => {
+  test('does not replace when table name is changed', async () => {
     const newTableName = 'newTableName';
-    const newTablePrivileges = [{ tableName: newTableName, actions: ['DROP'] }];
+    const newTablePrivileges = [{ tableId, tableName: newTableName, actions }];
+    const newResourceProperties = {
+      ...resourceProperties,
+      tablePrivileges: newTablePrivileges,
+    };
+
+    await expect(managePrivileges(newResourceProperties, event)).resolves.toMatchObject({
+      PhysicalResourceId: physicalResourceId,
+    });
+    expect(mockExecuteStatement).not.toHaveBeenCalledWith(expect.objectContaining({
+      Sql: `REVOKE INSERT, SELECT ON ${newTableName} FROM ${username}`,
+    }));
+    expect(mockExecuteStatement).not.toHaveBeenCalledWith(expect.objectContaining({
+      Sql: expect.stringMatching(new RegExp(`.+ ON ${tableName} TO ${username}`)),
+    }));
+  });
+
+  test('does not replace when table actions are changed', async () => {
+    const newTablePrivileges = [{ tableId, tableName, actions: ['DROP'] }];
     const newResourceProperties = {
       ...resourceProperties,
       tablePrivileges: newTablePrivileges,
@@ -162,7 +182,46 @@ describe('update', () => {
       Sql: `REVOKE INSERT, SELECT ON ${tableName} FROM ${username}`,
     }));
     expect(mockExecuteStatement).toHaveBeenCalledWith(expect.objectContaining({
-      Sql: `GRANT DROP ON ${newTableName} TO ${username}`,
+      Sql: `GRANT DROP ON ${tableName} TO ${username}`,
     }));
   });
-});
+
+  test('does not replace when table id is changed', async () => {
+    const newTableId = 'newTableId';
+    const newTablePrivileges = [{ tableId: newTableId, tableName, actions }];
+    const newResourceProperties = {
+      ...resourceProperties,
+      tablePrivileges: newTablePrivileges,
+    };
+
+    await expect(managePrivileges(newResourceProperties, event)).resolves.toMatchObject({
+      PhysicalResourceId: physicalResourceId,
+    });
+    expect(mockExecuteStatement).not.toHaveBeenCalledWith(expect.objectContaining({
+      Sql: `.+ ON ${tableName} FROM ${username}`,
+    }));
+  });
+
+  test('does not replace when table id is appended', async () => {
+    const newTablePrivileges = [{ tableId: 'newTableId', tableName, actions }];
+    const newResourceProperties = {
+      ...resourceProperties,
+      tablePrivileges: newTablePrivileges,
+    };
+
+    const newEvent = {
+      ...event,
+      OldResourceProperties: {
+        ...event.OldResourceProperties,
+        tablePrivileges: [{ tableName, actions }],
+      },
+    };
+
+    await expect(managePrivileges(newResourceProperties, newEvent)).resolves.toMatchObject({
+      PhysicalResourceId: physicalResourceId,
+    });
+    expect(mockExecuteStatement).not.toHaveBeenCalledWith(expect.objectContaining({
+      Sql: `.+ ON ${tableName} FROM ${username}`,
+    }));
+  });
+});