Fix setting ServerAddress property in NativeStore

[StealthyCoder]

• fixes docker credential helpers don't work with containerd image store #4653

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.

- 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)

[thaJeztah]

Thanks! We also merged a fix on the daemon-side in moby/moby#46779

Perhaps both won't do harm though

/cc @dmcgowan

[codecov-commenter]

Codecov Report

Merging #4655 (b24e7f8) into master (a9ae9b3) will increase coverage by 0.00%.
Report is 4 commits behind head on master.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4655   +/-   ##
=======================================
  Coverage   59.74%   59.75%           
=======================================
  Files         288      288           
  Lines       24849    24853    +4     
=======================================
+ Hits        14846    14850    +4     
  Misses       9117     9117           
  Partials      886      886           

[dmcgowan]

is ac.ServerAddress ever a non-empty value?

[StealthyCoder]

I do not believe so no. Since in both stores the code always sets it to what the Registry Hostname is. Is your idea to just have creds be returned there directly? 🙂

[thaJeztah]

@dmcgowan do you mean you prefer to make this conditional to keep that option open?

Something like this?;

if ac.ServerAddress  == "" {
    ac.ServerAddress = creds.ServerAddress
}

[StealthyCoder]

I had not read it that way yet. That makes sense. So we only set it when it is empty and not accidentally overwrite a potential value. I can add that real quick.

[dmcgowan]

@thaJeztah agreed, both changes make sense

[thaJeztah]

LGTM, thanks!

And thanks for the extra eyes, @dmcgowan