(ecr): (false positive policytext errors from Python since #24401)

[michael-k]

Describe the bug

#24401 introduced a warning when an ECR resource policy contains resources. When adding a resource policy from Python (haven't tried other programming languages) without resources, the warning is triggered nevertheless.

Expected Behavior

No warning.

Current Behavior

False-positive warning:

[Warning at /cdk-ecr-resource-policy/Repository] ECR resource policy does not allow resource statements.

Reproduction Steps

A sample repository is here: https://github.com/michael-k/cdk-ecr-resource-policy

The warning can be seen in the GitHub Actions output here: https://github.com/michael-k/cdk-ecr-resource-policy/actions/runs/4668379710/jobs/8265393358#step:10:13

Sample code:

from aws_cdk import Stack, aws_ecr as ecr, aws_iam as iam
from constructs import Construct


class CdkEcrResourcePolicyStack(Stack):
    def __init__(self, scope: Construct, construct_id: str, **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)

        repository = ecr.Repository(
            self,
            "Repository",
        )

        repository.add_to_resource_policy(
            iam.PolicyStatement(
                sid="AllowPullFromOtherAccounts",
                actions=[
                    "ecr:BatchCheckLayerAvailability",
                    "ecr:BatchGetImage",
                    "ecr:GetDownloadUrlForLayer",
                ],
                principals=[
                    iam.AccountPrincipal("111122223333"),
                    iam.AccountPrincipal("123456789012"),
                ],
                effect=iam.Effect.ALLOW,
            )
        )

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.73.0 (build 43e681e)

Framework Version

2.73.0

Node.js Version

16

OS

Linux

Language

Python

Language Version

3.10.11

Other information

Synthesized CloudFormation template:

Resources:
  Repository22E53BBD:
    Type: AWS::ECR::Repository
    Properties:
      RepositoryPolicyText:
        Statement:
          - Action:
              - ecr:BatchCheckLayerAvailability
              - ecr:BatchGetImage
              - ecr:GetDownloadUrlForLayer
            Effect: Allow
            Principal:
              AWS:
                - Fn::Join:
                    - ""
                    - - "arn:"
                      - Ref: AWS::Partition
                      - :iam::111122223333:root
                - Fn::Join:
                    - ""
                    - - "arn:"
                      - Ref: AWS::Partition
                      - :iam::123456789012:root
            Sid: AllowPullFromOtherAccounts
        Version: "2012-10-17"
    UpdateReplacePolicy: Retain
    DeletionPolicy: Retain
    Metadata:
      aws:cdk:path: cdk-ecr-resource-policy/Repository/Resource
Parameters:
  BootstrapVersion:
    Type: AWS::SSM::Parameter::Value<String>
    Default: /cdk-bootstrap/hnb659fds/version
    Description: Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]
Rules:
  CheckBootstrapVersion:
    Assertions:
      - Assert:
          Fn::Not:
            - Fn::Contains:
                - - "1"
                  - "2"
                  - "3"
                  - "4"
                  - "5"
                - Ref: BootstrapVersion
        AssertDescription: CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI.

[peterwoodworth]

This checks out. Thanks for reporting!

PolicyStatement.resources always returns an array, which is truthy. This check needs to be adjusted, and we need to add a test that verifies the warning doesn't throw when it's not supposed to

[scarytom]

I can confirm that this issue also affects the Java SDK.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.