diff --git a/pkg/api/pod/warnings.go b/pkg/api/pod/warnings.go index 3556f9d7603..ff8df957f25 100644 --- a/pkg/api/pod/warnings.go +++ b/pkg/api/pod/warnings.go @@ -26,10 +26,12 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" + utilfeature "k8s.io/apiserver/pkg/util/feature" nodeapi "k8s.io/kubernetes/pkg/api/node" pvcutil "k8s.io/kubernetes/pkg/api/persistentvolumeclaim" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/pods" + "k8s.io/kubernetes/pkg/features" ) func GetWarningsForPod(ctx context.Context, pod, oldPod *api.Pod) []string { @@ -142,7 +144,11 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta warnings = append(warnings, fmt.Sprintf("%s: deprecated in v1.11, non-functional in v1.16+", fieldPath.Child("spec", "volumes").Index(i).Child("photonPersistentDisk"))) } if v.GitRepo != nil { - warnings = append(warnings, fmt.Sprintf("%s: deprecated in v1.11", fieldPath.Child("spec", "volumes").Index(i).Child("gitRepo"))) + if !utilfeature.DefaultFeatureGate.Enabled(features.GitRepoVolumeDriver) { + warnings = append(warnings, fmt.Sprintf("%s: deprecated in v1.11, and disabled by default in v1.33+", fieldPath.Child("spec", "volumes").Index(i).Child("gitRepo"))) + } else { + warnings = append(warnings, fmt.Sprintf("%s: deprecated in v1.11", fieldPath.Child("spec", "volumes").Index(i).Child("gitRepo"))) + } } if v.ScaleIO != nil { warnings = append(warnings, fmt.Sprintf("%s: deprecated in v1.16, non-functional in v1.22+", fieldPath.Child("spec", "volumes").Index(i).Child("scaleIO"))) diff --git a/pkg/api/pod/warnings_test.go b/pkg/api/pod/warnings_test.go index 0524c5188e6..cc97b0f1a4c 100644 --- a/pkg/api/pod/warnings_test.go +++ b/pkg/api/pod/warnings_test.go @@ -26,7 +26,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" api "k8s.io/kubernetes/pkg/apis/core" + "k8s.io/kubernetes/pkg/features" utilpointer "k8s.io/utils/pointer" ) @@ -148,10 +151,11 @@ func TestWarnings(t *testing.T) { } testName := "Test" testcases := []struct { - name string - template *api.PodTemplateSpec - oldTemplate *api.PodTemplateSpec - expected []string + name string + template *api.PodTemplateSpec + oldTemplate *api.PodTemplateSpec + gitRepoPluginDisabled bool + expected []string }{ { name: "null", @@ -176,6 +180,16 @@ func TestWarnings(t *testing.T) { }, expected: []string{`spec.volumes[0].gitRepo: deprecated in v1.11`}, }, + { + name: "gitRepo plugin disabled", + template: &api.PodTemplateSpec{Spec: api.PodSpec{ + Volumes: []api.Volume{ + {Name: "s", VolumeSource: api.VolumeSource{GitRepo: &api.GitRepoVolumeSource{}}}, + }}, + }, + gitRepoPluginDisabled: true, + expected: []string{`spec.volumes[0].gitRepo: deprecated in v1.11, and disabled by default in v1.33+`}, + }, { name: "scaleIO", template: &api.PodTemplateSpec{Spec: api.PodSpec{ @@ -1786,6 +1800,7 @@ func TestWarnings(t *testing.T) { for _, tc := range testcases { t.Run("podspec_"+tc.name, func(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.GitRepoVolumeDriver, !tc.gitRepoPluginDisabled) var oldTemplate *api.PodTemplateSpec if tc.oldTemplate != nil { oldTemplate = tc.oldTemplate @@ -1805,6 +1820,7 @@ func TestWarnings(t *testing.T) { }) t.Run("pod_"+tc.name, func(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.GitRepoVolumeDriver, !tc.gitRepoPluginDisabled) var pod *api.Pod if tc.template != nil { pod = &api.Pod{ diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 5b9e4bdc79d..806d95302ad 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -264,6 +264,13 @@ const ( // Lock to default and remove after v1.22 based on user feedback that should be reflected in KEP #1972 update ExecProbeTimeout featuregate.Feature = "ExecProbeTimeout" + // owner: @vinayakankugoyal @thockin + // + // Controls if the gitRepo volume plugin is supported or not. + // KEP #5040 disables the gitRepo volume plugin by default starting v1.33, + // this provides a way for users to override it. + GitRepoVolumeDriver featuregate.Feature = "GitRepoVolumeDriver" + // owner: @bobbypage // Adds support for kubelet to detect node shutdown and gracefully terminate pods prior to the node being shutdown. GracefulNodeShutdown featuregate.Feature = "GracefulNodeShutdown" diff --git a/pkg/features/versioned_kube_features.go b/pkg/features/versioned_kube_features.go index 7ea7c0e9fa5..887c81c4a47 100644 --- a/pkg/features/versioned_kube_features.go +++ b/pkg/features/versioned_kube_features.go @@ -359,6 +359,11 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate {Version: version.MustParse("1.32"), Default: true, PreRelease: featuregate.Beta}, }, + GitRepoVolumeDriver: { + {Version: version.MustParse("1.0"), Default: true, PreRelease: featuregate.GA}, + {Version: version.MustParse("1.33"), Default: false, PreRelease: featuregate.Deprecated}, + }, + GracefulNodeShutdown: { {Version: version.MustParse("1.20"), Default: false, PreRelease: featuregate.Alpha}, {Version: version.MustParse("1.21"), Default: true, PreRelease: featuregate.Beta}, diff --git a/pkg/volume/git_repo/git_repo.go b/pkg/volume/git_repo/git_repo.go index 3a4bf850f58..985be19cd52 100644 --- a/pkg/volume/git_repo/git_repo.go +++ b/pkg/volume/git_repo/git_repo.go @@ -24,6 +24,8 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" volumeutil "k8s.io/kubernetes/pkg/volume/util" "k8s.io/utils/exec" @@ -169,6 +171,10 @@ func (b *gitRepoVolumeMounter) GetAttributes() volume.Attributes { // SetUp creates new directory and clones a git repo. func (b *gitRepoVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { + if !utilfeature.DefaultFeatureGate.Enabled(features.GitRepoVolumeDriver) { + return fmt.Errorf("git-repo volume plugin has been disabled; if necessary, it may be re-enabled by enabling the feature-gate `GitRepoVolumeDriver`") + } + return b.SetUpAt(b.GetPath(), mounterArgs) } diff --git a/pkg/volume/git_repo/git_repo_test.go b/pkg/volume/git_repo/git_repo_test.go index 5bcee89f9c9..bba41d32171 100644 --- a/pkg/volume/git_repo/git_repo_test.go +++ b/pkg/volume/git_repo/git_repo_test.go @@ -29,6 +29,9 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/emptydir" volumetest "k8s.io/kubernetes/pkg/volume/testing" @@ -70,16 +73,19 @@ type expectedCommand struct { dir string } +type scenario struct { + name string + vol *v1.Volume + expecteds []expectedCommand + isExpectedFailure bool + gitRepoPluginDisabled bool +} + func TestPlugin(t *testing.T) { gitURL := "https://github.com/kubernetes/kubernetes.git" revision := "2a30ce65c5ab586b98916d83385c5983edd353a1" - scenarios := []struct { - name string - vol *v1.Volume - expecteds []expectedCommand - isExpectedFailure bool - }{ + scenarios := []scenario{ { name: "target-dir", vol: &v1.Volume{ @@ -106,7 +112,21 @@ func TestPlugin(t *testing.T) { dir: "/target_dir", }, }, - isExpectedFailure: false, + }, + { + name: "target-dir", + gitRepoPluginDisabled: true, + vol: &v1.Volume{ + Name: "vol1", + VolumeSource: v1.VolumeSource{ + GitRepo: &v1.GitRepoVolumeSource{ + Repository: gitURL, + Revision: revision, + Directory: "target_dir", + }, + }, + }, + isExpectedFailure: true, }, { name: "target-dir-no-revision", @@ -125,7 +145,20 @@ func TestPlugin(t *testing.T) { dir: "", }, }, - isExpectedFailure: false, + }, + { + name: "target-dir-no-revision", + gitRepoPluginDisabled: true, + vol: &v1.Volume{ + Name: "vol1", + VolumeSource: v1.VolumeSource{ + GitRepo: &v1.GitRepoVolumeSource{ + Repository: gitURL, + Directory: "target_dir", + }, + }, + }, + isExpectedFailure: true, }, { name: "only-git-clone", @@ -143,7 +176,19 @@ func TestPlugin(t *testing.T) { dir: "", }, }, - isExpectedFailure: false, + }, + { + name: "only-git-clone", + gitRepoPluginDisabled: true, + vol: &v1.Volume{ + Name: "vol1", + VolumeSource: v1.VolumeSource{ + GitRepo: &v1.GitRepoVolumeSource{ + Repository: gitURL, + }, + }, + }, + isExpectedFailure: true, }, { name: "no-target-dir", @@ -171,7 +216,21 @@ func TestPlugin(t *testing.T) { dir: "/kubernetes", }, }, - isExpectedFailure: false, + }, + { + name: "no-target-dir", + gitRepoPluginDisabled: true, + vol: &v1.Volume{ + Name: "vol1", + VolumeSource: v1.VolumeSource{ + GitRepo: &v1.GitRepoVolumeSource{ + Repository: gitURL, + Revision: revision, + Directory: "", + }, + }, + }, + isExpectedFailure: true, }, { name: "current-dir", @@ -199,7 +258,21 @@ func TestPlugin(t *testing.T) { dir: "", }, }, - isExpectedFailure: false, + }, + { + name: "current-dir", + gitRepoPluginDisabled: true, + vol: &v1.Volume{ + Name: "vol1", + VolumeSource: v1.VolumeSource{ + GitRepo: &v1.GitRepoVolumeSource{ + Repository: gitURL, + Revision: revision, + Directory: ".", + }, + }, + }, + isExpectedFailure: true, }, { name: "current-dir-mess", @@ -227,7 +300,21 @@ func TestPlugin(t *testing.T) { dir: "", }, }, - isExpectedFailure: false, + }, + { + name: "current-dir-mess", + gitRepoPluginDisabled: true, + vol: &v1.Volume{ + Name: "vol1", + VolumeSource: v1.VolumeSource{ + GitRepo: &v1.GitRepoVolumeSource{ + Repository: gitURL, + Revision: revision, + Directory: "./.", + }, + }, + }, + isExpectedFailure: true, }, { name: "invalid-repository", @@ -241,6 +328,19 @@ func TestPlugin(t *testing.T) { }, isExpectedFailure: true, }, + { + name: "invalid-repository", + gitRepoPluginDisabled: true, + vol: &v1.Volume{ + Name: "vol1", + VolumeSource: v1.VolumeSource{ + GitRepo: &v1.GitRepoVolumeSource{ + Repository: "--foo", + }, + }, + }, + isExpectedFailure: true, + }, { name: "invalid-revision", vol: &v1.Volume{ @@ -254,6 +354,20 @@ func TestPlugin(t *testing.T) { }, isExpectedFailure: true, }, + { + name: "invalid-revision", + gitRepoPluginDisabled: true, + vol: &v1.Volume{ + Name: "vol1", + VolumeSource: v1.VolumeSource{ + GitRepo: &v1.GitRepoVolumeSource{ + Repository: gitURL, + Revision: "--bar", + }, + }, + }, + isExpectedFailure: true, + }, { name: "invalid-directory", vol: &v1.Volume{ @@ -267,6 +381,20 @@ func TestPlugin(t *testing.T) { }, isExpectedFailure: true, }, + { + name: "invalid-directory", + gitRepoPluginDisabled: true, + vol: &v1.Volume{ + Name: "vol1", + VolumeSource: v1.VolumeSource{ + GitRepo: &v1.GitRepoVolumeSource{ + Repository: gitURL, + Directory: "-b", + }, + }, + }, + isExpectedFailure: true, + }, { name: "invalid-revision-directory-combo", vol: &v1.Volume{ @@ -281,26 +409,40 @@ func TestPlugin(t *testing.T) { }, isExpectedFailure: true, }, + { + name: "invalid-revision-directory-combo", + gitRepoPluginDisabled: true, + vol: &v1.Volume{ + Name: "vol1", + VolumeSource: v1.VolumeSource{ + GitRepo: &v1.GitRepoVolumeSource{ + Repository: gitURL, + Revision: "main", + Directory: "foo/bar", + }, + }, + }, + isExpectedFailure: true, + }, } - for _, scenario := range scenarios { - allErrs := doTestPlugin(scenario, t) - if len(allErrs) == 0 && scenario.isExpectedFailure { - t.Errorf("Unexpected success for scenario: %s", scenario.name) - } - if len(allErrs) > 0 && !scenario.isExpectedFailure { - t.Errorf("Unexpected failure for scenario: %s - %+v", scenario.name, allErrs) - } + for _, sc := range scenarios { + t.Run(fmt.Sprintf("%s/gitRepoPluginDisabled:%v", sc.name, sc.gitRepoPluginDisabled), func(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.GitRepoVolumeDriver, !sc.gitRepoPluginDisabled) + allErrs := doTestPlugin(t, sc) + if len(allErrs) == 0 && sc.isExpectedFailure { + t.Errorf("Unexpected success for scenario: %s", sc.name) + } + if len(allErrs) > 0 && !sc.isExpectedFailure { + t.Errorf("Unexpected failure for scenario: %s - %+v", sc.name, allErrs) + } + }) + } } -func doTestPlugin(scenario struct { - name string - vol *v1.Volume - expecteds []expectedCommand - isExpectedFailure bool -}, t *testing.T) []error { +func doTestPlugin(t *testing.T, sc scenario) []error { allErrs := []error{} plugMgr := volume.VolumePluginMgr{} @@ -315,7 +457,7 @@ func doTestPlugin(scenario struct { return allErrs } pod := &v1.Pod{ObjectMeta: metav1.ObjectMeta{UID: types.UID("poduid")}} - mounter, err := plug.NewMounter(volume.NewSpecFromVolume(scenario.vol), pod) + mounter, err := plug.NewMounter(volume.NewSpecFromVolume(sc.vol), pod) if err != nil { allErrs = append(allErrs, @@ -329,7 +471,7 @@ func doTestPlugin(scenario struct { } path := mounter.GetPath() - suffix := filepath.Join("pods/poduid/volumes/kubernetes.io~git-repo", scenario.vol.Name) + suffix := filepath.Join("pods/poduid/volumes/kubernetes.io~git-repo", sc.vol.Name) if !strings.HasSuffix(path, suffix) { allErrs = append(allErrs, fmt.Errorf("got unexpected path: %s", path)) @@ -337,7 +479,7 @@ func doTestPlugin(scenario struct { } // Test setUp() - setUpErrs := doTestSetUp(scenario, mounter) + setUpErrs := doTestSetUp(sc, mounter) allErrs = append(allErrs, setUpErrs...) if _, err := os.Stat(path); err != nil { @@ -353,7 +495,7 @@ func doTestPlugin(scenario struct { } // gitRepo volume should create its own empty wrapper path - podWrapperMetadataDir := fmt.Sprintf("%v/pods/poduid/plugins/kubernetes.io~empty-dir/wrapped_%v", rootDir, scenario.vol.Name) + podWrapperMetadataDir := fmt.Sprintf("%v/pods/poduid/plugins/kubernetes.io~empty-dir/wrapped_%v", rootDir, sc.vol.Name) if _, err := os.Stat(podWrapperMetadataDir); err != nil { if os.IsNotExist(err) { @@ -392,13 +534,8 @@ func doTestPlugin(scenario struct { return allErrs } -func doTestSetUp(scenario struct { - name string - vol *v1.Volume - expecteds []expectedCommand - isExpectedFailure bool -}, mounter volume.Mounter) []error { - expecteds := scenario.expecteds +func doTestSetUp(sc scenario, mounter volume.Mounter) []error { + expecteds := sc.expecteds allErrs := []error{} // Construct combined outputs from expected commands diff --git a/test/compatibility_lifecycle/reference/versioned_feature_list.yaml b/test/compatibility_lifecycle/reference/versioned_feature_list.yaml index 0c9e20cc924..8498d5da327 100644 --- a/test/compatibility_lifecycle/reference/versioned_feature_list.yaml +++ b/test/compatibility_lifecycle/reference/versioned_feature_list.yaml @@ -475,6 +475,16 @@ lockToDefault: false preRelease: Alpha version: "1.32" +- name: GitRepoVolumeDriver + versionedSpecs: + - default: true + lockToDefault: false + preRelease: GA + version: "1.0" + - default: false + lockToDefault: false + preRelease: Deprecated + version: "1.33" - name: GracefulNodeShutdown versionedSpecs: - default: false