From fb7dca3e3f07d37ad542f0e7cdf177f616493fe9 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Wed, 15 Jun 2022 14:54:28 -0400 Subject: [PATCH] 16561: Replace ListenAddr with per-internal-url ListenURL. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- lib/config/cmd_test.go | 2 +- lib/config/config.default.yml | 76 +++++++++++++++-------------------- lib/service/cmd.go | 71 +++++++++++++++----------------- sdk/go/arvados/config.go | 6 +-- 4 files changed, 70 insertions(+), 85 deletions(-) diff --git a/lib/config/cmd_test.go b/lib/config/cmd_test.go index 7167982ccd..9503a54d2d 100644 --- a/lib/config/cmd_test.go +++ b/lib/config/cmd_test.go @@ -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) { diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml index c321434cb1..472a22c6b2 100644 --- a/lib/config/config.default.yml +++ b/lib/config/config.default.yml @@ -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: diff --git a/lib/service/cmd.go b/lib/service/cmd.go index 92c554ce53..5687ec1dad 100644 --- a/lib/service/cmd.go +++ b/lib/service/cmd.go @@ -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 { diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go index ad663b23ef..c90551a610 100644 --- a/sdk/go/arvados/config.go +++ b/sdk/go/arvados/config.go @@ -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"` } -- 2.30.2