[ARVADOS] created: 1.3.0-2543-g3013c31a1

Git user git at public.arvados.org
Mon May 11 21:09:40 UTC 2020


        at  3013c31a1a981d54639dcb30b9a655f51abefc52 (commit)


commit 3013c31a1a981d54639dcb30b9a655f51abefc52
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Mon May 11 17:07:57 2020 -0400

    16392: Add trailing slash to URLs like https://example in configs.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/lib/config/cmd_test.go b/lib/config/cmd_test.go
index 3c420a04e..74c3cc969 100644
--- a/lib/config/cmd_test.go
+++ b/lib/config/cmd_test.go
@@ -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) {
diff --git a/lib/config/deprecated.go b/lib/config/deprecated.go
index 0689efa44..bbbc9acf7 100644
--- a/lib/config/deprecated.go
+++ b/lib/config/deprecated.go
@@ -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
diff --git a/lib/config/deprecated_keepstore.go b/lib/config/deprecated_keepstore.go
index 401764c87..186ffc337 100644
--- a/lib/config/deprecated_keepstore.go
+++ b/lib/config/deprecated_keepstore.go
@@ -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"
diff --git a/lib/config/deprecated_keepstore_test.go b/lib/config/deprecated_keepstore_test.go
index abc507fda..dab308c9d 100644
--- a/lib/config/deprecated_keepstore_test.go
+++ b/lib/config/deprecated_keepstore_test.go
@@ -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.*`)
 }
 
diff --git a/lib/config/deprecated_test.go b/lib/config/deprecated_test.go
index 58c27e984..8a49c2cf8 100644
--- a/lib/config/deprecated_test.go
+++ b/lib/config/deprecated_test.go
@@ -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")
diff --git a/lib/service/cmd.go b/lib/service/cmd.go
index 1e7a9a36e..901fda228 100644
--- a/lib/service/cmd.go
+++ b/lib/service/cmd.go
@@ -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
 	}
 
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 38de6b8ea..69de3f05e 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -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
 }
diff --git a/sdk/go/arvados/config_test.go b/sdk/go/arvados/config_test.go
index e4d26e03f..8c77e2928 100644
--- a/sdk/go/arvados/config_test.go
+++ b/sdk/go/arvados/config_test.go
@@ -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)
+}
diff --git a/services/api/app/controllers/user_sessions_controller.rb b/services/api/app/controllers/user_sessions_controller.rb
index 85f32772b..582b98cf2 100644
--- a/services/api/app/controllers/user_sessions_controller.rb
+++ b/services/api/app/controllers/user_sessions_controller.rb
@@ -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
diff --git a/services/api/config/initializers/omniauth_init.rb b/services/api/config/initializers/omniauth_init.rb
index 5610999a9..35a318b94 100644
--- a/services/api/config/initializers/omniauth_init.rb
+++ b/services/api/config/initializers/omniauth_init.rb
@@ -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,

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list