Ignore regex capture groups when expanding environment vars (#754)

* ignore regex capture groups when expanding environment vars

* document behavior of regex capture group coercion

* remove weird formatting in changelog

CHANGELOG.md

@@ -19,15 +19,18 @@
 
 - [BUGFIX] Fix race condition that may occur and result in a panic when
   initializing scraping service cluster. (@rfratto)
-
+
+- [BUGFIX] Regex capture groups like `${1}` will now be kept intact when
+  using `-config.expand-env`.
+
 - [CHANGE] Breaking change: reduced verbosity of tracing autologging
   by not logging `STATUS_CODE_UNSET` status codes. (@mapno)
 
 - [DEPRECATION] The `loki` key at the root of the config file has been
   deprecated in favor of `logs`. `loki`-named fields in `automatic_logging`
   have been renamed accordinly: `loki_name` is now `logs_instance_name`,
   `loki_tag` is now `logs_instance_tag`, and `backend: loki` is now
-  `backend: logs_instance`.
+  `backend: logs_instance`. (@rfratto)
 
 # v0.16.1 (2021-06-22)
 

docs/configuration/_index.md

@@ -44,6 +44,17 @@ Where default_value is the value to use if the environment variable is
 undefined. The full list of supported syntax can be found at Drone's
 [envsubst repository](https://github.com/drone/envsubst).
 
+### Regex capture group references
+
+When using `-config.expand-env`, `VAR` must be an alphanumeric string with at
+least one non-digit character. If `VAR` is a number, the expander will assume
+you're trying to use a regex capture group reference, and will coerce the result
+to be one.
+
+This means references in your config file like `${1}` will remain
+untouched, but edge cases like `${1:-default}` will also be coerced to `${1}`,
+which may be slightly unexpected.
+
 ## Reloading (beta)
 
 The configuration file can be reloaded at runtime. Read the [API

pkg/config/config.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"unicode"
 
 	"github.com/go-kit/kit/log"
 	"github.com/go-kit/kit/log/level"
@@ -130,7 +131,7 @@ func LoadFile(filename string, expandEnvVars bool, c *Config) error {
 func LoadBytes(buf []byte, expandEnvVars bool, c *Config) error {
 	// (Optionally) expand with environment variables
 	if expandEnvVars {
-		s, err := envsubst.EvalEnv(string(buf))
+		s, err := envsubst.Eval(string(buf), getenv)
 		if err != nil {
 			return fmt.Errorf("unable to substitute config with environment variables: %w", err)
 		}
@@ -140,6 +141,25 @@ func LoadBytes(buf []byte, expandEnvVars bool, c *Config) error {
 	return yaml.UnmarshalStrict(buf, c)
 }
 
+// getenv is a wrapper around os.Getenv that ignores patterns that are numeric
+// regex capture groups (ie "${1}").
+func getenv(name string) string {
+	numericName := true
+
+	for _, r := range name {
+		if !unicode.IsDigit(r) {
+			numericName = false
+			break
+		}
+	}
+
+	if numericName {
+		// We need to add ${} back in since envsubst removes it.
+		return fmt.Sprintf("${%s}", name)
+	}
+	return os.Getenv(name)
+}
+
 // Load loads a config file from a flagset. Flags will be registered
 // to the flagset before parsing them with the values specified by
 // args.

pkg/config/config_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/grafana/agent/pkg/util"
 	"github.com/prometheus/common/model"
 	promCfg "github.com/prometheus/prometheus/config"
+	"github.com/prometheus/prometheus/pkg/labels"
 	"github.com/stretchr/testify/require"
 )
 
@@ -80,6 +81,23 @@ prometheus:
 	require.Equal(t, expect, c.Prometheus.Global)
 }
 
+func TestConfig_OverrideByEnvironmentOnLoad_NoDigits(t *testing.T) {
+	cfg := `
+prometheus:
+  wal_directory: /tmp/wal
+  global:
+    external_labels:
+      foo: ${1}`
+	expect := labels.Labels{{Name: "foo", Value: "${1}"}}
+
+	fs := flag.NewFlagSet("test", flag.ExitOnError)
+	c, err := load(fs, []string{"-config.file", "test"}, func(_ string, _ bool, c *Config) error {
+		return LoadBytes([]byte(cfg), true, c)
+	})
+	require.NoError(t, err)
+	require.Equal(t, expect, c.Prometheus.Global.Prometheus.ExternalLabels)
+}
+
 func TestConfig_FlagsAreAccepted(t *testing.T) {
 	cfg := `
 prometheus: