From f8a0ff77121a9d24ac3d6598a0992d1dc86ff16e Mon Sep 17 00:00:00 2001
From: Michelle Au <msau@google.com>
Date: Fri, 25 May 2018 16:30:27 -0700
Subject: [PATCH] Only mount subpath as readonly if specified in volumeMount

---
 mount.go            |  2 --
 mount_linux.go      |  4 ---
 mount_linux_test.go | 79 ---------------------------------------------
 3 files changed, 85 deletions(-)

diff --git a/mount.go b/mount.go
index 49351394..f5ce194c 100644
--- a/mount.go
+++ b/mount.go
@@ -136,8 +136,6 @@ type Subpath struct {
 	PodDir string
 	// Name of the container
 	ContainerName string
-	// True if the mount needs to be readonly
-	ReadOnly bool
 }
 
 // Exec executes command where mount utilities are. This can be either the host,
diff --git a/mount_linux.go b/mount_linux.go
index b02ca84f..e80dad59 100644
--- a/mount_linux.go
+++ b/mount_linux.go
@@ -884,10 +884,6 @@ func doBindSubPath(mounter Interface, subpath Subpath) (hostPath string, err err
 
 	// Do the bind mount
 	options := []string{"bind"}
-	if subpath.ReadOnly {
-		options = append(options, "ro")
-	}
-
 	glog.V(5).Infof("bind mounting %q at %q", mountSource, bindPathTarget)
 	if err = mounter.Mount(mountSource, bindPathTarget, "" /*fstype*/, options); err != nil {
 		return "", fmt.Errorf("error mounting %s: %s", subpath.Path, err)
diff --git a/mount_linux_test.go b/mount_linux_test.go
index 4c0cbd7b..0324bf88 100644
--- a/mount_linux_test.go
+++ b/mount_linux_test.go
@@ -1009,7 +1009,6 @@ func getTestPaths(base string) (string, string) {
 
 func TestBindSubPath(t *testing.T) {
 	defaultPerm := os.FileMode(0750)
-	readOnlyPerm := os.FileMode(0444)
 
 	tests := []struct {
 		name string
@@ -1017,7 +1016,6 @@ func TestBindSubPath(t *testing.T) {
 		// base.
 		prepare     func(base string) ([]string, string, string, error)
 		expectError bool
-		readOnly    bool
 	}{
 		{
 			name: "subpath-dir",
@@ -1214,55 +1212,6 @@ func TestBindSubPath(t *testing.T) {
 			},
 			expectError: false,
 		},
-		{
-			name: "subpath-dir-readonly",
-			prepare: func(base string) ([]string, string, string, error) {
-				volpath, _ := getTestPaths(base)
-				subpath := filepath.Join(volpath, "dir0")
-				return nil, volpath, subpath, os.MkdirAll(subpath, defaultPerm)
-			},
-			expectError: false,
-			readOnly:    true,
-		},
-		{
-			name: "subpath-file-readonly",
-			prepare: func(base string) ([]string, string, string, error) {
-				volpath, _ := getTestPaths(base)
-				subpath := filepath.Join(volpath, "file0")
-				if err := os.MkdirAll(volpath, defaultPerm); err != nil {
-					return nil, "", "", err
-				}
-				return nil, volpath, subpath, ioutil.WriteFile(subpath, []byte{}, defaultPerm)
-			},
-			expectError: false,
-			readOnly:    true,
-		},
-		{
-			name: "subpath-dir-and-volume-readonly",
-			prepare: func(base string) ([]string, string, string, error) {
-				volpath, _ := getTestPaths(base)
-				subpath := filepath.Join(volpath, "dir0")
-				if err := os.MkdirAll(subpath, defaultPerm); err != nil {
-					return nil, "", "", err
-				}
-				return nil, volpath, subpath, os.Chmod(subpath, readOnlyPerm)
-			},
-			expectError: false,
-			readOnly:    true,
-		},
-		{
-			name: "subpath-file-and-vol-readonly",
-			prepare: func(base string) ([]string, string, string, error) {
-				volpath, _ := getTestPaths(base)
-				subpath := filepath.Join(volpath, "file0")
-				if err := os.MkdirAll(volpath, defaultPerm); err != nil {
-					return nil, "", "", err
-				}
-				return nil, volpath, subpath, ioutil.WriteFile(subpath, []byte{}, readOnlyPerm)
-			},
-			expectError: false,
-			readOnly:    true,
-		},
 	}
 
 	for _, test := range tests {
@@ -1287,7 +1236,6 @@ func TestBindSubPath(t *testing.T) {
 			VolumePath:       volPath,
 			PodDir:           filepath.Join(base, "pod0"),
 			ContainerName:    testContainer,
-			ReadOnly:         test.readOnly,
 		}
 
 		_, subpathMount := getTestPaths(base)
@@ -1313,39 +1261,12 @@ func TestBindSubPath(t *testing.T) {
 			if err = validateFileExists(subpathMount); err != nil {
 				t.Errorf("test %q failed: %v", test.name, err)
 			}
-			if err = validateReadOnlyMount(test.readOnly, bindPathTarget, fm); err != nil {
-				t.Errorf("test %q failed: %v", test.name, err)
-			}
 		}
 
 		os.RemoveAll(base)
 	}
 }
 
-func validateReadOnlyMount(expectedReadOnly bool, bindPathTarget string, mounter *FakeMounter) error {
-	mps, err := mounter.List()
-	if err != nil {
-		return fmt.Errorf("fakeMounter.List() returned error: %v", err)
-	}
-	for _, mp := range mps {
-		if mp.Path == bindPathTarget {
-			foundReadOnly := false
-			for _, opts := range mp.Opts {
-				if opts == "ro" {
-					foundReadOnly = true
-					break
-				}
-			}
-			if expectedReadOnly != foundReadOnly {
-				return fmt.Errorf("expected readOnly %v, got %v for mount point %v", expectedReadOnly, foundReadOnly, bindPathTarget)
-			} else {
-				return nil
-			}
-		}
-	}
-	return fmt.Errorf("failed to find mountPoint %v", bindPathTarget)
-}
-
 func TestParseMountInfo(t *testing.T) {
 	info :=
 		`62 0 253:0 / / rw,relatime shared:1 - ext4 /dev/mapper/ssd-root rw,seclabel,data=ordered