[ARVADOS] created: 2.1.0-1907-g01877c7b5
Git user
git at public.arvados.org
Fri Feb 11 08:30:57 UTC 2022
at 01877c7b5119d29384490adceb862da30b4e81f5 (commit)
commit 01877c7b5119d29384490adceb862da30b4e81f5
Author: Tom Clegg <tom at curii.com>
Date: Fri Feb 11 03:29:25 2022 -0500
18690: Use bind mount to avoid writing secrets to output collection.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/crunchrun/crunchrun.go b/lib/crunchrun/crunchrun.go
index af0d49c80..4fa3f26ab 100644
--- a/lib/crunchrun/crunchrun.go
+++ b/lib/crunchrun/crunchrun.go
@@ -453,8 +453,8 @@ func (runner *ContainerRunner) SetupMounts() (map[string]bindmount, error) {
sort.Strings(binds)
for _, bind := range binds {
- mnt, ok := runner.Container.Mounts[bind]
- if !ok {
+ mnt, notSecret := runner.Container.Mounts[bind]
+ if !notSecret {
mnt = runner.SecretMounts[bind]
}
if bind == "stdout" || bind == "stderr" {
@@ -523,8 +523,7 @@ func (runner *ContainerRunner) SetupMounts() (map[string]bindmount, error) {
}
} else {
src = fmt.Sprintf("%s/tmp%d", runner.ArvMountPoint, tmpcount)
- arvMountCmd = append(arvMountCmd, "--mount-tmp")
- arvMountCmd = append(arvMountCmd, fmt.Sprintf("tmp%d", tmpcount))
+ arvMountCmd = append(arvMountCmd, "--mount-tmp", fmt.Sprintf("tmp%d", tmpcount))
tmpcount++
}
if mnt.Writable {
@@ -584,9 +583,32 @@ func (runner *ContainerRunner) SetupMounts() (map[string]bindmount, error) {
if err != nil {
return nil, fmt.Errorf("writing temp file: %v", err)
}
- if strings.HasPrefix(bind, runner.Container.OutputPath+"/") {
+ if strings.HasPrefix(bind, runner.Container.OutputPath+"/") && (notSecret || runner.Container.Mounts[runner.Container.OutputPath].Kind != "collection") {
+ // In most cases, if the container
+ // specifies a literal file inside the
+ // output path, we copy it into the
+ // output directory (either a mounted
+ // collection or a staging area on the
+ // host fs). If it's a secret, it will
+ // be skipped when copying output from
+ // staging to Keep later.
copyFiles = append(copyFiles, copyFile{tmpfn, runner.HostOutputDir + bind[len(runner.Container.OutputPath):]})
} else {
+ // If a secret is outside OutputPath,
+ // we bind mount the secret file
+ // directly just like other mounts. We
+ // also use this strategy when a
+ // secret is inside OutputPath but
+ // OutputPath is a live collection, to
+ // avoid writing the secret to
+ // Keep. Attempting to remove a
+ // bind-mounted secret file from
+ // inside the container will return a
+ // "Device or resource busy" error
+ // that might not be handled well by
+ // the container, which is why we
+ // don't use this strategy when
+ // OutputPath is a staging directory.
bindmounts[bind] = bindmount{HostPath: tmpfn, ReadOnly: true}
}
diff --git a/lib/crunchrun/crunchrun_test.go b/lib/crunchrun/crunchrun_test.go
index 2ec35f055..806a83eef 100644
--- a/lib/crunchrun/crunchrun_test.go
+++ b/lib/crunchrun/crunchrun_test.go
@@ -44,6 +44,7 @@ type TestSuite struct {
runner *ContainerRunner
executor *stubExecutor
keepmount string
+ keepmountTmp []string
testDispatcherKeepClient KeepTestClient
testContainerKeepClient KeepTestClient
}
@@ -62,10 +63,20 @@ func (s *TestSuite) SetUpTest(c *C) {
}
s.runner.RunArvMount = func(cmd []string, tok string) (*exec.Cmd, error) {
s.runner.ArvMountPoint = s.keepmount
+ for i, opt := range cmd {
+ if opt == "--mount-tmp" {
+ err := os.Mkdir(s.keepmount+"/"+cmd[i+1], 0700)
+ if err != nil {
+ return nil, err
+ }
+ s.keepmountTmp = append(s.keepmountTmp, cmd[i+1])
+ }
+ }
return nil, nil
}
s.keepmount = c.MkDir()
err = os.Mkdir(s.keepmount+"/by_id", 0755)
+ s.keepmountTmp = nil
c.Assert(err, IsNil)
err = os.Mkdir(s.keepmount+"/by_id/"+arvadostest.DockerImage112PDH, 0755)
c.Assert(err, IsNil)
@@ -638,8 +649,6 @@ func (s *TestSuite) fullRunHelper(c *C, record string, extraMounts []string, exi
s.runner.statInterval = 100 * time.Millisecond
s.runner.containerWatchdogInterval = time.Second
- am := &ArvMountCmdLine{}
- s.runner.RunArvMount = am.ArvMountTest
realTemp := c.MkDir()
tempcount := 0
@@ -2007,6 +2016,44 @@ func (s *TestSuite) TestSecretTextMountPoint(c *C) {
c.Check(s.api.CalledWith("container.state", "Complete"), NotNil)
c.Check(s.runner.ContainerArvClient.(*ArvTestClient).CalledWith("collection.manifest_text", ". 34819d7beeabb9260a5c854bc85b3e44+10 0:10:secret.conf\n"), IsNil)
c.Check(s.runner.ContainerArvClient.(*ArvTestClient).CalledWith("collection.manifest_text", ""), NotNil)
+
+ // under secret mounts, output dir is a collection, not captured in output
+ helperRecord = `{
+ "command": ["true"],
+ "container_image": "` + arvadostest.DockerImage112PDH + `",
+ "cwd": "/bin",
+ "mounts": {
+ "/tmp": {"kind": "collection", "writable": true}
+ },
+ "secret_mounts": {
+ "/tmp/secret.conf": {"kind": "text", "content": "mypassword"}
+ },
+ "output_path": "/tmp",
+ "priority": 1,
+ "runtime_constraints": {},
+ "state": "Locked"
+ }`
+
+ s.SetUpTest(c)
+ _, _, realtemp := s.fullRunHelper(c, helperRecord, nil, 0, func() {
+ // secret.conf should be provisioned as a separate
+ // bind mount, i.e., it should not appear in the
+ // (fake) fuse filesystem as viewed from the host.
+ content, err := ioutil.ReadFile(s.runner.HostOutputDir + "/secret.conf")
+ if !c.Check(errors.Is(err, os.ErrNotExist), Equals, true) {
+ c.Logf("secret.conf: content %q, err %#v", content, err)
+ }
+ err = ioutil.WriteFile(s.runner.HostOutputDir+"/.arvados#collection", []byte(`{"manifest_text":". acbd18db4cc2f85cedef654fccc4a4d8+3 0:3:foo.txt\n"}`), 0700)
+ c.Check(err, IsNil)
+ })
+
+ content, err := ioutil.ReadFile(realtemp + "/text1/mountdata.text")
+ c.Check(err, IsNil)
+ c.Check(string(content), Equals, "mypassword")
+ c.Check(s.executor.created.BindMounts["/tmp/secret.conf"], DeepEquals, bindmount{realtemp + "/text1/mountdata.text", true})
+ c.Check(s.api.CalledWith("container.exit_code", 0), NotNil)
+ c.Check(s.api.CalledWith("container.state", "Complete"), NotNil)
+ c.Check(s.runner.ContainerArvClient.(*ArvTestClient).CalledWith("collection.manifest_text", ". acbd18db4cc2f85cedef654fccc4a4d8+3 0:3:foo.txt\n"), NotNil)
}
type FakeProcess struct {
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list