Skip to content

Commit

Permalink
Fix nil pointer dereference when no account is set on gcloud.
Browse files Browse the repository at this point in the history
Fixes #3596

And add unit tests
  • Loading branch information
dgageot committed Jan 28, 2020
1 parent 974741c commit 550b8c6
Show file tree
Hide file tree
Showing 2 changed files with 129 additions and 3 deletions.
10 changes: 7 additions & 3 deletions pkg/skaffold/docker/authenticators.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,13 @@ import (
"github.com/google/go-containerregistry/pkg/v1/google"
)

var authenticators = Authenticators{}
var authenticators = Authenticators{
configDir: configDir,
}

// Authenticators stores an authenticator per registry.
type Authenticators struct {
configDir string
byRegistry map[string]*lockedAuthenticator
lock sync.Mutex
}
Expand Down Expand Up @@ -82,15 +85,16 @@ func (a *Authenticators) newAuthenticator(ref name.Reference) authn.Authenticato
registry := ref.Context().Registry.Name()

// 1. Use google.NewGcloudAuthenticator() authenticator if `gcloud` is configured
cfg, err := config.Load(configDir)
cfg, err := config.Load(a.configDir)
if err == nil && cfg.CredentialHelpers[registry] == "gcloud" {
if auth, err := google.NewGcloudAuthenticator(); err == nil {
return auth
}
}

// 2. Use whatever `non anonymous` credential helper is configured
if auth, _ := authn.DefaultKeychain.Resolve(ref.Context().Registry); auth != authn.Anonymous {
auth, err := authn.DefaultKeychain.Resolve(ref.Context().Registry)
if err == nil && auth != authn.Anonymous {
return auth
}

Expand Down
122 changes: 122 additions & 0 deletions pkg/skaffold/docker/authenticators_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
/*
Copyright 2019 The Skaffold Authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package docker

import (
"os"
"runtime"
"testing"

"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/name"

"github.com/GoogleContainerTools/skaffold/testutil"
)

func TestFor(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("test doesn't work on windows")
}

tests := []struct {
description string
dockerConfig string
img string
gcloudOutput string
gcloudInPath bool
expectAnonymous bool
}{
{
description: "gcloud is configured and working",
img: "gcr.io/img",
dockerConfig: `{"credHelpers":{"gcr.io": "gcloud"}}`,
gcloudInPath: true,
gcloudOutput: "#!/bin/sh\necho '{\"credential\":{\"access_token\":\"TOKEN\",\"token_expiry\":\"2999-01-01T08:48:55Z\"}}'",
expectAnonymous: false,
},
{
description: "gcloud is configured but not found (anonymous)",
img: "gcr.io/img",
dockerConfig: `{"credHelpers":{"gcr.io": "gcloud"}}`,
gcloudInPath: false,
expectAnonymous: true,
},
{
description: "gcloud is configured but not working (anonymous)",
img: "gcr.io/img",
dockerConfig: `{"credHelpers":{"gcr.io": "gcloud"}}`,
gcloudInPath: true,
gcloudOutput: `exit 1`,
expectAnonymous: true,
},
{
description: "gcloud is not configured but working",
img: "gcr.io/img",
dockerConfig: `{}`,
gcloudInPath: true,
gcloudOutput: "#!/bin/sh\necho '{\"credential\":{\"access_token\":\"TOKEN\",\"token_expiry\":\"2999-01-01T08:48:55Z\"}}'",
expectAnonymous: false,
},
{
description: "gcloud is not configured and not working (anonymous)",
img: "eu.gcr.io/img",
dockerConfig: `{}`,
gcloudInPath: true,
gcloudOutput: `exit 1`,
expectAnonymous: true,
},
{
description: "anonymous",
img: "docker/img",
dockerConfig: `{}`,
expectAnonymous: true,
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
tmpDir := t.NewTempDir().Write("config.json", test.dockerConfig)

var path string
if test.gcloudInPath {
path = tmpDir.Root() + ":" + os.Getenv("PATH")
tmpDir.Write("gcloud", test.gcloudOutput)
} else {
path = tmpDir.Root()
}

t.SetEnvs(map[string]string{
"DOCKER_CONFIG": tmpDir.Path("config.json"),
"PATH": path,
})

ref, err := name.ParseReference(test.img, name.WeakValidation)
t.CheckNoError(err)

auths := Authenticators{configDir: tmpDir.Root()}
authenticator := auths.For(ref)
t.CheckNotNil(authenticator)

authConfig, err := authenticator.Authorization()
if test.expectAnonymous {
t.CheckDeepEqual(&authn.AuthConfig{}, authConfig)
} else {
t.CheckDeepEqual("TOKEN", authConfig.RegistryToken)
}
t.CheckNoError(err)
})
}
}

0 comments on commit 550b8c6

Please sign in to comment.