diff --git a/pkg/api/persistentvolumeclaim/util.go b/pkg/api/persistentvolumeclaim/util.go index 2fbb207a32e..34de30e65d5 100644 --- a/pkg/api/persistentvolumeclaim/util.go +++ b/pkg/api/persistentvolumeclaim/util.go @@ -22,6 +22,11 @@ import ( "k8s.io/kubernetes/pkg/features" ) +const ( + pvc string = "PersistentVolumeClaim" + volumeSnapshot string = "VolumeSnapshot" +) + // DropDisabledFields removes disabled fields from the pvc spec. // This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a pvc spec. func DropDisabledFields(pvcSpec, oldPVCSpec *core.PersistentVolumeClaimSpec) { @@ -55,13 +60,13 @@ func dataSourceInUse(oldPVCSpec *core.PersistentVolumeClaimSpec) bool { func dataSourceIsEnabled(pvcSpec *core.PersistentVolumeClaimSpec) bool { if pvcSpec.DataSource != nil { - if pvcSpec.DataSource.Kind == "PersistentVolumeClaim" && + if pvcSpec.DataSource.Kind == pvc && *pvcSpec.DataSource.APIGroup == "" && utilfeature.DefaultFeatureGate.Enabled(features.VolumePVCDataSource) { return true } - if pvcSpec.DataSource.Kind == "VolumeSnapshot" && + if pvcSpec.DataSource.Kind == volumeSnapshot && *pvcSpec.DataSource.APIGroup == "snapshot.storage.k8s.io" && utilfeature.DefaultFeatureGate.Enabled(features.VolumeSnapshotDataSource) { return true diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 4e4cf4dbdb6..d12862a89bd 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -13312,24 +13312,48 @@ func testDataSourceInSpec(name string, kind string, apiGroup string) *core.Persi } func TestAlphaVolumePVCDataSource(t *testing.T) { - successTestCases := []core.PersistentVolumeClaimSpec{ - *testDataSourceInSpec("test_snapshot", "VolumeSnapshot", "snapshot.storage.k8s.io"), - *testDataSourceInSpec("test_pvc", "PersistentVolumeClaim", ""), - } - failedTestCases := []core.PersistentVolumeClaimSpec{ - *testDataSourceInSpec("", "VolumeSnapshot", "snapshot.storage.k8s.io"), - *testDataSourceInSpec("test_snapshot", "PersistentVolumeClaim", "snapshot.storage.k8s.io"), - *testDataSourceInSpec("test_snapshot", "VolumeSnapshot", "storage.k8s.io"), + + testCases := []struct { + testName string + claimSpec core.PersistentVolumeClaimSpec + expectedFail bool + }{ + { + testName: "test create from valid snapshot source", + claimSpec: *testDataSourceInSpec("test_snapshot", "VolumeSnapshot", "snapshot.storage.k8s.io"), + }, + { + testName: "test create from valid pvc source", + claimSpec: *testDataSourceInSpec("test_pvc", "PersistentVolumeClaim", ""), + }, + { + testName: "test missing name in snapshot datasource should fail", + claimSpec: *testDataSourceInSpec("", "VolumeSnapshot", "snapshot.storage.k8s.io"), + expectedFail: true, + }, + { + testName: "test specifying pvc with snapshot api group should fail", + claimSpec: *testDataSourceInSpec("test_snapshot", "PersistentVolumeClaim", "snapshot.storage.k8s.io"), + expectedFail: true, + }, + { + testName: "test invalid group name in snapshot datasource should fail", + claimSpec: *testDataSourceInSpec("test_snapshot", "VolumeSnapshot", "storage.k8s.io"), + expectedFail: true, + }, } - for _, tc := range successTestCases { - if errs := ValidatePersistentVolumeClaimSpec(&tc, field.NewPath("spec")); len(errs) != 0 { - t.Errorf("expected success: %v", errs) - } - } - for _, tc := range failedTestCases { - if errs := ValidatePersistentVolumeClaimSpec(&tc, field.NewPath("spec")); len(errs) == 0 { - t.Errorf("expected failure: %v", errs) + for _, tc := range testCases { + if tc.expectedFail { + if errs := ValidatePersistentVolumeClaimSpec(&tc.claimSpec, field.NewPath("spec")); len(errs) == 0 { + t.Errorf("expected failure: %v", errs) + } + + } else { + if errs := ValidatePersistentVolumeClaimSpec(&tc.claimSpec, field.NewPath("spec")); len(errs) != 0 { + t.Errorf("expected success: %v", errs) + } + } } }