From a76854ca97f6e4211e227b97845eaa3efa580129 Mon Sep 17 00:00:00 2001 From: Scott Creeley Date: Fri, 4 Nov 2016 14:06:34 -0400 Subject: [PATCH] Make pvc storage class annotation immutable after create --- pkg/api/validation/BUILD | 2 + pkg/api/validation/validation.go | 16 ++++- pkg/api/validation/validation_test.go | 100 ++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 1 deletion(-) diff --git a/pkg/api/validation/BUILD b/pkg/api/validation/BUILD index b1931075dd..f00bdcecae 100644 --- a/pkg/api/validation/BUILD +++ b/pkg/api/validation/BUILD @@ -30,6 +30,7 @@ go_library( "//pkg/api/util:go_default_library", "//pkg/api/v1:go_default_library", "//pkg/apimachinery/registered:go_default_library", + "//pkg/apis/storage/util:go_default_library", "//pkg/capabilities:go_default_library", "//pkg/labels:go_default_library", "//pkg/runtime:go_default_library", @@ -75,6 +76,7 @@ go_test( "//pkg/api/unversioned:go_default_library", "//pkg/apimachinery/registered:go_default_library", "//pkg/apis/extensions:go_default_library", + "//pkg/apis/storage/util:go_default_library", "//pkg/capabilities:go_default_library", "//pkg/runtime:go_default_library", "//pkg/security/apparmor:go_default_library", diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index e7783a5c8e..11f84e40d0 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -34,6 +34,7 @@ import ( apiservice "k8s.io/kubernetes/pkg/api/service" unversionedvalidation "k8s.io/kubernetes/pkg/api/unversioned/validation" "k8s.io/kubernetes/pkg/api/v1" + storageutil "k8s.io/kubernetes/pkg/apis/storage/util" "k8s.io/kubernetes/pkg/capabilities" "k8s.io/kubernetes/pkg/labels" "k8s.io/kubernetes/pkg/runtime/schema" @@ -351,6 +352,15 @@ func ValidateImmutableField(newVal, oldVal interface{}, fldPath *field.Path) fie return allErrs } +func ValidateImmutableAnnotation(newVal string, oldVal string, annotation string, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if oldVal != newVal { + allErrs = append(allErrs, field.Invalid(fldPath.Child("annotations", annotation), newVal, fieldImmutableErrorMsg)) + } + return allErrs +} + // ValidateObjectMeta validates an object's metadata on creation. It expects that name generation has already // been performed. // It doesn't return an error for rootscoped resources with namespace, because namespace should already be cleared before. @@ -1262,11 +1272,15 @@ func ValidatePersistentVolumeClaimUpdate(newPvc, oldPvc *api.PersistentVolumeCla oldPvc.Spec.VolumeName = newPvc.Spec.VolumeName defer func() { oldPvc.Spec.VolumeName = "" }() } - // changes to Spec are not allowed, but updates to label/annotations are OK. + // changes to Spec are not allowed, but updates to label/and some 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")) } + + // storageclass annotation should be immutable after creation + allErrs = append(allErrs, ValidateImmutableAnnotation(newPvc.ObjectMeta.Annotations[storageutil.StorageClassAnnotation], oldPvc.ObjectMeta.Annotations[storageutil.StorageClassAnnotation], storageutil.StorageClassAnnotation, field.NewPath("metadata"))...) + newPvc.Status = oldPvc.Status return allErrs } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 094a4a0d19..30448ee9d3 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -28,6 +28,7 @@ import ( "k8s.io/kubernetes/pkg/api/service" "k8s.io/kubernetes/pkg/api/unversioned" "k8s.io/kubernetes/pkg/apimachinery/registered" + storageutil "k8s.io/kubernetes/pkg/apis/storage/util" "k8s.io/kubernetes/pkg/capabilities" "k8s.io/kubernetes/pkg/security/apparmor" "k8s.io/kubernetes/pkg/util/intstr" @@ -661,6 +662,36 @@ func testVolumeClaim(name string, namespace string, spec api.PersistentVolumeCla } } +func testVolumeClaimStorageClass(name string, namespace string, annval string, spec api.PersistentVolumeClaimSpec) *api.PersistentVolumeClaim { + annotations := map[string]string{ + storageutil.StorageClassAnnotation: annval, + } + + return &api.PersistentVolumeClaim{ + ObjectMeta: api.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: annotations, + }, + Spec: spec, + } +} + +func testVolumeClaimAnnotation(name string, namespace string, ann string, annval string, spec api.PersistentVolumeClaimSpec) *api.PersistentVolumeClaim { + annotations := map[string]string{ + ann: annval, + } + + return &api.PersistentVolumeClaim{ + ObjectMeta: api.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: annotations, + }, + Spec: spec, + } +} + func TestValidatePersistentVolumeClaim(t *testing.T) { scenarios := map[string]struct { isExpectedFailure bool @@ -814,6 +845,26 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { }, }, }) + validClaimStorageClass := testVolumeClaimStorageClass("foo", "ns", "fast", api.PersistentVolumeClaimSpec{ + AccessModes: []api.PersistentVolumeAccessMode{ + api.ReadOnlyMany, + }, + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceName(api.ResourceStorage): resource.MustParse("10G"), + }, + }, + }) + validClaimAnnotation := testVolumeClaimAnnotation("foo", "ns", "description", "foo-description", api.PersistentVolumeClaimSpec{ + AccessModes: []api.PersistentVolumeAccessMode{ + 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, @@ -849,6 +900,40 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { }, VolumeName: "volume", }) + invalidUpdateClaimStorageClass := testVolumeClaimStorageClass("foo", "ns", "fast2", api.PersistentVolumeClaimSpec{ + AccessModes: []api.PersistentVolumeAccessMode{ + api.ReadOnlyMany, + }, + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceName(api.ResourceStorage): resource.MustParse("10G"), + }, + }, + VolumeName: "volume", + }) + validUpdateClaimMutableAnnotation := testVolumeClaimAnnotation("foo", "ns", "description", "updated-or-added-foo-description", api.PersistentVolumeClaimSpec{ + AccessModes: []api.PersistentVolumeAccessMode{ + api.ReadOnlyMany, + }, + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceName(api.ResourceStorage): resource.MustParse("10G"), + }, + }, + VolumeName: "volume", + }) + validAddClaimAnnotation := testVolumeClaimAnnotation("foo", "ns", "description", "updated-or-added-foo-description", api.PersistentVolumeClaimSpec{ + AccessModes: []api.PersistentVolumeAccessMode{ + api.ReadWriteOnce, + api.ReadOnlyMany, + }, + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceName(api.ResourceStorage): resource.MustParse("10G"), + }, + }, + VolumeName: "volume", + }) scenarios := map[string]struct { isExpectedFailure bool oldClaim *api.PersistentVolumeClaim @@ -874,6 +959,21 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { oldClaim: validUpdateClaim, newClaim: invalidUpdateClaimAccessModes, }, + "invalid-update-change-storage-class-annotation-after-creation": { + isExpectedFailure: true, + oldClaim: validClaimStorageClass, + newClaim: invalidUpdateClaimStorageClass, + }, + "valid-update-mutable-annotation": { + isExpectedFailure: false, + oldClaim: validClaimAnnotation, + newClaim: validUpdateClaimMutableAnnotation, + }, + "valid-update-add-annotation": { + isExpectedFailure: false, + oldClaim: validClaim, + newClaim: validAddClaimAnnotation, + }, } for name, scenario := range scenarios {