Skip to content

Commit

Permalink
Merge pull request #61308 from rootfs/automated-cherry-pick-of-#61284…
Browse files Browse the repository at this point in the history
…-upstream-release-1.9

Automatic merge from submit-queue.

Automated cherry pick of #61284 upstream release 1.9

**What this PR does / why we need it**:
cherrypick #61824 into 1.9
**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #61283

**Special notes for your reviewer**:
@jsafrane @liggitt 
**Release note**:

```release-note
Fix a regression preventing subpath mounts in pods using fsGroup from having set-GID bits set properly
```
  • Loading branch information
Kubernetes Submit Queue committed Mar 26, 2018
2 parents 8a247d5 + fced803 commit 3b721ed
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 6 deletions.
14 changes: 13 additions & 1 deletion pkg/util/mount/mount_linux.go
Expand Up @@ -996,7 +996,19 @@ func doSafeMakeDir(pathname string, base string, perm os.FileMode) error {
// so user can read/write it. This is the behavior of previous code.
// TODO: chmod all created directories, not just the last one.
// parentFD is the last created directory.
if err = syscall.Fchmod(parentFD, uint32(perm)&uint32(os.ModePerm)); err != nil {

// Translate perm (os.FileMode) to uint32 that fchmod() expects
kernelPerm := uint32(perm & os.ModePerm)
if perm&os.ModeSetgid > 0 {
kernelPerm |= syscall.S_ISGID
}
if perm&os.ModeSetuid > 0 {
kernelPerm |= syscall.S_ISUID
}
if perm&os.ModeSticky > 0 {
kernelPerm |= syscall.S_ISVTX
}
if err = syscall.Fchmod(parentFD, kernelPerm); err != nil {
return fmt.Errorf("chmod %q failed: %s", currentPath, err)
}
return nil
Expand Down
56 changes: 52 additions & 4 deletions pkg/util/mount/mount_linux_test.go
Expand Up @@ -418,14 +418,15 @@ func TestPathWithinBase(t *testing.T) {
}

func TestSafeMakeDir(t *testing.T) {
defaultPerm := os.FileMode(0750)
defaultPerm := os.FileMode(0750) + os.ModeDir
tests := []struct {
name string
// Function that prepares directory structure for the test under given
// base.
prepare func(base string) error
path string
checkPath string
perm os.FileMode
expectError bool
}{
{
Expand All @@ -435,6 +436,37 @@ func TestSafeMakeDir(t *testing.T) {
},
"test/directory",
"test/directory",
defaultPerm,
false,
},
{
"directory-with-sgid",
func(base string) error {
return nil
},
"test/directory",
"test/directory",
os.FileMode(0777) + os.ModeDir + os.ModeSetgid,
false,
},
{
"directory-with-suid",
func(base string) error {
return nil
},
"test/directory",
"test/directory",
os.FileMode(0777) + os.ModeDir + os.ModeSetuid,
false,
},
{
"directory-with-sticky-bit",
func(base string) error {
return nil
},
"test/directory",
"test/directory",
os.FileMode(0777) + os.ModeDir + os.ModeSticky,
false,
},
{
Expand All @@ -444,6 +476,7 @@ func TestSafeMakeDir(t *testing.T) {
},
"test/directory",
"test/directory",
defaultPerm,
false,
},
{
Expand All @@ -453,6 +486,7 @@ func TestSafeMakeDir(t *testing.T) {
},
"",
"",
defaultPerm,
false,
},
{
Expand All @@ -462,6 +496,7 @@ func TestSafeMakeDir(t *testing.T) {
},
"..",
"",
defaultPerm,
true,
},
{
Expand All @@ -471,6 +506,7 @@ func TestSafeMakeDir(t *testing.T) {
},
"test/../../..",
"",
defaultPerm,
true,
},
{
Expand All @@ -483,6 +519,7 @@ func TestSafeMakeDir(t *testing.T) {
},
"test/directory",
"destination/directory",
defaultPerm,
false,
},
{
Expand All @@ -492,6 +529,7 @@ func TestSafeMakeDir(t *testing.T) {
},
"test/directory",
"",
defaultPerm,
true,
},
{
Expand All @@ -514,6 +552,7 @@ func TestSafeMakeDir(t *testing.T) {
},
"test1/dir/dir/dir/dir/dir/dir/dir/foo",
"test2/foo",
defaultPerm,
false,
},
{
Expand All @@ -523,6 +562,7 @@ func TestSafeMakeDir(t *testing.T) {
},
"test/directory",
"",
defaultPerm,
true,
},
{
Expand All @@ -532,6 +572,7 @@ func TestSafeMakeDir(t *testing.T) {
},
"test/directory",
"",
defaultPerm,
true,
},
{
Expand All @@ -541,6 +582,7 @@ func TestSafeMakeDir(t *testing.T) {
},
"test",
"",
defaultPerm,
true,
},
{
Expand All @@ -556,6 +598,7 @@ func TestSafeMakeDir(t *testing.T) {
},
"dir/test",
"",
defaultPerm,
false,
},
{
Expand All @@ -568,6 +611,7 @@ func TestSafeMakeDir(t *testing.T) {
},
"dir/test",
"",
defaultPerm,
true,
},
{
Expand All @@ -577,6 +621,7 @@ func TestSafeMakeDir(t *testing.T) {
},
"test/directory",
"",
defaultPerm,
true,
},
}
Expand All @@ -589,7 +634,7 @@ func TestSafeMakeDir(t *testing.T) {
}
test.prepare(base)
pathToCreate := filepath.Join(base, test.path)
err = doSafeMakeDir(pathToCreate, base, defaultPerm)
err = doSafeMakeDir(pathToCreate, base, test.perm)
if err != nil && !test.expectError {
t.Errorf("test %q: %s", test.name, err)
}
Expand All @@ -601,14 +646,17 @@ func TestSafeMakeDir(t *testing.T) {
}

if test.checkPath != "" {
if _, err := os.Stat(filepath.Join(base, test.checkPath)); err != nil {
st, err := os.Stat(filepath.Join(base, test.checkPath))
if err != nil {
t.Errorf("test %q: cannot read path %s", test.name, test.checkPath)
}
if st.Mode() != test.perm {
t.Errorf("test %q: expected permissions %o, got %o", test.name, test.perm, st.Mode())
}
}

os.RemoveAll(base)
}

}

func validateDirEmpty(dir string) error {
Expand Down
4 changes: 3 additions & 1 deletion test/e2e/common/empty_dir.go
Expand Up @@ -41,7 +41,7 @@ var (
var _ = Describe("[sig-storage] EmptyDir volumes", func() {
f := framework.NewDefaultFramework("emptydir")

Context("when FSGroup is specified [Feature:FSGroup]", func() {
Context("when FSGroup is specified", func() {
It("new files should be created with FSGroup ownership when container is root", func() {
doTestSetgidFSGroup(f, testImageRootUid, v1.StorageMediumMemory)
})
Expand Down Expand Up @@ -252,6 +252,7 @@ func doTestSubPathFSGroup(f *framework.Framework, image string, medium v1.Storag
fmt.Sprintf("--fs_type=%v", volumePath),
fmt.Sprintf("--file_perm=%v", volumePath),
fmt.Sprintf("--file_owner=%v", volumePath),
fmt.Sprintf("--file_mode=%v", volumePath),
}

pod.Spec.Containers[0].VolumeMounts[0].SubPath = subPath
Expand All @@ -264,6 +265,7 @@ func doTestSubPathFSGroup(f *framework.Framework, image string, medium v1.Storag
"perms of file \"/test-volume\": -rwxrwxrwx",
"owner UID of \"/test-volume\": 0",
"owner GID of \"/test-volume\": 123",
"mode of file \"/test-volume\": dgtrwxrwxrwx",
}
if medium == v1.StorageMediumMemory {
out = append(out, "mount type of \"/test-volume\": tmpfs")
Expand Down

0 comments on commit 3b721ed

Please sign in to comment.