terraform-aws-modules / terraform-aws-ecr

Terraform module to create AWS ECR resources 🇺🇦
https://registry.terraform.io/modules/terraform-aws-modules/ecr/aws
Apache License 2.0
52 stars 109 forks source link

new feature: add roles to the repository policy #17

Closed briskt closed 1 year ago

briskt commented 1 year ago

Is your request related to a problem? Please describe.

We are currently using our own module to create ECR repositories. It might be good to use and contribute to a more versatile and popular module. A feature that is missing from this fine module is the ability to add roles to the repository policy.

Describe the solution you'd like.

Add a new variable repository_role_arns that accepts one or more IAM role ARNs. If this feature is compatible with this project's design objectives, I could work on a pull request to add this.

Describe alternatives you've considered.

The alternative is to continue using our own module. An alternative implementation would be to add a repository_policy variable that accepts a full policy instead of building in inside the module.

Additional context

This is the policy we use today:

{
    "Version": "2008-10-17",
    "Statement": [
        {
            "Sid": "ECS Pull Access",
            "Effect": "Allow",
            "Principal": {
                "AWS": [
                    "${ecsInstanceRole_arn}",
                    "${ecsServiceRole_arn}"
                ]
            },
            "Action": [
                "ecr:GetDownloadUrlForLayer",
                "ecr:BatchGetImage",
                "ecr:BatchCheckLayerAvailability"
            ]
        },
        {
            "Sid": "CD push/pull",
            "Effect": "Allow",
            "Principal": {
                "AWS": "${cd_user_arn}"
            },
            "Action": [
                "ecr:PutImage",
                "ecr:InitiateLayerUpload",
                "ecr:UploadLayerPart",
                "ecr:CompleteLayerUpload",
                "ecr:GetDownloadUrlForLayer",
                "ecr:BatchGetImage",
                "ecr:BatchCheckLayerAvailability"
            ]
        }
    ]
}

For reference, the above file is here

bryantbiggs commented 1 year ago

does the repository_read_access_arns not work for your use case?

briskt commented 1 year ago

does the repository_read_access_arns not work for your use case?

Ah, yes, I suppose it would. Somehow I was imagining that roles and users were specified differently in a policy.

github-actions[bot] commented 1 year ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.