From 9acc8cb9cd9ca4429712b0d31b647e9a6ecf2d96 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Thu, 28 Apr 2022 20:20:05 -0400 Subject: [PATCH] 18947: Refactor keep-web as arvados-server command. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- build/run-build-packages.sh | 2 +- cmd/arvados-server/cmd.go | 2 + .../arvados-server}/keep-web.service | 4 +- lib/boot/supervisor.go | 2 +- lib/cmd/cmd.go | 2 +- lib/install/deps.go | 1 - lib/service/cmd.go | 47 +++-- sdk/go/arvados/collection.go | 28 +++ sdk/go/arvadosclient/arvadosclient.go | 5 +- sdk/go/arvadostest/fixtures.go | 2 +- sdk/python/tests/run_test_server.py | 7 +- services/keep-web/cache.go | 25 ++- services/keep-web/cache_test.go | 23 ++- services/keep-web/cadaver_test.go | 10 +- services/keep-web/doc.go | 7 +- services/keep-web/handler.go | 104 ++++------- services/keep-web/handler_test.go | 176 ++++++++---------- services/keep-web/main.go | 129 +++---------- services/keep-web/ranges_test.go | 4 +- services/keep-web/s3.go | 30 +-- services/keep-web/s3_test.go | 29 +-- services/keep-web/s3aws_test.go | 4 +- services/keep-web/server.go | 45 ----- services/keep-web/server_test.go | 173 ++++++++++++----- services/keep-web/status_test.go | 8 +- services/keep-web/webdav.go | 2 +- services/keep-web/webdav_test.go | 2 +- .../docker/service/keep-web/run-service | 5 +- 28 files changed, 418 insertions(+), 460 deletions(-) rename {services/keep-web => cmd/arvados-server}/keep-web.service (82%) delete mode 100644 services/keep-web/server.go diff --git a/build/run-build-packages.sh b/build/run-build-packages.sh index 85e5f35222..adcbab8a09 100755 --- a/build/run-build-packages.sh +++ b/build/run-build-packages.sh @@ -262,7 +262,7 @@ package_go_binary cmd/arvados-server keepproxy "$FORMAT" "$ARCH" \ "Make a Keep cluster accessible to clients that are not on the LAN" package_go_binary cmd/arvados-server keepstore "$FORMAT" "$ARCH" \ "Keep storage daemon, accessible to clients on the LAN" -package_go_binary services/keep-web keep-web "$FORMAT" "$ARCH" \ +package_go_binary cmd/arvados-server keep-web "$FORMAT" "$ARCH" \ "Static web hosting service for user data stored in Arvados Keep" package_go_binary cmd/arvados-server arvados-ws "$FORMAT" "$ARCH" \ "Arvados Websocket server" diff --git a/cmd/arvados-server/cmd.go b/cmd/arvados-server/cmd.go index d65e18ddf5..e4bd39002a 100644 --- a/cmd/arvados-server/cmd.go +++ b/cmd/arvados-server/cmd.go @@ -22,6 +22,7 @@ import ( "git.arvados.org/arvados.git/lib/lsf" "git.arvados.org/arvados.git/lib/recovercollection" "git.arvados.org/arvados.git/services/githttpd" + keepweb "git.arvados.org/arvados.git/services/keep-web" "git.arvados.org/arvados.git/services/keepproxy" "git.arvados.org/arvados.git/services/keepstore" "git.arvados.org/arvados.git/services/ws" @@ -45,6 +46,7 @@ var ( "git-httpd": githttpd.Command, "install": install.Command, "init": install.InitCommand, + "keep-web": keepweb.Command, "keepproxy": keepproxy.Command, "keepstore": keepstore.Command, "recover-collection": recovercollection.Command, diff --git a/services/keep-web/keep-web.service b/cmd/arvados-server/keep-web.service similarity index 82% rename from services/keep-web/keep-web.service rename to cmd/arvados-server/keep-web.service index cb5fdf84fc..e889804cb5 100644 --- a/services/keep-web/keep-web.service +++ b/cmd/arvados-server/keep-web.service @@ -3,15 +3,17 @@ # SPDX-License-Identifier: AGPL-3.0 [Unit] -Description=Arvados Keep web gateway +Description=Arvados Keep WebDAV gateway Documentation=https://doc.arvados.org/ After=network.target +AssertPathExists=/etc/arvados/config.yml # systemd>=230 (debian:9) obeys StartLimitIntervalSec in the [Unit] section StartLimitIntervalSec=0 [Service] Type=notify +EnvironmentFile=-/etc/arvados/environment ExecStart=/usr/bin/keep-web # Set a reasonable default for the open file limit LimitNOFILE=65536 diff --git a/lib/boot/supervisor.go b/lib/boot/supervisor.go index 94cd5d0000..143529487b 100644 --- a/lib/boot/supervisor.go +++ b/lib/boot/supervisor.go @@ -368,7 +368,7 @@ func (super *Supervisor) runCluster() error { runGoProgram{src: "services/health", svc: super.cluster.Services.Health}, runServiceCommand{name: "keepproxy", svc: super.cluster.Services.Keepproxy, depends: []supervisedTask{runPassenger{src: "services/api"}}}, runServiceCommand{name: "keepstore", svc: super.cluster.Services.Keepstore}, - runGoProgram{src: "services/keep-web", svc: super.cluster.Services.WebDAV}, + runServiceCommand{name: "keep-web", svc: super.cluster.Services.WebDAV}, 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"}}}, diff --git a/lib/cmd/cmd.go b/lib/cmd/cmd.go index 63d7576b4c..a03cb90f68 100644 --- a/lib/cmd/cmd.go +++ b/lib/cmd/cmd.go @@ -60,7 +60,7 @@ func (versionCommand) RunCommand(prog string, args []string, stdin io.Reader, st // fmt.Println(args[0]) // return 2 // }), -// })("/usr/bin/multi", []string{"foobar", "baz"})) +// })("/usr/bin/multi", []string{"foobar", "baz"}, os.Stdin, os.Stdout, os.Stderr)) // // ...prints "baz" and exits 2. type Multi map[string]Handler diff --git a/lib/install/deps.go b/lib/install/deps.go index c6fba424d5..62b295a1f1 100644 --- a/lib/install/deps.go +++ b/lib/install/deps.go @@ -527,7 +527,6 @@ yarn install "services/crunch-dispatch-slurm", "services/health", "services/keep-balance", - "services/keep-web", "services/keepstore", "services/ws", } { diff --git a/lib/service/cmd.go b/lib/service/cmd.go index b7decd88a4..43357998d8 100644 --- a/lib/service/cmd.go +++ b/lib/service/cmd.go @@ -20,20 +20,20 @@ import ( "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/lib/config" "git.arvados.org/arvados.git/sdk/go/arvados" - "git.arvados.org/arvados.git/sdk/go/auth" "git.arvados.org/arvados.git/sdk/go/ctxlog" "git.arvados.org/arvados.git/sdk/go/health" "git.arvados.org/arvados.git/sdk/go/httpserver" "github.com/coreos/go-systemd/daemon" "github.com/julienschmidt/httprouter" "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promhttp" "github.com/sirupsen/logrus" ) type Handler interface { http.Handler CheckHealth() error + // Done returns a channel that closes when the handler shuts + // itself down, or nil if this never happens. Done() <-chan struct{} } @@ -74,6 +74,13 @@ func (c *command) RunCommand(prog string, args []string, stdin io.Reader, stdout loader := config.NewLoader(stdin, log) loader.SetupFlags(flags) + + // prog is [keepstore, keep-web, git-httpd, ...] but the + // legacy config flags are [-legacy-keepstore-config, + // -legacy-keepweb-config, -legacy-git-httpd-config, ...] + legacyFlag := "-legacy-" + strings.Replace(prog, "keep-", "keep", 1) + "-config" + args = loader.MungeLegacyConfigArgs(log, args, legacyFlag) + versionFlag := flags.Bool("version", false, "Write version information to stdout and exit 0") pprofAddr := flags.String("pprof", "", "Serve Go profile data at `[addr]:port`") if ok, code := cmd.ParseFlags(flags, prog, args, "", stderr); !ok { @@ -131,11 +138,10 @@ func (c *command) RunCommand(prog string, args []string, stdin io.Reader, stdout httpserver.AddRequestIDs( httpserver.LogRequests( interceptHealthReqs(cluster.ManagementToken, handler.CheckHealth, - interceptMetricsReqs(cluster.ManagementToken, reg, log, - httpserver.NewRequestLimiter(cluster.API.MaxConcurrentRequests, handler, reg))))))) + httpserver.NewRequestLimiter(cluster.API.MaxConcurrentRequests, handler, reg)))))) srv := &httpserver.Server{ Server: http.Server{ - Handler: instrumented.ServeAPI(cluster.ManagementToken, instrumented), + Handler: ifCollectionInHost(instrumented, instrumented.ServeAPI(cluster.ManagementToken, instrumented)), BaseContext: func(net.Listener) context.Context { return ctx }, }, Addr: listenURL.Host, @@ -178,6 +184,23 @@ func (c *command) RunCommand(prog string, args []string, stdin io.Reader, stdout return 0 } +// If an incoming request's target vhost has an embedded collection +// UUID or PDH, handle it with hTrue, otherwise handle it with +// hFalse. +// +// Facilitates routing "http://collections.example/metrics" to metrics +// and "http://{uuid}.collections.example/metrics" to a file in a +// collection. +func ifCollectionInHost(hTrue, hFalse http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if arvados.CollectionIDFromDNSName(r.Host) != "" { + hTrue.ServeHTTP(w, r) + } else { + hFalse.ServeHTTP(w, r) + } + }) +} + func interceptHealthReqs(mgtToken string, checkHealth func() error, next http.Handler) http.Handler { mux := httprouter.New() mux.Handler("GET", "/_health/ping", &health.Handler{ @@ -186,19 +209,7 @@ func interceptHealthReqs(mgtToken string, checkHealth func() error, next http.Ha Routes: health.Routes{"ping": checkHealth}, }) mux.NotFound = next - return mux -} - -func interceptMetricsReqs(mgtToken string, reg *prometheus.Registry, log logrus.FieldLogger, next http.Handler) http.Handler { - mux := httprouter.New() - metricsH := auth.RequireLiteralToken(mgtToken, - promhttp.HandlerFor(reg, promhttp.HandlerOpts{ - ErrorLog: log, - })) - mux.Handler("GET", "/metrics", metricsH) - mux.Handler("GET", "/metrics.json", metricsH) - mux.NotFound = next - return mux + return ifCollectionInHost(next, mux) } func getListenAddr(svcs arvados.Services, prog arvados.ServiceName, log logrus.FieldLogger) (arvados.URL, error) { diff --git a/sdk/go/arvados/collection.go b/sdk/go/arvados/collection.go index 785c18d4a7..389fe4e484 100644 --- a/sdk/go/arvados/collection.go +++ b/sdk/go/arvados/collection.go @@ -9,11 +9,17 @@ import ( "crypto/md5" "fmt" "regexp" + "strings" "time" "git.arvados.org/arvados.git/sdk/go/blockdigest" ) +var ( + UUIDMatch = regexp.MustCompile(`^[a-z0-9]{5}-[a-z0-9]{5}-[a-z0-9]{15}$`).MatchString + PDHMatch = regexp.MustCompile(`^[0-9a-f]{32}\+\d+$`).MatchString +) + // Collection is an arvados#collection resource. type Collection struct { UUID string `json:"uuid"` @@ -122,3 +128,25 @@ func PortableDataHash(mt string) string { }) return fmt.Sprintf("%x+%d", h.Sum(nil), size) } + +// CollectionIDFromDNSName returns a UUID or PDH if s begins with a +// UUID or URL-encoded PDH; otherwise "". +func CollectionIDFromDNSName(s string) string { + // Strip domain. + if i := strings.IndexRune(s, '.'); i >= 0 { + s = s[:i] + } + // Names like {uuid}--collections.example.com serve the same + // purpose as {uuid}.collections.example.com but can reduce + // cost/effort of using [additional] wildcard certificates. + if i := strings.Index(s, "--"); i >= 0 { + s = s[:i] + } + if UUIDMatch(s) { + return s + } + if pdh := strings.Replace(s, "-", "+", 1); PDHMatch(pdh) { + return pdh + } + return "" +} diff --git a/sdk/go/arvadosclient/arvadosclient.go b/sdk/go/arvadosclient/arvadosclient.go index 8f902d3a09..24070c5b06 100644 --- a/sdk/go/arvadosclient/arvadosclient.go +++ b/sdk/go/arvadosclient/arvadosclient.go @@ -19,7 +19,6 @@ import ( "net/http" "net/url" "os" - "regexp" "strings" "sync" "time" @@ -29,8 +28,8 @@ import ( type StringMatcher func(string) bool -var UUIDMatch StringMatcher = regexp.MustCompile(`^[a-z0-9]{5}-[a-z0-9]{5}-[a-z0-9]{15}$`).MatchString -var PDHMatch StringMatcher = regexp.MustCompile(`^[0-9a-f]{32}\+\d+$`).MatchString +var UUIDMatch StringMatcher = arvados.UUIDMatch +var PDHMatch StringMatcher = arvados.PDHMatch var MissingArvadosApiHost = errors.New("Missing required environment variable ARVADOS_API_HOST") var MissingArvadosApiToken = errors.New("Missing required environment variable ARVADOS_API_TOKEN") diff --git a/sdk/go/arvadostest/fixtures.go b/sdk/go/arvadostest/fixtures.go index 9281f51d0c..ec55725412 100644 --- a/sdk/go/arvadostest/fixtures.go +++ b/sdk/go/arvadostest/fixtures.go @@ -16,7 +16,7 @@ const ( AnonymousToken = "4kg6k6lzmp9kj4cpkcoxie964cmvjahbt4fod9zru44k4jqdmi" DataManagerToken = "320mkve8qkswstz7ff61glpk3mhgghmg67wmic7elw4z41pke1" SystemRootToken = "systemusertesttoken1234567890aoeuidhtnsqjkxbmwvzpy" - ManagementToken = "jg3ajndnq63sywcd50gbs5dskdc9ckkysb0nsqmfz08nwf17nl" + ManagementToken = "e687950a23c3a9bceec28c6223a06c79" ActiveUserUUID = "zzzzz-tpzed-xurymjxw79nv3jz" FederatedActiveUserUUID = "zbbbb-tpzed-xurymjxw79nv3jz" SpectatorUserUUID = "zzzzz-tpzed-l1s2piq4t4mps8r" diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py index 836776972e..74722b256e 100644 --- a/sdk/python/tests/run_test_server.py +++ b/sdk/python/tests/run_test_server.py @@ -518,7 +518,7 @@ def run_keep(num_servers=2, **kwargs): # If keepproxy and/or keep-web is running, send SIGHUP to make # them discover the new keepstore services. for svc in ('keepproxy', 'keep-web'): - pidfile = _pidfile('keepproxy') + pidfile = _pidfile(svc) if os.path.exists(pidfile): try: with open(pidfile) as pid: @@ -602,7 +602,7 @@ def run_keep_web(): env = os.environ.copy() logf = open(_logfilename('keep-web'), WRITE_MODE) keepweb = subprocess.Popen( - ['keep-web'], + ['arvados-server', 'keep-web'], env=env, stdin=open('/dev/null'), stdout=logf, stderr=logf) with open(_pidfile('keep-web'), 'w') as f: f.write(str(keepweb.pid)) @@ -680,7 +680,6 @@ def setup_config(): keepstore_ports = sorted([str(find_available_port()) for _ in range(0,4)]) keep_web_port = find_available_port() keep_web_external_port = find_available_port() - keep_web_dl_port = find_available_port() keep_web_dl_external_port = find_available_port() configsrc = os.environ.get("CONFIGSRC", None) @@ -762,7 +761,7 @@ def setup_config(): "WebDAVDownload": { "ExternalURL": "https://%s:%s" % (localhost, keep_web_dl_external_port), "InternalURLs": { - "http://%s:%s"%(localhost, keep_web_dl_port): {}, + "http://%s:%s"%(localhost, keep_web_port): {}, }, }, } diff --git a/services/keep-web/cache.go b/services/keep-web/cache.go index d2c79326af..d5fdc4997e 100644 --- a/services/keep-web/cache.go +++ b/services/keep-web/cache.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package keepweb import ( "sync" @@ -21,7 +21,6 @@ const metricsUpdateInterval = time.Second / 10 type cache struct { cluster *arvados.Cluster - config *arvados.WebDAVCacheConfig // TODO: use cluster.Collections.WebDAV instead logger logrus.FieldLogger registry *prometheus.Registry metrics cacheMetrics @@ -138,15 +137,15 @@ type cachedSession struct { func (c *cache) setup() { var err error - c.pdhs, err = lru.New2Q(c.config.MaxUUIDEntries) + c.pdhs, err = lru.New2Q(c.cluster.Collections.WebDAVCache.MaxUUIDEntries) if err != nil { panic(err) } - c.collections, err = lru.New2Q(c.config.MaxCollectionEntries) + c.collections, err = lru.New2Q(c.cluster.Collections.WebDAVCache.MaxCollectionEntries) if err != nil { panic(err) } - c.sessions, err = lru.New2Q(c.config.MaxSessions) + c.sessions, err = lru.New2Q(c.cluster.Collections.WebDAVCache.MaxSessions) if err != nil { panic(err) } @@ -207,12 +206,12 @@ func (c *cache) Update(client *arvados.Client, coll arvados.Collection, fs arvad return err } c.collections.Add(client.AuthToken+"\000"+updated.PortableDataHash, &cachedCollection{ - expire: time.Now().Add(time.Duration(c.config.TTL)), + expire: time.Now().Add(time.Duration(c.cluster.Collections.WebDAVCache.TTL)), collection: &updated, }) c.pdhs.Add(coll.UUID, &cachedPDH{ - expire: time.Now().Add(time.Duration(c.config.TTL)), - refresh: time.Now().Add(time.Duration(c.config.UUIDTTL)), + expire: time.Now().Add(time.Duration(c.cluster.Collections.WebDAVCache.TTL)), + refresh: time.Now().Add(time.Duration(c.cluster.Collections.WebDAVCache.UUIDTTL)), pdh: updated.PortableDataHash, }) return nil @@ -237,7 +236,7 @@ func (c *cache) GetSession(token string) (arvados.CustomFileSystem, *cachedSessi if sess == nil { c.metrics.sessionMisses.Inc() sess = &cachedSession{ - expire: now.Add(c.config.TTL.Duration()), + expire: now.Add(c.cluster.Collections.WebDAVCache.TTL.Duration()), } var err error sess.client, err = arvados.NewClientFromConfig(c.cluster) @@ -378,11 +377,11 @@ func (c *cache) Get(arv *arvadosclient.ArvadosClient, targetID string, forceRelo return nil, err } c.logger.Debugf("cache(%s): retrieved manifest, caching with pdh %s", targetID, retrieved.PortableDataHash) - exp := time.Now().Add(time.Duration(c.config.TTL)) + exp := time.Now().Add(time.Duration(c.cluster.Collections.WebDAVCache.TTL)) if targetID != retrieved.PortableDataHash { c.pdhs.Add(targetID, &cachedPDH{ expire: exp, - refresh: time.Now().Add(time.Duration(c.config.UUIDTTL)), + refresh: time.Now().Add(time.Duration(c.cluster.Collections.WebDAVCache.UUIDTTL)), pdh: retrieved.PortableDataHash, }) } @@ -390,7 +389,7 @@ func (c *cache) Get(arv *arvadosclient.ArvadosClient, targetID string, forceRelo expire: exp, collection: &retrieved, }) - if int64(len(retrieved.ManifestText)) > c.config.MaxCollectionBytes/int64(c.config.MaxCollectionEntries) { + if int64(len(retrieved.ManifestText)) > c.cluster.Collections.WebDAVCache.MaxCollectionBytes/int64(c.cluster.Collections.WebDAVCache.MaxCollectionEntries) { select { case c.chPruneCollections <- struct{}{}: default: @@ -430,7 +429,7 @@ func (c *cache) pruneCollections() { } } for i, k := range keys { - if size <= c.config.MaxCollectionBytes/2 { + if size <= c.cluster.Collections.WebDAVCache.MaxCollectionBytes/2 { break } if expired[i] { diff --git a/services/keep-web/cache_test.go b/services/keep-web/cache_test.go index 3e2faaff72..6b8f427171 100644 --- a/services/keep-web/cache_test.go +++ b/services/keep-web/cache_test.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package keepweb import ( "bytes" @@ -34,8 +34,11 @@ func (s *UnitSuite) TestCache(c *check.C) { arv, err := arvadosclient.MakeArvadosClient() c.Assert(err, check.Equals, nil) - cache := newConfig(ctxlog.TestLogger(c), s.Config).Cache - cache.registry = prometheus.NewRegistry() + cache := &cache{ + cluster: s.cluster, + logger: ctxlog.TestLogger(c), + registry: prometheus.NewRegistry(), + } // Hit the same collection 5 times using the same token. Only // the first req should cause an API call; the next 4 should @@ -111,8 +114,11 @@ func (s *UnitSuite) TestCacheForceReloadByPDH(c *check.C) { arv, err := arvadosclient.MakeArvadosClient() c.Assert(err, check.Equals, nil) - cache := newConfig(ctxlog.TestLogger(c), s.Config).Cache - cache.registry = prometheus.NewRegistry() + cache := &cache{ + cluster: s.cluster, + logger: ctxlog.TestLogger(c), + registry: prometheus.NewRegistry(), + } for _, forceReload := range []bool{false, true, false, true} { _, err := cache.Get(arv, arvadostest.FooCollectionPDH, forceReload) @@ -130,8 +136,11 @@ func (s *UnitSuite) TestCacheForceReloadByUUID(c *check.C) { arv, err := arvadosclient.MakeArvadosClient() c.Assert(err, check.Equals, nil) - cache := newConfig(ctxlog.TestLogger(c), s.Config).Cache - cache.registry = prometheus.NewRegistry() + cache := &cache{ + cluster: s.cluster, + logger: ctxlog.TestLogger(c), + registry: prometheus.NewRegistry(), + } for _, forceReload := range []bool{false, true, false, true} { _, err := cache.Get(arv, arvadostest.FooCollection, forceReload) diff --git a/services/keep-web/cadaver_test.go b/services/keep-web/cadaver_test.go index e965a6e8c2..742140f7f3 100644 --- a/services/keep-web/cadaver_test.go +++ b/services/keep-web/cadaver_test.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package keepweb import ( "bytes" @@ -52,8 +52,6 @@ func (s *IntegrationSuite) TestCadaverUserProject(c *check.C) { } func (s *IntegrationSuite) testCadaver(c *check.C, password string, pathFunc func(arvados.Collection) (string, string, string), skip func(string) bool) { - s.testServer.Config.cluster.Users.AnonymousUserToken = arvadostest.AnonymousToken - testdata := []byte("the human tragedy consists in the necessity of living with the consequences of actions performed under the pressure of compulsions we do not understand") tempdir, err := ioutil.TempDir("", "keep-web-test-") @@ -278,7 +276,7 @@ func (s *IntegrationSuite) testCadaver(c *check.C, password string, pathFunc fun match: `(?ms).*Locking .* failed:.*405 Method Not Allowed.*`, }, } { - c.Logf("%s %+v", "http://"+s.testServer.Addr, trial) + c.Logf("%s %+v", s.testServer.URL, trial) if skip != nil && skip(trial.path) { c.Log("(skip)") continue @@ -343,14 +341,14 @@ func (s *IntegrationSuite) runCadaver(c *check.C, password, path, stdin string) c.Assert(err, check.IsNil) defer os.RemoveAll(tempdir) - cmd := exec.Command("cadaver", "http://"+s.testServer.Addr+path) + cmd := exec.Command("cadaver", s.testServer.URL+path) if password != "" { // cadaver won't try username/password authentication // unless the server responds 401 to an // unauthenticated request, which it only does in // AttachmentOnlyHost, TrustAllContent, and // per-collection vhost cases. - s.testServer.Config.cluster.Services.WebDAVDownload.ExternalURL.Host = s.testServer.Addr + s.handler.Cluster.Services.WebDAVDownload.ExternalURL.Host = s.testServer.URL[7:] cmd.Env = append(os.Environ(), "HOME="+tempdir) f, err := os.OpenFile(filepath.Join(tempdir, ".netrc"), os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600) diff --git a/services/keep-web/doc.go b/services/keep-web/doc.go index be81bb68c7..d2b4c7eb54 100644 --- a/services/keep-web/doc.go +++ b/services/keep-web/doc.go @@ -49,8 +49,7 @@ // // Proxy configuration // -// Keep-web does not support TLS natively. Typically, it is installed -// behind a proxy like nginx. +// Typically, keep-web is installed behind a proxy like nginx. // // Here is an example nginx configuration. // @@ -72,7 +71,7 @@ // } // // It is not necessary to run keep-web on the same host as the nginx -// proxy. However, TLS is not used between nginx and keep-web, so +// proxy. However, if TLS is not used between nginx and keep-web, the // intervening networks must be secured by other means. // // Anonymous downloads @@ -149,4 +148,4 @@ // /metrics. The same information is also available as JSON at // /metrics.json. // -package main +package keepweb diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go index ef61b06873..54b8c02165 100644 --- a/services/keep-web/handler.go +++ b/services/keep-web/handler.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package keepweb import ( "encoding/json" @@ -23,7 +23,6 @@ import ( "git.arvados.org/arvados.git/sdk/go/arvadosclient" "git.arvados.org/arvados.git/sdk/go/auth" "git.arvados.org/arvados.git/sdk/go/ctxlog" - "git.arvados.org/arvados.git/sdk/go/health" "git.arvados.org/arvados.git/sdk/go/httpserver" "git.arvados.org/arvados.git/sdk/go/keepclient" "github.com/sirupsen/logrus" @@ -31,34 +30,11 @@ import ( ) type handler struct { - Config *Config - MetricsAPI http.Handler - clientPool *arvadosclient.ClientPool - setupOnce sync.Once - healthHandler http.Handler - webdavLS webdav.LockSystem -} - -// parseCollectionIDFromDNSName returns a UUID or PDH if s begins with -// a UUID or URL-encoded PDH; otherwise "". -func parseCollectionIDFromDNSName(s string) string { - // Strip domain. - if i := strings.IndexRune(s, '.'); i >= 0 { - s = s[:i] - } - // Names like {uuid}--collections.example.com serve the same - // purpose as {uuid}.collections.example.com but can reduce - // cost/effort of using [additional] wildcard certificates. - if i := strings.Index(s, "--"); i >= 0 { - s = s[:i] - } - if arvadosclient.UUIDMatch(s) { - return s - } - if pdh := strings.Replace(s, "-", "+", 1); arvadosclient.PDHMatch(pdh) { - return pdh - } - return "" + Cache cache + Cluster *arvados.Cluster + clientPool *arvadosclient.ClientPool + setupOnce sync.Once + webdavLS webdav.LockSystem } var urlPDHDecoder = strings.NewReplacer(" ", "+", "-", "+") @@ -81,16 +57,10 @@ func parseCollectionIDFromURL(s string) string { func (h *handler) setup() { // Errors will be handled at the client pool. - arv, _ := arvados.NewClientFromConfig(h.Config.cluster) + arv, _ := arvados.NewClientFromConfig(h.Cluster) h.clientPool = arvadosclient.MakeClientPoolWith(arv) - keepclient.RefreshServiceDiscoveryOnSIGHUP() - keepclient.DefaultBlockCache.MaxBlocks = h.Config.cluster.Collections.WebDAVCache.MaxBlockEntries - - h.healthHandler = &health.Handler{ - Token: h.Config.cluster.ManagementToken, - Prefix: "/_health/", - } + keepclient.DefaultBlockCache.MaxBlocks = h.Cluster.Collections.WebDAVCache.MaxBlockEntries // Even though we don't accept LOCK requests, every webdav // handler must have a non-nil LockSystem. @@ -195,6 +165,16 @@ func stripDefaultPort(host string) string { } } +// CheckHealth implements service.Handler. +func (h *handler) CheckHealth() error { + return nil +} + +// Done implements service.Handler. +func (h *handler) Done() <-chan struct{} { + return nil +} + // ServeHTTP implements http.Handler. func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { h.setupOnce.Do(h.setup) @@ -205,11 +185,6 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { w := httpserver.WrapResponseWriter(wOrig) - if strings.HasPrefix(r.URL.Path, "/_health/") && r.Method == "GET" { - h.healthHandler.ServeHTTP(w, r) - return - } - if method := r.Header.Get("Access-Control-Request-Method"); method != "" && r.Method == "OPTIONS" { if !browserMethod[method] && !webdavMethod[method] { w.WriteHeader(http.StatusMethodNotAllowed) @@ -250,10 +225,10 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { var pathToken bool var attachment bool var useSiteFS bool - credentialsOK := h.Config.cluster.Collections.TrustAllContent + credentialsOK := h.Cluster.Collections.TrustAllContent reasonNotAcceptingCredentials := "" - if r.Host != "" && stripDefaultPort(r.Host) == stripDefaultPort(h.Config.cluster.Services.WebDAVDownload.ExternalURL.Host) { + if r.Host != "" && stripDefaultPort(r.Host) == stripDefaultPort(h.Cluster.Services.WebDAVDownload.ExternalURL.Host) { credentialsOK = true attachment = true } else if r.FormValue("disposition") == "attachment" { @@ -262,18 +237,15 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { if !credentialsOK { reasonNotAcceptingCredentials = fmt.Sprintf("vhost %q does not specify a single collection ID or match Services.WebDAVDownload.ExternalURL %q, and Collections.TrustAllContent is false", - r.Host, h.Config.cluster.Services.WebDAVDownload.ExternalURL) + r.Host, h.Cluster.Services.WebDAVDownload.ExternalURL) } - if collectionID = parseCollectionIDFromDNSName(r.Host); collectionID != "" { + if collectionID = arvados.CollectionIDFromDNSName(r.Host); collectionID != "" { // http://ID.collections.example/PATH... credentialsOK = true } else if r.URL.Path == "/status.json" { h.serveStatus(w, r) return - } else if strings.HasPrefix(r.URL.Path, "/metrics") { - h.MetricsAPI.ServeHTTP(w, r) - return } else if siteFSDir[pathParts[0]] { useSiteFS = true } else if len(pathParts) >= 1 && strings.HasPrefix(pathParts[0], "c=") { @@ -365,8 +337,8 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { if tokens == nil { tokens = reqTokens - if h.Config.cluster.Users.AnonymousUserToken != "" { - tokens = append(tokens, h.Config.cluster.Users.AnonymousUserToken) + if h.Cluster.Users.AnonymousUserToken != "" { + tokens = append(tokens, h.Cluster.Users.AnonymousUserToken) } } @@ -402,7 +374,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { tokenResult := make(map[string]int) for _, arv.ApiToken = range tokens { var err error - collection, err = h.Config.Cache.Get(arv, collectionID, forceReload) + collection, err = h.Cache.Get(arv, collectionID, forceReload) if err == nil { // Success break @@ -485,8 +457,8 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { } // Check configured permission - _, sess, err := h.Config.Cache.GetSession(arv.ApiToken) - tokenUser, err = h.Config.Cache.GetTokenUser(arv.ApiToken) + _, sess, err := h.Cache.GetSession(arv.ApiToken) + tokenUser, err = h.Cache.GetTokenUser(arv.ApiToken) if webdavMethod[r.Method] { if !h.userPermittedToUploadOrDownload(r.Method, tokenUser) { @@ -504,7 +476,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { ResponseWriter: w, logger: ctxlog.FromContext(r.Context()), update: func() error { - return h.Config.Cache.Update(client, *collection, writefs) + return h.Cache.Update(client, *collection, writefs) }} } h := webdav.Handler{ @@ -601,12 +573,12 @@ func (h *handler) serveSiteFS(w http.ResponseWriter, r *http.Request, tokens []s return } - fs, sess, err := h.Config.Cache.GetSession(tokens[0]) + fs, sess, err := h.Cache.GetSession(tokens[0]) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return } - fs.ForwardSlashNameSubstitution(h.Config.cluster.Collections.ForwardSlashNameSubstitution) + fs.ForwardSlashNameSubstitution(h.Cluster.Collections.ForwardSlashNameSubstitution) f, err := fs.Open(r.URL.Path) if os.IsNotExist(err) { http.Error(w, err.Error(), http.StatusNotFound) @@ -625,7 +597,7 @@ func (h *handler) serveSiteFS(w http.ResponseWriter, r *http.Request, tokens []s return } - tokenUser, err := h.Config.Cache.GetTokenUser(tokens[0]) + tokenUser, err := h.Cache.GetTokenUser(tokens[0]) if !h.userPermittedToUploadOrDownload(r.Method, tokenUser) { http.Error(w, "Not permitted", http.StatusForbidden) return @@ -867,11 +839,11 @@ func (h *handler) userPermittedToUploadOrDownload(method string, tokenUser *arva var permitDownload bool var permitUpload bool if tokenUser != nil && tokenUser.IsAdmin { - permitUpload = h.Config.cluster.Collections.WebDAVPermission.Admin.Upload - permitDownload = h.Config.cluster.Collections.WebDAVPermission.Admin.Download + permitUpload = h.Cluster.Collections.WebDAVPermission.Admin.Upload + permitDownload = h.Cluster.Collections.WebDAVPermission.Admin.Download } else { - permitUpload = h.Config.cluster.Collections.WebDAVPermission.User.Upload - permitDownload = h.Config.cluster.Collections.WebDAVPermission.User.Download + permitUpload = h.Cluster.Collections.WebDAVPermission.User.Upload + permitDownload = h.Cluster.Collections.WebDAVPermission.User.Download } if (method == "PUT" || method == "POST") && !permitUpload { // Disallow operations that upload new files. @@ -903,7 +875,7 @@ func (h *handler) logUploadOrDownload( WithField("user_full_name", user.FullName) useruuid = user.UUID } else { - useruuid = fmt.Sprintf("%s-tpzed-anonymouspublic", h.Config.cluster.ClusterID) + useruuid = fmt.Sprintf("%s-tpzed-anonymouspublic", h.Cluster.ClusterID) } if collection == nil && fs != nil { collection, filepath = h.determineCollection(fs, filepath) @@ -924,7 +896,7 @@ func (h *handler) logUploadOrDownload( } if r.Method == "PUT" || r.Method == "POST" { log.Info("File upload") - if h.Config.cluster.Collections.WebDAVLogEvents { + if h.Cluster.Collections.WebDAVLogEvents { go func() { lr := arvadosclient.Dict{"log": arvadosclient.Dict{ "object_uuid": useruuid, @@ -942,7 +914,7 @@ func (h *handler) logUploadOrDownload( props["portable_data_hash"] = collection.PortableDataHash } log.Info("File download") - if h.Config.cluster.Collections.WebDAVLogEvents { + if h.Cluster.Collections.WebDAVLogEvents { go func() { lr := arvadosclient.Dict{"log": arvadosclient.Dict{ "object_uuid": useruuid, diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go index 6412789ff9..92fea87a01 100644 --- a/services/keep-web/handler_test.go +++ b/services/keep-web/handler_test.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package keepweb import ( "bytes" @@ -27,6 +27,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/prometheus/client_golang/prometheus" "github.com/sirupsen/logrus" check "gopkg.in/check.v1" ) @@ -38,19 +39,31 @@ func init() { } type UnitSuite struct { - Config *arvados.Config + cluster *arvados.Cluster + handler *handler } func (s *UnitSuite) SetUpTest(c *check.C) { - ldr := config.NewLoader(bytes.NewBufferString("Clusters: {zzzzz: {}}"), ctxlog.TestLogger(c)) + logger := ctxlog.TestLogger(c) + ldr := config.NewLoader(bytes.NewBufferString("Clusters: {zzzzz: {}}"), logger) ldr.Path = "-" cfg, err := ldr.Load() c.Assert(err, check.IsNil) - s.Config = cfg + cc, err := cfg.GetCluster("") + c.Assert(err, check.IsNil) + s.cluster = cc + s.handler = &handler{ + Cluster: cc, + Cache: cache{ + cluster: cc, + logger: logger, + registry: prometheus.NewRegistry(), + }, + } } func (s *UnitSuite) TestCORSPreflight(c *check.C) { - h := handler{Config: newConfig(ctxlog.TestLogger(c), s.Config)} + h := s.handler u := mustParseURL("http://keep-web.example/c=" + arvadostest.FooCollection + "/foo") req := &http.Request{ Method: "OPTIONS", @@ -109,7 +122,6 @@ func (s *UnitSuite) TestEmptyResponse(c *check.C) { c.Assert(err, check.IsNil) } - h := handler{Config: newConfig(ctxlog.TestLogger(c), s.Config)} u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/foo") req := &http.Request{ Method: "GET", @@ -130,7 +142,7 @@ func (s *UnitSuite) TestEmptyResponse(c *check.C) { req = req.WithContext(ctxlog.Context(context.Background(), logger)) resp := httptest.NewRecorder() - h.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) c.Check(resp.Code, check.Equals, trial.expectStatus) c.Check(resp.Body.String(), check.Equals, "") @@ -159,10 +171,8 @@ func (s *UnitSuite) TestInvalidUUID(c *check.C) { RequestURI: u.RequestURI(), } resp := httptest.NewRecorder() - cfg := newConfig(ctxlog.TestLogger(c), s.Config) - cfg.cluster.Users.AnonymousUserToken = arvadostest.AnonymousToken - h := handler{Config: cfg} - h.ServeHTTP(resp, req) + s.cluster.Users.AnonymousUserToken = arvadostest.AnonymousToken + s.handler.ServeHTTP(resp, req) c.Check(resp.Code, check.Equals, http.StatusNotFound) } } @@ -187,7 +197,7 @@ func (s *IntegrationSuite) TestVhost404(c *check.C) { URL: u, RequestURI: u.RequestURI(), } - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) c.Check(resp.Code, check.Equals, http.StatusNotFound) c.Check(resp.Body.String(), check.Equals, notFoundMessage+"\n") } @@ -335,7 +345,7 @@ func (s *IntegrationSuite) doVhostRequestsWithHostPath(c *check.C, authz authori func (s *IntegrationSuite) TestVhostPortMatch(c *check.C) { for _, host := range []string{"download.example.com", "DOWNLOAD.EXAMPLE.COM"} { for _, port := range []string{"80", "443", "8000"} { - s.testServer.Config.cluster.Services.WebDAVDownload.ExternalURL.Host = fmt.Sprintf("download.example.com:%v", port) + s.handler.Cluster.Services.WebDAVDownload.ExternalURL.Host = fmt.Sprintf("download.example.com:%v", port) u := mustParseURL(fmt.Sprintf("http://%v/by_id/%v/foo", host, arvadostest.FooCollection)) req := &http.Request{ Method: "GET", @@ -358,7 +368,7 @@ func (s *IntegrationSuite) TestVhostPortMatch(c *check.C) { func (s *IntegrationSuite) doReq(req *http.Request) (*http.Request, *httptest.ResponseRecorder) { resp := httptest.NewRecorder() - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) if resp.Code != http.StatusSeeOther { return req, resp } @@ -453,7 +463,7 @@ func (s *IntegrationSuite) TestVhostRedirectQueryTokenRequestAttachment(c *check } func (s *IntegrationSuite) TestVhostRedirectQueryTokenSiteFS(c *check.C) { - s.testServer.Config.cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com" + s.handler.Cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com" resp := s.testVhostRedirectTokenToCookie(c, "GET", "download.example.com/by_id/"+arvadostest.FooCollection+"/foo", "?api_token="+arvadostest.ActiveToken, @@ -466,7 +476,7 @@ func (s *IntegrationSuite) TestVhostRedirectQueryTokenSiteFS(c *check.C) { } func (s *IntegrationSuite) TestPastCollectionVersionFileAccess(c *check.C) { - s.testServer.Config.cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com" + s.handler.Cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com" resp := s.testVhostRedirectTokenToCookie(c, "GET", "download.example.com/c="+arvadostest.WazVersion1Collection+"/waz", "?api_token="+arvadostest.ActiveToken, @@ -488,7 +498,7 @@ func (s *IntegrationSuite) TestPastCollectionVersionFileAccess(c *check.C) { } func (s *IntegrationSuite) TestVhostRedirectQueryTokenTrustAllContent(c *check.C) { - s.testServer.Config.cluster.Collections.TrustAllContent = true + s.handler.Cluster.Collections.TrustAllContent = true s.testVhostRedirectTokenToCookie(c, "GET", "example.com/c="+arvadostest.FooCollection+"/foo", "?api_token="+arvadostest.ActiveToken, @@ -500,7 +510,7 @@ func (s *IntegrationSuite) TestVhostRedirectQueryTokenTrustAllContent(c *check.C } func (s *IntegrationSuite) TestVhostRedirectQueryTokenAttachmentOnlyHost(c *check.C) { - s.testServer.Config.cluster.Services.WebDAVDownload.ExternalURL.Host = "example.com:1234" + s.handler.Cluster.Services.WebDAVDownload.ExternalURL.Host = "example.com:1234" s.testVhostRedirectTokenToCookie(c, "GET", "example.com/c="+arvadostest.FooCollection+"/foo", @@ -545,7 +555,7 @@ func (s *IntegrationSuite) TestVhostRedirectPOSTFormTokenToCookie404(c *check.C) } func (s *IntegrationSuite) TestAnonymousTokenOK(c *check.C) { - s.testServer.Config.cluster.Users.AnonymousUserToken = arvadostest.AnonymousToken + s.handler.Cluster.Users.AnonymousUserToken = arvadostest.AnonymousToken s.testVhostRedirectTokenToCookie(c, "GET", "example.com/c="+arvadostest.HelloWorldCollection+"/Hello%20world.txt", "", @@ -557,7 +567,7 @@ func (s *IntegrationSuite) TestAnonymousTokenOK(c *check.C) { } func (s *IntegrationSuite) TestAnonymousTokenError(c *check.C) { - s.testServer.Config.cluster.Users.AnonymousUserToken = "anonymousTokenConfiguredButInvalid" + s.handler.Cluster.Users.AnonymousUserToken = "anonymousTokenConfiguredButInvalid" s.testVhostRedirectTokenToCookie(c, "GET", "example.com/c="+arvadostest.HelloWorldCollection+"/Hello%20world.txt", "", @@ -569,11 +579,11 @@ func (s *IntegrationSuite) TestAnonymousTokenError(c *check.C) { } func (s *IntegrationSuite) TestSpecialCharsInPath(c *check.C) { - s.testServer.Config.cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com" + s.handler.Cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com" - client := s.testServer.Config.Client + client := arvados.NewClientFromEnv() client.AuthToken = arvadostest.ActiveToken - fs, err := (&arvados.Collection{}).FileSystem(&client, nil) + fs, err := (&arvados.Collection{}).FileSystem(client, nil) c.Assert(err, check.IsNil) f, err := fs.OpenFile("https:\\\"odd' path chars", os.O_CREATE, 0777) c.Assert(err, check.IsNil) @@ -599,22 +609,22 @@ func (s *IntegrationSuite) TestSpecialCharsInPath(c *check.C) { }, } resp := httptest.NewRecorder() - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) c.Check(resp.Code, check.Equals, http.StatusOK) c.Check(resp.Body.String(), check.Matches, `(?ms).*href="./https:%5c%22odd%27%20path%20chars"\S+https:\\"odd' path chars.*`) } func (s *IntegrationSuite) TestForwardSlashSubstitution(c *check.C) { arv := arvados.NewClientFromEnv() - s.testServer.Config.cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com" - s.testServer.Config.cluster.Collections.ForwardSlashNameSubstitution = "{SOLIDUS}" + s.handler.Cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com" + s.handler.Cluster.Collections.ForwardSlashNameSubstitution = "{SOLIDUS}" name := "foo/bar/baz" nameShown := strings.Replace(name, "/", "{SOLIDUS}", -1) nameShownEscaped := strings.Replace(name, "/", "%7bSOLIDUS%7d", -1) - client := s.testServer.Config.Client + client := arvados.NewClientFromEnv() client.AuthToken = arvadostest.ActiveToken - fs, err := (&arvados.Collection{}).FileSystem(&client, nil) + fs, err := (&arvados.Collection{}).FileSystem(client, nil) c.Assert(err, check.IsNil) f, err := fs.OpenFile("filename", os.O_CREATE, 0777) c.Assert(err, check.IsNil) @@ -648,7 +658,7 @@ func (s *IntegrationSuite) TestForwardSlashSubstitution(c *check.C) { }, } resp := httptest.NewRecorder() - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) c.Check(resp.Code, check.Equals, http.StatusOK) c.Check(resp.Body.String(), check.Matches, expectRegexp) } @@ -676,7 +686,7 @@ func (s *IntegrationSuite) TestXHRNoRedirect(c *check.C) { }.Encode())), } resp := httptest.NewRecorder() - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) c.Check(resp.Code, check.Equals, http.StatusOK) c.Check(resp.Body.String(), check.Equals, "foo") c.Check(resp.Header().Get("Access-Control-Allow-Origin"), check.Equals, "*") @@ -695,7 +705,7 @@ func (s *IntegrationSuite) TestXHRNoRedirect(c *check.C) { }, } resp = httptest.NewRecorder() - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) c.Check(resp.Code, check.Equals, http.StatusOK) c.Check(resp.Body.String(), check.Equals, "foo") c.Check(resp.Header().Get("Access-Control-Allow-Origin"), check.Equals, "*") @@ -718,7 +728,7 @@ func (s *IntegrationSuite) testVhostRedirectTokenToCookie(c *check.C, method, ho c.Check(resp.Body.String(), check.Equals, expectRespBody) }() - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) if resp.Code != http.StatusSeeOther { return resp } @@ -738,23 +748,23 @@ func (s *IntegrationSuite) testVhostRedirectTokenToCookie(c *check.C, method, ho } resp = httptest.NewRecorder() - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) c.Check(resp.Header().Get("Location"), check.Equals, "") return resp } func (s *IntegrationSuite) TestDirectoryListingWithAnonymousToken(c *check.C) { - s.testServer.Config.cluster.Users.AnonymousUserToken = arvadostest.AnonymousToken + s.handler.Cluster.Users.AnonymousUserToken = arvadostest.AnonymousToken s.testDirectoryListing(c) } func (s *IntegrationSuite) TestDirectoryListingWithNoAnonymousToken(c *check.C) { - s.testServer.Config.cluster.Users.AnonymousUserToken = "" + s.handler.Cluster.Users.AnonymousUserToken = "" s.testDirectoryListing(c) } func (s *IntegrationSuite) testDirectoryListing(c *check.C) { - s.testServer.Config.cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com" + s.handler.Cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com" authHeader := http.Header{ "Authorization": {"OAuth2 " + arvadostest.ActiveToken}, } @@ -901,7 +911,7 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) { RequestURI: u.RequestURI(), Header: copyHeader(trial.header), } - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) var cookies []*http.Cookie for resp.Code == http.StatusSeeOther { u, _ := req.URL.Parse(resp.Header().Get("Location")) @@ -917,13 +927,13 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) { req.AddCookie(c) } resp = httptest.NewRecorder() - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) } if trial.redirect != "" { c.Check(req.URL.Path, check.Equals, trial.redirect, comment) } if trial.expect == nil { - if s.testServer.Config.cluster.Users.AnonymousUserToken == "" { + if s.handler.Cluster.Users.AnonymousUserToken == "" { c.Check(resp.Code, check.Equals, http.StatusUnauthorized, comment) } else { c.Check(resp.Code, check.Equals, http.StatusNotFound, comment) @@ -946,9 +956,9 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) { Body: ioutil.NopCloser(&bytes.Buffer{}), } resp = httptest.NewRecorder() - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) if trial.expect == nil { - if s.testServer.Config.cluster.Users.AnonymousUserToken == "" { + if s.handler.Cluster.Users.AnonymousUserToken == "" { c.Check(resp.Code, check.Equals, http.StatusUnauthorized, comment) } else { c.Check(resp.Code, check.Equals, http.StatusNotFound, comment) @@ -966,9 +976,9 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) { Body: ioutil.NopCloser(&bytes.Buffer{}), } resp = httptest.NewRecorder() - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) if trial.expect == nil { - if s.testServer.Config.cluster.Users.AnonymousUserToken == "" { + if s.handler.Cluster.Users.AnonymousUserToken == "" { c.Check(resp.Code, check.Equals, http.StatusUnauthorized, comment) } else { c.Check(resp.Code, check.Equals, http.StatusNotFound, comment) @@ -1003,7 +1013,7 @@ func (s *IntegrationSuite) TestDeleteLastFile(c *check.C) { var updated arvados.Collection for _, fnm := range []string{"foo.txt", "bar.txt"} { - s.testServer.Config.cluster.Services.WebDAVDownload.ExternalURL.Host = "example.com" + s.handler.Cluster.Services.WebDAVDownload.ExternalURL.Host = "example.com" u, _ := url.Parse("http://example.com/c=" + newCollection.UUID + "/" + fnm) req := &http.Request{ Method: "DELETE", @@ -1015,7 +1025,7 @@ func (s *IntegrationSuite) TestDeleteLastFile(c *check.C) { }, } resp := httptest.NewRecorder() - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) c.Check(resp.Code, check.Equals, http.StatusNoContent) updated = arvados.Collection{} @@ -1027,38 +1037,17 @@ func (s *IntegrationSuite) TestDeleteLastFile(c *check.C) { c.Check(updated.ManifestText, check.Equals, "") } -func (s *IntegrationSuite) TestHealthCheckPing(c *check.C) { - s.testServer.Config.cluster.ManagementToken = arvadostest.ManagementToken - authHeader := http.Header{ - "Authorization": {"Bearer " + arvadostest.ManagementToken}, - } - - resp := httptest.NewRecorder() - u := mustParseURL("http://download.example.com/_health/ping") - req := &http.Request{ - Method: "GET", - Host: u.Host, - URL: u, - RequestURI: u.RequestURI(), - Header: authHeader, - } - s.testServer.Handler.ServeHTTP(resp, req) - - c.Check(resp.Code, check.Equals, http.StatusOK) - c.Check(resp.Body.String(), check.Matches, `{"health":"OK"}\n`) -} - func (s *IntegrationSuite) TestFileContentType(c *check.C) { - s.testServer.Config.cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com" + s.handler.Cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com" - client := s.testServer.Config.Client + client := arvados.NewClientFromEnv() client.AuthToken = arvadostest.ActiveToken - arv, err := arvadosclient.New(&client) + arv, err := arvadosclient.New(client) c.Assert(err, check.Equals, nil) kc, err := keepclient.MakeKeepClient(arv) c.Assert(err, check.Equals, nil) - fs, err := (&arvados.Collection{}).FileSystem(&client, kc) + fs, err := (&arvados.Collection{}).FileSystem(client, kc) c.Assert(err, check.IsNil) trials := []struct { @@ -1101,7 +1090,7 @@ func (s *IntegrationSuite) TestFileContentType(c *check.C) { }, } resp := httptest.NewRecorder() - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) c.Check(resp.Code, check.Equals, http.StatusOK) c.Check(resp.Header().Get("Content-Type"), check.Matches, trial.contentType) c.Check(resp.Body.String(), check.Equals, trial.content) @@ -1109,7 +1098,7 @@ func (s *IntegrationSuite) TestFileContentType(c *check.C) { } func (s *IntegrationSuite) TestKeepClientBlockCache(c *check.C) { - s.testServer.Config.cluster.Collections.WebDAVCache.MaxBlockEntries = 42 + s.handler.Cluster.Collections.WebDAVCache.MaxBlockEntries = 42 c.Check(keepclient.DefaultBlockCache.MaxBlocks, check.Not(check.Equals), 42) u := mustParseURL("http://keep-web.example/c=" + arvadostest.FooCollection + "/t=" + arvadostest.ActiveToken + "/foo") req := &http.Request{ @@ -1119,7 +1108,7 @@ func (s *IntegrationSuite) TestKeepClientBlockCache(c *check.C) { RequestURI: u.RequestURI(), } resp := httptest.NewRecorder() - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) c.Check(resp.Code, check.Equals, http.StatusOK) c.Check(keepclient.DefaultBlockCache.MaxBlocks, check.Equals, 42) } @@ -1144,7 +1133,7 @@ func (s *IntegrationSuite) TestCacheWriteCollectionSamePDH(c *check.C) { req.URL.Host = strings.Replace(id, "+", "-", -1) + ".example" req.Host = req.URL.Host resp := httptest.NewRecorder() - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) c.Check(resp.Code, check.Equals, status) } @@ -1170,7 +1159,7 @@ func (s *IntegrationSuite) TestCacheWriteCollectionSamePDH(c *check.C) { reqPut.Host = req.URL.Host reqPut.Body = ioutil.NopCloser(bytes.NewBufferString("testdata")) resp := httptest.NewRecorder() - s.testServer.Handler.ServeHTTP(resp, &reqPut) + s.handler.ServeHTTP(resp, &reqPut) c.Check(resp.Code, check.Equals, http.StatusCreated) // new file should not appear in colls[1] @@ -1188,10 +1177,10 @@ func copyHeader(h http.Header) http.Header { return hc } -func (s *IntegrationSuite) checkUploadDownloadRequest(c *check.C, h *handler, req *http.Request, +func (s *IntegrationSuite) checkUploadDownloadRequest(c *check.C, req *http.Request, successCode int, direction string, perm bool, userUuid string, collectionUuid string, filepath string) { - client := s.testServer.Config.Client + client := arvados.NewClientFromEnv() client.AuthToken = arvadostest.AdminToken var logentries arvados.LogList limit1 := 1 @@ -1208,7 +1197,7 @@ func (s *IntegrationSuite) checkUploadDownloadRequest(c *check.C, h *handler, re logger.Out = &logbuf resp := httptest.NewRecorder() req = req.WithContext(ctxlog.Context(context.Background(), logger)) - h.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) if perm { c.Check(resp.Result().StatusCode, check.Equals, successCode) @@ -1245,16 +1234,14 @@ func (s *IntegrationSuite) checkUploadDownloadRequest(c *check.C, h *handler, re } func (s *IntegrationSuite) TestDownloadLoggingPermission(c *check.C) { - config := newConfig(ctxlog.TestLogger(c), s.ArvConfig) - h := handler{Config: config} u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/foo") - config.cluster.Collections.TrustAllContent = true + s.handler.Cluster.Collections.TrustAllContent = true for _, adminperm := range []bool{true, false} { for _, userperm := range []bool{true, false} { - config.cluster.Collections.WebDAVPermission.Admin.Download = adminperm - config.cluster.Collections.WebDAVPermission.User.Download = userperm + s.handler.Cluster.Collections.WebDAVPermission.Admin.Download = adminperm + s.handler.Cluster.Collections.WebDAVPermission.User.Download = userperm // Test admin permission req := &http.Request{ @@ -1266,7 +1253,7 @@ func (s *IntegrationSuite) TestDownloadLoggingPermission(c *check.C) { "Authorization": {"Bearer " + arvadostest.AdminToken}, }, } - s.checkUploadDownloadRequest(c, &h, req, http.StatusOK, "download", adminperm, + s.checkUploadDownloadRequest(c, req, http.StatusOK, "download", adminperm, arvadostest.AdminUserUUID, arvadostest.FooCollection, "foo") // Test user permission @@ -1279,12 +1266,12 @@ func (s *IntegrationSuite) TestDownloadLoggingPermission(c *check.C) { "Authorization": {"Bearer " + arvadostest.ActiveToken}, }, } - s.checkUploadDownloadRequest(c, &h, req, http.StatusOK, "download", userperm, + s.checkUploadDownloadRequest(c, req, http.StatusOK, "download", userperm, arvadostest.ActiveUserUUID, arvadostest.FooCollection, "foo") } } - config.cluster.Collections.WebDAVPermission.User.Download = true + s.handler.Cluster.Collections.WebDAVPermission.User.Download = true for _, tryurl := range []string{"http://" + arvadostest.MultilevelCollection1 + ".keep-web.example/dir1/subdir/file1", "http://keep-web/users/active/multilevel_collection_1/dir1/subdir/file1"} { @@ -1299,7 +1286,7 @@ func (s *IntegrationSuite) TestDownloadLoggingPermission(c *check.C) { "Authorization": {"Bearer " + arvadostest.ActiveToken}, }, } - s.checkUploadDownloadRequest(c, &h, req, http.StatusOK, "download", true, + s.checkUploadDownloadRequest(c, req, http.StatusOK, "download", true, arvadostest.ActiveUserUUID, arvadostest.MultilevelCollection1, "dir1/subdir/file1") } @@ -1313,18 +1300,15 @@ func (s *IntegrationSuite) TestDownloadLoggingPermission(c *check.C) { "Authorization": {"Bearer " + arvadostest.ActiveToken}, }, } - s.checkUploadDownloadRequest(c, &h, req, http.StatusOK, "download", true, + s.checkUploadDownloadRequest(c, req, http.StatusOK, "download", true, arvadostest.ActiveUserUUID, arvadostest.FooCollection, "foo") } func (s *IntegrationSuite) TestUploadLoggingPermission(c *check.C) { - config := newConfig(ctxlog.TestLogger(c), s.ArvConfig) - h := handler{Config: config} - for _, adminperm := range []bool{true, false} { for _, userperm := range []bool{true, false} { - arv := s.testServer.Config.Client + arv := arvados.NewClientFromEnv() arv.AuthToken = arvadostest.ActiveToken var coll arvados.Collection @@ -1342,8 +1326,8 @@ func (s *IntegrationSuite) TestUploadLoggingPermission(c *check.C) { u := mustParseURL("http://" + coll.UUID + ".keep-web.example/bar") - config.cluster.Collections.WebDAVPermission.Admin.Upload = adminperm - config.cluster.Collections.WebDAVPermission.User.Upload = userperm + s.handler.Cluster.Collections.WebDAVPermission.Admin.Upload = adminperm + s.handler.Cluster.Collections.WebDAVPermission.User.Upload = userperm // Test admin permission req := &http.Request{ @@ -1356,7 +1340,7 @@ func (s *IntegrationSuite) TestUploadLoggingPermission(c *check.C) { }, Body: io.NopCloser(bytes.NewReader([]byte("bar"))), } - s.checkUploadDownloadRequest(c, &h, req, http.StatusCreated, "upload", adminperm, + s.checkUploadDownloadRequest(c, req, http.StatusCreated, "upload", adminperm, arvadostest.AdminUserUUID, coll.UUID, "bar") // Test user permission @@ -1370,7 +1354,7 @@ func (s *IntegrationSuite) TestUploadLoggingPermission(c *check.C) { }, Body: io.NopCloser(bytes.NewReader([]byte("bar"))), } - s.checkUploadDownloadRequest(c, &h, req, http.StatusCreated, "upload", userperm, + s.checkUploadDownloadRequest(c, req, http.StatusCreated, "upload", userperm, arvadostest.ActiveUserUUID, coll.UUID, "bar") } } diff --git a/services/keep-web/main.go b/services/keep-web/main.go index 208b23b93b..7a23cd1fad 100644 --- a/services/keep-web/main.go +++ b/services/keep-web/main.go @@ -2,129 +2,48 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package keepweb import ( "context" - "flag" - "fmt" "mime" "os" - "git.arvados.org/arvados.git/lib/cmd" - "git.arvados.org/arvados.git/lib/config" + "git.arvados.org/arvados.git/lib/service" "git.arvados.org/arvados.git/sdk/go/arvados" "git.arvados.org/arvados.git/sdk/go/ctxlog" - "github.com/coreos/go-systemd/daemon" - "github.com/ghodss/yaml" - "github.com/sirupsen/logrus" - log "github.com/sirupsen/logrus" + "git.arvados.org/arvados.git/sdk/go/keepclient" + "github.com/prometheus/client_golang/prometheus" ) var ( version = "dev" ) -// Config specifies server configuration. -type Config struct { - Client arvados.Client - Cache cache - cluster *arvados.Cluster -} - -func newConfig(logger logrus.FieldLogger, arvCfg *arvados.Config) *Config { - cfg := Config{} - var cls *arvados.Cluster - var err error - if cls, err = arvCfg.GetCluster(""); err != nil { - log.Fatal(err) - } - cfg.cluster = cls - cfg.Cache.config = &cfg.cluster.Collections.WebDAVCache - cfg.Cache.cluster = cls - cfg.Cache.logger = logger - return &cfg -} - -func init() { - // MakeArvadosClient returns an error if this env var isn't - // available as a default token (even if we explicitly set a - // different token before doing anything with the client). We - // set this dummy value during init so it doesn't clobber the - // one used by "run test servers". - if os.Getenv("ARVADOS_API_TOKEN") == "" { - os.Setenv("ARVADOS_API_TOKEN", "xxx") - } +var Command = service.Command(arvados.ServiceNameKeepweb, newHandlerOrErrorHandler) - log.SetFormatter(&log.JSONFormatter{ - TimestampFormat: "2006-01-02T15:04:05.000000000Z07:00", - }) -} - -func configure(logger log.FieldLogger, args []string) (*Config, error) { - flags := flag.NewFlagSet(args[0], flag.ContinueOnError) - - loader := config.NewLoader(os.Stdin, logger) - loader.SetupFlags(flags) - - dumpConfig := flags.Bool("dump-config", false, - "write current configuration to stdout and exit") - getVersion := flags.Bool("version", false, - "print version information and exit.") - - prog := args[0] - args = loader.MungeLegacyConfigArgs(logger, args[1:], "-legacy-keepweb-config") - if ok, code := cmd.ParseFlags(flags, prog, args, "", os.Stderr); !ok { - os.Exit(code) - } else if *getVersion { - fmt.Printf("%s %s\n", args[0], version) - return nil, nil - } - - arvCfg, err := loader.Load() +func newHandlerOrErrorHandler(ctx context.Context, cluster *arvados.Cluster, token string, reg *prometheus.Registry) service.Handler { + h, err := newHandler(ctx, cluster, token, reg) if err != nil { - return nil, err - } - cfg := newConfig(logger, arvCfg) - - if *dumpConfig { - out, err := yaml.Marshal(cfg) - if err != nil { - return nil, err - } - _, err = os.Stdout.Write(out) - return nil, err + return service.ErrorHandler(ctx, cluster, err) } - return cfg, nil + return h } -func main() { - initLogger := log.StandardLogger() - logger := initLogger.WithField("PID", os.Getpid()) - cfg, err := configure(logger, os.Args) - if err != nil { - log.Fatal(err) - } else if cfg == nil { - return - } - logger = logger.WithField("ClusterID", cfg.cluster.ClusterID) - logger.Printf("keep-web %s started", version) - ctx := ctxlog.Context(context.Background(), logger) - +func newHandler(ctx context.Context, cluster *arvados.Cluster, token string, reg *prometheus.Registry) (*handler, error) { + logger := ctxlog.FromContext(ctx) if ext := ".txt"; mime.TypeByExtension(ext) == "" { - log.Warnf("cannot look up MIME type for %q -- this probably means /etc/mime.types is missing -- clients will see incorrect content types", ext) - } - - os.Setenv("ARVADOS_API_HOST", cfg.cluster.Services.Controller.ExternalURL.Host) - srv := &server{Config: cfg} - if err := srv.Start(ctx, initLogger); err != nil { - logger.Fatal(err) - } - if _, err := daemon.SdNotify(false, "READY=1"); err != nil { - logger.Printf("Error notifying init daemon: %v", err) - } - logger.Println("Listening at", srv.Addr) - if err := srv.Wait(); err != nil { - logger.Fatal(err) - } + logger.Warnf("cannot look up MIME type for %q -- this probably means /etc/mime.types is missing -- clients will see incorrect content types", ext) + } + + keepclient.RefreshServiceDiscoveryOnSIGHUP() + os.Setenv("ARVADOS_API_HOST", cluster.Services.Controller.ExternalURL.Host) + return &handler{ + Cluster: cluster, + Cache: cache{ + cluster: cluster, + logger: logger, + registry: reg, + }, + }, nil } diff --git a/services/keep-web/ranges_test.go b/services/keep-web/ranges_test.go index c91edd87c5..8e7dbadc4e 100644 --- a/services/keep-web/ranges_test.go +++ b/services/keep-web/ranges_test.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package keepweb import ( "fmt" @@ -78,7 +78,7 @@ func (s *IntegrationSuite) TestRanges(c *check.C) { "Range": {"bytes=" + trial.header}, }, } - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) if trial.expectObey { c.Check(resp.Code, check.Equals, http.StatusPartialContent) c.Check(resp.Body.Len(), check.Equals, len(trial.expectBody)) diff --git a/services/keep-web/s3.go b/services/keep-web/s3.go index 5af7ebb5d5..59ab3cd438 100644 --- a/services/keep-web/s3.go +++ b/services/keep-web/s3.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package keepweb import ( "crypto/hmac" @@ -222,8 +222,8 @@ func (h *handler) checks3signature(r *http.Request) (string, error) { } client := (&arvados.Client{ - APIHost: h.Config.cluster.Services.Controller.ExternalURL.Host, - Insecure: h.Config.cluster.TLS.Insecure, + APIHost: h.Cluster.Services.Controller.ExternalURL.Host, + Insecure: h.Cluster.TLS.Insecure, }).WithRequestID(r.Header.Get("X-Request-Id")) var aca arvados.APIClientAuthorization var secret string @@ -231,7 +231,7 @@ func (h *handler) checks3signature(r *http.Request) (string, error) { if len(key) == 27 && key[5:12] == "-gj3su-" { // Access key is the UUID of an Arvados token, secret // key is the secret part. - ctx := arvados.ContextWithAuthorization(r.Context(), "Bearer "+h.Config.cluster.SystemRootToken) + ctx := arvados.ContextWithAuthorization(r.Context(), "Bearer "+h.Cluster.SystemRootToken) err = client.RequestAndDecodeContext(ctx, &aca, "GET", "arvados/v1/api_client_authorizations/"+key, nil, nil) secret = aca.APIToken } else { @@ -316,7 +316,7 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool { // Use a single session (cached FileSystem) across // multiple read requests. var sess *cachedSession - fs, sess, err = h.Config.Cache.GetSession(token) + fs, sess, err = h.Cache.GetSession(token) if err != nil { s3ErrorResponse(w, InternalError, err.Error(), r.URL.Path, http.StatusInternalServerError) return true @@ -336,13 +336,13 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool { } defer release() fs = client.SiteFileSystem(kc) - fs.ForwardSlashNameSubstitution(h.Config.cluster.Collections.ForwardSlashNameSubstitution) + fs.ForwardSlashNameSubstitution(h.Cluster.Collections.ForwardSlashNameSubstitution) } var objectNameGiven bool var bucketName string fspath := "/by_id" - if id := parseCollectionIDFromDNSName(r.Host); id != "" { + if id := arvados.CollectionIDFromDNSName(r.Host); id != "" { fspath += "/" + id bucketName = id objectNameGiven = strings.Count(strings.TrimSuffix(r.URL.Path, "/"), "/") > 0 @@ -365,7 +365,7 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool { w.Header().Set("Content-Type", "application/xml") io.WriteString(w, xml.Header) fmt.Fprintln(w, ``+ - h.Config.cluster.ClusterID+ + h.Cluster.ClusterID+ ``) } else if reRawQueryIndicatesAPI.MatchString(r.URL.RawQuery) { // GetBucketWebsite ("GET /bucketid/?website"), GetBucketTagging, etc. @@ -393,7 +393,7 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool { } return true } - if err == nil && fi.IsDir() && objectNameGiven && strings.HasSuffix(fspath, "/") && h.Config.cluster.Collections.S3FolderObjects { + if err == nil && fi.IsDir() && objectNameGiven && strings.HasSuffix(fspath, "/") && h.Cluster.Collections.S3FolderObjects { w.Header().Set("Content-Type", "application/x-directory") w.WriteHeader(http.StatusOK) return true @@ -405,7 +405,7 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool { return true } - tokenUser, err := h.Config.Cache.GetTokenUser(token) + tokenUser, err := h.Cache.GetTokenUser(token) if !h.userPermittedToUploadOrDownload(r.Method, tokenUser) { http.Error(w, "Not permitted", http.StatusForbidden) return true @@ -429,7 +429,7 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool { } var objectIsDir bool if strings.HasSuffix(fspath, "/") { - if !h.Config.cluster.Collections.S3FolderObjects { + if !h.Cluster.Collections.S3FolderObjects { s3ErrorResponse(w, InvalidArgument, "invalid object name: trailing slash", r.URL.Path, http.StatusBadRequest) return true } @@ -496,7 +496,7 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool { } defer f.Close() - tokenUser, err := h.Config.Cache.GetTokenUser(token) + tokenUser, err := h.Cache.GetTokenUser(token) if !h.userPermittedToUploadOrDownload(r.Method, tokenUser) { http.Error(w, "Not permitted", http.StatusForbidden) return true @@ -523,7 +523,7 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool { return true } // Ensure a subsequent read operation will see the changes. - h.Config.Cache.ResetSession(token) + h.Cache.ResetSession(token) w.WriteHeader(http.StatusOK) return true case r.Method == http.MethodDelete: @@ -577,7 +577,7 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool { return true } // Ensure a subsequent read operation will see the changes. - h.Config.Cache.ResetSession(token) + h.Cache.ResetSession(token) w.WriteHeader(http.StatusNoContent) return true default: @@ -756,7 +756,7 @@ func (h *handler) s3list(bucket string, w http.ResponseWriter, r *http.Request, if path < params.marker || path < params.prefix || path <= params.startAfter { return nil } - if fi.IsDir() && !h.Config.cluster.Collections.S3FolderObjects { + if fi.IsDir() && !h.Cluster.Collections.S3FolderObjects { // Note we don't add anything to // commonPrefixes here even if delimiter is // "/". We descend into the directory, and diff --git a/services/keep-web/s3_test.go b/services/keep-web/s3_test.go index 966f39c28d..261ebb5741 100644 --- a/services/keep-web/s3_test.go +++ b/services/keep-web/s3_test.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package keepweb import ( "bytes" @@ -82,7 +82,7 @@ func (s *IntegrationSuite) s3setup(c *check.C) s3stage { auth := aws.NewAuth(arvadostest.ActiveTokenUUID, arvadostest.ActiveToken, "", time.Now().Add(time.Hour)) region := aws.Region{ Name: "zzzzz", - S3Endpoint: "http://" + s.testServer.Addr, + S3Endpoint: s.testServer.URL, } client := s3.New(*auth, region) client.Signature = aws.V4Signature @@ -282,7 +282,7 @@ func (s *IntegrationSuite) testS3PutObjectSuccess(c *check.C, bucket *s3.Bucket, c.Check(err, check.IsNil) rdr, err := bucket.GetReader(objname) - if strings.HasSuffix(trial.path, "/") && !s.testServer.Config.cluster.Collections.S3FolderObjects { + if strings.HasSuffix(trial.path, "/") && !s.handler.Cluster.Collections.S3FolderObjects { c.Check(err, check.NotNil) continue } else if !c.Check(err, check.IsNil) { @@ -352,7 +352,7 @@ func (s *IntegrationSuite) TestS3ProjectDeleteObject(c *check.C) { s.testS3DeleteObject(c, stage.projbucket, stage.coll.Name+"/") } func (s *IntegrationSuite) testS3DeleteObject(c *check.C, bucket *s3.Bucket, prefix string) { - s.testServer.Config.cluster.Collections.S3FolderObjects = true + s.handler.Cluster.Collections.S3FolderObjects = true for _, trial := range []struct { path string }{ @@ -389,7 +389,7 @@ func (s *IntegrationSuite) TestS3ProjectPutObjectFailure(c *check.C) { s.testS3PutObjectFailure(c, stage.projbucket, stage.coll.Name+"/") } func (s *IntegrationSuite) testS3PutObjectFailure(c *check.C, bucket *s3.Bucket, prefix string) { - s.testServer.Config.cluster.Collections.S3FolderObjects = false + s.handler.Cluster.Collections.S3FolderObjects = false var wg sync.WaitGroup for _, trial := range []struct { @@ -540,7 +540,7 @@ func (s *IntegrationSuite) TestS3VirtualHostStyleRequests(c *check.C) { c.Assert(err, check.IsNil) s.sign(c, req, arvadostest.ActiveTokenUUID, arvadostest.ActiveToken) rr := httptest.NewRecorder() - s.testServer.Server.Handler.ServeHTTP(rr, req) + s.handler.ServeHTTP(rr, req) resp := rr.Result() c.Check(resp.StatusCode, check.Equals, trial.responseCode) body, err := ioutil.ReadAll(resp.Body) @@ -710,7 +710,7 @@ func (s *IntegrationSuite) TestS3CollectionList(c *check.C) { defer stage.teardown(c) var markers int - for markers, s.testServer.Config.cluster.Collections.S3FolderObjects = range []bool{false, true} { + for markers, s.handler.Cluster.Collections.S3FolderObjects = range []bool{false, true} { dirs := 2 filesPerDir := 1001 stage.writeBigDirs(c, dirs, filesPerDir) @@ -725,7 +725,7 @@ func (s *IntegrationSuite) TestS3CollectionList(c *check.C) { } } func (s *IntegrationSuite) testS3List(c *check.C, bucket *s3.Bucket, prefix string, pageSize, expectFiles int) { - c.Logf("testS3List: prefix=%q pageSize=%d S3FolderObjects=%v", prefix, pageSize, s.testServer.Config.cluster.Collections.S3FolderObjects) + c.Logf("testS3List: prefix=%q pageSize=%d S3FolderObjects=%v", prefix, pageSize, s.handler.Cluster.Collections.S3FolderObjects) expectPageSize := pageSize if expectPageSize > 1000 { expectPageSize = 1000 @@ -761,7 +761,7 @@ func (s *IntegrationSuite) testS3List(c *check.C, bucket *s3.Bucket, prefix stri } func (s *IntegrationSuite) TestS3CollectionListRollup(c *check.C) { - for _, s.testServer.Config.cluster.Collections.S3FolderObjects = range []bool{false, true} { + for _, s.handler.Cluster.Collections.S3FolderObjects = range []bool{false, true} { s.testS3CollectionListRollup(c) } } @@ -790,7 +790,7 @@ func (s *IntegrationSuite) testS3CollectionListRollup(c *check.C) { } } markers := 0 - if s.testServer.Config.cluster.Collections.S3FolderObjects { + if s.handler.Cluster.Collections.S3FolderObjects { markers = 1 } c.Check(allfiles, check.HasLen, dirs*(filesPerDir+markers)+3+markers) @@ -903,7 +903,7 @@ func (s *IntegrationSuite) TestS3ListObjectsV2(c *check.C) { sess := aws_session.Must(aws_session.NewSession(&aws_aws.Config{ Region: aws_aws.String("auto"), - Endpoint: aws_aws.String("http://" + s.testServer.Addr), + Endpoint: aws_aws.String(s.testServer.URL), Credentials: aws_credentials.NewStaticCredentials(url.QueryEscape(arvadostest.ActiveTokenV2), url.QueryEscape(arvadostest.ActiveTokenV2), ""), S3ForcePathStyle: aws_aws.Bool(true), })) @@ -1049,7 +1049,7 @@ func (s *IntegrationSuite) TestS3ListObjectsV2EncodingTypeURL(c *check.C) { sess := aws_session.Must(aws_session.NewSession(&aws_aws.Config{ Region: aws_aws.String("auto"), - Endpoint: aws_aws.String("http://" + s.testServer.Addr), + Endpoint: aws_aws.String(s.testServer.URL), Credentials: aws_credentials.NewStaticCredentials(url.QueryEscape(arvadostest.ActiveTokenV2), url.QueryEscape(arvadostest.ActiveTokenV2), ""), S3ForcePathStyle: aws_aws.Bool(true), })) @@ -1097,7 +1097,7 @@ func (s *IntegrationSuite) TestS3cmd(c *check.C) { stage := s.s3setup(c) defer stage.teardown(c) - cmd := exec.Command("s3cmd", "--no-ssl", "--host="+s.testServer.Addr, "--host-bucket="+s.testServer.Addr, "--access_key="+arvadostest.ActiveTokenUUID, "--secret_key="+arvadostest.ActiveToken, "ls", "s3://"+arvadostest.FooCollection) + cmd := exec.Command("s3cmd", "--no-ssl", "--host="+s.testServer.URL[7:], "--host-bucket="+s.testServer.URL[7:], "--access_key="+arvadostest.ActiveTokenUUID, "--secret_key="+arvadostest.ActiveToken, "ls", "s3://"+arvadostest.FooCollection) buf, err := cmd.CombinedOutput() c.Check(err, check.IsNil) c.Check(string(buf), check.Matches, `.* 3 +s3://`+arvadostest.FooCollection+`/foo\n`) @@ -1106,7 +1106,8 @@ func (s *IntegrationSuite) TestS3cmd(c *check.C) { // keep-web's signature verification wrt chars like "|" // (neither reserved nor unreserved) and "," (not normally // percent-encoded in a path). - cmd = exec.Command("s3cmd", "--no-ssl", "--host="+s.testServer.Addr, "--host-bucket="+s.testServer.Addr, "--access_key="+arvadostest.ActiveTokenUUID, "--secret_key="+arvadostest.ActiveToken, "get", "s3://"+arvadostest.FooCollection+"/foo,;$[|]bar") + tmpfile := c.MkDir() + "/dstfile" + cmd = exec.Command("s3cmd", "--no-ssl", "--host="+s.testServer.URL[7:], "--host-bucket="+s.testServer.URL[7:], "--access_key="+arvadostest.ActiveTokenUUID, "--secret_key="+arvadostest.ActiveToken, "get", "s3://"+arvadostest.FooCollection+"/foo,;$[|]bar", tmpfile) buf, err = cmd.CombinedOutput() c.Check(err, check.NotNil) c.Check(string(buf), check.Matches, `(?ms).*NoSuchKey.*\n`) diff --git a/services/keep-web/s3aws_test.go b/services/keep-web/s3aws_test.go index d528dbaf79..145d987bf6 100644 --- a/services/keep-web/s3aws_test.go +++ b/services/keep-web/s3aws_test.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package keepweb import ( "bytes" @@ -31,7 +31,7 @@ func (s *IntegrationSuite) TestS3AWSSDK(c *check.C) { cfg.EndpointResolver = aws.EndpointResolverFunc(func(service, region string) (aws.Endpoint, error) { if service == "s3" { return aws.Endpoint{ - URL: "http://" + s.testServer.Addr, + URL: s.testServer.URL, SigningRegion: "custom-signing-region", }, nil } diff --git a/services/keep-web/server.go b/services/keep-web/server.go deleted file mode 100644 index 24f1b77be2..0000000000 --- a/services/keep-web/server.go +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright (C) The Arvados Authors. All rights reserved. -// -// SPDX-License-Identifier: AGPL-3.0 - -package main - -import ( - "context" - "net" - "net/http" - - "git.arvados.org/arvados.git/sdk/go/arvados" - "git.arvados.org/arvados.git/sdk/go/httpserver" - "github.com/prometheus/client_golang/prometheus" - "github.com/sirupsen/logrus" -) - -type server struct { - httpserver.Server - Config *Config -} - -func (srv *server) Start(ctx context.Context, logger *logrus.Logger) error { - h := &handler{Config: srv.Config} - reg := prometheus.NewRegistry() - h.Config.Cache.registry = reg - // Warning: when updating this to use Command() from - // lib/service, make sure to implement an exemption in - // httpserver.HandlerWithDeadline() so large file uploads are - // allowed to take longer than the usual API.RequestTimeout. - // See #13697. - mh := httpserver.Instrument(reg, logger, httpserver.AddRequestIDs(httpserver.LogRequests(h))) - h.MetricsAPI = mh.ServeAPI(h.Config.cluster.ManagementToken, http.NotFoundHandler()) - srv.Handler = mh - srv.BaseContext = func(net.Listener) context.Context { return ctx } - var listen arvados.URL - for listen = range srv.Config.cluster.Services.WebDAV.InternalURLs { - break - } - if len(srv.Config.cluster.Services.WebDAV.InternalURLs) > 1 { - logrus.Warn("Services.WebDAV.InternalURLs has more than one key; picked: ", listen) - } - srv.Addr = listen.Host - return srv.Server.Start() -} diff --git a/services/keep-web/server_test.go b/services/keep-web/server_test.go index 2d28dbc5d7..dd8ce06172 100644 --- a/services/keep-web/server_test.go +++ b/services/keep-web/server_test.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package keepweb import ( "bytes" @@ -14,16 +14,20 @@ import ( "io/ioutil" "net" "net/http" + "net/http/httptest" "os" "os/exec" + "regexp" "strings" "testing" + "time" "git.arvados.org/arvados.git/lib/config" "git.arvados.org/arvados.git/sdk/go/arvados" "git.arvados.org/arvados.git/sdk/go/arvadosclient" "git.arvados.org/arvados.git/sdk/go/arvadostest" "git.arvados.org/arvados.git/sdk/go/ctxlog" + "git.arvados.org/arvados.git/sdk/go/httpserver" "git.arvados.org/arvados.git/sdk/go/keepclient" check "gopkg.in/check.v1" ) @@ -34,8 +38,8 @@ var _ = check.Suite(&IntegrationSuite{}) // IntegrationSuite tests need an API server and a keep-web server type IntegrationSuite struct { - testServer *server - ArvConfig *arvados.Config + testServer *httptest.Server + handler *handler } func (s *IntegrationSuite) TestNoToken(c *check.C) { @@ -153,7 +157,7 @@ type curlCase struct { } func (s *IntegrationSuite) Test200(c *check.C) { - s.testServer.Config.cluster.Users.AnonymousUserToken = arvadostest.AnonymousToken + s.handler.Cluster.Users.AnonymousUserToken = arvadostest.AnonymousToken for _, spec := range []curlCase{ // My collection { @@ -261,7 +265,7 @@ func (s *IntegrationSuite) Test200(c *check.C) { // Return header block and body. func (s *IntegrationSuite) runCurl(c *check.C, auth, host, uri string, args ...string) (hdr, bodyPart string, bodySize int64) { curlArgs := []string{"--silent", "--show-error", "--include"} - testHost, testPort, _ := net.SplitHostPort(s.testServer.Addr) + testHost, testPort, _ := net.SplitHostPort(s.testServer.URL[7:]) curlArgs = append(curlArgs, "--resolve", host+":"+testPort+":"+testHost) if strings.Contains(auth, " ") { // caller supplied entire Authorization header value @@ -306,19 +310,97 @@ func (s *IntegrationSuite) runCurl(c *check.C, auth, host, uri string, args ...s return } +// Run a full-featured server, including the metrics/health routes +// that are added by service.Command. +func (s *IntegrationSuite) runServer(c *check.C) (cluster arvados.Cluster, srvaddr string, logbuf *bytes.Buffer) { + logbuf = &bytes.Buffer{} + cluster = *s.handler.Cluster + cluster.Services.WebDAV.InternalURLs = map[arvados.URL]arvados.ServiceInstance{{Scheme: "http", Host: "0.0.0.0:0"}: {}} + cluster.Services.WebDAVDownload.InternalURLs = map[arvados.URL]arvados.ServiceInstance{{Scheme: "http", Host: "0.0.0.0:0"}: {}} + + var configjson bytes.Buffer + json.NewEncoder(&configjson).Encode(arvados.Config{Clusters: map[string]arvados.Cluster{"zzzzz": cluster}}) + go Command.RunCommand("keep-web", []string{"-config=-"}, &configjson, os.Stderr, io.MultiWriter(os.Stderr, logbuf)) + for deadline := time.Now().Add(time.Second); deadline.After(time.Now()); time.Sleep(time.Second / 100) { + if m := regexp.MustCompile(`"Listen":"(.*?)"`).FindStringSubmatch(logbuf.String()); m != nil { + srvaddr = "http://" + m[1] + break + } + } + if srvaddr == "" { + c.Fatal("timed out") + } + return +} + +// Ensure uploads can take longer than API.RequestTimeout. +// +// Currently, this works only by accident: service.Command cancels the +// request context as usual (there is no exemption), but +// webdav.Handler doesn't notice if the request context is cancelled +// while waiting to send or receive file data. +func (s *IntegrationSuite) TestRequestTimeoutExemption(c *check.C) { + s.handler.Cluster.API.RequestTimeout = arvados.Duration(time.Second / 2) + _, srvaddr, _ := s.runServer(c) + + var coll arvados.Collection + arv, err := arvadosclient.MakeArvadosClient() + c.Assert(err, check.IsNil) + arv.ApiToken = arvadostest.ActiveTokenV2 + err = arv.Create("collections", map[string]interface{}{"ensure_unique_name": true}, &coll) + c.Assert(err, check.IsNil) + + pr, pw := io.Pipe() + go func() { + time.Sleep(time.Second) + pw.Write(make([]byte, 10000000)) + pw.Close() + }() + req, _ := http.NewRequest("PUT", srvaddr+"/testfile", pr) + req.Host = coll.UUID + ".example" + req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2) + resp, err := http.DefaultClient.Do(req) + c.Assert(err, check.IsNil) + c.Check(resp.StatusCode, check.Equals, http.StatusCreated) + + req, _ = http.NewRequest("GET", srvaddr+"/testfile", nil) + req.Host = coll.UUID + ".example" + req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2) + resp, err = http.DefaultClient.Do(req) + c.Assert(err, check.IsNil) + c.Check(resp.StatusCode, check.Equals, http.StatusOK) + time.Sleep(time.Second) + body, err := ioutil.ReadAll(resp.Body) + c.Check(err, check.IsNil) + c.Check(len(body), check.Equals, 10000000) +} + +func (s *IntegrationSuite) TestHealthCheckPing(c *check.C) { + cluster, srvaddr, _ := s.runServer(c) + req, _ := http.NewRequest("GET", srvaddr+"/_health/ping", nil) + req.Header.Set("Authorization", "Bearer "+cluster.ManagementToken) + resp, err := http.DefaultClient.Do(req) + c.Assert(err, check.IsNil) + c.Check(resp.StatusCode, check.Equals, http.StatusOK) + body, _ := ioutil.ReadAll(resp.Body) + c.Check(string(body), check.Matches, `{"health":"OK"}\n`) +} + func (s *IntegrationSuite) TestMetrics(c *check.C) { - s.testServer.Config.cluster.Services.WebDAVDownload.ExternalURL.Host = s.testServer.Addr - origin := "http://" + s.testServer.Addr - req, _ := http.NewRequest("GET", origin+"/notfound", nil) + cluster, srvaddr, _ := s.runServer(c) + + req, _ := http.NewRequest("GET", srvaddr+"/notfound", nil) + req.Host = cluster.Services.WebDAVDownload.ExternalURL.Host _, err := http.DefaultClient.Do(req) c.Assert(err, check.IsNil) - req, _ = http.NewRequest("GET", origin+"/by_id/", nil) + req, _ = http.NewRequest("GET", srvaddr+"/by_id/", nil) + req.Host = cluster.Services.WebDAVDownload.ExternalURL.Host req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken) resp, err := http.DefaultClient.Do(req) c.Assert(err, check.IsNil) - c.Check(resp.StatusCode, check.Equals, http.StatusOK) + c.Assert(resp.StatusCode, check.Equals, http.StatusOK) for i := 0; i < 2; i++ { - req, _ = http.NewRequest("GET", origin+"/foo", nil) + req, _ = http.NewRequest("GET", srvaddr+"/foo", nil) req.Host = arvadostest.FooCollection + ".example.com" req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken) resp, err = http.DefaultClient.Do(req) @@ -329,20 +411,23 @@ func (s *IntegrationSuite) TestMetrics(c *check.C) { resp.Body.Close() } - s.testServer.Config.Cache.updateGauges() + time.Sleep(metricsUpdateInterval * 2) - req, _ = http.NewRequest("GET", origin+"/metrics.json", nil) + req, _ = http.NewRequest("GET", srvaddr+"/metrics.json", nil) + req.Host = cluster.Services.WebDAVDownload.ExternalURL.Host resp, err = http.DefaultClient.Do(req) c.Assert(err, check.IsNil) c.Check(resp.StatusCode, check.Equals, http.StatusUnauthorized) - req, _ = http.NewRequest("GET", origin+"/metrics.json", nil) + req, _ = http.NewRequest("GET", srvaddr+"/metrics.json", nil) + req.Host = cluster.Services.WebDAVDownload.ExternalURL.Host req.Header.Set("Authorization", "Bearer badtoken") resp, err = http.DefaultClient.Do(req) c.Assert(err, check.IsNil) c.Check(resp.StatusCode, check.Equals, http.StatusForbidden) - req, _ = http.NewRequest("GET", origin+"/metrics.json", nil) + req, _ = http.NewRequest("GET", srvaddr+"/metrics.json", nil) + req.Host = cluster.Services.WebDAVDownload.ExternalURL.Host req.Header.Set("Authorization", "Bearer "+arvadostest.ManagementToken) resp, err = http.DefaultClient.Do(req) c.Assert(err, check.IsNil) @@ -400,13 +485,16 @@ func (s *IntegrationSuite) TestMetrics(c *check.C) { // If the Host header indicates a collection, /metrics.json // refers to a file in the collection -- the metrics handler - // must not intercept that route. - req, _ = http.NewRequest("GET", origin+"/metrics.json", nil) - req.Host = strings.Replace(arvadostest.FooCollectionPDH, "+", "-", -1) + ".example.com" - req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken) - resp, err = http.DefaultClient.Do(req) - c.Assert(err, check.IsNil) - c.Check(resp.StatusCode, check.Equals, http.StatusNotFound) + // must not intercept that route. Ditto health check paths. + for _, path := range []string{"/metrics.json", "/_health/ping"} { + c.Logf("path: %q", path) + req, _ = http.NewRequest("GET", srvaddr+path, nil) + req.Host = strings.Replace(arvadostest.FooCollectionPDH, "+", "-", -1) + ".example.com" + req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken) + resp, err = http.DefaultClient.Do(req) + c.Assert(err, check.IsNil) + c.Check(resp.StatusCode, check.Equals, http.StatusNotFound) + } } func (s *IntegrationSuite) SetUpSuite(c *check.C) { @@ -430,36 +518,31 @@ func (s *IntegrationSuite) TearDownSuite(c *check.C) { func (s *IntegrationSuite) SetUpTest(c *check.C) { arvadostest.ResetEnv() - ldr := config.NewLoader(bytes.NewBufferString("Clusters: {zzzzz: {}}"), ctxlog.TestLogger(c)) - ldr.Path = "-" - arvCfg, err := ldr.Load() - c.Check(err, check.IsNil) - cfg := newConfig(ctxlog.TestLogger(c), arvCfg) - c.Assert(err, check.IsNil) - cfg.Client = arvados.Client{ - APIHost: testAPIHost, - Insecure: true, - } - listen := "127.0.0.1:0" - cfg.cluster.Services.WebDAV.InternalURLs[arvados.URL{Host: listen}] = arvados.ServiceInstance{} - cfg.cluster.Services.WebDAVDownload.InternalURLs[arvados.URL{Host: listen}] = arvados.ServiceInstance{} - cfg.cluster.ManagementToken = arvadostest.ManagementToken - cfg.cluster.SystemRootToken = arvadostest.SystemRootToken - cfg.cluster.Users.AnonymousUserToken = arvadostest.AnonymousToken - s.ArvConfig = arvCfg - s.testServer = &server{Config: cfg} logger := ctxlog.TestLogger(c) + ldr := config.NewLoader(&bytes.Buffer{}, logger) + cfg, err := ldr.Load() + c.Assert(err, check.IsNil) + cluster, err := cfg.GetCluster("") + c.Assert(err, check.IsNil) + ctx := ctxlog.Context(context.Background(), logger) - err = s.testServer.Start(ctx, logger) - c.Assert(err, check.Equals, nil) + + s.handler = newHandlerOrErrorHandler(ctx, cluster, cluster.SystemRootToken, nil).(*handler) + s.testServer = httptest.NewUnstartedServer( + httpserver.AddRequestIDs( + httpserver.LogRequests( + s.handler))) + s.testServer.Config.BaseContext = func(net.Listener) context.Context { return ctx } + s.testServer.Start() + + cluster.Services.WebDAV.InternalURLs = map[arvados.URL]arvados.ServiceInstance{{Host: s.testServer.URL[7:]}: {}} + cluster.Services.WebDAVDownload.InternalURLs = map[arvados.URL]arvados.ServiceInstance{{Host: s.testServer.URL[7:]}: {}} } func (s *IntegrationSuite) TearDownTest(c *check.C) { - var err error if s.testServer != nil { - err = s.testServer.Close() + s.testServer.Close() } - c.Check(err, check.Equals, nil) } // Gocheck boilerplate diff --git a/services/keep-web/status_test.go b/services/keep-web/status_test.go index f90f85cd93..12479a80d8 100644 --- a/services/keep-web/status_test.go +++ b/services/keep-web/status_test.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package keepweb import ( "encoding/json" @@ -11,12 +11,10 @@ import ( "net/url" "git.arvados.org/arvados.git/sdk/go/arvadostest" - "git.arvados.org/arvados.git/sdk/go/ctxlog" "gopkg.in/check.v1" ) func (s *UnitSuite) TestStatus(c *check.C) { - h := handler{Config: newConfig(ctxlog.TestLogger(c), s.Config)} u, _ := url.Parse("http://keep-web.example/status.json") req := &http.Request{ Method: "GET", @@ -25,7 +23,7 @@ func (s *UnitSuite) TestStatus(c *check.C) { RequestURI: u.RequestURI(), } resp := httptest.NewRecorder() - h.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) c.Check(resp.Code, check.Equals, http.StatusOK) var status map[string]interface{} @@ -46,6 +44,6 @@ func (s *IntegrationSuite) TestNoStatusFromVHost(c *check.C) { }, } resp := httptest.NewRecorder() - s.testServer.Handler.ServeHTTP(resp, req) + s.handler.ServeHTTP(resp, req) c.Check(resp.Code, check.Equals, http.StatusNotFound) } diff --git a/services/keep-web/webdav.go b/services/keep-web/webdav.go index a7b7980995..501c355a73 100644 --- a/services/keep-web/webdav.go +++ b/services/keep-web/webdav.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package keepweb import ( "crypto/rand" diff --git a/services/keep-web/webdav_test.go b/services/keep-web/webdav_test.go index 473171e1f5..a450906d5f 100644 --- a/services/keep-web/webdav_test.go +++ b/services/keep-web/webdav_test.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package keepweb import "golang.org/x/net/webdav" diff --git a/tools/arvbox/lib/arvbox/docker/service/keep-web/run-service b/tools/arvbox/lib/arvbox/docker/service/keep-web/run-service index c082e8980e..ef34341d51 100755 --- a/tools/arvbox/lib/arvbox/docker/service/keep-web/run-service +++ b/tools/arvbox/lib/arvbox/docker/service/keep-web/run-service @@ -9,11 +9,12 @@ set -ex -o pipefail . /usr/local/lib/arvbox/common.sh . /usr/local/lib/arvbox/go-setup.sh -flock /var/lib/gopath/gopath.lock go install "git.arvados.org/arvados.git/services/keep-web" -install $GOPATH/bin/keep-web /usr/local/bin +(cd /usr/local/bin && ln -sf arvados-server keep-web) if test "$1" = "--only-deps" ; then exit fi +flock $ARVADOS_CONTAINER_PATH/cluster_config.yml.lock /usr/local/lib/arvbox/cluster-config.sh + exec /usr/local/bin/keep-web -- 2.30.2