Issues if the container registry (serverURL) isn't just the hostname

[simongottschlag]

Hi!

Thank you for the great work of maintaining this package. 🥇

I've been troubleshooting an issue with cosign when it comes to authenticating to the Azure Container Registry. One of the issues I've found is that somewhere along the way, the whole container image URL is passed to your helper and it causes the authentication to fail.

Reference for the method:

docker-credential-acr-env/pkg/credhelper/helper.go

Line 67 in fe33c00

func (a ACRCredHelper) Get(serverURL string) (string, string, error) {

If we look at the same Elastic Container Registry (AWS) helper, we can clearly see them extracting information before passing it to their other functions: https://github.com/awslabs/amazon-ecr-credential-helper/blob/69c85dc22db6511932bbf119e1a0cc5c90c69a7f/ecr-login/ecr.go#L46

I'll create a PR for you to review.

Reference for the cosign issue: sigstore/cosign#1350

[imjasonh]

Hi! 👋

I added that code in #2 and I think I see the bug. I'll send a PR now.

[simongottschlag]

@imjasonh hi! 😊

I was actually experiencing the issue before using your patch. Or at least I think so. 🤔

[simongottschlag]

@imjasonh please take a look at the following before: #4

[imjasonh]

@imjasonh hi! 😊

I was actually experiencing the issue before using your patch. Or at least I think so. 🤔

Yep, I thought I was looking for a bug in Get when it looks like it's actually just the refresh token flow when repos are included. Completely missed your PR, which is much better 😆

[simongottschlag]

@imjasonh Do you think we should still add this even tough we fixed it in go-containerregistry?

[imjasonh]

I'll leave that up to @chrismellard -- the normal way this code is called is with just the hostname and no path component, it was (seemingly) only due to a bug in that library's usage that it was being called in any other way.

In the spirit of Postel's Law it might make sense to be more flexible in what this accepts, but I don't think we need it anymore, since we're doing the right thing now.

[simongottschlag]

Great! It's up to you @chrismellard 👍