fixup! reloader: allow suppressing envvar errors

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
thanos-io · Jun 18, 2024 · f3a17f7 · f3a17f7
1 parent 5bcbc33
commit f3a17f7
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 26 deletions.
diff --git a/pkg/reloader/reloader.go b/pkg/reloader/reloader.go
@@ -87,15 +87,15 @@ import (
 // It optionally substitutes environment variables in the configuration.
 // Referenced environment variables must be of the form `$(var)` (not `$var` or `${var}`).
 type Reloader struct {
-	logger                       log.Logger
-	cfgFile                      string
-	cfgOutputFile                string
-	cfgDirs                      []CfgDirOption
-	discardEnvVarExpansionErrors bool
-	retryInterval                time.Duration
-	watchInterval                time.Duration
-	watchedDirs                  []string
-	watcher                      *watcher
+	logger                        log.Logger
+	cfgFile                       string
+	cfgOutputFile                 string
+	cfgDirs                       []CfgDirOption
+	tolerateEnvVarExpansionErrors bool
+	retryInterval                 time.Duration
+	watchInterval                 time.Duration
+	watchedDirs                   []string
+	watcher                       *watcher
 
 	tr TriggerReloader
 
@@ -174,9 +174,9 @@ type Options struct {
 	// RetryInterval controls how often the reloader retries a reloading of the
 	// configuration in case the reload operation returned an error.
 	RetryInterval time.Duration
-	// DiscardEnvVarExpansionErrors suppresses errors when expanding environment variables in the config file, and
+	// TolerateEnvVarExpansionErrors suppresses errors when expanding environment variables in the config file, and
 	// leaves the unset variables as is. All found environment variables are still expanded.
-	DiscardEnvVarExpansionErrors bool
+	TolerateEnvVarExpansionErrors bool
 }
 
 var firstGzipBytes = []byte{0x1f, 0x8b, 0x08}
@@ -188,16 +188,16 @@ func New(logger log.Logger, reg prometheus.Registerer, o *Options) *Reloader {
 		logger = log.NewNopLogger()
 	}
 	r := &Reloader{
-		logger:                       logger,
-		cfgFile:                      o.CfgFile,
-		cfgOutputFile:                o.CfgOutputFile,
-		cfgDirs:                      o.CfgDirs,
-		lastCfgDirFiles:              make([]map[string]struct{}, len(o.CfgDirs)),
-		watcher:                      newWatcher(logger, reg, o.DelayInterval),
-		watchedDirs:                  o.WatchedDirs,
-		watchInterval:                o.WatchInterval,
-		retryInterval:                o.RetryInterval,
-		discardEnvVarExpansionErrors: o.DiscardEnvVarExpansionErrors,
+		logger:                        logger,
+		cfgFile:                       o.CfgFile,
+		cfgOutputFile:                 o.CfgOutputFile,
+		cfgDirs:                       o.CfgDirs,
+		lastCfgDirFiles:               make([]map[string]struct{}, len(o.CfgDirs)),
+		watcher:                       newWatcher(logger, reg, o.DelayInterval),
+		watchedDirs:                   o.WatchedDirs,
+		watchInterval:                 o.WatchInterval,
+		retryInterval:                 o.RetryInterval,
+		tolerateEnvVarExpansionErrors: o.TolerateEnvVarExpansionErrors,
 
 		reloads: promauto.With(reg).NewCounter(
 			prometheus.CounterOpts{
@@ -705,6 +705,7 @@ func RuntimeInfoURLFromBase(u *url.URL) *url.URL {
 var envRe = regexp.MustCompile(`\$\(([a-zA-Z_0-9]+)\)`)
 
 func (r *Reloader) expandEnv(b []byte) (replaced []byte, err error) {
+	configEnvVarExpansionErrorsCount := 0
 	replaced = envRe.ReplaceAllFunc(b, func(n []byte) []byte {
 		if err != nil {
 			return nil
@@ -714,18 +715,18 @@ func (r *Reloader) expandEnv(b []byte) (replaced []byte, err error) {
 
 		v, ok := os.LookupEnv(string(n))
 		if !ok {
-			r.configEnvVarExpansionErrors.Inc()
+			configEnvVarExpansionErrorsCount++
 			errStr := errors.Errorf("found reference to unset environment variable %q", n)
-			if r.discardEnvVarExpansionErrors {
+			if r.tolerateEnvVarExpansionErrors {
 				level.Warn(r.logger).Log("msg", "expand environment variable", "err", errStr)
 				return m
 			}
-			level.Error(r.logger).Log("msg", "expand environment variable", "err", errStr)
 			err = errStr
 			return nil
 		}
 		return []byte(v)
 	})
+	r.configEnvVarExpansionErrors.Set(float64(configEnvVarExpansionErrorsCount))
 	return replaced, err
 }
 

diff --git a/pkg/reloader/reloader_test.go b/pkg/reloader/reloader_test.go
@@ -102,7 +102,7 @@ config:
 	configEnvVarExpansionErrorsCountPre := promtest.ToFloat64(reloader.configEnvVarExpansionErrors)
 
 	// Enable suppressing environment variables expansion errors.
-	reloader.discardEnvVarExpansionErrors = true
+	reloader.tolerateEnvVarExpansionErrors = true
 
 	// Set an environment variable while leaving the other unset, so as to ensure we don't break the flow when an unset
 	// variable is found.
@@ -111,7 +111,7 @@ config:
 	cancel2()
 
 	// Restore state.
-	reloader.discardEnvVarExpansionErrors = false
+	reloader.tolerateEnvVarExpansionErrors = false
 	testutil.Ok(t, os.Unsetenv("TEST_RELOADER_THANOS_ENV2"))
 
 	// The environment variable expansion errors should be suppressed, but recorded.