Cleanup config/credentials, remove dependency on config file.

Signed-off-by: Daniel Nephin <dnephin@docker.com>

cli/config/config.go

@@ -120,7 +120,7 @@ func LoadDefaultConfigFile(err io.Writer) *configfile.ConfigFile {
 		fmt.Fprintf(err, "WARNING: Error loading config file:%v\n", e)
 	}
 	if !configFile.ContainsAuth() {
-		credentials.DetectDefaultStore(configFile)
+		configFile.CredentialsStore = credentials.DetectDefaultStore(configFile.CredentialsStore)
 	}
 	return configFile
 }

cli/config/configfile/file.go

@@ -127,6 +127,11 @@ func (configFile *ConfigFile) ContainsAuth() bool {
 		len(configFile.AuthConfigs) > 0
 }
 
+// GetAuthConfigs returns the mapping of repo to auth configuration
+func (configFile *ConfigFile) GetAuthConfigs() map[string]types.AuthConfig {
+	return configFile.AuthConfigs
+}
+
 // SaveToWriter encodes and writes out all the authorization information to
 // the given writer
 func (configFile *ConfigFile) SaveToWriter(writer io.Writer) error {

cli/config/credentials/default_store.go

@@ -2,21 +2,18 @@ package credentials
 
 import (
 	"os/exec"
-
-	"github.com/docker/cli/cli/config/configfile"
 )
 
-// DetectDefaultStore sets the default credentials store
+// DetectDefaultStore return the default credentials store for the platform if
-// if the host includes the default store helper program.
+// the store executable is available.
-func DetectDefaultStore(c *configfile.ConfigFile) {
+func DetectDefaultStore(store string) string {
-	if c.CredentialsStore != "" {
+	// user defined or no default for platform
-		// user defined
+	if store != "" || defaultCredentialsStore == "" {
-		return
+		return store
 	}
 
-	if defaultCredentialsStore != "" {
+	if _, err := exec.LookPath(remoteCredentialsPrefix + defaultCredentialsStore); err == nil {
-		if _, err := exec.LookPath(remoteCredentialsPrefix + defaultCredentialsStore); err == nil {
+		return defaultCredentialsStore
-			c.CredentialsStore = defaultCredentialsStore
-		}
 	}
+	return ""
 }

cli/config/credentials/file_store.go

@@ -1,37 +1,39 @@
 package credentials
 
 import (
-	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/registry"
 )
 
+type store interface {
+	Save() error
+	GetAuthConfigs() map[string]types.AuthConfig
+}
+
 // fileStore implements a credentials store using
 // the docker configuration file to keep the credentials in plain text.
 type fileStore struct {
-	file *configfile.ConfigFile
+	file store
 }
 
 // NewFileStore creates a new file credentials store.
-func NewFileStore(file *configfile.ConfigFile) Store {
+func NewFileStore(file store) Store {
-	return &fileStore{
+	return &fileStore{file: file}
-		file: file,
-	}
 }
 
 // Erase removes the given credentials from the file store.
 func (c *fileStore) Erase(serverAddress string) error {
-	delete(c.file.AuthConfigs, serverAddress)
+	delete(c.file.GetAuthConfigs(), serverAddress)
 	return c.file.Save()
 }
 
 // Get retrieves credentials for a specific server from the file store.
 func (c *fileStore) Get(serverAddress string) (types.AuthConfig, error) {
-	authConfig, ok := c.file.AuthConfigs[serverAddress]
+	authConfig, ok := c.file.GetAuthConfigs()[serverAddress]
 	if !ok {
 		// 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.AuthConfigs {
+		for r, ac := range c.file.GetAuthConfigs() {
 			if serverAddress == registry.ConvertToHostname(r) {
 				return ac, nil
 			}
@@ -43,11 +45,11 @@ func (c *fileStore) Get(serverAddress string) (types.AuthConfig, error) {
 }
 
 func (c *fileStore) GetAll() (map[string]types.AuthConfig, error) {
-	return c.file.AuthConfigs, nil
+	return c.file.GetAuthConfigs(), nil
 }
 
 // Store saves the given credentials in the file store.
 func (c *fileStore) Store(authConfig types.AuthConfig) error {
-	c.file.AuthConfigs[authConfig.ServerAddress] = authConfig
+	c.file.GetAuthConfigs()[authConfig.ServerAddress] = authConfig
 	return c.file.Save()
 }

cli/config/credentials/file_store_test.go

@@ -1,55 +1,49 @@
 package credentials
 
 import (
-	"io/ioutil"
 	"testing"
 
-	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/docker/api/types"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-func newConfigFile(auths map[string]types.AuthConfig) *configfile.ConfigFile {
+type fakeStore struct {
-	tmp, _ := ioutil.TempFile("", "docker-test")
+	configs map[string]types.AuthConfig
-	name := tmp.Name()
+}
-	tmp.Close()
 
-	c := configfile.NewConfigFile(name)
+func (f *fakeStore) Save() error {
-	c.AuthConfigs = auths
+	return nil
-	return c
+}
+
+func (f *fakeStore) GetAuthConfigs() map[string]types.AuthConfig {
+	return f.configs
+}
+
+func newStore(auths map[string]types.AuthConfig) store {
+	return &fakeStore{configs: auths}
 }
 
 func TestFileStoreAddCredentials(t *testing.T) {
-	f := newConfigFile(make(map[string]types.AuthConfig))
+	f := newStore(make(map[string]types.AuthConfig))
 
 	s := NewFileStore(f)
-	err := s.Store(types.AuthConfig{
+	auth := types.AuthConfig{
 		Auth:          "super_secret_token",
 		Email:         "foo@example.com",
 		ServerAddress: "https://example.com",
-	})
+	}
+	err := s.Store(auth)
+	require.NoError(t, err)
+	assert.Len(t, f.GetAuthConfigs(), 1)
 
-	if err != nil {
+	actual, ok := f.GetAuthConfigs()["https://example.com"]
-		t.Fatal(err)
+	assert.True(t, ok)
-	}
+	assert.Equal(t, auth, actual)
-
-	if len(f.AuthConfigs) != 1 {
-		t.Fatalf("expected 1 auth config, got %d", len(f.AuthConfigs))
-	}
-
-	a, ok := f.AuthConfigs["https://example.com"]
-	if !ok {
-		t.Fatalf("expected auth for https://example.com, got %v", f.AuthConfigs)
-	}
-	if a.Auth != "super_secret_token" {
-		t.Fatalf("expected auth `super_secret_token`, got %s", a.Auth)
-	}
-	if a.Email != "foo@example.com" {
-		t.Fatalf("expected email `foo@example.com`, got %s", a.Email)
-	}
 }
 
 func TestFileStoreGet(t *testing.T) {
-	f := newConfigFile(map[string]types.AuthConfig{
+	f := newStore(map[string]types.AuthConfig{
 		"https://example.com": {
 			Auth:          "super_secret_token",
 			Email:         "foo@example.com",
@@ -73,7 +67,7 @@ func TestFileStoreGet(t *testing.T) {
 func TestFileStoreGetAll(t *testing.T) {
 	s1 := "https://example.com"
 	s2 := "https://example2.com"
-	f := newConfigFile(map[string]types.AuthConfig{
+	f := newStore(map[string]types.AuthConfig{
 		s1: {
 			Auth:          "super_secret_token",
 			Email:         "foo@example.com",
@@ -109,7 +103,7 @@ func TestFileStoreGetAll(t *testing.T) {
 }
 
 func TestFileStoreErase(t *testing.T) {
-	f := newConfigFile(map[string]types.AuthConfig{
+	f := newStore(map[string]types.AuthConfig{
 		"https://example.com": {
 			Auth:          "super_secret_token",
 			Email:         "foo@example.com",

cli/config/credentials/native_store.go

@@ -1,7 +1,6 @@
 package credentials
 
 import (
-	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/docker-credential-helpers/client"
 	"github.com/docker/docker-credential-helpers/credentials"
 	"github.com/docker/docker/api/types"
@@ -22,7 +21,7 @@ type nativeStore struct {
 
 // NewNativeStore creates a new native store that
 // uses a remote helper program to manage credentials.
-func NewNativeStore(file *configfile.ConfigFile, helperSuffix string) Store {
+func NewNativeStore(file store, helperSuffix string) Store {
 	name := remoteCredentialsPrefix + helperSuffix
 	return &nativeStore{
 		programFunc: client.NewShellProgramFunc(name),

cli/config/credentials/native_store_test.go

@@ -11,7 +11,10 @@ import (
 	"github.com/docker/docker-credential-helpers/client"
 	"github.com/docker/docker-credential-helpers/credentials"
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/pkg/testutil"
 	"github.com/pkg/errors"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -90,53 +93,32 @@ func mockCommandFn(args ...string) client.Program {
 }
 
 func TestNativeStoreAddCredentials(t *testing.T) {
-	f := newConfigFile(make(map[string]types.AuthConfig))
+	f := newStore(make(map[string]types.AuthConfig))
-	f.CredentialsStore = "mock"
-
 	s := &nativeStore{
 		programFunc: mockCommandFn,
 		fileStore:   NewFileStore(f),
 	}
-	err := s.Store(types.AuthConfig{
+	auth := types.AuthConfig{
 		Username:      "foo",
 		Password:      "bar",
 		Email:         "foo@example.com",
 		ServerAddress: validServerAddress,
-	})
+	}
+	err := s.Store(auth)
+	require.NoError(t, err)
+	assert.Len(t, f.GetAuthConfigs(), 1)
 
-	if err != nil {
+	actual, ok := f.GetAuthConfigs()[validServerAddress]
-		t.Fatal(err)
+	assert.True(t, ok)
-	}
+	expected := types.AuthConfig{
-
+		Email:         auth.Email,
-	if len(f.AuthConfigs) != 1 {
+		ServerAddress: auth.ServerAddress,
-		t.Fatalf("expected 1 auth config, got %d", len(f.AuthConfigs))
-	}
-
-	a, ok := f.AuthConfigs[validServerAddress]
-	if !ok {
-		t.Fatalf("expected auth for %s, got %v", validServerAddress, f.AuthConfigs)
-	}
-	if a.Auth != "" {
-		t.Fatalf("expected auth to be empty, got %s", a.Auth)
-	}
-	if a.Username != "" {
-		t.Fatalf("expected username to be empty, got %s", a.Username)
-	}
-	if a.Password != "" {
-		t.Fatalf("expected password to be empty, got %s", a.Password)
-	}
-	if a.IdentityToken != "" {
-		t.Fatalf("expected identity token to be empty, got %s", a.IdentityToken)
-	}
-	if a.Email != "foo@example.com" {
-		t.Fatalf("expected email `foo@example.com`, got %s", a.Email)
 	}
+	assert.Equal(t, expected, actual)
 }
 
 func TestNativeStoreAddInvalidCredentials(t *testing.T) {
-	f := newConfigFile(make(map[string]types.AuthConfig))
+	f := newStore(make(map[string]types.AuthConfig))
-	f.CredentialsStore = "mock"
-
 	s := &nativeStore{
 		programFunc: mockCommandFn,
 		fileStore:   NewFileStore(f),
@@ -147,102 +129,66 @@ func TestNativeStoreAddInvalidCredentials(t *testing.T) {
 		Email:         "foo@example.com",
 		ServerAddress: invalidServerAddress,
 	})
-
+	testutil.ErrorContains(t, err, "program failed")
-	if err == nil {
+	assert.Len(t, f.GetAuthConfigs(), 0)
-		t.Fatal("expected error, got nil")
-	}
-
-	if !strings.Contains(err.Error(), "program failed") {
-		t.Fatalf("expected `program failed`, got %v", err)
-	}
-
-	if len(f.AuthConfigs) != 0 {
-		t.Fatalf("expected 0 auth config, got %d", len(f.AuthConfigs))
-	}
 }
 
 func TestNativeStoreGet(t *testing.T) {
-	f := newConfigFile(map[string]types.AuthConfig{
+	f := newStore(map[string]types.AuthConfig{
 		validServerAddress: {
 			Email: "foo@example.com",
 		},
 	})
-	f.CredentialsStore = "mock"
-
 	s := &nativeStore{
 		programFunc: mockCommandFn,
 		fileStore:   NewFileStore(f),
 	}
-	a, err := s.Get(validServerAddress)
+	actual, err := s.Get(validServerAddress)
-	if err != nil {
+	require.NoError(t, err)
-		t.Fatal(err)
-	}
 
-	if a.Username != "foo" {
+	expected := types.AuthConfig{
-		t.Fatalf("expected username `foo`, got %s", a.Username)
+		Username: "foo",
-	}
+		Password: "bar",
-	if a.Password != "bar" {
+		Email:    "foo@example.com",
-		t.Fatalf("expected password `bar`, got %s", a.Password)
-	}
-	if a.IdentityToken != "" {
-		t.Fatalf("expected identity token to be empty, got %s", a.IdentityToken)
-	}
-	if a.Email != "foo@example.com" {
-		t.Fatalf("expected email `foo@example.com`, got %s", a.Email)
 	}
+	assert.Equal(t, expected, actual)
 }
 
 func TestNativeStoreGetIdentityToken(t *testing.T) {
-	f := newConfigFile(map[string]types.AuthConfig{
+	f := newStore(map[string]types.AuthConfig{
 		validServerAddress2: {
 			Email: "foo@example2.com",
 		},
 	})
-	f.CredentialsStore = "mock"
 
 	s := &nativeStore{
 		programFunc: mockCommandFn,
 		fileStore:   NewFileStore(f),
 	}
-	a, err := s.Get(validServerAddress2)
+	actual, err := s.Get(validServerAddress2)
-	if err != nil {
+	require.NoError(t, err)
-		t.Fatal(err)
-	}
 
-	if a.Username != "" {
+	expected := types.AuthConfig{
-		t.Fatalf("expected username to be empty, got %s", a.Username)
+		IdentityToken: "abcd1234",
-	}
+		Email:         "foo@example2.com",
-	if a.Password != "" {
-		t.Fatalf("expected password to be empty, got %s", a.Password)
-	}
-	if a.IdentityToken != "abcd1234" {
-		t.Fatalf("expected identity token `abcd1234`, got %s", a.IdentityToken)
-	}
-	if a.Email != "foo@example2.com" {
-		t.Fatalf("expected email `foo@example2.com`, got %s", a.Email)
 	}
+	assert.Equal(t, expected, actual)
 }
 
 func TestNativeStoreGetAll(t *testing.T) {
-	f := newConfigFile(map[string]types.AuthConfig{
+	f := newStore(map[string]types.AuthConfig{
 		validServerAddress: {
 			Email: "foo@example.com",
 		},
 	})
-	f.CredentialsStore = "mock"
 
 	s := &nativeStore{
 		programFunc: mockCommandFn,
 		fileStore:   NewFileStore(f),
 	}
 	as, err := s.GetAll()
-	if err != nil {
+	require.NoError(t, err)
-		t.Fatal(err)
+	assert.Len(t, as, 2)
-	}
-
-	if len(as) != 2 {
-		t.Fatalf("wanted 2, got %d", len(as))
-	}
 
 	if as[validServerAddress].Username != "foo" {
 		t.Fatalf("expected username `foo` for %s, got %s", validServerAddress, as[validServerAddress].Username)
@@ -271,86 +217,62 @@ func TestNativeStoreGetAll(t *testing.T) {
 }
 
 func TestNativeStoreGetMissingCredentials(t *testing.T) {
-	f := newConfigFile(map[string]types.AuthConfig{
+	f := newStore(map[string]types.AuthConfig{
 		validServerAddress: {
 			Email: "foo@example.com",
 		},
 	})
-	f.CredentialsStore = "mock"
 
 	s := &nativeStore{
 		programFunc: mockCommandFn,
 		fileStore:   NewFileStore(f),
 	}
 	_, err := s.Get(missingCredsAddress)
-	if err != nil {
+	assert.NoError(t, err)
-		// missing credentials do not produce an error
-		t.Fatal(err)
-	}
 }
 
 func TestNativeStoreGetInvalidAddress(t *testing.T) {
-	f := newConfigFile(map[string]types.AuthConfig{
+	f := newStore(map[string]types.AuthConfig{
 		validServerAddress: {
 			Email: "foo@example.com",
 		},
 	})
-	f.CredentialsStore = "mock"
 
 	s := &nativeStore{
 		programFunc: mockCommandFn,
 		fileStore:   NewFileStore(f),
 	}
 	_, err := s.Get(invalidServerAddress)
-	if err == nil {
+	testutil.ErrorContains(t, err, "program failed")
-		t.Fatal("expected error, got nil")
-	}
-
-	if !strings.Contains(err.Error(), "program failed") {
-		t.Fatalf("expected `program failed`, got %v", err)
-	}
 }
 
 func TestNativeStoreErase(t *testing.T) {
-	f := newConfigFile(map[string]types.AuthConfig{
+	f := newStore(map[string]types.AuthConfig{
 		validServerAddress: {
 			Email: "foo@example.com",
 		},
 	})
-	f.CredentialsStore = "mock"
 
 	s := &nativeStore{
 		programFunc: mockCommandFn,
 		fileStore:   NewFileStore(f),
 	}
 	err := s.Erase(validServerAddress)
-	if err != nil {
+	require.NoError(t, err)
-		t.Fatal(err)
+	assert.Len(t, f.GetAuthConfigs(), 0)
-	}
-
-	if len(f.AuthConfigs) != 0 {
-		t.Fatalf("expected 0 auth configs, got %d", len(f.AuthConfigs))
-	}
 }
 
 func TestNativeStoreEraseInvalidAddress(t *testing.T) {
-	f := newConfigFile(map[string]types.AuthConfig{
+	f := newStore(map[string]types.AuthConfig{
 		validServerAddress: {
 			Email: "foo@example.com",
 		},
 	})
-	f.CredentialsStore = "mock"
 
 	s := &nativeStore{
 		programFunc: mockCommandFn,
 		fileStore:   NewFileStore(f),
 	}
 	err := s.Erase(invalidServerAddress)
-	if err == nil {
+	testutil.ErrorContains(t, err, "program failed")
-		t.Fatal("expected error, got nil")
-	}
-
-	if !strings.Contains(err.Error(), "program failed") {
-		t.Fatalf("expected `program failed`, got %v", err)
-	}
 }