From bf35048cc5f907c7226f60aa8b3b4b8b500d2bc0 Mon Sep 17 00:00:00 2001 From: Joshua Weber <57131123+daschaa@users.noreply.github.com> Date: Tue, 12 Jul 2022 05:11:12 +0200 Subject: [PATCH 1/7] feat(glue): enable partition filtering on tables (#21081) Fixes #20825 Adds `partition_filtering.enabled` to `TableProps` in AWS Glue. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-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* --- packages/@aws-cdk/aws-glue/README.md | 25 ++++ packages/@aws-cdk/aws-glue/lib/table.ts | 14 ++- .../@aws-cdk/aws-glue/test/integ.table.ts | 8 ++ .../aws-cdk-glue.assets.json | 6 +- .../aws-cdk-glue.template.json | 70 ++++++++++++ .../test/table.integ.snapshot/cdk.out | 2 +- .../test/table.integ.snapshot/integ.json | 4 +- .../test/table.integ.snapshot/manifest.json | 14 ++- .../test/table.integ.snapshot/tree.json | 107 +++++++++++++++++- packages/@aws-cdk/aws-glue/test/table.test.ts | 94 ++++++++++++++- 10 files changed, 333 insertions(+), 11 deletions(-) diff --git a/packages/@aws-cdk/aws-glue/README.md b/packages/@aws-cdk/aws-glue/README.md index 639198c67ed9a..88cf57faaa10b 100644 --- a/packages/@aws-cdk/aws-glue/README.md +++ b/packages/@aws-cdk/aws-glue/README.md @@ -264,6 +264,31 @@ myTable.addPartitionIndex({ }); ``` +### Partition Filtering + +If you have a table with a large number of partitions that grows over time, consider using AWS Glue partition indexing and filtering. + +```ts +declare const myDatabase: glue.Database; +new glue.Table(this, 'MyTable', { + database: myDatabase, + tableName: 'my_table', + columns: [{ + name: 'col1', + type: glue.Schema.STRING, + }], + partitionKeys: [{ + name: 'year', + type: glue.Schema.SMALL_INT, + }, { + name: 'month', + type: glue.Schema.SMALL_INT, + }], + dataFormat: glue.DataFormat.JSON, + enablePartitionFiltering: true, +}); +``` + ## [Encryption](https://docs.aws.amazon.com/athena/latest/ug/encryption.html) You can enable encryption on a Table's data: diff --git a/packages/@aws-cdk/aws-glue/lib/table.ts b/packages/@aws-cdk/aws-glue/lib/table.ts index 1b17da32e5454..ea958879d816b 100644 --- a/packages/@aws-cdk/aws-glue/lib/table.ts +++ b/packages/@aws-cdk/aws-glue/lib/table.ts @@ -172,6 +172,15 @@ export interface TableProps { * @default false */ readonly storedAsSubDirectories?: boolean; + + /** + * Enables partition filtering. + * + * @see https://docs.aws.amazon.com/athena/latest/ug/glue-best-practices.html#glue-best-practices-partition-index + * + * @default - The parameter is not defined + */ + readonly enablePartitionFiltering?: boolean; } /** @@ -302,8 +311,9 @@ export class Table extends Resource implements ITable { partitionKeys: renderColumns(props.partitionKeys), parameters: { - classification: props.dataFormat.classificationString?.value, - has_encrypted_data: this.encryption !== TableEncryption.UNENCRYPTED, + 'classification': props.dataFormat.classificationString?.value, + 'has_encrypted_data': this.encryption !== TableEncryption.UNENCRYPTED, + 'partition_filtering.enabled': props.enablePartitionFiltering, }, storageDescriptor: { location: `s3://${this.bucket.bucketName}/${this.s3Prefix}`, diff --git a/packages/@aws-cdk/aws-glue/test/integ.table.ts b/packages/@aws-cdk/aws-glue/test/integ.table.ts index e9d54d659921e..d9d543a124d16 100644 --- a/packages/@aws-cdk/aws-glue/test/integ.table.ts +++ b/packages/@aws-cdk/aws-glue/test/integ.table.ts @@ -87,6 +87,14 @@ const encryptedTable = new glue.Table(stack, 'MyEncryptedTable', { encryptionKey: new kms.Key(stack, 'MyKey'), }); +new glue.Table(stack, 'MyPartitionFilteredTable', { + database, + tableName: 'partition_filtered_table', + columns, + dataFormat: glue.DataFormat.JSON, + enablePartitionFiltering: true, +}); + const user = new iam.User(stack, 'MyUser'); csvTable.grantReadWrite(user); encryptedTable.grantReadWrite(user); diff --git a/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/aws-cdk-glue.assets.json b/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/aws-cdk-glue.assets.json index 772ec2685ec29..0cd673b0f5be4 100644 --- a/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/aws-cdk-glue.assets.json +++ b/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/aws-cdk-glue.assets.json @@ -1,7 +1,7 @@ { - "version": "17.0.0", + "version": "20.0.0", "files": { - "92638b7a8efe38efd7c845883423f3767018a9e5bd3d67d8d638332f054d0d0f": { + "419b39f03d496de4fb02e795181e9a2ab218fb90bf7a5c9354cf93baa6fea2cf": { "source": { "path": "aws-cdk-glue.template.json", "packaging": "file" @@ -9,7 +9,7 @@ "destinations": { "current_account-current_region": { "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", - "objectKey": "92638b7a8efe38efd7c845883423f3767018a9e5bd3d67d8d638332f054d0d0f.json", + "objectKey": "419b39f03d496de4fb02e795181e9a2ab218fb90bf7a5c9354cf93baa6fea2cf.json", "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}" } } diff --git a/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/aws-cdk-glue.template.json b/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/aws-cdk-glue.template.json index 7f5f3e286f0ef..e64e4fe3b8a20 100644 --- a/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/aws-cdk-glue.template.json +++ b/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/aws-cdk-glue.template.json @@ -423,6 +423,76 @@ } } }, + "MyPartitionFilteredTableBucket6ACAA137": { + "Type": "AWS::S3::Bucket", + "UpdateReplacePolicy": "Retain", + "DeletionPolicy": "Retain" + }, + "MyPartitionFilteredTable324BA27A": { + "Type": "AWS::Glue::Table", + "Properties": { + "CatalogId": { + "Ref": "AWS::AccountId" + }, + "DatabaseName": { + "Ref": "MyDatabase1E2517DB" + }, + "TableInput": { + "Description": "partition_filtered_table generated by CDK", + "Name": "partition_filtered_table", + "Parameters": { + "classification": "json", + "has_encrypted_data": false, + "partition_filtering.enabled": true + }, + "StorageDescriptor": { + "Columns": [ + { + "Name": "col1", + "Type": "string" + }, + { + "Comment": "col2 comment", + "Name": "col2", + "Type": "string" + }, + { + "Name": "col3", + "Type": "array" + }, + { + "Name": "col4", + "Type": "map" + }, + { + "Name": "col5", + "Type": "struct" + } + ], + "Compressed": false, + "InputFormat": "org.apache.hadoop.mapred.TextInputFormat", + "Location": { + "Fn::Join": [ + "", + [ + "s3://", + { + "Ref": "MyPartitionFilteredTableBucket6ACAA137" + }, + "/" + ] + ] + }, + "OutputFormat": "org.apache.hadoop.hive.ql.io.HiveIgnoreKeyTextOutputFormat", + "SerdeInfo": { + "SerializationLibrary": "org.openx.data.jsonserde.JsonSerDe" + }, + "StoredAsSubDirectories": false + }, + "TableType": "EXTERNAL_TABLE" + } + } + }, "MyUserDC45028B": { "Type": "AWS::IAM::User" }, diff --git a/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/cdk.out b/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/cdk.out index 90bef2e09ad39..588d7b269d34f 100644 --- a/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/cdk.out +++ b/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/cdk.out @@ -1 +1 @@ -{"version":"17.0.0"} \ No newline at end of file +{"version":"20.0.0"} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/integ.json b/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/integ.json index 85b6fe8295b26..1f604630bc610 100644 --- a/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/integ.json +++ b/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/integ.json @@ -1,7 +1,7 @@ { - "version": "18.0.0", + "version": "20.0.0", "testCases": { - "aws-glue/test/integ.table": { + "integ.table": { "stacks": [ "aws-cdk-glue" ], diff --git a/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/manifest.json b/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/manifest.json index 221b7524100fa..aed4259921a6f 100644 --- a/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/manifest.json +++ b/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/manifest.json @@ -1,5 +1,5 @@ { - "version": "17.0.0", + "version": "20.0.0", "artifacts": { "Tree": { "type": "cdk:tree", @@ -69,6 +69,18 @@ "data": "MyEncryptedTable981A88C6" } ], + "/aws-cdk-glue/MyPartitionFilteredTable/Bucket/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "MyPartitionFilteredTableBucket6ACAA137" + } + ], + "/aws-cdk-glue/MyPartitionFilteredTable/Table": [ + { + "type": "aws:cdk:logicalId", + "data": "MyPartitionFilteredTable324BA27A" + } + ], "/aws-cdk-glue/MyUser/Resource": [ { "type": "aws:cdk:logicalId", diff --git a/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/tree.json b/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/tree.json index 5ec24f621772d..189e9f749a2d6 100644 --- a/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/tree.json +++ b/packages/@aws-cdk/aws-glue/test/table.integ.snapshot/tree.json @@ -9,7 +9,7 @@ "path": "Tree", "constructInfo": { "fqn": "constructs.Construct", - "version": "10.0.9" + "version": "10.1.33" } }, "aws-cdk-glue": { @@ -596,6 +596,111 @@ "version": "0.0.0" } }, + "MyPartitionFilteredTable": { + "id": "MyPartitionFilteredTable", + "path": "aws-cdk-glue/MyPartitionFilteredTable", + "children": { + "Bucket": { + "id": "Bucket", + "path": "aws-cdk-glue/MyPartitionFilteredTable/Bucket", + "children": { + "Resource": { + "id": "Resource", + "path": "aws-cdk-glue/MyPartitionFilteredTable/Bucket/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::S3::Bucket", + "aws:cdk:cloudformation:props": {} + }, + "constructInfo": { + "fqn": "@aws-cdk/aws-s3.CfnBucket", + "version": "0.0.0" + } + } + }, + "constructInfo": { + "fqn": "@aws-cdk/aws-s3.Bucket", + "version": "0.0.0" + } + }, + "Table": { + "id": "Table", + "path": "aws-cdk-glue/MyPartitionFilteredTable/Table", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::Glue::Table", + "aws:cdk:cloudformation:props": { + "catalogId": { + "Ref": "AWS::AccountId" + }, + "databaseName": { + "Ref": "MyDatabase1E2517DB" + }, + "tableInput": { + "name": "partition_filtered_table", + "description": "partition_filtered_table generated by CDK", + "parameters": { + "classification": "json", + "has_encrypted_data": false, + "partition_filtering.enabled": true + }, + "storageDescriptor": { + "location": { + "Fn::Join": [ + "", + [ + "s3://", + { + "Ref": "MyPartitionFilteredTableBucket6ACAA137" + }, + "/" + ] + ] + }, + "compressed": false, + "storedAsSubDirectories": false, + "columns": [ + { + "name": "col1", + "type": "string" + }, + { + "name": "col2", + "type": "string", + "comment": "col2 comment" + }, + { + "name": "col3", + "type": "array" + }, + { + "name": "col4", + "type": "map" + }, + { + "name": "col5", + "type": "struct" + } + ], + "inputFormat": "org.apache.hadoop.mapred.TextInputFormat", + "outputFormat": "org.apache.hadoop.hive.ql.io.HiveIgnoreKeyTextOutputFormat", + "serdeInfo": { + "serializationLibrary": "org.openx.data.jsonserde.JsonSerDe" + } + }, + "tableType": "EXTERNAL_TABLE" + } + } + }, + "constructInfo": { + "fqn": "@aws-cdk/aws-glue.CfnTable", + "version": "0.0.0" + } + } + }, + "constructInfo": { + "fqn": "@aws-cdk/aws-glue.Table", + "version": "0.0.0" + } + }, "MyUser": { "id": "MyUser", "path": "aws-cdk-glue/MyUser", diff --git a/packages/@aws-cdk/aws-glue/test/table.test.ts b/packages/@aws-cdk/aws-glue/test/table.test.ts index e3f5df9bb6a3f..79920d73e1a81 100644 --- a/packages/@aws-cdk/aws-glue/test/table.test.ts +++ b/packages/@aws-cdk/aws-glue/test/table.test.ts @@ -1,4 +1,4 @@ -import { Template } from '@aws-cdk/assertions'; +import { Template, Match } from '@aws-cdk/assertions'; import * as iam from '@aws-cdk/aws-iam'; import * as kms from '@aws-cdk/aws-kms'; import * as s3 from '@aws-cdk/aws-s3'; @@ -1567,6 +1567,98 @@ test('Table.fromTableArn', () => { expect(table.tableName).toEqual('tbl1'); }); +test.each([ + ['enabled', true], + ['disabled', false], +])('Partition filtering on table %s', (_, enabled) => { + const app = new cdk.App(); + const dbStack = new cdk.Stack(app, 'db'); + const database = new glue.Database(dbStack, 'Database', { + databaseName: 'database', + }); + + const tableStack = new cdk.Stack(app, 'table'); + new glue.Table(tableStack, 'Table', { + database, + tableName: 'table', + columns: [{ + name: 'col', + type: glue.Schema.STRING, + }], + partitionKeys: [{ + name: 'year', + type: glue.Schema.SMALL_INT, + }], + dataFormat: glue.DataFormat.JSON, + enablePartitionFiltering: enabled, + }); + + Template.fromStack(tableStack).hasResourceProperties('AWS::Glue::Table', { + CatalogId: { + Ref: 'AWS::AccountId', + }, + DatabaseName: { + 'Fn::ImportValue': 'db:ExportsOutputRefDatabaseB269D8BB88F4B1C4', + }, + TableInput: { + Name: 'table', + Description: 'table generated by CDK', + Parameters: { + 'classification': 'json', + 'has_encrypted_data': false, + 'partition_filtering.enabled': enabled, + }, + PartitionKeys: Match.anyValue(), + StorageDescriptor: Match.anyValue(), + TableType: Match.anyValue(), + }, + }); +}); + +test('Partition filtering on table is not defined (default behavior)', () => { + const app = new cdk.App(); + const dbStack = new cdk.Stack(app, 'db'); + const database = new glue.Database(dbStack, 'Database', { + databaseName: 'database', + }); + + const tableStack = new cdk.Stack(app, 'table'); + new glue.Table(tableStack, 'Table', { + database, + tableName: 'table', + columns: [{ + name: 'col', + type: glue.Schema.STRING, + }], + partitionKeys: [{ + name: 'year', + type: glue.Schema.SMALL_INT, + }], + dataFormat: glue.DataFormat.JSON, + enablePartitionFiltering: undefined, + }); + + Template.fromStack(tableStack).hasResourceProperties('AWS::Glue::Table', { + CatalogId: { + Ref: 'AWS::AccountId', + }, + DatabaseName: { + 'Fn::ImportValue': 'db:ExportsOutputRefDatabaseB269D8BB88F4B1C4', + }, + TableInput: { + Name: 'table', + Description: 'table generated by CDK', + Parameters: { + classification: 'json', + has_encrypted_data: false, + }, + PartitionKeys: Match.anyValue(), + StorageDescriptor: Match.anyValue(), + TableType: Match.anyValue(), + }, + }); +}); + function createTable(props: Pick>): void { const stack = new cdk.Stack(); new glue.Table(stack, 'table', { From f71144fcb728e54b541289fabee98484e11ee11a Mon Sep 17 00:00:00 2001 From: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com> Date: Tue, 12 Jul 2022 08:51:36 -0400 Subject: [PATCH 2/7] chore: re-update F# template for new addSubscription signature (#21098) Our build currently breaks because the F# template assumes that addSubscription returns void, which we recently changed. #21038 tried to solve this, but the build failed with a different error. I ran this change through our test pipeline, which succeeded. That is the only reason why I am at all confident in this change. Mostly flying blind in F#... ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] 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* --- .../src/%name.PascalCased%/%name.PascalCased%Stack.template.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/init-templates/sample-app/fsharp/src/%name.PascalCased%/%name.PascalCased%Stack.template.fs b/packages/aws-cdk/lib/init-templates/sample-app/fsharp/src/%name.PascalCased%/%name.PascalCased%Stack.template.fs index 74b2c76e2573f..9ad4fac32567c 100644 --- a/packages/aws-cdk/lib/init-templates/sample-app/fsharp/src/%name.PascalCased%/%name.PascalCased%Stack.template.fs +++ b/packages/aws-cdk/lib/init-templates/sample-app/fsharp/src/%name.PascalCased%/%name.PascalCased%Stack.template.fs @@ -11,4 +11,4 @@ type %name.PascalCased%Stack(scope, id, props) as this = let queue = Queue(this, "%name.PascalCased%Queue", QueueProps(VisibilityTimeout = Duration.Seconds(300.))) let topic = Topic(this, "%name.PascalCased%Topic") - topic.AddSubscription(SqsSubscription(queue)) |> ignore + do topic.AddSubscription(SqsSubscription(queue)) |> ignore From b5e9c1a99be6898c544f91781ceb4ee1d371a03e Mon Sep 17 00:00:00 2001 From: Joshua Weber <57131123+daschaa@users.noreply.github.com> Date: Tue, 12 Jul 2022 15:27:51 +0200 Subject: [PATCH 3/7] feat(redshift): adds classic or elastic resize type option (#21084) Fixes #19430. Adds the property `classicResizing` property to the `ClusterProps`. If not set or set to false, elastic resizing is used. I feel like an entry in the README is not necessary. If it is ok, please add `pr-linter/exempt-readme` label to this PR. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] 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* --- packages/@aws-cdk/aws-redshift/lib/cluster.ts | 14 ++++ .../aws-redshift/test/cluster.test.ts | 67 +++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/packages/@aws-cdk/aws-redshift/lib/cluster.ts b/packages/@aws-cdk/aws-redshift/lib/cluster.ts index ab86540d3d7b3..3a61f3ad93afe 100644 --- a/packages/@aws-cdk/aws-redshift/lib/cluster.ts +++ b/packages/@aws-cdk/aws-redshift/lib/cluster.ts @@ -322,6 +322,19 @@ export interface ClusterProps { * @default false */ readonly publiclyAccessible?: boolean + + /** + * If this flag is set, the cluster resizing type will be set to classic. + * When resizing a cluster, classic resizing will always provision a new cluster and transfer the data there. + * + * Classic resize takes more time to complete, but it can be useful in cases where the change in node count or + * the node type to migrate to doesn't fall within the bounds for elastic resize. + * + * @see https://docs.aws.amazon.com/redshift/latest/mgmt/managing-cluster-operations.html#elastic-resize + * + * @default - Elastic resize type + */ + readonly classicResizing?: boolean } /** @@ -485,6 +498,7 @@ export class Cluster extends ClusterBase { // Encryption kmsKeyId: props.encryptionKey?.keyId, encrypted: props.encrypted ?? true, + classic: props.classicResizing, }); cluster.applyRemovalPolicy(removalPolicy, { diff --git a/packages/@aws-cdk/aws-redshift/test/cluster.test.ts b/packages/@aws-cdk/aws-redshift/test/cluster.test.ts index 6db5f18ade684..80a03cb7ac207 100644 --- a/packages/@aws-cdk/aws-redshift/test/cluster.test.ts +++ b/packages/@aws-cdk/aws-redshift/test/cluster.test.ts @@ -422,6 +422,73 @@ test('default child returns a CfnCluster', () => { expect(cluster.node.defaultChild).toBeInstanceOf(CfnCluster); }); +test.each([ + ['elastic', false], + ['classic', true], +])('resize type (%s)', (_, classicResizing) => { + // WHEN + new Cluster(stack, 'Redshift', { + masterUser: { + masterUsername: 'admin', + masterPassword: cdk.SecretValue.unsafePlainText('tooshort'), + }, + classicResizing, + vpc, + }); + + // THEN + Template.fromStack(stack).hasResource('AWS::Redshift::Cluster', { + Properties: { + AllowVersionUpgrade: true, + MasterUsername: 'admin', + MasterUserPassword: 'tooshort', + ClusterType: 'multi-node', + AutomatedSnapshotRetentionPeriod: 1, + Encrypted: true, + NumberOfNodes: 2, + NodeType: 'dc2.large', + DBName: 'default_db', + PubliclyAccessible: false, + ClusterSubnetGroupName: { Ref: 'RedshiftSubnetsDFE70E0A' }, + VpcSecurityGroupIds: [{ 'Fn::GetAtt': ['RedshiftSecurityGroup796D74A7', 'GroupId'] }], + Classic: classicResizing, + }, + DeletionPolicy: 'Retain', + UpdateReplacePolicy: 'Retain', + }); +}); + +test('resize type not set', () => { + // WHEN + new Cluster(stack, 'Redshift', { + masterUser: { + masterUsername: 'admin', + masterPassword: cdk.SecretValue.unsafePlainText('tooshort'), + }, + vpc, + }); + + // THEN + Template.fromStack(stack).hasResource('AWS::Redshift::Cluster', { + Properties: { + AllowVersionUpgrade: true, + MasterUsername: 'admin', + MasterUserPassword: 'tooshort', + ClusterType: 'multi-node', + AutomatedSnapshotRetentionPeriod: 1, + Encrypted: true, + NumberOfNodes: 2, + NodeType: 'dc2.large', + DBName: 'default_db', + PubliclyAccessible: false, + ClusterSubnetGroupName: { Ref: 'RedshiftSubnetsDFE70E0A' }, + VpcSecurityGroupIds: [{ 'Fn::GetAtt': ['RedshiftSecurityGroup796D74A7', 'GroupId'] }], + }, + DeletionPolicy: 'Retain', + UpdateReplacePolicy: 'Retain', + }); +}); + function testStack() { const newTestStack = new cdk.Stack(undefined, undefined, { env: { account: '12345', region: 'us-test-1' } }); newTestStack.node.setContext('availability-zones:12345:us-test-1', ['us-test-1a', 'us-test-1b']); From 4332e2341a48d5d182c3fa605e43a0d20b0640d0 Mon Sep 17 00:00:00 2001 From: watany <76135106+watany-dev@users.noreply.github.com> Date: Tue, 12 Jul 2022 23:07:08 +0900 Subject: [PATCH 4/7] typo:Lambda Role Principal was a different service (#21106) ---- ### New Features typo: lambda doc. This is a minor fix, but it will behave incorrectly if deployed incorrectly. *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/aws-lambda/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-lambda/README.md b/packages/@aws-cdk/aws-lambda/README.md index 1db33fbd80631..dc40e77e1363d 100644 --- a/packages/@aws-cdk/aws-lambda/README.md +++ b/packages/@aws-cdk/aws-lambda/README.md @@ -105,7 +105,7 @@ it appropriate permissions: ```ts const myRole = new iam.Role(this, 'My Role', { - assumedBy: new iam.ServicePrincipal('sns.amazonaws.com'), + assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'), }); const fn = new lambda.Function(this, 'MyFunction', { From c9ed958a6e67bc9f2f59d8d4330e2ceade757962 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com> Date: Tue, 12 Jul 2022 11:59:22 -0400 Subject: [PATCH 5/7] chore(mergify): update current members of contribution/core (#21094) * chore: update current members of contribution/core * Update .mergify.yml --- .mergify.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.mergify.yml b/.mergify.yml index 487115e0aee56..c75f34e359160 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -10,7 +10,7 @@ pull_request_rules: label: add: [ contribution/core ] conditions: - - author~=^(RomainMuller|garnaat|skinny85|rix0rrr|NGL321|Jerry-AWS|MrArnoldPalmer|iliapolo|pkandasamy91|SoManyHs|uttarasridhar|otaviomacedo|madeline-k|kaizencc|comcalvi|Chriscbr|corymhall|peterwoodworth|ryparker|TheRealAmazonKendra|yuth|vinayak-kukreja)$ + - author~=^(RomainMuller|rix0rrr|Jerry-AWS|MrArnoldPalmer|iliapolo|uttarasridhar|otaviomacedo|madeline-k|kaizencc|comcalvi|corymhall|peterwoodworth|ryparker|TheRealAmazonKendra|yuth|vinayak-kukreja|Naumel|mrgrain)$ - -label~="contribution/core" - name: automatic merge actions: From 6e9963c38e091b37a097f176eae2854ab907ae40 Mon Sep 17 00:00:00 2001 From: Calvin Combs <66279577+comcalvi@users.noreply.github.com> Date: Tue, 12 Jul 2022 10:36:35 -0600 Subject: [PATCH 6/7] fix(aws-eks): cap generated stack names at 128 characters (#20528) When imported, Kubectl Providers result in a new nested stack being created. Nested stack names are created by generating a unique ID (which includes the entire construct id path). Kubectl provider ids are constructed the same way (they can't be specified manually, so the code uses a unique ID). This results in the final ID for the stack containing the id of the provider repeated twice. The makes the name hard to decipher, and more importantly can result in stack names being longer than 128 characters. This caps all generated stack names at 128 characters. Ideally, we would be able to change way in which these stack names are generated; however, that would be a breaking change. Fixes #20124. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../@aws-cdk/aws-eks/test/cluster.test.ts | 106 +----------------- packages/@aws-cdk/core/lib/stack.ts | 3 +- packages/@aws-cdk/core/test/stack.test.ts | 11 ++ 3 files changed, 15 insertions(+), 105 deletions(-) diff --git a/packages/@aws-cdk/aws-eks/test/cluster.test.ts b/packages/@aws-cdk/aws-eks/test/cluster.test.ts index bd670b6cc9d61..1f6ba84fc20fa 100644 --- a/packages/@aws-cdk/aws-eks/test/cluster.test.ts +++ b/packages/@aws-cdk/aws-eks/test/cluster.test.ts @@ -91,8 +91,6 @@ describe('cluster', () => { vpcSubnets: [{ subnetType: ec2.SubnetType.PUBLIC }], }); }).toThrow(/Cannot place cluster handler in the VPC since no private subnets could be selected/); - - }); test('throws when provided `clusterHandlerSecurityGroup` without `placeClusterHandlerInVpc: true`', () => { @@ -129,8 +127,6 @@ describe('cluster', () => { privateSubnetIds, isolatedSubnetIds, }); - - }); test('throws if selecting more than one subnet group', () => { @@ -140,8 +136,6 @@ describe('cluster', () => { defaultCapacity: 0, version: eks.KubernetesVersion.V1_21, })).toThrow(/cannot select multiple subnet groups/); - - }); test('synthesis works if only one subnet group is selected', () => { @@ -221,7 +215,6 @@ describe('cluster', () => { const clusterSg = cluster.clusterSecurityGroup; expect(clusterSg.securityGroupId).toEqual(clusterSgId); - }); test('cluster security group is attached when adding self-managed nodes', () => { @@ -351,7 +344,6 @@ describe('cluster', () => { cluster.connectAutoScalingGroupCapacity(selfManaged, { spotInterruptHandler: false }); expect(cluster.node.findAll().filter(c => c.node.id === 'chart-spot-interrupt-handler').length).toEqual(0); - }); test('throws when a non cdk8s chart construct is added as cdk8s chart', () => { @@ -439,8 +431,6 @@ describe('cluster', () => { { 'Fn::GetAtt': ['Cluster9EE0221C', 'ClusterSecurityGroupId'] }, { 'Fn::GetAtt': ['ClusterControlPlaneSecurityGroupD274242C', 'GroupId'] }, ]); - - }); test('can declare a security group from a different stack', () => { @@ -528,8 +518,6 @@ describe('cluster', () => { // make sure we can synth (no circular dependencies between the stacks) app.synth(); - - }); test('can declare a chart with a token from a different stack than the cluster that depends on the cluster stack', () => { @@ -571,8 +559,6 @@ describe('cluster', () => { // make sure we can synth (no circular dependencies between the stacks) app.synth(); - - }); test('can declare a HelmChart in a different stack than the cluster', () => { @@ -605,8 +591,6 @@ describe('cluster', () => { // make sure we can synth (no circular dependencies between the stacks) app.synth(); - - }); test('throws when declaring an ASG role in a different stack than the cluster', () => { @@ -651,8 +635,6 @@ describe('cluster', () => { }).toThrow( 'CapacityStack/autoScaling/InstanceRole should be defined in the scope of the ClusterStack stack to prevent circular dependencies', ); - - }); test('can declare a ServiceAccount in a different stack than the cluster', () => { @@ -683,8 +665,6 @@ describe('cluster', () => { // make sure we can synth (no circular dependencies between the stacks) app.synth(); - - }); test('a default cluster spans all subnets', () => { @@ -741,7 +721,6 @@ describe('cluster', () => { // THEN Template.fromStack(stack).hasResourceProperties('AWS::EC2::VPC', Match.anyValue()); - }); describe('default capacity', () => { @@ -765,7 +744,6 @@ describe('cluster', () => { MinSize: 2, }, }); - }); test('quantity and type can be customized', () => { @@ -790,7 +768,6 @@ describe('cluster', () => { }, }); // expect(stack).toHaveResource('AWS::AutoScaling::LaunchConfiguration', { InstanceType: 'm2.xlarge' })); - }); test('defaultCapacity=0 will not allocate at all', () => { @@ -804,7 +781,6 @@ describe('cluster', () => { expect(cluster.defaultCapacity).toBeUndefined(); Template.fromStack(stack).resourceCountIs('AWS::AutoScaling::AutoScalingGroup', 0); Template.fromStack(stack).resourceCountIs('AWS::AutoScaling::LaunchConfiguration', 0); - }); }); @@ -824,8 +800,6 @@ describe('cluster', () => { { Key: 'Name', Value: 'Stack/VPC/PrivateSubnet1' }, ], }); - - }); test('creating a cluster tags the public VPC subnets', () => { @@ -845,8 +819,6 @@ describe('cluster', () => { { Key: 'Name', Value: 'Stack/VPC/PublicSubnet1' }, ], }); - - }); test('adding capacity creates an ASG without a rolling update policy', () => { @@ -899,8 +871,6 @@ describe('cluster', () => { }, ], }); - - }); test('create nodegroup with existing role', () => { @@ -933,7 +903,6 @@ describe('cluster', () => { MinSize: 10, }, }); - }); test('adding bottlerocket capacity creates an ASG with tags', () => { @@ -967,7 +936,6 @@ describe('cluster', () => { }, ], }); - }); test('adding bottlerocket capacity with bootstrapOptions throws error', () => { @@ -985,7 +953,6 @@ describe('cluster', () => { machineImageType: eks.MachineImageType.BOTTLEROCKET, bootstrapOptions: {}, })).toThrow(/bootstrapOptions is not supported for Bottlerocket/); - }); test('import cluster with existing kubectl provider function', () => { @@ -1133,8 +1100,6 @@ describe('cluster', () => { 'KubectlSubnet0', 'KubectlSubnet1', ]); - - }); test('exercise export/import', () => { @@ -1188,7 +1153,6 @@ describe('cluster', () => { }, }, }); - }); test('mastersRole can be used to map an IAM role to "system:masters"', () => { @@ -1230,8 +1194,6 @@ describe('cluster', () => { ], }, }); - - }); test('addManifest can be used to apply k8s manifests on this cluster', () => { @@ -1256,8 +1218,6 @@ describe('cluster', () => { Template.fromStack(stack).hasResourceProperties(eks.KubernetesManifest.RESOURCE_TYPE, { Manifest: '[{"bar":123},{"boor":[1,2,3]}]', }); - - }); test('kubectl resources can be created in a separate stack', () => { @@ -1293,8 +1253,6 @@ describe('cluster', () => { }, }, }); - - }); test('adding capacity will automatically map its IAM role', () => { @@ -1344,8 +1302,6 @@ describe('cluster', () => { ], }, }); - - }); test('addAutoScalingGroupCapacity will *not* map the IAM role if mapRole is false', () => { @@ -1389,7 +1345,6 @@ describe('cluster', () => { ], }, }); - }); describe('outputs', () => { @@ -1407,7 +1362,6 @@ describe('cluster', () => { ClusterConfigCommand43AAE40F: { Value: { 'Fn::Join': ['', ['aws eks update-kubeconfig --name ', { Ref: 'Cluster9EE0221C' }, ' --region us-east-1 --role-arn ', { 'Fn::GetAtt': ['ClusterMastersRole9AA35625', 'Arn'] }]] } }, ClusterGetTokenCommand06AE992E: { Value: { 'Fn::Join': ['', ['aws eks get-token --cluster-name ', { Ref: 'Cluster9EE0221C' }, ' --region us-east-1 --role-arn ', { 'Fn::GetAtt': ['ClusterMastersRole9AA35625', 'Arn'] }]] } }, }); - }); test('if masters role is defined, it should be included in the config command', () => { @@ -1429,7 +1383,6 @@ describe('cluster', () => { ClusterConfigCommand43AAE40F: { Value: { 'Fn::Join': ['', ['aws eks update-kubeconfig --name ', { Ref: 'Cluster9EE0221C' }, ' --region us-east-1 --role-arn ', { 'Fn::GetAtt': ['masters0D04F23D', 'Arn'] }]] } }, ClusterGetTokenCommand06AE992E: { Value: { 'Fn::Join': ['', ['aws eks get-token --cluster-name ', { Ref: 'Cluster9EE0221C' }, ' --region us-east-1 --role-arn ', { 'Fn::GetAtt': ['masters0D04F23D', 'Arn'] }]] } }, }); - }); test('if `outputConfigCommand=false` will disabled the output', () => { @@ -1449,7 +1402,6 @@ describe('cluster', () => { const assembly = app.synth(); const template = assembly.getStackByName(stack.stackName).template; expect(template.Outputs).toBeUndefined(); // no outputs - }); test('`outputClusterName` can be used to synthesize an output with the cluster name', () => { @@ -1470,7 +1422,6 @@ describe('cluster', () => { expect(template.Outputs).toEqual({ ClusterClusterNameEB26049E: { Value: { Ref: 'Cluster9EE0221C' } }, }); - }); test('`outputMastersRoleArn` can be used to synthesize an output with the arn of the masters role if defined', () => { @@ -1492,7 +1443,6 @@ describe('cluster', () => { expect(template.Outputs).toEqual({ ClusterMastersRoleArnB15964B1: { Value: { 'Fn::GetAtt': ['masters0D04F23D', 'Arn'] } }, }); - }); describe('boostrap user-data', () => { @@ -1509,7 +1459,6 @@ describe('cluster', () => { const template = app.synth().getStackByName(stack.stackName).template; const userData = template.Resources.ClusterMyCapcityLaunchConfig58583345.Properties.UserData; expect(userData).toEqual({ 'Fn::Base64': { 'Fn::Join': ['', ['#!/bin/bash\nset -o xtrace\n/etc/eks/bootstrap.sh ', { Ref: 'Cluster9EE0221C' }, ' --kubelet-extra-args "--node-labels lifecycle=OnDemand" --apiserver-endpoint \'', { 'Fn::GetAtt': ['Cluster9EE0221C', 'Endpoint'] }, '\' --b64-cluster-ca \'', { 'Fn::GetAtt': ['Cluster9EE0221C', 'CertificateAuthorityData'] }, '\' --use-max-pods true\n/opt/aws/bin/cfn-signal --exit-code $? --stack Stack --resource ClusterMyCapcityASGD4CD8B97 --region us-east-1']] } }); - }); test('not rendered if bootstrap is disabled', () => { @@ -1527,7 +1476,6 @@ describe('cluster', () => { const template = app.synth().getStackByName(stack.stackName).template; const userData = template.Resources.ClusterMyCapcityLaunchConfig58583345.Properties.UserData; expect(userData).toEqual({ 'Fn::Base64': '#!/bin/bash' }); - }); // cursory test for options: see test.user-data.ts for full suite @@ -1548,7 +1496,6 @@ describe('cluster', () => { const template = app.synth().getStackByName(stack.stackName).template; const userData = template.Resources.ClusterMyCapcityLaunchConfig58583345.Properties.UserData; expect(userData).toEqual({ 'Fn::Base64': { 'Fn::Join': ['', ['#!/bin/bash\nset -o xtrace\n/etc/eks/bootstrap.sh ', { Ref: 'Cluster9EE0221C' }, ' --kubelet-extra-args "--node-labels lifecycle=OnDemand --node-labels FOO=42" --apiserver-endpoint \'', { 'Fn::GetAtt': ['Cluster9EE0221C', 'Endpoint'] }, '\' --b64-cluster-ca \'', { 'Fn::GetAtt': ['Cluster9EE0221C', 'CertificateAuthorityData'] }, '\' --use-max-pods true\n/opt/aws/bin/cfn-signal --exit-code $? --stack Stack --resource ClusterMyCapcityASGD4CD8B97 --region us-east-1']] } }); - }); describe('spot instances', () => { @@ -1568,7 +1515,6 @@ describe('cluster', () => { const template = app.synth().getStackByName(stack.stackName).template; const userData = template.Resources.ClusterMyCapcityLaunchConfig58583345.Properties.UserData; expect(userData).toEqual({ 'Fn::Base64': { 'Fn::Join': ['', ['#!/bin/bash\nset -o xtrace\n/etc/eks/bootstrap.sh ', { Ref: 'Cluster9EE0221C' }, ' --kubelet-extra-args "--node-labels lifecycle=Ec2Spot --register-with-taints=spotInstance=true:PreferNoSchedule" --apiserver-endpoint \'', { 'Fn::GetAtt': ['Cluster9EE0221C', 'Endpoint'] }, '\' --b64-cluster-ca \'', { 'Fn::GetAtt': ['Cluster9EE0221C', 'CertificateAuthorityData'] }, '\' --use-max-pods true\n/opt/aws/bin/cfn-signal --exit-code $? --stack Stack --resource ClusterMyCapcityASGD4CD8B97 --region us-east-1']] } }); - }); test('interrupt handler is added', () => { @@ -1590,7 +1536,6 @@ describe('cluster', () => { Namespace: 'kube-system', Repository: 'https://aws.github.io/eks-charts', }); - }); test('interrupt handler is not added when spotInterruptHandler is false', () => { @@ -1607,7 +1552,6 @@ describe('cluster', () => { // THEN expect(cluster.node.findAll().filter(c => c.node.id === 'chart-spot-interrupt-handler').length).toEqual(0); - }); test('its possible to add two capacities with spot instances and only one stop handler will be installed', () => { @@ -1628,11 +1572,8 @@ describe('cluster', () => { // THEN Template.fromStack(stack).resourceCountIs(eks.HelmChart.RESOURCE_TYPE, 1); - }); - }); - }); test('if bootstrap is disabled cannot specify options', () => { @@ -1646,7 +1587,6 @@ describe('cluster', () => { bootstrapEnabled: false, bootstrapOptions: { awsApiRetryAttempts: 10 }, })).toThrow(/Cannot specify "bootstrapOptions" if "bootstrapEnabled" is false/); - }); test('EksOptimizedImage() with no nodeType always uses STANDARD with LATEST_KUBERNETES_VERSION', () => { @@ -1668,7 +1608,6 @@ describe('cluster', () => { ([k, v]) => k.startsWith('SsmParameterValueawsserviceeksoptimizedami') && (v as any).Default.includes(LATEST_KUBERNETES_VERSION), )).toEqual(true); - }); test('EksOptimizedImage() with specific kubernetesVersion return correct AMI', () => { @@ -1689,7 +1628,6 @@ describe('cluster', () => { ([k, v]) => k.startsWith('SsmParameterValueawsserviceeksoptimizedami') && (v as any).Default.includes('/1.21/'), )).toEqual(true); - }); test('default cluster capacity with ARM64 instance type comes with nodegroup with correct AmiType', () => { @@ -1708,7 +1646,6 @@ describe('cluster', () => { Template.fromStack(stack).hasResourceProperties('AWS::EKS::Nodegroup', { AmiType: 'AL2_ARM_64', }); - }); test('addNodegroup with ARM64 instance type comes with nodegroup with correct AmiType', () => { @@ -1729,7 +1666,6 @@ describe('cluster', () => { Template.fromStack(stack).hasResourceProperties('AWS::EKS::Nodegroup', { AmiType: 'AL2_ARM_64', }); - }); test('addNodegroupCapacity with T4g instance type comes with nodegroup with correct AmiType', () => { @@ -1750,7 +1686,6 @@ describe('cluster', () => { Template.fromStack(stack).hasResourceProperties('AWS::EKS::Nodegroup', { AmiType: 'AL2_ARM_64', }); - }); test('addAutoScalingGroupCapacity with T4g instance type comes with nodegroup with correct AmiType', () => { @@ -1773,7 +1708,6 @@ describe('cluster', () => { ([k, v]) => k.startsWith('SsmParameterValueawsserviceeksoptimizedami') && (v as any).Default.includes('amazon-linux-2-arm64/'), )).toEqual(true); - }); test('addNodegroupCapacity with C7g instance type comes with nodegroup with correct AmiType', () => { @@ -1839,7 +1773,6 @@ describe('cluster', () => { expect(Object.entries(parameters).some( ([k, v]) => k.startsWith('SsmParameterValueawsserviceeksoptimizedami') && (v as any).Default.includes('amazon-linux-2-gpu'), )).toEqual(true); - }); test('EKS-Optimized AMI with ARM64 when addAutoScalingGroupCapacity', () => { @@ -1861,7 +1794,6 @@ describe('cluster', () => { expect(Object.entries(parameters).some( ([k, v]) => k.startsWith('SsmParameterValueawsserviceeksoptimizedami') && (v as any).Default.includes('/amazon-linux-2-arm64/'), )).toEqual(true); - }); test('BottleRocketImage() with specific kubernetesVersion return correct AMI', () => { @@ -1882,7 +1814,6 @@ describe('cluster', () => { ([k, v]) => k.startsWith('SsmParameterValueawsservicebottlerocketaws') && (v as any).Default.includes('/aws-k8s-1.21/'), )).toEqual(true); - }); test('when using custom resource a creation role & policy is defined', () => { @@ -2048,7 +1979,6 @@ describe('cluster', () => { Version: '2012-10-17', }, }); - }); test('if an explicit cluster name is not provided, the creation role policy is wider (allows interacting with all clusters)', () => { @@ -2122,7 +2052,6 @@ describe('cluster', () => { Version: '2012-10-17', }, }); - }); test('if helm charts are used, the provider role is allowed to assume the creation role', () => { @@ -2234,8 +2163,8 @@ describe('cluster', () => { ], }, }); - }); + test('if openIDConnectProvider a new OpenIDConnectProvider resource is created and exposed', () => { // GIVEN const { stack } = testFixtureNoVpc(); @@ -2266,7 +2195,6 @@ describe('cluster', () => { ], }, }); - }); test('inference instances are supported', () => { // GIVEN @@ -2285,7 +2213,6 @@ describe('cluster', () => { Template.fromStack(stack).hasResourceProperties(eks.KubernetesManifest.RESOURCE_TYPE, { Manifest: JSON.stringify([sanitized]), }); - }); test('kubectl resources are always created after all fargate profiles', () => { @@ -2332,8 +2259,6 @@ describe('cluster', () => { for (const r of kubectlResources) { expect(template.Resources[r].DependsOn).toEqual(['ClusterKubectlReadyBarrier200052AF']); } - - }); test('kubectl provider role can assume creation role', () => { @@ -2464,7 +2389,6 @@ describe('cluster', () => { }, }, }); - }); describe('kubectl provider passes iam role environment to kube ctl lambda', ()=>{ @@ -2502,8 +2426,8 @@ describe('cluster', () => { Ref: 'referencetoStackKubectlIamRole02F8947EArn', }, }); - }); + test('imported cluster', ()=> { const clusterName = 'my-cluster'; @@ -2541,12 +2465,9 @@ describe('cluster', () => { describe('endpoint access', () => { test('public restricted', () => { - expect(() => { eks.EndpointAccess.PUBLIC.onlyFrom('1.2.3.4/32'); }).toThrow(/Cannot restric public access to endpoint when private access is disabled. Use PUBLIC_AND_PRIVATE.onlyFrom\(\) instead./); - - }); test('public non restricted without private subnets', () => { @@ -2566,7 +2487,6 @@ describe('cluster', () => { Template.fromStack(nested).hasResourceProperties('AWS::Lambda::Function', { VpcConfig: Match.absent(), }); - }); test('public non restricted with private subnets', () => { @@ -2599,8 +2519,6 @@ describe('cluster', () => { vpcSubnets: [{ subnetType: ec2.SubnetType.PUBLIC }], }); }).toThrow(/Vpc must contain private subnets when public endpoint access is disabled/); - - }); test('private with private subnets', () => { @@ -2637,7 +2555,6 @@ describe('cluster', () => { Template.fromStack(nested).hasResourceProperties('AWS::Lambda::Function', { VpcConfig: Match.absent(), }); - }); test('private and non restricted public with private subnets', () => { @@ -2668,8 +2585,6 @@ describe('cluster', () => { vpcSubnets: [{ subnetType: ec2.SubnetType.PUBLIC }], }); }).toThrow(/Vpc must contain private subnets when public endpoint access is restricted/); - - }); test('private and restricted public with private subnets', () => { @@ -2687,7 +2602,6 @@ describe('cluster', () => { const functions = Template.fromStack(nested).findResources('AWS::Lambda::Function'); expect(functions.Handler886CB40B.Properties.VpcConfig.SubnetIds.length).not.toEqual(0); expect(functions.Handler886CB40B.Properties.VpcConfig.SecurityGroupIds.length).not.toEqual(0); - }); test('private endpoint access selects only private subnets from looked up vpc', () => { @@ -2873,8 +2787,6 @@ describe('cluster', () => { const template = app.synth().getStackArtifact(stack.stackName).template; expect(template.Resources.Cluster1B02DD5A2.Properties.Config.resourcesVpcConfig.endpointPrivateAccess).toEqual(true); expect(template.Resources.Cluster1B02DD5A2.Properties.Config.resourcesVpcConfig.endpointPublicAccess).toEqual(false); - - }); test('kubectl provider chooses only private subnets', () => { @@ -2933,8 +2845,6 @@ describe('cluster', () => { ], }, }); - - }); test('kubectl provider limits number of subnets to 16', () => { @@ -3055,8 +2965,6 @@ describe('cluster', () => { ], }, }); - - }); test('throw when private access is configured without dns support enabled for the VPC', () => { @@ -3072,7 +2980,6 @@ describe('cluster', () => { prune: false, }); }).toThrow(/Private endpoint access requires the VPC to have DNS support and DNS hostnames enabled/); - }); test('throw when private access is configured without dns hostnames enabled for the VPC', () => { @@ -3088,7 +2995,6 @@ describe('cluster', () => { prune: false, }); }).toThrow(/Private endpoint access requires the VPC to have DNS support and DNS hostnames enabled/); - }); test('throw when cidrs are configured without public access endpoint', () => { @@ -3096,9 +3002,7 @@ describe('cluster', () => { expect(() => { eks.EndpointAccess.PRIVATE.onlyFrom('1.2.3.4/5'); }).toThrow(/CIDR blocks can only be configured when public access is enabled/); - }); - }); test('getServiceLoadBalancerAddress', () => { @@ -3164,8 +3068,6 @@ describe('cluster', () => { Template.fromStack(providerStack).hasResourceProperties('AWS::Lambda::Function', { Layers: ['arn:of:layer'], }); - - }); test('create a cluster using custom resource with secrets encryption using KMS CMK', () => { @@ -3196,7 +3098,6 @@ describe('cluster', () => { }], }, }); - }); test('custom memory size for kubectl provider', () => { @@ -3214,7 +3115,6 @@ describe('cluster', () => { const casm = app.synth(); const providerNestedStackTemplate = JSON.parse(fs.readFileSync(path.join(casm.directory, 'StackawscdkawseksKubectlProvider7346F799.nested.template.json'), 'utf-8')); expect(providerNestedStackTemplate?.Resources?.Handler886CB40B?.Properties?.MemorySize).toEqual(2048); - }); test('custom memory size for imported clusters', () => { @@ -3234,7 +3134,6 @@ describe('cluster', () => { const casm = app.synth(); const providerNestedStackTemplate = JSON.parse(fs.readFileSync(path.join(casm.directory, 'StackStackImported1CBA9C50KubectlProviderAA00BA49.nested.template.json'), 'utf-8')); expect(providerNestedStackTemplate?.Resources?.Handler886CB40B?.Properties?.MemorySize).toEqual(4096); - }); test('create a cluster using custom kubernetes network config', () => { @@ -3256,6 +3155,5 @@ describe('cluster', () => { }, }, }); - }); }); diff --git a/packages/@aws-cdk/core/lib/stack.ts b/packages/@aws-cdk/core/lib/stack.ts index 65298001d5876..23097a90914f2 100644 --- a/packages/@aws-cdk/core/lib/stack.ts +++ b/packages/@aws-cdk/core/lib/stack.ts @@ -1315,7 +1315,7 @@ export function rootPathTo(construct: IConstruct, ancestor?: IConstruct): IConst */ function makeStackName(components: string[]) { if (components.length === 1) { return components[0]; } - return makeUniqueId(components); + return makeUniqueResourceName(components, { maxLength: 128 }); } function getCreateExportsScope(stack: Stack) { @@ -1388,4 +1388,5 @@ import { Token, Tokenization } from './token'; import { referenceNestedStackValueInParent } from './private/refs'; import { Fact, RegionInfo } from '@aws-cdk/region-info'; import { deployTimeLookup } from './private/region-lookup'; +import { makeUniqueResourceName } from './private/unique-resource-name'; diff --git a/packages/@aws-cdk/core/test/stack.test.ts b/packages/@aws-cdk/core/test/stack.test.ts index 48ae7d185d97f..35630f888f8d1 100644 --- a/packages/@aws-cdk/core/test/stack.test.ts +++ b/packages/@aws-cdk/core/test/stack.test.ts @@ -919,6 +919,17 @@ describe('stack', () => { expect(stack.stackName).toEqual('ProdStackD5279B22'); }); + test('generated stack names will not exceed 128 characters', () => { + // WHEN + const root = new App(); + const app = new Construct(root, 'ProdAppThisNameButItWillOnlyBeTooLongWhenCombinedWithTheStackName' + 'z'.repeat(60)); + const stack = new Stack(app, 'ThisNameIsVeryLongButItWillOnlyBeTooLongWhenCombinedWithTheAppNameStack'); + + // THEN + expect(stack.stackName.length).toEqual(128); + expect(stack.stackName).toEqual('ProdAppThisNameButItWillOnlyBeTooLongWhenCombinedWithTheStaceryLongButItWillOnlyBeTooLongWhenCombinedWithTheAppNameStack864CC1D3'); + }); + test('stack construct id does not go through stack name validation if there is an explicit stack name', () => { // GIVEN const app = new App(); From cad6fc5f0f6d963152ede3101d36d085e399f99a Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Tue, 12 Jul 2022 18:11:23 +0100 Subject: [PATCH 7/7] fix(cli): `--no-fail` flag is ignored in favor of the `enableDiffNoFail` feature flag (#21107) The CLI doesn't distinguish between `cdk diff --no-fail` and just `cdk diff`. In both cases, it's taking the value of `enableDiffNoFail` feature flag to decide which status code to return. Change the behavior to: | Is there a difference? | `--fail` (CLI option) | `enableDiffNoFail` (FF) | Return code | |------------------------|------------------------|-------------------------|-------------| | No | Don't care | Don't care | 0 | | Yes | false | Don't care | 0 | | Yes | true | Don't care | 1 | | Yes | null | false | 1 | | Yes | null | true | 0 | Fixes https://github.com/aws/aws-cdk/issues/19110. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] 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* --- packages/@aws-cdk/cx-api/lib/features.ts | 11 +++++- packages/aws-cdk/lib/cli.ts | 4 +- .../aws-cdk/test/api/deploy-stack.test.ts | 2 +- .../aws-cdk/test/integ/cli/cli.integtest.ts | 37 +++++++++++++++++++ 4 files changed, 49 insertions(+), 5 deletions(-) diff --git a/packages/@aws-cdk/cx-api/lib/features.ts b/packages/@aws-cdk/cx-api/lib/features.ts index 7790d3e49e679..06e4b8e66e87a 100644 --- a/packages/@aws-cdk/cx-api/lib/features.ts +++ b/packages/@aws-cdk/cx-api/lib/features.ts @@ -31,9 +31,16 @@ export const ENABLE_STACK_NAME_DUPLICATES_CONTEXT = '@aws-cdk/core:enableStackNameDuplicates'; /** - * IF this is set, `cdk diff` will always exit with 0. + * Determines what status code `cdk diff` should return when the specified stack + * differs from the deployed stack or the local CloudFormation template: * - * Use `cdk diff --fail` to exit with 1 if there's a diff. + * * aws-cdk:enableDiffNoFail=true => status code == 0 + * * aws-cdk:enableDiffNoFail=false => status code == 1 + * + * You can override this behavior with the --fail flag: + * + * * --fail => status code == 1 + * * --no-fail => status code == 0 */ export const ENABLE_DIFF_NO_FAIL_CONTEXT = 'aws-cdk:enableDiffNoFail'; /** @deprecated use `ENABLE_DIFF_NO_FAIL_CONTEXT` */ diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index 4533125708409..c037a24ba9d33 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -227,7 +227,7 @@ async function parseCommandLineArguments() { .option('template', { type: 'string', desc: 'The path to the CloudFormation template to compare with', requiresArg: true }) .option('strict', { type: 'boolean', desc: 'Do not filter out AWS::CDK::Metadata resources', default: false }) .option('security-only', { type: 'boolean', desc: 'Only diff for broadened security changes', default: false }) - .option('fail', { type: 'boolean', desc: 'Fail with exit code 1 in case of diff', default: false })) + .option('fail', { type: 'boolean', desc: 'Fail with exit code 1 in case of diff' })) .command('metadata [STACK]', 'Returns all metadata associated with this stack') .command(['acknowledge [ID]', 'ack [ID]'], 'Acknowledge a notice so that it does not show up anymore') .command('notices', 'Returns a list of relevant notices') @@ -416,7 +416,7 @@ async function initCommandLine() { strict: args.strict, contextLines: args.contextLines, securityOnly: args.securityOnly, - fail: args.fail || !enableDiffNoFail, + fail: args.fail != null ? args.fail : !enableDiffNoFail, stream: args.ci ? process.stdout : undefined, }); diff --git a/packages/aws-cdk/test/api/deploy-stack.test.ts b/packages/aws-cdk/test/api/deploy-stack.test.ts index 77f6c80755ac2..a8642e3d598a4 100644 --- a/packages/aws-cdk/test/api/deploy-stack.test.ts +++ b/packages/aws-cdk/test/api/deploy-stack.test.ts @@ -1,8 +1,8 @@ import { deployStack, DeployStackOptions, ToolkitInfo } from '../../lib/api'; import { tryHotswapDeployment } from '../../lib/api/hotswap-deployments'; +import { setCI } from '../../lib/logging'; import { DEFAULT_FAKE_TEMPLATE, testStack } from '../util'; import { MockedObject, mockResolvedEnvironment, MockSdk, MockSdkProvider, SyncHandlerSubsetOf } from '../util/mock-sdk'; -import { setCI } from '../../lib/logging'; jest.mock('../../lib/api/hotswap-deployments'); diff --git a/packages/aws-cdk/test/integ/cli/cli.integtest.ts b/packages/aws-cdk/test/integ/cli/cli.integtest.ts index 4b9d276eb3496..c2799525a452c 100644 --- a/packages/aws-cdk/test/integ/cli/cli.integtest.ts +++ b/packages/aws-cdk/test/integ/cli/cli.integtest.ts @@ -518,6 +518,43 @@ integTest('cdk diff', withDefaultFixture(async (fixture) => { .rejects.toThrow('exited with error'); })); +integTest('enableDiffNoFail', withDefaultFixture(async (fixture) => { + await diffShouldSucceedWith({ fail: false, enableDiffNoFail: false }); + await diffShouldSucceedWith({ fail: false, enableDiffNoFail: true }); + await diffShouldFailWith({ fail: true, enableDiffNoFail: false }); + await diffShouldFailWith({ fail: true, enableDiffNoFail: true }); + await diffShouldFailWith({ fail: undefined, enableDiffNoFail: false }); + await diffShouldSucceedWith({ fail: undefined, enableDiffNoFail: true }); + + async function diffShouldSucceedWith(props: DiffParameters) { + await expect(diff(props)).resolves.not.toThrowError(); + } + + async function diffShouldFailWith(props: DiffParameters) { + await expect(diff(props)).rejects.toThrow('exited with error'); + } + + async function diff(props: DiffParameters): Promise { + await updateContext(props.enableDiffNoFail); + const flag = props.fail != null + ? (props.fail ? '--fail' : '--no-fail') + : ''; + + return fixture.cdk(['diff', flag, fixture.fullStackName('test-1')]); + } + + async function updateContext(enableDiffNoFail: boolean) { + const cdkJson = JSON.parse(await fs.readFile(path.join(fixture.integTestDir, 'cdk.json'), 'utf8')); + cdkJson.context = { + ...cdkJson.context, + 'aws-cdk:enableDiffNoFail': enableDiffNoFail, + }; + await fs.writeFile(path.join(fixture.integTestDir, 'cdk.json'), JSON.stringify(cdkJson)); + } + + type DiffParameters = { fail?: boolean, enableDiffNoFail: boolean }; +})); + integTest('cdk diff --fail on multiple stacks exits with error if any of the stacks contains a diff', withDefaultFixture(async (fixture) => { // GIVEN const diff1 = await fixture.cdk(['diff', fixture.fullStackName('test-1')]);