diff --git a/.changelog/9923.txt b/.changelog/9923.txt new file mode 100644 index 000000000000..fb8f1bf2d9b8 --- /dev/null +++ b/.changelog/9923.txt @@ -0,0 +1,3 @@ +```release-notes:bug +http: fix a bug in Consul Enterprise that would cause the UI to believe namespaces were supported, resulting in warning logs and incorrect UI behaviour. +``` diff --git a/agent/http.go b/agent/http.go index 93c6cdc8f7aa..c4d228ea8a0a 100644 --- a/agent/http.go +++ b/agent/http.go @@ -287,7 +287,7 @@ func (s *HTTPHandlers) handler(enableDebug bool) http.Handler { uiHandler := uiserver.NewHandler( s.agent.config, s.agent.logger.Named(logging.HTTP), - s.uiTemplateDataTransform(), + s.uiTemplateDataTransform, ) s.configReloaders = append(s.configReloaders, uiHandler.ReloadConfig) @@ -297,10 +297,7 @@ func (s *HTTPHandlers) handler(enableDebug bool) http.Handler { uiHandler, s.agent.config.HTTPResponseHeaders, ) - mux.Handle( - "/robots.txt", - uiHandlerWithHeaders, - ) + mux.Handle("/robots.txt", uiHandlerWithHeaders) mux.Handle( s.agent.config.UIConfig.ContentPath, http.StripPrefix( diff --git a/agent/http_oss.go b/agent/http_oss.go index cb4f2c0a8e57..9eb46d90a8ab 100644 --- a/agent/http_oss.go +++ b/agent/http_oss.go @@ -8,7 +8,6 @@ import ( "strings" "github.com/hashicorp/consul/agent/structs" - "github.com/hashicorp/consul/agent/uiserver" ) func (s *HTTPHandlers) parseEntMeta(req *http.Request, entMeta *structs.EnterpriseMeta) error { @@ -71,6 +70,6 @@ func (s *HTTPHandlers) enterpriseHandler(next http.Handler) http.Handler { // uiTemplateDataTransform returns an optional uiserver.UIDataTransform to allow // altering UI data in enterprise. -func (s *HTTPHandlers) uiTemplateDataTransform() uiserver.UIDataTransform { +func (s *HTTPHandlers) uiTemplateDataTransform(data map[string]interface{}) error { return nil } diff --git a/agent/uiserver/uiserver.go b/agent/uiserver/uiserver.go index c98bcb4065b1..8a8bf7d01d90 100644 --- a/agent/uiserver/uiserver.go +++ b/agent/uiserver/uiserver.go @@ -12,9 +12,10 @@ import ( "sync/atomic" "text/template" + "github.com/hashicorp/go-hclog" + "github.com/hashicorp/consul/agent/config" "github.com/hashicorp/consul/logging" - "github.com/hashicorp/go-hclog" ) const ( @@ -26,20 +27,12 @@ const ( // transformations on the index.html file and includes a proxy for metrics // backends. type Handler struct { - // state is a reloadableState struct accessed through an atomic value to make + // runtimeConfig is a struct accessed through an atomic value to make // it safe to reload at run time. Each call to ServeHTTP will see the latest // version of the state without internal locking needed. - state atomic.Value - logger hclog.Logger - transform UIDataTransform -} - -// reloadableState encapsulates all the state that might be modified during -// ReloadConfig. -type reloadableState struct { - cfg *config.UIConfig - srv http.Handler - err error + runtimeConfig atomic.Value + logger hclog.Logger + transform UIDataTransform } // UIDataTransform is an optional dependency that allows the agent to add @@ -50,71 +43,60 @@ type reloadableState struct { // It is passed the current RuntimeConfig being applied and a map containing the // current data that will be passed to the template. It should be modified // directly to inject additional context. -type UIDataTransform func(cfg *config.RuntimeConfig, data map[string]interface{}) error +type UIDataTransform func(data map[string]interface{}) error // NewHandler returns a Handler that can be used to serve UI http requests. It // accepts a full agent config since properties like ACLs being enabled affect // the UI so we need more than just UIConfig parts. -func NewHandler(agentCfg *config.RuntimeConfig, logger hclog.Logger, transform UIDataTransform) *Handler { +func NewHandler(runtimeCfg *config.RuntimeConfig, logger hclog.Logger, transform UIDataTransform) *Handler { h := &Handler{ logger: logger.Named(logging.UIServer), transform: transform, } - // Don't return the error since this is likely the result of a - // misconfiguration and reloading config could fix it. Instead we'll capture - // it and return an error for all calls to ServeHTTP so the misconfiguration - // is visible. Sadly we can't log effectively - if err := h.ReloadConfig(agentCfg); err != nil { - h.state.Store(reloadableState{ - err: err, - }) - } + h.runtimeConfig.Store(runtimeCfg) return h } // ServeHTTP implements http.Handler and serves UI HTTP requests func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - // We need to support the path being trimmed by http.StripTags just like the // file servers do since http.StripPrefix will remove the leading slash in our // current config. Everything else works fine that way so we should to. pathTrimmed := strings.TrimLeft(r.URL.Path, "/") if pathTrimmed == compiledProviderJSPath { - h.serveUIMetricsProviders(w, r) + h.serveUIMetricsProviders(w) return } - s := h.getState() - if s == nil { - panic("nil state") - } - if s.err != nil { + srv, err := h.handleIndex() + if err != nil { http.Error(w, "UI server is misconfigured.", http.StatusInternalServerError) - h.logger.Error("Failed to configure UI server: %s", s.err) + h.logger.Error("Failed to configure UI server: %s", err) return } - s.srv.ServeHTTP(w, r) + srv.ServeHTTP(w, r) } // ReloadConfig is called by the agent when the configuration is reloaded and // updates the UIConfig values the handler uses to serve requests. func (h *Handler) ReloadConfig(newCfg *config.RuntimeConfig) error { - newState := reloadableState{ - cfg: &newCfg.UIConfig, - } + h.runtimeConfig.Store(newCfg) + return nil +} - var fs http.FileSystem +func (h *Handler) handleIndex() (http.Handler, error) { + cfg := h.getRuntimeConfig() - if newCfg.UIConfig.Dir == "" { - // Serve from assetFS + var fs http.FileSystem + if cfg.UIConfig.Dir == "" { fs = assetFS() } else { - fs = http.Dir(newCfg.UIConfig.Dir) + fs = http.Dir(cfg.UIConfig.Dir) } // Render a new index.html with the new config values ready to serve. - buf, info, err := h.renderIndex(newCfg, fs) - if _, ok := err.(*os.PathError); ok && newCfg.UIConfig.Dir != "" { + buf, info, err := h.renderIndex(cfg, fs) + if _, ok := err.(*os.PathError); ok && cfg.UIConfig.Dir != "" { // A Path error indicates that there is no index.html. This could happen if // the user configured their own UI dir and is serving something that is not // our usual UI. This won't work perfectly because our uiserver will still @@ -123,47 +105,39 @@ func (h *Handler) ReloadConfig(newCfg *config.RuntimeConfig) error { // breaking change although quite an edge case. Instead, continue but just // return a 404 response for the index.html and log a warning. h.logger.Warn("ui_config.dir does not contain an index.html. Index templating and redirects to index.html are disabled.") - } else if err != nil { - return err + return http.FileServer(fs), nil } - - // buf can be nil in the PathError case above. We should skip this part but - // still serve the rest of the files in that case. - if buf != nil { - // Create a new fs that serves the rendered index file or falls back to the - // underlying FS. - fs = &bufIndexFS{ - fs: fs, - indexRendered: buf, - indexInfo: info, - } - - // Wrap the buffering FS our redirect FS. This needs to happen later so that - // redirected requests for /index.html get served the rendered version not the - // original. - fs = &redirectFS{fs: fs} + if err != nil { + return nil, err } - newState.srv = http.FileServer(fs) + // Create a new fs that serves the rendered index file or falls back to the + // underlying FS. + fs = &bufIndexFS{ + fs: fs, + indexRendered: buf, + indexInfo: info, + } - // Store the new state - h.state.Store(newState) - return nil + // Wrap the buffering FS our redirect FS. This needs to happen later so that + // redirected requests for /index.html get served the rendered version not the + // original. + return http.FileServer(&redirectFS{fs: fs}), nil } -// getState is a helper to access the atomic internal state -func (h *Handler) getState() *reloadableState { - if cfg, ok := h.state.Load().(reloadableState); ok { - return &cfg +// getRuntimeConfig is a helper to atomically access the runtime config. +func (h *Handler) getRuntimeConfig() *config.RuntimeConfig { + if cfg, ok := h.runtimeConfig.Load().(*config.RuntimeConfig); ok { + return cfg } return nil } -func (h *Handler) serveUIMetricsProviders(resp http.ResponseWriter, req *http.Request) { +func (h *Handler) serveUIMetricsProviders(resp http.ResponseWriter) { // Reload config in case it's changed - state := h.getState() + cfg := h.getRuntimeConfig() - if len(state.cfg.MetricsProviderFiles) < 1 { + if len(cfg.UIConfig.MetricsProviderFiles) < 1 { http.Error(resp, "No provider JS files configured", http.StatusNotFound) return } @@ -171,7 +145,7 @@ func (h *Handler) serveUIMetricsProviders(resp http.ResponseWriter, req *http.Re var buf bytes.Buffer // Open each one and concatenate them - for _, file := range state.cfg.MetricsProviderFiles { + for _, file := range cfg.UIConfig.MetricsProviderFiles { if err := concatFile(&buf, file); err != nil { http.Error(resp, "Internal Server Error", http.StatusInternalServerError) h.logger.Error("failed serving metrics provider js file", "file", file, "error", err) @@ -237,7 +211,7 @@ func (h *Handler) renderIndex(cfg *config.RuntimeConfig, fs http.FileSystem) ([] // Allow caller to apply additional data transformations if needed. if h.transform != nil { - if err := h.transform(cfg, tplData); err != nil { + if err := h.transform(tplData); err != nil { return nil, nil, fmt.Errorf("failed running transform: %w", err) } } diff --git a/agent/uiserver/uiserver_test.go b/agent/uiserver/uiserver_test.go index 1ea8868b2972..3f56536088a6 100644 --- a/agent/uiserver/uiserver_test.go +++ b/agent/uiserver/uiserver_test.go @@ -11,11 +11,12 @@ import ( "strings" "testing" + "github.com/hashicorp/go-hclog" + "github.com/stretchr/testify/require" "golang.org/x/net/html" "github.com/hashicorp/consul/agent/config" "github.com/hashicorp/consul/sdk/testutil" - "github.com/stretchr/testify/require" ) func TestUIServerIndex(t *testing.T) { @@ -105,7 +106,7 @@ func TestUIServerIndex(t *testing.T) { withMetricsProvider("foo"), ), path: "/", - tx: func(cfg *config.RuntimeConfig, data map[string]interface{}) error { + tx: func(data map[string]interface{}) error { data["SSOEnabled"] = true o := data["UIConfig"].(map[string]interface{}) o["metrics_provider"] = "bar" @@ -359,3 +360,56 @@ func TestCompiledJS(t *testing.T) { } } + +func TestHandler_ServeHTTP_TransformIsEvaluatedOnEachRequest(t *testing.T) { + cfg := basicUIEnabledConfig() + + value := "seeds" + transform := func(data map[string]interface{}) error { + data["apple"] = value + return nil + } + h := NewHandler(cfg, hclog.New(nil), transform) + + t.Run("initial request", func(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + require.Equal(t, http.StatusOK, rec.Code) + expected := `{ + "ACLsEnabled": false, + "LocalDatacenter": "dc1", + "ContentPath": "/ui/", + "UIConfig": { + "metrics_provider": "", + "metrics_proxy_enabled": false, + "dashboard_url_templates": null + }, + "apple": "seeds" + }` + require.JSONEq(t, expected, extractUIConfig(t, rec.Body.String())) + }) + + t.Run("transform value has changed", func(t *testing.T) { + + value = "plant" + req := httptest.NewRequest("GET", "/", nil) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + require.Equal(t, http.StatusOK, rec.Code) + expected := `{ + "ACLsEnabled": false, + "LocalDatacenter": "dc1", + "ContentPath": "/ui/", + "UIConfig": { + "metrics_provider": "", + "metrics_proxy_enabled": false, + "dashboard_url_templates": null + }, + "apple": "plant" + }` + require.JSONEq(t, expected, extractUIConfig(t, rec.Body.String())) + }) +}