16392: Add trailing slash to URLs like https://example in configs.
authorTom Clegg <tom@tomclegg.ca>
Tue, 12 May 2020 07:51:38 +0000 (03:51 -0400)
committerTom Clegg <tom@tomclegg.ca>
Tue, 12 May 2020 07:51:38 +0000 (03:51 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@tomclegg.ca>

14 files changed:
lib/boot/supervisor.go
lib/config/cmd_test.go
lib/config/deprecated.go
lib/config/deprecated_keepstore.go
lib/config/deprecated_keepstore_test.go
lib/config/deprecated_test.go
lib/service/cmd.go
sdk/go/arvados/config.go
sdk/go/arvados/config_test.go
services/api/app/controllers/user_sessions_controller.rb
services/api/config/initializers/omniauth_init.rb
services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go
services/keepproxy/keepproxy.go
services/ws/service_test.go

index c444ec3001acd3b3b8b86faa40acae38eb110922..c3dd4bf9b2599251f9df48d9c947e8dad80f39c7 100644 (file)
@@ -568,7 +568,7 @@ func (super *Supervisor) autofillConfig(cfg *arvados.Config) error {
                if p == "0" {
                        p = nextPort(h)
                }
-               cluster.Services.Controller.ExternalURL = arvados.URL{Scheme: "https", Host: net.JoinHostPort(h, p)}
+               cluster.Services.Controller.ExternalURL = arvados.URL{Scheme: "https", Host: net.JoinHostPort(h, p), Path: "/"}
        }
        for _, svc := range []*arvados.Service{
                &cluster.Services.Controller,
@@ -594,14 +594,14 @@ func (super *Supervisor) autofillConfig(cfg *arvados.Config) error {
                                svc == &cluster.Services.WebDAV ||
                                svc == &cluster.Services.WebDAVDownload ||
                                svc == &cluster.Services.Workbench1 {
-                               svc.ExternalURL = arvados.URL{Scheme: "https", Host: fmt.Sprintf("%s:%s", super.ListenHost, nextPort(super.ListenHost))}
+                               svc.ExternalURL = arvados.URL{Scheme: "https", Host: fmt.Sprintf("%s:%s", super.ListenHost, nextPort(super.ListenHost)), Path: "/"}
                        } else if svc == &cluster.Services.Websocket {
-                               svc.ExternalURL = arvados.URL{Scheme: "wss", Host: fmt.Sprintf("%s:%s", super.ListenHost, nextPort(super.ListenHost))}
+                               svc.ExternalURL = arvados.URL{Scheme: "wss", Host: fmt.Sprintf("%s:%s", super.ListenHost, nextPort(super.ListenHost)), Path: "/"}
                        }
                }
                if len(svc.InternalURLs) == 0 {
                        svc.InternalURLs = map[arvados.URL]arvados.ServiceInstance{
-                               arvados.URL{Scheme: "http", Host: fmt.Sprintf("%s:%s", super.ListenHost, nextPort(super.ListenHost))}: arvados.ServiceInstance{},
+                               arvados.URL{Scheme: "http", Host: fmt.Sprintf("%s:%s", super.ListenHost, nextPort(super.ListenHost)), Path: "/"}: arvados.ServiceInstance{},
                        }
                }
        }
@@ -629,7 +629,7 @@ func (super *Supervisor) autofillConfig(cfg *arvados.Config) error {
        }
        if super.ClusterType == "test" {
                // Add a second keepstore process.
-               cluster.Services.Keepstore.InternalURLs[arvados.URL{Scheme: "http", Host: fmt.Sprintf("%s:%s", super.ListenHost, nextPort(super.ListenHost))}] = arvados.ServiceInstance{}
+               cluster.Services.Keepstore.InternalURLs[arvados.URL{Scheme: "http", Host: fmt.Sprintf("%s:%s", super.ListenHost, nextPort(super.ListenHost)), Path: "/"}] = arvados.ServiceInstance{}
 
                // Create a directory-backed volume for each keepstore
                // process.
index 3c420a04eb43e28e39fa69f8e1baa3330b803d64..74c3cc96947a88b4617f1e8f1fc8acece159fd79 100644 (file)
@@ -148,7 +148,7 @@ Clusters:
        code := DumpCommand.RunCommand("arvados config-dump", []string{"-config", "-"}, bytes.NewBufferString(in), &stdout, &stderr)
        c.Check(code, check.Equals, 0)
        c.Check(stdout.String(), check.Matches, `(?ms).*TimeoutBooting: 10m\n.*`)
-       c.Check(stdout.String(), check.Matches, `(?ms).*http://localhost:12345: {}\n.*`)
+       c.Check(stdout.String(), check.Matches, `(?ms).*http://localhost:12345/: {}\n.*`)
 }
 
 func (s *CommandSuite) TestDump_UnknownKey(c *check.C) {
index 0689efa440f5f611696680e64c631dc1be3b6a3f..bbbc9acf7534cd562c1e5dad0615fc688b07fd9b 100644 (file)
@@ -100,7 +100,7 @@ func applyDeprecatedNodeProfile(hostname string, ssi systemServiceInstance, svc
        if strings.HasPrefix(host, ":") {
                host = hostname + host
        }
-       svc.InternalURLs[arvados.URL{Scheme: scheme, Host: host}] = arvados.ServiceInstance{}
+       svc.InternalURLs[arvados.URL{Scheme: scheme, Host: host, Path: "/"}] = arvados.ServiceInstance{}
 }
 
 func (ldr *Loader) loadOldConfigHelper(component, path string, target interface{}) error {
@@ -153,6 +153,7 @@ func loadOldClientConfig(cluster *arvados.Cluster, client *arvados.Client) {
        }
        if client.APIHost != "" {
                cluster.Services.Controller.ExternalURL.Host = client.APIHost
+               cluster.Services.Controller.ExternalURL.Path = "/"
        }
        if client.Scheme != "" {
                cluster.Services.Controller.ExternalURL.Scheme = client.Scheme
@@ -268,7 +269,7 @@ func (ldr *Loader) loadOldWebsocketConfig(cfg *arvados.Config) error {
                cluster.PostgreSQL.ConnectionPool = *oc.PostgresPool
        }
        if oc.Listen != nil {
-               cluster.Services.Websocket.InternalURLs[arvados.URL{Host: *oc.Listen}] = arvados.ServiceInstance{}
+               cluster.Services.Websocket.InternalURLs[arvados.URL{Host: *oc.Listen, Path: "/"}] = arvados.ServiceInstance{}
        }
        if oc.LogLevel != nil {
                cluster.SystemLogs.LogLevel = *oc.LogLevel
@@ -327,7 +328,7 @@ func (ldr *Loader) loadOldKeepproxyConfig(cfg *arvados.Config) error {
        loadOldClientConfig(cluster, oc.Client)
 
        if oc.Listen != nil {
-               cluster.Services.Keepproxy.InternalURLs[arvados.URL{Host: *oc.Listen}] = arvados.ServiceInstance{}
+               cluster.Services.Keepproxy.InternalURLs[arvados.URL{Host: *oc.Listen, Path: "/"}] = arvados.ServiceInstance{}
        }
        if oc.DefaultReplicas != nil {
                cluster.Collections.DefaultReplication = *oc.DefaultReplicas
@@ -413,11 +414,11 @@ func (ldr *Loader) loadOldKeepWebConfig(cfg *arvados.Config) error {
        loadOldClientConfig(cluster, oc.Client)
 
        if oc.Listen != nil {
-               cluster.Services.WebDAV.InternalURLs[arvados.URL{Host: *oc.Listen}] = arvados.ServiceInstance{}
-               cluster.Services.WebDAVDownload.InternalURLs[arvados.URL{Host: *oc.Listen}] = arvados.ServiceInstance{}
+               cluster.Services.WebDAV.InternalURLs[arvados.URL{Host: *oc.Listen, Path: "/"}] = arvados.ServiceInstance{}
+               cluster.Services.WebDAVDownload.InternalURLs[arvados.URL{Host: *oc.Listen, Path: "/"}] = arvados.ServiceInstance{}
        }
        if oc.AttachmentOnlyHost != nil {
-               cluster.Services.WebDAVDownload.ExternalURL = arvados.URL{Host: *oc.AttachmentOnlyHost}
+               cluster.Services.WebDAVDownload.ExternalURL = arvados.URL{Host: *oc.AttachmentOnlyHost, Path: "/"}
        }
        if oc.ManagementToken != nil {
                cluster.ManagementToken = *oc.ManagementToken
index 401764c87add6e1e37dff63052302df34a3f5e54..186ffc3371ea6e80a80be442278fbf6b5cf596b4 100644 (file)
@@ -118,7 +118,7 @@ func (ldr *Loader) loadOldKeepstoreConfig(cfg *arvados.Config) error {
                return err
        }
 
-       myURL := arvados.URL{Scheme: "http"}
+       myURL := arvados.URL{Scheme: "http", Path: "/"}
        if oc.TLSCertificateFile != nil && oc.TLSKeyFile != nil {
                myURL.Scheme = "https"
        }
@@ -137,7 +137,7 @@ func (ldr *Loader) loadOldKeepstoreConfig(cfg *arvados.Config) error {
                cluster.TLS.Key = "file://" + *v
        }
        if v := oc.Listen; v != nil {
-               if _, ok := cluster.Services.Keepstore.InternalURLs[arvados.URL{Scheme: myURL.Scheme, Host: *v}]; ok {
+               if _, ok := cluster.Services.Keepstore.InternalURLs[arvados.URL{Scheme: myURL.Scheme, Host: *v, Path: "/"}]; ok {
                        // already listed
                        myURL.Host = *v
                } else if len(*v) > 1 && (*v)[0] == ':' {
@@ -537,6 +537,7 @@ func keepServiceURL(ks arvados.KeepService) arvados.URL {
        url := arvados.URL{
                Scheme: "http",
                Host:   net.JoinHostPort(ks.ServiceHost, strconv.Itoa(ks.ServicePort)),
+               Path:   "/",
        }
        if ks.ServiceSSLFlag {
                url.Scheme = "https"
index abc507fdadb94618fb56b5a4d49eda6b0b1540c4..dab308c9d154cf22e7d022d900378584de99ea0f 100644 (file)
@@ -109,7 +109,7 @@ Clusters:
     TLS: {Insecure: true}
     Services:
       Controller:
-        ExternalURL: "https://`+os.Getenv("ARVADOS_API_HOST")+`"
+        ExternalURL: "https://`+os.Getenv("ARVADOS_API_HOST")+`/"
 `, `
 Clusters:
   z1111:
@@ -120,7 +120,7 @@ Clusters:
         InternalURLs:
           "http://`+hostname+`:25107": {Rendezvous: `+s.ksByPort[25107].UUID[12:]+`}
       Controller:
-        ExternalURL: "https://`+os.Getenv("ARVADOS_API_HOST")+`"
+        ExternalURL: "https://`+os.Getenv("ARVADOS_API_HOST")+`/"
     SystemLogs:
       Format: text
       LogLevel: debug
@@ -254,7 +254,7 @@ Volumes:
   ReadOnly: true
   StorageAccountName: storageacctname
   StorageAccountKeyFile: `+secretkeyfile.Name()+`
-  StorageBaseURL: https://example.example
+  StorageBaseURL: https://example.example/
   ContainerName: testctr
   LocationConstraint: true
   AzureReplication: 4
@@ -268,7 +268,7 @@ Volumes:
        }, &arvados.AzureVolumeDriverParameters{
                StorageAccountName:   "storageacctname",
                StorageAccountKey:    "secretkeydata",
-               StorageBaseURL:       "https://example.example",
+               StorageBaseURL:       "https://example.example/",
                ContainerName:        "testctr",
                RequestTimeout:       arvados.Duration(time.Minute * 3),
                ListBlobsRetryDelay:  arvados.Duration(time.Minute * 4),
@@ -333,7 +333,7 @@ func (s *KeepstoreMigrationSuite) testDeprecatedVolume(c *check.C, oldconfigdata
                c.Check(v.Driver, check.Equals, expectvol.Driver)
                c.Check(v.Replication, check.Equals, expectvol.Replication)
 
-               avh, ok := v.AccessViaHosts[arvados.URL{Scheme: "http", Host: hostname + ":12345"}]
+               avh, ok := v.AccessViaHosts[arvados.URL{Scheme: "http", Host: hostname + ":12345", Path: "/"}]
                c.Check(ok, check.Equals, true)
                c.Check(avh.ReadOnly, check.Equals, expectvol.ReadOnly)
 
@@ -516,6 +516,7 @@ Volumes:
        url := arvados.URL{
                Scheme: "http",
                Host:   fmt.Sprintf("%s:%d", hostname, port),
+               Path:   "/",
        }
        _, ok := before["zzzzz-nyw5e-readonlyonother"].AccessViaHosts[url]
        c.Check(ok, check.Equals, false)
@@ -543,6 +544,7 @@ Volumes:
        url := arvados.URL{
                Scheme: "http",
                Host:   fmt.Sprintf("%s:%d", hostname, port),
+               Path:   "/",
        }
        _, ok := before["zzzzz-nyw5e-writableonother"].AccessViaHosts[url]
        c.Check(ok, check.Equals, false)
@@ -572,7 +574,7 @@ Volumes:
 
        hostname, err := os.Hostname()
        c.Assert(err, check.IsNil)
-       _, ok := newvol.AccessViaHosts[arvados.URL{Scheme: "http", Host: fmt.Sprintf("%s:%d", hostname, port)}]
+       _, ok := newvol.AccessViaHosts[arvados.URL{Scheme: "http", Host: fmt.Sprintf("%s:%d", hostname, port), Path: "/"}]
        c.Check(ok, check.Equals, true)
 }
 
@@ -601,7 +603,7 @@ Volumes:
        c.Check(logs, check.Matches, `(?ms).*you should remove the legacy keepstore config file.*`)
        c.Check(logs, check.Matches, `(?ms).*you should migrate the legacy keepstore configuration file on host keep1.zzzzz.example.com.*`)
        c.Check(logs, check.Not(check.Matches), `(?ms).*should migrate.*keep0.zzzzz.example.com.*`)
-       c.Check(logs, check.Matches, `(?ms).*keepstore configured at http://keep2.zzzzz.example.com:25107 does not have access to any volumes.*`)
+       c.Check(logs, check.Matches, `(?ms).*keepstore configured at http://keep2.zzzzz.example.com:25107/ does not have access to any volumes.*`)
        c.Check(logs, check.Matches, `(?ms).*Volumes.zzzzz-nyw5e-possconfigerror.AccessViaHosts refers to nonexistent keepstore server http://keep00.zzzzz.example.com:25107.*`)
 }
 
index 58c27e984ad4b6ce17176a15f43c6a69ac9df0ee..8a49c2cf8d2c7b744712f809f525a2d82afa36b5 100644 (file)
@@ -117,7 +117,7 @@ func (s *LoadSuite) TestLegacyKeepWebConfig(c *check.C) {
        cluster, err := testLoadLegacyConfig(content, "-legacy-keepweb-config", c)
        c.Check(err, check.IsNil)
 
-       c.Check(cluster.Services.Controller.ExternalURL, check.Equals, arvados.URL{Scheme: "https", Host: "example.com"})
+       c.Check(cluster.Services.Controller.ExternalURL, check.Equals, arvados.URL{Scheme: "https", Host: "example.com", Path: "/"})
        c.Check(cluster.SystemRootToken, check.Equals, "abcdefg")
 
        c.Check(cluster.Collections.WebDAVCache.TTL, check.Equals, arvados.Duration(60*time.Second))
@@ -127,7 +127,7 @@ func (s *LoadSuite) TestLegacyKeepWebConfig(c *check.C) {
        c.Check(cluster.Collections.WebDAVCache.MaxPermissionEntries, check.Equals, 100)
        c.Check(cluster.Collections.WebDAVCache.MaxUUIDEntries, check.Equals, 100)
 
-       c.Check(cluster.Services.WebDAVDownload.ExternalURL, check.Equals, arvados.URL{Host: "download.example.com"})
+       c.Check(cluster.Services.WebDAVDownload.ExternalURL, check.Equals, arvados.URL{Host: "download.example.com", Path: "/"})
        c.Check(cluster.Services.WebDAVDownload.InternalURLs[arvados.URL{Host: ":80"}], check.NotNil)
        c.Check(cluster.Services.WebDAV.InternalURLs[arvados.URL{Host: ":80"}], check.NotNil)
 
@@ -160,7 +160,7 @@ func (s *LoadSuite) TestLegacyKeepproxyConfig(c *check.C) {
 
        c.Check(err, check.IsNil)
        c.Check(cluster, check.NotNil)
-       c.Check(cluster.Services.Controller.ExternalURL, check.Equals, arvados.URL{Scheme: "https", Host: "example.com"})
+       c.Check(cluster.Services.Controller.ExternalURL, check.Equals, arvados.URL{Scheme: "https", Host: "example.com", Path: "/"})
        c.Check(cluster.SystemRootToken, check.Equals, "abcdefg")
        c.Check(cluster.ManagementToken, check.Equals, "xyzzy")
        c.Check(cluster.Services.Keepproxy.InternalURLs[arvados.URL{Host: ":80"}], check.Equals, arvados.ServiceInstance{})
@@ -228,7 +228,7 @@ func (s *LoadSuite) TestLegacyArvGitHttpdConfig(c *check.C) {
 
        c.Check(err, check.IsNil)
        c.Check(cluster, check.NotNil)
-       c.Check(cluster.Services.Controller.ExternalURL, check.Equals, arvados.URL{Scheme: "https", Host: "example.com"})
+       c.Check(cluster.Services.Controller.ExternalURL, check.Equals, arvados.URL{Scheme: "https", Host: "example.com", Path: "/"})
        c.Check(cluster.SystemRootToken, check.Equals, "abcdefg")
        c.Check(cluster.ManagementToken, check.Equals, "xyzzy")
        c.Check(cluster.Git.GitCommand, check.Equals, "/test/git")
index 1e7a9a36edd3a8142192d14bfcfbf12885e1e857..901fda22897cd301d60539c372d77bc6815a799e 100644 (file)
@@ -177,6 +177,9 @@ func getListenAddr(svcs arvados.Services, prog arvados.ServiceName, log logrus.F
        } else if url, err := url.Parse(want); err != nil {
                return arvados.URL{}, fmt.Errorf("$ARVADOS_SERVICE_INTERNAL_URL (%q): %s", want, err)
        } else {
+               if url.Path == "" {
+                       url.Path = "/"
+               }
                return arvados.URL(*url), nil
        }
 
index 38de6b8ea40983081a14db18b270e3db31a6a895..69de3f05e231d29321ddbaa0b0f5f6dc1d5659e0 100644 (file)
@@ -303,6 +303,10 @@ func (su *URL) UnmarshalText(text []byte) error {
        u, err := url.Parse(string(text))
        if err == nil {
                *su = URL(*u)
+               if su.Path == "" && su.Host != "" {
+                       // http://example really means http://example/
+                       su.Path = "/"
+               }
        }
        return err
 }
index e4d26e03fd3f8101ad339f648b1efbaa56208437..8c77e292875f23b536edcf8dbdef2a1d4f46d51d 100644 (file)
@@ -5,6 +5,8 @@
 package arvados
 
 import (
+       "encoding/json"
+
        "github.com/ghodss/yaml"
        check "gopkg.in/check.v1"
 )
@@ -71,3 +73,10 @@ func (s *ConfigSuite) TestInstanceTypeFixup(c *check.C) {
                c.Check(itm["foo8"].IncludedScratch, check.Equals, ByteSize(0))
        }
 }
+
+func (s *ConfigSuite) TestURLTrailingSlash(c *check.C) {
+       var a, b map[URL]bool
+       json.Unmarshal([]byte(`{"https://foo.example": true}`), &a)
+       json.Unmarshal([]byte(`{"https://foo.example/": true}`), &b)
+       c.Check(a, check.DeepEquals, b)
+}
index 85f32772b17d97ec104a070d4f39f89b973e19a0..582b98cf2dc9d9e20b88cf0180b7a9db19fbfd8f 100644 (file)
@@ -89,7 +89,7 @@ class UserSessionsController < ApplicationController
 
     flash[:notice] = 'You have logged off'
     return_to = params[:return_to] || root_url
-    redirect_to "#{Rails.configuration.Services.SSO.ExternalURL}/users/sign_out?redirect_uri=#{CGI.escape return_to}"
+    redirect_to "#{Rails.configuration.Services.SSO.ExternalURL}users/sign_out?redirect_uri=#{CGI.escape return_to}"
   end
 
   # login - Just bounce to /auth/joshid. The only purpose of this function is
index 5610999a9405c05464279a8031ec2bc13ae55bf1..35a318b94fe1f5bf4bc822f06608c8b031854a1e 100644 (file)
@@ -11,7 +11,7 @@ if defined? CUSTOM_PROVIDER_URL
   Rails.logger.warn "Copying omniauth from globals in legacy config file."
   Rails.configuration.Login["ProviderAppID"] = APP_ID
   Rails.configuration.Login["ProviderAppSecret"] = APP_SECRET
-  Rails.configuration.Services["SSO"]["ExternalURL"] = CUSTOM_PROVIDER_URL
+  Rails.configuration.Services["SSO"]["ExternalURL"] = CUSTOM_PROVIDER_URL.sub(/\/$/, "") + "/"
 else
   Rails.application.config.middleware.use OmniAuth::Builder do
     provider(:josh_id,
index 3c14f7044c7f8f6a9d9213ec0656af9c88d80cb9..480434de65d291fad8ac2ac8ff8fbb6092ece327 100644 (file)
@@ -424,7 +424,7 @@ BatchSize: 99
        err = s.disp.configure("crunch-dispatch-slurm", []string{"-config", tmpfile.Name()})
        c.Check(err, IsNil)
 
-       c.Check(s.disp.cluster.Services.Controller.ExternalURL, Equals, arvados.URL{Scheme: "https", Host: "example.com"})
+       c.Check(s.disp.cluster.Services.Controller.ExternalURL, Equals, arvados.URL{Scheme: "https", Host: "example.com", Path: "/"})
        c.Check(s.disp.cluster.SystemRootToken, Equals, "abcdefg")
        c.Check(s.disp.cluster.Containers.SLURM.SbatchArgumentsList, DeepEquals, []string{"--foo", "bar"})
        c.Check(s.disp.cluster.Containers.CloudVMs.PollInterval, Equals, arvados.Duration(12*time.Second))
index 188173274f2e40b86f14f6ed00d5b6541c4129e6..0191e5ba45391e4058b24e014ae4d2feab16d0e2 100644 (file)
@@ -119,7 +119,7 @@ func run(logger log.FieldLogger, cluster *arvados.Cluster) error {
        // If a config file is available, use the keepstores defined there
        // instead of the legacy autodiscover mechanism via the API server
        for k := range cluster.Services.Keepstore.InternalURLs {
-               arv.KeepServiceURIs = append(arv.KeepServiceURIs, k.String())
+               arv.KeepServiceURIs = append(arv.KeepServiceURIs, strings.TrimRight(k.String(), "/"))
        }
 
        if cluster.SystemLogs.LogLevel == "debug" {
index 7213dcad2a9ddbb967991d70a2f9b094ce317b98..13726836a4f4263d281a73539fa37653b4dc284a 100644 (file)
@@ -198,7 +198,7 @@ ManagementToken: qqqqq
        c.Check(err, check.IsNil)
        c.Check(cluster, check.NotNil)
 
-       c.Check(cluster.Services.Controller.ExternalURL, check.Equals, arvados.URL{Scheme: "https", Host: "example.com"})
+       c.Check(cluster.Services.Controller.ExternalURL, check.Equals, arvados.URL{Scheme: "https", Host: "example.com", Path: "/"})
        c.Check(cluster.SystemRootToken, check.Equals, "abcdefg")
 
        c.Check(cluster.PostgreSQL.Connection, check.DeepEquals, arvados.PostgreSQLConnection{