WIP: don't require IndexInfo

Lots to do here; too many wrappers everywhere, which may become easier
when content trust is removed (which adds another level of abstraction)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Author: thaJeztah

cli/command/cli_deprecated.go

@@ -49,6 +49,7 @@ func (*DockerCli) ManifestStore() manifeststore.Store {
 //
 // Deprecated: use [registryclient.NewRegistryClient]. This method is no longer used and will be removed in the next release.
 func (cli *DockerCli) RegistryClient(allowInsecure bool) registryclient.RegistryClient {
+	// FIXME(thaJezttah); this signature is problematic; we should also use a "resolver" everywhere (instead of passing around the creds?)
 	resolver := func(ctx context.Context, index *registry.IndexInfo) registry.AuthConfig {
 		return ResolveAuthConfig(cli.ConfigFile(), index)
 	}

cli/command/image/push.go

@@ -19,7 +19,6 @@ import (
 	"github.com/docker/cli/internal/tui"
 	"github.com/docker/docker/api/types/auxprogress"
 	"github.com/docker/docker/api/types/image"
-	registrytypes "github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/registry"
 	"github.com/morikuni/aec"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -112,13 +111,15 @@ To push the complete multi-platform image, remove the --platform flag.
 		return err
 	}
 
-	// Resolve the Auth config relevant for this server
+	// FIXME(thaJeztah): this os only needed for docker content trust, which needs the raw, non-encoded authconfig
 	authConfig := command.ResolveAuthConfig(dockerCli.ConfigFile(), repoInfo.Index)
-	encodedAuth, err := registrytypes.EncodeAuthConfig(authConfig)
+
+	// Resolve the Auth config relevant for this server
+	encodedAuth, err := command.RetrieveAuthTokenFromImage(dockerCli.ConfigFile(), ref.String())
 	if err != nil {
 		return err
 	}
-	requestPrivilege := command.RegistryAuthenticationPrivilegedFunc(dockerCli, repoInfo.Index, "push")
+	requestPrivilege := command.NewAuthRequester(dockerCli, reference.Domain(ref), "Login prior to push:")
 	options := image.PushOptions{
 		All:           opts.all,
 		RegistryAuth:  encodedAuth,

cli/command/image/trust.go

@@ -166,13 +166,14 @@ func getTrustedPullTargets(cli command.Cli, imgRefAndAuth trust.ImageRefAndAuth)
 
 // imagePullPrivileged pulls the image and displays it to the output
 func imagePullPrivileged(ctx context.Context, cli command.Cli, imgRefAndAuth trust.ImageRefAndAuth, opts PullOptions) error {
+	// TODO(thaJeztah): get rid of this trust.ImageRefAndAuth monstrosity; we're wrapping wrappers around wrappers; all we need here is the image ref (or even less: the registry name)
 	encodedAuth, err := registrytypes.EncodeAuthConfig(*imgRefAndAuth.AuthConfig())
 	if err != nil {
 		return err
 	}
-	requestPrivilege := command.RegistryAuthenticationPrivilegedFunc(cli, imgRefAndAuth.RepoInfo().Index, "pull")
+	requestPrivilege := command.NewAuthRequester(cli, reference.Domain(imgRefAndAuth.Reference()), "Login prior to pull:")
 	responseBody, err := cli.Client().ImagePull(ctx, reference.FamiliarString(imgRefAndAuth.Reference()), image.PullOptions{
-		RegistryAuth:  encodedAuth,
+		RegistryAuth:  encodedAuth, // TODO: can we get rid of this one altogether, and always use PrivilegeFunc to lazily fetch creds? (maybe we need a way to tell it to either use "stored", or request login)
 		PrivilegeFunc: requestPrivilege,
 		All:           opts.all,
 		Platform:      opts.platform,

cli/command/plugin/install.go

@@ -11,8 +11,6 @@ import (
 	"github.com/docker/cli/cli/command/image"
 	"github.com/docker/cli/cli/internal/jsonstream"
 	"github.com/docker/docker/api/types"
-	registrytypes "github.com/docker/docker/api/types/registry"
-	"github.com/docker/docker/registry"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
@@ -64,7 +62,9 @@ func buildPullConfig(ctx context.Context, dockerCli command.Cli, opts pluginOpti
 		return types.PluginInstallOptions{}, err
 	}
 
-	repoInfo, err := registry.ParseRepositoryInfo(ref)
+	// TODO(thaJeztah): looks like we need to do this here, because "ref" is mutated further below? (because .. docker content trust?)
+	privilegeFunc := command.NewAuthRequester(dockerCli, reference.Domain(ref), fmt.Sprintf("Login prior to %s:", cmdName))
+	encodedAuth, err := command.RetrieveAuthTokenFromImage(dockerCli.ConfigFile(), ref.String())
 	if err != nil {
 		return types.PluginInstallOptions{}, err
 	}
@@ -86,19 +86,13 @@ func buildPullConfig(ctx context.Context, dockerCli command.Cli, opts pluginOpti
 		remote = reference.FamiliarString(trusted)
 	}
 
-	authConfig := command.ResolveAuthConfig(dockerCli.ConfigFile(), repoInfo.Index)
-	encodedAuth, err := registrytypes.EncodeAuthConfig(authConfig)
-	if err != nil {
-		return types.PluginInstallOptions{}, err
-	}
-
 	options := types.PluginInstallOptions{
 		RegistryAuth:          encodedAuth,
 		RemoteRef:             remote,
 		Disabled:              opts.disable,
 		AcceptAllPermissions:  opts.grantPerms,
 		AcceptPermissionsFunc: acceptPrivileges(dockerCli, opts.remote),
-		PrivilegeFunc:         command.RegistryAuthenticationPrivilegedFunc(dockerCli, repoInfo.Index, cmdName),
+		PrivilegeFunc:         privilegeFunc,
 		Args:                  opts.args,
 	}
 	return options, nil

cli/command/plugin/push.go

@@ -8,7 +8,6 @@ import (
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/internal/jsonstream"
 	"github.com/docker/cli/cli/trust"
-	registrytypes "github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/registry"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -53,8 +52,10 @@ func runPush(ctx context.Context, dockerCli command.Cli, opts pushOptions) error
 	if err != nil {
 		return err
 	}
+
+	// FIXME(thaJeztah): this is now only used by docker content trust below
 	authConfig := command.ResolveAuthConfig(dockerCli.ConfigFile(), repoInfo.Index)
-	encodedAuth, err := registrytypes.EncodeAuthConfig(authConfig)
+	encodedAuth, err := command.RetrieveAuthTokenFromImage(dockerCli.ConfigFile(), named.String())
 	if err != nil {
 		return err
 	}

cli/command/registry.go

@@ -33,13 +33,39 @@ const (
 // [registry.IndexServer]: https://pkg.go.dev/github.com/docker/docker/registry#IndexServer
 const authConfigKey = "https:/index.docker.io/v1/"
 
+// NewAuthRequester returns a RequestPrivilegeFunc for the specified registry
+// and the given cmdName (used as informational message to the user).
+//
+// The returned function is a [registrytypes.RequestAuthConfig] to prompt the user
+// for credentials if needed. It is called as fallback if the credentials (if any)
+// used for the initial operation did not work.
+//
+// TODO(thaJeztah): cli Cli could be a Streams if it was not for cli.SetIn to be needed?
+// TODO(thaJeztah): ideally, this would accept reposName / imageRef as a regular string (we can parse it if needed!), or .. maybe generics and accept either?
+func NewAuthRequester(cli Cli, indexServer string, promptMsg string) registrytypes.RequestAuthConfig {
+	configKey := getAuthConfigKey(indexServer)
+	return newPrivilegeFunc(cli, configKey, promptMsg)
+}
+
 // RegistryAuthenticationPrivilegedFunc returns a RequestPrivilegeFunc from the specified registry index info
 // for the given command.
+//
+// Deprecated: this function was only used internally and will be removed in the next release. Use
 func RegistryAuthenticationPrivilegedFunc(cli Cli, index *registrytypes.IndexInfo, cmdName string) registrytypes.RequestAuthConfig {
-	indexServer := getAuthConfigKey(index.Name)
-	isDefaultRegistry := indexServer == authConfigKey || index.Official
+	indexServer := index.Name
+	configKey := getAuthConfigKey(indexServer)
+	if index.Official {
+		configKey = authConfigKey
+	}
+	promptMsg := fmt.Sprintf("Login prior to %s:", cmdName)
+	return newPrivilegeFunc(cli, configKey, promptMsg)
+}
+
+func newPrivilegeFunc(cli Cli, indexServer string, promptMsg string) registrytypes.RequestAuthConfig {
 	return func(ctx context.Context) (string, error) {
-		_, _ = fmt.Fprintf(cli.Out(), "\nLogin prior to %s:\n", cmdName)
+		// TODO(thaJeztah): can we make the prompt an argument? ("prompt func()" or "prompt func()?
+		_, _ = fmt.Fprintf(cli.Out(), "\n"+promptMsg+"\n")
+		isDefaultRegistry := indexServer == authConfigKey
 		authConfig, err := GetDefaultAuthConfig(cli.ConfigFile(), true, indexServer, isDefaultRegistry)
 		if err != nil {
 			_, _ = fmt.Fprintf(cli.Err(), "Unable to retrieve stored credentials for %s, error: %s.\n", authConfigKey, err)
@@ -66,7 +92,8 @@ func RegistryAuthenticationPrivilegedFunc(cli Cli, index *registrytypes.IndexInf
 // It is similar to [registry.ResolveAuthConfig], but uses the credentials-
 // store, instead of looking up credentials from a map.
 func ResolveAuthConfig(cfg *configfile.ConfigFile, index *registrytypes.IndexInfo) registrytypes.AuthConfig {
-	configKey := index.Name
+	indexServer := index.Name
+	configKey := getAuthConfigKey(indexServer)
 	if index.Official {
 		configKey = authConfigKey
 	}
@@ -79,6 +106,7 @@ func ResolveAuthConfig(cfg *configfile.ConfigFile, index *registrytypes.IndexInf
 // If credentials for given serverAddress exists in the credential store, the configuration will be populated with values in it
 func GetDefaultAuthConfig(cfg *configfile.ConfigFile, checkCredStore bool, serverAddress string, isDefaultRegistry bool) (registrytypes.AuthConfig, error) {
 	if !isDefaultRegistry {
+		// FIXME(thaJeztah): should the same logic be used for getAuthConfigKey ?? Looks like we're normalizing things here, but not elsewhere? why?
 		serverAddress = credentials.ConvertToHostname(serverAddress)
 	}
 	authconfig := configtypes.AuthConfig{}
@@ -122,6 +150,8 @@ func ConfigureAuth(ctx context.Context, cli Cli, flUser, flPassword string, auth
 // If defaultUsername is not empty, the username prompt includes that username
 // and the user can hit enter without inputting a username  to use that default
 // username.
+//
+// TODO(thaJeztah): cli Cli could be a Streams if it was not for cli.SetIn to be needed?
 func PromptUserForCredentials(ctx context.Context, cli Cli, argUser, argPassword, defaultUsername, serverAddress string) (registrytypes.AuthConfig, error) {
 	// On Windows, force the use of the regular OS stdin stream.
 	//
@@ -211,12 +241,15 @@ func PromptUserForCredentials(ctx context.Context, cli Cli, argUser, argPassword
 //
 // For details on base64url encoding, see:
 // - RFC4648, section 5:   https://tools.ietf.org/html/rfc4648#section-5
+//
+// FIXME(thaJeztah): do we need a variant like this, but with "indexServer" (domainName) as input?
 func RetrieveAuthTokenFromImage(cfg *configfile.ConfigFile, image string) (string, error) {
 	// Retrieve encoded auth token from the image reference
 	authConfig, err := resolveAuthConfigFromImage(cfg, image)
 	if err != nil {
 		return "", err
 	}
+	// FIXME(thaJeztah); implement stringer (.String()) or "MarshalText / UnmarshalText" on AuthConfig, so that we can pass it around as-is, and encode where it needs to be encoded (when making requests).
 	encodedAuth, err := registrytypes.EncodeAuthConfig(authConfig)
 	if err != nil {
 		return "", err
@@ -225,6 +258,8 @@ func RetrieveAuthTokenFromImage(cfg *configfile.ConfigFile, image string) (strin
 }
 
 // resolveAuthConfigFromImage retrieves that AuthConfig using the image string
+//
+// TODO(thaJeztah): export this and/or accept an image ref-type, and use instead of ResolveAuthConfig
 func resolveAuthConfigFromImage(cfg *configfile.ConfigFile, image string) (registrytypes.AuthConfig, error) {
 	registryRef, err := reference.ParseNormalizedNamed(image)
 	if err != nil {

cli/command/registry/search.go

@@ -52,6 +52,7 @@ func runSearch(ctx context.Context, dockerCli command.Cli, options searchOptions
 	if options.filter.Value().Contains("is-automated") {
 		_, _ = fmt.Fprintln(dockerCli.Err(), `WARNING: the "is-automated" filter is deprecated, and searching for "is-automated=true" will not yield any results in future.`)
 	}
+	// FIXME(thaJeztah): we need some equivalent of "splitReposSearchTerm" to get the registry hostname from the search term.
 	indexInfo, err := registry.ParseSearchIndexInfo(options.term)
 	if err != nil {
 		return err
@@ -63,7 +64,8 @@ func runSearch(ctx context.Context, dockerCli command.Cli, options searchOptions
 		return err
 	}
 
-	requestPrivilege := command.RegistryAuthenticationPrivilegedFunc(dockerCli, indexInfo, "search")
+	indexServer := indexInfo.Name
+	requestPrivilege := command.NewAuthRequester(dockerCli, indexServer, "Login prior to search:")
 	results, err := dockerCli.Client().ImageSearch(ctx, options.term, registrytypes.SearchOptions{
 		RegistryAuth:  encodedAuth,
 		PrivilegeFunc: requestPrivilege,

cli/command/trust/sign.go

@@ -82,7 +82,7 @@ func runSignImage(ctx context.Context, dockerCLI command.Cli, options signOption
 			return trust.NotaryError(imgRefAndAuth.RepoInfo().Name.Name(), err)
 		}
 	}
-	requestPrivilege := command.RegistryAuthenticationPrivilegedFunc(dockerCLI, imgRefAndAuth.RepoInfo().Index, "push")
+	requestPrivilege := command.NewAuthRequester(dockerCLI, reference.Domain(imgRefAndAuth.Reference()), "Login prior to push:")
 	target, err := createTarget(notaryRepo, imgRefAndAuth.Tag())
 	if err != nil || options.local {
 		switch err := err.(type) {