Match logic and signature for computeExternalURL

Signed-off-by: Sven Nebel <[email protected]>

go.mod

@@ -10,7 +10,6 @@ require (
 	github.com/prometheus/client_golang v1.0.0
 	github.com/prometheus/client_model v0.0.0-20190129233127-fd36f4220a90
 	github.com/prometheus/common v0.6.0
-	github.com/stretchr/testify v1.3.0
 	golang.org/x/crypto v0.0.0-20190617133340-57b3e21c3d56 // indirect
 	golang.org/x/net v0.0.0-20190613194153-d28f0bde5980
 	golang.org/x/sync v0.0.0-20190423024810-112230192c58 // indirect

main.go

@@ -224,23 +224,19 @@ func run() int {
 	level.Info(logger).Log("msg", "Loaded config file")
 
 	// Infer or set Blackbox exporter externalURL
-	beURL, err := extURL(logger, os.Hostname, *listenAddress, *externalURL)
+	beURL, err := computeExternalURL(*externalURL, *listenAddress)
 	if err != nil {
 		level.Error(logger).Log("msg", "failed to determine external URL", "err", err)
 		return 1
 	}
 	level.Debug(logger).Log("externalURL", beURL.String())
 
-	// Make routePrefix default to externalURL path if empty string.
+	// Default -web.route-prefix to path of -web.external-url.
 	if *routePrefix == "" {
 		*routePrefix = beURL.Path
 	}
+	// RoutePrefix must always be at least '/'.
 	*routePrefix = "/" + strings.Trim(*routePrefix, "/")
-	level.Debug(logger).Log("routePrefix", *routePrefix)
-
-	if *routePrefix == "/" {
-		*routePrefix = ""
-	}
 
 	hup := make(chan os.Signal, 1)
 	reloadCh := make(chan chan error)
@@ -394,36 +390,40 @@ func getTimeout(r *http.Request, module config.Module, offset float64) (timeoutS
 	return timeoutSeconds, nil
 }
 
-func extURL(logger log.Logger, hostnamef func() (string, error), listen, external string) (*url.URL, error) {
-	if external == "" {
-		hostname, err := hostnamef()
+func startsOrEndsWithQuote(s string) bool {
+	return strings.HasPrefix(s, "\"") || strings.HasPrefix(s, "'") ||
+		strings.HasSuffix(s, "\"") || strings.HasSuffix(s, "'")
+}
+
+// computeExternalURL computes a sanitized external URL from a raw input. It infers unset
+// URL parts from the OS and the given listen address.
+func computeExternalURL(u, listenAddr string) (*url.URL, error) {
+	if u == "" {
+		hostname, err := os.Hostname()
 		if err != nil {
 			return nil, err
 		}
-		_, port, err := net.SplitHostPort(listen)
+		_, port, err := net.SplitHostPort(listenAddr)
 		if err != nil {
 			return nil, err
 		}
-		if port == "" {
-			level.Warn(logger).Log("msg", "no port found for listen address", "address", listen)
-		}
+		u = fmt.Sprintf("http://%s:%s/", hostname, port)
+	}
 
-		external = fmt.Sprintf("http://%s:%s/", hostname, port)
+	if startsOrEndsWithQuote(u) {
+		return nil, errors.New("URL must not begin or end with quotes")
 	}
 
-	u, err := url.Parse(external)
+	eu, err := url.Parse(u)
 	if err != nil {
 		return nil, err
 	}
-	if u.Scheme != "http" && u.Scheme != "https" {
-		return nil, errors.Errorf("%q: invalid %q scheme, only 'http' and 'https' are supported", u.String(), u.Scheme)
-	}
 
-	ppref := strings.TrimRight(u.Path, "/")
+	ppref := strings.TrimRight(eu.Path, "/")
 	if ppref != "" && !strings.HasPrefix(ppref, "/") {
 		ppref = "/" + ppref
 	}
-	u.Path = ppref
+	eu.Path = ppref
 
-	return u, nil
+	return eu, nil
 }

main_test.go

@@ -15,15 +15,12 @@ package main
 
 import (
 	"bytes"
-	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/require"
-
 	"github.com/go-kit/kit/log"
 	"github.com/prometheus/client_golang/prometheus"
 	pconfig "github.com/prometheus/common/config"
@@ -141,78 +138,55 @@ func TestTimeoutIsSetCorrectly(t *testing.T) {
 	}
 }
 
-func TestExternalURL(t *testing.T) {
-	hostname := "foo"
-	for _, tc := range []struct {
-		hostnameResolver func() (string, error)
-		external         string
-		listen           string
-
-		expURL string
-		err    bool
+func TestComputeExternalURL(t *testing.T) {
+	tests := []struct {
+		input string
+		valid bool
 	}{
 		{
-			listen: ":9093",
-			expURL: "http://" + hostname + ":9093",
-		},
-		{
-			listen: "localhost:9093",
-			expURL: "http://" + hostname + ":9093",
-		},
-		{
-			listen: "localhost:",
-			expURL: "http://" + hostname + ":",
-		},
-		{
-			external: "https://host.example.com",
-			expURL:   "https://host.example.com",
-		},
-		{
-			external: "https://host.example.com/",
-			expURL:   "https://host.example.com",
+			input: "",
+			valid: true,
 		},
 		{
-			external: "http://host.example.com/alertmanager",
-			expURL:   "http://host.example.com/alertmanager",
+			input: "http://proxy.com/prometheus",
+			valid: true,
 		},
 		{
-			external: "http://host.example.com/alertmanager/",
-			expURL:   "http://host.example.com/alertmanager",
+			input: "'https://url/prometheus'",
+			valid: false,
 		},
 		{
-			external: "http://host.example.com/////alertmanager//",
-			expURL:   "http://host.example.com/////alertmanager",
+			input: "'relative/path/with/quotes'",
+			valid: false,
 		},
 		{
-			err: true,
+			input: "http://alertmanager.company.com",
+			valid: true,
 		},
 		{
-			hostnameResolver: func() (string, error) { return "", fmt.Errorf("some error") },
-			err:              true,
+			input: "https://double--dash.de",
+			valid: true,
 		},
 		{
-			external: "://broken url string",
-			err:      true,
+			input: "'http://starts/with/quote",
+			valid: false,
 		},
 		{
-			external: "host.example.com:8080",
-			err:      true,
+			input: "ends/with/quote\"",
+			valid: false,
 		},
-	} {
-		tc := tc
-		if tc.hostnameResolver == nil {
-			tc.hostnameResolver = func() (string, error) {
-				return hostname, nil
+	}
+
+	for _, test := range tests {
+		_, err := computeExternalURL(test.input, "0.0.0.0:9090")
+		if test.valid {
+			if err != nil {
+				t.Errorf("unexpected error %v", err)
 			}
-		}
-		t.Run(fmt.Sprintf("external=%q,listen=%q", tc.external, tc.listen), func(t *testing.T) {
-			u, err := extURL(log.NewNopLogger(), tc.hostnameResolver, tc.listen, tc.external)
-			if tc.err {
-				require.Error(t, err)
-				return
+		} else {
+			if err == nil {
+				t.Errorf("expected error computing %s got none", test.input)
 			}
-			require.NoError(t, err)
-			require.Equal(t, tc.expURL, u.String())
-		})
+		}
 	}
 }