diff --git a/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go index 0e2b9a7..21ea7af 100644 --- a/internal/pkg/config/config_test.go +++ b/internal/pkg/config/config_test.go @@ -160,6 +160,22 @@ var ( InitContainerCount: 0, ServiceAccount: "someaccount", }, + // also, if we inject a serviceAccount and any container has a VolumeMount + // with a mountPath of /var/run/secrets/kubernetes.io/serviceaccount, we + // must remove it, to allow the ServiceAccountController to inject the + // appropriate token volume + "service-account-default-token": testhelper.ConfigExpectation{ + Name: "service-account-default-token", + Version: "latest", + Path: fixtureSidecarsDir + "/service-account-default-token.yaml", + EnvCount: 0, + ContainerCount: 0, + VolumeCount: 0, + VolumeMountCount: 0, + HostAliasCount: 0, + InitContainerCount: 0, + ServiceAccount: "someaccount", + }, } ) diff --git a/pkg/server/webhook.go b/pkg/server/webhook.go index 2a77df7..a728821 100644 --- a/pkg/server/webhook.go +++ b/pkg/server/webhook.go @@ -26,9 +26,10 @@ const ( ) var ( - runtimeScheme = runtime.NewScheme() - codecs = serializer.NewCodecFactory(runtimeScheme) - deserializer = codecs.UniversalDeserializer() + serviceAccountTokenMountPath = "/var/run/secrets/kubernetes.io/serviceaccount" + runtimeScheme = runtime.NewScheme() + codecs = serializer.NewCodecFactory(runtimeScheme) + deserializer = codecs.UniversalDeserializer() // (https://github.com/kubernetes/kubernetes/issues/57982) defaulter = runtime.ObjectDefaulter(runtimeScheme) @@ -230,27 +231,6 @@ func addContainers(target, added []corev1.Container, basePath string) (patch []p return patch } -func addInitContainers(target, added []corev1.Container, basePath string) (patch []patchOperation) { - first := len(target) == 0 - var value interface{} - for _, add := range added { - value = add - path := basePath - if first { - first = false - value = []corev1.Container{add} - } else { - path = path + "/-" - } - patch = append(patch, patchOperation{ - Op: "add", - Path: path, - Value: value, - }) - } - return patch -} - func addVolumes(target, added []corev1.Volume, basePath string) (patch []patchOperation) { first := len(target) == 0 var value interface{} @@ -328,12 +308,39 @@ func addHostAliases(target, added []corev1.HostAlias, basePath string) (patch [] return patch } -func setServiceAccount(sa string, basePath string) (patch []patchOperation) { +func setServiceAccount(initContainers []corev1.Container, containers []corev1.Container, sa string, basePath string) (patch []patchOperation) { patch = append(patch, patchOperation{ Op: "replace", Path: path.Join(basePath, "serviceAccountName"), Value: sa, }) + + // if we find any pre-existing VolumeMounts that provide the default serviceaccount token, we need to snip + // them out, so the ServiceAccountController will create the correct VolumeMount once we patch this pod + // volumeMounts: + // - name: default-token-wlfz2 + // readOnly: true + // mountPath: /var/run/secrets/kubernetes.io/serviceaccount + for icIndex, ic := range initContainers { + for vmIndex, vm := range ic.VolumeMounts { + if vm.MountPath == serviceAccountTokenMountPath { + patch = append(patch, patchOperation{ + Op: "remove", + Path: fmt.Sprintf("%s/initContainers/%d/volumeMounts/%d", basePath, icIndex, vmIndex), + }) + } + } + } + for cIndex, c := range containers { + for vmIndex, vm := range c.VolumeMounts { + if vm.MountPath == serviceAccountTokenMountPath { + patch = append(patch, patchOperation{ + Op: "remove", + Path: fmt.Sprintf("%s/containers/%d/volumeMounts/%d", basePath, cIndex, vmIndex), + }) + } + } + } return patch } @@ -410,6 +417,14 @@ func updateAnnotations(target map[string]string, added map[string]string) (patch func createPatch(pod *corev1.Pod, inj *config.InjectionConfig, annotations map[string]string) ([]byte, error) { var patch []patchOperation + // be sure to inject the serviceAccountName before adding any volumeMounts, because we must prune out any existing + // volumeMounts that were added to support the default service account. Because this removal is by index, we splice + // them out before appending new volumes at the end. + if inj.ServiceAccountName != "" && (pod.Spec.ServiceAccountName == "" || pod.Spec.ServiceAccountName == "default") { + // only override the serviceaccount name if not set in the pod spec + patch = append(patch, setServiceAccount(pod.Spec.InitContainers, pod.Spec.Containers, inj.ServiceAccountName, "/spec")...) + } + // first, make sure any injected containers in our config get the EnvVars and VolumeMounts injected // this mutates inj.Containers with our environment vars mutatedInjectedContainers := mergeEnvVars(inj.Environment, inj.Containers) @@ -432,11 +447,6 @@ func createPatch(pod *corev1.Pod, inj *config.InjectionConfig, annotations map[s patch = append(patch, addHostAliases(pod.Spec.HostAliases, inj.HostAliases, "/spec/hostAliases")...) patch = append(patch, addVolumes(pod.Spec.Volumes, inj.Volumes, "/spec/volumes")...) - if inj.ServiceAccountName != "" && (pod.Spec.ServiceAccountName == "" || pod.Spec.ServiceAccountName == "default") { - // only override the serviceaccount name if not set in the pod spec - patch = append(patch, setServiceAccount(inj.ServiceAccountName, "/spec")...) - } - // last but not least, set annotations patch = append(patch, updateAnnotations(pod.Annotations, annotations)...) return json.Marshal(patch) diff --git a/pkg/server/webhook_test.go b/pkg/server/webhook_test.go index 68d877a..083c883 100644 --- a/pkg/server/webhook_test.go +++ b/pkg/server/webhook_test.go @@ -63,6 +63,7 @@ var ( {name: "service-account", allowed: true, patchExpected: true}, {name: "service-account-already-set", allowed: true, patchExpected: true}, {name: "service-account-set-default", allowed: true, patchExpected: true}, + {name: "service-account-default-token", allowed: true, patchExpected: true}, } sidecarConfigs, _ = filepath.Glob(path.Join(sidecars, "*.yaml")) expectedNumInjectionConfigs = len(sidecarConfigs) diff --git a/test/fixtures/k8s/admissioncontrol/patch/service-account-default-token.json b/test/fixtures/k8s/admissioncontrol/patch/service-account-default-token.json new file mode 100644 index 0000000..6126e6c --- /dev/null +++ b/test/fixtures/k8s/admissioncontrol/patch/service-account-default-token.json @@ -0,0 +1,20 @@ +[ + { + "op": "replace", + "path": "/spec/serviceAccountName", + "value": "someaccount" + }, + { + "op": "remove", + "path": "/spec/initContainers/0/volumeMounts/0" + }, + { + "op": "remove", + "path": "/spec/containers/1/volumeMounts/1" + }, + { + "op" : "add", + "path" : "/metadata/annotations/injector.unittest.com~1status", + "value" : "injected" + } +] diff --git a/test/fixtures/k8s/admissioncontrol/request/service-account-default-token.yaml b/test/fixtures/k8s/admissioncontrol/request/service-account-default-token.yaml new file mode 100644 index 0000000..499a663 --- /dev/null +++ b/test/fixtures/k8s/admissioncontrol/request/service-account-default-token.yaml @@ -0,0 +1,48 @@ +--- +# this is an AdmissionRequest object +# https://godoc.org/k8s.io/api/admission/v1beta1#AdmissionRequest +object: + metadata: + annotations: + injector.unittest.com/request: "service-account-default-token" + spec: + serviceAccountName: "default" # this should get replaced + volumes: + - name: bogusvolume + configMap: + name: config-production + defaultMode: 420 + - name: default-token-wlfz2 + secret: + secretName: default-token-wlfz2 + defaultMode: 420 + initContainers: + - name: init-ctr1-with-token + volumeMounts: + # this volume mount must be removed, because + # by default, a serviceAccount will mount its token, + # preventing the injected serviceAccount from settings up its mount + - name: default-token-wlfz2 + readOnly: true + mountPath: /var/run/secrets/kubernetes.io/serviceaccount + - name: init-ctr2 + volumeMounts: [] + containers: + - name: ctr1 + volumeMounts: + - name: bogusvolume + readOnly: true + mountPath: /app/config + - name: ctr2-with-token + volumeMounts: + - name: bogusvolume + readOnly: true + mountPath: /app/config + # this volume mount must be removed, because + # by default, a serviceAccount will mount its token, + # preventing the injected serviceAccount from settings up its mount + - name: default-token-wlfz2 + readOnly: true + mountPath: /var/run/secrets/kubernetes.io/serviceaccount + - name: ctr3 + volumeMounts: [] diff --git a/test/fixtures/sidecars/service-account-default-token.yaml b/test/fixtures/sidecars/service-account-default-token.yaml new file mode 100644 index 0000000..4c79686 --- /dev/null +++ b/test/fixtures/sidecars/service-account-default-token.yaml @@ -0,0 +1,3 @@ +--- +name: service-account-default-token +serviceAccountName: someaccount