From 51ae5a34b999c6232aaa71733ea95f718ab9b92c Mon Sep 17 00:00:00 2001 From: Seth Jennings Date: Thu, 17 Nov 2016 11:39:27 -0600 Subject: [PATCH] fix permissions when using fsGroup --- pkg/volume/volume_linux.go | 11 +++++++++++ test/e2e/common/configmap.go | 13 +++++++------ test/e2e/common/downwardapi_volume.go | 15 +++++++++++++++ test/e2e/common/secrets.go | 22 ++++++++++++++++++---- 4 files changed, 51 insertions(+), 10 deletions(-) diff --git a/pkg/volume/volume_linux.go b/pkg/volume/volume_linux.go index 1391e62256b..d82c7767244 100644 --- a/pkg/volume/volume_linux.go +++ b/pkg/volume/volume_linux.go @@ -51,6 +51,17 @@ func SetVolumeOwnership(mounter Mounter, fsGroup *int64) error { return err } + // chown and chmod pass through to the underlying file for symlinks. + // Symlinks have a mode of 777 but this really doesn't mean anything. + // The permissions of the underlying file are what matter. + // However, if one reads the mode of a symlink then chmods the symlink + // with that mode, it changes the mode of the underlying file, overridden + // the defaultMode and permissions initialized by the volume plugin, which + // is not what we want; thus, we skip chown/chmod for symlinks. + if info.Mode()&os.ModeSymlink != 0 { + return nil + } + stat, ok := info.Sys().(*syscall.Stat_t) if !ok { return nil diff --git a/test/e2e/common/configmap.go b/test/e2e/common/configmap.go index 20d7a0aa80b..f160686d5ac 100644 --- a/test/e2e/common/configmap.go +++ b/test/e2e/common/configmap.go @@ -41,6 +41,11 @@ var _ = framework.KubeDescribe("ConfigMap", func() { doConfigMapE2EWithoutMappings(f, 0, 0, &defaultMode) }) + It("should be consumable from pods in volume as non-root with defaultMode and fsGroup set [Feature:FSGroup]", func() { + defaultMode := int32(0440) /* setting fsGroup sets mode to at least 440 */ + doConfigMapE2EWithoutMappings(f, 1000, 1001, &defaultMode) + }) + It("should be consumable from pods in volume as non-root [Conformance]", func() { doConfigMapE2EWithoutMappings(f, 1000, 0, nil) }) @@ -343,14 +348,10 @@ func doConfigMapE2EWithoutMappings(f *framework.Framework, uid, fsGroup int64, d defaultMode = &mode } - // Just check file mode if fsGroup is not set. If fsGroup is set, the - // final mode is adjusted and we are not testing that case. + modeString := fmt.Sprintf("%v", os.FileMode(*defaultMode)) output := []string{ "content of file \"/etc/configmap-volume/data-1\": value-1", - } - if fsGroup == 0 { - modeString := fmt.Sprintf("%v", os.FileMode(*defaultMode)) - output = append(output, "mode of file \"/etc/configmap-volume/data-1\": "+modeString) + "mode of file \"/etc/configmap-volume/data-1\": " + modeString, } f.TestContainerOutput("consume configMaps", pod, 0, output) } diff --git a/test/e2e/common/downwardapi_volume.go b/test/e2e/common/downwardapi_volume.go index 3b2141f52e5..f6c2f57138e 100644 --- a/test/e2e/common/downwardapi_volume.go +++ b/test/e2e/common/downwardapi_volume.go @@ -81,6 +81,21 @@ var _ = framework.KubeDescribe("Downward API volume", func() { }) }) + It("should provide podname as non-root with fsgroup and defaultMode [Feature:FSGroup]", func() { + podName := "metadata-volume-" + string(uuid.NewUUID()) + uid := int64(1001) + gid := int64(1234) + mode := int32(0440) /* setting fsGroup sets mode to at least 440 */ + pod := downwardAPIVolumePodForModeTest(podName, "/etc/podname", &mode, nil) + pod.Spec.SecurityContext = &v1.PodSecurityContext{ + RunAsUser: &uid, + FSGroup: &gid, + } + f.TestContainerOutput("downward API volume plugin", pod, 0, []string{ + "mode of file \"/etc/podname\": -r--r-----", + }) + }) + It("should update labels on modification [Conformance]", func() { labels := map[string]string{} labels["key1"] = "value1" diff --git a/test/e2e/common/secrets.go b/test/e2e/common/secrets.go index 196fc1ffd05..341bd2b0702 100644 --- a/test/e2e/common/secrets.go +++ b/test/e2e/common/secrets.go @@ -31,12 +31,19 @@ var _ = framework.KubeDescribe("Secrets", func() { f := framework.NewDefaultFramework("secrets") It("should be consumable from pods in volume [Conformance]", func() { - doSecretE2EWithoutMapping(f, nil /* default mode */, "secret-test-"+string(uuid.NewUUID())) + doSecretE2EWithoutMapping(f, nil /* default mode */, "secret-test-"+string(uuid.NewUUID()), nil, nil) }) It("should be consumable from pods in volume with defaultMode set [Conformance]", func() { defaultMode := int32(0400) - doSecretE2EWithoutMapping(f, &defaultMode, "secret-test-"+string(uuid.NewUUID())) + doSecretE2EWithoutMapping(f, &defaultMode, "secret-test-"+string(uuid.NewUUID()), nil, nil) + }) + + It("should be consumable from pods in volume as non-root with defaultMode and fsGroup set [Conformance]", func() { + defaultMode := int32(0440) /* setting fsGroup sets mode to at least 440 */ + fsGroup := int64(1001) + uid := int64(1000) + doSecretE2EWithoutMapping(f, &defaultMode, "secret-test-"+string(uuid.NewUUID()), &fsGroup, &uid) }) It("should be consumable from pods in volume with mappings [Conformance]", func() { @@ -66,7 +73,7 @@ var _ = framework.KubeDescribe("Secrets", func() { if secret2, err = f.ClientSet.Core().Secrets(namespace2.Name).Create(secret2); err != nil { framework.Failf("unable to create test secret %s: %v", secret2.Name, err) } - doSecretE2EWithoutMapping(f, nil /* default mode */, secret2.Name) + doSecretE2EWithoutMapping(f, nil /* default mode */, secret2.Name, nil, nil) }) It("should be consumable in multiple volumes in a pod [Conformance]", func() { @@ -201,7 +208,7 @@ func secretForTest(namespace, name string) *v1.Secret { } } -func doSecretE2EWithoutMapping(f *framework.Framework, defaultMode *int32, secretName string) { +func doSecretE2EWithoutMapping(f *framework.Framework, defaultMode *int32, secretName string, fsGroup *int64, uid *int64) { var ( volumeName = "secret-volume" volumeMountPath = "/etc/secret-volume" @@ -256,6 +263,13 @@ func doSecretE2EWithoutMapping(f *framework.Framework, defaultMode *int32, secre defaultMode = &mode } + if fsGroup != nil || uid != nil { + pod.Spec.SecurityContext = &v1.PodSecurityContext{ + FSGroup: fsGroup, + RunAsUser: uid, + } + } + modeString := fmt.Sprintf("%v", os.FileMode(*defaultMode)) expectedOutput := []string{ "content of file \"/etc/secret-volume/data-1\": value-1",