feat(notes): add multi-cluster support for notes webhook subject lookup (#510)

## Summary

Notes can now be attached to resources that live in project control
planes. Previously, notes could only reference resources in the main
cluster, which caused errors when trying to attach notes to
project-specific resources like Domains.

## What changed

The notes webhook now searches across all connected clusters when
looking up the resource a note references. It checks the main cluster
first, then looks in any active project control planes.

## Test plan

- [x] Unit tests pass
- [ ] End-to-end tests validate notes work with project control plane
resources

Author: scotwells

cmd/milo/controller-manager/controllermanager.go

@@ -98,6 +98,7 @@ import (
 	quotav1alpha1 "go.miloapis.com/milo/pkg/apis/quota/v1alpha1"
 	resourcemanagerv1alpha1 "go.miloapis.com/milo/pkg/apis/resourcemanager/v1alpha1"
 	miloprovider "go.miloapis.com/milo/pkg/multicluster-runtime/milo"
+	milowebhook "go.miloapis.com/milo/pkg/webhook"
 	apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
 )
 
@@ -471,7 +472,11 @@ func Run(ctx context.Context, c *config.CompletedConfig, opts *Options) error {
 					MapperProvider: func(c *restclient.Config, httpClient *http.Client) (meta.RESTMapper, error) {
 						return controllerContext.RESTMapper, nil
 					},
-					WebhookServer: webhook.NewServer(webhook.Options{
+					// Wrap the webhook server with ClusterAwareServer to automatically inject
+					// project control plane context from UserInfo.Extra into the request context.
+					// This allows webhook handlers to use mccontext.ClusterFrom(ctx) to determine
+					// which project control plane the request is targeting.
+					WebhookServer: milowebhook.NewClusterAwareServer(webhook.NewServer(webhook.Options{
 						Port:    opts.ControllerRuntimeWebhookPort,
 						CertDir: opts.SecureServing.ServerCert.CertDirectory,
 						// The webhook server expects the key and cert files to be in the
@@ -481,7 +486,7 @@ func Run(ctx context.Context, c *config.CompletedConfig, opts *Options) error {
 						// key and cert file names.
 						KeyName:  strings.TrimPrefix(opts.SecureServing.ServerCert.CertKey.KeyFile, opts.SecureServing.ServerCert.CertDirectory+"/"),
 						CertName: strings.TrimPrefix(opts.SecureServing.ServerCert.CertKey.CertFile, opts.SecureServing.ServerCert.CertDirectory+"/"),
-					}),
+					})),
 				},
 			)
 			if err != nil {
@@ -553,14 +558,8 @@ func Run(ctx context.Context, c *config.CompletedConfig, opts *Options) error {
 				logger.Error(err, "Error setting up platform access rejection webhook")
 				klog.FlushAndExit(klog.ExitFlushTimeout, 1)
 			}
-			if err := notesv1alpha1webhook.SetupNoteWebhooksWithManager(ctrl); err != nil {
-				logger.Error(err, "Error setting up note webhook")
-				klog.FlushAndExit(klog.ExitFlushTimeout, 1)
-			}
-			if err := notesv1alpha1webhook.SetupClusterNoteWebhooksWithManager(ctrl); err != nil {
-				logger.Error(err, "Error setting up clusternote webhook")
-				klog.FlushAndExit(klog.ExitFlushTimeout, 1)
-			}
+			// Note webhooks are set up later after the multicluster manager is created,
+			// so they can use it for project control plane lookups.
 
 			projectCtrl := resourcemanagercontroller.ProjectController{
 				ControlPlaneClient: ctrl.GetClient(),
@@ -666,6 +665,16 @@ func Run(ctx context.Context, c *config.CompletedConfig, opts *Options) error {
 			}
 			logger.Info("Local cluster engaged successfully")
 
+			// Set up note webhooks with multicluster manager for project control plane lookups
+			if err := notesv1alpha1webhook.SetupNoteWebhooksWithManager(ctrl, mcMgr); err != nil {
+				logger.Error(err, "Error setting up note webhook")
+				klog.FlushAndExit(klog.ExitFlushTimeout, 1)
+			}
+			if err := notesv1alpha1webhook.SetupClusterNoteWebhooksWithManager(ctrl, mcMgr); err != nil {
+				logger.Error(err, "Error setting up clusternote webhook")
+				klog.FlushAndExit(klog.ExitFlushTimeout, 1)
+			}
+
 			// Start concurrently to resolve circular dependency between provider and manager
 			go func() {
 				logger.Info("Starting Datum cluster provider")

internal/webhooks/notes/v1alpha1/clusternote_webhook.go

@@ -19,18 +19,21 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+	mccontext "sigs.k8s.io/multicluster-runtime/pkg/context"
+	mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager"
 )
 
 var clusterNoteLog = logf.Log.WithName("clusternote-resource")
 
-func SetupClusterNoteWebhooksWithManager(mgr ctrl.Manager) error {
+func SetupClusterNoteWebhooksWithManager(mgr ctrl.Manager, mcMgr mcmanager.Manager) error {
 	clusterNoteLog.Info("Setting up notes.miloapis.com clusternote webhooks")
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(&notesv1alpha1.ClusterNote{}).
 		WithDefaulter(&ClusterNoteMutator{
-			Client:     mgr.GetClient(),
-			Scheme:     mgr.GetScheme(),
-			RESTMapper: mgr.GetRESTMapper(),
+			Client:         mgr.GetClient(),
+			Scheme:         mgr.GetScheme(),
+			RESTMapper:     mgr.GetRESTMapper(),
+			ClusterManager: mcMgr,
 		}).
 		WithValidator(&ClusterNoteValidator{
 			Client: mgr.GetClient(),
@@ -41,9 +44,10 @@ func SetupClusterNoteWebhooksWithManager(mgr ctrl.Manager) error {
 // +kubebuilder:webhook:path=/mutate-notes-miloapis-com-v1alpha1-clusternote,mutating=true,failurePolicy=fail,sideEffects=None,groups=notes.miloapis.com,resources=clusternotes,verbs=create,versions=v1alpha1,name=mclusternote.notes.miloapis.com,admissionReviewVersions={v1,v1beta1},serviceName=milo-controller-manager,servicePort=9443,serviceNamespace=milo-system
 
 type ClusterNoteMutator struct {
-	Client     client.Client
-	Scheme     *runtime.Scheme
-	RESTMapper meta.RESTMapper
+	Client         client.Client
+	Scheme         *runtime.Scheme
+	RESTMapper     meta.RESTMapper
+	ClusterManager mcmanager.Manager
 }
 
 var _ admission.CustomDefaulter = &ClusterNoteMutator{}
@@ -70,7 +74,6 @@ func (m *ClusterNoteMutator) Default(ctx context.Context, obj runtime.Object) er
 	}
 
 	// Set owner reference to the subject resource for automatic garbage collection
-	// This is critical for the ClusterNote to be garbage collected when the subject is deleted
 	if err := m.setSubjectOwnerReference(ctx, clusterNote); err != nil {
 		clusterNoteLog.Error(err, "Failed to set owner reference to subject", "clusternote", clusterNote.Name)
 		return errors.NewInternalError(fmt.Errorf("failed to set owner reference to subject: %w", err))
@@ -79,7 +82,8 @@ func (m *ClusterNoteMutator) Default(ctx context.Context, obj runtime.Object) er
 	return nil
 }
 
-// setSubjectOwnerReference sets the owner reference to the subject resource if it's cluster-scoped
+// setSubjectOwnerReference sets the owner reference to the subject resource if it's cluster-scoped.
+// The cluster context is expected to be injected by the ClusterAwareServer wrapper.
 func (m *ClusterNoteMutator) setSubjectOwnerReference(ctx context.Context, clusterNote *notesv1alpha1.ClusterNote) error {
 	// ClusterNote can only have owner references to other cluster-scoped resources
 	if clusterNote.Spec.SubjectRef.Namespace != "" {
@@ -97,24 +101,34 @@ func (m *ClusterNoteMutator) setSubjectOwnerReference(ctx context.Context, clust
 		return fmt.Errorf("failed to get REST mapping for %s: %w", groupKind, err)
 	}
 
+	key := types.NamespacedName{
+		Name: clusterNote.Spec.SubjectRef.Name,
+	}
+
+	// Determine which client to use based on cluster context (injected by ClusterAwareServer)
+	subjectClient := m.Client
+
+	if m.ClusterManager != nil {
+		if clusterName, ok := mccontext.ClusterFrom(ctx); ok && clusterName != "" {
+			cluster, err := m.ClusterManager.GetCluster(ctx, clusterName)
+			if err != nil {
+				return fmt.Errorf("failed to get project control plane %s: %w", clusterName, err)
+			}
+			subjectClient = cluster.GetClient()
+			clusterNoteLog.V(1).Info("Using project control plane client", "cluster", clusterName)
+		}
+	}
+
 	subject := &unstructured.Unstructured{}
 	subject.SetGroupVersionKind(mapping.GroupVersionKind)
-
-	if err := m.Client.Get(ctx, types.NamespacedName{
-		Name: clusterNote.Spec.SubjectRef.Name,
-	}, subject); err != nil {
+	if err := subjectClient.Get(ctx, key, subject); err != nil {
 		if errors.IsNotFound(err) {
 			return fmt.Errorf("subject resource not found: %w", err)
 		}
 		return fmt.Errorf("failed to get subject resource: %w", err)
 	}
 
-	// Set owner reference
-	if err := controllerutil.SetOwnerReference(subject, clusterNote, m.Scheme); err != nil {
-		return fmt.Errorf("failed to set owner reference: %w", err)
-	}
-
-	return nil
+	return controllerutil.SetOwnerReference(subject, clusterNote, m.Scheme)
 }
 
 // +kubebuilder:webhook:path=/validate-notes-miloapis-com-v1alpha1-clusternote,mutating=false,failurePolicy=fail,sideEffects=None,groups=notes.miloapis.com,resources=clusternotes,verbs=create;update,versions=v1alpha1,name=vclusternote.notes.miloapis.com,admissionReviewVersions={v1,v1beta1},serviceName=milo-controller-manager,servicePort=9443,serviceNamespace=milo-system

internal/webhooks/notes/v1alpha1/clusternote_webhook_test.go

@@ -126,9 +126,10 @@ func TestClusterNoteMutator_Default(t *testing.T) {
 
 			fakeClient := fake.NewClientBuilder().WithScheme(testScheme).WithObjects(objects...).Build()
 			mutator := &ClusterNoteMutator{
-				Client:     fakeClient,
-				Scheme:     testScheme,
-				RESTMapper: newTestRESTMapper(),
+				Client:         fakeClient,
+				Scheme:         testScheme,
+				RESTMapper:     newTestRESTMapper(),
+				ClusterManager: nil, // nil means local-only search (backwards compatible)
 			}
 
 			// Create admission request context

internal/webhooks/notes/v1alpha1/note_webhook.go

@@ -19,18 +19,21 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+	mccontext "sigs.k8s.io/multicluster-runtime/pkg/context"
+	mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager"
 )
 
 var noteLog = logf.Log.WithName("note-resource")
 
-func SetupNoteWebhooksWithManager(mgr ctrl.Manager) error {
+func SetupNoteWebhooksWithManager(mgr ctrl.Manager, mcMgr mcmanager.Manager) error {
 	noteLog.Info("Setting up notes.miloapis.com note webhooks")
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(&notesv1alpha1.Note{}).
 		WithDefaulter(&NoteMutator{
-			Client:     mgr.GetClient(),
-			Scheme:     mgr.GetScheme(),
-			RESTMapper: mgr.GetRESTMapper(),
+			Client:         mgr.GetClient(),
+			Scheme:         mgr.GetScheme(),
+			RESTMapper:     mgr.GetRESTMapper(),
+			ClusterManager: mcMgr,
 		}).
 		WithValidator(&NoteValidator{
 			Client: mgr.GetClient(),
@@ -41,9 +44,10 @@ func SetupNoteWebhooksWithManager(mgr ctrl.Manager) error {
 // +kubebuilder:webhook:path=/mutate-notes-miloapis-com-v1alpha1-note,mutating=true,failurePolicy=fail,sideEffects=None,groups=notes.miloapis.com,resources=notes,verbs=create,versions=v1alpha1,name=mnote.notes.miloapis.com,admissionReviewVersions={v1,v1beta1},serviceName=milo-controller-manager,servicePort=9443,serviceNamespace=milo-system
 
 type NoteMutator struct {
-	Client     client.Client
-	Scheme     *runtime.Scheme
-	RESTMapper meta.RESTMapper
+	Client         client.Client
+	Scheme         *runtime.Scheme
+	RESTMapper     meta.RESTMapper
+	ClusterManager mcmanager.Manager
 }
 
 var _ admission.CustomDefaulter = &NoteMutator{}
@@ -70,7 +74,6 @@ func (m *NoteMutator) Default(ctx context.Context, obj runtime.Object) error {
 	}
 
 	// Set owner reference to the subject resource for automatic garbage collection
-	// This is critical for the Note to be garbage collected when the subject is deleted
 	if err := m.setSubjectOwnerReference(ctx, note); err != nil {
 		noteLog.Error(err, "Failed to set owner reference to subject", "note", note.Name)
 		return errors.NewInternalError(fmt.Errorf("failed to set owner reference to subject: %w", err))
@@ -79,14 +82,15 @@ func (m *NoteMutator) Default(ctx context.Context, obj runtime.Object) error {
 	return nil
 }
 
-// setSubjectOwnerReference sets the owner reference to the subject resource if it's in the same namespace
+// setSubjectOwnerReference sets the owner reference to the subject resource if it's in the same namespace.
+// The cluster context is expected to be injected by the ClusterAwareServer wrapper.
 func (m *NoteMutator) setSubjectOwnerReference(ctx context.Context, note *notesv1alpha1.Note) error {
 	// Only set owner reference if the subject is in the same namespace
 	if note.Spec.SubjectRef.Namespace == "" || note.Spec.SubjectRef.Namespace != note.Namespace {
 		return nil
 	}
 
-	// Resolve the GVK using REST mapper to discover the correct API version
+	// Resolve the GVK using REST mapper
 	groupKind := schema.GroupKind{
 		Group: note.Spec.SubjectRef.APIGroup,
 		Kind:  note.Spec.SubjectRef.Kind,
@@ -97,25 +101,35 @@ func (m *NoteMutator) setSubjectOwnerReference(ctx context.Context, note *notesv
 		return fmt.Errorf("failed to get REST mapping for %s: %w", groupKind, err)
 	}
 
-	subject := &unstructured.Unstructured{}
-	subject.SetGroupVersionKind(mapping.GroupVersionKind)
-
-	if err := m.Client.Get(ctx, types.NamespacedName{
+	key := types.NamespacedName{
 		Name:      note.Spec.SubjectRef.Name,
 		Namespace: note.Spec.SubjectRef.Namespace,
-	}, subject); err != nil {
+	}
+
+	// Determine which client to use based on cluster context (injected by ClusterAwareServer)
+	subjectClient := m.Client
+
+	if m.ClusterManager != nil {
+		if clusterName, ok := mccontext.ClusterFrom(ctx); ok && clusterName != "" {
+			cluster, err := m.ClusterManager.GetCluster(ctx, clusterName)
+			if err != nil {
+				return fmt.Errorf("failed to get project control plane %s: %w", clusterName, err)
+			}
+			subjectClient = cluster.GetClient()
+			noteLog.V(1).Info("Using project control plane client", "cluster", clusterName)
+		}
+	}
+
+	subject := &unstructured.Unstructured{}
+	subject.SetGroupVersionKind(mapping.GroupVersionKind)
+	if err := subjectClient.Get(ctx, key, subject); err != nil {
 		if errors.IsNotFound(err) {
 			return fmt.Errorf("subject resource not found: %w", err)
 		}
 		return fmt.Errorf("failed to get subject resource: %w", err)
 	}
 
-	// Set owner reference
-	if err := controllerutil.SetOwnerReference(subject, note, m.Scheme); err != nil {
-		return fmt.Errorf("failed to set owner reference: %w", err)
-	}
-
-	return nil
+	return controllerutil.SetOwnerReference(subject, note, m.Scheme)
 }
 
 // +kubebuilder:webhook:path=/validate-notes-miloapis-com-v1alpha1-note,mutating=false,failurePolicy=fail,sideEffects=None,groups=notes.miloapis.com,resources=notes,verbs=create;update,versions=v1alpha1,name=vnote.notes.miloapis.com,admissionReviewVersions={v1,v1beta1},serviceName=milo-controller-manager,servicePort=9443,serviceNamespace=milo-system

internal/webhooks/notes/v1alpha1/note_webhook_test.go

@@ -160,9 +160,10 @@ func TestNoteMutator_Default(t *testing.T) {
 
 			fakeClient := fake.NewClientBuilder().WithScheme(testScheme).WithObjects(objects...).Build()
 			mutator := &NoteMutator{
-				Client:     fakeClient,
-				Scheme:     testScheme,
-				RESTMapper: newTestRESTMapper(),
+				Client:         fakeClient,
+				Scheme:         testScheme,
+				RESTMapper:     newTestRESTMapper(),
+				ClusterManager: nil, // nil means local-only search (backwards compatible)
 			}
 
 			// Create admission request context

pkg/multicluster-runtime/milo/provider.go

@@ -156,7 +156,9 @@ func (p *Provider) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result
 	log := p.log.WithValues("project", req.Name)
 	log.Info("Reconciling Project")
 
-	key := req.String()
+	// Use just the project name as the key for cluster lookup.
+	// This matches the project name used in URL paths and ParentNameExtraKey.
+	key := req.Name
 	var project unstructured.Unstructured
 
 	if p.opts.InternalServiceDiscovery {

pkg/multicluster-runtime/milo/provider_test.go

@@ -61,7 +61,7 @@ func TestReadyProject(t *testing.T) {
 	assert.Zero(t, result.RequeueAfter)
 	assert.Len(t, provider.projects, 1)
 
-	cl, err := provider.Get(context.Background(), "/test-project")
+	cl, err := provider.Get(context.Background(), "test-project")
 	assert.NoError(t, err)
 	apiHost, err := url.Parse(cl.GetConfig().Host)
 	assert.NoError(t, err)