search

concourse / registry-image-resource

a resource for images in a Docker registry
Apache License 2.0
89 stars 107 forks source link

Relax auth around ECR #323

Closed taylorsilva closed 2 years ago

taylorsilva commented 2 years ago

Fixes #277

Kudo's to @samed and his comment here: https://github.com/concourse/registry-image-resource/issues/277#issuecomment-960285967

Basically the same patch with an added conditional for optionally adding creds if they're present.

I've tested this on a Concourse worker running on AWS with an instance profile that has the relevant ECR permissions. check, get, and put steps all work.

taylorsilva commented 2 years ago

One "gotcha" I thought of is that with this change I'm assuming that anyone that has aws_region WANTS to use ECR authentication. I have a feeling that may not always be true, or at least don't have any evidence to tell me otherwise. Going to make a change, probably adding another field, so ECR auth will trigger when either key ID/secret are set OR a new aws_auth is set.

taylorsilva commented 2 years ago

Looking at the before and after of the code I realized that no one would have set aws_region and NOT also set the key ID/secret. Currently if you set only aws_region then the resource does nothing different and you'll get an auth error. This is also true even if your worker is not running inside aws infrastructure. Therefore I don't think this is a breaking change at all. There's no scenario where someone has JUST aws_region and wouldn't want to also authenticate to ecr.