backport of commit b0eb3ec (#17788)

Co-authored-by: Michael Zalimeni <michael.zalimeni@hashicorp.com>
hashicorp · Jun 16, 2023 · 7eaa131 · 7eaa131
1 parent d371bfa
commit 7eaa131
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 3 deletions.
diff --git 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
@@ -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)
+				}
 			}
 		})
 	}