From f6d55e1d253aa9e0e08935a0d8d39077832320ae Mon Sep 17 00:00:00 2001 From: Paul Banks Date: Thu, 17 Sep 2020 11:48:14 +0100 Subject: [PATCH] Fix reload test; address other PR feedback --- agent/agent.go | 4 +++- agent/config/builder.go | 46 +++++++++++++++++++++--------------- agent/config/runtime_test.go | 8 +++---- agent/http.go | 3 --- 4 files changed, 34 insertions(+), 27 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 6d59ed0f5f..43135abc49 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -290,7 +290,6 @@ type Agent struct { // IP. httpConnLimiter connlimit.Limiter - // enterpriseAgent embeds fields that we only access in consul-enterprise builds enterpriseAgent } @@ -3574,6 +3573,9 @@ func (a *Agent) reloadConfigInternal(newCfg *config.RuntimeConfig) error { a.State.SetDiscardCheckOutput(newCfg.DiscardCheckOutput) + // Reload metrics config + a.uiConfig.Store(newCfg.UIConfig) + return nil } diff --git a/agent/config/builder.go b/agent/config/builder.go index 96a6035360..666ff0b655 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -1109,11 +1109,26 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) { return rt, nil } +// reBasicName validates that a field contains only lower case alphanumerics, +// underscore and dash and is non-empty. +var reBasicName = regexp.MustCompile("^[a-z0-9_-]+$") + +func validateBasicName(field, value string, allowEmpty bool) error { + if value == "" { + if allowEmpty { + return nil + } + return fmt.Errorf("%s cannot be empty", field) + } + if !reBasicName.MatchString(value) { + return fmt.Errorf("%s can only contain lowercase alphanumeric, - or _ characters."+ + " received: %q", field, value) + } + return nil +} + // Validate performs semantic validation of the runtime configuration. func (b *Builder) Validate(rt RuntimeConfig) error { - // reDatacenter defines a regexp for a valid datacenter name - var reBasicName = regexp.MustCompile("^[a-z0-9_-]+$") - var reDatacenter = reBasicName // validContentPath defines a regexp for a valid content path name. var validContentPath = regexp.MustCompile(`^[A-Za-z0-9/_-]+$`) @@ -1122,11 +1137,8 @@ func (b *Builder) Validate(rt RuntimeConfig) error { // check required params we cannot recover from first // - if rt.Datacenter == "" { - return fmt.Errorf("datacenter cannot be empty") - } - if !reDatacenter.MatchString(rt.Datacenter) { - return fmt.Errorf("datacenter cannot be %q. Please use only [a-z0-9-_]", rt.Datacenter) + if err := validateBasicName("datacenter", rt.Datacenter, false); err != nil { + return err } if rt.DataDir == "" && !rt.DevMode { return fmt.Errorf("data_dir cannot be empty") @@ -1140,17 +1152,14 @@ func (b *Builder) Validate(rt RuntimeConfig) error { return fmt.Errorf("ui-content-path cannot have 'v[0-9]'. received: %q", rt.UIConfig.ContentPath) } - if rt.UIConfig.MetricsProvider != "" && - !reBasicName.MatchString(rt.UIConfig.MetricsProvider) { - return fmt.Errorf("ui_config.metrics_provider can only contain lowercase "+ - "alphanumeric or _ characters. received: %q", rt.UIConfig.MetricsProvider) + if err := validateBasicName("ui_config.metrics_provider", rt.UIConfig.MetricsProvider, true); err != nil { + return err } if rt.UIConfig.MetricsProviderOptionsJSON != "" { // Attempt to parse the JSON to ensure it's valid, parsing into a map // ensures we get an object. var dummyMap map[string]interface{} - err := json.Unmarshal([]byte(rt.UIConfig.MetricsProviderOptionsJSON), - &dummyMap) + err := json.Unmarshal([]byte(rt.UIConfig.MetricsProviderOptionsJSON), &dummyMap) if err != nil { return fmt.Errorf("ui_config.metrics_provider_options_json must be empty "+ "or a string containing a valid JSON object. received: %q", @@ -1166,9 +1175,8 @@ func (b *Builder) Validate(rt RuntimeConfig) error { } } for k, v := range rt.UIConfig.DashboardURLTemplates { - if !reBasicName.MatchString(k) { - return fmt.Errorf("ui_config.dashboard_url_templates key names can only "+ - "contain lowercase alphanumeric or _ characters. received: %q", k) + if err := validateBasicName("ui_config.dashboard_url_templates key names", k, false); err != nil { + return err } u, err := url.Parse(v) if err != nil || !(u.Scheme == "http" || u.Scheme == "https") { @@ -1247,8 +1255,8 @@ func (b *Builder) Validate(rt RuntimeConfig) error { if rt.AutopilotMaxTrailingLogs < 0 { return fmt.Errorf("autopilot.max_trailing_logs cannot be %d. Must be greater than or equal to zero", rt.AutopilotMaxTrailingLogs) } - if rt.ACLDatacenter != "" && !reDatacenter.MatchString(rt.ACLDatacenter) { - return fmt.Errorf("acl_datacenter cannot be %q. Please use only [a-z0-9-_]", rt.ACLDatacenter) + if err := validateBasicName("acl_datacenter", rt.ACLDatacenter, true); err != nil { + return err } // In DevMode, UI is enabled by default, so to enable rt.UIDir, don't perform this check if !rt.DevMode && rt.UIConfig.Enabled && rt.UIConfig.Dir != "" { diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index b92654c26b..5af04f2d2f 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -1712,7 +1712,7 @@ func TestBuilder_BuildAndValide_ConfigFlagsAndEdgecases(t *testing.T) { }, json: []string{`{ "acl_datacenter": "%" }`}, hcl: []string{`acl_datacenter = "%"`}, - err: `acl_datacenter cannot be "%". Please use only [a-z0-9-_]`, + err: `acl_datacenter can only contain lowercase alphanumeric, - or _ characters.`, warns: []string{`The 'acl_datacenter' field is deprecated. Use the 'primary_datacenter' field instead.`}, }, { @@ -1881,7 +1881,7 @@ func TestBuilder_BuildAndValide_ConfigFlagsAndEdgecases(t *testing.T) { args: []string{`-data-dir=` + dataDir}, json: []string{`{ "datacenter": "%" }`}, hcl: []string{`datacenter = "%"`}, - err: `datacenter cannot be "%". Please use only [a-z0-9-_]`, + err: `datacenter can only contain lowercase alphanumeric, - or _ characters.`, }, { desc: "dns does not allow socket", @@ -4306,7 +4306,7 @@ func TestBuilder_BuildAndValide_ConfigFlagsAndEdgecases(t *testing.T) { metrics_provider = "((((lisp 4 life))))" } `}, - err: `ui_config.metrics_provider can only contain lowercase alphanumeric or _ characters.`, + err: `ui_config.metrics_provider can only contain lowercase alphanumeric, - or _ characters.`, }, { desc: "metrics_provider_options_json invalid JSON", @@ -4393,7 +4393,7 @@ func TestBuilder_BuildAndValide_ConfigFlagsAndEdgecases(t *testing.T) { } } `}, - err: `ui_config.dashboard_url_templates key names can only contain lowercase alphanumeric or _ characters.`, + err: `ui_config.dashboard_url_templates key names can only contain lowercase alphanumeric, - or _ characters.`, }, { desc: "dashboard_url_templates value format", diff --git a/agent/http.go b/agent/http.go index f7e0bf05b2..ea66ba72c9 100644 --- a/agent/http.go +++ b/agent/http.go @@ -29,7 +29,6 @@ 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. @@ -170,7 +169,6 @@ 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) } @@ -184,7 +182,6 @@ 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) }