PodSecurity: restricted capabilities: cleanup

Fix formatting of container names,
Add unit test for containers missing drop, containers with invalid adds
Consolidate integration test fixtures

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

@@ -32,6 +32,25 @@ const (
 	capabilityNetBindService = "NET_BIND_SERVICE"
 )
 
+/*
+Containers must drop ALL, and may only add NET_BIND_SERVICE.
+
+**Restricted Fields:**
+spec.containers[*].securityContext.capabilities.drop
+spec.initContainers[*].securityContext.capabilities.drop
+
+**Allowed Values:**
+Must include "ALL"
+
+**Restricted Fields:**
+spec.containers[*].securityContext.capabilities.add
+spec.initContainers[*].securityContext.capabilities.add
+
+**Allowed Values:**
+undefined / empty
+"NET_BIND_SERVICE"
+*/
+
 func init() {
 	addCheck(CheckCapabilitiesRestricted)
 }
@@ -64,36 +83,46 @@ func capabilitiesRestricted_1_22(podMetadata *metav1.ObjectMeta, podSpec *corev1
 			return
 		}
 
-		found := false
+		droppedAll := false
 		for _, c := range container.SecurityContext.Capabilities.Drop {
 			if c == capabilityAll {
-				found = true
+				droppedAll = true
 				break
 			}
 		}
-		if !found {
+		if !droppedAll {
 			containersMissingDropAll = append(containersMissingDropAll, container.Name)
 		}
 
+		addedForbidden := false
 		for _, c := range container.SecurityContext.Capabilities.Add {
 			if c != capabilityNetBindService {
-				containersAddingForbidden = append(containersAddingForbidden, container.Name)
+				addedForbidden = true
 				forbiddenCapabilities.Insert(string(c))
 			}
 		}
-
+		if addedForbidden {
+			containersAddingForbidden = append(containersAddingForbidden, container.Name)
+		}
 	})
 	var forbiddenDetails []string
 	if len(containersMissingDropAll) > 0 {
-		forbiddenDetails = append(forbiddenDetails, fmt.Sprintf("containers %q must drop ALL", containersMissingDropAll))
+		forbiddenDetails = append(forbiddenDetails, fmt.Sprintf(
+			`%s %s must set securityContext.capabilities.drop=["ALL"]`,
+			pluralize("container", "containers", len(containersMissingDropAll)),
+			joinQuote(containersMissingDropAll)))
 	}
 	if len(containersAddingForbidden) > 0 {
-		forbiddenDetails = append(forbiddenDetails, fmt.Sprintf("containers %q must not add %q", containersAddingForbidden, forbiddenCapabilities.List()))
+		forbiddenDetails = append(forbiddenDetails, fmt.Sprintf(
+			`%s %s must not include %s in securityContext.capabilities.add`,
+			pluralize("container", "containers", len(containersAddingForbidden)),
+			joinQuote(containersAddingForbidden),
+			joinQuote(forbiddenCapabilities.List())))
 	}
 	if len(forbiddenDetails) > 0 {
 		return CheckResult{
 			Allowed:         false,
-			ForbiddenReason: "containers must restrict capabilities",
+			ForbiddenReason: "unrestricted capabilities",
 			ForbiddenDetail: strings.Join(forbiddenDetails, "; "),
 		}
 	}

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

@@ -0,0 +1,59 @@
+/*
+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 TestCapabilitiesRestricted(t *testing.T) {
+	tests := []struct {
+		name         string
+		pod          *corev1.Pod
+		expectReason string
+		expectDetail string
+	}{
+		{
+			name: "multiple containers",
+			pod: &corev1.Pod{Spec: corev1.PodSpec{
+				Containers: []corev1.Container{
+					{Name: "a", SecurityContext: &corev1.SecurityContext{Capabilities: &corev1.Capabilities{Add: []corev1.Capability{"FOO", "BAR"}}}},
+					{Name: "b", SecurityContext: &corev1.SecurityContext{Capabilities: &corev1.Capabilities{Add: []corev1.Capability{"BAR", "BAZ"}}}},
+					{Name: "c", SecurityContext: &corev1.SecurityContext{Capabilities: &corev1.Capabilities{Add: []corev1.Capability{"NET_BIND_SERVICE", "CHOWN"}, Drop: []corev1.Capability{"ALL", "FOO"}}}},
+				}}},
+			expectReason: `unrestricted capabilities`,
+			expectDetail: `containers "a", "b" must set securityContext.capabilities.drop=["ALL"]; containers "a", "b", "c" must not include "BAR", "BAZ", "CHOWN", "FOO" in securityContext.capabilities.add`,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			result := capabilitiesRestricted_1_22(&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_capabilities_restricted.go

@@ -30,14 +30,10 @@ containerFields: []string{
 
 func init() {
 	fixtureData_1_22 := fixtureGenerator{
-		expectErrorSubstring: "restrict capabilities",
+		expectErrorSubstring: "unrestricted capabilities",
 		generatePass: func(p *corev1.Pod) []*corev1.Pod {
 			p = ensureCapabilities(p)
 			return []*corev1.Pod{
-				tweak(p, func(p *corev1.Pod) {
-					p.Spec.Containers[0].SecurityContext.Capabilities.Drop = []corev1.Capability{"ALL"}
-					p.Spec.InitContainers[0].SecurityContext.Capabilities.Drop = []corev1.Capability{"ALL"}
-				}),
 				tweak(p, func(p *corev1.Pod) {
 					p.Spec.Containers[0].SecurityContext.Capabilities.Drop = []corev1.Capability{"ALL"}
 					p.Spec.InitContainers[0].SecurityContext.Capabilities.Drop = []corev1.Capability{"ALL"}
@@ -49,10 +45,15 @@ func init() {
 		generateFail: func(p *corev1.Pod) []*corev1.Pod {
 			p = ensureCapabilities(p)
 			return []*corev1.Pod{
+				// test container
 				tweak(p, func(p *corev1.Pod) {
 					p.Spec.Containers[0].SecurityContext.Capabilities.Drop = []corev1.Capability{}
+				}),
+				// test initContainer
+				tweak(p, func(p *corev1.Pod) {
 					p.Spec.InitContainers[0].SecurityContext.Capabilities.Drop = []corev1.Capability{}
 				}),
+				// dropping all individual capabilities is not sufficient
 				tweak(p, func(p *corev1.Pod) {
 					p.Spec.Containers[0].SecurityContext.Capabilities.Drop = []corev1.Capability{
 						"SYS_TIME", "SYS_MODULE", "SYS_RAWIO", "SYS_PACCT", "SYS_ADMIN", "SYS_NICE",