16561: Replace ListenAddr with per-internal-url ListenURL.
authorTom Clegg <tom@curii.com>
Wed, 15 Jun 2022 18:54:28 +0000 (14:54 -0400)
committerTom Clegg <tom@curii.com>
Tue, 21 Jun 2022 13:50:59 +0000 (09:50 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

lib/config/cmd_test.go
lib/config/config.default.yml
lib/service/cmd.go
sdk/go/arvados/config.go

index 7167982ccd7021f3b43a895190909694c493b7da..9503a54d2d7c137ec5c2e805a3aaec9b990014ce 100644 (file)
@@ -217,7 +217,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 +ListenURL: ""\n.*`)
 }
 
 func (s *CommandSuite) TestDump_UnknownKey(c *check.C) {
index c321434cb137761153942a745c7260c61b7c6fa5..472a22c6b2cb11a3566d882e6420f52400ca4b13 100644 (file)
@@ -22,8 +22,8 @@ Clusters:
 
     Services:
 
-      # Each of the service sections below specifies ListenAddress,
-      # InternalURLs, and ExternalURL.
+      # Each of the service sections below specifies InternalURLs
+      # (each with optional ListenURL) and ExternalURL.
       #
       # InternalURLs specify how other Arvados service processes will
       # connect to the service. Typically these use internal hostnames
@@ -33,16 +33,21 @@ Clusters:
       #   "http://host1.internal.example:12345": {}
       #   "http://host2.internal.example:12345": {}
       #
-      # ListenAddress specifies the address and port the service
-      # process's HTTP server should listen on. Example:
+      # ListenURL specifies the address and port the service process's
+      # HTTP server should listen on, if different from the
+      # InternalURL itself. Example, using an intermediate TLS proxy:
       #
-      # ListenAddress: "0.0.0.0:12345"
+      # InternalURLs:
+      #   "https://host1.internal.example":
+      #     ListenURL: "http://10.0.0.7:12345"
       #
-      # If ListenAddress is blank, the service will try listening on
-      # the host:port part of each InternalURLs entry until one
-      # works. This approach only works if the host names resolve (via
-      # /etc/hosts, DNS, etc) to the IP addresses of the host's
-      # network interfaces.
+      # When there are multiple InternalURLs configured, the service
+      # process will try listening on each InternalURLs (using
+      # ListenURL if provided) until one works. If you use a ListenURL
+      # like "0.0.0.0" which can be bound on any machine, use an
+      # environment variable
+      # ARVADOS_SERVICE_INTERNAL_URL=http://host1.internal.example to
+      # control which entry to use.
       #
       # ExternalURL specifies how applications/clients will connect to
       # the service, regardless of whether they are inside or outside
@@ -58,46 +63,37 @@ Clusters:
       # address (or its external gateway or load balancer).
 
       RailsAPI:
-        ListenAddress: ""
-        InternalURLs: {SAMPLE: {}}
+        InternalURLs: {SAMPLE: {ListenURL: ""}}
         ExternalURL: ""
       Controller:
-        ListenAddress: ""
-        InternalURLs: {SAMPLE: {}}
+        InternalURLs: {SAMPLE: {ListenURL: ""}}
         ExternalURL: ""
       Websocket:
-        ListenAddress: ""
-        InternalURLs: {SAMPLE: {}}
+        InternalURLs: {SAMPLE: {ListenURL: ""}}
         ExternalURL: ""
       Keepbalance:
-        ListenAddress: ""
-        InternalURLs: {SAMPLE: {}}
+        InternalURLs: {SAMPLE: {ListenURL: ""}}
         ExternalURL: ""
       GitHTTP:
-        ListenAddress: ""
-        InternalURLs: {SAMPLE: {}}
+        InternalURLs: {SAMPLE: {ListenURL: ""}}
         ExternalURL: ""
       GitSSH:
-        ListenAddress: ""
-        InternalURLs: {SAMPLE: {}}
+        InternalURLs: {SAMPLE: {ListenURL: ""}}
         ExternalURL: ""
       DispatchCloud:
-        ListenAddress: ""
-        InternalURLs: {SAMPLE: {}}
+        InternalURLs: {SAMPLE: {ListenURL: ""}}
         ExternalURL: ""
       DispatchLSF:
-        InternalURLs: {SAMPLE: {}}
+        InternalURLs: {SAMPLE: {ListenURL: ""}}
         ExternalURL: ""
       DispatchSLURM:
-        InternalURLs: {SAMPLE: {}}
+        InternalURLs: {SAMPLE: {ListenURL: ""}}
         ExternalURL: ""
       Keepproxy:
-        ListenAddress: ""
-        InternalURLs: {SAMPLE: {}}
+        InternalURLs: {SAMPLE: {ListenURL: ""}}
         ExternalURL: ""
       WebDAV:
-        ListenAddress: ""
-        InternalURLs: {SAMPLE: {}}
+        InternalURLs: {SAMPLE: {ListenURL: ""}}
         # Base URL for Workbench inline preview.  If blank, use
         # WebDAVDownload instead, and disable inline preview.
         # If both are empty, downloading collections from workbench
@@ -136,8 +132,7 @@ Clusters:
         ExternalURL: ""
 
       WebDAVDownload:
-        ListenAddress: ""
-        InternalURLs: {SAMPLE: {}}
+        InternalURLs: {SAMPLE: {ListenURL: ""}}
         # Base URL for download links. If blank, serve links to WebDAV
         # with disposition=attachment query param.  Unlike preview links,
         # browsers do not render attachments, so there is no risk of XSS.
@@ -151,9 +146,9 @@ Clusters:
         ExternalURL: ""
 
       Keepstore:
-        ListenAddress: ""
         InternalURLs:
           SAMPLE:
+            ListenURL: ""
             # Rendezvous is normally empty/omitted. When changing the
             # URL of a Keepstore service, Rendezvous should be set to
             # the old URL (with trailing slash omitted) to preserve
@@ -161,12 +156,10 @@ Clusters:
             Rendezvous: ""
         ExternalURL: ""
       Composer:
-        ListenAddress: ""
-        InternalURLs: {SAMPLE: {}}
+        InternalURLs: {SAMPLE: {ListenURL: ""}}
         ExternalURL: ""
       WebShell:
-        ListenAddress: ""
-        InternalURLs: {SAMPLE: {}}
+        InternalURLs: {SAMPLE: {ListenURL: ""}}
         # ShellInABox service endpoint URL for a given VM.  If empty, do not
         # offer web shell logins.
         #
@@ -177,16 +170,13 @@ Clusters:
         # https://*.webshell.uuid_prefix.arvadosapi.com
         ExternalURL: ""
       Workbench1:
-        ListenAddress: ""
-        InternalURLs: {SAMPLE: {}}
+        InternalURLs: {SAMPLE: {ListenURL: ""}}
         ExternalURL: ""
       Workbench2:
-        ListenAddress: ""
-        InternalURLs: {SAMPLE: {}}
+        InternalURLs: {SAMPLE: {ListenURL: ""}}
         ExternalURL: ""
       Health:
-        ListenAddress: ""
-        InternalURLs: {SAMPLE: {}}
+        InternalURLs: {SAMPLE: {ListenURL: ""}}
         ExternalURL: ""
 
     PostgreSQL:
index 92c554ce532386352b3ecf766b3dcd8b2b885fb8..5687ec1dadcddb3f3f6a1e3458a46eeca45dc990 100644 (file)
@@ -223,6 +223,18 @@ func interceptHealthReqs(mgtToken string, checkHealth func() error, next http.Ha
        return ifCollectionInHost(next, mux)
 }
 
+// Determine listenURL (addr:port where server should bind) and
+// internalURL (target url that client should connect to) for a
+// service.
+//
+// If the config does not specify ListenURL, we check all of the
+// configured InternalURLs. If there is exactly one that matches our
+// hostname, or exactly one that matches a local interface address,
+// then we use that as listenURL.
+//
+// Note that listenURL and internalURL may use different protocols
+// (e.g., listenURL is http, but the service sits behind a proxy, so
+// clients connect using https).
 func getListenAddr(svcs arvados.Services, prog arvados.ServiceName, log logrus.FieldLogger) (arvados.URL, arvados.URL, error) {
        svc, ok := svcs.Map()[prog]
        if !ok {
@@ -236,51 +248,34 @@ func getListenAddr(svcs arvados.Services, prog arvados.ServiceName, log logrus.F
                if url.Path == "" {
                        url.Path = "/"
                }
-               internalURL := arvados.URL(*url)
-               listenURL := arvados.URL(*url)
-               if svc.ListenAddress != "" {
-                       listenURL.Host = svc.ListenAddress
-               }
-               return listenURL, internalURL, nil
-       }
-
-       if svc.ListenAddress != "" {
-               scheme := ""
-               for internalURL := range svc.InternalURLs {
-                       if internalURL.Host == svc.ListenAddress {
-                               if len(svc.InternalURLs) > 1 {
-                                       log.Warnf("possible configuration error: multiple InternalURLs entries exist for %s but only %q will ever be used because it matches ListenAddress", prog, internalURL.String())
+               for internalURL, conf := range svc.InternalURLs {
+                       if internalURL.String() == url.String() {
+                               listenURL := conf.ListenURL
+                               if listenURL.Host == "" {
+                                       listenURL = internalURL
                                }
-                               return internalURL, internalURL, nil
+                               return listenURL, internalURL, nil
                        }
-                       switch scheme {
-                       case "":
-                               scheme = internalURL.Scheme
-                       case internalURL.Scheme:
-                       default:
-                               scheme = "-" // different InternalURLs have different schemes
-                       }
-               }
-               if scheme == "-" {
-                       return arvados.URL{}, arvados.URL{}, fmt.Errorf("cannot use ListenAddress %q: InternalURLs use multiple schemes and none have host %q", svc.ListenAddress, svc.ListenAddress)
-               }
-               if scheme == "" {
-                       // No entries at all in InternalURLs
-                       scheme = "http"
                }
-               listenURL := arvados.URL{}
-               listenURL.Host = svc.ListenAddress
-               listenURL.Scheme = scheme
-               listenURL.Path = "/"
-               return listenURL, listenURL, nil
+               log.Warnf("possible configuration error: listening on %s (from $ARVADOS_SERVICE_INTERNAL_URL) even though configuration does not have a matching InternalURLs entry", url)
+               internalURL := arvados.URL(*url)
+               return internalURL, internalURL, nil
        }
 
        errors := []string{}
-       for internalURL := range svc.InternalURLs {
-               listener, err := net.Listen("tcp", internalURL.Host)
+       for internalURL, conf := range svc.InternalURLs {
+               listenURL := conf.ListenURL
+               if listenURL.Host == "" {
+                       // If ListenURL is not specified, assume
+                       // InternalURL is also usable as the listening
+                       // proto/addr/port (i.e., simple case with no
+                       // intermediate proxy/routing)
+                       listenURL = internalURL
+               }
+               listener, err := net.Listen("tcp", listenURL.Host)
                if err == nil {
                        listener.Close()
-                       return internalURL, internalURL, nil
+                       return listenURL, internalURL, nil
                } else if strings.Contains(err.Error(), "cannot assign requested address") {
                        // If 'Host' specifies a different server than
                        // the current one, it'll resolve the hostname
@@ -288,7 +283,7 @@ func getListenAddr(svcs arvados.Services, prog arvados.ServiceName, log logrus.F
                        // can't bind an IP address it doesn't own.
                        continue
                } else {
-                       errors = append(errors, fmt.Sprintf("tried %v, got %v", internalURL, err))
+                       errors = append(errors, fmt.Sprintf("%s: %s", listenURL, err))
                }
        }
        if len(errors) > 0 {
index ad663b23ef0cc8e1c83a994c2646b3df6116833a..c90551a6109af9dc9afbdd33bed9c78f5f7bc5ed 100644 (file)
@@ -366,9 +366,8 @@ type Services struct {
 }
 
 type Service struct {
-       ListenAddress string
-       InternalURLs  map[URL]ServiceInstance
-       ExternalURL   URL
+       InternalURLs map[URL]ServiceInstance
+       ExternalURL  URL
 }
 
 type TestUser struct {
@@ -402,6 +401,7 @@ func (su URL) String() string {
 }
 
 type ServiceInstance struct {
+       ListenURL  URL
        Rendezvous string `json:",omitempty"`
 }