fix(plugins): handle nil intermediate values in JSON path traversal (llamastack#136)

Simplify field mutator implementation by leveraging jsonpointer v0.21.2 fix for nil intermediate values during field mutation. Replaces complex manual traversal with direct jsonpointer calls and streamlined path creation logic. Improved error handling through jsonpointer's improved error messages.

Previously worked around: panic: reflect: call of reflect.Value.Type on zero Value.

Approved-by: rhdedgar
(cherry picked from commit b11bd68)

Author: mfleader

go.mod

@@ -4,10 +4,10 @@ go 1.23.0
 
 require (
 	github.com/go-logr/logr v1.4.1
+	github.com/go-openapi/jsonpointer v0.21.2
 	github.com/google/go-cmp v0.7.0
-	github.com/onsi/ginkgo/v2 v2.17.1
-	github.com/onsi/gomega v1.32.0
 	github.com/stretchr/testify v1.10.0
+	go.uber.org/zap v1.26.0
 	gopkg.in/yaml.v2 v2.4.0
 	gopkg.in/yaml.v3 v3.0.1
 	k8s.io/api v0.29.2
@@ -31,10 +31,8 @@ require (
 	github.com/fsnotify/fsnotify v1.7.0 // indirect
 	github.com/go-errors/errors v1.4.2 // indirect
 	github.com/go-logr/zapr v1.3.0 // indirect
-	github.com/go-openapi/jsonpointer v0.21.1 // indirect
 	github.com/go-openapi/jsonreference v0.21.0 // indirect
 	github.com/go-openapi/swag v0.23.1 // indirect
-	github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 // indirect
 	github.com/golang/protobuf v1.5.4 // indirect
@@ -52,6 +50,8 @@ require (
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
+	github.com/onsi/ginkgo/v2 v2.17.1 // indirect
+	github.com/onsi/gomega v1.32.0 // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/prometheus/client_golang v1.18.0 // indirect
@@ -61,15 +61,13 @@ require (
 	github.com/spf13/pflag v1.0.5 // indirect
 	github.com/xlab/treeprint v1.2.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
-	go.uber.org/zap v1.26.0 // indirect
 	golang.org/x/exp v0.0.0-20250506013437-ce4c2cf36ca6 // indirect
 	golang.org/x/net v0.40.0 // indirect
 	golang.org/x/oauth2 v0.30.0 // indirect
 	golang.org/x/sys v0.33.0 // indirect
 	golang.org/x/term v0.32.0 // indirect
 	golang.org/x/text v0.25.0 // indirect
 	golang.org/x/time v0.11.0 // indirect
-	golang.org/x/tools v0.33.0 // indirect
 	gomodules.xyz/jsonpatch/v2 v2.5.0 // indirect
 	google.golang.org/protobuf v1.36.6 // indirect
 	gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect

go.sum

@@ -23,8 +23,8 @@ github.com/go-logr/logr v1.4.1 h1:pKouT5E8xu9zeFC39JXRDukb6JFQPXM5p5I91188VAQ=
 github.com/go-logr/logr v1.4.1/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
 github.com/go-logr/zapr v1.3.0 h1:XGdV8XW8zdwFiwOA2Dryh1gj2KRQyOOoNmBy4EplIcQ=
 github.com/go-logr/zapr v1.3.0/go.mod h1:YKepepNBd1u/oyhd/yQmtjVXmm9uML4IXUgMOwR8/Gg=
-github.com/go-openapi/jsonpointer v0.21.1 h1:whnzv/pNXtK2FbX/W9yJfRmE2gsmkfahjMKB0fZvcic=
-github.com/go-openapi/jsonpointer v0.21.1/go.mod h1:50I1STOfbY1ycR8jGz8DaMeLCdXiI6aDteEdRNNzpdk=
+github.com/go-openapi/jsonpointer v0.21.2 h1:AqQaNADVwq/VnkCmQg6ogE+M3FOsKTytwges0JdwVuA=
+github.com/go-openapi/jsonpointer v0.21.2/go.mod h1:50I1STOfbY1ycR8jGz8DaMeLCdXiI6aDteEdRNNzpdk=
 github.com/go-openapi/jsonreference v0.21.0 h1:Rs+Y7hSXT83Jacb7kFyjn4ijOuVGSvOdF2+tg1TRrwQ=
 github.com/go-openapi/jsonreference v0.21.0/go.mod h1:LmZmgsrTkVg9LG4EaHeY8cBDslNPMo06cago5JNLkm4=
 github.com/go-openapi/swag v0.23.1 h1:lpsStH0n2ittzTnbaSloVZLuB5+fvSY/+hnagBjSNZU=
@@ -103,7 +103,6 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+
 github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
 github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
-github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
 github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=

pkg/deploy/plugins/field_mutator.go

@@ -1,6 +1,7 @@
 package plugins
 
 import (
+	"encoding/json"
 	"fmt"
 	"strings"
 
@@ -126,53 +127,60 @@ func setTargetField(res *resource.Resource, value any, mapping FieldMapping) err
 }
 
 func setWithPathCreation(data any, ptr jsonpointer.Pointer, value any) (any, error) {
-	// try setting value if the path already exists
-	if updatedData, err := ptr.Set(data, value); err == nil {
-		return updatedData, nil
+	// try direct set first
+	if result, err := ptr.Set(data, value); err == nil {
+		return result, nil
 	}
 
-	// otherwise, we need to create the path first
+	// create missing path structure
 	tokens := ptr.DecodedTokens()
 	if len(tokens) == 0 {
 		return value, nil
 	}
-	result := data
 
-	// JSON Pointer Set() fails if intermediate paths don't exist, so we must
-	// build the path incrementally from root to target, creating missing containers
-	for i := 1; i <= len(tokens)-1; i++ {
-		partialPath := "/" + strings.Join(tokens[:i], "/")
-		partialPtr, err := jsonpointer.New(partialPath)
-		if err != nil {
-			return nil, fmt.Errorf("failed to create partial pointer: %w", err)
+	result := deepCopyData(data)
+
+	// create each missing parent container
+	for i := 0; i < len(tokens)-1; i++ {
+		parentPath := "/" + strings.Join(tokens[:i+1], "/")
+		parentPtr, _ := jsonpointer.New(parentPath)
+
+		// skip if parent already exists and is not nil
+		if parent, _, err := parentPtr.Get(result); err == nil && parent != nil {
+			continue
 		}
-		// Get() is used as existence test - error means path doesn't exist and needs creation
-		_, _, err = partialPtr.Get(result)
-		if err != nil {
-			nextToken := tokens[i]
-			var newContainer any
-			// create array if next token is numeric (e.g., "/ports/0")
-			if isNumericString(nextToken) {
-				newContainer = make([]any, 0)
-			} else {
-				// create map otherwise (e.g., "/spec/strategy")
-				newContainer = make(map[string]any)
-			}
 
-			// create the missing path segment
-			result, err = partialPtr.Set(result, newContainer)
-			if err != nil {
-				return nil, fmt.Errorf("failed to create intermediate path %q: %w", partialPath, err)
-			}
+		// create container based on next token type
+		var container any
+		if isNumericString(tokens[i+1]) {
+			container = make([]any, 0)
+		} else {
+			container = make(map[string]any)
+		}
+
+		// set the container (jsonpointer handles the rest)
+		var err error
+		if result, err = parentPtr.Set(result, container); err != nil {
+			return nil, fmt.Errorf("failed to create path at %q: %w", parentPath, err)
 		}
 	}
 
-	result, err := ptr.Set(result, value)
+	// final set should now succeed
+	return ptr.Set(result, value)
+}
+
+// deepCopyData creates a deep copy of the data structure using JSON marshal/unmarshal.
+func deepCopyData(data any) any {
+	jsonBytes, err := json.Marshal(data)
 	if err != nil {
-		return nil, fmt.Errorf("failed to set final value: %w", err)
+		return data // fallback to original if copy fails
 	}
 
-	return result, nil
+	var result any
+	if err := json.Unmarshal(jsonBytes, &result); err != nil {
+		return data // fallback to original if copy fails
+	}
+	return result
 }
 
 func updateResource(res *resource.Resource, updatedData any) error {

pkg/deploy/plugins/field_mutator_test.go

@@ -293,3 +293,60 @@ func TestTransform(t *testing.T) {
 		})
 	}
 }
+
+// TestJsonPointerNilTraversalPanic reproduces the panic when jsonpointer encounters nil intermediate values.
+func TestJsonPointerNilTraversalPanic(t *testing.T) {
+	t.Run("panic when traversing through nil intermediate value", func(t *testing.T) {
+		// create a data structure with nil intermediate value that causes jsonpointer to panic
+		testData := map[string]any{
+			"apiVersion": "v1",
+			"kind":       "TestResource",
+			"metadata": map[string]any{
+				"name": "test-resource",
+			},
+			"spec": map[string]any{
+				"container": map[string]any{
+					"properties": nil, // this nil causes the panic when traversed
+				},
+			},
+		}
+
+		rf := resource.NewFactory(nil)
+		res, err := rf.FromMap(testData)
+		require.NoError(t, err)
+
+		resMap := resmap.New()
+		require.NoError(t, resMap.Append(res))
+
+		// field mapping that attempts to traverse through the nil intermediate value
+		transformer := CreateFieldMutator(FieldMutatorConfig{
+			Mappings: []FieldMapping{
+				{
+					SourceValue:       nil,
+					DefaultValue:      "test-value",
+					TargetField:       "/spec/container/properties/name", // this path goes through nil
+					TargetKind:        "TestResource",
+					CreateIfNotExists: true,
+				},
+			},
+		})
+
+		err = transformer.Transform(resMap)
+		require.NoError(t, err, "Transform should succeed after the fix")
+
+		// nil properties should be replaced with a proper map
+		updatedRes := resMap.GetByIndex(0)
+		resMap2, err := updatedRes.Map()
+		require.NoError(t, err)
+
+		spec, ok := resMap2["spec"].(map[string]any)
+		require.True(t, ok, "spec should be a map")
+		container, ok := spec["container"].(map[string]any)
+		require.True(t, ok, "container should be a map")
+		properties, ok := container["properties"].(map[string]any)
+		require.True(t, ok, "properties should be a map")
+
+		require.Equal(t, "test-value", properties["name"], "name should be set correctly")
+		require.NotNil(t, properties, "properties should no longer be nil")
+	})
+}