Merge pull request kubernetes#25830 from smarterclayton/init_containe…

…r_psp Automatic merge from submit-queue Add init containers to PSP admission Treat them just like regular containers. @pweil-
lavalamp · May 21, 2016 · 009ae74 · 009ae74
2 parents 4d69e2c + 588f158
commit 009ae74
Show file tree

Hide file tree

Showing 10 changed files with 268 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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/security/podsecuritypolicy/admission.go b/plugin/pkg/admission/security/podsecuritypolicy/admission.go
@@ -186,6 +186,7 @@ func (c *podSecurityPolicyPlugin) Admit(a admission.Attributes) error {
 // the same psp or is not considered valid.
 func assignSecurityContext(provider psp.Provider, pod *api.Pod, fldPath *field.Path) field.ErrorList {
 	generatedSCs := make([]*api.SecurityContext, len(pod.Spec.Containers))
+	var generatedInitSCs []*api.SecurityContext
 
 	errs := field.ErrorList{}
 
@@ -201,6 +202,26 @@ func assignSecurityContext(provider psp.Provider, pod *api.Pod, fldPath *field.P
 	pod.Spec.SecurityContext = psc
 	errs = append(errs, provider.ValidatePodSecurityContext(pod, field.NewPath("spec", "securityContext"))...)
 
+	// Note: this is not changing the original container, we will set container SCs later so long
+	// as all containers validated under the same PSP.
+	for i, containerCopy := range pod.Spec.InitContainers {
+		// We will determine the effective security context for the container and validate against that
+		// since that is how the sc provider will eventually apply settings in the runtime.
+		// This results in an SC that is based on the Pod's PSC with the set fields from the container
+		// overriding pod level settings.
+		containerCopy.SecurityContext = sc.DetermineEffectiveSecurityContext(pod, &containerCopy)
+
+		sc, err := provider.CreateContainerSecurityContext(pod, &containerCopy)
+		if err != nil {
+			errs = append(errs, field.Invalid(field.NewPath("spec", "initContainers").Index(i).Child("securityContext"), "", err.Error()))
+			continue
+		}
+		generatedInitSCs = append(generatedInitSCs, sc)
+
+		containerCopy.SecurityContext = sc
+		errs = append(errs, provider.ValidateContainerSecurityContext(pod, &containerCopy, field.NewPath("spec", "initContainers").Index(i).Child("securityContext"))...)
+	}
+
 	// Note: this is not changing the original container, we will set container SCs later so long
 	// as all containers validated under the same PSP.
 	for i, containerCopy := range pod.Spec.Containers {
@@ -229,6 +250,9 @@ func assignSecurityContext(provider psp.Provider, pod *api.Pod, fldPath *field.P
 
 	// if we've reached this code then we've generated and validated an SC for every container in the
 	// pod so let's apply what we generated.  Note: the psc is already applied.
+	for i, sc := range generatedInitSCs {
+		pod.Spec.InitContainers[i].SecurityContext = sc
+	}
 	for i, sc := range generatedSCs {
 		pod.Spec.Containers[i].SecurityContext = sc
 	}

diff --git a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go
@@ -44,6 +44,12 @@ func NewTestAdmission(store cache.Store, kclient clientset.Interface) kadmission
 	}
 }
 
+func useInitContainers(pod *kapi.Pod) *kapi.Pod {
+	pod.Spec.InitContainers = pod.Spec.Containers
+	pod.Spec.Containers = []kapi.Container{}
+	return pod
+}
+
 func TestAdmitPrivileged(t *testing.T) {
 	createPodWithPriv := func(priv bool) *kapi.Pod {
 		pod := goodPod()
@@ -203,6 +209,18 @@ func TestAdmitCaps(t *testing.T) {
 			}
 		}
 	}
+
+	for k, v := range tc {
+		useInitContainers(v.pod)
+		testPSPAdmit(k, v.psps, v.pod, v.shouldPass, v.expectedPSP, t)
+
+		if v.expectedCapabilities != nil {
+			if !reflect.DeepEqual(v.expectedCapabilities, v.pod.Spec.InitContainers[0].SecurityContext.Capabilities) {
+				t.Errorf("%s resulted in caps that were not expected - expected: %v, received: %v", k, v.expectedCapabilities, v.pod.Spec.InitContainers[0].SecurityContext.Capabilities)
+			}
+		}
+	}
+
 }
 
 func TestAdmitVolumes(t *testing.T) {
@@ -235,6 +253,10 @@ func TestAdmitVolumes(t *testing.T) {
 		// expect a denial for this PSP
 		testPSPAdmit(fmt.Sprintf("%s denial", string(fsType)), []*extensions.PodSecurityPolicy{psp}, pod, false, "", t)
 
+		// also expect a denial for this PSP if it's an init container
+		useInitContainers(pod)
+		testPSPAdmit(fmt.Sprintf("%s denial", string(fsType)), []*extensions.PodSecurityPolicy{psp}, pod, false, "", t)
+
 		// now add the fstype directly to the psp and it should validate
 		psp.Spec.Volumes = []extensions.FSType{fsType}
 		testPSPAdmit(fmt.Sprintf("%s direct accept", string(fsType)), []*extensions.PodSecurityPolicy{psp}, pod, true, psp.Name, t)
@@ -304,6 +326,18 @@ func TestAdmitHostNetwork(t *testing.T) {
 			}
 		}
 	}
+
+	// test again with init containers
+	for k, v := range tests {
+		useInitContainers(v.pod)
+		testPSPAdmit(k, v.psps, v.pod, v.shouldPass, v.expectedPSP, t)
+
+		if v.shouldPass {
+			if v.pod.Spec.SecurityContext.HostNetwork != v.expectedHostNetwork {
+				t.Errorf("%s expected hostNetwork to be %t", k, v.expectedHostNetwork)
+			}
+		}
+	}
 }
 
 func TestAdmitHostPorts(t *testing.T) {

diff --git 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
@@ -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
@@ -328,6 +328,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 {
@@ -399,6 +409,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 {