Merge pull request #2709 from aodinokov/fixmount

Fixed incorrect docker mount arguments generation

kyaml/fn/runtime/container/container_test.go

@@ -73,14 +73,16 @@ metadata:
 				"--network", "none",
 				"--user", "nobody",
 				"--security-opt=no-new-privileges",
-				"--mount", fmt.Sprintf("type=%s,src=%s,dst=%s:ro", "bind", "/mount/path", "/local/"),
-				"--mount", fmt.Sprintf("type=%s,src=%s,dst=%s:ro", "volume", "myvol", "/local/"),
-				"--mount", fmt.Sprintf("type=%s,src=%s,dst=%s:ro", "tmpfs", "", "/local/"),
+				"--mount", fmt.Sprintf("type=%s,source=%s,target=%s,readonly", "bind", "/mount/path", "/local/"),
+				"--mount", fmt.Sprintf("type=%s,source=%s,target=%s", "bind", "/mount/pathrw", "/localrw/"),
+				"--mount", fmt.Sprintf("type=%s,source=%s,target=%s,readonly", "volume", "myvol", "/local/"),
+				"--mount", fmt.Sprintf("type=%s,source=%s,target=%s,readonly", "tmpfs", "", "/local/"),
 			},
 			instance: Filter{
 				Image: "example.com:version",
 				StorageMounts: []runtimeutil.StorageMount{
 					{MountType: "bind", Src: "/mount/path", DstPath: "/local/"},
+					{MountType: "bind", Src: "/mount/pathrw", DstPath: "/localrw/", ReadWriteMode: true},
 					{MountType: "volume", Src: "myvol", DstPath: "/local/"},
 					{MountType: "tmpfs", Src: "", DstPath: "/local/"},
 				},

kyaml/fn/runtime/runtimeutil/functiontypes.go

@@ -84,10 +84,18 @@ type StorageMount struct {
 
 	// The path where the file or directory is mounted in the container.
 	DstPath string `json:"dst,omitempty" yaml:"dst,omitempty"`
+
+	// Mount in ReadWrite mode if it's explicitly configured
+	// See https://docs.docker.com/storage/bind-mounts/#use-a-read-only-bind-mount
+	ReadWriteMode bool `json:"rw,omitempty" yaml:"rw,omitempty"`
 }
 
 func (s *StorageMount) String() string {
-	return fmt.Sprintf("type=%s,src=%s,dst=%s:ro", s.MountType, s.Src, s.DstPath)
+	mode := ""
+	if !s.ReadWriteMode {
+		mode = ",readonly"
+	}
+	return fmt.Sprintf("type=%s,source=%s,target=%s%s", s.MountType, s.Src, s.DstPath, mode)
 }
 
 // GetFunctionSpec returns the FunctionSpec for a resource.  Returns
@@ -149,17 +157,21 @@ func StringToStorageMount(s string) StorageMount {
 	options := strings.Split(s, ",")
 	for _, option := range options {
 		keyVal := strings.SplitN(option, "=", 2)
-		m[keyVal[0]] = keyVal[1]
+		if len(keyVal) == 2 {
+			m[keyVal[0]] = keyVal[1]
+		}
 	}
 	var sm StorageMount
 	for key, value := range m {
 		switch {
 		case key == "type":
 			sm.MountType = value
-		case key == "src":
+		case key == "src" || key == "source":
 			sm.Src = value
-		case key == "dst":
+		case key == "dst" || key == "target":
 			sm.DstPath = value
+		case key == "rw" && value == "true":
+			sm.ReadWriteMode = true
 		}
 	}
 	return sm

kyaml/fn/runtime/runtimeutil/runtimeutil_test.go

@@ -1379,3 +1379,52 @@ metadata:
 		assert.Equal(t, tc.required, fn.Container.Network.Required)
 	}
 }
+
+func Test_StringToStorageMount(t *testing.T) {
+	tests := []struct {
+		in          string
+		expectedOut string
+	}{
+		{
+			in:          "type=bind,src=/tmp/test/,dst=/tmp/source/",
+			expectedOut: "type=bind,source=/tmp/test/,target=/tmp/source/,readonly",
+		},
+		{
+			in:          "type=bind,src=/tmp/test/,dst=/tmp/source/,rw=true",
+			expectedOut: "type=bind,source=/tmp/test/,target=/tmp/source/",
+		},
+		{
+			in:          "type=bind,src=/tmp/test/,dst=/tmp/source/,rw=false",
+			expectedOut: "type=bind,source=/tmp/test/,target=/tmp/source/,readonly",
+		},
+		{
+			in:          "type=bind,src=/tmp/test/,dst=/tmp/source/,rw=",
+			expectedOut: "type=bind,source=/tmp/test/,target=/tmp/source/,readonly",
+		},
+		{
+			in:          "type=tmpfs,src=/tmp/test/,dst=/tmp/source/,rw=invalid",
+			expectedOut: "type=tmpfs,source=/tmp/test/,target=/tmp/source/,readonly",
+		},
+		{
+			in:          "type=tmpfs,src=/tmp/test/,dst=/tmp/source/,rwe=invalid",
+			expectedOut: "type=tmpfs,source=/tmp/test/,target=/tmp/source/,readonly",
+		},
+		{
+			in:          "type=tmpfs,src=/tmp/test/,dst",
+			expectedOut: "type=tmpfs,source=/tmp/test/,target=,readonly",
+		},
+		{
+			in:          "type=bind,source=/tmp/test/,target=/tmp/source/,rw=true",
+			expectedOut: "type=bind,source=/tmp/test/,target=/tmp/source/",
+		},
+		{
+			in:          "type=bind,source=/tmp/test/,target=/tmp/source/",
+			expectedOut: "type=bind,source=/tmp/test/,target=/tmp/source/,readonly",
+		},
+	}
+
+	for _, tc := range tests {
+		s := StringToStorageMount(tc.in)
+		assert.Equal(t, tc.expectedOut, (&s).String())
+	}
+}