diff --git a/patches.go b/patches.go index b40a07d..715e6ac 100644 --- a/patches.go +++ b/patches.go @@ -7,6 +7,7 @@ import ( "github.com/pkg/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/json" "k8s.io/utils/ptr" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" @@ -77,6 +78,16 @@ func ApplyFromFieldPathPatch(p PatchInterface, from, to runtime.Object) error { return err } + // Round-trip the "from" source field value through Kubernetes JSON decoder, + // so that the json integers are unmarshalled as int64 consistent with "to"/dest value handling. + // Kubernetes JSON decoder will get us a map[string]any where number values are int64, + // but protojson and structpb will get us one where number values are float64. + // https://pkg.go.dev/sigs.k8s.io/json#UnmarshalCaseSensitivePreserveInts + v, err := toValidJSON(out) + if err != nil { + return err + } + mo, err := toMergeOption(p) if err != nil { return err @@ -84,10 +95,22 @@ func ApplyFromFieldPathPatch(p PatchInterface, from, to runtime.Object) error { // ComposedPatch all expanded fields if the ToFieldPath contains wildcards if strings.Contains(p.GetToFieldPath(), "[*]") { - return patchFieldValueToMultiple(p.GetToFieldPath(), out, to, mo) + return patchFieldValueToMultiple(p.GetToFieldPath(), v, to, mo) } - return errors.Wrap(patchFieldValueToObject(p.GetToFieldPath(), out, to, mo), "cannot patch to object") + return errors.Wrap(patchFieldValueToObject(p.GetToFieldPath(), v, to, mo), "cannot patch to object") +} + +func toValidJSON(value any) (any, error) { + var v any + j, err := json.Marshal(value) + if err != nil { + return nil, errors.Wrap(err, "cannot marshal value to JSON") + } + if err := json.Unmarshal(j, &v); err != nil { + return nil, errors.Wrap(err, "cannot unmarshal value from JSON") + } + return v, nil } // toMergeOption returns the MergeOptions from the PatchPolicy's ToFieldPathPolicy, if defined. diff --git a/patches_test.go b/patches_test.go index bc26cbe..77d012c 100644 --- a/patches_test.go +++ b/patches_test.go @@ -1,7 +1,6 @@ package main import ( - "encoding/json" "testing" "github.com/google/go-cmp/cmp" @@ -9,6 +8,7 @@ import ( extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/json" "k8s.io/utils/ptr" "github.com/crossplane/crossplane-runtime/pkg/fieldpath" @@ -195,6 +195,78 @@ func TestApplyFromFieldPathPatch(t *testing.T) { err: errors.Errorf(errFmtExpandingArrayFieldPaths, "metadata.ownerReferences[*].badField"), }, }, + "ValidToFieldPathWithWildcardsAndMergePolicy": { + reason: "When passed a wildcarded path with appendSlice policy, appends the from field slice items to each of the expanded array field, with slice deduplication", + args: args{ + p: &v1beta1.ComposedPatch{ + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("spec.parameters.allowedGroups"), + ToFieldPath: ptr.To[string]("spec.forProvider.accessRules[*].allowedGroups"), + Policy: &v1beta1.PatchPolicy{ + ToFieldPath: ptr.To(v1beta1.ToFieldPathPolicyForceMergeObjectsAppendArrays), + }, + }, + }, + from: &composite.Unstructured{ + Unstructured: unstructured.Unstructured{Object: MustObject(`{ + "apiVersion": "test.crossplane.io/v1", + "kind": "XR", + "spec": { + "parameters": { + "allowedGroups": [12345678, 7891234] + } + } + }`)}, + }, + to: &composed.Unstructured{ + Unstructured: unstructured.Unstructured{Object: MustObject(`{ + "apiVersion": "test.crossplane.io/v1", + "kind": "Composed", + "spec": { + "forProvider": { + "accessRules": [ + { + "action": "Allow", + "destination": "e1", + "allowedGroups": [12345678] + }, + { + "action": "Allow", + "destination": "e2", + "allowedGroups": [12345678] + } + ] + } + } + }`)}, + }, + }, + want: want{ + to: &composed.Unstructured{ + Unstructured: unstructured.Unstructured{Object: MustObject(`{ + "apiVersion": "test.crossplane.io/v1", + "kind": "Composed", + "spec": { + "forProvider": { + "accessRules": [ + { + "action": "Allow", + "destination": "e1", + "allowedGroups": [12345678, 7891234] + }, + { + "action": "Allow", + "destination": "e2", + "allowedGroups": [12345678, 7891234] + } + ] + } + } + }`)}, + }, + }, + }, "DefaultToFieldCompositeFieldPathPatch": { reason: "Should correctly default the ToFieldPath value if not specified.", args: args{