Merge pull request #10 from phisco/fix-reference-not-found

fix: return fatal on required extra resource missing

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.
@@ -173,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))

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,14 +489,91 @@ 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 {
 		t.Run(name, func(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)
 			}