cloudposse / terraform-aws-ecr

Terraform Module to manage Docker Container Registries on AWS ECR
https://cloudposse.com/accelerate
Apache License 2.0
185 stars 133 forks source link

Allow cache though enabled repositories to fetch image from upstream #117

Closed mfuhrmeisterDM closed 9 months ago

mfuhrmeisterDM commented 9 months ago

what

Add a principal list (principals_pull_though_access) which are allowed to use specific repositories as pull through cache (import images from upstream). This holds for repositories where one of the strings in prefixes_pull_through_repositories is a prefix of the repository name.

why

We are using ecr-public pull through cache and we want also new images to be downloaded automatically to the cache. Allowed principals for respective repos can use it with the newly introduced variables.

hans-d commented 9 months ago

@mfuhrmeisterDM, thanks ! Debatable if this can be considered "read-only" access.

Perhaps better to add a variable for additional rights / or set the default rights, in that case also allowing someone to add the ecr:TagResource that might be needed for some pull-through setups.

mfuhrmeisterDM commented 9 months ago

@mfuhrmeisterDM, thanks ! Debatable if this can be considered "read-only" access.

Yes I know, and I'm also more on the side that this not read-only.

Perhaps better to add a variable for additional rights / or set the default rights, in that case also allowing someone to add the ecr:TagResource that might be needed for some pull-through setups.

yes good idea, i will prepare something, but it can take until tomorrow.

mfuhrmeisterDM commented 9 months ago

May be I will add another list with prefixes which limits it to specific repositories.

mfuhrmeisterDM commented 9 months ago

@hans-d I think this is my solution, can you please have another look and run the workflow tests and/or add a review?

mfuhrmeisterDM commented 9 months ago

@hans-d any updates regarding this pr?

hans-d commented 9 months ago

/terratest

mfuhrmeisterDM commented 9 months ago

/terratest

edit: looks like I'm not allowed to do this :-(

mfuhrmeisterDM commented 9 months ago

Some unfortunate dance re the readme (see checks output). Perhaps you can help with the check re the policy output as well.

readme is fixed

hans-d commented 9 months ago

/terratest