Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Revert ECR-outside-AWS default behaviour #2070

Closed
squaremo opened this issue May 22, 2019 · 3 comments
Closed

Revert ECR-outside-AWS default behaviour #2070

squaremo opened this issue May 22, 2019 · 3 comments

Comments

@squaremo
Copy link
Member

Before #1863, if you were using ECR from outside AWS, you would see a warning in the log (from the region detection), then it would proceed as for regular images -- that is, attempt to get credentials from imagePullSecrets etc.

After #1863, if fluxd cannot detect an AWS region and is not told one explicitly, it will ignore all images from ECR.

This change in behaviour comes from the separation of region detection and image inclusion/exclusion (filtering). Before, if a region wasn't detected, no image filtering took place. After, image filtering is done whether or not a region is detected or supplied. This gives rise to the question of what to do if a region is neither detected nor supplied -- and the somewhat arbitrary decision was to treat that as excluding all regions, rather than including all regions.

Thus: the surprising situation in which a working cluster stopped scanning ECR images, when upgraded from flux 1.10 to 1.12, without any config changes.

@gtseres
Copy link
Contributor

gtseres commented Mar 30, 2020

Hello @squaremo! We have been using Flux for quite some time and I have recently started learning Go, so I thought that I could give this a shot if no concern!

Reading through the issue details, I understand that the desired behaviour would be to populate the regions configuration here with a value. Would that value be all the available ECR regions? If that is the case, would using the SSM service to get all the available ECR regions make sense? For example, when using the aws cli, the output is the following:

$ aws ssm get-parameters-by-path   --path /aws/service/global-infrastructure/services/ecr/regions --output json |   jq .Parameters[].Value
"ap-east-1"
"ap-northeast-1"
"ap-south-1"
"ap-southeast-1"
"ap-southeast-2"
"cn-north-1"
"eu-west-2"
"eu-west-3"
"me-south-1"
"sa-east-1"
"ca-central-1"
"cn-northwest-1"
"eu-central-1"
"eu-west-1"
"us-east-1"
"us-east-2"
"us-gov-east-1"
"us-gov-west-1"
"us-west-1"
"us-west-2"
"ap-northeast-2"
"eu-north-1"

Is this idea along the lines with what you have in mind for this or is it in the wrong direction?

If this is in the right direction, I could implement a solution that involves the github.com/aws/aws-sdk-go/service/ssm Go package to get the available regions.

@squaremo
Copy link
Member Author

Hi @gtseres, sorry for the long wait. I think it would suffice to change the shouldScan predicate (in both varieties) so that a nil value for region allowed any region.

@kingdonb
Copy link
Member

This should be addressed in #3124 #3485, which I believe will be released soon in Flux v1.23.2.

Closing as a duplicate of #3015 (that also affects not only instances running outside of AWS, but also fargate users, and EKS users who have locked down access to the instance metadata API according to the best-practices advice.)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants