Skip to content

Commit 5140418

Browse files
committed
fix(redshift-alpha): extract tableName from custom resource functions
1 parent d0a4a78 commit 5140418

File tree

154 files changed

+23720
-55785
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

154 files changed

+23720
-55785
lines changed

packages/@aws-cdk/aws-pipes-targets-alpha/test/integ.lambda.js.snapshot/asset.6f50ad1670be7195f01a64eddc86541c357b2087e992904bdcb9f5ae67c82ecb.handler/handler.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,18 @@ export async function handler(props: UserTablePrivilegesHandlerProps & ClusterPr
1111
const clusterProps = props;
1212

1313
if (event.RequestType === 'Create') {
14-
await grantPrivileges(username, tablePrivileges, clusterProps);
14+
await grantPrivileges(username, tablePrivileges, clusterProps, event.StackId);
1515
return { PhysicalResourceId: makePhysicalId(username, clusterProps, event.RequestId) };
1616
} else if (event.RequestType === 'Delete') {
17-
await revokePrivileges(username, tablePrivileges, clusterProps);
17+
await revokePrivileges(username, tablePrivileges, clusterProps, event.StackId);
1818
return;
1919
} else if (event.RequestType === 'Update') {
2020
const { replace } = await updatePrivileges(
2121
username,
2222
tablePrivileges,
2323
clusterProps,
2424
event.OldResourceProperties as unknown as UserTablePrivilegesHandlerProps & ClusterProps,
25+
event.StackId,
2526
);
2627
const physicalId = replace ? makePhysicalId(username, clusterProps, event.RequestId) : event.PhysicalResourceId;
2728
return { PhysicalResourceId: physicalId };
@@ -31,19 +32,35 @@ export async function handler(props: UserTablePrivilegesHandlerProps & ClusterPr
3132
}
3233
}
3334

34-
async function revokePrivileges(username: string, tablePrivileges: TablePrivilege[], clusterProps: ClusterProps) {
35+
async function revokePrivileges(
36+
username: string,
37+
tablePrivileges: TablePrivilege[],
38+
clusterProps: ClusterProps,
39+
stackId: string,
40+
) {
3541
// Limited by human input
3642
// eslint-disable-next-line @cdklabs/promiseall-no-unbounded-parallelism
3743
await Promise.all(tablePrivileges.map(({ tableName, actions }) => {
38-
return executeStatement(`REVOKE ${actions.join(', ')} ON ${tableName} FROM ${username}`, clusterProps);
44+
return executeStatement(
45+
`REVOKE ${actions.join(', ')} ON ${normalizedTableName(tableName, stackId)} FROM ${username}`,
46+
clusterProps,
47+
);
3948
}));
4049
}
4150

42-
async function grantPrivileges(username: string, tablePrivileges: TablePrivilege[], clusterProps: ClusterProps) {
51+
async function grantPrivileges(
52+
username: string,
53+
tablePrivileges: TablePrivilege[],
54+
clusterProps: ClusterProps,
55+
stackId: string,
56+
) {
4357
// Limited by human input
4458
// eslint-disable-next-line @cdklabs/promiseall-no-unbounded-parallelism
4559
await Promise.all(tablePrivileges.map(({ tableName, actions }) => {
46-
return executeStatement(`GRANT ${actions.join(', ')} ON ${tableName} TO ${username}`, clusterProps);
60+
return executeStatement(
61+
`GRANT ${actions.join(', ')} ON ${normalizedTableName(tableName, stackId)} TO ${username}`,
62+
clusterProps,
63+
);
4764
}));
4865
}
4966

@@ -52,16 +69,17 @@ async function updatePrivileges(
5269
tablePrivileges: TablePrivilege[],
5370
clusterProps: ClusterProps,
5471
oldResourceProperties: UserTablePrivilegesHandlerProps & ClusterProps,
72+
stackId: string,
5573
): Promise<{ replace: boolean }> {
5674
const oldClusterProps = oldResourceProperties;
5775
if (clusterProps.clusterName !== oldClusterProps.clusterName || clusterProps.databaseName !== oldClusterProps.databaseName) {
58-
await grantPrivileges(username, tablePrivileges, clusterProps);
76+
await grantPrivileges(username, tablePrivileges, clusterProps, stackId);
5977
return { replace: true };
6078
}
6179

6280
const oldUsername = oldResourceProperties.username;
6381
if (oldUsername !== username) {
64-
await grantPrivileges(username, tablePrivileges, clusterProps);
82+
await grantPrivileges(username, tablePrivileges, clusterProps, stackId);
6583
return { replace: true };
6684
}
6785

@@ -72,7 +90,7 @@ async function updatePrivileges(
7290
))
7391
));
7492
if (tablesToRevoke.length > 0) {
75-
await revokePrivileges(username, tablesToRevoke, clusterProps);
93+
await revokePrivileges(username, tablesToRevoke, clusterProps, stackId);
7694
}
7795

7896
const tablesToGrant = tablePrivileges.filter(({ tableId, tableName, actions }) => {
@@ -85,8 +103,21 @@ async function updatePrivileges(
85103
return tableAdded || actionsAdded;
86104
});
87105
if (tablesToGrant.length > 0) {
88-
await grantPrivileges(username, tablesToGrant, clusterProps);
106+
await grantPrivileges(username, tablesToGrant, clusterProps, stackId);
89107
}
90108

91109
return { replace: false };
92110
}
111+
112+
/**
113+
* We need this normalization logic because some of the `TableName` values
114+
* are physical IDs generated in the `./util.ts` module.
115+
* */
116+
const normalizedTableName = (tableName: string, stackId: string): string => {
117+
const segments = tableName.split(':');
118+
const suffix = segments.slice(-1);
119+
if (suffix != null && stackId.endsWith(suffix[0])) {
120+
return segments.slice(-2)[0] ?? tableName;
121+
}
122+
return tableName;
123+
};

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export async function handler(props: TableAndClusterProps, event: AWSLambda.Clou
1515

1616
if (event.RequestType === 'Create') {
1717
tableName = await createTable(tableNamePrefix, getTableNameSuffix(props.tableName.generateSuffix), tableColumns, tableAndClusterProps);
18-
return { PhysicalResourceId: makePhysicalId(tableNamePrefix, tableAndClusterProps, event.StackId.substring(event.StackId.length - 12)) };
18+
return { PhysicalResourceId: makePhysicalId(tableName, tableAndClusterProps, event.StackId.substring(event.StackId.length - 12)) };
1919
} else if (event.RequestType === 'Delete') {
2020
await dropTable(
2121
event.PhysicalResourceId.includes(event.StackId.substring(event.StackId.length - 12)) ? tableName : event.PhysicalResourceId,

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

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { ITable, TableAction } from '../table';
55
import { IUser } from '../user';
66
import { DatabaseQuery } from './database-query';
77
import { HandlerName } from './database-query-provider/handler-name';
8-
import { TablePrivilege as SerializedTablePrivilege, UserTablePrivilegesHandlerProps } from './handler-props';
8+
import { UserTablePrivilegesHandlerProps } from './handler-props';
99

1010
/**
1111
* The Redshift table and action that make up a privilege that can be granted to a Redshift user.
@@ -62,33 +62,25 @@ export class UserTablePrivileges extends Construct {
6262
username: props.user.username,
6363
tablePrivileges: cdk.Lazy.any({
6464
produce: () => {
65-
const reducedPrivileges = this.privileges.reduce((privileges, { table, actions }) => {
66-
const tableId = table.node.id;
67-
if (!(tableId in privileges)) {
68-
privileges[tableId] = {
65+
const groupedPrivileges = this.privileges.reduce(
66+
(privileges, { table, actions }) => ({
67+
...privileges,
68+
[table.node.id]: {
69+
actions: [
70+
...(privileges[table.node.id]?.actions ?? []),
71+
...actions,
72+
],
6973
tableName: table.tableName,
70-
actions: [],
71-
};
72-
}
73-
actions = actions.concat(privileges[tableId].actions);
74-
if (actions.includes(TableAction.ALL)) {
75-
actions = [TableAction.ALL];
76-
}
77-
if (actions.includes(TableAction.UPDATE) || actions.includes(TableAction.DELETE)) {
78-
actions.push(TableAction.SELECT);
79-
}
80-
privileges[tableId] = {
81-
tableName: table.tableName,
82-
actions: Array.from(new Set(actions)),
83-
};
84-
return privileges;
85-
}, {} as { [key: string]: { tableName: string; actions: TableAction[] } });
86-
const serializedPrivileges: SerializedTablePrivilege[] = Object.entries(reducedPrivileges).map(([tableId, config]) => ({
74+
},
75+
}),
76+
{} as Record<string, { tableName: string; actions: TableAction[] }>,
77+
);
78+
79+
return Object.entries(groupedPrivileges).map(([tableId, config]) => ({
8780
tableId,
8881
tableName: config.tableName,
89-
actions: config.actions.map(action => TableAction[action]),
82+
actions: unifyTableActions(config.actions).map(action => TableAction[action]),
9083
}));
91-
return serializedPrivileges;
9284
},
9385
}) as any,
9486
},
@@ -102,3 +94,17 @@ export class UserTablePrivileges extends Construct {
10294
this.privileges.push({ table, actions });
10395
}
10496
}
97+
98+
const unifyTableActions = (tableActions: TableAction[]): TableAction[] => {
99+
const set = new Set<TableAction>(tableActions);
100+
101+
if (set.has(TableAction.ALL)) {
102+
return [TableAction.ALL];
103+
}
104+
105+
if (set.has(TableAction.UPDATE) || set.has(TableAction.DELETE)) {
106+
set.add(TableAction.SELECT);
107+
}
108+
109+
return [...set];
110+
};

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ export class Table extends TableBase {
272272
properties: {
273273
tableName: {
274274
prefix: props.tableName ?? cdk.Names.uniqueId(this),
275-
generateSuffix: !props.tableName ? 'true' : 'false',
275+
generateSuffix: (props.tableName == null).toString(),
276276
},
277277
tableColumns: this.tableColumns,
278278
distStyle: props.distStyle,
@@ -282,7 +282,7 @@ export class Table extends TableBase {
282282
},
283283
});
284284

285-
this.tableName = this.resource.ref;
285+
this.tableName = props.tableName ?? this.resource.ref;
286286
}
287287

288288
/**

packages/@aws-cdk/aws-redshift-alpha/test/integ.cluster-az-relocation.js.snapshot/manifest.json

Lines changed: 0 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)