diff --git a/pkg/api/persistentvolumeclaim/util.go b/pkg/api/persistentvolumeclaim/util.go index 095c1d82192..3612881107c 100644 --- a/pkg/api/persistentvolumeclaim/util.go +++ b/pkg/api/persistentvolumeclaim/util.go @@ -55,11 +55,15 @@ func dataSourceInUse(oldPVCSpec *core.PersistentVolumeClaimSpec) bool { func dataSourceIsEnabled(pvcSpec *core.PersistentVolumeClaimSpec) bool { if pvcSpec.DataSource != nil { - if pvcSpec.DataSource.Kind == "PersistentVolumeClaim" && utilfeature.DefaultFeatureGate.Enabled(features.VolumeDataSource) { + if pvcSpec.DataSource.Kind == "PersistentVolumeClaim" && + *pvcSpec.DataSource.APIGroup == "" && + utilfeature.DefaultFeatureGate.Enabled(features.VolumePVCDataSource) { return true } - if pvcSpec.DataSource.Kind == "VolumeSnapshot" && utilfeature.DefaultFeatureGate.Enabled(features.VolumeSnapshotDataSource) { + if pvcSpec.DataSource.Kind == "VolumeSnapshot" && + *pvcSpec.DataSource.APIGroup == "snapshot.storage.k8s.io" && + utilfeature.DefaultFeatureGate.Enabled(features.VolumeSnapshotDataSource) { return true } diff --git a/pkg/api/persistentvolumeclaim/util_test.go b/pkg/api/persistentvolumeclaim/util_test.go index e970dce4c88..171dc56e3f7 100644 --- a/pkg/api/persistentvolumeclaim/util_test.go +++ b/pkg/api/persistentvolumeclaim/util_test.go @@ -118,7 +118,7 @@ func TestDropAlphaPVCVolumeMode(t *testing.T) { } } -func TestDropDisabledDataSource(t *testing.T) { +func TestDropDisabledSnapshotDataSource(t *testing.T) { pvcWithoutDataSource := func() *core.PersistentVolumeClaim { return &core.PersistentVolumeClaim{ Spec: core.PersistentVolumeClaimSpec{ @@ -210,3 +210,139 @@ func TestDropDisabledDataSource(t *testing.T) { } } } + +func TestDropDisabledPVCDataSource(t *testing.T) { + pvcWithoutDataSource := func() *core.PersistentVolumeClaim { + return &core.PersistentVolumeClaim{ + Spec: core.PersistentVolumeClaimSpec{ + DataSource: nil, + }, + } + } + apiGroup := "" + pvcWithDataSource := func() *core.PersistentVolumeClaim { + return &core.PersistentVolumeClaim{ + Spec: core.PersistentVolumeClaimSpec{ + DataSource: &core.TypedLocalObjectReference{ + APIGroup: &apiGroup, + Kind: "PersistentVolumeClaim", + Name: "test_clone", + }, + }, + } + } + + pvcInfo := []struct { + description string + hasDataSource bool + pvc func() *core.PersistentVolumeClaim + }{ + { + description: "pvc without DataSource", + hasDataSource: false, + pvc: pvcWithoutDataSource, + }, + { + description: "pvc with DataSource", + hasDataSource: true, + pvc: pvcWithDataSource, + }, + { + description: "is nil", + hasDataSource: false, + pvc: func() *core.PersistentVolumeClaim { return nil }, + }, + } + + for _, enabled := range []bool{true, false} { + for _, oldpvcInfo := range pvcInfo { + for _, newpvcInfo := range pvcInfo { + oldPvcHasDataSource, oldpvc := oldpvcInfo.hasDataSource, oldpvcInfo.pvc() + newPvcHasDataSource, newpvc := newpvcInfo.hasDataSource, newpvcInfo.pvc() + if newpvc == nil { + continue + } + + t.Run(fmt.Sprintf("feature enabled=%v, old pvc %v, new pvc %v", enabled, oldpvcInfo.description, newpvcInfo.description), func(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumePVCDataSource, enabled)() + + var oldpvcSpec *core.PersistentVolumeClaimSpec + if oldpvc != nil { + oldpvcSpec = &oldpvc.Spec + } + DropDisabledFields(&newpvc.Spec, oldpvcSpec) + + // old pvc should never be changed + if !reflect.DeepEqual(oldpvc, oldpvcInfo.pvc()) { + t.Errorf("old pvc changed: %v", diff.ObjectReflectDiff(oldpvc, oldpvcInfo.pvc())) + } + + switch { + case enabled || oldPvcHasDataSource: + // new pvc should not be changed if the feature is enabled, or if the old pvc had DataSource + if !reflect.DeepEqual(newpvc, newpvcInfo.pvc()) { + t.Errorf("new pvc changed: %v", diff.ObjectReflectDiff(newpvc, newpvcInfo.pvc())) + } + case newPvcHasDataSource: + // new pvc should be changed + if reflect.DeepEqual(newpvc, newpvcInfo.pvc()) { + t.Errorf("new pvc was not changed") + } + // new pvc should not have DataSource + if !reflect.DeepEqual(newpvc, pvcWithoutDataSource()) { + t.Errorf("new pvc had DataSource: %v", diff.ObjectReflectDiff(newpvc, pvcWithoutDataSource())) + } + default: + // new pvc should not need to be changed + if !reflect.DeepEqual(newpvc, newpvcInfo.pvc()) { + t.Errorf("new pvc changed: %v", diff.ObjectReflectDiff(newpvc, newpvcInfo.pvc())) + } + } + }) + } + } + } +} + +// TestDropInvalidPVCDataSource checks specifically for invalid APIGroup settings for PVCDataSource +func TestDropInvalidPVCDataSource(t *testing.T) { + apiGroup := "" + pvcWithDataSource := func() *core.PersistentVolumeClaim { + return &core.PersistentVolumeClaim{ + Spec: core.PersistentVolumeClaimSpec{ + DataSource: &core.TypedLocalObjectReference{ + APIGroup: &apiGroup, + Kind: "PersistentVolumeClaim", + Name: "test_clone", + }, + }, + } + } + + invalidAPIGroup := "invalid.pvc.api.group" + pvcWithInvalidDataSource := func() *core.PersistentVolumeClaim { + return &core.PersistentVolumeClaim{ + Spec: core.PersistentVolumeClaimSpec{ + DataSource: &core.TypedLocalObjectReference{ + APIGroup: &invalidAPIGroup, + Kind: "PersistentVolumeClaim", + Name: "test_clone_invalid", + }, + }, + } + } + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumePVCDataSource, true)() + validPvc := pvcWithDataSource() + originalDS := validPvc.Spec.DataSource + DropDisabledFields(&validPvc.Spec, nil) + if validPvc.Spec.DataSource == nil { + t.Errorf("valid PVC DataSource was dropped: Kind: %s, APIGroup: %s, Name: %s\n", originalDS.Kind, *originalDS.APIGroup, originalDS.Name) + } + + invalidPvc := pvcWithInvalidDataSource() + originalDS = invalidPvc.Spec.DataSource + DropDisabledFields(&invalidPvc.Spec, nil) + if invalidPvc.Spec.DataSource != nil { + t.Errorf("invalid PVC DataSource was not dropped: Kind: %s, APIGroup: %s, Name: %s\n", originalDS.Kind, *originalDS.APIGroup, originalDS.Name) + } +} diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index 978de4ce1ee..408f7daeec3 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -414,10 +414,10 @@ type PersistentVolumeClaimSpec struct { // +optional VolumeMode *PersistentVolumeMode // This field can be used to specify either: - // * An existing VolumeSnapshot - // * An existing Volume (PVC, Clone operation) + // * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot) + // * An existing PVC (""/PersistentVolumeClaim) // In order to use either of these DataSource types, the appropriate feature gate - // must be enabled (VolumeSnapshotDataSource, VolumeDataSource) + // must be enabled (VolumeSnapshotDataSource, VolumePVCDataSource) // If the provisioner can support the specified data source, it will create // a new volume based on the contents of the specified PVC or Snapshot. // If the provisioner does not support the specified data source, the volume will diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 2a06e283740..4e4cf4dbdb6 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -13311,7 +13311,7 @@ func testDataSourceInSpec(name string, kind string, apiGroup string) *core.Persi return &dataSourceInSpec } -func TestAlphaVolumeDataSource(t *testing.T) { +func TestAlphaVolumePVCDataSource(t *testing.T) { successTestCases := []core.PersistentVolumeClaimSpec{ *testDataSourceInSpec("test_snapshot", "VolumeSnapshot", "snapshot.storage.k8s.io"), *testDataSourceInSpec("test_pvc", "PersistentVolumeClaim", ""), diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 20fc26e2cb8..0dd42697f04 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -465,6 +465,12 @@ const ( // // Enables NonPreempting option for priorityClass and pod. NonPreemptingPriority featuregate.Feature = "NonPreemptingPriority" + + // owner: @j-griffith + // alpha: v1.15 + // + // Enable support for specifying an existing PVC as a DataSource + VolumePVCDataSource featuregate.Feature = "VolumePVCDataSource" ) func init() { @@ -543,6 +549,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS WindowsGMSA: {Default: false, PreRelease: featuregate.Alpha}, LocalStorageCapacityIsolationFSQuotaMonitoring: {Default: false, PreRelease: featuregate.Alpha}, NonPreemptingPriority: {Default: false, PreRelease: featuregate.Alpha}, + VolumePVCDataSource: {Default: false, PreRelease: featuregate.Alpha}, // inherited features from generic apiserver, relisted here to get a conflict if it is changed // unintentionally on either side: