From d44c458a194d89c6044e7de2c7d3f369c7e00967 Mon Sep 17 00:00:00 2001 From: Adam Worrall Date: Fri, 19 May 2017 18:07:52 -0700 Subject: [PATCH] Support sandbox images from private registries **What this PR does / why we need it**: The --pod-infra-container-image parameter allows the user to specify an arbitrary image to be used as the pod infra container (AKA sandbox), an internal piece of the dockershim implementation of the Container Runtime Interface. The dockershim does not have access to any of the pod-level image pull credentials configuration, so if the user specifies an image from a private registry, the image pull will fail. This change allows the dockershim to read local docker configuration (e.g. /root/.docker/config.json) and use it when pulling the pod infra container image. **Which issue this PR fixes**: fixes #45738 **Special notes for your reviewer**: The changes to fake_client for writing local config files deserve some attention. **Release note**: ```release-note NONE ``` --- pkg/kubelet/dockershim/BUILD | 1 + pkg/kubelet/dockershim/helpers.go | 33 +++++++++- pkg/kubelet/dockershim/helpers_test.go | 64 +++++++++++++++++-- .../dockershim/libdocker/fake_client.go | 27 +++++++- 4 files changed, 116 insertions(+), 9 deletions(-) diff --git a/pkg/kubelet/dockershim/BUILD b/pkg/kubelet/dockershim/BUILD index fcc0f9ae3e2..61d22f58adb 100644 --- a/pkg/kubelet/dockershim/BUILD +++ b/pkg/kubelet/dockershim/BUILD @@ -31,6 +31,7 @@ go_library( deps = [ "//pkg/api/v1:go_default_library", "//pkg/apis/componentconfig:go_default_library", + "//pkg/credentialprovider:go_default_library", "//pkg/kubelet/apis/cri:go_default_library", "//pkg/kubelet/apis/cri/v1alpha1:go_default_library", "//pkg/kubelet/cm:go_default_library", diff --git a/pkg/kubelet/dockershim/helpers.go b/pkg/kubelet/dockershim/helpers.go index bb4acd53cbc..eb8d95534bc 100644 --- a/pkg/kubelet/dockershim/helpers.go +++ b/pkg/kubelet/dockershim/helpers.go @@ -22,6 +22,7 @@ import ( "encoding/json" "fmt" "io/ioutil" + "os" "path/filepath" "regexp" "strconv" @@ -34,6 +35,7 @@ import ( "github.com/golang/glog" "k8s.io/kubernetes/pkg/api/v1" + "k8s.io/kubernetes/pkg/credentialprovider" runtimeapi "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1" "k8s.io/kubernetes/pkg/kubelet/types" "k8s.io/kubernetes/pkg/security/apparmor" @@ -375,6 +377,11 @@ func getSecurityOptSeparator(v *semver.Version) rune { // ensureSandboxImageExists pulls the sandbox image when it's not present. func ensureSandboxImageExists(client libdocker.Interface, image string) error { + dockerCfgSearchPath := []string{"/.docker", filepath.Join(os.Getenv("HOME"), ".docker")} + return ensureSandboxImageExistsDockerCfg(client, image, dockerCfgSearchPath) +} + +func ensureSandboxImageExistsDockerCfg(client libdocker.Interface, image string, dockerCfgSearchPath []string) error { _, err := client.InspectImageByRef(image) if err == nil { return nil @@ -382,8 +389,32 @@ func ensureSandboxImageExists(client libdocker.Interface, image string) error { if !libdocker.IsImageNotFoundError(err) { return fmt.Errorf("failed to inspect sandbox image %q: %v", image, err) } - err = client.PullImage(image, dockertypes.AuthConfig{}, dockertypes.ImagePullOptions{}) + + // To support images in private registries, try to read docker config + authConfig := dockertypes.AuthConfig{} + keyring := &credentialprovider.BasicDockerKeyring{} + var cfgLoadErr error + if cfg, err := credentialprovider.ReadDockerConfigJSONFile(dockerCfgSearchPath); err == nil { + keyring.Add(cfg) + } else if cfg, err := credentialprovider.ReadDockercfgFile(dockerCfgSearchPath); err == nil { + keyring.Add(cfg) + } else { + cfgLoadErr = err + } + if creds, withCredentials := keyring.Lookup(image); withCredentials { + // Use the first one that matched our image + for _, cred := range creds { + authConfig.Username = cred.Username + authConfig.Password = cred.Password + break + } + } + + err = client.PullImage(image, authConfig, dockertypes.ImagePullOptions{}) if err != nil { + if cfgLoadErr != nil { + glog.Warningf("Couldn't load Docker cofig. If sandbox image %q is in a private registry, this will cause further errors. Error: %v", image, cfgLoadErr) + } return fmt.Errorf("unable to pull sandbox image %q: %v", image, err) } return nil diff --git a/pkg/kubelet/dockershim/helpers_test.go b/pkg/kubelet/dockershim/helpers_test.go index 19ca1c48a95..8138228e2da 100644 --- a/pkg/kubelet/dockershim/helpers_test.go +++ b/pkg/kubelet/dockershim/helpers_test.go @@ -17,8 +17,12 @@ limitations under the License. package dockershim import ( + "encoding/base64" "fmt" + "io/ioutil" + "os" "path" + "path/filepath" "testing" "github.com/blang/semver" @@ -247,13 +251,33 @@ func TestGetSecurityOptSeparator(t *testing.T) { } } +// writeDockerConfig will write a config file into a temporary dir, and return that dir. +// Caller is responsible for deleting the dir and its contents. +func writeDockerConfig(cfg string) (string, error) { + tmpdir, err := ioutil.TempDir("", "dockershim=helpers_test.go=") + if err != nil { + return "", err + } + dir := filepath.Join(tmpdir, ".docker") + if err := os.Mkdir(dir, 0755); err != nil { + return "", err + } + return tmpdir, ioutil.WriteFile(filepath.Join(dir, "config.json"), []byte(cfg), 0644) +} + func TestEnsureSandboxImageExists(t *testing.T) { sandboxImage := "gcr.io/test/image" + registryHost := "https://gcr.io/" + authConfig := dockertypes.AuthConfig{Username: "user", Password: "pass"} + authB64 := base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", authConfig.Username, authConfig.Password))) + authJSON := fmt.Sprintf("{\"auths\": {\"%s\": {\"auth\": \"%s\"} } }", registryHost, authB64) for desc, test := range map[string]struct { - injectImage bool - injectErr error - calls []string - err bool + injectImage bool + imgNeedsAuth bool + injectErr error + calls []string + err bool + configJSON string }{ "should not pull image when it already exists": { injectImage: true, @@ -271,14 +295,42 @@ func TestEnsureSandboxImageExists(t *testing.T) { calls: []string{"inspect_image"}, err: true, }, + "should return error when image pull needs private auth, but none provided": { + injectImage: true, + imgNeedsAuth: true, + injectErr: libdocker.ImageNotFoundError{ID: "image_id"}, + calls: []string{"inspect_image", "pull"}, + err: true, + }, + "should pull private image using dockerauth if image doesn't exist": { + injectImage: true, + imgNeedsAuth: true, + injectErr: libdocker.ImageNotFoundError{ID: "image_id"}, + calls: []string{"inspect_image", "pull"}, + configJSON: authJSON, + err: false, + }, } { t.Logf("TestCase: %q", desc) _, fakeDocker, _ := newTestDockerService() if test.injectImage { - fakeDocker.InjectImages([]dockertypes.Image{{ID: sandboxImage}}) + images := []dockertypes.Image{{ID: sandboxImage}} + fakeDocker.InjectImages(images) + if test.imgNeedsAuth { + fakeDocker.MakeImagesPrivate(images, authConfig) + } } fakeDocker.InjectError("inspect_image", test.injectErr) - err := ensureSandboxImageExists(fakeDocker, sandboxImage) + + var dockerCfgSearchPath []string + if test.configJSON != "" { + tmpdir, err := writeDockerConfig(test.configJSON) + require.NoError(t, err, "could not create a temp docker config file") + dockerCfgSearchPath = append(dockerCfgSearchPath, filepath.Join(tmpdir, ".docker")) + defer os.RemoveAll(tmpdir) + } + + err := ensureSandboxImageExistsDockerCfg(fakeDocker, sandboxImage, dockerCfgSearchPath) assert.NoError(t, fakeDocker.AssertCalls(test.calls)) assert.Equal(t, test.err, err != nil) } diff --git a/pkg/kubelet/dockershim/libdocker/fake_client.go b/pkg/kubelet/dockershim/libdocker/fake_client.go index 1eac1684f28..6884f5e1556 100644 --- a/pkg/kubelet/dockershim/libdocker/fake_client.go +++ b/pkg/kubelet/dockershim/libdocker/fake_client.go @@ -55,6 +55,7 @@ type FakeDockerClient struct { ContainerMap map[string]*dockertypes.ContainerJSON ImageInspects map[string]*dockertypes.ImageInspect Images []dockertypes.Image + ImageIDsNeedingAuth map[string]dockertypes.AuthConfig Errors map[string]error called []calledDetail pulled []string @@ -91,8 +92,9 @@ func NewFakeDockerClient() *FakeDockerClient { ContainerMap: make(map[string]*dockertypes.ContainerJSON), Clock: clock.RealClock{}, // default this to true, so that we trace calls, image pulls and container lifecycle - EnableTrace: true, - ImageInspects: make(map[string]*dockertypes.ImageInspect), + EnableTrace: true, + ImageInspects: make(map[string]*dockertypes.ImageInspect), + ImageIDsNeedingAuth: make(map[string]dockertypes.AuthConfig), } } @@ -632,6 +634,14 @@ func (f *FakeDockerClient) Logs(id string, opts dockertypes.ContainerLogsOptions return f.popError("logs") } +func (f *FakeDockerClient) isAuthorizedForImage(image string, auth dockertypes.AuthConfig) bool { + if reqd, exists := f.ImageIDsNeedingAuth[image]; !exists { + return true // no auth needed + } else { + return auth.Username == reqd.Username && auth.Password == reqd.Password + } +} + // PullImage is a test-spy implementation of Interface.PullImage. // It adds an entry "pull" to the internal method call record. func (f *FakeDockerClient) PullImage(image string, auth dockertypes.AuthConfig, opts dockertypes.ImagePullOptions) error { @@ -640,6 +650,10 @@ func (f *FakeDockerClient) PullImage(image string, auth dockertypes.AuthConfig, f.appendCalled(calledDetail{name: "pull"}) err := f.popError("pull") if err == nil { + if !f.isAuthorizedForImage(image, auth) { + return ImageNotFoundError{ID: image} + } + authJson, _ := json.Marshal(auth) inspect := createImageInspectFromRef(image) f.ImageInspects[image] = inspect @@ -720,11 +734,20 @@ func (f *FakeDockerClient) InjectImages(images []dockertypes.Image) { } } +func (f *FakeDockerClient) MakeImagesPrivate(images []dockertypes.Image, auth dockertypes.AuthConfig) { + f.Lock() + defer f.Unlock() + for _, i := range images { + f.ImageIDsNeedingAuth[i.ID] = auth + } +} + func (f *FakeDockerClient) ResetImages() { f.Lock() defer f.Unlock() f.Images = []dockertypes.Image{} f.ImageInspects = make(map[string]*dockertypes.ImageInspect) + f.ImageIDsNeedingAuth = make(map[string]dockertypes.AuthConfig) } func (f *FakeDockerClient) InjectImageInspects(inspects []dockertypes.ImageInspect) {