Skip to content
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

[24.0 backport] Fix setting ServerAddress property in NativeStore #4824

Merged

Conversation

thaJeztah
Copy link
Member

This will return the ServerAddress property when using the NativeStore. This happens when you use docker credential helpers, not the credential store.

The reason this fix is needed is because it needs to be propagated properly down towards moby/moby project in the following logic:

func authorizationCredsFromAuthConfig(authConfig registrytypes.AuthConfig) docker.AuthorizerOpt {
	cfgHost := registry.ConvertToHostname(authConfig.ServerAddress)
	if cfgHost == "" || cfgHost == registry.IndexHostname {
		cfgHost = registry.DefaultRegistryHost
	}

	return docker.WithAuthCreds(func(host string) (string, string, error) {
		if cfgHost != host {
			logrus.WithFields(logrus.Fields{
				"host":    host,
				"cfgHost": cfgHost,
			}).Warn("Host doesn't match")
			return "", "", nil
		}
		if authConfig.IdentityToken != "" {
			return "", authConfig.IdentityToken, nil
		}
		return authConfig.Username, authConfig.Password, nil
	})
}

This logic resides in the following file :
daemon/containerd/resolver.go .

In the case when using the containerd storage feature when setting the cfgHost variable from the authConfig.ServerAddress it will always be empty. Since it will never be returned from the NativeStore currently. Therefore Docker Hub images will work fine, but anything else will fail since the cfgHost will always be the registry.DefaultRegistryHost.

(cherry picked from commit b24e7f8)

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

This will return the ServerAddress property when using the NativeStore.
This happens when you use docker credential helpers, not the credential
store.

The reason this fix is needed is because it needs to be propagated
properly down towards `moby/moby` project in the following logic:

```golang
func authorizationCredsFromAuthConfig(authConfig registrytypes.AuthConfig) docker.AuthorizerOpt {
	cfgHost := registry.ConvertToHostname(authConfig.ServerAddress)
	if cfgHost == "" || cfgHost == registry.IndexHostname {
		cfgHost = registry.DefaultRegistryHost
	}

	return docker.WithAuthCreds(func(host string) (string, string, error) {
		if cfgHost != host {
			logrus.WithFields(logrus.Fields{
				"host":    host,
				"cfgHost": cfgHost,
			}).Warn("Host doesn't match")
			return "", "", nil
		}
		if authConfig.IdentityToken != "" {
			return "", authConfig.IdentityToken, nil
		}
		return authConfig.Username, authConfig.Password, nil
	})
}
```
This logic resides in the following file :
`daemon/containerd/resolver.go` .

In the case when using the containerd storage feature when setting the
`cfgHost` variable from the `authConfig.ServerAddress` it will always be
empty. Since it will never be returned from the NativeStore currently.
Therefore Docker Hub images will work fine, but anything else will fail
since the `cfgHost` will always be the `registry.DefaultRegistryHost`.

Signed-off-by: Eric Bode <[email protected]>
(cherry picked from commit b24e7f8)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah added this to the 24.0.8 milestone Jan 24, 2024
@thaJeztah thaJeztah self-assigned this Jan 24, 2024
@thaJeztah thaJeztah merged commit 02d2f48 into docker:24.0 Jan 24, 2024
74 checks passed
@thaJeztah thaJeztah deleted the 24.0_backport_4653-fix-credential-helper branch January 24, 2024 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants