From 00f65f60e69326839447f431146312481db05f01 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Fri, 15 Apr 2022 03:28:36 -0400 Subject: [PATCH] 18947: Refactor arv-git-httpd as an arvados-server subcommand. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- build/run-build-packages.sh | 2 +- build/run-tests.sh | 13 ++-- .../arvados-server}/arvados-git-httpd.service | 4 + cmd/arvados-server/cmd.go | 2 + .../management-token.html.textile.liquid | 2 +- lib/boot/supervisor.go | 2 +- lib/config/deprecated.go | 2 +- lib/config/load.go | 2 +- lib/install/deps.go | 1 - sdk/go/arvados/config.go | 22 +++--- sdk/go/health/aggregator_test.go | 1 + sdk/python/tests/run_test_server.py | 16 ++-- services/arv-git-httpd/.gitignore | 1 - services/arv-git-httpd/main.go | 77 ------------------- services/arv-git-httpd/server.go | 36 --------- .../auth_handler.go | 59 +++++++------- .../auth_handler_test.go | 19 +++-- services/githttpd/cmd.go | 32 ++++++++ .../git_handler.go | 11 +-- .../git_handler_test.go | 7 +- .../gitolite_test.go | 8 +- .../integration_test.go | 17 ++-- .../server_test.go | 19 +---- .../docker/service/arv-git-httpd/run-service | 10 +-- 24 files changed, 139 insertions(+), 226 deletions(-) rename {services/arv-git-httpd => cmd/arvados-server}/arvados-git-httpd.service (78%) delete mode 100644 services/arv-git-httpd/.gitignore delete mode 100644 services/arv-git-httpd/main.go delete mode 100644 services/arv-git-httpd/server.go rename services/{arv-git-httpd => githttpd}/auth_handler.go (85%) rename services/{arv-git-httpd => githttpd}/auth_handler_test.go (91%) create mode 100644 services/githttpd/cmd.go rename services/{arv-git-httpd => githttpd}/git_handler.go (82%) rename services/{arv-git-httpd => githttpd}/git_handler_test.go (94%) rename services/{arv-git-httpd => githttpd}/gitolite_test.go (94%) rename services/{arv-git-httpd => githttpd}/integration_test.go (90%) rename services/{arv-git-httpd => githttpd}/server_test.go (87%) diff --git a/build/run-build-packages.sh b/build/run-build-packages.sh index 9b09b7fae0..85e5f35222 100755 --- a/build/run-build-packages.sh +++ b/build/run-build-packages.sh @@ -244,7 +244,7 @@ package_go_binary cmd/arvados-server arvados-dispatch-cloud "$FORMAT" "$ARCH" \ "Arvados cluster cloud dispatch" package_go_binary cmd/arvados-server arvados-dispatch-lsf "$FORMAT" "$ARCH" \ "Dispatch Arvados containers to an LSF cluster" -package_go_binary services/arv-git-httpd arvados-git-httpd "$FORMAT" "$ARCH" \ +package_go_binary cmd/arvados-server arvados-git-httpd "$FORMAT" "$ARCH" \ "Provide authenticated http access to Arvados-hosted git repositories" package_go_binary services/crunch-dispatch-local crunch-dispatch-local "$FORMAT" "$ARCH" \ "Dispatch Crunch containers on the local system" diff --git a/build/run-tests.sh b/build/run-tests.sh index 54535cfef5..8ae9ade322 100755 --- a/build/run-tests.sh +++ b/build/run-tests.sh @@ -40,7 +40,7 @@ sdk/python_test="--test-suite tests.test_keep_locator" Restrict Python SDK tests to the given class apps/workbench_test="TEST=test/integration/pipeline_instances_test.rb" Restrict Workbench tests to the given file -services/arv-git-httpd_test="-check.vv" +services/githttp_test="-check.vv" Show all log messages, even when tests pass (also works with services/keepstore_test etc.) ARVADOS_DEBUG=1 @@ -92,7 +92,7 @@ lib/mount lib/pam lib/service services/api -services/arv-git-httpd +services/githttp services/crunchstat services/dockercleaner services/fuse @@ -410,7 +410,7 @@ start_services() { return 0 fi . "$VENV3DIR/bin/activate" - echo 'Starting API, controller, keepproxy, keep-web, arv-git-httpd, ws, and nginx ssl proxy...' + echo 'Starting API, controller, keepproxy, keep-web, githttp, ws, and nginx ssl proxy...' if [[ ! -d "$WORKSPACE/services/api/log" ]]; then mkdir -p "$WORKSPACE/services/api/log" fi @@ -438,8 +438,8 @@ start_services() { && python3 sdk/python/tests/run_test_server.py start_keep-web \ && checkpidfile keep-web \ && checkhealth WebDAV \ - && python3 sdk/python/tests/run_test_server.py start_arv-git-httpd \ - && checkpidfile arv-git-httpd \ + && python3 sdk/python/tests/run_test_server.py start_githttp \ + && checkpidfile githttp \ && checkhealth GitHTTP \ && python3 sdk/python/tests/run_test_server.py start_ws \ && checkpidfile ws \ @@ -461,7 +461,7 @@ stop_services() { . "$VENV3DIR/bin/activate" || return cd "$WORKSPACE" \ && python3 sdk/python/tests/run_test_server.py stop_nginx \ - && python3 sdk/python/tests/run_test_server.py stop_arv-git-httpd \ + && python3 sdk/python/tests/run_test_server.py stop_githttp \ && python3 sdk/python/tests/run_test_server.py stop_ws \ && python3 sdk/python/tests/run_test_server.py stop_keep-web \ && python3 sdk/python/tests/run_test_server.py stop_keep_proxy \ @@ -1083,7 +1083,6 @@ install_deps() { do_install sdk/python pip "${VENV3DIR}/bin/" do_install sdk/ruby do_install services/api - do_install services/arv-git-httpd go do_install services/keepproxy go do_install services/keep-web go } diff --git a/services/arv-git-httpd/arvados-git-httpd.service b/cmd/arvados-server/arvados-git-httpd.service similarity index 78% rename from services/arv-git-httpd/arvados-git-httpd.service rename to cmd/arvados-server/arvados-git-httpd.service index 6c2b3bc58d..b45587ffc0 100644 --- a/services/arv-git-httpd/arvados-git-httpd.service +++ b/cmd/arvados-server/arvados-git-httpd.service @@ -6,13 +6,17 @@ Description=Arvados git server 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/arvados-git-httpd +# Set a reasonable default for the open file limit +LimitNOFILE=65536 Restart=always RestartSec=1 diff --git a/cmd/arvados-server/cmd.go b/cmd/arvados-server/cmd.go index 26c3f485ea..d65e18ddf5 100644 --- a/cmd/arvados-server/cmd.go +++ b/cmd/arvados-server/cmd.go @@ -21,6 +21,7 @@ import ( "git.arvados.org/arvados.git/lib/install" "git.arvados.org/arvados.git/lib/lsf" "git.arvados.org/arvados.git/lib/recovercollection" + "git.arvados.org/arvados.git/services/githttpd" "git.arvados.org/arvados.git/services/keepproxy" "git.arvados.org/arvados.git/services/keepstore" "git.arvados.org/arvados.git/services/ws" @@ -41,6 +42,7 @@ var ( "crunch-run": crunchrun.Command, "dispatch-cloud": dispatchcloud.Command, "dispatch-lsf": lsf.DispatchCommand, + "git-httpd": githttpd.Command, "install": install.Command, "init": install.InitCommand, "keepproxy": keepproxy.Command, diff --git a/doc/admin/management-token.html.textile.liquid b/doc/admin/management-token.html.textile.liquid index abdd8db734..a4939b740c 100644 --- a/doc/admin/management-token.html.textile.liquid +++ b/doc/admin/management-token.html.textile.liquid @@ -21,7 +21,7 @@ h2. API server and other services The following services also support monitoring. * API server -* arv-git-httpd +* arvados-git-httpd * controller * keep-balance * keepproxy diff --git a/lib/boot/supervisor.go b/lib/boot/supervisor.go index 7daceccb93..1fdb8eba31 100644 --- a/lib/boot/supervisor.go +++ b/lib/boot/supervisor.go @@ -244,7 +244,7 @@ func (super *Supervisor) run(cfg *arvados.Config) error { runPostgreSQL{}, runNginx{}, runServiceCommand{name: "controller", svc: super.cluster.Services.Controller, depends: []supervisedTask{seedDatabase{}}}, - runGoProgram{src: "services/arv-git-httpd", svc: super.cluster.Services.GitHTTP}, + runServiceCommand{name: "git-httpd", svc: super.cluster.Services.GitHTTP}, 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}, diff --git a/lib/config/deprecated.go b/lib/config/deprecated.go index e9c5da1064..1016745b1c 100644 --- a/lib/config/deprecated.go +++ b/lib/config/deprecated.go @@ -536,7 +536,7 @@ func (ldr *Loader) loadOldGitHttpdConfig(cfg *arvados.Config) error { return nil } var oc oldGitHttpdConfig - err := ldr.loadOldConfigHelper("arv-git-httpd", ldr.GitHttpdPath, &oc) + err := ldr.loadOldConfigHelper("arvados-git-httpd", ldr.GitHttpdPath, &oc) if os.IsNotExist(err) && ldr.GitHttpdPath == defaultGitHttpdConfigPath { return nil } else if err != nil { diff --git a/lib/config/load.go b/lib/config/load.go index 5afb51c5ad..6099215edc 100644 --- a/lib/config/load.go +++ b/lib/config/load.go @@ -76,7 +76,7 @@ func (ldr *Loader) SetupFlags(flagset *flag.FlagSet) { flagset.StringVar(&ldr.CrunchDispatchSlurmPath, "legacy-crunch-dispatch-slurm-config", defaultCrunchDispatchSlurmConfigPath, "Legacy crunch-dispatch-slurm configuration `file`") flagset.StringVar(&ldr.WebsocketPath, "legacy-ws-config", defaultWebsocketConfigPath, "Legacy arvados-ws configuration `file`") flagset.StringVar(&ldr.KeepproxyPath, "legacy-keepproxy-config", defaultKeepproxyConfigPath, "Legacy keepproxy configuration `file`") - flagset.StringVar(&ldr.GitHttpdPath, "legacy-git-httpd-config", defaultGitHttpdConfigPath, "Legacy arv-git-httpd configuration `file`") + flagset.StringVar(&ldr.GitHttpdPath, "legacy-git-httpd-config", defaultGitHttpdConfigPath, "Legacy arvados-git-httpd configuration `file`") flagset.StringVar(&ldr.KeepBalancePath, "legacy-keepbalance-config", defaultKeepBalanceConfigPath, "Legacy keep-balance configuration `file`") flagset.BoolVar(&ldr.SkipLegacy, "skip-legacy", false, "Don't load legacy config files") } diff --git a/lib/install/deps.go b/lib/install/deps.go index af3cc656c1..c6fba424d5 100644 --- a/lib/install/deps.go +++ b/lib/install/deps.go @@ -523,7 +523,6 @@ yarn install for _, srcdir := range []string{ "cmd/arvados-client", "cmd/arvados-server", - "services/arv-git-httpd", "services/crunch-dispatch-local", "services/crunch-dispatch-slurm", "services/health", diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go index 6c9324e478..1295350a4d 100644 --- a/sdk/go/arvados/config.go +++ b/sdk/go/arvados/config.go @@ -625,35 +625,37 @@ func (ss *StringSet) UnmarshalJSON(data []byte) error { type ServiceName string const ( - ServiceNameRailsAPI ServiceName = "arvados-api-server" ServiceNameController ServiceName = "arvados-controller" ServiceNameDispatchCloud ServiceName = "arvados-dispatch-cloud" ServiceNameDispatchLSF ServiceName = "arvados-dispatch-lsf" + ServiceNameGitHTTP ServiceName = "arvados-git-httpd" ServiceNameHealth ServiceName = "arvados-health" - ServiceNameWorkbench1 ServiceName = "arvados-workbench1" - ServiceNameWorkbench2 ServiceName = "arvados-workbench2" - ServiceNameWebsocket ServiceName = "arvados-ws" ServiceNameKeepbalance ServiceName = "keep-balance" - ServiceNameKeepweb ServiceName = "keep-web" ServiceNameKeepproxy ServiceName = "keepproxy" ServiceNameKeepstore ServiceName = "keepstore" + ServiceNameKeepweb ServiceName = "keep-web" + ServiceNameRailsAPI ServiceName = "arvados-api-server" + ServiceNameWebsocket ServiceName = "arvados-ws" + ServiceNameWorkbench1 ServiceName = "arvados-workbench1" + ServiceNameWorkbench2 ServiceName = "arvados-workbench2" ) // Map returns all services as a map, suitable for iterating over all // services or looking up a service by name. func (svcs Services) Map() map[ServiceName]Service { return map[ServiceName]Service{ - ServiceNameRailsAPI: svcs.RailsAPI, ServiceNameController: svcs.Controller, ServiceNameDispatchCloud: svcs.DispatchCloud, ServiceNameDispatchLSF: svcs.DispatchLSF, + ServiceNameGitHTTP: svcs.GitHTTP, ServiceNameHealth: svcs.Health, - ServiceNameWorkbench1: svcs.Workbench1, - ServiceNameWorkbench2: svcs.Workbench2, - ServiceNameWebsocket: svcs.Websocket, ServiceNameKeepbalance: svcs.Keepbalance, - ServiceNameKeepweb: svcs.WebDAV, ServiceNameKeepproxy: svcs.Keepproxy, ServiceNameKeepstore: svcs.Keepstore, + ServiceNameKeepweb: svcs.WebDAV, + ServiceNameRailsAPI: svcs.RailsAPI, + ServiceNameWebsocket: svcs.Websocket, + ServiceNameWorkbench1: svcs.Workbench1, + ServiceNameWorkbench2: svcs.Workbench2, } } diff --git a/sdk/go/health/aggregator_test.go b/sdk/go/health/aggregator_test.go index 04106caa44..f8507ef4f5 100644 --- a/sdk/go/health/aggregator_test.go +++ b/sdk/go/health/aggregator_test.go @@ -154,6 +154,7 @@ func (s *AggregatorSuite) setAllServiceURLs(listen string) { &svcs.Controller, &svcs.DispatchCloud, &svcs.DispatchLSF, + &svcs.GitHTTP, &svcs.Keepbalance, &svcs.Keepproxy, &svcs.Keepstore, diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py index 6514c2af45..6935e5b714 100644 --- a/sdk/python/tests/run_test_server.py +++ b/sdk/python/tests/run_test_server.py @@ -544,7 +544,7 @@ def run_keep_proxy(): env['ARVADOS_API_TOKEN'] = auth_token('anonymous') logf = open(_logfilename('keepproxy'), WRITE_MODE) kp = subprocess.Popen( - ['keepproxy'], env=env, stdin=open('/dev/null'), stdout=logf, stderr=logf, close_fds=True) + ['arvados-server', 'keepproxy'], env=env, stdin=open('/dev/null'), stdout=logf, stderr=logf, close_fds=True) with open(_pidfile('keepproxy'), 'w') as f: f.write(str(kp.pid)) @@ -581,17 +581,17 @@ def run_arv_git_httpd(): gitport = internal_port_from_config("GitHTTP") env = os.environ.copy() env.pop('ARVADOS_API_TOKEN', None) - logf = open(_logfilename('arv-git-httpd'), WRITE_MODE) - agh = subprocess.Popen(['arv-git-httpd'], + logf = open(_logfilename('githttp'), WRITE_MODE) + agh = subprocess.Popen(['arvados-server', 'git-httpd'], env=env, stdin=open('/dev/null'), stdout=logf, stderr=logf) - with open(_pidfile('arv-git-httpd'), 'w') as f: + with open(_pidfile('githttp'), 'w') as f: f.write(str(agh.pid)) _wait_until_port_listens(gitport) def stop_arv_git_httpd(): if 'ARVADOS_TEST_PROXY_SERVICES' in os.environ: return - kill_server_pid(_pidfile('arv-git-httpd')) + kill_server_pid(_pidfile('githttp')) def run_keep_web(): if 'ARVADOS_TEST_PROXY_SERVICES' in os.environ: @@ -942,7 +942,7 @@ if __name__ == "__main__": 'start_keep', 'stop_keep', 'start_keep_proxy', 'stop_keep_proxy', 'start_keep-web', 'stop_keep-web', - 'start_arv-git-httpd', 'stop_arv-git-httpd', + 'start_githttp', 'stop_githttp', 'start_nginx', 'stop_nginx', 'setup_config', ] parser = argparse.ArgumentParser() @@ -987,9 +987,9 @@ if __name__ == "__main__": run_keep_proxy() elif args.action == 'stop_keep_proxy': stop_keep_proxy() - elif args.action == 'start_arv-git-httpd': + elif args.action == 'start_githttp': run_arv_git_httpd() - elif args.action == 'stop_arv-git-httpd': + elif args.action == 'stop_githttp': stop_arv_git_httpd() elif args.action == 'start_keep-web': run_keep_web() diff --git a/services/arv-git-httpd/.gitignore b/services/arv-git-httpd/.gitignore deleted file mode 100644 index 1ae1045c29..0000000000 --- a/services/arv-git-httpd/.gitignore +++ /dev/null @@ -1 +0,0 @@ -arv-git-httpd diff --git a/services/arv-git-httpd/main.go b/services/arv-git-httpd/main.go deleted file mode 100644 index b926ac2735..0000000000 --- a/services/arv-git-httpd/main.go +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright (C) The Arvados Authors. All rights reserved. -// -// SPDX-License-Identifier: AGPL-3.0 - -package main - -import ( - "flag" - "fmt" - "os" - - "git.arvados.org/arvados.git/lib/cmd" - "git.arvados.org/arvados.git/lib/config" - "github.com/coreos/go-systemd/daemon" - "github.com/ghodss/yaml" - log "github.com/sirupsen/logrus" -) - -var version = "dev" - -func main() { - logger := log.New() - log.SetFormatter(&log.JSONFormatter{ - TimestampFormat: "2006-01-02T15:04:05.000000000Z07:00", - }) - - flags := flag.NewFlagSet(os.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 (useful for migrating from command line flags to config file)") - getVersion := flags.Bool("version", false, "print version information and exit.") - - args := loader.MungeLegacyConfigArgs(logger, os.Args[1:], "-legacy-git-httpd-config") - if ok, code := cmd.ParseFlags(flags, os.Args[0], args, "", os.Stderr); !ok { - os.Exit(code) - } else if *getVersion { - fmt.Printf("arv-git-httpd %s\n", version) - return - } - - cfg, err := loader.Load() - if err != nil { - log.Fatal(err) - } - - cluster, err := cfg.GetCluster("") - if err != nil { - log.Fatal(err) - } - - if *dumpConfig { - out, err := yaml.Marshal(cfg) - if err != nil { - log.Fatal(err) - } - _, err = os.Stdout.Write(out) - if err != nil { - log.Fatal(err) - } - return - } - - srv := &server{cluster: cluster} - if err := srv.Start(); err != nil { - log.Fatal(err) - } - if _, err := daemon.SdNotify(false, "READY=1"); err != nil { - log.Printf("Error notifying init daemon: %v", err) - } - log.Printf("arv-git-httpd %s started", version) - log.Println("Listening at", srv.Addr) - log.Println("Repository root", cluster.Git.Repositories) - if err := srv.Wait(); err != nil { - log.Fatal(err) - } -} diff --git a/services/arv-git-httpd/server.go b/services/arv-git-httpd/server.go deleted file mode 100644 index 38a018ab3d..0000000000 --- a/services/arv-git-httpd/server.go +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright (C) The Arvados Authors. All rights reserved. -// -// SPDX-License-Identifier: AGPL-3.0 - -package main - -import ( - "net/http" - - "git.arvados.org/arvados.git/sdk/go/arvados" - "git.arvados.org/arvados.git/sdk/go/health" - "git.arvados.org/arvados.git/sdk/go/httpserver" -) - -type server struct { - httpserver.Server - cluster *arvados.Cluster -} - -func (srv *server) Start() error { - mux := http.NewServeMux() - mux.Handle("/", &authHandler{handler: newGitHandler(srv.cluster), cluster: srv.cluster}) - mux.Handle("/_health/", &health.Handler{ - Token: srv.cluster.ManagementToken, - Prefix: "/_health/", - }) - - var listen arvados.URL - for listen = range srv.cluster.Services.GitHTTP.InternalURLs { - break - } - - srv.Handler = mux - srv.Addr = listen.Host - return srv.Server.Start() -} diff --git a/services/arv-git-httpd/auth_handler.go b/services/githttpd/auth_handler.go similarity index 85% rename from services/arv-git-httpd/auth_handler.go rename to services/githttpd/auth_handler.go index 13706ae3e8..c6b23fd4c8 100644 --- a/services/arv-git-httpd/auth_handler.go +++ b/services/githttpd/auth_handler.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package githttpd import ( "errors" @@ -11,44 +11,33 @@ import ( "os" "regexp" "strings" - "sync" "time" "git.arvados.org/arvados.git/sdk/go/arvados" "git.arvados.org/arvados.git/sdk/go/arvadosclient" "git.arvados.org/arvados.git/sdk/go/auth" "git.arvados.org/arvados.git/sdk/go/httpserver" + "github.com/sirupsen/logrus" ) type authHandler struct { handler http.Handler clientPool *arvadosclient.ClientPool cluster *arvados.Cluster - setupOnce sync.Once } -func (h *authHandler) setup() { - client, err := arvados.NewClientFromConfig(h.cluster) - if err != nil { - log.Fatal(err) - } - - ac, err := arvadosclient.New(client) - if err != nil { - log.Fatalf("Error setting up arvados client prototype %v", err) - } +func (h *authHandler) CheckHealth() error { + return nil +} - h.clientPool = &arvadosclient.ClientPool{Prototype: ac} +func (h *authHandler) Done() <-chan struct{} { + return nil } func (h *authHandler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { - h.setupOnce.Do(h.setup) - var statusCode int var statusText string var apiToken string - var repoName string - var validAPIToken bool w := httpserver.WrapResponseWriter(wOrig) @@ -84,19 +73,6 @@ func (h *authHandler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { w.Write([]byte(statusText)) } } - - // If the given password is a valid token, log the first 10 characters of the token. - // Otherwise: log the string if a password is given, else an empty string. - passwordToLog := "" - if !validAPIToken { - if len(apiToken) > 0 { - passwordToLog = "" - } - } else { - passwordToLog = apiToken[0:10] - } - - httpserver.Log(r.RemoteAddr, passwordToLog, w.WroteStatus(), statusText, repoName, r.Method, r.URL.Path) }() creds := auth.CredentialsFromRequest(r) @@ -115,8 +91,11 @@ func (h *authHandler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { statusCode, statusText = http.StatusNotFound, "not found" return } - repoName = pathParts[0] + repoName := pathParts[0] repoName = strings.TrimRight(repoName, "/") + httpserver.SetResponseLogFields(r.Context(), logrus.Fields{ + "repoName": repoName, + }) arv := h.clientPool.Get() if arv == nil { @@ -125,6 +104,21 @@ func (h *authHandler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { } defer h.clientPool.Put(arv) + // Log the UUID if the supplied token is a v2 token, otherwise + // just the last five characters. + httpserver.SetResponseLogFields(r.Context(), logrus.Fields{ + "tokenUUID": func() string { + if strings.HasPrefix(apiToken, "v2/") && strings.IndexRune(apiToken[3:], '/') == 27 { + // UUID part of v2 token + return apiToken[3:30] + } else if len(apiToken) > 5 { + return "[...]" + apiToken[len(apiToken)-5:] + } else { + return apiToken + } + }(), + }) + // Ask API server whether the repository is readable using // this token (by trying to read it!) arv.ApiToken = apiToken @@ -133,7 +127,6 @@ func (h *authHandler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { statusCode, statusText = http.StatusInternalServerError, err.Error() return } - validAPIToken = true if repoUUID == "" { statusCode, statusText = http.StatusNotFound, "not found" return diff --git a/services/arv-git-httpd/auth_handler_test.go b/services/githttpd/auth_handler_test.go similarity index 91% rename from services/arv-git-httpd/auth_handler_test.go rename to services/githttpd/auth_handler_test.go index 688b256dcb..2d1ec966a4 100644 --- a/services/arv-git-httpd/auth_handler_test.go +++ b/services/githttpd/auth_handler_test.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package githttpd import ( "io" @@ -15,6 +15,7 @@ import ( "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" check "gopkg.in/check.v1" @@ -43,10 +44,18 @@ func (s *AuthHandlerSuite) SetUpTest(c *check.C) { } func (s *AuthHandlerSuite) TestPermission(c *check.C) { - h := &authHandler{handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - log.Printf("%v", r.URL) - io.WriteString(w, r.URL.Path) - }), cluster: s.cluster} + client, err := arvados.NewClientFromConfig(s.cluster) + c.Assert(err, check.IsNil) + ac, err := arvadosclient.New(client) + c.Assert(err, check.IsNil) + h := &authHandler{ + cluster: s.cluster, + clientPool: &arvadosclient.ClientPool{Prototype: ac}, + handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + log.Printf("%v", r.URL) + io.WriteString(w, r.URL.Path) + }), + } baseURL, err := url.Parse("http://git.example/") c.Assert(err, check.IsNil) for _, trial := range []struct { diff --git a/services/githttpd/cmd.go b/services/githttpd/cmd.go new file mode 100644 index 0000000000..e6ca3c0743 --- /dev/null +++ b/services/githttpd/cmd.go @@ -0,0 +1,32 @@ +// Copyright (C) The Arvados Authors. All rights reserved. +// +// SPDX-License-Identifier: AGPL-3.0 + +package githttpd + +import ( + "context" + + "git.arvados.org/arvados.git/lib/service" + "git.arvados.org/arvados.git/sdk/go/arvados" + "git.arvados.org/arvados.git/sdk/go/arvadosclient" + "github.com/prometheus/client_golang/prometheus" +) + +var Command = service.Command(arvados.ServiceNameGitHTTP, newHandler) + +func newHandler(ctx context.Context, cluster *arvados.Cluster, token string, reg *prometheus.Registry) service.Handler { + client, err := arvados.NewClientFromConfig(cluster) + if err != nil { + return service.ErrorHandler(ctx, cluster, err) + } + ac, err := arvadosclient.New(client) + if err != nil { + return service.ErrorHandler(ctx, cluster, err) + } + return &authHandler{ + clientPool: &arvadosclient.ClientPool{Prototype: ac}, + cluster: cluster, + handler: newGitHandler(ctx, cluster), + } +} diff --git a/services/arv-git-httpd/git_handler.go b/services/githttpd/git_handler.go similarity index 82% rename from services/arv-git-httpd/git_handler.go rename to services/githttpd/git_handler.go index bb1b1afc73..7c94294c04 100644 --- a/services/arv-git-httpd/git_handler.go +++ b/services/githttpd/git_handler.go @@ -2,16 +2,17 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package githttpd import ( - "log" + "context" "net" "net/http" "net/http/cgi" "os" "git.arvados.org/arvados.git/sdk/go/arvados" + "git.arvados.org/arvados.git/sdk/go/ctxlog" ) // gitHandler is an http.Handler that invokes git-http-backend (or @@ -22,7 +23,7 @@ type gitHandler struct { cgi.Handler } -func newGitHandler(cluster *arvados.Cluster) http.Handler { +func newGitHandler(ctx context.Context, cluster *arvados.Cluster) http.Handler { const glBypass = "GL_BYPASS_ACCESS_CHECKS" const glHome = "GITOLITE_HTTP_HOME" var env []string @@ -34,7 +35,7 @@ func newGitHandler(cluster *arvados.Cluster) http.Handler { path = path + ":" + cluster.Git.GitoliteHome + "/bin" } else if home, bypass := os.Getenv(glHome), os.Getenv(glBypass); home != "" || bypass != "" { env = append(env, glHome+"="+home, glBypass+"="+bypass) - log.Printf("DEPRECATED: Passing through %s and %s environment variables. Use GitoliteHome configuration instead.", glHome, glBypass) + ctxlog.FromContext(ctx).Printf("DEPRECATED: Passing through %s and %s environment variables. Use GitoliteHome configuration instead.", glHome, glBypass) } var listen arvados.URL @@ -59,7 +60,7 @@ func newGitHandler(cluster *arvados.Cluster) http.Handler { func (h *gitHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { remoteHost, remotePort, err := net.SplitHostPort(r.RemoteAddr) if err != nil { - log.Printf("Internal error: SplitHostPort(r.RemoteAddr==%q): %s", r.RemoteAddr, err) + ctxlog.FromContext(r.Context()).Errorf("Internal error: SplitHostPort(r.RemoteAddr==%q): %s", r.RemoteAddr, err) w.WriteHeader(http.StatusInternalServerError) return } diff --git a/services/arv-git-httpd/git_handler_test.go b/services/githttpd/git_handler_test.go similarity index 94% rename from services/arv-git-httpd/git_handler_test.go rename to services/githttpd/git_handler_test.go index dafe5d31d7..ef2ee28e79 100644 --- a/services/arv-git-httpd/git_handler_test.go +++ b/services/githttpd/git_handler_test.go @@ -2,9 +2,10 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package githttpd import ( + "context" "net/http" "net/http/httptest" "net/url" @@ -42,7 +43,7 @@ func (s *GitHandlerSuite) TestEnvVars(c *check.C) { URL: u, RemoteAddr: "[::1]:12345", } - h := newGitHandler(s.cluster) + h := newGitHandler(context.Background(), s.cluster) h.(*gitHandler).Path = "/bin/sh" h.(*gitHandler).Args = []string{"-c", "printf 'Content-Type: text/plain\r\n\r\n'; env"} @@ -67,7 +68,7 @@ func (s *GitHandlerSuite) TestCGIErrorOnSplitHostPortError(c *check.C) { URL: u, RemoteAddr: "test.bad.address.missing.port", } - h := newGitHandler(s.cluster) + h := newGitHandler(context.Background(), s.cluster) h.ServeHTTP(resp, req) c.Check(resp.Code, check.Equals, http.StatusInternalServerError) c.Check(resp.Body.String(), check.Equals, "") diff --git a/services/arv-git-httpd/gitolite_test.go b/services/githttpd/gitolite_test.go similarity index 94% rename from services/arv-git-httpd/gitolite_test.go rename to services/githttpd/gitolite_test.go index cfedd88f17..d34c413c1b 100644 --- a/services/arv-git-httpd/gitolite_test.go +++ b/services/githttpd/gitolite_test.go @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package githttpd import ( "io/ioutil" @@ -18,8 +18,8 @@ import ( var _ = check.Suite(&GitoliteSuite{}) -// GitoliteSuite tests need an API server, an arv-git-httpd server, -// and a repository hosted by gitolite. +// GitoliteSuite tests need an API server, an arvados-git-httpd +// server, and a repository hosted by gitolite. type GitoliteSuite struct { IntegrationSuite gitoliteHome string @@ -27,7 +27,7 @@ type GitoliteSuite struct { func (s *GitoliteSuite) SetUpTest(c *check.C) { var err error - s.gitoliteHome, err = ioutil.TempDir("", "arv-git-httpd") + s.gitoliteHome, err = ioutil.TempDir("", "githttp") c.Assert(err, check.Equals, nil) runGitolite := func(prog string, args ...string) { diff --git a/services/arv-git-httpd/integration_test.go b/services/githttpd/integration_test.go similarity index 90% rename from services/arv-git-httpd/integration_test.go rename to services/githttpd/integration_test.go index 93a46e2248..c819272d3e 100644 --- a/services/arv-git-httpd/integration_test.go +++ b/services/githttpd/integration_test.go @@ -2,9 +2,10 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package githttpd import ( + "context" "errors" "io/ioutil" "os" @@ -16,6 +17,7 @@ import ( "git.arvados.org/arvados.git/sdk/go/arvados" "git.arvados.org/arvados.git/sdk/go/arvadostest" "git.arvados.org/arvados.git/sdk/go/ctxlog" + "git.arvados.org/arvados.git/sdk/go/httpserver" check "gopkg.in/check.v1" ) @@ -24,12 +26,12 @@ func Test(t *testing.T) { check.TestingT(t) } -// IntegrationSuite tests need an API server and an arv-git-httpd +// IntegrationSuite tests need an API server and an arvados-git-httpd // server. See GitSuite and GitoliteSuite. type IntegrationSuite struct { tmpRepoRoot string tmpWorkdir string - testServer *server + testServer *httpserver.Server cluster *arvados.Cluster } @@ -38,10 +40,10 @@ func (s *IntegrationSuite) SetUpTest(c *check.C) { var err error if s.tmpRepoRoot == "" { - s.tmpRepoRoot, err = ioutil.TempDir("", "arv-git-httpd") + s.tmpRepoRoot, err = ioutil.TempDir("", "githttp") c.Assert(err, check.Equals, nil) } - s.tmpWorkdir, err = ioutil.TempDir("", "arv-git-httpd") + s.tmpWorkdir, err = ioutil.TempDir("", "githttp") c.Assert(err, check.Equals, nil) _, err = exec.Command("git", "init", "--bare", s.tmpRepoRoot+"/zzzzz-s0uqq-382brsig8rp3666.git").Output() c.Assert(err, check.Equals, nil) @@ -70,7 +72,8 @@ func (s *IntegrationSuite) SetUpTest(c *check.C) { s.cluster.ManagementToken = arvadostest.ManagementToken } - s.testServer = &server{cluster: s.cluster} + s.testServer = &httpserver.Server{} + s.testServer.Handler = httpserver.LogRequests(newHandler(context.Background(), s.cluster, "", nil)) err = s.testServer.Start() c.Assert(err, check.Equals, nil) @@ -86,7 +89,7 @@ func (s *IntegrationSuite) SetUpTest(c *check.C) { c.Assert(err, check.Equals, nil) // Clear ARVADOS_API_* env vars before starting up the server, - // to make sure arv-git-httpd doesn't use them or complain + // to make sure arvados-git-httpd doesn't use them or complain // about them being missing. os.Unsetenv("ARVADOS_API_HOST") os.Unsetenv("ARVADOS_API_HOST_INSECURE") diff --git a/services/arv-git-httpd/server_test.go b/services/githttpd/server_test.go similarity index 87% rename from services/arv-git-httpd/server_test.go rename to services/githttpd/server_test.go index a92aa1fb86..02c13a3112 100644 --- a/services/arv-git-httpd/server_test.go +++ b/services/githttpd/server_test.go @@ -2,16 +2,12 @@ // // SPDX-License-Identifier: AGPL-3.0 -package main +package githttpd import ( - "net/http" - "net/http/httptest" "os" "os/exec" - "git.arvados.org/arvados.git/sdk/go/arvadostest" - check "gopkg.in/check.v1" ) @@ -108,16 +104,3 @@ func (s *GitSuite) makeArvadosRepo(c *check.C) { c.Log(string(msg)) c.Assert(err, check.Equals, nil) } - -func (s *GitSuite) TestHealthCheckPing(c *check.C) { - req, err := http.NewRequest("GET", - "http://"+s.testServer.Addr+"/_health/ping", - nil) - c.Assert(err, check.Equals, nil) - req.Header.Set("Authorization", "Bearer "+arvadostest.ManagementToken) - - resp := httptest.NewRecorder() - s.testServer.Handler.ServeHTTP(resp, req) - c.Check(resp.Code, check.Equals, 200) - c.Check(resp.Body.String(), check.Matches, `{"health":"OK"}\n`) -} diff --git a/tools/arvbox/lib/arvbox/docker/service/arv-git-httpd/run-service b/tools/arvbox/lib/arvbox/docker/service/arv-git-httpd/run-service index b369ff6228..c8d4a4c170 100755 --- a/tools/arvbox/lib/arvbox/docker/service/arv-git-httpd/run-service +++ b/tools/arvbox/lib/arvbox/docker/service/arv-git-httpd/run-service @@ -9,16 +9,14 @@ 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/arv-git-httpd" -install $GOPATH/bin/arv-git-httpd /usr/local/bin +(cd /usr/local/bin && ln -sf arvados-server arvados-git-httpd) if test "$1" = "--only-deps" ; then exit fi -export ARVADOS_API_HOST=$localip:${services[controller-ssl]} -export ARVADOS_API_HOST_INSECURE=1 +/usr/local/lib/arvbox/runsu.sh flock $ARVADOS_CONTAINER_PATH/cluster_config.yml.lock /usr/local/lib/arvbox/cluster-config.sh + export PATH="$PATH:$ARVADOS_CONTAINER_PATH/git/bin" cd ~git - -exec /usr/local/bin/arv-git-httpd +exec /usr/local/bin/arvados-git-httpd -- 2.30.2