fix(redshift): tableNameSuffix evaluation

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

@@ -6,7 +6,7 @@ import { ClusterProps, executeStatement } from './util';
 
 export async function handler(props: TableHandlerProps & ClusterProps, event: AWSLambda.CloudFormationCustomResourceEvent) {
   const tableNamePrefix = props.tableName.prefix;
-  const tableNameSuffix = props.tableName.generateSuffix ? `${event.RequestId.substring(0, 8)}` : '';
+  const tableNameSuffix = props.tableName.generateSuffix === 'true' ? `${event.RequestId.substring(0, 8)}` : '';
   const tableColumns = props.tableColumns;
   const clusterProps = props;
 

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

@@ -15,7 +15,7 @@ export interface UserHandlerProps {
 export interface TableHandlerProps {
   readonly tableName: {
     readonly prefix: string;
-    readonly generateSuffix: boolean;
+    readonly generateSuffix: string;
   };
   readonly tableColumns: Column[];
 }

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

@@ -164,7 +164,7 @@ export class Table extends TableBase {
   /**
    * Specify a Redshift table using a table name and schema that already exists.
    */
-  static fromTableAttributes(scope: Construct, id: string, attrs: TableAttributes): ITable {
+   static fromTableAttributes(scope: Construct, id: string, attrs: TableAttributes): ITable {
     return new class extends TableBase {
       readonly tableName = attrs.tableName;
       readonly tableColumns = attrs.tableColumns;
@@ -194,7 +194,7 @@ export class Table extends TableBase {
       properties: {
         tableName: {
           prefix: props.tableName ?? cdk.Names.uniqueId(this),
-          generateSuffix: !props.tableName,
+          generateSuffix: !props.tableName ? 'true' : 'false',
         },
         tableColumns: this.tableColumns,
       },

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

@@ -10,7 +10,7 @@ const physicalResourceId = 'PhysicalResourceId';
 const resourceProperties = {
   tableName: {
     prefix: tableNamePrefix,
-    generateSuffix: true,
+    generateSuffix: 'true',
   },
   tableColumns,
   clusterName,
@@ -30,11 +30,19 @@ const genericEvent: AWSLambda.CloudFormationCustomResourceEventCommon = {
   ResourceType: '',
 };
 
-const mockExecuteStatement = jest.fn(() => ({ promise: jest.fn(() => ({ Id: 'statementId' })) }));
-jest.mock('aws-sdk/clients/redshiftdata', () => class {
-  executeStatement = mockExecuteStatement;
-  describeStatement = () => ({ promise: jest.fn(() => ({ Status: 'FINISHED' })) });
-});
+const mockExecuteStatement = jest.fn(() => ({
+  promise: jest.fn(() => ({ Id: 'statementId' })),
+}));
+jest.mock(
+  'aws-sdk/clients/redshiftdata',
+  () =>
+    class {
+      executeStatement = mockExecuteStatement;
+      describeStatement = () => ({
+        promise: jest.fn(() => ({ Status: 'FINISHED' })),
+      });
+    },
+);
 import { handler as manageTable } from '../../lib/private/database-query-provider/table';
 
 beforeEach(() => {
@@ -53,9 +61,11 @@ describe('create', () => {
     await expect(manageTable(resourceProperties, event)).resolves.toEqual({
       PhysicalResourceId: `${tableNamePrefix}${requestIdTruncated}`,
     });
-    expect(mockExecuteStatement).toHaveBeenCalledWith(expect.objectContaining({
-      Sql: `CREATE TABLE ${tableNamePrefix}${requestIdTruncated} (col1 varchar(1))`,
-    }));
+    expect(mockExecuteStatement).toHaveBeenCalledWith(
+      expect.objectContaining({
+        Sql: `CREATE TABLE ${tableNamePrefix}${requestIdTruncated} (col1 varchar(1))`,
+      }),
+    );
   });
 
   test('does not modify table name if no suffix generation requested', async () => {
@@ -64,16 +74,18 @@ describe('create', () => {
       ...resourceProperties,
       tableName: {
         ...resourceProperties.tableName,
-        generateSuffix: false,
+        generateSuffix: 'false',
       },
     };
 
     await expect(manageTable(newResourceProperties, event)).resolves.toEqual({
       PhysicalResourceId: tableNamePrefix,
     });
-    expect(mockExecuteStatement).toHaveBeenCalledWith(expect.objectContaining({
-      Sql: `CREATE TABLE ${tableNamePrefix} (col1 varchar(1))`,
-    }));
+    expect(mockExecuteStatement).toHaveBeenCalledWith(
+      expect.objectContaining({
+        Sql: `CREATE TABLE ${tableNamePrefix} (col1 varchar(1))`,
+      }),
+    );
   });
 });
 
@@ -89,9 +101,11 @@ describe('delete', () => {
 
     await manageTable(resourceProperties, event);
 
-    expect(mockExecuteStatement).toHaveBeenCalledWith(expect.objectContaining({
-      Sql: `DROP TABLE ${physicalResourceId}`,
-    }));
+    expect(mockExecuteStatement).toHaveBeenCalledWith(
+      expect.objectContaining({
+        Sql: `DROP TABLE ${physicalResourceId}`,
+      }),
+    );
   });
 });
 
@@ -110,13 +124,19 @@ describe('update', () => {
       clusterName: newClusterName,
     };
 
-    await expect(manageTable(newResourceProperties, event)).resolves.not.toMatchObject({
+    await expect(
+      manageTable(newResourceProperties, event),
+    ).resolves.not.toMatchObject({
       PhysicalResourceId: physicalResourceId,
     });
-    expect(mockExecuteStatement).toHaveBeenCalledWith(expect.objectContaining({
-      ClusterIdentifier: newClusterName,
-      Sql: expect.stringMatching(new RegExp(`CREATE TABLE ${tableNamePrefix}${requestIdTruncated}`)),
-    }));
+    expect(mockExecuteStatement).toHaveBeenCalledWith(
+      expect.objectContaining({
+        ClusterIdentifier: newClusterName,
+        Sql: expect.stringMatching(
+          new RegExp(`CREATE TABLE ${tableNamePrefix}${requestIdTruncated}`),
+        ),
+      }),
+    );
   });
 
   test('does not replace if admin user ARN changes', async () => {
@@ -126,7 +146,9 @@ describe('update', () => {
       adminUserArn: newAdminUserArn,
     };
 
-    await expect(manageTable(newResourceProperties, event)).resolves.toMatchObject({
+    await expect(
+      manageTable(newResourceProperties, event),
+    ).resolves.toMatchObject({
       PhysicalResourceId: physicalResourceId,
     });
     expect(mockExecuteStatement).not.toHaveBeenCalled();
@@ -139,13 +161,19 @@ describe('update', () => {
       databaseName: newDatabaseName,
     };
 
-    await expect(manageTable(newResourceProperties, event)).resolves.not.toMatchObject({
+    await expect(
+      manageTable(newResourceProperties, event),
+    ).resolves.not.toMatchObject({
       PhysicalResourceId: physicalResourceId,
     });
-    expect(mockExecuteStatement).toHaveBeenCalledWith(expect.objectContaining({
-      Database: newDatabaseName,
-      Sql: expect.stringMatching(new RegExp(`CREATE TABLE ${tableNamePrefix}${requestIdTruncated}`)),
-    }));
+    expect(mockExecuteStatement).toHaveBeenCalledWith(
+      expect.objectContaining({
+        Database: newDatabaseName,
+        Sql: expect.stringMatching(
+          new RegExp(`CREATE TABLE ${tableNamePrefix}${requestIdTruncated}`),
+        ),
+      })
+    );
   });
 
   test('replaces if table name changes', async () => {
@@ -158,45 +186,64 @@ describe('update', () => {
       },
     };
 
-    await expect(manageTable(newResourceProperties, event)).resolves.not.toMatchObject({
+    await expect(
+      manageTable(newResourceProperties, event),
+    ).resolves.not.toMatchObject({
       PhysicalResourceId: physicalResourceId,
     });
-    expect(mockExecuteStatement).toHaveBeenCalledWith(expect.objectContaining({
-      Sql: expect.stringMatching(new RegExp(`CREATE TABLE ${newTableNamePrefix}${requestIdTruncated}`)),
-    }));
+    expect(mockExecuteStatement).toHaveBeenCalledWith(
+      expect.objectContaining({
+        Sql: expect.stringMatching(
+          new RegExp(`CREATE TABLE ${newTableNamePrefix}${requestIdTruncated}`),
+        ),
+      }),
+    );
   });
 
   test('replaces if table columns change', async () => {
     const newTableColumnName = 'col2';
     const newTableColumnDataType = 'varchar(1)';
-    const newTableColumns = [{ name: newTableColumnName, dataType: newTableColumnDataType }];
+    const newTableColumns = [
+      { name: newTableColumnName, dataType: newTableColumnDataType },
+    ];
     const newResourceProperties = {
       ...resourceProperties,
       tableColumns: newTableColumns,
     };
 
-    await expect(manageTable(newResourceProperties, event)).resolves.not.toMatchObject({
+    await expect(
+      manageTable(newResourceProperties, event),
+    ).resolves.not.toMatchObject({
       PhysicalResourceId: physicalResourceId,
     });
-    expect(mockExecuteStatement).toHaveBeenCalledWith(expect.objectContaining({
-      Sql: `CREATE TABLE ${tableNamePrefix}${requestIdTruncated} (${newTableColumnName} ${newTableColumnDataType})`,
-    }));
+    expect(mockExecuteStatement).toHaveBeenCalledWith(
+      expect.objectContaining({
+        Sql: `CREATE TABLE ${tableNamePrefix}${requestIdTruncated} (${newTableColumnName} ${newTableColumnDataType})`,
+      }),
+    );
   });
 
   test('does not replace if table columns added', async () => {
     const newTableColumnName = 'col2';
     const newTableColumnDataType = 'varchar(1)';
-    const newTableColumns = [{ name: 'col1', dataType: 'varchar(1)' }, { name: newTableColumnName, dataType: newTableColumnDataType }];
+    const newTableColumns = [
+      { name: 'col1', dataType: 'varchar(1)' },
+      { name: newTableColumnName, dataType: newTableColumnDataType },
+    ];
     const newResourceProperties = {
       ...resourceProperties,
       tableColumns: newTableColumns,
     };
 
-    await expect(manageTable(newResourceProperties, event)).resolves.toMatchObject({
+    await expect(
+      manageTable(newResourceProperties, event),
+    ).resolves.toMatchObject({
       PhysicalResourceId: physicalResourceId,
     });
-    expect(mockExecuteStatement).toHaveBeenCalledWith(expect.objectContaining({
-      Sql: `ALTER TABLE ${physicalResourceId} ADD ${newTableColumnName} ${newTableColumnDataType}`,
-    }));
+    expect(mockExecuteStatement).toHaveBeenCalledWith(
+      expect.objectContaining({
+        Sql: `ALTER TABLE ${physicalResourceId} ADD ${newTableColumnName} ${newTableColumnDataType}`,
+      }),
+    );
   });
 });

packages/@aws-cdk/aws-redshift/test/integ.database.expected.json

@@ -1369,7 +1369,7 @@
         "databaseName": "my_db",
         "tableName": {
           "prefix": "awscdkredshiftclusterdatabaseTable24923533",
-          "generateSuffix": true
+          "generateSuffix": "true"
         },
         "tableColumns": [
           {

packages/@aws-cdk/aws-redshift/test/table.test.ts

@@ -37,13 +37,16 @@ describe('cluster table', () => {
       tableColumns,
     });
 
-    Template.fromStack(stack).hasResourceProperties('Custom::RedshiftDatabaseQuery', {
-      tableName: {
-        prefix: 'Table',
-        generateSuffix: true,
+    Template.fromStack(stack).hasResourceProperties(
+      'Custom::RedshiftDatabaseQuery',
+      {
+        tableName: {
+          prefix: 'Table',
+          generateSuffix: 'true',
+        },
+        tableColumns,
       },
-      tableColumns,
-    });
+    );
   });
 
   it('tableName property is pulled from custom resource', () => {
@@ -64,20 +67,23 @@ describe('cluster table', () => {
       tableColumns,
     });
 
-    Template.fromStack(stack).hasResourceProperties('Custom::RedshiftDatabaseQuery', {
-      tableName: {
-        prefix: tableName,
-        generateSuffix: false,
+    Template.fromStack(stack).hasResourceProperties(
+      'Custom::RedshiftDatabaseQuery',
+      {
+        tableName: {
+          prefix: tableName,
+          generateSuffix: 'false',
+        },
       },
-    });
+    );
   });
 
   it('can import from name and columns', () => {
     const table = redshift.Table.fromTableAttributes(stack, 'Table', {
       tableName,
       tableColumns,
       cluster,
-      databaseName: 'databaseName',
+      databaseName: 'databaseName'
     });
 
     expect(table.tableName).toBe(tableName);
@@ -101,9 +107,12 @@ describe('cluster table', () => {
 
     table.grant(user, redshift.TableAction.INSERT);
 
-    Template.fromStack(stack).hasResourceProperties('Custom::RedshiftDatabaseQuery', {
-      handler: 'user-table-privileges',
-    });
+    Template.fromStack(stack).hasResourceProperties(
+      'Custom::RedshiftDatabaseQuery',
+      {
+        handler: 'user-table-privileges'
+      },
+    );
   });
 
   it('retains table on deletion by default', () => {
@@ -116,14 +125,14 @@ describe('cluster table', () => {
       Properties: {
         handler: 'table',
       },
-      DeletionPolicy: 'Retain',
+      DeletionPolicy: 'Retain'
     });
   });
 
   it('destroys table on deletion if requested', () => {
     const table = new redshift.Table(stack, 'Table', {
       ...databaseOptions,
-      tableColumns,
+      tableColumns
     });
 
     table.applyRemovalPolicy(cdk.RemovalPolicy.DESTROY);
@@ -132,7 +141,7 @@ describe('cluster table', () => {
       Properties: {
         handler: 'table',
       },
-      DeletionPolicy: 'Delete',
+      DeletionPolicy: 'Delete'
     });
   });
 });