Repository: crossplane/crossplane (go)

Issue:
[v2] Crossplane v2 prototype allows a namespaced XR to compose cluster scoped resources
<!--
Thank you for helping to improve Crossplane!

Please be sure to search for open issues before raising a new one. We use issues
for bug reports and feature requests. Please find us at https://slack.crossplane.io
for questions, support, and discussion.
-->

### What happened?
<!--
Please let us know what behaviour you expected and how Crossplane diverged from
that behaviour.
-->

In the Crossplane v2 prototype a namespaced XR shouldn't be allowed to compose cluster scoped resources. This is because Kubernetes doesn't allow a cluster scoped resource to be owned by (i.e. have owner references to) a namespaced resource. We recommend using cluster scoped XRs to compose cluster scoped (and/or namespaced) resources.

Currently the prototype will let you compose cluster scoped resources. It'll successfully create them, but they won't have owner references back to the XR. This means they won't be deleted when the XR is deleted. We use Kubernetes garbage collection, which relies on owner references, to delete composed resources when an XR is deleted.

### How can we reproduce it?
<!--
Help us to reproduce your bug as succinctly and precisely as possible. Artifacts
such as example manifests or a script that triggers the issue are highly
appreciated!
-->

Create a namespaced XR and use a composition that composes cluster scoped resources.

### What environment did it happen in?
Crossplane version: [22f775c](https://github.com/crossplane/crossplane/commit/22f775cb6571838f816236d326cfc0b6ee1dba1f)

<!--
Include at least the version or commit of Crossplane you were running. Consider
also including your:

* Cloud provider or hardware configuration
* Kubernetes version (use `kubectl version`)
* Kubernetes distribution (e.g. Tectonic, GKE, OpenShift)
* OS (e.g. from /etc/os-release)
* Kernel (e.g. `uname -a`)
-->


Pull request description:
Block namespaced XRs from composing cluster scoped resources
<!--
Thank you for helping to improve Crossplane! Please read the contribution docs
(linked below) if this is your first Crossplane pull request.
-->

### Description of your changes

As stated in #6361, the v2 namespace scoped XRs should not be allowed to compose cluster scoped resources. This is because Kubernetes doesn't allow a cluster scoped resource to be owned by (i.e. have owner references to) a namespaced resource. We recommend using cluster scoped XRs to compose cluster scoped (and/or namespaced) resources.

This PR adds a check for this situation. The check is after the composed resources are rendered and before they are applied. 

The check is skipped if the XR is not namespaced, or if the resource in question has a namespace set. If all goes good, the resource should be checked only once if it is namespaced, as in future reconciliations it will have the namespace set from the observation in the reconcile loop.

If a namespaced XR tries to compose a cluster scoped resource the XR will get into a `ReconcileError` condition with the corresponding message.

```yaml
Status:
  Conditions:
    Last Transition Time:  2025-07-04T18:26:01Z
    Message:               cannot compose resources: cannot apply cluster scoped composed resource "cluster-role-test" (a ClusterRole named cluster-role) for a namespaced composite resource.
    Observed Generation:   2
    Reason:                ReconcileError
    Status:                False
    Type:                  Synced
Events:
  Type     Reason             Age                From                                                             Message
  ----     ------             ----               ----                                                             -------
  Normal   SelectComposition  21s                defined/compositeresourcedefinition.apiextensions.crossplane.io  Selected composition revision: invalid-namespaced-xr-cluster-composition-9a79fe1
  Warning  ComposeResources   11s (x5 over 20s)  defined/compositeresourcedefinition.apiextensions.crossplane.io  cannot compose resources: cannot apply cluster scoped composed resource "cluster-role-test" (a ClusterRole named cluster-role) for a namespaced composite resource.

```

<!--
Briefly describe what this pull request does, and how it is covered by tests.
Be proactive - direct your reviewers' attention to anything that needs special
consideration.

We love pull requests that fix an open issue. If yours does, use the below line
to indicate which issue it fixes, for example "Fixes #500".
-->

Fixes #6361 

I have: <!--You MUST either [x] check or [ ] ~strike through~ every item.-->

- [x] Read and followed Crossplane's [contribution process].
- [x] Run `earthly +reviewable` to ensure this PR is ready for review.
- [x] Added or updated unit tests.
- [x] Added or updated e2e tests.
- ~[ ] Linked a PR or a [docs tracking issue] to [document this change].~
- ~[ ] Added `backport release-x.y` labels to auto-backport this PR.~
- ~[ ] Followed the [API promotion workflow] if this PR introduces, removes, or promotes an API.~

Need help with this checklist? See the [cheat sheet].

[contribution process]: https://github.com/crossplane/crossplane/tree/main/contributing
[docs tracking issue]: https://github.com/crossplane/docs/issues/new
[document this change]: https://docs.crossplane.io/contribute/contribute
[cheat sheet]: https://github.com/crossplane/crossplane/tree/main/contributing#checklist-cheat-sheet
[API promotion workflow]: https://github.com/crossplane/crossplane/blob/main/contributing/guide-api-promotion.md

Patch:
diff --git a/internal/controller/apiextensions/composite/composition_functions.go b/internal/controller/apiextensions/composite/composition_functions.go
index 96f187f46..334160932 100644
--- a/internal/controller/apiextensions/composite/composition_functions.go
+++ b/internal/controller/apiextensions/composite/composition_functions.go
@@ -74,20 +74,22 @@ const (
 	errGetComposed              = "cannot get composed resource"
 	errMarshalJSON              = "cannot marshal to JSON"
 
-	errFmtApplyCD                    = "cannot apply composed resource %q"
-	errFmtFetchCDConnectionDetails   = "cannot fetch connection details for composed resource %q (a %s named %s)"
-	errFmtUnmarshalPipelineStepInput = "cannot unmarshal input for Composition pipeline step %q"
-	errFmtGetCredentialsFromSecret   = "cannot get Composition pipeline step %q credential %q from Secret"
-	errFmtRunPipelineStep            = "cannot run Composition pipeline step %q"
-	errFmtControllerMismatch         = "refusing to delete composed resource %q that is controlled by %s %q"
-	errFmtCleanupLabelsCD            = "cannot cleanup composed resource labels of resource %q (a %s named %s)"
-	errFmtDeleteCD                   = "cannot delete composed resource %q (a %s named %s)"
-	errFmtUnmarshalDesiredCD         = "cannot unmarshal desired composed resource %q from RunFunctionResponse"
-	errFmtRenderMetadata             = "cannot render metadata for composed resource %q"
-	errFmtGenerateName               = "cannot generate a name for composed resource %q"
-	errFmtCDAsStruct                 = "cannot encode composed resource %q to protocol buffer Struct well-known type"
-	errFmtFatalResult                = "pipeline step %q returned a fatal result: %s"
-	errFmtInvalidName                = "cannot apply composed resource %q because it has an invalid name %q. Must be a valid RFC 1123 subdomain name."
+	errFmtApplyCD                     = "cannot apply composed resource %q"
+	errFmtFetchCDConnectionDetails    = "cannot fetch connection details for composed resource %q (a %s named %s)"
+	errFmtUnmarshalPipelineStepInput  = "cannot unmarshal input for Composition pipeline step %q"
+	errFmtGetCredentialsFromSecret    = "cannot get Composition pipeline step %q credential %q from Secret"
+	errFmtRunPipelineStep             = "cannot run Composition pipeline step %q"
+	errFmtControllerMismatch          = "refusing to delete composed resource %q that is controlled by %s %q"
+	errFmtCleanupLabelsCD             = "cannot cleanup composed resource labels of resource %q (a %s named %s)"
+	errFmtDeleteCD                    = "cannot delete composed resource %q (a %s named %s)"
+	errFmtUnmarshalDesiredCD          = "cannot unmarshal desired composed resource %q from RunFunctionResponse"
+	errFmtRenderMetadata              = "cannot render metadata for composed resource %q"
+	errFmtGenerateName                = "cannot generate a name for composed resource %q"
+	errFmtCDAsStruct                  = "cannot encode composed resource %q to protocol buffer Struct well-known type"
+	errFmtFatalResult                 = "pipeline step %q returned a fatal result: %s"
+	errFmtInvalidName                 = "cannot apply composed resource %q because it has an invalid name %q. Must be a valid RFC 1123 subdomain name."
+	errFmtGetResourceMapping          = "cannot check if composed resource %q is namespaced (a %s named %s)"
+	errFmtNamespacedXRClusterResource = "cannot apply cluster scoped composed resource %q (a %s named %s) for a namespaced composite resource."
 )
 
 // Server-side-apply field owners. We need two of these because it's possible
@@ -436,6 +438,24 @@ func (c *FunctionComposer) Compose(ctx context.Context, xr *composite.Unstructur
 			cd.SetName(or.Resource.GetName())
 		}
 
+		// If the XR is namespaced then the composed resource must be too.
+		// NOTE(lsviben): We only check resources which do not have a namespace
+		// set, although the namespace for all resources composed by a
+		// namespaced XR is set in the RenderComposedResourceMetadata
+		// step. But as in the previous step we set the namespace of observed
+		// resources, we are effectively checking only once per resource,
+		// before we create it.
+		if xr.GetNamespace() != "" && cd.GetNamespace() == "" {
+			isNs, err := c.client.IsObjectNamespaced(cd)
+			if err != nil {
+				return CompositionResult{}, errors.Wrapf(err, errFmtGetResourceMapping, name, cd.GetKind(), cd.GetName())
+			}
+
+			if !isNs {
+				return CompositionResult{}, errors.Errorf(errFmtNamespacedXRClusterResource, name, cd.GetKind(), cd.GetName())
+			}
+		}
+
 		// Set standard composed resource metadata that is derived from the XR.
 		if err := RenderComposedResourceMetadata(cd, xr, ResourceName(name)); err != nil {
 			return CompositionResult{}, errors.Wrapf(err, errFmtRenderMetadata, name)


Test patch:
diff --git a/internal/controller/apiextensions/composite/composition_functions_test.go b/internal/controller/apiextensions/composite/composition_functions_test.go
index 8911b8186..f84ede600 100644
--- a/internal/controller/apiextensions/composite/composition_functions_test.go
+++ b/internal/controller/apiextensions/composite/composition_functions_test.go
@@ -558,6 +558,193 @@ func TestFunctionCompose(t *testing.T) {
 				err: errors.Wrap(errBoom, errGarbageCollectCDs),
 			},
 		},
+		"NamespacedXRClusterCompositionError": {
+			reason: "We should return an error when a namespaced XR tries to compose cluster-scoped resources",
+			params: params{
+				c: &test.MockClient{
+					MockPatch:              test.NewMockPatchFn(nil),
+					MockStatusPatch:        test.NewMockSubResourcePatchFn(nil),
+					MockIsObjectNamespaced: test.NewMockIsObjectNamespacedFn(nil, false),
+				},
+				uc: &test.MockClient{
+					MockGet: test.NewMockGetFn(errBoom),
+				},
+				r: FunctionRunnerFn(func(_ context.Context, _ string, _ *fnv1.RunFunctionRequest) (rsp *fnv1.RunFunctionResponse, err error) {
+					d := &fnv1.State{
+						Resources: map[string]*fnv1.Resource{
+							"cluster-resource": {
+								Resource: MustStruct(map[string]any{
+									"apiVersion": "test.crossplane.io/v1",
+									"kind":       "ClusterComposed",
+									"metadata": map[string]any{
+										"name": "cluster-resource",
+									},
+								}),
+							},
+						},
+					}
+					return &fnv1.RunFunctionResponse{Desired: d}, nil
+				}),
+				o: []FunctionComposerOption{
+					WithCompositeConnectionDetailsFetcher(ConnectionDetailsFetcherFn(func(_ context.Context, _ ConnectionSecretOwner) (managed.ConnectionDetails, error) {
+						return nil, nil
+					})),
+					WithComposedResourceObserver(ComposedResourceObserverFn(func(_ context.Context, _ resource.Composite) (ComposedResourceStates, error) {
+						return nil, nil
+					})),
+					WithComposedResourceGarbageCollector(ComposedResourceGarbageCollectorFn(func(_ context.Context, _ metav1.Object, _, _ ComposedResourceStates) error {
+						return nil
+					})),
+				},
+			},
+			args: args{
+				xr: func() *composite.Unstructured {
+					xr := WithParentLabel()
+					xr.SetNamespace("test-namespace") // Make the XR namespaced
+					return xr
+				}(),
+				req: CompositionRequest{
+					Revision: &v1.CompositionRevision{
+						Spec: v1.CompositionRevisionSpec{
+							Pipeline: []v1.PipelineStep{
+								{
+									Step:        "run-cool-function",
+									FunctionRef: v1.FunctionReference{Name: "cool-function"},
+								},
+							},
+						},
+					},
+				},
+			},
+			want: want{
+				err: errors.Errorf(errFmtNamespacedXRClusterResource, "cluster-resource", "ClusterComposed", "cluster-resource"),
+			},
+		},
+		"NamespacedXRNamespacedComposition": {
+			reason: "We should succeed when a namespaced XR tries to compose namespaced-scoped resources",
+			params: params{
+				c: &test.MockClient{
+					MockPatch:              test.NewMockPatchFn(nil),
+					MockStatusPatch:        test.NewMockSubResourcePatchFn(nil),
+					MockIsObjectNamespaced: test.NewMockIsObjectNamespacedFn(nil, true),
+				},
+				uc: &test.MockClient{
+					MockGet: test.NewMockGetFn(errBoom),
+				},
+				r: FunctionRunnerFn(func(_ context.Context, _ string, _ *fnv1.RunFunctionRequest) (rsp *fnv1.RunFunctionResponse, err error) {
+					d := &fnv1.State{
+						Resources: map[string]*fnv1.Resource{
+							"ns-resource": {
+								Resource: MustStruct(map[string]any{
+									"apiVersion": "test.crossplane.io/v1",
+									"kind":       "NamespaceComposed",
+									"metadata": map[string]any{
+										"name": "ns-resource",
+									},
+								}),
+							},
+						},
+					}
+					return &fnv1.RunFunctionResponse{Desired: d}, nil
+				}),
+				o: []FunctionComposerOption{
+					WithCompositeConnectionDetailsFetcher(ConnectionDetailsFetcherFn(func(_ context.Context, _ ConnectionSecretOwner) (managed.ConnectionDetails, error) {
+						return nil, nil
+					})),
+					WithComposedResourceObserver(ComposedResourceObserverFn(func(_ context.Context, _ resource.Composite) (ComposedResourceStates, error) {
+						return nil, nil
+					})),
+					WithComposedResourceGarbageCollector(ComposedResourceGarbageCollectorFn(func(_ context.Context, _ metav1.Object, _, _ ComposedResourceStates) error {
+						return nil
+					})),
+				},
+			},
+			args: args{
+				xr: func() *composite.Unstructured {
+					xr := WithParentLabel()
+					xr.SetNamespace("test-namespace") // Make the XR namespaced
+					return xr
+				}(),
+				req: CompositionRequest{
+					Revision: &v1.CompositionRevision{
+						Spec: v1.CompositionRevisionSpec{
+							Pipeline: []v1.PipelineStep{
+								{
+									Step:        "run-cool-function",
+									FunctionRef: v1.FunctionReference{Name: "cool-function"},
+								},
+							},
+						},
+					},
+				},
+			},
+			want: want{
+				res: CompositionResult{
+					Composed: []ComposedResource{{ResourceName: "ns-resource", Ready: false, Synced: true}},
+				},
+			},
+		},
+		"ClusterXRClusterCompositionSuccess": {
+			reason: "Cluster-scoped XRs should be allowed to compose cluster-scoped resources",
+			params: params{
+				c: &test.MockClient{
+					MockGet:                test.NewMockGetFn(kerrors.NewNotFound(schema.GroupResource{Resource: "ClusterComposed"}, "")), // all names are available
+					MockPatch:              test.NewMockPatchFn(nil),
+					MockStatusPatch:        test.NewMockSubResourcePatchFn(nil),
+					MockIsObjectNamespaced: test.NewMockIsObjectNamespacedFn(errBoom, false),
+				},
+				uc: &test.MockClient{
+					MockGet: test.NewMockGetFn(errBoom),
+				},
+				r: FunctionRunnerFn(func(_ context.Context, _ string, _ *fnv1.RunFunctionRequest) (rsp *fnv1.RunFunctionResponse, err error) {
+					d := &fnv1.State{
+						Resources: map[string]*fnv1.Resource{
+							"cluster-resource": {
+								Resource: MustStruct(map[string]any{
+									"apiVersion": "test.crossplane.io/v1",
+									"kind":       "ClusterComposed",
+									"metadata": map[string]any{
+										"name": "cluster-resource",
+									},
+								}),
+							},
+						},
+					}
+					return &fnv1.RunFunctionResponse{Desired: d}, nil
+				}),
+				o: []FunctionComposerOption{
+					WithCompositeConnectionDetailsFetcher(ConnectionDetailsFetcherFn(func(_ context.Context, _ ConnectionSecretOwner) (managed.ConnectionDetails, error) {
+						return nil, nil
+					})),
+					WithComposedResourceObserver(ComposedResourceObserverFn(func(_ context.Context, _ resource.Composite) (ComposedResourceStates, error) {
+						return nil, nil
+					})),
+					WithComposedResourceGarbageCollector(ComposedResourceGarbageCollectorFn(func(_ context.Context, _ metav1.Object, _, _ ComposedResourceStates) error {
+						return nil
+					})),
+				},
+			},
+			args: args{
+				xr: WithParentLabel(), // Cluster-scoped XR (no namespace)
+				req: CompositionRequest{
+					Revision: &v1.CompositionRevision{
+						Spec: v1.CompositionRevisionSpec{
+							Pipeline: []v1.PipelineStep{
+								{
+									Step:        "run-cool-function",
+									FunctionRef: v1.FunctionReference{Name: "cool-function"},
+								},
+							},
+						},
+					},
+				},
+			},
+			want: want{
+				res: CompositionResult{
+					Composed: []ComposedResource{{ResourceName: "cluster-resource", Ready: false, Synced: true}},
+				},
+			},
+		},
 		"ApplyXRResourceReferencesError": {
 			reason: "We should return any error we encounter when applying the composite resource's resource references",
 			params: params{
diff --git a/test/e2e/apiextensions_compositions_test.go b/test/e2e/apiextensions_compositions_test.go
index 093b43d67..ab129f7a7 100644
--- a/test/e2e/apiextensions_compositions_test.go
+++ b/test/e2e/apiextensions_compositions_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package e2e
 
 import (
+	"errors"
 	"testing"
 	"time"
 
@@ -254,3 +255,32 @@ func TestCompositionValidation(t *testing.T) {
 			Feature(),
 	)
 }
+
+func TestNamespacedXRClusterComposition(t *testing.T) {
+	manifests := "test/e2e/manifests/apiextensions/composition/namespaced-xr-no-cluster-scoped-resource"
+	environment.Test(t,
+		features.NewWithDescription(t.Name(), "Tests that namespaced XRs cannot compose cluster-scoped resources, ensuring proper validation and error handling when such invalid compositions are attempted.").
+			WithLabel(LabelArea, LabelAreaAPIExtensions).
+			WithLabel(LabelSize, LabelSizeSmall).
+			WithLabel(config.LabelTestSuite, config.TestSuiteDefault).
+			WithSetup("CreatePrerequisites", funcs.AllOf(
+				funcs.ApplyResources(FieldManager, manifests, "setup/*.yaml"),
+				funcs.ResourcesCreatedWithin(30*time.Second, manifests, "setup/*.yaml"),
+				funcs.ResourcesHaveConditionWithin(1*time.Minute, manifests, "setup/definition.yaml", apiextensionsv1.WatchingComposite()),
+				funcs.ResourcesHaveConditionWithin(2*time.Minute, manifests, "setup/functions.yaml", pkgv1.Healthy(), pkgv1.Active()),
+			)).
+			Assess("CreateNamespacedXR", funcs.AllOf(
+				funcs.ApplyResources(FieldManager, manifests, "xr.yaml"),
+				funcs.ResourcesCreatedWithin(30*time.Second, manifests, "xr.yaml"),
+			)).
+			Assess("XRHasReconcileError",
+				funcs.ResourcesHaveConditionWithin(1*time.Minute, manifests, "xr.yaml", xpv1.ReconcileError(errors.New(""))),
+			).
+			WithTeardown("DeleteXR", funcs.AllOf(
+				funcs.DeleteResources(manifests, "xr.yaml"),
+				funcs.ResourcesDeletedWithin(2*time.Minute, manifests, "xr.yaml"),
+			)).
+			WithTeardown("DeletePrerequisites", funcs.ResourcesDeletedAfterListedAreGone(3*time.Minute, manifests, "setup/*.yaml", nopList)).
+			Feature(),
+	)
+}
diff --git a/test/e2e/manifests/apiextensions/composition/namespaced-xr-no-cluster-scoped-resource/setup/composition.yaml b/test/e2e/manifests/apiextensions/composition/namespaced-xr-no-cluster-scoped-resource/setup/composition.yaml
new file mode 100644
index 000000000..8b5a09676
--- /dev/null
+++ b/test/e2e/manifests/apiextensions/composition/namespaced-xr-no-cluster-scoped-resource/setup/composition.yaml
@@ -0,0 +1,33 @@
+apiVersion: apiextensions.crossplane.io/v1
+kind: Composition
+metadata:
+  name: invalid-namespaced-xr-cluster-composition
+spec:
+  compositeTypeRef:
+    apiVersion: validation.example.org/v1alpha1
+    kind: NamespacedTest
+  mode: Pipeline
+  pipeline:
+  - step: create-cluster-resource
+    functionRef:
+      name: function-dummy-ext-basic
+    input:
+      apiVersion: dummy.fn.crossplane.io/v1beta1
+      kind: Response
+      response:
+        desired:
+          resources:
+            cluster-role-test:
+              resource:
+                apiVersion: rbac.authorization.k8s.io/v1
+                kind: ClusterRole
+                metadata:
+                  name: cluster-role
+                rules:
+                  - apiGroups:
+                      - example.org
+                    resources:
+                      - clustertests
+                    verbs:
+                      - '*'
+              ready: READY_TRUE
diff --git a/test/e2e/manifests/apiextensions/composition/namespaced-xr-no-cluster-scoped-resource/setup/definition.yaml b/test/e2e/manifests/apiextensions/composition/namespaced-xr-no-cluster-scoped-resource/setup/definition.yaml
new file mode 100644
index 000000000..109eaeebf
--- /dev/null
+++ b/test/e2e/manifests/apiextensions/composition/namespaced-xr-no-cluster-scoped-resource/setup/definition.yaml
@@ -0,0 +1,25 @@
+apiVersion: apiextensions.crossplane.io/v2alpha1
+kind: CompositeResourceDefinition
+metadata:
+  name: namespacedtests.validation.example.org
+spec:
+  scope: Namespaced
+  group: validation.example.org
+  names:
+    kind: NamespacedTest
+    plural: namespacedtests
+  versions:
+  - name: v1alpha1
+    served: true
+    referenceable: true
+    schema:
+     openAPIV3Schema:
+       type: object
+       properties:
+        spec:
+          type: object
+          properties:
+            testField:
+              type: string
+          required:
+          - testField
diff --git a/test/e2e/manifests/apiextensions/composition/namespaced-xr-no-cluster-scoped-resource/setup/functions.yaml b/test/e2e/manifests/apiextensions/composition/namespaced-xr-no-cluster-scoped-resource/setup/functions.yaml
new file mode 100644
index 000000000..22deb63d6
--- /dev/null
+++ b/test/e2e/manifests/apiextensions/composition/namespaced-xr-no-cluster-scoped-resource/setup/functions.yaml
@@ -0,0 +1,7 @@
+---
+apiVersion: pkg.crossplane.io/v1beta1
+kind: Function
+metadata:
+  name: function-dummy-ext-basic
+spec:
+  package: xpkg.upbound.io/crossplane-contrib/function-dummy:v0.2.1
diff --git a/test/e2e/manifests/apiextensions/composition/namespaced-xr-no-cluster-scoped-resource/xr.yaml b/test/e2e/manifests/apiextensions/composition/namespaced-xr-no-cluster-scoped-resource/xr.yaml
new file mode 100644
index 000000000..7924be6b6
--- /dev/null
+++ b/test/e2e/manifests/apiextensions/composition/namespaced-xr-no-cluster-scoped-resource/xr.yaml
@@ -0,0 +1,10 @@
+apiVersion: validation.example.org/v1alpha1
+kind: NamespacedTest
+metadata:
+  name: test-namespaced-xr
+  namespace: crossplane-system
+spec:
+  testField: "test-value"
+  crossplane:
+    compositionRef:
+      name: invalid-namespaced-xr-cluster-composition