diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index ce7008bdbc8..fe641a511e5 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -1192,6 +1192,17 @@ func testVolumeClaimStorageClassInSpec(name, namespace, scName string, spec core } } +func testVolumeClaimStorageClassNilInSpec(name, namespace string, spec core.PersistentVolumeClaimSpec) *core.PersistentVolumeClaim { + spec.StorageClassName = nil + return &core.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: spec, + } +} + func testVolumeSnapshotDataSourceInSpec(name string, kind string, apiGroup string) *core.PersistentVolumeClaimSpec { scName := "csi-plugin" dataSourceInSpec := core.PersistentVolumeClaimSpec{ @@ -1249,6 +1260,18 @@ func testVolumeClaimStorageClassInAnnotationAndSpec(name, namespace, scNameInAnn } } +func testVolumeClaimStorageClassInAnnotationAndNilInSpec(name, namespace, scNameInAnn string, spec core.PersistentVolumeClaimSpec) *core.PersistentVolumeClaim { + spec.StorageClassName = nil + return &core.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{v1.BetaStorageClassAnnotation: scNameInAnn}, + }, + Spec: spec, + } +} + func testValidatePVC(t *testing.T, ephemeral bool) { invalidClassName := "-invalid-" validClassName := "valid" @@ -1972,6 +1995,28 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { }, }) + validClaimStorageClassInSpecChanged := testVolumeClaimStorageClassInSpec("foo", "ns", "fast2", core.PersistentVolumeClaimSpec{ + AccessModes: []core.PersistentVolumeAccessMode{ + core.ReadOnlyMany, + }, + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceName(core.ResourceStorage): resource.MustParse("10G"), + }, + }, + }) + + validClaimStorageClassNil := testVolumeClaimStorageClassNilInSpec("foo", "ns", core.PersistentVolumeClaimSpec{ + AccessModes: []core.PersistentVolumeAccessMode{ + core.ReadOnlyMany, + }, + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceName(core.ResourceStorage): resource.MustParse("10G"), + }, + }, + }) + invalidClaimStorageClassInSpec := testVolumeClaimStorageClassInSpec("foo", "ns", "fast2", core.PersistentVolumeClaimSpec{ AccessModes: []core.PersistentVolumeAccessMode{ core.ReadOnlyMany, @@ -1995,6 +2040,18 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { }, }) + validClaimStorageClassInAnnotationAndNilInSpec := testVolumeClaimStorageClassInAnnotationAndNilInSpec( + "foo", "ns", "fast", core.PersistentVolumeClaimSpec{ + AccessModes: []core.PersistentVolumeAccessMode{ + core.ReadOnlyMany, + }, + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceName(core.ResourceStorage): resource.MustParse("10G"), + }, + }, + }) + invalidClaimStorageClassInAnnotationAndSpec := testVolumeClaimStorageClassInAnnotationAndSpec( "foo", "ns", "fast2", "fast", core.PersistentVolumeClaimSpec{ AccessModes: []core.PersistentVolumeAccessMode{ @@ -2116,10 +2173,11 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { }) scenarios := map[string]struct { - isExpectedFailure bool - oldClaim *core.PersistentVolumeClaim - newClaim *core.PersistentVolumeClaim - enableRecoverFromExpansion bool + isExpectedFailure bool + oldClaim *core.PersistentVolumeClaim + newClaim *core.PersistentVolumeClaim + enableRecoverFromExpansion bool + enableRetroactiveDefaultStorageClass bool }{ "valid-update-volumeName-only": { isExpectedFailure: false, @@ -2230,6 +2288,69 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { oldClaim: validClaimStorageClass, newClaim: validClaimStorageClassInSpec, }, + "valid-upgrade-nil-storage-class-spec-to-spec": { + isExpectedFailure: false, + oldClaim: validClaimStorageClassNil, + newClaim: validClaimStorageClassInSpec, + enableRetroactiveDefaultStorageClass: true, + // Feature enabled - change from nil sc name is valid if there is no beta annotation. + }, + "invalid-upgrade-nil-storage-class-spec-to-spec": { + isExpectedFailure: true, + oldClaim: validClaimStorageClassNil, + newClaim: validClaimStorageClassInSpec, + enableRetroactiveDefaultStorageClass: false, + // Feature disabled - change from nil sc name is invalid if there is no beta annotation. + }, + "invalid-upgrade-not-nil-storage-class-spec-to-spec": { + isExpectedFailure: true, + oldClaim: validClaimStorageClassInSpec, + newClaim: validClaimStorageClassInSpecChanged, + enableRetroactiveDefaultStorageClass: true, + // Feature enablement must not allow non nil value change. + }, + "invalid-upgrade-to-nil-storage-class-spec-to-spec": { + isExpectedFailure: true, + oldClaim: validClaimStorageClassInSpec, + newClaim: validClaimStorageClassNil, + enableRetroactiveDefaultStorageClass: true, + // Feature enablement must not allow change to nil value change. + }, + "valid-upgrade-storage-class-annotation-and-nil-spec-to-spec": { + isExpectedFailure: false, + oldClaim: validClaimStorageClassInAnnotationAndNilInSpec, + newClaim: validClaimStorageClassInAnnotationAndSpec, + enableRetroactiveDefaultStorageClass: false, + // Change from nil sc name is valid if annotations match. + }, + "valid-upgrade-storage-class-annotation-and-nil-spec-to-spec-retro": { + isExpectedFailure: false, + oldClaim: validClaimStorageClassInAnnotationAndNilInSpec, + newClaim: validClaimStorageClassInAnnotationAndSpec, + enableRetroactiveDefaultStorageClass: true, + // Change from nil sc name is valid if annotations match, feature enablement must not break this old behavior. + }, + "invalid-upgrade-storage-class-annotation-and-spec-to-spec": { + isExpectedFailure: true, + oldClaim: validClaimStorageClassInAnnotationAndSpec, + newClaim: validClaimStorageClassInSpecChanged, + enableRetroactiveDefaultStorageClass: false, + // Change from non nil sc name is invalid if annotations don't match. + }, + "invalid-upgrade-storage-class-annotation-and-spec-to-spec-retro": { + isExpectedFailure: true, + oldClaim: validClaimStorageClassInAnnotationAndSpec, + newClaim: validClaimStorageClassInSpecChanged, + enableRetroactiveDefaultStorageClass: true, + // Change from non nil sc name is invalid if annotations don't match, feature enablement must not break this old behavior. + }, + "invalid-upgrade-storage-class-annotation-and-no-spec": { + isExpectedFailure: true, + oldClaim: validClaimStorageClassInAnnotationAndNilInSpec, + newClaim: validClaimStorageClassInSpecChanged, + enableRetroactiveDefaultStorageClass: true, + // Change from nil sc name is invalid if annotations don't match, feature enablement must not break this old behavior. + }, "invalid-upgrade-storage-class-annotation-to-spec": { isExpectedFailure: true, oldClaim: validClaimStorageClass, @@ -2294,6 +2415,7 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { for name, scenario := range scenarios { t.Run(name, func(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, scenario.enableRecoverFromExpansion)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RetroactiveDefaultStorageClass, scenario.enableRetroactiveDefaultStorageClass)() scenario.oldClaim.ResourceVersion = "1" scenario.newClaim.ResourceVersion = "1" opts := ValidationOptionsForPersistentVolumeClaim(scenario.newClaim, scenario.oldClaim)