Use docker-credential-helpers client to talk with native creds stores.

Signed-off-by: David Calavera <david.calavera@gmail.com>

cliconfig/credentials/native_store.go

@@ -1,14 +1,8 @@
 package credentials
 
 import (
-	"bytes"
-	"encoding/json"
-	"errors"
-	"fmt"
-	"io"
-	"strings"
-
-	"github.com/Sirupsen/logrus"
+	"github.com/docker/docker-credential-helpers/client"
+	"github.com/docker/docker-credential-helpers/credentials"
 	"github.com/docker/docker/cliconfig/configfile"
 	"github.com/docker/engine-api/types"
 )
@@ -18,50 +12,27 @@ const (
 	tokenUsername           = "<token>"
 )
 
-// Standarize the not found error, so every helper returns
-// the same message and docker can handle it properly.
-var errCredentialsNotFound = errors.New("credentials not found in native keychain")
-
-// command is an interface that remote executed commands implement.
-type command interface {
-	Output() ([]byte, error)
-	Input(in io.Reader)
-}
-
-// credentialsRequest holds information shared between docker and a remote credential store.
-type credentialsRequest struct {
-	ServerURL string
-	Username  string
-	Secret    string
-}
-
-// credentialsGetResponse is the information serialized from a remote store
-// when the plugin sends requests to get the user credentials.
-type credentialsGetResponse struct {
-	Username string
-	Secret   string
-}
-
 // nativeStore implements a credentials store
 // using native keychain to keep credentials secure.
 // It piggybacks into a file store to keep users' emails.
 type nativeStore struct {
-	commandFn func(args ...string) command
-	fileStore Store
+	programFunc client.ProgramFunc
+	fileStore   Store
 }
 
 // NewNativeStore creates a new native store that
 // uses a remote helper program to manage credentials.
 func NewNativeStore(file *configfile.ConfigFile) Store {
+	name := remoteCredentialsPrefix + file.CredentialsStore
 	return &nativeStore{
-		commandFn: shellCommandFn(file.CredentialsStore),
-		fileStore: NewFileStore(file),
+		programFunc: client.NewShellProgramFunc(name),
+		fileStore:   NewFileStore(file),
 	}
 }
 
 // Erase removes the given credentials from the native store.
 func (c *nativeStore) Erase(serverAddress string) error {
-	if err := c.eraseCredentialsFromStore(serverAddress); err != nil {
+	if err := client.Erase(c.programFunc, serverAddress); err != nil {
 		return err
 	}
 
@@ -115,8 +86,7 @@ func (c *nativeStore) Store(authConfig types.AuthConfig) error {
 
 // storeCredentialsInStore executes the command to store the credentials in the native store.
 func (c *nativeStore) storeCredentialsInStore(config types.AuthConfig) error {
-	cmd := c.commandFn("store")
-	creds := &credentialsRequest{
+	creds := &credentials.Credentials{
 		ServerURL: config.ServerAddress,
 		Username:  config.Username,
 		Secret:    config.Password,
@@ -127,70 +97,30 @@ func (c *nativeStore) storeCredentialsInStore(config types.AuthConfig) error {
 		creds.Secret = config.IdentityToken
 	}
 
-	buffer := new(bytes.Buffer)
-	if err := json.NewEncoder(buffer).Encode(creds); err != nil {
-		return err
-	}
-	cmd.Input(buffer)
-
-	out, err := cmd.Output()
-	if err != nil {
-		t := strings.TrimSpace(string(out))
-		logrus.Debugf("error adding credentials - err: %v, out: `%s`", err, t)
-		return fmt.Errorf(t)
-	}
-
-	return nil
+	return client.Store(c.programFunc, creds)
 }
 
 // getCredentialsFromStore executes the command to get the credentials from the native store.
 func (c *nativeStore) getCredentialsFromStore(serverAddress string) (types.AuthConfig, error) {
 	var ret types.AuthConfig
 
-	cmd := c.commandFn("get")
-	cmd.Input(strings.NewReader(serverAddress))
-
-	out, err := cmd.Output()
+	creds, err := client.Get(c.programFunc, serverAddress)
 	if err != nil {
-		t := strings.TrimSpace(string(out))
-
-		// do not return an error if the credentials are not
-		// in the keyckain. Let docker ask for new credentials.
-		if t == errCredentialsNotFound.Error() {
+		if credentials.IsErrCredentialsNotFound(err) {
+			// do not return an error if the credentials are not
+			// in the keyckain. Let docker ask for new credentials.
 			return ret, nil
 		}
-
-		logrus.Debugf("error getting credentials - err: %v, out: `%s`", err, t)
-		return ret, fmt.Errorf(t)
-	}
-
-	var resp credentialsGetResponse
-	if err := json.NewDecoder(bytes.NewReader(out)).Decode(&resp); err != nil {
 		return ret, err
 	}
 
-	if resp.Username == tokenUsername {
-		ret.IdentityToken = resp.Secret
+	if creds.Username == tokenUsername {
+		ret.IdentityToken = creds.Secret
 	} else {
-		ret.Password = resp.Secret
-		ret.Username = resp.Username
+		ret.Password = creds.Secret
+		ret.Username = creds.Username
 	}
 
 	ret.ServerAddress = serverAddress
 	return ret, nil
 }
-
-// eraseCredentialsFromStore executes the command to remove the server credentails from the native store.
-func (c *nativeStore) eraseCredentialsFromStore(serverURL string) error {
-	cmd := c.commandFn("erase")
-	cmd.Input(strings.NewReader(serverURL))
-
-	out, err := cmd.Output()
-	if err != nil {
-		t := strings.TrimSpace(string(out))
-		logrus.Debugf("error erasing credentials - err: %v, out: `%s`", err, t)
-		return fmt.Errorf(t)
-	}
-
-	return nil
-}

cliconfig/credentials/native_store_test.go

@@ -8,6 +8,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/docker/docker-credential-helpers/client"
+	"github.com/docker/docker-credential-helpers/credentials"
 	"github.com/docker/engine-api/types"
 )
 
@@ -43,7 +45,7 @@ func (m *mockCommand) Output() ([]byte, error) {
 		case validServerAddress:
 			return nil, nil
 		default:
-			return []byte("error erasing credentials"), errCommandExited
+			return []byte("program failed"), errCommandExited
 		}
 	case "get":
 		switch inS {
@@ -52,21 +54,21 @@ func (m *mockCommand) Output() ([]byte, error) {
 		case validServerAddress2:
 			return []byte(`{"Username": "<token>", "Secret": "abcd1234"}`), nil
 		case missingCredsAddress:
-			return []byte(errCredentialsNotFound.Error()), errCommandExited
+			return []byte(credentials.NewErrCredentialsNotFound().Error()), errCommandExited
 		case invalidServerAddress:
-			return []byte("error getting credentials"), errCommandExited
+			return []byte("program failed"), errCommandExited
 		}
 	case "store":
-		var c credentialsRequest
+		var c credentials.Credentials
 		err := json.NewDecoder(strings.NewReader(inS)).Decode(&c)
 		if err != nil {
-			return []byte("error storing credentials"), errCommandExited
+			return []byte("program failed"), errCommandExited
 		}
 		switch c.ServerURL {
 		case validServerAddress:
 			return nil, nil
 		default:
-			return []byte("error storing credentials"), errCommandExited
+			return []byte("program failed"), errCommandExited
 		}
 	}
 
@@ -78,7 +80,7 @@ func (m *mockCommand) Input(in io.Reader) {
 	m.input = in
 }
 
-func mockCommandFn(args ...string) command {
+func mockCommandFn(args ...string) client.Program {
 	return &mockCommand{
 		arg: args[0],
 	}
@@ -89,8 +91,8 @@ func TestNativeStoreAddCredentials(t *testing.T) {
 	f.CredentialsStore = "mock"
 
 	s := &nativeStore{
-		commandFn: mockCommandFn,
-		fileStore: NewFileStore(f),
+		programFunc: mockCommandFn,
+		fileStore:   NewFileStore(f),
 	}
 	err := s.Store(types.AuthConfig{
 		Username:      "foo",
@@ -133,8 +135,8 @@ func TestNativeStoreAddInvalidCredentials(t *testing.T) {
 	f.CredentialsStore = "mock"
 
 	s := &nativeStore{
-		commandFn: mockCommandFn,
-		fileStore: NewFileStore(f),
+		programFunc: mockCommandFn,
+		fileStore:   NewFileStore(f),
 	}
 	err := s.Store(types.AuthConfig{
 		Username:      "foo",
@@ -147,8 +149,8 @@ func TestNativeStoreAddInvalidCredentials(t *testing.T) {
 		t.Fatal("expected error, got nil")
 	}
 
-	if err.Error() != "error storing credentials" {
-		t.Fatalf("expected `error storing credentials`, got %v", err)
+	if !strings.Contains(err.Error(), "program failed") {
+		t.Fatalf("expected `program failed`, got %v", err)
 	}
 
 	if len(f.AuthConfigs) != 0 {
@@ -165,8 +167,8 @@ func TestNativeStoreGet(t *testing.T) {
 	f.CredentialsStore = "mock"
 
 	s := &nativeStore{
-		commandFn: mockCommandFn,
-		fileStore: NewFileStore(f),
+		programFunc: mockCommandFn,
+		fileStore:   NewFileStore(f),
 	}
 	a, err := s.Get(validServerAddress)
 	if err != nil {
@@ -196,8 +198,8 @@ func TestNativeStoreGetIdentityToken(t *testing.T) {
 	f.CredentialsStore = "mock"
 
 	s := &nativeStore{
-		commandFn: mockCommandFn,
-		fileStore: NewFileStore(f),
+		programFunc: mockCommandFn,
+		fileStore:   NewFileStore(f),
 	}
 	a, err := s.Get(validServerAddress2)
 	if err != nil {
@@ -230,8 +232,8 @@ func TestNativeStoreGetAll(t *testing.T) {
 	f.CredentialsStore = "mock"
 
 	s := &nativeStore{
-		commandFn: mockCommandFn,
-		fileStore: NewFileStore(f),
+		programFunc: mockCommandFn,
+		fileStore:   NewFileStore(f),
 	}
 	as, err := s.GetAll()
 	if err != nil {
@@ -277,8 +279,8 @@ func TestNativeStoreGetMissingCredentials(t *testing.T) {
 	f.CredentialsStore = "mock"
 
 	s := &nativeStore{
-		commandFn: mockCommandFn,
-		fileStore: NewFileStore(f),
+		programFunc: mockCommandFn,
+		fileStore:   NewFileStore(f),
 	}
 	_, err := s.Get(missingCredsAddress)
 	if err != nil {
@@ -296,16 +298,16 @@ func TestNativeStoreGetInvalidAddress(t *testing.T) {
 	f.CredentialsStore = "mock"
 
 	s := &nativeStore{
-		commandFn: mockCommandFn,
-		fileStore: NewFileStore(f),
+		programFunc: mockCommandFn,
+		fileStore:   NewFileStore(f),
 	}
 	_, err := s.Get(invalidServerAddress)
 	if err == nil {
 		t.Fatal("expected error, got nil")
 	}
 
-	if err.Error() != "error getting credentials" {
-		t.Fatalf("expected `error getting credentials`, got %v", err)
+	if !strings.Contains(err.Error(), "program failed") {
+		t.Fatalf("expected `program failed`, got %v", err)
 	}
 }
 
@@ -318,8 +320,8 @@ func TestNativeStoreErase(t *testing.T) {
 	f.CredentialsStore = "mock"
 
 	s := &nativeStore{
-		commandFn: mockCommandFn,
-		fileStore: NewFileStore(f),
+		programFunc: mockCommandFn,
+		fileStore:   NewFileStore(f),
 	}
 	err := s.Erase(validServerAddress)
 	if err != nil {
@@ -340,15 +342,15 @@ func TestNativeStoreEraseInvalidAddress(t *testing.T) {
 	f.CredentialsStore = "mock"
 
 	s := &nativeStore{
-		commandFn: mockCommandFn,
-		fileStore: NewFileStore(f),
+		programFunc: mockCommandFn,
+		fileStore:   NewFileStore(f),
 	}
 	err := s.Erase(invalidServerAddress)
 	if err == nil {
 		t.Fatal("expected error, got nil")
 	}
 
-	if err.Error() != "error erasing credentials" {
-		t.Fatalf("expected `error erasing credentials`, got %v", err)
+	if !strings.Contains(err.Error(), "program failed") {
+		t.Fatalf("expected `program failed`, got %v", err)
 	}
 }

cliconfig/credentials/shell_command.go

@@ -1,28 +0,0 @@
-package credentials
-
-import (
-	"io"
-	"os/exec"
-)
-
-func shellCommandFn(storeName string) func(args ...string) command {
-	name := remoteCredentialsPrefix + storeName
-	return func(args ...string) command {
-		return &shell{cmd: exec.Command(name, args...)}
-	}
-}
-
-// shell invokes shell commands to talk with a remote credentials helper.
-type shell struct {
-	cmd *exec.Cmd
-}
-
-// Output returns responses from the remote credentials helper.
-func (s *shell) Output() ([]byte, error) {
-	return s.cmd.Output()
-}
-
-// Input sets the input to send to a remote credentials helper.
-func (s *shell) Input(in io.Reader) {
-	s.cmd.Stdin = in
-}