Skip to content

Commit

Permalink
fix(ecr): repository grant uses correct resource ARN
Browse files Browse the repository at this point in the history
When granting to a cross-account principal the repository would
use a self-reference to obtain the right ARN to use in its own
resource policy, which can obviously never work.

The solution is to use a '*' resource ARN.

Fixes #2473.
  • Loading branch information
rix0rrr committed Jul 5, 2019
1 parent d60d673 commit 9356f07
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 2 deletions.
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-ecr/lib/repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ export abstract class RepositoryBase extends Resource implements IRepository {
grantee,
actions,
resourceArns: [this.repositoryArn],
resourceSelfArns: ['*'],
resource: this,
});
}
Expand Down
33 changes: 31 additions & 2 deletions packages/@aws-cdk/aws-ecr/test/test.repository.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { expect, haveResource, haveResourceLike, ResourcePart } from '@aws-cdk/assert';
import iam = require('@aws-cdk/aws-iam');
import cdk = require('@aws-cdk/core');
import { RemovalPolicy } from '@aws-cdk/core';
import { RemovalPolicy, Stack } from '@aws-cdk/core';
import { Test } from 'nodeunit';
import ecr = require('../lib');

Expand Down Expand Up @@ -352,6 +352,35 @@ export = {
"DeletionPolicy": "Delete"
}, ResourcePart.CompleteDefinition));
test.done();
}
},

'grant adds appropriate resource-*'(test: Test) {
// GIVEN
const stack = new Stack();
const repo = new ecr.Repository(stack, 'TestHarnessRepo');

// WHEN
repo.grantPull(new iam.AnyPrincipal());

// THEN
expect(stack).to(haveResource('AWS::ECR::Repository', {
"RepositoryPolicyText": {
"Statement": [
{
"Action": [
"ecr:BatchCheckLayerAvailability",
"ecr:GetDownloadUrlForLayer",
"ecr:BatchGetImage"
],
"Effect": "Allow",
"Principal": "*",
"Resource": "*",
}
],
"Version": "2012-10-17"
}
}));

test.done();
},
};

0 comments on commit 9356f07

Please sign in to comment.