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

@@ -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.

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)

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).
 

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

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=

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
 }

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}")
 }

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)

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)

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
 	}
 

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