Add Patch index to Prop Override validation errors (#17777)

When a patch is found invalid, include its index for easier debugging
when multiple patches are provided.
This commit is contained in:
Michael Zalimeni 2023-06-16 09:37:47 -04:00 committed by GitHub
parent 730c599adc
commit 265c003033
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 54 additions and 3 deletions

View File

@ -259,9 +259,9 @@ func (p *propertyOverride) validate() error {
} }
var resultErr error var resultErr error
for _, patch := range p.Patches { for i, patch := range p.Patches {
if err := patch.validate(p.Debug); err != nil { 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))
} }
} }

View File

@ -63,6 +63,7 @@ func TestConstructor(t *testing.T) {
expected propertyOverride expected propertyOverride
ok bool ok bool
errMsg string errMsg string
errFunc func(*testing.T, error)
} }
validTestCase := func(o Op, d extensioncommon.TrafficDirection, t ResourceType) testCase { validTestCase := func(o Op, d extensioncommon.TrafficDirection, t ResourceType) testCase {
@ -216,6 +217,50 @@ func TestConstructor(t *testing.T) {
ok: false, ok: false,
errMsg: fmt.Sprintf("field Value is not supported for %s operation", OpRemove), 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": { "empty service name": {
arguments: makeArguments(map[string]any{"Patches": []map[string]any{ arguments: makeArguments(map[string]any{"Patches": []map[string]any{
makePatch(map[string]any{ makePatch(map[string]any{
@ -347,7 +392,13 @@ func TestConstructor(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, &extensioncommon.BasicEnvoyExtender{Extension: &tc.expected}, e) require.Equal(t, &extensioncommon.BasicEnvoyExtender{Extension: &tc.expected}, e)
} else { } 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)
}
} }
}) })
} }