From 942285adf9ed9288543dbad5a20ec1a2d6b48897 Mon Sep 17 00:00:00 2001 From: Shiv Lakshminarayan Date: Thu, 30 Apr 2020 02:13:41 -0700 Subject: [PATCH 1/5] chore(dynamodb): clear linter exclusions (#7702) Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- packages/@aws-cdk/aws-dynamodb/lib/table.ts | 31 +++++++++++++++++++++ packages/@aws-cdk/aws-dynamodb/package.json | 15 ---------- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/packages/@aws-cdk/aws-dynamodb/lib/table.ts b/packages/@aws-cdk/aws-dynamodb/lib/table.ts index 98fbf49c04bcc..0888dfe57f4bb 100644 --- a/packages/@aws-cdk/aws-dynamodb/lib/table.ts +++ b/packages/@aws-cdk/aws-dynamodb/lib/table.ts @@ -36,6 +36,10 @@ const WRITE_DATA_ACTIONS = [ 'dynamodb:DeleteItem', ]; +/** + * Represents an attribute for describing the key schema for the table + * and indexes. + */ export interface Attribute { /** * The name of an attribute. @@ -48,6 +52,11 @@ export interface Attribute { readonly type: AttributeType; } +/** + * Properties of a DynamoDB Table + * + * Use {@link TableProps} for all table properties + */ export interface TableOptions { /** * Partition key attribute definition. @@ -129,6 +138,9 @@ export interface TableOptions { readonly replicationRegions?: string[]; } +/** + * Properties for a DynamoDB Table + */ export interface TableProps extends TableOptions { /** * Enforces a particular physical table name. @@ -137,6 +149,9 @@ export interface TableProps extends TableOptions { readonly tableName?: string; } +/** + * Properties for a secondary index + */ export interface SecondaryIndexProps { /** * The name of the secondary index. @@ -156,6 +171,9 @@ export interface SecondaryIndexProps { readonly nonKeyAttributes?: string[]; } +/** + * Properties for a global secondary index + */ export interface GlobalSecondaryIndexProps extends SecondaryIndexProps { /** * The attribute of a partition key for the global secondary index. @@ -187,6 +205,9 @@ export interface GlobalSecondaryIndexProps extends SecondaryIndexProps { readonly writeCapacity?: number; } +/** + * Properties for a local secondary index + */ export interface LocalSecondaryIndexProps extends SecondaryIndexProps { /** * The attribute of a sort key for the local secondary index. @@ -1110,6 +1131,11 @@ export class Table extends TableBase { } } +/** + * Data types for attributes within a table + * + * @see https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/HowItWorks.NamingRulesDataTypes.html#HowItWorks.DataTypes + */ export enum AttributeType { /** Up to 400KiB of binary data (which must be encoded as base64 before sending to DynamoDB) */ BINARY = 'B', @@ -1133,6 +1159,11 @@ export enum BillingMode { PROVISIONED = 'PROVISIONED', } +/** + * The set of attributes that are projected into the index + * + * @see https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_Projection.html + */ export enum ProjectionType { /** Only the index and primary keys are projected into the index. */ KEYS_ONLY = 'KEYS_ONLY', diff --git a/packages/@aws-cdk/aws-dynamodb/package.json b/packages/@aws-cdk/aws-dynamodb/package.json index 5e84d04c1dc25..6fa3ddbc68189 100644 --- a/packages/@aws-cdk/aws-dynamodb/package.json +++ b/packages/@aws-cdk/aws-dynamodb/package.json @@ -99,21 +99,6 @@ "node": ">= 10.12.0" }, "stability": "stable", - "awslint": { - "exclude": [ - "docs-public-apis:@aws-cdk/aws-dynamodb.TableProps", - "docs-public-apis:@aws-cdk/aws-dynamodb.Table.tableName", - "docs-public-apis:@aws-cdk/aws-dynamodb.Table.tableStreamArn", - "docs-public-apis:@aws-cdk/aws-dynamodb.Attribute", - "docs-public-apis:@aws-cdk/aws-dynamodb.GlobalSecondaryIndexProps", - "docs-public-apis:@aws-cdk/aws-dynamodb.LocalSecondaryIndexProps", - "docs-public-apis:@aws-cdk/aws-dynamodb.SecondaryIndexProps", - "docs-public-apis:@aws-cdk/aws-dynamodb.TableOptions", - "docs-public-apis:@aws-cdk/aws-dynamodb.Table.tableArn", - "docs-public-apis:@aws-cdk/aws-dynamodb.AttributeType", - "docs-public-apis:@aws-cdk/aws-dynamodb.ProjectionType" - ] - }, "awscdkio": { "announce": false }, From a58b8b3818c00b38d11d5247d683c880797583c9 Mon Sep 17 00:00:00 2001 From: Shiv Lakshminarayan Date: Thu, 30 Apr 2020 02:57:30 -0700 Subject: [PATCH 2/5] chore(cfnspec): update missing libraries to align with added eslint config Latest spec update #7664 broke because the expectations are currently on the wrong file. Additionally we're missing the exclude from `.npmignore` --- .../@aws-cdk/cfnspec/build-tools/create-missing-libraries.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/cfnspec/build-tools/create-missing-libraries.ts b/packages/@aws-cdk/cfnspec/build-tools/create-missing-libraries.ts index 581d2aeb53f3e..fe53d019943fb 100644 --- a/packages/@aws-cdk/cfnspec/build-tools/create-missing-libraries.ts +++ b/packages/@aws-cdk/cfnspec/build-tools/create-missing-libraries.ts @@ -229,6 +229,8 @@ async function main() { '*.tsbuildinfo', '', 'tsconfig.json', + '', + '.eslintrc.js', ]); await write('lib/index.ts', [ @@ -264,7 +266,7 @@ async function main() { '```', ]); - await write('.eslintrc.json', [ + await write('.eslintrc.js', [ "const baseConfig = require('../../../tools/cdk-build-tools/config/eslintrc');", 'module.exports = baseConfig;', ]); From 5a1a929db7eb8f7be4ad973abc1cccda1cb24d23 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 30 Apr 2020 12:43:38 +0200 Subject: [PATCH 3/5] fix(logs): grants don't work on imported LogGroups In CloudFormation, `{ Fn::GetAtt: [MyLogGroup, Arn] }` always returned the ARN with a `:*` appended, presumably so you could stick the returned value directly into an IAM policy and get the result you wanted (doing something to Log Groups usually entails doing something to the Log *Streams* inside them). The CDK construct did not do anything special, leading to imports done without a `:*` at the end having incorrect permissions. This change makes the behavior between imported and constructed Log Groups consistent. Fixes #7096. --- packages/@aws-cdk/aws-logs/README.md | 6 ++ packages/@aws-cdk/aws-logs/lib/log-group.ts | 17 ++-- .../@aws-cdk/aws-logs/test/test.loggroup.ts | 89 ++++++++++++++++++- 3 files changed, 104 insertions(+), 8 deletions(-) diff --git a/packages/@aws-cdk/aws-logs/README.md b/packages/@aws-cdk/aws-logs/README.md index a38a753ba9698..478e981010d63 100644 --- a/packages/@aws-cdk/aws-logs/README.md +++ b/packages/@aws-cdk/aws-logs/README.md @@ -216,3 +216,9 @@ const pattern = FilterPattern.spaceDelimited('time', 'component', '...', 'result .whereString('component', '=', 'HttpServer') .whereNumber('result_code', '!=', 200); ``` + +### Notes + +Be aware that Log Group ARNs will always have the string `:*` appended to +them, to match the behavior of [the CloudFormation `AWS::Logs::LogGroup` +resource](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-logs-loggroup.html#aws-resource-logs-loggroup-return-values). \ No newline at end of file diff --git a/packages/@aws-cdk/aws-logs/lib/log-group.ts b/packages/@aws-cdk/aws-logs/lib/log-group.ts index 0c5d512348be3..70512828aea3b 100644 --- a/packages/@aws-cdk/aws-logs/lib/log-group.ts +++ b/packages/@aws-cdk/aws-logs/lib/log-group.ts @@ -9,7 +9,8 @@ import { ILogSubscriptionDestination, SubscriptionFilter } from './subscription- export interface ILogGroup extends IResource { /** - * The ARN of this log group + * The ARN of this log group, with ':*' appended + * * @attribute */ readonly logGroupArn: string; @@ -76,7 +77,7 @@ export interface ILogGroup extends IResource { */ abstract class LogGroupBase extends Resource implements ILogGroup { /** - * The ARN of this log group + * The ARN of this log group, with ':*' appended */ public abstract readonly logGroupArn: string; @@ -308,9 +309,11 @@ export class LogGroup extends LogGroupBase { * Import an existing LogGroup given its ARN */ public static fromLogGroupArn(scope: Construct, id: string, logGroupArn: string): ILogGroup { + const baseLogGroupArn = logGroupArn.replace(/:\*$/, ''); + class Import extends LogGroupBase { - public readonly logGroupArn = logGroupArn; - public readonly logGroupName = Stack.of(scope).parseArn(logGroupArn, ':').resourceName!; + public readonly logGroupArn = `${baseLogGroupArn}:*`; + public readonly logGroupName = Stack.of(scope).parseArn(baseLogGroupArn, ':').resourceName!; } return new Import(scope, id); @@ -320,13 +323,15 @@ export class LogGroup extends LogGroupBase { * Import an existing LogGroup given its name */ public static fromLogGroupName(scope: Construct, id: string, logGroupName: string): ILogGroup { + const baseLogGroupName = logGroupName.replace(/:\*$/, ''); + class Import extends LogGroupBase { - public readonly logGroupName = logGroupName; + public readonly logGroupName = baseLogGroupName; public readonly logGroupArn = Stack.of(scope).formatArn({ service: 'logs', resource: 'log-group', sep: ':', - resourceName: logGroupName, + resourceName: baseLogGroupName + ':*', }); } diff --git a/packages/@aws-cdk/aws-logs/test/test.loggroup.ts b/packages/@aws-cdk/aws-logs/test/test.loggroup.ts index e700ef3056c73..ac88fb9c35a7f 100644 --- a/packages/@aws-cdk/aws-logs/test/test.loggroup.ts +++ b/packages/@aws-cdk/aws-logs/test/test.loggroup.ts @@ -140,7 +140,7 @@ export = { // THEN test.deepEqual(imported.logGroupName, 'my-log-group'); - test.deepEqual(imported.logGroupArn, 'arn:aws:logs:us-east-1:123456789012:log-group:my-log-group'); + test.deepEqual(imported.logGroupArn, 'arn:aws:logs:us-east-1:123456789012:log-group:my-log-group:*'); expect(stack2).to(haveResource('AWS::Logs::LogStream', { LogGroupName: 'my-log-group', })); @@ -157,7 +157,7 @@ export = { // THEN test.deepEqual(imported.logGroupName, 'my-log-group'); - test.ok(/^arn:.+:logs:.+:.+:log-group:my-log-group$/.test(imported.logGroupArn), + test.ok(/^arn:.+:logs:.+:.+:log-group:my-log-group:\*$/.test(imported.logGroupArn), `LogGroup ARN ${imported.logGroupArn} does not match the expected pattern`); expect(stack).to(haveResource('AWS::Logs::LogStream', { LogGroupName: 'my-log-group', @@ -165,6 +165,80 @@ export = { test.done(); }, + 'loggroups imported by name have stream wildcard appended to grant ARN': dataDrivenTests([ + // Regardless of whether the user put :* there already because of this bug, we + // don't want to append it twice. + [''], + [':*'], + ], (test: Test, suffix: string) => { + // GIVEN + const stack = new Stack(); + const user = new iam.User(stack, 'Role'); + const imported = LogGroup.fromLogGroupName(stack, 'lg', `my-log-group${suffix}`); + + // WHEN + imported.grantWrite(user); + + // THEN + expect(stack).to(haveResource('AWS::IAM::Policy', { + PolicyDocument: { + Version: '2012-10-17', + Statement: [ + { + Action: ['logs:CreateLogStream', 'logs:PutLogEvents'], + Effect: 'Allow', + Resource: { + 'Fn::Join': [ '', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':logs:', + { Ref: 'AWS::Region' }, + ':', + { Ref: 'AWS::AccountId' }, + ':log-group:my-log-group:*', + ]], + }, + }, + ], + }, + })); + test.equal(imported.logGroupName, 'my-log-group'); + + test.done(); + }), + + 'loggroups imported by ARN have stream wildcard appended to grant ARN': dataDrivenTests([ + // Regardless of whether the user put :* there already because of this bug, we + // don't want to append it twice. + [''], + [':*'], + ], (test: Test, suffix: string) => { + // GIVEN + const stack = new Stack(); + const user = new iam.User(stack, 'Role'); + const imported = LogGroup.fromLogGroupArn(stack, 'lg', `arn:aws:logs:us-west-1:123456789012:log-group:my-log-group${suffix}`); + + // WHEN + imported.grantWrite(user); + + // THEN + expect(stack).to(haveResource('AWS::IAM::Policy', { + PolicyDocument: { + Version: '2012-10-17', + Statement: [ + { + Action: ['logs:CreateLogStream', 'logs:PutLogEvents'], + Effect: 'Allow', + Resource: 'arn:aws:logs:us-west-1:123456789012:log-group:my-log-group:*', + }, + ], + }, + })); + test.equal(imported.logGroupName, 'my-log-group'); + + test.done(); + }), + 'extractMetric'(test: Test) { // GIVEN const stack = new Stack(); @@ -242,3 +316,14 @@ export = { test.done(); }, }; + +function dataDrivenTests(cases: any[][], body: (test: Test, ...args: any[]) => void) { + const ret: any = {}; + for (let i = 0; i < cases.length; i++) { + const args = cases[i]; // Need to capture inside loop for safe use inside closure. + ret[`case ${i + 1}`] = function(test: Test) { + return body.apply(this, [test, ...args]); + }; + } + return ret; +} \ No newline at end of file From 884188068ed1d064bfcef4fc3f6bbedea21638a4 Mon Sep 17 00:00:00 2001 From: Eli Polonsky Date: Thu, 30 Apr 2020 14:29:05 +0300 Subject: [PATCH 4/5] chore(cli): install specific cli version in integ tests (#7704) Currently, we install the CLI in the integration tests by simply running `npm install aws-cdk`. We then make sure the version that gets installed is the version we have locally. This will fail immediately after the release of a new version, and before the merge back PR was merged with the following error: ```console | | ============================================================================================ | Expected CDK version: 1.36.0 | ============================================================================================ | Found CDK: /tmp/cdk-rundist/node_modules/.bin/cdk | Mismatched CDK version. Expected: 1.36.0, actual: 1.36.1 1.36.1 (build 4df7dac) ``` It happens because the verdaccio instance we have in the tests has an npm uplink configured, so the latest version as far as he is concerned is the latest published one, which doesn't match the version number we have locally since the merge back PR wasn't merged yet. This PR makes it so we always install the CLI version we want to test (i.e the local version). --- packages/aws-cdk/test/integ/run-against-dist | 6 ++++-- packages/aws-cdk/test/integ/run-against-dist.bash | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/aws-cdk/test/integ/run-against-dist b/packages/aws-cdk/test/integ/run-against-dist index d10ef39298b1d..0f95be1225cdf 100755 --- a/packages/aws-cdk/test/integ/run-against-dist +++ b/packages/aws-cdk/test/integ/run-against-dist @@ -23,13 +23,15 @@ if [[ ! -f $dist_root/build.json ]]; then exit 1 fi +local local_cli_version="$(node -e "console.log(require('${dist_root}/build.json').version)")" + serve_npm_packages # Install the CLI and put it on the path -(cd $npmws && npm install aws-cdk) +(cd $npmws && npm install aws-cdk@${local_cli_version}) export PATH=$npmws/node_modules/.bin:$PATH -verify_installed_cli_version +verify_installed_cli_version ${local_cli_version} prepare_java_packages prepare_nuget_packages prepare_python_packages diff --git a/packages/aws-cdk/test/integ/run-against-dist.bash b/packages/aws-cdk/test/integ/run-against-dist.bash index 185365e13f53d..f20c3d65a8676 100644 --- a/packages/aws-cdk/test/integ/run-against-dist.bash +++ b/packages/aws-cdk/test/integ/run-against-dist.bash @@ -80,7 +80,9 @@ function serve_npm_packages() { # Make sure that installed CLI matches the build version function verify_installed_cli_version() { - local expected_version="$(node -e "console.log(require('${dist_root}/build.json').version)")" + + expected_version=$1 + header "Expected CDK version: ${expected_version}" log "Found CDK: $(type -p cdk)" From 360305794f7e185431adc7d5fffd7dad4216b7db Mon Sep 17 00:00:00 2001 From: Eli Polonsky Date: Thu, 30 Apr 2020 16:30:18 +0300 Subject: [PATCH 5/5] chore(cli): integ tests breakage (#7711) Copy paste error. `local` cannot be used outside of a function. --- packages/aws-cdk/test/integ/run-against-dist | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk/test/integ/run-against-dist b/packages/aws-cdk/test/integ/run-against-dist index 0f95be1225cdf..eebf758ec2823 100755 --- a/packages/aws-cdk/test/integ/run-against-dist +++ b/packages/aws-cdk/test/integ/run-against-dist @@ -23,7 +23,7 @@ if [[ ! -f $dist_root/build.json ]]; then exit 1 fi -local local_cli_version="$(node -e "console.log(require('${dist_root}/build.json').version)")" +local_cli_version="$(node -e "console.log(require('${dist_root}/build.json').version)")" serve_npm_packages