diff --git a/pkg/api/types.go b/pkg/api/types.go index 3c25441f46f..558426da78c 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -1151,6 +1151,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 6911c3ba1a8..da1de85a2c0 100644 --- a/pkg/api/v1/conversion_generated.go +++ b/pkg/api/v1/conversion_generated.go @@ -2020,6 +2020,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 } @@ -4284,6 +4294,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 5932c792c23..360f34d425e 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -1143,6 +1143,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 99c9281c31d..c31c2a50f49 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -994,6 +994,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 49ef3e5635b..0ddbb6e8f0f 100644 --- a/pkg/api/v1beta2/types.go +++ b/pkg/api/v1beta2/types.go @@ -998,6 +998,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 bb504d1a220..486744c3e30 100644 --- a/pkg/api/v1beta3/conversion_generated.go +++ b/pkg/api/v1beta3/conversion_generated.go @@ -1959,6 +1959,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 } @@ -4156,6 +4166,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 a3c65a8ed56..2f5b9663ca2 100644 --- a/pkg/api/v1beta3/types.go +++ b/pkg/api/v1beta3/types.go @@ -1150,6 +1150,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) + } +}