From f6d97b5d2b139c593991a50064e3aa352600ca39 Mon Sep 17 00:00:00 2001 From: Michelle Au Date: Mon, 5 Mar 2018 09:14:44 +0100 Subject: [PATCH] Add feature gate for subpath --- pkg/apis/core/validation/validation.go | 6 +- pkg/apis/core/validation/validation_test.go | 62 +++++++++++++++++++++ pkg/features/kube_features.go | 8 +++ pkg/kubelet/kubelet_pods.go | 4 ++ pkg/kubelet/kubelet_pods_test.go | 56 +++++++++++++++++++ 5 files changed, 135 insertions(+), 1 deletion(-) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index b534b89f441..5b54d6fe642 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -2264,7 +2264,11 @@ func ValidateVolumeMounts(mounts []core.VolumeMount, voldevices map[string]strin } if len(mnt.SubPath) > 0 { - allErrs = append(allErrs, validateLocalDescendingPath(mnt.SubPath, fldPath.Child("subPath"))...) + if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeSubpath) { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("subPath"), "subPath is disabled by feature-gate")) + } else { + allErrs = append(allErrs, validateLocalDescendingPath(mnt.SubPath, fldPath.Child("subPath"))...) + } } if mnt.MountPropagation != nil { diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index ce19b450e61..a48c04f7c52 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -4766,6 +4766,68 @@ func TestValidateVolumeMounts(t *testing.T) { } } +func TestValidateDisabledSubpath(t *testing.T) { + utilfeature.DefaultFeatureGate.Set("VolumeSubpath=false") + defer utilfeature.DefaultFeatureGate.Set("VolumeSubpath=true") + + volumes := []core.Volume{ + {Name: "abc", VolumeSource: core.VolumeSource{PersistentVolumeClaim: &core.PersistentVolumeClaimVolumeSource{ClaimName: "testclaim1"}}}, + {Name: "abc-123", VolumeSource: core.VolumeSource{PersistentVolumeClaim: &core.PersistentVolumeClaimVolumeSource{ClaimName: "testclaim2"}}}, + {Name: "123", VolumeSource: core.VolumeSource{HostPath: &core.HostPathVolumeSource{Path: "/foo/baz", Type: newHostPathType(string(core.HostPathUnset))}}}, + } + vols, v1err := ValidateVolumes(volumes, field.NewPath("field")) + if len(v1err) > 0 { + t.Errorf("Invalid test volume - expected success %v", v1err) + return + } + + container := core.Container{ + SecurityContext: nil, + } + + goodVolumeDevices := []core.VolumeDevice{ + {Name: "xyz", DevicePath: "/foofoo"}, + {Name: "uvw", DevicePath: "/foofoo/share/test"}, + } + + cases := map[string]struct { + mounts []core.VolumeMount + expectError bool + }{ + "subpath not specified": { + []core.VolumeMount{ + { + Name: "abc-123", + MountPath: "/bab", + }, + }, + false, + }, + "subpath specified": { + []core.VolumeMount{ + { + Name: "abc-123", + MountPath: "/bab", + SubPath: "baz", + }, + }, + true, + }, + } + + for name, test := range cases { + errs := ValidateVolumeMounts(test.mounts, GetVolumeDeviceMap(goodVolumeDevices), vols, &container, field.NewPath("field")) + + if len(errs) != 0 && !test.expectError { + t.Errorf("test %v failed: %+v", name, errs) + } + + if len(errs) == 0 && test.expectError { + t.Errorf("test %v failed, expected error", name) + } + } +} + func TestValidateMountPropagation(t *testing.T) { bTrue := true bFalse := false diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 10a58493dfd..4ec194d55ee 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -267,6 +267,13 @@ const ( // // Enables control over the primary group ID of containers' init processes. RunAsGroup utilfeature.Feature = "RunAsGroup" + + // owner: @saad-ali + // ga + // + // Allow mounting a subpath of a volume in a container + // Do not remove this feature gate even though it's GA + VolumeSubpath utilfeature.Feature = "VolumeSubpath" ) func init() { @@ -313,6 +320,7 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS CRIContainerLogRotation: {Default: false, PreRelease: utilfeature.Alpha}, GCERegionalPersistentDisk: {Default: true, PreRelease: utilfeature.Beta}, RunAsGroup: {Default: false, PreRelease: utilfeature.Alpha}, + VolumeSubpath: {Default: true, PreRelease: utilfeature.GA}, // inherited features from generic apiserver, relisted here to get a conflict if it is changed // unintentionally on either side: diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index 6698a5df87a..2712448cf81 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -194,6 +194,10 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h return nil, cleanupAction, err } if mount.SubPath != "" { + if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeSubpath) { + return nil, cleanupAction, fmt.Errorf("volume subpaths are disabled") + } + if filepath.IsAbs(mount.SubPath) { return nil, cleanupAction, fmt.Errorf("error SubPath `%s` must not be an absolute path", mount.SubPath) } diff --git a/pkg/kubelet/kubelet_pods_test.go b/pkg/kubelet/kubelet_pods_test.go index ee00373bf31..92552f5227b 100644 --- a/pkg/kubelet/kubelet_pods_test.go +++ b/pkg/kubelet/kubelet_pods_test.go @@ -355,6 +355,62 @@ func TestMakeMounts(t *testing.T) { } } +func TestDisabledSubpath(t *testing.T) { + fm := &mount.FakeMounter{} + pod := v1.Pod{ + Spec: v1.PodSpec{ + HostNetwork: true, + }, + } + podVolumes := kubecontainer.VolumeMap{ + "disk": kubecontainer.VolumeInfo{Mounter: &stubVolume{path: "/mnt/disk"}}, + } + + cases := map[string]struct { + container v1.Container + expectError bool + }{ + "subpath not specified": { + v1.Container{ + VolumeMounts: []v1.VolumeMount{ + { + MountPath: "/mnt/path3", + Name: "disk", + ReadOnly: true, + }, + }, + }, + false, + }, + "subpath specified": { + v1.Container{ + VolumeMounts: []v1.VolumeMount{ + { + MountPath: "/mnt/path3", + SubPath: "/must/not/be/absolute", + Name: "disk", + ReadOnly: true, + }, + }, + }, + true, + }, + } + + utilfeature.DefaultFeatureGate.Set("VolumeSubpath=false") + defer utilfeature.DefaultFeatureGate.Set("VolumeSubpath=true") + + for name, test := range cases { + _, _, err := makeMounts(&pod, "/pod", &test.container, "fakepodname", "", "", podVolumes, fm) + if err != nil && !test.expectError { + t.Errorf("test %v failed: %v", name, err) + } + if err == nil && test.expectError { + t.Errorf("test %v failed: expected error", name) + } + } +} + func TestMakeBlockVolumes(t *testing.T) { testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */) defer testKubelet.Cleanup()