Skip to content

Commit

Permalink
[confmap] Allow using any type as a string (#10800)
Browse files Browse the repository at this point in the history
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

Allows any type to be used as a string if the target field is a string
or the value is expanded in inline position. Inspired by #10794. This is
not a breaking change (previously we would return an error for these).

Some things that you can do after this PR that you couldn't do before
it:

1. Set `HOST` to `[2001:db8::8a2e:370:7334]` and use it in an endpoint:
```yaml
exporters:
  otlp:
    endpoint: http://${env:HOST}/api/v1/traces
```

2. Pass really big numbers as strings (e.g. `10000000000000000000`)

3. Pass `null` as a string.

<details>
<summary>Types that can be returned by our current YAML parser</summary>

Our current way of using the YAML parser has these types: `string`,
`nil`, `int`, `uint64`, `float64`, `map[any]any`, `map[string]any`,
`[]any`.

There is no documentation for this, but the following fuzzing test did
not find any failing cases after 20 minutes of continous run:

```go
package main

import (
	"testing"
	"gopkg.in/yaml.v3"
)

func FuzzTest(f *testing.F) {
	f.Fuzz(func(t *testing.T, data []byte){
		var b any
		err := yaml.Unmarshal([]byte(data), &b)
		if err != nil {
			return
		}

		switch b.(type) {
			case string, nil, int, uint64, float64, map[any]any, map[string]any, []any:
				return
			default:
			  t.Errorf("Unexpected type %T", b)
		}
	})	
}

```
</details>
  • Loading branch information
mx-psi authored Aug 5, 2024
1 parent 7c2002a commit 93cbae4
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 17 deletions.
26 changes: 26 additions & 0 deletions .chloggen/mx-psi_add-string-representation-for-everything.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# 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: confmap

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Allow using any YAML structure as a string when loading configuration.

# One or more tracking issues or pull requests related to the change
issues: [10800]

# (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: |
Previous to this change, slices could not be used as strings in configuration.
# 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: []
16 changes: 0 additions & 16 deletions confmap/expand_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -542,22 +542,6 @@ func TestResolverExpandUnsupportedScheme(t *testing.T) {
assert.EqualError(t, err, `scheme "unsupported" is not supported for uri "unsupported:VALUE"`)
}

func TestResolverExpandStringValueInvalidReturnValue(t *testing.T) {
provider := newFakeProvider("input", func(context.Context, string, WatcherFunc) (*Retrieved, error) {
return NewRetrievedFromYAML([]byte(`test: "localhost:${test:PORT}"`))
})

testProvider := newFakeProvider("test", func(context.Context, string, WatcherFunc) (*Retrieved, error) {
return NewRetrievedFromYAML([]byte("[1243]"))
})

resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, ProviderFactories: []ProviderFactory{provider, testProvider}, ConverterFactories: nil})
require.NoError(t, err)

_, err = resolver.Resolve(context.Background())
assert.EqualError(t, err, `expanding ${test:PORT}: retrieved value does not have unambiguous string representation: [1243]`)
}

func TestResolverDefaultProviderExpand(t *testing.T) {
provider := newFakeProvider("input", func(context.Context, string, WatcherFunc) (*Retrieved, error) {
return NewRetrieved(map[string]any{"foo": "${HOST}"})
Expand Down
10 changes: 10 additions & 0 deletions confmap/internal/e2e/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,16 @@ func TestStrictTypeCasting(t *testing.T) {
targetField: TargetFieldSlice,
expected: []any{"filelog", "windowseventlog/application"},
},
{
value: `[filelog,windowseventlog/application]`,
targetField: TargetFieldString,
expected: "[filelog,windowseventlog/application]",
},
{
value: `[filelog,windowseventlog/application]`,
targetField: TargetFieldInlineString,
expected: "inline field with [filelog,windowseventlog/application] expansion",
},
}

previousValue := globalgates.StrictlyTypedInputGate.IsEnabled()
Expand Down
2 changes: 1 addition & 1 deletion confmap/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func NewRetrievedFromYAML(yamlBytes []byte, opts ...RetrievedOption) (*Retrieved
val = string(yamlBytes)
}
return NewRetrieved(val, append(opts, withStringRepresentation(val))...)
case int, int32, int64, float32, float64, bool, map[string]any:
default:
opts = append(opts, withStringRepresentation(string(yamlBytes)))
}

Expand Down

0 comments on commit 93cbae4

Please sign in to comment.