From d0288ea4171977f998d8f17754135628ee00b160 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Tue, 18 Oct 2022 10:28:39 +0300 Subject: [PATCH] all: log error when environment variables referred from `-promscrape.config` are missing This should prevent from using incorrect config files --- app/vmalert/config/config.go | 7 ++++-- app/vmauth/auth_config.go | 6 ++++- docs/CHANGELOG.md | 1 + go.mod | 2 +- go.sum | 4 +-- lib/envtemplate/envtemplate.go | 20 +++++++++------ lib/envtemplate/envtemplate_test.go | 25 ++++++++++++++++--- lib/promrelabel/config.go | 5 +++- lib/promscrape/config.go | 15 ++++++++--- .../valyala/fasttemplate/template.go | 3 +-- vendor/modules.txt | 2 +- 11 files changed, 66 insertions(+), 24 deletions(-) diff --git a/app/vmalert/config/config.go b/app/vmalert/config/config.go index fbdd9ab58..d0a2ef68a 100644 --- a/app/vmalert/config/config.go +++ b/app/vmalert/config/config.go @@ -243,9 +243,12 @@ func Parse(pathPatterns []string, validateTplFn ValidateTplFn, validateExpressio func parseFile(path string) ([]Group, error) { data, err := os.ReadFile(path) if err != nil { - return nil, fmt.Errorf("error reading alert rule file: %w", err) + return nil, fmt.Errorf("error reading alert rule file %q: %w", path, err) + } + data, err = envtemplate.Replace(data) + if err != nil { + return nil, fmt.Errorf("cannot expand environment vars in %q: %w", path, err) } - data = envtemplate.Replace(data) g := struct { Groups []Group `yaml:"groups"` // Catches all undefined fields and must be empty after parsing. diff --git a/app/vmauth/auth_config.go b/app/vmauth/auth_config.go index e19086069..46eee7524 100644 --- a/app/vmauth/auth_config.go +++ b/app/vmauth/auth_config.go @@ -250,7 +250,11 @@ func readAuthConfig(path string) (map[string]*UserInfo, error) { } func parseAuthConfig(data []byte) (map[string]*UserInfo, error) { - data = envtemplate.Replace(data) + var err error + data, err = envtemplate.Replace(data) + if err != nil { + return nil, fmt.Errorf("cannot expand environment vars: %w", err) + } var ac AuthConfig if err := yaml.UnmarshalStrict(data, &ac); err != nil { return nil, fmt.Errorf("cannot unmarshal AuthConfig data: %w", err) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index e86098217..553a38ff2 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -39,6 +39,7 @@ The following tip changes can be tested by building VictoriaMetrics components f * FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent.html): allow controlling staleness tracking on a per-[scrape_config](https://docs.victoriametrics.com/sd_configs.html#scrape_configs) basis by specifying `no_stale_markers: true` or `no_stale_markers: false` option in the corresponding [scrape_config](https://docs.victoriametrics.com/sd_configs.html#scrape_configs). * FEATURE: [vmui](https://docs.victoriametrics.com/#vmui): limit the number of plotted series. This should prevent from browser crashes or hangs when the query returns big number of time series. See [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3155). +* FEATURE: log error if some environment variables referred at `-promscrape.config` via `%{ENV_VAR}` aren't found. This should prevent from using incorrect config files. * BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): properly merge buckets with identical `le` values, but with different string representation of these values when calculating [histogram_quantile](https://docs.victoriametrics.com/MetricsQL.html#histogram_quantile) and [histogram_share](https://docs.victoriametrics.com/MetricsQL.html#histogram_share). For example, `http_request_duration_seconds_bucket{le="5"}` and `http_requests_duration_seconds_bucket{le="5.0"}`. Such buckets may be returned from distinct targets. Thanks to @647-coder for the [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3225). diff --git a/go.mod b/go.mod index 856887f8b..4ef3aa504 100644 --- a/go.mod +++ b/go.mod @@ -26,7 +26,7 @@ require ( github.com/urfave/cli/v2 v2.19.2 github.com/valyala/fastjson v1.6.3 github.com/valyala/fastrand v1.1.0 - github.com/valyala/fasttemplate v1.2.1 + github.com/valyala/fasttemplate v1.2.2 github.com/valyala/gozstd v1.17.0 github.com/valyala/quicktemplate v1.7.0 golang.org/x/net v0.0.0-20221014081412-f15817d10f9b diff --git a/go.sum b/go.sum index 687c2e7d1..00db19c03 100644 --- a/go.sum +++ b/go.sum @@ -893,8 +893,8 @@ github.com/valyala/fastjson v1.6.3 h1:tAKFnnwmeMGPbwJ7IwxcTPCNr3uIzoIj3/Fh90ra4x github.com/valyala/fastjson v1.6.3/go.mod h1:CLCAqky6SMuOcxStkYQvblddUtoRxhYMGLrsQns1aXY= github.com/valyala/fastrand v1.1.0 h1:f+5HkLW4rsgzdNoleUOB69hyT9IlD2ZQh9GyDMfb5G8= github.com/valyala/fastrand v1.1.0/go.mod h1:HWqCzkrkg6QXT8V2EXWvXCoow7vLwOFN002oeRzjapQ= -github.com/valyala/fasttemplate v1.2.1 h1:TVEnxayobAdVkhQfrfes2IzOB6o+z4roRkPF52WA1u4= -github.com/valyala/fasttemplate v1.2.1/go.mod h1:KHLXt3tVN2HBp8eijSv/kGJopbvo7S+qRAEEKiv+SiQ= +github.com/valyala/fasttemplate v1.2.2 h1:lxLXG0uE3Qnshl9QyaK6XJxMXlQZELvChBOCmQD0Loo= +github.com/valyala/fasttemplate v1.2.2/go.mod h1:KHLXt3tVN2HBp8eijSv/kGJopbvo7S+qRAEEKiv+SiQ= github.com/valyala/gozstd v1.17.0 h1:M4Ds4MIrw+pD+s6vYtuFZ8D3iEw9htzfdytOV3C3iQU= github.com/valyala/gozstd v1.17.0/go.mod h1:y5Ew47GLlP37EkTB+B4s7r6A5rdaeB7ftbl9zoYiIPQ= github.com/valyala/histogram v1.2.0 h1:wyYGAZZt3CpwUiIb9AU/Zbllg1llXyrtApRS815OLoQ= diff --git a/lib/envtemplate/envtemplate.go b/lib/envtemplate/envtemplate.go index c92eb827f..14a84a90c 100644 --- a/lib/envtemplate/envtemplate.go +++ b/lib/envtemplate/envtemplate.go @@ -2,6 +2,7 @@ package envtemplate import ( "bytes" + "fmt" "io" "os" @@ -9,17 +10,22 @@ import ( ) // Replace replaces `%{ENV_VAR}` placeholders in b with the corresponding ENV_VAR values. -func Replace(b []byte) []byte { +// +// Error is returned if ENV_VAR isn't set for some `%{ENV_VAR}` placeholder. +func Replace(b []byte) ([]byte, error) { if !bytes.Contains(b, []byte("%{")) { // Fast path - nothing to replace. - return b + return b, nil } - s := fasttemplate.ExecuteFuncString(string(b), "%{", "}", func(w io.Writer, tag string) (int, error) { - v := os.Getenv(tag) - if v == "" { - v = "%{" + tag + "}" + s, err := fasttemplate.ExecuteFuncStringWithErr(string(b), "%{", "}", func(w io.Writer, tag string) (int, error) { + v, ok := os.LookupEnv(tag) + if !ok { + return 0, fmt.Errorf("missing %q environment variable", tag) } return w.Write([]byte(v)) }) - return []byte(s) + if err != nil { + return nil, err + } + return []byte(s), nil } diff --git a/lib/envtemplate/envtemplate_test.go b/lib/envtemplate/envtemplate_test.go index 4ba4633cd..db899c80d 100644 --- a/lib/envtemplate/envtemplate_test.go +++ b/lib/envtemplate/envtemplate_test.go @@ -1,19 +1,36 @@ package envtemplate import ( + "os" "testing" ) -func TestReplace(t *testing.T) { +func TestReplaceSuccess(t *testing.T) { + if err := os.Setenv("foo", "bar"); err != nil { + t.Fatalf("cannot set env var: %s", err) + } f := func(s, resultExpected string) { t.Helper() - result := Replace([]byte(s)) + result, err := Replace([]byte(s)) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } if string(result) != resultExpected { t.Fatalf("unexpected result;\ngot\n%q\nwant\n%q", result, resultExpected) } } f("", "") f("foo", "foo") - f("%{foo}", "%{foo}") - f("foo %{bar} %{baz}", "foo %{bar} %{baz}") + f("a %{foo}-x", "a bar-x") +} + +func TestReplaceFailure(t *testing.T) { + f := func(s string) { + t.Helper() + _, err := Replace([]byte(s)) + if err == nil { + t.Fatalf("expecting non-nil error") + } + } + f("foo %{bar} %{baz}") } diff --git a/lib/promrelabel/config.go b/lib/promrelabel/config.go index eee9f002c..16bf3e9e9 100644 --- a/lib/promrelabel/config.go +++ b/lib/promrelabel/config.go @@ -152,7 +152,10 @@ func LoadRelabelConfigs(path string, relabelDebug bool) (*ParsedConfigs, error) if err != nil { return nil, fmt.Errorf("cannot read `relabel_configs` from %q: %w", path, err) } - data = envtemplate.Replace(data) + data, err = envtemplate.Replace(data) + if err != nil { + return nil, fmt.Errorf("cannot expand environment vars at %q: %w", path, err) + } pcs, err := ParseRelabelConfigsData(data, relabelDebug) if err != nil { return nil, fmt.Errorf("cannot unmarshal `relabel_configs` from %q: %w", path, err) diff --git a/lib/promscrape/config.go b/lib/promscrape/config.go index 26cf4dae1..998b84505 100644 --- a/lib/promscrape/config.go +++ b/lib/promscrape/config.go @@ -92,8 +92,11 @@ type Config struct { } func (cfg *Config) unmarshal(data []byte, isStrict bool) error { - data = envtemplate.Replace(data) var err error + data, err = envtemplate.Replace(data) + if err != nil { + return fmt.Errorf("cannot expand environment variables: %w", err) + } if isStrict { if err = yaml.UnmarshalStrict(data, cfg); err != nil { err = fmt.Errorf("%w; pass -promscrape.config.strictParse=false command-line flag for ignoring unknown fields in yaml config", err) @@ -372,7 +375,10 @@ func loadStaticConfigs(path string) ([]StaticConfig, error) { if err != nil { return nil, fmt.Errorf("cannot read `static_configs` from %q: %w", path, err) } - data = envtemplate.Replace(data) + data, err = envtemplate.Replace(data) + if err != nil { + return nil, fmt.Errorf("cannot expand environment vars in %q: %w", path, err) + } var stcs []StaticConfig if err := yaml.UnmarshalStrict(data, &stcs); err != nil { return nil, fmt.Errorf("cannot unmarshal `static_configs` from %q: %w", path, err) @@ -413,7 +419,10 @@ func loadScrapeConfigFiles(baseDir string, scrapeConfigFiles []string) ([]*Scrap if err != nil { return nil, nil, fmt.Errorf("cannot load %q: %w", path, err) } - data = envtemplate.Replace(data) + data, err = envtemplate.Replace(data) + if err != nil { + return nil, nil, fmt.Errorf("cannot expand environment vars in %q: %w", path, err) + } var scs []*ScrapeConfig if err = yaml.UnmarshalStrict(data, &scs); err != nil { return nil, nil, fmt.Errorf("cannot parse %q: %w", path, err) diff --git a/vendor/github.com/valyala/fasttemplate/template.go b/vendor/github.com/valyala/fasttemplate/template.go index 186200134..f2d3261f8 100644 --- a/vendor/github.com/valyala/fasttemplate/template.go +++ b/vendor/github.com/valyala/fasttemplate/template.go @@ -112,8 +112,7 @@ func ExecuteFuncString(template, startTag, endTag string, f TagFunc) string { // but when f returns an error, ExecuteFuncStringWithErr won't panic like ExecuteFuncString // it just returns an empty string and the error f returned func ExecuteFuncStringWithErr(template, startTag, endTag string, f TagFunc) (string, error) { - tagsCount := bytes.Count(unsafeString2Bytes(template), unsafeString2Bytes(startTag)) - if tagsCount == 0 { + if n := bytes.Index(unsafeString2Bytes(template), unsafeString2Bytes(startTag)); n < 0 { return template, nil } diff --git a/vendor/modules.txt b/vendor/modules.txt index 68e389ec6..24ae20872 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -342,7 +342,7 @@ github.com/valyala/fastjson/fastfloat # github.com/valyala/fastrand v1.1.0 ## explicit github.com/valyala/fastrand -# github.com/valyala/fasttemplate v1.2.1 +# github.com/valyala/fasttemplate v1.2.2 ## explicit; go 1.12 github.com/valyala/fasttemplate # github.com/valyala/gozstd v1.17.0