From 9c30c537e1d47f00feb9416309fd062f3e971d50 Mon Sep 17 00:00:00 2001 From: Iris Grace Endozo Date: Mon, 23 Sep 2024 17:43:54 +1000 Subject: [PATCH 1/8] Remove explicit check --- confmap/expand_test.go | 7 +++++++ confmap/provider.go | 17 ----------------- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/confmap/expand_test.go b/confmap/expand_test.go index a9a3b49b850..ec26f974873 100644 --- a/confmap/expand_test.go +++ b/confmap/expand_test.go @@ -185,6 +185,11 @@ func TestResolverExpandStringValues(t *testing.T) { input: "test_${env:BOOL}", output: "test_true", }, + { + name: "Bool", + input: "test_${env:TIMESTAMP}", + output: "test_2023-03-20T03:17:55.432328Z", + }, { name: "MultipleSameMatches", input: "test_${env:BOOL}_test_${env:BOOL}", @@ -414,6 +419,8 @@ func newEnvProvider() ProviderFactory { return NewRetrievedFromYAML([]byte("[localhost:3042]")) case "env:HOST": return NewRetrievedFromYAML([]byte("localhost")) + case "env:TIMESTAMP": + return NewRetrievedFromYAML([]byte("2023-03-20T03:17:55.432328Z")) case "env:OS": return NewRetrievedFromYAML([]byte("ubuntu")) case "env:PR": diff --git a/confmap/provider.go b/confmap/provider.go index daf508bf925..94394e45d50 100644 --- a/confmap/provider.go +++ b/confmap/provider.go @@ -165,9 +165,6 @@ func NewRetrievedFromYAML(yamlBytes []byte, opts ...RetrievedOption) (*Retrieved // - []any; // - map[string]any; func NewRetrieved(rawConf any, opts ...RetrievedOption) (*Retrieved, error) { - if err := checkRawConfType(rawConf); err != nil { - return nil, err - } set := retrievedSettings{} for _, opt := range opts { opt.apply(&set) @@ -229,17 +226,3 @@ func (r *Retrieved) Close(ctx context.Context) error { // CloseFunc a function equivalent to Retrieved.Close. type CloseFunc func(context.Context) error - -func checkRawConfType(rawConf any) error { - if rawConf == nil { - return nil - } - switch rawConf.(type) { - case int, int32, int64, float32, float64, bool, string, []any, map[string]any: - return nil - default: - return fmt.Errorf( - "unsupported type=%T for retrieved config,"+ - " ensure that values are wrapped in quotes", rawConf) - } -} From a1279efe1e43d5e077deb2553c7abfda7d921eb6 Mon Sep 17 00:00:00 2001 From: Iris Grace Endozo Date: Mon, 23 Sep 2024 17:48:51 +1000 Subject: [PATCH 2/8] Add test case --- confmap/provider_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/confmap/provider_test.go b/confmap/provider_test.go index e82ddf90355..3877222865c 100644 --- a/confmap/provider_test.go +++ b/confmap/provider_test.go @@ -7,6 +7,7 @@ import ( "context" "errors" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -103,6 +104,10 @@ func TestNewRetrievedFromYAMLString(t *testing.T) { yaml: "123", value: 123, }, + { + yaml: "2023-03-20T03:17:55.432328Z", + value: time.Date(2023, 3, 20, 3, 17, 55, 432328000, time.UTC), + }, { yaml: "true", value: true, From ed88f37ec67c1b545fc3ececa9ad40134eb926a5 Mon Sep 17 00:00:00 2001 From: Iris Grace Endozo Date: Mon, 23 Sep 2024 19:46:37 +1000 Subject: [PATCH 3/8] Change test case name --- confmap/expand_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/confmap/expand_test.go b/confmap/expand_test.go index ec26f974873..d634564d5b6 100644 --- a/confmap/expand_test.go +++ b/confmap/expand_test.go @@ -186,7 +186,7 @@ func TestResolverExpandStringValues(t *testing.T) { output: "test_true", }, { - name: "Bool", + name: "Timestamp", input: "test_${env:TIMESTAMP}", output: "test_2023-03-20T03:17:55.432328Z", }, From a8790f998c724fbc230fa3eac287d08e3fda8615 Mon Sep 17 00:00:00 2001 From: Iris Grace Endozo Date: Mon, 23 Sep 2024 19:59:46 +1000 Subject: [PATCH 4/8] Add e2e test case --- confmap/internal/e2e/types_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/confmap/internal/e2e/types_test.go b/confmap/internal/e2e/types_test.go index 6085ba0a438..3c3d89e6ad1 100644 --- a/confmap/internal/e2e/types_test.go +++ b/confmap/internal/e2e/types_test.go @@ -240,6 +240,11 @@ func TestStrictTypeCasting(t *testing.T) { targetField: TargetFieldInlineString, expected: "inline field with [filelog,windowseventlog/application] expansion", }, + { + value: `2023-03-20T03:17:55.432328Z`, + targetField: TargetFieldString, + expected: `2023-03-20T03:17:55.432328Z`, + }, { value: "$$ENV", targetField: TargetFieldString, From 0d378852e75dbc2b064f35ef19cff05c4daf5308 Mon Sep 17 00:00:00 2001 From: Iris Grace Endozo Date: Mon, 23 Sep 2024 20:40:27 +1000 Subject: [PATCH 5/8] Add changelog --- .chloggen/support-time-envvar-expansion.yaml | 26 ++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 .chloggen/support-time-envvar-expansion.yaml diff --git a/.chloggen/support-time-envvar-expansion.yaml b/.chloggen/support-time-envvar-expansion.yaml new file mode 100644 index 00000000000..c126c2fafee --- /dev/null +++ b/.chloggen/support-time-envvar-expansion.yaml @@ -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 including time.Time formats + +# One or more tracking issues or pull requests related to the change +issues: [10659] + +# (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: | + Previously, fields with time.Time formats could not be used as strings in configurations + +# 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: [] From d12a7b41b69b3ed12f6b1499db11ac96309c42fc Mon Sep 17 00:00:00 2001 From: Iris Grace Endozo Date: Mon, 23 Sep 2024 20:44:53 +1000 Subject: [PATCH 6/8] Add more test case --- confmap/internal/e2e/types_test.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/confmap/internal/e2e/types_test.go b/confmap/internal/e2e/types_test.go index 3c3d89e6ad1..6e335daaa6c 100644 --- a/confmap/internal/e2e/types_test.go +++ b/confmap/internal/e2e/types_test.go @@ -197,6 +197,16 @@ func TestStrictTypeCasting(t *testing.T) { targetField: TargetFieldInlineString, expected: "inline field with 2006-01-02T15:04:05Z07:00 expansion", }, + { + value: "2023-03-20T03:17:55.432328Z", + targetField: TargetFieldString, + expected: "2023-03-20T03:17:55.432328Z", + }, + { + value: "2023-03-20T03:17:55.432328Z", + targetField: TargetFieldInlineString, + expected: "inline field with 2023-03-20T03:17:55.432328Z expansion", + }, // issue 10787 { value: "true # comment with a ${env:hello.world} reference", @@ -240,11 +250,6 @@ func TestStrictTypeCasting(t *testing.T) { targetField: TargetFieldInlineString, expected: "inline field with [filelog,windowseventlog/application] expansion", }, - { - value: `2023-03-20T03:17:55.432328Z`, - targetField: TargetFieldString, - expected: `2023-03-20T03:17:55.432328Z`, - }, { value: "$$ENV", targetField: TargetFieldString, From dd97ee812263ce673c072b7055beee6cb776b3fc Mon Sep 17 00:00:00 2001 From: Iris Grace Endozo Date: Mon, 23 Sep 2024 21:25:49 +1000 Subject: [PATCH 7/8] Remove tests for expecting errors on unsupported types --- confmap/provider_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/confmap/provider_test.go b/confmap/provider_test.go index 3877222865c..a8bbf41a9fb 100644 --- a/confmap/provider_test.go +++ b/confmap/provider_test.go @@ -32,11 +32,6 @@ func TestNewRetrievedWithOptions(t *testing.T) { assert.Equal(t, want, ret.Close(context.Background())) } -func TestNewRetrievedUnsupportedType(t *testing.T) { - _, err := NewRetrieved(errors.New("my error")) - require.Error(t, err) -} - func TestNewRetrievedFromYAML(t *testing.T) { ret, err := NewRetrievedFromYAML([]byte{}) require.NoError(t, err) From 0b2af624175b02e1b88a9cf57c33d8a9740f19a0 Mon Sep 17 00:00:00 2001 From: Iris Grace Endozo Date: Tue, 24 Sep 2024 07:33:40 +1000 Subject: [PATCH 8/8] Add back check and add time.Time as type --- confmap/provider.go | 18 ++++++++++++++++++ confmap/provider_test.go | 5 +++++ 2 files changed, 23 insertions(+) diff --git a/confmap/provider.go b/confmap/provider.go index 94394e45d50..c462b9bb6fe 100644 --- a/confmap/provider.go +++ b/confmap/provider.go @@ -6,6 +6,7 @@ package confmap // import "go.opentelemetry.io/collector/confmap" import ( "context" "fmt" + "time" "go.uber.org/zap" "gopkg.in/yaml.v3" @@ -165,6 +166,9 @@ func NewRetrievedFromYAML(yamlBytes []byte, opts ...RetrievedOption) (*Retrieved // - []any; // - map[string]any; func NewRetrieved(rawConf any, opts ...RetrievedOption) (*Retrieved, error) { + if err := checkRawConfType(rawConf); err != nil { + return nil, err + } set := retrievedSettings{} for _, opt := range opts { opt.apply(&set) @@ -226,3 +230,17 @@ func (r *Retrieved) Close(ctx context.Context) error { // CloseFunc a function equivalent to Retrieved.Close. type CloseFunc func(context.Context) error + +func checkRawConfType(rawConf any) error { + if rawConf == nil { + return nil + } + switch rawConf.(type) { + case int, int32, int64, float32, float64, bool, string, []any, map[string]any, time.Time: + return nil + default: + return fmt.Errorf( + "unsupported type=%T for retrieved config,"+ + " ensure that values are wrapped in quotes", rawConf) + } +} diff --git a/confmap/provider_test.go b/confmap/provider_test.go index a8bbf41a9fb..3877222865c 100644 --- a/confmap/provider_test.go +++ b/confmap/provider_test.go @@ -32,6 +32,11 @@ func TestNewRetrievedWithOptions(t *testing.T) { assert.Equal(t, want, ret.Close(context.Background())) } +func TestNewRetrievedUnsupportedType(t *testing.T) { + _, err := NewRetrieved(errors.New("my error")) + require.Error(t, err) +} + func TestNewRetrievedFromYAML(t *testing.T) { ret, err := NewRetrievedFromYAML([]byte{}) require.NoError(t, err)