diff --git a/agent/envoyextensions/builtin/property-override/property_override.go b/agent/envoyextensions/builtin/property-override/property_override.go index d42e3d2d50..41e98074b7 100644 --- a/agent/envoyextensions/builtin/property-override/property_override.go +++ b/agent/envoyextensions/builtin/property-override/property_override.go @@ -259,9 +259,9 @@ func (p *propertyOverride) validate() error { } var resultErr error - for _, patch := range p.Patches { + for i, patch := range p.Patches { if err := patch.validate(p.Debug); err != nil { - resultErr = multierror.Append(resultErr, err) + resultErr = multierror.Append(resultErr, fmt.Errorf("invalid Patches[%d]: %w", i, err)) } } diff --git a/agent/envoyextensions/builtin/property-override/property_override_test.go b/agent/envoyextensions/builtin/property-override/property_override_test.go index 4a80db8671..0e4317f9dd 100644 --- a/agent/envoyextensions/builtin/property-override/property_override_test.go +++ b/agent/envoyextensions/builtin/property-override/property_override_test.go @@ -63,6 +63,7 @@ func TestConstructor(t *testing.T) { expected propertyOverride ok bool errMsg string + errFunc func(*testing.T, error) } validTestCase := func(o Op, d extensioncommon.TrafficDirection, t ResourceType) testCase { @@ -216,6 +217,50 @@ func TestConstructor(t *testing.T) { ok: false, errMsg: fmt.Sprintf("field Value is not supported for %s operation", OpRemove), }, + "multiple patches includes indexed errors": { + arguments: makeArguments(map[string]any{"Patches": []map[string]any{ + makePatch(map[string]any{ + "Op": OpRemove, + "Value": 0, + }), + makePatch(map[string]any{ + "Op": OpAdd, + "Value": nil, + }), + makePatch(map[string]any{ + "Op": OpAdd, + "Path": "/foo", + }), + }}), + ok: false, + errFunc: func(t *testing.T, err error) { + require.ErrorContains(t, err, "invalid Patches[0]: field Value is not supported for remove operation") + require.ErrorContains(t, err, "invalid Patches[1]: non-nil Value is required") + require.ErrorContains(t, err, "invalid Patches[2]: no match for field 'foo'") + }, + }, + "multiple patches single error contains correct index": { + arguments: makeArguments(map[string]any{"Patches": []map[string]any{ + makePatch(map[string]any{ + "Op": OpAdd, + "Value": "foo", + }), + makePatch(map[string]any{ + "Op": OpRemove, + "Value": 1, + }), + makePatch(map[string]any{ + "Op": OpAdd, + "Value": "bar", + }), + }}), + ok: false, + errFunc: func(t *testing.T, err error) { + require.ErrorContains(t, err, "invalid Patches[1]: field Value is not supported for remove operation") + require.NotContains(t, err.Error(), "invalid Patches[0]") + require.NotContains(t, err.Error(), "invalid Patches[2]") + }, + }, "empty service name": { arguments: makeArguments(map[string]any{"Patches": []map[string]any{ makePatch(map[string]any{ @@ -347,7 +392,13 @@ func TestConstructor(t *testing.T) { require.NoError(t, err) require.Equal(t, &extensioncommon.BasicEnvoyExtender{Extension: &tc.expected}, e) } else { - require.ErrorContains(t, err, tc.errMsg) + require.Error(t, err) + if tc.errMsg != "" { + require.ErrorContains(t, err, tc.errMsg) + } + if tc.errFunc != nil { + tc.errFunc(t, err) + } } }) }