Merge branch 'master' of git.curoverse.com:arvados into 11876-r-sdk
authorFuad Muhic <fmuhic@capeannenterprises.com>
Tue, 19 Dec 2017 14:36:46 +0000 (15:36 +0100)
committerFuad Muhic <fmuhic@capeannenterprises.com>
Tue, 19 Dec 2017 14:36:46 +0000 (15:36 +0100)
Arvados-DCO-1.1-Signed-off-by: Fuad Muhic <fmuhic@capeannenterprises.com>

apps/workbench/app/assets/javascripts/work_unit_log.js
apps/workbench/app/views/layouts/body.html.erb
apps/workbench/config/application.default.yml
apps/workbench/test/integration/work_units_test.rb
sdk/go/arvadostest/fixtures.go
services/api/test/fixtures/api_client_authorizations.yml
services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
services/keep-web/handler.go
services/keep-web/webdav.go
services/keepproxy/keepproxy.go
services/keepproxy/keepproxy_test.go

index 543690b200504fd6578b25e1f3fc530bdfb2d774..4962994cdef7aec94e2d1430454b07131628da1f 100644 (file)
@@ -39,10 +39,7 @@ $(document).on('arv-log-event', '.arv-log-event-handler-append-logs', function(e
                 txt += stamp + "Container "+eventData.object_uuid+" started\n";
                 break;
             case "Complete":
-                var outcome = eventData.properties.new_attributes.exit_code === 0 ? "success" : "failure";
-                txt += stamp + "Container "+eventData.object_uuid+" finished with exit code " +
-                    eventData.properties.new_attributes.exit_code +
-                    " ("+outcome+")\n";
+                txt += stamp + "Container "+eventData.object_uuid+" finished\n";
                 break;
             case "Cancelled":
                 txt += stamp + "Container "+eventData.object_uuid+" was cancelled\n";
index c1399f2602dc151907253d080af08504a53c0875..174e35fbb64705a1614479c9b2e731947b8bf13a 100644 (file)
@@ -72,7 +72,13 @@ SPDX-License-Identifier: AGPL-3.0 %>
                 </li>
                 <% if current_user.is_active %>
                 <li role="menuitem"><a href="/projects/<%=current_user.uuid%>" role="menuitem"><i class="fa fa-lg fa-home fa-fw"></i> Home project </a></li>
-
+                  <% if Rails.configuration.composer_url %>
+                    <li role="menuitem">
+                      <%= link_to Rails.configuration.composer_url, role: 'menu-item' do %>
+                      <i class="fa fa-lg fa-share-alt fa-fw"></i> Workflow Composer
+                  <% end %>
+                    </li>
+                  <% end %>
                 <li role="menuitem">
                   <%= link_to virtual_machines_user_path(current_user), role: 'menu-item' do %>
                     <i class="fa fa-lg fa-terminal fa-fw"></i> Virtual machines
@@ -85,7 +91,6 @@ SPDX-License-Identifier: AGPL-3.0 %>
                     <i class="fa fa-lg fa-key fa-fw"></i> SSH keys
                   <% end %>
                 </li>
-
                 <% if Rails.configuration.user_profile_form_fields %>
                   <li role="menuitem"><a href="/users/<%=current_user.uuid%>/profile" role="menuitem"><i class="fa fa-lg fa-user fa-fw"></i> Manage profile</a></li>
                 <% end %>
index eda6d0a423203dcec81a876352ad709766bcb28d..187845038ea3c48449ccd1e7d1c002657ffe6e37 100644 (file)
@@ -309,3 +309,8 @@ common:
   # Example:
   # multi_site_search: https://workbench.qr1hi.arvadosapi.com/collections/multisite
   multi_site_search: false
+
+  #
+  # Link to use for Arvados Workflow Composer app, or false if not available.
+  #
+  composer_url: false
\ No newline at end of file
index ed214d62cc674d62d479fac90cc00aff9ee95d76..511e5119dbe1a8548f34d6208560ad2e45b22d9e 100644 (file)
@@ -216,7 +216,7 @@ class WorkUnitsTest < ActionDispatch::IntegrationTest
           old_attributes: {state: 'Running'},
           new_attributes: {state: 'Complete', exit_code: 1},
         },
-      }, "Container #{c['uuid']} finished with exit code 1 (failure)"],
+      }, "Container #{c['uuid']} finished"],
      # It's unrealistic for state to change again once it's Complete,
      # but the logging code doesn't care, so we do it to keep the test
      # simple.
index eab7a2708c48f16fb9df967c9a88912e5b0f4f4f..3a611a3bfb14359cca0ab890f47da13314d16a9b 100644 (file)
@@ -37,6 +37,9 @@ const (
        FooRepoName     = "active/foo"
        Repository2UUID = "zzzzz-s0uqq-382brsig8rp3667"
        Repository2Name = "active/foo2"
+
+       FooCollectionSharingTokenUUID = "zzzzz-gj3su-gf02tdm4g1z3e3u"
+       FooCollectionSharingToken     = "iknqgmunrhgsyfok8uzjlwun9iscwm3xacmzmg65fa1j1lpdss"
 )
 
 // PathologicalManifest : A valid manifest designed to test
index 23e63f26c94126682a2bdb1ea32b864e63ae19c0..8b283db45bf285674a0af9b6fa550453afb06e0a 100644 (file)
@@ -316,3 +316,14 @@ permission_perftest:
   user: permission_perftest
   api_token: 3kg6k6lzmp9kjabonentustoecn5bahbt2fod9zru30k1jqdmi
   expires_at: 2038-01-01 00:00:00
+
+foo_collection_sharing_token:
+  uuid: zzzzz-gj3su-gf02tdm4g1z3e3u
+  api_client: untrusted
+  user: active
+  api_token: iknqgmunrhgsyfok8uzjlwun9iscwm3xacmzmg65fa1j1lpdss
+  expires_at: 2038-01-01 00:00:00
+  scopes:
+  - GET /arvados/v1/collections/zzzzz-4zz18-znfnqtbbv4spc3w
+  - GET /arvados/v1/collections/zzzzz-4zz18-znfnqtbbv4spc3w/
+  - GET /arvados/v1/keep_services/accessible
index 3d094c7f6e3a68f745f02be1950cc76eb8fdecd2..3c89103f38dbc1cd094047d173c10bfe0b28f08e 100644 (file)
@@ -222,7 +222,12 @@ func submit(dispatcher *dispatch.Dispatcher, container arvados.Container, crunch
 
        // Send a tiny script on stdin to execute the crunch-run
        // command (slurm requires this to be a #! script)
-       cmd.Stdin = strings.NewReader(execScript(append(crunchRunCommand, container.UUID)))
+
+       // append() here avoids modifying crunchRunCommand's
+       // underlying array, which is shared with other goroutines.
+       args := append([]string(nil), crunchRunCommand...)
+       args = append(args, container.UUID)
+       cmd.Stdin = strings.NewReader(execScript(args))
 
        var stdout, stderr bytes.Buffer
        cmd.Stdout = &stdout
index a1476d3a8eb1b62fad8ea519702ec290e7f3472c..19a2040b4a5735551c0f7bf8a610c1fb109399b9 100644 (file)
@@ -441,8 +441,9 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
                h := webdav.Handler{
                        Prefix: "/" + strings.Join(pathParts[:stripParts], "/"),
                        FileSystem: &webdavFS{
-                               collfs:  fs,
-                               writing: writeMethod[r.Method],
+                               collfs:        fs,
+                               writing:       writeMethod[r.Method],
+                               alwaysReadEOF: r.Method == "PROPFIND",
                        },
                        LockSystem: h.webdavLS,
                        Logger: func(_ *http.Request, err error) {
index 3ceb0ed5c9ea6c71f06e5ad857ad1223bb277433..af83681f9c4b435285ebe1e6f3b9c1a1f7a807a4 100644 (file)
@@ -8,6 +8,7 @@ import (
        "crypto/rand"
        "errors"
        "fmt"
+       "io"
        prand "math/rand"
        "os"
        "path"
@@ -37,6 +38,12 @@ var (
 type webdavFS struct {
        collfs  arvados.CollectionFileSystem
        writing bool
+       // webdav PROPFIND reads the first few bytes of each file
+       // whose filename extension isn't recognized, which is
+       // prohibitively expensive: we end up fetching multiple 64MiB
+       // blocks. Avoid this by returning EOF on all reads when
+       // handling a PROPFIND.
+       alwaysReadEOF bool
 }
 
 func (fs *webdavFS) makeparents(name string) {
@@ -71,6 +78,9 @@ func (fs *webdavFS) OpenFile(ctx context.Context, name string, flag int, perm os
                // have 405.
                f = writeFailer{File: f, err: errReadOnly}
        }
+       if fs.alwaysReadEOF {
+               f = readEOF{File: f}
+       }
        return
 }
 
@@ -106,6 +116,14 @@ func (wf writeFailer) Close() error {
        return wf.err
 }
 
+type readEOF struct {
+       webdav.File
+}
+
+func (readEOF) Read(p []byte) (int, error) {
+       return 0, io.EOF
+}
+
 // noLockSystem implements webdav.LockSystem by returning success for
 // every possible locking operation, even though it has no side
 // effects such as actually locking anything. This works for a
index 145b39d4c3d1e643983c6f517eb31ff2c8d417fd..0c0c08fe4d7d0e2f66650150f71440e83f424790 100644 (file)
@@ -232,31 +232,43 @@ func GetRemoteAddress(req *http.Request) string {
 }
 
 func CheckAuthorizationHeader(kc *keepclient.KeepClient, cache *ApiTokenCache, req *http.Request) (pass bool, tok string) {
-       var auth string
-       if auth = req.Header.Get("Authorization"); auth == "" {
+       parts := strings.SplitN(req.Header.Get("Authorization"), " ", 2)
+       if len(parts) < 2 || !(parts[0] == "OAuth2" || parts[0] == "Bearer") || len(parts[1]) == 0 {
                return false, ""
        }
+       tok = parts[1]
 
-       _, err := fmt.Sscanf(auth, "OAuth2 %s", &tok)
-       if err != nil {
-               // Scanning error
-               return false, ""
+       // Tokens are validated differently depending on what kind of
+       // operation is being performed. For example, tokens in
+       // collection-sharing links permit GET requests, but not
+       // PUT requests.
+       var op string
+       if req.Method == "GET" || req.Method == "HEAD" {
+               op = "read"
+       } else {
+               op = "write"
        }
 
-       if cache.RecallToken(tok) {
+       if cache.RecallToken(op + ":" + tok) {
                // Valid in the cache, short circuit
                return true, tok
        }
 
+       var err error
        arv := *kc.Arvados
        arv.ApiToken = tok
-       if err := arv.Call("HEAD", "users", "", "current", nil, nil); err != nil {
+       if op == "read" {
+               err = arv.Call("HEAD", "keep_services", "", "accessible", nil, nil)
+       } else {
+               err = arv.Call("HEAD", "users", "", "current", nil, nil)
+       }
+       if err != nil {
                log.Printf("%s: CheckAuthorizationHeader error: %v", GetRemoteAddress(req), err)
                return false, ""
        }
 
        // Success!  Update cache
-       cache.RememberToken(tok)
+       cache.RememberToken(op + ":" + tok)
 
        return true, tok
 }
index a7b608b69c462fd4149f932c16dbabcaddbca6c6..bb0e9bbf6874859211fefea70955472cc797afb3 100644 (file)
@@ -323,41 +323,26 @@ func (s *ServerRequiredSuite) TestPutAskGetForbidden(c *C) {
        kc := runProxy(c, nil, true)
        defer closeListener()
 
-       hash := fmt.Sprintf("%x", md5.Sum([]byte("bar")))
+       hash := fmt.Sprintf("%x+3", md5.Sum([]byte("bar")))
 
-       {
-               _, _, err := kc.Ask(hash)
-               errNotFound, _ := err.(keepclient.ErrNotFound)
-               c.Check(errNotFound, NotNil)
-               c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
-               c.Log("Ask 1")
-       }
+       _, _, err := kc.Ask(hash)
+       c.Check(err, FitsTypeOf, &keepclient.ErrNotFound{})
 
-       {
-               hash2, rep, err := kc.PutB([]byte("bar"))
-               c.Check(hash2, Equals, "")
-               c.Check(rep, Equals, 0)
-               c.Check(err, FitsTypeOf, keepclient.InsufficientReplicasError(errors.New("")))
-               c.Log("PutB")
-       }
+       hash2, rep, err := kc.PutB([]byte("bar"))
+       c.Check(hash2, Equals, "")
+       c.Check(rep, Equals, 0)
+       c.Check(err, FitsTypeOf, keepclient.InsufficientReplicasError(errors.New("")))
 
-       {
-               blocklen, _, err := kc.Ask(hash)
-               errNotFound, _ := err.(keepclient.ErrNotFound)
-               c.Check(errNotFound, NotNil)
-               c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
-               c.Check(blocklen, Equals, int64(0))
-               c.Log("Ask 2")
-       }
+       blocklen, _, err := kc.Ask(hash)
+       c.Check(err, FitsTypeOf, &keepclient.ErrNotFound{})
+       c.Check(err, ErrorMatches, ".*not found.*")
+       c.Check(blocklen, Equals, int64(0))
+
+       _, blocklen, _, err = kc.Get(hash)
+       c.Check(err, FitsTypeOf, &keepclient.ErrNotFound{})
+       c.Check(err, ErrorMatches, ".*not found.*")
+       c.Check(blocklen, Equals, int64(0))
 
-       {
-               _, blocklen, _, err := kc.Get(hash)
-               errNotFound, _ := err.(keepclient.ErrNotFound)
-               c.Check(errNotFound, NotNil)
-               c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
-               c.Check(blocklen, Equals, int64(0))
-               c.Log("Get")
-       }
 }
 
 func (s *ServerRequiredSuite) TestGetDisabled(c *C) {
@@ -544,35 +529,53 @@ func (s *ServerRequiredSuite) TestGetIndex(c *C) {
        c.Assert((err != nil), Equals, true)
 }
 
+func (s *ServerRequiredSuite) TestCollectionSharingToken(c *C) {
+       kc := runProxy(c, nil, false)
+       defer closeListener()
+       hash, _, err := kc.PutB([]byte("shareddata"))
+       c.Check(err, IsNil)
+       kc.Arvados.ApiToken = arvadostest.FooCollectionSharingToken
+       rdr, _, _, err := kc.Get(hash)
+       c.Assert(err, IsNil)
+       data, err := ioutil.ReadAll(rdr)
+       c.Check(err, IsNil)
+       c.Check(data, DeepEquals, []byte("shareddata"))
+}
+
 func (s *ServerRequiredSuite) TestPutAskGetInvalidToken(c *C) {
        kc := runProxy(c, nil, false)
        defer closeListener()
 
        // Put a test block
        hash, rep, err := kc.PutB([]byte("foo"))
-       c.Check(err, Equals, nil)
+       c.Check(err, IsNil)
        c.Check(rep, Equals, 2)
 
-       for _, token := range []string{
+       for _, badToken := range []string{
                "nosuchtoken",
                "2ym314ysp27sk7h943q6vtc378srb06se3pq6ghurylyf3pdmx", // expired
        } {
-               // Change token to given bad token
-               kc.Arvados.ApiToken = token
+               kc.Arvados.ApiToken = badToken
+
+               // Ask and Get will fail only if the upstream
+               // keepstore server checks for valid signatures.
+               // Without knowing the blob signing key, there is no
+               // way for keepproxy to know whether a given token is
+               // permitted to read a block.  So these tests fail:
+               if false {
+                       _, _, err = kc.Ask(hash)
+                       c.Assert(err, FitsTypeOf, &keepclient.ErrNotFound{})
+                       c.Check(err.(*keepclient.ErrNotFound).Temporary(), Equals, false)
+                       c.Check(err, ErrorMatches, ".*HTTP 403.*")
+
+                       _, _, _, err = kc.Get(hash)
+                       c.Assert(err, FitsTypeOf, &keepclient.ErrNotFound{})
+                       c.Check(err.(*keepclient.ErrNotFound).Temporary(), Equals, false)
+                       c.Check(err, ErrorMatches, ".*HTTP 403 \"Missing or invalid Authorization header\".*")
+               }
 
-               // Ask should result in error
-               _, _, err = kc.Ask(hash)
-               c.Check(err, NotNil)
-               errNotFound, _ := err.(keepclient.ErrNotFound)
-               c.Check(errNotFound.Temporary(), Equals, false)
-               c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
-
-               // Get should result in error
-               _, _, _, err = kc.Get(hash)
-               c.Check(err, NotNil)
-               errNotFound, _ = err.(keepclient.ErrNotFound)
-               c.Check(errNotFound.Temporary(), Equals, false)
-               c.Assert(strings.Contains(err.Error(), "HTTP 403 \"Missing or invalid Authorization header\""), Equals, true)
+               _, _, err = kc.PutB([]byte("foo"))
+               c.Check(err, ErrorMatches, ".*403.*Missing or invalid Authorization header")
        }
 }