Merge pull request crossplane-contrib#111 from jakubramut/feat/crossplane-v1

Crossplane v1-compatible build of function-extra-resources

Author: phisco

README.md

@@ -63,6 +63,30 @@ spec:
             {{- end}}
 ```
 
+## Crossplane compatibility
+
+A single build works on Crossplane v1.20+ and v2.x. It requests extra resources
+over the deprecated `extra_resources` protocol fields, which Crossplane v2 still
+resolves, rather than the `required_resources` fields that only exist on v2.
+
+`namespace` behaves differently across the two majors, because Crossplane v1 has
+no notion of a namespace on an extra resource selector:
+
+- On v2, matches are filtered server-side and only the namespace's resources are
+  sent to the function.
+- On v1, the namespace is dropped from the request. Selector matches are
+  resolved across every namespace and filtered down by the function, so the
+  result is the same but the full cluster-wide match still crosses the wire —
+  which can exceed the 4MB default gRPC receive limit. Raise it with
+  `--max-recv-message-size` if you hit it.
+- On v1, a `Reference` to a namespaced resource cannot be resolved at all, since
+  the lookup is by name only. Such a reference fails rather than resolving to
+  the wrong object.
+
+In both cases `namespace` only applies to namespaced kinds. Cluster-scoped
+objects — `EnvironmentConfig` among them — have an empty namespace, so setting
+one selects nothing.
+
 ## Local dev.
 
 ### Air

fn.go

@@ -61,13 +61,13 @@ func (f *Function) RunFunction(_ context.Context, req *fnv1.RunFunctionRequest)
 	// function-extra-resources does not know if it has requested the resources already or not.
 	//
 	// If it has and these resources are now present, proceed with verification and conversion.
-	if req.RequiredResources == nil {
+	if req.ExtraResources == nil { //nolint:staticcheck // Deprecated field used intentionally for Crossplane v1.x compatibility.
 		f.log.Debug("No extra resources present, exiting", "requirements", rsp.GetRequirements())
 		return rsp, nil
 	}
 
 	// Pull extra resources from the ExtraResources request field.
-	extraResources, err := request.GetRequiredResources(req)
+	extraResources, err := request.GetExtraResources(req) //nolint:staticcheck // Deprecated helper used intentionally for Crossplane v1.x compatibility.
 	if err != nil {
 		response.Fatal(rsp, errors.Errorf("fetching extra resources %T: %w", req, err))
 		return rsp, nil
@@ -143,11 +143,11 @@ func buildRequirements(in *v1beta1.Input, xr *resource.Composite) (*fnv1.Require
 			}
 		}
 	}
-	return &fnv1.Requirements{Resources: extraResources}, nil
+	return &fnv1.Requirements{ExtraResources: extraResources}, nil
 }
 
 // Verify Min/Max and sort extra resources by field path within a single kind.
-func verifyAndSortExtras(in *v1beta1.Input, extraResources map[string][]resource.Required, //nolint:gocyclo // TODO(reedjosh): refactor
+func verifyAndSortExtras(in *v1beta1.Input, extraResources map[string][]resource.Required, //nolint:gocyclo,gocognit // TODO(reedjosh): refactor
 ) (map[string]any, error) {
 	cleanedExtras := make(map[string]any)
 	for _, extraResource := range in.Spec.ExtraResources {
@@ -156,12 +156,29 @@ func verifyAndSortExtras(in *v1beta1.Input, extraResources map[string][]resource
 		if !ok {
 			return nil, errors.Errorf("cannot find expected extra resource %q", extraResName)
 		}
+		// When a namespace is set, keep only resources in that namespace so the
+		// min/max counting below sees the filtered set. Crossplane v2 filters
+		// server-side (a no-op here); Crossplane v1.20 ignores the namespace on
+		// label selectors and returns matches from every namespace, so this
+		// keeps cross-namespace resources out of the context.
+		if extraResource.Namespace != nil {
+			kept := make([]resource.Required, 0, len(resources))
+			for _, r := range resources {
+				if r.Resource.GetNamespace() == *extraResource.Namespace {
+					kept = append(kept, r)
+				}
+			}
+			resources = kept
+		}
 		switch extraResource.GetType() {
 		case v1beta1.ResourceSourceTypeReference:
 			if len(resources) == 0 {
 				if in.Spec.Policy.IsResolutionPolicyOptional() {
 					continue
 				}
+				if extraResource.Namespace != nil {
+					return nil, errors.Errorf("required extra resource %q not found in namespace %q: check that it exists there, that its kind is namespaced (a namespace never matches a cluster-scoped kind), and note that Crossplane before v2.0 ignores the namespace on Reference lookups", extraResName, *extraResource.Namespace)
+				}
 				return nil, errors.Errorf("Required extra resource %q not found", extraResName)
 			}
 			if len(resources) > 1 {

fn_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"strings"
 	"testing"
 
 	"github.com/crossplane/crossplane-runtime/v2/pkg/fieldpath"
@@ -160,7 +161,7 @@ func TestRunFunction(t *testing.T) {
 					Meta:    &fnv1.ResponseMeta{Tag: "hello", Ttl: durationpb.New(response.DefaultTTL)},
 					Results: []*fnv1.Result{},
 					Requirements: &fnv1.Requirements{
-						Resources: map[string]*fnv1.ResourceSelector{
+						ExtraResources: map[string]*fnv1.ResourceSelector{
 							"obj-0": {
 								ApiVersion: "apiextensions.crossplane.io/v1beta1",
 								Kind:       "EnvironmentConfig",
@@ -244,7 +245,7 @@ func TestRunFunction(t *testing.T) {
 							}`),
 						},
 					},
-					RequiredResources: map[string]*fnv1.Resources{
+					ExtraResources: map[string]*fnv1.Resources{
 						"obj-0": {
 							Items: []*fnv1.Resource{
 								{
@@ -418,7 +419,7 @@ func TestRunFunction(t *testing.T) {
 					Meta:    &fnv1.ResponseMeta{Tag: "hello", Ttl: durationpb.New(response.DefaultTTL)},
 					Results: []*fnv1.Result{},
 					Requirements: &fnv1.Requirements{
-						Resources: map[string]*fnv1.ResourceSelector{
+						ExtraResources: map[string]*fnv1.ResourceSelector{
 							"obj-0": {
 								ApiVersion: "apiextensions.crossplane.io/v1beta1",
 								Kind:       "EnvironmentConfig",
@@ -555,7 +556,7 @@ func TestRunFunction(t *testing.T) {
 							}`),
 						},
 					},
-					RequiredResources: map[string]*fnv1.Resources{
+					ExtraResources: map[string]*fnv1.Resources{
 						"environment-config-0": {
 							Items: []*fnv1.Resource{},
 						},
@@ -589,7 +590,74 @@ func TestRunFunction(t *testing.T) {
 						},
 					},
 					Requirements: &fnv1.Requirements{
-						Resources: map[string]*fnv1.ResourceSelector{
+						ExtraResources: map[string]*fnv1.ResourceSelector{
+							"obj-0": {
+								ApiVersion: "apiextensions.crossplane.io/v1beta1",
+								Kind:       "EnvironmentConfig",
+								Match: &fnv1.ResourceSelector_MatchName{
+									MatchName: "my-env-config",
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		"IgnoresRequiredResources": {
+			reason: "The Function should ignore the v2-only RequiredResources field and only read the legacy ExtraResources one.",
+			args: args{
+				req: &fnv1.RunFunctionRequest{
+					Meta: &fnv1.RequestMeta{Tag: "hello"},
+					Observed: &fnv1.State{
+						Composite: &fnv1.Resource{
+							Resource: resource.MustStructJSON(`{
+								"apiVersion": "test.crossplane.io/v1alpha1",
+								"kind": "XR",
+								"metadata": {
+									"name": "my-xr"
+								}
+							}`),
+						},
+					},
+					RequiredResources: map[string]*fnv1.Resources{
+						"obj-0": {
+							Items: []*fnv1.Resource{
+								{
+									Resource: resource.MustStructJSON(`{
+										"apiVersion": "apiextensions.crossplane.io/v1beta1",
+										"kind": "EnvironmentConfig",
+										"metadata": {
+											"name": "my-env-config"
+										}
+									}`),
+								},
+							},
+						},
+					},
+					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/v1beta1",
+									"ref": {
+										"name": "my-env-config"
+									}
+								}
+							]
+						}
+					}`),
+				},
+			},
+			want: want{
+				rsp: &fnv1.RunFunctionResponse{
+					Meta: &fnv1.ResponseMeta{Tag: "hello", Ttl: durationpb.New(response.DefaultTTL)},
+					Requirements: &fnv1.Requirements{
+						ExtraResources: map[string]*fnv1.ResourceSelector{
 							"obj-0": {
 								ApiVersion: "apiextensions.crossplane.io/v1beta1",
 								Kind:       "EnvironmentConfig",
@@ -618,7 +686,7 @@ func TestRunFunction(t *testing.T) {
 							}`),
 						},
 					},
-					RequiredResources: map[string]*fnv1.Resources{
+					ExtraResources: map[string]*fnv1.Resources{
 						"obj-0": {
 							Items: []*fnv1.Resource{
 								{
@@ -660,7 +728,7 @@ func TestRunFunction(t *testing.T) {
 					Meta:    &fnv1.ResponseMeta{Tag: "hello", Ttl: durationpb.New(response.DefaultTTL)},
 					Results: []*fnv1.Result{},
 					Requirements: &fnv1.Requirements{
-						Resources: map[string]*fnv1.ResourceSelector{
+						ExtraResources: map[string]*fnv1.ResourceSelector{
 							"obj-0": {
 								ApiVersion: "apiextensions.crossplane.io/v1beta1",
 								Kind:       "EnvironmentConfig",
@@ -734,6 +802,138 @@ func resourceWithFieldPathValue(path string, value any) resource.Required {
 	}
 }
 
+func TestVerifyAndSortExtras(t *testing.T) {
+	nsResource := func(ns, name string) resource.Required {
+		return resource.Required{
+			Resource: &unstructured.Unstructured{
+				Object: map[string]any{
+					"apiVersion": "example.org/v1",
+					"kind":       "Thing",
+					"metadata":   map[string]any{"name": name, "namespace": ns},
+				},
+			},
+		}
+	}
+
+	clusterResource := func(name string) resource.Required {
+		return resource.Required{
+			Resource: &unstructured.Unstructured{
+				Object: map[string]any{
+					"apiVersion": "example.org/v1",
+					"kind":       "Thing",
+					"metadata":   map[string]any{"name": name},
+				},
+			},
+		}
+	}
+
+	cases := map[string]struct {
+		reason    string
+		in        *v1beta1.Input
+		extra     map[string][]resource.Required
+		wantNames []string
+		wantErr   string
+	}{
+		"SelectorNamespaceFilter": {
+			reason: "A namespace on a Selector drops resources from other namespaces before counting.",
+			in: &v1beta1.Input{Spec: v1beta1.InputSpec{ExtraResources: []v1beta1.ResourceSource{{
+				Type:      v1beta1.ResourceSourceTypeSelector,
+				Into:      "objs",
+				Namespace: ptr.To("ns-a"),
+				Selector:  &v1beta1.ResourceSourceSelector{},
+			}}}},
+			extra:     map[string][]resource.Required{"objs": {nsResource("ns-a", "keep"), nsResource("ns-b", "drop")}},
+			wantNames: []string{"keep"},
+		},
+		"SelectorNamespaceFilterAppliesBeforeMinMatch": {
+			reason: "Resources dropped by the namespace filter do not count towards minMatch.",
+			in: &v1beta1.Input{Spec: v1beta1.InputSpec{ExtraResources: []v1beta1.ResourceSource{{
+				Type:      v1beta1.ResourceSourceTypeSelector,
+				Into:      "objs",
+				Namespace: ptr.To("ns-a"),
+				Selector:  &v1beta1.ResourceSourceSelector{MinMatch: ptr.To[uint64](2)},
+			}}}},
+			extra:   map[string][]resource.Required{"objs": {nsResource("ns-a", "keep"), nsResource("ns-b", "drop")}},
+			wantErr: `expected at least 2 extra resources "objs", got 1`,
+		},
+		"SelectorNamespaceFilterAppliesBeforeSortAndMaxMatch": {
+			reason: "The namespace filter runs before sorting and truncation, so maxMatch selects from the in-namespace resources only.",
+			in: &v1beta1.Input{Spec: v1beta1.InputSpec{ExtraResources: []v1beta1.ResourceSource{{
+				Type:      v1beta1.ResourceSourceTypeSelector,
+				Into:      "objs",
+				Namespace: ptr.To("ns-a"),
+				Selector: &v1beta1.ResourceSourceSelector{
+					MaxMatch:        ptr.To[uint64](2),
+					SortByFieldPath: "metadata.name",
+				},
+			}}}},
+			extra: map[string][]resource.Required{"objs": {
+				nsResource("ns-a", "c"),
+				nsResource("ns-b", "a"),
+				nsResource("ns-a", "b"),
+				nsResource("ns-a", "a"),
+			}},
+			wantNames: []string{"a", "b"},
+		},
+		"SelectorNamespaceOnClusterScopedKind": {
+			reason: "Cluster-scoped resources have an empty namespace, so setting one selects nothing rather than everything.",
+			in: &v1beta1.Input{Spec: v1beta1.InputSpec{ExtraResources: []v1beta1.ResourceSource{{
+				Type:      v1beta1.ResourceSourceTypeSelector,
+				Into:      "objs",
+				Namespace: ptr.To("ns-a"),
+				Selector:  &v1beta1.ResourceSourceSelector{MinMatch: ptr.To[uint64](1)},
+			}}}},
+			extra:   map[string][]resource.Required{"objs": {clusterResource("env-a"), clusterResource("env-b")}},
+			wantErr: `expected at least 1 extra resources "objs", got 0`,
+		},
+		"SelectorEmptyNamespaceKeepsClusterScoped": {
+			reason: "An empty namespace is not the same as an unset one: it matches the empty namespace of cluster-scoped resources.",
+			in: &v1beta1.Input{Spec: v1beta1.InputSpec{ExtraResources: []v1beta1.ResourceSource{{
+				Type:      v1beta1.ResourceSourceTypeSelector,
+				Into:      "objs",
+				Namespace: ptr.To(""),
+				Selector:  &v1beta1.ResourceSourceSelector{},
+			}}}},
+			extra:     map[string][]resource.Required{"objs": {clusterResource("cluster-scoped"), nsResource("ns-a", "namespaced")}},
+			wantNames: []string{"cluster-scoped"},
+		},
+		"ReferenceNamespaceHint": {
+			reason: "A Reference that set a namespace but resolved nothing returns a namespace hint.",
+			in: &v1beta1.Input{Spec: v1beta1.InputSpec{ExtraResources: []v1beta1.ResourceSource{{
+				Type:      v1beta1.ResourceSourceTypeReference,
+				Into:      "obj",
+				Namespace: ptr.To("ns-a"),
+				Ref:       &v1beta1.ResourceSourceReference{Name: "missing"},
+			}}}},
+			extra:   map[string][]resource.Required{"obj": {}},
+			wantErr: `required extra resource "obj" not found in namespace "ns-a"`,
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			got, err := verifyAndSortExtras(tc.in, tc.extra)
+			if tc.wantErr != "" {
+				if err == nil || !strings.Contains(err.Error(), tc.wantErr) {
+					t.Fatalf("%s\nwant error containing %q, got: %v", tc.reason, tc.wantErr, err)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("%s\nunexpected error: %v", tc.reason, err)
+			}
+			var gotNames []string
+			for _, o := range got["objs"].([]any) {
+				md := o.(map[string]any)["metadata"].(map[string]any)
+				gotNames = append(gotNames, md["name"].(string))
+			}
+			if diff := cmp.Diff(tc.wantNames, gotNames); diff != "" {
+				t.Errorf("%s\n-want +got:\n%s", tc.reason, diff)
+			}
+		})
+	}
+}
+
 func TestSortExtrasByFieldPath(t *testing.T) {
 	type args struct {
 		extras []resource.Required

input/v1beta1/resource_select.go

@@ -117,6 +117,11 @@ type ResourceSource struct {
 
 	// Namespace is the namespace in which to look for the ExtraResource.
 	// If not set, the resource is assumed to be cluster-scoped.
+	//
+	// Only applies to namespaced kinds: setting it on a cluster-scoped kind
+	// selects nothing. Crossplane v1 does not honor it server-side, so Selector
+	// matches are filtered by the function and namespaced References cannot be
+	// resolved at all.
 	// +optional
 	Namespace *string `json:"namespace,omitempty"`
 

package/crossplane.yaml

@@ -16,6 +16,6 @@ metadata:
       can be used by other functions.
 spec:
   crossplane:
-    version: ">=v2.0.0-0"
+    version: ">=v1.20.0-0"
   capabilities:
-    - required-resources
+    - composition