From 81bea7e8e63d327305ab81f1ddc4a681c28b1fc1 Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Sun, 2 Aug 2026 19:30:18 -0700 Subject: [PATCH] fix: set namespace on ResourceSelector when using matchLabels Motivation: ExtraResourcesRequirement.ToResourceSelector() returned early right after building the matchLabels selector, before the code that copies Namespace onto the resulting fnv1.ResourceSelector ran. An ExtraResources requirement combining matchLabels with a namespace therefore silently lost its namespace scoping: the selector matched labelled resources across all namespaces instead of only the requested one. Requirements using matchName were unaffected, since the namespace-setting code lived after the matchName branch. Approach: Restructure the if/else in ToResourceSelector so the namespace-setting block runs regardless of whether the selector was built from matchName or matchLabels. Validation: Added extraresources_test.go with a table-driven test, TestExtraResourcesRequirementToResourceSelector, covering matchLabels with a namespace, matchLabels without a namespace, and matchName with a namespace. Confirmed the new test fails against the pre-fix code (missing namespace in the matchLabels+namespace case) and passes after the fix. Ran: go build ./... go vet ./... go test ./... All commands passed. Report: https://github.com/crossplane-contrib/function-go-templating/issues/600 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> --- extraresources.go | 9 ++--- extraresources_test.go | 87 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 5 deletions(-) create mode 100644 extraresources_test.go diff --git a/extraresources.go b/extraresources.go index 20b89a7..b898804 100644 --- a/extraresources.go +++ b/extraresources.go @@ -48,11 +48,10 @@ func (e *ExtraResourcesRequirement) ToResourceSelector() *fnv1.ResourceSelector out.Match = &fnv1.ResourceSelector_MatchLabels{ MatchLabels: &fnv1.MatchLabels{Labels: e.MatchLabels}, } - return out - } - - out.Match = &fnv1.ResourceSelector_MatchName{ - MatchName: e.MatchName, + } else { + out.Match = &fnv1.ResourceSelector_MatchName{ + MatchName: e.MatchName, + } } if e.Namespace != "" { diff --git a/extraresources_test.go b/extraresources_test.go new file mode 100644 index 0000000..ef45fa2 --- /dev/null +++ b/extraresources_test.go @@ -0,0 +1,87 @@ +package main + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "google.golang.org/protobuf/testing/protocmp" + + fnv1 "github.com/crossplane/function-sdk-go/proto/v1" +) + +func TestExtraResourcesRequirementToResourceSelector(t *testing.T) { + ns := "test" + + cases := map[string]struct { + reason string + e ExtraResourcesRequirement + want *fnv1.ResourceSelector + }{ + "MatchLabelsWithNamespace": { + reason: "Namespace must be set on the selector when MatchLabels is used, not just MatchName.", + e: ExtraResourcesRequirement{ + APIVersion: "example.org/v1", + Kind: "CoolExtraResource", + MatchLabels: map[string]string{ + "cool": "true", + }, + Namespace: ns, + }, + want: &fnv1.ResourceSelector{ + ApiVersion: "example.org/v1", + Kind: "CoolExtraResource", + Match: &fnv1.ResourceSelector_MatchLabels{ + MatchLabels: &fnv1.MatchLabels{Labels: map[string]string{ + "cool": "true", + }}, + }, + Namespace: &ns, + }, + }, + "MatchLabelsWithoutNamespace": { + reason: "Namespace must be left unset when it is empty.", + e: ExtraResourcesRequirement{ + APIVersion: "example.org/v1", + Kind: "CoolExtraResource", + MatchLabels: map[string]string{ + "cool": "true", + }, + }, + want: &fnv1.ResourceSelector{ + ApiVersion: "example.org/v1", + Kind: "CoolExtraResource", + Match: &fnv1.ResourceSelector_MatchLabels{ + MatchLabels: &fnv1.MatchLabels{Labels: map[string]string{ + "cool": "true", + }}, + }, + }, + }, + "MatchNameWithNamespace": { + reason: "Namespace must still be set when MatchName is used.", + e: ExtraResourcesRequirement{ + APIVersion: "example.org/v1", + Kind: "CoolExtraResource", + MatchName: "cool-extra-resource", + Namespace: ns, + }, + want: &fnv1.ResourceSelector{ + ApiVersion: "example.org/v1", + Kind: "CoolExtraResource", + Match: &fnv1.ResourceSelector_MatchName{ + MatchName: "cool-extra-resource", + }, + Namespace: &ns, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got := tc.e.ToResourceSelector() + if diff := cmp.Diff(tc.want, got, protocmp.Transform()); diff != "" { + t.Errorf("\n%s\nToResourceSelector(...): -want, +got:\n%s", tc.reason, diff) + } + }) + } +}