Merge pull request #98918 from pohly/psp-ephemeral-validation

PSP ephemeral volume validation

pkg/apis/policy/types.go

@@ -309,6 +309,7 @@ const (
 	PortworxVolume        FSType = "portworxVolume"
 	ScaleIO               FSType = "scaleIO"
 	CSI                   FSType = "csi"
+	Ephemeral             FSType = "ephemeral"
 	All                   FSType = "*"
 )
 

pkg/apis/policy/validation/validation.go

@@ -92,19 +92,26 @@ func ValidatePodDisruptionBudgetStatusUpdate(status, oldStatus policy.PodDisrupt
 // trailing dashes are allowed.
 var ValidatePodSecurityPolicyName = apimachineryvalidation.NameIsDNSSubdomain
 
+// PodSecurityPolicyValidationOptions contains additional parameters for ValidatePodSecurityPolicy.
+type PodSecurityPolicyValidationOptions struct {
+	// AllowEphemeralVolumeType determines whether Ephemeral is a valid entry
+	// in PodSecurityPolicySpec.Volumes.
+	AllowEphemeralVolumeType bool
+}
+
 // ValidatePodSecurityPolicy validates a PodSecurityPolicy and returns an ErrorList
 // with any errors.
-func ValidatePodSecurityPolicy(psp *policy.PodSecurityPolicy) field.ErrorList {
+func ValidatePodSecurityPolicy(psp *policy.PodSecurityPolicy, opts PodSecurityPolicyValidationOptions) field.ErrorList {
 	allErrs := field.ErrorList{}
 	allErrs = append(allErrs, apivalidation.ValidateObjectMeta(&psp.ObjectMeta, false, ValidatePodSecurityPolicyName, field.NewPath("metadata"))...)
 	allErrs = append(allErrs, ValidatePodSecurityPolicySpecificAnnotations(psp.Annotations, field.NewPath("metadata").Child("annotations"))...)
-	allErrs = append(allErrs, ValidatePodSecurityPolicySpec(&psp.Spec, field.NewPath("spec"))...)
+	allErrs = append(allErrs, ValidatePodSecurityPolicySpec(&psp.Spec, opts, field.NewPath("spec"))...)
 	return allErrs
 }
 
 // ValidatePodSecurityPolicySpec validates a PodSecurityPolicySpec and returns an ErrorList
 // with any errors.
-func ValidatePodSecurityPolicySpec(spec *policy.PodSecurityPolicySpec, fldPath *field.Path) field.ErrorList {
+func ValidatePodSecurityPolicySpec(spec *policy.PodSecurityPolicySpec, opts PodSecurityPolicyValidationOptions, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
 	allErrs = append(allErrs, validatePSPRunAsUser(fldPath.Child("runAsUser"), &spec.RunAsUser)...)
@@ -112,7 +119,7 @@ func ValidatePodSecurityPolicySpec(spec *policy.PodSecurityPolicySpec, fldPath *
 	allErrs = append(allErrs, validatePSPSELinux(fldPath.Child("seLinux"), &spec.SELinux)...)
 	allErrs = append(allErrs, validatePSPSupplementalGroup(fldPath.Child("supplementalGroups"), &spec.SupplementalGroups)...)
 	allErrs = append(allErrs, validatePSPFSGroup(fldPath.Child("fsGroup"), &spec.FSGroup)...)
-	allErrs = append(allErrs, validatePodSecurityPolicyVolumes(fldPath, spec.Volumes)...)
+	allErrs = append(allErrs, validatePodSecurityPolicyVolumes(opts, fldPath, spec.Volumes)...)
 	if len(spec.RequiredDropCapabilities) > 0 && hasCap(policy.AllowAllCapabilities, spec.AllowedCapabilities) {
 		allErrs = append(allErrs, field.Invalid(field.NewPath("requiredDropCapabilities"), spec.RequiredDropCapabilities,
 			"must be empty when all capabilities are allowed by a wildcard"))
@@ -320,11 +327,15 @@ func validatePSPSupplementalGroup(fldPath *field.Path, groupOptions *policy.Supp
 }
 
 // validatePodSecurityPolicyVolumes validates the volume fields of PodSecurityPolicy.
-func validatePodSecurityPolicyVolumes(fldPath *field.Path, volumes []policy.FSType) field.ErrorList {
+func validatePodSecurityPolicyVolumes(opts PodSecurityPolicyValidationOptions, fldPath *field.Path, volumes []policy.FSType) field.ErrorList {
 	allErrs := field.ErrorList{}
 	allowed := psputil.GetAllFSTypesAsSet()
 	// add in the * value since that is a pseudo type that is not included by default
 	allowed.Insert(string(policy.All))
+	// Ephemeral may or may not be allowed.
+	if !opts.AllowEphemeralVolumeType {
+		allowed.Delete(string(policy.Ephemeral))
+	}
 	for _, v := range volumes {
 		if !allowed.Has(string(v)) {
 			allErrs = append(allErrs, field.NotSupported(fldPath.Child("volumes"), v, allowed.List()))
@@ -519,11 +530,11 @@ func validateRuntimeClassStrategy(fldPath *field.Path, rc *policy.RuntimeClassSt
 }
 
 // ValidatePodSecurityPolicyUpdate validates a PSP for updates.
-func ValidatePodSecurityPolicyUpdate(old *policy.PodSecurityPolicy, new *policy.PodSecurityPolicy) field.ErrorList {
+func ValidatePodSecurityPolicyUpdate(old *policy.PodSecurityPolicy, new *policy.PodSecurityPolicy, opts PodSecurityPolicyValidationOptions) field.ErrorList {
 	allErrs := field.ErrorList{}
 	allErrs = append(allErrs, apivalidation.ValidateObjectMetaUpdate(&new.ObjectMeta, &old.ObjectMeta, field.NewPath("metadata"))...)
 	allErrs = append(allErrs, ValidatePodSecurityPolicySpecificAnnotations(new.Annotations, field.NewPath("metadata").Child("annotations"))...)
-	allErrs = append(allErrs, ValidatePodSecurityPolicySpec(&new.Spec, field.NewPath("spec"))...)
+	allErrs = append(allErrs, ValidatePodSecurityPolicySpec(&new.Spec, opts, field.NewPath("spec"))...)
 	return allErrs
 }
 

pkg/apis/policy/validation/validation_test.go

@@ -590,7 +590,7 @@ func TestValidatePodSecurityPolicy(t *testing.T) {
 	}
 
 	for k, v := range errorCases {
-		errs := ValidatePodSecurityPolicy(v.psp)
+		errs := ValidatePodSecurityPolicy(v.psp, PodSecurityPolicyValidationOptions{})
 		if len(errs) == 0 {
 			t.Errorf("%s expected errors but got none", k)
 			continue
@@ -613,7 +613,7 @@ func TestValidatePodSecurityPolicy(t *testing.T) {
 	// Should not be able to update to an invalid policy.
 	for k, v := range errorCases {
 		v.psp.ResourceVersion = "444" // Required for updates.
-		errs := ValidatePodSecurityPolicyUpdate(validPSP(), v.psp)
+		errs := ValidatePodSecurityPolicyUpdate(validPSP(), v.psp, PodSecurityPolicyValidationOptions{})
 		if len(errs) == 0 {
 			t.Errorf("[%s] expected update errors but got none", k)
 			continue
@@ -743,13 +743,13 @@ func TestValidatePodSecurityPolicy(t *testing.T) {
 	}
 
 	for k, v := range successCases {
-		if errs := ValidatePodSecurityPolicy(v.psp); len(errs) != 0 {
+		if errs := ValidatePodSecurityPolicy(v.psp, PodSecurityPolicyValidationOptions{}); len(errs) != 0 {
 			t.Errorf("Expected success for %s, got %v", k, errs)
 		}
 
 		// Should be able to update to a valid PSP.
 		v.psp.ResourceVersion = "444" // Required for updates.
-		if errs := ValidatePodSecurityPolicyUpdate(validPSP(), v.psp); len(errs) != 0 {
+		if errs := ValidatePodSecurityPolicyUpdate(validPSP(), v.psp, PodSecurityPolicyValidationOptions{}); len(errs) != 0 {
 			t.Errorf("Expected success for %s update, got %v", k, errs)
 		}
 	}
@@ -786,7 +786,7 @@ func TestValidatePSPVolumes(t *testing.T) {
 	for _, strVolume := range volumes.List() {
 		psp := validPSP()
 		psp.Spec.Volumes = []policy.FSType{policy.FSType(strVolume)}
-		errs := ValidatePodSecurityPolicy(psp)
+		errs := ValidatePodSecurityPolicy(psp, PodSecurityPolicyValidationOptions{AllowEphemeralVolumeType: true})
 		if len(errs) != 0 {
 			t.Errorf("%s validation expected no errors but received %v", strVolume, errs)
 		}
@@ -1063,3 +1063,89 @@ func TestValidateRuntimeClassStrategy(t *testing.T) {
 		})
 	}
 }
+
+func TestAllowEphemeralVolumeType(t *testing.T) {
+	pspWithoutGenericVolume := func() *policy.PodSecurityPolicy {
+		return &policy.PodSecurityPolicy{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:            "psp",
+				ResourceVersion: "1",
+			},
+			Spec: policy.PodSecurityPolicySpec{
+				RunAsUser: policy.RunAsUserStrategyOptions{
+					Rule: policy.RunAsUserStrategyMustRunAs,
+				},
+				SupplementalGroups: policy.SupplementalGroupsStrategyOptions{
+					Rule: policy.SupplementalGroupsStrategyMustRunAs,
+				},
+				SELinux: policy.SELinuxStrategyOptions{
+					Rule: policy.SELinuxStrategyMustRunAs,
+				},
+				FSGroup: policy.FSGroupStrategyOptions{
+					Rule: policy.FSGroupStrategyMustRunAs,
+				},
+			},
+		}
+	}
+	pspWithGenericVolume := func() *policy.PodSecurityPolicy {
+		psp := pspWithoutGenericVolume()
+		psp.Spec.Volumes = append(psp.Spec.Volumes, policy.Ephemeral)
+		return psp
+	}
+	pspNil := func() *policy.PodSecurityPolicy {
+		return nil
+	}
+
+	pspInfo := []struct {
+		description      string
+		hasGenericVolume bool
+		psp              func() *policy.PodSecurityPolicy
+	}{
+		{
+			description:      "PodSecurityPolicySpec Without GenericVolume",
+			hasGenericVolume: false,
+			psp:              pspWithoutGenericVolume,
+		},
+		{
+			description:      "PodSecurityPolicySpec With GenericVolume",
+			hasGenericVolume: true,
+			psp:              pspWithGenericVolume,
+		},
+		{
+			description:      "is nil",
+			hasGenericVolume: false,
+			psp:              pspNil,
+		},
+	}
+
+	for _, allowed := range []bool{true, false} {
+		for _, oldPSPInfo := range pspInfo {
+			for _, newPSPInfo := range pspInfo {
+				oldPSP := oldPSPInfo.psp()
+				newPSP := newPSPInfo.psp()
+				if newPSP == nil {
+					continue
+				}
+
+				t.Run(fmt.Sprintf("feature enabled=%v, old PodSecurityPolicySpec %v, new PodSecurityPolicySpec %v", allowed, oldPSPInfo.description, newPSPInfo.description), func(t *testing.T) {
+					opts := PodSecurityPolicyValidationOptions{
+						AllowEphemeralVolumeType: allowed,
+					}
+					var errs field.ErrorList
+					expectErrors := newPSPInfo.hasGenericVolume && !allowed
+					if oldPSP == nil {
+						errs = ValidatePodSecurityPolicy(newPSP, opts)
+					} else {
+						errs = ValidatePodSecurityPolicyUpdate(oldPSP, newPSP, opts)
+					}
+					if expectErrors && len(errs) == 0 {
+						t.Error("expected errors, got none")
+					}
+					if !expectErrors && len(errs) > 0 {
+						t.Errorf("expected no errors, got: %v", errs)
+					}
+				})
+			}
+		}
+	}
+}

pkg/registry/policy/podsecuritypolicy/strategy.go

@@ -23,10 +23,12 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/registry/rest"
 	"k8s.io/apiserver/pkg/storage/names"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	psputil "k8s.io/kubernetes/pkg/api/podsecuritypolicy"
 	"k8s.io/kubernetes/pkg/apis/policy"
 	"k8s.io/kubernetes/pkg/apis/policy/validation"
+	"k8s.io/kubernetes/pkg/features"
 )
 
 // strategy implements behavior for PodSecurityPolicy objects
@@ -72,9 +74,31 @@ func (strategy) Canonicalize(obj runtime.Object) {
 }
 
 func (strategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
-	return validation.ValidatePodSecurityPolicy(obj.(*policy.PodSecurityPolicy))
+	opts := validation.PodSecurityPolicyValidationOptions{
+		// Only allowed if the feature is enabled.
+		AllowEphemeralVolumeType: utilfeature.DefaultFeatureGate.Enabled(features.GenericEphemeralVolume),
+	}
+	return validation.ValidatePodSecurityPolicy(obj.(*policy.PodSecurityPolicy), opts)
 }
 
 func (strategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
-	return validation.ValidatePodSecurityPolicyUpdate(old.(*policy.PodSecurityPolicy), obj.(*policy.PodSecurityPolicy))
+	opts := validation.PodSecurityPolicyValidationOptions{
+		// Allowed if the feature is enabled or the old policy already had it.
+		// A policy that had the type set when that was valid must remain valid.
+		AllowEphemeralVolumeType: utilfeature.DefaultFeatureGate.Enabled(features.GenericEphemeralVolume) ||
+			volumeInUse(old.(*policy.PodSecurityPolicy), policy.Ephemeral),
+	}
+	return validation.ValidatePodSecurityPolicyUpdate(old.(*policy.PodSecurityPolicy), obj.(*policy.PodSecurityPolicy), opts)
+}
+
+func volumeInUse(oldPSP *policy.PodSecurityPolicy, volume policy.FSType) bool {
+	if oldPSP == nil {
+		return false
+	}
+	for _, v := range oldPSP.Spec.Volumes {
+		if v == volume {
+			return true
+		}
+	}
+	return false
 }

pkg/registry/policy/podsecuritypolicy/strategy_test.go

@@ -0,0 +1,117 @@
+/*
+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 podsecuritypolicy
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	featuregatetesting "k8s.io/component-base/featuregate/testing"
+	"k8s.io/kubernetes/pkg/apis/policy"
+	"k8s.io/kubernetes/pkg/features"
+)
+
+func TestAllowEphemeralVolumeType(t *testing.T) {
+	pspWithoutGenericVolume := func() *policy.PodSecurityPolicy {
+		return &policy.PodSecurityPolicy{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:            "psp",
+				ResourceVersion: "1",
+			},
+			Spec: policy.PodSecurityPolicySpec{
+				RunAsUser: policy.RunAsUserStrategyOptions{
+					Rule: policy.RunAsUserStrategyMustRunAs,
+				},
+				SupplementalGroups: policy.SupplementalGroupsStrategyOptions{
+					Rule: policy.SupplementalGroupsStrategyMustRunAs,
+				},
+				SELinux: policy.SELinuxStrategyOptions{
+					Rule: policy.SELinuxStrategyMustRunAs,
+				},
+				FSGroup: policy.FSGroupStrategyOptions{
+					Rule: policy.FSGroupStrategyMustRunAs,
+				},
+			},
+		}
+	}
+	pspWithGenericVolume := func() *policy.PodSecurityPolicy {
+		psp := pspWithoutGenericVolume()
+		psp.Spec.Volumes = append(psp.Spec.Volumes, policy.Ephemeral)
+		return psp
+	}
+	pspNil := func() *policy.PodSecurityPolicy {
+		return nil
+	}
+
+	pspInfo := []struct {
+		description      string
+		hasGenericVolume bool
+		psp              func() *policy.PodSecurityPolicy
+	}{
+		{
+			description:      "PodSecurityPolicySpec Without GenericVolume",
+			hasGenericVolume: false,
+			psp:              pspWithoutGenericVolume,
+		},
+		{
+			description:      "PodSecurityPolicySpec With GenericVolume",
+			hasGenericVolume: true,
+			psp:              pspWithGenericVolume,
+		},
+		{
+			description:      "is nil",
+			hasGenericVolume: false,
+			psp:              pspNil,
+		},
+	}
+
+	for _, enabled := range []bool{true, false} {
+		for _, oldPSPInfo := range pspInfo {
+			for _, newPSPInfo := range pspInfo {
+				oldPSP := oldPSPInfo.psp()
+				newPSP := newPSPInfo.psp()
+				if newPSP == nil {
+					continue
+				}
+
+				t.Run(fmt.Sprintf("feature enabled=%v, old PodSecurityPolicySpec %v, new PodSecurityPolicySpec %v", enabled, oldPSPInfo.description, newPSPInfo.description), func(t *testing.T) {
+					defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.GenericEphemeralVolume, enabled)()
+
+					var errs field.ErrorList
+					var expectErrors bool
+					if oldPSP == nil {
+						errs = Strategy.Validate(context.Background(), newPSP)
+						expectErrors = newPSPInfo.hasGenericVolume && !enabled
+					} else {
+						errs = Strategy.ValidateUpdate(context.Background(), newPSP, oldPSP)
+						expectErrors = !oldPSPInfo.hasGenericVolume && newPSPInfo.hasGenericVolume && !enabled
+					}
+					if expectErrors && len(errs) == 0 {
+						t.Error("expected errors, got none")
+					}
+					if !expectErrors && len(errs) > 0 {
+						t.Errorf("expected no errors, got: %v", errs)
+					}
+				})
+			}
+		}
+	}
+}

pkg/security/podsecuritypolicy/provider_test.go

@@ -502,6 +502,15 @@ func TestValidatePodFailures(t *testing.T) {
 			psp:           defaultPSP(),
 			expectedError: "ephemeral volumes are not allowed to be used",
 		},
+		"generic ephemeral volumes with other volume type allowed": {
+			pod: failGenericEphemeralPod,
+			psp: func() *policy.PodSecurityPolicy {
+				psp := defaultPSP()
+				psp.Spec.Volumes = []policy.FSType{policy.NFS}
+				return psp
+			}(),
+			expectedError: "ephemeral volumes are not allowed to be used",
+		},
 	}
 	for name, test := range errorCases {
 		t.Run(name, func(t *testing.T) {

pkg/security/podsecuritypolicy/util/util.go

@@ -29,6 +29,8 @@ const (
 	ValidatedPSPAnnotation = "kubernetes.io/psp"
 )
 
+// GetAllFSTypesExcept returns the result of GetAllFSTypesAsSet minus
+// the given exceptions.
 func GetAllFSTypesExcept(exceptions ...string) sets.String {
 	fstypes := GetAllFSTypesAsSet()
 	for _, e := range exceptions {
@@ -37,6 +39,8 @@ func GetAllFSTypesExcept(exceptions ...string) sets.String {
 	return fstypes
 }
 
+// GetAllFSTypesAsSet returns all actual volume types, regardless
+// of feature gates. The special policy.All pseudo type is not included.
 func GetAllFSTypesAsSet() sets.String {
 	fstypes := sets.NewString()
 	fstypes.Insert(