From cf4ac27f058b60663e6b25a74e8f2820ed212139 Mon Sep 17 00:00:00 2001 From: Philippe Scorsolini Date: Fri, 29 Mar 2024 07:41:21 +0000 Subject: [PATCH 1/2] fix: return fatal instead of erroring out Signed-off-by: Philippe Scorsolini --- fn.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fn.go b/fn.go index 0bda531..1d2b633 100644 --- a/fn.go +++ b/fn.go @@ -84,7 +84,8 @@ func (f *Function) RunFunction(_ context.Context, req *fnv1beta1.RunFunctionRequ // Sorting is required for determinism. verifiedExtras, err := verifyAndSortExtras(in, extraResources) if err != nil { - return nil, errors.Wrapf(err, "sorting and verifying results") + response.Fatal(rsp, errors.Errorf("verifying and sorting extra resources: %w", err)) + return rsp, nil } // For now cheaply convert to JSON for serializing. From 784a9dfc30dbce41008c006828b6fc524151865b Mon Sep 17 00:00:00 2001 From: Philippe Scorsolini Date: Fri, 29 Mar 2024 07:41:44 +0000 Subject: [PATCH 2/2] fix: return fatal on required envconfig missing Signed-off-by: Philippe Scorsolini --- fn.go | 7 +++-- fn_test.go | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 86 insertions(+), 6 deletions(-) diff --git a/fn.go b/fn.go index 1d2b633..edd11d1 100644 --- a/fn.go +++ b/fn.go @@ -174,8 +174,11 @@ func verifyAndSortExtras(in *v1beta1.Input, extraResources map[string][]resource } switch extraResource.GetType() { case v1beta1.ResourceSourceTypeReference: - if len(resources) == 0 && in.Spec.Policy.IsResolutionPolicyOptional() { - continue + if len(resources) == 0 { + if in.Spec.Policy.IsResolutionPolicyOptional() { + continue + } + return nil, errors.Errorf("Required extra resource %q not found", extraResName) } if len(resources) > 1 { return nil, errors.Errorf("expected exactly one extra resource %q, got %d", extraResName, len(resources)) diff --git a/fn_test.go b/fn_test.go index 0a8959b..dea92d8 100644 --- a/fn_test.go +++ b/fn_test.go @@ -6,7 +6,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "google.golang.org/protobuf/testing/protocmp" + "google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/types/known/durationpb" "google.golang.org/protobuf/types/known/structpb" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -53,7 +53,7 @@ func TestRunFunction(t *testing.T) { }, }, Input: resource.MustStructJSON(`{ - "apiVersion": "template.fn.crossplane.io/v1beta1", + "apiVersion": "extra-resources.fn.crossplane.io/v1beta1", "kind": "Input", "spec": { "extraResources": [ @@ -291,7 +291,7 @@ func TestRunFunction(t *testing.T) { }, }, Input: resource.MustStructJSON(`{ - "apiVersion": "template.fn.crossplane.io/v1beta1", + "apiVersion": "extra-resources.fn.crossplane.io/v1beta1", "kind": "Input", "spec": { "extraResources": [ @@ -489,6 +489,68 @@ func TestRunFunction(t *testing.T) { }, }, }, + "RequestEnvironmentConfigsNotFoundRequired": { + reason: "The Function should return fatal if a required EnvironmentConfig is not found", + args: args{ + req: &fnv1beta1.RunFunctionRequest{ + Meta: &fnv1beta1.RequestMeta{Tag: "hello"}, + Observed: &fnv1beta1.State{ + Composite: &fnv1beta1.Resource{ + Resource: resource.MustStructJSON(`{ + "apiVersion": "test.crossplane.io/v1alpha1", + "kind": "XR", + "metadata": { + "name": "my-xr" + } + }`), + }, + }, + ExtraResources: map[string]*fnv1beta1.Resources{ + "environment-config-0": { + Items: []*fnv1beta1.Resource{}, + }, + }, + Input: resource.MustStructJSON(`{ + "apiVersion": "extra-resources.fn.crossplane.io/v1beta1", + "kind": "Input", + "spec": { + "extraResources": [ + { + "type": "Reference", + "into": "obj-0", + "kind": "EnvironmentConfig", + "apiVersion": "apiextensions.crossplane.io/v1alpha1", + "ref": { + "name": "my-env-config" + } + } + ] + } + }`), + }, + }, + want: want{ + rsp: &fnv1beta1.RunFunctionResponse{ + Meta: &fnv1beta1.ResponseMeta{Tag: "hello", Ttl: durationpb.New(response.DefaultTTL)}, + Results: []*fnv1beta1.Result{ + { + Severity: fnv1beta1.Severity_SEVERITY_FATAL, + }, + }, + Requirements: &fnv1beta1.Requirements{ + ExtraResources: map[string]*fnv1beta1.ResourceSelector{ + "obj-0": { + ApiVersion: "apiextensions.crossplane.io/v1alpha1", + Kind: "EnvironmentConfig", + Match: &fnv1beta1.ResourceSelector_MatchName{ + MatchName: "my-env-config", + }, + }, + }, + }, + }, + }, + }, } for name, tc := range cases { @@ -496,7 +558,22 @@ func TestRunFunction(t *testing.T) { f := &Function{log: logging.NewNopLogger()} rsp, err := f.RunFunction(tc.args.ctx, tc.args.req) - if diff := cmp.Diff(tc.want.rsp, rsp, protocmp.Transform()); diff != "" { + diff := cmp.Diff(tc.want.rsp, rsp, cmpopts.AcyclicTransformer("toJsonWithoutResultMessages", func(r *fnv1beta1.RunFunctionResponse) []byte { + // We don't care about messages. + // cmptopts.IgnoreField wasn't working with protocmp.Transform + // We can't split this to another transformer as + // transformers are applied not in order but as soon as they + // match the type, which are walked from the root (RunFunctionResponse). + for _, result := range r.GetResults() { + result.Message = "" + } + out, err := protojson.Marshal(r) + if err != nil { + t.Fatalf("cannot marshal %T to JSON: %s", r, err) + } + return out + })) + if diff != "" { t.Errorf("%s\nf.RunFunction(...): -want rsp, +got rsp:\n%s", tc.reason, diff) }