Merge pull request #1642 from tiborvass/better-dep-graph-auth-type

thaJeztah · web-flow · commit 234462756460 · 2019-01-31T23:37:13.000+01:00
Remove docker api dependency from cli/config
diff --git a/cli/command/container/create.go b/cli/command/container/create.go
@@ -62,7 +62,7 @@ func NewCreateCommand(dockerCli command.Cli) *cobra.Command {
 }
 
 func runCreate(dockerCli command.Cli, flags *pflag.FlagSet, options *createOptions, copts *containerOptions) error {
-	proxyConfig := dockerCli.ConfigFile().ParseProxyConfig(dockerCli.Client().DaemonHost(), copts.env.GetAll())
+	proxyConfig := dockerCli.ConfigFile().ParseProxyConfig(dockerCli.Client().DaemonHost(), opts.ConvertKVStringsToMapWithNil(copts.env.GetAll()))
 	newEnv := []string{}
 	for k, v := range proxyConfig {
 		if v == nil {
diff --git a/cli/command/container/run.go b/cli/command/container/run.go
@@ -68,7 +68,7 @@ func NewRunCommand(dockerCli command.Cli) *cobra.Command {
 }
 
 func runRun(dockerCli command.Cli, flags *pflag.FlagSet, ropts *runOptions, copts *containerOptions) error {
-	proxyConfig := dockerCli.ConfigFile().ParseProxyConfig(dockerCli.Client().DaemonHost(), copts.env.GetAll())
+	proxyConfig := dockerCli.ConfigFile().ParseProxyConfig(dockerCli.Client().DaemonHost(), opts.ConvertKVStringsToMapWithNil(copts.env.GetAll()))
 	newEnv := []string{}
 	for k, v := range proxyConfig {
 		if v == nil {
diff --git a/cli/command/image/build.go b/cli/command/image/build.go
@@ -383,7 +383,11 @@ func runBuild(dockerCli command.Cli, options buildOptions) error {
 	}
 
 	configFile := dockerCli.ConfigFile()
-	authConfigs, _ := configFile.GetAllCredentials()
+	creds, _ := configFile.GetAllCredentials()
+	authConfigs := make(map[string]types.AuthConfig, len(creds))
+	for k, auth := range creds {
+		authConfigs[k] = types.AuthConfig(auth)
+	}
 	buildOptions := imageBuildOptions(dockerCli, options)
 	buildOptions.Version = types.BuilderV1
 	buildOptions.Dockerfile = relDockerfile
@@ -619,7 +623,7 @@ func imageBuildOptions(dockerCli command.Cli, options buildOptions) types.ImageB
 		CgroupParent:   options.cgroupParent,
 		ShmSize:        options.shmSize.Value(),
 		Ulimits:        options.ulimits.GetList(),
-		BuildArgs:      configFile.ParseProxyConfig(dockerCli.Client().DaemonHost(), options.buildArgs.GetAll()),
+		BuildArgs:      configFile.ParseProxyConfig(dockerCli.Client().DaemonHost(), opts.ConvertKVStringsToMapWithNil(options.buildArgs.GetAll())),
 		Labels:         opts.ConvertKVStringsToMap(options.labels.GetAll()),
 		CacheFrom:      options.cacheFrom,
 		SecurityOpt:    options.securityOpt,
diff --git a/cli/command/registry.go b/cli/command/registry.go
@@ -11,6 +11,7 @@ import (
 	"runtime"
 	"strings"
 
+	configtypes "github.com/docker/cli/cli/config/types"
 	"github.com/docker/cli/cli/debug"
 	"github.com/docker/cli/cli/streams"
 	"github.com/docker/distribution/reference"
@@ -77,7 +78,7 @@ func ResolveAuthConfig(ctx context.Context, cli Cli, index *registrytypes.IndexI
 	}
 
 	a, _ := cli.ConfigFile().GetAuthConfig(configKey)
-	return a
+	return types.AuthConfig(a)
 }
 
 // GetDefaultAuthConfig gets the default auth config given a serverAddress
@@ -86,16 +87,17 @@ func GetDefaultAuthConfig(cli Cli, checkCredStore bool, serverAddress string, is
 	if !isDefaultRegistry {
 		serverAddress = registry.ConvertToHostname(serverAddress)
 	}
-	var authconfig types.AuthConfig
+	var authconfig configtypes.AuthConfig
 	var err error
 	if checkCredStore {
 		authconfig, err = cli.ConfigFile().GetAuthConfig(serverAddress)
 	} else {
-		authconfig = types.AuthConfig{}
+		authconfig = configtypes.AuthConfig{}
 	}
 	authconfig.ServerAddress = serverAddress
 	authconfig.IdentityToken = ""
-	return &authconfig, err
+	res := types.AuthConfig(authconfig)
+	return &res, err
 }
 
 // ConfigureAuth handles prompting of user's username and password if needed
diff --git a/cli/command/registry/login.go b/cli/command/registry/login.go
@@ -8,6 +8,7 @@ import (
 
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
+	configtypes "github.com/docker/cli/cli/config/types"
 	"github.com/docker/docker/api/types"
 	registrytypes "github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/client"
@@ -149,7 +150,7 @@ func runLogin(dockerCli command.Cli, opts loginOptions) error { //nolint: gocycl
 		}
 	}
 
-	if err := creds.Store(*authConfig); err != nil {
+	if err := creds.Store(configtypes.AuthConfig(*authConfig)); err != nil {
 		return errors.Errorf("Error saving credentials: %v", err)
 	}
 
diff --git a/cli/command/registry/login_test.go b/cli/command/registry/login_test.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"testing"
 
+	configtypes "github.com/docker/cli/cli/config/types"
 	"github.com/docker/cli/internal/test"
 	"github.com/docker/docker/api/types"
 	registrytypes "github.com/docker/docker/api/types/registry"
@@ -79,21 +80,21 @@ func TestRunLogin(t *testing.T) {
 	const validPassword = "p1"
 	const validPassword2 = "p2"
 
-	validAuthConfig := types.AuthConfig{
+	validAuthConfig := configtypes.AuthConfig{
 		ServerAddress: storedServerAddress,
 		Username:      validUsername,
 		Password:      validPassword,
 	}
-	expiredAuthConfig := types.AuthConfig{
+	expiredAuthConfig := configtypes.AuthConfig{
 		ServerAddress: storedServerAddress,
 		Username:      validUsername,
 		Password:      expiredPassword,
 	}
 	testCases := []struct {
 		inputLoginOption  loginOptions
-		inputStoredCred   *types.AuthConfig
+		inputStoredCred   *configtypes.AuthConfig
 		expectedErr       string
-		expectedSavedCred types.AuthConfig
+		expectedSavedCred configtypes.AuthConfig
 	}{
 		{
 			inputLoginOption: loginOptions{
@@ -118,7 +119,7 @@ func TestRunLogin(t *testing.T) {
 			},
 			inputStoredCred: &validAuthConfig,
 			expectedErr:     "",
-			expectedSavedCred: types.AuthConfig{
+			expectedSavedCred: configtypes.AuthConfig{
 				ServerAddress: storedServerAddress,
 				Username:      validUsername,
 				Password:      validPassword2,
diff --git a/cli/command/registry_test.go b/cli/command/registry_test.go
@@ -13,6 +13,7 @@ import (
 	// Prevents a circular import with "github.com/docker/cli/internal/test"
 
 	. "github.com/docker/cli/cli/command"
+	configtypes "github.com/docker/cli/cli/config/types"
 	"github.com/docker/cli/cli/debug"
 	"github.com/docker/cli/internal/test"
 	"github.com/docker/docker/api/types"
@@ -134,7 +135,7 @@ func TestGetDefaultAuthConfig(t *testing.T) {
 	errBuf := new(bytes.Buffer)
 	cli.SetErr(errBuf)
 	for _, authconfig := range testAuthConfigs {
-		cli.ConfigFile().GetCredentialsStore(authconfig.ServerAddress).Store(authconfig)
+		cli.ConfigFile().GetCredentialsStore(authconfig.ServerAddress).Store(configtypes.AuthConfig(authconfig))
 	}
 	for _, tc := range testCases {
 		serverAddress := tc.inputServerAddress
diff --git a/cli/config/config.go b/cli/config/config.go
@@ -8,7 +8,7 @@ import (
 
 	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/cli/cli/config/credentials"
-	"github.com/docker/docker/api/types"
+	"github.com/docker/cli/cli/config/types"
 	"github.com/docker/docker/pkg/homedir"
 	"github.com/pkg/errors"
 )
diff --git a/cli/config/configfile/file.go b/cli/config/configfile/file.go
@@ -11,8 +11,7 @@ import (
 	"strings"
 
 	"github.com/docker/cli/cli/config/credentials"
-	"github.com/docker/cli/opts"
-	"github.com/docker/docker/api/types"
+	"github.com/docker/cli/cli/config/types"
 	"github.com/pkg/errors"
 )
 
@@ -196,7 +195,7 @@ func (configFile *ConfigFile) Save() error {
 
 // ParseProxyConfig computes proxy configuration by retrieving the config for the provided host and
 // then checking this against any environment variables provided to the container
-func (configFile *ConfigFile) ParseProxyConfig(host string, runOpts []string) map[string]*string {
+func (configFile *ConfigFile) ParseProxyConfig(host string, runOpts map[string]*string) map[string]*string {
 	var cfgKey string
 
 	if _, ok := configFile.Proxies[host]; !ok {
@@ -212,7 +211,10 @@ func (configFile *ConfigFile) ParseProxyConfig(host string, runOpts []string) ma
 		"NO_PROXY":    &config.NoProxy,
 		"FTP_PROXY":   &config.FTPProxy,
 	}
-	m := opts.ConvertKVStringsToMapWithNil(runOpts)
+	m := runOpts
+	if m == nil {
+		m = make(map[string]*string)
+	}
 	for k := range permitted {
 		if *permitted[k] == "" {
 			continue
diff --git a/cli/config/configfile/file_test.go b/cli/config/configfile/file_test.go
@@ -1,13 +1,12 @@
 package configfile
 
 import (
-	"fmt"
 	"io/ioutil"
 	"os"
 	"testing"
 
 	"github.com/docker/cli/cli/config/credentials"
-	"github.com/docker/docker/api/types"
+	"github.com/docker/cli/cli/config/types"
 	"gotest.tools/assert"
 	is "gotest.tools/assert/cmp"
 )
@@ -41,7 +40,7 @@ func TestProxyConfig(t *testing.T) {
 		},
 	}
 
-	proxyConfig := cfg.ParseProxyConfig("/var/run/docker.sock", []string{})
+	proxyConfig := cfg.ParseProxyConfig("/var/run/docker.sock", nil)
 	expected := map[string]*string{
 		"HTTP_PROXY":  &httpProxy,
 		"http_proxy":  &httpProxy,
@@ -75,9 +74,14 @@ func TestProxyConfigOverride(t *testing.T) {
 		},
 	}
 
-	ropts := []string{
-		fmt.Sprintf("HTTP_PROXY=%s", overrideHTTPProxy),
-		"NO_PROXY=",
+	clone := func(s string) *string {
+		s2 := s
+		return &s2
+	}
+
+	ropts := map[string]*string{
+		"HTTP_PROXY": clone(overrideHTTPProxy),
+		"NO_PROXY":   clone(overrideNoProxy),
 	}
 	proxyConfig := cfg.ParseProxyConfig("/var/run/docker.sock", ropts)
 	expected := map[string]*string{
@@ -124,7 +128,7 @@ func TestProxyConfigPerHost(t *testing.T) {
 		},
 	}
 
-	proxyConfig := cfg.ParseProxyConfig("tcp://example.docker.com:2376", []string{})
+	proxyConfig := cfg.ParseProxyConfig("tcp://example.docker.com:2376", nil)
 	expected := map[string]*string{
 		"HTTP_PROXY":  &extHTTPProxy,
 		"http_proxy":  &extHTTPProxy,
diff --git a/cli/config/credentials/credentials.go b/cli/config/credentials/credentials.go
@@ -1,7 +1,7 @@
 package credentials
 
 import (
-	"github.com/docker/docker/api/types"
+	"github.com/docker/cli/cli/config/types"
 )
 
 // Store is the interface that any credentials store must implement.
diff --git a/cli/config/credentials/file_store.go b/cli/config/credentials/file_store.go
@@ -1,8 +1,9 @@
 package credentials
 
 import (
-	"github.com/docker/docker/api/types"
-	"github.com/docker/docker/registry"
+	"strings"
+
+	"github.com/docker/cli/cli/config/types"
 )
 
 type store interface {
@@ -35,7 +36,7 @@ func (c *fileStore) Get(serverAddress string) (types.AuthConfig, error) {
 		// Maybe they have a legacy config file, we will iterate the keys converting
 		// them to the new format and testing
 		for r, ac := range c.file.GetAuthConfigs() {
-			if serverAddress == registry.ConvertToHostname(r) {
+			if serverAddress == ConvertToHostname(r) {
 				return ac, nil
 			}
 		}
@@ -62,3 +63,19 @@ func (c *fileStore) GetFilename() string {
 func (c *fileStore) IsFileStore() bool {
 	return true
 }
+
+// ConvertToHostname converts a registry url which has http|https prepended
+// to just an hostname.
+// Copied from github.com/docker/docker/registry.ConvertToHostname to reduce dependencies.
+func ConvertToHostname(url string) string {
+	stripped := url
+	if strings.HasPrefix(url, "http://") {
+		stripped = strings.TrimPrefix(url, "http://")
+	} else if strings.HasPrefix(url, "https://") {
+		stripped = strings.TrimPrefix(url, "https://")
+	}
+
+	nameParts := strings.SplitN(stripped, "/", 2)
+
+	return nameParts[0]
+}
diff --git a/cli/config/credentials/file_store_test.go b/cli/config/credentials/file_store_test.go
@@ -3,7 +3,7 @@ package credentials
 import (
 	"testing"
 
-	"github.com/docker/docker/api/types"
+	"github.com/docker/cli/cli/config/types"
 	"gotest.tools/assert"
 	is "gotest.tools/assert/cmp"
 )
diff --git a/cli/config/credentials/native_store.go b/cli/config/credentials/native_store.go
@@ -1,9 +1,9 @@
 package credentials
 
 import (
+	"github.com/docker/cli/cli/config/types"
 	"github.com/docker/docker-credential-helpers/client"
 	"github.com/docker/docker-credential-helpers/credentials"
-	"github.com/docker/docker/api/types"
 )
 
 const (
diff --git a/cli/config/credentials/native_store_test.go b/cli/config/credentials/native_store_test.go
@@ -8,9 +8,9 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/docker/cli/cli/config/types"
 	"github.com/docker/docker-credential-helpers/client"
 	"github.com/docker/docker-credential-helpers/credentials"
-	"github.com/docker/docker/api/types"
 	"github.com/pkg/errors"
 	"gotest.tools/assert"
 	is "gotest.tools/assert/cmp"
diff --git a/cli/config/types/authconfig.go b/cli/config/types/authconfig.go
@@ -0,0 +1,22 @@
+package types
+
+// AuthConfig contains authorization information for connecting to a Registry
+type AuthConfig struct {
+	Username string `json:"username,omitempty"`
+	Password string `json:"password,omitempty"`
+	Auth     string `json:"auth,omitempty"`
+
+	// Email is an optional value associated with the username.
+	// This field is deprecated and will be removed in a later
+	// version of docker.
+	Email string `json:"email,omitempty"`
+
+	ServerAddress string `json:"serveraddress,omitempty"`
+
+	// IdentityToken is used to authenticate the user and get
+	// an access token for the registry.
+	IdentityToken string `json:"identitytoken,omitempty"`
+
+	// RegistryToken is a bearer token to be sent to a registry
+	RegistryToken string `json:"registrytoken,omitempty"`
+}
diff --git a/internal/test/store.go b/internal/test/store.go
@@ -2,7 +2,7 @@ package test
 
 import (
 	"github.com/docker/cli/cli/config/credentials"
-	"github.com/docker/docker/api/types"
+	"github.com/docker/cli/cli/config/types"
 )
 
 // FakeStore implements a credentials.Store that only acts as an in memory map

Original file line number	Diff line number	Diff line change
`@@ -62,7 +62,7 @@ func NewCreateCommand(dockerCli command.Cli) *cobra.Command {`
`62`	`62`	`}`
`63`	`63`
`64`	`64`	`func runCreate(dockerCli command.Cli, flags pflag.FlagSet, options createOptions, copts *containerOptions) error {`
`65`		`- proxyConfig := dockerCli.ConfigFile().ParseProxyConfig(dockerCli.Client().DaemonHost(), copts.env.GetAll())`
	`65`	`+ proxyConfig := dockerCli.ConfigFile().ParseProxyConfig(dockerCli.Client().DaemonHost(), opts.ConvertKVStringsToMapWithNil(copts.env.GetAll()))`
`66`	`66`	`newEnv := []string{}`
`67`	`67`	`for k, v := range proxyConfig {`
`68`	`68`	`if v == nil {`
Original file line number	Diff line number	Diff line change
`@@ -68,7 +68,7 @@ func NewRunCommand(dockerCli command.Cli) *cobra.Command {`
`68`	`68`	`}`
`69`	`69`
`70`	`70`	`func runRun(dockerCli command.Cli, flags pflag.FlagSet, ropts runOptions, copts *containerOptions) error {`
`71`		`- proxyConfig := dockerCli.ConfigFile().ParseProxyConfig(dockerCli.Client().DaemonHost(), copts.env.GetAll())`
	`71`	`+ proxyConfig := dockerCli.ConfigFile().ParseProxyConfig(dockerCli.Client().DaemonHost(), opts.ConvertKVStringsToMapWithNil(copts.env.GetAll()))`
`72`	`72`	`newEnv := []string{}`
`73`	`73`	`for k, v := range proxyConfig {`
`74`	`74`	`if v == nil {`
Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,7 @@ import (`
`8`	`8`
`9`	`9`	`"github.com/docker/cli/cli"`
`10`	`10`	`"github.com/docker/cli/cli/command"`
	`11`	`+ configtypes "github.com/docker/cli/cli/config/types"`
`11`	`12`	`"github.com/docker/docker/api/types"`
`12`	`13`	`registrytypes "github.com/docker/docker/api/types/registry"`
`13`	`14`	`"github.com/docker/docker/client"`
`@@ -149,7 +150,7 @@ func runLogin(dockerCli command.Cli, opts loginOptions) error { //nolint: gocycl`
`149`	`150`	`}`
`150`	`151`	`}`
`151`	`152`
`152`		`- if err := creds.Store(*authConfig); err != nil {`
	`153`	`+ if err := creds.Store(configtypes.AuthConfig(*authConfig)); err != nil {`
`153`	`154`	`return errors.Errorf("Error saving credentials: %v", err)`
`154`	`155`	`}`
`155`	`156`
Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ import (`
`8`	`8`
`9`	`9`	`"github.com/docker/cli/cli/config/configfile"`
`10`	`10`	`"github.com/docker/cli/cli/config/credentials"`
`11`		`- "github.com/docker/docker/api/types"`
	`11`	`+ "github.com/docker/cli/cli/config/types"`
`12`	`12`	`"github.com/docker/docker/pkg/homedir"`
`13`	`13`	`"github.com/pkg/errors"`
`14`	`14`	`)`