From 5ac6e7768a2130d925f6afb13cc49b352dd6ff64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=91=A8=F0=9F=8F=BC=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier-Muller?= Date: Wed, 15 May 2019 23:55:48 +0200 Subject: [PATCH] fix(s3): Make IBucket.arnForObject accept only (exactly) one key pattern The variadic signature allowed the method to be invoked with zero arguments, resulting in an unusable ARN. Instead of accepting an array of path elements that will be concatenated, accept only one pattern and use it as-is. BREAKING CHANGE: The `IBucket.arnForObject` method no longer concatenates path fragments on your behalf. Pass the `/`-concatenated key pattern instead. --- packages/@aws-cdk/aws-s3/lib/bucket.ts | 11 +++-------- packages/@aws-cdk/aws-s3/test/test.bucket.ts | 4 ++-- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/@aws-cdk/aws-s3/lib/bucket.ts b/packages/@aws-cdk/aws-s3/lib/bucket.ts index 1774c650827df..eb17df7b618c4 100644 --- a/packages/@aws-cdk/aws-s3/lib/bucket.ts +++ b/packages/@aws-cdk/aws-s3/lib/bucket.ts @@ -83,13 +83,8 @@ export interface IBucket extends IResource { /** * Returns an ARN that represents all objects within the bucket that match * the key pattern specified. To represent all keys, specify ``"*"``. - * - * If you specify multiple components for keyPattern, they will be concatenated:: - * - * arnForObjects('home/', team, '/', user, '/*') - * */ - arnForObjects(...keyPattern: string[]): string; + arnForObjects(keyPattern: string): string; /** * Grant read permissions for this bucket and it's contents to an IAM @@ -359,8 +354,8 @@ abstract class BucketBase extends Resource implements IBucket { * arnForObjects('home/', team, '/', user, '/*') * */ - public arnForObjects(...keyPattern: string[]): string { - return `${this.bucketArn}/${keyPattern.join('')}`; + public arnForObjects(keyPattern: string): string { + return `${this.bucketArn}/${keyPattern}`; } /** diff --git a/packages/@aws-cdk/aws-s3/test/test.bucket.ts b/packages/@aws-cdk/aws-s3/test/test.bucket.ts index 4203b96916346..7bbc356e10de2 100644 --- a/packages/@aws-cdk/aws-s3/test/test.bucket.ts +++ b/packages/@aws-cdk/aws-s3/test/test.bucket.ts @@ -507,7 +507,7 @@ export = { const user = new iam.User(stack, 'MyUser'); const team = new iam.Group(stack, 'MyTeam'); - const resource = bucket.arnForObjects('home/', team.groupName, '/', user.userName, '/*'); + const resource = bucket.arnForObjects(`home/${team.groupName}/${user.userName}/*`); const p = new iam.PolicyStatement().addResource(resource).addAction('s3:GetObject'); test.deepEqual(bucket.node.resolve(p), { @@ -587,7 +587,7 @@ export = { // you can even use the bucket name, which will be extracted from the arn provided. const user = new iam.User(stack, 'MyUser'); user.addToPolicy(new iam.PolicyStatement() - .addResource(bucket.arnForObjects('my/folder/', bucket.bucketName)) + .addResource(bucket.arnForObjects(`my/folder/${bucket.bucketName}`)) .addAction('s3:*')); expect(stack).toMatch({