From a0743e691279201232d7cd90b0394e7cc3de4935 Mon Sep 17 00:00:00 2001
From: Karl Isenberg <karlisenberg@google.com>
Date: Fri, 7 Jan 2022 18:27:57 -0800
Subject: [PATCH] fix: Clean up object.FieldPath

- Use string.Builder to be slightly more efficient
- Format invalid types with Go syntax, wrapped with brackets
- Test that invalid types don't panic
---
 pkg/object/field.go      | 22 ++++++++++++++--------
 pkg/object/field_test.go |  9 +++++++++
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/pkg/object/field.go b/pkg/object/field.go
index c4f8cca..7d530b4 100644
--- a/pkg/object/field.go
+++ b/pkg/object/field.go
@@ -6,6 +6,7 @@ package object
 import (
 	"fmt"
 	"regexp"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/util/validation/field"
 )
@@ -76,25 +77,30 @@ func NotFound(fieldPath []interface{}, value interface{}) *field.Error {
 	}
 }
 
-// Simplistic jsonpath formatter, just for NestedField errors.
+// FieldPath formats a list of KRM field keys as a JSONPath expression.
+// The only valid field keys in KRM are strings (map keys) and ints (list keys).
+// Simple strings (see isSimpleString) will be delimited with a period.
+// Complex strings will be wrapped with square brackets and double quotes.
+// Integers will be wrapped with square brackets.
+// All other types will be formatted best-effort within square brackets.
 func FieldPath(fieldPath []interface{}) string {
-	path := ""
+	var sb strings.Builder
 	for _, field := range fieldPath {
 		switch typedField := field.(type) {
 		case string:
 			if isSimpleString(typedField) {
-				path += fmt.Sprintf(".%s", typedField)
+				_, _ = fmt.Fprintf(&sb, ".%s", typedField)
 			} else {
-				path += fmt.Sprintf("[%q]", typedField)
+				_, _ = fmt.Fprintf(&sb, "[%q]", typedField)
 			}
 		case int:
-			path += fmt.Sprintf("[%d]", typedField)
+			_, _ = fmt.Fprintf(&sb, "[%d]", typedField)
 		default:
-			// invalid. try anyway...
-			path += fmt.Sprintf(".%v", typedField)
+			// invalid type. try anyway...
+			_, _ = fmt.Fprintf(&sb, "[%#v]", typedField)
 		}
 	}
-	return path
+	return sb.String()
 }
 
 var simpleStringRegex = regexp.MustCompile(`^[a-zA-Z]([a-zA-Z0-9_-]*[a-zA-Z0-9])?$`)
diff --git a/pkg/object/field_test.go b/pkg/object/field_test.go
index b5791e0..21146c9 100644
--- a/pkg/object/field_test.go
+++ b/pkg/object/field_test.go
@@ -78,6 +78,15 @@ func TestFieldPath(t *testing.T) {
 			fieldPath: []interface{}{"spec", "abc\n123"},
 			expected:  `.spec["abc\n123"]`,
 		},
+		// result from invalid input doesn't matter, as long as it doesn't panic
+		"invalid type: float": {
+			fieldPath: []interface{}{"spec", float64(-1.0)},
+			expected:  `.spec[-1]`,
+		},
+		"invalid type: struct": {
+			fieldPath: []interface{}{"spec", struct{ Field string }{Field: "value"}},
+			expected:  `.spec[struct { Field string }{Field:"value"}]`,
+		},
 	}
 
 	for name, tc := range tests {