Merge pull request #1 from phisco/write-context

merge full context, avoid injecting keys, allow map[string]any as con…

context.go

@@ -4,25 +4,16 @@ import (
 	"dario.cat/mergo"
 	"github.com/crossplane/function-sdk-go/errors"
 	fnv1beta1 "github.com/crossplane/function-sdk-go/proto/v1beta1"
-	"github.com/crossplane/function-sdk-go/request"
-	"github.com/crossplane/function-sdk-go/resource"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
-// MergeContextKey merges existing Context at a key with context data val
-func (f *Function) MergeContextKey(key string, val map[string]interface{}, req *fnv1beta1.RunFunctionRequest) (*unstructured.Unstructured, error) {
-	// Check if key is already defined in the context and merge fields
-	var mergedContext *unstructured.Unstructured
-	if v, ok := request.GetContextKey(req, key); ok {
-		mergedContext = &unstructured.Unstructured{}
-		if err := resource.AsObject(v.GetStructValue(), mergedContext); err != nil {
-			return mergedContext, errors.Wrapf(err, "cannot get Composition environment from %T context key %q", req, key)
-		}
-		f.log.Debug("Loaded Existing Function Context", "context-key", key)
-		if err := mergo.Merge(&mergedContext.Object, val, mergo.WithOverride); err != nil {
-			return mergedContext, errors.Wrapf(err, "cannot merge data %T at context key %q", req, key)
-		}
+// MergeContext merges existing Context with new values provided
+func (f *Function) MergeContext(req *fnv1beta1.RunFunctionRequest, val map[string]interface{}) (map[string]interface{}, error) {
+	mergedContext := req.GetContext().AsMap()
+	if len(val) == 0 {
 		return mergedContext, nil
 	}
-	return &unstructured.Unstructured{Object: val}, nil
+	if err := mergo.Merge(&mergedContext, val, mergo.WithOverride); err != nil {
+		return mergedContext, errors.Wrapf(err, "cannot merge data %T", req)
+	}
+	return mergedContext, nil
 }

context_test.go

@@ -3,51 +3,21 @@ package main
 import (
 	"testing"
 
-	"github.com/crossplane-contrib/function-go-templating/input/v1beta1"
 	"github.com/crossplane/crossplane-runtime/pkg/logging"
 	fnv1beta1 "github.com/crossplane/function-sdk-go/proto/v1beta1"
 	"github.com/crossplane/function-sdk-go/resource"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"google.golang.org/protobuf/testing/protocmp"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
-const (
-	environmentKey                  = "apiextensions.crossplane.io/environment"
-	contextFromEnvironment          = `{"apiextensions.crossplane.io/environment":{"complex":{"a":"b","c":{"d":"e","f":"1","overWrite": "fromContext"}}}}`
-	malformedContextFromEnvironment = `{"apiextensions.crossplane.io/environment":{"badkey":,"complex":{"a":"b","c":{"d":"e","f":"1","overWrite": "fromContext"}}}}`
-	contextNew                      = `apiVersion: meta.gotemplating.fn.crossplane.io/v1alpha1
-	kind: Context
-	data:
-	  newKey:
-		hello: world
-      overWrite: fromFunction`
-
-	contextWithMergeKey = `apiVersion: meta.gotemplating.fn.crossplane.io/v1alpha1
-kind: Context
-data:
-  "apiextensions.crossplane.io/environment":
-	 update: environment
-	 nestedEnvUpdate:
-	   hello: world
-	complex:
-	  c:
-	    overWrite: fromFunction
-  "other-context-key":
-	complex: {{ index .context "apiextensions.crossplane.io/environment" "complex" | toYaml | nindent 6 }}
-  newkey:
-	hello: world`
-)
-
-func TestMergeContextKeys(t *testing.T) {
+func TestMergeContext(t *testing.T) {
 	type args struct {
-		key string
 		val map[string]interface{}
 		req *fnv1beta1.RunFunctionRequest
 	}
 	type want struct {
-		us  *unstructured.Unstructured
+		us  map[string]any
 		err error
 	}
 
@@ -59,77 +29,36 @@ func TestMergeContextKeys(t *testing.T) {
 		"NoContextAtKey": {
 			reason: "When there is no existing context data at the key to merge, return the value",
 			args: args{
-				key: "newkey",
 				req: &fnv1beta1.RunFunctionRequest{
-					Input: resource.MustStructObject(
-						&v1beta1.GoTemplate{
-							Source: v1beta1.InlineSource,
-							Inline: &v1beta1.TemplateSourceInline{Template: contextNew},
-						}),
-					Observed: &fnv1beta1.State{
-						Composite: &fnv1beta1.Resource{
-							Resource: resource.MustStructJSON(xr),
-						},
-						Resources: map[string]*fnv1beta1.Resource{
-							"cool-cd": {
-								Resource: resource.MustStructJSON(cd),
-							},
-						},
-					},
-					Desired: &fnv1beta1.State{
-						Composite: &fnv1beta1.Resource{
-							Resource: resource.MustStructJSON(xr),
-						},
-					},
+					Context: nil,
 				},
 				val: map[string]interface{}{"hello": "world"},
 			},
 			want: want{
-				us: &unstructured.Unstructured{
-					Object: map[string]interface{}{"hello": "world"},
-				},
+				us:  map[string]interface{}{"hello": "world"},
 				err: nil,
 			},
 		},
 		"SuccessfulMerge": {
 			reason: "Confirm that keys are merged with source overwriting destination",
 			args: args{
-				key: environmentKey,
 				req: &fnv1beta1.RunFunctionRequest{
-					Context: resource.MustStructJSON(contextFromEnvironment),
-					Input: resource.MustStructObject(
-						&v1beta1.GoTemplate{
-							Source: v1beta1.InlineSource,
-							Inline: &v1beta1.TemplateSourceInline{Template: contextWithMergeKey},
-						}),
-					Observed: &fnv1beta1.State{
-						Composite: &fnv1beta1.Resource{
-							Resource: resource.MustStructJSON(xr),
-						},
-						Resources: map[string]*fnv1beta1.Resource{
-							"cool-cd": {
-								Resource: resource.MustStructJSON(cd),
-							},
-						},
-					},
-					Desired: &fnv1beta1.State{
-						Composite: &fnv1beta1.Resource{
-							Resource: resource.MustStructJSON(xr),
-						},
-					},
+					Context: resource.MustStructJSON(`{"apiextensions.crossplane.io/environment":{"complex":{"a":"b","c":{"d":"e","f":"1","overWrite": "fromContext"}}}}`),
 				},
 				val: map[string]interface{}{
 					"newKey": "newValue",
-					"complex": map[string]any{
-						"c": map[string]any{
-							"overWrite": "fromFunction",
+					"apiextensions.crossplane.io/environment": map[string]any{
+						"complex": map[string]any{
+							"c": map[string]any{
+								"overWrite": "fromFunction",
+							},
 						},
 					},
 				},
 			},
 			want: want{
-				us: &unstructured.Unstructured{
-					Object: map[string]interface{}{
+				us: map[string]interface{}{
+					"apiextensions.crossplane.io/environment": map[string]any{
 						"complex": map[string]any{
 							"a": "b",
 							"c": map[string]any{
@@ -138,8 +67,8 @@ func TestMergeContextKeys(t *testing.T) {
 								"overWrite": "fromFunction",
 							},
 						},
-						"newKey": "newValue"},
-				},
+					},
+					"newKey": "newValue"},
 				err: nil,
 			},
 		},
@@ -149,10 +78,10 @@ func TestMergeContextKeys(t *testing.T) {
 			f := &Function{
 				log: logging.NewNopLogger(),
 			}
-			rsp, err := f.MergeContextKey(tc.args.key, tc.args.val, tc.args.req)
+			rsp, err := f.MergeContext(tc.args.req, tc.args.val)
 
 			if diff := cmp.Diff(tc.want.us, rsp, protocmp.Transform()); diff != "" {
-				t.Errorf("%s\nf.MergeContextKey(...): -want rsp, +got rsp:\n%s", tc.reason, diff)
+				t.Errorf("%s\nf.MergeContext(...): -want rsp, +got rsp:\n%s", tc.reason, diff)
 			}
 
 			if diff := cmp.Diff(tc.want.err, err, cmpopts.EquateErrors()); diff != "" {

example/context/composition.yaml

@@ -34,6 +34,8 @@ spec:
           data:
             # update existing EnvironmentConfig by using the "apiextensions.crossplane.io/environment" key
             "apiextensions.crossplane.io/environment":
+               kind: Environment,
+               apiVersion: internal.crossplane.io/v1alpha1,
                update: environment
                nestedEnvUpdate:
                  hello: world

fn.go

@@ -12,7 +12,6 @@ import (
 	"google.golang.org/protobuf/encoding/protojson"
 	"google.golang.org/protobuf/types/known/structpb"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/json"
 	"k8s.io/apimachinery/pkg/util/yaml"
 
@@ -197,28 +196,20 @@ func (f *Function) RunFunction(_ context.Context, req *fnv1beta1.RunFunctionRequ
 					response.Fatal(rsp, errors.Wrap(err, "cannot get Contexts from input"))
 					return rsp, nil
 				}
-				for key, data := range contextData {
-					val, ok := data.(map[string]interface{})
-					if !ok {
-						response.Fatal(rsp, errors.Errorf("error parsing Context data from %T context key %q. Must be a map.", req, key))
-						return rsp, nil
-					}
-					mergedCtx, err := f.MergeContextKey(key, val, req)
-					if err != nil {
-						response.Fatal(rsp, errors.Wrapf(err, "cannot merge Context"))
-						return rsp, nil
-					}
+				mergedCtx, err := f.MergeContext(req, contextData)
+				if err != nil {
+					response.Fatal(rsp, errors.Wrapf(err, "cannot merge Context"))
+					return rsp, nil
+				}
 
-					if mergedCtx.GroupVersionKind().Empty() {
-						mergedCtx.SetGroupVersionKind(schema.GroupVersionKind{Group: "internal.crossplane.io", Kind: "Environment", Version: "v1alpha1"})
-					}
-					v, err := resource.AsStruct(mergedCtx)
+				for key, v := range mergedCtx {
+					vv, err := structpb.NewValue(v)
 					if err != nil {
-						response.Fatal(rsp, errors.Wrap(err, "cannot convert Context to protobuf Struct well-known type"))
+						response.Fatal(rsp, errors.Wrap(err, "cannot convert value to structpb.Value"))
 						return rsp, nil
 					}
-					f.log.Debug("Updating Composition environment", "key", key, "with data", v)
-					response.SetContextKey(rsp, key, structpb.NewStructValue(v))
+					f.log.Debug("Updating Composition environment", "key", key, "data", v)
+					response.SetContextKey(rsp, key, vv)
 				}
 			case "ExtraResources":
 				// Set extra resources requirements.

fn_test.go

@@ -31,7 +31,7 @@ var (
 
 	metaResourceInvalid        = `{"apiVersion":"meta.gotemplating.fn.crossplane.io/v1alpha1","kind":"InvalidMeta"}`
 	metaResourceConDet         = `{"apiVersion":"meta.gotemplating.fn.crossplane.io/v1alpha1","kind":"CompositeConnectionDetails","data":{"key":"dmFsdWU="}}` // encoded string "value"
-	metaResourceContextInvalid = `{"apiVersion":"meta.gotemplating.fn.crossplane.io/v1alpha1","kind":"Context","data":{"new": "value"}}`
+	metaResourceContextInvalid = `{"apiVersion":"meta.gotemplating.fn.crossplane.io/v1alpha1","kind":"Context","data": 1 }`
 	metaResourceContext        = `{"apiVersion":"meta.gotemplating.fn.crossplane.io/v1alpha1","kind":"Context","data":{"apiextensions.crossplane.io/environment":{ "new":"value"}}}`
 
 	xr                    = `{"apiVersion":"example.org/v1","kind":"XR","metadata":{"name":"cool-xr"},"spec":{"count":2}}`
@@ -627,7 +627,7 @@ func TestRunFunction(t *testing.T) {
 					Results: []*fnv1beta1.Result{
 						{
 							Severity: fnv1beta1.Severity_SEVERITY_FATAL,
-							Message:  "error parsing Context data from *v1beta1.RunFunctionRequest context key \"new\". Must be a map.",
+							Message:  "cannot get Contexts from input: cannot unmarshal value from JSON: json: cannot unmarshal number into Go value of type map[string]interface {}",
 						},
 					},
 				},
@@ -665,8 +665,6 @@ func TestRunFunction(t *testing.T) {
 					Context: resource.MustStructJSON(
 						`{
 							"apiextensions.crossplane.io/environment": {
-							  "kind": "Environment",
-							  "apiVersion": "internal.crossplane.io/v1alpha1",
 							  "new": "value"
 							}
 						  }`,