Merge pull request kubernetes#126733 from carlory/fix-image-volume-validation

If old pod spec has used image volume source, we must allow it

Author: k8s-ci-robot

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,

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
 }

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)