From 684fbd6f206736a677463561fe210f12d3ad27cc Mon Sep 17 00:00:00 2001 From: carlory Date: Fri, 13 Sep 2024 23:41:06 +0800 Subject: [PATCH] remove AllowImageVolumeSource --- pkg/api/pod/util.go | 17 --------- pkg/api/pod/util_test.go | 42 --------------------- pkg/apis/core/validation/validation.go | 18 ++++----- pkg/apis/core/validation/validation_test.go | 25 +++++------- 4 files changed, 17 insertions(+), 85 deletions(-) diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 11d5dee8b60..1d1a211a28e 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -385,7 +385,6 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po AllowInvalidTopologySpreadConstraintLabelSelector: false, AllowNamespacedSysctlsForHostNetAndHostIPC: false, AllowNonLocalProjectedTokenPath: false, - AllowImageVolumeSource: utilfeature.DefaultFeatureGate.Enabled(features.ImageVolume), } // If old spec uses relaxed validation or enabled the RelaxedEnvironmentVariableValidation feature gate, @@ -416,9 +415,6 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po } } } - - // if old spec has used image volume source, we must allow it - opts.AllowImageVolumeSource = opts.AllowImageVolumeSource || hasUsedImageVolumeSourceWithPodSpec(oldPodSpec) } if oldPodMeta != nil && !opts.AllowInvalidPodDeletionCost { // This is an update, so validate only if the existing object was valid. @@ -584,19 +580,6 @@ func hasUsedDownwardAPIFieldPathWithContainer(container *api.Container, fieldPat return false } -func hasUsedImageVolumeSourceWithPodSpec(podSpec *api.PodSpec) bool { - if podSpec == nil { - return false - } - - for _, vol := range podSpec.Volumes { - if vol.Image != nil { - return true - } - } - return false -} - // GetValidationOptionsFromPodTemplate will return pod validation options for specified template. func GetValidationOptionsFromPodTemplate(podTemplate, oldPodTemplate *api.PodTemplateSpec) apivalidation.PodValidationOptions { var newPodSpec, oldPodSpec *api.PodSpec diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index 610b9c838ad..c94005283a4 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -2553,48 +2553,6 @@ func TestValidateAllowNonLocalProjectedTokenPathOption(t *testing.T) { } } -func TestValidateAllowImageVolumeSourceOption(t *testing.T) { - testCases := []struct { - name string - oldPodSpec *api.PodSpec - featureEnabled bool - wantOption bool - }{ - { - name: "CreateFeatureEnabled", - featureEnabled: true, - wantOption: true, - }, - { - name: "CreateFeatureDisabled", - featureEnabled: false, - wantOption: false, - }, - { - name: "UpdateFeatureDisabled", - oldPodSpec: &api.PodSpec{Volumes: []api.Volume{{VolumeSource: api.VolumeSource{Image: &api.ImageVolumeSource{Reference: "image"}}}}}, - featureEnabled: false, - wantOption: true, - }, - { - name: "UpdateFeatureEnabled", - oldPodSpec: &api.PodSpec{Volumes: []api.Volume{{VolumeSource: api.VolumeSource{Image: &api.ImageVolumeSource{Reference: "image"}}}}}, - featureEnabled: true, - wantOption: true, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ImageVolume, tc.featureEnabled) - gotOptions := GetValidationOptionsFromPodSpecAndMeta(nil, tc.oldPodSpec, nil, nil) - if tc.wantOption != gotOptions.AllowImageVolumeSource { - t.Errorf("unexpected diff, want: %v, got: %v", tc.wantOption, gotOptions.AllowImageVolumeSource) - } - }) - } -} - func TestDropInPlacePodVerticalScaling(t *testing.T) { podWithInPlaceVerticalScaling := func() *api.Pod { return &api.Pod{ diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 874b80cc83b..9f6f8da5b42 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -741,7 +741,7 @@ func validateVolumeSource(source *core.VolumeSource, fldPath *field.Path, volNam } } } - if opts.AllowImageVolumeSource && source.Image != nil { + if source.Image != nil { if numVolumes > 0 { allErrs = append(allErrs, field.Forbidden(fldPath.Child("image"), "may not specify more than 1 volume type")) } else { @@ -2940,14 +2940,12 @@ func ValidateVolumeMounts(mounts []core.VolumeMount, voldevices map[string]strin } // Disallow subPath/subPathExpr for image volumes - if opts.AllowImageVolumeSource { - if v, ok := volumes[mnt.Name]; ok && v.Image != nil { - if len(mnt.SubPath) != 0 { - allErrs = append(allErrs, field.Invalid(idxPath.Child("subPath"), mnt.SubPath, "not allowed in image volume sources")) - } - if len(mnt.SubPathExpr) != 0 { - allErrs = append(allErrs, field.Invalid(idxPath.Child("subPathExpr"), mnt.SubPathExpr, "not allowed in image volume sources")) - } + if v, ok := volumes[mnt.Name]; ok && v.Image != nil { + if len(mnt.SubPath) != 0 { + allErrs = append(allErrs, field.Invalid(idxPath.Child("subPath"), mnt.SubPath, "not allowed in image volume sources")) + } + if len(mnt.SubPathExpr) != 0 { + allErrs = append(allErrs, field.Invalid(idxPath.Child("subPathExpr"), mnt.SubPathExpr, "not allowed in image volume sources")) } } @@ -4049,8 +4047,6 @@ type PodValidationOptions struct { ResourceIsPod bool // Allow relaxed validation of environment variable names AllowRelaxedEnvironmentVariableValidation bool - // Allow the use of the ImageVolumeSource API. - AllowImageVolumeSource bool // Allow the use of a relaxed DNS search AllowRelaxedDNSSearchValidation bool } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 69ee7362118..d88335aae9e 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -5378,19 +5378,14 @@ func TestValidateVolumes(t *testing.T) { }, }, }, - opts: PodValidationOptions{AllowImageVolumeSource: true}, + opts: PodValidationOptions{}, }, { - name: "feature disabled", + name: "no volume source", vol: core.Volume{ - Name: "image-volume", - VolumeSource: core.VolumeSource{ - Image: &core.ImageVolumeSource{ - Reference: "quay.io/my/artifact:v1", - PullPolicy: "IfNotPresent", - }, - }, + Name: "volume", + VolumeSource: core.VolumeSource{}, }, - opts: PodValidationOptions{AllowImageVolumeSource: false}, + opts: PodValidationOptions{}, errs: []verr{{ etype: field.ErrorTypeRequired, field: "field[0]", @@ -5407,7 +5402,7 @@ func TestValidateVolumes(t *testing.T) { }, }, }, - opts: PodValidationOptions{AllowImageVolumeSource: true}, + opts: PodValidationOptions{}, errs: []verr{{ etype: field.ErrorTypeRequired, field: "name", @@ -5423,7 +5418,7 @@ func TestValidateVolumes(t *testing.T) { }, }, }, - opts: PodValidationOptions{AllowImageVolumeSource: true, ResourceIsPod: true}, + opts: PodValidationOptions{ResourceIsPod: true}, errs: []verr{{ etype: field.ErrorTypeRequired, field: "image.reference", @@ -5439,7 +5434,7 @@ func TestValidateVolumes(t *testing.T) { }, }, }, - opts: PodValidationOptions{AllowImageVolumeSource: true, ResourceIsPod: false}, + opts: PodValidationOptions{ResourceIsPod: false}, }, { name: "image volume with wrong pullPolicy", vol: core.Volume{ @@ -5451,7 +5446,7 @@ func TestValidateVolumes(t *testing.T) { }, }, }, - opts: PodValidationOptions{AllowImageVolumeSource: true}, + opts: PodValidationOptions{}, errs: []verr{{ etype: field.ErrorTypeNotSupported, field: "image.pullPolicy", @@ -7066,7 +7061,7 @@ func TestValidateVolumeMounts(t *testing.T) { }}}}, {Name: "image-volume", VolumeSource: core.VolumeSource{Image: &core.ImageVolumeSource{Reference: "quay.io/my/artifact:v1", PullPolicy: "IfNotPresent"}}}, } - opts := PodValidationOptions{AllowImageVolumeSource: true} + opts := PodValidationOptions{} vols, v1err := ValidateVolumes(volumes, nil, field.NewPath("field"), opts) if len(v1err) > 0 { t.Errorf("Invalid test volume - expected success %v", v1err)