PodSecurity: seLinuxOptions: cleanup

rename to seLinuxOptions
make message consistent
add unit tests for message
consolidate integration test fixtures

staging/src/k8s.io/pod-security-admission/policy/check_seLinuxOptions.go

@@ -17,6 +17,7 @@ limitations under the License.
 package policy
 
 import (
+	"fmt"
 	"strings"
 
 	corev1 "k8s.io/api/core/v1"
@@ -52,19 +53,19 @@ spec.initContainers[*].securityContext.seLinuxOptions.role
 */
 
 func init() {
-	addCheck(CheckSELinux)
+	addCheck(CheckSELinuxOptions)
 }
 
-// CheckSELinux returns a baseline level check
+// CheckSELinuxOptions returns a baseline level check
 // that limits seLinuxOptions type, user, and role values in 1.0+
-func CheckSELinux() Check {
+func CheckSELinuxOptions() Check {
 	return Check{
-		ID:    "selinux",
+		ID:    "seLinuxOptions",
 		Level: api.LevelBaseline,
 		Versions: []VersionedCheck{
 			{
 				MinimumVersion: api.MajorMinorVersion(1, 0),
-				CheckPod:       checkSelinux_1_0,
+				CheckPod:       seLinuxOptions_1_0,
 			},
 		},
 	}
@@ -74,36 +75,85 @@ var (
 	selinux_allowed_types_1_0 = sets.NewString("", "container_t", "container_init_t", "container_kvm_t")
 )
 
-func checkSelinux_1_0(podMetadata *metav1.ObjectMeta, podSpec *corev1.PodSpec) CheckResult {
+func seLinuxOptions_1_0(podMetadata *metav1.ObjectMeta, podSpec *corev1.PodSpec) CheckResult {
-	var forbiddenDetails []string
+	var (
+		// sources that set bad seLinuxOptions
+		badSetters []string
 
-	checkSelinuxOptions := func(path *field.Path, opts *corev1.SELinuxOptions) {
+		// invalid type values set
+		badTypes = sets.NewString()
+		// was user set?
+		setUser = false
+		// was role set?
+		setRole = false
+	)
+
+	validSELinuxOptions := func(opts *corev1.SELinuxOptions) bool {
+		valid := true
 		if !selinux_allowed_types_1_0.Has(opts.Type) {
-			forbiddenDetails = append(forbiddenDetails, path.Child("securityContext", "seLinuxOptions", "type").String())
+			valid = false
+			badTypes.Insert(opts.Type)
 		}
 		if len(opts.User) > 0 {
-			forbiddenDetails = append(forbiddenDetails, path.Child("securityContext", "seLinuxOptions", "user").String())
+			valid = false
+			setUser = true
 		}
 		if len(opts.Role) > 0 {
-			forbiddenDetails = append(forbiddenDetails, path.Child("securityContext", "seLinuxOptions", "role").String())
+			valid = false
+			setRole = true
 		}
+		return valid
 	}
 
 	if podSpec.SecurityContext != nil && podSpec.SecurityContext.SELinuxOptions != nil {
-		checkSelinuxOptions(field.NewPath("spec"), podSpec.SecurityContext.SELinuxOptions)
+		if !validSELinuxOptions(podSpec.SecurityContext.SELinuxOptions) {
+			badSetters = append(badSetters, "pod")
+		}
 	}
 
+	var badContainers []string
 	visitContainersWithPath(podSpec, field.NewPath("spec"), func(container *corev1.Container, path *field.Path) {
 		if container.SecurityContext != nil && container.SecurityContext.SELinuxOptions != nil {
-			checkSelinuxOptions(path, container.SecurityContext.SELinuxOptions)
+			if !validSELinuxOptions(container.SecurityContext.SELinuxOptions) {
+				badContainers = append(badContainers, container.Name)
+			}
 		}
 	})
+	if len(badContainers) > 0 {
+		badSetters = append(
+			badSetters,
+			fmt.Sprintf(
+				"%s %s",
+				pluralize("container", "containers", len(badContainers)),
+				joinQuote(badContainers),
+			),
+		)
+	}
+
+	if len(badSetters) > 0 {
+		var badData []string
+		if len(badTypes) > 0 {
+			badData = append(badData, fmt.Sprintf(
+				"%s %s",
+				pluralize("type", "types", len(badTypes)),
+				joinQuote(badTypes.List()),
+			))
+			if setUser {
+				badData = append(badData, "user may not be set")
+			}
+			if setRole {
+				badData = append(badData, "role may not be set")
+			}
+		}
 
-	if len(forbiddenDetails) > 0 {
 		return CheckResult{
 			Allowed:         false,
-			ForbiddenReason: "forbidden seLinuxOptions",
+			ForbiddenReason: "seLinuxOptions",
-			ForbiddenDetail: strings.Join(forbiddenDetails, ", "),
+			ForbiddenDetail: fmt.Sprintf(
+				`%s set forbidden securityContext.seLinuxOptions: %s`,
+				strings.Join(badSetters, " and "),
+				strings.Join(badData, "; "),
+			),
 		}
 	}
 	return CheckResult{Allowed: true}

staging/src/k8s.io/pod-security-admission/policy/check_seLinuxOptions_test.go

@@ -0,0 +1,137 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package policy
+
+import (
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+)
+
+func TestSELinuxOptions(t *testing.T) {
+	tests := []struct {
+		name         string
+		pod          *corev1.Pod
+		expectReason string
+		expectDetail string
+	}{
+		{
+			name: "invalid pod and containers",
+			pod: &corev1.Pod{Spec: corev1.PodSpec{
+				SecurityContext: &corev1.PodSecurityContext{
+					SELinuxOptions: &corev1.SELinuxOptions{
+						Type: "foo",
+						User: "bar",
+						Role: "baz",
+					},
+				},
+				Containers: []corev1.Container{
+					{Name: "a", SecurityContext: &corev1.SecurityContext{SELinuxOptions: &corev1.SELinuxOptions{
+						Type: "container_t",
+					}}},
+					{Name: "b", SecurityContext: &corev1.SecurityContext{SELinuxOptions: &corev1.SELinuxOptions{
+						Type: "container_init_t",
+					}}},
+					{Name: "c", SecurityContext: &corev1.SecurityContext{SELinuxOptions: &corev1.SELinuxOptions{
+						Type: "container_kvm_t",
+					}}},
+					{Name: "d", SecurityContext: &corev1.SecurityContext{SELinuxOptions: &corev1.SELinuxOptions{
+						Type: "bar",
+					}}},
+					{Name: "e", SecurityContext: &corev1.SecurityContext{SELinuxOptions: &corev1.SELinuxOptions{
+						User: "bar",
+					}}},
+					{Name: "f", SecurityContext: &corev1.SecurityContext{SELinuxOptions: &corev1.SELinuxOptions{
+						Role: "baz",
+					}}},
+				},
+			}},
+			expectReason: `seLinuxOptions`,
+			expectDetail: `pod and containers "d", "e", "f" set forbidden securityContext.seLinuxOptions: types "bar", "foo"; user may not be set; role may not be set`,
+		},
+		{
+			name: "invalid pod",
+			pod: &corev1.Pod{Spec: corev1.PodSpec{
+				SecurityContext: &corev1.PodSecurityContext{
+					SELinuxOptions: &corev1.SELinuxOptions{
+						Type: "foo",
+						User: "bar",
+						Role: "baz",
+					},
+				},
+				Containers: []corev1.Container{
+					{Name: "a", SecurityContext: &corev1.SecurityContext{SELinuxOptions: &corev1.SELinuxOptions{
+						Type: "container_t",
+					}}},
+					{Name: "b", SecurityContext: &corev1.SecurityContext{SELinuxOptions: &corev1.SELinuxOptions{
+						Type: "container_init_t",
+					}}},
+					{Name: "c", SecurityContext: &corev1.SecurityContext{SELinuxOptions: &corev1.SELinuxOptions{
+						Type: "container_kvm_t",
+					}}},
+				},
+			}},
+			expectReason: `seLinuxOptions`,
+			expectDetail: `pod set forbidden securityContext.seLinuxOptions: type "foo"; user may not be set; role may not be set`,
+		},
+		{
+			name: "invalid containers",
+			pod: &corev1.Pod{Spec: corev1.PodSpec{
+				SecurityContext: &corev1.PodSecurityContext{
+					SELinuxOptions: &corev1.SELinuxOptions{},
+				},
+				Containers: []corev1.Container{
+					{Name: "a", SecurityContext: &corev1.SecurityContext{SELinuxOptions: &corev1.SELinuxOptions{
+						Type: "container_t",
+					}}},
+					{Name: "b", SecurityContext: &corev1.SecurityContext{SELinuxOptions: &corev1.SELinuxOptions{
+						Type: "container_init_t",
+					}}},
+					{Name: "c", SecurityContext: &corev1.SecurityContext{SELinuxOptions: &corev1.SELinuxOptions{
+						Type: "container_kvm_t",
+					}}},
+					{Name: "d", SecurityContext: &corev1.SecurityContext{SELinuxOptions: &corev1.SELinuxOptions{
+						Type: "bar",
+					}}},
+					{Name: "e", SecurityContext: &corev1.SecurityContext{SELinuxOptions: &corev1.SELinuxOptions{
+						User: "bar",
+					}}},
+					{Name: "f", SecurityContext: &corev1.SecurityContext{SELinuxOptions: &corev1.SELinuxOptions{
+						Role: "baz",
+					}}},
+				},
+			}},
+			expectReason: `seLinuxOptions`,
+			expectDetail: `containers "d", "e", "f" set forbidden securityContext.seLinuxOptions: type "bar"; user may not be set; role may not be set`,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			result := seLinuxOptions_1_0(&tc.pod.ObjectMeta, &tc.pod.Spec)
+			if result.Allowed {
+				t.Fatal("expected disallowed")
+			}
+			if e, a := tc.expectReason, result.ForbiddenReason; e != a {
+				t.Errorf("expected\n%s\ngot\n%s", e, a)
+			}
+			if e, a := tc.expectDetail, result.ForbiddenDetail; e != a {
+				t.Errorf("expected\n%s\ngot\n%s", e, a)
+			}
+		})
+	}
+}

staging/src/k8s.io/pod-security-admission/test/fixtures_seLinuxOptions.go

@@ -0,0 +1,87 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package test
+
+import (
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/pod-security-admission/api"
+)
+
+/*
+TODO: include field paths in reflect-based unit test
+
+podFields: []string{
+	`spec.securityContext.seLinuxOptions.type`,
+	`spec.securityContext.seLinuxOptions.user`,
+	`spec.securityContext.seLinuxOptions.role`,
+},
+containerFields: []string{
+	`securityContext.seLinuxOptions.type`,
+	`securityContext.seLinuxOptions.user`,
+	`securityContext.seLinuxOptions.role`,
+},
+*/
+
+func init() {
+	fixtureData_1_0 := fixtureGenerator{
+		expectErrorSubstring: "seLinuxOptions",
+		generatePass: func(p *corev1.Pod) []*corev1.Pod {
+			p = ensureSELinuxOptions(p)
+			return []*corev1.Pod{
+				// security context with no seLinuxOptions
+				tweak(p, func(p *corev1.Pod) {
+					// no pod-level
+					p.Spec.SecurityContext.SELinuxOptions = nil
+					// no container-level
+					p.Spec.Containers[0].SecurityContext.SELinuxOptions = nil
+					// empty container-level
+					p.Spec.InitContainers[0].SecurityContext.SELinuxOptions = &corev1.SELinuxOptions{}
+				}),
+				tweak(p, func(p *corev1.Pod) {
+					// seLinuxOptions with all valid types
+					p.Spec.SecurityContext.SELinuxOptions.Type = "container_t"
+					p.Spec.Containers[0].SecurityContext.SELinuxOptions.Type = "container_init_t"
+					p.Spec.InitContainers[0].SecurityContext.SELinuxOptions.Type = "container_kvm_t"
+					// with empty level and arbitrary level
+					p.Spec.SecurityContext.SELinuxOptions.Level = ""
+					p.Spec.Containers[0].SecurityContext.SELinuxOptions.Level = "somevalue"
+				}),
+			}
+		},
+		generateFail: func(p *corev1.Pod) []*corev1.Pod {
+			p = ensureSELinuxOptions(p)
+			return []*corev1.Pod{
+				// seLinuxOptions with out of bounds type
+				// pod-level
+				tweak(p, func(p *corev1.Pod) { p.Spec.SecurityContext.SELinuxOptions.Type = "somevalue" }),
+				// container
+				tweak(p, func(p *corev1.Pod) { p.Spec.Containers[0].SecurityContext.SELinuxOptions.Type = "somevalue" }),
+				// initContainer
+				tweak(p, func(p *corev1.Pod) { p.Spec.InitContainers[0].SecurityContext.SELinuxOptions.Type = "somevalue" }),
+				// seLinuxOptions with out of bounds user
+				tweak(p, func(p *corev1.Pod) { p.Spec.SecurityContext.SELinuxOptions.User = "somevalue" }),
+				// seLinuxOptions with out of bounds role
+				tweak(p, func(p *corev1.Pod) { p.Spec.SecurityContext.SELinuxOptions.Role = "somevalue" }),
+			}
+		},
+	}
+
+	registerFixtureGenerator(
+		fixtureKey{level: api.LevelBaseline, version: api.MajorMinorVersion(1, 0), check: "seLinuxOptions"},
+		fixtureData_1_0,
+	)
+}

staging/src/k8s.io/pod-security-admission/test/fixtures_selinux.go

@@ -1,98 +0,0 @@
-/*
-Copyright 2021 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package test
-
-import (
-	corev1 "k8s.io/api/core/v1"
-	"k8s.io/pod-security-admission/api"
-)
-
-/*
-TODO: include field paths in reflect-based unit test
-
-podFields: []string{
-	`spec.securityContext.seLinuxOptions.type`,
-	`spec.securityContext.seLinuxOptions.user`,
-	`spec.securityContext.seLinuxOptions.role`,
-},
-containerFields: []string{
-	`securityContext.seLinuxOptions.type`,
-	`securityContext.seLinuxOptions.user`,
-	`securityContext.seLinuxOptions.role`,
-},
-*/
-
-func init() {
-	fixtureData_1_0 := fixtureGenerator{
-		expectErrorSubstring: "seLinuxOptions",
-		generatePass: func(p *corev1.Pod) []*corev1.Pod {
-			p = ensureSELinuxOptions(p)
-			return []*corev1.Pod{
-				// security context with no seLinuxOptions
-				tweak(p, func(p *corev1.Pod) { p.Spec.SecurityContext.SELinuxOptions = nil }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.Containers[0].SecurityContext.SELinuxOptions = nil }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.InitContainers[0].SecurityContext.SELinuxOptions = nil }),
-				// seLinuxOptions with type=""
-				tweak(p, func(p *corev1.Pod) { p.Spec.SecurityContext.SELinuxOptions.Type = "" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.Containers[0].SecurityContext.SELinuxOptions.Type = "" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.InitContainers[0].SecurityContext.SELinuxOptions.Type = "" }),
-				// seLinuxOptions with type="container_t"
-				tweak(p, func(p *corev1.Pod) { p.Spec.SecurityContext.SELinuxOptions.Type = "container_t" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.Containers[0].SecurityContext.SELinuxOptions.Type = "container_t" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.InitContainers[0].SecurityContext.SELinuxOptions.Type = "container_t" }),
-				// seLinuxOptions with type="container_init_t"
-				tweak(p, func(p *corev1.Pod) { p.Spec.SecurityContext.SELinuxOptions.Type = "container_init_t" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.Containers[0].SecurityContext.SELinuxOptions.Type = "container_init_t" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.InitContainers[0].SecurityContext.SELinuxOptions.Type = "container_init_t" }),
-				// seLinuxOptions with type="container_kvm_t"
-				tweak(p, func(p *corev1.Pod) { p.Spec.SecurityContext.SELinuxOptions.Type = "container_kvm_t" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.Containers[0].SecurityContext.SELinuxOptions.Type = "container_kvm_t" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.InitContainers[0].SecurityContext.SELinuxOptions.Type = "container_kvm_t" }),
-				// seLinuxOptions with level=""
-				tweak(p, func(p *corev1.Pod) { p.Spec.SecurityContext.SELinuxOptions.Level = "" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.Containers[0].SecurityContext.SELinuxOptions.Level = "" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.InitContainers[0].SecurityContext.SELinuxOptions.Level = "" }),
-				// seLinuxOptions with arbitrary level=""
-				tweak(p, func(p *corev1.Pod) { p.Spec.SecurityContext.SELinuxOptions.Level = "somevalue" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.Containers[0].SecurityContext.SELinuxOptions.Level = "somevalue" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.InitContainers[0].SecurityContext.SELinuxOptions.Level = "somevalue" }),
-			}
-		},
-		generateFail: func(p *corev1.Pod) []*corev1.Pod {
-			p = ensureSELinuxOptions(p)
-			return []*corev1.Pod{
-				// seLinuxOptions with out of bounds type
-				tweak(p, func(p *corev1.Pod) { p.Spec.SecurityContext.SELinuxOptions.Type = "somevalue" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.Containers[0].SecurityContext.SELinuxOptions.Type = "somevalue" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.InitContainers[0].SecurityContext.SELinuxOptions.Type = "somevalue" }),
-				// seLinuxOptions with out of bounds user
-				tweak(p, func(p *corev1.Pod) { p.Spec.SecurityContext.SELinuxOptions.User = "somevalue" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.Containers[0].SecurityContext.SELinuxOptions.User = "somevalue" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.InitContainers[0].SecurityContext.SELinuxOptions.User = "somevalue" }),
-				// seLinuxOptions with out of bounds role
-				tweak(p, func(p *corev1.Pod) { p.Spec.SecurityContext.SELinuxOptions.Role = "somevalue" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.Containers[0].SecurityContext.SELinuxOptions.Role = "somevalue" }),
-				tweak(p, func(p *corev1.Pod) { p.Spec.InitContainers[0].SecurityContext.SELinuxOptions.Role = "somevalue" }),
-			}
-		},
-	}
-
-	registerFixtureGenerator(
-		fixtureKey{level: api.LevelBaseline, version: api.MajorMinorVersion(1, 0), check: "selinux"},
-		fixtureData_1_0,
-	)
-}