modify error message returned when GCR login is attempted through docker

Change-Id: Ia093bc391bb21b9d0544115c404fea160af3bf91

credhelper/helper.go

@@ -21,6 +21,7 @@ package credhelper
 import (
 	"bytes"
 	"fmt"
+	"net/url"
 	"os/exec"
 	"strings"
 
@@ -84,7 +85,7 @@ func (ch *gcrCredHelper) List() ([]string, []string, error) {
 func (ch *gcrCredHelper) Add(creds *credentials.Credentials) error {
 	serverURL := creds.ServerURL
 	if isAGCRHostname(serverURL) {
-		return helperErr("add is unimplemented for GCR, please use one of the supported authentication methods", nil)
+		return helperErr("this operation is unsupported for GCR, please see docker-credential-gcr documentation for supported login methods", nil)
 	}
 	if err := ch.store.SetOtherCreds(creds); err != nil {
 		return helperErr("could not store 3p credentials for "+serverURL, err)
@@ -117,6 +118,9 @@ func (ch *gcrCredHelper) Get(serverURL string) (string, string, error) {
 	// Attempt to retrieve credentials for another repository
 	creds, err := ch.store.GetOtherCreds(serverURL)
 	if err != nil {
+		if credentials.IsErrCredentialsNotFound(err) {
+			return "", "", err
+		}
 		return "", "", helperErr("could not retrieve 3p credentials for "+serverURL, err)
 	}
 	return creds.Username, creds.Secret, nil
@@ -224,9 +228,13 @@ func tokenFromPrivateStore(store store.GCRCredStore) (string, error) {
 	return tok.AccessToken, nil
 }
 
-// isAGCRHostname returns true if the given registry server URL is one of GCR's
+// isAGCRHostname returns true if the given hostname is one of GCR's
 func isAGCRHostname(serverURL string) bool {
-	return config.SupportedGCRRegistries[serverURL]
+	URL, err := url.Parse(serverURL)
+	if err != nil {
+		return false
+	}
+	return config.SupportedGCRRegistries[URL.Host] || config.SupportedGCRRegistries[serverURL]
 }
 
 func helperErr(message string, err error) error {

credhelper/helper_unit_test.go

@@ -30,17 +30,35 @@ const (
 	expectedGCRUsername = "oauth2accesstoken"
 )
 
-var gcrHosts = [...]string{"gcr.io", "us.gcr.io", "eu.gcr.io", "asia.gcr.io", "b.gcr.io", "bucket.gcr.io", "appengine.gcr.io", "gcr.kubernetes.io"}
+var gcrHosts = [...]string{
+	"gcr.io",
+	"us.gcr.io",
+	"eu.gcr.io",
+	"asia.gcr.io",
+	"b.gcr.io",
+	"bucket.gcr.io",
+	"appengine.gcr.io",
+	"gcr.kubernetes.io",
+}
 var otherHosts = [...]string{"docker.io", "otherrepo.com"}
 
 func TestIsAGCRHostname(t *testing.T) {
 	t.Parallel()
+	// test for GCR hosts
 	for _, host := range gcrHosts {
 		if !isAGCRHostname(host) {
 			t.Error("Expected to be detected as a GCR hostname: ", host)
 		}
 	}
 
+	// test for GCR hosts + scheme
+	for _, host := range gcrHosts {
+		if !isAGCRHostname("https://" + host) {
+			t.Error("Expected to be detected as a GCR hostname: ", "https://"+host)
+		}
+	}
+
+	// test for non-GCR hosts
 	for _, host := range otherHosts {
 		if isAGCRHostname(host) {
 			t.Error("Expected to not be a GCR host: ", host)
@@ -56,13 +74,14 @@ func TestAdd_GCRCredentials(t *testing.T) {
 	// create a mock store to use
 	mockStore := mock_store.NewMockGCRCredStore(mockCtrl)
 	tested := NewGCRCredentialHelper(mockStore)
+
 	creds := credentials.Credentials{
 		Username: "foobarre",
 		Secret:   "secret",
 	}
 
 	for _, host := range gcrHosts {
-		creds.ServerURL = host
+		creds.ServerURL = "https://" + host
 		err := tested.Add(&creds)
 		if err == nil {
 			t.Error("Adding GCR credentials should return an error.")
@@ -86,7 +105,7 @@ func TestAdd_OtherCredentials(t *testing.T) {
 	}
 
 	for _, host := range otherHosts {
-		creds.ServerURL = host
+		creds.ServerURL = "https://" + host
 		mockStore.EXPECT().SetOtherCreds(&creds).Return(nil)
 
 		err := tested.Add(&creds)
@@ -151,7 +170,7 @@ func TestGet_GCRCredentials(t *testing.T) {
 	}
 
 	for _, host := range gcrHosts {
-		username, secret, err := tested.Get(host)
+		username, secret, err := tested.Get("https://" + host)
 		if err != nil {
 			t.Errorf("Get returned an error: %v", err)
 		} else if username != expectedGCRUsername {
@@ -172,7 +191,7 @@ func TestDelete_GCRCredentials(t *testing.T) {
 	tested := NewGCRCredentialHelper(mockStore)
 
 	for _, host := range gcrHosts {
-		err := tested.Delete(host)
+		err := tested.Delete("https://" + host)
 		if err == nil {
 			t.Error("Deleting GCR credentials should return an error.")
 		}
@@ -190,9 +209,10 @@ func TestDelete_OtherCredentials(t *testing.T) {
 	tested := NewGCRCredentialHelper(mockStore)
 
 	for _, host := range otherHosts {
-		mockStore.EXPECT().DeleteOtherCreds(host).Return(nil)
+		schemedHost := "https://" + host
+		mockStore.EXPECT().DeleteOtherCreds(schemedHost).Return(nil)
 
-		err := tested.Delete(host)
+		err := tested.Delete(schemedHost)
 
 		if err != nil {
 			t.Errorf("Delete returned an error: %v", err)

store/store.go

@@ -100,7 +100,7 @@ func (s *credStore) GetOtherCreds(serverURL string) (*credentials.Credentials, e
 
 	creds, present := all3pCreds[serverURL]
 	if !present {
-		return nil, authErr("no credentials present for "+serverURL, nil)
+		return nil, credentials.NewErrCredentialsNotFound()
 	}
 
 	return &creds, nil