From 1fcf0b75ecb1bbd1f23dc0db952f972911c4b50c Mon Sep 17 00:00:00 2001 From: Philippe Scorsolini Date: Tue, 8 Oct 2024 10:11:13 +0100 Subject: [PATCH] merge full context, avoid injecting keys, allow map[string]any as context Signed-off-by: Philippe Scorsolini --- context.go | 25 +++----- context_test.go | 103 +++++-------------------------- example/context/composition.yaml | 2 + fn.go | 29 +++------ fn_test.go | 6 +- 5 files changed, 38 insertions(+), 127 deletions(-) diff --git a/context.go b/context.go index d0e8578..ee90fdc 100644 --- a/context.go +++ b/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 } diff --git a/context_test.go b/context_test.go index 14a1960..e058380 100644 --- a/context_test.go +++ b/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 != "" { diff --git a/example/context/composition.yaml b/example/context/composition.yaml index ac471bb..4b53355 100644 --- a/example/context/composition.yaml +++ b/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 diff --git a/fn.go b/fn.go index 1cafd8c..e5588e8 100644 --- a/fn.go +++ b/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. diff --git a/fn_test.go b/fn_test.go index 2bced54..05cc370 100644 --- a/fn_test.go +++ b/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" } }`,