Skip to content

Commit e5ac5dd

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 e5ac5dd

File tree

2 files changed

+194
-36
lines changed

2 files changed

+194
-36
lines changed

pkg/deploy/plugins/field_mutator.go

Lines changed: 137 additions & 36 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,154 @@ 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-
}
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)
157-
} else {
158-
// create map otherwise (e.g., "/spec/strategy")
159-
newContainer = make(map[string]any)
160-
}
148+
if len(tokens) == 1 {
149+
return setFinalValue(data, tokens[0], value)
150+
}
161151

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)
166-
}
167-
}
152+
return setIntermediateValue(data, tokens, value)
153+
}
154+
155+
// setFinalValue sets the final value in the path (last token).
156+
func setFinalValue(data any, token string, value any) (any, error) {
157+
if isNumericString(token) {
158+
return setArrayValue(data, token, value)
168159
}
160+
return setMapValue(data, token, value)
161+
}
169162

170-
result, err := ptr.Set(result, value)
163+
// setArrayValue sets a value at an array index.
164+
func setArrayValue(data any, token string, value any) (any, error) {
165+
arr, ok := data.([]any)
166+
if !ok {
167+
return nil, fmt.Errorf("failed to access array index %q: expected array", token)
168+
}
169+
index := parseNumericToken(token)
170+
// extend array if necessary
171+
for len(arr) <= index {
172+
arr = append(arr, nil)
173+
}
174+
arr[index] = value
175+
return arr, nil
176+
}
177+
178+
// setMapValue sets a value at a map key.
179+
func setMapValue(data any, token string, value any) (any, error) {
180+
m, ok := data.(map[string]any)
181+
if !ok {
182+
return nil, fmt.Errorf("failed to access map key %q: expected map", token)
183+
}
184+
m[token] = value
185+
return m, nil
186+
}
187+
188+
// setIntermediateValue handles intermediate tokens in the path.
189+
func setIntermediateValue(data any, tokens []string, value any) (any, error) {
190+
token := tokens[0]
191+
if isNumericString(token) {
192+
return setIntermediateArrayValue(data, tokens, value)
193+
}
194+
return setIntermediateMapValue(data, tokens, value)
195+
}
196+
197+
// setIntermediateArrayValue handles array traversal for intermediate tokens.
198+
func setIntermediateArrayValue(data any, tokens []string, value any) (any, error) {
199+
arr, ok := data.([]any)
200+
if !ok {
201+
return nil, fmt.Errorf("failed to access array index %q: expected array", tokens[0])
202+
}
203+
index := parseNumericToken(tokens[0])
204+
// extend array if necessary
205+
for len(arr) <= index {
206+
arr = append(arr, nil)
207+
}
208+
209+
// get or create next container
210+
next := arr[index]
211+
if next == nil {
212+
next = createContainer(tokens[1])
213+
}
214+
215+
// recursively set the rest of the path
216+
updated, err := setValueAtPath(next, tokens[1:], value)
171217
if err != nil {
172-
return nil, fmt.Errorf("failed to set final value: %w", err)
218+
return nil, err
219+
}
220+
arr[index] = updated
221+
return arr, nil
222+
}
223+
224+
// setIntermediateMapValue handles map traversal for intermediate tokens.
225+
func setIntermediateMapValue(data any, tokens []string, value any) (any, error) {
226+
m, ok := data.(map[string]any)
227+
if !ok {
228+
return nil, fmt.Errorf("failed to access map key %q: expected map", tokens[0])
229+
}
230+
231+
// get or create next container
232+
next, exists := m[tokens[0]]
233+
if !exists || next == nil {
234+
next = createContainer(tokens[1])
235+
}
236+
237+
// recursively set the rest of the path
238+
updated, err := setValueAtPath(next, tokens[1:], value)
239+
if err != nil {
240+
return nil, err
241+
}
242+
m[tokens[0]] = updated
243+
return m, nil
244+
}
245+
246+
// createContainer creates an appropriate container based on the next token.
247+
func createContainer(nextToken string) any {
248+
if isNumericString(nextToken) {
249+
return make([]any, 0)
250+
}
251+
return make(map[string]any)
252+
}
253+
254+
// parseNumericToken converts a numeric string to int, assuming it's already validated.
255+
func parseNumericToken(s string) int {
256+
result := 0
257+
for _, r := range s {
258+
result = result*10 + int(r-'0')
259+
}
260+
return result
261+
}
262+
263+
// deepCopyData creates a deep copy of the data structure using JSON marshal/unmarshal.
264+
func deepCopyData(data any) any {
265+
// Use JSON marshal/unmarshal for deep copy - simple and reliable
266+
jsonBytes, err := json.Marshal(data)
267+
if err != nil {
268+
return data // fallback to original if copy fails
269+
}
270+
271+
var result any
272+
if err := json.Unmarshal(jsonBytes, &result); err != nil {
273+
return data // fallback to original if copy fails
173274
}
174275

175-
return result, nil
276+
return result
176277
}
177278

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

pkg/deploy/plugins/field_mutator_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,3 +293,60 @@ 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, ok := resMap2["spec"].(map[string]any)
343+
require.True(t, ok, "spec should be a map")
344+
container, ok := spec["container"].(map[string]any)
345+
require.True(t, ok, "container should be a map")
346+
properties, ok := container["properties"].(map[string]any)
347+
require.True(t, ok, "properties should be a map")
348+
349+
require.Equal(t, "test-value", properties["name"], "name should be set correctly")
350+
require.NotNil(t, properties, "properties should no longer be nil")
351+
})
352+
}

0 commit comments

Comments
 (0)