From 588f15844b50ce038d0a582cd020cb057c0497ef Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 18 May 2016 22:32:08 -0400 Subject: [PATCH] Add init container support to other admission controllers --- .../admission/alwayspullimages/admission.go | 4 + .../alwayspullimages/admission_test.go | 11 ++ plugin/pkg/admission/exec/admission.go | 8 ++ plugin/pkg/admission/exec/admission_test.go | 30 +++++ .../securitycontext/scdeny/admission.go | 11 ++ .../securitycontext/scdeny/admission_test.go | 22 +++- .../pkg/admission/serviceaccount/admission.go | 24 ++++ .../serviceaccount/admission_test.go | 104 ++++++++++++++++++ 8 files changed, 210 insertions(+), 4 deletions(-) diff --git a/plugin/pkg/admission/alwayspullimages/admission.go b/plugin/pkg/admission/alwayspullimages/admission.go index 0feb6dc9eaf..2d2b5147efb 100644 --- a/plugin/pkg/admission/alwayspullimages/admission.go +++ b/plugin/pkg/admission/alwayspullimages/admission.go @@ -56,6 +56,10 @@ func (a *alwaysPullImages) Admit(attributes admission.Attributes) (err error) { return apierrors.NewBadRequest("Resource was marked with kind Pod but was unable to be converted") } + for i := range pod.Spec.InitContainers { + pod.Spec.InitContainers[i].ImagePullPolicy = api.PullAlways + } + for i := range pod.Spec.Containers { pod.Spec.Containers[i].ImagePullPolicy = api.PullAlways } diff --git a/plugin/pkg/admission/alwayspullimages/admission_test.go b/plugin/pkg/admission/alwayspullimages/admission_test.go index 3393ac76564..da9d044923d 100644 --- a/plugin/pkg/admission/alwayspullimages/admission_test.go +++ b/plugin/pkg/admission/alwayspullimages/admission_test.go @@ -32,6 +32,12 @@ func TestAdmission(t *testing.T) { pod := api.Pod{ ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace}, Spec: api.PodSpec{ + InitContainers: []api.Container{ + {Name: "init1", Image: "image"}, + {Name: "init2", Image: "image", ImagePullPolicy: api.PullNever}, + {Name: "init3", Image: "image", ImagePullPolicy: api.PullIfNotPresent}, + {Name: "init4", Image: "image", ImagePullPolicy: api.PullAlways}, + }, Containers: []api.Container{ {Name: "ctr1", Image: "image"}, {Name: "ctr2", Image: "image", ImagePullPolicy: api.PullNever}, @@ -44,6 +50,11 @@ func TestAdmission(t *testing.T) { if err != nil { t.Errorf("Unexpected error returned from admission handler") } + for _, c := range pod.Spec.InitContainers { + if c.ImagePullPolicy != api.PullAlways { + t.Errorf("Container %v: expected pull always, got %v", c, c.ImagePullPolicy) + } + } for _, c := range pod.Spec.Containers { if c.ImagePullPolicy != api.PullAlways { t.Errorf("Container %v: expected pull always, got %v", c, c.ImagePullPolicy) diff --git a/plugin/pkg/admission/exec/admission.go b/plugin/pkg/admission/exec/admission.go index 737ee693672..d98bff6759d 100644 --- a/plugin/pkg/admission/exec/admission.go +++ b/plugin/pkg/admission/exec/admission.go @@ -108,6 +108,14 @@ func (d *denyExec) Admit(a admission.Attributes) (err error) { // isPrivileged will return true a pod has any privileged containers func isPrivileged(pod *api.Pod) bool { + for _, c := range pod.Spec.InitContainers { + if c.SecurityContext == nil { + continue + } + if *c.SecurityContext.Privileged { + return true + } + } for _, c := range pod.Spec.Containers { if c.SecurityContext == nil { continue diff --git a/plugin/pkg/admission/exec/admission_test.go b/plugin/pkg/admission/exec/admission_test.go index 748ecc30c72..c85ed8c3835 100644 --- a/plugin/pkg/admission/exec/admission_test.go +++ b/plugin/pkg/admission/exec/admission_test.go @@ -85,6 +85,29 @@ func TestAdmission(t *testing.T) { for _, tc := range testCases { testAdmission(t, tc.pod, handler, true) } + + // run against an init container + handler = &denyExec{ + Handler: admission.NewHandler(admission.Connect), + hostIPC: true, + hostPID: true, + privileged: true, + } + + for _, tc := range testCases { + tc.pod.Spec.InitContainers = tc.pod.Spec.Containers + tc.pod.Spec.Containers = nil + testAdmission(t, tc.pod, handler, tc.shouldAccept) + } + + // run with a permissive config and all cases should pass + handler.privileged = false + handler.hostPID = false + handler.hostIPC = false + + for _, tc := range testCases { + testAdmission(t, tc.pod, handler, true) + } } func testAdmission(t *testing.T, pod *api.Pod, handler *denyExec, shouldAccept bool) { @@ -173,6 +196,13 @@ func TestDenyExecOnPrivileged(t *testing.T) { for _, tc := range testCases { testAdmission(t, tc.pod, handler, tc.shouldAccept) } + + // test init containers + for _, tc := range testCases { + tc.pod.Spec.InitContainers = tc.pod.Spec.Containers + tc.pod.Spec.Containers = nil + testAdmission(t, tc.pod, handler, tc.shouldAccept) + } } func validPod(name string) *api.Pod { diff --git a/plugin/pkg/admission/securitycontext/scdeny/admission.go b/plugin/pkg/admission/securitycontext/scdeny/admission.go index c2d7ca61262..c7fa3901695 100644 --- a/plugin/pkg/admission/securitycontext/scdeny/admission.go +++ b/plugin/pkg/admission/securitycontext/scdeny/admission.go @@ -74,6 +74,17 @@ func (p *plugin) Admit(a admission.Attributes) (err error) { return apierrors.NewForbidden(a.GetResource().GroupResource(), pod.Name, fmt.Errorf("SecurityContext.FSGroup is forbidden")) } + for _, v := range pod.Spec.InitContainers { + if v.SecurityContext != nil { + if v.SecurityContext.SELinuxOptions != nil { + return apierrors.NewForbidden(a.GetResource().GroupResource(), pod.Name, fmt.Errorf("SecurityContext.SELinuxOptions is forbidden")) + } + if v.SecurityContext.RunAsUser != nil { + return apierrors.NewForbidden(a.GetResource().GroupResource(), pod.Name, fmt.Errorf("SecurityContext.RunAsUser is forbidden")) + } + } + } + for _, v := range pod.Spec.Containers { if v.SecurityContext != nil { if v.SecurityContext.SELinuxOptions != nil { diff --git a/plugin/pkg/admission/securitycontext/scdeny/admission_test.go b/plugin/pkg/admission/securitycontext/scdeny/admission_test.go index ab965548f7b..aeb6585200f 100644 --- a/plugin/pkg/admission/securitycontext/scdeny/admission_test.go +++ b/plugin/pkg/admission/securitycontext/scdeny/admission_test.go @@ -78,11 +78,25 @@ func TestAdmission(t *testing.T) { } for _, tc := range cases { - pod := pod() - pod.Spec.SecurityContext = tc.podSc - pod.Spec.Containers[0].SecurityContext = tc.sc + p := pod() + p.Spec.SecurityContext = tc.podSc + p.Spec.Containers[0].SecurityContext = tc.sc - err := handler.Admit(admission.NewAttributesRecord(pod, api.Kind("Pod").WithVersion("version"), "foo", "name", api.Resource("pods").WithVersion("version"), "", "ignored", nil)) + err := handler.Admit(admission.NewAttributesRecord(p, api.Kind("Pod").WithVersion("version"), "foo", "name", api.Resource("pods").WithVersion("version"), "", "ignored", nil)) + if err != nil && !tc.expectError { + t.Errorf("%v: unexpected error: %v", tc.name, err) + } else if err == nil && tc.expectError { + t.Errorf("%v: expected error", tc.name) + } + + // verify init containers are also checked + p = pod() + p.Spec.SecurityContext = tc.podSc + p.Spec.Containers[0].SecurityContext = tc.sc + p.Spec.InitContainers = p.Spec.Containers + p.Spec.Containers = nil + + err = handler.Admit(admission.NewAttributesRecord(p, api.Kind("Pod").WithVersion("version"), "foo", "name", api.Resource("pods").WithVersion("version"), "", "ignored", nil)) if err != nil && !tc.expectError { t.Errorf("%v: unexpected error: %v", tc.name, err) } else if err == nil && tc.expectError { diff --git a/plugin/pkg/admission/serviceaccount/admission.go b/plugin/pkg/admission/serviceaccount/admission.go index 55b7bfaa6fc..de2dc7d2e92 100644 --- a/plugin/pkg/admission/serviceaccount/admission.go +++ b/plugin/pkg/admission/serviceaccount/admission.go @@ -324,6 +324,16 @@ func (s *serviceAccount) limitSecretReferences(serviceAccount *api.ServiceAccoun } } + for _, container := range pod.Spec.InitContainers { + for _, env := range container.Env { + if env.ValueFrom != nil && env.ValueFrom.SecretKeyRef != nil { + if !mountableSecrets.Has(env.ValueFrom.SecretKeyRef.Name) { + return fmt.Errorf("Init container %s with envVar %s referencing secret.secretName=\"%s\" is not allowed because service account %s does not reference that secret", container.Name, env.Name, env.ValueFrom.SecretKeyRef.Name, serviceAccount.Name) + } + } + } + } + for _, container := range pod.Spec.Containers { for _, env := range container.Env { if env.ValueFrom != nil && env.ValueFrom.SecretKeyRef != nil { @@ -394,6 +404,20 @@ func (s *serviceAccount) mountServiceAccountToken(serviceAccount *api.ServiceAcc // Ensure every container mounts the APISecret volume needsTokenVolume := false + for i, container := range pod.Spec.InitContainers { + existingContainerMount := false + for _, volumeMount := range container.VolumeMounts { + // Existing mounts at the default mount path prevent mounting of the API token + if volumeMount.MountPath == DefaultAPITokenMountPath { + existingContainerMount = true + break + } + } + if !existingContainerMount { + pod.Spec.InitContainers[i].VolumeMounts = append(pod.Spec.InitContainers[i].VolumeMounts, volumeMount) + needsTokenVolume = true + } + } for i, container := range pod.Spec.Containers { existingContainerMount := false for _, volumeMount := range container.VolumeMounts { diff --git a/plugin/pkg/admission/serviceaccount/admission_test.go b/plugin/pkg/admission/serviceaccount/admission_test.go index f6d9addbbf1..2e6301d6c5e 100644 --- a/plugin/pkg/admission/serviceaccount/admission_test.go +++ b/plugin/pkg/admission/serviceaccount/admission_test.go @@ -290,6 +290,33 @@ func TestAutomountsAPIToken(t *testing.T) { 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]) } + + pod = &api.Pod{ + Spec: api.PodSpec{ + InitContainers: []api.Container{ + {}, + }, + }, + } + attrs = admission.NewAttributesRecord(pod, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, 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) { @@ -366,6 +393,35 @@ func TestRespectsExistingMount(t *testing.T) { 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, + }, + }, + }, + }, + } + attrs = admission.NewAttributesRecord(pod, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, 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) { @@ -421,6 +477,30 @@ func TestAllowsReferencedSecret(t *testing.T) { if err := admit.Admit(attrs); err != nil { t.Errorf("Unexpected error: %v", err) } + + pod2 = &api.Pod{ + Spec: api.PodSpec{ + InitContainers: []api.Container{ + { + Name: "container-1", + Env: []api.EnvVar{ + { + Name: "env-1", + ValueFrom: &api.EnvVarSource{ + SecretKeyRef: &api.SecretKeySelector{ + LocalObjectReference: api.LocalObjectReference{Name: "foo"}, + }, + }, + }, + }, + }, + }, + }, + } + attrs = admission.NewAttributesRecord(pod2, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, nil) + if err := admit.Admit(attrs); err != nil { + t.Errorf("Unexpected error: %v", err) + } } func TestRejectsUnreferencedSecretVolumes(t *testing.T) { @@ -473,6 +553,30 @@ func TestRejectsUnreferencedSecretVolumes(t *testing.T) { if err := admit.Admit(attrs); err == nil || !strings.Contains(err.Error(), "with envVar") { t.Errorf("Unexpected error: %v", err) } + + pod2 = &api.Pod{ + Spec: api.PodSpec{ + InitContainers: []api.Container{ + { + Name: "container-1", + Env: []api.EnvVar{ + { + Name: "env-1", + ValueFrom: &api.EnvVarSource{ + SecretKeyRef: &api.SecretKeySelector{ + LocalObjectReference: api.LocalObjectReference{Name: "foo"}, + }, + }, + }, + }, + }, + }, + }, + } + attrs = admission.NewAttributesRecord(pod2, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, nil) + if err := admit.Admit(attrs); err == nil || !strings.Contains(err.Error(), "with envVar") { + t.Errorf("Unexpected error: %v", err) + } } func TestAllowUnreferencedSecretVolumesForPermissiveSAs(t *testing.T) {