diff --git a/README.md b/README.md index 964581a..34488e8 100644 --- a/README.md +++ b/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 diff --git a/fn.go b/fn.go index 2b5a947..9e16f4b 100644 --- a/fn.go +++ b/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 { diff --git a/fn_test.go b/fn_test.go index dcd0007..2e53f0a 100644 --- a/fn_test.go +++ b/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 diff --git a/input/v1beta1/resource_select.go b/input/v1beta1/resource_select.go index b7d9701..3c45c22 100644 --- a/input/v1beta1/resource_select.go +++ b/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"` diff --git a/package/crossplane.yaml b/package/crossplane.yaml index 1751634..bc3e92b 100644 --- a/package/crossplane.yaml +++ b/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 diff --git a/package/input/extra-resources.fn.crossplane.io_inputs.yaml b/package/input/extra-resources.fn.crossplane.io_inputs.yaml index c5fa61f..4ee3502 100644 --- a/package/input/extra-resources.fn.crossplane.io_inputs.yaml +++ b/package/input/extra-resources.fn.crossplane.io_inputs.yaml @@ -77,6 +77,11 @@ spec: description: |- 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. type: string ref: description: |-