Disable $$ expansion for backward compatibility with old versions

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 ## Unreleased
 
+### 🛑 Breaking changes 🛑
+
+- (Splunk) Don't expand environment variables starting with $$ in configuration files. This behavior was introduced
+  in v0.42.0 to support a bug causing double expansion. $$ is treated as an escape sequence representing a literal
+  $ character ([#5134](https://github.com/signalfx/splunk-otel-collector/pull/5134))
+
 ### 💡 Enhancements 💡
 
 - (Splunk) Update bundled OpenJDK to [11.0.24_8](https://github.com/adoptium/temurin11-binaries/releases/tag/jdk-11.0.24%2B8) ([#5113](https://github.com/signalfx/splunk-otel-collector/pull/5113))

internal/configsource/source.go

@@ -17,10 +17,8 @@ package configsource
 import (
 	"context"
 	"fmt"
-	"log"
 	"net/url"
 	"os"
-	"strconv"
 	"strings"
 
 	"github.com/knadh/koanf/maps"
@@ -42,22 +40,13 @@ const (
 	// typeAndNameSeparator is the separator that is used between type and name in type/name
 	// composite keys.
 	typeAndNameSeparator = '/'
-	// dollarDollarCompatEnvVar is a temporary env var to disable backward compatibility (true by default)
-	dollarDollarCompatEnvVar = "SPLUNK_DOUBLE_DOLLAR_CONFIG_SOURCE_COMPATIBLE"
 )
 
 // private error types to help with testability
 type (
 	errUnknownConfigSource struct{ error }
 )
 
-var ddBackwardCompatible = func() bool {
-	if v, err := strconv.ParseBool(strings.ToLower(os.Getenv(dollarDollarCompatEnvVar))); err == nil {
-		return v
-	}
-	return true
-}()
-
 type ConfigSource interface {
 	// Retrieve goes to the configuration source and retrieves the selected data which
 	// contains the value to be injected in the configuration and the corresponding watcher that
@@ -346,56 +335,18 @@ func resolveStringValue(ctx context.Context, configSources map[string]ConfigSour
 			w := 0 // number of bytes consumed on this pass
 
 			switch {
-			case s[j+1] == expandPrefixChar:
-				// temporary backward compatibility to support updated $${config_source:value} functionality
-				// in provided configs from 0.37.0 until 0.42.0
-				bwCompatibilityRequired := false
-
-				var expanded, sourceName string
-				var ww int
-				if ddBackwardCompatible && len(s[j+1:]) > 2 {
-					if s[j+2] == '{' {
-						if expanded, ww, sourceName = getBracketedExpandableContent(s, j+2); sourceName != "" {
-							bwCompatibilityRequired = true
-						}
-					} else {
-						if expanded, ww, sourceName = getBareExpandableContent(s, j+2); sourceName != "" {
-							if len(expanded) > (len(sourceName) + 1) {
-								if !strings.Contains(expanded[len(sourceName)+1:], "$") {
-									bwCompatibilityRequired = true
-								}
-							}
-						}
-					}
-				}
-
-				if bwCompatibilityRequired {
-					log.Printf(
-						`Deprecated config source directive %q has been replaced with %q. Please update your config as necessary as this will be removed in future release. To disable this replacement set the SPLUNK_DOUBLE_DOLLAR_CONFIG_SOURCE_COMPATIBLE environment variable to "false" before restarting the Collector.`,
-						s[j:j+2+ww], s[j+1:j+2+ww],
-					)
-					expandableContent = expanded
-					w = ww + 1
-					cfgSrcName = sourceName
-				} else {
-					// Escaping the prefix so $$ becomes a single $ without attempting
-					// to treat the string after it as a config source or env var.
-					expandableContent = string(expandPrefixChar)
-					w = 1 // consumed a single char
-				}
-
 			case s[j+1] == '{':
 				expandableContent, w, cfgSrcName = getBracketedExpandableContent(s, j+1)
-
 			default:
+				// TODO: To be deprecated removed to align with the upstream behavior
 				expandableContent, w, cfgSrcName = getBareExpandableContent(s, j+1)
-
 			}
 
 			// At this point expandableContent contains a string to be expanded, evaluate and expand it.
 			switch {
 			case cfgSrcName == "":
 				// Not a config source, expand as os.ExpandEnv
+				// TODO: Align with confmap.strictlyTypedInput feature gate
 				buf = osExpandEnv(buf, expandableContent, w)
 
 			default:

internal/configsource/testdata/envvar_cfgsrc_mix_expected.yaml

@@ -10,10 +10,10 @@ envvar_legacy_05: /test
 cfgsrc_suffix: prefix-42
 cfgsrc_middle: prefix-42-suffix
 cfgsrc_in_str: integer 42 injected as string
-cfgsrc_workaround_suffix: prefix-42
-cfgsrc_braces_workaround_suffix: prefix-42
-cfgsrc_braces_workaround_middle: prefix-42-suffix
-cfgsrc_braces_workaround_in_str: integer 42 injected as string
+cfgsrc_workaround_suffix: prefix-$tstcfgsrc:int_key
+cfgsrc_braces_workaround_suffix: prefix-${     tstcfgsrc:int_key    }
+cfgsrc_braces_workaround_middle: prefix-${tstcfgsrc:int_key}-suffix
+cfgsrc_braces_workaround_in_str: integer ${      tstcfgsrc:int_key} injected as string
 cfgsrc_params0:
   p0: true
   p1: envvar_value

tests/general/envvar_expansion_test.go

@@ -41,26 +41,9 @@ func TestExpandedDollarSignsViaStandardEnvVar(t *testing.T) {
 	)
 }
 
-func TestExpandedDollarSignsViaEnvConfigSource(t *testing.T) {
-	testutils.CheckGoldenFileWithCollectorOptions(t, "env_config_source_labels.yaml", "env_config_source_labels_expected.yaml", func(collector testutils.Collector) testutils.Collector {
-		return collector.WithEnv(map[string]string{"AN_ENVVAR": "an-envvar-value"})
-	},
-		pmetrictest.IgnoreScopeVersion(),
-		pmetrictest.IgnoreTimestamp(),
-		pmetrictest.IgnoreStartTimestamp(),
-		pmetrictest.IgnoreMetricDataPointsOrder(),
-		pmetrictest.IgnoreScopeMetricsOrder(),
-		pmetrictest.IgnoreResourceMetricsOrder(),
-		pmetrictest.IgnoreMetricsOrder(),
-		pmetrictest.IgnoreMetricValues(),
-	)
-}
-
 func TestIncompatibleExpandedDollarSignsViaEnvConfigSource(t *testing.T) {
 	testutils.CheckGoldenFileWithCollectorOptions(t, "env_config_source_labels.yaml", "incompat_env_config_source_labels_expected.yaml", func(collector testutils.Collector) testutils.Collector {
-		return collector.WithEnv(map[string]string{
-			"SPLUNK_DOUBLE_DOLLAR_CONFIG_SOURCE_COMPATIBLE": "false",
-			"AN_ENVVAR": "an-envvar-value"})
+		return collector.WithEnv(map[string]string{"AN_ENVVAR": "an-envvar-value"})
 	},
 		pmetrictest.IgnoreScopeVersion(),
 		pmetrictest.IgnoreTimestamp(),