diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 022dbae2c44..755e36bf9c6 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -903,8 +903,14 @@ func ValidatePersistentVolumeClaim(pvc *api.PersistentVolumeClaim) field.ErrorLi } func ValidatePersistentVolumeClaimUpdate(newPvc, oldPvc *api.PersistentVolumeClaim) field.ErrorList { - allErrs := field.ErrorList{} - allErrs = ValidatePersistentVolumeClaim(newPvc) + 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 + if len(oldPvc.Spec.VolumeName) != 0 { + if !api.Semantic.DeepEqual(newPvc.Spec, oldPvc.Spec) { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "spec is immutable once a claim has been bound to a volume")) + } + } newPvc.Status = oldPvc.Status return allErrs } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 19a1b702838..5f93808c0e4 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -463,6 +463,89 @@ func TestValidatePersistentVolumeClaim(t *testing.T) { } } +func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { + validClaim := testVolumeClaim("foo", "ns", api.PersistentVolumeClaimSpec{ + AccessModes: []api.PersistentVolumeAccessMode{ + api.ReadWriteOnce, + api.ReadOnlyMany, + }, + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceName(api.ResourceStorage): resource.MustParse("10G"), + }, + }, + }) + validUpdateClaim := testVolumeClaim("foo", "ns", api.PersistentVolumeClaimSpec{ + AccessModes: []api.PersistentVolumeAccessMode{ + api.ReadWriteOnce, + api.ReadOnlyMany, + }, + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceName(api.ResourceStorage): resource.MustParse("10G"), + }, + }, + VolumeName: "volume", + }) + invalidUpdateClaimResources := testVolumeClaim("foo", "ns", api.PersistentVolumeClaimSpec{ + AccessModes: []api.PersistentVolumeAccessMode{ + api.ReadWriteOnce, + api.ReadOnlyMany, + }, + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceName(api.ResourceStorage): resource.MustParse("20G"), + }, + }, + VolumeName: "volume", + }) + invalidUpdateClaimAccessModes := testVolumeClaim("foo", "ns", api.PersistentVolumeClaimSpec{ + AccessModes: []api.PersistentVolumeAccessMode{ + api.ReadWriteOnce, + }, + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceName(api.ResourceStorage): resource.MustParse("10G"), + }, + }, + VolumeName: "volume", + }) + scenarios := map[string]struct { + isExpectedFailure bool + oldClaim *api.PersistentVolumeClaim + newClaim *api.PersistentVolumeClaim + }{ + "valid-update": { + isExpectedFailure: false, + oldClaim: validClaim, + newClaim: validUpdateClaim, + }, + "invalid-update-change-resources-on-bound-claim": { + isExpectedFailure: true, + oldClaim: validUpdateClaim, + newClaim: invalidUpdateClaimResources, + }, + "invalid-update-change-access-modes-on-bound-claim": { + isExpectedFailure: true, + oldClaim: validUpdateClaim, + newClaim: invalidUpdateClaimAccessModes, + }, + } + + for name, scenario := range scenarios { + // ensure we have a resource version specified for updates + scenario.oldClaim.ResourceVersion = "1" + scenario.newClaim.ResourceVersion = "1" + errs := ValidatePersistentVolumeClaimUpdate(scenario.newClaim, scenario.oldClaim) + if len(errs) == 0 && scenario.isExpectedFailure { + t.Errorf("Unexpected success for scenario: %s", name) + } + if len(errs) > 0 && !scenario.isExpectedFailure { + t.Errorf("Unexpected failure for scenario: %s - %+v", name, errs) + } + } +} + func TestValidateVolumes(t *testing.T) { lun := 1 successCase := []api.Volume{