Add an option to secure the test registry

[pierretasci]

Note: Fat fingers hit enter too early. Sorry.

It would be nice to have an option to allow the test registry located here https://github.com/google/go-containerregistry/blob/master/pkg/registry/registry.go to support an option to secure the registry.

This would allow someone to setup a test where they need to authenticate into the registry.

[jonjohnsonjr]

I'm leaning towards no, but I'd be really interested to hear more about your use case.

Registry auth is pretty complicated, so regardless of what we add to the registry package, I'd worry that it wouldn't really be sufficient to test that the client works against any registry...

What are you trying to test?

[pierretasci]

Oh I am well aware how complicated it is 😃

For context, what we're trying to do is here: tektoncd/pipeline#2137
The gist is to use go-containerregistry to fetch an image where the layers are K8s crds. The image is likely to be located in an authenticated registry and we want to fetch it from a K8s controller.

That means we should have a secret that contains the credentials and we can roll a authn.Keychain implementation that produces an AuthConfig from the Secret but testing it is hard if the test registry doesn't require any authentication. Perhaps there is another way to test it without bringing up a registry?

[jonjohnsonjr]

That means we should have a secret that contains the credentials and we can roll a authn.Keychain implementation that produces an AuthConfig from the Secret

Can you use k8schain for this?

testing it is hard if the test registry doesn't require any authentication

Makes sense. If you're trying to do an e2e test, you might be better off spinning up a real registry and testing against that. In fact, you'd probably want to set up e2e tests against real, production registries (acr, ecr, gcr, dockerhub), since then you're actually be testing real auth flows instead of whatever we add here.

For unit tests, I would probably just test separately that:

1. the keychain returns the expected credentials, and
2. pulling from a registry (without auth) works as expected.

I'm not completely against adding some auth flows here for testing -- it would simplify a lot of my own registry tests -- but I don't think it would be all that useful for tekton.

Does that make sense?

[pierretasci]

Yeah that makes sense. I will try mocking things out into separate functional tests. Thanks for the heads up on the k8schain, that will save me some implementation!

[jonjohnsonjr]

No problem!

Going to close this issue for now but feel free to ping back if you think this is the best way forward.

[dfreilich]

@jonjohnsonjr We (buildpacks/pack, as well as the related projects) are looking into using the test registry for (at least some of) our tests (so we don't have to run an actual registry image, and deal with the headaches that arise from it), and auth/security would be really helpful for us to provide some level of assurance that our images can be built to authenticated registries.

Should we pile onto this issue, or open a new issue to present our case (or get your thoughts on how we can do it ourselves)?

[imjasonh]

In the short term my suggestion would be to fork pkg/registry and add whatever other logic you need for your case.

If it turns out to be reliable and flexible enough that others might benefit from it, feel free to propose a PR at that time.

I think there's obviously benefit in having auth be part of a test registry, but as @jonjohnsonjr said there's also a lot of potential complexity in supporting all the cases. If you can solve it for your case first then we can work on generalizing it.

[jonjohnsonjr]

The registry package is exposed as a standard http.Handler:

go-containerregistry/pkg/registry/registry.go

Line 76 in 6a2e261

func New(opts ...Option) http.Handler {

It should be pretty straightforward to wrap this in some middleware that checks for Authorization headers and returns a 401/403 based on whatever auth model you want.

func terribleAuthMiddleware(h http.Handler) http.Handler {
  return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
    user, pass, ok := r.BasicAuth()
    if !ok {
      w.WriteHeader(http.StatusUnauthorized)
      return
    }
    if user != "AzureDiamond" || pass != "hunter2" {
      w.WriteHeader(http.StatusForbidden)
      return
    }
    h.ServeHTTP(w, r)
  })
}

func main() {
  reg := terribleAuthMiddleware(registry.New())
  log.Fatal(http.ListenAndServe(":1338", reg))
}

[dfreilich]

Thank you for the help!