Skip to content

Commit

Permalink
Default to gcloud to authenticate to *.gcr.io
Browse files Browse the repository at this point in the history
Signed-off-by: David Gageot <[email protected]>
  • Loading branch information
dgageot committed Nov 27, 2019
1 parent 87abe53 commit ef6fc8b
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 48 deletions.
20 changes: 15 additions & 5 deletions pkg/skaffold/docker/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"path/filepath"

"github.com/docker/cli/cli/config"
"github.com/docker/cli/cli/config/configfile"
"github.com/docker/distribution/reference"
"github.com/docker/docker/api/types"
"github.com/docker/docker/pkg/homedir"
Expand Down Expand Up @@ -61,13 +62,22 @@ type AuthConfigHelper interface {

type credsHelper struct{}

func (credsHelper) GetAuthConfig(registry string) (types.AuthConfig, error) {
func loadDockerConfig() (*configfile.ConfigFile, error) {
cf, err := config.Load(configDir)
if err != nil {
return types.AuthConfig{}, errors.Wrap(err, "docker config")
return nil, errors.Wrap(err, "docker config")
}

gcp.AutoConfigureGCRCredentialHelper(cf, registry)
gcp.AutoConfigureGCRCredentialHelper(cf)

return cf, nil
}

func (credsHelper) GetAuthConfig(registry string) (types.AuthConfig, error) {
cf, err := loadDockerConfig()
if err != nil {
return types.AuthConfig{}, err
}

auth, err := cf.GetAuthConfig(registry)
if err != nil {
Expand All @@ -78,9 +88,9 @@ func (credsHelper) GetAuthConfig(registry string) (types.AuthConfig, error) {
}

func (credsHelper) GetAllAuthConfigs() (map[string]types.AuthConfig, error) {
cf, err := config.Load(configDir)
cf, err := loadDockerConfig()
if err != nil {
return nil, errors.Wrap(err, "docker config")
return nil, err
}

credentials, err := cf.GetAllCredentials()
Expand Down
25 changes: 10 additions & 15 deletions pkg/skaffold/gcp/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"context"
"encoding/json"
"os/exec"
"strings"
"sync"

"github.com/docker/cli/cli/config/configfile"
Expand All @@ -37,31 +36,27 @@ var (
credsErr error
)

// TODO:(dgageot) Is there a way to not hard code those values?
var gcrPrefixes = []string{"gcr.io", "us.gcr.io", "eu.gcr.io", "asia.gcr.io", "staging-k8s.gcr.io", "marketplace.gcr.io"}

// AutoConfigureGCRCredentialHelper automatically adds the `gcloud` credential helper
// to docker's configuration.
// This doesn't modify the ~/.docker/config.json. It's only in-memory
func AutoConfigureGCRCredentialHelper(cf *configfile.ConfigFile, registry string) {
if registry != "gcr.io" && !strings.HasSuffix(registry, ".gcr.io") {
logrus.Debugln("Skipping credential configuration because registry is not gcr.")
return
}

if cf.CredentialHelpers != nil && cf.CredentialHelpers[registry] != "" {
logrus.Debugln("Skipping credential configuration because credentials are already configured.")
return
}

func AutoConfigureGCRCredentialHelper(cf *configfile.ConfigFile) {
if path, _ := exec.LookPath("docker-credential-gcloud"); path == "" {
logrus.Debugln("Skipping credential configuration because docker-credential-gcloud is not on PATH.")
return
}

logrus.Debugf("Adding `gcloud` as a docker credential helper for [%s]", registry)

if cf.CredentialHelpers == nil {
cf.CredentialHelpers = make(map[string]string)
}
cf.CredentialHelpers[registry] = "gcloud"

for _, gcrPrefix := range gcrPrefixes {
if _, present := cf.CredentialHelpers[gcrPrefix]; !present {
cf.CredentialHelpers[gcrPrefix] = "gcloud"
}
}
}

func activeUserCredentials() (*google.Credentials, error) {
Expand Down
53 changes: 25 additions & 28 deletions pkg/skaffold/gcp/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,70 +29,67 @@ import (
func TestAutoConfigureGCRCredentialHelper(t *testing.T) {
tests := []struct {
description string
registry string
helperInPath bool
config *configfile.ConfigFile
expected *configfile.ConfigFile
}{
{
description: "add to nil map",
registry: "gcr.io",
helperInPath: true,
config: &configfile.ConfigFile{},
expected: &configfile.ConfigFile{
CredentialHelpers: map[string]string{
"gcr.io": "gcloud",
"gcr.io": "gcloud",
"us.gcr.io": "gcloud",
"eu.gcr.io": "gcloud",
"asia.gcr.io": "gcloud",
"staging-k8s.gcr.io": "gcloud",
"marketplace.gcr.io": "gcloud",
},
},
},
{
description: "add to empty map",
registry: "gcr.io",
helperInPath: true,
config: &configfile.ConfigFile{
CredentialHelpers: map[string]string{},
},
expected: &configfile.ConfigFile{
CredentialHelpers: map[string]string{
"gcr.io": "gcloud",
"gcr.io": "gcloud",
"us.gcr.io": "gcloud",
"eu.gcr.io": "gcloud",
"asia.gcr.io": "gcloud",
"staging-k8s.gcr.io": "gcloud",
"marketplace.gcr.io": "gcloud",
},
},
},
{
description: "leave existing helper",
registry: "gcr.io",
config: &configfile.ConfigFile{
CredentialHelpers: map[string]string{
"gcr.io": "existing",
"gcr.io": "existing",
"us.gcr.io": "existing",
"eu.gcr.io": "existing",
"asia.gcr.io": "existing",
"staging-k8s.gcr.io": "existing",
"marketplace.gcr.io": "existing",
},
},
expected: &configfile.ConfigFile{
CredentialHelpers: map[string]string{
"gcr.io": "existing",
"gcr.io": "existing",
"us.gcr.io": "existing",
"eu.gcr.io": "existing",
"asia.gcr.io": "existing",
"staging-k8s.gcr.io": "existing",
"marketplace.gcr.io": "existing",
},
},
},
{
description: "any.gcr.io",
registry: "any.gcr.io",
helperInPath: true,
config: &configfile.ConfigFile{},
expected: &configfile.ConfigFile{
CredentialHelpers: map[string]string{
"any.gcr.io": "gcloud",
},
},
},
{
description: "case is important",
registry: "GCR.io",
helperInPath: true,
config: &configfile.ConfigFile{},
expected: &configfile.ConfigFile{},
},
{
description: "ignore if gcloud is not in PATH",
registry: "gcr.io",
helperInPath: false,
config: &configfile.ConfigFile{},
expected: &configfile.ConfigFile{},
Expand All @@ -107,7 +104,7 @@ func TestAutoConfigureGCRCredentialHelper(t *testing.T) {
tmpDir.Write("docker-credential-gcloud", "")
}

AutoConfigureGCRCredentialHelper(test.config, test.registry)
AutoConfigureGCRCredentialHelper(test.config)

t.CheckDeepEqual(test.expected, test.config)
})
Expand Down

0 comments on commit ef6fc8b

Please sign in to comment.