Repository: crossplane/crossplane

Issue:
[v2] Drop `spec.crossplane.writeConnectionSecretToRef` from XRs
<!--
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 problem are you facing?
<!--
Please tell us a little about your use case - it's okay if it's hypothetical!
Leading with this context helps frame the feature request so we can ensure we
implement it sensibly.
--->

All Crossplane MRs have a `spec.writeConnectionSecretToRef` field. You set this to the name and namespace of a secret, and the MR controller will write the MR's "connection details" to that secret. These are things like usernames, passwords, URLs, etc required to connect to MRs (which are typically "infra stuff" like a database or IAM role.)

Historically we've thought of XRs as (only) composed of MRs. So XRs also had a `spec.writeConnectionSecretToRef` field. We moved that to `spec.crossplane.writeConnectionSecretToRef` in the v2 preview.

An XR's connection details are usually produced by aggregating/filtering/mutating the connection details of their composed MRs. In the v2 world functions handle this. We pass composed resource connection details (if any) to the function pipeline. The pipeline can then use them (or whatever other input) to set an XR's connection details. Crossplane then writes the connection details returned by the function pipeline to the secret specified by the XR's `spec.crossplane.writeConnectionSecretToRef` (if any).

In the Crossplane v2 world we think XRs will be composed of MRs less often. We aim to shift the abstraction XRs provide up a level - from infra stuff to an app (that might be composed of infra stuff). So for example instead of an XR representing a database that's consumed by an app, the XR could represent the app itself (including its database).

In this world many XRs won't need to expose connection details. I suspect they're starting to become an unnecessary complication.

### How could Crossplane help solve your problem?
<!--
Let us know how you think Crossplane could help with your use case. 
-->

I propose we drop `spec.crossplane.writeConnectionSecretToRef` in Crossplane v2 (i.e. in XRs of `scope: Cluster` or `scope: Namespaced`, but not `scope: LegacyCluster`).

I feel comfortable dropping this functionality (from core) for v2 XRs because in v2 you can recreate it using functions - just have your XR compose a secret with the XR's connection details in it.

Discussion:
@wasabii: >I feel comfortable dropping this functionality (from core) for v2 XRs because in v2 you can recreate it using functions - just have your XR compose a secret with the XR's connection details in it.

What sort of guarentee might exist that these values formally in the connectDetails of a MR are put anywhere at all where the XR might be able to retrieve them?

Will they still be under `observed.resources.name.connectionDetails`? I should note that as of last time I checked, this map was empty UNLESS I specified writeConnectionSecretToRef on a MR.@wasabii: Another consideration I do not see addressed here: If ability for a MR to write to a secret is removed, and we now rely ONLY on a XR to access the values and write them to it's own secret, how would users make use of the new namespaced scoped resources WITHOUT a XR?

We now have namespace scoped resources. There is nothing REQUIRING somebody to use the composition features of Crossplane at all if they don't need to. Except, if the only way to access these values becomes through an XR. Suddenly the power of the provider becomes much less useful for users who are not intending to use XRs at all.

Contradicting: 

```
The goal of this proposal is to:

Make Crossplane less opinionated and more broadly useful
Make Crossplane more intuitive and easier to use
```

I don't know if you get notifications for closed issues, @negz, so I tagged you. Sorry about that if you do.@negz: @wasabii We dropped built-in support connection details from XRs, not MRs. No changes to MR connection details, secrets, etc. It sounds like you might have read it the other way around?@wasabii: Ooh. You seem to be correct. Silly me.

Solution patch:
diff --git a/apis/apiextensions/v1/xrd_types.go b/apis/apiextensions/v1/xrd_types.go
index 20a10da61..f6757c99a 100644
--- a/apis/apiextensions/v1/xrd_types.go
+++ b/apis/apiextensions/v1/xrd_types.go
@@ -37,6 +37,7 @@ const (
 
 // CompositeResourceDefinitionSpec specifies the desired state of the definition.
 // +kubebuilder:validation:XValidation:rule="self.scope == 'LegacyCluster' || !has(self.claimNames)",message="Only LegacyCluster composite resources can offer claims"
+// +kubebuilder:validation:XValidation:rule="self.scope == 'LegacyCluster' || !has(self.connectionSecretKeys)",message="Only LegacyCluster composite resources support connection secrets"
 type CompositeResourceDefinitionSpec struct {
 	// Group specifies the API group of the defined composite resource.
 	// Composite resources are served under `/apis/<group>/...`. Must match the
@@ -76,9 +77,11 @@ type CompositeResourceDefinitionSpec struct {
 	// +kubebuilder:validation:XValidation:rule="!has(self.singular) || self.singular == self.singular.lowerAscii()",message="Singular name must be lowercase"
 	ClaimNames *extv1.CustomResourceDefinitionNames `json:"claimNames,omitempty"`
 
-	// ConnectionSecretKeys is the list of keys that will be exposed to the end
-	// user of the defined kind.
-	// If the list is empty, all keys will be published.
+	// ConnectionSecretKeys is the list of connection secret keys the
+	// defined XR can publish. If the list is empty, all keys will be
+	// published. If the list isn't empty, any connection secret keys that
+	// don't appear in the list will be filtered out. Only LegacyCluster XRs
+	// support connection secrets.
 	// +optional
 	ConnectionSecretKeys []string `json:"connectionSecretKeys,omitempty"`
 
diff --git a/apis/apiextensions/v2alpha1/xrd_types.go b/apis/apiextensions/v2alpha1/xrd_types.go
index b398b7bc8..96815bf94 100644
--- a/apis/apiextensions/v2alpha1/xrd_types.go
+++ b/apis/apiextensions/v2alpha1/xrd_types.go
@@ -36,6 +36,7 @@ const (
 
 // CompositeResourceDefinitionSpec specifies the desired state of the definition.
 // +kubebuilder:validation:XValidation:rule="!has(self.claimNames)",message="Claims aren't supported in apiextensions.crossplane.io/v2"
+// +kubebuilder:validation:XValidation:rule="!has(self.connectionSecretKeys)",message="XR connection secrets aren't supported in apiextensions.crossplane.io/v2"
 type CompositeResourceDefinitionSpec struct {
 	// Group specifies the API group of the defined composite resource.
 	// Composite resources are served under `/apis/<group>/...`. Must match the
@@ -58,12 +59,6 @@ type CompositeResourceDefinitionSpec struct {
 	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	Scope CompositeResourceScope `json:"scope,omitempty"`
 
-	// ConnectionSecretKeys is the list of keys that will be exposed to the end
-	// user of the defined kind.
-	// If the list is empty, all keys will be published.
-	// +optional
-	ConnectionSecretKeys []string `json:"connectionSecretKeys,omitempty"`
-
 	// DefaultCompositionRef refers to the Composition resource that will be used
 	// in case no composition selector is given.
 	// +optional
@@ -121,6 +116,17 @@ type CompositeResourceDefinitionSpec struct {
 	// Deprecated: Claims aren't supported in apiextensions.crossplane.io/v2.
 	// +optional
 	DefaultCompositeDeletePolicy *xpv1.CompositeDeletePolicy `json:"defaultCompositeDeletePolicy,omitempty"`
+
+	// ConnectionSecretKeys is the list of connection secret keys the
+	// defined XR can publish. If the list is empty, all keys will be
+	// published. If the list isn't empty, any connection secret keys that
+	// don't appear in the list will be filtered out. Only LegacyCluster XRs
+	// support connection secrets.
+	//
+	// Deprecated: XR connection secrets aren't supported in
+	// apiextensions.crossplane.io/v2. Compose a secret instead.
+	// +optional
+	ConnectionSecretKeys []string `json:"connectionSecretKeys,omitempty"`
 }
 
 // A CompositionReference references a Composition.
diff --git a/apis/apiextensions/v2alpha1/zz_generated.deepcopy.go b/apis/apiextensions/v2alpha1/zz_generated.deepcopy.go
index f67d2dd42..2af34140f 100644
--- a/apis/apiextensions/v2alpha1/zz_generated.deepcopy.go
+++ b/apis/apiextensions/v2alpha1/zz_generated.deepcopy.go
@@ -106,11 +106,6 @@ func (in *CompositeResourceDefinitionList) DeepCopyObject() runtime.Object {
 func (in *CompositeResourceDefinitionSpec) DeepCopyInto(out *CompositeResourceDefinitionSpec) {
 	*out = *in
 	in.Names.DeepCopyInto(&out.Names)
-	if in.ConnectionSecretKeys != nil {
-		in, out := &in.ConnectionSecretKeys, &out.ConnectionSecretKeys
-		*out = make([]string, len(*in))
-		copy(*out, *in)
-	}
 	if in.DefaultCompositionRef != nil {
 		in, out := &in.DefaultCompositionRef, &out.DefaultCompositionRef
 		*out = new(CompositionReference)
@@ -153,6 +148,11 @@ func (in *CompositeResourceDefinitionSpec) DeepCopyInto(out *CompositeResourceDe
 		*out = new(v1.CompositeDeletePolicy)
 		**out = **in
 	}
+	if in.ConnectionSecretKeys != nil {
+		in, out := &in.ConnectionSecretKeys, &out.ConnectionSecretKeys
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CompositeResourceDefinitionSpec.
diff --git a/apis/pkg/v1/package_runtime_types.go b/apis/pkg/v1/package_runtime_types.go
index 53074dac4..c9e064425 100644
--- a/apis/pkg/v1/package_runtime_types.go
+++ b/apis/pkg/v1/package_runtime_types.go
@@ -20,11 +20,6 @@ package v1
 // Only used by packages that uses a runtime, i.e. by providers and functions
 // but not for configurations.
 type PackageRuntimeSpec struct {
-	// ControllerConfigRef references a ControllerConfig resource that will be
-	// used to configure the packaged controller Deployment.
-	// Deprecated: Use RuntimeConfigReference instead.
-	// +optional
-	ControllerConfigReference *ControllerConfigReference `json:"controllerConfigRef,omitempty"`
 	// RuntimeConfigRef references a RuntimeConfig resource that will be used
 	// to configure the package runtime.
 	// +optional
@@ -48,13 +43,6 @@ type PackageRevisionRuntimeSpec struct {
 	TLSClientSecretName *string `json:"tlsClientSecretName,omitempty"`
 }
 
-// A ControllerConfigReference to a ControllerConfig resource that will be used
-// to configure the packaged controller Deployment.
-type ControllerConfigReference struct {
-	// Name of the ControllerConfig.
-	Name string `json:"name"`
-}
-
 // A RuntimeConfigReference to a runtime config resource that will be used
 // to configure the package runtime.
 type RuntimeConfigReference struct {
diff --git a/apis/pkg/v1/zz_generated.deepcopy.go b/apis/pkg/v1/zz_generated.deepcopy.go
index 4a823d917..2dc86c7df 100644
--- a/apis/pkg/v1/zz_generated.deepcopy.go
+++ b/apis/pkg/v1/zz_generated.deepcopy.go
@@ -177,21 +177,6 @@ func (in *ConfigurationStatus) DeepCopy() *ConfigurationStatus {
 	return out
 }
 
-// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
-func (in *ControllerConfigReference) DeepCopyInto(out *ControllerConfigReference) {
-	*out = *in
-}
-
-// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ControllerConfigReference.
-func (in *ControllerConfigReference) DeepCopy() *ControllerConfigReference {
-	if in == nil {
-		return nil
-	}
-	out := new(ControllerConfigReference)
-	in.DeepCopyInto(out)
-	return out
-}
-
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ControllerReference) DeepCopyInto(out *ControllerReference) {
 	*out = *in
@@ -504,11 +489,6 @@ func (in *PackageRevisionStatus) DeepCopy() *PackageRevisionStatus {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *PackageRuntimeSpec) DeepCopyInto(out *PackageRuntimeSpec) {
 	*out = *in
-	if in.ControllerConfigReference != nil {
-		in, out := &in.ControllerConfigReference, &out.ControllerConfigReference
-		*out = new(ControllerConfigReference)
-		**out = **in
-	}
 	if in.RuntimeConfigReference != nil {
 		in, out := &in.RuntimeConfigReference, &out.RuntimeConfigReference
 		*out = new(RuntimeConfigReference)
diff --git a/apis/pkg/v1beta1/zz_generated.deepcopy.go b/apis/pkg/v1beta1/zz_generated.deepcopy.go
index b4b5b5cdc..32d1cb034 100644
--- a/apis/pkg/v1beta1/zz_generated.deepcopy.go
+++ b/apis/pkg/v1beta1/zz_generated.deepcopy.go
@@ -42,21 +42,6 @@ func (in *Attestation) DeepCopy() *Attestation {
 	return out
 }
 
-// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
-func (in *ControllerConfigReference) DeepCopyInto(out *ControllerConfigReference) {
-	*out = *in
-}
-
-// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ControllerConfigReference.
-func (in *ControllerConfigReference) DeepCopy() *ControllerConfigReference {
-	if in == nil {
-		return nil
-	}
-	out := new(ControllerConfigReference)
-	in.DeepCopyInto(out)
-	return out
-}
-
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ControllerReference) DeepCopyInto(out *ControllerReference) {
 	*out = *in
@@ -941,11 +926,6 @@ func (in *PackageRevisionStatus) DeepCopy() *PackageRevisionStatus {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *PackageRuntimeSpec) DeepCopyInto(out *PackageRuntimeSpec) {
 	*out = *in
-	if in.ControllerConfigReference != nil {
-		in, out := &in.ControllerConfigReference, &out.ControllerConfigReference
-		*out = new(ControllerConfigReference)
-		**out = **in
-	}
 	if in.RuntimeConfigReference != nil {
 		in, out := &in.RuntimeConfigReference, &out.RuntimeConfigReference
 		*out = new(RuntimeConfigReference)
diff --git a/apis/pkg/v1beta1/zz_generated.package_runtime_types.go b/apis/pkg/v1beta1/zz_generated.package_runtime_types.go
index 6fc44451c..3e28b7d19 100644
--- a/apis/pkg/v1beta1/zz_generated.package_runtime_types.go
+++ b/apis/pkg/v1beta1/zz_generated.package_runtime_types.go
@@ -22,11 +22,6 @@ package v1beta1
 // Only used by packages that uses a runtime, i.e. by providers and functions
 // but not for configurations.
 type PackageRuntimeSpec struct {
-	// ControllerConfigRef references a ControllerConfig resource that will be
-	// used to configure the packaged controller Deployment.
-	// Deprecated: Use RuntimeConfigReference instead.
-	// +optional
-	ControllerConfigReference *ControllerConfigReference `json:"controllerConfigRef,omitempty"`
 	// RuntimeConfigRef references a RuntimeConfig resource that will be used
 	// to configure the package runtime.
 	// +optional
@@ -50,13 +45,6 @@ type PackageRevisionRuntimeSpec struct {
 	TLSClientSecretName *string `json:"tlsClientSecretName,omitempty"`
 }
 
-// A ControllerConfigReference to a ControllerConfig resource that will be used
-// to configure the packaged controller Deployment.
-type ControllerConfigReference struct {
-	// Name of the ControllerConfig.
-	Name string `json:"name"`
-}
-
 // A RuntimeConfigReference to a runtime config resource that will be used
 // to configure the package runtime.
 type RuntimeConfigReference struct {
diff --git a/cluster/charts/crossplane/values.yaml b/cluster/charts/crossplane/values.yaml
index 719dfd1fa..aa0f5c101 100755
--- a/cluster/charts/crossplane/values.yaml
+++ b/cluster/charts/crossplane/values.yaml
@@ -205,13 +205,3 @@ extraVolumeMountsCrossplane: {}
 
 # -- To add arbitrary Kubernetes Objects during a Helm Install
 extraObjects: []
-  # - apiVersion: pkg.crossplane.io/v1alpha1
-  #   kind: ControllerConfig
-  #   metadata:
-  #     name: aws-config
-  #     annotations:
-  #       eks.amazonaws.com/role-arn: arn:aws:iam::123456789101:role/example
-  #       helm.sh/hook: post-install
-  #   spec:
-  #     podSecurityContext:
-  #       fsGroup: 2000
diff --git a/cluster/crds/apiextensions.crossplane.io_compositeresourcedefinitions.yaml b/cluster/crds/apiextensions.crossplane.io_compositeresourcedefinitions.yaml
index 54a6dfbb7..8d567b8f4 100644
--- a/cluster/crds/apiextensions.crossplane.io_compositeresourcedefinitions.yaml
+++ b/cluster/crds/apiextensions.crossplane.io_compositeresourcedefinitions.yaml
@@ -122,9 +122,11 @@ spec:
                   rule: '!has(self.singular) || self.singular == self.singular.lowerAscii()'
               connectionSecretKeys:
                 description: |-
-                  ConnectionSecretKeys is the list of keys that will be exposed to the end
-                  user of the defined kind.
-                  If the list is empty, all keys will be published.
+                  ConnectionSecretKeys is the list of connection secret keys the
+                  defined XR can publish. If the list is empty, all keys will be
+                  published. If the list isn't empty, any connection secret keys that
+                  don't appear in the list will be filtered out. Only LegacyCluster XRs
+                  support connection secrets.
                 items:
                   type: string
                 type: array
@@ -505,6 +507,8 @@ spec:
             x-kubernetes-validations:
             - message: Only LegacyCluster composite resources can offer claims
               rule: self.scope == 'LegacyCluster' || !has(self.claimNames)
+            - message: Only LegacyCluster composite resources support connection secrets
+              rule: self.scope == 'LegacyCluster' || !has(self.connectionSecretKeys)
           status:
             description: CompositeResourceDefinitionStatus shows the observed state
               of the definition.
@@ -701,9 +705,14 @@ spec:
                 type: object
               connectionSecretKeys:
                 description: |-
-                  ConnectionSecretKeys is the list of keys that will be exposed to the end
-                  user of the defined kind.
-                  If the list is empty, all keys will be published.
+                  ConnectionSecretKeys is the list of connection secret keys the
+                  defined XR can publish. If the list is empty, all keys will be
+                  published. If the list isn't empty, any connection secret keys that
+                  don't appear in the list will be filtered out. Only LegacyCluster XRs
+                  support connection secrets.
+
+                  Deprecated: XR connection secrets aren't supported in
+                  apiextensions.crossplane.io/v2. Compose a secret instead.
                 items:
                   type: string
                 type: array
@@ -1078,6 +1087,8 @@ spec:
             x-kubernetes-validations:
             - message: Claims aren't supported in apiextensions.crossplane.io/v2
               rule: '!has(self.claimNames)'
+            - message: XR connection secrets aren't supported in apiextensions.crossplane.io/v2
+              rule: '!has(self.connectionSecretKeys)'
           status:
             description: CompositeResourceDefinitionStatus shows the observed state
               of the definition.
diff --git a/cluster/crds/pkg.crossplane.io_functionrevisions.yaml b/cluster/crds/pkg.crossplane.io_functionrevisions.yaml
index 3bc4c096f..a826dcd92 100644
--- a/cluster/crds/pkg.crossplane.io_functionrevisions.yaml
+++ b/cluster/crds/pkg.crossplane.io_functionrevisions.yaml
@@ -84,18 +84,6 @@ spec:
                   and services.
                   More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/
                 type: object
-              controllerConfigRef:
-                description: |-
-                  ControllerConfigRef references a ControllerConfig resource that will be
-                  used to configure the packaged controller Deployment.
-                  Deprecated: Use RuntimeConfigReference instead.
-                properties:
-                  name:
-                    description: Name of the ControllerConfig.
-                    type: string
-                required:
-                - name
-                type: object
               desiredState:
                 description: DesiredState of the PackageRevision. Can be either Active
                   or Inactive.
@@ -382,18 +370,6 @@ spec:
                   and services.
                   More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/
                 type: object
-              controllerConfigRef:
-                description: |-
-                  ControllerConfigRef references a ControllerConfig resource that will be
-                  used to configure the packaged controller Deployment.
-                  Deprecated: Use RuntimeConfigReference instead.
-                properties:
-                  name:
-                    description: Name of the ControllerConfig.
-                    type: string
-                required:
-                - name
-                type: object
               desiredState:
                 description: DesiredState of the PackageRevision. Can be either Active
                   or Inactive.
diff --git a/cluster/crds/pkg.crossplane.io_functions.yaml b/cluster/crds/pkg.crossplane.io_functions.yaml
index 67b74eedf..d331fefe0 100644
--- a/cluster/crds/pkg.crossplane.io_functions.yaml
+++ b/cluster/crds/pkg.crossplane.io_functions.yaml
@@ -69,18 +69,6 @@ spec:
                   and services.
                   More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/
                 type: object
-              controllerConfigRef:
-                description: |-
-                  ControllerConfigRef references a ControllerConfig resource that will be
-                  used to configure the packaged controller Deployment.
-                  Deprecated: Use RuntimeConfigReference instead.
-                properties:
-                  name:
-                    description: Name of the ControllerConfig.
-                    type: string
-                required:
-                - name
-                type: object
               ignoreCrossplaneConstraints:
                 default: false
                 description: |-
@@ -314,18 +302,6 @@ spec:
                   and services.
                   More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/
                 type: object
-              controllerConfigRef:
-                description: |-
-                  ControllerConfigRef references a ControllerConfig resource that will be
-                  used to configure the packaged controller Deployment.
-                  Deprecated: Use RuntimeConfigReference instead.
-                properties:
-                  name:
-                    description: Name of the ControllerConfig.
-                    type: string
-                required:
-                - name
-                type: object
               ignoreCrossplaneConstraints:
                 default: false
                 description: |-
diff --git a/cluster/crds/pkg.crossplane.io_providerrevisions.yaml b/cluster/crds/pkg.crossplane.io_providerrevisions.yaml
index 687d12b23..c1f5c967b 100644
--- a/cluster/crds/pkg.crossplane.io_providerrevisions.yaml
+++ b/cluster/crds/pkg.crossplane.io_providerrevisions.yaml
@@ -84,18 +84,6 @@ spec:
                   and services.
                   More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/
                 type: object
-              controllerConfigRef:
-                description: |-
-                  ControllerConfigRef references a ControllerConfig resource that will be
-                  used to configure the packaged controller Deployment.
-                  Deprecated: Use RuntimeConfigReference instead.
-                properties:
-                  name:
-                    description: Name of the ControllerConfig.
-                    type: string
-                required:
-                - name
-                type: object
               desiredState:
                 description: DesiredState of the PackageRevision. Can be either Active
                   or Inactive.
diff --git a/cluster/crds/pkg.crossplane.io_providers.yaml b/cluster/crds/pkg.crossplane.io_providers.yaml
index 69abaefd8..7416d0e2f 100644
--- a/cluster/crds/pkg.crossplane.io_providers.yaml
+++ b/cluster/crds/pkg.crossplane.io_providers.yaml
@@ -71,18 +71,6 @@ spec:
                   and services.
                   More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/
                 type: object
-              controllerConfigRef:
-                description: |-
-                  ControllerConfigRef references a ControllerConfig resource that will be
-                  used to configure the packaged controller Deployment.
-                  Deprecated: Use RuntimeConfigReference instead.
-                properties:
-                  name:
-                    description: Name of the ControllerConfig.
-                    type: string
-                required:
-                - name
-                type: object
               ignoreCrossplaneConstraints:
                 default: false
                 description: |-
diff --git a/internal/controller/apiextensions/composite/reconciler.go b/internal/controller/apiextensions/composite/reconciler.go
index 3fa6b21f6..95f3d0a32 100644
--- a/internal/controller/apiextensions/composite/reconciler.go
+++ b/internal/controller/apiextensions/composite/reconciler.go
@@ -417,10 +417,13 @@ func NewReconciler(cached client.Client, of schema.GroupVersionKind, opts ...Rec
 			CompositionSelector: NewAPILabelSelectorResolver(cached),
 			Configurator:        NewConfiguratorChain(NewAPINamingConfigurator(cached), NewAPIConfigurator(cached)),
 
-			// TODO(negz): In practice this is a filtered publisher that will
-			// never filter any keys. Is there an unfiltered variant we could
-			// use by default instead?
-			ConnectionPublisher: NewAPIFilteredSecretPublisher(cached, []string{}),
+			// Modern v2 XRs don't support connection details, but
+			// legacy v1 XRs do. Publishing is disabled by default.
+			// The definition (XRD) reconciler configures a real
+			// secret publisher for legacy XRs.
+			ConnectionPublisher: ConnectionPublisherFn(func(_ context.Context, _ xresource.ConnectionSecretOwner, _ managed.ConnectionDetails) (bool, error) {
+				return false, nil
+			}),
 		},
 
 		// We use a nop Composer by default. The real composed is passed in by
diff --git a/internal/controller/apiextensions/definition/reconciler.go b/internal/controller/apiextensions/definition/reconciler.go
index 5ed241c2d..56334ad5e 100644
--- a/internal/controller/apiextensions/definition/reconciler.go
+++ b/internal/controller/apiextensions/definition/reconciler.go
@@ -492,7 +492,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
 
 	ro := []composite.ReconcilerOption{
 		composite.WithCompositeSchema(schema),
-		composite.WithConnectionPublishers(composite.NewAPIFilteredSecretPublisher(r.engine.GetCached(), d.GetConnectionSecretKeys())),
 		composite.WithCompositionSelector(composite.NewCompositionSelectorChain(
 			composite.NewEnforcedCompositionSelector(*d, r.record),
 			composite.NewAPIDefaultCompositionSelector(r.engine.GetCached(), *meta.ReferenceTo(d, v1.CompositeResourceDefinitionGroupVersionKind), r.record),
@@ -505,6 +504,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
 		composite.WithFeatures(r.options.Features),
 	}
 
+	if schema == ucomposite.SchemaLegacy {
+		ro = append(ro,
+			composite.WithConnectionPublishers(composite.NewAPIFilteredSecretPublisher(r.engine.GetCached(), d.GetConnectionSecretKeys())),
+		)
+	}
+
 	// If realtime compositions are enabled we pass the ControllerEngine to the
 	// XR reconciler so that it can start watches for composed resources.
 	if r.options.Features.Enabled(features.EnableBetaRealtimeCompositions) {
diff --git a/internal/controller/apiextensions/definition/watch.go b/internal/controller/apiextensions/definition/watch.go
index d56c1478e..559b35489 100644
--- a/internal/controller/apiextensions/definition/watch.go
+++ b/internal/controller/apiextensions/definition/watch.go
@@ -7,6 +7,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 
 	"github.com/crossplane/crossplane-runtime/pkg/resource"
+
 	"github.com/crossplane/crossplane/internal/xcrd"
 )
 
diff --git a/internal/xcrd/schemas.go b/internal/xcrd/schemas.go
index 3e1222be9..ab75b20a3 100644
--- a/internal/xcrd/schemas.go
+++ b/internal/xcrd/schemas.go
@@ -147,18 +147,10 @@ func CompositeResourceSpecProps(s v1.CompositeResourceScope, defaultPol *xpv1.Up
 			// Controllers should replace the entire resourceRefs array.
 			XListType: ptr.To("atomic"),
 		},
-		"writeConnectionSecretToRef": {
-			Type:     "object",
-			Required: []string{"name", "namespace"},
-			Properties: map[string]extv1.JSONSchemaProps{
-				"name":      {Type: "string"},
-				"namespace": {Type: "string"},
-			},
-		},
 	}
 
-	// Namespaced XRs don't get to reference secrets or composed resources in
-	// other namespaces.
+	// Namespaced XRs don't get to reference composed resources in other
+	// namespaces.
 	if s == v1.CompositeResourceScopeNamespaced {
 		props["resourceRefs"] = extv1.JSONSchemaProps{
 			Type: "array",
@@ -176,18 +168,10 @@ func CompositeResourceSpecProps(s v1.CompositeResourceScope, defaultPol *xpv1.Up
 			// Controllers should replace the entire resourceRefs array.
 			XListType: ptr.To("atomic"),
 		}
-
-		props["writeConnectionSecretToRef"] = extv1.JSONSchemaProps{
-			Type:     "object",
-			Required: []string{"name"},
-			Properties: map[string]extv1.JSONSchemaProps{
-				"name": {Type: "string"},
-			},
-		}
 	}
 
 	// Legacy XRs have their Crossplane machinery fields directly under spec.
-	// They also support referencing a claim.
+	// They also support referencing a claim, and writing a secret.
 	if s == v1.CompositeResourceScopeLegacyCluster {
 		props["claimRef"] = extv1.JSONSchemaProps{
 			Type:     "object",
@@ -200,6 +184,15 @@ func CompositeResourceSpecProps(s v1.CompositeResourceScope, defaultPol *xpv1.Up
 			},
 		}
 
+		props["writeConnectionSecretToRef"] = extv1.JSONSchemaProps{
+			Type:     "object",
+			Required: []string{"name", "namespace"},
+			Properties: map[string]extv1.JSONSchemaProps{
+				"name":      {Type: "string"},
+				"namespace": {Type: "string"},
+			},
+		}
+
 		return props
 	}
 
diff --git a/internal/xresource/unstructured/composite/composite.go b/internal/xresource/unstructured/composite/composite.go
index e47a95d34..352854382 100644
--- a/internal/xresource/unstructured/composite/composite.go
+++ b/internal/xresource/unstructured/composite/composite.go
@@ -293,13 +293,13 @@ func (c *Unstructured) GetReference() *reference.Composite {
 
 // GetWriteConnectionSecretToReference of this composite resource.
 func (c *Unstructured) GetWriteConnectionSecretToReference() *xpv1.SecretReference {
-	path := "spec.crossplane.writeConnectionSecretToRef"
-	if c.Schema == SchemaLegacy {
-		path = "spec.writeConnectionSecretToRef"
+	// Only legacy XRs support connection secrets.
+	if c.Schema != SchemaLegacy {
+		return nil
 	}
 
 	out := &xpv1.SecretReference{}
-	if err := fieldpath.Pave(c.Object).GetValueInto(path, out); err != nil {
+	if err := fieldpath.Pave(c.Object).GetValueInto("spec.writeConnectionSecretToRef", out); err != nil {
 		return nil
 	}
 	return out
@@ -307,12 +307,12 @@ func (c *Unstructured) GetWriteConnectionSecretToReference() *xpv1.SecretReferen
 
 // SetWriteConnectionSecretToReference of this composite resource.
 func (c *Unstructured) SetWriteConnectionSecretToReference(ref *xpv1.SecretReference) {
-	path := "spec.crossplane.writeConnectionSecretToRef"
-	if c.Schema == SchemaLegacy {
-		path = "spec.writeConnectionSecretToRef"
+	// Only legacy XRs support connection secrets.
+	if c.Schema != SchemaLegacy {
+		return
 	}
 
-	_ = fieldpath.Pave(c.Object).SetValue(path, ref)
+	_ = fieldpath.Pave(c.Object).SetValue("spec.writeConnectionSecretToRef", ref)
 }
 
 // GetCondition of this composite resource.



Test patch:
diff --git a/cmd/crank/beta/trace/internal/resource/xrm/client_test.go b/cmd/crank/beta/trace/internal/resource/xrm/client_test.go
index 497e07601..4b628cd5e 100644
--- a/cmd/crank/beta/trace/internal/resource/xrm/client_test.go
+++ b/cmd/crank/beta/trace/internal/resource/xrm/client_test.go
@@ -65,12 +65,6 @@ func withXRRefs(refs ...v1.ObjectReference) xrOpt {
 	}
 }
 
-func withXRSecretRef(ref *xpv1.SecretReference) xrOpt {
-	return func(c *composite.Unstructured) {
-		c.SetWriteConnectionSecretToReference(ref)
-	}
-}
-
 func buildXR(name string, opts ...xrOpt) *unstructured.Unstructured {
 	c := composite.New()
 	c.SetName(name)
@@ -219,68 +213,6 @@ func TestGetResourceChildrenRefs(t *testing.T) {
 				},
 			},
 		},
-		"XRWithChildrenAndSecret": {
-			reason: "Should return a list of children refs for an XR.",
-			args: args{
-				witSecrets: true,
-				resource: &resource2.Resource{
-					Unstructured: *buildXR("root-xr", withXRSecretRef(&xpv1.SecretReference{
-						Name:      "secret-1",
-						Namespace: "ns-1",
-					}), withXRRefs(v1.ObjectReference{
-						APIVersion: "example.com/v1",
-						Kind:       "MR",
-						Name:       "mr-1",
-					},
-						v1.ObjectReference{
-							APIVersion: "example2.com/v1",
-							Kind:       "MR",
-							Name:       "mr-2",
-						},
-						v1.ObjectReference{
-							APIVersion: "example2.com/v1",
-							Kind:       "XR",
-							Name:       "xr-1",
-						},
-						v1.ObjectReference{
-							APIVersion: "example2.com/v1",
-							Kind:       "XRC",
-							Name:       "xrc-1",
-						},
-					)),
-				},
-			},
-			want: want{
-				refs: []v1.ObjectReference{
-					{
-						APIVersion: "v1",
-						Kind:       "Secret",
-						Namespace:  "ns-1",
-						Name:       "secret-1",
-					},
-					{
-						APIVersion: "example.com/v1",
-						Kind:       "MR",
-						Name:       "mr-1",
-					},
-					{
-						APIVersion: "example2.com/v1",
-						Kind:       "MR",
-						Name:       "mr-2",
-					},
-					{
-						APIVersion: "example2.com/v1",
-						Kind:       "XR",
-						Name:       "xr-1",
-					},
-					{
-						APIVersion: "example2.com/v1",
-						Kind:       "XRC",
-						Name:       "xrc-1",
-					},
-				},
-			},
-		},
 	}
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
diff --git a/internal/xcrd/crd_test.go b/internal/xcrd/crd_test.go
index 5945ab568..71bec7b0d 100644
--- a/internal/xcrd/crd_test.go
+++ b/internal/xcrd/crd_test.go
@@ -387,13 +387,6 @@ func TestForCompositeResource(t *testing.T) {
 															},
 															XListType: ptr.To("atomic"),
 														},
-														"writeConnectionSecretToRef": {
-															Type:     "object",
-															Required: []string{"name"},
-															Properties: map[string]extv1.JSONSchemaProps{
-																"name": {Type: "string"},
-															},
-														},
 													},
 												},
 											},
diff --git a/internal/xresource/unstructured/composite/composite_test.go b/internal/xresource/unstructured/composite/composite_test.go
index ed709f56a..91ed911b9 100644
--- a/internal/xresource/unstructured/composite/composite_test.go
+++ b/internal/xresource/unstructured/composite/composite_test.go
@@ -398,7 +398,7 @@ func TestWriteConnectionSecretToReference(t *testing.T) {
 		want *xpv1.SecretReference
 	}{
 		"NewRef": {
-			u:    New(),
+			u:    New(WithSchema(SchemaLegacy)),
 			set:  ref,
 			want: ref,
 		},
diff --git a/test/e2e/apiextensions_compositions_legacy_test.go b/test/e2e/apiextensions_compositions_legacy_test.go
index f0c54bf9e..0537e6973 100644
--- a/test/e2e/apiextensions_compositions_legacy_test.go
+++ b/test/e2e/apiextensions_compositions_legacy_test.go
@@ -20,6 +20,8 @@ import (
 	"testing"
 	"time"
 
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/e2e-framework/pkg/features"
 	"sigs.k8s.io/e2e-framework/third_party/helm"
 
@@ -53,6 +55,9 @@ func TestLegacyComposition(t *testing.T) {
 			Assess("ClaimHasPatchedField",
 				funcs.ResourcesHaveFieldValueWithin(5*time.Minute, manifests, "claim.yaml", "status.coolerField", "I'M COOLER!"),
 			).
+			Assess("ConnectionSecretCreated",
+				funcs.ResourceHasFieldValueWithin(30*time.Second, &v1.Secret{ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "basic-secret"}}, "data[super]", "c2VjcmV0Cg=="),
+			).
 			WithTeardown("DeleteClaim", funcs.AllOf(
 				funcs.DeleteResources(manifests, "claim.yaml"),
 				funcs.ResourcesDeletedWithin(2*time.Minute, manifests, "claim.yaml"),
diff --git a/test/e2e/apiextensions_compositions_test.go b/test/e2e/apiextensions_compositions_test.go
index dc3b8867d..bb5aea82b 100644
--- a/test/e2e/apiextensions_compositions_test.go
+++ b/test/e2e/apiextensions_compositions_test.go
@@ -21,7 +21,6 @@ import (
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/e2e-framework/pkg/features"
 	"sigs.k8s.io/e2e-framework/third_party/helm"
 
@@ -118,9 +117,6 @@ func TestBasicCompositionNamespaced(t *testing.T) {
 			Assess("XRHasStatusField",
 				funcs.ResourcesHaveFieldValueWithin(5*time.Minute, manifests, "xr.yaml", "status.coolerField", "I'M COOLER!"),
 			).
-			Assess("ConnectionSecretCreated",
-				funcs.ResourceHasFieldValueWithin(30*time.Second, &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "basic-secret-namespaced"}}, "data[super]", "c2VjcmV0Cg=="),
-			).
 			WithTeardown("DeleteXR", funcs.AllOf(
 				funcs.DeleteResources(manifests, "xr.yaml"),
 				funcs.ResourcesDeletedWithin(2*time.Minute, manifests, "xr.yaml"),
@@ -151,9 +147,6 @@ func TestBasicCompositionCluster(t *testing.T) {
 			Assess("XRHasStatusField",
 				funcs.ResourcesHaveFieldValueWithin(5*time.Minute, manifests, "xr.yaml", "status.coolerField", "I'M COOLER!"),
 			).
-			Assess("ConnectionSecretCreated",
-				funcs.ResourceHasFieldValueWithin(30*time.Second, &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "basic-secret-cluster"}}, "data[super]", "c2VjcmV0Cg=="),
-			).
 			WithTeardown("DeleteXR", funcs.AllOf(
 				funcs.DeleteResources(manifests, "xr.yaml"),
 				funcs.ResourcesDeletedWithin(2*time.Minute, manifests, "xr.yaml"),
diff --git a/test/e2e/manifests/apiextensions/composition/basic-cluster/setup/composition.yaml b/test/e2e/manifests/apiextensions/composition/basic-cluster/setup/composition.yaml
index 66c782482..864da5da1 100644
--- a/test/e2e/manifests/apiextensions/composition/basic-cluster/setup/composition.yaml
+++ b/test/e2e/manifests/apiextensions/composition/basic-cluster/setup/composition.yaml
@@ -22,8 +22,6 @@ spec:
             resource:
               status:
                 coolerField: "I'M COOLER!"
-            connectionDetails:
-              super: c2VjcmV0Cg==  # The string 'secret'
           resources:
             configmap:
               resource:
diff --git a/test/e2e/manifests/apiextensions/composition/basic-cluster/setup/definition.yaml b/test/e2e/manifests/apiextensions/composition/basic-cluster/setup/definition.yaml
index 9d9f1918f..75ddbc4ad 100644
--- a/test/e2e/manifests/apiextensions/composition/basic-cluster/setup/definition.yaml
+++ b/test/e2e/manifests/apiextensions/composition/basic-cluster/setup/definition.yaml
@@ -8,8 +8,6 @@ spec:
   names:
     kind: ClusterTest
     plural: clustertests
-  connectionSecretKeys:
-  - super
   versions:
   - name: v1alpha1
     served: true
diff --git a/test/e2e/manifests/apiextensions/composition/basic-cluster/xr.yaml b/test/e2e/manifests/apiextensions/composition/basic-cluster/xr.yaml
index f0f597aa0..1967f47c5 100644
--- a/test/e2e/manifests/apiextensions/composition/basic-cluster/xr.yaml
+++ b/test/e2e/manifests/apiextensions/composition/basic-cluster/xr.yaml
@@ -7,6 +7,3 @@ spec:
   crossplane:
     compositionRef:
       name: basic-composition-cluster
-    writeConnectionSecretToRef:
-      name: basic-secret-cluster
-      namespace: default
diff --git a/test/e2e/manifests/apiextensions/composition/basic-namespaced/setup/composition.yaml b/test/e2e/manifests/apiextensions/composition/basic-namespaced/setup/composition.yaml
index 2f82f4fa7..14629bda1 100644
--- a/test/e2e/manifests/apiextensions/composition/basic-namespaced/setup/composition.yaml
+++ b/test/e2e/manifests/apiextensions/composition/basic-namespaced/setup/composition.yaml
@@ -22,8 +22,6 @@ spec:
             resource:
               status:
                 coolerField: "I'M COOLER!"
-            connectionDetails:
-              super: c2VjcmV0Cg==  # The string 'secret'
           resources:
             configmap:
               resource:
diff --git a/test/e2e/manifests/apiextensions/composition/basic-namespaced/setup/definition.yaml b/test/e2e/manifests/apiextensions/composition/basic-namespaced/setup/definition.yaml
index ec59f8896..aba3e0e81 100644
--- a/test/e2e/manifests/apiextensions/composition/basic-namespaced/setup/definition.yaml
+++ b/test/e2e/manifests/apiextensions/composition/basic-namespaced/setup/definition.yaml
@@ -8,8 +8,6 @@ spec:
   names:
     kind: Test
     plural: tests
-  connectionSecretKeys:
-  - super
   versions:
   - name: v1alpha1
     served: true
diff --git a/test/e2e/manifests/apiextensions/composition/basic-namespaced/xr.yaml b/test/e2e/manifests/apiextensions/composition/basic-namespaced/xr.yaml
index 82dfa114b..1f7c779b8 100644
--- a/test/e2e/manifests/apiextensions/composition/basic-namespaced/xr.yaml
+++ b/test/e2e/manifests/apiextensions/composition/basic-namespaced/xr.yaml
@@ -8,5 +8,3 @@ spec:
   crossplane:
     compositionRef:
       name: basic-composition-namespaced
-    writeConnectionSecretToRef:
-      name: basic-secret-namespaced
diff --git a/test/e2e/manifests/apiextensions/composition/legacy/claim.yaml b/test/e2e/manifests/apiextensions/composition/legacy/claim.yaml
index 563592abf..7b29d5c59 100644
--- a/test/e2e/manifests/apiextensions/composition/legacy/claim.yaml
+++ b/test/e2e/manifests/apiextensions/composition/legacy/claim.yaml
@@ -10,3 +10,5 @@ spec:
   # This is necessary to ensure the claim's MRs are actually gone before we
   # delete the Provider - https://github.com/crossplane/crossplane/issues/4251
   compositeDeletePolicy: Foreground
+  writeConnectionSecretToRef:
+    name: basic-secret
diff --git a/test/e2e/manifests/apiextensions/composition/legacy/setup/composition.yaml b/test/e2e/manifests/apiextensions/composition/legacy/setup/composition.yaml
index 8c1463135..74eef01be 100644
--- a/test/e2e/manifests/apiextensions/composition/legacy/setup/composition.yaml
+++ b/test/e2e/manifests/apiextensions/composition/legacy/setup/composition.yaml
@@ -22,6 +22,8 @@ spec:
             resource:
               status:
                 coolerField: "I'M COOLER!"
+            connectionDetails:
+              super: c2VjcmV0Cg==  # The string 'secret'
           resources:
             # We compose a nested XR, which in turn composes an MR. This tests
             # regression of https://github.com/crossplane/crossplane/issues/4715.
@@ -45,3 +47,4 @@ spec:
   - step: detect-readiness
     functionRef:
       name: function-auto-ready-ext-functions
+  writeConnectionSecretsToNamespace: crossplane-system
diff --git a/test/e2e/manifests/apiextensions/composition/legacy/setup/definition.yaml b/test/e2e/manifests/apiextensions/composition/legacy/setup/definition.yaml
index afe55de66..e09f3133c 100644
--- a/test/e2e/manifests/apiextensions/composition/legacy/setup/definition.yaml
+++ b/test/e2e/manifests/apiextensions/composition/legacy/setup/definition.yaml
@@ -11,7 +11,7 @@ spec:
     kind: NopResource
     plural: nopresources
   connectionSecretKeys:
-  - test
+  - super
   versions:
   - name: v1alpha1
     served: true
@@ -31,4 +31,4 @@ spec:
           type: object
           properties:
             coolerField:
-              type: string
\ No newline at end of file
+              type: string