Skip to content

Commit 9ef2a27

Browse files
committed
fix(plugins): handle nil intermediate values in JSON path traversal
Replace jsonpointer library calls with defensive manual traversal to prevent panic on nil intermediate values during field mutation. Adds recursive setValueAtPath function with proper array and map handling. Fixes: panic: reflect: call of reflect.Value.Type on zero Value Signed-off-by: Matthew F Leader <mleader@redhat.com>
1 parent e2cc886 commit 9ef2a27

File tree

2 files changed

+167
-31
lines changed

2 files changed

+167
-31
lines changed

pkg/deploy/plugins/field_mutator.go

Lines changed: 113 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
package plugins
22

33
import (
4+
"encoding/json"
45
"fmt"
5-
"strings"
66

77
"github.com/go-openapi/jsonpointer"
88
"sigs.k8s.io/kustomize/api/resmap"
@@ -126,53 +126,135 @@ func setTargetField(res *resource.Resource, value any, mapping FieldMapping) err
126126
}
127127

128128
func setWithPathCreation(data any, ptr jsonpointer.Pointer, value any) (any, error) {
129-
// try setting value if the path already exists
130-
if updatedData, err := ptr.Set(data, value); err == nil {
131-
return updatedData, nil
129+
// use manual path traversal to avoid jsonpointer panics on nil intermediate values
130+
tokens := ptr.DecodedTokens()
131+
if len(tokens) == 0 {
132+
return value, nil
132133
}
133134

134-
// otherwise, we need to create the path first
135-
tokens := ptr.DecodedTokens()
135+
// start with a copy of the original data to avoid mutations on errors
136+
result := deepCopyData(data)
137+
138+
// build path step by step using recursive helper
139+
return setValueAtPath(result, tokens, value)
140+
}
141+
142+
// setValueAtPath recursively sets a value at the given path, creating intermediate structures as needed
143+
func setValueAtPath(data any, tokens []string, value any) (any, error) {
136144
if len(tokens) == 0 {
137145
return value, nil
138146
}
139-
result := data
140147

141-
// JSON Pointer Set() fails if intermediate paths don't exist, so we must
142-
// build the path incrementally from root to target, creating missing containers
143-
for i := 1; i <= len(tokens)-1; i++ {
144-
partialPath := "/" + strings.Join(tokens[:i], "/")
145-
partialPtr, err := jsonpointer.New(partialPath)
146-
if err != nil {
147-
return nil, fmt.Errorf("failed to create partial pointer: %w", err)
148+
if len(tokens) == 1 {
149+
// final token - set the value
150+
token := tokens[0]
151+
if isNumericString(token) {
152+
// array index
153+
arr, ok := data.([]any)
154+
if !ok {
155+
return nil, fmt.Errorf("expected array for numeric index %q", token)
156+
}
157+
index := parseNumericToken(token)
158+
// extend array if necessary
159+
for len(arr) <= index {
160+
arr = append(arr, nil)
161+
}
162+
arr[index] = value
163+
return arr, nil
164+
} else {
165+
// map key
166+
m, ok := data.(map[string]any)
167+
if !ok {
168+
return nil, fmt.Errorf("expected map for key %q", token)
169+
}
170+
m[token] = value
171+
return m, nil
148172
}
149-
// Get() is used as existence test - error means path doesn't exist and needs creation
150-
_, _, err = partialPtr.Get(result)
151-
if err != nil {
152-
nextToken := tokens[i]
153-
var newContainer any
154-
// create array if next token is numeric (e.g., "/ports/0")
155-
if isNumericString(nextToken) {
156-
newContainer = make([]any, 0)
173+
}
174+
175+
// intermediate token - traverse or create structure
176+
token := tokens[0]
177+
if isNumericString(token) {
178+
// array index
179+
arr, ok := data.([]any)
180+
if !ok {
181+
return nil, fmt.Errorf("expected array for numeric index %q", token)
182+
}
183+
index := parseNumericToken(token)
184+
// extend array if necessary
185+
for len(arr) <= index {
186+
arr = append(arr, nil)
187+
}
188+
189+
// get or create next container
190+
next := arr[index]
191+
if next == nil {
192+
// create based on next token
193+
if isNumericString(tokens[1]) {
194+
next = make([]any, 0)
157195
} else {
158-
// create map otherwise (e.g., "/spec/strategy")
159-
newContainer = make(map[string]any)
196+
next = make(map[string]any)
160197
}
198+
}
161199

162-
// create the missing path segment
163-
result, err = partialPtr.Set(result, newContainer)
164-
if err != nil {
165-
return nil, fmt.Errorf("failed to create intermediate path %q: %w", partialPath, err)
200+
// recursively set the rest of the path
201+
updated, err := setValueAtPath(next, tokens[1:], value)
202+
if err != nil {
203+
return nil, err
204+
}
205+
arr[index] = updated
206+
return arr, nil
207+
} else {
208+
// map key
209+
m, ok := data.(map[string]any)
210+
if !ok {
211+
return nil, fmt.Errorf("expected map for key %q", token)
212+
}
213+
214+
// get or create next container
215+
next, exists := m[token]
216+
if !exists || next == nil {
217+
// create based on next token
218+
if isNumericString(tokens[1]) {
219+
next = make([]any, 0)
220+
} else {
221+
next = make(map[string]any)
166222
}
167223
}
224+
225+
// recursively set the rest of the path
226+
updated, err := setValueAtPath(next, tokens[1:], value)
227+
if err != nil {
228+
return nil, err
229+
}
230+
m[token] = updated
231+
return m, nil
232+
}
233+
}
234+
235+
// parseNumericToken converts a numeric string to int, assuming it's already validated
236+
func parseNumericToken(s string) int {
237+
result := 0
238+
for _, r := range s {
239+
result = result*10 + int(r-'0')
168240
}
241+
return result
242+
}
169243

170-
result, err := ptr.Set(result, value)
244+
// deepCopyData creates a deep copy of the data structure using JSON marshal/unmarshal
245+
func deepCopyData(data any) any {
246+
// Use JSON marshal/unmarshal for deep copy - simple and reliable
247+
jsonBytes, err := json.Marshal(data)
171248
if err != nil {
172-
return nil, fmt.Errorf("failed to set final value: %w", err)
249+
return data // fallback to original if copy fails
250+
}
251+
252+
var result any
253+
if err := json.Unmarshal(jsonBytes, &result); err != nil {
254+
return data // fallback to original if copy fails
173255
}
174256

175-
return result, nil
257+
return result
176258
}
177259

178260
func updateResource(res *resource.Resource, updatedData any) error {

pkg/deploy/plugins/field_mutator_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,3 +293,57 @@ func TestTransform(t *testing.T) {
293293
})
294294
}
295295
}
296+
297+
// TestJsonPointerNilTraversalPanic reproduces the panic when jsonpointer encounters nil intermediate values
298+
func TestJsonPointerNilTraversalPanic(t *testing.T) {
299+
t.Run("panic when traversing through nil intermediate value", func(t *testing.T) {
300+
// create a data structure with nil intermediate value that causes jsonpointer to panic
301+
testData := map[string]any{
302+
"apiVersion": "v1",
303+
"kind": "TestResource",
304+
"metadata": map[string]any{
305+
"name": "test-resource",
306+
},
307+
"spec": map[string]any{
308+
"container": map[string]any{
309+
"properties": nil, // this nil causes the panic when traversed
310+
},
311+
},
312+
}
313+
314+
rf := resource.NewFactory(nil)
315+
res, err := rf.FromMap(testData)
316+
require.NoError(t, err)
317+
318+
resMap := resmap.New()
319+
require.NoError(t, resMap.Append(res))
320+
321+
// field mapping that attempts to traverse through the nil intermediate value
322+
transformer := CreateFieldMutator(FieldMutatorConfig{
323+
Mappings: []FieldMapping{
324+
{
325+
SourceValue: nil,
326+
DefaultValue: "test-value",
327+
TargetField: "/spec/container/properties/name", // this path goes through nil
328+
TargetKind: "TestResource",
329+
CreateIfNotExists: true,
330+
},
331+
},
332+
})
333+
334+
err = transformer.Transform(resMap)
335+
require.NoError(t, err, "Transform should succeed after the fix")
336+
337+
// nil properties should be replaced with a proper map
338+
updatedRes := resMap.GetByIndex(0)
339+
resMap2, err := updatedRes.Map()
340+
require.NoError(t, err)
341+
342+
spec := resMap2["spec"].(map[string]any)
343+
container := spec["container"].(map[string]any)
344+
properties := container["properties"].(map[string]any)
345+
346+
require.Equal(t, "test-value", properties["name"], "name should be set correctly")
347+
require.NotNil(t, properties, "properties should no longer be nil")
348+
})
349+
}

0 commit comments

Comments
 (0)