From 590bd048a5f8913d014aab1e7dcbddbf5f446f6c Mon Sep 17 00:00:00 2001 From: deads2k Date: Wed, 20 May 2015 13:08:49 -0400 Subject: [PATCH] add pull secrets to service accounts --- pkg/api/types.go | 5 + pkg/api/v1/conversion_generated.go | 20 +++ pkg/api/v1/types.go | 5 + pkg/api/v1beta1/types.go | 5 + pkg/api/v1beta2/types.go | 5 + pkg/api/v1beta3/conversion_generated.go | 20 +++ pkg/api/v1beta3/types.go | 5 + .../pkg/admission/serviceaccount/admission.go | 25 +++- .../serviceaccount/admission_test.go | 125 ++++++++++++++++++ 9 files changed, 211 insertions(+), 4 deletions(-) diff --git a/pkg/api/types.go b/pkg/api/types.go index 8deac6862f9..4532770e9d6 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -1062,6 +1062,11 @@ type ServiceAccount struct { // Secrets is the list of secrets allowed to be used by pods running using this ServiceAccount Secrets []ObjectReference `json:"secrets"` + + // ImagePullSecrets is a list of references to secrets in the same namespace to use for pulling any images + // in pods that reference this ServiceAccount. ImagePullSecrets are distinct from Secrets because Secrets + // can be mounted in the pod, but ImagePullSecrets are only accessed by the kubelet. + ImagePullSecrets []LocalObjectReference `json:"imagePullSecrets,omitempty" description:"list of references to secrets in the same namespace available for pulling container images"` } // ServiceAccountList is a list of ServiceAccount objects diff --git a/pkg/api/v1/conversion_generated.go b/pkg/api/v1/conversion_generated.go index 2d44cdf893b..61f8a39ac45 100644 --- a/pkg/api/v1/conversion_generated.go +++ b/pkg/api/v1/conversion_generated.go @@ -1981,6 +1981,16 @@ func convert_api_ServiceAccount_To_v1_ServiceAccount(in *api.ServiceAccount, out } else { out.Secrets = nil } + if in.ImagePullSecrets != nil { + out.ImagePullSecrets = make([]LocalObjectReference, len(in.ImagePullSecrets)) + for i := range in.ImagePullSecrets { + if err := convert_api_LocalObjectReference_To_v1_LocalObjectReference(&in.ImagePullSecrets[i], &out.ImagePullSecrets[i], s); err != nil { + return err + } + } + } else { + out.ImagePullSecrets = nil + } return nil } @@ -4194,6 +4204,16 @@ func convert_v1_ServiceAccount_To_api_ServiceAccount(in *ServiceAccount, out *ap } else { out.Secrets = nil } + if in.ImagePullSecrets != nil { + out.ImagePullSecrets = make([]api.LocalObjectReference, len(in.ImagePullSecrets)) + for i := range in.ImagePullSecrets { + if err := convert_v1_LocalObjectReference_To_api_LocalObjectReference(&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 498c10ada37..5ad73f5f19f 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -1053,6 +1053,11 @@ type ServiceAccount struct { // Secrets is the list of secrets allowed to be used by pods running using this ServiceAccount Secrets []ObjectReference `json:"secrets,omitempty" description:"list of secrets that can be used by pods running as this service account" patchStrategy:"merge" patchMergeKey:"name"` + + // ImagePullSecrets is a list of references to secrets in the same namespace to use for pulling any images + // in pods that reference this ServiceAccount. ImagePullSecrets are distinct from Secrets because Secrets + // can be mounted in the pod, but ImagePullSecrets are only accessed by the kubelet. + ImagePullSecrets []LocalObjectReference `json:"imagePullSecrets,omitempty" description:"list of references to secrets in the same namespace available for pulling container images"` } // ServiceAccountList is a list of ServiceAccount objects diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go index 9cd06001ab2..42f97bd1866 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -901,6 +901,11 @@ type ServiceAccount struct { // Secrets is the list of secrets allowed to be used by pods running using this ServiceAccount Secrets []ObjectReference `json:"secrets" description:"list of secrets that can be used by pods running as this service account" patchStrategy:"merge" patchMergeKey:"name"` + + // ImagePullSecrets is a list of references to secrets in the same namespace to use for pulling any images + // in pods that reference this ServiceAccount. ImagePullSecrets are distinct from Secrets because Secrets + // can be mounted in the pod, but ImagePullSecrets are only accessed by the kubelet. + ImagePullSecrets []LocalObjectReference `json:"imagePullSecrets,omitempty" description:"list of references to secrets in the same namespace available for pulling container images"` } // ServiceAccountList is a list of ServiceAccount objects diff --git a/pkg/api/v1beta2/types.go b/pkg/api/v1beta2/types.go index 0826c75aeef..cb81ab81645 100644 --- a/pkg/api/v1beta2/types.go +++ b/pkg/api/v1beta2/types.go @@ -905,6 +905,11 @@ type ServiceAccount struct { // Secrets is the list of secrets allowed to be used by pods running using this ServiceAccount Secrets []ObjectReference `json:"secrets" description:"list of secrets that can be used by pods running as this service account" patchStrategy:"merge" patchMergeKey:"name"` + + // ImagePullSecrets is a list of references to secrets in the same namespace to use for pulling any images + // in pods that reference this ServiceAccount. ImagePullSecrets are distinct from Secrets because Secrets + // can be mounted in the pod, but ImagePullSecrets are only accessed by the kubelet. + ImagePullSecrets []LocalObjectReference `json:"imagePullSecrets,omitempty" description:"list of references to secrets in the same namespace available for pulling container images"` } // ServiceAccountList is a list of ServiceAccount objects diff --git a/pkg/api/v1beta3/conversion_generated.go b/pkg/api/v1beta3/conversion_generated.go index 606cadc813b..15ff9db64c4 100644 --- a/pkg/api/v1beta3/conversion_generated.go +++ b/pkg/api/v1beta3/conversion_generated.go @@ -1888,6 +1888,16 @@ func convert_api_ServiceAccount_To_v1beta3_ServiceAccount(in *api.ServiceAccount } else { out.Secrets = nil } + if in.ImagePullSecrets != nil { + out.ImagePullSecrets = make([]LocalObjectReference, len(in.ImagePullSecrets)) + for i := range in.ImagePullSecrets { + if err := convert_api_LocalObjectReference_To_v1beta3_LocalObjectReference(&in.ImagePullSecrets[i], &out.ImagePullSecrets[i], s); err != nil { + return err + } + } + } else { + out.ImagePullSecrets = nil + } return nil } @@ -4038,6 +4048,16 @@ func convert_v1beta3_ServiceAccount_To_api_ServiceAccount(in *ServiceAccount, ou } else { out.Secrets = nil } + if in.ImagePullSecrets != nil { + out.ImagePullSecrets = make([]api.LocalObjectReference, len(in.ImagePullSecrets)) + for i := range in.ImagePullSecrets { + if err := convert_v1beta3_LocalObjectReference_To_api_LocalObjectReference(&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 dcdb007956b..ce5628fd3e3 100644 --- a/pkg/api/v1beta3/types.go +++ b/pkg/api/v1beta3/types.go @@ -1057,6 +1057,11 @@ type ServiceAccount struct { // Secrets is the list of secrets allowed to be used by pods running using this ServiceAccount Secrets []ObjectReference `json:"secrets,omitempty" description:"list of secrets that can be used by pods running as this service account" patchStrategy:"merge" patchMergeKey:"name"` + + // ImagePullSecrets is a list of references to secrets in the same namespace to use for pulling any images + // in pods that reference this ServiceAccount. ImagePullSecrets are distinct from Secrets because Secrets + // can be mounted in the pod, but ImagePullSecrets are only accessed by the kubelet. + ImagePullSecrets []LocalObjectReference `json:"imagePullSecrets,omitempty" description:"list of references to secrets in the same namespace available for pulling container images"` } // ServiceAccountList is a list of ServiceAccount objects diff --git a/plugin/pkg/admission/serviceaccount/admission.go b/plugin/pkg/admission/serviceaccount/admission.go index 6b8d0754753..5e3d486757c 100644 --- a/plugin/pkg/admission/serviceaccount/admission.go +++ b/plugin/pkg/admission/serviceaccount/admission.go @@ -74,7 +74,8 @@ type serviceAccount struct { // 1. If the pod does not specify a ServiceAccount, it sets the pod's ServiceAccount to "default" // 2. It ensures the ServiceAccount referenced by the pod exists // 3. If LimitSecretReferences is true, it rejects the pod if the pod references Secret objects which the pod's ServiceAccount does not reference -// 4. If MountServiceAccountToken is true, it adds a VolumeMount with the pod's ServiceAccount's api token secret to containers +// 4. If the pod does not contain any ImagePullSecrets, the ImagePullSecrets of the service account are added. +// 5. If MountServiceAccountToken is true, it adds a VolumeMount with the pod's ServiceAccount's api token secret to containers func NewServiceAccount(cl client.Interface) *serviceAccount { serviceAccountsIndexer, serviceAccountsReflector := cache.NewNamespaceKeyedIndexerAndReflector( &cache.ListWatch{ @@ -186,6 +187,11 @@ func (s *serviceAccount) Admit(a admission.Attributes) (err error) { } } + if len(pod.Spec.ImagePullSecrets) == 0 { + pod.Spec.ImagePullSecrets = make([]api.LocalObjectReference, len(serviceAccount.ImagePullSecrets)) + copy(pod.Spec.ImagePullSecrets, serviceAccount.ImagePullSecrets) + } + return nil } @@ -282,9 +288,9 @@ func (s *serviceAccount) getServiceAccountTokens(serviceAccount *api.ServiceAcco func (s *serviceAccount) limitSecretReferences(serviceAccount *api.ServiceAccount, pod *api.Pod) error { // Ensure all secrets the pod references are allowed by the service account - referencedSecrets := util.NewStringSet() + mountableSecrets := util.NewStringSet() for _, s := range serviceAccount.Secrets { - referencedSecrets.Insert(s.Name) + mountableSecrets.Insert(s.Name) } for _, volume := range pod.Spec.Volumes { source := volume.VolumeSource @@ -292,10 +298,21 @@ func (s *serviceAccount) limitSecretReferences(serviceAccount *api.ServiceAccoun continue } secretName := source.Secret.SecretName - if !referencedSecrets.Has(secretName) { + if !mountableSecrets.Has(secretName) { return fmt.Errorf("Volume with secret.secretName=\"%s\" is not allowed because service account %s does not reference that secret", secretName, serviceAccount.Name) } } + + // limit pull secret references as well + pullSecrets := util.NewStringSet() + for _, s := range serviceAccount.ImagePullSecrets { + pullSecrets.Insert(s.Name) + } + for i, pullSecretRef := range pod.Spec.ImagePullSecrets { + if !pullSecrets.Has(pullSecretRef.Name) { + return fmt.Errorf(`imagePullSecrets[%d].name="%s" is not allowed because service account %s does not reference that imagePullSecret`, i, pullSecretRef.Name, serviceAccount.Name) + } + } return nil } diff --git a/plugin/pkg/admission/serviceaccount/admission_test.go b/plugin/pkg/admission/serviceaccount/admission_test.go index d7f6b6d0f2e..81977a546a9 100644 --- a/plugin/pkg/admission/serviceaccount/admission_test.go +++ b/plugin/pkg/admission/serviceaccount/admission_test.go @@ -398,3 +398,128 @@ func TestRejectsUnreferencedSecretVolumes(t *testing.T) { t.Errorf("Expected rejection for using a secret the service account does not reference") } } + +func TestAllowsReferencedImagePullSecrets(t *testing.T) { + ns := "myns" + + admit := NewServiceAccount(nil) + admit.LimitSecretReferences = true + + // Add the default service account for the ns with a secret reference into the cache + admit.serviceAccounts.Add(&api.ServiceAccount{ + ObjectMeta: api.ObjectMeta{ + Name: DefaultServiceAccountName, + Namespace: ns, + }, + ImagePullSecrets: []api.LocalObjectReference{ + {Name: "foo"}, + }, + }) + + pod := &api.Pod{ + Spec: api.PodSpec{ + ImagePullSecrets: []api.LocalObjectReference{{Name: "foo"}}, + }, + } + attrs := admission.NewAttributesRecord(pod, "Pod", ns, string(api.ResourcePods), "CREATE", nil) + err := admit.Admit(attrs) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } +} + +func TestRejectsUnreferencedImagePullSecrets(t *testing.T) { + ns := "myns" + + admit := NewServiceAccount(nil) + admit.LimitSecretReferences = true + + // Add the default service account for the ns into the cache + admit.serviceAccounts.Add(&api.ServiceAccount{ + ObjectMeta: api.ObjectMeta{ + Name: DefaultServiceAccountName, + Namespace: ns, + }, + }) + + pod := &api.Pod{ + Spec: api.PodSpec{ + ImagePullSecrets: []api.LocalObjectReference{{Name: "foo"}}, + }, + } + attrs := admission.NewAttributesRecord(pod, "Pod", ns, string(api.ResourcePods), "CREATE", nil) + err := admit.Admit(attrs) + if err == nil { + t.Errorf("Expected rejection for using a secret the service account does not reference") + } +} + +func TestDoNotAddImagePullSecrets(t *testing.T) { + ns := "myns" + + admit := NewServiceAccount(nil) + admit.LimitSecretReferences = true + + // Add the default service account for the ns with a secret reference into the cache + admit.serviceAccounts.Add(&api.ServiceAccount{ + ObjectMeta: api.ObjectMeta{ + Name: DefaultServiceAccountName, + Namespace: ns, + }, + ImagePullSecrets: []api.LocalObjectReference{ + {Name: "foo"}, + {Name: "bar"}, + }, + }) + + pod := &api.Pod{ + Spec: api.PodSpec{ + ImagePullSecrets: []api.LocalObjectReference{{Name: "foo"}}, + }, + } + attrs := admission.NewAttributesRecord(pod, "Pod", ns, string(api.ResourcePods), "CREATE", nil) + err := admit.Admit(attrs) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + if len(pod.Spec.ImagePullSecrets) != 1 || pod.Spec.ImagePullSecrets[0].Name != "foo" { + t.Errorf("unexpected image pull secrets: %v", pod.Spec.ImagePullSecrets) + } +} + +func TestAddImagePullSecrets(t *testing.T) { + ns := "myns" + + admit := NewServiceAccount(nil) + admit.LimitSecretReferences = true + + sa := &api.ServiceAccount{ + ObjectMeta: api.ObjectMeta{ + Name: DefaultServiceAccountName, + Namespace: ns, + }, + ImagePullSecrets: []api.LocalObjectReference{ + {Name: "foo"}, + {Name: "bar"}, + }, + } + // Add the default service account for the ns with a secret reference into the cache + admit.serviceAccounts.Add(sa) + + pod := &api.Pod{} + attrs := admission.NewAttributesRecord(pod, "Pod", ns, string(api.ResourcePods), "CREATE", nil) + err := admit.Admit(attrs) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + if len(pod.Spec.ImagePullSecrets) != 2 || !reflect.DeepEqual(sa.ImagePullSecrets, pod.Spec.ImagePullSecrets) { + t.Errorf("expected %v, got %v", sa.ImagePullSecrets, pod.Spec.ImagePullSecrets) + } + + pod.Spec.ImagePullSecrets[1] = api.LocalObjectReference{Name: "baz"} + if reflect.DeepEqual(sa.ImagePullSecrets, pod.Spec.ImagePullSecrets) { + t.Errorf("accidentally mutated the ServiceAccount.ImagePullSecrets: %v", sa.ImagePullSecrets) + } +}