From 39e373fc4550c5d0e3976af04cdb75b87e0e9b1f Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 9 Jan 2020 13:20:45 -0500 Subject: [PATCH] Do not require token secrets when using bound service account tokens --- plugin/pkg/admission/serviceaccount/BUILD | 1 + .../pkg/admission/serviceaccount/admission.go | 17 +++-- .../serviceaccount/admission_test.go | 68 +++++++++++++++++++ 3 files changed, 81 insertions(+), 5 deletions(-) diff --git a/plugin/pkg/admission/serviceaccount/BUILD b/plugin/pkg/admission/serviceaccount/BUILD index 0222f20cfd7..88639322e49 100644 --- a/plugin/pkg/admission/serviceaccount/BUILD +++ b/plugin/pkg/admission/serviceaccount/BUILD @@ -46,6 +46,7 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission/testing:go_default_library", "//staging/src/k8s.io/client-go/informers:go_default_library", diff --git a/plugin/pkg/admission/serviceaccount/admission.go b/plugin/pkg/admission/serviceaccount/admission.go index 0867718cc6c..17f931890f4 100644 --- a/plugin/pkg/admission/serviceaccount/admission.go +++ b/plugin/pkg/admission/serviceaccount/admission.go @@ -424,12 +424,19 @@ func (s *Plugin) limitSecretReferences(serviceAccount *corev1.ServiceAccount, po } func (s *Plugin) mountServiceAccountToken(serviceAccount *corev1.ServiceAccount, pod *api.Pod) error { - // Find the name of a referenced ServiceAccountToken secret we can mount - serviceAccountToken, err := s.getReferencedServiceAccountToken(serviceAccount) - if err != nil { - return fmt.Errorf("Error looking up service account token for %s/%s: %v", serviceAccount.Namespace, serviceAccount.Name, err) + var ( + // serviceAccountToken holds the name of a secret containing a legacy service account token + serviceAccountToken string + err error + ) + if !s.boundServiceAccountTokenVolume { + // Find the name of a referenced ServiceAccountToken secret we can mount + serviceAccountToken, err = s.getReferencedServiceAccountToken(serviceAccount) + if err != nil { + return fmt.Errorf("Error looking up service account token for %s/%s: %v", serviceAccount.Namespace, serviceAccount.Name, err) + } } - if len(serviceAccountToken) == 0 { + if len(serviceAccountToken) == 0 && !s.boundServiceAccountTokenVolume { // We don't have an API token to mount, so return if s.RequireAPIToken { // If a token is required, this is considered an error diff --git a/plugin/pkg/admission/serviceaccount/admission_test.go b/plugin/pkg/admission/serviceaccount/admission_test.go index 7111a77022f..a8e90b94089 100644 --- a/plugin/pkg/admission/serviceaccount/admission_test.go +++ b/plugin/pkg/admission/serviceaccount/admission_test.go @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apiserver/pkg/admission" admissiontesting "k8s.io/apiserver/pkg/admission/testing" "k8s.io/client-go/informers" @@ -217,6 +218,73 @@ func TestAssignsDefaultServiceAccountAndRejectsMissingAPIToken(t *testing.T) { } } +func TestAssignsDefaultServiceAccountAndBoundTokenWithNoSecretTokens(t *testing.T) { + ns := "myns" + + admit := NewServiceAccount() + informerFactory := informers.NewSharedInformerFactory(nil, controller.NoResyncPeriodFunc()) + admit.SetExternalKubeInformerFactory(informerFactory) + admit.MountServiceAccountToken = true + admit.RequireAPIToken = true + admit.boundServiceAccountTokenVolume = true + + // Add the default service account for the ns into the cache + informerFactory.Core().V1().ServiceAccounts().Informer().GetStore().Add(&corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultServiceAccountName, + Namespace: ns, + }, + }) + + pod := &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{{}}, + }, + } + attrs := admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, nil) + err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil) + if err != nil { + t.Fatalf("Expected success, got: %v", err) + } + + expectedVolumes := []api.Volume{{ + Name: "cleared", + VolumeSource: api.VolumeSource{ + Projected: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{ + {ServiceAccountToken: &api.ServiceAccountTokenProjection{ExpirationSeconds: 3600, Path: "token"}}, + {ConfigMap: &api.ConfigMapProjection{LocalObjectReference: api.LocalObjectReference{Name: "kube-root-ca.crt"}, Items: []api.KeyToPath{{Key: "ca.crt", Path: "ca.crt"}}}}, + {DownwardAPI: &api.DownwardAPIProjection{Items: []api.DownwardAPIVolumeFile{{Path: "namespace", FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.namespace"}}}}}, + }, + }, + }, + }} + expectedVolumeMounts := []api.VolumeMount{{ + Name: "cleared", + ReadOnly: true, + MountPath: "/var/run/secrets/kubernetes.io/serviceaccount", + }} + + // clear generated volume names + for i := range pod.Spec.Volumes { + if len(pod.Spec.Volumes[i].Name) > 0 { + pod.Spec.Volumes[i].Name = "cleared" + } + } + for i := range pod.Spec.Containers[0].VolumeMounts { + if len(pod.Spec.Containers[0].VolumeMounts[i].Name) > 0 { + pod.Spec.Containers[0].VolumeMounts[i].Name = "cleared" + } + } + + if !reflect.DeepEqual(expectedVolumes, pod.Spec.Volumes) { + t.Errorf("unexpected volumes: %s", diff.ObjectReflectDiff(expectedVolumes, pod.Spec.Volumes)) + } + if !reflect.DeepEqual(expectedVolumeMounts, pod.Spec.Containers[0].VolumeMounts) { + t.Errorf("unexpected volumes: %s", diff.ObjectReflectDiff(expectedVolumeMounts, pod.Spec.Containers[0].VolumeMounts)) + } +} + func TestFetchesUncachedServiceAccount(t *testing.T) { ns := "myns"