diff --git a/plugin/pkg/admission/serviceaccount/BUILD b/plugin/pkg/admission/serviceaccount/BUILD index 2bf06f298d..b7e1566d0c 100644 --- a/plugin/pkg/admission/serviceaccount/BUILD +++ b/plugin/pkg/admission/serviceaccount/BUILD @@ -16,6 +16,7 @@ go_library( deps = [ "//pkg/api/pod:go_default_library", "//pkg/apis/core:go_default_library", + "//pkg/features:go_default_library", "//pkg/kubeapiserver/admission/util:go_default_library", "//pkg/serviceaccount:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", @@ -27,6 +28,7 @@ go_library( "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission/initializer:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/names:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/informers:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library", @@ -40,12 +42,14 @@ go_test( deps = [ "//pkg/apis/core:go_default_library", "//pkg/controller:go_default_library", + "//pkg/features:go_default_library", "//pkg/kubelet/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//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/apiserver/pkg/admission:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/informers:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library", diff --git a/plugin/pkg/admission/serviceaccount/admission.go b/plugin/pkg/admission/serviceaccount/admission.go index f9727018fb..a1c9d43db4 100644 --- a/plugin/pkg/admission/serviceaccount/admission.go +++ b/plugin/pkg/admission/serviceaccount/admission.go @@ -21,6 +21,7 @@ import ( "io" "math/rand" "strconv" + "strings" "time" corev1 "k8s.io/api/core/v1" @@ -32,28 +33,34 @@ import ( "k8s.io/apiserver/pkg/admission" genericadmissioninitializer "k8s.io/apiserver/pkg/admission/initializer" "k8s.io/apiserver/pkg/storage/names" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" corev1listers "k8s.io/client-go/listers/core/v1" podutil "k8s.io/kubernetes/pkg/api/pod" api "k8s.io/kubernetes/pkg/apis/core" + kubefeatures "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/kubeapiserver/admission/util" "k8s.io/kubernetes/pkg/serviceaccount" ) -// DefaultServiceAccountName is the name of the default service account to set on pods which do not specify a service account -const DefaultServiceAccountName = "default" +const ( + // DefaultServiceAccountName is the name of the default service account to set on pods which do not specify a service account + DefaultServiceAccountName = "default" -// EnforceMountableSecretsAnnotation is a default annotation that indicates that a service account should enforce mountable secrets. -// The value must be true to have this annotation take effect -const EnforceMountableSecretsAnnotation = "kubernetes.io/enforce-mountable-secrets" + // EnforceMountableSecretsAnnotation is a default annotation that indicates that a service account should enforce mountable secrets. + // The value must be true to have this annotation take effect + EnforceMountableSecretsAnnotation = "kubernetes.io/enforce-mountable-secrets" -// DefaultAPITokenMountPath is the path that ServiceAccountToken secrets are automounted to. -// The token file would then be accessible at /var/run/secrets/kubernetes.io/serviceaccount -const DefaultAPITokenMountPath = "/var/run/secrets/kubernetes.io/serviceaccount" + ServiceAccountVolumeName = "kube-api-access" -// PluginName is the name of this admission plugin -const PluginName = "ServiceAccount" + // DefaultAPITokenMountPath is the path that ServiceAccountToken secrets are automounted to. + // The token file would then be accessible at /var/run/secrets/kubernetes.io/serviceaccount + DefaultAPITokenMountPath = "/var/run/secrets/kubernetes.io/serviceaccount" + + // PluginName is the name of this admission plugin + PluginName = "ServiceAccount" +) // Register registers a plugin func Register(plugins *admission.Plugins) { @@ -79,6 +86,10 @@ type serviceAccount struct { serviceAccountLister corev1listers.ServiceAccountLister secretLister corev1listers.SecretLister + + generateName func(string) string + + featureGate utilfeature.FeatureGate } var _ admission.MutationInterface = &serviceAccount{} @@ -101,6 +112,10 @@ func NewServiceAccount() *serviceAccount { MountServiceAccountToken: true, // Reject pod creation until a service account token is available RequireAPIToken: true, + + generateName: names.SimpleNameGenerator.GenerateName, + + featureGate: utilfeature.DefaultFeatureGate, } } @@ -434,7 +449,8 @@ func (s *serviceAccount) mountServiceAccountToken(serviceAccount *corev1.Service allVolumeNames := sets.NewString() for _, volume := range pod.Spec.Volumes { allVolumeNames.Insert(volume.Name) - if volume.Secret != nil && volume.Secret.SecretName == serviceAccountToken { + if (!s.featureGate.Enabled(kubefeatures.BoundServiceAccountTokenVolume) && volume.Secret != nil && volume.Secret.SecretName == serviceAccountToken) || + (s.featureGate.Enabled(kubefeatures.BoundServiceAccountTokenVolume) && strings.HasPrefix(volume.Name, ServiceAccountVolumeName+"-")) { tokenVolumeName = volume.Name hasTokenVolume = true break @@ -443,10 +459,14 @@ func (s *serviceAccount) mountServiceAccountToken(serviceAccount *corev1.Service // Determine a volume name for the ServiceAccountTokenSecret in case we need it if len(tokenVolumeName) == 0 { - // Try naming the volume the same as the serviceAccountToken, and uniquify if needed - tokenVolumeName = serviceAccountToken - if allVolumeNames.Has(tokenVolumeName) { - tokenVolumeName = names.SimpleNameGenerator.GenerateName(fmt.Sprintf("%s-", serviceAccountToken)) + if s.featureGate.Enabled(kubefeatures.BoundServiceAccountTokenVolume) { + tokenVolumeName = s.generateName(ServiceAccountVolumeName + "-") + } else { + // Try naming the volume the same as the serviceAccountToken, and uniquify if needed + tokenVolumeName = serviceAccountToken + if allVolumeNames.Has(tokenVolumeName) { + tokenVolumeName = s.generateName(fmt.Sprintf("%s-", serviceAccountToken)) + } } } @@ -490,15 +510,61 @@ func (s *serviceAccount) mountServiceAccountToken(serviceAccount *corev1.Service // Add the volume if a container needs it if !hasTokenVolume && needsTokenVolume { - volume := api.Volume{ - Name: tokenVolumeName, - VolumeSource: api.VolumeSource{ - Secret: &api.SecretVolumeSource{ - SecretName: serviceAccountToken, - }, - }, - } - pod.Spec.Volumes = append(pod.Spec.Volumes, volume) + pod.Spec.Volumes = append(pod.Spec.Volumes, s.createVolume(tokenVolumeName, serviceAccountToken)) } return nil } + +func (s *serviceAccount) createVolume(tokenVolumeName, secretName string) api.Volume { + if s.featureGate.Enabled(kubefeatures.BoundServiceAccountTokenVolume) { + return api.Volume{ + Name: tokenVolumeName, + VolumeSource: api.VolumeSource{ + Projected: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{ + { + ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "token", + ExpirationSeconds: 60 * 60, + }, + }, + { + 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", + }, + }, + }, + }, + }, + }, + }, + }, + } + } + return api.Volume{ + Name: tokenVolumeName, + VolumeSource: api.VolumeSource{ + Secret: &api.SecretVolumeSource{ + SecretName: secretName, + }, + }, + } +} diff --git a/plugin/pkg/admission/serviceaccount/admission_test.go b/plugin/pkg/admission/serviceaccount/admission_test.go index 538e700258..be5ca5237f 100644 --- a/plugin/pkg/admission/serviceaccount/admission_test.go +++ b/plugin/pkg/admission/serviceaccount/admission_test.go @@ -28,15 +28,31 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apiserver/pkg/admission" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" corev1listers "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/cache" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/controller" + kubefeatures "k8s.io/kubernetes/pkg/features" kubelet "k8s.io/kubernetes/pkg/kubelet/types" ) +var ( + deprecationDisabledFeature = utilfeature.NewFeatureGate() + deprecationEnabledFeature = utilfeature.NewFeatureGate() +) + +func init() { + if err := deprecationDisabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{kubefeatures.BoundServiceAccountTokenVolume: {Default: false}}); err != nil { + panic(err) + } + if err := deprecationEnabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{kubefeatures.BoundServiceAccountTokenVolume: {Default: true}}); err != nil { + panic(err) + } +} + func TestIgnoresNonCreate(t *testing.T) { for _, op := range []admission.Operation{admission.Delete, admission.Connect} { handler := NewServiceAccount() @@ -267,266 +283,270 @@ func TestDeniesInvalidServiceAccount(t *testing.T) { } func TestAutomountsAPIToken(t *testing.T) { - ns := "myns" - tokenName := "token-name" - serviceAccountName := DefaultServiceAccountName - serviceAccountUID := "12345" + testBoundServiceAccountTokenVolumePhases(t, func(t *testing.T, applyFeatures func(*serviceAccount) *serviceAccount) { - expectedVolume := api.Volume{ - Name: tokenName, - VolumeSource: api.VolumeSource{ - Secret: &api.SecretVolumeSource{SecretName: tokenName}, - }, - } - expectedVolumeMount := api.VolumeMount{ - Name: tokenName, - ReadOnly: true, - MountPath: DefaultAPITokenMountPath, - } + admit := applyFeatures(NewServiceAccount()) + informerFactory := informers.NewSharedInformerFactory(nil, controller.NoResyncPeriodFunc()) + admit.SetExternalKubeInformerFactory(informerFactory) + admit.generateName = testGenerateName + admit.MountServiceAccountToken = true + admit.RequireAPIToken = true - admit := NewServiceAccount() - informerFactory := informers.NewSharedInformerFactory(nil, controller.NoResyncPeriodFunc()) - admit.SetExternalKubeInformerFactory(informerFactory) - admit.MountServiceAccountToken = true - admit.RequireAPIToken = true + ns := "myns" + serviceAccountName := DefaultServiceAccountName + serviceAccountUID := "12345" - // Add the default service account for the ns with a token into the cache - informerFactory.Core().V1().ServiceAccounts().Informer().GetStore().Add(&corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: serviceAccountName, - Namespace: ns, - UID: types.UID(serviceAccountUID), - }, - Secrets: []corev1.ObjectReference{ - {Name: tokenName}, - }, - }) - // Add a token for the service account into the cache - informerFactory.Core().V1().Secrets().Informer().GetStore().Add(&corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ + tokenName := "token-name" + if admit.featureGate.Enabled(kubefeatures.BoundServiceAccountTokenVolume) { + tokenName = generatedVolumeName + } + + expectedVolume := admit.createVolume(tokenName, tokenName) + expectedVolumeMount := api.VolumeMount{ Name: tokenName, - Namespace: ns, - Annotations: map[string]string{ - corev1.ServiceAccountNameKey: serviceAccountName, - corev1.ServiceAccountUIDKey: serviceAccountUID, + ReadOnly: true, + MountPath: DefaultAPITokenMountPath, + } + // Add the default service account for the ns with a token into the cache + informerFactory.Core().V1().ServiceAccounts().Informer().GetStore().Add(&corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: serviceAccountName, + Namespace: ns, + UID: types.UID(serviceAccountUID), }, - }, - Type: corev1.SecretTypeServiceAccountToken, - Data: map[string][]byte{ - api.ServiceAccountTokenKey: []byte("token-data"), - }, - }) - - pod := &api.Pod{ - Spec: api.PodSpec{ - Containers: []api.Container{ - {}, + Secrets: []corev1.ObjectReference{ + {Name: tokenName}, }, - }, - } - attrs := admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, false, nil) - err := admit.Admit(attrs) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - if pod.Spec.ServiceAccountName != DefaultServiceAccountName { - t.Errorf("Expected service account %s assigned, got %s", DefaultServiceAccountName, pod.Spec.ServiceAccountName) - } - if len(pod.Spec.Volumes) != 1 { - t.Fatalf("Expected 1 volume, got %d", len(pod.Spec.Volumes)) - } - if !reflect.DeepEqual(expectedVolume, pod.Spec.Volumes[0]) { - t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolume, pod.Spec.Volumes[0]) - } - if len(pod.Spec.Containers[0].VolumeMounts) != 1 { - t.Fatalf("Expected 1 volume mount, got %d", len(pod.Spec.Containers[0].VolumeMounts)) - } - if !reflect.DeepEqual(expectedVolumeMount, pod.Spec.Containers[0].VolumeMounts[0]) { - t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolumeMount, pod.Spec.Containers[0].VolumeMounts[0]) - } + }) + // Add a token for the service account into the cache + informerFactory.Core().V1().Secrets().Informer().GetStore().Add(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: tokenName, + Namespace: ns, + Annotations: map[string]string{ + corev1.ServiceAccountNameKey: serviceAccountName, + corev1.ServiceAccountUIDKey: serviceAccountUID, + }, + }, + Type: corev1.SecretTypeServiceAccountToken, + Data: map[string][]byte{ + api.ServiceAccountTokenKey: []byte("token-data"), + }, + }) - // Test ServiceAccount admission plugin applies the same changes if the - // operation is an update to an uninitialized pod. - oldPod := &api.Pod{ - Spec: api.PodSpec{ - Containers: []api.Container{ - { - // the volumeMount in the oldPod shouldn't affect the result. - VolumeMounts: []api.VolumeMount{ - { - Name: "wrong-" + tokenName, - ReadOnly: true, - MountPath: DefaultAPITokenMountPath, + 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, false, nil) + err := admit.Admit(attrs) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if pod.Spec.ServiceAccountName != DefaultServiceAccountName { + t.Errorf("Expected service account %s assigned, got %s", DefaultServiceAccountName, pod.Spec.ServiceAccountName) + } + if len(pod.Spec.Volumes) != 1 { + t.Fatalf("Expected 1 volume, got %d", len(pod.Spec.Volumes)) + } + if !reflect.DeepEqual(expectedVolume, pod.Spec.Volumes[0]) { + t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolume, pod.Spec.Volumes[0]) + } + if len(pod.Spec.Containers[0].VolumeMounts) != 1 { + t.Fatalf("Expected 1 volume mount, got %d", len(pod.Spec.Containers[0].VolumeMounts)) + } + if !reflect.DeepEqual(expectedVolumeMount, pod.Spec.Containers[0].VolumeMounts[0]) { + t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolumeMount, pod.Spec.Containers[0].VolumeMounts[0]) + } + + // Test ServiceAccount admission plugin applies the same changes if the + // operation is an update to an uninitialized pod. + oldPod := &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{ + { + // the volumeMount in the oldPod shouldn't affect the result. + VolumeMounts: []api.VolumeMount{ + { + Name: "wrong-" + tokenName, + ReadOnly: true, + MountPath: DefaultAPITokenMountPath, + }, }, }, }, }, - }, - } - // oldPod is not intialized. - oldPod.Initializers = &metav1.Initializers{Pending: []metav1.Initializer{{Name: "init"}}} - pod = &api.Pod{ - Spec: api.PodSpec{ - Containers: []api.Container{ - {}, + } + // oldPod is not intialized. + oldPod.Initializers = &metav1.Initializers{Pending: []metav1.Initializer{{Name: "init"}}} + pod = &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{ + {}, + }, }, - }, - } - attrs = admission.NewAttributesRecord(pod, oldPod, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Update, false, nil) - err = admit.Admit(attrs) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - if pod.Spec.ServiceAccountName != DefaultServiceAccountName { - t.Errorf("Expected service account %s assigned, got %s", DefaultServiceAccountName, pod.Spec.ServiceAccountName) - } - if len(pod.Spec.Volumes) != 1 { - t.Fatalf("Expected 1 volume, got %d", len(pod.Spec.Volumes)) - } - if !reflect.DeepEqual(expectedVolume, pod.Spec.Volumes[0]) { - t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolume, pod.Spec.Volumes[0]) - } - if len(pod.Spec.Containers[0].VolumeMounts) != 1 { - t.Fatalf("Expected 1 volume mount, got %d", len(pod.Spec.Containers[0].VolumeMounts)) - } - if !reflect.DeepEqual(expectedVolumeMount, pod.Spec.Containers[0].VolumeMounts[0]) { - t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolumeMount, pod.Spec.Containers[0].VolumeMounts[0]) - } + } + attrs = admission.NewAttributesRecord(pod, oldPod, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Update, false, nil) + err = admit.Admit(attrs) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if pod.Spec.ServiceAccountName != DefaultServiceAccountName { + t.Errorf("Expected service account %s assigned, got %s", DefaultServiceAccountName, pod.Spec.ServiceAccountName) + } + if len(pod.Spec.Volumes) != 1 { + t.Fatalf("Expected 1 volume, got %d", len(pod.Spec.Volumes)) + } + if !reflect.DeepEqual(expectedVolume, pod.Spec.Volumes[0]) { + t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolume, pod.Spec.Volumes[0]) + } + if len(pod.Spec.Containers[0].VolumeMounts) != 1 { + t.Fatalf("Expected 1 volume mount, got %d", len(pod.Spec.Containers[0].VolumeMounts)) + } + if !reflect.DeepEqual(expectedVolumeMount, pod.Spec.Containers[0].VolumeMounts[0]) { + t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolumeMount, pod.Spec.Containers[0].VolumeMounts[0]) + } - // testing InitContainers - pod = &api.Pod{ - Spec: api.PodSpec{ - InitContainers: []api.Container{ - {}, + // testing InitContainers + pod = &api.Pod{ + Spec: api.PodSpec{ + InitContainers: []api.Container{ + {}, + }, }, - }, - } - attrs = admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, false, nil) - if err := admit.Admit(attrs); err != nil { - t.Errorf("Unexpected error: %v", err) - } - if pod.Spec.ServiceAccountName != DefaultServiceAccountName { - t.Errorf("Expected service account %s assigned, got %s", DefaultServiceAccountName, pod.Spec.ServiceAccountName) - } - if len(pod.Spec.Volumes) != 1 { - t.Fatalf("Expected 1 volume, got %d", len(pod.Spec.Volumes)) - } - if !reflect.DeepEqual(expectedVolume, pod.Spec.Volumes[0]) { - t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolume, pod.Spec.Volumes[0]) - } - if len(pod.Spec.InitContainers[0].VolumeMounts) != 1 { - t.Fatalf("Expected 1 volume mount, got %d", len(pod.Spec.InitContainers[0].VolumeMounts)) - } - if !reflect.DeepEqual(expectedVolumeMount, pod.Spec.InitContainers[0].VolumeMounts[0]) { - t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolumeMount, pod.Spec.InitContainers[0].VolumeMounts[0]) - } + } + attrs = admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, false, nil) + if err := admit.Admit(attrs); err != nil { + t.Errorf("Unexpected error: %v", err) + } + if pod.Spec.ServiceAccountName != DefaultServiceAccountName { + t.Errorf("Expected service account %s assigned, got %s", DefaultServiceAccountName, pod.Spec.ServiceAccountName) + } + if len(pod.Spec.Volumes) != 1 { + t.Fatalf("Expected 1 volume, got %d", len(pod.Spec.Volumes)) + } + if !reflect.DeepEqual(expectedVolume, pod.Spec.Volumes[0]) { + t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolume, pod.Spec.Volumes[0]) + } + if len(pod.Spec.InitContainers[0].VolumeMounts) != 1 { + t.Fatalf("Expected 1 volume mount, got %d", len(pod.Spec.InitContainers[0].VolumeMounts)) + } + if !reflect.DeepEqual(expectedVolumeMount, pod.Spec.InitContainers[0].VolumeMounts[0]) { + t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolumeMount, pod.Spec.InitContainers[0].VolumeMounts[0]) + } + }) } func TestRespectsExistingMount(t *testing.T) { - ns := "myns" - tokenName := "token-name" - serviceAccountName := DefaultServiceAccountName - serviceAccountUID := "12345" + testBoundServiceAccountTokenVolumePhases(t, func(t *testing.T, applyFeatures func(*serviceAccount) *serviceAccount) { + ns := "myns" + tokenName := "token-name" + serviceAccountName := DefaultServiceAccountName + serviceAccountUID := "12345" - expectedVolumeMount := api.VolumeMount{ - Name: "my-custom-mount", - ReadOnly: false, - MountPath: DefaultAPITokenMountPath, - } + expectedVolumeMount := api.VolumeMount{ + Name: "my-custom-mount", + ReadOnly: false, + MountPath: DefaultAPITokenMountPath, + } - admit := NewServiceAccount() - informerFactory := informers.NewSharedInformerFactory(nil, controller.NoResyncPeriodFunc()) - admit.SetExternalKubeInformerFactory(informerFactory) - admit.MountServiceAccountToken = true - admit.RequireAPIToken = true + admit := applyFeatures(NewServiceAccount()) + informerFactory := informers.NewSharedInformerFactory(nil, controller.NoResyncPeriodFunc()) + admit.SetExternalKubeInformerFactory(informerFactory) + admit.MountServiceAccountToken = true + admit.RequireAPIToken = true - // Add the default service account for the ns with a token into the cache - informerFactory.Core().V1().ServiceAccounts().Informer().GetStore().Add(&corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: serviceAccountName, - Namespace: ns, - UID: types.UID(serviceAccountUID), - }, - Secrets: []corev1.ObjectReference{ - {Name: tokenName}, - }, - }) - // Add a token for the service account into the cache - informerFactory.Core().V1().Secrets().Informer().GetStore().Add(&corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: tokenName, - Namespace: ns, - Annotations: map[string]string{ - corev1.ServiceAccountNameKey: serviceAccountName, - corev1.ServiceAccountUIDKey: serviceAccountUID, + // Add the default service account for the ns with a token into the cache + informerFactory.Core().V1().ServiceAccounts().Informer().GetStore().Add(&corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: serviceAccountName, + Namespace: ns, + UID: types.UID(serviceAccountUID), }, - }, - Type: corev1.SecretTypeServiceAccountToken, - Data: map[string][]byte{ - corev1.ServiceAccountTokenKey: []byte("token-data"), - }, - }) + Secrets: []corev1.ObjectReference{ + {Name: tokenName}, + }, + }) + // Add a token for the service account into the cache + informerFactory.Core().V1().Secrets().Informer().GetStore().Add(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: tokenName, + Namespace: ns, + Annotations: map[string]string{ + corev1.ServiceAccountNameKey: serviceAccountName, + corev1.ServiceAccountUIDKey: serviceAccountUID, + }, + }, + Type: corev1.SecretTypeServiceAccountToken, + Data: map[string][]byte{ + corev1.ServiceAccountTokenKey: []byte("token-data"), + }, + }) - // Define a pod with a container that already mounts a volume at the API token path - // Admission should respect that - // Additionally, no volume should be created if no container is going to use it - pod := &api.Pod{ - Spec: api.PodSpec{ - Containers: []api.Container{ - { - VolumeMounts: []api.VolumeMount{ - expectedVolumeMount, + // Define a pod with a container that already mounts a volume at the API token path + // Admission should respect that + // Additionally, no volume should be created if no container is going to use it + pod := &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{ + { + VolumeMounts: []api.VolumeMount{ + expectedVolumeMount, + }, }, }, }, - }, - } - attrs := admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, false, nil) - err := admit.Admit(attrs) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - if pod.Spec.ServiceAccountName != DefaultServiceAccountName { - t.Errorf("Expected service account %s assigned, got %s", DefaultServiceAccountName, pod.Spec.ServiceAccountName) - } - if len(pod.Spec.Volumes) != 0 { - t.Fatalf("Expected 0 volumes (shouldn't create a volume for a secret we don't need), got %d", len(pod.Spec.Volumes)) - } - if len(pod.Spec.Containers[0].VolumeMounts) != 1 { - t.Fatalf("Expected 1 volume mount, got %d", len(pod.Spec.Containers[0].VolumeMounts)) - } - if !reflect.DeepEqual(expectedVolumeMount, pod.Spec.Containers[0].VolumeMounts[0]) { - t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolumeMount, pod.Spec.Containers[0].VolumeMounts[0]) - } + } + attrs := admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, false, nil) + err := admit.Admit(attrs) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if pod.Spec.ServiceAccountName != DefaultServiceAccountName { + t.Errorf("Expected service account %s assigned, got %s", DefaultServiceAccountName, pod.Spec.ServiceAccountName) + } + if len(pod.Spec.Volumes) != 0 { + t.Fatalf("Expected 0 volumes (shouldn't create a volume for a secret we don't need), got %d", len(pod.Spec.Volumes)) + } + if len(pod.Spec.Containers[0].VolumeMounts) != 1 { + t.Fatalf("Expected 1 volume mount, got %d", len(pod.Spec.Containers[0].VolumeMounts)) + } + if !reflect.DeepEqual(expectedVolumeMount, pod.Spec.Containers[0].VolumeMounts[0]) { + t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolumeMount, pod.Spec.Containers[0].VolumeMounts[0]) + } - // check init containers - pod = &api.Pod{ - Spec: api.PodSpec{ - InitContainers: []api.Container{ - { - VolumeMounts: []api.VolumeMount{ - expectedVolumeMount, + // check init containers + pod = &api.Pod{ + Spec: api.PodSpec{ + InitContainers: []api.Container{ + { + VolumeMounts: []api.VolumeMount{ + expectedVolumeMount, + }, }, }, }, - }, - } - attrs = admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, false, nil) - if err := admit.Admit(attrs); err != nil { - t.Errorf("Unexpected error: %v", err) - } - if pod.Spec.ServiceAccountName != DefaultServiceAccountName { - t.Errorf("Expected service account %s assigned, got %s", DefaultServiceAccountName, pod.Spec.ServiceAccountName) - } - if len(pod.Spec.Volumes) != 0 { - t.Fatalf("Expected 0 volumes (shouldn't create a volume for a secret we don't need), got %d", len(pod.Spec.Volumes)) - } - if len(pod.Spec.InitContainers[0].VolumeMounts) != 1 { - t.Fatalf("Expected 1 volume mount, got %d", len(pod.Spec.InitContainers[0].VolumeMounts)) - } - if !reflect.DeepEqual(expectedVolumeMount, pod.Spec.InitContainers[0].VolumeMounts[0]) { - t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolumeMount, pod.Spec.InitContainers[0].VolumeMounts[0]) - } + } + attrs = admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, false, nil) + if err := admit.Admit(attrs); err != nil { + t.Errorf("Unexpected error: %v", err) + } + if pod.Spec.ServiceAccountName != DefaultServiceAccountName { + t.Errorf("Expected service account %s assigned, got %s", DefaultServiceAccountName, pod.Spec.ServiceAccountName) + } + if len(pod.Spec.Volumes) != 0 { + t.Fatalf("Expected 0 volumes (shouldn't create a volume for a secret we don't need), got %d", len(pod.Spec.Volumes)) + } + if len(pod.Spec.InitContainers[0].VolumeMounts) != 1 { + t.Fatalf("Expected 1 volume mount, got %d", len(pod.Spec.InitContainers[0].VolumeMounts)) + } + if !reflect.DeepEqual(expectedVolumeMount, pod.Spec.InitContainers[0].VolumeMounts[0]) { + t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolumeMount, pod.Spec.InitContainers[0].VolumeMounts[0]) + } + }) } func TestAllowsReferencedSecret(t *testing.T) { @@ -950,43 +970,174 @@ func newSecret(secretType corev1.SecretType, namespace, name, serviceAccountName } func TestGetServiceAccountTokens(t *testing.T) { - admit := NewServiceAccount() - indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) - admit.secretLister = corev1listers.NewSecretLister(indexer) + testBoundServiceAccountTokenVolumePhases(t, func(t *testing.T, applyFeatures func(*serviceAccount) *serviceAccount) { + admit := applyFeatures(NewServiceAccount()) + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + admit.secretLister = corev1listers.NewSecretLister(indexer) - ns := "namespace" + ns := "namespace" + serviceAccountUID := "12345" + + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultServiceAccountName, + Namespace: ns, + UID: types.UID(serviceAccountUID), + }, + } + + nonSATokenSecret := newSecret(corev1.SecretTypeDockercfg, ns, "nonSATokenSecret", DefaultServiceAccountName, serviceAccountUID) + indexer.Add(nonSATokenSecret) + + differentSAToken := newSecret(corev1.SecretTypeServiceAccountToken, ns, "differentSAToken", "someOtherSA", "someOtherUID") + indexer.Add(differentSAToken) + + matchingSAToken := newSecret(corev1.SecretTypeServiceAccountToken, ns, "matchingSAToken", DefaultServiceAccountName, serviceAccountUID) + indexer.Add(matchingSAToken) + + tokens, err := admit.getServiceAccountTokens(sa) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(tokens) != 1 { + names := make([]string, 0, len(tokens)) + for _, token := range tokens { + names = append(names, token.Name) + } + t.Fatalf("expected only 1 token, got %v", names) + } + if e, a := matchingSAToken.Name, tokens[0].Name; e != a { + t.Errorf("expected token %s, got %s", e, a) + } + }) +} + +func TestAutomountIsBackwardsCompatible(t *testing.T) { + ns := "myns" + tokenName := "token-name" + serviceAccountName := DefaultServiceAccountName serviceAccountUID := "12345" + defaultTokenName := "default-token-abc123" - sa := &corev1.ServiceAccount{ + expectedVolume := api.Volume{ + Name: defaultTokenName, + VolumeSource: api.VolumeSource{ + Secret: &api.SecretVolumeSource{ + SecretName: defaultTokenName, + }, + }, + } + expectedVolumeMount := api.VolumeMount{ + Name: defaultTokenName, + ReadOnly: true, + MountPath: DefaultAPITokenMountPath, + } + + admit := NewServiceAccount() + admit.generateName = testGenerateName + admit.featureGate = deprecationEnabledFeature + + informerFactory := informers.NewSharedInformerFactory(nil, controller.NoResyncPeriodFunc()) + admit.SetExternalKubeInformerFactory(informerFactory) + admit.MountServiceAccountToken = true + admit.RequireAPIToken = true + + // Add the default service account for the ns with a token into the cache + informerFactory.Core().V1().ServiceAccounts().Informer().GetStore().Add(&corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ - Name: DefaultServiceAccountName, + Name: serviceAccountName, Namespace: ns, UID: types.UID(serviceAccountUID), }, + Secrets: []corev1.ObjectReference{ + {Name: tokenName}, + }, + }) + // Add a token for the service account into the cache + informerFactory.Core().V1().Secrets().Informer().GetStore().Add(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: tokenName, + Namespace: ns, + Annotations: map[string]string{ + corev1.ServiceAccountNameKey: serviceAccountName, + corev1.ServiceAccountUIDKey: serviceAccountUID, + }, + }, + Type: corev1.SecretTypeServiceAccountToken, + Data: map[string][]byte{ + api.ServiceAccountTokenKey: []byte("token-data"), + }, + }) + + pod := &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: "c-1", + VolumeMounts: []api.VolumeMount{ + { + Name: defaultTokenName, + MountPath: DefaultAPITokenMountPath, + ReadOnly: true, + }, + }, + }, + }, + Volumes: []api.Volume{ + { + Name: defaultTokenName, + VolumeSource: api.VolumeSource{ + Secret: &api.SecretVolumeSource{ + SecretName: defaultTokenName, + }, + }, + }, + }, + }, } - - nonSATokenSecret := newSecret(corev1.SecretTypeDockercfg, ns, "nonSATokenSecret", DefaultServiceAccountName, serviceAccountUID) - indexer.Add(nonSATokenSecret) - - differentSAToken := newSecret(corev1.SecretTypeServiceAccountToken, ns, "differentSAToken", "someOtherSA", "someOtherUID") - indexer.Add(differentSAToken) - - matchingSAToken := newSecret(corev1.SecretTypeServiceAccountToken, ns, "matchingSAToken", DefaultServiceAccountName, serviceAccountUID) - indexer.Add(matchingSAToken) - - tokens, err := admit.getServiceAccountTokens(sa) + attrs := admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, false, nil) + err := admit.Admit(attrs) if err != nil { - t.Fatalf("unexpected error: %v", err) + t.Errorf("Unexpected error: %v", err) } - - if len(tokens) != 1 { - names := make([]string, 0, len(tokens)) - for _, token := range tokens { - names = append(names, token.Name) - } - t.Fatalf("expected only 1 token, got %v", names) + if pod.Spec.ServiceAccountName != DefaultServiceAccountName { + t.Errorf("Expected service account %s assigned, got %s", DefaultServiceAccountName, pod.Spec.ServiceAccountName) } - if e, a := matchingSAToken.Name, tokens[0].Name; e != a { - t.Errorf("expected token %s, got %s", e, a) + _ = expectedVolume + _ = expectedVolumeMount + if len(pod.Spec.Volumes) != 1 { + t.Fatalf("Expected 1 volume, got %d", len(pod.Spec.Volumes)) + } + if !reflect.DeepEqual(expectedVolume, pod.Spec.Volumes[0]) { + t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolume, pod.Spec.Volumes[0]) + } + if len(pod.Spec.Containers[0].VolumeMounts) != 1 { + t.Fatalf("Expected 1 volume mount, got %d", len(pod.Spec.Containers[0].VolumeMounts)) + } + if !reflect.DeepEqual(expectedVolumeMount, pod.Spec.Containers[0].VolumeMounts[0]) { + t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolumeMount, pod.Spec.Containers[0].VolumeMounts[0]) } } + +func testGenerateName(n string) string { + return n + "abc123" +} + +var generatedVolumeName = testGenerateName(ServiceAccountVolumeName + "-") + +func testBoundServiceAccountTokenVolumePhases(t *testing.T, f func(*testing.T, func(*serviceAccount) *serviceAccount)) { + t.Run("BoundServiceAccountTokenVolume disabled", func(t *testing.T) { + f(t, func(s *serviceAccount) *serviceAccount { + s.featureGate = deprecationDisabledFeature + return s + }) + }) + + t.Run("BoundServiceAccountTokenVolume enabled", func(t *testing.T) { + f(t, func(s *serviceAccount) *serviceAccount { + s.featureGate = deprecationEnabledFeature + return s + }) + }) +}