[arvados] updated: 2.1.0-2618-g2df5de69d
git repository hosting
git at public.arvados.org
Tue Jun 28 19:34:30 UTC 2022
Summary of changes:
lib/service/cmd.go | 19 ++++++--
lib/service/cmd_test.go | 120 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 134 insertions(+), 5 deletions(-)
via 2df5de69d74ab1fbf1fbcec23d392193522b0364 (commit)
via 04666c2304ea7700243756cfb0278cc139e969ce (commit)
from bc614b56c377861b1a51a83778a02320c09025ce (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 2df5de69d74ab1fbf1fbcec23d392193522b0364
Author: Tom Clegg <tom at curii.com>
Date: Tue Jun 28 15:28:59 2022 -0400
16561: Handle implicit port numbers in getListenAddress, add tests.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/service/cmd.go b/lib/service/cmd.go
index b5e395bec..9e45e0f7e 100644
--- a/lib/service/cmd.go
+++ b/lib/service/cmd.go
@@ -273,7 +273,15 @@ func getListenAddr(svcs arvados.Services, prog arvados.ServiceName, log logrus.F
// intermediate proxy/routing)
listenURL = internalURL
}
- listener, err := net.Listen("tcp", listenURL.Host)
+ listenAddr := listenURL.Host
+ if _, _, err := net.SplitHostPort(listenAddr); err != nil {
+ // url "https://foo.example/" (with no
+ // explicit port name/number) means listen on
+ // the well-known port for the specified
+ // protocol, "foo.example:https".
+ listenAddr = net.JoinHostPort(listenAddr, listenURL.Scheme)
+ }
+ listener, err := net.Listen("tcp", listenAddr)
if err == nil {
listener.Close()
return listenURL, internalURL, nil
diff --git a/lib/service/cmd_test.go b/lib/service/cmd_test.go
index 10591d9b5..7a1f98a8f 100644
--- a/lib/service/cmd_test.go
+++ b/lib/service/cmd_test.go
@@ -11,7 +11,9 @@ import (
"crypto/tls"
"fmt"
"io/ioutil"
+ "net"
"net/http"
+ "net/url"
"os"
"testing"
"time"
@@ -35,6 +37,124 @@ const (
contextKey key = iota
)
+func (*Suite) TestGetListenAddress(c *check.C) {
+ // Find an available port on the testing host, so the test
+ // cases don't get confused by "already in use" errors.
+ listener, err := net.Listen("tcp", ":")
+ c.Assert(err, check.IsNil)
+ _, unusedPort, err := net.SplitHostPort(listener.Addr().String())
+ c.Assert(err, check.IsNil)
+ listener.Close()
+
+ defer os.Unsetenv("ARVADOS_SERVICE_INTERNAL_URL")
+ for idx, trial := range []struct {
+ // internalURL => listenURL, both with trailing "/"
+ // because config loader always adds it
+ internalURLs map[string]string
+ envVar string
+ expectErrorMatch string
+ expectLogsMatch string
+ expectListen string
+ expectInternal string
+ }{
+ {
+ internalURLs: map[string]string{"http://localhost:" + unusedPort + "/": ""},
+ expectListen: "http://localhost:" + unusedPort + "/",
+ expectInternal: "http://localhost:" + unusedPort + "/",
+ },
+ { // implicit port 80 in InternalURLs
+ internalURLs: map[string]string{"http://localhost/": ""},
+ expectListen: "http://localhost/",
+ expectInternal: "http://localhost/",
+ expectErrorMatch: `.*:80: bind: permission denied`,
+ },
+ { // implicit port 443 in InternalURLs
+ internalURLs: map[string]string{"https://host.example/": "http://localhost:" + unusedPort + "/"},
+ expectListen: "http://localhost:" + unusedPort + "/",
+ expectInternal: "https://host.example/",
+ },
+ {
+ internalURLs: map[string]string{"https://hostname.example/": "http://localhost:8000/"},
+ expectListen: "http://localhost:8000/",
+ expectInternal: "https://hostname.example/",
+ },
+ {
+ internalURLs: map[string]string{
+ "https://hostname1.example/": "http://localhost:12435/",
+ "https://hostname2.example/": "http://localhost:" + unusedPort + "/",
+ },
+ envVar: "https://hostname2.example", // note this works despite missing trailing "/"
+ expectListen: "http://localhost:" + unusedPort + "/",
+ expectInternal: "https://hostname2.example/",
+ },
+ { // cannot listen on any of the ListenURLs
+ internalURLs: map[string]string{
+ "https://hostname1.example/": "http://1.2.3.4:" + unusedPort + "/",
+ "https://hostname2.example/": "http://1.2.3.4:" + unusedPort + "/",
+ },
+ expectErrorMatch: "configuration does not enable the \"arvados-controller\" service on this host",
+ },
+ { // cannot listen on any of the (implied) ListenURLs
+ internalURLs: map[string]string{
+ "https://1.2.3.4/": "",
+ "https://1.2.3.5/": "",
+ },
+ expectErrorMatch: "configuration does not enable the \"arvados-controller\" service on this host",
+ },
+ { // impossible port number
+ internalURLs: map[string]string{
+ "https://host.example/": "http://0.0.0.0:1234567",
+ },
+ expectErrorMatch: `.*:1234567: listen tcp: address 1234567: invalid port`,
+ },
+ {
+ // env var URL not mentioned in config = obey env var, with warning
+ internalURLs: map[string]string{"https://hostname1.example/": "http://localhost:8000/"},
+ envVar: "https://hostname2.example",
+ expectListen: "https://hostname2.example/",
+ expectInternal: "https://hostname2.example/",
+ expectLogsMatch: `.*\Qpossible configuration error: listening on https://hostname2.example/ (from $ARVADOS_SERVICE_INTERNAL_URL) even though configuration does not have a matching InternalURLs entry\E.*\n`,
+ },
+ {
+ // env var + empty config = obey env var, with warning
+ envVar: "https://hostname.example",
+ expectListen: "https://hostname.example/",
+ expectInternal: "https://hostname.example/",
+ expectLogsMatch: `.*\Qpossible configuration error: listening on https://hostname.example/ (from $ARVADOS_SERVICE_INTERNAL_URL) even though configuration does not have a matching InternalURLs entry\E.*\n`,
+ },
+ } {
+ c.Logf("trial %d %+v", idx, trial)
+ os.Setenv("ARVADOS_SERVICE_INTERNAL_URL", trial.envVar)
+ var logbuf bytes.Buffer
+ log := ctxlog.New(&logbuf, "text", "info")
+ services := arvados.Services{Controller: arvados.Service{InternalURLs: map[arvados.URL]arvados.ServiceInstance{}}}
+ for k, v := range trial.internalURLs {
+ u, err := url.Parse(k)
+ c.Assert(err, check.IsNil)
+ si := arvados.ServiceInstance{}
+ if v != "" {
+ u, err := url.Parse(v)
+ c.Assert(err, check.IsNil)
+ si.ListenURL = arvados.URL(*u)
+ }
+ services.Controller.InternalURLs[arvados.URL(*u)] = si
+ }
+ listenURL, internalURL, err := getListenAddr(services, "arvados-controller", log)
+ if trial.expectLogsMatch != "" {
+ c.Check(logbuf.String(), check.Matches, trial.expectLogsMatch)
+ }
+ if trial.expectErrorMatch != "" {
+ c.Check(err, check.ErrorMatches, trial.expectErrorMatch)
+ continue
+ }
+ if !c.Check(err, check.IsNil) {
+ continue
+ }
+ c.Check(listenURL.String(), check.Equals, trial.expectListen)
+ c.Check(internalURL.String(), check.Equals, trial.expectInternal)
+ }
+}
+
func (*Suite) TestCommand(c *check.C) {
cf, err := ioutil.TempFile("", "cmd_test.")
c.Assert(err, check.IsNil)
commit 04666c2304ea7700243756cfb0278cc139e969ce
Author: Tom Clegg <tom at curii.com>
Date: Tue Jun 28 10:39:33 2022 -0400
16561: Rearrange control flow for clarity.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/service/cmd.go b/lib/service/cmd.go
index 5687ec1da..b5e395bec 100644
--- a/lib/service/cmd.go
+++ b/lib/service/cmd.go
@@ -241,10 +241,11 @@ func getListenAddr(svcs arvados.Services, prog arvados.ServiceName, log logrus.F
return arvados.URL{}, arvados.URL{}, fmt.Errorf("unknown service name %q", prog)
}
- if want := os.Getenv("ARVADOS_SERVICE_INTERNAL_URL"); want == "" {
- } else if url, err := url.Parse(want); err != nil {
- return arvados.URL{}, arvados.URL{}, fmt.Errorf("$ARVADOS_SERVICE_INTERNAL_URL (%q): %s", want, err)
- } else {
+ if want := os.Getenv("ARVADOS_SERVICE_INTERNAL_URL"); want != "" {
+ url, err := url.Parse(want)
+ if err != nil {
+ return arvados.URL{}, arvados.URL{}, fmt.Errorf("$ARVADOS_SERVICE_INTERNAL_URL (%q): %s", want, err)
+ }
if url.Path == "" {
url.Path = "/"
}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list