feat(redshift): Tables can include comments (#23847)

Adds comments to tables. closes #22682 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Construct Runtime Dependencies: * [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
aws · Feb 3, 2023 · 46cadd4 · 46cadd4
1 parent 5bde92c
commit 46cadd4
Show file tree

Hide file tree

Showing 22 changed files with 245 additions and 142 deletions.
diff --git a/packages/@aws-cdk/aws-redshift/README.md b/packages/@aws-cdk/aws-redshift/README.md
@@ -54,15 +54,15 @@ import * as ec2 from '@aws-cdk/aws-ec2';
 import * as s3 from '@aws-cdk/aws-s3';
 
 const vpc = new ec2.Vpc(this, 'Vpc');
-const bucket = s3.Bucket.fromBucketName(stack, 'bucket', 'logging-bucket');
+const bucket = s3.Bucket.fromBucketName(this, 'bucket', 'logging-bucket');
 
 const cluster = new Cluster(this, 'Redshift', {
   masterUser: {
     masterUsername: 'admin',
   },
   vpc,
   loggingProperties: {
-    loggingBucket = bucket,
+    loggingBucket: bucket,
     loggingKeyPrefix: 'prefix',
   }
 });
@@ -200,6 +200,20 @@ new Table(this, 'Table', {
 });
 ```
 
+Tables can also be configured with a comment:
+
+```ts fixture=cluster
+new Table(this, 'Table', {
+  tableColumns: [
+    { name: 'col1', dataType: 'varchar(4)' }, 
+    { name: 'col2', dataType: 'float' }
+  ],
+  cluster: cluster,
+  databaseName: 'databaseName',
+  comment: 'This is a comment',
+});
+```
+
 ### Granting Privileges
 
 You can give a user privileges to perform certain actions on a table by using the
@@ -305,7 +319,9 @@ cluster.addRotationMultiUser('MultiUserRotation', {
 You can add a parameter to a parameter group with`ClusterParameterGroup.addParameter()`.
 
 ```ts
-const params = new ClusterParameterGroup(stack, 'Params', {
+import { ClusterParameterGroup } from '@aws-cdk/aws-redshift';
+
+const params = new ClusterParameterGroup(this, 'Params', {
   description: 'desc',
   parameters: {
     require_ssl: 'true',
@@ -318,6 +334,8 @@ params.addParameter('enable_user_activity_logging', 'true');
 Additionally, you can add a parameter to the cluster's associated parameter group with `Cluster.addToParameterGroup()`. If the cluster does not have an associated parameter group, a new parameter group is created.
 
 ```ts
+import * as ec2 from '@aws-cdk/aws-ec2';
+import * as cdk from '@aws-cdk/core';
 declare const vpc: ec2.Vpc;
 
 const cluster = new Cluster(this, 'Cluster', {
@@ -336,9 +354,11 @@ cluster.addToParameterGroup('enable_user_activity_logging', 'true');
 If you configure your cluster to be publicly accessible, you can optionally select an *elastic IP address* to use for the external IP address. An elastic IP address is a static IP address that is associated with your AWS account. You can use an elastic IP address to connect to your cluster from outside the VPC. An elastic IP address gives you the ability to change your underlying configuration without affecting the IP address that clients use to connect to your cluster. This approach can be helpful for situations such as recovery after a failure.
 
 ```ts
+import * as ec2 from '@aws-cdk/aws-ec2';
+import * as cdk from '@aws-cdk/core';
 declare const vpc: ec2.Vpc;
 
-new Cluster(stack, 'Redshift', {
+new Cluster(this, 'Redshift', {
     masterUser: {
       masterUsername: 'admin',
       masterPassword: cdk.SecretValue.unsafePlainText('tooshort'),
@@ -352,6 +372,7 @@ new Cluster(stack, 'Redshift', {
 If the Cluster is in a VPC and you want to connect to it using the private IP address from within the cluster, it is important to enable *DNS resolution* and *DNS hostnames* in the VPC config. If these parameters would not be set, connections from within the VPC would connect to the elastic IP address and not the private IP address.
 
 ```ts
+import * as ec2 from '@aws-cdk/aws-ec2';
 const vpc = new ec2.Vpc(this, 'VPC', {
   enableDnsSupport: true,
   enableDnsHostnames: true,
@@ -373,9 +394,11 @@ In some cases, you might want to associate the cluster with an elastic IP addres
 When you use Amazon Redshift enhanced VPC routing, Amazon Redshift forces all COPY and UNLOAD traffic between your cluster and your data repositories through your virtual private cloud (VPC) based on the Amazon VPC service. By using enhanced VPC routing, you can use standard VPC features, such as VPC security groups, network access control lists (ACLs), VPC endpoints, VPC endpoint policies, internet gateways, and Domain Name System (DNS) servers, as described in the Amazon VPC User Guide. You use these features to tightly manage the flow of data between your Amazon Redshift cluster and other resources. When you use enhanced VPC routing to route traffic through your VPC, you can also use VPC flow logs to monitor COPY and UNLOAD traffic.
 
 ```ts
+import * as ec2 from '@aws-cdk/aws-ec2';
+import * as cdk from '@aws-cdk/core';
 declare const vpc: ec2.Vpc;
 
-new Cluster(stack, 'Redshift', {
+new Cluster(this, 'Redshift', {
     masterUser: {
       masterUsername: 'admin',
       masterPassword: cdk.SecretValue.unsafePlainText('tooshort'),

diff --git a/packages/@aws-cdk/aws-redshift/lib/private/database-query-provider/table.ts b/packages/@aws-cdk/aws-redshift/lib/private/database-query-provider/table.ts
@@ -60,6 +60,11 @@ async function createTable(
   }
 
   await executeStatement(statement, tableAndClusterProps);
+
+  if (tableAndClusterProps.tableComment) {
+    await executeStatement(`COMMENT ON TABLE ${tableName} IS '${tableAndClusterProps.tableComment}'`, tableAndClusterProps);
+  }
+
   return tableName;
 }
 
@@ -143,6 +148,12 @@ async function updateTable(
     }
   }
 
+  const oldComment = oldResourceProperties.tableComment;
+  const newComment = tableAndClusterProps.tableComment;
+  if (oldComment !== newComment) {
+    alterationStatements.push(`COMMENT ON TABLE ${tableName} IS ${newComment ? `'${newComment}'` : 'NULL'}`);
+  }
+
   await Promise.all(alterationStatements.map(statement => executeStatement(statement, tableAndClusterProps)));
 
   return tableName;

diff --git a/packages/@aws-cdk/aws-redshift/lib/private/handler-props.ts b/packages/@aws-cdk/aws-redshift/lib/private/handler-props.ts
@@ -20,6 +20,7 @@ export interface TableHandlerProps {
   readonly tableColumns: Column[];
   readonly distStyle?: TableDistStyle;
   readonly sortStyle: TableSortStyle;
+  readonly tableComment?: string;
 }
 
 export interface TablePrivilege {

diff --git a/packages/@aws-cdk/aws-redshift/lib/table.ts b/packages/@aws-cdk/aws-redshift/lib/table.ts
@@ -117,6 +117,13 @@ export interface TableProps extends DatabaseOptions {
    * @default cdk.RemovalPolicy.Retain
    */
   readonly removalPolicy?: cdk.RemovalPolicy;
+
+  /**
+     * A comment to attach to the table.
+     *
+     * @default - no comment
+     */
+  readonly tableComment?: string;
 }
 
 /**
@@ -234,6 +241,7 @@ export class Table extends TableBase {
         tableColumns: this.tableColumns,
         distStyle: props.distStyle,
         sortStyle: props.sortStyle ?? this.getDefaultSortStyle(props.tableColumns),
+        tableComment: props.tableComment,
       },
     });
 

diff --git a/packages/@aws-cdk/aws-redshift/test/database-query-provider/table.test.ts b/packages/@aws-cdk/aws-redshift/test/database-query-provider/table.test.ts
@@ -134,6 +134,20 @@ describe('create', () => {
       Sql: `CREATE TABLE ${tableNamePrefix}${requestIdTruncated} (col1 varchar(4),col2 float,col3 float) DISTSTYLE KEY DISTKEY(col1) COMPOUND SORTKEY(col2,col3)`,
     }));
   });
+
+  test('serializes table comment in statement', async () => {
+    const event = baseEvent;
+    const newResourceProperties: ResourcePropertiesType = {
+      ...resourceProperties,
+      tableComment: 'table comment',
+    };
+
+    await manageTable(newResourceProperties, event);
+
+    expect(mockExecuteStatement).toHaveBeenCalledWith(expect.objectContaining({
+      Sql: `COMMENT ON TABLE ${tableNamePrefix}${requestIdTruncated} IS 'table comment'`,
+    }));
+  });
 });
 
 describe('delete', () => {
@@ -502,4 +516,40 @@ describe('update', () => {
     });
   });
 
+  describe('table comment', () => {
+    test('does not replace if comment added on table', async () => {
+      const newComment = 'newComment';
+      const newResourceProperties = {
+        ...resourceProperties,
+        tableComment: newComment,
+      };
+
+      await expect(manageTable(newResourceProperties, event)).resolves.toMatchObject({
+        PhysicalResourceId: physicalResourceId,
+      });
+      expect(mockExecuteStatement).toHaveBeenCalledWith(expect.objectContaining({
+        Sql: `COMMENT ON TABLE ${physicalResourceId} IS '${newComment}'`,
+      }));
+    });
+
+    test('does not replace if comment removed on table', async () => {
+      const newEvent = {
+        ...event,
+        OldResourceProperties: {
+          ...event.OldResourceProperties,
+          tableComment: 'oldComment',
+        },
+      };
+      const newResourceProperties = {
+        ...resourceProperties,
+      };
+
+      await expect(manageTable(newResourceProperties, newEvent)).resolves.toMatchObject({
+        PhysicalResourceId: physicalResourceId,
+      });
+      expect(mockExecuteStatement).toHaveBeenCalledWith(expect.objectContaining({
+        Sql: `COMMENT ON TABLE ${physicalResourceId} IS NULL`,
+      }));
+    });
+  });
 });
diff --git a/....snapshot/asset.8359857aa9b7c3fbc8bba9a505282ba848915383c4549f21b9f93f9f35b56415/table.js b/....snapshot/asset.8359857aa9b7c3fbc8bba9a505282ba848915383c4549f21b9f93f9f35b56415/table.js
diff --git a/...3c4549f21b9f93f9f35b56415/handler-name.js → ...8807ab82e4559967d3b885b08/handler-name.js b/...3c4549f21b9f93f9f35b56415/handler-name.js → ...8807ab82e4559967d3b885b08/handler-name.js
diff --git a/...48915383c4549f21b9f93f9f35b56415/index.js → ...96fb3ae8807ab82e4559967d3b885b08/index.js b/...48915383c4549f21b9f93f9f35b56415/index.js → ...96fb3ae8807ab82e4559967d3b885b08/index.js
diff --git a/...383c4549f21b9f93f9f35b56415/privileges.js → ...ae8807ab82e4559967d3b885b08/privileges.js b/...383c4549f21b9f93f9f35b56415/privileges.js → ...ae8807ab82e4559967d3b885b08/privileges.js
diff --git a/...c4549f21b9f93f9f35b56415/redshift-data.js → ...807ab82e4559967d3b885b08/redshift-data.js b/...c4549f21b9f93f9f35b56415/redshift-data.js → ...807ab82e4559967d3b885b08/redshift-data.js
diff --git a/....snapshot/asset.ab58b1384030fef0fc8663c06f6fd62196fb3ae8807ab82e4559967d3b885b08/table.js b/....snapshot/asset.ab58b1384030fef0fc8663c06f6fd62196fb3ae8807ab82e4559967d3b885b08/table.js
diff --git a/...48915383c4549f21b9f93f9f35b56415/types.js → ...96fb3ae8807ab82e4559967d3b885b08/types.js b/...48915383c4549f21b9f93f9f35b56415/types.js → ...96fb3ae8807ab82e4559967d3b885b08/types.js
diff --git a/...848915383c4549f21b9f93f9f35b56415/user.js → ...196fb3ae8807ab82e4559967d3b885b08/user.js b/...848915383c4549f21b9f93f9f35b56415/user.js → ...196fb3ae8807ab82e4559967d3b885b08/user.js
diff --git a/...848915383c4549f21b9f93f9f35b56415/util.js → ...196fb3ae8807ab82e4559967d3b885b08/util.js b/...848915383c4549f21b9f93f9f35b56415/util.js → ...196fb3ae8807ab82e4559967d3b885b08/util.js
diff --git a/...ws-redshift/test/integ.database.js.snapshot/aws-cdk-redshift-cluster-database.assets.json b/...ws-redshift/test/integ.database.js.snapshot/aws-cdk-redshift-cluster-database.assets.json
@@ -1,15 +1,15 @@
 {
-  "version": "22.0.0",
+  "version": "29.0.0",
   "files": {
-    "8359857aa9b7c3fbc8bba9a505282ba848915383c4549f21b9f93f9f35b56415": {
+    "ab58b1384030fef0fc8663c06f6fd62196fb3ae8807ab82e4559967d3b885b08": {
       "source": {
-        "path": "asset.8359857aa9b7c3fbc8bba9a505282ba848915383c4549f21b9f93f9f35b56415",
+        "path": "asset.ab58b1384030fef0fc8663c06f6fd62196fb3ae8807ab82e4559967d3b885b08",
         "packaging": "zip"
       },
       "destinations": {
         "current_account-current_region": {
           "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
-          "objectKey": "8359857aa9b7c3fbc8bba9a505282ba848915383c4549f21b9f93f9f35b56415.zip",
+          "objectKey": "ab58b1384030fef0fc8663c06f6fd62196fb3ae8807ab82e4559967d3b885b08.zip",
           "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
         }
       }
@@ -27,15 +27,15 @@
         }
       }
     },
-    "fadca82b7c081a8b6de68f952878e92ba5610dce63ccbead865e1b854073bff0": {
+    "fd9bc22f4d8ca7fabbbe054d374117616ffa6a2393152ecb529d2b385432d259": {
       "source": {
         "path": "aws-cdk-redshift-cluster-database.template.json",
         "packaging": "file"
       },
       "destinations": {
         "current_account-current_region": {
           "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
-          "objectKey": "fadca82b7c081a8b6de68f952878e92ba5610dce63ccbead865e1b854073bff0.json",
+          "objectKey": "fd9bc22f4d8ca7fabbbe054d374117616ffa6a2393152ecb529d2b385432d259.json",
           "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
         }
       }

diff --git a/...-redshift/test/integ.database.js.snapshot/aws-cdk-redshift-cluster-database.template.json b/...-redshift/test/integ.database.js.snapshot/aws-cdk-redshift-cluster-database.template.json
@@ -1004,7 +1004,7 @@
      "S3Bucket": {
       "Fn::Sub": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}"
      },
-     "S3Key": "8359857aa9b7c3fbc8bba9a505282ba848915383c4549f21b9f93f9f35b56415.zip"
+     "S3Key": "ab58b1384030fef0fc8663c06f6fd62196fb3ae8807ab82e4559967d3b885b08.zip"
     },
     "Role": {
      "Fn::GetAtt": [
@@ -1176,7 +1176,8 @@
      }
     ],
     "distStyle": "KEY",
-    "sortStyle": "INTERLEAVED"
+    "sortStyle": "INTERLEAVED",
+    "tableComment": "A test table"
    },
    "UpdateReplacePolicy": "Delete",
    "DeletionPolicy": "Delete"

diff --git a/packages/@aws-cdk/aws-redshift/test/integ.database.js.snapshot/cdk.out b/packages/@aws-cdk/aws-redshift/test/integ.database.js.snapshot/cdk.out
@@ -1 +1 @@
-{"version":"22.0.0"}
+{"version":"29.0.0"}
diff --git a/packages/@aws-cdk/aws-redshift/test/integ.database.js.snapshot/integ.json b/packages/@aws-cdk/aws-redshift/test/integ.database.js.snapshot/integ.json
@@ -1,5 +1,5 @@
 {
-  "version": "22.0.0",
+  "version": "29.0.0",
   "testCases": {
     "redshift-cluster-database-integ/DefaultTest": {
       "stacks": [

diff --git a/packages/@aws-cdk/aws-redshift/test/integ.database.js.snapshot/manifest.json b/packages/@aws-cdk/aws-redshift/test/integ.database.js.snapshot/manifest.json
@@ -1,5 +1,5 @@
 {
-  "version": "22.0.0",
+  "version": "29.0.0",
   "artifacts": {
     "aws-cdk-redshift-cluster-database.assets": {
       "type": "cdk:asset-manifest",
@@ -17,7 +17,7 @@
         "validateOnSynth": false,
         "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
         "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
-        "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/fadca82b7c081a8b6de68f952878e92ba5610dce63ccbead865e1b854073bff0.json",
+        "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/fd9bc22f4d8ca7fabbbe054d374117616ffa6a2393152ecb529d2b385432d259.json",
         "requiresBootstrapStackVersion": 6,
         "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
         "additionalDependencies": [

diff --git a/...abase.js.snapshot/redshiftclusterdatabaseintegDefaultTestDeployAssert4339FB48.assets.json b/...abase.js.snapshot/redshiftclusterdatabaseintegDefaultTestDeployAssert4339FB48.assets.json
@@ -1,5 +1,5 @@
 {
-  "version": "22.0.0",
+  "version": "29.0.0",
   "files": {
     "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
       "source": {

diff --git a/packages/@aws-cdk/aws-redshift/test/integ.database.js.snapshot/tree.json b/packages/@aws-cdk/aws-redshift/test/integ.database.js.snapshot/tree.json
@@ -1228,7 +1228,7 @@
                 },
                 "constructInfo": {
                   "fqn": "constructs.Construct",
-                  "version": "10.1.182"
+                  "version": "10.1.209"
                 }
               },
               "TablePrivileges": {
@@ -1470,13 +1470,13 @@
                     },
                     "constructInfo": {
                       "fqn": "constructs.Construct",
-                      "version": "10.1.182"
+                      "version": "10.1.209"
                     }
                   }
                 },
                 "constructInfo": {
                   "fqn": "constructs.Construct",
-                  "version": "10.1.182"
+                  "version": "10.1.209"
                 }
               }
             },
@@ -1639,7 +1639,7 @@
                       "s3Bucket": {
                         "Fn::Sub": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}"
                       },
-                      "s3Key": "8359857aa9b7c3fbc8bba9a505282ba848915383c4549f21b9f93f9f35b56415.zip"
+                      "s3Key": "ab58b1384030fef0fc8663c06f6fd62196fb3ae8807ab82e4559967d3b885b08.zip"
                     },
                     "role": {
                       "Fn::GetAtt": [
@@ -1902,7 +1902,7 @@
                 },
                 "constructInfo": {
                   "fqn": "constructs.Construct",
-                  "version": "10.1.182"
+                  "version": "10.1.209"
                 }
               }
             },
@@ -1946,7 +1946,7 @@
                 "path": "redshift-cluster-database-integ/DefaultTest/Default",
                 "constructInfo": {
                   "fqn": "constructs.Construct",
-                  "version": "10.1.182"
+                  "version": "10.1.209"
                 }
               },
               "DeployAssert": {
@@ -1992,7 +1992,7 @@
         "path": "Tree",
         "constructInfo": {
           "fqn": "constructs.Construct",
-          "version": "10.1.182"
+          "version": "10.1.209"
         }
       }
     },

diff --git a/packages/@aws-cdk/aws-redshift/test/integ.database.ts b/packages/@aws-cdk/aws-redshift/test/integ.database.ts
@@ -49,6 +49,7 @@ const table = new redshift.Table(stack, 'Table', {
   ],
   distStyle: redshift.TableDistStyle.KEY,
   sortStyle: redshift.TableSortStyle.INTERLEAVED,
+  tableComment: 'A test table',
 });
 table.grant(user, redshift.TableAction.INSERT, redshift.TableAction.DELETE);