make the dockerkeyring handle mutiple matching credentials

pull/6/head
deads2k 2015-05-08 13:30:59 -04:00
parent 16a76f1bd3
commit 2ecb0ebd73
9 changed files with 142 additions and 65 deletions

View File

@ -104,10 +104,15 @@ func TestJwtProvider(t *testing.T) {
// Verify that we get the expected username/password combo for
// a gcr.io image name.
registryUrl := "gcr.io/foo/bar"
val, ok := keyring.Lookup(registryUrl)
creds, ok := keyring.Lookup(registryUrl)
if !ok {
t.Errorf("Didn't find expected URL: %s", registryUrl)
return
}
if len(creds) > 1 {
t.Errorf("Got more hits than expected: %s", creds)
}
val := creds[0]
if "_token" != val.Username {
t.Errorf("Unexpected username value, want: _token, got: %s", val.Username)

View File

@ -76,10 +76,15 @@ func TestDockerKeyringFromGoogleDockerConfigMetadata(t *testing.T) {
keyring.Add(provider.Provide())
val, ok := keyring.Lookup(registryUrl)
creds, ok := keyring.Lookup(registryUrl)
if !ok {
t.Errorf("Didn't find expected URL: %s", registryUrl)
return
}
if len(creds) > 1 {
t.Errorf("Got more hits than expected: %s", creds)
}
val := creds[0]
if username != val.Username {
t.Errorf("Unexpected username value, want: %s, got: %s", username, val.Username)
@ -143,10 +148,15 @@ func TestDockerKeyringFromGoogleDockerConfigMetadataUrl(t *testing.T) {
keyring.Add(provider.Provide())
val, ok := keyring.Lookup(registryUrl)
creds, ok := keyring.Lookup(registryUrl)
if !ok {
t.Errorf("Didn't find expected URL: %s", registryUrl)
return
}
if len(creds) > 1 {
t.Errorf("Got more hits than expected: %s", creds)
}
val := creds[0]
if username != val.Username {
t.Errorf("Unexpected username value, want: %s, got: %s", username, val.Username)
@ -211,10 +221,15 @@ func TestContainerRegistryBasics(t *testing.T) {
keyring.Add(provider.Provide())
val, ok := keyring.Lookup(registryUrl)
creds, ok := keyring.Lookup(registryUrl)
if !ok {
t.Errorf("Didn't find expected URL: %s", registryUrl)
return
}
if len(creds) > 1 {
t.Errorf("Got more hits than expected: %s", creds)
}
val := creds[0]
if "_token" != val.Username {
t.Errorf("Unexpected username value, want: %s, got: %s", "_token", val.Username)

View File

@ -23,6 +23,8 @@ import (
docker "github.com/fsouza/go-dockerclient"
"github.com/golang/glog"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
)
// DockerKeyring tracks a set of docker registry credentials, maintaining a
@ -33,13 +35,13 @@ import (
// most specific match for a given image
// - iterating a map does not yield predictable results
type DockerKeyring interface {
Lookup(image string) (docker.AuthConfiguration, bool)
Lookup(image string) ([]docker.AuthConfiguration, bool)
}
// BasicDockerKeyring is a trivial map-backed implementation of DockerKeyring
type BasicDockerKeyring struct {
index []string
creds map[string]docker.AuthConfiguration
creds map[string][]docker.AuthConfiguration
}
// lazyDockerKeyring is an implementation of DockerKeyring that lazily
@ -51,9 +53,10 @@ type lazyDockerKeyring struct {
func (dk *BasicDockerKeyring) Add(cfg DockerConfig) {
if dk.index == nil {
dk.index = make([]string, 0)
dk.creds = make(map[string]docker.AuthConfiguration)
dk.creds = make(map[string][]docker.AuthConfiguration)
}
for loc, ident := range cfg {
creds := docker.AuthConfiguration{
Username: ident.Username,
Password: ident.Password,
@ -73,15 +76,19 @@ func (dk *BasicDockerKeyring) Add(cfg DockerConfig) {
// See ResolveAuthConfig in docker/registry/auth.go.
if parsed.Host != "" {
// NOTE: foo.bar.com comes through as Path.
dk.creds[parsed.Host] = creds
dk.creds[parsed.Host] = append(dk.creds[parsed.Host], creds)
dk.index = append(dk.index, parsed.Host)
}
if parsed.Path != "/" {
dk.creds[parsed.Host+parsed.Path] = creds
dk.index = append(dk.index, parsed.Host+parsed.Path)
if (len(parsed.Path) > 0) && (parsed.Path != "/") {
key := parsed.Host + parsed.Path
dk.creds[key] = append(dk.creds[key], creds)
dk.index = append(dk.index, key)
}
}
eliminateDupes := util.NewStringSet(dk.index...)
dk.index = eliminateDupes.List()
// Update the index used to identify which credentials to use for a given
// image. The index is reverse-sorted so more specific paths are matched
// first. For example, if for the given image "quay.io/coreos/etcd",
@ -109,11 +116,12 @@ func isDefaultRegistryMatch(image string) bool {
return !strings.ContainsAny(parts[0], ".:")
}
// Lookup implements the DockerKeyring method for fetching credentials
// based on image name.
func (dk *BasicDockerKeyring) Lookup(image string) (docker.AuthConfiguration, bool) {
// range over the index as iterating over a map does not provide
// a predictable ordering
// Lookup implements the DockerKeyring method for fetching credentials based on image name.
// Multiple credentials may be returned if there are multiple potentially valid credentials
// available. This allows for rotation.
func (dk *BasicDockerKeyring) Lookup(image string) ([]docker.AuthConfiguration, bool) {
// range over the index as iterating over a map does not provide a predictable ordering
ret := []docker.AuthConfiguration{}
for _, k := range dk.index {
// NOTE: prefix is a sufficient check because while scheme is allowed,
// it is stripped as part of 'Add'
@ -121,7 +129,11 @@ func (dk *BasicDockerKeyring) Lookup(image string) (docker.AuthConfiguration, bo
continue
}
return dk.creds[k], true
ret = append(ret, dk.creds[k]...)
}
if len(ret) > 0 {
return ret, true
}
// Use credentials for the default registry if provided, and appropriate
@ -129,12 +141,12 @@ func (dk *BasicDockerKeyring) Lookup(image string) (docker.AuthConfiguration, bo
return auth, true
}
return docker.AuthConfiguration{}, false
return []docker.AuthConfiguration{}, false
}
// Lookup implements the DockerKeyring method for fetching credentials
// based on image name.
func (dk *lazyDockerKeyring) Lookup(image string) (docker.AuthConfiguration, bool) {
func (dk *lazyDockerKeyring) Lookup(image string) ([]docker.AuthConfiguration, bool) {
keyring := &BasicDockerKeyring{}
for _, p := range dk.Providers {
@ -145,10 +157,10 @@ func (dk *lazyDockerKeyring) Lookup(image string) (docker.AuthConfiguration, boo
}
type FakeKeyring struct {
auth docker.AuthConfiguration
auth []docker.AuthConfiguration
ok bool
}
func (f *FakeKeyring) Lookup(image string) (docker.AuthConfiguration, bool) {
func (f *FakeKeyring) Lookup(image string) ([]docker.AuthConfiguration, bool) {
return f.auth, f.ok
}

View File

@ -42,10 +42,15 @@ func TestDockerKeyringFromBytes(t *testing.T) {
keyring.Add(cfg)
}
val, ok := keyring.Lookup(url + "/foo/bar")
creds, ok := keyring.Lookup(url + "/foo/bar")
if !ok {
t.Errorf("Didn't find expected URL: %s", url)
return
}
if len(creds) > 1 {
t.Errorf("Got more hits than expected: %s", creds)
}
val := creds[0]
if username != val.Username {
t.Errorf("Unexpected username value, want: %s, got: %s", username, val.Username)
@ -130,10 +135,15 @@ func TestKeyringHitWithUnqualifiedDockerHub(t *testing.T) {
keyring.Add(cfg)
}
val, ok := keyring.Lookup("google/docker-registry")
creds, ok := keyring.Lookup("google/docker-registry")
if !ok {
t.Errorf("Didn't find expected URL: %s", url)
return
}
if len(creds) > 1 {
t.Errorf("Got more hits than expected: %s", creds)
}
val := creds[0]
if username != val.Username {
t.Errorf("Unexpected username value, want: %s, got: %s", username, val.Username)
@ -166,10 +176,15 @@ func TestKeyringHitWithUnqualifiedLibraryDockerHub(t *testing.T) {
keyring.Add(cfg)
}
val, ok := keyring.Lookup("jenkins")
creds, ok := keyring.Lookup("jenkins")
if !ok {
t.Errorf("Didn't find expected URL: %s", url)
return
}
if len(creds) > 1 {
t.Errorf("Got more hits than expected: %s", creds)
}
val := creds[0]
if username != val.Username {
t.Errorf("Unexpected username value, want: %s, got: %s", username, val.Username)
@ -202,10 +217,15 @@ func TestKeyringHitWithQualifiedDockerHub(t *testing.T) {
keyring.Add(cfg)
}
val, ok := keyring.Lookup(url + "/google/docker-registry")
creds, ok := keyring.Lookup(url + "/google/docker-registry")
if !ok {
t.Errorf("Didn't find expected URL: %s", url)
return
}
if len(creds) > 2 {
t.Errorf("Got more hits than expected: %s", creds)
}
val := creds[0]
if username != val.Username {
t.Errorf("Unexpected username value, want: %s, got: %s", username, val.Username)

View File

@ -30,6 +30,7 @@ import (
kubeletTypes "github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/types"
"github.com/GoogleCloudPlatform/kubernetes/pkg/types"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
utilerrors "github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors"
"github.com/docker/docker/pkg/parsers"
docker "github.com/fsouza/go-dockerclient"
"github.com/golang/glog"
@ -124,25 +125,39 @@ func (p dockerPuller) Pull(image string) error {
Tag: tag,
}
creds, ok := p.keyring.Lookup(repoToPull)
if !ok {
creds, haveCredentials := p.keyring.Lookup(repoToPull)
if !haveCredentials {
glog.V(1).Infof("Pulling image %s without credentials", image)
}
err := p.client.PullImage(opts, creds)
// If there was no error, or we had credentials, just return the error.
if err == nil || ok {
err := p.client.PullImage(opts, docker.AuthConfiguration{})
if err == nil {
return nil
}
// Image spec: [<registry>/]<repository>/<image>[:<version] so we count '/'
explicitRegistry := (strings.Count(image, "/") == 2)
// Hack, look for a private registry, and decorate the error with the lack of
// credentials. This is heuristic, and really probably could be done better
// by talking to the registry API directly from the kubelet here.
if explicitRegistry {
return fmt.Errorf("image pull failed for %s, this may be because there are no credentials on this request. details: (%v)", image, err)
}
return err
}
// Image spec: [<registry>/]<repository>/<image>[:<version] so we count '/'
explicitRegistry := (strings.Count(image, "/") == 2)
// Hack, look for a private registry, and decorate the error with the lack of
// credentials. This is heuristic, and really probably could be done better
// by talking to the registry API directly from the kubelet here.
if explicitRegistry {
return fmt.Errorf("image pull failed for %s, this may be because there are no credentials on this request. details: (%v)", image, err)
var pullErrs []error
for _, currentCreds := range creds {
err := p.client.PullImage(opts, currentCreds)
// If there was no error, return success
if err == nil {
return nil
}
pullErrs = append(pullErrs, err)
}
return err
return utilerrors.NewAggregate(pullErrs)
}
func (p throttledDockerPuller) Pull(image string) error {

View File

@ -198,18 +198,18 @@ func TestParseImageName(t *testing.T) {
}
}
func TestPull(t *testing.T) {
func TestPullWithNoSecrets(t *testing.T) {
tests := []struct {
imageName string
expectedImage string
}{
{"ubuntu", "ubuntu:latest"},
{"ubuntu:2342", "ubuntu:2342"},
{"ubuntu:latest", "ubuntu:latest"},
{"foo/bar:445566", "foo/bar:445566"},
{"registry.example.com:5000/foobar", "registry.example.com:5000/foobar:latest"},
{"registry.example.com:5000/foobar:5342", "registry.example.com:5000/foobar:5342"},
{"registry.example.com:5000/foobar:latest", "registry.example.com:5000/foobar:latest"},
{"ubuntu", "ubuntu:latest using {}"},
{"ubuntu:2342", "ubuntu:2342 using {}"},
{"ubuntu:latest", "ubuntu:latest using {}"},
{"foo/bar:445566", "foo/bar:445566 using {}"},
{"registry.example.com:5000/foobar", "registry.example.com:5000/foobar:latest using {}"},
{"registry.example.com:5000/foobar:5342", "registry.example.com:5000/foobar:5342 using {}"},
{"registry.example.com:5000/foobar:latest", "registry.example.com:5000/foobar:latest using {}"},
}
for _, test := range tests {
fakeKeyring := &credentialprovider.FakeKeyring{}
@ -259,7 +259,6 @@ func TestDockerKeyringLookupFails(t *testing.T) {
}
func TestDockerKeyringLookup(t *testing.T) {
empty := docker.AuthConfiguration{}
ada := docker.AuthConfiguration{
Username: "ada",
@ -289,27 +288,27 @@ func TestDockerKeyringLookup(t *testing.T) {
tests := []struct {
image string
match docker.AuthConfiguration
match []docker.AuthConfiguration
ok bool
}{
// direct match
{"bar.example.com", ada, true},
{"bar.example.com", []docker.AuthConfiguration{ada}, true},
// direct match deeper than other possible matches
{"bar.example.com/pong", grace, true},
{"bar.example.com/pong", []docker.AuthConfiguration{grace, ada}, true},
// no direct match, deeper path ignored
{"bar.example.com/ping", ada, true},
{"bar.example.com/ping", []docker.AuthConfiguration{ada}, true},
// match first part of path token
{"bar.example.com/pongz", grace, true},
{"bar.example.com/pongz", []docker.AuthConfiguration{grace, ada}, true},
// match regardless of sub-path
{"bar.example.com/pong/pang", grace, true},
{"bar.example.com/pong/pang", []docker.AuthConfiguration{grace, ada}, true},
// no host match
{"example.com", empty, false},
{"foo.example.com", empty, false},
{"example.com", []docker.AuthConfiguration{}, false},
{"foo.example.com", []docker.AuthConfiguration{}, false},
}
for i, tt := range tests {
@ -345,15 +344,15 @@ func TestIssue3797(t *testing.T) {
tests := []struct {
image string
match docker.AuthConfiguration
match []docker.AuthConfiguration
ok bool
}{
// direct match
{"quay.io", rex, true},
{"quay.io", []docker.AuthConfiguration{rex}, true},
// partial matches
{"quay.io/foo", rex, true},
{"quay.io/foo/bar", rex, true},
{"quay.io/foo", []docker.AuthConfiguration{rex}, true},
{"quay.io/foo/bar", []docker.AuthConfiguration{rex}, true},
}
for i, tt := range tests {

View File

@ -17,14 +17,16 @@ limitations under the License.
package dockertools
import (
"encoding/json"
"fmt"
"os"
"reflect"
"sort"
"sync"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
"github.com/fsouza/go-dockerclient"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
)
// FakeDockerClient is a simple fake docker client, so that kubelet can be run for testing without requiring a real docker setup.
@ -264,7 +266,8 @@ func (f *FakeDockerClient) PullImage(opts docker.PullImageOptions, auth docker.A
if len(registry) != 0 {
registry = registry + "/"
}
f.pulled = append(f.pulled, fmt.Sprintf("%s%s:%s", registry, opts.Repository, opts.Tag))
authJson, _ := json.Marshal(auth)
f.pulled = append(f.pulled, fmt.Sprintf("%s%s:%s using %s", registry, opts.Repository, opts.Tag, string(authJson)))
}
return err
}

View File

@ -728,7 +728,7 @@ func (dm *DockerManager) ListImages() ([]kubecontainer.Image, error) {
// TODO(vmarmol): Consider unexporting.
// PullImage pulls an image from network to local storage.
func (dm *DockerManager) PullImage(image kubecontainer.ImageSpec, _ []api.Secret) error {
func (dm *DockerManager) PullImage(image kubecontainer.ImageSpec, secrets []api.Secret) error {
return dm.Puller.Pull(image.Image)
}
@ -1151,6 +1151,7 @@ func (dm *DockerManager) createPodInfraContainer(pod *api.Pod) (kubeletTypes.Doc
return "", err
}
if !ok {
// TODO get the pull secrets from the container's ImageSpec and the pod's service account
if err := dm.PullImage(spec, nil); err != nil {
if ref != nil {
dm.recorder.Eventf(ref, "failed", "Failed to pull image %q: %v", container.Image, err)
@ -1341,6 +1342,7 @@ func (dm *DockerManager) pullImage(pod *api.Pod, container *api.Container) error
return nil
}
// TODO get the pull secrets from the container's ImageSpec and the pod's service account
err = dm.PullImage(spec, nil)
dm.runtimeHooks.ReportImagePull(pod, container, err)
return err

View File

@ -700,7 +700,13 @@ func (r *runtime) Version() (kubecontainer.Version, error) {
// writeDockerAuthConfig writes the docker credentials to rkt auth config files.
// This enables rkt to pull docker images from docker registry with credentials.
func (r *runtime) writeDockerAuthConfig(image string, creds docker.AuthConfiguration) error {
func (r *runtime) writeDockerAuthConfig(image string, credsSlice []docker.AuthConfiguration) error {
creds := docker.AuthConfiguration{}
// TODO handle multiple creds
if len(credsSlice) >= 1 {
creds = credsSlice[0]
}
registry := "index.docker.io"
// Image spec: [<registry>/]<repository>/<image>[:<version]
explicitRegistry := (strings.Count(image, "/") == 2)
@ -739,7 +745,7 @@ func (r *runtime) writeDockerAuthConfig(image string, creds docker.AuthConfigura
//
// https://github.com/GoogleCloudPlatform/kubernetes/issues/7203
//
func (r *runtime) PullImage(image kubecontainer.ImageSpec, _ []api.Secret) error {
func (r *runtime) PullImage(image kubecontainer.ImageSpec, pullSecrets []api.Secret) error {
img := image.Image
// TODO(yifan): The credential operation is a copy from dockertools package,
// Need to resolve the code duplication.