Handle error message from token server with containerd backend #50176

dmcgowan · 2025-06-12T00:58:54Z

Token server may return an error in the format {"details": "some errors message"}, for example requesting a token with a PAT which does not have enough scope returns {"details":"access token has insufficient scopes"}. Without this change, those details are not returned to the user.

containerd image store: Improve handling of errors returned by the token server during `docker pull/push`

thaJeztah · 2025-06-12T05:48:18Z

Failure looks unrelated, but hadn't seen this one before, so posting;

=== FAIL: amd64.integration-cli TestDockerDaemonSuite/TestDaemonRestartRestoreBridgeNetwork (2.05s)
    docker_cli_network_unix_test.go:1695: 80 port is allocated to old running container, it should failed on allocating to new container
    check_test.go:554: [d967e42e0e2cf] daemon is not started
    --- FAIL: TestDockerDaemonSuite/TestDaemonRestartRestoreBridgeNetwork (2.05s)

edit: known flaky test; tracked in #50168

thaJeztah

left some minor comment about formatting (preventing linters to complain).

Interesting though that the docker hub response uses details (plural); took me a bit to spot that difference, and now I'm wondering if that was a mistake; might be worth adding a comment on the tokenServerError type to outline this (I added a link to the old implementation that we can align with)

thaJeztah · 2025-06-12T09:00:25Z

daemon/containerd/registry_errors.go

+type tokenServerError struct {
+	Details string `json:"details"`
+}


I was curious if there wasn't a definition for this, but looking at the old code from docker/distribution, it looks like this type was added for incorrectly formatted responses that are missing some of the other fields (?); perhaps adding a similar comment here could be useful.

moby/vendor/github.com/docker/distribution/registry/client/errors.go

Lines 64 to 72 in e1d0aac

// For backward compatibility, handle irregularly formatted

// messages that contain a "details" field.

var detailsErr struct {

Details string `json:"details"`

}

err = json.Unmarshal(body, &detailsErr)

if err == nil && detailsErr.Details != "" {

return makeError(statusCode, detailsErr.Details)

}

Looks like the old code also looks for mediatype, and if it doesn't match, it returns the whole body, but not sure if that's a good idea (at least not without a limit-reader;

moby/vendor/github.com/docker/distribution/registry/client/errors.go

Lines 60 to 62 in e1d0aac

if contentType != "application/json" && contentType != "application/vnd.api+json" {

return makeError(statusCode, string(body))

}

I'm slightly curious though if the current type would work for this; I see code further down handling the docker.Errors type (which awkwardly uses a interface{} for details, not a string);

moby/daemon/containerd/registry_errors.go

Lines 36 to 37 in 3b1d2f7

var derr docker.Error

if errors.As(err, &derr) {

moby/vendor/github.com/containerd/containerd/v2/core/remotes/docker/errcode.go

Lines 114 to 118 in 0aa8fe0

// Error provides a wrapper around ErrorCode with extra Details provided.

type Error struct {

Code ErrorCode `json:"code"`

Message string `json:"message"`

Detail interface{} `json:"detail,omitempty"`

☝️ that error looks to be what's now in the distribution-spec (perhaps containerd should use that as well, but not really important); https://github.com/opencontainers/distribution-spec/blob/583e014d15418d839d67f68152bc2c83821770e0/specs-go/v1/error.go#L35-L40

I'm slightly curious though if the current type would work for this

🙈 🤔 never mind; looks like that's using detail (singular) not details (plural); and now I wonder where the plural came from! Was that a mistake on the Docker Hub side, and is that's what's causing the error-details to not be picked up?

This is just an old format from before the distribution-spec. The token server still uses it in some situations, if the token server no longer used then we wouldn't need to consider. The distribution code path today already has a compatibility path for this though.

daemon/containerd/registry_errors.go

Signed-off-by: Derek McGowan <[email protected]>

thaJeztah

LGTM

thaJeztah

LGTM

thaJeztah · 2025-06-13T08:40:05Z

daemon/containerd/registry_errors.go

+						return cerrdefs.ErrPermissionDenied.WithMessage(fmt.Sprintf("%s - %s", docker.ErrorCodeDenied.Message(), tokenErr.Details))
+					}
+				}
+			}
 		} else {


We need to cleanup some of this function; following the logic then, this "else" branch is used to return if err == nil - which we probably should handle at the start.

dmcgowan force-pushed the handle-token-server-error-messages branch from 4587d18 to ae68a4e Compare June 12, 2025 02:00

thaJeztah requested changes Jun 12, 2025

View reviewed changes

Handle error message from token server with containerd backend

941d09e

Signed-off-by: Derek McGowan <[email protected]>

dmcgowan force-pushed the handle-token-server-error-messages branch from ae68a4e to 941d09e Compare June 12, 2025 18:13

thaJeztah approved these changes Jun 12, 2025

View reviewed changes

thaJeztah added area/distribution status/2-code-review kind/bugfix PR's that fix bugs containerd-integration Issues and PRs related to containerd integration area/authentication labels Jun 12, 2025

thaJeztah added this to the 28.3.0 milestone Jun 12, 2025

thaJeztah approved these changes Jun 12, 2025

View reviewed changes

vvoland approved these changes Jun 13, 2025

View reviewed changes

thaJeztah reviewed Jun 13, 2025

View reviewed changes

thaJeztah merged commit 52a54d9 into moby:master Jun 13, 2025
329 of 366 checks passed

vvoland added impact/changelog kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. labels Jun 13, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Handle error message from token server with containerd backend #50176

Handle error message from token server with containerd backend #50176

dmcgowan commented Jun 12, 2025 •

edited by vvoland

Loading

Uh oh!

thaJeztah commented Jun 12, 2025 •

edited

Loading

Uh oh!

thaJeztah left a comment

Uh oh!

thaJeztah Jun 12, 2025

Uh oh!

thaJeztah Jun 12, 2025

Uh oh!

dmcgowan Jun 12, 2025

Uh oh!

Uh oh!

thaJeztah left a comment

Uh oh!

thaJeztah left a comment

Uh oh!

thaJeztah Jun 13, 2025

Uh oh!

Uh oh!

Uh oh!

	// For backward compatibility, handle irregularly formatted
	// messages that contain a "details" field.
	var detailsErr struct {
	Details string `json:"details"`
	}
	err = json.Unmarshal(body, &detailsErr)
	if err == nil && detailsErr.Details != "" {
	return makeError(statusCode, detailsErr.Details)
	}

	if contentType != "application/json" && contentType != "application/vnd.api+json" {
	return makeError(statusCode, string(body))
	}

	// Error provides a wrapper around ErrorCode with extra Details provided.
	type Error struct {
	Code ErrorCode `json:"code"`
	Message string `json:"message"`
	Detail interface{} `json:"detail,omitempty"`

Handle error message from token server with containerd backend #50176

Handle error message from token server with containerd backend #50176

Conversation

dmcgowan commented Jun 12, 2025 • edited by vvoland Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

thaJeztah Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

dmcgowan Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah Jun 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dmcgowan commented Jun 12, 2025 •

edited by vvoland

Loading

thaJeztah commented Jun 12, 2025 •

edited

Loading