From bb76b60380adb73b28af330f6b81fed1bff37d76 Mon Sep 17 00:00:00 2001 From: vthurimella Date: Thu, 9 Jan 2025 18:38:22 -0500 Subject: [PATCH] Update volume validation to include hostPath (#15669) * Update volume validation to include hostPath * Remove want: nil * Fix tests to incorporate all errors * Fix compilation --- pkg/apis/serving/k8s_validation.go | 11 +++++++ pkg/apis/serving/k8s_validation_test.go | 41 +++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/pkg/apis/serving/k8s_validation.go b/pkg/apis/serving/k8s_validation.go index 42946b80ef99..ece260a9689b 100644 --- a/pkg/apis/serving/k8s_validation.go +++ b/pkg/apis/serving/k8s_validation.go @@ -125,6 +125,10 @@ func validateVolume(ctx context.Context, volume corev1.Volume) *apis.FieldError errs = errs.Also(&apis.FieldError{Message: fmt.Sprintf("EmptyDir volume support is disabled, "+ "but found EmptyDir volume %s", volume.Name)}) } + if volume.HostPath != nil && features.PodSpecVolumesHostPath != config.Enabled { + errs = errs.Also(&apis.FieldError{Message: fmt.Sprintf("HostPath volume support is disabled, "+ + "but found HostPath volume %s", volume.Name)}) + } errs = errs.Also(apis.CheckDisallowedFields(volume, *VolumeMask(ctx, &volume))) if volume.Name == "" { errs = apis.ErrMissingField("name") @@ -161,6 +165,10 @@ func validateVolume(ctx context.Context, volume corev1.Volume) *apis.FieldError specified = append(specified, "persistentVolumeClaim") } + if vs.HostPath != nil { + specified = append(specified, "hostPath") + } + if len(specified) == 0 { fieldPaths := []string{"secret", "configMap", "projected"} cfg := config.FromContextOrDefaults(ctx) @@ -170,6 +178,9 @@ func validateVolume(ctx context.Context, volume corev1.Volume) *apis.FieldError if cfg.Features.PodSpecPersistentVolumeClaim == config.Enabled { fieldPaths = append(fieldPaths, "persistentVolumeClaim") } + if cfg.Features.PodSpecVolumesHostPath == config.Enabled { + fieldPaths = append(fieldPaths, "hostPath") + } errs = errs.Also(apis.ErrMissingOneOf(fieldPaths...)) } else if len(specified) > 1 { errs = errs.Also(apis.ErrMultipleOneOf(specified...)) diff --git a/pkg/apis/serving/k8s_validation_test.go b/pkg/apis/serving/k8s_validation_test.go index cc12b8457ab6..0cc355940a2d 100644 --- a/pkg/apis/serving/k8s_validation_test.go +++ b/pkg/apis/serving/k8s_validation_test.go @@ -129,6 +129,13 @@ func withPodSpecPersistentVolumeClaimEnabled() configOption { } } +func withPodSpecVolumesHostPathEnabled() configOption { + return func(cfg *config.Config) *config.Config { + cfg.Features.PodSpecVolumesHostPath = config.Enabled + return cfg + } +} + func withPodSpecPersistentVolumeWriteEnabled() configOption { return func(cfg *config.Config) *config.Config { cfg.Features.PodSpecPersistentVolumeWrite = config.Enabled @@ -2892,6 +2899,40 @@ func TestVolumeValidation(t *testing.T) { }, want: apis.ErrInvalidValue(-1, "emptyDir.sizeLimit"), cfgOpts: []configOption{withPodSpecVolumesEmptyDirEnabled()}, + }, { + name: "valid hostPath volume with feature enabled", + v: corev1.Volume{ + Name: "foo", + VolumeSource: corev1.VolumeSource{ + HostPath: &corev1.HostPathVolumeSource{ + Path: "/valid/path", + }, + }, + }, + cfgOpts: []configOption{withPodSpecVolumesHostPathEnabled()}, + }, { + name: "hostPath volume with feature disabled", + v: corev1.Volume{ + Name: "foo", + VolumeSource: corev1.VolumeSource{ + HostPath: &corev1.HostPathVolumeSource{ + Path: "/valid/path", + }, + }, + }, + want: (&apis.FieldError{ + Message: `HostPath volume support is disabled, but found HostPath volume foo`, + }).Also( + &apis.FieldError{ + Message: "must not set the field(s)", Paths: []string{"hostPath"}, + }), + }, { + name: "missing hostPath volume when required", + v: corev1.Volume{ + Name: "foo", + }, + cfgOpts: []configOption{withPodSpecVolumesHostPathEnabled()}, + want: apis.ErrMissingOneOf("secret", "configMap", "projected", "emptyDir", "hostPath"), }, { name: "valid PVC with PVC feature enabled", v: corev1.Volume{