Merge branch 'main' into main

.chloggen/fix-env-var-double-escaping.yaml

@@ -0,0 +1,27 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: confmap
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Fix wrong expansion of environment variables escaped with `$$`, e.g. `$${ENV_VAR}` and `$$ENV_VAR`.
+
+# One or more tracking issues or pull requests related to the change
+issues: [10713]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: |
+  This change fixes the issue where environment variables escaped with $$ were expanded. 
+  The collector now converts `$${ENV_VAR}` to `${ENV_VAR}` and `$$ENV_VAR` to `$ENV_VAR` without further expansion.
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [api]

.chloggen/mx-psi_string-value-for-string-fields.yaml

@@ -0,0 +1,27 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: breaking
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: confmap
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: When passing configuration for a string field using any provider, use the verbatim string representation as the value.
+
+# One or more tracking issues or pull requests related to the change
+issues: [10605, 10405]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: | 
+  This matches the behavior of `${ENV}` syntax prior to the promotion of the `confmap.unifyEnvVarExpansion` feature gate
+  to beta. It changes the behavior of the `${env:ENV}` syntax with escaped strings.
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: []

.chloggen/mx-psi_validate-uris.yaml

@@ -0,0 +1,25 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: httpprovider, httpsprovider
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Validate URIs in HTTP and HTTPS providers before fetching.
+
+# One or more tracking issues or pull requests related to the change
+issues: [10468]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: []

confmap/confmap.go

@@ -108,9 +108,34 @@ func (l *Conf) Marshal(rawVal any, _ ...MarshalOption) error {
 	return l.Merge(NewFromStringMap(out))
 }
 
+func (l *Conf) unsanitizedGet(key string) any {
+	return l.k.Get(key)
+}
+
+func sanitize(a any) any {
+	switch m := a.(type) {
+	case map[string]any:
+		c := maps.Copy(m)
+		for k, v := range m {
+			c[k] = sanitize(v)
+		}
+		return c
+	case []any:
+		var newSlice []any
+		for _, e := range m {
+			newSlice = append(newSlice, sanitize(e))
+		}
+		return newSlice
+	case expandedValue:
+		return m.Value
+	}
+	return a
+}
+
 // Get can retrieve any value given the key to use.
 func (l *Conf) Get(key string) any {
-	return l.k.Get(key)
+	val := l.unsanitizedGet(key)
+	return sanitize(val)
 }
 
 // IsSet checks to see if the key has been set in any of the data locations.
@@ -128,7 +153,7 @@ func (l *Conf) Merge(in *Conf) error {
 // It returns an error is the sub-config is not a map[string]any (use Get()), and an empty Map if none exists.
 func (l *Conf) Sub(key string) (*Conf, error) {
 	// Code inspired by the koanf "Cut" func, but returns an error instead of empty map for unsupported sub-config type.
-	data := l.Get(key)
+	data := l.unsanitizedGet(key)
 	if data == nil {
 		return New(), nil
 	}
@@ -140,9 +165,14 @@ func (l *Conf) Sub(key string) (*Conf, error) {
 	return nil, fmt.Errorf("unexpected sub-config value kind for key:%s value:%v kind:%v", key, data, reflect.TypeOf(data).Kind())
 }
 
+func (l *Conf) toStringMapWithExpand() map[string]any {
+	m := maps.Unflatten(l.k.All(), KeyDelimiter)
+	return m
+}
+
 // ToStringMap creates a map[string]any from a Parser.
 func (l *Conf) ToStringMap() map[string]any {
-	return maps.Unflatten(l.k.All(), KeyDelimiter)
+	return sanitize(l.toStringMapWithExpand()).(map[string]any)
 }
 
 // decodeConfig decodes the contents of the Conf into the result argument, using a
@@ -160,6 +190,7 @@ func decodeConfig(m *Conf, result any, errorUnused bool, skipTopLevelUnmarshaler
 		WeaklyTypedInput: !globalgates.StrictlyTypedInputGate.IsEnabled(),
 		MatchName:        caseSensitiveMatchName,
 		DecodeHook: mapstructure.ComposeDecodeHookFunc(
+			useExpandValue(),
 			expandNilStructPointersHookFunc(),
 			mapstructure.StringToSliceHookFunc(","),
 			mapKeyStringToMapKeyTextUnmarshalerHookFunc(),
@@ -177,7 +208,7 @@ func decodeConfig(m *Conf, result any, errorUnused bool, skipTopLevelUnmarshaler
 	if err != nil {
 		return err
 	}
-	if err = decoder.Decode(m.ToStringMap()); err != nil {
+	if err = decoder.Decode(m.toStringMapWithExpand()); err != nil {
 		if strings.HasPrefix(err.Error(), "error decoding ''") {
 			return errors.Unwrap(err)
 		}
@@ -206,6 +237,40 @@ func caseSensitiveMatchName(a, b string) bool {
 	return a == b
 }
 
+func castTo(exp expandedValue, useOriginal bool) (any, error) {
+	// If the target field is a string, use `exp.Original` or fail if not available.
+	if globalgates.StrictlyTypedInputGate.IsEnabled() && useOriginal {
+		if !exp.HasOriginal {
+			return nil, fmt.Errorf("cannot expand value to string: original value not set")
+		}
+		return exp.Original, nil
+	}
+	// Otherwise, use the parsed value (previous behavior).
+	return exp.Value, nil
+}
+
+// When a value has been loaded from an external source via a provider, we keep both the
+// parsed value and the original string value. This allows us to expand the value to its
+// original string representation when decoding into a string field, and use the original otherwise.
+func useExpandValue() mapstructure.DecodeHookFuncType {
+	return func(
+		_ reflect.Type,
+		to reflect.Type,
+		data any) (any, error) {
+		if exp, ok := data.(expandedValue); ok {
+			return castTo(exp, to.Kind() == reflect.String)
+		}
+
+		// If the target field is a map or slice, sanitize input to remove expandedValue references.
+		switch to.Kind() {
+		case reflect.Array, reflect.Slice, reflect.Map:
+			// This does not handle map[string]string and []string explicitly.
+			return sanitize(data), nil
+		}
+		return data, nil
+	}
+}
+
 // In cases where a config has a mapping of something to a struct pointers
 // we want nil values to resolve to a pointer to the zero value of the
 // underlying struct just as we want nil values of a mapping of something

confmap/confmap_test.go

@@ -845,3 +845,35 @@ func TestRecursiveUnmarshaling(t *testing.T) {
 	require.NoError(t, conf.Unmarshal(r))
 	require.Equal(t, "something", r.Foo)
 }
+
+func TestExpandedValue(t *testing.T) {
+	cm := NewFromStringMap(map[string]any{
+		"key": expandedValue{
+			Value:       0xdeadbeef,
+			HasOriginal: true,
+			Original:    "original",
+		}})
+	assert.Equal(t, 0xdeadbeef, cm.Get("key"))
+	assert.Equal(t, map[string]any{"key": 0xdeadbeef}, cm.ToStringMap())
+
+	type ConfigStr struct {
+		Key string `mapstructure:"key"`
+	}
+
+	cfgStr := ConfigStr{}
+	assert.NoError(t, cm.Unmarshal(&cfgStr))
+	assert.Equal(t, "original", cfgStr.Key)
+
+	type ConfigInt struct {
+		Key int `mapstructure:"key"`
+	}
+	cfgInt := ConfigInt{}
+	assert.NoError(t, cm.Unmarshal(&cfgInt))
+	assert.Equal(t, 0xdeadbeef, cfgInt.Key)
+
+	type ConfigBool struct {
+		Key bool `mapstructure:"key"`
+	}
+	cfgBool := ConfigBool{}
+	assert.Error(t, cm.Unmarshal(&cfgBool))
+}

confmap/converter/expandconverter/expand.go

@@ -85,6 +85,7 @@ func (c converter) expandEnv(s string) (string, error) {
 		// - $FOO will be substituted with env var FOO
 		// - $$FOO will be replaced with $FOO
 		// - $$$FOO will be replaced with $ + substituted env var FOO
+		// TODO: Move the escaping of $$ out from the expand converter to the resolver.
 		if str == "$" {
 			return "$"
 		}

confmap/expand.go

@@ -43,6 +43,40 @@ func (mr *Resolver) expandValueRecursively(ctx context.Context, value any) (any,
 
 func (mr *Resolver) expandValue(ctx context.Context, value any) (any, bool, error) {
 	switch v := value.(type) {
+	case expandedValue:
+		expanded, changed, err := mr.expandValue(ctx, v.Value)
+		if err != nil {
+			return nil, false, err
+		}
+
+		switch exp := expanded.(type) {
+		case expandedValue, string:
+			// Return expanded values or strings verbatim.
+			return exp, changed, nil
+		}
+
+		// At this point we don't know the target field type, so we need to expand the original representation as well.
+		originalExpanded, originalChanged, err := mr.expandValue(ctx, v.Original)
+		if err != nil {
+			return nil, false, err
+		}
+
+		if originalExpanded, ok := originalExpanded.(string); ok {
+			// If the original representation is a string, return the expanded value with the original representation.
+			return expandedValue{
+				Value:       expanded,
+				Original:    originalExpanded,
+				HasOriginal: true,
+			}, changed || originalChanged, nil
+		}
+
+		result := expandedValue{
+			Value:       expanded,
+			Original:    v.Original,
+			HasOriginal: v.HasOriginal,
+		}
+
+		return result, changed || originalChanged, nil
 	case string:
 		if !strings.Contains(v, "${") || !strings.Contains(v, "}") {
 			// No URIs to expand.
@@ -117,6 +151,20 @@ func (mr *Resolver) findURI(input string) string {
 	return input[openIndex : closeIndex+1]
 }
 
+// expandedValue holds the YAML parsed value and original representation of a value.
+// It keeps track of the original representation to be used by the 'useExpandValue' hook
+// if the target field is a string. We need to keep both representations because we don't know
+// what the target field type is until `Unmarshal` is called.
+type expandedValue struct {
+	// Value is the expanded value.
+	Value any
+	// HasOriginal is true if the original representation is set.
+	HasOriginal bool
+	// Original is the original representation of the value.
+	// It is only valid if HasOriginal is true.
+	Original string
+}
+
 // findAndExpandURI attempts to find and expand the first occurrence of an expandable URI in input. If an expandable URI is found it
 // returns the input with the URI expanded, true and nil. Otherwise, it returns the unchanged input, false and the expanding error.
 // This method expects input to start with ${ and end with }
@@ -134,10 +182,17 @@ func (mr *Resolver) findAndExpandURI(ctx context.Context, input string) (any, bo
 			return input, false, err
 		}
 
-		expanded, err := ret.AsRaw()
+		expanded := expandedValue{}
+		expanded.Value, err = ret.AsRaw()
 		if err != nil {
 			return input, false, err
 		}
+
+		if asStr, err2 := ret.AsString(); err2 == nil {
+			expanded.HasOriginal = true
+			expanded.Original = asStr
+		}
+
 		return expanded, true, err
 	}
 	expanded, err := mr.expandURI(ctx, uri)

confmap/expand_test.go

@@ -577,46 +577,3 @@ func TestResolverDefaultProviderExpand(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, map[string]any{"foo": "localhost"}, cfgMap.ToStringMap())
 }
-
-func Test_EscapedEnvVars(t *testing.T) {
-	const mapValue2 = "some map value"
-
-	expectedMap := map[string]any{
-		"test_map": map[string]any{
-			"recv.1":  "$MAP_VALUE_1",
-			"recv.2":  "$$MAP_VALUE_2",
-			"recv.3":  "$$MAP_VALUE_3",
-			"recv.4":  "$" + mapValue2,
-			"recv.5":  "some${MAP_VALUE_4}text",
-			"recv.6":  "${ONE}${TWO}",
-			"recv.7":  "text$",
-			"recv.8":  "$",
-			"recv.9":  "${1}${env:2}",
-			"recv.10": "some${env:MAP_VALUE_4}text",
-			"recv.11": "${env:" + mapValue2 + "}",
-			"recv.12": "${env:${MAP_VALUE_2}}",
-			"recv.13": "env:MAP_VALUE_2}${MAP_VALUE_2}{",
-			"recv.14": "${env:MAP_VALUE_2${MAP_VALUE_2}",
-			"recv.15": "$" + mapValue2,
-		}}
-
-	fileProvider := newFakeProvider("file", func(_ context.Context, uri string, _ WatcherFunc) (*Retrieved, error) {
-		return NewRetrieved(newConfFromFile(t, uri[5:]))
-	})
-	envProvider := newFakeProvider("env", func(_ context.Context, uri string, _ WatcherFunc) (*Retrieved, error) {
-		if uri == "env:MAP_VALUE_2" {
-			return NewRetrieved(mapValue2)
-		}
-		return nil, errors.New("should not be expanding any other env vars")
-	})
-
-	resolver, err := NewResolver(ResolverSettings{URIs: []string{filepath.Join("testdata", "expand-escaped-env.yaml")}, ProviderFactories: []ProviderFactory{fileProvider, envProvider}, ConverterFactories: nil, DefaultScheme: "env"})
-	require.NoError(t, err)
-
-	// Test that expanded configs are the same with the simple config with no env vars.
-	cfgMap, err := resolver.Resolve(context.Background())
-	require.NoError(t, err)
-	m := cfgMap.ToStringMap()
-	assert.Equal(t, expectedMap, m)
-
-}