PodSecurity: hostPathVolumes: cleanup

Rename id to hostPathVolumes
Simplify message construction
Add unit test to exercise messages
Simplify integration test fixtures

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

@@ -21,7 +21,6 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/pod-security-admission/api"
 )
 
@@ -32,34 +31,34 @@ HostPath volumes must be forbidden.
 
 spec.volumes[*].hostPath
 
-**Allowed Values:** undefined/nil
+**Allowed Values:** undefined/null
 */
 
 func init() {
-	addCheck(CheckHostPath)
+	addCheck(CheckHostPathVolumes)
 }
 
-// CheckHostPath returns a baseline level check
-// that requires hostPath=undefined/nil in 1.0+
-func CheckHostPath() Check {
+// CheckHostPathVolumes returns a baseline level check
+// that requires hostPath=undefined/null in 1.0+
+func CheckHostPathVolumes() Check {
 	return Check{
-		ID:    "hostPath",
+		ID:    "hostPathVolumes",
 		Level: api.LevelBaseline,
 		Versions: []VersionedCheck{
 			{
 				MinimumVersion: api.MajorMinorVersion(1, 0),
-				CheckPod:       hostPath_1_0,
+				CheckPod:       hostPathVolumes_1_0,
 			},
 		},
 	}
 }
 
-func hostPath_1_0(podMetadata *metav1.ObjectMeta, podSpec *corev1.PodSpec) CheckResult {
-	hostVolumes := sets.NewString()
+func hostPathVolumes_1_0(podMetadata *metav1.ObjectMeta, podSpec *corev1.PodSpec) CheckResult {
+	var hostVolumes []string
 
 	for _, volume := range podSpec.Volumes {
 		if volume.HostPath != nil {
-			hostVolumes.Insert(volume.Name)
+			hostVolumes = append(hostVolumes, volume.Name)
 		}
 	}
 
@@ -67,7 +66,7 @@ func hostPath_1_0(podMetadata *metav1.ObjectMeta, podSpec *corev1.PodSpec) Check
 		return CheckResult{
 			Allowed:         false,
 			ForbiddenReason: "hostPath volumes",
-			ForbiddenDetail: fmt.Sprintf("volumes %q", hostVolumes.List()),
+			ForbiddenDetail: fmt.Sprintf("%s %s", pluralize("volume", "volumes", len(hostVolumes)), joinQuote(hostVolumes)),
 		}
 	}
 

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

@@ -0,0 +1,60 @@
+/*
+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 TestHostPathVolumes(t *testing.T) {
+	tests := []struct {
+		name         string
+		pod          *corev1.Pod
+		expectReason string
+		expectDetail string
+	}{
+		{
+			name: "host path volumes",
+			pod: &corev1.Pod{Spec: corev1.PodSpec{
+				Volumes: []corev1.Volume{
+					{Name: "a", VolumeSource: corev1.VolumeSource{HostPath: &corev1.HostPathVolumeSource{}}},
+					{Name: "b", VolumeSource: corev1.VolumeSource{HostPath: &corev1.HostPathVolumeSource{}}},
+					{Name: "c", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}},
+				},
+			}},
+			expectReason: `hostPath volumes`,
+			expectDetail: `volumes "a", "b"`,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			result := hostPathVolumes_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_hostPathVolumes.go

@@ -38,14 +38,6 @@ func init() {
 				// mix of hostPath and non-hostPath volumes
 				tweak(p, func(p *corev1.Pod) {
 					p.Spec.Volumes = []corev1.Volume{
-						{
-							Name: "volume-hostpath",
-							VolumeSource: corev1.VolumeSource{
-								HostPath: &corev1.HostPathVolumeSource{
-									Path: "/dev/null",
-								},
-							},
-						},
 						{
 							Name: "volume-emptydir",
 							VolumeSource: corev1.VolumeSource{
@@ -53,27 +45,10 @@ func init() {
 							},
 						},
 						{
-							Name: "volume-configmap",
+							Name: "volume-hostpath",
 							VolumeSource: corev1.VolumeSource{
-								ConfigMap: &corev1.ConfigMapVolumeSource{
-									LocalObjectReference: corev1.LocalObjectReference{
-										Name: "configmap",
-									},
-									Items: []corev1.KeyToPath{
-										{
-											Key:  "log_level",
-											Path: "log_level",
-										},
-									},
-								},
-							},
-						},
-						{
-							Name: "configmap",
-							VolumeSource: corev1.VolumeSource{
-								PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
-									ClaimName: "hello",
-									ReadOnly:  true,
+								HostPath: &corev1.HostPathVolumeSource{
+									Path: "/a",
 								},
 							},
 						},
@@ -83,26 +58,18 @@ func init() {
 				tweak(p, func(p *corev1.Pod) {
 					p.Spec.Volumes = []corev1.Volume{
 						{
-							Name: "volume-hostpath-null",
+							Name: "volume-hostpath-a",
 							VolumeSource: corev1.VolumeSource{
 								HostPath: &corev1.HostPathVolumeSource{
-									Path: "/dev/null",
+									Path: "/a",
 								},
 							},
 						},
 						{
-							Name: "volume-hostpath-docker",
+							Name: "volume-hostpath-b",
 							VolumeSource: corev1.VolumeSource{
 								HostPath: &corev1.HostPathVolumeSource{
-									Path: "/var/lib/docker",
-								},
-							},
-						},
-						{
-							Name: "volume-hostpath-sys",
-							VolumeSource: corev1.VolumeSource{
-								HostPath: &corev1.HostPathVolumeSource{
-									Path: "/sys",
+									Path: "/b",
 								},
 							},
 						},
@@ -113,7 +80,7 @@ func init() {
 	}
 
 	registerFixtureGenerator(
-		fixtureKey{level: api.LevelBaseline, version: api.MajorMinorVersion(1, 0), check: "hostPath"},
+		fixtureKey{level: api.LevelBaseline, version: api.MajorMinorVersion(1, 0), check: "hostPathVolumes"},
 		fixtureData_1_0,
 	)
 }