From 116e6df0965abe74d1ff69491e1f0da71dac6075 Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Fri, 12 Jun 2015 13:39:59 +0200 Subject: [PATCH] config: raise error on unknown config parameters. The YAML parser ignores additional parameters on unmarshaling. This causes frequent confusion with bad configs that pass parsing. These changes raise errors on additional parameters. --- config/config.go | 69 +++++++++++++++++++++++----- config/config_test.go | 11 +++-- config/testdata/unknown_attr.bad.yml | 20 ++++++++ 3 files changed, 84 insertions(+), 16 deletions(-) create mode 100644 config/testdata/unknown_attr.bad.yml diff --git a/config/config.go b/config/config.go index 77da619c2..5b2356cff 100644 --- a/config/config.go +++ b/config/config.go @@ -93,10 +93,24 @@ type Config struct { RuleFiles []string `yaml:"rule_files,omitempty"` ScrapeConfigs []*ScrapeConfig `yaml:"scrape_configs,omitempty"` + // Catches all undefined fields and must be empty after parsing. + XXX map[string]interface{} `yaml:",inline"` + // original is the input from which the config was parsed. original string } +func checkOverflow(m map[string]interface{}, ctx string) error { + if len(m) > 0 { + var keys []string + for k := range m { + keys = append(keys, k) + } + return fmt.Errorf("unknown fields in %s: %s", ctx, strings.Join(keys, ", ")) + } + return nil +} + func (c Config) String() string { if c.original != "" { return c.original @@ -138,7 +152,7 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { } jobNames[scfg.JobName] = struct{}{} } - return nil + return checkOverflow(c.XXX, "config") } // GlobalConfig configures values that are used across other configuration @@ -150,9 +164,11 @@ type GlobalConfig struct { ScrapeTimeout Duration `yaml:"scrape_timeout,omitempty"` // How frequently to evaluate rules by default. EvaluationInterval Duration `yaml:"evaluation_interval,omitempty"` - // The labels to add to any timeseries that this Prometheus instance scrapes. Labels clientmodel.LabelSet `yaml:"labels,omitempty"` + + // Catches all undefined fields and must be empty after parsing. + XXX map[string]interface{} `yaml:",inline"` } // UnmarshalYAML implements the yaml.Unmarshaler interface. @@ -162,7 +178,7 @@ func (c *GlobalConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { if err := unmarshal((*plain)(c)); err != nil { return err } - return nil + return checkOverflow(c.XXX, "global config") } // ScrapeConfig configures a scraping unit for Prometheus. @@ -178,7 +194,7 @@ type ScrapeConfig struct { // The URL scheme with which to fetch metrics from targets. Scheme string `yaml:"scheme,omitempty"` // The HTTP basic authentication credentials for the targets. - BasicAuth *BasicAuth `yaml:"basic_auth"` + BasicAuth *BasicAuth `yaml:"basic_auth,omitempty"` // List of labeled target groups for this job. TargetGroups []*TargetGroup `yaml:"target_groups,omitempty"` @@ -190,6 +206,9 @@ type ScrapeConfig struct { ConsulSDConfigs []*ConsulSDConfig `yaml:"consul_sd_configs,omitempty"` // List of relabel configurations. RelabelConfigs []*RelabelConfig `yaml:"relabel_configs,omitempty"` + + // Catches all undefined fields and must be empty after parsing. + XXX map[string]interface{} `yaml:",inline"` } // UnmarshalYAML implements the yaml.Unmarshaler interface. @@ -203,13 +222,26 @@ func (c *ScrapeConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { if !patJobName.MatchString(c.JobName) { return fmt.Errorf("%q is not a valid job name", c.JobName) } - return nil + return checkOverflow(c.XXX, "scrape_config") } // BasicAuth contains basic HTTP authentication credentials. type BasicAuth struct { Username string `yaml:"username"` Password string `yaml:"password"` + + // Catches all undefined fields and must be empty after parsing. + XXX map[string]interface{} `yaml:",inline"` +} + +// UnmarshalYAML implements the yaml.Unmarshaler interface. +func (a *BasicAuth) UnmarshalYAML(unmarshal func(interface{}) error) error { + type plain BasicAuth + err := unmarshal((*plain)(a)) + if err != nil { + return err + } + return checkOverflow(a.XXX, "basic_auth") } // TargetGroup is a set of targets with a common label set. @@ -231,8 +263,9 @@ func (tg TargetGroup) String() string { // UnmarshalYAML implements the yaml.Unmarshaler interface. func (tg *TargetGroup) UnmarshalYAML(unmarshal func(interface{}) error) error { g := struct { - Targets []string `yaml:"targets"` - Labels clientmodel.LabelSet `yaml:"labels"` + Targets []string `yaml:"targets"` + Labels clientmodel.LabelSet `yaml:"labels"` + XXX map[string]interface{} `yaml:",inline"` }{} if err := unmarshal(&g); err != nil { return err @@ -247,7 +280,7 @@ func (tg *TargetGroup) UnmarshalYAML(unmarshal func(interface{}) error) error { }) } tg.Labels = g.Labels - return nil + return checkOverflow(g.XXX, "target_group") } // MarshalYAML implements the yaml.Marshaler interface. @@ -291,6 +324,9 @@ func (tg *TargetGroup) UnmarshalJSON(b []byte) error { type DNSSDConfig struct { Names []string `yaml:"names"` RefreshInterval Duration `yaml:"refresh_interval,omitempty"` + + // Catches all undefined fields and must be empty after parsing. + XXX map[string]interface{} `yaml:",inline"` } // UnmarshalYAML implements the yaml.Unmarshaler interface. @@ -304,13 +340,16 @@ func (c *DNSSDConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { if len(c.Names) == 0 { return fmt.Errorf("DNS-SD config must contain at least one SRV record name") } - return nil + return checkOverflow(c.XXX, "dns_sd_config") } // FileSDConfig is the configuration for file based discovery. type FileSDConfig struct { Names []string `yaml:"names"` RefreshInterval Duration `yaml:"refresh_interval,omitempty"` + + // Catches all undefined fields and must be empty after parsing. + XXX map[string]interface{} `yaml:",inline"` } // UnmarshalYAML implements the yaml.Unmarshaler interface. @@ -329,7 +368,7 @@ func (c *FileSDConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { return fmt.Errorf("path name %q is not valid for file discovery", name) } } - return nil + return checkOverflow(c.XXX, "file_sd_config") } // ConsulSDConfig is the configuration for Consul service discovery. @@ -343,6 +382,9 @@ type ConsulSDConfig struct { Password string `yaml:"password"` // The list of services for which targets are discovered. Services []string `yaml:"services"` + + // Catches all undefined fields and must be empty after parsing. + XXX map[string]interface{} `yaml:",inline"` } // UnmarshalYAML implements the yaml.Unmarshaler interface. @@ -359,7 +401,7 @@ func (c *ConsulSDConfig) UnmarshalYAML(unmarshal func(interface{}) error) error if len(c.Services) == 0 { return fmt.Errorf("Consul SD configuration requires at least one service name") } - return nil + return checkOverflow(c.XXX, "consul_sd_config") } // RelabelAction is the action to be performed on relabeling. @@ -403,6 +445,9 @@ type RelabelConfig struct { Replacement string `yaml:"replacement,omitempty"` // Action is the action to be performed for the relabeling. Action RelabelAction `yaml:"action,omitempty"` + + // Catches all undefined fields and must be empty after parsing. + XXX map[string]interface{} `yaml:",inline"` } // UnmarshalYAML implements the yaml.Unmarshaler interface. @@ -415,7 +460,7 @@ func (c *RelabelConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { if c.Regex == nil { return fmt.Errorf("relabel configuration requires a regular expression") } - return nil + return checkOverflow(c.XXX, "relabel_config") } // Regexp encapsulates a regexp.Regexp and makes it YAML marshallable. diff --git a/config/config_test.go b/config/config_test.go index 65c70a166..c65e5e715 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -146,20 +146,20 @@ func TestLoadConfig(t *testing.T) { c, err := LoadFromFile("testdata/conf.good.yml") if err != nil { - t.Errorf("Error parsing %s: %s", "testdata/conf.good.yml", err) + t.Fatalf("Error parsing %s: %s", "testdata/conf.good.yml", err) } bgot, err := yaml.Marshal(c) if err != nil { - t.Errorf("%s", err) + t.Fatalf("%s", err) } bexp, err := yaml.Marshal(expectedConf) if err != nil { - t.Errorf("%s", err) + t.Fatalf("%s", err) } expectedConf.original = c.original if !reflect.DeepEqual(c, expectedConf) { - t.Errorf("%s: unexpected config result: \n\n%s\n expected\n\n%s", "testdata/conf.good.yml", bgot, bexp) + t.Fatalf("%s: unexpected config result: \n\n%s\n expected\n\n%s", "testdata/conf.good.yml", bgot, bexp) } } @@ -188,6 +188,9 @@ var expectedErrors = []struct { }, { filename: "rules.bad.yml", errMsg: "invalid rule file path", + }, { + filename: "unknown_attr.bad.yml", + errMsg: "unknown fields in scrape_config: consult_sd_configs", }, } diff --git a/config/testdata/unknown_attr.bad.yml b/config/testdata/unknown_attr.bad.yml new file mode 100644 index 000000000..4baaf565c --- /dev/null +++ b/config/testdata/unknown_attr.bad.yml @@ -0,0 +1,20 @@ +# my global config +global: + scrape_interval: 15s + evaluation_interval: 30s + # scrape_timeout is set to the global default (10s). + + labels: + monitor: codelab + foo: bar + +rule_files: + - "first.rules" + - "second.rules" + - "my/*.rules" + +scrape_configs: + - job_name: prometheus + + consult_sd_configs: + - server: 'localhost:1234'