Merge branch '17749-s3-prefixes'
authorTom Clegg <tom@curii.com>
Thu, 23 Sep 2021 13:54:26 +0000 (09:54 -0400)
committerTom Clegg <tom@curii.com>
Thu, 23 Sep 2021 13:54:26 +0000 (09:54 -0400)
closes #17749

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

16 files changed:
doc/api/methods/containers.html.textile.liquid
doc/api/methods/links.html.textile.liquid
doc/architecture/keep-data-lifecycle.html.textile.liquid
lib/crunchrun/crunchrun.go
lib/crunchrun/crunchrun_test.go
lib/crunchrun/docker.go
lib/crunchrun/executor.go
lib/crunchrun/logscanner.go [new file with mode: 0644]
lib/crunchrun/logscanner_test.go [new file with mode: 0644]
lib/crunchrun/singularity.go
sdk/go/arvados/blob_signature.go
sdk/go/arvados/blob_signature_test.go
sdk/python/setup.py
services/api/app/controllers/arvados/v1/links_controller.rb
services/api/app/models/arvados_model.rb
services/api/test/integration/permissions_test.rb

index 7da05cbd0b6812b68a136212a50b060fe1920476..18fb4f01330e033f06086f3125d8d01680955ee3 100644 (file)
@@ -88,8 +88,8 @@ table(table table-bordered table-condensed).
 |error|string|The existance of this key indicates the container will definitely fail, or has already failed.|Optional.|
 |warning|string|Indicates something unusual happened or is currently happening, but isn't considered fatal.|Optional.|
 |activity|string|A message for the end user about what state the container is currently in.|Optional.|
-|errorDetails|string|Additional structured error details.|Optional.|
-|warningDetails|string|Additional structured warning details.|Optional.|
+|errorDetail|string|Additional structured error details.|Optional.|
+|warningDetail|string|Additional structured warning details.|Optional.|
 
 h2(#scheduling_parameters). {% include 'container_scheduling_parameters' %}
 
index c71105c74569a88b26c594b32482a696e1ab0165..eceea296da72a17313567a38f4c6616be4a8aa81 100644 (file)
@@ -143,3 +143,13 @@ table(table table-bordered table-condensed).
 |_. Argument |_. Type |_. Description |_. Location |_. Example |
 {background:#ccffcc}.|uuid|string|The UUID of the Link in question.|path||
 |link|object||query||
+
+h3. get_permissions
+
+Get all permission links that point directly to given UUID (in the head_uuid field).  The requesting user must have @can_manage@ permission or be an admin.
+
+Arguments:
+
+table(table table-bordered table-condensed).
+|_. Argument |_. Type |_. Description |_. Location |_. Example |
+{background:#ccffcc}.|uuid|string|The UUID of the object.|path||
index a2e31fb0eb62ee2236a54154d1c9c511b0f833c4..e3253277cba658e289db45e8a86b95e1795c3a2e 100644 (file)
@@ -36,7 +36,7 @@ table(table table-bordered table-condensed).
 |_. collection state|_. is_trashed|_. trash_at|_. delete_at|_. get|_. list|_. list?include_trash=true|_. can be modified|
 |persisted collection|false |null |null |yes |yes |yes |yes |
 |expiring collection|false |future |future |yes  |yes |yes |yes |
-|trashed collection|true |past |future |no |no |yes |only is_trashed, trash_at and delete_at attribtues|
+|trashed collection|true |past |future |no |no |yes |only is_trashed, trash_at and delete_at attributes|
 |deleted collection|true|past |past |no |no |no |no |
 
 h2(#block_lifecycle). Block lifecycle
index 01141674a6b329fc6ef4f0cb2feb6b60628e96e8..42f143f1cb8fe6bb97bfff2511e9f318f37359af 100644 (file)
@@ -66,7 +66,7 @@ type IKeepClient interface {
 // NewLogWriter is a factory function to create a new log writer.
 type NewLogWriter func(name string) (io.WriteCloser, error)
 
-type RunArvMount func(args []string, tok string) (*exec.Cmd, error)
+type RunArvMount func(cmdline []string, tok string) (*exec.Cmd, error)
 
 type MkTempDir func(string, string) (string, error)
 
@@ -273,8 +273,8 @@ func (runner *ContainerRunner) LoadImage() (string, error) {
        return imageID, nil
 }
 
-func (runner *ContainerRunner) ArvMountCmd(arvMountCmd []string, token string) (c *exec.Cmd, err error) {
-       c = exec.Command("arv-mount", arvMountCmd...)
+func (runner *ContainerRunner) ArvMountCmd(cmdline []string, token string) (c *exec.Cmd, err error) {
+       c = exec.Command(cmdline[0], cmdline[1:]...)
 
        // Copy our environment, but override ARVADOS_API_TOKEN with
        // the container auth token.
@@ -291,8 +291,16 @@ func (runner *ContainerRunner) ArvMountCmd(arvMountCmd []string, token string) (
                return nil, err
        }
        runner.arvMountLog = NewThrottledLogger(w)
+       scanner := logScanner{
+               Patterns: []string{
+                       "Keep write error",
+                       "Block not found error",
+                       "Unhandled exception during FUSE operation",
+               },
+               ReportFunc: runner.reportArvMountWarning,
+       }
        c.Stdout = runner.arvMountLog
-       c.Stderr = io.MultiWriter(runner.arvMountLog, os.Stderr)
+       c.Stderr = io.MultiWriter(runner.arvMountLog, os.Stderr, &scanner)
 
        runner.CrunchLog.Printf("Running %v", c.Args)
 
@@ -392,6 +400,7 @@ func (runner *ContainerRunner) SetupMounts() (map[string]bindmount, error) {
        pdhOnly := true
        tmpcount := 0
        arvMountCmd := []string{
+               "arv-mount",
                "--foreground",
                "--allow-other",
                "--read-write",
@@ -1100,6 +1109,21 @@ func (runner *ContainerRunner) updateLogs() {
        }
 }
 
+func (runner *ContainerRunner) reportArvMountWarning(pattern, text string) {
+       var updated arvados.Container
+       err := runner.DispatcherArvClient.Update("containers", runner.Container.UUID, arvadosclient.Dict{
+               "container": arvadosclient.Dict{
+                       "runtime_status": arvadosclient.Dict{
+                               "warning":       "arv-mount: " + pattern,
+                               "warningDetail": text,
+                       },
+               },
+       }, &updated)
+       if err != nil {
+               runner.CrunchLog.Printf("error updating container runtime_status: %s", err)
+       }
+}
+
 // CaptureOutput saves data from the container's output directory if
 // needed, and updates the container output accordingly.
 func (runner *ContainerRunner) CaptureOutput(bindmounts map[string]bindmount) error {
@@ -1377,7 +1401,7 @@ func (runner *ContainerRunner) NewArvLogWriter(name string) (io.WriteCloser, err
 // Run the full container lifecycle.
 func (runner *ContainerRunner) Run() (err error) {
        runner.CrunchLog.Printf("crunch-run %s started", cmd.Version.String())
-       runner.CrunchLog.Printf("Executing container '%s'", runner.Container.UUID)
+       runner.CrunchLog.Printf("Executing container '%s' using %s runtime", runner.Container.UUID, runner.executor.Runtime())
 
        hostname, hosterr := os.Hostname()
        if hosterr != nil {
index bb982cdee76c32cb9321ce88e8fa47fa0588f2f1..1131982de04c87f4b7327d6d7dc911e3176cb30b 100644 (file)
@@ -117,6 +117,7 @@ func (e *stubExecutor) LoadImage(imageId string, tarball string, container arvad
        e.loaded = tarball
        return e.loadErr
 }
+func (e *stubExecutor) Runtime() string                 { return "stub" }
 func (e *stubExecutor) Create(spec containerSpec) error { e.created = spec; return e.createErr }
 func (e *stubExecutor) Start() error                    { e.exit = make(chan int, 1); go e.runFunc(); return e.startErr }
 func (e *stubExecutor) CgroupID() string                { return "cgroupid" }
@@ -849,6 +850,26 @@ func (s *TestSuite) TestNodeInfoLog(c *C) {
        c.Check(json, Matches, `(?ms).*Disk INodes.*`)
 }
 
+func (s *TestSuite) TestLogVersionAndRuntime(c *C) {
+       s.fullRunHelper(c, `{
+               "command": ["sleep", "1"],
+               "container_image": "`+arvadostest.DockerImage112PDH+`",
+               "cwd": ".",
+               "environment": {},
+               "mounts": {"/tmp": {"kind": "tmp"} },
+               "output_path": "/tmp",
+               "priority": 1,
+               "runtime_constraints": {},
+               "state": "Locked"
+       }`, nil, 0,
+               func() {
+               })
+
+       c.Assert(s.api.Logs["crunch-run"], NotNil)
+       c.Check(s.api.Logs["crunch-run"].String(), Matches, `(?ms).*crunch-run \S+ \(go\S+\) start.*`)
+       c.Check(s.api.Logs["crunch-run"].String(), Matches, `(?ms).*Executing container 'zzzzz-zzzzz-zzzzzzzzzzzzzzz' using stub runtime.*`)
+}
+
 func (s *TestSuite) TestContainerRecordLog(c *C) {
        s.fullRunHelper(c, `{
                "command": ["sleep", "1"],
@@ -1103,7 +1124,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
                cr.statInterval = 5 * time.Second
                bindmounts, err := cr.SetupMounts()
                c.Check(err, IsNil)
-               c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other",
+               c.Check(am.Cmd, DeepEquals, []string{"arv-mount", "--foreground", "--allow-other",
                        "--read-write", "--storage-classes", "default", "--crunchstat-interval=5",
                        "--mount-by-pdh", "by_id", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
                c.Check(bindmounts, DeepEquals, map[string]bindmount{"/tmp": {realTemp + "/tmp2", false}})
@@ -1123,7 +1144,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
 
                bindmounts, err := cr.SetupMounts()
                c.Check(err, IsNil)
-               c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other",
+               c.Check(am.Cmd, DeepEquals, []string{"arv-mount", "--foreground", "--allow-other",
                        "--read-write", "--storage-classes", "foo,bar", "--crunchstat-interval=5",
                        "--mount-by-pdh", "by_id", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
                c.Check(bindmounts, DeepEquals, map[string]bindmount{"/out": {realTemp + "/tmp2", false}, "/tmp": {realTemp + "/tmp3", false}})
@@ -1143,7 +1164,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
 
                bindmounts, err := cr.SetupMounts()
                c.Check(err, IsNil)
-               c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other",
+               c.Check(am.Cmd, DeepEquals, []string{"arv-mount", "--foreground", "--allow-other",
                        "--read-write", "--storage-classes", "default", "--crunchstat-interval=5",
                        "--mount-by-pdh", "by_id", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
                c.Check(bindmounts, DeepEquals, map[string]bindmount{"/tmp": {realTemp + "/tmp2", false}, "/etc/arvados/ca-certificates.crt": {stubCertPath, true}})
@@ -1166,7 +1187,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
 
                bindmounts, err := cr.SetupMounts()
                c.Check(err, IsNil)
-               c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other",
+               c.Check(am.Cmd, DeepEquals, []string{"arv-mount", "--foreground", "--allow-other",
                        "--read-write", "--storage-classes", "default", "--crunchstat-interval=5",
                        "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
                c.Check(bindmounts, DeepEquals, map[string]bindmount{"/keeptmp": {realTemp + "/keep1/tmp0", false}})
@@ -1189,7 +1210,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
 
                bindmounts, err := cr.SetupMounts()
                c.Check(err, IsNil)
-               c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other",
+               c.Check(am.Cmd, DeepEquals, []string{"arv-mount", "--foreground", "--allow-other",
                        "--read-write", "--storage-classes", "default", "--crunchstat-interval=5",
                        "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
                c.Check(bindmounts, DeepEquals, map[string]bindmount{
@@ -1216,7 +1237,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
 
                bindmounts, err := cr.SetupMounts()
                c.Check(err, IsNil)
-               c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other",
+               c.Check(am.Cmd, DeepEquals, []string{"arv-mount", "--foreground", "--allow-other",
                        "--read-write", "--storage-classes", "default", "--crunchstat-interval=5",
                        "--file-cache", "512", "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
                c.Check(bindmounts, DeepEquals, map[string]bindmount{
@@ -1299,7 +1320,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
 
                bindmounts, err := cr.SetupMounts()
                c.Check(err, IsNil)
-               c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other",
+               c.Check(am.Cmd, DeepEquals, []string{"arv-mount", "--foreground", "--allow-other",
                        "--read-write", "--storage-classes", "default", "--crunchstat-interval=5",
                        "--file-cache", "512", "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", "--mount-by-id", "by_uuid", realTemp + "/keep1"})
                c.Check(bindmounts, DeepEquals, map[string]bindmount{
@@ -1533,6 +1554,37 @@ func (s *TestSuite) TestFullRunSetOutput(c *C) {
        c.Check(s.api.CalledWith("container.output", arvadostest.DockerImage112PDH), NotNil)
 }
 
+func (s *TestSuite) TestArvMountRuntimeStatusWarning(c *C) {
+       s.runner.RunArvMount = func([]string, string) (*exec.Cmd, error) {
+               os.Mkdir(s.runner.ArvMountPoint+"/by_id", 0666)
+               ioutil.WriteFile(s.runner.ArvMountPoint+"/by_id/README", nil, 0666)
+               return s.runner.ArvMountCmd([]string{"bash", "-c", "echo >&2 Test: Keep write error: I am a teapot; sleep 3"}, "")
+       }
+       s.executor.runFunc = func() {
+               time.Sleep(time.Second)
+               s.executor.exit <- 0
+       }
+       record := `{
+    "command": ["sleep", "1"],
+    "container_image": "` + arvadostest.DockerImage112PDH + `",
+    "cwd": "/bin",
+    "environment": {},
+    "mounts": {"/tmp": {"kind": "tmp"} },
+    "output_path": "/tmp",
+    "priority": 1,
+    "runtime_constraints": {"API": true},
+    "state": "Locked"
+}`
+       err := json.Unmarshal([]byte(record), &s.api.Container)
+       c.Assert(err, IsNil)
+       err = s.runner.Run()
+       c.Assert(err, IsNil)
+       c.Check(s.api.CalledWith("container.exit_code", 0), NotNil)
+       c.Check(s.api.CalledWith("container.runtime_status.warning", "arv-mount: Keep write error"), NotNil)
+       c.Check(s.api.CalledWith("container.runtime_status.warningDetail", "Test: Keep write error: I am a teapot"), NotNil)
+       c.Check(s.api.CalledWith("container.state", "Complete"), NotNil)
+}
+
 func (s *TestSuite) TestStdoutWithExcludeFromOutputMountPointUnderOutputDir(c *C) {
        helperRecord := `{
                "command": ["/bin/sh", "-c", "echo $FROBIZ"],
index 656061b77ec552a811c26dfe18be870b154c1b1e..07f79bbcc2d11f0239a6231288a94d84a89f87fb 100644 (file)
@@ -46,6 +46,8 @@ func newDockerExecutor(containerUUID string, logf func(string, ...interface{}),
        }, err
 }
 
+func (e *dockerExecutor) Runtime() string { return "docker" }
+
 func (e *dockerExecutor) LoadImage(imageID string, imageTarballPath string, container arvados.Container, arvMountPoint string,
        containerClient *arvados.Client) error {
        _, _, err := e.dockerclient.ImageInspectWithRaw(context.TODO(), imageID)
index 65bf7427b9601c465fb21d811c5cb79d2d41a0f8..b7c341f3186b1af319780d19777d892be082a1cc 100644 (file)
@@ -58,4 +58,7 @@ type containerExecutor interface {
 
        // Release resources (temp dirs, stopped containers)
        Close()
+
+       // Name of runtime engine ("docker", "singularity")
+       Runtime() string
 }
diff --git a/lib/crunchrun/logscanner.go b/lib/crunchrun/logscanner.go
new file mode 100644 (file)
index 0000000..aa0a834
--- /dev/null
@@ -0,0 +1,53 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package crunchrun
+
+import (
+       "bytes"
+       "strings"
+)
+
+// logScanner is an io.Writer that calls ReportFunc(pattern) the first
+// time one of the Patterns appears in the data. Patterns must not
+// contain newlines.
+type logScanner struct {
+       Patterns   []string
+       ReportFunc func(pattern, text string)
+       reported   bool
+       buf        bytes.Buffer
+}
+
+func (s *logScanner) Write(p []byte) (int, error) {
+       if s.reported {
+               // We only call reportFunc once. Once we've called it
+               // there's no need to buffer/search subsequent writes.
+               return len(p), nil
+       }
+       split := bytes.LastIndexByte(p, '\n')
+       if split < 0 {
+               return s.buf.Write(p)
+       }
+       s.buf.Write(p[:split+1])
+       txt := s.buf.String()
+       for _, pattern := range s.Patterns {
+               if found := strings.Index(txt, pattern); found >= 0 {
+                       // Report the entire line where the pattern
+                       // was found.
+                       txt = txt[strings.LastIndexByte(txt[:found], '\n')+1:]
+                       if end := strings.IndexByte(txt, '\n'); end >= 0 {
+                               txt = txt[:end]
+                       }
+                       s.ReportFunc(pattern, txt)
+                       s.reported = true
+                       return len(p), nil
+               }
+       }
+       s.buf.Reset()
+       if split == len(p) {
+               return len(p), nil
+       }
+       n, err := s.buf.Write(p[split+1:])
+       return n + split + 1, err
+}
diff --git a/lib/crunchrun/logscanner_test.go b/lib/crunchrun/logscanner_test.go
new file mode 100644 (file)
index 0000000..f6b4ba3
--- /dev/null
@@ -0,0 +1,56 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package crunchrun
+
+import (
+       check "gopkg.in/check.v1"
+)
+
+var _ = check.Suite(&logScannerSuite{})
+
+type logScannerSuite struct {
+}
+
+func (s *logScannerSuite) TestCallReportFuncOnce(c *check.C) {
+       var reported []string
+       ls := logScanner{
+               Patterns: []string{"foobar", "barbaz"},
+               ReportFunc: func(pattern, detail string) {
+                       reported = append(reported, pattern, detail)
+               },
+       }
+       ls.Write([]byte("foo\nbar\n2021-01-01T00:00:00.000Z: bar"))
+       ls.Write([]byte("baz: it's a detail\nwaz\nqux"))
+       ls.Write([]byte("\nfoobar\n"))
+       c.Check(reported, check.DeepEquals, []string{"barbaz", "2021-01-01T00:00:00.000Z: barbaz: it's a detail"})
+}
+
+func (s *logScannerSuite) TestOneWritePerLine(c *check.C) {
+       var reported []string
+       ls := logScanner{
+               Patterns: []string{"barbaz"},
+               ReportFunc: func(pattern, detail string) {
+                       reported = append(reported, pattern, detail)
+               },
+       }
+       ls.Write([]byte("foo\n"))
+       ls.Write([]byte("2021-01-01T00:00:00.000Z: barbaz: it's a detail\n"))
+       ls.Write([]byte("waz\n"))
+       c.Check(reported, check.DeepEquals, []string{"barbaz", "2021-01-01T00:00:00.000Z: barbaz: it's a detail"})
+}
+
+func (s *logScannerSuite) TestNoDetail(c *check.C) {
+       var reported []string
+       ls := logScanner{
+               Patterns: []string{"barbaz"},
+               ReportFunc: func(pattern, detail string) {
+                       reported = append(reported, pattern, detail)
+               },
+       }
+       ls.Write([]byte("foo\n"))
+       ls.Write([]byte("barbaz\n"))
+       ls.Write([]byte("waz\n"))
+       c.Check(reported, check.DeepEquals, []string{"barbaz", "barbaz"})
+}
index 61fecad0a13c06664890a9cf2dfffb8346b7a47e..99c5cef95c169155d9673b5e59c90f7f6d81e5a6 100644 (file)
@@ -36,6 +36,8 @@ func newSingularityExecutor(logf func(string, ...interface{})) (*singularityExec
        }, nil
 }
 
+func (e *singularityExecutor) Runtime() string { return "singularity" }
+
 func (e *singularityExecutor) getOrCreateProject(ownerUuid string, name string, containerClient *arvados.Client) (*arvados.Group, error) {
        var gp arvados.GroupList
        err := containerClient.RequestAndDecode(&gp,
index 2202016bcc6b8a607c7f7d8241c80166247b87b4..47b31a18e893d0a848f39d0b9a16e5e736c84c71 100644 (file)
@@ -9,13 +9,13 @@
 package arvados
 
 import (
+       "bytes"
        "crypto/hmac"
        "crypto/sha1"
        "errors"
        "fmt"
        "regexp"
        "strconv"
-       "strings"
        "time"
 )
 
@@ -33,9 +33,9 @@ var (
 
 // makePermSignature generates a SHA-1 HMAC digest for the given blob,
 // token, expiry, and site secret.
-func makePermSignature(blobHash, apiToken, expiry, blobSignatureTTL string, permissionSecret []byte) string {
+func makePermSignature(blobHash []byte, apiToken, expiry, blobSignatureTTL string, permissionSecret []byte) string {
        hmac := hmac.New(sha1.New, permissionSecret)
-       hmac.Write([]byte(blobHash))
+       hmac.Write(blobHash)
        hmac.Write([]byte("@"))
        hmac.Write([]byte(apiToken))
        hmac.Write([]byte("@"))
@@ -73,7 +73,10 @@ func SignLocator(blobLocator, apiToken string, expiry time.Time, blobSignatureTT
                return blobLocator
        }
        // Strip off all hints: only the hash is used to sign.
-       blobHash := strings.Split(blobLocator, "+")[0]
+       blobHash := []byte(blobLocator)
+       if hints := bytes.IndexRune(blobHash, '+'); hints > 0 {
+               blobHash = blobHash[:hints]
+       }
        timestampHex := fmt.Sprintf("%08x", expiry.Unix())
        blobSignatureTTLHex := strconv.FormatInt(int64(blobSignatureTTL.Seconds()), 16)
        return blobLocator +
@@ -100,7 +103,7 @@ func VerifySignature(signedLocator, apiToken string, blobSignatureTTL time.Durat
        if matches == nil {
                return ErrSignatureMissing
        }
-       blobHash := matches[1]
+       blobHash := []byte(matches[1])
        signatureHex := matches[6]
        expiryHex := matches[7]
        if expiryTime, err := parseHexTimestamp(expiryHex); err != nil {
index 847f9a8ae2ef08f5fbee1548af3528fc2d9f90d7..d23a18ac74ba5b976812e72fcb919b5e12345f3f 100644 (file)
@@ -32,6 +32,17 @@ var _ = check.Suite(&BlobSignatureSuite{})
 
 type BlobSignatureSuite struct{}
 
+func (s *BlobSignatureSuite) BenchmarkSignManifest(c *check.C) {
+       DebugLocksPanicMode = false
+       ts, err := parseHexTimestamp(knownTimestamp)
+       c.Check(err, check.IsNil)
+       c.Logf("test manifest is %d bytes", len(bigmanifest))
+       for i := 0; i < c.N; i++ {
+               m := SignManifest(bigmanifest, knownToken, ts, blobSignatureTTL, []byte(knownKey))
+               c.Check(m, check.Not(check.Equals), "")
+       }
+}
+
 func (s *BlobSignatureSuite) TestSignLocator(c *check.C) {
        ts, err := parseHexTimestamp(knownTimestamp)
        c.Check(err, check.IsNil)
index 311a139906aa4cb9d0a3d28b178767324a49837f..8d637303b466f90712b9ef4b44846277d5af6ed0 100644 (file)
@@ -49,9 +49,10 @@ setup(name='arvados-python-client',
           'ciso8601 >=2.0.0',
           'future',
           'google-api-python-client >=1.6.2, <2',
+          'google-auth<2',
           'httplib2 >=0.9.2',
           'pycurl >=7.19.5.1',
-          'ruamel.yaml >=0.15.54, <=0.17.11',
+          'ruamel.yaml >=0.15.54, <0.17.11',
           'setuptools',
           'ws4py >=0.4.2',
       ],
index f54c4a9a519c563ca8fc08e9bb480b254e616608..384ffd64b7080190c21a4ec46f2ab7ea31d97c83 100644 (file)
@@ -47,17 +47,6 @@ class Arvados::V1::LinksController < ApplicationController
         .first
     else
       super
-      if @object.nil?
-        # Normally group permission links are not readable_by users.
-        # Make an exception for users with permission to manage the group.
-        # FIXME: Solve this more generally - see the controller tests.
-        link = Link.find_by_uuid(params[:uuid])
-        if (not link.nil?) and
-            (link.link_class == "permission") and
-            (@read_users.any? { |u| u.can?(manage: link.head_uuid) })
-          @object = link
-        end
-      end
     end
   end
 
index f2bae3a4b5b2c79a9a3aacc882b7e570d713bc92..af226cde821399a0d115fdddfc698ec57b39ace9 100644 (file)
@@ -386,11 +386,17 @@ class ArvadosModel < ApplicationRecord
 
       links_cond = ""
       if sql_table == "links"
-        # Match any permission link that gives one of the authorized
-        # users some permission _or_ gives anyone else permission to
-        # view one of the authorized users.
+        # 1) Match permission links incoming or outgoing on the
+        # user, i.e. granting permission on the user, or granting
+        # permission to the user.
+        #
+        # 2) Match permission links which grant permission on an
+        # object that this user can_manage.
+        #
         links_cond = "OR (#{sql_table}.link_class IN (:permission_link_classes) AND "+
-                       "(#{sql_table}.head_uuid IN (#{user_uuids_subquery}) OR #{sql_table}.tail_uuid IN (#{user_uuids_subquery})))"
+                     "   ((#{sql_table}.head_uuid IN (#{user_uuids_subquery}) OR #{sql_table}.tail_uuid IN (#{user_uuids_subquery})) OR " +
+                     "    #{sql_table}.head_uuid IN (SELECT target_uuid FROM #{PERMISSION_VIEW} "+
+                     "    WHERE user_uuid IN (#{user_uuids_subquery}) AND perm_level >= 3))) "
       end
 
       sql_conds = "(#{owner_check} #{direct_check} #{links_cond}) #{trashed_check.empty? ? "" : "AND"} #{trashed_check}"
@@ -408,7 +414,7 @@ class ArvadosModel < ApplicationRecord
 
     self.where(sql_conds,
                user_uuids: all_user_uuids.collect{|c| c["target_uuid"]},
-               permission_link_classes: ['permission', 'resources'])
+               permission_link_classes: ['permission'])
   end
 
   def save_with_unique_name!
index 66a62543bb4364590019d42697bae5220e75fc93..9eae518c1d0f7ffd0700fc110de375754713fdc1 100644 (file)
@@ -347,8 +347,16 @@ class PermissionsTest < ActionDispatch::IntegrationTest
       headers: auth(:active)
     assert_response 404
 
-    # add some permissions, including can_manage
-    # permission for user :active
+    get "/arvados/v1/links",
+        params: {
+          :filters => [["link_class", "=", "permission"], ["head_uuid", "=", groups(:public).uuid]].to_json
+        },
+      headers: auth(:active)
+    assert_response :success
+    assert_equal [], json_response['items']
+
+    ### add some permissions, including can_manage
+    ### permission for user :active
     post "/arvados/v1/links",
       params: {
         :format => :json,
@@ -379,6 +387,27 @@ class PermissionsTest < ActionDispatch::IntegrationTest
     assert_response :success
     can_write_uuid = json_response['uuid']
 
+    # Still should not be able read these permission links
+    get "/arvados/v1/permissions/#{groups(:public).uuid}",
+      params: nil,
+      headers: auth(:active)
+    assert_response 404
+
+    get "/arvados/v1/links",
+        params: {
+          :filters => [["link_class", "=", "permission"], ["head_uuid", "=", groups(:public).uuid]].to_json
+        },
+      headers: auth(:active)
+    assert_response :success
+    assert_equal [], json_response['items']
+
+    # Shouldn't be able to read links directly either
+    get "/arvados/v1/links/#{can_read_uuid}",
+        params: {},
+      headers: auth(:active)
+    assert_response 404
+
+    ### Now add a can_manage link
     post "/arvados/v1/links",
       params: {
         :format => :json,
@@ -394,8 +423,8 @@ class PermissionsTest < ActionDispatch::IntegrationTest
     assert_response :success
     can_manage_uuid = json_response['uuid']
 
-    # Now user :active should be able to retrieve permissions
-    # on group :public.
+    # user :active should be able to retrieve permissions
+    # on group :public using get_permissions
     get("/arvados/v1/permissions/#{groups(:public).uuid}",
       params: { :format => :json },
       headers: auth(:active))
@@ -405,6 +434,52 @@ class PermissionsTest < ActionDispatch::IntegrationTest
     assert_includes perm_uuids, can_read_uuid, "can_read_uuid not found"
     assert_includes perm_uuids, can_write_uuid, "can_write_uuid not found"
     assert_includes perm_uuids, can_manage_uuid, "can_manage_uuid not found"
+
+    # user :active should be able to retrieve permissions
+    # on group :public using link list
+    get "/arvados/v1/links",
+        params: {
+          :filters => [["link_class", "=", "permission"], ["head_uuid", "=", groups(:public).uuid]].to_json
+        },
+      headers: auth(:active)
+    assert_response :success
+
+    perm_uuids = json_response['items'].map { |item| item['uuid'] }
+    assert_includes perm_uuids, can_read_uuid, "can_read_uuid not found"
+    assert_includes perm_uuids, can_write_uuid, "can_write_uuid not found"
+    assert_includes perm_uuids, can_manage_uuid, "can_manage_uuid not found"
+
+    # Should be able to read links directly too
+    get "/arvados/v1/links/#{can_read_uuid}",
+        params: {},
+      headers: auth(:active)
+    assert_response :success
+
+    ### Now delete the can_manage link
+    delete "/arvados/v1/links/#{can_manage_uuid}",
+      params: nil,
+      headers: auth(:active)
+    assert_response :success
+
+    # Should not be able read these permission links again
+    get "/arvados/v1/permissions/#{groups(:public).uuid}",
+      params: nil,
+      headers: auth(:active)
+    assert_response 404
+
+    get "/arvados/v1/links",
+        params: {
+          :filters => [["link_class", "=", "permission"], ["head_uuid", "=", groups(:public).uuid]].to_json
+        },
+      headers: auth(:active)
+    assert_response :success
+    assert_equal [], json_response['items']
+
+    # Should not be able to read links directly either
+    get "/arvados/v1/links/#{can_read_uuid}",
+        params: {},
+      headers: auth(:active)
+    assert_response 404
   end
 
   test "get_permissions returns 404 for nonexistent uuid" do