Merge pull request #26354 from KingEmet/fixbranch

make GetAll use client.List as the source of truth

cliconfig/credentials/native_store.go

@@ -58,17 +58,29 @@ func (c *nativeStore) Get(serverAddress string) (types.AuthConfig, error) {
 
 // GetAll retrieves all the credentials from the native store.
 func (c *nativeStore) GetAll() (map[string]types.AuthConfig, error) {
-	auths, _ := c.fileStore.GetAll()
+	auths, err := c.listCredentialsInStore()
+	if err != nil {
+		return nil, err
+	}
+
+	// Emails are only stored in the file store.
+	// This call can be safely eliminated when emails are removed.
+	fileConfigs, _ := c.fileStore.GetAll()
 
-	for s, ac := range auths {
-		creds, _ := c.getCredentialsFromStore(s)
+	authConfigs := make(map[string]types.AuthConfig)
+	for registry := range auths {
+		creds, err := c.getCredentialsFromStore(registry)
+		if err != nil {
+			return nil, err
+		}
+		ac, _ := fileConfigs[registry] // might contain Email
 		ac.Username = creds.Username
 		ac.Password = creds.Password
 		ac.IdentityToken = creds.IdentityToken
-		auths[s] = ac
+		authConfigs[registry] = ac
 	}
 
-	return auths, nil
+	return authConfigs, nil
 }
 
 // Store saves the given credentials in the file store.
@@ -124,3 +136,9 @@ func (c *nativeStore) getCredentialsFromStore(serverAddress string) (types.AuthC
 	ret.ServerAddress = serverAddress
 	return ret, nil
 }
+
+// listCredentialsInStore returns a listing of stored credentials as a map of
+// URL -> username.
+func (c *nativeStore) listCredentialsInStore() (map[string]string, error) {
+	return client.List(c.programFunc)
+}

cliconfig/credentials/native_store_test.go

@@ -70,6 +70,8 @@ func (m *mockCommand) Output() ([]byte, error) {
 		default:
 			return []byte("program failed"), errCommandExited
 		}
+	case "list":
+		return []byte(fmt.Sprintf(`{"%s": "%s", "%s": "%s"}`, validServerAddress, "foo", validServerAddress2, "<token>")), nil
 	}
 
 	return []byte(fmt.Sprintf("unknown argument %q with %q", m.arg, inS)), errCommandExited
@@ -225,9 +227,6 @@ func TestNativeStoreGetAll(t *testing.T) {
 		validServerAddress: {
 			Email: "foo@example.com",
 		},
-		validServerAddress2: {
-			Email: "foo@example2.com",
-		},
 	})
 	f.CredentialsStore = "mock"
 
@@ -265,8 +264,8 @@ func TestNativeStoreGetAll(t *testing.T) {
 	if as[validServerAddress2].IdentityToken != "abcd1234" {
 		t.Fatalf("expected identity token `abcd1324` for %s, got %s", validServerAddress2, as[validServerAddress2].IdentityToken)
 	}
-	if as[validServerAddress2].Email != "foo@example2.com" {
-		t.Fatalf("expected email `foo@example2.com` for %s, got %s", validServerAddress2, as[validServerAddress2].Email)
+	if as[validServerAddress2].Email != "" {
+		t.Fatalf("expected no email for %s, got %s", validServerAddress2, as[validServerAddress2].Email)
 	}
 }
 

hack/vendor.sh

@@ -140,7 +140,7 @@ clone git google.golang.org/api dc6d2353af16e2a2b0ff6986af051d473a4ed468 https:/
 clone git google.golang.org/cloud dae7e3d993bc3812a2185af60552bb6b847e52a0 https://code.googlesource.com/gocloud
 
 # native credentials
-clone git github.com/docker/docker-credential-helpers v0.3.0
+clone git github.com/docker/docker-credential-helpers f72c04f1d8e71959a6d103f808c50ccbad79b9fd
 
 # containerd
 clone git github.com/docker/containerd 52ef1ceb4b660c42cf4ea9013180a5663968d4c7

integration-cli/fixtures/auth/docker-credential-shell-test

@@ -2,29 +2,51 @@
 
 set -e
 
+listFile=shell_test_list.json
+
 case $1 in
 	"store")
 		in=$(</dev/stdin)
-		server=$(echo "$in" | jq --raw-output ".ServerURL" | sha1sum - | awk '{print $1}')
+		server=$(echo "$in" | jq --raw-output ".ServerURL")
+		serverHash=$(echo "$server" | sha1sum - | awk '{print $1}')
 
 		username=$(echo "$in" | jq --raw-output ".Username")
 		password=$(echo "$in" | jq --raw-output ".Secret")
-		echo "{ \"Username\": \"${username}\", \"Secret\": \"${password}\" }" > $TEMP/$server
+		echo "{ \"Username\": \"${username}\", \"Secret\": \"${password}\" }" > $TEMP/$serverHash
+		# add the server to the list file
+		if [[ ! -f $TEMP/$listFile ]]; then
+			echo "{ \"${server}\": \"${username}\" }" > $TEMP/$listFile
+		else
+			list=$(<$TEMP/$listFile)
+			echo "$list" | jq ". + {\"${server}\": \"${username}\"}" > $TEMP/$listFile
+		fi
 		;;
 	"get")
 		in=$(</dev/stdin)
-		server=$(echo "$in" | sha1sum - | awk '{print $1}')
-		if [[ ! -f $TEMP/$server ]]; then
+		serverHash=$(echo "$in" | sha1sum - | awk '{print $1}')
+		if [[ ! -f $TEMP/$serverHash ]]; then
 			echo "credentials not found in native keychain"
 			exit 1
 		fi
-		payload=$(<$TEMP/$server)
+		payload=$(<$TEMP/$serverHash)
 		echo "$payload"
 		;;
 	"erase")
 		in=$(</dev/stdin)
-		server=$(echo "$in" | sha1sum - | awk '{print $1}')
-		rm -f $TEMP/$server
+		serverHash=$(echo "$in" | sha1sum - | awk '{print $1}')
+		rm -f $TEMP/$serverHash
+
+		# Remove the server from the list
+		list=$(<$TEMP/$listFile)
+		echo "$list" | jq "del(.\"${in}\")" > $TEMP/$listFile
+		;;
+	"list")
+		if [[ ! -f $TEMP/$listFile ]]; then
+			echo "{}"
+		else
+			payload=$(<$TEMP/$listFile)
+			echo "$payload"
+		fi
 		;;
 	*)
 		echo "unknown credential option"

vendor/src/github.com/docker/docker-credential-helpers/client/client.go

@@ -55,11 +55,10 @@ func Get(program ProgramFunc, serverURL string) (*credentials.Credentials, error
 	return resp, nil
 }
 
-// Erase executes a program to remove the server credentails from the native store.
+// Erase executes a program to remove the server credentials from the native store.
 func Erase(program ProgramFunc, serverURL string) error {
 	cmd := program("erase")
 	cmd.Input(strings.NewReader(serverURL))
-
 	out, err := cmd.Output()
 	if err != nil {
 		t := strings.TrimSpace(string(out))
@@ -68,3 +67,21 @@ func Erase(program ProgramFunc, serverURL string) error {
 
 	return nil
 }
+
+// List executes a program to list server credentials in the native store.
+func List(program ProgramFunc) (map[string]string, error) {
+	cmd := program("list")
+	cmd.Input(strings.NewReader("unused"))
+	out, err := cmd.Output()
+	if err != nil {
+		t := strings.TrimSpace(string(out))
+		return nil, fmt.Errorf("error listing credentials - err: %v, out: `%s`", err, t)
+	}
+
+	var resp map[string]string
+	if err = json.NewDecoder(bytes.NewReader(out)).Decode(&resp); err != nil {
+		return nil, err
+	}
+
+	return resp, nil
+}

vendor/src/github.com/docker/docker-credential-helpers/credentials/credentials.go

@@ -25,7 +25,7 @@ type Credentials struct {
 func Serve(helper Helper) {
 	var err error
 	if len(os.Args) != 2 {
-		err = fmt.Errorf("Usage: %s <store|get|erase>", os.Args[0])
+		err = fmt.Errorf("Usage: %s <store|get|erase|list>", os.Args[0])
 	}
 
 	if err == nil {
@@ -47,6 +47,8 @@ func HandleCommand(helper Helper, key string, in io.Reader, out io.Writer) error
 		return Get(helper, in, out)
 	case "erase":
 		return Erase(helper, in)
+	case "list":
+		return List(helper, out)
 	}
 	return fmt.Errorf("Unknown credential action `%s`", key)
 }
@@ -127,3 +129,13 @@ func Erase(helper Helper, reader io.Reader) error {
 
 	return helper.Delete(serverURL)
 }
+
+//List returns all the serverURLs of keys in
+//the OS store as a list of strings
+func List(helper Helper, writer io.Writer) error {
+	accts, err := helper.List()
+	if err != nil {
+		return err
+	}
+	return json.NewEncoder(writer).Encode(accts)
+}

vendor/src/github.com/docker/docker-credential-helpers/credentials/helper.go

@@ -9,4 +9,6 @@ type Helper interface {
 	// Get retrieves credentials from the store.
 	// It returns username and secret as strings.
 	Get(serverURL string) (string, string, error)
+	// List returns the stored serverURLs and their associated usernames.
+	List() (map[string]string, error)
 }