Fix JSON encoding of metrics options which broke the index but didn't break tests.

Also add tests that do catch that error.

agent/http.go

@@ -29,6 +29,7 @@ import (
 	"github.com/hashicorp/go-cleanhttp"
 	"github.com/mitchellh/mapstructure"
 	"github.com/pkg/errors"
+	"github.com/ryboe/q"
 )
 
 // MethodNotAllowedError should be returned by a handler when the HTTP method is not allowed.
@@ -169,6 +170,7 @@ func (fs *settingsInjectedIndexFS) Open(name string) (http.File, error) {
 	}
 
 	content, err := ioutil.ReadAll(file)
+	q.Q(err)
 	if err != nil {
 		return nil, fmt.Errorf("failed reading index.html: %s", err)
 	}
@@ -182,6 +184,7 @@ func (fs *settingsInjectedIndexFS) Open(name string) (http.File, error) {
 
 	// First built an escaped, JSON blob from the settings passed.
 	bs, err := json.Marshal(fs.UISettings)
+	q.Q(string(bs))
 	if err != nil {
 		return nil, fmt.Errorf("failed marshalling UI settings JSON: %s", err)
 	}
@@ -401,10 +404,9 @@ func (s *HTTPHandlers) GetUIENVFromConfig() map[string]interface{} {
 	uiCfg := s.agent.getUIConfig()
 
 	vars := map[string]interface{}{
-		"CONSUL_CONTENT_PATH":             uiCfg.ContentPath,
+		"CONSUL_CONTENT_PATH":     uiCfg.ContentPath,
-		"CONSUL_ACLS_ENABLED":             s.agent.config.ACLsEnabled,
+		"CONSUL_ACLS_ENABLED":     s.agent.config.ACLsEnabled,
-		"CONSUL_METRICS_PROVIDER":         uiCfg.MetricsProvider,
+		"CONSUL_METRICS_PROVIDER": uiCfg.MetricsProvider,
-		"CONSUL_METRICS_PROVIDER_OPTIONS": json.RawMessage(uiCfg.MetricsProviderOptionsJSON),
 		// We explicitly MUST NOT pass the metrics_proxy object since it might
 		// contain add_headers with secrets that the UI shouldn't know e.g. API
 		// tokens for the backend. The provider should either require the proxy to
@@ -414,6 +416,12 @@ func (s *HTTPHandlers) GetUIENVFromConfig() map[string]interface{} {
 		"CONSUL_DASHBOARD_URL_TEMPLATES": uiCfg.DashboardURLTemplates,
 	}
 
+	// Only set this if there is some actual JSON or we'll cause a JSON
+	// marshalling error later during serving which ends up being silent.
+	if uiCfg.MetricsProviderOptionsJSON != "" {
+		vars["CONSUL_METRICS_PROVIDER_OPTIONS"] = json.RawMessage(uiCfg.MetricsProviderOptionsJSON)
+	}
+
 	s.addEnterpriseUIENVVars(vars)
 
 	return vars

agent/http_test.go

@@ -1198,16 +1198,46 @@ func TestACLResolution(t *testing.T) {
 func TestEnableWebUI(t *testing.T) {
 	t.Parallel()
 	a := NewTestAgent(t, `
-		ui = true
+		ui_config {
+			enabled = true
+		}
 	`)
 	defer a.Shutdown()
 
 	req, _ := http.NewRequest("GET", "/ui/", nil)
 	resp := httptest.NewRecorder()
 	a.srv.handler(true).ServeHTTP(resp, req)
-	if resp.Code != 200 {
+	require.Equal(t, http.StatusOK, resp.Code)
-		t.Fatalf("should handle ui")
+
-	}
+	// Validate that it actually sent the index page we expect since an error
+	// during serving the special intercepted index.html in
+	// settingsInjectedIndexFS.Open will actually result in http.FileServer just
+	// serving a plain directory listing instead which still passes the above HTTP
+	// status assertion. This comment is part of our index.html template
+	require.Contains(t, resp.Body.String(), `<!-- CONSUL_VERSION:`)
+}
+
+func TestEnableWebUIWithMetricsOptions(t *testing.T) {
+	t.Parallel()
+	a := NewTestAgent(t, `
+		ui_config {
+			enabled = true
+			metrics_provider_options_json = "{\"foo\": 1}"
+		}
+	`)
+	defer a.Shutdown()
+
+	req, _ := http.NewRequest("GET", "/ui/", nil)
+	resp := httptest.NewRecorder()
+	a.srv.handler(true).ServeHTTP(resp, req)
+	require.Equal(t, http.StatusOK, resp.Code)
+
+	// Validate that it actually sent the index page we expect since an error
+	// during serving the special intercepted index.html in
+	// settingsInjectedIndexFS.Open will actually result in http.FileServer just
+	// serving a plain directory listing instead which still passes the above HTTP
+	// status assertion. This comment is part of our index.html template
+	require.Contains(t, resp.Body.String(), `<!-- CONSUL_VERSION:`)
 }
 
 func TestAllowedNets(t *testing.T) {