Merge pull request #105 from ravilr/json_unmarshall

round-trip the fieldPath value in FromFieldPath patches through integer-preserving JSON unmarhsal

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,17 +78,39 @@ 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
 	}
 
 	// 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.

patches_test.go

@@ -1,14 +1,14 @@
 package main
 
 import (
-	"encoding/json"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/pkg/errors"
 	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{