From 2091215deb8f3042a19b21bb80fda8df7535a807 Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Wed, 6 Oct 2021 18:37:14 +0100 Subject: [PATCH] Replace ECR scanning TODOs with explanations The TODO items for scanning ECR were based on Flux v1's ECR-specific code for image scanning; but it turns out they are not necessary for a viable implementation. I have removed the TODOs, and given an explanation why it's fit for purpose as it is. Signed-off-by: Michael Bridgen --- controllers/imagerepository_controller.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/controllers/imagerepository_controller.go b/controllers/imagerepository_controller.go index d1d2de90..410894ae 100644 --- a/controllers/imagerepository_controller.go +++ b/controllers/imagerepository_controller.go @@ -205,13 +205,17 @@ func parseAwsImage(image string) (accountId, awsEcrRegion string, ok bool) { } // getAwsEcrLoginAuth obtains authentication for ECR given the account -// ID and region (from the image), assuming it is available via +// ID and region (taken from the image). This assumes that the pod has +// IAM permissions to get an authentication token, which will usually +// be the case if it's running in EKS, and may need additional setup +// otherwise (visit +// https://docs.aws.amazon.com/sdk-for-go/api/aws/session/ as a +// starting point). func getAwsECRLoginAuth(accountId, awsEcrRegion string) (authn.AuthConfig, error) { - // TODO: Still missing from Flux 1: - // - Caching of tokens (one per account/region pair), this fetches a fresh token every time - // - handling of expiry - // - Back-Off in case of errors - // - Possibly: special behaviour for non-global partitions (China, GovCloud) + // No caching of tokens is attempted; the quota for getting an + // auth token is high enough that getting a token every time you + // scan an image is viable for O(1000) images per region. See + // https://docs.aws.amazon.com/general/latest/gr/ecr.html. var authConfig authn.AuthConfig accountIDs := []string{accountId}