Allow overriding base path for UI/API routes; rm --query.prefix (#748)

Signed-off-by: Yuri Shkuro <[email protected]>
jaegertracing · Mar 23, 2018 · 83d06da · 83d06da
1 parent 37e6430
commit 83d06da
Show file tree

Hide file tree

Showing 12 changed files with 137 additions and 70 deletions.
diff --git a/cmd/query/app/fixture/index.html b/cmd/query/app/fixture/index.html
@@ -1,6 +1,7 @@
 <!DOCTYPE html>
 <html lang="en">
     <meta charset="UTF-8">
+    <base href="/" data-inject-target="BASE_URL"/>
     <title>Test Page</title>
     <!-- JAEGER_CONFIG=DEFAULT_CONFIG; -->
 </html>
diff --git a/cmd/query/app/fixture/static/asset.txt b/cmd/query/app/fixture/static/asset.txt
@@ -0,0 +1 @@
+some asset
diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go
@@ -22,7 +22,7 @@ import (
 
 const (
 	queryPort                = "query.port"
-	queryPrefix              = "query.prefix"
+	queryBasePath            = "query.base-path"
 	queryStaticFiles         = "query.static-files"
 	queryUIConfig            = "query.ui-config"
 	queryHealthCheckHTTPPort = "query.health-check-http-port"
@@ -32,8 +32,8 @@ const (
 type QueryOptions struct {
 	// Port is the port that the query service listens in on
 	Port int
-	// Prefix is the prefix of the query service api
-	Prefix string
+	// BasePath is the prefix for all UI and API HTTP routes
+	BasePath string
 	// StaticAssets is the path for the static assets for the UI (https://github.com/uber/jaeger-ui)
 	StaticAssets string
 	// UIConfig is the path to a configuration file for the UI
@@ -45,16 +45,16 @@ type QueryOptions struct {
 // AddFlags adds flags for QueryOptions
 func AddFlags(flagSet *flag.FlagSet) {
 	flagSet.Int(queryPort, 16686, "The port for the query service")
-	flagSet.String(queryPrefix, "api", "The prefix for the url of the query service")
-	flagSet.String(queryStaticFiles, "jaeger-ui-build/build/", "The path for the static assets for the UI")
+	flagSet.String(queryBasePath, "/", "The base path for all HTTP routes, e.g. /jaeger; useful when running behind a reverse proxy")
+	flagSet.String(queryStaticFiles, "jaeger-ui-build/build/", "The directory path for the static assets for the UI")
 	flagSet.String(queryUIConfig, "", "The path to the UI configuration file in JSON format")
 	flagSet.Int(queryHealthCheckHTTPPort, 16687, "The http port for the health check service")
 }
 
 // InitFromViper initializes QueryOptions with properties from viper
 func (qOpts *QueryOptions) InitFromViper(v *viper.Viper) *QueryOptions {
 	qOpts.Port = v.GetInt(queryPort)
-	qOpts.Prefix = v.GetString(queryPrefix)
+	qOpts.BasePath = v.GetString(queryBasePath)
 	qOpts.StaticAssets = v.GetString(queryStaticFiles)
 	qOpts.UIConfig = v.GetString(queryUIConfig)
 	qOpts.HealthCheckHTTPPort = v.GetInt(queryHealthCheckHTTPPort)

diff --git a/cmd/query/app/flags_test.go b/cmd/query/app/flags_test.go
@@ -27,12 +27,12 @@ func TestQueryBuilderFlags(t *testing.T) {
 	command.ParseFlags([]string{
 		"--query.static-files=/dev/null",
 		"--query.ui-config=some.json",
-		"--query.prefix=api",
+		"--query.base-path=/jaeger",
 		"--query.port=80",
 	})
 	qOpts := new(QueryOptions).InitFromViper(v)
 	assert.Equal(t, "/dev/null", qOpts.StaticAssets)
 	assert.Equal(t, "some.json", qOpts.UIConfig)
-	assert.Equal(t, "api", qOpts.Prefix)
+	assert.Equal(t, "/jaeger", qOpts.BasePath)
 	assert.Equal(t, 80, qOpts.Port)
 }
diff --git a/cmd/query/app/handler.go b/cmd/query/app/handler.go
@@ -43,7 +43,7 @@ const (
 
 	defaultDependencyLookbackDuration = time.Hour * 24
 	defaultTraceQueryLookbackDuration = time.Hour * 24 * 2
-	defaultHTTPPrefix                 = "api"
+	defaultAPIPrefix                  = "api"
 )
 
 var (
@@ -78,7 +78,8 @@ type APIHandler struct {
 	adjuster          adjuster.Adjuster
 	logger            *zap.Logger
 	queryParser       queryParser
-	httpPrefix        string
+	basePath          string
+	apiPrefix         string
 	tracer            opentracing.Tracer
 }
 
@@ -96,8 +97,8 @@ func NewAPIHandler(spanReader spanstore.Reader, dependencyReader dependencystore
 	for _, option := range options {
 		option(aH)
 	}
-	if aH.httpPrefix == "" {
-		aH.httpPrefix = defaultHTTPPrefix
+	if aH.apiPrefix == "" {
+		aH.apiPrefix = defaultAPIPrefix
 	}
 	if aH.adjuster == nil {
 		aH.adjuster = adjuster.Sequence(StandardAdjusters...)
@@ -141,7 +142,7 @@ func (aH *APIHandler) handleFunc(
 }
 
 func (aH *APIHandler) route(route string, args ...interface{}) string {
-	args = append([]interface{}{aH.httpPrefix}, args...)
+	args = append([]interface{}{aH.apiPrefix}, args...)
 	return fmt.Sprintf("/%s"+route, args...)
 }
 

diff --git a/cmd/query/app/handler_options.go b/cmd/query/app/handler_options.go
@@ -47,10 +47,17 @@ func (handlerOptions) Adjusters(adjusters ...adjuster.Adjuster) HandlerOption {
 	}
 }
 
-// Prefix creates a HandlerOption that initializes prefix HTTP prefix of the API
+// BasePath creates a HandlerOption that initializes the base path for all HTTP routes
+func (handlerOptions) BasePath(prefix string) HandlerOption {
+	return func(apiHandler *APIHandler) {
+		apiHandler.basePath = prefix
+	}
+}
+
+// Prefix creates a HandlerOption that initializes the HTTP prefix for API routes
 func (handlerOptions) Prefix(prefix string) HandlerOption {
 	return func(apiHandler *APIHandler) {
-		apiHandler.httpPrefix = prefix
+		apiHandler.apiPrefix = prefix
 	}
 }
 

diff --git a/cmd/query/app/handler_test.go b/cmd/query/app/handler_test.go
@@ -85,7 +85,9 @@ func initializeTestServerWithHandler(options ...HandlerOption) (*httptest.Server
 		append(
 			[]HandlerOption{
 				HandlerOptions.Logger(zap.NewNop()),
-				HandlerOptions.Prefix(defaultHTTPPrefix),
+				// add options for test coverage
+				HandlerOptions.Prefix(defaultAPIPrefix),
+				HandlerOptions.BasePath("/"),
 				HandlerOptions.QueryLookbackDuration(defaultTraceQueryLookbackDuration),
 			},
 			options...,

diff --git a/cmd/query/app/static_handler.go b/cmd/query/app/static_handler.go
@@ -29,13 +29,20 @@ import (
 )
 
 var (
-	staticRootFiles = []string{"favicon.ico"}
+	favoriteIcon    = "favicon.ico"
+	staticRootFiles = []string{favoriteIcon}
 	configPattern   = regexp.MustCompile("JAEGER_CONFIG *= *DEFAULT_CONFIG;")
+	basePathPattern = regexp.MustCompile(`<base href="/"`)
+	basePathReplace = `<base href="%s/"`
+	errBadBasePath  = "Invalid base path '%s'. Must start but not end with a slash '/', e.g. '/jaeger/ui'"
 )
 
 // RegisterStaticHandler adds handler for static assets to the router.
 func RegisterStaticHandler(r *mux.Router, logger *zap.Logger, qOpts *QueryOptions) {
-	staticHandler, err := NewStaticAssetsHandler(qOpts.StaticAssets, qOpts.UIConfig)
+	staticHandler, err := NewStaticAssetsHandler(qOpts.StaticAssets, StaticAssetsHandlerOptions{
+		BasePath:     qOpts.BasePath,
+		UIConfigPath: qOpts.UIConfig,
+	})
 	if err != nil {
 		logger.Panic("Could not create static assets handler", zap.Error(err))
 	}
@@ -48,12 +55,19 @@ func RegisterStaticHandler(r *mux.Router, logger *zap.Logger, qOpts *QueryOption
 
 // StaticAssetsHandler handles static assets
 type StaticAssetsHandler struct {
+	options          StaticAssetsHandlerOptions
 	staticAssetsRoot string
 	indexHTML        []byte
 }
 
+// StaticAssetsHandlerOptions defines options for NewStaticAssetsHandler
+type StaticAssetsHandlerOptions struct {
+	BasePath     string
+	UIConfigPath string
+}
+
 // NewStaticAssetsHandler returns a StaticAssetsHandler
-func NewStaticAssetsHandler(staticAssetsRoot string, uiConfig string) (*StaticAssetsHandler, error) {
+func NewStaticAssetsHandler(staticAssetsRoot string, options StaticAssetsHandlerOptions) (*StaticAssetsHandler, error) {
 	if staticAssetsRoot == "" {
 		return nil, nil
 	}
@@ -65,7 +79,7 @@ func NewStaticAssetsHandler(staticAssetsRoot string, uiConfig string) (*StaticAs
 		return nil, errors.Wrap(err, "Cannot read UI static assets")
 	}
 	configString := "JAEGER_CONFIG = DEFAULT_CONFIG"
-	if config, err := loadUIConfig(uiConfig); err != nil {
+	if config, err := loadUIConfig(options.UIConfigPath); err != nil {
 		return nil, err
 	} else if config != nil {
 		// TODO if we want to support other config formats like YAML, we need to normalize `config` to be
@@ -74,9 +88,20 @@ func NewStaticAssetsHandler(staticAssetsRoot string, uiConfig string) (*StaticAs
 		bytes, _ := json.Marshal(config)
 		configString = fmt.Sprintf("JAEGER_CONFIG = %v", string(bytes))
 	}
+	indexBytes = configPattern.ReplaceAll(indexBytes, []byte(configString+";"))
+	if options.BasePath == "" {
+		options.BasePath = "/"
+	}
+	if options.BasePath != "/" {
+		if !strings.HasPrefix(options.BasePath, "/") || strings.HasSuffix(options.BasePath, "/") {
+			return nil, fmt.Errorf(errBadBasePath, options.BasePath)
+		}
+		indexBytes = basePathPattern.ReplaceAll(indexBytes, []byte(fmt.Sprintf(basePathReplace, options.BasePath)))
+	}
 	return &StaticAssetsHandler{
+		options:          options,
 		staticAssetsRoot: staticAssetsRoot,
-		indexHTML:        configPattern.ReplaceAll(indexBytes, []byte(configString+";")),
+		indexHTML:        indexBytes,
 	}, nil
 }
 
@@ -108,7 +133,7 @@ func loadUIConfig(uiConfig string) (map[string]interface{}, error) {
 
 // RegisterRoutes registers routes for this handler on the given router
 func (sH *StaticAssetsHandler) RegisterRoutes(router *mux.Router) {
-	router.PathPrefix("/static").Handler(http.FileServer(http.Dir(sH.staticAssetsRoot)))
+	router.PathPrefix("/static").Handler(sH.fileHandler())
 	for _, file := range staticRootFiles {
 		router.Path("/" + file).HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			http.ServeFile(w, r, sH.staticAssetsRoot+file)
@@ -117,6 +142,22 @@ func (sH *StaticAssetsHandler) RegisterRoutes(router *mux.Router) {
 	router.NotFoundHandler = http.HandlerFunc(sH.notFound)
 }
 
+func (sH *StaticAssetsHandler) fileHandler() http.Handler {
+	fs := http.FileServer(http.Dir(sH.staticAssetsRoot))
+	base := sH.options.BasePath
+	if base == "/" {
+		return fs
+	}
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if strings.HasPrefix(r.URL.Path, base) {
+			// gorilla Subroute() is a bit odd, it keeps the base path in the URL,
+			// which prevents the FileServer from locating the files, so we strip the prefix.
+			r.URL.Path = r.URL.Path[len(base):]
+		}
+		fs.ServeHTTP(w, r)
+	})
+}
+
 func (sH *StaticAssetsHandler) notFound(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "text/html; charset=utf-8")
 	w.Write(sH.indexHTML)

diff --git a/cmd/query/app/static_handler_test.go b/cmd/query/app/static_handler_test.go
@@ -19,7 +19,6 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
-	"strings"
 	"testing"
 	"time"
 
@@ -30,32 +29,14 @@ import (
 	"github.com/jaegertracing/jaeger/pkg/testutils"
 )
 
-func TestStaticAssetsHandler(t *testing.T) {
-	r := mux.NewRouter()
-	handler, err := NewStaticAssetsHandler("fixture", "")
-	require.NoError(t, err)
-	handler.RegisterRoutes(r)
-	server := httptest.NewServer(r)
-	defer server.Close()
-
-	httpClient = &http.Client{
-		Timeout: 2 * time.Second,
-	}
-
-	resp, err := httpClient.Get(server.URL)
-	assert.NoError(t, err)
-	defer resp.Body.Close()
-	assert.Equal(t, http.StatusOK, resp.StatusCode)
-}
-
 func TestDefaultStaticAssetsRoot(t *testing.T) {
-	handler, err := NewStaticAssetsHandler("", "")
+	handler, err := NewStaticAssetsHandler("", StaticAssetsHandlerOptions{})
 	assert.Nil(t, handler)
 	assert.Nil(t, err)
 }
 
 func TestNotExistingUiConfig(t *testing.T) {
-	handler, err := NewStaticAssetsHandler("/foo/bar", "")
+	handler, err := NewStaticAssetsHandler("/foo/bar", StaticAssetsHandlerOptions{})
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "Cannot read UI static assets")
 	assert.Nil(t, handler)
@@ -77,40 +58,69 @@ func TestRegisterStaticHandlerNotCreated(t *testing.T) {
 }
 
 func TestRegisterStaticHandler(t *testing.T) {
-	logger, buf := testutils.NewLogger()
-	r := mux.NewRouter()
-	RegisterStaticHandler(r, logger, &QueryOptions{StaticAssets: "fixture/"})
-	assert.Empty(t, buf.String())
-
-	server := httptest.NewServer(r)
-	defer server.Close()
-	expectedRespString := "Test Favicon\n"
-
+	testCases := []struct {
+		basePath         string // input to the test
+		subroute         bool   // should we create a subroute?
+		baseURL          string // expected URL prefix
+		expectedBaseHTML string // substring to match in the home page
+	}{
+		{basePath: "", baseURL: "/", expectedBaseHTML: `<base href="/"`},
+		{basePath: "/", baseURL: "/", expectedBaseHTML: `<base href="/"`},
+		{basePath: "/jaeger", baseURL: "/jaeger/", expectedBaseHTML: `<base href="/jaeger/"`, subroute: true},
+	}
 	httpClient = &http.Client{
 		Timeout: 2 * time.Second,
 	}
+	for _, testCase := range testCases {
+		t.Run("basePath="+testCase.basePath, func(t *testing.T) {
+			logger, buf := testutils.NewLogger()
+			r := mux.NewRouter()
+			if testCase.subroute {
+				r = r.PathPrefix(testCase.basePath).Subrouter()
+			}
+			RegisterStaticHandler(r, logger, &QueryOptions{
+				StaticAssets: "fixture/",
+				BasePath:     testCase.basePath,
+				UIConfig:     "fixture/ui-config.json",
+			})
+			assert.Empty(t, buf.String(), "no logs during construction")
+
+			server := httptest.NewServer(r)
+			defer server.Close()
+
+			httpGet := func(path string) string {
+				resp, err := httpClient.Get(fmt.Sprintf("%s%s%s", server.URL, testCase.baseURL, path))
+				require.NoError(t, err)
+				defer resp.Body.Close()
+
+				respByteArray, err := ioutil.ReadAll(resp.Body)
+				require.NoError(t, err)
+				require.Equal(t, http.StatusOK, resp.StatusCode)
+				return string(respByteArray)
+			}
 
-	resp, err := httpClient.Get(fmt.Sprintf("%s/favicon.ico", server.URL))
-	require.NoError(t, err)
-	defer resp.Body.Close()
+			respString := httpGet(favoriteIcon)
+			assert.Contains(t, respString, "Test Favicon") // this text is present in fixtures/favicon.ico
 
-	respByteArray, _ := ioutil.ReadAll(resp.Body)
-	respString := string(respByteArray)
+			html := httpGet("") // get home page
+			assert.Contains(t, html, `JAEGER_CONFIG = {"x":"y"};`, "actual: %v", html)
+			assert.Contains(t, html, testCase.expectedBaseHTML, "actual: %v", html)
 
-	assert.NoError(t, err)
-	assert.Equal(t, http.StatusOK, resp.StatusCode)
-	assert.Equal(t, expectedRespString, respString)
+			asset := httpGet("static/asset.txt")
+			assert.Contains(t, asset, "some asset", "actual: %v", asset)
+		})
+	}
 }
 
-func TestNewStaticAssetsHandlerWithConfig(t *testing.T) {
-	_, err := NewStaticAssetsHandler("fixture", "fixture/invalid-config")
+func TestNewStaticAssetsHandlerErrors(t *testing.T) {
+	_, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{UIConfigPath: "fixture/invalid-config"})
 	assert.Error(t, err)
 
-	handler, err := NewStaticAssetsHandler("fixture", "fixture/ui-config.json")
-	require.NoError(t, err)
-	require.NotNil(t, handler)
-	html := string(handler.indexHTML)
-	assert.True(t, strings.Contains(html, `JAEGER_CONFIG = {"x":"y"};`), "actual: %v", html)
+	for _, base := range []string{"x", "x/", "/x/"} {
+		_, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{UIConfigPath: "fixture/ui-config.json", BasePath: base})
+		require.Error(t, err, "basePath=%s", base)
+		assert.Contains(t, err.Error(), "Invalid base path")
+	}
 }
 
 func TestLoadUIConfig(t *testing.T) {

diff --git a/cmd/query/main.go b/cmd/query/main.go
@@ -109,7 +109,6 @@ func main() {
 			}
 
 			apiHandlerOptions := []app.HandlerOption{
-				app.HandlerOptions.Prefix(queryOpts.Prefix),
 				app.HandlerOptions.Logger(logger),
 				app.HandlerOptions.Tracer(tracer),
 			}
@@ -119,6 +118,9 @@ func main() {
 				dependencyReader,
 				apiHandlerOptions...)
 			r := mux.NewRouter()
+			if queryOpts.BasePath != "/" {
+				r = r.PathPrefix(queryOpts.BasePath).Subrouter()
+			}
 			apiHandler.RegisterRoutes(r)
 			app.RegisterStaticHandler(r, logger, queryOpts)