From 4193b2669001b3d588eeda0ce0a5d5ab0ca0500f Mon Sep 17 00:00:00 2001 From: i05nagai Date: Thu, 9 Jun 2022 00:27:55 +0200 Subject: [PATCH 1/3] feat(dynamodb): an imported table always grants permissions for indexes --- packages/@aws-cdk/aws-dynamodb/README.md | 2 + packages/@aws-cdk/aws-dynamodb/lib/table.ts | 8 +++- .../aws-dynamodb/test/dynamodb.test.ts | 40 +++++++++++++++++-- 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/packages/@aws-cdk/aws-dynamodb/README.md b/packages/@aws-cdk/aws-dynamodb/README.md index f006eb277d984..efd9ba9deed92 100644 --- a/packages/@aws-cdk/aws-dynamodb/README.md +++ b/packages/@aws-cdk/aws-dynamodb/README.md @@ -36,6 +36,8 @@ If you intend to use the `tableStreamArn` (including indirectly, for example by `@aws-cdk/aws-lambda-event-source.DynamoEventSource` on the imported table), you *must* use the `Table.fromTableAttributes` method and the `tableStreamArn` property *must* be populated. +Imported tables always grant permissions for indexes when a grant method is called. + ## Keys When a table is defined, you must define it's schema using the `partitionKey` diff --git a/packages/@aws-cdk/aws-dynamodb/lib/table.ts b/packages/@aws-cdk/aws-dynamodb/lib/table.ts index dc4497e2ae58f..2c3add774a44f 100644 --- a/packages/@aws-cdk/aws-dynamodb/lib/table.ts +++ b/packages/@aws-cdk/aws-dynamodb/lib/table.ts @@ -586,6 +586,8 @@ export interface TableAttributes { * to grant permissions for indexes as well as the table itself. * * @default - no global indexes + * + * @deprecated grant methods grant permssions for indexes without specifying this parameter. */ readonly globalIndexes?: string[]; @@ -597,6 +599,8 @@ export interface TableAttributes { * to grant permissions for indexes as well as the table itself. * * @default - no local indexes + * + * @deprecated grant methods grant permssions for indexes without specifying this parameter. */ readonly localIndexes?: string[]; } @@ -1078,8 +1082,8 @@ export class Table extends TableBase { public readonly tableArn: string; public readonly tableStreamArn?: string; public readonly encryptionKey?: kms.IKey; - protected readonly hasIndex = (attrs.globalIndexes ?? []).length > 0 || - (attrs.localIndexes ?? []).length > 0; + // Always to grant permissions to indexes + protected readonly hasIndex = true; constructor(_tableArn: string, tableName: string, tableStreamArn?: string) { super(scope, id); diff --git a/packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts b/packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts index aedd50acd335d..f38715c7ca0cf 100644 --- a/packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts +++ b/packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts @@ -2210,7 +2210,24 @@ describe('grants', () => { ], }, { - Ref: 'AWS::NoValue', + 'Fn::Join': [ + '', + [ + 'arn:', + { + Ref: 'AWS::Partition', + }, + ':dynamodb:', + { + Ref: 'AWS::Region', + }, + ':', + { + Ref: 'AWS::AccountId', + }, + ':table/my-table/index/*', + ], + ], }, ], }, @@ -2292,7 +2309,7 @@ describe('import', () => { 'Effect': 'Allow', 'Resource': [ tableArn, - { 'Ref': 'AWS::NoValue' }, + `${tableArn}/index/*`, ], }, ], @@ -2359,7 +2376,24 @@ describe('import', () => { ], }, { - 'Ref': 'AWS::NoValue', + 'Fn::Join': [ + '', + [ + 'arn:', + { + 'Ref': 'AWS::Partition', + }, + ':dynamodb:', + { + 'Ref': 'AWS::Region', + }, + ':', + { + 'Ref': 'AWS::AccountId', + }, + ':table/MyTable/index/*', + ], + ], }, ], }, From 84bb985d7bc0c8c3c667bdfed7ba92f591ac0f7a Mon Sep 17 00:00:00 2001 From: i05nagai Date: Mon, 20 Jun 2022 11:07:47 +0200 Subject: [PATCH 2/3] feat(dynamodb): Add a flag to grant permissions for all indexes If grantIndexPermissions is provided, grant methods always grant permissions for all indexes. --- packages/@aws-cdk/aws-dynamodb/README.md | 3 +- packages/@aws-cdk/aws-dynamodb/lib/table.ts | 18 ++-- .../aws-dynamodb/test/dynamodb.test.ts | 98 ++++++++++++------- 3 files changed, 75 insertions(+), 44 deletions(-) diff --git a/packages/@aws-cdk/aws-dynamodb/README.md b/packages/@aws-cdk/aws-dynamodb/README.md index efd9ba9deed92..3f0263046a6f5 100644 --- a/packages/@aws-cdk/aws-dynamodb/README.md +++ b/packages/@aws-cdk/aws-dynamodb/README.md @@ -36,7 +36,8 @@ If you intend to use the `tableStreamArn` (including indirectly, for example by `@aws-cdk/aws-lambda-event-source.DynamoEventSource` on the imported table), you *must* use the `Table.fromTableAttributes` method and the `tableStreamArn` property *must* be populated. -Imported tables always grant permissions for indexes when a grant method is called. +To grant permissions for indexes with imported tables, `grantIndexPermissions` property or indexes of the importing tables +must be provided. ## Keys diff --git a/packages/@aws-cdk/aws-dynamodb/lib/table.ts b/packages/@aws-cdk/aws-dynamodb/lib/table.ts index 2c3add774a44f..a3ebe5f61967e 100644 --- a/packages/@aws-cdk/aws-dynamodb/lib/table.ts +++ b/packages/@aws-cdk/aws-dynamodb/lib/table.ts @@ -586,8 +586,6 @@ export interface TableAttributes { * to grant permissions for indexes as well as the table itself. * * @default - no global indexes - * - * @deprecated grant methods grant permssions for indexes without specifying this parameter. */ readonly globalIndexes?: string[]; @@ -599,10 +597,17 @@ export interface TableAttributes { * to grant permissions for indexes as well as the table itself. * * @default - no local indexes - * - * @deprecated grant methods grant permssions for indexes without specifying this parameter. */ readonly localIndexes?: string[]; + + /** + * If set to true, grant methods always grant permissions for all indexes. + * If false is provided, grant methods grant the permissions + * only when {@link globalIndexes} or {@link localIndexes} is specified. + * + * @default - false + */ + readonly grantIndexPermissions?: boolean; } abstract class TableBase extends Resource implements ITable { @@ -1082,8 +1087,9 @@ export class Table extends TableBase { public readonly tableArn: string; public readonly tableStreamArn?: string; public readonly encryptionKey?: kms.IKey; - // Always to grant permissions to indexes - protected readonly hasIndex = true; + protected readonly hasIndex = (attrs.grantIndexPermissions ?? false) || + (attrs.globalIndexes ?? []).length > 0 || + (attrs.localIndexes ?? []).length > 0; constructor(_tableArn: string, tableName: string, tableStreamArn?: string) { super(scope, id); diff --git a/packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts b/packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts index f38715c7ca0cf..09d45edaac057 100644 --- a/packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts +++ b/packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts @@ -2210,24 +2210,7 @@ describe('grants', () => { ], }, { - 'Fn::Join': [ - '', - [ - 'arn:', - { - Ref: 'AWS::Partition', - }, - ':dynamodb:', - { - Ref: 'AWS::Region', - }, - ':', - { - Ref: 'AWS::AccountId', - }, - ':table/my-table/index/*', - ], - ], + Ref: 'AWS::NoValue', }, ], }, @@ -2309,7 +2292,7 @@ describe('import', () => { 'Effect': 'Allow', 'Resource': [ tableArn, - `${tableArn}/index/*`, + { 'Ref': 'AWS::NoValue' }, ], }, ], @@ -2376,24 +2359,7 @@ describe('import', () => { ], }, { - 'Fn::Join': [ - '', - [ - 'arn:', - { - 'Ref': 'AWS::Partition', - }, - ':dynamodb:', - { - 'Ref': 'AWS::Region', - }, - ':', - { - 'Ref': 'AWS::AccountId', - }, - ':table/MyTable/index/*', - ], - ], + 'Ref': 'AWS::NoValue', }, ], }, @@ -2542,6 +2508,64 @@ describe('import', () => { }, }); }); + + test('creates the index permissions if grantIndexPermissions is provided', () => { + const stack = new Stack(); + + const table = Table.fromTableAttributes(stack, 'ImportedTable', { + tableName: 'MyTableName', + grantIndexPermissions: true, + }); + + const role = new iam.Role(stack, 'Role', { + assumedBy: new iam.AnyPrincipal(), + }); + + table.grantReadData(role); + + Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', { + PolicyDocument: { + Statement: [ + { + Action: [ + 'dynamodb:BatchGetItem', + 'dynamodb:GetRecords', + 'dynamodb:GetShardIterator', + 'dynamodb:Query', + 'dynamodb:GetItem', + 'dynamodb:Scan', + 'dynamodb:ConditionCheckItem', + 'dynamodb:DescribeTable', + ], + Resource: [ + { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':dynamodb:', + { Ref: 'AWS::Region' }, + ':', + { Ref: 'AWS::AccountId' }, + ':table/MyTableName', + ]], + }, + { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':dynamodb:', + { Ref: 'AWS::Region' }, + ':', + { Ref: 'AWS::AccountId' }, + ':table/MyTableName/index/*', + ]], + }, + ], + }, + ], + }, + }); + }); }); }); From dbdf94e373a528cc0507bd28af305fa5f425b777 Mon Sep 17 00:00:00 2001 From: i05nagai Date: Fri, 1 Jul 2022 23:50:03 +0200 Subject: [PATCH 3/3] Improve README.md --- packages/@aws-cdk/aws-dynamodb/README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-dynamodb/README.md b/packages/@aws-cdk/aws-dynamodb/README.md index 3f0263046a6f5..f9b5771953b14 100644 --- a/packages/@aws-cdk/aws-dynamodb/README.md +++ b/packages/@aws-cdk/aws-dynamodb/README.md @@ -36,8 +36,7 @@ If you intend to use the `tableStreamArn` (including indirectly, for example by `@aws-cdk/aws-lambda-event-source.DynamoEventSource` on the imported table), you *must* use the `Table.fromTableAttributes` method and the `tableStreamArn` property *must* be populated. -To grant permissions for indexes with imported tables, `grantIndexPermissions` property or indexes of the importing tables -must be provided. +In order to grant permissions to indexes on imported tables you can either set `grantIndexPermissions` to `true`, or you can provide the indexes via the `globalIndexes` or `localIndexes` properties. This will enable `grant*` methods to also grant permissions to *all* table indexes. ## Keys