Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions cmd/milo/controller-manager/controllermanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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")
Expand Down
52 changes: 33 additions & 19 deletions internal/webhooks/notes/v1alpha1/clusternote_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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{}
Expand All @@ -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))
Expand All @@ -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 != "" {
Expand All @@ -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
Expand Down
7 changes: 4 additions & 3 deletions internal/webhooks/notes/v1alpha1/clusternote_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
56 changes: 35 additions & 21 deletions internal/webhooks/notes/v1alpha1/note_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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{}
Expand All @@ -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))
Expand All @@ -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,
Expand All @@ -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
Expand Down
7 changes: 4 additions & 3 deletions internal/webhooks/notes/v1alpha1/note_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion pkg/multicluster-runtime/milo/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/multicluster-runtime/milo/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading
Loading