Merge pull request #28690 from markturansky/immutable_claim

Automatic merge from submit-queue

PersistentVolumeClaim.Spec is immutable once created

Per https://github.com/kubernetes/kubernetes/pull/28636, PVCs are immutable post-creation in order to enforce quota, limitRange, etc. without being able to game the system.

@derekwaynecarr @abhgupta @smarterclayton @kubernetes/sig-storage 

[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/.github/PULL_REQUEST_TEMPLATE.md?pixel)]()

pkg/api/validation/validation.go

@@ -1022,11 +1022,18 @@ func ValidatePersistentVolumeClaim(pvc *api.PersistentVolumeClaim) field.ErrorLi
 func ValidatePersistentVolumeClaimUpdate(newPvc, oldPvc *api.PersistentVolumeClaim) field.ErrorList {
 	allErrs := ValidateObjectMetaUpdate(&newPvc.ObjectMeta, &oldPvc.ObjectMeta, field.NewPath("metadata"))
 	allErrs = append(allErrs, ValidatePersistentVolumeClaim(newPvc)...)
-	// if a pvc had a bound volume, we should not allow updates to resources or access modes
+	// PVController needs to update PVC.Spec w/ VolumeName.
-	if len(oldPvc.Spec.VolumeName) != 0 {
+	// Claims are immutable in order to enforce quota, range limits, etc. without gaming the system.
-		if !api.Semantic.DeepEqual(newPvc.Spec, oldPvc.Spec) {
+	if len(oldPvc.Spec.VolumeName) == 0 {
-			allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "spec is immutable once a claim has been bound to a volume"))
+		// volumeName changes are allowed once.
-		}
+		// Reset back to empty string after equality check
+		oldPvc.Spec.VolumeName = newPvc.Spec.VolumeName
+		defer func() { oldPvc.Spec.VolumeName = "" }()
+	}
+	// changes to Spec are not allowed, but updates to label/annotations are OK.
+	// no-op updates pass validation.
+	if !api.Semantic.DeepEqual(newPvc.Spec, oldPvc.Spec) {
+		allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "field is immutable after creation"))
 	}
 	newPvc.Status = oldPvc.Status
 	return allErrs

pkg/api/validation/validation_test.go

@@ -740,11 +740,16 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) {
 		oldClaim          *api.PersistentVolumeClaim
 		newClaim          *api.PersistentVolumeClaim
 	}{
-		"valid-update": {
+		"valid-update-volumeName-only": {
 			isExpectedFailure: false,
 			oldClaim:          validClaim,
 			newClaim:          validUpdateClaim,
 		},
+		"valid-no-op-update": {
+			isExpectedFailure: false,
+			oldClaim:          validUpdateClaim,
+			newClaim:          validUpdateClaim,
+		},
 		"invalid-update-change-resources-on-bound-claim": {
 			isExpectedFailure: true,
 			oldClaim:          validUpdateClaim,

pkg/registry/persistentvolumeclaim/etcd/etcd_test.go

@@ -86,11 +86,7 @@ func TestUpdate(t *testing.T) {
 		// updateFunc
 		func(obj runtime.Object) runtime.Object {
 			object := obj.(*api.PersistentVolumeClaim)
-			object.Spec.Resources = api.ResourceRequirements{
+			object.Spec.VolumeName = "onlyVolumeNameUpdateAllowed"
-				Requests: api.ResourceList{
-					api.ResourceName(api.ResourceStorage): resource.MustParse("20G"),
-				},
-			}
 			return object
 		},
 	)