Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support sandbox images from private registries #46236

Merged
merged 1 commit into from
May 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/kubelet/dockershim/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
33 changes: 32 additions & 1 deletion pkg/kubelet/dockershim/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"encoding/json"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"regexp"
"strconv"
Expand All @@ -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"
Expand Down Expand Up @@ -375,15 +377,44 @@ 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")}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is /.docker on the search path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's there to be compatible with pkg/credentialprovider/config.go:DefaultDockercfgPaths, the other place that uses docker config to pull images. IIRC, putting docker config in / was a common enough (anti?) pattern to warrant support.

return ensureSandboxImageExistsDockerCfg(client, image, dockerCfgSearchPath)
}

func ensureSandboxImageExistsDockerCfg(client libdocker.Interface, image string, dockerCfgSearchPath []string) error {
_, err := client.InspectImageByRef(image)
if err == nil {
return nil
}
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
Expand Down
64 changes: 58 additions & 6 deletions pkg/kubelet/dockershim/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand All @@ -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)
}
Expand Down
27 changes: 25 additions & 2 deletions pkg/kubelet/dockershim/libdocker/fake_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
}
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down