From d28246e337cefa158b984adf8a5104ed8cb832ed Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Mon, 3 Dec 2018 18:09:02 +0800 Subject: [PATCH] Fix config loading panics on nil pointer slice elements (#4942) Fixes https://github.com/prometheus/prometheus/issues/4902 Fixes https://github.com/prometheus/prometheus/issues/4889 Signed-off-by: Julius Volz --- config/config.go | 60 +++++++++++++++- config/config_test.go | 36 ++++++++++ .../empty_alert_relabel_config.bad.yml | 3 + .../empty_alertmanager_relabel_config.bad.yml | 4 ++ .../empty_metric_relabel_config.bad.yml | 4 ++ config/testdata/empty_rr_config.bad.yml | 2 + config/testdata/empty_rw_config.bad.yml | 2 + .../testdata/empty_rw_relabel_config.bad.yml | 4 ++ config/testdata/empty_scrape_config.bad.yml | 2 + config/testdata/empty_static_config.bad.yml | 4 ++ .../empty_target_relabel_config.bad.yml | 4 ++ discovery/config/config.go | 69 +++++++++++++++++-- 12 files changed, 189 insertions(+), 5 deletions(-) create mode 100644 config/testdata/empty_alert_relabel_config.bad.yml create mode 100644 config/testdata/empty_alertmanager_relabel_config.bad.yml create mode 100644 config/testdata/empty_metric_relabel_config.bad.yml create mode 100644 config/testdata/empty_rr_config.bad.yml create mode 100644 config/testdata/empty_rw_config.bad.yml create mode 100644 config/testdata/empty_rw_relabel_config.bad.yml create mode 100644 config/testdata/empty_scrape_config.bad.yml create mode 100644 config/testdata/empty_static_config.bad.yml create mode 100644 config/testdata/empty_target_relabel_config.bad.yml diff --git a/config/config.go b/config/config.go index 8ac49209f..253fb2c38 100644 --- a/config/config.go +++ b/config/config.go @@ -233,6 +233,9 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { // Do global overrides and validate unique names. jobNames := map[string]struct{}{} for _, scfg := range c.ScrapeConfigs { + if scfg == nil { + return fmt.Errorf("empty or null scrape config section") + } // First set the correct scrape interval, then check that the timeout // (inferred or explicit) is not greater than that. if scfg.ScrapeInterval == 0 { @@ -254,6 +257,16 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { } jobNames[scfg.JobName] = struct{}{} } + for _, rwcfg := range c.RemoteWriteConfigs { + if rwcfg == nil { + return fmt.Errorf("empty or null remote write config section") + } + } + for _, rrcfg := range c.RemoteReadConfigs { + if rrcfg == nil { + return fmt.Errorf("empty or null remote read config section") + } + } return nil } @@ -360,6 +373,13 @@ func (c *ScrapeConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { return err } + // The UnmarshalYAML method of ServiceDiscoveryConfig is not being called because it's not a pointer. + // We cannot make it a pointer as the parser panics for inlined pointer structs. + // Thus we just do its validation here. + if err := c.ServiceDiscoveryConfig.Validate(); err != nil { + return err + } + // Check for users putting URLs in target groups. if len(c.RelabelConfigs) == 0 { for _, tg := range c.ServiceDiscoveryConfig.StaticConfigs { @@ -371,6 +391,17 @@ func (c *ScrapeConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { } } + for _, rlcfg := range c.RelabelConfigs { + if rlcfg == nil { + return fmt.Errorf("empty or null target relabeling rule in scrape config") + } + } + for _, rlcfg := range c.MetricRelabelConfigs { + if rlcfg == nil { + return fmt.Errorf("empty or null metric relabeling rule in scrape config") + } + } + // Add index to the static config target groups for unique identification // within scrape pool. for i, tg := range c.ServiceDiscoveryConfig.StaticConfigs { @@ -392,7 +423,16 @@ func (c *AlertingConfig) UnmarshalYAML(unmarshal func(interface{}) error) error // by the default due to the YAML parser behavior for empty blocks. *c = AlertingConfig{} type plain AlertingConfig - return unmarshal((*plain)(c)) + if err := unmarshal((*plain)(c)); err != nil { + return err + } + + for _, rlcfg := range c.AlertRelabelConfigs { + if rlcfg == nil { + return fmt.Errorf("empty or null alert relabeling rule") + } + } + return nil } // AlertmanagerConfig configures how Alertmanagers can be discovered and communicated with. @@ -429,6 +469,13 @@ func (c *AlertmanagerConfig) UnmarshalYAML(unmarshal func(interface{}) error) er return err } + // The UnmarshalYAML method of ServiceDiscoveryConfig is not being called because it's not a pointer. + // We cannot make it a pointer as the parser panics for inlined pointer structs. + // Thus we just do its validation here. + if err := c.ServiceDiscoveryConfig.Validate(); err != nil { + return err + } + // Check for users putting URLs in target groups. if len(c.RelabelConfigs) == 0 { for _, tg := range c.ServiceDiscoveryConfig.StaticConfigs { @@ -440,6 +487,12 @@ func (c *AlertmanagerConfig) UnmarshalYAML(unmarshal func(interface{}) error) er } } + for _, rlcfg := range c.RelabelConfigs { + if rlcfg == nil { + return fmt.Errorf("empty or null Alertmanager target relabeling rule") + } + } + // Add index to the static config target groups for unique identification // within scrape pool. for i, tg := range c.ServiceDiscoveryConfig.StaticConfigs { @@ -632,6 +685,11 @@ func (c *RemoteWriteConfig) UnmarshalYAML(unmarshal func(interface{}) error) err if c.URL == nil { return fmt.Errorf("url for remote_write is empty") } + for _, rlcfg := range c.WriteRelabelConfigs { + if rlcfg == nil { + return fmt.Errorf("empty or null relabeling rule in remote write config") + } + } // The UnmarshalYAML method of HTTPClientConfig is not being called because it's not a pointer. // We cannot make it a pointer as the parser panics for inlined pointer structs. diff --git a/config/config_test.go b/config/config_test.go index 08a27d37e..e04fd56d2 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -767,6 +767,42 @@ var expectedErrors = []struct { filename: "azure_tenant_id_missing.bad.yml", errMsg: "Azure SD configuration requires a tenant_id", }, + { + filename: "empty_scrape_config.bad.yml", + errMsg: "empty or null scrape config section", + }, + { + filename: "empty_rw_config.bad.yml", + errMsg: "empty or null remote write config section", + }, + { + filename: "empty_rr_config.bad.yml", + errMsg: "empty or null remote read config section", + }, + { + filename: "empty_target_relabel_config.bad.yml", + errMsg: "empty or null target relabeling rule", + }, + { + filename: "empty_metric_relabel_config.bad.yml", + errMsg: "empty or null metric relabeling rule", + }, + { + filename: "empty_alert_relabel_config.bad.yml", + errMsg: "empty or null alert relabeling rule", + }, + { + filename: "empty_alertmanager_relabel_config.bad.yml", + errMsg: "empty or null Alertmanager target relabeling rule", + }, + { + filename: "empty_rw_relabel_config.bad.yml", + errMsg: "empty or null relabeling rule in remote write config", + }, + { + filename: "empty_static_config.bad.yml", + errMsg: "empty or null section in static_configs", + }, } func TestBadConfigs(t *testing.T) { diff --git a/config/testdata/empty_alert_relabel_config.bad.yml b/config/testdata/empty_alert_relabel_config.bad.yml new file mode 100644 index 000000000..b863bf23a --- /dev/null +++ b/config/testdata/empty_alert_relabel_config.bad.yml @@ -0,0 +1,3 @@ +alerting: + alert_relabel_configs: + - diff --git a/config/testdata/empty_alertmanager_relabel_config.bad.yml b/config/testdata/empty_alertmanager_relabel_config.bad.yml new file mode 100644 index 000000000..6d99ac4dc --- /dev/null +++ b/config/testdata/empty_alertmanager_relabel_config.bad.yml @@ -0,0 +1,4 @@ +alerting: + alertmanagers: + - relabel_configs: + - diff --git a/config/testdata/empty_metric_relabel_config.bad.yml b/config/testdata/empty_metric_relabel_config.bad.yml new file mode 100644 index 000000000..d2485e352 --- /dev/null +++ b/config/testdata/empty_metric_relabel_config.bad.yml @@ -0,0 +1,4 @@ +scrape_configs: +- job_name: "test" + metric_relabel_configs: + - diff --git a/config/testdata/empty_rr_config.bad.yml b/config/testdata/empty_rr_config.bad.yml new file mode 100644 index 000000000..e3bcca598 --- /dev/null +++ b/config/testdata/empty_rr_config.bad.yml @@ -0,0 +1,2 @@ +remote_read: +- diff --git a/config/testdata/empty_rw_config.bad.yml b/config/testdata/empty_rw_config.bad.yml new file mode 100644 index 000000000..6f16030e6 --- /dev/null +++ b/config/testdata/empty_rw_config.bad.yml @@ -0,0 +1,2 @@ +remote_write: +- diff --git a/config/testdata/empty_rw_relabel_config.bad.yml b/config/testdata/empty_rw_relabel_config.bad.yml new file mode 100644 index 000000000..6d5418290 --- /dev/null +++ b/config/testdata/empty_rw_relabel_config.bad.yml @@ -0,0 +1,4 @@ +remote_write: + - url: "foo" + write_relabel_configs: + - \ No newline at end of file diff --git a/config/testdata/empty_scrape_config.bad.yml b/config/testdata/empty_scrape_config.bad.yml new file mode 100644 index 000000000..8c300deaa --- /dev/null +++ b/config/testdata/empty_scrape_config.bad.yml @@ -0,0 +1,2 @@ +scrape_configs: +- \ No newline at end of file diff --git a/config/testdata/empty_static_config.bad.yml b/config/testdata/empty_static_config.bad.yml new file mode 100644 index 000000000..464a0a6fb --- /dev/null +++ b/config/testdata/empty_static_config.bad.yml @@ -0,0 +1,4 @@ +scrape_configs: +- job_name: "test" + static_configs: + - diff --git a/config/testdata/empty_target_relabel_config.bad.yml b/config/testdata/empty_target_relabel_config.bad.yml new file mode 100644 index 000000000..7324b1041 --- /dev/null +++ b/config/testdata/empty_target_relabel_config.bad.yml @@ -0,0 +1,4 @@ +scrape_configs: +- job_name: "test" + relabel_configs: + - diff --git a/discovery/config/config.go b/discovery/config/config.go index 68a9bc1c5..80cab513b 100644 --- a/discovery/config/config.go +++ b/discovery/config/config.go @@ -14,6 +14,8 @@ package config import ( + "fmt" + "github.com/prometheus/prometheus/discovery/azure" "github.com/prometheus/prometheus/discovery/consul" "github.com/prometheus/prometheus/discovery/dns" @@ -58,8 +60,67 @@ type ServiceDiscoveryConfig struct { TritonSDConfigs []*triton.SDConfig `yaml:"triton_sd_configs,omitempty"` } -// UnmarshalYAML implements the yaml.Unmarshaler interface. -func (c *ServiceDiscoveryConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { - type plain ServiceDiscoveryConfig - return unmarshal((*plain)(c)) +// Validate validates the ServiceDiscoveryConfig. +func (c *ServiceDiscoveryConfig) Validate() error { + for _, cfg := range c.AzureSDConfigs { + if cfg == nil { + return fmt.Errorf("empty or null section in azure_sd_configs") + } + } + for _, cfg := range c.ConsulSDConfigs { + if cfg == nil { + return fmt.Errorf("empty or null section in consul_sd_configs") + } + } + for _, cfg := range c.DNSSDConfigs { + if cfg == nil { + return fmt.Errorf("empty or null section in dns_sd_configs") + } + } + for _, cfg := range c.EC2SDConfigs { + if cfg == nil { + return fmt.Errorf("empty or null section in ec2_sd_configs") + } + } + for _, cfg := range c.FileSDConfigs { + if cfg == nil { + return fmt.Errorf("empty or null section in file_sd_configs") + } + } + for _, cfg := range c.GCESDConfigs { + if cfg == nil { + return fmt.Errorf("empty or null section in gce_sd_configs") + } + } + for _, cfg := range c.KubernetesSDConfigs { + if cfg == nil { + return fmt.Errorf("empty or null section in kubernetes_sd_configs") + } + } + for _, cfg := range c.MarathonSDConfigs { + if cfg == nil { + return fmt.Errorf("empty or null section in marathon_sd_configs") + } + } + for _, cfg := range c.NerveSDConfigs { + if cfg == nil { + return fmt.Errorf("empty or null section in nerve_sd_configs") + } + } + for _, cfg := range c.OpenstackSDConfigs { + if cfg == nil { + return fmt.Errorf("empty or null section in openstack_sd_configs") + } + } + for _, cfg := range c.ServersetSDConfigs { + if cfg == nil { + return fmt.Errorf("empty or null section in serverset_sd_configs") + } + } + for _, cfg := range c.StaticConfigs { + if cfg == nil { + return fmt.Errorf("empty or null section in static_configs") + } + } + return nil }