Skip to content

Commit

Permalink
fix: ecr policy warning always throws (#25041)
Browse files Browse the repository at this point in the history
A change recently added a warning when the policy added to a Repository resource policy. Check length of array instead of existence of array

Closes #25028 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
peterwoodworth committed Apr 12, 2023
1 parent 91553e5 commit c0c3d19
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-ecr/lib/repository.ts
Expand Up @@ -669,7 +669,7 @@ export class Repository extends RepositoryBase {
* It will fail if a resource section is present at all.
*/
public addToResourcePolicy(statement: iam.PolicyStatement): iam.AddToResourcePolicyResult {
if (statement.resources) {
if (statement.resources.length) {
Annotations.of(this).addWarning('ECR resource policy does not allow resource statements.');
}
if (this.policyDocument === undefined) {
Expand Down
16 changes: 16 additions & 0 deletions packages/aws-cdk-lib/aws-ecr/test/repository.test.ts
Expand Up @@ -386,6 +386,22 @@ describe('repository', () => {
Annotations.fromStack(stack).hasWarning('*', 'ECR resource policy does not allow resource statements.');
});

test('does not warn if repository policy does not have resources', () => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'my-stack');
const repo = new ecr.Repository(stack, 'Repo');

// WHEN
repo.addToResourcePolicy(new iam.PolicyStatement({
actions: ['ecr:*'],
principals: [new iam.AnyPrincipal()],
}));

// THEN
Annotations.fromStack(stack).hasNoWarning('*', 'ECR resource policy does not allow resource statements.');
});

test('default encryption configuration', () => {
// GIVEN
const app = new cdk.App();
Expand Down

0 comments on commit c0c3d19

Please sign in to comment.