From 9abf5fca3cb679612f39e331c44ce8b2c33e3d41 Mon Sep 17 00:00:00 2001
From: Sam Dowell <sdowell@google.com>
Date: Wed, 26 Jan 2022 22:30:30 +0000
Subject: [PATCH] fix: set FieldPath for SequenceNode elements

The FieldPath was not being set for nodes underneath a SequenceNode
during fieldspec's traversal. This is in part because handleSequence
uses VisitElements in contrast to a PathGetter as is done by handleMap.

The accuracy of FieldPath is more relevant now with the recently added
support of mutation trackers in filtersutil. This change fixes the
case where a mutation tracker callback is called on a SequenceNode
element and the node does not have an accurate FieldPath value.
---
 api/filters/fieldspec/fieldspec.go      |  2 +
 api/filters/fieldspec/fieldspec_test.go | 82 +++++++++++++++++++++++++
 2 files changed, 84 insertions(+)

diff --git a/api/filters/fieldspec/fieldspec.go b/api/filters/fieldspec/fieldspec.go
index 152bbb1bc..8ee052865 100644
--- a/api/filters/fieldspec/fieldspec.go
+++ b/api/filters/fieldspec/fieldspec.go
@@ -137,6 +137,8 @@ func (fltr Filter) handleMap(obj *yaml.RNode) error {
 // seq calls filter on all sequence elements
 func (fltr Filter) handleSequence(obj *yaml.RNode) error {
 	if err := obj.VisitElements(func(node *yaml.RNode) error {
+		// set an accurate FieldPath for nested elements
+		node.AppendToFieldPath(obj.FieldPath()...)
 		// recurse on each element -- re-allocating a Filter is
 		// not strictly required, but is more consistent with field
 		// and less likely to have side effects
diff --git a/api/filters/fieldspec/fieldspec_test.go b/api/filters/fieldspec/fieldspec_test.go
index bfa3a4341..fdc48a28e 100644
--- a/api/filters/fieldspec/fieldspec_test.go
+++ b/api/filters/fieldspec/fieldspec_test.go
@@ -558,3 +558,85 @@ a:
 		})
 	}
 }
+
+func TestFilter_FieldPaths(t *testing.T) {
+	testCases := map[string]struct {
+		input     string
+		fieldSpec string
+		expected  []string
+	}{
+		"fieldpath containing SequenceNode": {
+			input: `
+apiVersion: v1
+kind: Pod
+metadata:
+  name: app
+spec:
+  containers:
+  - name: store
+    image: redis:6.2.6
+  - name: server
+    image: nginx:latest
+`,
+			fieldSpec: `
+path: spec/containers[]/image
+kind: Pod
+`,
+			expected: []string{
+				"spec.containers.image",
+				"spec.containers.image",
+			},
+		},
+		"fieldpath with MappingNode": {
+			input: `
+apiVersion: v1
+kind: Pod
+metadata:
+  name: app
+spec:
+  containers:
+  - name: store
+    image: redis:6.2.6
+  - name: server
+    image: nginx:latest
+`,
+			fieldSpec: `
+path: metadata/name
+kind: Pod
+`,
+			expected: []string{
+				"metadata.name",
+			},
+		},
+	}
+	for name, tc := range testCases {
+		var fieldPaths []string
+		trackableSetter := filtersutil.TrackableSetter{}
+		trackableSetter.WithMutationTracker(func(key, value, tag string, node *yaml.RNode) {
+			fieldPaths = append(fieldPaths, strings.Join(node.FieldPath(), "."))
+		})
+		filter := fieldspec.Filter{
+			SetValue: trackableSetter.SetScalar("foo"),
+		}
+
+		t.Run(name, func(t *testing.T) {
+			err := yaml.Unmarshal([]byte(tc.fieldSpec), &filter.FieldSpec)
+			assert.NoError(t, err)
+			rw := &kio.ByteReadWriter{
+				Reader:                bytes.NewBufferString(tc.input),
+				Writer:                &bytes.Buffer{},
+				OmitReaderAnnotations: true,
+			}
+
+			// run the filter
+			err = kio.Pipeline{
+				Inputs:  []kio.Reader{rw},
+				Filters: []kio.Filter{kio.FilterAll(filter)},
+				Outputs: []kio.Writer{rw},
+			}.Execute()
+
+			assert.NoError(t, err)
+			assert.Equal(t, tc.expected, fieldPaths)
+		})
+	}
+}