Fixed incorrect docker mount arguments generation

The previous implementation combined --mount and -v notation
of argument [1] adding :ro to make the read-only mount point.
E.g. the command [2] called docker with the following
params: [3]. As a result instead of the read-only
folder /tmp/source, the read-write folder /tmp/source/:ro/'
is created.

This PR:
1. substitutes ':ro' with correct ',readonly'.
2. changes 'src=' and 'dst=' with 'source=' and 'target=' as
   it is stated in the documentation [1]
3. introduces the ability to EXPLICITLY create a mountpoint
   with read-write access. To do so it's necessary to add
   ',rw=true' to the --mount argument
4. corrects UTs adds some additional coverage for added
   functionality

[1]
https://docs.docker.com/storage/bind-mounts/#use-a-read-only-bind-mount

[2]
kustomize fn run ./d --mount type=bind,src=$(pwd)/test/,dst=/tmp/source/

[3]
--mount type=bind,src=/home/ubuntu/kpt-functions-catalog/functions/ts/test/,dst=/tmp/source/:ro
This commit is contained in:
Alexey Odinokov
2020-07-11 05:21:42 +00:00
parent 556eb48651
commit 63f9f79fc0
3 changed files with 60 additions and 5 deletions

View File

@@ -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/"},
},

View File

@@ -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,7 +157,9 @@ 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 {
@@ -160,6 +170,8 @@ func StringToStorageMount(s string) StorageMount {
sm.Src = value
case key == "dst":
sm.DstPath = value
case key == "rw" && value == "true":
sm.ReadWriteMode = true
}
}
return sm

View File

@@ -1379,3 +1379,44 @@ 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",
},
}
for _, tc := range tests {
s := StringToStorageMount(tc.in)
assert.Equal(t, tc.expectedOut, (&s).String())
}
}