From df910c544fa2e9c6c12e2af4f62b82d48adc93c3 Mon Sep 17 00:00:00 2001 From: Philipp Matthes Date: Thu, 5 Feb 2026 09:35:57 +0100 Subject: [PATCH 1/8] Use gvk in multicluster client to map resources --- cmd/main.go | 26 +- docs/guides/multicluster/readme.md | 2 +- internal/scheduling/explanation/controller.go | 3 +- pkg/conf/conf.go | 4 +- pkg/multicluster/builder.go | 8 +- pkg/multicluster/builder_test.go | 129 ++++++--- pkg/multicluster/client.go | 229 ++++++--------- pkg/multicluster/client_test.go | 264 +++++++++++++----- 8 files changed, 409 insertions(+), 256 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 9ceb90bf0..537c08a53 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -257,8 +257,32 @@ func main() { HomeRestConfig: restConfig, HomeScheme: scheme, } + // Get all registered runtime.Object types in the current scheme and use + // them to configure remote clusters. + var objectsByGVK = make(map[string]runtime.Object) + for gvk := range scheme.AllKnownTypes() { + obj, err := scheme.New(gvk) + if err != nil { + setupLog.Error(err, "unable to create object for gvk", "gvk", gvk) + os.Exit(1) + } + // This produces something like: "cortex.cloud/v1alpha1/Decision" which can + // be used to look up the right cluster for a given API server override. + formatted := gvk.GroupVersion().String() + "/" + gvk.Kind + objectsByGVK[formatted] = obj + } + for gvkStr := range objectsByGVK { + setupLog.Info("scheme gvk registered", "gvk", gvkStr) + } for _, override := range config.APIServerOverrides { - cluster, err := multiclusterClient.AddRemote(override.Resource, override.Host, override.CACert) + // Check if we have any registered objects for this API server override. + obj, ok := objectsByGVK[override.GVK] + if !ok { + setupLog.Error(nil, "no registered objects for apiserver override resource", + "apiserver", override.Host, "gvk", override.GVK) + os.Exit(1) + } + cluster, err := multiclusterClient.AddRemote(ctx, override.Host, override.CACert, obj) if err != nil { setupLog.Error(err, "unable to create cluster for apiserver override", "apiserver", override.Host) os.Exit(1) diff --git a/docs/guides/multicluster/readme.md b/docs/guides/multicluster/readme.md index fa0fd3c83..99b1ed0cd 100644 --- a/docs/guides/multicluster/readme.md +++ b/docs/guides/multicluster/readme.md @@ -79,7 +79,7 @@ tee $TILT_OVERRIDES_PATH </", e.g. "cortex.cloud/v1alpha1/Decision" + GVK string `json:"gvk"` // The remote kubernetes apiserver url, e.g. "https://my-apiserver:6443" Host string `json:"host"` // The root CA certificate to verify the remote apiserver. diff --git a/pkg/multicluster/builder.go b/pkg/multicluster/builder.go index 6b4d84abe..004c4810d 100644 --- a/pkg/multicluster/builder.go +++ b/pkg/multicluster/builder.go @@ -37,12 +37,8 @@ type MulticlusterBuilder struct { // resource URI. If your builder needs this method, pass it to the builder // as the first call and then proceed with other builder methods. func (b MulticlusterBuilder) WatchesMulticluster(object client.Object, eventHandler handler.TypedEventHandler[client.Object, reconcile.Request], predicates ...predicate.Predicate) MulticlusterBuilder { - resource, ok := object.(Resource) - if !ok { - b.Builder = b.Watches(object, eventHandler, builder.WithPredicates(predicates...)) - return b - } - cl := b.multiclusterClient.ClusterForResource(resource.URI()) + gvk := object.GetObjectKind().GroupVersionKind() + cl := b.multiclusterClient.ClusterForResource(gvk) clusterCache := cl.GetCache() b.Builder = b.WatchesRawSource(source.Kind(clusterCache, object, eventHandler, predicates...)) return b diff --git a/pkg/multicluster/builder_test.go b/pkg/multicluster/builder_test.go index 8e8a08283..628d37e15 100644 --- a/pkg/multicluster/builder_test.go +++ b/pkg/multicluster/builder_test.go @@ -6,56 +6,107 @@ package multicluster import ( "testing" - "sigs.k8s.io/controller-runtime/pkg/client" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/cluster" ) -type mockResource struct { - client.Object - uri string +// TestBuildController tests that BuildController creates a MulticlusterBuilder. +// Note: Full integration testing requires a running manager, which is not +// practical for unit tests. This test verifies the basic structure. +func TestBuildController_Structure(t *testing.T) { + // We can't easily test BuildController without a real manager + // because ctrl.NewControllerManagedBy requires a manager implementation. + // Instead, we verify that MulticlusterBuilder has the expected fields. + + // Test that MulticlusterBuilder can be created with required fields + c := &Client{ + remoteClusters: make(map[schema.GroupVersionKind]cluster.Cluster), + } + + // Verify the Client field is accessible + if c.remoteClusters == nil { + t.Error("expected remoteClusters to be initialized") + } } -func (m *mockResource) URI() string { - return m.uri +// TestMulticlusterBuilder_Fields verifies the structure of MulticlusterBuilder. +func TestMulticlusterBuilder_Fields(t *testing.T) { + // Create a minimal client for testing + c := &Client{} + + // Create a MulticlusterBuilder manually to test its fields + mb := MulticlusterBuilder{ + Builder: nil, // Can't create without manager + multiclusterClient: c, + } + + // Verify multiclusterClient is set + if mb.multiclusterClient != c { + t.Error("expected multiclusterClient to be set") + } + + // Verify Builder can be nil initially + if mb.Builder != nil { + t.Error("expected Builder to be nil when not set") + } } -type mockNonResource struct { - client.Object +// TestMulticlusterBuilder_WatchesMulticluster_RequiresClient tests that +// WatchesMulticluster requires a multicluster client. +func TestMulticlusterBuilder_WatchesMulticluster_RequiresClient(t *testing.T) { + // Create a client with remote clusters + c := &Client{ + remoteClusters: make(map[schema.GroupVersionKind]cluster.Cluster), + } + + // Verify the client can be used with the builder + mb := MulticlusterBuilder{ + multiclusterClient: c, + } + + if mb.multiclusterClient == nil { + t.Error("expected multiclusterClient to be set") + } } -func TestBuilder_ResourceInterface(t *testing.T) { - // Test that our mock resource implements the Resource interface - var _ Resource = &mockResource{} +// TestClient_ClusterForResource_ReturnsHomeCluster tests that ClusterForResource +// returns the home cluster when no remote cluster is configured for the GVK. +func TestClient_ClusterForResource_Integration(t *testing.T) { + // Test with nil remote clusters - should return home cluster + c := &Client{ + HomeCluster: nil, // Will return nil + remoteClusters: nil, + } - resource := &mockResource{uri: "test-uri"} - if resource.URI() != "test-uri" { - t.Errorf("Expected URI 'test-uri', got '%s'", resource.URI()) + gvk := schema.GroupVersionKind{ + Group: "apps", + Version: "v1", + Kind: "Deployment", + } + + result := c.ClusterForResource(gvk) + if result != nil { + t.Error("expected nil when HomeCluster is nil") } } -func TestResource_TypeAssertion(t *testing.T) { - tests := []struct { - name string - object client.Object - isResource bool - }{ - { - name: "resource object", - object: &mockResource{uri: "test-uri"}, - isResource: true, - }, - { - name: "non-resource object", - object: &mockNonResource{}, - isResource: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - _, ok := tt.object.(Resource) - if ok != tt.isResource { - t.Errorf("Expected isResource=%v, got %v", tt.isResource, ok) - } - }) +// TestClient_ClusterForResource_LookupOrder tests the lookup order: +// first check remote clusters, then fall back to home cluster. +func TestClient_ClusterForResource_LookupOrder(t *testing.T) { + // Create client with empty remote clusters map + c := &Client{ + remoteClusters: make(map[schema.GroupVersionKind]cluster.Cluster), + } + + gvk := schema.GroupVersionKind{ + Group: "apps", + Version: "v1", + Kind: "Deployment", + } + + // Should return HomeCluster (nil) since GVK is not in remoteClusters + result := c.ClusterForResource(gvk) + if result != nil { + t.Error("expected nil when GVK not in remoteClusters and HomeCluster is nil") } } diff --git a/pkg/multicluster/client.go b/pkg/multicluster/client.go index 5b7501746..3878037dc 100644 --- a/pkg/multicluster/client.go +++ b/pkg/multicluster/client.go @@ -5,18 +5,18 @@ package multicluster import ( "context" + "errors" "sync" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/cluster" ) -type Resource interface{ URI() string } - type Client struct { // The cluster in which cortex is deployed. HomeCluster cluster.Cluster @@ -26,10 +26,10 @@ type Client struct { // This scheme should include all types used in the remote clusters. HomeScheme *runtime.Scheme - // Remote clusters to use by resource URI. + // Remote clusters to use by resource type. // The clusters provided are expected to accept the home cluster's // service account tokens and know about the resources being managed. - remoteClusters map[string]cluster.Cluster + remoteClusters map[schema.GroupVersionKind]cluster.Cluster // Mutex to protect access to remoteClusters. remoteClustersMu sync.RWMutex } @@ -40,7 +40,8 @@ type Client struct { // This can be used when the remote cluster accepts the home cluster's service // account tokens. See the kubernetes documentation on structured auth to // learn more about jwt-based authentication across clusters. -func (c *Client) AddRemote(uri, host, caCert string) (cluster.Cluster, error) { +func (c *Client) AddRemote(ctx context.Context, host, caCert string, objs ...runtime.Object) (cluster.Cluster, error) { + log := ctrl.LoggerFrom(ctx) homeRestConfig := *c.HomeRestConfig restConfigCopy := homeRestConfig restConfigCopy.Host = host @@ -55,20 +56,24 @@ func (c *Client) AddRemote(uri, host, caCert string) (cluster.Cluster, error) { c.remoteClustersMu.Lock() defer c.remoteClustersMu.Unlock() if c.remoteClusters == nil { - c.remoteClusters = make(map[string]cluster.Cluster) + c.remoteClusters = make(map[schema.GroupVersionKind]cluster.Cluster) + } + for _, obj := range objs { + gvk := obj.GetObjectKind().GroupVersionKind() + log.Info("adding remote cluster for resource", "gvk", gvk, "host", host) + c.remoteClusters[gvk] = cl } - c.remoteClusters[uri] = cl return cl, nil } -// Get the cluster for the given resource URI. +// Get the cluster for the given group version kind. // -// If this URI does not have a remote cluster configured, the home cluster -// is returned. -func (c *Client) ClusterForResource(uri string) cluster.Cluster { +// If this object kind does not have a remote cluster configured, +// the home cluster is returned. +func (c *Client) ClusterForResource(gvk schema.GroupVersionKind) cluster.Cluster { c.remoteClustersMu.RLock() defer c.remoteClustersMu.RUnlock() - cl, ok := c.remoteClusters[uri] + cl, ok := c.remoteClusters[gvk] if ok { return cl } @@ -79,104 +84,73 @@ func (c *Client) ClusterForResource(uri string) cluster.Cluster { // // If this URI does not have a remote cluster configured, the home cluster's // client is returned. -func (c *Client) ClientForResource(uri string) client.Client { - return c.ClusterForResource(uri).GetClient() +func (c *Client) ClientForResource(gvk schema.GroupVersionKind) client.Client { + return c. + ClusterForResource(gvk). + GetClient() } -// Pick the right cluster based on the resource URI and perform a Get operation. +// Pick the right cluster based on the resource type and perform a Get operation. // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *Client) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { - resource, ok := obj.(Resource) - if !ok { - return c.HomeCluster.GetClient().Get(ctx, key, obj, opts...) - } - cl := c.ClusterForResource(resource.URI()) - return cl.GetClient().Get(ctx, key, obj, opts...) + gvk := obj.GetObjectKind().GroupVersionKind() + return c.ClientForResource(gvk).Get(ctx, key, obj, opts...) } -// Pick the right cluster based on the resource URI and perform a List operation. +// Pick the right cluster based on the resource type and perform a List operation. // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *Client) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { - resource, ok := list.(Resource) - if !ok { - return c.HomeCluster.GetClient().List(ctx, list, opts...) - } - cl := c.ClusterForResource(resource.URI()) - return cl.GetClient().List(ctx, list, opts...) + gvk := list.GetObjectKind().GroupVersionKind() + return c.ClientForResource(gvk).List(ctx, list, opts...) } -// Pick the right cluster based on the resource URI and perform an Apply operation. -// If the object does not implement Resource or no custom cluster is configured, -// the home cluster is used. +// Apply is not supported in the multicluster client as the group version kind +// cannot be inferred from the ApplyConfiguration. Use ClientForResource +// and call Apply on the returned client instead. func (c *Client) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...client.ApplyOption) error { - resource, ok := obj.(Resource) - if !ok { - return c.HomeCluster.GetClient().Apply(ctx, obj, opts...) - } - cl := c.ClusterForResource(resource.URI()) - return cl.GetClient().Apply(ctx, obj, opts...) + return errors.New("apply operation is not supported in multicluster client") } -// Pick the right cluster based on the resource URI and perform a Create operation. +// Pick the right cluster based on the resource type and perform a Create operation. // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *Client) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { - resource, ok := obj.(Resource) - if !ok { - return c.HomeCluster.GetClient().Create(ctx, obj, opts...) - } - cl := c.ClusterForResource(resource.URI()) - return cl.GetClient().Create(ctx, obj, opts...) + gvk := obj.GetObjectKind().GroupVersionKind() + return c.ClientForResource(gvk).Create(ctx, obj, opts...) } -// Pick the right cluster based on the resource URI and perform a Create operation. +// Pick the right cluster based on the resource type and perform a Create operation. // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *Client) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { - resource, ok := obj.(Resource) - if !ok { - return c.HomeCluster.GetClient().Delete(ctx, obj, opts...) - } - cl := c.ClusterForResource(resource.URI()) - return cl.GetClient().Delete(ctx, obj, opts...) + gvk := obj.GetObjectKind().GroupVersionKind() + return c.ClientForResource(gvk).Delete(ctx, obj, opts...) } -// Pick the right cluster based on the resource URI and perform an Update operation. +// Pick the right cluster based on the resource type and perform an Update operation. // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *Client) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - resource, ok := obj.(Resource) - if !ok { - return c.HomeCluster.GetClient().Update(ctx, obj, opts...) - } - cl := c.ClusterForResource(resource.URI()) - return cl.GetClient().Update(ctx, obj, opts...) + gvk := obj.GetObjectKind().GroupVersionKind() + return c.ClientForResource(gvk).Update(ctx, obj, opts...) } -// Pick the right cluster based on the resource URI and perform a Patch operation. +// Pick the right cluster based on the resource type and perform a Patch operation. // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *Client) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { - resource, ok := obj.(Resource) - if !ok { - return c.HomeCluster.GetClient().Patch(ctx, obj, patch, opts...) - } - cl := c.ClusterForResource(resource.URI()) - return cl.GetClient().Patch(ctx, obj, patch, opts...) + gvk := obj.GetObjectKind().GroupVersionKind() + return c.ClientForResource(gvk).Patch(ctx, obj, patch, opts...) } -// Pick the right cluster based on the resource URI and perform a DeleteAllOf operation. +// Pick the right cluster based on the resource type and perform a DeleteAllOf operation. // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *Client) DeleteAllOf(ctx context.Context, obj client.Object, opts ...client.DeleteAllOfOption) error { - resource, ok := obj.(Resource) - if !ok { - return c.HomeCluster.GetClient().DeleteAllOf(ctx, obj, opts...) - } - cl := c.ClusterForResource(resource.URI()) - return cl.GetClient().DeleteAllOf(ctx, obj, opts...) + gvk := obj.GetObjectKind().GroupVersionKind() + return c.ClientForResource(gvk).DeleteAllOf(ctx, obj, opts...) } // Return the scheme of the home cluster. @@ -200,63 +174,49 @@ func (c *Client) IsObjectNamespaced(obj runtime.Object) (bool, error) { } // Provide a wrapper around the status subresource client which picks the right cluster -// based on the resource URI. +// based on the resource type. func (c *Client) Status() client.StatusWriter { return &statusClient{multiclusterClient: c} } // Wrapper around the status subresource client which picks the right cluster -// based on the resource URI. +// based on the resource type. type statusClient struct{ multiclusterClient *Client } -// Pick the right cluster based on the resource URI and perform a Create operation. +// Pick the right cluster based on the resource type and perform a Create operation. // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *statusClient) Create(ctx context.Context, obj, subResource client.Object, opts ...client.SubResourceCreateOption) error { - resource, ok := obj.(Resource) - if !ok { - return c.multiclusterClient.HomeCluster.GetClient().Status().Create(ctx, obj, subResource, opts...) - } - cl := c.multiclusterClient.ClusterForResource(resource.URI()) - return cl.GetClient().Status().Create(ctx, obj, subResource, opts...) + gvk := obj.GetObjectKind().GroupVersionKind() + return c.multiclusterClient.ClientForResource(gvk). + Status().Create(ctx, obj, subResource, opts...) } -// Pick the right cluster based on the resource URI and perform an Update operation. +// Pick the right cluster based on the resource type and perform an Update operation. // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *statusClient) Update(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { - resource, ok := obj.(Resource) - if !ok { - return c.multiclusterClient.HomeCluster.GetClient().Status().Update(ctx, obj, opts...) - } - cl := c.multiclusterClient.ClusterForResource(resource.URI()) - return cl.GetClient().Status().Update(ctx, obj, opts...) + gvk := obj.GetObjectKind().GroupVersionKind() + return c.multiclusterClient.ClientForResource(gvk). + Status().Update(ctx, obj, opts...) } -// Pick the right cluster based on the resource URI and perform a Patch operation. +// Pick the right cluster based on the resource type and perform a Patch operation. // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *statusClient) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.SubResourcePatchOption) error { - resource, ok := obj.(Resource) - if !ok { - return c.multiclusterClient.HomeCluster.GetClient().Status().Patch(ctx, obj, patch, opts...) - } - cl := c.multiclusterClient.ClusterForResource(resource.URI()) - return cl.GetClient().Status().Patch(ctx, obj, patch, opts...) + gvk := obj.GetObjectKind().GroupVersionKind() + return c.multiclusterClient.ClientForResource(gvk). + Status().Patch(ctx, obj, patch, opts...) } -// Pick the right cluster based on the resource URI and perform an Apply operation. -// If the object does not implement Resource or no custom cluster is configured, -// the home cluster is used. +// Apply is not supported in the multicluster status client as the group version kind +// cannot be inferred from the ApplyConfiguration. Use ClientForResource +// and call Apply on the returned client instead. func (c *statusClient) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...client.SubResourceApplyOption) error { - resource, ok := obj.(Resource) - if !ok { - return c.multiclusterClient.HomeCluster.GetClient().Status().Apply(ctx, obj, opts...) - } - cl := c.multiclusterClient.ClusterForResource(resource.URI()) - return cl.GetClient().Status().Apply(ctx, obj, opts...) + return errors.New("apply operation is not supported in multicluster status client") } // Provide a wrapper around the given subresource client which picks the right cluster -// based on the resource URI. +// based on the resource type. func (c *Client) SubResource(subResource string) client.SubResourceClient { return &subResourceClient{ multiclusterClient: c, @@ -265,7 +225,7 @@ func (c *Client) SubResource(subResource string) client.SubResourceClient { } // Wrapper around a subresource client which picks the right cluster -// based on the resource URI. +// based on the resource type. type subResourceClient struct { // The parent multicluster client. multiclusterClient *Client @@ -273,62 +233,45 @@ type subResourceClient struct { subResource string } -// Pick the right cluster based on the resource URI and perform a Get operation. +// Pick the right cluster based on the resource type and perform a Get operation. // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *subResourceClient) Get(ctx context.Context, obj, subResource client.Object, opts ...client.SubResourceGetOption) error { - resource, ok := obj.(Resource) - if !ok { - return c.multiclusterClient.HomeCluster.GetClient().SubResource(c.subResource).Get(ctx, obj, subResource, opts...) - } - cl := c.multiclusterClient.ClusterForResource(resource.URI()) - return cl.GetClient().SubResource(c.subResource).Get(ctx, obj, subResource, opts...) + gvk := obj.GetObjectKind().GroupVersionKind() + return c.multiclusterClient.ClientForResource(gvk). + SubResource(c.subResource).Get(ctx, obj, subResource, opts...) } -// Pick the right cluster based on the resource URI and perform a Create operation. +// Pick the right cluster based on the resource type and perform a Create operation. // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *subResourceClient) Create(ctx context.Context, obj, subResource client.Object, opts ...client.SubResourceCreateOption) error { - resource, ok := obj.(Resource) - if !ok { - return c.multiclusterClient.HomeCluster.GetClient().SubResource(c.subResource).Create(ctx, obj, subResource, opts...) - } - cl := c.multiclusterClient.ClusterForResource(resource.URI()) - return cl.GetClient().SubResource(c.subResource).Create(ctx, obj, subResource, opts...) + gvk := obj.GetObjectKind().GroupVersionKind() + return c.multiclusterClient.ClientForResource(gvk). + SubResource(c.subResource).Create(ctx, obj, subResource, opts...) } -// Pick the right cluster based on the resource URI and perform an Update operation. +// Pick the right cluster based on the resource type and perform an Update operation. // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *subResourceClient) Update(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { - resource, ok := obj.(Resource) - if !ok { - return c.multiclusterClient.HomeCluster.GetClient().SubResource(c.subResource).Update(ctx, obj, opts...) - } - cl := c.multiclusterClient.ClusterForResource(resource.URI()) - return cl.GetClient().SubResource(c.subResource).Update(ctx, obj, opts...) + gvk := obj.GetObjectKind().GroupVersionKind() + return c.multiclusterClient.ClientForResource(gvk). + SubResource(c.subResource).Update(ctx, obj, opts...) } -// Pick the right cluster based on the resource URI and perform a Patch operation. +// Pick the right cluster based on the resource type and perform a Patch operation. // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *subResourceClient) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.SubResourcePatchOption) error { - resource, ok := obj.(Resource) - if !ok { - return c.multiclusterClient.HomeCluster.GetClient().SubResource(c.subResource).Patch(ctx, obj, patch, opts...) - } - cl := c.multiclusterClient.ClusterForResource(resource.URI()) - return cl.GetClient().SubResource(c.subResource).Patch(ctx, obj, patch, opts...) + gvk := obj.GetObjectKind().GroupVersionKind() + return c.multiclusterClient.ClientForResource(gvk). + SubResource(c.subResource).Patch(ctx, obj, patch, opts...) } -// Pick the right cluster based on the resource URI and perform an Apply operation. -// If the object does not implement Resource or no custom cluster is configured, -// the home cluster is used. +// Apply is not supported in the multicluster subresource client as the group version kind +// cannot be inferred from the ApplyConfiguration. Use ClientForResource +// and call Apply on the returned client instead. func (c *subResourceClient) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...client.SubResourceApplyOption) error { - resource, ok := obj.(Resource) - if !ok { - return c.multiclusterClient.HomeCluster.GetClient().SubResource(c.subResource).Apply(ctx, obj, opts...) - } - cl := c.multiclusterClient.ClusterForResource(resource.URI()) - return cl.GetClient().SubResource(c.subResource).Apply(ctx, obj, opts...) + return errors.New("apply operation is not supported in multicluster subresource client") } diff --git a/pkg/multicluster/client_test.go b/pkg/multicluster/client_test.go index 7a06b820f..b609dd0d1 100644 --- a/pkg/multicluster/client_test.go +++ b/pkg/multicluster/client_test.go @@ -4,96 +4,234 @@ package multicluster import ( + "context" "testing" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/cluster" + + "github.com/cobaltcore-dev/cortex/api/v1alpha1" ) -type testResource struct { - client.Object - uri string +func newTestScheme(t *testing.T) *runtime.Scheme { + t.Helper() + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add corev1 to scheme: %v", err) + } + if err := v1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add v1alpha1 to scheme: %v", err) + } + return scheme } -func (t *testResource) URI() string { - return t.uri -} +// TestClient_Apply tests that the Apply method returns an error. +func TestClient_Apply(t *testing.T) { + scheme := newTestScheme(t) -type testNonResource struct { - client.Object + c := &Client{ + HomeScheme: scheme, + } + + ctx := context.Background() + + t.Run("apply returns error", func(t *testing.T) { + err := c.Apply(ctx, nil) + if err == nil { + t.Error("expected error for Apply operation") + } + if err.Error() != "apply operation is not supported in multicluster client" { + t.Errorf("unexpected error message: %v", err) + } + }) } -func TestResource_Interface(t *testing.T) { - // Test that our test resource implements the Resource interface - var _ Resource = &testResource{} +// TestStatusClient_Apply tests that the status client Apply method returns an error. +func TestStatusClient_Apply(t *testing.T) { + sc := &statusClient{multiclusterClient: &Client{}} + + ctx := context.Background() - resource := &testResource{uri: "test-uri"} - if resource.URI() != "test-uri" { - t.Errorf("Expected URI 'test-uri', got '%s'", resource.URI()) + err := sc.Apply(ctx, nil) + if err == nil { + t.Error("expected error for Apply operation") + } + if err.Error() != "apply operation is not supported in multicluster status client" { + t.Errorf("unexpected error message: %v", err) } } -func TestClient_ResourceTypeDetection(t *testing.T) { - tests := []struct { - name string - object client.Object - isResource bool - expectedURI string - }{ - { - name: "resource with URI", - object: &testResource{uri: "test-uri"}, - isResource: true, - expectedURI: "test-uri", - }, - { - name: "resource with empty URI", - object: &testResource{uri: ""}, - isResource: true, - expectedURI: "", - }, - { - name: "non-resource object", - object: &testNonResource{}, - isResource: false, - expectedURI: "", - }, +// TestSubResourceClientApply tests that the subresource client Apply method returns an error. +func TestSubResourceClientApply(t *testing.T) { + src := &subResourceClient{ + multiclusterClient: &Client{}, + subResource: "status", } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - resource, ok := tt.object.(Resource) - if ok != tt.isResource { - t.Errorf("Expected isResource=%v, got %v", tt.isResource, ok) - } + ctx := context.Background() - if ok { - uri := resource.URI() - if uri != tt.expectedURI { - t.Errorf("Expected URI '%s', got '%s'", tt.expectedURI, uri) - } - } - }) + err := src.Apply(ctx, nil) + if err == nil { + t.Error("expected error for Apply operation") + } + if err.Error() != "apply operation is not supported in multicluster subresource client" { + t.Errorf("unexpected error message: %v", err) } } -func TestClient_ClusterForResource_NilSafety(t *testing.T) { - // Test that ClusterForResource handles nil home cluster gracefully - client := &Client{} +// TestClient_ClusterForResource_NilRemoteClusters tests behavior when no remote clusters are configured. +func TestClient_ClusterForResource_NilRemoteClusters(t *testing.T) { + c := &Client{ + remoteClusters: nil, + } - cluster := client.ClusterForResource("any-uri") - if cluster != nil { - t.Error("Expected nil cluster when home cluster is nil") + gvk := schema.GroupVersionKind{ + Group: "test", + Version: "v1", + Kind: "TestKind", + } + + // When remoteClusters is nil and HomeCluster is nil, we should get nil + result := c.ClusterForResource(gvk) + if result != nil { + t.Error("expected nil when no home cluster is set") } } +// TestClient_ClusterForResource_EmptyRemoteClusters tests behavior with empty remote clusters map. func TestClient_ClusterForResource_EmptyRemoteClusters(t *testing.T) { - // Test behavior with nil remote clusters map - client := &Client{ + c := &Client{ + remoteClusters: make(map[schema.GroupVersionKind]cluster.Cluster), + } + + gvk := schema.GroupVersionKind{ + Group: "test", + Version: "v1", + Kind: "TestKind", + } + + // When remoteClusters is empty and HomeCluster is nil, we should get nil + result := c.ClusterForResource(gvk) + if result != nil { + t.Error("expected nil when no home cluster is set and GVK not found") + } +} + +// TestClient_Status returns a status writer. +func TestClient_Status(t *testing.T) { + c := &Client{} + + status := c.Status() + if status == nil { + t.Error("expected non-nil status writer") + } + + // Verify it's the right type + if _, ok := status.(*statusClient); !ok { + t.Error("expected statusClient type") + } +} + +// TestClient_SubResource returns a subresource client. +func TestClient_SubResource(t *testing.T) { + c := &Client{} + + subResource := c.SubResource("scale") + if subResource == nil { + t.Error("expected non-nil subresource client") + } + + // Verify it's the right type + src, ok := subResource.(*subResourceClient) + if !ok { + t.Error("expected subResourceClient type") + } + + if src.subResource != "scale" { + t.Errorf("expected subResource='scale', got '%s'", src.subResource) + } +} + +// TestClient_AddRemote_NilRemoteClusters initializes the remote clusters map. +func TestClient_AddRemote_NilRemoteClusters(t *testing.T) { + c := &Client{ remoteClusters: nil, } - cluster := client.ClusterForResource("any-uri") - if cluster != nil { - t.Error("Expected nil cluster when both home and remote clusters are nil") + // Just verify the lock mechanism works without panicking + c.remoteClustersMu.Lock() + if c.remoteClusters == nil { + c.remoteClusters = make(map[schema.GroupVersionKind]cluster.Cluster) + } + c.remoteClustersMu.Unlock() + + // Should not panic + if c.remoteClusters == nil { + t.Error("expected remoteClusters to be initialized") + } +} + +// TestClient_ConcurrentAccess tests thread safety of ClusterForResource. +func TestClient_ConcurrentAccess(t *testing.T) { + c := &Client{ + remoteClusters: make(map[schema.GroupVersionKind]cluster.Cluster), + } + + gvk := schema.GroupVersionKind{ + Group: "test", + Version: "v1", + Kind: "TestKind", + } + + // Test concurrent reads - should not panic or race + done := make(chan bool) + for range 10 { + go func() { + _ = c.ClusterForResource(gvk) + done <- true + }() + } + + for range 10 { + <-done + } +} + +// TestObjectKeyFromConfigMap tests that we can construct object keys properly. +func TestObjectKeyFromConfigMap(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-config", + Namespace: "default", + }, + } + + key := client.ObjectKeyFromObject(cm) + if key.Name != "test-config" { + t.Errorf("expected Name='test-config', got '%s'", key.Name) + } + if key.Namespace != "default" { + t.Errorf("expected Namespace='default', got '%s'", key.Namespace) + } +} + +// TestGVKExtraction tests that GVK can be properly set and retrieved. +func TestGVKExtraction(t *testing.T) { + cm := &corev1.ConfigMap{} + gvk := schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + } + + cm.SetGroupVersionKind(gvk) + + result := cm.GetObjectKind().GroupVersionKind() + if result != gvk { + t.Errorf("expected GVK %v, got %v", gvk, result) } } From 2db9fd73731574ab5176b71c725b34840da72be9 Mon Sep 17 00:00:00 2001 From: Philipp Matthes Date: Thu, 5 Feb 2026 09:44:49 +0100 Subject: [PATCH 2/8] Remove URI methods --- api/v1alpha1/datasource_types.go | 3 --- api/v1alpha1/decision_types.go | 3 --- api/v1alpha1/descheduling_types.go | 3 --- api/v1alpha1/knowledge_types.go | 3 --- api/v1alpha1/kpi_types.go | 3 --- api/v1alpha1/pipeline_types.go | 3 --- api/v1alpha1/reservation_types.go | 3 --- 7 files changed, 21 deletions(-) diff --git a/api/v1alpha1/datasource_types.go b/api/v1alpha1/datasource_types.go index 6538e2c58..699523e6b 100644 --- a/api/v1alpha1/datasource_types.go +++ b/api/v1alpha1/datasource_types.go @@ -278,9 +278,6 @@ type DatasourceList struct { Items []Datasource `json:"items"` } -func (*Datasource) URI() string { return "datasources.cortex.cloud/v1alpha1" } -func (*DatasourceList) URI() string { return "datasources.cortex.cloud/v1alpha1" } - func init() { SchemeBuilder.Register(&Datasource{}, &DatasourceList{}) } diff --git a/api/v1alpha1/decision_types.go b/api/v1alpha1/decision_types.go index 40ced0683..c3f02de1e 100644 --- a/api/v1alpha1/decision_types.go +++ b/api/v1alpha1/decision_types.go @@ -136,9 +136,6 @@ type DecisionList struct { Items []Decision `json:"items"` } -func (*Decision) URI() string { return "decisions.cortex.cloud/v1alpha1" } -func (*DecisionList) URI() string { return "decisions.cortex.cloud/v1alpha1" } - func init() { SchemeBuilder.Register(&Decision{}, &DecisionList{}) } diff --git a/api/v1alpha1/descheduling_types.go b/api/v1alpha1/descheduling_types.go index 17d33dfb3..9e38056fe 100644 --- a/api/v1alpha1/descheduling_types.go +++ b/api/v1alpha1/descheduling_types.go @@ -88,9 +88,6 @@ type DeschedulingList struct { Items []Descheduling `json:"items"` } -func (*Descheduling) URI() string { return "deschedulings.cortex.cloud/v1alpha1" } -func (*DeschedulingList) URI() string { return "deschedulings.cortex.cloud/v1alpha1" } - func init() { SchemeBuilder.Register(&Descheduling{}, &DeschedulingList{}) } diff --git a/api/v1alpha1/knowledge_types.go b/api/v1alpha1/knowledge_types.go index 74c9c1957..d90f76565 100644 --- a/api/v1alpha1/knowledge_types.go +++ b/api/v1alpha1/knowledge_types.go @@ -141,9 +141,6 @@ type KnowledgeList struct { Items []Knowledge `json:"items"` } -func (*Knowledge) URI() string { return "knowledges.cortex.cloud/v1alpha1" } -func (*KnowledgeList) URI() string { return "knowledges.cortex.cloud/v1alpha1" } - func init() { SchemeBuilder.Register(&Knowledge{}, &KnowledgeList{}) } diff --git a/api/v1alpha1/kpi_types.go b/api/v1alpha1/kpi_types.go index 628a09c3e..eca769ef2 100644 --- a/api/v1alpha1/kpi_types.go +++ b/api/v1alpha1/kpi_types.go @@ -96,9 +96,6 @@ type KPIList struct { Items []KPI `json:"items"` } -func (*KPI) URI() string { return "kpis.cortex.cloud/v1alpha1" } -func (*KPIList) URI() string { return "kpis.cortex.cloud/v1alpha1" } - func init() { SchemeBuilder.Register(&KPI{}, &KPIList{}) } diff --git a/api/v1alpha1/pipeline_types.go b/api/v1alpha1/pipeline_types.go index 84d1aeec4..c2d6a5f32 100644 --- a/api/v1alpha1/pipeline_types.go +++ b/api/v1alpha1/pipeline_types.go @@ -218,9 +218,6 @@ type PipelineList struct { Items []Pipeline `json:"items"` } -func (*Pipeline) URI() string { return "pipelines.cortex.cloud/v1alpha1" } -func (*PipelineList) URI() string { return "pipelines.cortex.cloud/v1alpha1" } - func init() { SchemeBuilder.Register(&Pipeline{}, &PipelineList{}) } diff --git a/api/v1alpha1/reservation_types.go b/api/v1alpha1/reservation_types.go index 37be917fd..847a1017c 100644 --- a/api/v1alpha1/reservation_types.go +++ b/api/v1alpha1/reservation_types.go @@ -97,9 +97,6 @@ type ReservationList struct { Items []Reservation `json:"items"` } -func (*Reservation) URI() string { return "reservations.cortex.cloud/v1alpha1" } -func (*ReservationList) URI() string { return "reservations.cortex.cloud/v1alpha1" } - func init() { SchemeBuilder.Register(&Reservation{}, &ReservationList{}) } From c8abd7e77c077cd2d45f64d3068d2b8f829cd2f2 Mon Sep 17 00:00:00 2001 From: Philipp Matthes Date: Thu, 5 Feb 2026 10:49:35 +0100 Subject: [PATCH 3/8] AddRemote with gvk directly to avoid empty objects --- cmd/main.go | 22 +++++++++------------- pkg/multicluster/client.go | 7 +++---- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 537c08a53..1a282acd0 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -18,6 +18,7 @@ import ( _ "k8s.io/client-go/plugin/pkg/client/auth" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" @@ -257,32 +258,27 @@ func main() { HomeRestConfig: restConfig, HomeScheme: scheme, } - // Get all registered runtime.Object types in the current scheme and use - // them to configure remote clusters. - var objectsByGVK = make(map[string]runtime.Object) + // Map the formatted gvk from the config to the actual gvk object so that we + // can look up the right cluster for a given API server override. + var gvksByConfStr = make(map[string]schema.GroupVersionKind) for gvk := range scheme.AllKnownTypes() { - obj, err := scheme.New(gvk) - if err != nil { - setupLog.Error(err, "unable to create object for gvk", "gvk", gvk) - os.Exit(1) - } // This produces something like: "cortex.cloud/v1alpha1/Decision" which can // be used to look up the right cluster for a given API server override. formatted := gvk.GroupVersion().String() + "/" + gvk.Kind - objectsByGVK[formatted] = obj + gvksByConfStr[formatted] = gvk } - for gvkStr := range objectsByGVK { + for gvkStr := range gvksByConfStr { setupLog.Info("scheme gvk registered", "gvk", gvkStr) } for _, override := range config.APIServerOverrides { - // Check if we have any registered objects for this API server override. - obj, ok := objectsByGVK[override.GVK] + // Check if we have any registered gvk for this API server override. + gvk, ok := gvksByConfStr[override.GVK] if !ok { setupLog.Error(nil, "no registered objects for apiserver override resource", "apiserver", override.Host, "gvk", override.GVK) os.Exit(1) } - cluster, err := multiclusterClient.AddRemote(ctx, override.Host, override.CACert, obj) + cluster, err := multiclusterClient.AddRemote(ctx, override.Host, override.CACert, gvk) if err != nil { setupLog.Error(err, "unable to create cluster for apiserver override", "apiserver", override.Host) os.Exit(1) diff --git a/pkg/multicluster/client.go b/pkg/multicluster/client.go index 3878037dc..c7168a8c7 100644 --- a/pkg/multicluster/client.go +++ b/pkg/multicluster/client.go @@ -35,12 +35,12 @@ type Client struct { } // Add a remote cluster which uses the same REST config as the home cluster, -// but a different host, for the given resource URI. +// but a different host, for the given resource gvks. // // This can be used when the remote cluster accepts the home cluster's service // account tokens. See the kubernetes documentation on structured auth to // learn more about jwt-based authentication across clusters. -func (c *Client) AddRemote(ctx context.Context, host, caCert string, objs ...runtime.Object) (cluster.Cluster, error) { +func (c *Client) AddRemote(ctx context.Context, host, caCert string, gvks ...schema.GroupVersionKind) (cluster.Cluster, error) { log := ctrl.LoggerFrom(ctx) homeRestConfig := *c.HomeRestConfig restConfigCopy := homeRestConfig @@ -58,8 +58,7 @@ func (c *Client) AddRemote(ctx context.Context, host, caCert string, objs ...run if c.remoteClusters == nil { c.remoteClusters = make(map[schema.GroupVersionKind]cluster.Cluster) } - for _, obj := range objs { - gvk := obj.GetObjectKind().GroupVersionKind() + for _, gvk := range gvks { log.Info("adding remote cluster for resource", "gvk", gvk, "host", host) c.remoteClusters[gvk] = cl } From c07e3750abeb4bd0f10df9875cb3c32e97712ef3 Mon Sep 17 00:00:00 2001 From: Philipp Matthes Date: Thu, 5 Feb 2026 11:31:31 +0100 Subject: [PATCH 4/8] Use HomeScheme to get gvk --- pkg/multicluster/builder.go | 6 +- pkg/multicluster/client.go | 85 ++- pkg/multicluster/client_test.go | 1214 +++++++++++++++++++++++++++++++ 3 files changed, 1289 insertions(+), 16 deletions(-) diff --git a/pkg/multicluster/builder.go b/pkg/multicluster/builder.go index 004c4810d..484936011 100644 --- a/pkg/multicluster/builder.go +++ b/pkg/multicluster/builder.go @@ -37,8 +37,10 @@ type MulticlusterBuilder struct { // resource URI. If your builder needs this method, pass it to the builder // as the first call and then proceed with other builder methods. func (b MulticlusterBuilder) WatchesMulticluster(object client.Object, eventHandler handler.TypedEventHandler[client.Object, reconcile.Request], predicates ...predicate.Predicate) MulticlusterBuilder { - gvk := object.GetObjectKind().GroupVersionKind() - cl := b.multiclusterClient.ClusterForResource(gvk) + cl := b.multiclusterClient.HomeCluster // default cluster + if gvk, err := b.multiclusterClient.GVKFromHomeScheme(object); err == nil { + cl = b.multiclusterClient.ClusterForResource(gvk) + } clusterCache := cl.GetCache() b.Builder = b.WatchesRawSource(source.Kind(clusterCache, object, eventHandler, predicates...)) return b diff --git a/pkg/multicluster/client.go b/pkg/multicluster/client.go index c7168a8c7..cec0d8199 100644 --- a/pkg/multicluster/client.go +++ b/pkg/multicluster/client.go @@ -65,6 +65,21 @@ func (c *Client) AddRemote(ctx context.Context, host, caCert string, gvks ...sch return cl, nil } +// Get the gvk registered for the given resource in the home cluster's RESTMapper. +func (c *Client) GVKFromHomeScheme(obj runtime.Object) (gvk schema.GroupVersionKind, err error) { + gvks, unversioned, err := c.HomeScheme.ObjectKinds(obj) + if err != nil { + return gvk, err + } + if unversioned { + return gvk, errors.New("cannot list unversioned resource") + } + if len(gvks) != 1 { + return gvk, errors.New("expected exactly one gvk for list object") + } + return gvks[0], nil +} + // Get the cluster for the given group version kind. // // If this object kind does not have a remote cluster configured, @@ -93,7 +108,10 @@ func (c *Client) ClientForResource(gvk schema.GroupVersionKind) client.Client { // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *Client) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { - gvk := obj.GetObjectKind().GroupVersionKind() + gvk, err := c.GVKFromHomeScheme(obj) + if err != nil { + return err + } return c.ClientForResource(gvk).Get(ctx, key, obj, opts...) } @@ -101,7 +119,10 @@ func (c *Client) Get(ctx context.Context, key client.ObjectKey, obj client.Objec // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *Client) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { - gvk := list.GetObjectKind().GroupVersionKind() + gvk, err := c.GVKFromHomeScheme(list) + if err != nil { + return err + } return c.ClientForResource(gvk).List(ctx, list, opts...) } @@ -116,7 +137,10 @@ func (c *Client) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *Client) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { - gvk := obj.GetObjectKind().GroupVersionKind() + gvk, err := c.GVKFromHomeScheme(obj) + if err != nil { + return err + } return c.ClientForResource(gvk).Create(ctx, obj, opts...) } @@ -124,7 +148,10 @@ func (c *Client) Create(ctx context.Context, obj client.Object, opts ...client.C // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *Client) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { - gvk := obj.GetObjectKind().GroupVersionKind() + gvk, err := c.GVKFromHomeScheme(obj) + if err != nil { + return err + } return c.ClientForResource(gvk).Delete(ctx, obj, opts...) } @@ -132,7 +159,10 @@ func (c *Client) Delete(ctx context.Context, obj client.Object, opts ...client.D // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *Client) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - gvk := obj.GetObjectKind().GroupVersionKind() + gvk, err := c.GVKFromHomeScheme(obj) + if err != nil { + return err + } return c.ClientForResource(gvk).Update(ctx, obj, opts...) } @@ -140,7 +170,10 @@ func (c *Client) Update(ctx context.Context, obj client.Object, opts ...client.U // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *Client) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { - gvk := obj.GetObjectKind().GroupVersionKind() + gvk, err := c.GVKFromHomeScheme(obj) + if err != nil { + return err + } return c.ClientForResource(gvk).Patch(ctx, obj, patch, opts...) } @@ -148,7 +181,10 @@ func (c *Client) Patch(ctx context.Context, obj client.Object, patch client.Patc // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *Client) DeleteAllOf(ctx context.Context, obj client.Object, opts ...client.DeleteAllOfOption) error { - gvk := obj.GetObjectKind().GroupVersionKind() + gvk, err := c.GVKFromHomeScheme(obj) + if err != nil { + return err + } return c.ClientForResource(gvk).DeleteAllOf(ctx, obj, opts...) } @@ -184,7 +220,10 @@ type statusClient struct{ multiclusterClient *Client } // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *statusClient) Create(ctx context.Context, obj, subResource client.Object, opts ...client.SubResourceCreateOption) error { - gvk := obj.GetObjectKind().GroupVersionKind() + gvk, err := c.multiclusterClient.GVKFromHomeScheme(obj) + if err != nil { + return err + } return c.multiclusterClient.ClientForResource(gvk). Status().Create(ctx, obj, subResource, opts...) } @@ -193,7 +232,10 @@ func (c *statusClient) Create(ctx context.Context, obj, subResource client.Objec // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *statusClient) Update(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { - gvk := obj.GetObjectKind().GroupVersionKind() + gvk, err := c.multiclusterClient.GVKFromHomeScheme(obj) + if err != nil { + return err + } return c.multiclusterClient.ClientForResource(gvk). Status().Update(ctx, obj, opts...) } @@ -202,7 +244,10 @@ func (c *statusClient) Update(ctx context.Context, obj client.Object, opts ...cl // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *statusClient) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.SubResourcePatchOption) error { - gvk := obj.GetObjectKind().GroupVersionKind() + gvk, err := c.multiclusterClient.GVKFromHomeScheme(obj) + if err != nil { + return err + } return c.multiclusterClient.ClientForResource(gvk). Status().Patch(ctx, obj, patch, opts...) } @@ -236,7 +281,10 @@ type subResourceClient struct { // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *subResourceClient) Get(ctx context.Context, obj, subResource client.Object, opts ...client.SubResourceGetOption) error { - gvk := obj.GetObjectKind().GroupVersionKind() + gvk, err := c.multiclusterClient.GVKFromHomeScheme(obj) + if err != nil { + return err + } return c.multiclusterClient.ClientForResource(gvk). SubResource(c.subResource).Get(ctx, obj, subResource, opts...) } @@ -245,7 +293,10 @@ func (c *subResourceClient) Get(ctx context.Context, obj, subResource client.Obj // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *subResourceClient) Create(ctx context.Context, obj, subResource client.Object, opts ...client.SubResourceCreateOption) error { - gvk := obj.GetObjectKind().GroupVersionKind() + gvk, err := c.multiclusterClient.GVKFromHomeScheme(obj) + if err != nil { + return err + } return c.multiclusterClient.ClientForResource(gvk). SubResource(c.subResource).Create(ctx, obj, subResource, opts...) } @@ -254,7 +305,10 @@ func (c *subResourceClient) Create(ctx context.Context, obj, subResource client. // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *subResourceClient) Update(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { - gvk := obj.GetObjectKind().GroupVersionKind() + gvk, err := c.multiclusterClient.GVKFromHomeScheme(obj) + if err != nil { + return err + } return c.multiclusterClient.ClientForResource(gvk). SubResource(c.subResource).Update(ctx, obj, opts...) } @@ -263,7 +317,10 @@ func (c *subResourceClient) Update(ctx context.Context, obj client.Object, opts // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *subResourceClient) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.SubResourcePatchOption) error { - gvk := obj.GetObjectKind().GroupVersionKind() + gvk, err := c.multiclusterClient.GVKFromHomeScheme(obj) + if err != nil { + return err + } return c.multiclusterClient.ClientForResource(gvk). SubResource(c.subResource).Patch(ctx, obj, patch, opts...) } diff --git a/pkg/multicluster/client_test.go b/pkg/multicluster/client_test.go index b609dd0d1..8ee9373d7 100644 --- a/pkg/multicluster/client_test.go +++ b/pkg/multicluster/client_test.go @@ -5,6 +5,7 @@ package multicluster import ( "context" + "sync" "testing" corev1 "k8s.io/api/core/v1" @@ -12,11 +13,47 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/cluster" "github.com/cobaltcore-dev/cortex/api/v1alpha1" ) +// unversionedType is a type that is registered as unversioned in the scheme. +type unversionedType struct { + metav1.TypeMeta `json:",inline"` +} + +func (u *unversionedType) DeepCopyObject() runtime.Object { + return &unversionedType{TypeMeta: u.TypeMeta} +} + +// unknownType is a type that is NOT registered in the scheme. +type unknownType struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` +} + +func (u *unknownType) DeepCopyObject() runtime.Object { + return &unknownType{TypeMeta: u.TypeMeta, ObjectMeta: u.ObjectMeta} +} + +// fakeCluster implements cluster.Cluster interface for testing. +type fakeCluster struct { + cluster.Cluster + fakeClient client.Client +} + +func (f *fakeCluster) GetClient() client.Client { + return f.fakeClient +} + +func newFakeCluster(scheme *runtime.Scheme, objs ...client.Object) *fakeCluster { + return &fakeCluster{ + fakeClient: fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build(), + } +} + func newTestScheme(t *testing.T) *runtime.Scheme { t.Helper() scheme := runtime.NewScheme() @@ -235,3 +272,1180 @@ func TestGVKExtraction(t *testing.T) { t.Errorf("expected GVK %v, got %v", gvk, result) } } + +// TestGVKFromHomeScheme_Success tests successful GVK lookup for registered types. +func TestGVKFromHomeScheme_Success(t *testing.T) { + scheme := newTestScheme(t) + + c := &Client{ + HomeScheme: scheme, + } + + tests := []struct { + name string + obj runtime.Object + expectedGVK schema.GroupVersionKind + }{ + { + name: "ConfigMap", + obj: &corev1.ConfigMap{}, + expectedGVK: schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + }, + }, + { + name: "ConfigMapList", + obj: &corev1.ConfigMapList{}, + expectedGVK: schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMapList", + }, + }, + { + name: "Secret", + obj: &corev1.Secret{}, + expectedGVK: schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "Secret", + }, + }, + { + name: "Pod", + obj: &corev1.Pod{}, + expectedGVK: schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "Pod", + }, + }, + { + name: "Service", + obj: &corev1.Service{}, + expectedGVK: schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "Service", + }, + }, + { + name: "v1alpha1 Decision", + obj: &v1alpha1.Decision{}, + expectedGVK: schema.GroupVersionKind{ + Group: "cortex.cloud", + Version: "v1alpha1", + Kind: "Decision", + }, + }, + { + name: "v1alpha1 DecisionList", + obj: &v1alpha1.DecisionList{}, + expectedGVK: schema.GroupVersionKind{ + Group: "cortex.cloud", + Version: "v1alpha1", + Kind: "DecisionList", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gvk, err := c.GVKFromHomeScheme(tt.obj) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if gvk != tt.expectedGVK { + t.Errorf("expected GVK %v, got %v", tt.expectedGVK, gvk) + } + }) + } +} + +// TestGVKFromHomeScheme_UnknownType tests error handling for unregistered types. +func TestGVKFromHomeScheme_UnknownType(t *testing.T) { + scheme := newTestScheme(t) + + c := &Client{ + HomeScheme: scheme, + } + + obj := &unknownType{} + _, err := c.GVKFromHomeScheme(obj) + if err == nil { + t.Error("expected error for unknown type") + } +} + +// TestGVKFromHomeScheme_UnversionedType tests error handling for unversioned types. +func TestGVKFromHomeScheme_UnversionedType(t *testing.T) { + scheme := runtime.NewScheme() + + // Register the type as unversioned + scheme.AddUnversionedTypes(schema.GroupVersion{Group: "", Version: "v1"}, &unversionedType{}) + + c := &Client{ + HomeScheme: scheme, + } + + obj := &unversionedType{} + _, err := c.GVKFromHomeScheme(obj) + if err == nil { + t.Error("expected error for unversioned type") + } + if err.Error() != "cannot list unversioned resource" { + t.Errorf("unexpected error message: %v", err) + } +} + +// TestGVKFromHomeScheme_NilScheme tests behavior with nil scheme. +func TestGVKFromHomeScheme_NilScheme(t *testing.T) { + c := &Client{ + HomeScheme: nil, + } + + obj := &corev1.ConfigMap{} + + // Should panic or return error when scheme is nil + defer func() { + if r := recover(); r == nil { + t.Error("expected panic with nil scheme") + } + }() + + _, err := c.GVKFromHomeScheme(obj) + if err == nil { + t.Error("expected error with nil scheme") + } +} + +// TestClient_ClusterForResource_WithRemoteCluster tests ClusterForResource with a remote cluster configured. +func TestClient_ClusterForResource_WithRemoteCluster(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) + remoteCluster := newFakeCluster(scheme) + + gvk := schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + } + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, + } + + // Should return the remote cluster for the registered GVK + result := c.ClusterForResource(gvk) + if result != remoteCluster { + t.Error("expected remote cluster for registered GVK") + } + + // Should return home cluster for non-registered GVK + otherGVK := schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "Secret", + } + result = c.ClusterForResource(otherGVK) + if result != homeCluster { + t.Error("expected home cluster for non-registered GVK") + } +} + +// TestClient_ClientForResource tests ClientForResource returns the correct client. +func TestClient_ClientForResource(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) + remoteCluster := newFakeCluster(scheme) + + gvk := schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + } + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, + } + + // Should return the remote cluster's client for the registered GVK + result := c.ClientForResource(gvk) + if result != remoteCluster.GetClient() { + t.Error("expected remote cluster client for registered GVK") + } + + // Should return home cluster's client for non-registered GVK + otherGVK := schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "Secret", + } + result = c.ClientForResource(otherGVK) + if result != homeCluster.GetClient() { + t.Error("expected home cluster client for non-registered GVK") + } +} + +// TestClient_Scheme tests that Scheme returns the home cluster's client scheme. +func TestClient_Scheme(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + } + + result := c.Scheme() + if result == nil { + t.Error("expected non-nil scheme") + } +} + +// TestClient_RESTMapper tests that RESTMapper returns the home cluster's client RESTMapper. +func TestClient_RESTMapper(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + } + + result := c.RESTMapper() + if result == nil { + t.Error("expected non-nil RESTMapper") + } +} + +// TestClient_GroupVersionKindFor tests GroupVersionKindFor returns correct GVK. +func TestClient_GroupVersionKindFor(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + } + + gvk, err := c.GroupVersionKindFor(&corev1.ConfigMap{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expected := schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + } + if gvk != expected { + t.Errorf("expected GVK %v, got %v", expected, gvk) + } +} + +// TestClient_IsObjectNamespaced tests IsObjectNamespaced delegates to home cluster client. +func TestClient_IsObjectNamespaced(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + } + + // The fake client's RESTMapper doesn't have all mappings, so we just test + // that the method delegates properly to the home cluster's client. + // We expect an error due to the fake client's limited RESTMapper. + _, err := c.IsObjectNamespaced(&corev1.ConfigMap{}) + // The fake client doesn't have a proper RESTMapper, so this will fail, + // but we're testing that the delegation works. + _ = err // Error expected with fake client +} + +// TestClient_Get tests the Get method routes to the correct cluster. +func TestClient_Get(t *testing.T) { + scheme := newTestScheme(t) + + // Create a ConfigMap in the remote cluster + existingCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cm", + Namespace: "default", + }, + Data: map[string]string{"key": "remote-value"}, + } + + remoteCluster := newFakeCluster(scheme, existingCM) + homeCluster := newFakeCluster(scheme) + + gvk := schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + } + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, + } + + ctx := context.Background() + + // Get from remote cluster (ConfigMap GVK is registered) + cm := &corev1.ConfigMap{} + err := c.Get(ctx, client.ObjectKey{Name: "test-cm", Namespace: "default"}, cm) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cm.Data["key"] != "remote-value" { + t.Errorf("expected 'remote-value', got '%s'", cm.Data["key"]) + } +} + +// TestClient_Get_UnknownType tests Get returns error for unknown types. +func TestClient_Get_UnknownType(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + } + + ctx := context.Background() + + obj := &unknownType{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + } + + err := c.Get(ctx, client.ObjectKey{Name: "test", Namespace: "default"}, obj) + if err == nil { + t.Error("expected error for unknown type") + } +} + +// TestClient_List tests the List method routes to the correct cluster. +func TestClient_List(t *testing.T) { + scheme := newTestScheme(t) + + // Create ConfigMaps in the remote cluster + cm1 := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cm1", + Namespace: "default", + }, + } + cm2 := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cm2", + Namespace: "default", + }, + } + + remoteCluster := newFakeCluster(scheme, cm1, cm2) + homeCluster := newFakeCluster(scheme) + + gvk := schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMapList", + } + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, + } + + ctx := context.Background() + + // List from remote cluster + cmList := &corev1.ConfigMapList{} + err := c.List(ctx, cmList, client.InNamespace("default")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(cmList.Items) != 2 { + t.Errorf("expected 2 items, got %d", len(cmList.Items)) + } +} + +// TestClient_Create tests the Create method routes to the correct cluster. +func TestClient_Create(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) + remoteCluster := newFakeCluster(scheme) + + gvk := schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + } + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, + } + + ctx := context.Background() + + // Create in remote cluster + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-cm", + Namespace: "default", + }, + } + err := c.Create(ctx, cm) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Verify it was created in remote cluster + result := &corev1.ConfigMap{} + err = remoteCluster.GetClient().Get(ctx, client.ObjectKey{Name: "new-cm", Namespace: "default"}, result) + if err != nil { + t.Fatalf("failed to get created object from remote cluster: %v", err) + } +} + +// TestClient_Delete tests the Delete method routes to the correct cluster. +func TestClient_Delete(t *testing.T) { + scheme := newTestScheme(t) + + existingCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "to-delete", + Namespace: "default", + }, + } + + homeCluster := newFakeCluster(scheme) + remoteCluster := newFakeCluster(scheme, existingCM) + + gvk := schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + } + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, + } + + ctx := context.Background() + + // Delete from remote cluster + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "to-delete", + Namespace: "default", + }, + } + err := c.Delete(ctx, cm) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Verify it was deleted from remote cluster + result := &corev1.ConfigMap{} + err = remoteCluster.GetClient().Get(ctx, client.ObjectKey{Name: "to-delete", Namespace: "default"}, result) + if err == nil { + t.Error("expected object to be deleted from remote cluster") + } +} + +// TestClient_Update tests the Update method routes to the correct cluster. +func TestClient_Update(t *testing.T) { + scheme := newTestScheme(t) + + existingCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "to-update", + Namespace: "default", + }, + Data: map[string]string{"key": "old-value"}, + } + + homeCluster := newFakeCluster(scheme) + remoteCluster := newFakeCluster(scheme, existingCM) + + gvk := schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + } + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, + } + + ctx := context.Background() + + // First get the object to have the correct resource version + cm := &corev1.ConfigMap{} + err := remoteCluster.GetClient().Get(ctx, client.ObjectKey{Name: "to-update", Namespace: "default"}, cm) + if err != nil { + t.Fatalf("failed to get object: %v", err) + } + + // Update in remote cluster + cm.Data["key"] = "new-value" + err = c.Update(ctx, cm) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Verify it was updated in remote cluster + result := &corev1.ConfigMap{} + err = remoteCluster.GetClient().Get(ctx, client.ObjectKey{Name: "to-update", Namespace: "default"}, result) + if err != nil { + t.Fatalf("failed to get updated object: %v", err) + } + if result.Data["key"] != "new-value" { + t.Errorf("expected 'new-value', got '%s'", result.Data["key"]) + } +} + +// TestClient_Patch tests the Patch method routes to the correct cluster. +func TestClient_Patch(t *testing.T) { + scheme := newTestScheme(t) + + existingCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "to-patch", + Namespace: "default", + }, + Data: map[string]string{"key": "old-value"}, + } + + homeCluster := newFakeCluster(scheme) + remoteCluster := newFakeCluster(scheme, existingCM) + + gvk := schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + } + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, + } + + ctx := context.Background() + + // Patch in remote cluster + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "to-patch", + Namespace: "default", + }, + } + patch := client.MergeFrom(cm.DeepCopy()) + cm.Data = map[string]string{"key": "patched-value"} + err := c.Patch(ctx, cm, patch) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Verify it was patched in remote cluster + result := &corev1.ConfigMap{} + err = remoteCluster.GetClient().Get(ctx, client.ObjectKey{Name: "to-patch", Namespace: "default"}, result) + if err != nil { + t.Fatalf("failed to get patched object: %v", err) + } + if result.Data["key"] != "patched-value" { + t.Errorf("expected 'patched-value', got '%s'", result.Data["key"]) + } +} + +// TestClient_DeleteAllOf tests the DeleteAllOf method routes to the correct cluster. +func TestClient_DeleteAllOf(t *testing.T) { + scheme := newTestScheme(t) + + cm1 := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cm1", + Namespace: "default", + Labels: map[string]string{"app": "test"}, + }, + } + cm2 := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cm2", + Namespace: "default", + Labels: map[string]string{"app": "test"}, + }, + } + + homeCluster := newFakeCluster(scheme) + remoteCluster := newFakeCluster(scheme, cm1, cm2) + + gvk := schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + } + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, + } + + ctx := context.Background() + + // DeleteAllOf in remote cluster + err := c.DeleteAllOf(ctx, &corev1.ConfigMap{}, client.InNamespace("default"), client.MatchingLabels{"app": "test"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Verify all were deleted from remote cluster + cmList := &corev1.ConfigMapList{} + err = remoteCluster.GetClient().List(ctx, cmList, client.InNamespace("default")) + if err != nil { + t.Fatalf("failed to list objects: %v", err) + } + if len(cmList.Items) != 0 { + t.Errorf("expected 0 items, got %d", len(cmList.Items)) + } +} + +// TestClient_ConcurrentAddRemote tests thread safety of adding remote clusters. +func TestClient_ConcurrentAddRemote(t *testing.T) { + c := &Client{ + remoteClusters: make(map[schema.GroupVersionKind]cluster.Cluster), + } + + var wg sync.WaitGroup + for i := range 10 { + wg.Add(1) + go func(i int) { + defer wg.Done() + gvk := schema.GroupVersionKind{ + Group: "test", + Version: "v1", + Kind: "TestKind" + string(rune('A'+i)), + } + c.remoteClustersMu.Lock() + c.remoteClusters[gvk] = nil + c.remoteClustersMu.Unlock() + }(i) + } + wg.Wait() + + if len(c.remoteClusters) != 10 { + t.Errorf("expected 10 remote clusters, got %d", len(c.remoteClusters)) + } +} + +// TestClient_ConcurrentClusterForResourceAndAddRemote tests concurrent read/write operations. +func TestClient_ConcurrentClusterForResourceAndAddRemote(t *testing.T) { + c := &Client{ + remoteClusters: make(map[schema.GroupVersionKind]cluster.Cluster), + } + + gvk := schema.GroupVersionKind{ + Group: "test", + Version: "v1", + Kind: "TestKind", + } + + var wg sync.WaitGroup + + // Readers + for range 10 { + wg.Add(1) + go func() { + defer wg.Done() + for range 100 { + _ = c.ClusterForResource(gvk) + } + }() + } + + // Writers + for range 5 { + wg.Add(1) + go func() { + defer wg.Done() + for range 100 { + c.remoteClustersMu.Lock() + c.remoteClusters[gvk] = nil + c.remoteClustersMu.Unlock() + } + }() + } + + wg.Wait() +} + +// TestStatusClient_Create tests the status client Create method. +func TestStatusClient_Create(t *testing.T) { + scheme := newTestScheme(t) + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + } + + homeCluster := newFakeCluster(scheme, pod) + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + } + + ctx := context.Background() + + // Create requires a subresource object, but we're just testing the routing + sc := c.Status() + + // The fake client doesn't support status subresource creation in the standard way, + // but we can verify the method exists and routes correctly + err := sc.Create(ctx, pod, &corev1.Pod{}) + // We expect an error because the fake client doesn't support this, + // but we're testing that the routing works + _ = err // Error is expected with fake client +} + +// TestStatusClient_Update tests the status client Update method. +func TestStatusClient_Update(t *testing.T) { + scheme := newTestScheme(t) + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + Status: corev1.PodStatus{ + Phase: corev1.PodPending, + }, + } + + homeCluster := newFakeCluster(scheme, pod) + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + } + + ctx := context.Background() + + // Get the pod first + existingPod := &corev1.Pod{} + err := homeCluster.GetClient().Get(ctx, client.ObjectKey{Name: "test-pod", Namespace: "default"}, existingPod) + if err != nil { + t.Fatalf("failed to get pod: %v", err) + } + + // Update status + existingPod.Status.Phase = corev1.PodRunning + err = c.Status().Update(ctx, existingPod) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// TestStatusClient_Patch tests the status client Patch method. +func TestStatusClient_Patch(t *testing.T) { + scheme := newTestScheme(t) + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + Status: corev1.PodStatus{ + Phase: corev1.PodPending, + }, + } + + homeCluster := newFakeCluster(scheme, pod) + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + } + + ctx := context.Background() + + // Get the pod first + existingPod := &corev1.Pod{} + err := homeCluster.GetClient().Get(ctx, client.ObjectKey{Name: "test-pod", Namespace: "default"}, existingPod) + if err != nil { + t.Fatalf("failed to get pod: %v", err) + } + + // Patch status + patch := client.MergeFrom(existingPod.DeepCopy()) + existingPod.Status.Phase = corev1.PodRunning + err = c.Status().Patch(ctx, existingPod, patch) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// TestStatusClient_RoutesToRemoteCluster tests that status client routes to remote cluster. +func TestStatusClient_RoutesToRemoteCluster(t *testing.T) { + scheme := newTestScheme(t) + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + Status: corev1.PodStatus{ + Phase: corev1.PodPending, + }, + } + + homeCluster := newFakeCluster(scheme) + remoteCluster := newFakeCluster(scheme, pod) + + gvk := schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "Pod", + } + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, + } + + ctx := context.Background() + + // Get the pod from remote cluster + existingPod := &corev1.Pod{} + err := remoteCluster.GetClient().Get(ctx, client.ObjectKey{Name: "test-pod", Namespace: "default"}, existingPod) + if err != nil { + t.Fatalf("failed to get pod: %v", err) + } + + // Update status via multicluster client - should go to remote cluster + existingPod.Status.Phase = corev1.PodRunning + err = c.Status().Update(ctx, existingPod) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Verify it was updated in remote cluster + result := &corev1.Pod{} + err = remoteCluster.GetClient().Get(ctx, client.ObjectKey{Name: "test-pod", Namespace: "default"}, result) + if err != nil { + t.Fatalf("failed to get updated pod: %v", err) + } + if result.Status.Phase != corev1.PodRunning { + t.Errorf("expected PodRunning, got %s", result.Status.Phase) + } +} + +// TestSubResourceClient_Get tests the subresource client Get method. +func TestSubResourceClient_Get(t *testing.T) { + scheme := newTestScheme(t) + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + } + + homeCluster := newFakeCluster(scheme, pod) + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + } + + ctx := context.Background() + + // The fake client may not support all subresources, but we can test the routing + src := c.SubResource("status") + err := src.Get(ctx, pod, &corev1.Pod{}) + // Error is expected with fake client for subresource operations + _ = err +} + +// TestSubResourceClient_Create tests the subresource client Create method. +func TestSubResourceClient_Create(t *testing.T) { + scheme := newTestScheme(t) + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + } + + homeCluster := newFakeCluster(scheme, pod) + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + } + + ctx := context.Background() + + src := c.SubResource("eviction") + err := src.Create(ctx, pod, &corev1.Pod{}) + // Error is expected with fake client for subresource operations + _ = err +} + +// TestSubResourceClient_Update tests the subresource client Update method. +func TestSubResourceClient_Update(t *testing.T) { + scheme := newTestScheme(t) + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + } + + homeCluster := newFakeCluster(scheme, pod) + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + } + + ctx := context.Background() + + // Get the pod first + existingPod := &corev1.Pod{} + err := homeCluster.GetClient().Get(ctx, client.ObjectKey{Name: "test-pod", Namespace: "default"}, existingPod) + if err != nil { + t.Fatalf("failed to get pod: %v", err) + } + + src := c.SubResource("status") + err = src.Update(ctx, existingPod) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// TestSubResourceClient_Patch tests the subresource client Patch method. +func TestSubResourceClient_Patch(t *testing.T) { + scheme := newTestScheme(t) + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + } + + homeCluster := newFakeCluster(scheme, pod) + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + } + + ctx := context.Background() + + // Get the pod first + existingPod := &corev1.Pod{} + err := homeCluster.GetClient().Get(ctx, client.ObjectKey{Name: "test-pod", Namespace: "default"}, existingPod) + if err != nil { + t.Fatalf("failed to get pod: %v", err) + } + + patch := client.MergeFrom(existingPod.DeepCopy()) + src := c.SubResource("status") + err = src.Patch(ctx, existingPod, patch) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// TestSubResourceClient_RoutesToRemoteCluster tests that subresource client routes to remote cluster. +func TestSubResourceClient_RoutesToRemoteCluster(t *testing.T) { + scheme := newTestScheme(t) + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "default", + }, + } + + homeCluster := newFakeCluster(scheme) + remoteCluster := newFakeCluster(scheme, pod) + + gvk := schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "Pod", + } + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, + } + + ctx := context.Background() + + // Get the pod from remote cluster + existingPod := &corev1.Pod{} + err := remoteCluster.GetClient().Get(ctx, client.ObjectKey{Name: "test-pod", Namespace: "default"}, existingPod) + if err != nil { + t.Fatalf("failed to get pod: %v", err) + } + + // Update via subresource client - should go to remote cluster + src := c.SubResource("status") + err = src.Update(ctx, existingPod) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// TestGVKFromHomeScheme_WithDifferentAPIGroups tests GVK lookup for different API groups. +func TestGVKFromHomeScheme_WithDifferentAPIGroups(t *testing.T) { + scheme := newTestScheme(t) + + c := &Client{ + HomeScheme: scheme, + } + + tests := []struct { + name string + obj runtime.Object + expectedGrp string + }{ + { + name: "core API group (empty string)", + obj: &corev1.ConfigMap{}, + expectedGrp: "", + }, + { + name: "custom API group", + obj: &v1alpha1.Decision{}, + expectedGrp: "cortex.cloud", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gvk, err := c.GVKFromHomeScheme(tt.obj) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if gvk.Group != tt.expectedGrp { + t.Errorf("expected group '%s', got '%s'", tt.expectedGrp, gvk.Group) + } + }) + } +} + +// TestClient_Operations_WithHomeClusterOnly tests operations when no remote clusters are configured. +func TestClient_Operations_WithHomeClusterOnly(t *testing.T) { + scheme := newTestScheme(t) + + existingCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "home-cm", + Namespace: "default", + }, + Data: map[string]string{"key": "home-value"}, + } + + homeCluster := newFakeCluster(scheme, existingCM) + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + } + + ctx := context.Background() + + // Get from home cluster + cm := &corev1.ConfigMap{} + err := c.Get(ctx, client.ObjectKey{Name: "home-cm", Namespace: "default"}, cm) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cm.Data["key"] != "home-value" { + t.Errorf("expected 'home-value', got '%s'", cm.Data["key"]) + } + + // List from home cluster + cmList := &corev1.ConfigMapList{} + err = c.List(ctx, cmList, client.InNamespace("default")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(cmList.Items) != 1 { + t.Errorf("expected 1 item, got %d", len(cmList.Items)) + } +} + +// TestClient_StatusAndSubResource_ErrorOnUnknownType tests error handling for unknown types. +func TestClient_StatusAndSubResource_ErrorOnUnknownType(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + } + + ctx := context.Background() + + obj := &unknownType{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + } + + // Status client should return error for unknown type + err := c.Status().Update(ctx, obj) + if err == nil { + t.Error("expected error for unknown type in status Update") + } + + err = c.Status().Patch(ctx, obj, client.MergeFrom(obj)) + if err == nil { + t.Error("expected error for unknown type in status Patch") + } + + // SubResource client should return error for unknown type + err = c.SubResource("status").Update(ctx, obj) + if err == nil { + t.Error("expected error for unknown type in subresource Update") + } + + err = c.SubResource("status").Patch(ctx, obj, client.MergeFrom(obj)) + if err == nil { + t.Error("expected error for unknown type in subresource Patch") + } +} From 2cf5bdc695caebb0df0832cfb09d4a312518124c Mon Sep 17 00:00:00 2001 From: Philipp Matthes Date: Thu, 5 Feb 2026 11:33:49 +0100 Subject: [PATCH 5/8] Fix explanations controller setup method --- internal/scheduling/explanation/controller.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/scheduling/explanation/controller.go b/internal/scheduling/explanation/controller.go index 26c977a83..720b8aa17 100644 --- a/internal/scheduling/explanation/controller.go +++ b/internal/scheduling/explanation/controller.go @@ -188,7 +188,10 @@ func (c *Controller) StartupCallback(ctx context.Context) error { // This function sets up the controller with the provided manager. func (c *Controller) SetupWithManager(mgr manager.Manager, mcl *multicluster.Client) error { if !c.SkipIndexFields { - gvk := (&v1alpha1.Decision{}).GetObjectKind().GroupVersionKind() + gvk, err := mcl.GVKFromHomeScheme(&v1alpha1.Decision{}) + if err != nil { + return err + } cluster := mcl.ClusterForResource(gvk) if err := cluster.GetCache().IndexField( context.Background(), &v1alpha1.Decision{}, "spec.resourceID", From 68a984cbd83fffd463eeab91531140c2ea0bbaf6 Mon Sep 17 00:00:00 2001 From: Philipp Matthes <27271818+PhilippMatthes@users.noreply.github.com> Date: Thu, 5 Feb 2026 13:29:47 +0100 Subject: [PATCH 6/8] Update pkg/multicluster/client.go Co-authored-by: Markus Wieland <44964229+SoWieMarkus@users.noreply.github.com> --- pkg/multicluster/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/multicluster/client.go b/pkg/multicluster/client.go index cec0d8199..01ed9492c 100644 --- a/pkg/multicluster/client.go +++ b/pkg/multicluster/client.go @@ -144,7 +144,7 @@ func (c *Client) Create(ctx context.Context, obj client.Object, opts ...client.C return c.ClientForResource(gvk).Create(ctx, obj, opts...) } -// Pick the right cluster based on the resource type and perform a Create operation. +// Pick the right cluster based on the resource type and perform a Delete operation. // If the object does not implement Resource or no custom cluster is configured, // the home cluster is used. func (c *Client) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { From 13152a621b557edd0502c689da7fd74380b7d8f5 Mon Sep 17 00:00:00 2001 From: Philipp Matthes <27271818+PhilippMatthes@users.noreply.github.com> Date: Thu, 5 Feb 2026 13:29:59 +0100 Subject: [PATCH 7/8] Update pkg/multicluster/client.go Co-authored-by: Markus Wieland <44964229+SoWieMarkus@users.noreply.github.com> --- pkg/multicluster/client.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/multicluster/client.go b/pkg/multicluster/client.go index 01ed9492c..fb2cd3cf6 100644 --- a/pkg/multicluster/client.go +++ b/pkg/multicluster/client.go @@ -97,6 +97,9 @@ func (c *Client) ClusterForResource(gvk schema.GroupVersionKind) cluster.Cluster // Get the client for the given resource URI. // // If this URI does not have a remote cluster configured, the home cluster's +// Get the client for the given resource group version kind. +// +// If this object kind does not have a remote cluster configured, the home cluster's // client is returned. func (c *Client) ClientForResource(gvk schema.GroupVersionKind) client.Client { return c. From b2fb4e3701fa6b90af0fdccbccc7ed743089b02b Mon Sep 17 00:00:00 2001 From: Philipp Matthes <27271818+PhilippMatthes@users.noreply.github.com> Date: Thu, 5 Feb 2026 13:30:36 +0100 Subject: [PATCH 8/8] Update pkg/multicluster/builder.go Co-authored-by: Markus Wieland <44964229+SoWieMarkus@users.noreply.github.com> --- pkg/multicluster/builder.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/multicluster/builder.go b/pkg/multicluster/builder.go index 484936011..e5d8d7e3a 100644 --- a/pkg/multicluster/builder.go +++ b/pkg/multicluster/builder.go @@ -35,6 +35,10 @@ type MulticlusterBuilder struct { // // If the object implements Resource, we pick the right cluster based on the // resource URI. If your builder needs this method, pass it to the builder +// Watch resources, potentially in a remote cluster. +// +// Determines the appropriate cluster by looking up the object's GroupVersionKind (GVK) +// in the home scheme. If your builder needs this method, pass it to the builder // as the first call and then proceed with other builder methods. func (b MulticlusterBuilder) WatchesMulticluster(object client.Object, eventHandler handler.TypedEventHandler[client.Object, reconcile.Request], predicates ...predicate.Predicate) MulticlusterBuilder { cl := b.multiclusterClient.HomeCluster // default cluster