From 8b59bb7b0efef439eb428e6f4765217b8005e291 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Wed, 8 Jan 2025 14:39:14 +0100 Subject: [PATCH] cover error cases with invalid index types Signed-off-by: odubajDT --- .chloggen/indexing-pkg-ottl.yaml | 2 +- pkg/ottl/contexts/internal/map.go | 7 +++++-- pkg/ottl/contexts/internal/map_test.go | 4 ++-- pkg/ottl/contexts/internal/slice.go | 4 ++-- pkg/ottl/contexts/internal/slice_test.go | 4 ++-- pkg/ottl/e2e/e2e_test.go | 19 ++++++++++++++++++- 6 files changed, 30 insertions(+), 10 deletions(-) diff --git a/.chloggen/indexing-pkg-ottl.yaml b/.chloggen/indexing-pkg-ottl.yaml index 6ec875b3be5a..678525bfac99 100644 --- a/.chloggen/indexing-pkg-ottl.yaml +++ b/.chloggen/indexing-pkg-ottl.yaml @@ -1,7 +1,7 @@ # Use this changelog template to create an entry for release notes. # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' -change_type: enhancement +change_type: breaking # The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) component: pkg/ottl diff --git a/pkg/ottl/contexts/internal/map.go b/pkg/ottl/contexts/internal/map.go index 185069b74d2d..9965d6dc5c86 100644 --- a/pkg/ottl/contexts/internal/map.go +++ b/pkg/ottl/contexts/internal/map.go @@ -24,7 +24,7 @@ func GetMapValue[K any](ctx context.Context, tCtx K, m pcommon.Map, keys []ottl. if s == nil { resString, err := FetchValueFromExpression[K, string](ctx, tCtx, keys[0]) if err != nil { - return nil, fmt.Errorf("unable to resolve a string index: %w", err) + return nil, fmt.Errorf("unable to resolve a string index in map: %w", err) } s = resString } @@ -49,7 +49,7 @@ func SetMapValue[K any](ctx context.Context, tCtx K, m pcommon.Map, keys []ottl. if s == nil { resString, err := FetchValueFromExpression[K, string](ctx, tCtx, keys[0]) if err != nil { - return fmt.Errorf("unable to resolve a string index: %w", err) + return fmt.Errorf("unable to resolve a string index in map: %w", err) } s = resString } @@ -66,6 +66,9 @@ func FetchValueFromExpression[K any, T int64 | string](ctx context.Context, tCtx if err != nil { return nil, err } + if p == nil { + return nil, fmt.Errorf("invalid key type") + } res, err := p.Get(ctx, tCtx) if err != nil { return nil, err diff --git a/pkg/ottl/contexts/internal/map_test.go b/pkg/ottl/contexts/internal/map_test.go index 2e6ef4fa06c2..cc1544e18e5e 100644 --- a/pkg/ottl/contexts/internal/map_test.go +++ b/pkg/ottl/contexts/internal/map_test.go @@ -37,7 +37,7 @@ func Test_GetMapValue_Invalid(t *testing.T) { G: getSetter, }, }, - err: fmt.Errorf("unable to resolve a string index: could not resolve key for map/slice, expecting 'string' but got ''"), + err: fmt.Errorf("unable to resolve a string index in map: could not resolve key for map/slice, expecting 'string' but got ''"), }, { name: "index map with int", @@ -169,7 +169,7 @@ func Test_SetMapValue_Invalid(t *testing.T) { G: getSetter, }, }, - err: fmt.Errorf("unable to resolve a string index: could not resolve key for map/slice, expecting 'string' but got ''"), + err: fmt.Errorf("unable to resolve a string index in map: could not resolve key for map/slice, expecting 'string' but got ''"), }, { name: "index map with int", diff --git a/pkg/ottl/contexts/internal/slice.go b/pkg/ottl/contexts/internal/slice.go index cb407e40d165..9004d08ae836 100644 --- a/pkg/ottl/contexts/internal/slice.go +++ b/pkg/ottl/contexts/internal/slice.go @@ -24,7 +24,7 @@ func GetSliceValue[K any](ctx context.Context, tCtx K, s pcommon.Slice, keys []o if i == nil { resInt, err := FetchValueFromExpression[K, int64](ctx, tCtx, keys[0]) if err != nil { - return nil, fmt.Errorf("unable to resolve an integer index: %w", err) + return nil, fmt.Errorf("unable to resolve an integer index in slice: %w", err) } i = resInt } @@ -50,7 +50,7 @@ func SetSliceValue[K any](ctx context.Context, tCtx K, s pcommon.Slice, keys []o if i == nil { resInt, err := FetchValueFromExpression[K, int64](ctx, tCtx, keys[0]) if err != nil { - return fmt.Errorf("unable to resolve an integer index: %w", err) + return fmt.Errorf("unable to resolve an integer index in slice: %w", err) } i = resInt } diff --git a/pkg/ottl/contexts/internal/slice_test.go b/pkg/ottl/contexts/internal/slice_test.go index 3fe5378d5aa5..8e1b7e0b977b 100644 --- a/pkg/ottl/contexts/internal/slice_test.go +++ b/pkg/ottl/contexts/internal/slice_test.go @@ -37,7 +37,7 @@ func Test_GetSliceValue_Invalid(t *testing.T) { G: getSetter, }, }, - err: fmt.Errorf(`unable to resolve an integer index: could not resolve key for map/slice, expecting 'int64' but got ''`), + err: fmt.Errorf(`unable to resolve an integer index in slice: could not resolve key for map/slice, expecting 'int64' but got ''`), }, { name: "index too large", @@ -113,7 +113,7 @@ func Test_SetSliceValue_Invalid(t *testing.T) { G: getSetter, }, }, - err: fmt.Errorf(`unable to resolve an integer index: could not resolve key for map/slice, expecting 'int64' but got ''`), + err: fmt.Errorf(`unable to resolve an integer index in slice: could not resolve key for map/slice, expecting 'int64' but got ''`), }, { name: "index too large", diff --git a/pkg/ottl/e2e/e2e_test.go b/pkg/ottl/e2e/e2e_test.go index e60dbfff04a0..9084874e3eec 100644 --- a/pkg/ottl/e2e/e2e_test.go +++ b/pkg/ottl/e2e/e2e_test.go @@ -323,7 +323,19 @@ func Test_e2e_converters(t *testing.T) { tests := []struct { statement string want func(tCtx ottllog.TransformContext) + wantErr bool + errMsg string }{ + { + statement: `set(attributes["newOne"], attributes[1])`, + want: func(tCtx ottllog.TransformContext) {}, + errMsg: "unable to resolve a string index in map: invalid key type", + }, + { + statement: `set(attributes["array"][0.0], "bar")`, + want: func(tCtx ottllog.TransformContext) {}, + errMsg: "unable to resolve an integer index in slice: invalid key type", + }, { statement: `set(attributes[ConvertCase(attributes["A|B|C"], "upper")], "myvalue")`, want: func(tCtx ottllog.TransformContext) { @@ -1029,7 +1041,12 @@ func Test_e2e_converters(t *testing.T) { assert.NoError(t, err) tCtx := constructLogTransformContext() - _, _, _ = logStatements.Execute(context.Background(), tCtx) + _, _, err = logStatements.Execute(context.Background(), tCtx) + if tt.errMsg == "" { + assert.NoError(t, err) + } else { + assert.Contains(t, err.Error(), tt.errMsg) + } exTCtx := constructLogTransformContext() tt.want(exTCtx)