diff --git a/cmd/main.go b/cmd/main.go index 2b7bbb31f..f8188f93a 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" @@ -273,10 +274,14 @@ func main() { setupLog.Error(err, "unable to add home cluster") os.Exit(1) } + hvGVK := schema.GroupVersionKind{Group: "kvm.cloud.sap", Version: "v1", Kind: "Hypervisor"} multiclusterClient := &multicluster.Client{ HomeCluster: homeCluster, HomeRestConfig: restConfig, HomeScheme: scheme, + ResourceRouters: map[schema.GroupVersionKind]multicluster.ResourceRouter{ + hvGVK: multicluster.HypervisorResourceRouter{}, + }, } multiclusterClientConfig := conf.GetConfigOrDie[multicluster.ClientConfig]() if err := multiclusterClient.InitFromConf(ctx, mgr, multiclusterClientConfig); err != nil { diff --git a/docs/guides/multicluster/cleanup.sh b/docs/guides/multicluster/cleanup.sh new file mode 100755 index 000000000..cfa166036 --- /dev/null +++ b/docs/guides/multicluster/cleanup.sh @@ -0,0 +1,17 @@ +#!/bin/bash + +set -e + +echo "Deleting home cluster" +kind delete cluster --name cortex-home + +echo "Deleting az-a and az-b clusters" +kind delete cluster --name cortex-remote-az-a +kind delete cluster --name cortex-remote-az-b + +echo "Cleaning up temporary files" +rm -f /tmp/root-ca-home.pem \ + /tmp/root-ca-remote-az-a.pem \ + /tmp/root-ca-remote-az-b.pem \ + /tmp/cortex-values.yaml \ + /tmp/hypervisor-crd.yaml \ No newline at end of file diff --git a/docs/guides/multicluster/cortex-remote.yaml b/docs/guides/multicluster/cortex-remote-az-a.yaml similarity index 97% rename from docs/guides/multicluster/cortex-remote.yaml rename to docs/guides/multicluster/cortex-remote-az-a.yaml index 675a14063..61fa24631 100644 --- a/docs/guides/multicluster/cortex-remote.yaml +++ b/docs/guides/multicluster/cortex-remote-az-a.yaml @@ -1,6 +1,6 @@ kind: Cluster apiVersion: kind.x-k8s.io/v1alpha4 -name: cortex-remote +name: cortex-remote-az-a nodes: - role: control-plane extraPortMappings: diff --git a/docs/guides/multicluster/cortex-remote-az-b.yaml b/docs/guides/multicluster/cortex-remote-az-b.yaml new file mode 100644 index 000000000..47848e749 --- /dev/null +++ b/docs/guides/multicluster/cortex-remote-az-b.yaml @@ -0,0 +1,27 @@ +kind: Cluster +apiVersion: kind.x-k8s.io/v1alpha4 +name: cortex-remote-az-b +nodes: + - role: control-plane + extraPortMappings: + - containerPort: 6443 + hostPort: 8445 + extraMounts: + - hostPath: /tmp/root-ca-home.pem + containerPath: /etc/ca-certificates/root-ca.pem + kubeadmConfigPatches: + - | + kind: ClusterConfiguration + apiServer: + extraArgs: + oidc-client-id: "https://host.docker.internal:8443" # = audience + oidc-issuer-url: "https://host.docker.internal:8443" + oidc-username-claim: sub + oidc-ca-file: /etc/ca-certificates/root-ca.pem + certSANs: + - api-proxy + - api-proxy.default.svc + - api-proxy.default.svc.cluster.local + - localhost + - 127.0.0.1 + - host.docker.internal \ No newline at end of file diff --git a/docs/guides/multicluster/hypervisors-az-a.yaml b/docs/guides/multicluster/hypervisors-az-a.yaml new file mode 100644 index 000000000..f6417afb3 --- /dev/null +++ b/docs/guides/multicluster/hypervisors-az-a.yaml @@ -0,0 +1,13 @@ +apiVersion: kvm.cloud.sap/v1 +kind: Hypervisor +metadata: + name: hypervisor-1-az-a + labels: + topology.kubernetes.io/zone: cortex-remote-az-a +--- +apiVersion: kvm.cloud.sap/v1 +kind: Hypervisor +metadata: + name: hypervisor-2-az-a + labels: + topology.kubernetes.io/zone: cortex-remote-az-a \ No newline at end of file diff --git a/docs/guides/multicluster/hypervisors-az-b.yaml b/docs/guides/multicluster/hypervisors-az-b.yaml new file mode 100644 index 000000000..19c636c16 --- /dev/null +++ b/docs/guides/multicluster/hypervisors-az-b.yaml @@ -0,0 +1,13 @@ +apiVersion: kvm.cloud.sap/v1 +kind: Hypervisor +metadata: + name: hypervisor-1-az-b + labels: + topology.kubernetes.io/zone: cortex-remote-az-b +--- +apiVersion: kvm.cloud.sap/v1 +kind: Hypervisor +metadata: + name: hypervisor-2-az-b + labels: + topology.kubernetes.io/zone: cortex-remote-az-b \ No newline at end of file diff --git a/docs/guides/multicluster/readme.md b/docs/guides/multicluster/readme.md index 2b421b443..f65d19486 100644 --- a/docs/guides/multicluster/readme.md +++ b/docs/guides/multicluster/readme.md @@ -1,38 +1,45 @@ # Cortex Multi-Cluster Testing -Cortex provides support for multi-cluster deployments, where a "home" cluster hosts the cortex pods and one or more "remote" clusters are used to persist CRDs. A typical use case for this would be to offload the etcd storage for Cortex CRDs to a remote cluster, reducing the resource usage on the home cluster. +> [!NOTE] +> If you want to skip the reading part, there's `run.sh` and `cleanup.sh` scripts in this directory that will set up and tear down the multi-cluster environment for you. -This guide will walk you through setting up a multi-cluster Cortex deployment using [kind](https://kind.sigs.k8s.io/). We will create two kind clusters: `cortex-home` and `cortex-remote`. The `cortex-home` cluster will host the Cortex control plane, while the `cortex-remote` cluster will be used to store CRDs. +Cortex provides support for multi-cluster deployments, where a "home" cluster hosts the cortex pods and one or more "remote" clusters are used to persist CRDs. A typical use case for this would be to offload the etcd storage for Cortex CRDs to a remote cluster, reducing the resource usage on the home cluster. Similarly, another use case is to have multiple remote clusters that maintain all the compute workloads and expose resources that Cortex needs to access, such as the `Hypervisor` resource. -To store its CRDs in the `cortex-remote` cluster, the `cortex-home` cluster needs to be able to authenticate to the `cortex-remote` cluster's API server. We will achieve this by configuring the `cortex-remote` cluster to trust the service account tokens issued by the `cortex-home` cluster. In this way, no external OIDC provider is needed, because the `cortex-home` cluster's own OIDC issuer for service accounts acts as the identity provider. +This guide will walk you through setting up a multi-cluster Cortex deployment using [kind](https://kind.sigs.k8s.io/). We will create three kind clusters: `cortex-home`, `cortex-remote-az-a`, and `cortex-remote-az-b`. The `cortex-home` cluster will host the Cortex control plane, while the `cortex-remote-az-a` and `cortex-remote-az-b` clusters will be used to store hypervisor CRDs. + +To store its CRDs in the `cortex-remote-*` clusters, the `cortex-home` cluster needs to be able to authenticate to the `cortex-remote-*` clusters' API servers. We will achieve this by configuring the `cortex-remote-*` clusters to trust the service account tokens issued by the `cortex-home` cluster. In this way, no external OIDC provider is needed, because the `cortex-home` cluster's own OIDC issuer for service accounts acts as the identity provider. Here is a diagram illustrating the authentication flow: ```mermaid sequenceDiagram participant Home as cortex-home - participant Remote as cortex-remote + participant RemoteA as cortex-remote-az-a + participant RemoteB as cortex-remote-az-b Home->>Home: Service Account Token Issued - Home->>Remote: API Request with Token - Remote->>Remote: Token Verified Against Home's OIDC Issuer - Remote->>Home: API Response + Home->>RemoteA: API Request with Token + RemoteA->>RemoteA: Token Verified Against Home's OIDC Issuer + RemoteA->>Home: API Response + Home->>RemoteB: API Request with Token + RemoteB->>RemoteB: Token Verified Against Home's OIDC Issuer + RemoteB->>Home: API Response ``` ## Home Cluster Setup -First we set up the `cortex-home` cluster. The provided kind configuration file `cortex-home.yaml` sets up the cluster with the necessary port mappings to allow communication between the two clusters. `cortex-home` will expose its API server on port `8443`, which `cortex-remote` will use to verify service account tokens through `https://host.docker.internal:8443`. +First we set up the `cortex-home` cluster. The provided kind configuration file `cortex-home.yaml` sets up the cluster with the necessary port mappings to allow communication between the three clusters. `cortex-home` will expose its API server on port `8443`, which `cortex-remote-az-a` and `cortex-remote-az-b` will use to verify service account tokens through `https://host.docker.internal:8443`. ```bash kind create cluster --config docs/guides/multicluster/cortex-home.yaml ``` -Next, we need to expose the OIDC issuer endpoint of the `cortex-home` cluster's API server to the `cortex-remote` cluster. We do this by creating a `ClusterRoleBinding` that grants the `system:service-account-issuer-discovery` role to the `kube-system` service account in the `cortex-home` cluster. +Next, we need to expose the OIDC issuer endpoint of the `cortex-home` cluster's API server to the `cortex-remote-*` clusters. We do this by creating a `ClusterRoleBinding` that grants the `system:service-account-issuer-discovery` role to the `kube-system` service account in the `cortex-home` cluster. ```bash kubectl --context kind-cortex-home apply -f docs/guides/multicluster/cortex-home-crb.yaml ``` -To talk back to the `cortex-home` cluster's OIDC endpoint, the `cortex-remote` cluster needs to trust the root CA certificate used by the `cortex-home` cluster's API server. We can extract this certificate from the `extension-apiserver-authentication` config map in the `kube-system` namespace, and save it to a temporary file for later use. +To talk back to the `cortex-home` cluster's OIDC endpoint, the `cortex-remote-*` clusters need to trust the root CA certificate used by the `cortex-home` cluster's API server. We can extract this certificate from the `extension-apiserver-authentication` config map in the `kube-system` namespace, and save it to a temporary file for later use. ```bash kubectl --context kind-cortex-home --namespace kube-system \ @@ -42,67 +49,111 @@ kubectl --context kind-cortex-home --namespace kube-system \ ## Remote Cluster Setup -With all the prerequisites in place, we can now set up the `cortex-remote` cluster. We create the cluster using the provided kind configuration file `cortex-remote.yaml`. This configuration will tell the `cortex-remote` cluster to trust the `cortex-home` cluster's API server as OIDC issuer for service account token verification. Also, the `cortex-remote` cluster will trust the root CA certificate we extracted earlier. The `cortex-remote` apiserver will be accessible at `https://host.docker.internal:8444`. +With all the prerequisites in place, we can now set up the `cortex-remote-*` clusters. We create the clusters using the provided kind configuration files `cortex-remote-az-a.yaml` and `cortex-remote-az-b.yaml`. These configurations will tell the `cortex-remote-*` clusters to trust the `cortex-home` cluster's API server as OIDC issuer for service account token verification. Also, the `cortex-remote-*` clusters will trust the root CA certificate we extracted earlier. The `cortex-remote-*` apiservers will be accessible at `https://host.docker.internal:8444` and `https://host.docker.internal:8445`, respectively. ```bash -kind create cluster --config docs/guides/multicluster/cortex-remote.yaml +kind create cluster --config docs/guides/multicluster/cortex-remote-az-a.yaml +kind create cluster --config docs/guides/multicluster/cortex-remote-az-b.yaml ``` -Next, we need to create a `ClusterRoleBinding` in the `cortex-remote` cluster that grants service accounts coming from the `cortex-home` cluster access to the appropriate resources. We do this by applying the provided `cortex-remote-crb.yaml` file. +Next, we need to create a `ClusterRoleBinding` in the `cortex-remote-*` clusters that grants service accounts coming from the `cortex-home` cluster access to the appropriate resources. We do this by applying the provided `cortex-remote-crb.yaml` file. ```bash -kubectl --context kind-cortex-remote apply -f docs/guides/multicluster/cortex-remote-crb.yaml +kubectl --context kind-cortex-remote-az-a apply -f docs/guides/multicluster/cortex-remote-crb.yaml +kubectl --context kind-cortex-remote-az-b apply -f docs/guides/multicluster/cortex-remote-crb.yaml ``` ## Deploying Cortex -Before we launch cortex make sure that the CRDs are installed in the `cortex-remote` cluster. +Before we launch cortex make sure that the CRDs are installed in the `cortex-remote-*` clusters. ```bash -kubectl config use-context kind-cortex-remote +kubectl config use-context kind-cortex-remote-az-a +helm install helm/bundles/cortex-crds --generate-name +kubectl config use-context kind-cortex-remote-az-b helm install helm/bundles/cortex-crds --generate-name ``` -Also, we need to extract the root CA certificate used by the `cortex-remote` cluster's API server, so that we can configure the cortex pods in the `cortex-home` cluster to trust it. +Let's also install the hypervisor crd to all three cluster which is needed as an external dependency for this example: +```bash +curl -L https://raw.githubusercontent.com/cobaltcore-dev/openstack-hypervisor-operator/refs/heads/main/charts/openstack-hypervisor-operator/crds/kvm.cloud.sap_hypervisors.yaml > /tmp/hypervisor-crd.yaml +kubectl --context kind-cortex-home apply -f /tmp/hypervisor-crd.yaml +kubectl --context kind-cortex-remote-az-a apply -f /tmp/hypervisor-crd.yaml +kubectl --context kind-cortex-remote-az-b apply -f /tmp/hypervisor-crd.yaml +``` + +Also, we need to extract the root CA certificate used by the `cortex-remote-*` clusters' API servers, so that we can configure the cortex pods in the `cortex-home` cluster to trust them. ```bash -kubectl --context kind-cortex-remote --namespace kube-system \ +kubectl --context kind-cortex-remote-az-a --namespace kube-system \ + get configmap extension-apiserver-authentication \ + -o jsonpath="{.data['client-ca-file']}" > /tmp/root-ca-remote-az-a.pem +kubectl --context kind-cortex-remote-az-b --namespace kube-system \ get configmap extension-apiserver-authentication \ - -o jsonpath="{.data['client-ca-file']}" > /tmp/root-ca-remote.pem + -o jsonpath="{.data['client-ca-file']}" > /tmp/root-ca-remote-az-b.pem ``` -Now we can deploy cortex to the `cortex-home` cluster, configuring it to use the `cortex-remote` cluster for CRD storage. We create a temporary Helm values override file that specifies the API server URL and root CA certificate for the `cortex-remote` cluster. In this example, we are configuring the `decisions.cortex.cloud/v1alpha1` resource to be stored in the `cortex-remote` cluster. +Now we can deploy cortex to the `cortex-home` cluster, configuring it to use the `cortex-remote-*` clusters for CRD storage. We create a temporary Helm values override file that specifies the API server URLs and root CA certificate for the `cortex-remote-*` clusters. In this example, we are configuring the `kvm.cloud.sap/v1/Hypervisor` resource to be stored in the `cortex-remote-*` clusters. ```bash export TILT_OVERRIDES_PATH=/tmp/cortex-values.yaml tee $TILT_OVERRIDES_PATH < /tmp/root-ca-home.pem + +echo "Creating az-a and az-b clusters" +kind create cluster --config docs/guides/multicluster/cortex-remote-az-a.yaml +kind create cluster --config docs/guides/multicluster/cortex-remote-az-b.yaml + +echo "Granting cortex-home sa tokens access to az-a and az-b clusters" +kubectl --context kind-cortex-remote-az-a apply -f docs/guides/multicluster/cortex-remote-crb.yaml +kubectl --context kind-cortex-remote-az-b apply -f docs/guides/multicluster/cortex-remote-crb.yaml + +echo "Installing cortex crds in az-a and az-b clusters" +kubectl config use-context kind-cortex-remote-az-a +helm install helm/bundles/cortex-crds --generate-name +kubectl config use-context kind-cortex-remote-az-b +helm install helm/bundles/cortex-crds --generate-name + +echo "Installing hypervisor crd as external dependency to all three clusters" +curl -L https://raw.githubusercontent.com/cobaltcore-dev/openstack-hypervisor-operator/refs/heads/main/charts/openstack-hypervisor-operator/crds/kvm.cloud.sap_hypervisors.yaml > /tmp/hypervisor-crd.yaml +kubectl --context kind-cortex-home apply -f /tmp/hypervisor-crd.yaml +kubectl --context kind-cortex-remote-az-a apply -f /tmp/hypervisor-crd.yaml +kubectl --context kind-cortex-remote-az-b apply -f /tmp/hypervisor-crd.yaml + +echo "Storing az-a and az-b cluster certs under /tmp/root-ca-remote-az-a.pem and /tmp/root-ca-remote-az-b.pem" +kubectl --context kind-cortex-remote-az-a --namespace kube-system \ + get configmap extension-apiserver-authentication \ + -o jsonpath="{.data['client-ca-file']}" > /tmp/root-ca-remote-az-a.pem +kubectl --context kind-cortex-remote-az-b --namespace kube-system \ + get configmap extension-apiserver-authentication \ + -o jsonpath="{.data['client-ca-file']}" > /tmp/root-ca-remote-az-b.pem + +echo "Setting up tilt overrides for cortex values" +export TILT_OVERRIDES_PATH=/tmp/cortex-values.yaml +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" + // Apiserver configuration mapping GVKs to home or remote clusters. + // Every GVK used through the multicluster client must be listed + // in either Home or Remotes. Unknown GVKs will cause an error. + APIServers APIServersConfig `json:"apiservers"` +} + +// APIServersConfig separates resources into home and remote clusters. +type APIServersConfig struct { + // Resources managed in the cluster where cortex is deployed. + Home HomeConfig `json:"home"` + // Resources managed in remote clusters. + Remotes []RemoteConfig `json:"remotes,omitempty"` +} + +// HomeConfig lists GVKs that are managed in the home cluster. +type HomeConfig struct { + // The resource GVKs formatted as "//". + GVKs []string `json:"gvks"` +} + +// RemoteConfig maps multiple GVKs to a remote kubernetes apiserver with +// routing labels. It is assumed that the remote apiserver accepts the +// serviceaccount tokens issued by the local cluster. +type RemoteConfig struct { + // The remote kubernetes apiserver url, e.g. "https://my-apiserver:6443". Host string `json:"host"` // The root CA certificate to verify the remote apiserver. CACert string `json:"caCert,omitempty"` + // The resource GVKs this apiserver serves, formatted as "//". + GVKs []string `json:"gvks"` + // Labels used by ResourceRouters to match resources to this cluster + // for write operations (Create/Update/Delete/Patch). + Labels map[string]string `json:"labels,omitempty"` } // Helper function to initialize a new multicluster client during service startup, @@ -59,29 +91,39 @@ func (c *Client) InitFromConf(ctx context.Context, mgr ctrl.Manager, conf Client log.Info("initializing multicluster client with config", "config", conf) // 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) + gvksByConfStr := make(map[string]schema.GroupVersionKind) for gvk := range c.HomeScheme.AllKnownTypes() { - // 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 gvksByConfStr[formatted] = gvk } for gvkStr := range gvksByConfStr { log.Info("scheme gvk registered", "gvk", gvkStr) } - for _, override := range conf.APIServerOverrides { - // Check if we have any registered gvk for this API server override. - gvk, ok := gvksByConfStr[override.GVK] + // Parse home GVKs. + c.homeGVKs = make(map[schema.GroupVersionKind]bool) + for _, gvkStr := range conf.APIServers.Home.GVKs { + gvk, ok := gvksByConfStr[gvkStr] if !ok { - return errors.New("no gvk registered for API server override " + override.GVK) + return errors.New("no gvk registered for home " + gvkStr) + } + log.Info("registering home gvk", "gvk", gvk) + c.homeGVKs[gvk] = true + } + // Parse remote apiserver configs. + for _, remote := range conf.APIServers.Remotes { + var resolvedGVKs []schema.GroupVersionKind + for _, gvkStr := range remote.GVKs { + gvk, ok := gvksByConfStr[gvkStr] + if !ok { + return errors.New("no gvk registered for remote apiserver " + gvkStr) + } + resolvedGVKs = append(resolvedGVKs, gvk) } - cluster, err := c.AddRemote(ctx, override.Host, override.CACert, gvk) + cl, err := c.AddRemote(ctx, remote.Host, remote.CACert, remote.Labels, resolvedGVKs...) if err != nil { return err } - // Also tell the manager about this cluster so that controllers can use it. - // This will execute the cluster.Start function when the manager starts. - if err := mgr.Add(cluster); err != nil { + if err := mgr.Add(cl); err != nil { return err } } @@ -94,12 +136,11 @@ func (c *Client) InitFromConf(ctx context.Context, mgr ctrl.Manager, conf Client // 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, gvks ...schema.GroupVersionKind) (cluster.Cluster, error) { +func (c *Client) AddRemote(ctx context.Context, host, caCert string, labels map[string]string, gvks ...schema.GroupVersionKind) (cluster.Cluster, error) { log := ctrl.LoggerFrom(ctx) homeRestConfig := *c.HomeRestConfig restConfigCopy := homeRestConfig restConfigCopy.Host = host - // This must be the CA data for the remote apiserver. restConfigCopy.CAData = []byte(caCert) cl, err := cluster.New(&restConfigCopy, func(o *cluster.Options) { o.Scheme = c.HomeScheme @@ -110,16 +151,19 @@ func (c *Client) AddRemote(ctx context.Context, host, caCert string, gvks ...sch c.remoteClustersMu.Lock() defer c.remoteClustersMu.Unlock() if c.remoteClusters == nil { - c.remoteClusters = make(map[schema.GroupVersionKind]cluster.Cluster) + c.remoteClusters = make(map[schema.GroupVersionKind][]remoteCluster) } for _, gvk := range gvks { - log.Info("adding remote cluster for resource", "gvk", gvk, "host", host) - c.remoteClusters[gvk] = cl + log.Info("adding remote cluster for resource", "gvk", gvk, "host", host, "labels", labels) + c.remoteClusters[gvk] = append(c.remoteClusters[gvk], remoteCluster{ + cluster: cl, + labels: labels, + }) } return cl, nil } -// Get the gvk registered for the given resource in the home cluster's RESTMapper. +// Get the gvk registered for the given resource in the home cluster's scheme. func (c *Client) GVKFromHomeScheme(obj runtime.Object) (gvk schema.GroupVersionKind, err error) { gvks, unversioned, err := c.HomeScheme.ObjectKinds(obj) if err != nil { @@ -134,115 +178,236 @@ func (c *Client) GVKFromHomeScheme(obj runtime.Object) (gvk schema.GroupVersionK return gvks[0], nil } -// Get the cluster for the given group version kind. -// -// 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 { +// ClustersForGVK returns all clusters that serve the given GVK. +// The GVK must be explicitly configured in either homeGVKs or remoteClusters. +// Returns an error if the GVK is unknown. +func (c *Client) ClustersForGVK(gvk schema.GroupVersionKind) ([]cluster.Cluster, error) { c.remoteClustersMu.RLock() defer c.remoteClustersMu.RUnlock() - cl, ok := c.remoteClusters[gvk] - if ok { - return cl + remotes := c.remoteClusters[gvk] + isHome := c.homeGVKs[gvk] + if len(remotes) == 0 && !isHome { + return nil, fmt.Errorf("GVK %s is not configured in home or any remote cluster", gvk) + } + clusters := make([]cluster.Cluster, 0, len(remotes)+1) + for _, r := range remotes { + clusters = append(clusters, r.cluster) + } + if isHome && c.HomeCluster != nil { + clusters = append(clusters, c.HomeCluster) } - return c.HomeCluster + return clusters, nil } -// Get the client for the given resource URI. +// clusterForWrite uses a ResourceRouter to determine which remote cluster +// a resource should be written to based on the resource content and cluster labels. // -// 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. - ClusterForResource(gvk). - GetClient() +// The GVK must be explicitly configured. If configured for home, the home cluster +// is returned. If configured for remotes, the ResourceRouter determines the target. +// Returns an error if the GVK is unknown or no remote cluster matches. +func (c *Client) clusterForWrite(gvk schema.GroupVersionKind, obj any) (cluster.Cluster, error) { + c.remoteClustersMu.RLock() + defer c.remoteClustersMu.RUnlock() + + remotes := c.remoteClusters[gvk] + + if len(remotes) > 0 { + router, ok := c.ResourceRouters[gvk] + if !ok { + return nil, fmt.Errorf("no ResourceRouter configured for GVK %s with %d remote clusters", gvk, len(remotes)) + } + for _, r := range remotes { + match, err := router.Match(obj, r.labels) + if err != nil { + return nil, fmt.Errorf("resource router match error for GVK %s: %w", gvk, err) + } + if match { + return r.cluster, nil + } + } + } + + // If we couldn't find a matching remote cluster (not configured or not found) but the GVK is configured for home, return the home cluster. + if c.homeGVKs[gvk] { + return c.HomeCluster, nil + } + return nil, fmt.Errorf("no cluster matched for GVK %s", gvk) } -// 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. +// Get iterates over all clusters with the GVK and returns the result. +// Returns an error if the resource is found in multiple clusters (duplicate). +// If no cluster has the resource, a NotFound error is returned. +// Non-NotFound errors from individual clusters are logged and silently skipped +// so that a single unavailable cluster does not block the entire read path. func (c *Client) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + log := ctrl.LoggerFrom(ctx) gvk, err := c.GVKFromHomeScheme(obj) if err != nil { return err } - return c.ClientForResource(gvk).Get(ctx, key, obj, opts...) + clusters, err := c.ClustersForGVK(gvk) + if err != nil { + return err + } + found := false + for _, cl := range clusters { + // If we already found the resource in a previous cluster, we want to check if it also exists in this cluster to detect duplicates. + if found { + candidate := obj.DeepCopyObject().(client.Object) + err := cl.GetClient().Get(ctx, key, candidate, opts...) + if err == nil { + return fmt.Errorf("duplicate resource found: %s %s/%s exists in multiple clusters", gvk, key.Namespace, key.Name) + } + if !apierrors.IsNotFound(err) { + log.Error(err, "error checking for duplicate resource in cluster", "gvk", gvk, "namespace", key.Namespace, "name", key.Name) + } + continue + } + + err := cl.GetClient().Get(ctx, key, obj, opts...) + if err == nil { + found = true + continue + } + if !apierrors.IsNotFound(err) { + log.Error(err, "error getting resource from cluster", "gvk", gvk, "namespace", key.Namespace, "name", key.Name) + } + } + if !found { + return apierrors.NewNotFound(schema.GroupResource{Group: gvk.Group, Resource: gvk.Kind}, key.Name) + } + return nil } -// 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. +// List iterates over all clusters with the GVK and returns a combined list. +// Returns an error if any resources share the same namespace/name across clusters. +// Errors from individual clusters are logged and silently skipped so that a +// single unavailable cluster does not block the entire read path. func (c *Client) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + log := ctrl.LoggerFrom(ctx) gvk, err := c.GVKFromHomeScheme(list) if err != nil { return err } - return c.ClientForResource(gvk).List(ctx, list, opts...) + clusters, err := c.ClustersForGVK(gvk) + if err != nil { + return err + } + + var allItems []runtime.Object + for _, cl := range clusters { + listCopy := list.DeepCopyObject().(client.ObjectList) + if err := cl.GetClient().List(ctx, listCopy, opts...); err != nil { + log.Error(err, "error listing resources from cluster", "gvk", gvk) + continue + } + items, err := meta.ExtractList(listCopy) + if err != nil { + return err + } + allItems = append(allItems, items...) + } + + // Check for duplicate namespace/name pairs across clusters. + seen := make(map[string]bool, len(allItems)) + var duplicates []string + for _, item := range allItems { + accessor, err := meta.Accessor(item) + if err != nil { + return fmt.Errorf("failed to access object metadata: %w", err) + } + key := accessor.GetNamespace() + "/" + accessor.GetName() + if _, exists := seen[key]; exists { + duplicates = append(duplicates, key) + continue + } + seen[key] = true + } + if len(duplicates) > 0 { + return fmt.Errorf("duplicate resources found in multiple clusters for %s: %v", gvk, duplicates) + } + + return meta.SetList(list, allItems) } // 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. +// cannot be inferred from the ApplyConfiguration. func (c *Client) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...client.ApplyOption) error { return errors.New("apply operation is not supported in multicluster client") } -// 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. +// Create routes the object to the matching cluster using the ResourceRouter +// and performs a Create operation. func (c *Client) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { gvk, err := c.GVKFromHomeScheme(obj) if err != nil { return err } - return c.ClientForResource(gvk).Create(ctx, obj, opts...) + cl, err := c.clusterForWrite(gvk, obj) + if err != nil { + return err + } + return cl.GetClient().Create(ctx, obj, opts...) } -// 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. +// Delete routes the object to the matching cluster using the ResourceRouter +// and performs a Delete operation. func (c *Client) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { gvk, err := c.GVKFromHomeScheme(obj) if err != nil { return err } - return c.ClientForResource(gvk).Delete(ctx, obj, opts...) + cl, err := c.clusterForWrite(gvk, obj) + if err != nil { + return err + } + return cl.GetClient().Delete(ctx, obj, opts...) } -// 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. +// Update routes the object to the matching cluster using the ResourceRouter +// and performs an Update operation. func (c *Client) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { gvk, err := c.GVKFromHomeScheme(obj) if err != nil { return err } - return c.ClientForResource(gvk).Update(ctx, obj, opts...) + cl, err := c.clusterForWrite(gvk, obj) + if err != nil { + return err + } + return cl.GetClient().Update(ctx, obj, opts...) } -// 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. +// Patch routes the object to the matching cluster using the ResourceRouter +// and performs a Patch operation. func (c *Client) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { gvk, err := c.GVKFromHomeScheme(obj) if err != nil { return err } - return c.ClientForResource(gvk).Patch(ctx, obj, patch, opts...) + cl, err := c.clusterForWrite(gvk, obj) + if err != nil { + return err + } + return cl.GetClient().Patch(ctx, obj, patch, opts...) } -// 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. +// DeleteAllOf iterates over all clusters with the GVK and performs DeleteAllOf on each. func (c *Client) DeleteAllOf(ctx context.Context, obj client.Object, opts ...client.DeleteAllOfOption) error { gvk, err := c.GVKFromHomeScheme(obj) if err != nil { return err } - return c.ClientForResource(gvk).DeleteAllOf(ctx, obj, opts...) + clusters, err := c.ClustersForGVK(gvk) + if err != nil { + return err + } + for _, cl := range clusters { + if err := cl.GetClient().DeleteAllOf(ctx, obj, opts...); err != nil { + return err + } + } + return nil } // Return the scheme of the home cluster. @@ -269,49 +434,50 @@ func (c *Client) IsObjectNamespaced(obj runtime.Object) (bool, error) { // 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 type. +// Wrapper around the status subresource client which routes to the correct cluster. type statusClient struct{ multiclusterClient *Client } -// 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. +// Create routes the status create to the matching cluster using the ResourceRouter. func (c *statusClient) Create(ctx context.Context, obj, subResource client.Object, opts ...client.SubResourceCreateOption) error { gvk, err := c.multiclusterClient.GVKFromHomeScheme(obj) if err != nil { return err } - return c.multiclusterClient.ClientForResource(gvk). - Status().Create(ctx, obj, subResource, opts...) + cl, err := c.multiclusterClient.clusterForWrite(gvk, obj) + if err != nil { + return err + } + return cl.GetClient().Status().Create(ctx, obj, subResource, opts...) } -// 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. +// Update routes the status update to the matching cluster using the ResourceRouter. func (c *statusClient) Update(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { gvk, err := c.multiclusterClient.GVKFromHomeScheme(obj) if err != nil { return err } - return c.multiclusterClient.ClientForResource(gvk). - Status().Update(ctx, obj, opts...) + cl, err := c.multiclusterClient.clusterForWrite(gvk, obj) + if err != nil { + return err + } + return cl.GetClient().Status().Update(ctx, obj, opts...) } -// 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. +// Patch routes the status patch to the matching cluster using the ResourceRouter. func (c *statusClient) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.SubResourcePatchOption) error { gvk, err := c.multiclusterClient.GVKFromHomeScheme(obj) if err != nil { return err } - return c.multiclusterClient.ClientForResource(gvk). - Status().Patch(ctx, obj, patch, opts...) + cl, err := c.multiclusterClient.clusterForWrite(gvk, obj) + if err != nil { + return err + } + return cl.GetClient().Status().Patch(ctx, obj, patch, opts...) } // 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. +// cannot be inferred from the ApplyConfiguration. func (c *statusClient) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...client.SubResourceApplyOption) error { return errors.New("apply operation is not supported in multicluster status client") } @@ -325,99 +491,142 @@ func (c *Client) SubResource(subResource string) client.SubResourceClient { } } -// Wrapper around a subresource client which picks the right cluster -// based on the resource type. +// Wrapper around a subresource client which routes to the correct cluster. type subResourceClient struct { - // The parent multicluster client. multiclusterClient *Client - // The name of the subresource. - subResource string + subResource string } -// 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. +// Get iterates over all clusters with the GVK and returns the result. +// Returns an error if the resource is found in multiple clusters (duplicate). func (c *subResourceClient) Get(ctx context.Context, obj, subResource client.Object, opts ...client.SubResourceGetOption) error { + log := ctrl.LoggerFrom(ctx) + gvk, err := c.multiclusterClient.GVKFromHomeScheme(obj) if err != nil { return err } - return c.multiclusterClient.ClientForResource(gvk). - SubResource(c.subResource).Get(ctx, obj, subResource, opts...) + clusters, err := c.multiclusterClient.ClustersForGVK(gvk) + if err != nil { + return err + } + + found := false + for _, cl := range clusters { + if found { + candidateObj := obj.DeepCopyObject().(client.Object) + candidateSub := subResource.DeepCopyObject().(client.Object) + err := cl.GetClient().SubResource(c.subResource).Get(ctx, candidateObj, candidateSub, opts...) + if err == nil { + return fmt.Errorf("duplicate sub-resource found: %s %s/%s exists in multiple clusters", gvk, obj.GetNamespace(), obj.GetName()) + } + if !apierrors.IsNotFound(err) { + log.Error(err, "error checking for duplicate sub-resource in cluster", "gvk", gvk, "namespace", obj.GetNamespace(), "name", obj.GetName(), "subresource", c.subResource) + } + continue + } + + err := cl.GetClient().SubResource(c.subResource).Get(ctx, obj, subResource, opts...) + if err == nil { + found = true + continue + } + if !apierrors.IsNotFound(err) { + log.Error(err, "error getting sub-resource from cluster", "gvk", gvk, "namespace", obj.GetNamespace(), "name", obj.GetName(), "subresource", c.subResource) + } + } + if !found { + return apierrors.NewNotFound(schema.GroupResource{Group: gvk.Group, Resource: gvk.Kind}, obj.GetName()) + } + return nil } -// 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. +// Create routes the subresource create to the matching cluster using the ResourceRouter. func (c *subResourceClient) Create(ctx context.Context, obj, subResource client.Object, opts ...client.SubResourceCreateOption) error { gvk, err := c.multiclusterClient.GVKFromHomeScheme(obj) if err != nil { return err } - return c.multiclusterClient.ClientForResource(gvk). - SubResource(c.subResource).Create(ctx, obj, subResource, opts...) + cl, err := c.multiclusterClient.clusterForWrite(gvk, obj) + if err != nil { + return err + } + return cl.GetClient().SubResource(c.subResource).Create(ctx, obj, subResource, opts...) } -// 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. +// Update routes the subresource update to the matching cluster using the ResourceRouter. func (c *subResourceClient) Update(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { gvk, err := c.multiclusterClient.GVKFromHomeScheme(obj) if err != nil { return err } - return c.multiclusterClient.ClientForResource(gvk). - SubResource(c.subResource).Update(ctx, obj, opts...) + cl, err := c.multiclusterClient.clusterForWrite(gvk, obj) + if err != nil { + return err + } + return cl.GetClient().SubResource(c.subResource).Update(ctx, obj, opts...) } -// 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. +// Patch routes the subresource patch to the matching cluster using the ResourceRouter. func (c *subResourceClient) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.SubResourcePatchOption) error { gvk, err := c.multiclusterClient.GVKFromHomeScheme(obj) if err != nil { return err } - return c.multiclusterClient.ClientForResource(gvk). - SubResource(c.subResource).Patch(ctx, obj, patch, opts...) + cl, err := c.multiclusterClient.clusterForWrite(gvk, obj) + if err != nil { + return err + } + return cl.GetClient().SubResource(c.subResource).Patch(ctx, obj, patch, opts...) } // 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. +// cannot be inferred from the ApplyConfiguration. func (c *subResourceClient) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...client.SubResourceApplyOption) error { return errors.New("apply operation is not supported in multicluster subresource client") } -// Index a field for a resource in the matching cluster's cache. +// Index a field for a resource in all matching cluster caches. // Usually, you want to index the same field in both the object and list type, // as both would be mapped to individual clients based on their GVK. func (c *Client) IndexField(ctx context.Context, obj client.Object, list client.ObjectList, field string, extractValue client.IndexerFunc) error { - gvkObj, err := c.GVKFromHomeScheme(obj) + gvk, err := c.GVKFromHomeScheme(obj) if err != nil { return err } - objCluster := c.ClusterForResource(gvkObj) - if err := objCluster. - GetCache(). - IndexField(ctx, obj, field, extractValue); err != nil { + gvkList, err := c.GVKFromHomeScheme(list) + if err != nil { return err } - // Index the object in the list cluster as well. - gvkList, err := c.GVKFromHomeScheme(list) + // Collect all unique caches to index. + indexed := make(map[any]bool) + clusters, err := c.ClustersForGVK(gvk) if err != nil { return err } - objListCluster := c.ClusterForResource(gvkList) - // If the object and list map to the same cluster, we have already indexed - // the field above and re-defining the index will lead to an indexer conflict. - if objCluster == objListCluster { - return nil + for _, cl := range clusters { + ch := cl.GetCache() + if indexed[ch] { + continue + } + indexed[ch] = true + if err := ch.IndexField(ctx, obj, field, extractValue); err != nil { + return err + } } - if err := objListCluster. - GetCache(). - IndexField(ctx, obj, field, extractValue); err != nil { + clustersList, err := c.ClustersForGVK(gvkList) + if err != nil { return err } + for _, cl := range clustersList { + ch := cl.GetCache() + if indexed[ch] { + continue + } + indexed[ch] = true + if err := ch.IndexField(ctx, obj, field, extractValue); err != nil { + return err + } + } return nil } diff --git a/pkg/multicluster/client_test.go b/pkg/multicluster/client_test.go index 74d595e78..64b0ae94c 100644 --- a/pkg/multicluster/client_test.go +++ b/pkg/multicluster/client_test.go @@ -5,6 +5,7 @@ package multicluster import ( "context" + "strings" "sync" "testing" @@ -44,8 +45,7 @@ func (u *unknownType) DeepCopyObject() runtime.Object { // fakeCache implements cache.Cache interface for testing IndexField. type fakeCache struct { cache.Cache - indexFieldFunc func(ctx context.Context, obj client.Object, field string, extractValue client.IndexerFunc) error - // Track calls to IndexField for verification + indexFieldFunc func(ctx context.Context, obj client.Object, field string, extractValue client.IndexerFunc) error indexFieldCalls []indexFieldCall mu sync.Mutex } @@ -113,289 +113,367 @@ func newTestScheme(t *testing.T) *runtime.Scheme { return scheme } -// TestClient_Apply tests that the Apply method returns an error. -func TestClient_Apply(t *testing.T) { - scheme := newTestScheme(t) +// testRouter is a simple ResourceRouter for testing. +type testRouter struct{} - c := &Client{ - HomeScheme: scheme, - } +// alwaysMatchRouter matches any object to any cluster. +type alwaysMatchRouter struct{} - ctx := context.Background() +func (r alwaysMatchRouter) Match(any, map[string]string) (bool, error) { + return true, nil +} - 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 (r testRouter) Match(obj any, labels map[string]string) (bool, error) { + cm, ok := obj.(*corev1.ConfigMap) + if !ok { + return false, nil + } + az, ok := labels["az"] + if !ok { + return false, nil + } + objAZ, ok := cm.Labels["az"] + if !ok { + return false, nil + } + return objAZ == az, nil } -// TestStatusClient_Apply tests that the status client Apply method returns an error. -func TestStatusClient_Apply(t *testing.T) { - sc := &statusClient{multiclusterClient: &Client{}} +var configMapGVK = schema.GroupVersionKind{Group: "", Version: "v1", Kind: "ConfigMap"} +var configMapListGVK = schema.GroupVersionKind{Group: "", Version: "v1", Kind: "ConfigMapList"} +var podGVK = schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"} - ctx := context.Background() +func TestClient_Apply(t *testing.T) { + c := &Client{HomeScheme: newTestScheme(t)} - err := sc.Apply(ctx, nil) + // Check if apply will throw an error since it's not supported by multicluster client. + err := c.Apply(context.Background(), 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) - } } -// TestSubResourceClientApply tests that the subresource client Apply method returns an error. -func TestSubResourceClientApply(t *testing.T) { - src := &subResourceClient{ - multiclusterClient: &Client{}, - subResource: "status", +func TestStatusClient_Apply(t *testing.T) { + sc := &statusClient{multiclusterClient: &Client{}} + + // Check if apply will throw an error since it's not supported by multicluster client. + err := sc.Apply(context.Background(), nil) + if err == nil { + t.Error("expected error for Apply operation") } +} - ctx := context.Background() +func TestSubResourceClient_Apply(t *testing.T) { + src := &subResourceClient{multiclusterClient: &Client{}, subResource: "status"} - err := src.Apply(ctx, nil) + // Check if apply will throw an error since it's not supported by multicluster client. + err := src.Apply(context.Background(), 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) - } } -// TestClient_ClusterForResource_NilRemoteClusters tests behavior when no remote clusters are configured. -func TestClient_ClusterForResource_NilRemoteClusters(t *testing.T) { +func TestClient_ClustersForGVK_HomeGVKOnly(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) c := &Client{ - remoteClusters: nil, + HomeCluster: homeCluster, + HomeScheme: scheme, + homeGVKs: map[schema.GroupVersionKind]bool{configMapGVK: true}, } - gvk := schema.GroupVersionKind{ - Group: "test", - Version: "v1", - Kind: "TestKind", + clusters, err := c.ClustersForGVK(configMapGVK) + if err != nil { + t.Fatalf("unexpected error: %v", err) } - - // 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") + if len(clusters) != 1 { + t.Fatalf("expected 1 cluster, got %d", len(clusters)) + } + if clusters[0] != homeCluster { + t.Error("expected home cluster") } } -// TestClient_ClusterForResource_EmptyRemoteClusters tests behavior with empty remote clusters map. -func TestClient_ClusterForResource_EmptyRemoteClusters(t *testing.T) { +func TestClient_ClustersForGVK_UnknownGVK(t *testing.T) { + scheme := newTestScheme(t) c := &Client{ - remoteClusters: make(map[schema.GroupVersionKind]cluster.Cluster), + HomeCluster: newFakeCluster(scheme), + HomeScheme: scheme, } - gvk := schema.GroupVersionKind{ - Group: "test", - Version: "v1", - Kind: "TestKind", + _, err := c.ClustersForGVK(configMapGVK) + if err == nil { + t.Error("expected error for unknown GVK") } +} - // 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") +func TestClient_ClustersForGVK_SingleRemoteCluster(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) + remote := newFakeCluster(scheme) + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: {{cluster: remote, labels: map[string]string{"az": "az-1"}}}, + }, } -} -// TestClient_Status returns a status writer. -func TestClient_Status(t *testing.T) { - c := &Client{} + clusters, err := c.ClustersForGVK(configMapGVK) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(clusters) != 1 { + t.Fatalf("expected 1 cluster, got %d", len(clusters)) + } + if clusters[0] != remote { + t.Error("expected remote cluster") + } +} - status := c.Status() - if status == nil { - t.Error("expected non-nil status writer") +func TestClient_ClustersForGVK_MultipleRemoteClusters(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) + remote1 := newFakeCluster(scheme) + remote2 := newFakeCluster(scheme) + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: { + {cluster: remote1, labels: map[string]string{"az": "az-1"}}, + {cluster: remote2, labels: map[string]string{"az": "az-2"}}, + }, + }, } - // Verify it's the right type - if _, ok := status.(*statusClient); !ok { - t.Error("expected statusClient type") + clusters, err := c.ClustersForGVK(configMapGVK) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(clusters) != 2 { + t.Fatalf("expected 2 clusters, got %d", len(clusters)) } } -// TestClient_SubResource returns a subresource client. -func TestClient_SubResource(t *testing.T) { - c := &Client{} +func TestClient_ClustersForGVK_HomeAndRemote(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) + remote := newFakeCluster(scheme) + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: {{cluster: remote, labels: map[string]string{"az": "az-1"}}}, + }, + homeGVKs: map[schema.GroupVersionKind]bool{configMapGVK: true}, + } - subResource := c.SubResource("scale") - if subResource == nil { - t.Error("expected non-nil subresource client") + clusters, err := c.ClustersForGVK(configMapGVK) + if err != nil { + t.Fatalf("unexpected error: %v", err) } + if len(clusters) != 2 { + t.Fatalf("expected 2 clusters (remote + home), got %d", len(clusters)) + } + if clusters[0] != remote { + t.Error("expected remote cluster first") + } + if clusters[1] != homeCluster { + t.Error("expected home cluster second") + } +} - // Verify it's the right type - src, ok := subResource.(*subResourceClient) - if !ok { - t.Error("expected subResourceClient type") +func TestClient_clusterForWrite_HomeGVK(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + homeGVKs: map[schema.GroupVersionKind]bool{configMapGVK: true}, } - if src.subResource != "scale" { - t.Errorf("expected subResource='scale', got '%s'", src.subResource) + cl, err := c.clusterForWrite(configMapGVK, &corev1.ConfigMap{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cl != homeCluster { + t.Error("expected home cluster for home GVK") } } -// TestClient_AddRemote_NilRemoteClusters initializes the remote clusters map. -func TestClient_AddRemote_NilRemoteClusters(t *testing.T) { +func TestClient_clusterForWrite_UnknownGVK(t *testing.T) { + scheme := newTestScheme(t) c := &Client{ - remoteClusters: nil, + HomeCluster: newFakeCluster(scheme), + HomeScheme: scheme, + } + + _, err := c.clusterForWrite(configMapGVK, &corev1.ConfigMap{}) + if err == nil { + t.Error("expected error for unknown GVK") } +} - // Just verify the lock mechanism works without panicking - c.remoteClustersMu.Lock() - if c.remoteClusters == nil { - c.remoteClusters = make(map[schema.GroupVersionKind]cluster.Cluster) +func TestClient_clusterForWrite_SingleRemoteCluster(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) + remote := newFakeCluster(scheme) + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + ResourceRouters: map[schema.GroupVersionKind]ResourceRouter{ + configMapGVK: testRouter{}, + }, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: {{cluster: remote, labels: map[string]string{"az": "az-1"}}}, + }, } - c.remoteClustersMu.Unlock() - // Should not panic - if c.remoteClusters == nil { - t.Error("expected remoteClusters to be initialized") + cl, err := c.clusterForWrite(configMapGVK, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"az": "az-1"}}, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cl != remote { + t.Error("expected remote cluster for single remote") } } -// TestClient_ConcurrentAccess tests thread safety of ClusterForResource. -func TestClient_ConcurrentAccess(t *testing.T) { +func TestClient_clusterForWrite_NoRouterSingleRemote(t *testing.T) { + scheme := newTestScheme(t) c := &Client{ - remoteClusters: make(map[schema.GroupVersionKind]cluster.Cluster), + HomeCluster: newFakeCluster(scheme), + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: {{cluster: newFakeCluster(scheme)}}, + }, } - gvk := schema.GroupVersionKind{ - Group: "test", - Version: "v1", - Kind: "TestKind", + _, err := c.clusterForWrite(configMapGVK, &corev1.ConfigMap{}) + if err == nil { + t.Error("expected error when no router configured for remote cluster") } +} - // Test concurrent reads - should not panic or race - done := make(chan bool) - for range 10 { - go func() { - _ = c.ClusterForResource(gvk) - done <- true - }() +func TestClient_clusterForWrite_NoMatchFallsBackToHome(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + ResourceRouters: map[schema.GroupVersionKind]ResourceRouter{ + configMapGVK: testRouter{}, + }, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: {{cluster: newFakeCluster(scheme), labels: map[string]string{"az": "az-1"}}}, + }, + homeGVKs: map[schema.GroupVersionKind]bool{configMapGVK: true}, } - for range 10 { - <-done + cl, err := c.clusterForWrite(configMapGVK, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"az": "az-99"}}, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cl != homeCluster { + t.Error("expected home cluster as fallback when no remote matches") } } -// 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", +func TestClient_clusterForWrite_RouterMatches(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) + remote1 := newFakeCluster(scheme) + remote2 := newFakeCluster(scheme) + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + ResourceRouters: map[schema.GroupVersionKind]ResourceRouter{ + configMapGVK: testRouter{}, + }, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: { + {cluster: remote1, labels: map[string]string{"az": "az-1"}}, + {cluster: remote2, labels: map[string]string{"az": "az-2"}}, + }, }, } - key := client.ObjectKeyFromObject(cm) - if key.Name != "test-config" { - t.Errorf("expected Name='test-config', got '%s'", key.Name) + obj := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"az": "az-2"}}, + } + cl, err := c.clusterForWrite(configMapGVK, obj) + if err != nil { + t.Fatalf("unexpected error: %v", err) } - if key.Namespace != "default" { - t.Errorf("expected Namespace='default', got '%s'", key.Namespace) + if cl != remote2 { + t.Error("expected second remote cluster for az-2") } } -// 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", +func TestClient_clusterForWrite_NoMatch(t *testing.T) { + scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) + remote1 := newFakeCluster(scheme) + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + ResourceRouters: map[schema.GroupVersionKind]ResourceRouter{ + configMapGVK: testRouter{}, + }, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: { + {cluster: remote1, labels: map[string]string{"az": "az-1"}}, + {cluster: newFakeCluster(scheme), labels: map[string]string{"az": "az-2"}}, + }, + }, } - cm.SetGroupVersionKind(gvk) - - result := cm.GetObjectKind().GroupVersionKind() - if result != gvk { - t.Errorf("expected GVK %v, got %v", gvk, result) + obj := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"az": "az-3"}}, + } + _, err := c.clusterForWrite(configMapGVK, obj) + if err == nil { + t.Error("expected error when no remote cluster matches") } } -// TestGVKFromHomeScheme_Success tests successful GVK lookup for registered types. -func TestGVKFromHomeScheme_Success(t *testing.T) { +func TestClient_clusterForWrite_NoRouterMultipleClusters(t *testing.T) { scheme := newTestScheme(t) - + homeCluster := newFakeCluster(scheme) c := &Client{ - HomeScheme: scheme, + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: { + {cluster: newFakeCluster(scheme), labels: map[string]string{"az": "az-1"}}, + {cluster: newFakeCluster(scheme), labels: map[string]string{"az": "az-2"}}, + }, + }, } + _, err := c.clusterForWrite(configMapGVK, &corev1.ConfigMap{}) + if err == nil { + t.Error("expected error when no router with multiple clusters") + } +} + +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", - }, - }, + {"ConfigMap", &corev1.ConfigMap{}, configMapGVK}, + {"ConfigMapList", &corev1.ConfigMapList{}, configMapListGVK}, + {"Decision", &v1alpha1.Decision{}, schema.GroupVersionKind{Group: "cortex.cloud", Version: "v1alpha1", Kind: "Decision"}}, + {"DecisionList", &v1alpha1.DecisionList{}, schema.GroupVersionKind{Group: "cortex.cloud", Version: "v1alpha1", Kind: "DecisionList"}}, } for _, tt := range tests { @@ -411,457 +489,565 @@ func TestGVKFromHomeScheme_Success(t *testing.T) { } } -// 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) + c := &Client{HomeScheme: newTestScheme(t)} + _, err := c.GVKFromHomeScheme(&unknownType{}) 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) + c := &Client{HomeScheme: scheme} + _, err := c.GVKFromHomeScheme(&unversionedType{}) 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 + c := &Client{HomeScheme: 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") + if _, err := c.GVKFromHomeScheme(&corev1.ConfigMap{}); err == nil { + t.Error("expected panic with nil scheme") } } -// TestClient_ClusterForResource_WithRemoteCluster tests ClusterForResource with a remote cluster configured. -func TestClient_ClusterForResource_WithRemoteCluster(t *testing.T) { +func TestClient_Get_SingleRemoteCluster(t *testing.T) { scheme := newTestScheme(t) - homeCluster := newFakeCluster(scheme) - remoteCluster := newFakeCluster(scheme) - - gvk := schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "ConfigMap", + existingCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "test-cm", Namespace: "default"}, + Data: map[string]string{"key": "remote-value"}, } + remote := newFakeCluster(scheme, existingCM) + homeCluster := newFakeCluster(scheme) 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") + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: {{cluster: remote}}, + }, } - // Should return home cluster for non-registered GVK - otherGVK := schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "Secret", + cm := &corev1.ConfigMap{} + err := c.Get(context.Background(), client.ObjectKey{Name: "test-cm", Namespace: "default"}, cm) + if err != nil { + t.Fatalf("unexpected error: %v", err) } - result = c.ClusterForResource(otherGVK) - if result != homeCluster { - t.Error("expected home cluster for non-registered GVK") + if cm.Data["key"] != "remote-value" { + t.Errorf("expected 'remote-value', got '%s'", cm.Data["key"]) } } -// TestClient_ClientForResource tests ClientForResource returns the correct client. -func TestClient_ClientForResource(t *testing.T) { - scheme := newTestScheme(t) - homeCluster := newFakeCluster(scheme) - remoteCluster := newFakeCluster(scheme) +func TestClient_Get_MultiCluster_SingleResult(t *testing.T) { + // Iterate all remote clusters and return the object. In this test, only remote2 has the object, so it should be returned. - gvk := schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "ConfigMap", + scheme := newTestScheme(t) + existingCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "test-cm", Namespace: "default"}, + Data: map[string]string{"key": "from-cluster-2"}, } + remote1 := newFakeCluster(scheme) // empty + remote2 := newFakeCluster(scheme, existingCM) 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") + HomeCluster: newFakeCluster(scheme), + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: { + {cluster: remote1, labels: map[string]string{"az": "az-1"}}, + {cluster: remote2, labels: map[string]string{"az": "az-2"}}, + }, + }, } - // Should return home cluster's client for non-registered GVK - otherGVK := schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "Secret", + cm := &corev1.ConfigMap{} + err := c.Get(context.Background(), client.ObjectKey{Name: "test-cm", Namespace: "default"}, cm) + if err != nil { + t.Fatalf("unexpected error: %v", err) } - result = c.ClientForResource(otherGVK) - if result != homeCluster.GetClient() { - t.Error("expected home cluster client for non-registered GVK") + if cm.Data["key"] != "from-cluster-2" { + t.Errorf("expected 'from-cluster-2', got '%s'", cm.Data["key"]) } } -// TestClient_Scheme tests that Scheme returns the home cluster's client scheme. -func TestClient_Scheme(t *testing.T) { +func TestClient_Get_MultiCluster_NotFound(t *testing.T) { + // Iterate all remote clusters and return NotFound if object is not found in any cluster. + // In this test, the object doesn't exist in any cluster, so NotFound should be returned. + scheme := newTestScheme(t) - homeCluster := newFakeCluster(scheme) + remote1 := newFakeCluster(scheme) // empty + remote2 := newFakeCluster(scheme) // empty c := &Client{ - HomeCluster: homeCluster, + HomeCluster: newFakeCluster(scheme), HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: { + {cluster: remote1}, + {cluster: remote2}, + }, + }, } - result := c.Scheme() - if result == nil { - t.Error("expected non-nil scheme") + cm := &corev1.ConfigMap{} + err := c.Get(context.Background(), client.ObjectKey{Name: "missing", Namespace: "default"}, cm) + if err == nil { + t.Error("expected NotFound error") } } -// TestClient_RESTMapper tests that RESTMapper returns the home cluster's client RESTMapper. -func TestClient_RESTMapper(t *testing.T) { +func TestClient_Get_UnknownType(t *testing.T) { scheme := newTestScheme(t) - homeCluster := newFakeCluster(scheme) - c := &Client{ - HomeCluster: homeCluster, + HomeCluster: newFakeCluster(scheme), HomeScheme: scheme, } - - result := c.RESTMapper() - if result == nil { - t.Error("expected non-nil RESTMapper") + obj := &unknownType{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}} + err := c.Get(context.Background(), client.ObjectKey{Name: "test", Namespace: "default"}, obj) + if err == nil { + t.Error("expected error for unknown type") } } -// TestClient_GroupVersionKindFor tests GroupVersionKindFor returns correct GVK. -func TestClient_GroupVersionKindFor(t *testing.T) { +func TestClient_Get_HomeGVKCluster(t *testing.T) { scheme := newTestScheme(t) - homeCluster := newFakeCluster(scheme) + homeCluster := newFakeCluster(scheme, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "home-cm", Namespace: "default"}, + Data: map[string]string{"key": "from-home"}, + }) + remote := newFakeCluster(scheme) // empty c := &Client{ HomeCluster: homeCluster, HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: {{cluster: remote}}, + }, + homeGVKs: map[schema.GroupVersionKind]bool{configMapGVK: true}, } - gvk, err := c.GroupVersionKindFor(&corev1.ConfigMap{}) + cm := &corev1.ConfigMap{} + err := c.Get(context.Background(), client.ObjectKey{Name: "home-cm", Namespace: "default"}, cm) 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) + if cm.Data["key"] != "from-home" { + t.Errorf("expected 'from-home', got '%s'", cm.Data["key"]) } } -// TestClient_IsObjectNamespaced tests IsObjectNamespaced delegates to home cluster client. -func TestClient_IsObjectNamespaced(t *testing.T) { +func TestClient_Get_MultiCluster_DuplicateError(t *testing.T) { + // If the same resource exists in multiple remote clusters, Get should return a duplicate error. scheme := newTestScheme(t) - homeCluster := newFakeCluster(scheme) + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "shared-cm", Namespace: "default"}, + Data: map[string]string{"key": "value"}, + } + remote1 := newFakeCluster(scheme, cm.DeepCopy()) + remote2 := newFakeCluster(scheme, cm.DeepCopy()) c := &Client{ - HomeCluster: homeCluster, + HomeCluster: newFakeCluster(scheme), HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: { + {cluster: remote1, labels: map[string]string{"az": "az-1"}}, + {cluster: remote2, labels: map[string]string{"az": "az-2"}}, + }, + }, } - // 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 + result := &corev1.ConfigMap{} + err := c.Get(context.Background(), client.ObjectKey{Name: "shared-cm", Namespace: "default"}, result) + if err == nil { + t.Fatal("expected duplicate error, got nil") + } + if !strings.Contains(err.Error(), "duplicate") { + t.Errorf("expected duplicate error, got: %v", err) + } } -// TestClient_Get tests the Get method routes to the correct cluster. -func TestClient_Get(t *testing.T) { +func TestClient_Get_HomeAndRemote_DuplicateError(t *testing.T) { + // If the same resource exists in both home and a remote cluster, Get should return a duplicate error. scheme := newTestScheme(t) + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "shared-cm", Namespace: "default"}, + Data: map[string]string{"key": "value"}, + } + homeCluster := newFakeCluster(scheme, cm.DeepCopy()) + remote := newFakeCluster(scheme, cm.DeepCopy()) - // Create a ConfigMap in the remote cluster - existingCM := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cm", - Namespace: "default", + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: {{cluster: remote, labels: map[string]string{"az": "az-1"}}}, }, - Data: map[string]string{"key": "remote-value"}, + homeGVKs: map[schema.GroupVersionKind]bool{configMapGVK: true}, } - remoteCluster := newFakeCluster(scheme, existingCM) + result := &corev1.ConfigMap{} + err := c.Get(context.Background(), client.ObjectKey{Name: "shared-cm", Namespace: "default"}, result) + if err == nil { + t.Fatal("expected duplicate error, got nil") + } + if !strings.Contains(err.Error(), "duplicate") { + t.Errorf("expected duplicate error, got: %v", err) + } +} + +func TestClient_List_SingleRemoteCluster(t *testing.T) { + scheme := newTestScheme(t) + cm1 := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cm1", Namespace: "default"}} + cm2 := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cm2", Namespace: "default"}} + remote := newFakeCluster(scheme, cm1, cm2) homeCluster := newFakeCluster(scheme) - gvk := schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "ConfigMap", + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapListGVK: {{cluster: remote}}, + }, + } + + cmList := &corev1.ConfigMapList{} + err := c.List(context.Background(), 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)) + } +} + +func TestClient_List_MultipleClusters_CombinesResults(t *testing.T) { + scheme := newTestScheme(t) + remote1 := newFakeCluster(scheme, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cm-az1-1", Namespace: "default"}}, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cm-az1-2", Namespace: "default"}}, + ) + remote2 := newFakeCluster(scheme, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cm-az2-1", Namespace: "default"}}, + ) c := &Client{ - HomeCluster: homeCluster, - HomeScheme: scheme, - remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, + HomeCluster: newFakeCluster(scheme), + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapListGVK: { + {cluster: remote1, labels: map[string]string{"az": "az-1"}}, + {cluster: remote2, labels: map[string]string{"az": "az-2"}}, + }, + }, } - ctx := context.Background() + cmList := &corev1.ConfigMapList{} + err := c.List(context.Background(), cmList, client.InNamespace("default")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(cmList.Items) != 3 { + t.Errorf("expected 3 combined items, got %d", len(cmList.Items)) + } +} - // Get from remote cluster (ConfigMap GVK is registered) - cm := &corev1.ConfigMap{} - err := c.Get(ctx, client.ObjectKey{Name: "test-cm", Namespace: "default"}, cm) +func TestClient_List_HomeGVKIncludesHome(t *testing.T) { + scheme := newTestScheme(t) + remote := newFakeCluster(scheme, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "remote-cm", Namespace: "default"}}, + ) + homeCluster := newFakeCluster(scheme, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "home-cm", Namespace: "default"}}, + ) + + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapListGVK: {{cluster: remote}}, + }, + homeGVKs: map[schema.GroupVersionKind]bool{configMapListGVK: true}, + } + + cmList := &corev1.ConfigMapList{} + err := c.List(context.Background(), cmList, client.InNamespace("default")) 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"]) + if len(cmList.Items) != 2 { + t.Errorf("expected 2 items (remote + home), got %d", len(cmList.Items)) } } -// TestClient_Get_UnknownType tests Get returns error for unknown types. -func TestClient_Get_UnknownType(t *testing.T) { +func TestClient_List_HomeClusterOnly(t *testing.T) { scheme := newTestScheme(t) - homeCluster := newFakeCluster(scheme) + homeCluster := newFakeCluster(scheme, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "home-cm", Namespace: "default"}}, + ) c := &Client{ HomeCluster: homeCluster, HomeScheme: scheme, + homeGVKs: map[schema.GroupVersionKind]bool{configMapListGVK: true}, } - ctx := context.Background() + cmList := &corev1.ConfigMapList{} + err := c.List(context.Background(), 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)) + } +} - obj := &unknownType{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", +func TestClient_List_MultipleClusters_DuplicateError(t *testing.T) { + // If the same namespace/name exists in multiple remote clusters, List should return a duplicate error. + scheme := newTestScheme(t) + remote1 := newFakeCluster(scheme, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "shared-cm", Namespace: "default"}}, + ) + remote2 := newFakeCluster(scheme, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "shared-cm", Namespace: "default"}}, + ) + + c := &Client{ + HomeCluster: newFakeCluster(scheme), + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapListGVK: { + {cluster: remote1, labels: map[string]string{"az": "az-1"}}, + {cluster: remote2, labels: map[string]string{"az": "az-2"}}, + }, }, } - err := c.Get(ctx, client.ObjectKey{Name: "test", Namespace: "default"}, obj) + cmList := &corev1.ConfigMapList{} + err := c.List(context.Background(), cmList, client.InNamespace("default")) if err == nil { - t.Error("expected error for unknown type") + t.Fatal("expected duplicate error, got nil") + } + if !strings.Contains(err.Error(), "duplicate") { + t.Errorf("expected duplicate error, got: %v", err) + } + if !strings.Contains(err.Error(), "default/shared-cm") { + t.Errorf("expected error to contain duplicated resource name, got: %v", err) } } -// TestClient_List tests the List method routes to the correct cluster. -func TestClient_List(t *testing.T) { +func TestClient_List_HomeAndRemote_DuplicateError(t *testing.T) { + // If the same namespace/name exists in both home and remote, List should return a duplicate error. scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "shared-cm", Namespace: "default"}}, + ) + remote := newFakeCluster(scheme, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "shared-cm", Namespace: "default"}}, + ) - // Create ConfigMaps in the remote cluster - cm1 := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cm1", - Namespace: "default", + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapListGVK: {{cluster: remote, labels: map[string]string{"az": "az-1"}}}, }, + homeGVKs: map[schema.GroupVersionKind]bool{configMapListGVK: true}, } - cm2 := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cm2", - Namespace: "default", - }, + + cmList := &corev1.ConfigMapList{} + err := c.List(context.Background(), cmList, client.InNamespace("default")) + if err == nil { + t.Fatal("expected duplicate error, got nil") } + if !strings.Contains(err.Error(), "duplicate") { + t.Errorf("expected duplicate error, got: %v", err) + } +} - remoteCluster := newFakeCluster(scheme, cm1, cm2) +func TestClient_Create_SingleRemoteCluster(t *testing.T) { + scheme := newTestScheme(t) homeCluster := newFakeCluster(scheme) - - gvk := schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "ConfigMapList", - } + remote := newFakeCluster(scheme) c := &Client{ - HomeCluster: homeCluster, - HomeScheme: scheme, - remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, + HomeCluster: homeCluster, + HomeScheme: scheme, + ResourceRouters: map[schema.GroupVersionKind]ResourceRouter{ + configMapGVK: alwaysMatchRouter{}, + }, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: {{cluster: remote}}, + }, } - ctx := context.Background() - - // List from remote cluster - cmList := &corev1.ConfigMapList{} - err := c.List(ctx, cmList, client.InNamespace("default")) + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "new-cm", Namespace: "default"}, + } + err := c.Create(context.Background(), cm) if err != nil { t.Fatalf("unexpected error: %v", err) } - if len(cmList.Items) != 2 { - t.Errorf("expected 2 items, got %d", len(cmList.Items)) + + // Verify it was created in remote, not home. + result := &corev1.ConfigMap{} + err = remote.GetClient().Get(context.Background(), client.ObjectKey{Name: "new-cm", Namespace: "default"}, result) + if err != nil { + t.Fatalf("expected to find in remote cluster: %v", err) } } -// TestClient_Create tests the Create method routes to the correct cluster. -func TestClient_Create(t *testing.T) { +func TestClient_Create_RouterMatchesCluster(t *testing.T) { scheme := newTestScheme(t) homeCluster := newFakeCluster(scheme) - remoteCluster := newFakeCluster(scheme) - - gvk := schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "ConfigMap", - } + remote1 := newFakeCluster(scheme) + remote2 := newFakeCluster(scheme) c := &Client{ - HomeCluster: homeCluster, - HomeScheme: scheme, - remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, + HomeCluster: homeCluster, + HomeScheme: scheme, + ResourceRouters: map[schema.GroupVersionKind]ResourceRouter{ + configMapGVK: testRouter{}, + }, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: { + {cluster: remote1, labels: map[string]string{"az": "az-1"}}, + {cluster: remote2, labels: map[string]string{"az": "az-2"}}, + }, + }, } - ctx := context.Background() - - // Create in remote cluster cm := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: "new-cm", Namespace: "default", + Labels: map[string]string{"az": "az-2"}, }, } - err := c.Create(ctx, cm) + err := c.Create(context.Background(), cm) if err != nil { t.Fatalf("unexpected error: %v", err) } - // Verify it was created in remote cluster + // Should be in remote2, not remote1. result := &corev1.ConfigMap{} - err = remoteCluster.GetClient().Get(ctx, client.ObjectKey{Name: "new-cm", Namespace: "default"}, result) + err = remote2.GetClient().Get(context.Background(), client.ObjectKey{Name: "new-cm", Namespace: "default"}, result) if err != nil { - t.Fatalf("failed to get created object from remote cluster: %v", err) + t.Fatalf("expected to find in remote2: %v", err) + } + err = remote1.GetClient().Get(context.Background(), client.ObjectKey{Name: "new-cm", Namespace: "default"}, result) + if err == nil { + t.Error("should NOT be in remote1") } } -// TestClient_Delete tests the Delete method routes to the correct cluster. -func TestClient_Delete(t *testing.T) { +func TestClient_Create_NoMatchReturnsError(t *testing.T) { scheme := newTestScheme(t) + homeCluster := newFakeCluster(scheme) - existingCM := &corev1.ConfigMap{ + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + ResourceRouters: map[schema.GroupVersionKind]ResourceRouter{ + configMapGVK: testRouter{}, + }, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: { + {cluster: newFakeCluster(scheme), labels: map[string]string{"az": "az-1"}}, + }, + }, + } + + // Object with az-99 doesn't match any remote — should error. + cm := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: "to-delete", + Name: "no-match-cm", Namespace: "default", + Labels: map[string]string{"az": "az-99"}, }, } - homeCluster := newFakeCluster(scheme) - remoteCluster := newFakeCluster(scheme, existingCM) - - gvk := schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "ConfigMap", + err := c.Create(context.Background(), cm) + if err == nil { + t.Error("expected error when no remote cluster matches") } +} - c := &Client{ - HomeCluster: homeCluster, - HomeScheme: scheme, - remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, +func TestClient_Delete_SingleRemoteCluster(t *testing.T) { + scheme := newTestScheme(t) + existingCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "to-delete", Namespace: "default"}, } + homeCluster := newFakeCluster(scheme) + remote := newFakeCluster(scheme, existingCM) - ctx := context.Background() - - // Delete from remote cluster - cm := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "to-delete", - Namespace: "default", + c := &Client{ + HomeCluster: homeCluster, + HomeScheme: scheme, + ResourceRouters: map[schema.GroupVersionKind]ResourceRouter{ + configMapGVK: alwaysMatchRouter{}, + }, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: {{cluster: remote}}, }, } - err := c.Delete(ctx, cm) + + cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "to-delete", Namespace: "default"}} + err := c.Delete(context.Background(), 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) + err = remote.GetClient().Get(context.Background(), 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) { +func TestClient_Update_SingleRemoteCluster(t *testing.T) { scheme := newTestScheme(t) - existingCM := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "to-update", - Namespace: "default", - }, - Data: map[string]string{"key": "old-value"}, + 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", - } + remote := newFakeCluster(scheme, existingCM) c := &Client{ - HomeCluster: homeCluster, - HomeScheme: scheme, - remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, + HomeCluster: homeCluster, + HomeScheme: scheme, + ResourceRouters: map[schema.GroupVersionKind]ResourceRouter{ + configMapGVK: alwaysMatchRouter{}, + }, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: {{cluster: remote}}, + }, } 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) + err := remote.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) + err = remote.GetClient().Get(ctx, client.ObjectKey{Name: "to-update", Namespace: "default"}, result) if err != nil { t.Fatalf("failed to get updated object: %v", err) } @@ -870,42 +1056,28 @@ func TestClient_Update(t *testing.T) { } } -// TestClient_Patch tests the Patch method routes to the correct cluster. -func TestClient_Patch(t *testing.T) { +func TestClient_Patch_SingleRemoteCluster(t *testing.T) { scheme := newTestScheme(t) - existingCM := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "to-patch", - Namespace: "default", - }, - Data: map[string]string{"key": "old-value"}, + 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", - } + remote := newFakeCluster(scheme, existingCM) c := &Client{ - HomeCluster: homeCluster, - HomeScheme: scheme, - remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, + HomeCluster: homeCluster, + HomeScheme: scheme, + ResourceRouters: map[schema.GroupVersionKind]ResourceRouter{ + configMapGVK: alwaysMatchRouter{}, + }, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: {{cluster: remote}}, + }, } ctx := context.Background() - - // Patch in remote cluster - cm := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "to-patch", - Namespace: "default", - }, - } + 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) @@ -913,9 +1085,8 @@ func TestClient_Patch(t *testing.T) { 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) + err = remote.GetClient().Get(ctx, client.ObjectKey{Name: "to-patch", Namespace: "default"}, result) if err != nil { t.Fatalf("failed to get patched object: %v", err) } @@ -924,51 +1095,32 @@ func TestClient_Patch(t *testing.T) { } } -// TestClient_DeleteAllOf tests the DeleteAllOf method routes to the correct cluster. -func TestClient_DeleteAllOf(t *testing.T) { +func TestClient_DeleteAllOf_SingleRemoteCluster(t *testing.T) { scheme := newTestScheme(t) - cm1 := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cm1", - Namespace: "default", - Labels: map[string]string{"app": "test"}, - }, + 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"}, - }, + 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", - } + remote := newFakeCluster(scheme, cm1, cm2) c := &Client{ - HomeCluster: homeCluster, - HomeScheme: scheme, - remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{gvk: remoteCluster}, + HomeCluster: homeCluster, + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: {{cluster: remote}}, + }, } - ctx := context.Background() - - // DeleteAllOf in remote cluster - err := c.DeleteAllOf(ctx, &corev1.ConfigMap{}, client.InNamespace("default"), client.MatchingLabels{"app": "test"}) + err := c.DeleteAllOf(context.Background(), &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")) + err = remote.GetClient().List(context.Background(), cmList, client.InNamespace("default")) if err != nil { t.Fatalf("failed to list objects: %v", err) } @@ -977,232 +1129,134 @@ func TestClient_DeleteAllOf(t *testing.T) { } } -// TestClient_ConcurrentAddRemote tests thread safety of adding remote clusters. -func TestClient_ConcurrentAddRemote(t *testing.T) { +func TestClient_DeleteAllOf_MultipleClusters(t *testing.T) { + scheme := newTestScheme(t) + remote1 := newFakeCluster(scheme, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cm1", Namespace: "default", Labels: map[string]string{"app": "test"}}}, + ) + remote2 := newFakeCluster(scheme, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cm2", Namespace: "default", Labels: map[string]string{"app": "test"}}}, + ) + c := &Client{ - remoteClusters: make(map[schema.GroupVersionKind]cluster.Cluster), + HomeCluster: newFakeCluster(scheme), + HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: { + {cluster: remote1}, + {cluster: remote2}, + }, + }, } - 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) + err := c.DeleteAllOf(context.Background(), &corev1.ConfigMap{}, client.InNamespace("default"), client.MatchingLabels{"app": "test"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) } - wg.Wait() - if len(c.remoteClusters) != 10 { - t.Errorf("expected 10 remote clusters, got %d", len(c.remoteClusters)) + for i, remote := range []*fakeCluster{remote1, remote2} { + cmList := &corev1.ConfigMapList{} + if err := remote.GetClient().List(context.Background(), cmList, client.InNamespace("default")); err != nil { + t.Fatalf("failed to list objects in remote%d: %v", i+1, err) + } + + if len(cmList.Items) != 0 { + t.Errorf("expected 0 items in remote%d, got %d", i+1, len(cmList.Items)) + } } } -// TestClient_ConcurrentClusterForResourceAndAddRemote tests concurrent read/write operations. -func TestClient_ConcurrentClusterForResourceAndAddRemote(t *testing.T) { - c := &Client{ - remoteClusters: make(map[schema.GroupVersionKind]cluster.Cluster), +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, homeGVKs: map[schema.GroupVersionKind]bool{podGVK: true}} - gvk := schema.GroupVersionKind{ - Group: "test", - Version: "v1", - Kind: "TestKind", + ctx := context.Background() + existingPod := &corev1.Pod{} + if err := homeCluster.GetClient().Get(ctx, client.ObjectKey{Name: "test-pod", Namespace: "default"}, existingPod); err != nil { + t.Fatalf("failed to get pod: %v", err) + } + existingPod.Status.Phase = corev1.PodRunning + if err := c.Status().Update(ctx, existingPod); err != nil { + t.Fatalf("unexpected error: %v", err) } - var wg sync.WaitGroup + // Validate the update took effect + result := &corev1.Pod{} + if err := homeCluster.GetClient().Get(ctx, client.ObjectKey{Name: "test-pod", Namespace: "default"}, result); 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) + } +} - // Readers - for range 10 { - wg.Add(1) - go func() { - defer wg.Done() - for range 100 { - _ = c.ClusterForResource(gvk) - } - }() +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, homeGVKs: map[schema.GroupVersionKind]bool{podGVK: true}} - // 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() - } - }() + ctx := context.Background() + existingPod := &corev1.Pod{} + if err := homeCluster.GetClient().Get(ctx, client.ObjectKey{Name: "test-pod", Namespace: "default"}, existingPod); err != nil { + t.Fatalf("failed to get pod: %v", err) + } + patch := client.MergeFrom(existingPod.DeepCopy()) + existingPod.Status.Phase = corev1.PodRunning + if err := c.Status().Patch(ctx, existingPod, patch); err != nil { + t.Fatalf("unexpected error: %v", err) } - wg.Wait() + // Validate the patch took effect + result := &corev1.Pod{} + if err := homeCluster.GetClient().Get(ctx, client.ObjectKey{Name: "test-pod", Namespace: "default"}, result); err != nil { + t.Fatalf("failed to get patched pod: %v", err) + } + if result.Status.Phase != corev1.PodRunning { + t.Errorf("expected PodRunning, got %s", result.Status.Phase) + } } -// TestStatusClient_Create tests the status client Create method. -func TestStatusClient_Create(t *testing.T) { +func TestStatusClient_RoutesToRemoteCluster(t *testing.T) { scheme := newTestScheme(t) - pod := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pod", - Namespace: "default", - }, + ObjectMeta: metav1.ObjectMeta{Name: "test-pod", Namespace: "default"}, + Status: corev1.PodStatus{Phase: corev1.PodPending}, } - - homeCluster := newFakeCluster(scheme, pod) + homeCluster := newFakeCluster(scheme) + remote := newFakeCluster(scheme, pod) c := &Client{ HomeCluster: homeCluster, HomeScheme: scheme, + ResourceRouters: map[schema.GroupVersionKind]ResourceRouter{ + podGVK: alwaysMatchRouter{}, + }, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + podGVK: {{cluster: remote}}, + }, } 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 { + if err := remote.GetClient().Get(ctx, client.ObjectKey{Name: "test-pod", Namespace: "default"}, existingPod); 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 { + if err := c.Status().Update(ctx, existingPod); 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 { + if err := remote.GetClient().Get(ctx, client.ObjectKey{Name: "test-pod", Namespace: "default"}, result); err != nil { t.Fatalf("failed to get updated pod: %v", err) } if result.Status.Phase != corev1.PodRunning { @@ -1210,456 +1264,196 @@ func TestStatusClient_RoutesToRemoteCluster(t *testing.T) { } } -// 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) { +func TestClient_StatusAndSubResource_ErrorOnUnknownType(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, - } - + c := &Client{HomeCluster: newFakeCluster(scheme), HomeScheme: scheme} ctx := context.Background() + obj := &unknownType{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}} - 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", - }, + if err := c.Status().Update(ctx, obj); err == nil { + t.Error("expected error for unknown type in status Update") } - - homeCluster := newFakeCluster(scheme, pod) - - c := &Client{ - HomeCluster: homeCluster, - HomeScheme: scheme, + if err := c.Status().Patch(ctx, obj, client.MergeFrom(obj)); err == nil { + t.Error("expected error for unknown type in status Patch") } - - 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) + if err := c.SubResource("status").Update(ctx, obj); err == nil { + t.Error("expected error for unknown type in subresource Update") } - - src := c.SubResource("status") - err = src.Update(ctx, existingPod) - if err != nil { - t.Fatalf("unexpected error: %v", err) + if err := c.SubResource("status").Patch(ctx, obj, client.MergeFrom(obj)); err == nil { + t.Error("expected error for unknown type in subresource Patch") } } -// TestSubResourceClient_Patch tests the subresource client Patch method. -func TestSubResourceClient_Patch(t *testing.T) { +func TestSubResourceClient_RoutesToRemoteCluster(t *testing.T) { scheme := newTestScheme(t) - pod := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pod", - Namespace: "default", - }, + ObjectMeta: metav1.ObjectMeta{Name: "test-pod", Namespace: "default"}, } - - homeCluster := newFakeCluster(scheme, pod) + homeCluster := newFakeCluster(scheme) + remote := 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", + ResourceRouters: map[schema.GroupVersionKind]ResourceRouter{ + podGVK: alwaysMatchRouter{}, + }, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + podGVK: {{cluster: remote}}, }, - } - - 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 { + if err := remote.GetClient().Get(ctx, client.ObjectKey{Name: "test-pod", Namespace: "default"}, existingPod); 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 { + if err := src.Update(ctx, existingPod); 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) { +func TestClient_GroupVersionKindFor(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")) + c := &Client{HomeCluster: newFakeCluster(scheme), HomeScheme: scheme} + gvk, err := c.GroupVersionKindFor(&corev1.ConfigMap{}) if err != nil { t.Fatalf("unexpected error: %v", err) } - if len(cmList.Items) != 1 { - t.Errorf("expected 1 item, got %d", len(cmList.Items)) + if gvk != configMapGVK { + t.Errorf("expected GVK %v, got %v", configMapGVK, gvk) } } -// TestClient_StatusAndSubResource_ErrorOnUnknownType tests error handling for unknown types. -func TestClient_StatusAndSubResource_ErrorOnUnknownType(t *testing.T) { +func TestClient_IndexField_WithRemoteClusters(t *testing.T) { scheme := newTestScheme(t) - homeCluster := newFakeCluster(scheme) + homeCache := &fakeCache{} + homeCluster := newFakeClusterWithCache(scheme, homeCache) + remoteObjCache := &fakeCache{} + remoteObjCluster := newFakeClusterWithCache(scheme, remoteObjCache) + remoteListCache := &fakeCache{} + remoteListCluster := newFakeClusterWithCache(scheme, remoteListCache) c := &Client{ HomeCluster: homeCluster, HomeScheme: scheme, - } - - ctx := context.Background() - - obj := &unknownType{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: {{cluster: remoteObjCluster}}, + configMapListGVK: {{cluster: remoteListCluster}}, }, } - // 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.IndexField(context.Background(), &corev1.ConfigMap{}, &corev1.ConfigMapList{}, "metadata.name", func(obj client.Object) []string { + return []string{obj.GetName()} + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) } - err = c.Status().Patch(ctx, obj, client.MergeFrom(obj)) - if err == nil { - t.Error("expected error for unknown type in status Patch") + if len(remoteObjCache.getIndexFieldCalls()) != 1 { + t.Errorf("expected 1 IndexField call on remote obj cache, got %d", len(remoteObjCache.getIndexFieldCalls())) } - - // 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") + if len(remoteListCache.getIndexFieldCalls()) != 1 { + t.Errorf("expected 1 IndexField call on remote list cache, got %d", len(remoteListCache.getIndexFieldCalls())) } - - err = c.SubResource("status").Patch(ctx, obj, client.MergeFrom(obj)) - if err == nil { - t.Error("expected error for unknown type in subresource Patch") + if len(homeCache.getIndexFieldCalls()) != 0 { + t.Errorf("expected 0 IndexField calls on home cache, got %d", len(homeCache.getIndexFieldCalls())) } } -// TestClient_IndexField_WithRemoteClusters tests IndexField with remote clusters configured. -func TestClient_IndexField_WithRemoteClusters(t *testing.T) { +func TestClient_IndexField_SameClusterSkipsDuplicate(t *testing.T) { scheme := newTestScheme(t) - + remoteCache := &fakeCache{} + remote := newFakeClusterWithCache(scheme, remoteCache) homeCache := &fakeCache{} homeCluster := newFakeClusterWithCache(scheme, homeCache) - remoteObjCache := &fakeCache{} - remoteObjCluster := newFakeClusterWithCache(scheme, remoteObjCache) - - remoteListCache := &fakeCache{} - remoteListCluster := newFakeClusterWithCache(scheme, remoteListCache) - - objGVK := schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "ConfigMap", - } - listGVK := schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "ConfigMapList", - } - c := &Client{ HomeCluster: homeCluster, HomeScheme: scheme, - remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{ - objGVK: remoteObjCluster, - listGVK: remoteListCluster, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: {{cluster: remote}}, + configMapListGVK: {{cluster: remote}}, // same cluster }, } - ctx := context.Background() - - obj := &corev1.ConfigMap{} - list := &corev1.ConfigMapList{} - field := "metadata.name" - extractValue := func(obj client.Object) []string { + err := c.IndexField(context.Background(), &corev1.ConfigMap{}, &corev1.ConfigMapList{}, "metadata.name", func(obj client.Object) []string { return []string{obj.GetName()} - } - - err := c.IndexField(ctx, obj, list, field, extractValue) + }) if err != nil { t.Fatalf("unexpected error: %v", err) } - // Verify IndexField was called on the remote object cluster's cache - objCalls := remoteObjCache.getIndexFieldCalls() - if len(objCalls) != 1 { - t.Errorf("expected 1 IndexField call on remote object cluster, got %d", len(objCalls)) - } - - // Verify IndexField was called on the remote list cluster's cache - listCalls := remoteListCache.getIndexFieldCalls() - if len(listCalls) != 1 { - t.Errorf("expected 1 IndexField call on remote list cluster, got %d", len(listCalls)) + if len(remoteCache.getIndexFieldCalls()) != 1 { + t.Errorf("expected 1 IndexField call (deduped), got %d", len(remoteCache.getIndexFieldCalls())) } - - // Verify home cluster cache was NOT called - homeCalls := homeCache.getIndexFieldCalls() - if len(homeCalls) != 0 { - t.Errorf("expected 0 IndexField calls on home cluster, got %d", len(homeCalls)) + if len(homeCache.getIndexFieldCalls()) != 0 { + t.Errorf("expected 0 IndexField calls on home, got %d", len(homeCache.getIndexFieldCalls())) } } -// TestClient_IndexField_SameClusterSkipsSecondIndex tests that when object and list map to -// the same cluster, IndexField is only called once to avoid re-defining the index (which would error). -func TestClient_IndexField_SameClusterSkipsSecondIndex(t *testing.T) { +func TestClient_IndexField_HomeClusterOnly(t *testing.T) { scheme := newTestScheme(t) - - // Use the same cache/cluster for both object and list GVKs - remoteCache := &fakeCache{} - remoteCluster := newFakeClusterWithCache(scheme, remoteCache) - homeCache := &fakeCache{} homeCluster := newFakeClusterWithCache(scheme, homeCache) - objGVK := schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "ConfigMap", - } - listGVK := schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "ConfigMapList", - } - - // Both object and list GVKs point to the SAME remote cluster instance c := &Client{ HomeCluster: homeCluster, HomeScheme: scheme, - remoteClusters: map[schema.GroupVersionKind]cluster.Cluster{ - objGVK: remoteCluster, - listGVK: remoteCluster, // Same cluster instance + homeGVKs: map[schema.GroupVersionKind]bool{ + configMapGVK: true, + configMapListGVK: true, }, } - ctx := context.Background() - - obj := &corev1.ConfigMap{} - list := &corev1.ConfigMapList{} - field := "metadata.name" - extractValue := func(obj client.Object) []string { + err := c.IndexField(context.Background(), &corev1.ConfigMap{}, &corev1.ConfigMapList{}, "metadata.name", func(obj client.Object) []string { return []string{obj.GetName()} - } - - err := c.IndexField(ctx, obj, list, field, extractValue) + }) if err != nil { t.Fatalf("unexpected error: %v", err) } - // Key assertion: IndexField should only be called ONCE because both object - // and list resolve to the same cluster. Calling it twice would cause an error - // from re-defining the same index. - remoteCalls := remoteCache.getIndexFieldCalls() - if len(remoteCalls) != 1 { - t.Errorf("expected 1 IndexField call (skipping duplicate for same cluster), got %d", len(remoteCalls)) - } - - // Home cluster should not be called at all - homeCalls := homeCache.getIndexFieldCalls() - if len(homeCalls) != 0 { - t.Errorf("expected 0 IndexField calls on home cluster, got %d", len(homeCalls)) + if len(homeCache.getIndexFieldCalls()) != 1 { + t.Errorf("expected 1 IndexField call on home (deduped), got %d", len(homeCache.getIndexFieldCalls())) } } -// TestClient_IndexField_HomeClusterSkipsSecondIndex tests that when both object and list -// use the home cluster (no remote clusters configured), IndexField is only called once. -func TestClient_IndexField_HomeClusterSkipsSecondIndex(t *testing.T) { +func TestClient_ConcurrentAddRemoteAndRead(t *testing.T) { scheme := newTestScheme(t) - - homeCache := &fakeCache{} - homeCluster := newFakeClusterWithCache(scheme, homeCache) - - // No remote clusters configured - both object and list will use home cluster c := &Client{ - HomeCluster: homeCluster, + HomeCluster: newFakeCluster(scheme), HomeScheme: scheme, + remoteClusters: map[schema.GroupVersionKind][]remoteCluster{ + configMapGVK: {{}}, + }, } - ctx := context.Background() + var wg sync.WaitGroup - obj := &corev1.ConfigMap{} - list := &corev1.ConfigMapList{} - field := "metadata.name" - extractValue := func(obj client.Object) []string { - return []string{obj.GetName()} + // Readers + for range 10 { + wg.Go(func() { + for range 100 { + if _, err := c.ClustersForGVK(configMapGVK); err != nil { + t.Errorf("unexpected error: %v", err) + } + } + }) } - err := c.IndexField(ctx, obj, list, field, extractValue) - if err != nil { - t.Fatalf("unexpected error: %v", err) + // Writers + for range 5 { + wg.Go(func() { + for range 100 { + c.remoteClustersMu.Lock() + c.remoteClusters[configMapGVK] = append(c.remoteClusters[configMapGVK], remoteCluster{}) + c.remoteClustersMu.Unlock() + } + }) } - // Key assertion: IndexField should only be called ONCE because both object - // and list resolve to the same home cluster. Calling it twice would cause - // an error from re-defining the same index. - homeCalls := homeCache.getIndexFieldCalls() - if len(homeCalls) != 1 { - t.Errorf("expected 1 IndexField call (skipping duplicate for same home cluster), got %d", len(homeCalls)) - } + wg.Wait() } // fakeManager implements ctrl.Manager for testing InitFromConf. @@ -1679,429 +1473,86 @@ func (f *fakeManager) Add(runnable manager.Runnable) error { return nil } -// TestClient_InitFromConf_EmptyConfig tests that InitFromConf succeeds with an empty config. func TestClient_InitFromConf_EmptyConfig(t *testing.T) { scheme := newTestScheme(t) - homeCluster := newFakeCluster(scheme) - c := &Client{ - HomeCluster: homeCluster, + HomeCluster: newFakeCluster(scheme), HomeScheme: scheme, } - - ctx := context.Background() - conf := ClientConfig{ - APIServerOverrides: []APIServerOverride{}, - } - - // Create a fake manager - we won't actually use it since there are no overrides mgr := &fakeManager{} - err := c.InitFromConf(ctx, mgr, conf) + err := c.InitFromConf(context.Background(), mgr, ClientConfig{}) if err != nil { t.Fatalf("unexpected error with empty config: %v", err) } - - // Verify no runnables were added if len(mgr.addedRunnables) != 0 { t.Errorf("expected 0 runnables added, got %d", len(mgr.addedRunnables)) } } -// TestClient_InitFromConf_UnregisteredGVK tests that InitFromConf returns an error -// when the config contains a GVK that is not registered in the scheme. -func TestClient_InitFromConf_UnregisteredGVK(t *testing.T) { +func TestClient_InitFromConf_UnregisteredRemoteGVK(t *testing.T) { scheme := newTestScheme(t) - homeCluster := newFakeCluster(scheme) - c := &Client{ - HomeCluster: homeCluster, + HomeCluster: newFakeCluster(scheme), HomeScheme: scheme, } - - ctx := context.Background() - conf := ClientConfig{ - APIServerOverrides: []APIServerOverride{ - { - GVK: "unregistered.group/v1/UnknownKind", - Host: "https://remote-api:6443", - }, - }, - } - mgr := &fakeManager{} - - err := c.InitFromConf(ctx, mgr, conf) - if err == nil { - t.Fatal("expected error for unregistered GVK, got nil") - } - - expectedErrMsg := "no gvk registered for API server override unregistered.group/v1/UnknownKind" - if err.Error() != expectedErrMsg { - t.Errorf("expected error message '%s', got '%s'", expectedErrMsg, err.Error()) - } -} - -// TestClient_InitFromConf_GVKFormatting tests that the GVK formatting works correctly -// and matches the expected format for registered types. -func TestClient_InitFromConf_GVKFormatting(t *testing.T) { - scheme := newTestScheme(t) - - // Test that the GVK formatting matches what InitFromConf expects - tests := []struct { - name string - gvk schema.GroupVersionKind - expectedStr string - }{ - { - name: "core API ConfigMap", - gvk: schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "ConfigMap", - }, - expectedStr: "v1/ConfigMap", - }, - { - name: "cortex Decision", - gvk: schema.GroupVersionKind{ - Group: "cortex.cloud", - Version: "v1alpha1", - Kind: "Decision", - }, - expectedStr: "cortex.cloud/v1alpha1/Decision", - }, - { - name: "cortex DecisionList", - gvk: schema.GroupVersionKind{ - Group: "cortex.cloud", - Version: "v1alpha1", - Kind: "DecisionList", - }, - expectedStr: "cortex.cloud/v1alpha1/DecisionList", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Verify the GVK is in the scheme - _, found := scheme.AllKnownTypes()[tt.gvk] - if !found { - t.Skipf("GVK %v not found in scheme, skipping", tt.gvk) - } - - // Format the GVK the same way InitFromConf does - formatted := tt.gvk.GroupVersion().String() + "/" + tt.gvk.Kind - if formatted != tt.expectedStr { - t.Errorf("expected formatted GVK '%s', got '%s'", tt.expectedStr, formatted) - } - }) - } -} - -// TestClient_InitFromConf_MultipleUnregisteredGVKs tests that the first unregistered -// GVK in the list causes an error. -func TestClient_InitFromConf_MultipleUnregisteredGVKs(t *testing.T) { - scheme := newTestScheme(t) - homeCluster := newFakeCluster(scheme) - - c := &Client{ - HomeCluster: homeCluster, - HomeScheme: scheme, - } - - ctx := context.Background() conf := ClientConfig{ - APIServerOverrides: []APIServerOverride{ - { - GVK: "first.unregistered/v1/Type1", - Host: "https://remote-api-1:6443", - }, - { - GVK: "second.unregistered/v1/Type2", - Host: "https://remote-api-2:6443", + APIServers: APIServersConfig{ + Remotes: []RemoteConfig{ + { + Host: "https://remote-api:6443", + GVKs: []string{"unregistered.group/v1/UnknownKind"}, + }, }, }, } - mgr := &fakeManager{} - - err := c.InitFromConf(ctx, mgr, conf) + err := c.InitFromConf(context.Background(), mgr, conf) if err == nil { - t.Fatal("expected error for unregistered GVK, got nil") - } - - // Should fail on the first unregistered GVK - expectedErrMsg := "no gvk registered for API server override first.unregistered/v1/Type1" - if err.Error() != expectedErrMsg { - t.Errorf("expected error message '%s', got '%s'", expectedErrMsg, err.Error()) - } -} - -// TestClient_InitFromConf_NilScheme tests behavior when HomeScheme is nil. -func TestClient_InitFromConf_NilScheme(t *testing.T) { - c := &Client{ - HomeScheme: nil, - } - - ctx := context.Background() - conf := ClientConfig{ - APIServerOverrides: []APIServerOverride{ - { - GVK: "test/v1/SomeKind", - Host: "https://remote-api:6443", - }, - }, - } - - mgr := &fakeManager{} - - // Should panic or return error due to nil scheme - defer func() { - if r := recover(); r == nil { - t.Error("expected panic with nil scheme") - } - }() - - err := c.InitFromConf(ctx, mgr, conf) - if err != nil { - t.Fatalf("unexpected error: %v", err) + t.Fatal("expected error for unregistered remote GVK") } } -// TestClient_InitFromConf_EmptyGVKInConfig tests behavior when an empty GVK string is provided. -func TestClient_InitFromConf_EmptyGVKInConfig(t *testing.T) { +func TestClient_InitFromConf_UnregisteredHomeGVK(t *testing.T) { scheme := newTestScheme(t) - homeCluster := newFakeCluster(scheme) - c := &Client{ - HomeCluster: homeCluster, + HomeCluster: newFakeCluster(scheme), HomeScheme: scheme, } - - ctx := context.Background() + mgr := &fakeManager{} conf := ClientConfig{ - APIServerOverrides: []APIServerOverride{ - { - GVK: "", - Host: "https://remote-api:6443", - }, + APIServers: APIServersConfig{ + Home: HomeConfig{GVKs: []string{"unregistered.group/v1/UnknownKind"}}, }, } - mgr := &fakeManager{} - - err := c.InitFromConf(ctx, mgr, conf) + err := c.InitFromConf(context.Background(), mgr, conf) if err == nil { - t.Fatal("expected error for empty GVK, got nil") - } - - // Empty GVK won't match any registered type - expectedErrMsg := "no gvk registered for API server override " - if err.Error() != expectedErrMsg { - t.Errorf("expected error message '%s', got '%s'", expectedErrMsg, err.Error()) + t.Fatal("expected error for unregistered home GVK") } } -// TestClient_InitFromConf_PartialGVKMatch tests that partial GVK matches don't work. -func TestClient_InitFromConf_PartialGVKMatch(t *testing.T) { +func TestClient_InitFromConf_GVKFormatting(t *testing.T) { scheme := newTestScheme(t) - homeCluster := newFakeCluster(scheme) - - c := &Client{ - HomeCluster: homeCluster, - HomeScheme: scheme, - } - - ctx := context.Background() - - // Try variations that shouldn't match tests := []struct { - name string - gvkStr string + name string + gvk schema.GroupVersionKind + expectedStr string }{ - { - name: "missing kind", - gvkStr: "cortex.cloud/v1alpha1", - }, - { - name: "wrong case", - gvkStr: "cortex.cloud/v1alpha1/decision", // lowercase 'd' - }, - { - name: "extra slash", - gvkStr: "cortex.cloud/v1alpha1/Decision/", - }, - { - name: "wrong version", - gvkStr: "cortex.cloud/v2/Decision", - }, + {"core ConfigMap", configMapGVK, "v1/ConfigMap"}, + {"cortex Decision", schema.GroupVersionKind{Group: "cortex.cloud", Version: "v1alpha1", Kind: "Decision"}, "cortex.cloud/v1alpha1/Decision"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - conf := ClientConfig{ - APIServerOverrides: []APIServerOverride{ - { - GVK: tt.gvkStr, - Host: "https://remote-api:6443", - }, - }, + if _, found := scheme.AllKnownTypes()[tt.gvk]; !found { + t.Skipf("GVK %v not in scheme", tt.gvk) } - - mgr := &fakeManager{} - - err := c.InitFromConf(ctx, mgr, conf) - if err == nil { - t.Errorf("expected error for GVK string '%s', got nil", tt.gvkStr) - } - }) - } -} - -// TestClient_InitFromConf_ValidGVKLookup tests that valid GVK strings are correctly -// identified in the scheme. Note: This doesn't test the full AddRemote flow since -// that requires a real REST config connection. -func TestClient_InitFromConf_ValidGVKLookup(t *testing.T) { - scheme := newTestScheme(t) - - // Build the gvksByConfStr map the same way InitFromConf does - gvksByConfStr := make(map[string]schema.GroupVersionKind) - for gvk := range scheme.AllKnownTypes() { - formatted := gvk.GroupVersion().String() + "/" + gvk.Kind - gvksByConfStr[formatted] = gvk - } - - // These GVK strings should be found in the map - validGVKs := []string{ - "v1/ConfigMap", - "v1/ConfigMapList", - "v1/Secret", - "v1/SecretList", - "v1/Pod", - "v1/PodList", - "cortex.cloud/v1alpha1/Decision", - "cortex.cloud/v1alpha1/DecisionList", - } - - for _, gvkStr := range validGVKs { - t.Run(gvkStr, func(t *testing.T) { - gvk, found := gvksByConfStr[gvkStr] - if !found { - t.Errorf("expected GVK string '%s' to be found in scheme", gvkStr) - return - } - - // Verify the GVK is correctly structured - if gvk.Kind == "" { - t.Errorf("expected non-empty Kind for GVK string '%s'", gvkStr) + formatted := tt.gvk.GroupVersion().String() + "/" + tt.gvk.Kind + if formatted != tt.expectedStr { + t.Errorf("expected '%s', got '%s'", tt.expectedStr, formatted) } }) } } - -// TestAPIServerOverride_Structure tests the APIServerOverride struct. -func TestAPIServerOverride_Structure(t *testing.T) { - override := APIServerOverride{ - GVK: "cortex.cloud/v1alpha1/Decision", - Host: "https://remote-api:6443", - CACert: "-----BEGIN CERTIFICATE-----\nMIIC...\n-----END CERTIFICATE-----", - } - - if override.GVK != "cortex.cloud/v1alpha1/Decision" { - t.Errorf("unexpected GVK: %s", override.GVK) - } - if override.Host != "https://remote-api:6443" { - t.Errorf("unexpected Host: %s", override.Host) - } - if override.CACert == "" { - t.Error("expected non-empty CACert") - } -} - -// TestClientConfig_Structure tests the ClientConfig struct. -func TestClientConfig_Structure(t *testing.T) { - conf := ClientConfig{ - APIServerOverrides: []APIServerOverride{ - { - GVK: "cortex.cloud/v1alpha1/Decision", - Host: "https://remote-api:6443", - }, - { - GVK: "cortex.cloud/v1alpha1/DecisionList", - Host: "https://remote-api:6443", - CACert: "cert-data", - }, - }, - } - - if len(conf.APIServerOverrides) != 2 { - t.Errorf("expected 2 overrides, got %d", len(conf.APIServerOverrides)) - } - - // First override - if conf.APIServerOverrides[0].GVK != "cortex.cloud/v1alpha1/Decision" { - t.Errorf("unexpected first override GVK: %s", conf.APIServerOverrides[0].GVK) - } - - // Second override - if conf.APIServerOverrides[1].CACert != "cert-data" { - t.Errorf("unexpected second override CACert: %s", conf.APIServerOverrides[1].CACert) - } -} - -// TestClient_InitFromConf_NilConfig tests behavior with nil APIServerOverrides slice. -func TestClient_InitFromConf_NilOverrides(t *testing.T) { - scheme := newTestScheme(t) - homeCluster := newFakeCluster(scheme) - - c := &Client{ - HomeCluster: homeCluster, - HomeScheme: scheme, - } - - ctx := context.Background() - conf := ClientConfig{ - APIServerOverrides: nil, // nil slice - } - - mgr := &fakeManager{} - - err := c.InitFromConf(ctx, mgr, conf) - if err != nil { - t.Fatalf("unexpected error with nil overrides: %v", err) - } -} - -// TestClient_InitFromConf_LogsRegisteredGVKs verifies that the function processes -// all registered GVKs from the scheme. This is a structural test. -func TestClient_InitFromConf_SchemeGVKCount(t *testing.T) { - scheme := newTestScheme(t) - - // Count the number of types in the scheme - allTypes := scheme.AllKnownTypes() - if len(allTypes) == 0 { - t.Error("expected scheme to have registered types") - } - - // Build the formatted GVK map - gvksByConfStr := make(map[string]schema.GroupVersionKind) - for gvk := range allTypes { - formatted := gvk.GroupVersion().String() + "/" + gvk.Kind - gvksByConfStr[formatted] = gvk - } - - // The map should have the same number of entries as types - // (assuming no duplicate formatted strings, which shouldn't happen) - if len(gvksByConfStr) == 0 { - t.Error("expected formatted GVK map to have entries") - } - - // Verify some specific entries exist - if _, found := gvksByConfStr["v1/ConfigMap"]; !found { - t.Error("expected v1/ConfigMap to be in formatted GVK map") - } - if _, found := gvksByConfStr["cortex.cloud/v1alpha1/Decision"]; !found { - t.Error("expected cortex.cloud/v1alpha1/Decision to be in formatted GVK map") - } -} diff --git a/pkg/multicluster/routers.go b/pkg/multicluster/routers.go new file mode 100644 index 000000000..1e0496791 --- /dev/null +++ b/pkg/multicluster/routers.go @@ -0,0 +1,43 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package multicluster + +import ( + "errors" + + hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + corev1 "k8s.io/api/core/v1" +) + +// ResourceRouter determines which remote cluster a resource should be written to +// by matching the resource content against the cluster's labels. +type ResourceRouter interface { + Match(obj any, labels map[string]string) (bool, error) +} + +// HypervisorResourceRouter routes hypervisors to clusters based on availability zone. +type HypervisorResourceRouter struct{} + +func (h HypervisorResourceRouter) Match(obj any, labels map[string]string) (bool, error) { + var hv hv1.Hypervisor + switch v := obj.(type) { + case *hv1.Hypervisor: + hv = *v + case hv1.Hypervisor: + hv = v + default: + return false, errors.New("object is not a Hypervisor") + } + az, ok := labels["az"] + if !ok { + return false, errors.New("cluster does not have availability zone label") + } + hvAZ, ok := hv.Labels[corev1.LabelTopologyZone] + if !ok { + return false, errors.New("hypervisor does not have availability zone label") + } + return hvAZ == az, nil +} + +// TODO: Add router for Decision CRD and reservations after their refactoring is done. diff --git a/pkg/multicluster/routers_test.go b/pkg/multicluster/routers_test.go new file mode 100644 index 000000000..d9598b767 --- /dev/null +++ b/pkg/multicluster/routers_test.go @@ -0,0 +1,95 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package multicluster + +import ( + "testing" + + hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestHypervisorResourceRouter_Match(t *testing.T) { + router := HypervisorResourceRouter{} + + tests := []struct { + name string + obj any + labels map[string]string + wantMatch bool + wantErr bool + }{ + { + name: "matching AZ", + obj: hv1.Hypervisor{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"topology.kubernetes.io/zone": "qa-de-1a"}, + }, + }, + labels: map[string]string{"az": "qa-de-1a"}, + wantMatch: true, + }, + { + name: "matching AZ pointer", + obj: &hv1.Hypervisor{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"topology.kubernetes.io/zone": "qa-de-1a"}, + }, + }, + labels: map[string]string{"az": "qa-de-1a"}, + wantMatch: true, + }, + { + name: "non-matching AZ", + obj: hv1.Hypervisor{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"topology.kubernetes.io/zone": "qa-de-1a"}, + }, + }, + labels: map[string]string{"az": "qa-de-1b"}, + wantMatch: false, + }, + { + name: "not a Hypervisor", + obj: "not-a-hypervisor", + labels: map[string]string{"az": "qa-de-1a"}, + wantErr: true, + }, + { + name: "cluster missing az label", + obj: hv1.Hypervisor{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"topology.kubernetes.io/zone": "qa-de-1a"}, + }, + }, + labels: map[string]string{}, + wantErr: true, + }, + { + name: "hypervisor missing zone label", + obj: hv1.Hypervisor{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + }, + }, + labels: map[string]string{"az": "qa-de-1a"}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + match, err := router.Match(tt.obj, tt.labels) + if tt.wantErr && err == nil { + t.Fatal("expected error, got nil") + } + if !tt.wantErr && err != nil { + t.Fatalf("unexpected error: %v", err) + } + if match != tt.wantMatch { + t.Errorf("expected match=%v, got %v", tt.wantMatch, match) + } + }) + } +}