diff --git a/go.mod b/go.mod index 0cf0480cd..6992ac932 100644 --- a/go.mod +++ b/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,7 +61,6 @@ 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 @@ -69,7 +68,6 @@ require ( 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 diff --git a/go.sum b/go.sum index 57564a3f3..9abf241ef 100644 --- a/go.sum +++ b/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= diff --git a/pkg/deploy/plugins/field_mutator.go b/pkg/deploy/plugins/field_mutator.go index 7fb75ad09..1737a67f3 100644 --- a/pkg/deploy/plugins/field_mutator.go +++ b/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 { diff --git a/pkg/deploy/plugins/field_mutator_test.go b/pkg/deploy/plugins/field_mutator_test.go index c6027da52..123cda832 100644 --- a/pkg/deploy/plugins/field_mutator_test.go +++ b/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") + }) +}