diff --git a/pkg/kubelet/util/util.go b/pkg/kubelet/util/util.go index cb70c07f86f..eb7cf142754 100644 --- a/pkg/kubelet/util/util.go +++ b/pkg/kubelet/util/util.go @@ -19,8 +19,6 @@ package util import ( "fmt" "net/url" - "path/filepath" - "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -47,14 +45,3 @@ func parseEndpoint(endpoint string) (string, string, error) { return u.Scheme, "", fmt.Errorf("protocol %q not supported", u.Scheme) } } - -func pathWithinBase(fullPath, basePath string) bool { - rel, err := filepath.Rel(basePath, fullPath) - if err != nil { - return false - } - if strings.HasPrefix(rel, "..") { - return false - } - return true -} diff --git a/pkg/util/mount/mount.go b/pkg/util/mount/mount.go index f241679b565..f30db7692e8 100644 --- a/pkg/util/mount/mount.go +++ b/pkg/util/mount/mount.go @@ -325,9 +325,15 @@ func pathWithinBase(fullPath, basePath string) bool { if err != nil { return false } - if strings.HasPrefix(rel, "..") { + if startsWithBackstep(rel) { // Needed to escape the base path return false } return true } + +// startsWithBackstep checks if the given path starts with a backstep segment +func startsWithBackstep(rel string) bool { + // normalize to / and check for ../ + return rel == ".." || strings.HasPrefix(filepath.ToSlash(rel), "../") +} diff --git a/pkg/util/mount/mount_linux_test.go b/pkg/util/mount/mount_linux_test.go index 25bdfc6fe6f..14e0c9af49e 100644 --- a/pkg/util/mount/mount_linux_test.go +++ b/pkg/util/mount/mount_linux_test.go @@ -402,6 +402,12 @@ func TestPathWithinBase(t *testing.T) { basePath: "/a", expected: false, }, + { + name: "configmap subpath", + fullPath: "/var/lib/kubelet/pods/uuid/volumes/kubernetes.io~configmap/config/..timestamp/file.txt", + basePath: "/var/lib/kubelet/pods/uuid/volumes/kubernetes.io~configmap/config", + expected: true, + }, } for _, test := range tests { if pathWithinBase(test.fullPath, test.basePath) != test.expected { diff --git a/pkg/util/mount/mount_windows.go b/pkg/util/mount/mount_windows.go index 9ba4417f979..12e1bca118a 100644 --- a/pkg/util/mount/mount_windows.go +++ b/pkg/util/mount/mount_windows.go @@ -291,7 +291,7 @@ func lockAndCheckSubPathWithoutSymlink(volumePath, subPath string) ([]uintptr, e if err != nil { return []uintptr{}, fmt.Errorf("Rel(%s, %s) error: %v", volumePath, subPath, err) } - if strings.HasPrefix(relSubPath, "..") { + if startsWithBackstep(relSubPath) { return []uintptr{}, fmt.Errorf("SubPath %q not within volume path %q", subPath, volumePath) } @@ -552,7 +552,7 @@ func findExistingPrefix(base, pathname string) (string, []string, error) { return base, nil, err } - if strings.HasPrefix(rel, "..") { + if startsWithBackstep(rel) { return base, nil, fmt.Errorf("pathname(%s) is not within base(%s)", pathname, base) } diff --git a/pkg/util/mount/mount_windows_test.go b/pkg/util/mount/mount_windows_test.go index 76ef08ccc9d..e31217e975d 100644 --- a/pkg/util/mount/mount_windows_test.go +++ b/pkg/util/mount/mount_windows_test.go @@ -538,6 +538,11 @@ func TestPathWithinBase(t *testing.T) { basePath: `c:\tmp\a\b\c`, expectedResult: false, }, + { + fullPath: `c:\kubelet\pods\uuid\volumes\kubernetes.io~configmap\config\..timestamp\file.txt`, + basePath: `c:\kubelet\pods\uuid\volumes\kubernetes.io~configmap\config`, + expectedResult: true, + }, } for _, test := range tests { diff --git a/test/e2e/storage/subpath.go b/test/e2e/storage/subpath.go index 4a0aa1ff4ff..f6c5f4889fe 100644 --- a/test/e2e/storage/subpath.go +++ b/test/e2e/storage/subpath.go @@ -22,6 +22,7 @@ import ( "strings" "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" utilerrors "k8s.io/apimachinery/pkg/util/errors" @@ -78,6 +79,120 @@ var _ = utils.SIGDescribe("Subpath", func() { f := framework.NewDefaultFramework("subpath") + Context("Atomic writer volumes", func() { + var err error + + BeforeEach(func() { + By("Setting up data") + secret := &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "my-secret"}, Data: map[string][]byte{"secret-key": []byte("secret-value")}} + secret, err = f.ClientSet.CoreV1().Secrets(f.Namespace.Name).Create(secret) + if err != nil && !apierrors.IsAlreadyExists(err) { + Expect(err).ToNot(HaveOccurred()) + } + + configmap := &v1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "my-configmap"}, Data: map[string]string{"configmap-key": "configmap-value"}} + configmap, err = f.ClientSet.CoreV1().ConfigMaps(f.Namespace.Name).Create(configmap) + if err != nil && !apierrors.IsAlreadyExists(err) { + Expect(err).ToNot(HaveOccurred()) + } + }) + + gracePeriod := int64(1) + + It("should support subpaths with secret pod", func() { + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "pod-subpath-test-secret"}, + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "test", + Image: mountImage, + Args: []string{"--file_content=" + volumePath}, + VolumeMounts: []v1.VolumeMount{{Name: "secret", MountPath: volumePath, SubPath: "secret-key"}}, + }}, + RestartPolicy: v1.RestartPolicyNever, + TerminationGracePeriodSeconds: &gracePeriod, + Volumes: []v1.Volume{{Name: "secret", VolumeSource: v1.VolumeSource{Secret: &v1.SecretVolumeSource{SecretName: "my-secret"}}}}, + }, + } + By(fmt.Sprintf("Creating pod %s", pod.Name)) + f.TestContainerOutput("secret", pod, 0, []string{"secret-value"}) + }) + + It("should support subpaths with configmap pod", func() { + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "pod-subpath-test-configmap"}, + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "test", + Image: mountImage, + Args: []string{"--file_content=" + volumePath}, + VolumeMounts: []v1.VolumeMount{{Name: "configmap", MountPath: volumePath, SubPath: "configmap-key"}}, + }}, + RestartPolicy: v1.RestartPolicyNever, + TerminationGracePeriodSeconds: &gracePeriod, + Volumes: []v1.Volume{{Name: "configmap", VolumeSource: v1.VolumeSource{ConfigMap: &v1.ConfigMapVolumeSource{LocalObjectReference: v1.LocalObjectReference{Name: "my-configmap"}}}}}, + }, + } + By(fmt.Sprintf("Creating pod %s", pod.Name)) + f.TestContainerOutput("configmap", pod, 0, []string{"configmap-value"}) + }) + + It("should support subpaths with downward pod", func() { + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "pod-subpath-test-downward"}, + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "test", + Image: mountImage, + Args: []string{"--file_content=" + volumePath}, + VolumeMounts: []v1.VolumeMount{{Name: "downward", MountPath: volumePath, SubPath: "downward/podname"}}, + }}, + RestartPolicy: v1.RestartPolicyNever, + TerminationGracePeriodSeconds: &gracePeriod, + Volumes: []v1.Volume{ + {Name: "downward", VolumeSource: v1.VolumeSource{ + DownwardAPI: &v1.DownwardAPIVolumeSource{ + Items: []v1.DownwardAPIVolumeFile{{Path: "downward/podname", FieldRef: &v1.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name"}}}, + }, + }}, + }, + }, + } + By(fmt.Sprintf("Creating pod %s", pod.Name)) + f.TestContainerOutput("downward", pod, 0, []string{"content of file \"" + volumePath + "\": " + pod.Name}) + }) + + It("should support subpaths with projected pod", func() { + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "pod-subpath-test-secret"}, + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "test", + Image: mountImage, + Args: []string{"--file_content=" + volumePath}, + VolumeMounts: []v1.VolumeMount{{Name: "projected", MountPath: volumePath, SubPath: "projected/configmap-key"}}, + }}, + RestartPolicy: v1.RestartPolicyNever, + TerminationGracePeriodSeconds: &gracePeriod, + Volumes: []v1.Volume{ + {Name: "projected", VolumeSource: v1.VolumeSource{ + Projected: &v1.ProjectedVolumeSource{ + Sources: []v1.VolumeProjection{ + {ConfigMap: &v1.ConfigMapProjection{ + LocalObjectReference: v1.LocalObjectReference{Name: "my-configmap"}, + Items: []v1.KeyToPath{{Path: "projected/configmap-key", Key: "configmap-key"}}, + }}, + }, + }, + }}, + }, + }, + } + By(fmt.Sprintf("Creating pod %s", pod.Name)) + f.TestContainerOutput("projected", pod, 0, []string{"configmap-value"}) + }) + }) + for volType, volInit := range initVolSources { curVolType := volType curVolInit := volInit