From 0c14e0cbdb37697c0a09697c61fe9c941ad6b07f Mon Sep 17 00:00:00 2001 From: deads2k Date: Fri, 8 May 2015 13:53:00 -0400 Subject: [PATCH] add pull secret references to pods --- pkg/api/types.go | 8 ++ pkg/api/v1/conversion_generated.go | 20 +++++ pkg/api/v1/types.go | 4 + pkg/api/v1beta1/conversion.go | 6 ++ pkg/api/v1beta1/types.go | 8 ++ pkg/api/v1beta2/conversion.go | 6 ++ pkg/api/v1beta2/types.go | 8 ++ pkg/api/v1beta3/conversion_generated.go | 20 +++++ pkg/api/v1beta3/types.go | 4 + pkg/api/validation/validation.go | 14 ++++ pkg/api/validation/validation_test.go | 18 +++++ pkg/credentialprovider/keyring.go | 49 ++++++++++++ pkg/kubelet/container/runtime.go | 4 +- pkg/kubelet/dockertools/docker.go | 15 ++-- pkg/kubelet/dockertools/docker_test.go | 76 ++++++++++++++++++- pkg/kubelet/dockertools/fake_docker_client.go | 3 +- pkg/kubelet/dockertools/manager.go | 18 ++--- pkg/kubelet/kubelet.go | 26 ++++++- pkg/kubelet/rkt/rkt.go | 9 ++- pkg/registry/pod/rest.go | 7 ++ 20 files changed, 300 insertions(+), 23 deletions(-) diff --git a/pkg/api/types.go b/pkg/api/types.go index c6d6a39f3b..002b1dd0f8 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -831,6 +831,10 @@ type PodSpec struct { // used must be specified. // Optional: Default to false. HostNetwork bool `json:"hostNetwork,omitempty"` + // ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used by this PodSpec. + // If specified, these secrets will be passed to individual puller implementations for them to use. For example, + // in the case of docker, only DockerConfig type secrets are honored. + ImagePullSecrets []ObjectReference `json:"imagePullSecrets,omitempty" description:"list of references to secrets in the same namespace available for pulling the container images"` } // PodStatus represents information about the status of a pod. Status may trail the actual @@ -1705,6 +1709,10 @@ type ContainerManifest struct { // Required: Set DNS policy. DNSPolicy DNSPolicy `json:"dnsPolicy"` HostNetwork bool `json:"hostNetwork,omitempty"` + // ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used by this PodSpec. + // If specified, these secrets will be passed to individual puller implementations for them to use. For example, + // in the case of docker, only DockerConfig type secrets are honored. + ImagePullSecrets []ObjectReference `json:"imagePullSecrets,omitempty" description:"list of references to secrets in the same namespace available for pulling the container images"` } // ContainerManifestList is used to communicate container manifests to kubelet. diff --git a/pkg/api/v1/conversion_generated.go b/pkg/api/v1/conversion_generated.go index 6ee42f21dd..07223883f5 100644 --- a/pkg/api/v1/conversion_generated.go +++ b/pkg/api/v1/conversion_generated.go @@ -2670,6 +2670,16 @@ func convert_v1_PodSpec_To_api_PodSpec(in *PodSpec, out *newer.PodSpec, s conver out.ServiceAccount = in.ServiceAccount out.Host = in.Host out.HostNetwork = in.HostNetwork + if in.ImagePullSecrets != nil { + out.ImagePullSecrets = make([]newer.ObjectReference, len(in.ImagePullSecrets)) + for i := range in.ImagePullSecrets { + if err := convert_v1_ObjectReference_To_api_ObjectReference(&in.ImagePullSecrets[i], &out.ImagePullSecrets[i], s); err != nil { + return err + } + } + } else { + out.ImagePullSecrets = nil + } return nil } @@ -2722,6 +2732,16 @@ func convert_api_PodSpec_To_v1_PodSpec(in *newer.PodSpec, out *PodSpec, s conver out.ServiceAccount = in.ServiceAccount out.Host = in.Host out.HostNetwork = in.HostNetwork + if in.ImagePullSecrets != nil { + out.ImagePullSecrets = make([]ObjectReference, len(in.ImagePullSecrets)) + for i := range in.ImagePullSecrets { + if err := convert_api_ObjectReference_To_v1_ObjectReference(&in.ImagePullSecrets[i], &out.ImagePullSecrets[i], s); err != nil { + return err + } + } + } else { + out.ImagePullSecrets = nil + } return nil } diff --git a/pkg/api/v1/types.go b/pkg/api/v1/types.go index 162c34291e..b5877bde93 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -829,6 +829,10 @@ type PodSpec struct { // used must be specified. // Optional: Default to false. HostNetwork bool `json:"hostNetwork,omitempty" description:"host networking requested for this pod"` + // ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used by this PodSpec. + // If specified, these secrets will be passed to individual puller implementations for them to use. For example, + // in the case of docker, only DockerConfig type secrets are honored. + ImagePullSecrets []ObjectReference `json:"imagePullSecrets,omitempty" description:"list of references to secrets in the same namespace available for pulling the container images" patchStrategy:"merge" patchMergeKey:"name"` } // PodStatus represents information about the status of a pod. Status may trail the actual diff --git a/pkg/api/v1beta1/conversion.go b/pkg/api/v1beta1/conversion.go index dea7f90d79..56cf5c2395 100644 --- a/pkg/api/v1beta1/conversion.go +++ b/pkg/api/v1beta1/conversion.go @@ -706,6 +706,9 @@ func addConversionFuncs() { if err := s.Convert(&in.RestartPolicy, &out.RestartPolicy, 0); err != nil { return err } + if err := s.Convert(&in.ImagePullSecrets, &out.ImagePullSecrets, 0); err != nil { + return err + } if in.TerminationGracePeriodSeconds != nil { out.TerminationGracePeriodSeconds = new(int64) *out.TerminationGracePeriodSeconds = *in.TerminationGracePeriodSeconds @@ -729,6 +732,9 @@ func addConversionFuncs() { if err := s.Convert(&in.RestartPolicy, &out.RestartPolicy, 0); err != nil { return err } + if err := s.Convert(&in.ImagePullSecrets, &out.ImagePullSecrets, 0); err != nil { + return err + } if in.TerminationGracePeriodSeconds != nil { out.TerminationGracePeriodSeconds = new(int64) *out.TerminationGracePeriodSeconds = *in.TerminationGracePeriodSeconds diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go index b7807e84d1..a3b9513572 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -75,6 +75,10 @@ type ContainerManifest struct { // used must be specified. // Optional: Default to false. HostNetwork bool `json:"hostNetwork,omitempty" description:"host networking requested for this pod"` + // ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used by this PodSpec. + // If specified, these secrets will be passed to individual puller implementations for them to use. For example, + // in the case of docker, only DockerConfig type secrets are honored. + ImagePullSecrets []ObjectReference `json:"imagePullSecrets,omitempty" description:"list of references to secrets in the same namespace available for pulling the container images"` } // ContainerManifestList is used to communicate container manifests to kubelet. @@ -1505,6 +1509,10 @@ type PodSpec struct { // used must be specified. // Optional: Default to false. HostNetwork bool `json:"hostNetwork,omitempty" description:"host networking requested for this pod"` + // ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used by this PodSpec. + // If specified, these secrets will be passed to individual puller implementations for them to use. For example, + // in the case of docker, only DockerConfig type secrets are honored. + ImagePullSecrets []ObjectReference `json:"imagePullSecrets,omitempty" description:"list of references to secrets in the same namespace available for pulling the container images"` } // List holds a list of objects, which may not be known by the server. diff --git a/pkg/api/v1beta2/conversion.go b/pkg/api/v1beta2/conversion.go index 27b29e9019..685c486652 100644 --- a/pkg/api/v1beta2/conversion.go +++ b/pkg/api/v1beta2/conversion.go @@ -486,6 +486,9 @@ func addConversionFuncs() { if err := s.Convert(&in.RestartPolicy, &out.RestartPolicy, 0); err != nil { return err } + if err := s.Convert(&in.ImagePullSecrets, &out.ImagePullSecrets, 0); err != nil { + return err + } if in.TerminationGracePeriodSeconds != nil { out.TerminationGracePeriodSeconds = new(int64) *out.TerminationGracePeriodSeconds = *in.TerminationGracePeriodSeconds @@ -509,6 +512,9 @@ func addConversionFuncs() { if err := s.Convert(&in.RestartPolicy, &out.RestartPolicy, 0); err != nil { return err } + if err := s.Convert(&in.ImagePullSecrets, &out.ImagePullSecrets, 0); err != nil { + return err + } if in.TerminationGracePeriodSeconds != nil { out.TerminationGracePeriodSeconds = new(int64) *out.TerminationGracePeriodSeconds = *in.TerminationGracePeriodSeconds diff --git a/pkg/api/v1beta2/types.go b/pkg/api/v1beta2/types.go index 1e6caa1133..6285f1e4ce 100644 --- a/pkg/api/v1beta2/types.go +++ b/pkg/api/v1beta2/types.go @@ -1537,6 +1537,10 @@ type ContainerManifest struct { // used must be specified. // Optional: Default to false. HostNetwork bool `json:"hostNetwork,omitempty" description:"host networking requested for this pod"` + // ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used by this PodSpec. + // If specified, these secrets will be passed to individual puller implementations for them to use. For example, + // in the case of docker, only DockerConfig type secrets are honored. + ImagePullSecrets []ObjectReference `json:"imagePullSecrets,omitempty" description:"list of references to secrets in the same namespace available for pulling the container images"` } // ContainerManifestList is used to communicate container manifests to kubelet. @@ -1581,6 +1585,10 @@ type PodSpec struct { // used must be specified. // Optional: Default to false. HostNetwork bool `json:"hostNetwork,omitempty" description:"host networking requested for this pod"` + // ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used by this PodSpec. + // If specified, these secrets will be passed to individual puller implementations for them to use. For example, + // in the case of docker, only DockerConfig type secrets are honored. + ImagePullSecrets []ObjectReference `json:"imagePullSecrets,omitempty" description:"list of references to secrets in the same namespace available for pulling the container images"` } // List holds a list of objects, which may not be known by the server. diff --git a/pkg/api/v1beta3/conversion_generated.go b/pkg/api/v1beta3/conversion_generated.go index 069390927b..681bb595af 100644 --- a/pkg/api/v1beta3/conversion_generated.go +++ b/pkg/api/v1beta3/conversion_generated.go @@ -2670,6 +2670,16 @@ func convert_v1beta3_PodSpec_To_api_PodSpec(in *PodSpec, out *newer.PodSpec, s c out.ServiceAccount = in.ServiceAccount out.Host = in.Host out.HostNetwork = in.HostNetwork + if in.ImagePullSecrets != nil { + out.ImagePullSecrets = make([]newer.ObjectReference, len(in.ImagePullSecrets)) + for i := range in.ImagePullSecrets { + if err := convert_v1beta3_ObjectReference_To_api_ObjectReference(&in.ImagePullSecrets[i], &out.ImagePullSecrets[i], s); err != nil { + return err + } + } + } else { + out.ImagePullSecrets = nil + } return nil } @@ -2722,6 +2732,16 @@ func convert_api_PodSpec_To_v1beta3_PodSpec(in *newer.PodSpec, out *PodSpec, s c out.ServiceAccount = in.ServiceAccount out.Host = in.Host out.HostNetwork = in.HostNetwork + if in.ImagePullSecrets != nil { + out.ImagePullSecrets = make([]ObjectReference, len(in.ImagePullSecrets)) + for i := range in.ImagePullSecrets { + if err := convert_api_ObjectReference_To_v1beta3_ObjectReference(&in.ImagePullSecrets[i], &out.ImagePullSecrets[i], s); err != nil { + return err + } + } + } else { + out.ImagePullSecrets = nil + } return nil } diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go index 53cd3ef9b8..d190a854d3 100644 --- a/pkg/api/v1beta3/types.go +++ b/pkg/api/v1beta3/types.go @@ -829,6 +829,10 @@ type PodSpec struct { // used must be specified. // Optional: Default to false. HostNetwork bool `json:"hostNetwork,omitempty" description:"host networking requested for this pod"` + // ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used by this PodSpec. + // If specified, these secrets will be passed to individual puller implementations for them to use. For example, + // in the case of docker, only DockerConfig type secrets are honored. + ImagePullSecrets []ObjectReference `json:"imagePullSecrets,omitempty" description:"list of references to secrets in the same namespace available for pulling the container images" patchStrategy:"merge" patchMergeKey:"name"` } // PodStatus represents information about the status of a pod. Status may trail the actual diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 2b519b309e..fb97ab7565 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -21,6 +21,7 @@ import ( "fmt" "net" "path" + "reflect" "regexp" "strings" @@ -890,6 +891,18 @@ func validateHostNetwork(hostNetwork bool, containers []api.Container) errs.Vali return allErrors } +func validateImagePullSecrets(imagePullSecrets []api.ObjectReference) errs.ValidationErrorList { + allErrors := errs.ValidationErrorList{} + for i, currPullSecret := range imagePullSecrets { + strippedRef := api.ObjectReference{Name: currPullSecret.Name} + + if !reflect.DeepEqual(strippedRef, currPullSecret) { + allErrors = append(allErrors, errs.NewFieldInvalid(fmt.Sprintf("[%d]", i), currPullSecret, "only name may be set")) + } + } + return allErrors +} + // ValidatePod tests if required fields in the pod are set. func ValidatePod(pod *api.Pod) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} @@ -913,6 +926,7 @@ func ValidatePodSpec(spec *api.PodSpec) errs.ValidationErrorList { allErrs = append(allErrs, validateDNSPolicy(&spec.DNSPolicy).Prefix("dnsPolicy")...) allErrs = append(allErrs, ValidateLabels(spec.NodeSelector, "nodeSelector")...) allErrs = append(allErrs, validateHostNetwork(spec.HostNetwork, spec.Containers).Prefix("hostNetwork")...) + allErrs = append(allErrs, validateImagePullSecrets(spec.ImagePullSecrets).Prefix("imagePullSecrets")...) if spec.ActiveDeadlineSeconds != nil { if *spec.ActiveDeadlineSeconds <= 0 { diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index bb780eee1a..dab336add2 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -1054,6 +1054,24 @@ func TestValidatePodSpec(t *testing.T) { DNSPolicy: api.DNSClusterFirst, Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}}, }, + "namespace on imagePullSecret": { + // basic valid fields + Volumes: []api.Volume{{Name: "vol", VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}}, + Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}}, + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSClusterFirst, + + ImagePullSecrets: []api.ObjectReference{{Name: "foo", Namespace: "bar"}}, + }, + "kind on imagePullSecret": { + // basic valid fields + Volumes: []api.Volume{{Name: "vol", VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}}, + Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}}, + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSClusterFirst, + + ImagePullSecrets: []api.ObjectReference{{Name: "foo", Kind: "bar"}}, + }, "with hostNetwork hostPort not equal to containerPort": { Containers: []api.Container{ {Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", Ports: []api.ContainerPort{ diff --git a/pkg/credentialprovider/keyring.go b/pkg/credentialprovider/keyring.go index 94855b8a71..334cfadb1a 100644 --- a/pkg/credentialprovider/keyring.go +++ b/pkg/credentialprovider/keyring.go @@ -17,6 +17,7 @@ limitations under the License. package credentialprovider import ( + "encoding/json" "net/url" "sort" "strings" @@ -24,6 +25,7 @@ import ( docker "github.com/fsouza/go-dockerclient" "github.com/golang/glog" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) @@ -164,3 +166,50 @@ type FakeKeyring struct { func (f *FakeKeyring) Lookup(image string) ([]docker.AuthConfiguration, bool) { return f.auth, f.ok } + +// unionDockerKeyring delegates to a set of keyrings. +type unionDockerKeyring struct { + keyrings []DockerKeyring +} + +func (k *unionDockerKeyring) Lookup(image string) ([]docker.AuthConfiguration, bool) { + authConfigs := []docker.AuthConfiguration{} + + for _, subKeyring := range k.keyrings { + if subKeyring == nil { + continue + } + + currAuthResults, _ := subKeyring.Lookup(image) + authConfigs = append(authConfigs, currAuthResults...) + } + + return authConfigs, (len(authConfigs) > 0) +} + +// MakeDockerKeyring inspects the passedSecrets to see if they contain any DockerConfig secrets. If they do, +// then a DockerKeyring is built based on every hit and unioned with the defaultKeyring. +// If they do not, then the default keyring is returned +func MakeDockerKeyring(passedSecrets []api.Secret, defaultKeyring DockerKeyring) (DockerKeyring, error) { + passedCredentials := []DockerConfig{} + for _, passedSecret := range passedSecrets { + if dockercfgBytes, dockercfgExists := passedSecret.Data[api.DockerConfigKey]; (passedSecret.Type == api.SecretTypeDockercfg) && dockercfgExists && (len(dockercfgBytes) > 0) { + dockercfg := DockerConfig{} + if err := json.Unmarshal(dockercfgBytes, &dockercfg); err != nil { + return nil, err + } + + passedCredentials = append(passedCredentials, dockercfg) + } + } + + if len(passedCredentials) > 0 { + basicKeyring := &BasicDockerKeyring{} + for _, currCredentials := range passedCredentials { + basicKeyring.Add(currCredentials) + } + return &unionDockerKeyring{[]DockerKeyring{basicKeyring, defaultKeyring}}, nil + } + + return defaultKeyring, nil +} diff --git a/pkg/kubelet/container/runtime.go b/pkg/kubelet/container/runtime.go index b96424c9dd..6fa2073da4 100644 --- a/pkg/kubelet/container/runtime.go +++ b/pkg/kubelet/container/runtime.go @@ -52,7 +52,7 @@ type Runtime interface { // exited and dead containers (used for garbage collection). GetPods(all bool) ([]*Pod, error) // Syncs the running pod into the desired pod. - SyncPod(pod *api.Pod, runningPod Pod, podStatus api.PodStatus) error + SyncPod(pod *api.Pod, runningPod Pod, podStatus api.PodStatus, pullSecrets []api.Secret) error // KillPod kills all the containers of a pod. KillPod(pod Pod) error // GetPodStatus retrieves the status of the pod, including the information of @@ -60,7 +60,7 @@ type Runtime interface { GetPodStatus(*api.Pod) (*api.PodStatus, error) // PullImage pulls an image from the network to local storage using the supplied // secrets if necessary. - PullImage(image ImageSpec, secrets []api.Secret) error + PullImage(image ImageSpec, pullSecrets []api.Secret) error // IsImagePresent checks whether the container image is already in the local storage. IsImagePresent(image ImageSpec) (bool, error) // Gets all images currently on the machine. diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index 9952d4e429..340838e89e 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -78,7 +78,7 @@ type KubeletContainerName struct { // DockerPuller is an abstract interface for testability. It abstracts image pull operations. type DockerPuller interface { - Pull(image string) error + Pull(image string, secrets []api.Secret) error IsImagePresent(image string) (bool, error) } @@ -113,7 +113,7 @@ func parseImageName(image string) (string, string) { return parsers.ParseRepositoryTag(image) } -func (p dockerPuller) Pull(image string) error { +func (p dockerPuller) Pull(image string, secrets []api.Secret) error { repoToPull, tag := parseImageName(image) // If no tag was specified, use the default "latest". @@ -126,7 +126,12 @@ func (p dockerPuller) Pull(image string) error { Tag: tag, } - creds, haveCredentials := p.keyring.Lookup(repoToPull) + keyring, err := credentialprovider.MakeDockerKeyring(secrets, p.keyring) + if err != nil { + return err + } + + creds, haveCredentials := keyring.Lookup(repoToPull) if !haveCredentials { glog.V(1).Infof("Pulling image %s without credentials", image) @@ -161,9 +166,9 @@ func (p dockerPuller) Pull(image string) error { return utilerrors.NewAggregate(pullErrs) } -func (p throttledDockerPuller) Pull(image string) error { +func (p throttledDockerPuller) Pull(image string, secrets []api.Secret) error { if p.limiter.CanAccept() { - return p.puller.Pull(image) + return p.puller.Pull(image, secrets) } return fmt.Errorf("pull QPS exceeded.") } diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index 8c4e1c0490..7c5394de1f 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/pkg/kubelet/dockertools/docker_test.go @@ -17,6 +17,7 @@ limitations under the License. package dockertools import ( + "encoding/json" "fmt" "hash/adler32" "reflect" @@ -220,7 +221,7 @@ func TestPullWithNoSecrets(t *testing.T) { keyring: fakeKeyring, } - err := dp.Pull(test.imageName) + err := dp.Pull(test.imageName, []api.Secret{}) if err != nil { t.Errorf("unexpected non-nil err: %s", err) continue @@ -237,6 +238,77 @@ func TestPullWithNoSecrets(t *testing.T) { } } +func TestPullWithSecrets(t *testing.T) { + dockercfgAuth := credentialprovider.DockerConfigEntry{ + Username: "passed-user", + Password: "passed-password", + Email: "passed-email", + }.ConvertToDockerConfigCompatible() + dockerCfg := map[string]credentialprovider.DockerConfigEntryWithAuth{"index.docker.io/v1/": dockercfgAuth} + dockercfgContent, err := json.Marshal(dockerCfg) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + tests := map[string]struct { + imageName string + passedSecrets []api.Secret + builtInDockerConfig credentialprovider.DockerConfig + expectedPulls []string + }{ + "no matching secrets": { + "ubuntu", + []api.Secret{}, + credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{}), + []string{"ubuntu:latest using {}"}, + }, + "default keyring secrets": { + "ubuntu", + []api.Secret{}, + credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{"index.docker.io/v1/": {"built-in", "password", "email"}}), + []string{`ubuntu:latest using {"username":"built-in","password":"password","email":"email"}`}, + }, + "default keyring secrets unused": { + "ubuntu", + []api.Secret{}, + credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{"extraneous": {"built-in", "password", "email"}}), + []string{`ubuntu:latest using {}`}, + }, + "builtin keyring secrets, but use passed": { + "ubuntu", + []api.Secret{{Type: api.SecretTypeDockercfg, Data: map[string][]byte{api.DockerConfigKey: dockercfgContent}}}, + credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{"index.docker.io/v1/": {"built-in", "password", "email"}}), + []string{`ubuntu:latest using {"username":"passed-user","password":"passed-password","email":"passed-email"}`}, + }, + } + for _, test := range tests { + builtInKeyRing := &credentialprovider.BasicDockerKeyring{} + builtInKeyRing.Add(test.builtInDockerConfig) + + fakeClient := &FakeDockerClient{} + + dp := dockerPuller{ + client: fakeClient, + keyring: builtInKeyRing, + } + + err := dp.Pull(test.imageName, test.passedSecrets) + if err != nil { + t.Errorf("unexpected non-nil err: %s", err) + continue + } + + if e, a := 1, len(fakeClient.pulled); e != a { + t.Errorf("%s: expected 1 pulled image, got %d: %v", test.imageName, a, fakeClient.pulled) + continue + } + + if e, a := test.expectedPulls, fakeClient.pulled; !reflect.DeepEqual(e, a) { + t.Errorf("%s: expected pull of %v, but got %v", test.imageName, e, a) + } + } +} + func TestDockerKeyringLookupFails(t *testing.T) { fakeKeyring := &credentialprovider.FakeKeyring{} fakeClient := &FakeDockerClient{ @@ -248,7 +320,7 @@ func TestDockerKeyringLookupFails(t *testing.T) { keyring: fakeKeyring, } - err := dp.Pull("host/repository/image:version") + err := dp.Pull("host/repository/image:version", []api.Secret{}) if err == nil { t.Errorf("unexpected non-error") } diff --git a/pkg/kubelet/dockertools/fake_docker_client.go b/pkg/kubelet/dockertools/fake_docker_client.go index 1a29fdef1f..1ae9e0954a 100644 --- a/pkg/kubelet/dockertools/fake_docker_client.go +++ b/pkg/kubelet/dockertools/fake_docker_client.go @@ -26,6 +26,7 @@ import ( "github.com/fsouza/go-dockerclient" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) @@ -327,7 +328,7 @@ type FakeDockerPuller struct { } // Pull records the image pull attempt, and optionally injects an error. -func (f *FakeDockerPuller) Pull(image string) (err error) { +func (f *FakeDockerPuller) Pull(image string, secrets []api.Secret) (err error) { f.Lock() defer f.Unlock() f.ImagesPulled = append(f.ImagesPulled, image) diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index 924945c887..f8a6cba61b 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/pkg/kubelet/dockertools/manager.go @@ -774,7 +774,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, secrets []api.Secret) error { - return dm.Puller.Pull(image.Image) + return dm.Puller.Pull(image.Image, secrets) } // IsImagePresent checks whether the container image is already in the local storage. @@ -1230,7 +1230,7 @@ func (dm *DockerManager) runContainerInPod(pod *api.Pod, container *api.Containe } // createPodInfraContainer starts the pod infra container for a pod. Returns the docker container ID of the newly created container. -func (dm *DockerManager) createPodInfraContainer(pod *api.Pod) (kubeletTypes.DockerID, error) { +func (dm *DockerManager) createPodInfraContainer(pod *api.Pod, pullSecrets []api.Secret) (kubeletTypes.DockerID, error) { // Use host networking if specified. netNamespace := "" var ports []api.ContainerPort @@ -1264,8 +1264,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 err := dm.PullImage(spec, pullSecrets); err != nil { if ref != nil { dm.recorder.Eventf(ref, "failed", "Failed to pull image %q: %v", container.Image, err) } @@ -1438,7 +1437,7 @@ func (dm *DockerManager) clearReasonCache(pod *api.Pod, container *api.Container } // Pull the image for the specified pod and container. -func (dm *DockerManager) pullImage(pod *api.Pod, container *api.Container) error { +func (dm *DockerManager) pullImage(pod *api.Pod, container *api.Container, pullSecrets []api.Secret) error { spec := kubecontainer.ImageSpec{container.Image} present, err := dm.IsImagePresent(spec) @@ -1456,14 +1455,13 @@ 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) + err = dm.PullImage(spec, pullSecrets) dm.runtimeHooks.ReportImagePull(pod, container, err) return err } // Sync the running pod to match the specified desired pod. -func (dm *DockerManager) SyncPod(pod *api.Pod, runningPod kubecontainer.Pod, podStatus api.PodStatus) error { +func (dm *DockerManager) SyncPod(pod *api.Pod, runningPod kubecontainer.Pod, podStatus api.PodStatus, pullSecrets []api.Secret) error { podFullName := kubecontainer.GetPodFullName(pod) containerChanges, err := dm.computePodContainerChanges(pod, runningPod, podStatus) glog.V(3).Infof("Got container changes for pod %q: %+v", podFullName, containerChanges) @@ -1501,7 +1499,7 @@ func (dm *DockerManager) SyncPod(pod *api.Pod, runningPod kubecontainer.Pod, pod podInfraContainerID := containerChanges.InfraContainerId if containerChanges.StartInfraContainer && (len(containerChanges.ContainersToStart) > 0) { glog.V(4).Infof("Creating pod infra container for %q", podFullName) - podInfraContainerID, err = dm.createPodInfraContainer(pod) + podInfraContainerID, err = dm.createPodInfraContainer(pod, pullSecrets) // Call the networking plugin if err == nil { @@ -1517,7 +1515,7 @@ func (dm *DockerManager) SyncPod(pod *api.Pod, runningPod kubecontainer.Pod, pod for idx := range containerChanges.ContainersToStart { container := &pod.Spec.Containers[idx] glog.V(4).Infof("Creating container %+v", container) - err := dm.pullImage(pod, container) + err := dm.pullImage(pod, container, pullSecrets) dm.updateReasonCache(pod, container, err) if err != nil { glog.Warningf("Failed to pull image %q from pod %q and container %q: %v", container.Image, kubecontainer.GetPodFullName(pod), container.Name, err) diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 39e06ed4c5..fa32f5e8b8 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1063,7 +1063,13 @@ func (kl *Kubelet) syncPod(pod *api.Pod, mirrorPod *api.Pod, runningPod kubecont return err } - err = kl.containerRuntime.SyncPod(pod, runningPod, podStatus) + pullSecrets, err := kl.getPullSecretsForPod(pod) + if err != nil { + glog.Errorf("Unable to get pull secrets for pod %q (uid %q): %v", podFullName, uid, err) + return err + } + + err = kl.containerRuntime.SyncPod(pod, runningPod, podStatus, pullSecrets) if err != nil { return err } @@ -1091,6 +1097,24 @@ func (kl *Kubelet) syncPod(pod *api.Pod, mirrorPod *api.Pod, runningPod kubecont return nil } +// getPullSecretsForPod inspects the Pod and retrieves the referenced pull secrets +// TODO transitively search through the referenced service account to find the required secrets +// TODO duplicate secrets are being retrieved multiple times and there is no cache. Creating and using a secret manager interface will make this easier to address. +func (kl *Kubelet) getPullSecretsForPod(pod *api.Pod) ([]api.Secret, error) { + pullSecrets := []api.Secret{} + + for _, secretRef := range pod.Spec.ImagePullSecrets { + secret, err := kl.kubeClient.Secrets(pod.Namespace).Get(secretRef.Name) + if err != nil { + return nil, err + } + + pullSecrets = append(pullSecrets, *secret) + } + + return pullSecrets, nil +} + // Stores all volumes defined by the set of pods into a map. // Keys for each entry are in the format (POD_ID)/(VOLUME_NAME) func getDesiredVolumes(pods []*api.Pod) map[string]api.Volume { diff --git a/pkg/kubelet/rkt/rkt.go b/pkg/kubelet/rkt/rkt.go index 66c0a7aad7..c36393a7a8 100644 --- a/pkg/kubelet/rkt/rkt.go +++ b/pkg/kubelet/rkt/rkt.go @@ -789,7 +789,12 @@ func (r *runtime) PullImage(image kubecontainer.ImageSpec, pullSecrets []api.Sec tag = "latest" } - creds, ok := r.dockerKeyring.Lookup(repoToPull) + keyring, err := credentialprovider.MakeDockerKeyring(pullSecrets, r.dockerKeyring) + if err != nil { + return err + } + + creds, ok := keyring.Lookup(repoToPull) if !ok { glog.V(1).Infof("Pulling image %s without credentials", img) } @@ -827,7 +832,7 @@ func (r *runtime) RemoveImage(image kubecontainer.ImageSpec) error { } // SyncPod syncs the running pod to match the specified desired pod. -func (r *runtime) SyncPod(pod *api.Pod, runningPod kubecontainer.Pod, podStatus api.PodStatus) error { +func (r *runtime) SyncPod(pod *api.Pod, runningPod kubecontainer.Pod, podStatus api.PodStatus, pullSecrets []api.Secret) error { podFullName := kubecontainer.GetPodFullName(pod) if len(runningPod.Containers) == 0 { glog.V(4).Infof("Pod %q is not running, will start it", podFullName) diff --git a/pkg/registry/pod/rest.go b/pkg/registry/pod/rest.go index ff54ff0d0b..b7bc416aef 100644 --- a/pkg/registry/pod/rest.go +++ b/pkg/registry/pod/rest.go @@ -55,6 +55,9 @@ func (podStrategy) PrepareForCreate(obj runtime.Object) { pod.Status = api.PodStatus{ Phase: api.PodPending, } + for i := range pod.Spec.ImagePullSecrets { + pod.Spec.ImagePullSecrets[i] = api.ObjectReference{Name: pod.Spec.ImagePullSecrets[i].Name} + } } // PrepareForUpdate clears fields that are not allowed to be set by end users on update. @@ -62,6 +65,10 @@ func (podStrategy) PrepareForUpdate(obj, old runtime.Object) { newPod := obj.(*api.Pod) oldPod := old.(*api.Pod) newPod.Status = oldPod.Status + + for i := range newPod.Spec.ImagePullSecrets { + newPod.Spec.ImagePullSecrets[i] = api.ObjectReference{Name: newPod.Spec.ImagePullSecrets[i].Name} + } } // Validate validates a new pod.