Accept and ignore 'null' as value for X-Registry-Auth #9028
Conversation
There was a problem hiding this comment.
Any tests that can be added.
@vrothberg WDYT? I'm a bit leery about making this change, but I'm not seeing another way after going through the issue and this PR.
|
@TomSweeneyRedHat I am also not happy about the proposed change, that is why I took a look at docker/moby code to see how they do it (https://github.com/moby/moby/blob/master/api/server/router/distribution/distribution_routes.go#L37)
It looks like they are parsing this value in various places in the code, and therefore they can make the behavior different from case to case. In Podman this code is centralized (of course much better, less copy/paste code), but it is more difficult to handle such exceptional cases. I can of course create an issue in other project (docker-client), and let them solve the problem ("null" as value is definitely wrong), if you find this change quite ugly. |
| @@ -297,7 +297,7 @@ func imageAuthToDockerAuth(authConfig types.DockerAuthConfig) dockerAPITypes.Aut | |||
| func singleAuthHeader(r *http.Request) (map[string]types.DockerAuthConfig, error) { | |||
| authHeader := r.Header.Get(string(XRegistryAuthHeader)) | |||
| authConfig := dockerAPITypes.AuthConfig{} | |||
| if len(authHeader) > 0 { | |||
| if len(authHeader) > 0 && authHeader != "null" { | |||
There was a problem hiding this comment.
Please add a comment to both places that explains why need to ignore "null".
|
@mlegenovic can you run |
|
CI is failing but will be fixed with #9031. Once merged, please rebase. |
docker-client is a library written in Java and used in Eclipse to speak with Docker API. When endpoint /images/search is called, HTTP header attribute X-Registry-Auth has value "null". This is for sure wrong but Docker tolerates this value, and call works. With this patch call works also with Podman. #7857 Signed-off-by: Milivoje Legenovic <[email protected]>
|
Thanks @mlegenovic |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mlegenovic, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
github-actions
bot
added
the
locked - please file new issue/PR
docker-client is a library written in Java and used in Eclipse to
speak with Docker API. When endpoint /images/search is called,
HTTP header attribute X-Registry-Auth has value "null". This is for
sure wrong but Docker tolerates this value, and call works. With this
patch call works also with Podman. #7857
Signed-off-by: Milivoje Legenovic [email protected]