Merge branch '17507-listobjectsv2'
authorTom Clegg <tom@curii.com>
Mon, 26 Apr 2021 14:14:15 +0000 (10:14 -0400)
committerTom Clegg <tom@curii.com>
Mon, 26 Apr 2021 14:14:15 +0000 (10:14 -0400)
fixes #17507

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

18 files changed:
lib/boot/cmd.go
lib/boot/nginx.go
lib/boot/supervisor.go
lib/controller/federation/federation_test.go
lib/controller/handler.go
lib/controller/integration_test.go
lib/controller/router/request.go
lib/controller/router/router.go
lib/controller/router/router_test.go
sdk/cwl/arvados_cwl/fsaccess.py
sdk/cwl/tests/10380-trailing-slash-dir.cwl [new file with mode: 0644]
sdk/cwl/tests/17521-dot-slash-glob.cwl [new file with mode: 0644]
sdk/cwl/tests/arvados-tests.yml
sdk/go/arvadosclient/arvadosclient_test.go
sdk/python/tests/nginx.conf
services/keep-web/handler.go
services/keep-web/handler_test.go
tools/sync-groups/federation_test.go

index 963d16226b343341cdf3394af646097b498b9b1c..001504e203f24221ade1668af7bb4d532e6c991d 100644 (file)
@@ -66,6 +66,7 @@ func (bcmd bootCommand) run(ctx context.Context, prog string, args []string, std
        flags.StringVar(&super.ClusterType, "type", "production", "cluster `type`: development, test, or production")
        flags.StringVar(&super.ListenHost, "listen-host", "localhost", "host name or interface address for service listeners")
        flags.StringVar(&super.ControllerAddr, "controller-address", ":0", "desired controller address, `host:port` or `:port`")
+       flags.BoolVar(&super.NoWorkbench1, "no-workbench1", false, "do not run workbench1")
        flags.BoolVar(&super.OwnTemporaryDatabase, "own-temporary-database", false, "bring up a postgres server and create a temporary database")
        timeout := flags.Duration("timeout", 0, "maximum time to wait for cluster to be ready")
        shutdown := flags.Bool("shutdown", false, "shut down when the cluster becomes ready")
index dc4aebd528d4bf3f6c8d43c359efb8b51ed6b73a..5826e5c0136cf7707753a6593c8fb756c1252c5d 100644 (file)
@@ -53,12 +53,17 @@ func (runNginx) Run(ctx context.Context, fail func(error), super *Supervisor) er
                {"WORKBENCH1", super.cluster.Services.Workbench1},
                {"WS", super.cluster.Services.Websocket},
        } {
-               host, port, err := internalPort(cmpt.svc)
-               if err != nil {
+               var host, port string
+               if len(cmpt.svc.InternalURLs) == 0 {
+                       // We won't run this service, but we need an
+                       // upstream port to write in our templated
+                       // nginx config. Choose a port that will
+                       // return 502 Bad Gateway.
+                       port = "9"
+               } else if host, port, err = internalPort(cmpt.svc); err != nil {
                        return fmt.Errorf("%s internal port: %w (%v)", cmpt.varname, err, cmpt.svc)
-               }
-               if ok, err := addrIsLocal(net.JoinHostPort(host, port)); !ok || err != nil {
-                       return fmt.Errorf("urlIsLocal() failed for host %q port %q: %v", host, port, err)
+               } else if ok, err := addrIsLocal(net.JoinHostPort(host, port)); !ok || err != nil {
+                       return fmt.Errorf("%s addrIsLocal() failed for host %q port %q: %v", cmpt.varname, host, port, err)
                }
                vars[cmpt.varname+"PORT"] = port
 
@@ -66,8 +71,9 @@ func (runNginx) Run(ctx context.Context, fail func(error), super *Supervisor) er
                if err != nil {
                        return fmt.Errorf("%s external port: %w (%v)", cmpt.varname, err, cmpt.svc)
                }
-               if ok, err := addrIsLocal(net.JoinHostPort(super.ListenHost, port)); !ok || err != nil {
-                       return fmt.Errorf("urlIsLocal() failed for host %q port %q: %v", super.ListenHost, port, err)
+               listenAddr := net.JoinHostPort(super.ListenHost, port)
+               if ok, err := addrIsLocal(listenAddr); !ok || err != nil {
+                       return fmt.Errorf("%s addrIsLocal(%q) failed: %w", cmpt.varname, listenAddr, err)
                }
                vars[cmpt.varname+"SSLPORT"] = port
        }
index 961ed55de37e1f7fb5d165edfa63f562689ceeff..0f497a443befbbb7b7942fcd69f1bb7bb0ac9dec 100644 (file)
@@ -42,6 +42,7 @@ type Supervisor struct {
        ClusterType          string // e.g., production
        ListenHost           string // e.g., localhost
        ControllerAddr       string // e.g., 127.0.0.1:8000
+       NoWorkbench1         bool
        OwnTemporaryDatabase bool
        Stderr               io.Writer
 
@@ -249,10 +250,14 @@ func (super *Supervisor) run(cfg *arvados.Config) error {
                runServiceCommand{name: "ws", svc: super.cluster.Services.Websocket, depends: []supervisedTask{seedDatabase{}}},
                installPassenger{src: "services/api"},
                runPassenger{src: "services/api", varlibdir: "railsapi", svc: super.cluster.Services.RailsAPI, depends: []supervisedTask{createCertificates{}, seedDatabase{}, installPassenger{src: "services/api"}}},
-               installPassenger{src: "apps/workbench", depends: []supervisedTask{seedDatabase{}}}, // dependency ensures workbench doesn't delay api install/startup
-               runPassenger{src: "apps/workbench", varlibdir: "workbench1", svc: super.cluster.Services.Workbench1, depends: []supervisedTask{installPassenger{src: "apps/workbench"}}},
                seedDatabase{},
        }
+       if !super.NoWorkbench1 {
+               tasks = append(tasks,
+                       installPassenger{src: "apps/workbench", depends: []supervisedTask{seedDatabase{}}}, // dependency ensures workbench doesn't delay api install/startup
+                       runPassenger{src: "apps/workbench", varlibdir: "workbench1", svc: super.cluster.Services.Workbench1, depends: []supervisedTask{installPassenger{src: "apps/workbench"}}},
+               )
+       }
        if super.ClusterType != "test" {
                tasks = append(tasks,
                        runServiceCommand{name: "dispatch-cloud", svc: super.cluster.Services.DispatchCloud},
@@ -678,6 +683,14 @@ func (super *Supervisor) autofillConfig(cfg *arvados.Config) error {
                                svc.ExternalURL = arvados.URL{Scheme: "wss", Host: fmt.Sprintf("%s:%s", super.ListenHost, nextPort(super.ListenHost)), Path: "/websocket"}
                        }
                }
+               if super.NoWorkbench1 && svc == &cluster.Services.Workbench1 {
+                       // When workbench1 is disabled, it gets an
+                       // ExternalURL (so we have a valid listening
+                       // port to write in our Nginx config) but no
+                       // InternalURLs (so health checker doesn't
+                       // complain).
+                       continue
+               }
                if len(svc.InternalURLs) == 0 {
                        svc.InternalURLs = map[arvados.URL]arvados.ServiceInstance{
                                {Scheme: "http", Host: fmt.Sprintf("%s:%s", super.ListenHost, nextPort(super.ListenHost)), Path: "/"}: {},
index 50f7eea42b3fe162de4282f009d31f162df5cd4f..fdc4d96cfaa90b3e28dd2048c2c5bd0f73bf9dc5 100644 (file)
@@ -86,7 +86,7 @@ func (s *FederationSuite) addDirectRemote(c *check.C, id string, backend backend
 
 func (s *FederationSuite) addHTTPRemote(c *check.C, id string, backend backend) {
        srv := httpserver.Server{Addr: ":"}
-       srv.Handler = router.New(backend, nil)
+       srv.Handler = router.New(backend, router.Config{})
        c.Check(srv.Start(), check.IsNil)
        s.cluster.RemoteClusters[id] = arvados.RemoteCluster{
                Scheme: "http",
index 578bfc7d24bd26b8417a1e1bdcda5b6ab7aafd14..a35d0030194e8bf9e79d1f2f256ff9fab5621fe7 100644 (file)
@@ -92,7 +92,10 @@ func (h *Handler) setup() {
        })
 
        oidcAuthorizer := localdb.OIDCAccessTokenAuthorizer(h.Cluster, h.db)
-       rtr := router.New(federation.New(h.Cluster), api.ComposeWrappers(ctrlctx.WrapCallsInTransactions(h.db), oidcAuthorizer.WrapCalls))
+       rtr := router.New(federation.New(h.Cluster), router.Config{
+               MaxRequestSize: h.Cluster.API.MaxRequestSize,
+               WrapCalls:      api.ComposeWrappers(ctrlctx.WrapCallsInTransactions(h.db), oidcAuthorizer.WrapCalls),
+       })
        mux.Handle("/arvados/v1/config", rtr)
        mux.Handle("/"+arvados.EndpointUserAuthenticate.Path, rtr) // must come before .../users/
        mux.Handle("/arvados/v1/collections", rtr)
index aeaede427ecf6c5106d86a890304418d5d8cb4c7..7b1dcbea6655bcf5f86b175c58ad2f9d5382b74d 100644 (file)
@@ -130,8 +130,9 @@ func (s *IntegrationSuite) SetUpSuite(c *check.C) {
                tc := boot.NewTestCluster(
                        filepath.Join(cwd, "..", ".."),
                        id, cfg, "127.0.0."+id[3:], c.Log)
+               tc.Super.NoWorkbench1 = true
+               tc.Start()
                s.testClusters[id] = tc
-               s.testClusters[id].Start()
        }
        for _, tc := range s.testClusters {
                ok := tc.WaitReady()
index eae9e0a8cebc974dca813bba3dfa8f03381bbf2e..06141b1033e3f0034e003eab07da11c17153496e 100644 (file)
@@ -63,7 +63,11 @@ func guessAndParse(k, v string) (interface{}, error) {
 func (rtr *router) loadRequestParams(req *http.Request, attrsKey string) (map[string]interface{}, error) {
        err := req.ParseForm()
        if err != nil {
-               return nil, httpError(http.StatusBadRequest, err)
+               if err.Error() == "http: request body too large" {
+                       return nil, httpError(http.StatusRequestEntityTooLarge, err)
+               } else {
+                       return nil, httpError(http.StatusBadRequest, err)
+               }
        }
        params := map[string]interface{}{}
 
index a313ebc8bed94c5b5b7e32b6c086644b4faae77f..5ceabbfb1d56fab171d8d4a8dfabca585f1362f6 100644 (file)
@@ -7,6 +7,7 @@ package router
 import (
        "context"
        "fmt"
+       "math"
        "net/http"
        "strings"
 
@@ -20,24 +21,32 @@ import (
 )
 
 type router struct {
-       mux       *mux.Router
-       backend   arvados.API
-       wrapCalls func(api.RoutableFunc) api.RoutableFunc
+       mux     *mux.Router
+       backend arvados.API
+       config  Config
+}
+
+type Config struct {
+       // Return an error if request body exceeds this size. 0 means
+       // unlimited.
+       MaxRequestSize int
+
+       // If wrapCalls is not nil, it is called once for each API
+       // method, and the returned method is used in its place. This
+       // can be used to install hooks before and after each API call
+       // and alter responses; see localdb.WrapCallsInTransaction for
+       // an example.
+       WrapCalls func(api.RoutableFunc) api.RoutableFunc
 }
 
 // New returns a new router (which implements the http.Handler
 // interface) that serves requests by calling Arvados API methods on
 // the given backend.
-//
-// If wrapCalls is not nil, it is called once for each API method, and
-// the returned method is used in its place. This can be used to
-// install hooks before and after each API call and alter responses;
-// see localdb.WrapCallsInTransaction for an example.
-func New(backend arvados.API, wrapCalls func(api.RoutableFunc) api.RoutableFunc) *router {
+func New(backend arvados.API, config Config) *router {
        rtr := &router{
-               mux:       mux.NewRouter(),
-               backend:   backend,
-               wrapCalls: wrapCalls,
+               mux:     mux.NewRouter(),
+               backend: backend,
+               config:  config,
        }
        rtr.addRoutes()
        return rtr
@@ -433,8 +442,8 @@ func (rtr *router) addRoutes() {
                },
        } {
                exec := route.exec
-               if rtr.wrapCalls != nil {
-                       exec = rtr.wrapCalls(exec)
+               if rtr.config.WrapCalls != nil {
+                       exec = rtr.config.WrapCalls(exec)
                }
                rtr.addRoute(route.endpoint, route.defaultOpts, exec)
        }
@@ -524,8 +533,26 @@ func (rtr *router) ServeHTTP(w http.ResponseWriter, r *http.Request) {
        if r.Method == "OPTIONS" {
                return
        }
+       if r.Body != nil {
+               // Wrap r.Body in a http.MaxBytesReader(), otherwise
+               // r.ParseForm() uses a default max request body size
+               // of 10 megabytes. Note we rely on the Nginx
+               // configuration to enforce the real max body size.
+               max := int64(rtr.config.MaxRequestSize)
+               if max < 1 {
+                       max = math.MaxInt64 - 1
+               }
+               r.Body = http.MaxBytesReader(w, r.Body, max)
+       }
        if r.Method == "POST" {
-               r.ParseForm()
+               err := r.ParseForm()
+               if err != nil {
+                       if err.Error() == "http: request body too large" {
+                               err = httpError(http.StatusRequestEntityTooLarge, err)
+                       }
+                       rtr.sendError(w, err)
+                       return
+               }
                if m := r.FormValue("_method"); m != "" {
                        r2 := *r
                        r = &r2
index 18fff7c9cc4f5d4a10f347c3da55ab79ca1bf38d..0330ec4252c9ad3ee8f461faf9ce7508c17bd3fc 100644 (file)
@@ -169,7 +169,7 @@ func (s *RouterIntegrationSuite) SetUpTest(c *check.C) {
        cluster.TLS.Insecure = true
        arvadostest.SetServiceURL(&cluster.Services.RailsAPI, "https://"+os.Getenv("ARVADOS_TEST_API_HOST"))
        url, _ := url.Parse("https://" + os.Getenv("ARVADOS_TEST_API_HOST"))
-       s.rtr = New(rpc.NewConn("zzzzz", url, true, rpc.PassthroughTokenProvider), nil)
+       s.rtr = New(rpc.NewConn("zzzzz", url, true, rpc.PassthroughTokenProvider), Config{})
 }
 
 func (s *RouterIntegrationSuite) TearDownSuite(c *check.C) {
@@ -226,6 +226,34 @@ func (s *RouterIntegrationSuite) TestCollectionResponses(c *check.C) {
        c.Check(jresp["kind"], check.Equals, "arvados#collection")
 }
 
+func (s *RouterIntegrationSuite) TestMaxRequestSize(c *check.C) {
+       token := arvadostest.ActiveTokenV2
+       for _, maxRequestSize := range []int{
+               // Ensure 5M limit is enforced.
+               5000000,
+               // Ensure 50M limit is enforced, and that a >25M body
+               // is accepted even though the default Go request size
+               // limit is 10M.
+               50000000,
+       } {
+               s.rtr.config.MaxRequestSize = maxRequestSize
+               okstr := "a"
+               for len(okstr) < maxRequestSize/2 {
+                       okstr = okstr + okstr
+               }
+
+               hdr := http.Header{"Content-Type": {"application/x-www-form-urlencoded"}}
+
+               body := bytes.NewBufferString(url.Values{"foo_bar": {okstr}}.Encode())
+               _, rr, _ := doRequest(c, s.rtr, token, "POST", `/arvados/v1/collections`, hdr, body)
+               c.Check(rr.Code, check.Equals, http.StatusOK)
+
+               body = bytes.NewBufferString(url.Values{"foo_bar": {okstr + okstr}}.Encode())
+               _, rr, _ = doRequest(c, s.rtr, token, "POST", `/arvados/v1/collections`, hdr, body)
+               c.Check(rr.Code, check.Equals, http.StatusRequestEntityTooLarge)
+       }
+}
+
 func (s *RouterIntegrationSuite) TestContainerList(c *check.C) {
        token := arvadostest.ActiveTokenV2
 
index 16ec2b2f968e868104a4e0b18dad0a23654d4619..1e339d5bb7d4ab20f90438ed5670f3d67d51e548 100644 (file)
@@ -100,7 +100,8 @@ class CollectionFsAccess(cwltool.stdfsaccess.StdFsAccess):
         if p.startswith("keep:") and (arvados.util.keep_locator_pattern.match(p[5:]) or
                                       arvados.util.collection_uuid_pattern.match(p[5:])):
             locator = p[5:]
-            return (self.collection_cache.get(locator), urllib.parse.unquote(sp[1]) if len(sp) == 2 else None)
+            rest = os.path.normpath(urllib.parse.unquote(sp[1])) if len(sp) == 2 else None
+            return (self.collection_cache.get(locator), rest)
         else:
             return (None, path)
 
diff --git a/sdk/cwl/tests/10380-trailing-slash-dir.cwl b/sdk/cwl/tests/10380-trailing-slash-dir.cwl
new file mode 100644 (file)
index 0000000..5eec729
--- /dev/null
@@ -0,0 +1,15 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+cwlVersion: v1.2
+class: CommandLineTool
+inputs: []
+outputs:
+  stuff:
+    type: Directory
+    outputBinding:
+      glob: './foo/'
+requirements:
+  ShellCommandRequirement: {}
+arguments: [{shellQuote: false, valueFrom: "mkdir -p foo && touch baz.txt && touch foo/bar.txt"}]
diff --git a/sdk/cwl/tests/17521-dot-slash-glob.cwl b/sdk/cwl/tests/17521-dot-slash-glob.cwl
new file mode 100644 (file)
index 0000000..1e182c1
--- /dev/null
@@ -0,0 +1,15 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+cwlVersion: v1.2
+class: CommandLineTool
+inputs: []
+outputs:
+  stuff:
+    type: File
+    outputBinding:
+      glob: './foo/*.txt'
+requirements:
+  ShellCommandRequirement: {}
+arguments: [{shellQuote: false, valueFrom: "mkdir -p foo && touch baz.txt && touch foo/bar.txt"}]
index 48255f85756a67ed7fa6026940695c8652bdb41c..199ced9f41fc936a92eb6a8fe07809675971c2c4 100644 (file)
   output: {}
   tool: wf/trick_defaults2.cwl
   doc: "Test issue 17462 - secondary file objects on file defaults are not resolved"
+
+- job: null
+  output: {}
+  tool: 17521-dot-slash-glob.cwl
+  doc: "Test issue 17521 - bug with leading './' capturing files in subdirectories"
+
+- job: null
+  output: {}
+  tool: 10380-trailing-slash-dir.cwl
+  doc: "Test issue 10380 - bug with trailing slash when capturing an output directory"
index fc686ad63739e51340d5e254f8f68d65ac4db3e7..9d6e4fe7e8f7e702287db4865c348715c104a4ff 100644 (file)
@@ -10,7 +10,9 @@ import (
        "net/http"
        "os"
        "testing"
+       "time"
 
+       "git.arvados.org/arvados.git/sdk/go/arvados"
        "git.arvados.org/arvados.git/sdk/go/arvadostest"
        . "gopkg.in/check.v1"
 )
@@ -28,14 +30,12 @@ var _ = Suite(&MockArvadosServerSuite{})
 type ServerRequiredSuite struct{}
 
 func (s *ServerRequiredSuite) SetUpSuite(c *C) {
-       arvadostest.StartAPI()
        arvadostest.StartKeep(2, false)
        RetryDelay = 0
 }
 
 func (s *ServerRequiredSuite) TearDownSuite(c *C) {
        arvadostest.StopKeep(2)
-       arvadostest.StopAPI()
 }
 
 func (s *ServerRequiredSuite) SetUpTest(c *C) {
@@ -158,6 +158,32 @@ func (s *ServerRequiredSuite) TestAPIDiscovery_Get_noSuchParameter(c *C) {
        c.Assert(value, IsNil)
 }
 
+func (s *ServerRequiredSuite) TestCreateLarge(c *C) {
+       arv, err := MakeArvadosClient()
+       c.Assert(err, IsNil)
+
+       txt := arvados.SignLocator("d41d8cd98f00b204e9800998ecf8427e+0", arv.ApiToken, time.Now().Add(time.Minute), time.Minute, []byte(arvadostest.SystemRootToken))
+       // Ensure our request body is bigger than the Go http server's
+       // default max size, 10 MB.
+       for len(txt) < 12000000 {
+               txt = txt + " " + txt
+       }
+       txt = ". " + txt + " 0:0:foo\n"
+
+       resp := Dict{}
+       err = arv.Create("collections", Dict{
+               "ensure_unique_name": true,
+               "collection": Dict{
+                       "is_trashed":    true,
+                       "name":          "test",
+                       "manifest_text": txt,
+               },
+       }, &resp)
+       c.Check(err, IsNil)
+       c.Check(resp["portable_data_hash"], Not(Equals), "")
+       c.Check(resp["portable_data_hash"], Not(Equals), "d41d8cd98f00b204e9800998ecf8427e+0")
+}
+
 type UnitSuite struct{}
 
 func (s *UnitSuite) TestUUIDMatch(c *C) {
index a4336049f2447bd18cf396cbec0b76e7cdf69356..35b780071a356f5bb7ab38e053798908b0dafe6b 100644 (file)
@@ -24,6 +24,7 @@ http {
     server_name controller ~.*;
     ssl_certificate "{{SSLCERT}}";
     ssl_certificate_key "{{SSLKEY}}";
+    client_max_body_size 0;
     location  / {
       proxy_pass http://controller;
       proxy_set_header Host $http_host;
index 4ea2fa2f6dea89af1b3a744b09a2da6d36e61169..94b59ebd41ea843d80cf32fade7e5dd6168a225e 100644 (file)
@@ -485,13 +485,18 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
        }
 
        openPath := "/" + strings.Join(targetPath, "/")
-       if f, err := fs.Open(openPath); os.IsNotExist(err) {
+       f, err := fs.Open(openPath)
+       if os.IsNotExist(err) {
                // Requested non-existent path
                http.Error(w, notFoundMessage, http.StatusNotFound)
+               return
        } else if err != nil {
                // Some other (unexpected) error
                http.Error(w, "open: "+err.Error(), http.StatusInternalServerError)
-       } else if stat, err := f.Stat(); err != nil {
+               return
+       }
+       defer f.Close()
+       if stat, err := f.Stat(); err != nil {
                // Can't get Size/IsDir (shouldn't happen with a collectionFS!)
                http.Error(w, "stat: "+err.Error(), http.StatusInternalServerError)
        } else if stat.IsDir() && !strings.HasSuffix(r.URL.Path, "/") {
@@ -504,15 +509,14 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
                h.serveDirectory(w, r, collection.Name, fs, openPath, true)
        } else {
                http.ServeContent(w, r, basename, stat.ModTime(), f)
-               if wrote := int64(w.WroteBodyBytes()); wrote != stat.Size() && r.Header.Get("Range") == "" {
+               if wrote := int64(w.WroteBodyBytes()); wrote != stat.Size() && w.WroteStatus() == http.StatusOK {
                        // If we wrote fewer bytes than expected, it's
                        // too late to change the real response code
                        // or send an error message to the client, but
                        // at least we can try to put some useful
                        // debugging info in the logs.
                        n, err := f.Read(make([]byte, 1024))
-                       ctxlog.FromContext(r.Context()).Errorf("stat.Size()==%d but only wrote %d bytes; read(1024) returns %d, %s", stat.Size(), wrote, n, err)
-
+                       ctxlog.FromContext(r.Context()).Errorf("stat.Size()==%d but only wrote %d bytes; read(1024) returns %d, %v", stat.Size(), wrote, n, err)
                }
        }
 }
index 5291efeb822a4a2fe22af022cf15208d0ee1ba7f..9252bd82d7e15cb86424106c1c76a5009061189f 100644 (file)
@@ -6,6 +6,7 @@ package main
 
 import (
        "bytes"
+       "context"
        "fmt"
        "html"
        "io/ioutil"
@@ -16,6 +17,7 @@ import (
        "path/filepath"
        "regexp"
        "strings"
+       "time"
 
        "git.arvados.org/arvados.git/lib/config"
        "git.arvados.org/arvados.git/sdk/go/arvados"
@@ -24,6 +26,7 @@ import (
        "git.arvados.org/arvados.git/sdk/go/auth"
        "git.arvados.org/arvados.git/sdk/go/ctxlog"
        "git.arvados.org/arvados.git/sdk/go/keepclient"
+       "github.com/sirupsen/logrus"
        check "gopkg.in/check.v1"
 )
 
@@ -72,6 +75,64 @@ func (s *UnitSuite) TestCORSPreflight(c *check.C) {
        c.Check(resp.Code, check.Equals, http.StatusMethodNotAllowed)
 }
 
+func (s *UnitSuite) TestEmptyResponse(c *check.C) {
+       for _, trial := range []struct {
+               dataExists    bool
+               sendIMSHeader bool
+               expectStatus  int
+               logRegexp     string
+       }{
+               // If we return no content due to a Keep read error,
+               // we should emit a log message.
+               {false, false, http.StatusOK, `(?ms).*only wrote 0 bytes.*`},
+
+               // If we return no content because the client sent an
+               // If-Modified-Since header, our response should be
+               // 304, and we should not emit a log message.
+               {true, true, http.StatusNotModified, ``},
+       } {
+               c.Logf("trial: %+v", trial)
+               arvadostest.StartKeep(2, true)
+               if trial.dataExists {
+                       arv, err := arvadosclient.MakeArvadosClient()
+                       c.Assert(err, check.IsNil)
+                       arv.ApiToken = arvadostest.ActiveToken
+                       kc, err := keepclient.MakeKeepClient(arv)
+                       c.Assert(err, check.IsNil)
+                       _, _, err = kc.PutB([]byte("foo"))
+                       c.Assert(err, check.IsNil)
+               }
+
+               h := handler{Config: newConfig(s.Config)}
+               u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/foo")
+               req := &http.Request{
+                       Method:     "GET",
+                       Host:       u.Host,
+                       URL:        u,
+                       RequestURI: u.RequestURI(),
+                       Header: http.Header{
+                               "Authorization": {"Bearer " + arvadostest.ActiveToken},
+                       },
+               }
+               if trial.sendIMSHeader {
+                       req.Header.Set("If-Modified-Since", strings.Replace(time.Now().UTC().Format(time.RFC1123), "UTC", "GMT", -1))
+               }
+
+               var logbuf bytes.Buffer
+               logger := logrus.New()
+               logger.Out = &logbuf
+               req = req.WithContext(ctxlog.Context(context.Background(), logger))
+
+               resp := httptest.NewRecorder()
+               h.ServeHTTP(resp, req)
+               c.Check(resp.Code, check.Equals, trial.expectStatus)
+               c.Check(resp.Body.String(), check.Equals, "")
+
+               c.Log(logbuf.String())
+               c.Check(logbuf.String(), check.Matches, trial.logRegexp)
+       }
+}
+
 func (s *UnitSuite) TestInvalidUUID(c *check.C) {
        bogusID := strings.Replace(arvadostest.FooCollectionPDH, "+", "-", 1) + "-"
        token := arvadostest.ActiveToken
@@ -237,7 +298,6 @@ func (s *IntegrationSuite) doVhostRequestsWithHostPath(c *check.C, authz authori
                if tok == arvadostest.ActiveToken {
                        c.Check(code, check.Equals, http.StatusOK)
                        c.Check(body, check.Equals, "foo")
-
                } else {
                        c.Check(code >= 400, check.Equals, true)
                        c.Check(code < 500, check.Equals, true)
index aebac21de6bf8236550def7be2972fb88e14a0ec..1bbdaa3fa3cb5d3b60387ca0cc1d17723957be55 100644 (file)
@@ -114,8 +114,9 @@ func (s *FederationSuite) SetUpSuite(c *check.C) {
                tc := boot.NewTestCluster(
                        filepath.Join(cwd, "..", ".."),
                        id, cfg, "127.0.0."+id[3:], c.Log)
+               tc.Super.NoWorkbench1 = true
+               tc.Start()
                s.testClusters[id] = tc
-               s.testClusters[id].Start()
        }
        for _, tc := range s.testClusters {
                ok := tc.WaitReady()