-
Notifications
You must be signed in to change notification settings - Fork 2
fix registry authentication headers encoding #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
i only check it with digitalocean private registry but i require to change the base64 encoding to make it work. Also, during debug, i ve found a bug in ONLY_KNOWN_REGISITRIES logic.
|
Are you sure that this fixes things? I am pretty sure the code already works as is? With Docker Harbor we needed to use base64url as far as I remember. See moby/moby#33434 |
|
I released as 0.9.3 for the ONLY_KNOWN_REGISTRIES part of this patch |
|
Yes, I’ve checked and double-checked with registry.digitalocean.com and ghcr.io. And strictly speaking, the header is not a URL. Also, see: https://stackoverflow.com/a/50755411 |
|
The header is not a URL, the encoding should be a base64url encoded string. Can you produce a minimum reproducer? Or show me logs of an error? |
|
Sure! Also, I’d like to check with you if the current behavior is desirable. Or maybe the proxy doesn’t need to strip the original headers when no auth overrides are set? |
|
The stripping of auth is intentional. I am open to have this as a feature flag, but for us a main pain point was the fact that if we deploy things with user credentials, then those user credentials are then stored in docker swarm. This makes it unpredictable to know which credentials are currently in use in a cluster. This makes things harder if you need to rotate a credential or offboard employees. |
|
The good one fails because it correctly uses the credentials, but they are invalid: The bad one ignores the credentials completely and pulls the image anonymously: |
|
please try to do this with the docker cli. The docker cli uses base64url - pretty sure. But to be sure, please verify your hunch. |
|
How? Do you mean using |
|
what i mean is: Create a swarmgate instance with a docker/config.json that contains the credentials. Then create a context in your docker cli, and point it to swarmgate. Then try to create a service that uses your docker image on a private registry. |
|
Excuse me if I’m wrong, but this is exactly what I did before, and that’s where the error started. Then, after debugging with This was the original error when I tried to deploy a service: |
|
I will verify the behavior on our end hopefully next week. |
|
Nice! I still pretty sure the change will fix the bug but no hurries because I'm running my own fork right now. |
|
do you achieve to verify the behavior finally? |
|
ping |
1 similar comment
|
ping |
|
can we get this merged. This change its working for me during 8 months. |
i only check it with digitalocean private registry but i require to change the base64 encoding to make it work.
Also, during debug, i ve found a bug in ONLY_KNOWN_REGISTRIES logic.