Add dynamic environment variable substitution to subpaths

pkg/features/kube_features.go

@@ -286,6 +286,13 @@ const (
 	//
 	// Extend the default scheduler to be aware of volume topology and handle PV provisioning
 	DynamicProvisioningScheduling utilfeature.Feature = "DynamicProvisioningScheduling"
+
+	// owner: @kevtaylor
+	// alpha: v1.11
+	//
+	// Allow subpath environment variable substitution
+	// Only applicable if the VolumeSubpath feature is also enabled
+	VolumeSubpathEnvExpansion utilfeature.Feature = "VolumeSubpathEnvExpansion"
 )
 
 func init() {
@@ -335,6 +342,7 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS
 	VolumeSubpath:                               {Default: true, PreRelease: utilfeature.GA},
 	BalanceAttachedNodeVolumes:                  {Default: false, PreRelease: utilfeature.Alpha},
 	DynamicProvisioningScheduling:               {Default: false, PreRelease: utilfeature.Alpha},
+	VolumeSubpathEnvExpansion:                   {Default: false, PreRelease: utilfeature.Alpha},
 
 	// inherited features from generic apiserver, relisted here to get a conflict if it is changed
 	// unintentionally on either side:

pkg/kubelet/container/helpers.go

@@ -133,6 +133,11 @@ func ExpandContainerCommandOnlyStatic(containerCommand []string, envs []v1.EnvVa
 	return command
 }
 
+func ExpandContainerVolumeMounts(mount v1.VolumeMount, envs []EnvVar) (expandedSubpath string) {
+	mapping := expansion.MappingFuncFor(EnvVarsToMap(envs))
+	return expansion.Expand(mount.SubPath, mapping)
+}
+
 func ExpandContainerCommandAndArgs(container *v1.Container, envs []EnvVar) (command []string, args []string) {
 	mapping := expansion.MappingFuncFor(EnvVarsToMap(envs))
 

pkg/kubelet/container/helpers_test.go

@@ -138,6 +138,114 @@ func TestExpandCommandAndArgs(t *testing.T) {
 	}
 }
 
+func TestExpandVolumeMountsWithSubpath(t *testing.T) {
+	cases := []struct {
+		name              string
+		container         *v1.Container
+		envs              []EnvVar
+		expectedSubPath   string
+		expectedMountPath string
+	}{
+		{
+			name: "subpath with no expansion",
+			container: &v1.Container{
+				VolumeMounts: []v1.VolumeMount{{SubPath: "foo"}},
+			},
+			expectedSubPath:   "foo",
+			expectedMountPath: "",
+		},
+		{
+			name: "volumes with expanded subpath",
+			container: &v1.Container{
+				VolumeMounts: []v1.VolumeMount{{SubPath: "foo/$(POD_NAME)"}},
+			},
+			envs: []EnvVar{
+				{
+					Name:  "POD_NAME",
+					Value: "bar",
+				},
+			},
+			expectedSubPath:   "foo/bar",
+			expectedMountPath: "",
+		},
+		{
+			name: "volumes expanded with empty subpath",
+			container: &v1.Container{
+				VolumeMounts: []v1.VolumeMount{{SubPath: ""}},
+			},
+			envs: []EnvVar{
+				{
+					Name:  "POD_NAME",
+					Value: "bar",
+				},
+			},
+			expectedSubPath:   "",
+			expectedMountPath: "",
+		},
+		{
+			name: "volumes expanded with no envs subpath",
+			container: &v1.Container{
+				VolumeMounts: []v1.VolumeMount{{SubPath: "/foo/$(POD_NAME)"}},
+			},
+			expectedSubPath:   "/foo/$(POD_NAME)",
+			expectedMountPath: "",
+		},
+		{
+			name: "volumes expanded with leading environment variable",
+			container: &v1.Container{
+				VolumeMounts: []v1.VolumeMount{{SubPath: "$(POD_NAME)/bar"}},
+			},
+			envs: []EnvVar{
+				{
+					Name:  "POD_NAME",
+					Value: "foo",
+				},
+			},
+			expectedSubPath:   "foo/bar",
+			expectedMountPath: "",
+		},
+		{
+			name: "volumes with volume and subpath",
+			container: &v1.Container{
+				VolumeMounts: []v1.VolumeMount{{MountPath: "/foo", SubPath: "$(POD_NAME)/bar"}},
+			},
+			envs: []EnvVar{
+				{
+					Name:  "POD_NAME",
+					Value: "foo",
+				},
+			},
+			expectedSubPath:   "foo/bar",
+			expectedMountPath: "/foo",
+		},
+		{
+			name: "volumes with volume and no subpath",
+			container: &v1.Container{
+				VolumeMounts: []v1.VolumeMount{{MountPath: "/foo"}},
+			},
+			envs: []EnvVar{
+				{
+					Name:  "POD_NAME",
+					Value: "foo",
+				},
+			},
+			expectedSubPath:   "",
+			expectedMountPath: "/foo",
+		},
+	}
+
+	for _, tc := range cases {
+		actualSubPath := ExpandContainerVolumeMounts(tc.container.VolumeMounts[0], tc.envs)
+		if e, a := tc.expectedSubPath, actualSubPath; !reflect.DeepEqual(e, a) {
+			t.Errorf("%v: unexpected subpath; expected %v, got %v", tc.name, e, a)
+		}
+		if e, a := tc.expectedMountPath, tc.container.VolumeMounts[0].MountPath; !reflect.DeepEqual(e, a) {
+			t.Errorf("%v: unexpected mountpath; expected %v, got %v", tc.name, e, a)
+		}
+	}
+
+}
+
 func TestShouldContainerBeRestarted(t *testing.T) {
 	pod := &v1.Pod{
 		ObjectMeta: metav1.ObjectMeta{

pkg/kubelet/kubelet_pods.go

@@ -129,7 +129,8 @@ func (kl *Kubelet) makeBlockVolumes(pod *v1.Pod, container *v1.Container, podVol
 }
 
 // makeMounts determines the mount points for the given container.
-func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, hostDomain, podIP string, podVolumes kubecontainer.VolumeMap, mounter mountutil.Interface) ([]kubecontainer.Mount, func(), error) {
+func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, hostDomain, podIP string, podVolumes kubecontainer.VolumeMap, mounter mountutil.Interface, expandEnvs []kubecontainer.EnvVar) ([]kubecontainer.Mount, func(), error) {
+
 	// Kubernetes only mounts on /etc/hosts if:
 	// - container is not an infrastructure (pause) container
 	// - container is not already mounting on /etc/hosts
@@ -166,6 +167,11 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h
 				return nil, cleanupAction, fmt.Errorf("volume subpaths are disabled")
 			}
 
+			// Expand subpath variables
+			if utilfeature.DefaultFeatureGate.Enabled(features.VolumeSubpathEnvExpansion) {
+				mount.SubPath = kubecontainer.ExpandContainerVolumeMounts(mount, expandEnvs)
+			}
+
 			if filepath.IsAbs(mount.SubPath) {
 				return nil, cleanupAction, fmt.Errorf("error SubPath `%s` must not be an absolute path", mount.SubPath)
 			}
@@ -454,17 +460,17 @@ func (kl *Kubelet) GenerateRunContainerOptions(pod *v1.Pod, container *v1.Contai
 		opts.Devices = append(opts.Devices, blkVolumes...)
 	}
 
-	mounts, cleanupAction, err := makeMounts(pod, kl.getPodDir(pod.UID), container, hostname, hostDomainName, podIP, volumes, kl.mounter)
+	envs, err := kl.makeEnvironmentVariables(pod, container, podIP)
 	if err != nil {
-		return nil, cleanupAction, err
+		return nil, nil, err
 	}
-	opts.Mounts = append(opts.Mounts, mounts...)
+	opts.Envs = append(opts.Envs, envs...)
 
-	envs, err := kl.makeEnvironmentVariables(pod, container, podIP)
+	mounts, cleanupAction, err := makeMounts(pod, kl.getPodDir(pod.UID), container, hostname, hostDomainName, podIP, volumes, kl.mounter, opts.Envs)
 	if err != nil {
 		return nil, cleanupAction, err
 	}
-	opts.Envs = append(opts.Envs, envs...)
+	opts.Mounts = append(opts.Mounts, mounts...)
 
 	// Disabling adding TerminationMessagePath on Windows as these files would be mounted as docker volume and
 	// Docker for Windows has a bug where only directories can be mounted

pkg/kubelet/kubelet_pods_test.go

@@ -273,7 +273,7 @@ func TestMakeMounts(t *testing.T) {
 				return
 			}
 
-			mounts, _, err := makeMounts(&pod, "/pod", &tc.container, "fakepodname", "", "", tc.podVolumes, fm)
+			mounts, _, err := makeMounts(&pod, "/pod", &tc.container, "fakepodname", "", "", tc.podVolumes, fm, nil)
 
 			// validate only the error if we expect an error
 			if tc.expectErr {
@@ -296,7 +296,7 @@ func TestMakeMounts(t *testing.T) {
 				t.Errorf("Failed to enable feature gate for MountPropagation: %v", err)
 				return
 			}
-			mounts, _, err = makeMounts(&pod, "/pod", &tc.container, "fakepodname", "", "", tc.podVolumes, fm)
+			mounts, _, err = makeMounts(&pod, "/pod", &tc.container, "fakepodname", "", "", tc.podVolumes, fm, nil)
 			if !tc.expectErr {
 				expectedPrivateMounts := []kubecontainer.Mount{}
 				for _, mount := range tc.expectedMounts {
@@ -357,7 +357,7 @@ func TestDisabledSubpath(t *testing.T) {
 	defer utilfeature.DefaultFeatureGate.Set("VolumeSubpath=true")
 
 	for name, test := range cases {
-		_, _, err := makeMounts(&pod, "/pod", &test.container, "fakepodname", "", "", podVolumes, fm)
+		_, _, err := makeMounts(&pod, "/pod", &test.container, "fakepodname", "", "", podVolumes, fm, nil)
 		if err != nil && !test.expectError {
 			t.Errorf("test %v failed: %v", name, err)
 		}

pkg/kubelet/kubelet_pods_windows_test.go

@@ -66,7 +66,7 @@ func TestMakeMountsWindows(t *testing.T) {
 	}
 
 	fm := &mount.FakeMounter{}
-	mounts, _, _ := makeMounts(&pod, "/pod", &container, "fakepodname", "", "", podVolumes, fm)
+	mounts, _, _ := makeMounts(&pod, "/pod", &container, "fakepodname", "", "", podVolumes, fm, nil)
 
 	expectedMounts := []kubecontainer.Mount{
 		{

test/e2e/common/expansion.go

@@ -19,8 +19,13 @@ package common
 import (
 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/util/uuid"
 	"k8s.io/kubernetes/test/e2e/framework"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"time"
 )
 
 // These tests exercise the Kubernetes expansion syntax $(VAR).
@@ -144,4 +149,188 @@ var _ = framework.KubeDescribe("Variable Expansion", func() {
 			"test-value",
 		})
 	})
+
+	/*
+		    Testname: var-expansion-subpath
+		    Description: Make sure a container's subpath can be set using an
+			expansion of environment variables.
+	*/
+	It("should allow substituting values in a volume subpath [Feature:VolumeSubpathEnvExpansion][NodeAlphaFeature:VolumeSubpathEnvExpansion]", func() {
+		podName := "var-expansion-" + string(uuid.NewUUID())
+		pod := &v1.Pod{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   podName,
+				Labels: map[string]string{"name": podName},
+			},
+			Spec: v1.PodSpec{
+				Containers: []v1.Container{
+					{
+						Name:    "dapi-container",
+						Image:   busyboxImage,
+						Command: []string{"sh", "-c", "test -d /testcontainer/" + podName + ";echo $?"},
+						Env: []v1.EnvVar{
+							{
+								Name:  "POD_NAME",
+								Value: podName,
+							},
+						},
+						VolumeMounts: []v1.VolumeMount{
+							{
+								Name:      "workdir1",
+								MountPath: "/logscontainer",
+								SubPath:   "$(POD_NAME)",
+							},
+							{
+								Name:      "workdir2",
+								MountPath: "/testcontainer",
+							},
+						},
+					},
+				},
+				RestartPolicy: v1.RestartPolicyNever,
+				Volumes: []v1.Volume{
+					{
+						Name: "workdir1",
+						VolumeSource: v1.VolumeSource{
+							HostPath: &v1.HostPathVolumeSource{Path: "/tmp"},
+						},
+					},
+					{
+						Name: "workdir2",
+						VolumeSource: v1.VolumeSource{
+							HostPath: &v1.HostPathVolumeSource{Path: "/tmp"},
+						},
+					},
+				},
+			},
+		}
+
+		f.TestContainerOutput("substitution in volume subpath", pod, 0, []string{
+			"0",
+		})
+	})
+
+	/*
+		    Testname: var-expansion-subpath-with-backticks
+		    Description: Make sure a container's subpath can not be set using an
+			expansion of environment variables when backticks are supplied.
+	*/
+	It("should fail substituting values in a volume subpath with backticks [Feature:VolumeSubpathEnvExpansion][NodeAlphaFeature:VolumeSubpathEnvExpansion][Slow]", func() {
+
+		podName := "var-expansion-" + string(uuid.NewUUID())
+		pod := &v1.Pod{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   podName,
+				Labels: map[string]string{"name": podName},
+			},
+			Spec: v1.PodSpec{
+				Containers: []v1.Container{
+					{
+						Name:  "dapi-container",
+						Image: busyboxImage,
+						Env: []v1.EnvVar{
+							{
+								Name:  "POD_NAME",
+								Value: "..",
+							},
+						},
+						VolumeMounts: []v1.VolumeMount{
+							{
+								Name:      "workdir1",
+								MountPath: "/logscontainer",
+								SubPath:   "$(POD_NAME)",
+							},
+						},
+					},
+				},
+				RestartPolicy: v1.RestartPolicyNever,
+				Volumes: []v1.Volume{
+					{
+						Name: "workdir1",
+						VolumeSource: v1.VolumeSource{
+							EmptyDir: &v1.EmptyDirVolumeSource{},
+						},
+					},
+				},
+			},
+		}
+
+		// Pod should fail
+		testPodFailSubpath(f, pod, "SubPath `..`: must not contain '..'")
+	})
+
+	/*
+		    Testname: var-expansion-subpath-with-absolute-path
+		    Description: Make sure a container's subpath can not be set using an
+			expansion of environment variables when absoluete path is supplied.
+	*/
+	It("should fail substituting values in a volume subpath with absolute path [Feature:VolumeSubpathEnvExpansion][NodeAlphaFeature:VolumeSubpathEnvExpansion][Slow]", func() {
+
+		podName := "var-expansion-" + string(uuid.NewUUID())
+		pod := &v1.Pod{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   podName,
+				Labels: map[string]string{"name": podName},
+			},
+			Spec: v1.PodSpec{
+				Containers: []v1.Container{
+					{
+						Name:  "dapi-container",
+						Image: busyboxImage,
+						Env: []v1.EnvVar{
+							{
+								Name:  "POD_NAME",
+								Value: "/tmp",
+							},
+						},
+						VolumeMounts: []v1.VolumeMount{
+							{
+								Name:      "workdir1",
+								MountPath: "/logscontainer",
+								SubPath:   "$(POD_NAME)",
+							},
+						},
+					},
+				},
+				RestartPolicy: v1.RestartPolicyNever,
+				Volumes: []v1.Volume{
+					{
+						Name: "workdir1",
+						VolumeSource: v1.VolumeSource{
+							EmptyDir: &v1.EmptyDirVolumeSource{},
+						},
+					},
+				},
+			},
+		}
+
+		// Pod should fail
+		testPodFailSubpath(f, pod, "SubPath `/tmp` must not be an absolute path")
+	})
 })
+
+func testPodFailSubpath(f *framework.Framework, pod *v1.Pod, errorText string) {
+
+	pod, err := f.ClientSet.CoreV1().Pods(f.Namespace.Name).Create(pod)
+	Expect(err).ToNot(HaveOccurred(), "while creating pod")
+
+	defer func() {
+		framework.DeletePodWithWait(f, f.ClientSet, pod)
+	}()
+
+	err = framework.WaitTimeoutForPodRunningInNamespace(f.ClientSet, pod.Name, pod.Namespace, 30*time.Second)
+	Expect(err).To(HaveOccurred(), "while waiting for pod to be running")
+
+	selector := fields.Set{
+		"involvedObject.kind":      "Pod",
+		"involvedObject.name":      pod.Name,
+		"involvedObject.namespace": f.Namespace.Name,
+		"reason":                   "Failed",
+	}.AsSelector().String()
+
+	options := metav1.ListOptions{FieldSelector: selector}
+	events, err := f.ClientSet.CoreV1().Events(f.Namespace.Name).List(options)
+	Expect(err).NotTo(HaveOccurred(), "while getting pod events")
+	Expect(len(events.Items)).NotTo(Equal(0), "no events found")
+	Expect(events.Items[0].Message).To(ContainSubstring(errorText), "subpath error not found")
+}