diff --git a/packages/@aws-cdk/aws-dynamodb/lib/table.ts b/packages/@aws-cdk/aws-dynamodb/lib/table.ts index cd8ebef7be464..f857a7d8be470 100644 --- a/packages/@aws-cdk/aws-dynamodb/lib/table.ts +++ b/packages/@aws-cdk/aws-dynamodb/lib/table.ts @@ -429,7 +429,7 @@ export class Table extends Construct { return; } principal.addToPolicy(new iam.PolicyStatement() - .addResource(this.tableArn) + .addResources(this.tableArn, new cdk.Token(() => this.hasIndex ? `${this.tableArn}/index/*` : new cdk.Aws().noValue).toString()) .addActions(...actions)); } @@ -622,6 +622,13 @@ export class Table extends Construct { }) }); } + + /** + * Whether this table has indexes + */ + private get hasIndex(): boolean { + return this.globalSecondaryIndexes.length + this.localSecondaryIndexes.length > 0; + } } export enum AttributeType { diff --git a/packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.expected.json b/packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.expected.json index d508449c163f7..f16e8e27d15d0 100644 --- a/packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.expected.json +++ b/packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.expected.json @@ -9,16 +9,16 @@ "KeyType": "HASH" } ], - "ProvisionedThroughput": { - "ReadCapacityUnits": 5, - "WriteCapacityUnits": 5 - }, "AttributeDefinitions": [ { "AttributeName": "hashKey", "AttributeType": "S" } - ] + ], + "ProvisionedThroughput": { + "ReadCapacityUnits": 5, + "WriteCapacityUnits": 5 + } } }, "TableWithGlobalAndLocalSecondaryIndexBC540710": { @@ -34,10 +34,6 @@ "KeyType": "RANGE" } ], - "ProvisionedThroughput": { - "ReadCapacityUnits": 5, - "WriteCapacityUnits": 5 - }, "AttributeDefinitions": [ { "AttributeName": "hashKey", @@ -251,6 +247,10 @@ "PointInTimeRecoverySpecification": { "PointInTimeRecoveryEnabled": true }, + "ProvisionedThroughput": { + "ReadCapacityUnits": 5, + "WriteCapacityUnits": 5 + }, "SSESpecification": { "SSEEnabled": true }, @@ -278,10 +278,6 @@ "KeyType": "HASH" } ], - "ProvisionedThroughput": { - "ReadCapacityUnits": 5, - "WriteCapacityUnits": 5 - }, "AttributeDefinitions": [ { "AttributeName": "hashKey", @@ -309,7 +305,11 @@ "WriteCapacityUnits": 5 } } - ] + ], + "ProvisionedThroughput": { + "ReadCapacityUnits": 5, + "WriteCapacityUnits": 5 + } } }, "TableWithLocalSecondaryIndex4DA3D08F": { @@ -325,10 +325,6 @@ "KeyType": "RANGE" } ], - "ProvisionedThroughput": { - "ReadCapacityUnits": 5, - "WriteCapacityUnits": 5 - }, "AttributeDefinitions": [ { "AttributeName": "hashKey", @@ -360,6 +356,84 @@ "ProjectionType": "ALL" } } + ], + "ProvisionedThroughput": { + "ReadCapacityUnits": 5, + "WriteCapacityUnits": 5 + } + } + }, + "User00B015A1": { + "Type": "AWS::IAM::User" + }, + "UserDefaultPolicy1F97781E": { + "Type": "AWS::IAM::Policy", + "Properties": { + "PolicyDocument": { + "Statement": [ + { + "Action": [ + "dynamodb:BatchGetItem", + "dynamodb:GetRecords", + "dynamodb:GetShardIterator", + "dynamodb:Query", + "dynamodb:GetItem", + "dynamodb:Scan" + ], + "Effect": "Allow", + "Resource": [ + { + "Fn::GetAtt": [ + "TableCD117FA1", + "Arn" + ] + }, + { + "Ref": "AWS::NoValue" + } + ] + }, + { + "Action": [ + "dynamodb:BatchGetItem", + "dynamodb:GetRecords", + "dynamodb:GetShardIterator", + "dynamodb:Query", + "dynamodb:GetItem", + "dynamodb:Scan" + ], + "Effect": "Allow", + "Resource": [ + { + "Fn::GetAtt": [ + "TableWithGlobalAndLocalSecondaryIndexBC540710", + "Arn" + ] + }, + { + "Fn::Join": [ + "", + [ + { + "Fn::GetAtt": [ + "TableWithGlobalAndLocalSecondaryIndexBC540710", + "Arn" + ] + }, + "/index/*" + ] + ] + } + ] + } + ], + "Version": "2012-10-17" + }, + "PolicyName": "UserDefaultPolicy1F97781E", + "Users": [ + { + "Ref": "User00B015A1" + } ] } } diff --git a/packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.ts b/packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.ts index e8124d879bea9..9be63b69ddbc0 100644 --- a/packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.ts +++ b/packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.ts @@ -1,3 +1,4 @@ +import iam = require('@aws-cdk/aws-iam'); import { App, Stack } from '@aws-cdk/cdk'; import { Attribute, AttributeType, ProjectionType, StreamViewType, Table } from '../lib'; @@ -118,4 +119,8 @@ tableWithLocalSecondaryIndex.addLocalSecondaryIndex({ sortKey: LSI_SORT_KEY }); +const user = new iam.User(stack, 'User'); +table.grantReadData(user); +tableWithGlobalAndLocalSecondaryIndex.grantReadData(user); + app.run(); diff --git a/packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts b/packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts index c19eaeaf6521b..6724f9c89ed38 100644 --- a/packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts +++ b/packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts @@ -1353,7 +1353,46 @@ export = { '"grantFullAccess" allows the principal to perform any action on the table ("*")'(test: Test) { testGrant(test, [ '*' ], (p, t) => t.grantFullAccess(p)); - } + }, + + 'if table has an index grant gives access to the index'(test: Test) { + // GIVEN + const stack = new Stack(); + + const table = new Table(stack, 'my-table'); + table.addPartitionKey({ name: 'ID', type: AttributeType.String }); + table.addGlobalSecondaryIndex({ indexName: 'MyIndex', partitionKey: { name: 'Age', type: AttributeType.Number }}); + const user = new iam.User(stack, 'user'); + + // WHEN + table.grantReadData(user); + + // THEN + expect(stack).to(haveResource('AWS::IAM::Policy', { + "PolicyDocument": { + "Statement": [ + { + "Action": [ + 'dynamodb:BatchGetItem', + 'dynamodb:GetRecords', + 'dynamodb:GetShardIterator', + 'dynamodb:Query', + 'dynamodb:GetItem', + 'dynamodb:Scan' + ], + "Effect": "Allow", + "Resource": [ + { "Fn::GetAtt": ["mytable0324D45C", "Arn"] }, + { "Fn::Join": [ "", [ { "Fn::GetAtt": [ "mytable0324D45C", "Arn" ] }, "/index/*" ] ] } + ] + } + ], + "Version": "2012-10-17" + }, + })); + test.done(); + }, + }, }; @@ -1387,12 +1426,10 @@ function testGrant(test: Test, expectedActions: string[], invocation: (user: iam { "Action": action, "Effect": "Allow", - "Resource": { - "Fn::GetAtt": [ - "mytable0324D45C", - "Arn" - ] - } + "Resource": [ + { "Fn::GetAtt": [ "mytable0324D45C", "Arn" ] }, + { "Ref" : "AWS::NoValue" } + ] } ], "Version": "2012-10-17" diff --git a/packages/@aws-cdk/cloudformation-diff/lib/render-intrinsics.ts b/packages/@aws-cdk/cloudformation-diff/lib/render-intrinsics.ts index ea392d2de8444..bbd17c994ea7b 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/render-intrinsics.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/render-intrinsics.ts @@ -13,14 +13,18 @@ * * Evaluates Fn::Join directly if the second argument is a literal list of strings. * + * Removes list and object values evaluating to { Ref: 'AWS::NoValue' }. + * * For other intrinsics we choose a string representation that CloudFormation * cannot actually parse, but is comprehensible to humans. */ export function renderIntrinsics(x: any): any { if (Array.isArray(x)) { - return x.map(renderIntrinsics); + return x.filter(el => !isNoValue(el)).map(renderIntrinsics); } + if (isNoValue(x)) { return undefined; } + const intrinsic = getIntrinsic(x); if (intrinsic) { if (intrinsic.fn === 'Ref') { return '${' + intrinsic.args + '}'; } @@ -32,7 +36,9 @@ export function renderIntrinsics(x: any): any { if (typeof x === 'object' && x !== null) { const ret: any = {}; for (const [key, value] of Object.entries(x)) { - ret[key] = renderIntrinsics(value); + if (!isNoValue(value)) { + ret[key] = renderIntrinsics(value); + } } return ret; } @@ -41,7 +47,7 @@ export function renderIntrinsics(x: any): any { function unCloudFormationFnJoin(separator: string, args: any) { if (Array.isArray(args)) { - return args.map(renderIntrinsics).join(separator); + return args.filter(el => !isNoValue(el)).map(renderIntrinsics).join(separator); } return stringifyIntrinsic('Fn::Join', [separator, args]); } @@ -57,6 +63,11 @@ function getIntrinsic(x: any): Intrinsic | undefined { return keys.length === 1 && (keys[0] === 'Ref' || keys[0].startsWith('Fn::')) ? { fn: keys[0], args: x[keys[0]] } : undefined; } +function isNoValue(x: any) { + const int = getIntrinsic(x); + return int && int.fn === 'Ref' && int.args === 'AWS::NoValue'; +} + interface Intrinsic { fn: string; args: any; diff --git a/packages/@aws-cdk/cloudformation-diff/test/test.render-intrinsics.ts b/packages/@aws-cdk/cloudformation-diff/test/test.render-intrinsics.ts index f0c012cd8b786..6272df1caefa4 100644 --- a/packages/@aws-cdk/cloudformation-diff/test/test.render-intrinsics.ts +++ b/packages/@aws-cdk/cloudformation-diff/test/test.render-intrinsics.ts @@ -27,6 +27,15 @@ export = { test.done(); }, + 'removes AWS::NoValue from Fn::Join'(test: Test) { + test.equals( + renderIntrinsics({ 'Fn::Join': ['/', ['a', { Ref: 'AWS::NoValue' }, 'b', 'c']] }), + 'a/b/c' + ); + + test.done(); + }, + 'does not resolve Fn::Join if the second argument is not a list literal'(test: Test) { test.equals( renderIntrinsics({ 'Fn::Join': ['/', { Ref: 'ListParameter' }] }), @@ -63,4 +72,30 @@ export = { ); test.done(); }, + + 'removes NoValue from object'(test: Test) { + test.deepEqual( + renderIntrinsics({ + Deeper1: { Ref: 'SomeLogicalId' }, + Deeper2: { Ref: 'AWS::NoValue' } + }), + { + Deeper1: '${SomeLogicalId}', + } + ); + test.done(); + }, + + 'removes NoValue from array'(test: Test) { + test.deepEqual( + renderIntrinsics([ + { Ref: 'SomeLogicalId' }, + { Ref: 'AWS::NoValue' }, + ]), + [ + '${SomeLogicalId}', + ] + ); + test.done(); + }, };