KEP-5040: Disable git_repo volume driver.

This commit is contained in:
Vinayak Goyal 2025-01-31 02:44:41 +00:00
parent 6ef1a1f98e
commit 282e1490d4
7 changed files with 229 additions and 42 deletions

View File

@ -26,10 +26,12 @@ import (
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/util/validation/field"
utilfeature "k8s.io/apiserver/pkg/util/feature"
nodeapi "k8s.io/kubernetes/pkg/api/node" nodeapi "k8s.io/kubernetes/pkg/api/node"
pvcutil "k8s.io/kubernetes/pkg/api/persistentvolumeclaim" pvcutil "k8s.io/kubernetes/pkg/api/persistentvolumeclaim"
api "k8s.io/kubernetes/pkg/apis/core" api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/apis/core/pods" "k8s.io/kubernetes/pkg/apis/core/pods"
"k8s.io/kubernetes/pkg/features"
) )
func GetWarningsForPod(ctx context.Context, pod, oldPod *api.Pod) []string { func GetWarningsForPod(ctx context.Context, pod, oldPod *api.Pod) []string {
@ -142,8 +144,12 @@ 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"))) 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 { if v.GitRepo != nil {
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"))) warnings = append(warnings, fmt.Sprintf("%s: deprecated in v1.11", fieldPath.Child("spec", "volumes").Index(i).Child("gitRepo")))
} }
}
if v.ScaleIO != nil { 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"))) warnings = append(warnings, fmt.Sprintf("%s: deprecated in v1.16, non-functional in v1.22+", fieldPath.Child("spec", "volumes").Index(i).Child("scaleIO")))
} }

View File

@ -26,7 +26,10 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation/field" "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" api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/features"
utilpointer "k8s.io/utils/pointer" utilpointer "k8s.io/utils/pointer"
) )
@ -151,6 +154,7 @@ func TestWarnings(t *testing.T) {
name string name string
template *api.PodTemplateSpec template *api.PodTemplateSpec
oldTemplate *api.PodTemplateSpec oldTemplate *api.PodTemplateSpec
gitRepoPluginDisabled bool
expected []string expected []string
}{ }{
{ {
@ -176,6 +180,16 @@ func TestWarnings(t *testing.T) {
}, },
expected: []string{`spec.volumes[0].gitRepo: deprecated in v1.11`}, 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", name: "scaleIO",
template: &api.PodTemplateSpec{Spec: api.PodSpec{ template: &api.PodTemplateSpec{Spec: api.PodSpec{
@ -1786,6 +1800,7 @@ func TestWarnings(t *testing.T) {
for _, tc := range testcases { for _, tc := range testcases {
t.Run("podspec_"+tc.name, func(t *testing.T) { t.Run("podspec_"+tc.name, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.GitRepoVolumeDriver, !tc.gitRepoPluginDisabled)
var oldTemplate *api.PodTemplateSpec var oldTemplate *api.PodTemplateSpec
if tc.oldTemplate != nil { if tc.oldTemplate != nil {
oldTemplate = tc.oldTemplate oldTemplate = tc.oldTemplate
@ -1805,6 +1820,7 @@ func TestWarnings(t *testing.T) {
}) })
t.Run("pod_"+tc.name, func(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 var pod *api.Pod
if tc.template != nil { if tc.template != nil {
pod = &api.Pod{ pod = &api.Pod{

View File

@ -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 // Lock to default and remove after v1.22 based on user feedback that should be reflected in KEP #1972 update
ExecProbeTimeout featuregate.Feature = "ExecProbeTimeout" 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 // owner: @bobbypage
// Adds support for kubelet to detect node shutdown and gracefully terminate pods prior to the node being shutdown. // Adds support for kubelet to detect node shutdown and gracefully terminate pods prior to the node being shutdown.
GracefulNodeShutdown featuregate.Feature = "GracefulNodeShutdown" GracefulNodeShutdown featuregate.Feature = "GracefulNodeShutdown"

View File

@ -359,6 +359,11 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
{Version: version.MustParse("1.32"), Default: true, PreRelease: featuregate.Beta}, {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: { GracefulNodeShutdown: {
{Version: version.MustParse("1.20"), Default: false, PreRelease: featuregate.Alpha}, {Version: version.MustParse("1.20"), Default: false, PreRelease: featuregate.Alpha},
{Version: version.MustParse("1.21"), Default: true, PreRelease: featuregate.Beta}, {Version: version.MustParse("1.21"), Default: true, PreRelease: featuregate.Beta},

View File

@ -24,6 +24,8 @@ import (
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/types"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume"
volumeutil "k8s.io/kubernetes/pkg/volume/util" volumeutil "k8s.io/kubernetes/pkg/volume/util"
"k8s.io/utils/exec" "k8s.io/utils/exec"
@ -169,6 +171,10 @@ func (b *gitRepoVolumeMounter) GetAttributes() volume.Attributes {
// SetUp creates new directory and clones a git repo. // SetUp creates new directory and clones a git repo.
func (b *gitRepoVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { 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) return b.SetUpAt(b.GetPath(), mounterArgs)
} }

View File

@ -29,6 +29,9 @@ import (
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types" "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"
"k8s.io/kubernetes/pkg/volume/emptydir" "k8s.io/kubernetes/pkg/volume/emptydir"
volumetest "k8s.io/kubernetes/pkg/volume/testing" volumetest "k8s.io/kubernetes/pkg/volume/testing"
@ -70,16 +73,19 @@ type expectedCommand struct {
dir string dir string
} }
func TestPlugin(t *testing.T) { type scenario struct {
gitURL := "https://github.com/kubernetes/kubernetes.git"
revision := "2a30ce65c5ab586b98916d83385c5983edd353a1"
scenarios := []struct {
name string name string
vol *v1.Volume vol *v1.Volume
expecteds []expectedCommand expecteds []expectedCommand
isExpectedFailure bool isExpectedFailure bool
}{ gitRepoPluginDisabled bool
}
func TestPlugin(t *testing.T) {
gitURL := "https://github.com/kubernetes/kubernetes.git"
revision := "2a30ce65c5ab586b98916d83385c5983edd353a1"
scenarios := []scenario{
{ {
name: "target-dir", name: "target-dir",
vol: &v1.Volume{ vol: &v1.Volume{
@ -106,7 +112,21 @@ func TestPlugin(t *testing.T) {
dir: "/target_dir", 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", name: "target-dir-no-revision",
@ -125,7 +145,20 @@ func TestPlugin(t *testing.T) {
dir: "", 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", name: "only-git-clone",
@ -143,7 +176,19 @@ func TestPlugin(t *testing.T) {
dir: "", 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", name: "no-target-dir",
@ -171,7 +216,21 @@ func TestPlugin(t *testing.T) {
dir: "/kubernetes", 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", name: "current-dir",
@ -199,7 +258,21 @@ func TestPlugin(t *testing.T) {
dir: "", 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", name: "current-dir-mess",
@ -227,7 +300,21 @@ func TestPlugin(t *testing.T) {
dir: "", 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", name: "invalid-repository",
@ -241,6 +328,19 @@ func TestPlugin(t *testing.T) {
}, },
isExpectedFailure: true, 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", name: "invalid-revision",
vol: &v1.Volume{ vol: &v1.Volume{
@ -254,6 +354,20 @@ func TestPlugin(t *testing.T) {
}, },
isExpectedFailure: true, 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", name: "invalid-directory",
vol: &v1.Volume{ vol: &v1.Volume{
@ -267,6 +381,20 @@ func TestPlugin(t *testing.T) {
}, },
isExpectedFailure: true, 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", name: "invalid-revision-directory-combo",
vol: &v1.Volume{ vol: &v1.Volume{
@ -281,26 +409,40 @@ func TestPlugin(t *testing.T) {
}, },
isExpectedFailure: true, 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 { for _, sc := range scenarios {
allErrs := doTestPlugin(scenario, t) t.Run(fmt.Sprintf("%s/gitRepoPluginDisabled:%v", sc.name, sc.gitRepoPluginDisabled), func(t *testing.T) {
if len(allErrs) == 0 && scenario.isExpectedFailure { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.GitRepoVolumeDriver, !sc.gitRepoPluginDisabled)
t.Errorf("Unexpected success for scenario: %s", scenario.name) allErrs := doTestPlugin(t, sc)
if len(allErrs) == 0 && sc.isExpectedFailure {
t.Errorf("Unexpected success for scenario: %s", sc.name)
} }
if len(allErrs) > 0 && !scenario.isExpectedFailure { if len(allErrs) > 0 && !sc.isExpectedFailure {
t.Errorf("Unexpected failure for scenario: %s - %+v", scenario.name, allErrs) t.Errorf("Unexpected failure for scenario: %s - %+v", sc.name, allErrs)
} }
})
} }
} }
func doTestPlugin(scenario struct { func doTestPlugin(t *testing.T, sc scenario) []error {
name string
vol *v1.Volume
expecteds []expectedCommand
isExpectedFailure bool
}, t *testing.T) []error {
allErrs := []error{} allErrs := []error{}
plugMgr := volume.VolumePluginMgr{} plugMgr := volume.VolumePluginMgr{}
@ -315,7 +457,7 @@ func doTestPlugin(scenario struct {
return allErrs return allErrs
} }
pod := &v1.Pod{ObjectMeta: metav1.ObjectMeta{UID: types.UID("poduid")}} 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 { if err != nil {
allErrs = append(allErrs, allErrs = append(allErrs,
@ -329,7 +471,7 @@ func doTestPlugin(scenario struct {
} }
path := mounter.GetPath() 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) { if !strings.HasSuffix(path, suffix) {
allErrs = append(allErrs, allErrs = append(allErrs,
fmt.Errorf("got unexpected path: %s", path)) fmt.Errorf("got unexpected path: %s", path))
@ -337,7 +479,7 @@ func doTestPlugin(scenario struct {
} }
// Test setUp() // Test setUp()
setUpErrs := doTestSetUp(scenario, mounter) setUpErrs := doTestSetUp(sc, mounter)
allErrs = append(allErrs, setUpErrs...) allErrs = append(allErrs, setUpErrs...)
if _, err := os.Stat(path); err != nil { if _, err := os.Stat(path); err != nil {
@ -353,7 +495,7 @@ func doTestPlugin(scenario struct {
} }
// gitRepo volume should create its own empty wrapper path // 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 _, err := os.Stat(podWrapperMetadataDir); err != nil {
if os.IsNotExist(err) { if os.IsNotExist(err) {
@ -392,13 +534,8 @@ func doTestPlugin(scenario struct {
return allErrs return allErrs
} }
func doTestSetUp(scenario struct { func doTestSetUp(sc scenario, mounter volume.Mounter) []error {
name string expecteds := sc.expecteds
vol *v1.Volume
expecteds []expectedCommand
isExpectedFailure bool
}, mounter volume.Mounter) []error {
expecteds := scenario.expecteds
allErrs := []error{} allErrs := []error{}
// Construct combined outputs from expected commands // Construct combined outputs from expected commands

View File

@ -475,6 +475,16 @@
lockToDefault: false lockToDefault: false
preRelease: Alpha preRelease: Alpha
version: "1.32" 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 - name: GracefulNodeShutdown
versionedSpecs: versionedSpecs:
- default: false - default: false