diff --git a/changelog/current.md b/changelog/current.md index 4b43a6cac..1e0cba579 100644 --- a/changelog/current.md +++ b/changelog/current.md @@ -6,6 +6,8 @@ Record image-affecting changes to `manager/`, `worker/`, `copaw/`, `openclaw-bas - fix(agent): update file-sharing path guidance for CoPaw and Team Leader agents to use `/root/hiclaw-fs/agents/...` instead of the retired `/root/.hiclaw-worker/...` path. - feat(controller): add per-agent `spec.resources` support for Manager, Worker, Team Leader, and Team Worker CRDs. +- fix(worker): pass `X-HiClaw-Cluster-ID` when remote Workers refresh controller-issued STS credentials for OSS and Nacos AI registry access. +- feat(hiclaw-controller): support a separate agent-pod-template for `deployMode: Remote` Workers via an optional `pod-template-remote.yaml` key on the controller-scoped pod-template ConfigMap; remote-mode Pod creation prefers this key and transparently falls back to `pod-template.yaml` when it is absent or empty, while non-remote/Sandbox paths keep ignoring it. - **OpenHuman runtime**: OpenHuman added as the fourth Worker runtime with native Matrix support via `channel-matrix` feature flag; includes controller routing (K8s + Docker backends), Dockerfile, entrypoint script, agent template, Helm chart integration, and Makefile build targets. - **Multi model providers**: Worker, Team, and Manager specs can now select a Higress model provider via `spec.modelProvider`; the controller resolves the provider, injects the matching gateway URL into runtime config, and authorizes consumers only on the selected AI route. @@ -21,3 +23,6 @@ Record image-affecting changes to `manager/`, `worker/`, `copaw/`, `openclaw-bas - **CoPaw worker replies**: CoPaw's no-text debounce only recognized copaw `Content` objects, but the custom Matrix channel builds plain-dict content parts, so every message was buffered and the agent never replied. The channel now also recognizes dict-based text/audio parts; workers and team leaders reply as expected. - **Worker Matrix token refresh**: `POST /api/v1/credentials/matrix-token` was registered against the `credentials` resource kind, but the worker credentials branch only permitted `ActionSTS`, so the route was dead and workers could not self-refresh on a homeserver 401. The self-scoped credentials branch now also permits `ActionRefreshMatrixToken` (and the misplaced dead entry was removed). - **Helm AppService tokens**: The Helm `runtime-env` Secret now generates and preserves the AppService `as_token` / `hs_token` (values override -> existing Secret -> generated) so a default Helm/in-cluster install no longer crash-loops the controller, and a template comment trim-marker that broke `helm lint` YAML parsing was fixed. +- **Remote Worker authentication boundary**: Remote Worker tokens are now bound to the matching local Worker CR's `deployMode: Remote`, target cluster ID, target namespace, and ServiceAccount name before authorization. +- **Remote Worker applied target auth**: Remote Worker authentication now prefers the status-pinned deployment target and falls back to spec only before first provisioning, so spec target edits do not immediately break the running remote Worker or trust a target before it is applied. +- **Remote Worker lifecycle boundary**: Workers now record the applied deployment target in status, reject running target changes until the Worker is Stopped, clean up using the applied target, and register remote Pod watches for Worker/Team status updates. diff --git a/docs/agent-pod-template.md b/docs/agent-pod-template.md index 54641249e..ff1a2b219 100644 --- a/docs/agent-pod-template.md +++ b/docs/agent-pod-template.md @@ -62,6 +62,56 @@ data: Find a ready-to-apply example at [`docs/examples/agent-pod-template-cm.yaml`](examples/agent-pod-template-cm.yaml). +## Per-deploy-mode override (Remote) + +When a Worker is created with `spec.deployMode: Remote`, the controller +schedules its Pod into a remote cluster fronted by a virtual-kubelet style +provider, where node taints, image-pull secrets, and tolerations almost +always differ from the in-cluster defaults. To avoid forcing those +remote-only fields onto every local Pod, the same ConfigMap may carry an +additional key: + +| Key | Consumed when | +|---|---| +| `pod-template.yaml` | Default and `deployMode: Local` only. NOT used as fallback for Remote mode. | +| `pod-template-remote.yaml` | Only when `deployMode: Remote` and the key is present and non-empty. | + +Resolution rules: + +- `deployMode: Remote` → if `pod-template-remote.yaml` is present and + non-empty, it is used; otherwise the controller applies the default Pod + spec with no overlay (it does **not** fall back to `pod-template.yaml`). +- Any other deploy mode (default / `Local`) → the remote key is ignored + even when present, and only `pod-template.yaml` is consulted. +- Non-K8s backends never consume the remote key. + +This lets you keep one ConfigMap per controller while shipping different +`tolerations`, `nodeSelector`, `imagePullSecrets`, annotations or sidecars +to remote-cluster Pods, e.g.: + +```yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: + namespace: +data: + pod-template.yaml: | + spec: + nodeSelector: + kubernetes.io/os: linux + pod-template-remote.yaml: | + spec: + nodeSelector: + type: virtual-kubelet + tolerations: + - key: virtual-kubelet.io/provider + operator: Exists + effect: NoSchedule + imagePullSecrets: + - name: remote-registry-pull-secret +``` + ## Merge semantics Fields the **template wins**: diff --git a/helm/hiclaw/crds/teams.hiclaw.io.yaml b/helm/hiclaw/crds/teams.hiclaw.io.yaml index 34309f0c6..788a6fd4f 100644 --- a/helm/hiclaw/crds/teams.hiclaw.io.yaml +++ b/helm/hiclaw/crds/teams.hiclaw.io.yaml @@ -175,6 +175,9 @@ spec: memory: type: string description: Kubernetes memory quantity, e.g. 3Gi + serviceEnabled: + type: boolean + description: "Whether to create a ClusterIP Service alongside the leader pod." labels: type: object additionalProperties: @@ -390,6 +393,9 @@ spec: memory: type: string description: Kubernetes memory quantity, e.g. 2Gi + serviceEnabled: + type: boolean + description: "Whether to create a ClusterIP Service alongside this team worker pod." accessEntries: type: array description: > @@ -493,6 +499,21 @@ spec: ready: type: boolean description: "Mirrors backend.Status Running/Ready. Aggregates into leaderReady and readyWorkers." + phase: + type: string + enum: [Pending, Starting, Running, Updating, Stopping, Sleeping, Stopped, Failed] + containerState: + type: string + message: + type: string + lastActiveAt: + type: string + format: date-time + description: "Latest runtime-reported business activity time for this member." + lastHeartbeat: + type: string + format: date-time + description: "Latest heartbeat timestamp for this member." exposedPorts: type: array description: "Ports currently exposed via Higress for this member (workers only)." diff --git a/helm/hiclaw/crds/workers.hiclaw.io.yaml b/helm/hiclaw/crds/workers.hiclaw.io.yaml index 197b7737c..750e1841b 100644 --- a/helm/hiclaw/crds/workers.hiclaw.io.yaml +++ b/helm/hiclaw/crds/workers.hiclaw.io.yaml @@ -184,6 +184,24 @@ spec: ignored with a warning log — the system value always wins. additionalProperties: type: string + deployMode: + type: string + enum: ["Local", "Remote"] + description: "Where the worker pod runs. Local (default): controller's own cluster; Remote: cluster identified by targetCluster." + targetCluster: + type: object + description: "Remote cluster target for deployment. Required when deployMode is Remote." + properties: + id: + type: string + description: "ACS/ACK cluster ID." + namespace: + type: string + description: "Target namespace in the remote cluster." + required: ["id", "namespace"] + serviceEnabled: + type: boolean + description: "Whether to create a ClusterIP Service alongside the worker pod." labels: type: object additionalProperties: @@ -235,7 +253,7 @@ spec: format: int64 phase: type: string - enum: [Pending, Running, Sleeping, Updating, Stopped, Failed] + enum: [Pending, Starting, Running, Updating, Stopping, Sleeping, Stopped, Failed] matrixUserID: type: string roomID: @@ -257,6 +275,21 @@ spec: type: integer domain: type: string + deployMode: + type: string + enum: ["Local", "Remote"] + description: "Applied deployment mode for the current Worker runtime resource." + targetCluster: + type: object + description: "Applied remote cluster target for the current Worker runtime resource." + properties: + id: + type: string + description: "ACS/ACK cluster ID." + namespace: + type: string + description: "Target namespace in the remote cluster." + required: ["id", "namespace"] subresources: status: {} additionalPrinterColumns: diff --git a/hiclaw-controller/api/v1beta1/types.go b/hiclaw-controller/api/v1beta1/types.go index a6cff883a..f6685fb09 100644 --- a/hiclaw-controller/api/v1beta1/types.go +++ b/hiclaw-controller/api/v1beta1/types.go @@ -89,6 +89,20 @@ type AgentResourceValues struct { Memory string `json:"memory,omitempty"` } +// DeployMode constants define where the worker pod runs. +const ( + DeployModeLocal = "Local" + DeployModeRemote = "Remote" +) + +// TargetClusterSpec identifies the remote cluster and namespace for deployment. +type TargetClusterSpec struct { + // ID is the ACS/ACK cluster ID. + ID string `json:"id"` + // Namespace is the target namespace in the remote cluster. + Namespace string `json:"namespace"` +} + // +genclient // +kubebuilder:subresource:status // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -101,7 +115,6 @@ type Worker struct { Status WorkerStatus `json:"status,omitempty"` } - type WorkerSpec struct { Model string `json:"model"` ModelProvider string `json:"modelProvider,omitempty"` // APIG Model API name for per-worker LLM provider @@ -136,6 +149,19 @@ type WorkerSpec struct { // scoped to agents//* and shared/*). AccessEntries []AccessEntry `json:"accessEntries,omitempty"` + // DeployMode specifies where the worker pod runs. + // "Local" (default): created in the controller's own cluster. + // "Remote": created in the cluster identified by TargetCluster. + DeployMode *string `json:"deployMode,omitempty"` + + // TargetCluster specifies the remote cluster target for deployment. + // Required when DeployMode is "Remote". + TargetCluster *TargetClusterSpec `json:"targetCluster,omitempty"` + + // ServiceEnabled controls whether a ClusterIP Service is created + // alongside the worker pod (same cluster, namespace, name). + ServiceEnabled *bool `json:"serviceEnabled,omitempty"` + // Env holds user-defined environment variables injected into the worker // container. Keys that collide with variables already set by the // controller or backend (HICLAW_*, OPENCLAW_*, HOME, and similar @@ -152,7 +178,6 @@ type WorkerSpec struct { // embed WorkerSpec-shaped hashes keep a stable spec hash when the // field is absent. Labels map[string]string `json:"labels,omitempty"` - } // DesiredContainerMan returns the effective desired containerManaged, defaulting to true. @@ -205,6 +230,12 @@ type WorkerStatus struct { LastHeartbeat string `json:"lastHeartbeat,omitempty"` Message string `json:"message,omitempty"` ExposedPorts []ExposedPortStatus `json:"exposedPorts,omitempty"` + + // DeployMode/TargetCluster record where the current backend resource was + // actually provisioned. Once set, target changes are only accepted after + // the Worker has reached the Stopped phase. + DeployMode string `json:"deployMode,omitempty"` + TargetCluster *TargetClusterSpec `json:"targetCluster,omitempty"` } // ExposedPortStatus records a port that has been exposed via Higress. @@ -285,6 +316,10 @@ type LeaderSpec struct { // + shared/* + teams//* on the configured bucket). AccessEntries []AccessEntry `json:"accessEntries,omitempty"` + // ServiceEnabled controls whether a ClusterIP Service is created + // alongside the leader pod (same cluster, namespace, name). + ServiceEnabled *bool `json:"serviceEnabled,omitempty"` + // Env holds user-defined environment variables injected into the // leader container. See WorkerSpec.Env for the collision policy. Env map[string]string `json:"env,omitempty"` @@ -295,7 +330,6 @@ type LeaderSpec struct { // hashMemberSourceSpec stability for Teams that never set this // field. Labels map[string]string `json:"labels,omitempty"` - } type TeamLeaderHeartbeatSpec struct { @@ -328,6 +362,10 @@ type TeamWorkerSpec struct { // + shared/* + teams//* on the configured bucket). AccessEntries []AccessEntry `json:"accessEntries,omitempty"` + // ServiceEnabled controls whether a ClusterIP Service is created + // alongside the team worker pod (same cluster, namespace, name). + ServiceEnabled *bool `json:"serviceEnabled,omitempty"` + // Env holds user-defined environment variables injected into this // team worker's container. See WorkerSpec.Env for the collision policy. Env map[string]string `json:"env,omitempty"` @@ -337,7 +375,6 @@ type TeamWorkerSpec struct { // system labels (see WorkerSpec.Labels godoc). omitempty preserves // hashMemberSourceSpec stability for existing Teams. Labels map[string]string `json:"labels,omitempty"` - } // EffectiveWorkerName returns the runtime identity key for a team leader. @@ -436,6 +473,17 @@ type TeamMemberStatus struct { // summarizeBackendReadiness on each reconcile pass. Aggregates into // Team.Status.LeaderReady and Team.Status.ReadyWorkers. Ready bool `json:"ready,omitempty"` + // Phase is the member lifecycle phase: Pending, Starting, Running, + // Updating, Stopping, Sleeping, Stopped, Failed. + Phase string `json:"phase,omitempty"` + // ContainerState is the raw backend container status. + ContainerState string `json:"containerState,omitempty"` + // Message holds per-member error detail from reconcile. Cleared on success. + Message string `json:"message,omitempty"` + // LastActiveAt is the latest runtime-reported business activity time. + LastActiveAt string `json:"lastActiveAt,omitempty"` + // LastHeartbeat is the latest heartbeat timestamp for this member. + LastHeartbeat string `json:"lastHeartbeat,omitempty"` // ExposedPorts records the ports currently exposed via Higress for this // member. Leader members never expose ports (this field stays nil). ExposedPorts []ExposedPortStatus `json:"exposedPorts,omitempty"` @@ -545,7 +593,6 @@ type ManagerSpec struct { // godoc): pod-template < CR metadata.labels < CR spec.labels < // controller system labels. Labels map[string]string `json:"labels,omitempty"` - } // DesiredState returns the effective desired state, defaulting to "Running". diff --git a/hiclaw-controller/api/v1beta1/types_test.go b/hiclaw-controller/api/v1beta1/types_test.go index 46e8fdf58..8e9894965 100644 --- a/hiclaw-controller/api/v1beta1/types_test.go +++ b/hiclaw-controller/api/v1beta1/types_test.go @@ -1,10 +1,155 @@ package v1beta1 import ( + "encoding/json" "reflect" + "strings" "testing" ) +// strPtr / boolPtr are tiny helpers used by the cross-cluster deployment +// serialization tests below. Kept package-private to avoid leaking generic +// helpers from the API package. +func strPtr(s string) *string { return &s } +func boolPtr(b bool) *bool { return &b } + +// TestWorkerSpec_DeployFieldsJSONTags verifies the new cross-cluster +// deployment fields (DeployMode, TargetCluster, ServiceEnabled) marshal +// with stable, lowerCamelCase JSON keys and omit cleanly when nil. +func TestWorkerSpec_DeployFieldsJSONTags(t *testing.T) { + cases := []struct { + name string + spec WorkerSpec + wantSub []string // substrings expected in JSON + absent []string // substrings that must NOT appear in JSON + }{ + { + name: "local_with_target", + spec: WorkerSpec{ + Model: "m", + DeployMode: strPtr("Local"), + TargetCluster: &TargetClusterSpec{ + ID: "c-123", + Namespace: "agents", + }, + ServiceEnabled: boolPtr(true), + }, + wantSub: []string{`"deployMode":"Local"`, `"id":"c-123"`, `"namespace":"agents"`, `"serviceEnabled":true`}, + }, + { + name: "remote_with_target", + spec: WorkerSpec{ + Model: "m", + DeployMode: strPtr("Remote"), + TargetCluster: &TargetClusterSpec{ + ID: "c-remote", + Namespace: "team-a", + }, + }, + wantSub: []string{`"deployMode":"Remote"`, `"id":"c-remote"`, `"namespace":"team-a"`}, + absent: []string{`"serviceEnabled"`}, + }, + { + name: "all_nil_omitted", + spec: WorkerSpec{Model: "m"}, + absent: []string{`"deployMode"`, `"targetCluster"`, `"serviceEnabled"`}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + data, err := json.Marshal(tc.spec) + if err != nil { + t.Fatalf("Marshal: %v", err) + } + got := string(data) + for _, sub := range tc.wantSub { + if !strings.Contains(got, sub) { + t.Errorf("JSON missing %q: %s", sub, got) + } + } + for _, sub := range tc.absent { + if strings.Contains(got, sub) { + t.Errorf("JSON should omit %q: %s", sub, got) + } + } + }) + } +} + +// TestWorkerSpec_DeployFieldsRoundTrip verifies the new fields survive a +// JSON marshal/unmarshal cycle without value drift. +func TestWorkerSpec_DeployFieldsRoundTrip(t *testing.T) { + orig := WorkerSpec{ + Model: "m", + DeployMode: strPtr("Remote"), + TargetCluster: &TargetClusterSpec{ + ID: "c-xyz", + Namespace: "prod", + }, + ServiceEnabled: boolPtr(false), + } + data, err := json.Marshal(orig) + if err != nil { + t.Fatalf("Marshal: %v", err) + } + var got WorkerSpec + if err := json.Unmarshal(data, &got); err != nil { + t.Fatalf("Unmarshal: %v", err) + } + if got.DeployMode == nil || *got.DeployMode != "Remote" { + t.Fatalf("DeployMode = %v, want *Remote", got.DeployMode) + } + if got.TargetCluster == nil || got.TargetCluster.ID != "c-xyz" || got.TargetCluster.Namespace != "prod" { + t.Fatalf("TargetCluster = %+v", got.TargetCluster) + } + if got.ServiceEnabled == nil || *got.ServiceEnabled != false { + t.Fatalf("ServiceEnabled = %v, want *false", got.ServiceEnabled) + } +} + +// TestWorkerSpec_BackwardCompatOldJSON verifies that JSON payloads written +// before the cross-cluster fields existed deserialize cleanly with all +// new pointer fields left nil. +func TestWorkerSpec_BackwardCompatOldJSON(t *testing.T) { + old := []byte(`{"model":"m","runtime":"openclaw"}`) + var got WorkerSpec + if err := json.Unmarshal(old, &got); err != nil { + t.Fatalf("Unmarshal old payload: %v", err) + } + if got.DeployMode != nil { + t.Errorf("DeployMode should default to nil, got %v", *got.DeployMode) + } + if got.TargetCluster != nil { + t.Errorf("TargetCluster should default to nil, got %+v", got.TargetCluster) + } + if got.ServiceEnabled != nil { + t.Errorf("ServiceEnabled should default to nil, got %v", *got.ServiceEnabled) + } +} + +// TestTargetClusterSpec_JSONTags pins down the TargetClusterSpec field tags +// so a future rename does not silently break stored CRs (the apiserver +// stores the JSON form in etcd via the structural CRD schema). +func TestTargetClusterSpec_JSONTags(t *testing.T) { + spec := TargetClusterSpec{ID: "c-1", Namespace: "ns-1"} + data, err := json.Marshal(spec) + if err != nil { + t.Fatalf("Marshal: %v", err) + } + want := `{"id":"c-1","namespace":"ns-1"}` + if string(data) != want { + t.Fatalf("Marshal = %s, want %s", data, want) + } + + var back TargetClusterSpec + if err := json.Unmarshal(data, &back); err != nil { + t.Fatalf("Unmarshal: %v", err) + } + if back != spec { + t.Fatalf("round-trip = %+v, want %+v", back, spec) + } +} + // TestWorkerSpec_DeepCopyLabels verifies WorkerSpec.Labels is deep-copied: // mutating the source map after DeepCopy must not mutate the copy. Covers // nil, empty-but-non-nil, and populated variants because our hand-edited diff --git a/hiclaw-controller/api/v1beta1/zz_generated.deepcopy.go b/hiclaw-controller/api/v1beta1/zz_generated.deepcopy.go index 07b6d939c..3cd3ced5a 100644 --- a/hiclaw-controller/api/v1beta1/zz_generated.deepcopy.go +++ b/hiclaw-controller/api/v1beta1/zz_generated.deepcopy.go @@ -277,6 +277,11 @@ func (in *LeaderSpec) DeepCopyInto(out *LeaderSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.ServiceEnabled != nil { + in, out := &in.ServiceEnabled, &out.ServiceEnabled + *out = new(bool) + **out = **in + } if in.Env != nil { in, out := &in.Env, &out.Env *out = make(map[string]string, len(*in)) @@ -499,6 +504,21 @@ func (in *RemoteSkillSource) DeepCopy() *RemoteSkillSource { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TargetClusterSpec) DeepCopyInto(out *TargetClusterSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TargetClusterSpec. +func (in *TargetClusterSpec) DeepCopy() *TargetClusterSpec { + if in == nil { + return nil + } + out := new(TargetClusterSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Team) DeepCopyInto(out *Team) { *out = *in @@ -735,6 +755,11 @@ func (in *TeamWorkerSpec) DeepCopyInto(out *TeamWorkerSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.ServiceEnabled != nil { + in, out := &in.ServiceEnabled, &out.ServiceEnabled + *out = new(bool) + **out = **in + } if in.Env != nil { in, out := &in.Env, &out.Env *out = make(map[string]string, len(*in)) @@ -872,6 +897,21 @@ func (in *WorkerSpec) DeepCopyInto(out *WorkerSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.DeployMode != nil { + in, out := &in.DeployMode, &out.DeployMode + *out = new(string) + **out = **in + } + if in.TargetCluster != nil { + in, out := &in.TargetCluster, &out.TargetCluster + *out = new(TargetClusterSpec) + **out = **in + } + if in.ServiceEnabled != nil { + in, out := &in.ServiceEnabled, &out.ServiceEnabled + *out = new(bool) + **out = **in + } if in.Env != nil { in, out := &in.Env, &out.Env *out = make(map[string]string, len(*in)) @@ -906,6 +946,11 @@ func (in *WorkerStatus) DeepCopyInto(out *WorkerStatus) { *out = make([]ExposedPortStatus, len(*in)) copy(*out, *in) } + if in.TargetCluster != nil { + in, out := &in.TargetCluster, &out.TargetCluster + *out = new(TargetClusterSpec) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WorkerStatus. diff --git a/hiclaw-controller/cmd/hiclaw/client.go b/hiclaw-controller/cmd/hiclaw/client.go index c9f388dda..c0921c6c7 100644 --- a/hiclaw-controller/cmd/hiclaw/client.go +++ b/hiclaw-controller/cmd/hiclaw/client.go @@ -16,6 +16,7 @@ import ( type APIClient struct { BaseURL string Token string + ClusterID string // remote cluster identifier; empty for local mode HTTPClient *http.Client } @@ -38,14 +39,21 @@ func NewAPIClient() *APIClient { baseURL = strings.TrimRight(baseURL, "/") return &APIClient{ - BaseURL: baseURL, - Token: discoverToken(), + BaseURL: baseURL, + Token: discoverToken(), + ClusterID: discoverClusterID(), HTTPClient: &http.Client{ Timeout: 30 * time.Second, }, } } +// discoverClusterID returns the cluster ID injected into the Worker Pod via +// the HICLAW_CLUSTER_ID environment variable. Empty string means local mode. +func discoverClusterID() string { + return os.Getenv("HICLAW_CLUSTER_ID") +} + // discoverToken returns a bearer token using a multi-level fallback: // 1. HICLAW_AUTH_TOKEN env var (Docker containers, injected by Reconciler) // 2. HICLAW_AUTH_TOKEN_FILE env var pointing to a token file (K8s projected volume) @@ -89,6 +97,9 @@ func (c *APIClient) Do(method, path string, body interface{}) (*http.Response, e if c.Token != "" { req.Header.Set("Authorization", "Bearer "+c.Token) } + if c.ClusterID != "" { + req.Header.Set("X-HiClaw-Cluster-ID", c.ClusterID) + } return c.HTTPClient.Do(req) } @@ -160,6 +171,9 @@ func (c *APIClient) DoMultipart(path, fieldName, fileName string, fileData []byt if c.Token != "" { req.Header.Set("Authorization", "Bearer "+c.Token) } + if c.ClusterID != "" { + req.Header.Set("X-HiClaw-Cluster-ID", c.ClusterID) + } resp, err := c.HTTPClient.Do(req) if err != nil { diff --git a/hiclaw-controller/cmd/hiclaw/client_test.go b/hiclaw-controller/cmd/hiclaw/client_test.go new file mode 100644 index 000000000..67107ca61 --- /dev/null +++ b/hiclaw-controller/cmd/hiclaw/client_test.go @@ -0,0 +1,93 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +// TestAPIClient_ClusterIDHeader verifies that the APIClient sends the +// X-HiClaw-Cluster-ID header when HICLAW_CLUSTER_ID is set. +func TestAPIClient_ClusterIDHeader(t *testing.T) { + var receivedHeaders http.Header + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedHeaders = r.Header.Clone() + w.WriteHeader(http.StatusOK) + })) + defer ts.Close() + + // Test with ClusterID set. + client := &APIClient{ + BaseURL: ts.URL, + Token: "test-token", + ClusterID: "test-cluster", + HTTPClient: ts.Client(), + } + + resp, err := client.Do("GET", "/api/test", nil) + if err != nil { + t.Fatalf("Do: %v", err) + } + resp.Body.Close() + + got := receivedHeaders.Get("X-HiClaw-Cluster-ID") + if got != "test-cluster" { + t.Fatalf("expected X-HiClaw-Cluster-ID=test-cluster, got %q", got) + } + + // Verify Authorization header is also present. + authHeader := receivedHeaders.Get("Authorization") + if authHeader != "Bearer test-token" { + t.Fatalf("expected Authorization=Bearer test-token, got %q", authHeader) + } +} + +// TestAPIClient_NoClusterIDHeader verifies that the X-HiClaw-Cluster-ID header +// is NOT sent when ClusterID is empty. +func TestAPIClient_NoClusterIDHeader(t *testing.T) { + var receivedHeaders http.Header + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedHeaders = r.Header.Clone() + w.WriteHeader(http.StatusOK) + })) + defer ts.Close() + + // Test without ClusterID. + client := &APIClient{ + BaseURL: ts.URL, + Token: "test-token", + ClusterID: "", + HTTPClient: ts.Client(), + } + + resp, err := client.Do("GET", "/api/test", nil) + if err != nil { + t.Fatalf("Do: %v", err) + } + resp.Body.Close() + + got := receivedHeaders.Get("X-HiClaw-Cluster-ID") + if got != "" { + t.Fatalf("expected no X-HiClaw-Cluster-ID header, got %q", got) + } +} + +// TestDiscoverClusterID verifies the discoverClusterID function reads from env. +func TestDiscoverClusterID(t *testing.T) { + t.Setenv("HICLAW_CLUSTER_ID", "env-cluster") + got := discoverClusterID() + if got != "env-cluster" { + t.Fatalf("expected env-cluster, got %q", got) + } +} + +// TestDiscoverClusterID_Empty verifies empty env returns empty string. +func TestDiscoverClusterID_Empty(t *testing.T) { + t.Setenv("HICLAW_CLUSTER_ID", "") + got := discoverClusterID() + if got != "" { + t.Fatalf("expected empty string, got %q", got) + } +} diff --git a/hiclaw-controller/config/crd/teams.hiclaw.io.yaml b/hiclaw-controller/config/crd/teams.hiclaw.io.yaml index 34309f0c6..788a6fd4f 100644 --- a/hiclaw-controller/config/crd/teams.hiclaw.io.yaml +++ b/hiclaw-controller/config/crd/teams.hiclaw.io.yaml @@ -175,6 +175,9 @@ spec: memory: type: string description: Kubernetes memory quantity, e.g. 3Gi + serviceEnabled: + type: boolean + description: "Whether to create a ClusterIP Service alongside the leader pod." labels: type: object additionalProperties: @@ -390,6 +393,9 @@ spec: memory: type: string description: Kubernetes memory quantity, e.g. 2Gi + serviceEnabled: + type: boolean + description: "Whether to create a ClusterIP Service alongside this team worker pod." accessEntries: type: array description: > @@ -493,6 +499,21 @@ spec: ready: type: boolean description: "Mirrors backend.Status Running/Ready. Aggregates into leaderReady and readyWorkers." + phase: + type: string + enum: [Pending, Starting, Running, Updating, Stopping, Sleeping, Stopped, Failed] + containerState: + type: string + message: + type: string + lastActiveAt: + type: string + format: date-time + description: "Latest runtime-reported business activity time for this member." + lastHeartbeat: + type: string + format: date-time + description: "Latest heartbeat timestamp for this member." exposedPorts: type: array description: "Ports currently exposed via Higress for this member (workers only)." diff --git a/hiclaw-controller/config/crd/workers.hiclaw.io.yaml b/hiclaw-controller/config/crd/workers.hiclaw.io.yaml index 197b7737c..750e1841b 100644 --- a/hiclaw-controller/config/crd/workers.hiclaw.io.yaml +++ b/hiclaw-controller/config/crd/workers.hiclaw.io.yaml @@ -184,6 +184,24 @@ spec: ignored with a warning log — the system value always wins. additionalProperties: type: string + deployMode: + type: string + enum: ["Local", "Remote"] + description: "Where the worker pod runs. Local (default): controller's own cluster; Remote: cluster identified by targetCluster." + targetCluster: + type: object + description: "Remote cluster target for deployment. Required when deployMode is Remote." + properties: + id: + type: string + description: "ACS/ACK cluster ID." + namespace: + type: string + description: "Target namespace in the remote cluster." + required: ["id", "namespace"] + serviceEnabled: + type: boolean + description: "Whether to create a ClusterIP Service alongside the worker pod." labels: type: object additionalProperties: @@ -235,7 +253,7 @@ spec: format: int64 phase: type: string - enum: [Pending, Running, Sleeping, Updating, Stopped, Failed] + enum: [Pending, Starting, Running, Updating, Stopping, Sleeping, Stopped, Failed] matrixUserID: type: string roomID: @@ -257,6 +275,21 @@ spec: type: integer domain: type: string + deployMode: + type: string + enum: ["Local", "Remote"] + description: "Applied deployment mode for the current Worker runtime resource." + targetCluster: + type: object + description: "Applied remote cluster target for the current Worker runtime resource." + properties: + id: + type: string + description: "ACS/ACK cluster ID." + namespace: + type: string + description: "Target namespace in the remote cluster." + required: ["id", "namespace"] subresources: status: {} additionalPrinterColumns: diff --git a/hiclaw-controller/internal/app/app.go b/hiclaw-controller/internal/app/app.go index cc70101ba..89ad8135d 100644 --- a/hiclaw-controller/internal/app/app.go +++ b/hiclaw-controller/internal/app/app.go @@ -23,6 +23,7 @@ import ( "github.com/hiclaw/hiclaw-controller/internal/initializer" "github.com/hiclaw/hiclaw-controller/internal/matrix" "github.com/hiclaw/hiclaw-controller/internal/oss" + "github.com/hiclaw/hiclaw-controller/internal/remoteclient" "github.com/hiclaw/hiclaw-controller/internal/server" "github.com/hiclaw/hiclaw-controller/internal/service" "github.com/hiclaw/hiclaw-controller/internal/store" @@ -76,6 +77,12 @@ type App struct { agentGen *agentconfig.Generator registry *backend.Registry + // Remote-cluster k8s client cache. Non-nil only when the credential + // provider sidecar is configured; consumed by the K8s worker backend + // to route operations against Workers/Managers deployed to remote + // clusters and refreshed by a background maintenance loop. + remoteClientCache *remoteclient.Cache + // Service layer provisioner *service.Provisioner deployer *service.Deployer @@ -94,8 +101,12 @@ func New(ctx context.Context, cfg *config.Config) (*App, error) { }{ {"scheme", a.initScheme}, {"infra-clients", a.initInfraClients}, - {"backends", a.initBackends}, + // controller-manager must be initialized before backends so that + // initBackends can construct the remote-client cache with + // mgr.GetClient() (only used by the maintenance loop, not yet at + // construction time). {"controller-manager", a.initControllerManager}, + {"backends", a.initBackends}, {"field-indexers", a.initFieldIndexers}, {"auth", a.initAuth}, {"service-layer", a.initServiceLayer}, @@ -144,6 +155,13 @@ func (a *App) Start(ctx context.Context) error { } }) + // Launch the remote-client cache maintenance loop. StartMaintenanceLoop + // internally spawns its own goroutine and returns immediately; it + // runs until ctx is cancelled. + if a.remoteClientCache != nil { + a.remoteClientCache.StartMaintenanceLoop(ctx) + } + // Run cluster initialization only after this instance becomes the leader. // In embedded mode (no leader election) Elected() closes immediately. a.wg.Go(func() { @@ -175,12 +193,12 @@ func (a *App) Start(ctx context.Context) error { TuwunelURL: a.cfg.MatrixServerURL, ElementWebURL: a.cfg.ElementWebURL, ControllerName: a.cfg.ControllerName, - AppServiceEnabled: a.cfg.MatrixAppServiceEnabled, - AppServiceID: a.cfg.MatrixAppServiceID, - AppServiceToken: a.cfg.MatrixAppServiceASToken, - AppServiceHSToken: a.cfg.MatrixAppServiceHSToken, - AppServiceSenderLocalpart: a.cfg.MatrixAppServiceSenderLocalpart, - MatrixDomain: a.cfg.MatrixDomain, + AppServiceEnabled: a.cfg.MatrixAppServiceEnabled, + AppServiceID: a.cfg.MatrixAppServiceID, + AppServiceToken: a.cfg.MatrixAppServiceASToken, + AppServiceHSToken: a.cfg.MatrixAppServiceHSToken, + AppServiceSenderLocalpart: a.cfg.MatrixAppServiceSenderLocalpart, + MatrixDomain: a.cfg.MatrixDomain, }, } if err := init.Run(ctx); err != nil { @@ -356,7 +374,20 @@ func (s *ossControllerCredSource) Resolve(ctx context.Context) (oss.Credentials, } func (a *App) initBackends(_ context.Context) error { - workerBackends := buildWorkerBackends(a.cfg, a.scheme) + // Initialize the remote-cluster k8s client cache when the credential + // provider sidecar is configured. The cache holds references to + // mgr.GetClient() and the credential client; actual List calls happen + // later from the maintenance loop and from GetOrCreate, by which time + // the manager's cache will be running. + if a.credProvider != nil { + a.remoteClientCache = remoteclient.NewCache(remoteclient.CacheConfig{ + CredClient: a.credProvider, + CtrlClient: a.mgr.GetClient(), + Scheme: a.scheme, + ControllerName: a.cfg.ControllerName, + }) + } + workerBackends := buildWorkerBackends(a.cfg, a.scheme, a.remoteClientCache) a.registry = backend.NewRegistry(workerBackends) return nil } @@ -435,7 +466,7 @@ func (a *App) initAuth(ctx context.Context) error { if err != nil { return fmt.Errorf("create kubernetes client: %w", err) } - authenticator := authpkg.NewTokenReviewAuthenticator(a.k8sClient, a.cfg.AuthAudience, authpkg.ResourcePrefix(a.cfg.ResourcePrefix)) + authenticator := authpkg.NewTokenReviewAuthenticator(a.k8sClient, a.cfg.AuthAudience, authpkg.ResourcePrefix(a.cfg.ResourcePrefix), a.remoteClientCache) go authenticator.StartCleanup(ctx) enricher := authpkg.NewCREnricher(a.mgr.GetClient(), a.namespace) authorizer := authpkg.NewAuthorizer() @@ -496,6 +527,7 @@ func (a *App) initServiceLayer(_ context.Context) error { AIGatewayURL: cfg.WorkerEnv.AIGatewayURL, ManagerModel: cfg.ManagerModel, MatrixConfig: cfg.MatrixConfig(), + RemoteCache: a.remoteClientCache, }) a.envBuilder = service.NewWorkerEnvBuilder(cfg.WorkerEnv) @@ -513,15 +545,15 @@ func (a *App) initServiceLayer(_ context.Context) error { } a.deployer = service.NewDeployer(service.DeployerConfig{ - AgentConfig: a.agentGen, - OSS: a.oss, - Executor: a.shell, - Packages: a.packages, - Legacy: a.legacy, - AgentFSDir: cfg.AgentFSDir(), - WorkerAgentDir: cfg.WorkerAgentDir(), - MatrixDomain: cfg.MatrixDomain, - NacosCredClient: a.credProvider, + AgentConfig: a.agentGen, + OSS: a.oss, + Executor: a.shell, + Packages: a.packages, + Legacy: a.legacy, + AgentFSDir: cfg.AgentFSDir(), + WorkerAgentDir: cfg.WorkerAgentDir(), + MatrixDomain: cfg.MatrixDomain, + NacosCredClient: a.credProvider, }) return nil @@ -529,33 +561,41 @@ func (a *App) initServiceLayer(_ context.Context) error { func (a *App) initReconcilers(_ context.Context) error { resourcePrefix := authpkg.ResourcePrefix(a.cfg.ResourcePrefix) + var remoteWatchRegistrar controller.RemoteWatchRegistrar + if a.remoteClientCache != nil { + remoteWatchRegistrar = a.remoteClientCache + } if err := (&controller.WorkerReconciler{ - Client: a.mgr.GetClient(), - Provisioner: a.provisioner, - Deployer: a.deployer, - Backend: a.registry, - EnvBuilder: a.envBuilder, - ResourcePrefix: resourcePrefix, - Legacy: a.legacy, - DefaultRuntime: a.cfg.DefaultWorkerRuntime, - ControllerName: a.cfg.ControllerName, - GatewayClient: a.gateway, + Client: a.mgr.GetClient(), + Provisioner: a.provisioner, + Deployer: a.deployer, + Backend: a.registry, + EnvBuilder: a.envBuilder, + ResourcePrefix: resourcePrefix, + Legacy: a.legacy, + DefaultRuntime: a.cfg.DefaultWorkerRuntime, + ControllerName: a.cfg.ControllerName, + Namespace: a.namespace, + RemoteWatchRegistrar: remoteWatchRegistrar, + GatewayClient: a.gateway, }).SetupWithManager(a.mgr); err != nil { return fmt.Errorf("setup WorkerReconciler: %w", err) } if err := (&controller.TeamReconciler{ - Client: a.mgr.GetClient(), - Provisioner: a.provisioner, - Deployer: a.deployer, - Backend: a.registry, - EnvBuilder: a.envBuilder, - Legacy: a.legacy, - DefaultRuntime: a.cfg.DefaultWorkerRuntime, - AgentFSDir: a.cfg.AgentFSDir(), - ControllerName: a.cfg.ControllerName, - ResourcePrefix: resourcePrefix, - GatewayClient: a.gateway, + Client: a.mgr.GetClient(), + Provisioner: a.provisioner, + Deployer: a.deployer, + Backend: a.registry, + EnvBuilder: a.envBuilder, + Legacy: a.legacy, + DefaultRuntime: a.cfg.DefaultWorkerRuntime, + AgentFSDir: a.cfg.AgentFSDir(), + ControllerName: a.cfg.ControllerName, + Namespace: a.namespace, + RemoteWatchRegistrar: remoteWatchRegistrar, + ResourcePrefix: resourcePrefix, + GatewayClient: a.gateway, }).SetupWithManager(a.mgr); err != nil { return fmt.Errorf("setup TeamReconciler: %w", err) } @@ -793,7 +833,7 @@ func bootstrapAdminCLIToken(ctx context.Context, prov *service.Provisioner) erro // backend doesn't need it. // Gateway selection is handled in initInfraClients via gateway.Client, // so this function only cares about worker runtimes (docker vs k8s). -func buildWorkerBackends(cfg *config.Config, scheme *runtime.Scheme) []backend.WorkerBackend { +func buildWorkerBackends(cfg *config.Config, scheme *runtime.Scheme, remoteCache backend.RemoteClientProvider) []backend.WorkerBackend { var workers []backend.WorkerBackend if cfg.KubeMode == "embedded" { @@ -807,7 +847,10 @@ func buildWorkerBackends(cfg *config.Config, scheme *runtime.Scheme) []backend.W switch effectiveBackend { case "k8s": - if k8s, err := backend.NewK8sBackend(cfg.K8sConfig(), cfg.ContainerPrefix, scheme); err != nil { + // remoteCache is nil when the credential provider sidecar is not + // configured; in that case NewK8sBackendWithCache behaves + // identically to NewK8sBackend. + if k8s, err := backend.NewK8sBackendWithCache(cfg.K8sConfig(), cfg.ContainerPrefix, scheme, remoteCache); err != nil { log.Printf("[WARN] Failed to create K8s backend: %v", err) } else { workers = append(workers, k8s) diff --git a/hiclaw-controller/internal/auth/authenticator.go b/hiclaw-controller/internal/auth/authenticator.go index fcaaccb8a..75727c9f0 100644 --- a/hiclaw-controller/internal/auth/authenticator.go +++ b/hiclaw-controller/internal/auth/authenticator.go @@ -7,6 +7,8 @@ import ( "sync" "time" + "github.com/hiclaw/hiclaw-controller/internal/backend" + authenticationv1 "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" @@ -32,15 +34,19 @@ const ( // CallerIdentity represents the authenticated caller. type CallerIdentity struct { - Role string // admin | manager | team-leader | worker - Username string // canonical name (worker name, "manager", or "admin") - Team string // team name (filled by Enricher, empty for standalone) - WorkerName string // equals Username when Role is worker or team-leader + Role string // admin | manager | team-leader | worker + Username string // canonical name (worker name, "manager", or "admin") + Team string // team name (filled by Enricher, empty for standalone) + WorkerName string // equals Username when Role is worker or team-leader + ClusterID string // remote cluster selected by TokenReview; empty for local + ServiceAccountNamespace string // namespace parsed from TokenReview username + ServiceAccountName string // service account parsed from TokenReview username } // Authenticator validates a bearer token and returns a basic identity. +// clusterID selects the cluster for TokenReview; empty string means local. type Authenticator interface { - Authenticate(ctx context.Context, token string) (*CallerIdentity, error) + Authenticate(ctx context.Context, token string, clusterID string) (*CallerIdentity, error) } // TokenReviewAuthenticator validates tokens via the K8s TokenReview API. @@ -50,9 +56,10 @@ type Authenticator interface { // oldest-expiry eviction. This keeps memory usage proportional to live tokens // even under adversarial input. type TokenReviewAuthenticator struct { - client kubernetes.Interface - audience string - prefix ResourcePrefix + client kubernetes.Interface + audience string + prefix ResourcePrefix + remoteCache backend.RemoteClientProvider // remote cluster client resolver; nil = local-only cacheMu sync.RWMutex cache map[[32]byte]cachedResult @@ -70,7 +77,7 @@ type cachedResult struct { // TokenReview API. audience is the expected token audience (typically // "hiclaw-controller"); prefix is the tenant resource prefix used to parse // SA usernames back into CallerIdentity. -func NewTokenReviewAuthenticator(client kubernetes.Interface, audience string, prefix ResourcePrefix) *TokenReviewAuthenticator { +func NewTokenReviewAuthenticator(client kubernetes.Interface, audience string, prefix ResourcePrefix, remoteCache backend.RemoteClientProvider) *TokenReviewAuthenticator { if audience == "" { audience = DefaultAudience } @@ -78,6 +85,7 @@ func NewTokenReviewAuthenticator(client kubernetes.Interface, audience string, p client: client, audience: audience, prefix: prefix.Or(DefaultResourcePrefix), + remoteCache: remoteCache, cache: make(map[[32]byte]cachedResult), cacheTTL: defaultCacheTTL, cacheMax: defaultCacheMax, @@ -85,12 +93,13 @@ func NewTokenReviewAuthenticator(client kubernetes.Interface, audience string, p } } -func (a *TokenReviewAuthenticator) Authenticate(ctx context.Context, token string) (*CallerIdentity, error) { +func (a *TokenReviewAuthenticator) Authenticate(ctx context.Context, token string, clusterID string) (*CallerIdentity, error) { if token == "" { return nil, fmt.Errorf("empty token") } - key := sha256.Sum256([]byte(token)) + // Include clusterID in cache key so tokens for different clusters don't collide. + key := sha256.Sum256([]byte(clusterID + ":" + token)) if id := a.getFromCache(key); id != nil { return id, nil @@ -103,7 +112,23 @@ func (a *TokenReviewAuthenticator) Authenticate(ctx context.Context, token strin }, } - result, err := a.client.AuthenticationV1().TokenReviews().Create(ctx, review, metav1.CreateOptions{}) + var result *authenticationv1.TokenReview + var err error + + if clusterID == "" { + // Local cluster TokenReview. + result, err = a.client.AuthenticationV1().TokenReviews().Create(ctx, review, metav1.CreateOptions{}) + } else { + // Remote cluster TokenReview. + if a.remoteCache == nil { + return nil, fmt.Errorf("remote cluster authentication not supported") + } + cli, resolveErr := a.remoteCache.ResolveClient(ctx, clusterID) + if resolveErr != nil { + return nil, fmt.Errorf("resolve remote cluster %q: %w", clusterID, resolveErr) + } + result, err = cli.TokenReviews().Create(ctx, review, metav1.CreateOptions{}) + } if err != nil { return nil, fmt.Errorf("token review request failed: %w", err) } @@ -116,6 +141,10 @@ func (a *TokenReviewAuthenticator) Authenticate(ctx context.Context, token strin if err != nil { return nil, err } + identity.ClusterID = clusterID + if clusterID != "" && (identity.Role == RoleAdmin || identity.Role == RoleManager) { + return nil, fmt.Errorf("remote cluster token cannot authenticate as %s", identity.Role) + } a.putInCache(key, identity) return identity, nil diff --git a/hiclaw-controller/internal/auth/authenticator_test.go b/hiclaw-controller/internal/auth/authenticator_test.go index b1314a808..532140318 100644 --- a/hiclaw-controller/internal/auth/authenticator_test.go +++ b/hiclaw-controller/internal/auth/authenticator_test.go @@ -1,6 +1,16 @@ package auth -import "testing" +import ( + "context" + "fmt" + "testing" + + "github.com/hiclaw/hiclaw-controller/internal/backend" + authenticationv1 "k8s.io/api/authentication/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + fakeclient "k8s.io/client-go/kubernetes/fake" +) func TestParseSAUsername_Admin(t *testing.T) { id, err := DefaultResourcePrefix.ParseSAUsername("system:serviceaccount:hiclaw:hiclaw-admin") @@ -10,6 +20,9 @@ func TestParseSAUsername_Admin(t *testing.T) { if id.Role != RoleAdmin || id.Username != "admin" { t.Errorf("expected admin, got %+v", id) } + if id.ServiceAccountNamespace != "hiclaw" || id.ServiceAccountName != "hiclaw-admin" { + t.Errorf("unexpected service account identity: %+v", id) + } } func TestParseSAUsername_Manager(t *testing.T) { @@ -30,6 +43,9 @@ func TestParseSAUsername_Worker(t *testing.T) { if id.Role != RoleWorker || id.Username != "alice" || id.WorkerName != "alice" { t.Errorf("expected worker alice, got %+v", id) } + if id.ServiceAccountNamespace != "hiclaw" || id.ServiceAccountName != "hiclaw-worker-alice" { + t.Errorf("unexpected service account identity: %+v", id) + } } func TestParseSAUsername_WorkerHyphenatedName(t *testing.T) { @@ -69,6 +85,9 @@ func TestParseSAUsername_CustomPrefix(t *testing.T) { if id.Role != RoleWorker || id.Username != "alice" { t.Errorf("expected worker alice, got %+v", id) } + if id.ServiceAccountNamespace != "hiclaw" || id.ServiceAccountName != "teamB-worker-alice" { + t.Errorf("unexpected service account identity: %+v", id) + } if _, err := p.ParseSAUsername("system:serviceaccount:hiclaw:hiclaw-worker-alice"); err == nil { t.Errorf("default-prefixed SA must not match the teamB prefix") @@ -81,6 +100,9 @@ func TestParseSAUsername_CustomPrefix(t *testing.T) { if id.Role != RoleManager || id.Username != "manager" { t.Errorf("expected manager, got %+v", id) } + if id.ServiceAccountNamespace != "hiclaw" || id.ServiceAccountName != "teamB-manager" { + t.Errorf("unexpected service account identity: %+v", id) + } } func TestSAName(t *testing.T) { @@ -146,3 +168,148 @@ func TestResourcePrefix_EmptyFallsBackToDefault(t *testing.T) { t.Errorf("empty prefix should fall back to default, got %q", p.WorkerNamePrefix()) } } + +// --- Mock types for remote TokenReview tests --- + +// fakeTokenReviewClient implements backend.K8sTokenReviewClient. +type fakeTokenReviewClient struct { + authenticated bool + username string + err error +} + +func (f *fakeTokenReviewClient) Create(_ context.Context, review *authenticationv1.TokenReview, _ metav1.CreateOptions) (*authenticationv1.TokenReview, error) { + if f.err != nil { + return nil, f.err + } + review.Status = authenticationv1.TokenReviewStatus{ + Authenticated: f.authenticated, + User: authenticationv1.UserInfo{ + Username: f.username, + }, + } + if !f.authenticated { + review.Status.Error = "token is invalid" + } + return review, nil +} + +// fakeRemoteCoreClient implements backend.K8sCoreClient for authenticator test. +type fakeRemoteCoreClient struct { + tokenReviewClient *fakeTokenReviewClient +} + +func (f *fakeRemoteCoreClient) Pods(_ string) backend.K8sPodClient { return nil } +func (f *fakeRemoteCoreClient) ConfigMaps(_ string) backend.K8sConfigMapClient { return nil } +func (f *fakeRemoteCoreClient) Services(_ string) backend.K8sServiceClient { return nil } +func (f *fakeRemoteCoreClient) Namespaces() backend.K8sNamespaceClient { return nil } +func (f *fakeRemoteCoreClient) ServiceAccounts(_ string) backend.K8sServiceAccountClient { return nil } +func (f *fakeRemoteCoreClient) TokenReviews() backend.K8sTokenReviewClient { + return f.tokenReviewClient +} + +// fakeRemoteProvider implements backend.RemoteClientProvider for authenticator test. +type fakeRemoteProvider struct { + clients map[string]backend.K8sCoreClient +} + +func (f *fakeRemoteProvider) ResolveClient(_ context.Context, clusterID string) (backend.K8sCoreClient, error) { + if cli, ok := f.clients[clusterID]; ok { + return cli, nil + } + return nil, fmt.Errorf("cluster %q not found", clusterID) +} + +// --- Remote Authenticate tests --- + +func TestAuthenticate_RemoteCluster(t *testing.T) { + remoteTR := &fakeTokenReviewClient{ + authenticated: true, + username: "system:serviceaccount:hiclaw:hiclaw-worker-alice", + } + remoteCli := &fakeRemoteCoreClient{tokenReviewClient: remoteTR} + remoteProvider := &fakeRemoteProvider{ + clients: map[string]backend.K8sCoreClient{"remote-cluster": remoteCli}, + } + + localClient := fakeclient.NewSimpleClientset() + auth := NewTokenReviewAuthenticator(localClient, DefaultAudience, DefaultResourcePrefix, remoteProvider) + + // Remote cluster path: clusterID != "" uses remote client. + id, err := auth.Authenticate(context.Background(), "remote-token", "remote-cluster") + if err != nil { + t.Fatalf("Authenticate remote: %v", err) + } + if id.Role != RoleWorker || id.Username != "alice" { + t.Fatalf("expected worker alice, got %+v", id) + } + if id.ClusterID != "remote-cluster" || id.ServiceAccountNamespace != "hiclaw" || id.ServiceAccountName != "hiclaw-worker-alice" { + t.Fatalf("unexpected remote identity metadata: %+v", id) + } +} + +func TestAuthenticate_RemoteClusterRejectsAdminAndManager(t *testing.T) { + for _, username := range []string{ + "system:serviceaccount:hiclaw:hiclaw-admin", + "system:serviceaccount:hiclaw:hiclaw-manager", + } { + remoteTR := &fakeTokenReviewClient{ + authenticated: true, + username: username, + } + remoteCli := &fakeRemoteCoreClient{tokenReviewClient: remoteTR} + remoteProvider := &fakeRemoteProvider{ + clients: map[string]backend.K8sCoreClient{"remote-cluster": remoteCli}, + } + auth := NewTokenReviewAuthenticator(fakeclient.NewSimpleClientset(), DefaultAudience, DefaultResourcePrefix, remoteProvider) + + if _, err := auth.Authenticate(context.Background(), "remote-token-"+username, "remote-cluster"); err == nil { + t.Fatalf("expected remote token for %s to be rejected", username) + } + } +} + +func TestAuthenticate_RemoteCacheNil(t *testing.T) { + // When remoteCache is nil, remote authentication should fail. + localClient := fakeclient.NewSimpleClientset() + auth := NewTokenReviewAuthenticator(localClient, DefaultAudience, DefaultResourcePrefix, nil) + + _, err := auth.Authenticate(context.Background(), "some-token", "remote-cluster") + if err == nil { + t.Fatal("expected error when remoteCache is nil and clusterID is non-empty") + } +} + +func TestAuthenticate_EmptyClusterID_UsesLocalPath(t *testing.T) { + // When clusterID is empty, should use the local k8s client (regression test). + // We use the fake k8s client which won't have a proper TokenReview handler, + // so this tests that the code path goes through local client, not remote. + localClient := fakeclient.NewSimpleClientset() + + remoteTR := &fakeTokenReviewClient{ + authenticated: true, + username: "system:serviceaccount:hiclaw:hiclaw-worker-remote-only", + } + remoteCli := &fakeRemoteCoreClient{tokenReviewClient: remoteTR} + remoteProvider := &fakeRemoteProvider{ + clients: map[string]backend.K8sCoreClient{"remote-cluster": remoteCli}, + } + + auth := NewTokenReviewAuthenticator(localClient, DefaultAudience, DefaultResourcePrefix, remoteProvider) + + // Empty clusterID goes local. fake k8s client returns Authenticated=false + // (TokenReview is not supported by fakeclient), so we expect an auth error. + _, err := auth.Authenticate(context.Background(), "local-token", "") + if err == nil { + t.Fatal("expected error from local fake client (no TokenReview support)") + } + // Confirm it's NOT a "remote cluster authentication not supported" error. + if err.Error() == "remote cluster authentication not supported" { + t.Fatal("should not take remote path when clusterID is empty") + } +} + +// Ensure unused imports are consumed. +var ( + _ = corev1.ServiceAccount{} +) diff --git a/hiclaw-controller/internal/auth/enricher.go b/hiclaw-controller/internal/auth/enricher.go index 4284162d1..1eedcef9d 100644 --- a/hiclaw-controller/internal/auth/enricher.go +++ b/hiclaw-controller/internal/auth/enricher.go @@ -31,10 +31,15 @@ type IdentityEnricher interface { type CREnricher struct { client client.Client namespace string + prefix ResourcePrefix } -func NewCREnricher(c client.Client, namespace string) *CREnricher { - return &CREnricher{client: c, namespace: namespace} +func NewCREnricher(c client.Client, namespace string, prefix ...ResourcePrefix) *CREnricher { + p := DefaultResourcePrefix + if len(prefix) > 0 { + p = prefix[0].Or(DefaultResourcePrefix) + } + return &CREnricher{client: c, namespace: namespace, prefix: p} } func (e *CREnricher) EnrichIdentity(ctx context.Context, identity *CallerIdentity) error { @@ -53,6 +58,9 @@ func (e *CREnricher) EnrichIdentity(ctx context.Context, identity *CallerIdentit err := e.client.Get(ctx, key, &worker) switch { case err == nil: + if err := e.validateRemoteWorkerIdentity(identity, &worker); err != nil { + return err + } runtimeName := worker.Spec.EffectiveWorkerName(worker.Name) identity.WorkerName = runtimeName if role := worker.Annotations["hiclaw.io/role"]; role == "team_leader" { @@ -65,6 +73,9 @@ func (e *CREnricher) EnrichIdentity(ctx context.Context, identity *CallerIdentit case !apierrors.IsNotFound(err): return fmt.Errorf("enrich identity: get worker %q: %w", identity.Username, err) } + if identity.ClusterID != "" { + return fmt.Errorf("remote identity %q is not backed by a Worker CR", identity.Username) + } // 2. Worker CR missing — fall back to Team CR reverse lookup. A worker // name can only belong to one team at a time; the same is true for @@ -99,6 +110,47 @@ func (e *CREnricher) EnrichIdentity(ctx context.Context, identity *CallerIdentit return nil } +func (e *CREnricher) validateRemoteWorkerIdentity(identity *CallerIdentity, worker *v1beta1.Worker) error { + if identity.ClusterID == "" { + return nil + } + deployMode, targetCluster := remoteWorkerAppliedTarget(worker) + if deployMode != v1beta1.DeployModeRemote { + return fmt.Errorf("remote identity %q is not backed by a remote Worker", identity.Username) + } + if targetCluster == nil { + return fmt.Errorf("remote Worker %q has no targetCluster", worker.Name) + } + if targetCluster.ID != identity.ClusterID { + return fmt.Errorf("remote identity %q cluster %q does not match Worker target cluster %q", + identity.Username, identity.ClusterID, targetCluster.ID) + } + if targetCluster.Namespace != identity.ServiceAccountNamespace { + return fmt.Errorf("remote identity %q namespace %q does not match Worker target namespace %q", + identity.Username, identity.ServiceAccountNamespace, targetCluster.Namespace) + } + expectedSA := e.prefix.SAName(RoleWorker, worker.Name) + if identity.ServiceAccountName != expectedSA { + return fmt.Errorf("remote identity %q serviceAccount %q does not match Worker serviceAccount %q", + identity.Username, identity.ServiceAccountName, expectedSA) + } + return nil +} + +func remoteWorkerAppliedTarget(worker *v1beta1.Worker) (string, *v1beta1.TargetClusterSpec) { + if worker.Status.DeployMode != "" || worker.Status.TargetCluster != nil { + deployMode := worker.Status.DeployMode + if deployMode == "" { + deployMode = v1beta1.DeployModeLocal + } + return deployMode, worker.Status.TargetCluster + } + if worker.Spec.DeployMode == nil || *worker.Spec.DeployMode == "" { + return v1beta1.DeployModeLocal, nil + } + return *worker.Spec.DeployMode, worker.Spec.TargetCluster +} + func (e *CREnricher) lookupTeamByField(ctx context.Context, field, value string) (*v1beta1.Team, bool, error) { var list v1beta1.TeamList if err := e.client.List(ctx, &list, diff --git a/hiclaw-controller/internal/auth/enricher_test.go b/hiclaw-controller/internal/auth/enricher_test.go index 83892a972..f86c834b2 100644 --- a/hiclaw-controller/internal/auth/enricher_test.go +++ b/hiclaw-controller/internal/auth/enricher_test.go @@ -116,6 +116,185 @@ func TestCREnricher_TeamLeaderKeepsCRNameAndStoresRuntimeWorkerName(t *testing.T } } +func TestCREnricher_RemoteWorkerRequiresMatchingTarget(t *testing.T) { + scheme := newAuthTestScheme(t) + remoteMode := v1beta1.DeployModeRemote + worker := &v1beta1.Worker{ + ObjectMeta: metav1.ObjectMeta{ + Name: "alice", + Namespace: "default", + }, + Spec: v1beta1.WorkerSpec{ + DeployMode: &remoteMode, + TargetCluster: &v1beta1.TargetClusterSpec{ + ID: "remote-cluster", + Namespace: "remote-ns", + }, + }, + } + + k8sClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(worker). + Build() + enricher := NewCREnricher(k8sClient, "default") + + identity := &CallerIdentity{ + Role: RoleWorker, + Username: "alice", + WorkerName: "alice", + ClusterID: "remote-cluster", + ServiceAccountNamespace: "remote-ns", + ServiceAccountName: "hiclaw-worker-alice", + } + if err := enricher.EnrichIdentity(context.Background(), identity); err != nil { + t.Fatalf("EnrichIdentity: %v", err) + } +} + +func TestCREnricher_RemoteWorkerPrefersStatusTarget(t *testing.T) { + scheme := newAuthTestScheme(t) + remoteMode := v1beta1.DeployModeRemote + worker := &v1beta1.Worker{ + ObjectMeta: metav1.ObjectMeta{ + Name: "alice", + Namespace: "default", + }, + Spec: v1beta1.WorkerSpec{ + DeployMode: &remoteMode, + TargetCluster: &v1beta1.TargetClusterSpec{ + ID: "new-cluster", + Namespace: "new-ns", + }, + }, + Status: v1beta1.WorkerStatus{ + DeployMode: v1beta1.DeployModeRemote, + TargetCluster: &v1beta1.TargetClusterSpec{ + ID: "old-cluster", + Namespace: "old-ns", + }, + }, + } + + k8sClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(worker). + Build() + enricher := NewCREnricher(k8sClient, "default") + + oldTargetIdentity := &CallerIdentity{ + Role: RoleWorker, + Username: "alice", + WorkerName: "alice", + ClusterID: "old-cluster", + ServiceAccountNamespace: "old-ns", + ServiceAccountName: "hiclaw-worker-alice", + } + if err := enricher.EnrichIdentity(context.Background(), oldTargetIdentity); err != nil { + t.Fatalf("EnrichIdentity old target: %v", err) + } + + newTargetIdentity := &CallerIdentity{ + Role: RoleWorker, + Username: "alice", + WorkerName: "alice", + ClusterID: "new-cluster", + ServiceAccountNamespace: "new-ns", + ServiceAccountName: "hiclaw-worker-alice", + } + if err := enricher.EnrichIdentity(context.Background(), newTargetIdentity); err == nil { + t.Fatal("expected spec-only new target to be rejected while status still pins old target") + } +} + +func TestCREnricher_RemoteWorkerRejectsTargetMismatch(t *testing.T) { + scheme := newAuthTestScheme(t) + remoteMode := v1beta1.DeployModeRemote + localMode := v1beta1.DeployModeLocal + + tests := []struct { + name string + worker *v1beta1.Worker + identity *CallerIdentity + }{ + { + name: "wrong namespace", + worker: &v1beta1.Worker{ + ObjectMeta: metav1.ObjectMeta{Name: "alice", Namespace: "default"}, + Spec: v1beta1.WorkerSpec{ + DeployMode: &remoteMode, + TargetCluster: &v1beta1.TargetClusterSpec{ID: "remote-cluster", Namespace: "remote-ns"}, + }, + }, + identity: &CallerIdentity{ + Role: RoleWorker, Username: "alice", WorkerName: "alice", ClusterID: "remote-cluster", + ServiceAccountNamespace: "other-ns", ServiceAccountName: "hiclaw-worker-alice", + }, + }, + { + name: "wrong cluster", + worker: &v1beta1.Worker{ + ObjectMeta: metav1.ObjectMeta{Name: "alice", Namespace: "default"}, + Spec: v1beta1.WorkerSpec{ + DeployMode: &remoteMode, + TargetCluster: &v1beta1.TargetClusterSpec{ID: "remote-cluster", Namespace: "remote-ns"}, + }, + }, + identity: &CallerIdentity{ + Role: RoleWorker, Username: "alice", WorkerName: "alice", ClusterID: "other-cluster", + ServiceAccountNamespace: "remote-ns", ServiceAccountName: "hiclaw-worker-alice", + }, + }, + { + name: "wrong service account", + worker: &v1beta1.Worker{ + ObjectMeta: metav1.ObjectMeta{Name: "alice", Namespace: "default"}, + Spec: v1beta1.WorkerSpec{ + DeployMode: &remoteMode, + TargetCluster: &v1beta1.TargetClusterSpec{ID: "remote-cluster", Namespace: "remote-ns"}, + }, + }, + identity: &CallerIdentity{ + Role: RoleWorker, Username: "alice", WorkerName: "alice", ClusterID: "remote-cluster", + ServiceAccountNamespace: "remote-ns", ServiceAccountName: "hiclaw-worker-bob", + }, + }, + { + name: "local worker", + worker: &v1beta1.Worker{ + ObjectMeta: metav1.ObjectMeta{Name: "alice", Namespace: "default"}, + Spec: v1beta1.WorkerSpec{DeployMode: &localMode}, + }, + identity: &CallerIdentity{ + Role: RoleWorker, Username: "alice", WorkerName: "alice", ClusterID: "remote-cluster", + ServiceAccountNamespace: "remote-ns", ServiceAccountName: "hiclaw-worker-alice", + }, + }, + { + name: "missing worker cr", + worker: nil, + identity: &CallerIdentity{ + Role: RoleWorker, Username: "alice", WorkerName: "alice", ClusterID: "remote-cluster", + ServiceAccountNamespace: "remote-ns", ServiceAccountName: "hiclaw-worker-alice", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + builder := fake.NewClientBuilder().WithScheme(scheme) + if tc.worker != nil { + builder = builder.WithObjects(tc.worker) + } + enricher := NewCREnricher(builder.Build(), "default") + + if err := enricher.EnrichIdentity(context.Background(), tc.identity); err == nil { + t.Fatal("expected remote identity mismatch to fail") + } + }) + } +} + func newAuthTestScheme(t *testing.T) *runtime.Scheme { t.Helper() scheme := runtime.NewScheme() diff --git a/hiclaw-controller/internal/auth/middleware.go b/hiclaw-controller/internal/auth/middleware.go index e158b9ac2..6469e8a2b 100644 --- a/hiclaw-controller/internal/auth/middleware.go +++ b/hiclaw-controller/internal/auth/middleware.go @@ -140,7 +140,9 @@ func (m *Middleware) authenticateAndEnrich(r *http.Request) (*CallerIdentity, bo return nil, false } - identity, err := m.authenticator.Authenticate(r.Context(), token) + clusterID := r.Header.Get("X-HiClaw-Cluster-ID") + + identity, err := m.authenticator.Authenticate(r.Context(), token, clusterID) if err != nil { log.Printf("[AUTH] authentication failed: %v", err) return nil, false @@ -149,6 +151,9 @@ func (m *Middleware) authenticateAndEnrich(r *http.Request) (*CallerIdentity, bo if m.enricher != nil { if err := m.enricher.EnrichIdentity(r.Context(), identity); err != nil { log.Printf("[AUTH] identity enrichment failed for %s: %v", identity.Username, err) + if clusterID != "" || identity.ClusterID != "" { + return nil, false + } } } diff --git a/hiclaw-controller/internal/auth/middleware_test.go b/hiclaw-controller/internal/auth/middleware_test.go index b2c61c887..e1d3720a8 100644 --- a/hiclaw-controller/internal/auth/middleware_test.go +++ b/hiclaw-controller/internal/auth/middleware_test.go @@ -13,7 +13,7 @@ type mockAuthenticator struct { tokens map[string]*CallerIdentity } -func (m *mockAuthenticator) Authenticate(_ context.Context, token string) (*CallerIdentity, error) { +func (m *mockAuthenticator) Authenticate(_ context.Context, token string, clusterID string) (*CallerIdentity, error) { if id, ok := m.tokens[token]; ok { cp := *id return &cp, nil @@ -26,6 +26,12 @@ type noopEnricher struct{} func (n *noopEnricher) EnrichIdentity(_ context.Context, _ *CallerIdentity) error { return nil } +type failingEnricher struct{} + +func (f *failingEnricher) EnrichIdentity(_ context.Context, _ *CallerIdentity) error { + return fmt.Errorf("enrich failed") +} + func newTestMiddleware(tokens map[string]*CallerIdentity) *Middleware { return NewMiddleware( &mockAuthenticator{tokens: tokens}, @@ -73,6 +79,57 @@ func TestAuthenticate_InvalidToken(t *testing.T) { } } +func TestAuthenticate_RemoteEnrichmentFailureDenies(t *testing.T) { + mw := NewMiddleware( + &mockAuthenticator{tokens: map[string]*CallerIdentity{ + "worker-token": {Role: RoleWorker, Username: "alice", ClusterID: "remote-cluster"}, + }}, + &failingEnricher{}, + NewAuthorizer(), + nil, "", + ) + + handler := mw.Authenticate(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Error("handler should not be called") + })) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/workers", nil) + req.Header.Set("Authorization", "Bearer worker-token") + req.Header.Set("X-HiClaw-Cluster-ID", "remote-cluster") + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Errorf("expected 401, got %d", w.Code) + } +} + +func TestAuthenticate_LocalEnrichmentFailureKeepsLegacyBehavior(t *testing.T) { + mw := NewMiddleware( + &mockAuthenticator{tokens: map[string]*CallerIdentity{ + "worker-token": {Role: RoleWorker, Username: "alice"}, + }}, + &failingEnricher{}, + NewAuthorizer(), + nil, "", + ) + + called := false + handler := mw.Authenticate(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + w.WriteHeader(http.StatusOK) + })) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/workers", nil) + req.Header.Set("Authorization", "Bearer worker-token") + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + if !called || w.Code != http.StatusOK { + t.Errorf("expected local legacy path to continue, called=%v code=%d", called, w.Code) + } +} + func TestAuthenticate_MissingHeader(t *testing.T) { mw := newTestMiddleware(map[string]*CallerIdentity{}) diff --git a/hiclaw-controller/internal/auth/prefix.go b/hiclaw-controller/internal/auth/prefix.go index 7f2a83a12..7026b8bbf 100644 --- a/hiclaw-controller/internal/auth/prefix.go +++ b/hiclaw-controller/internal/auth/prefix.go @@ -128,16 +128,17 @@ func (p ResourcePrefix) ParseSAUsername(username string) (*CallerIdentity, error if len(parts) != 2 { return nil, fmt.Errorf("cannot parse SA from username: %q", username) } + saNamespace := parts[0] saName := parts[1] switch { case saName == p.AdminName(): - return &CallerIdentity{Role: RoleAdmin, Username: "admin"}, nil + return &CallerIdentity{Role: RoleAdmin, Username: "admin", ServiceAccountNamespace: saNamespace, ServiceAccountName: saName}, nil case saName == p.ManagerDefaultName(): - return &CallerIdentity{Role: RoleManager, Username: "manager"}, nil + return &CallerIdentity{Role: RoleManager, Username: "manager", ServiceAccountNamespace: saNamespace, ServiceAccountName: saName}, nil case strings.HasPrefix(saName, p.WorkerNamePrefix()): name := saName[len(p.WorkerNamePrefix()):] - return &CallerIdentity{Role: RoleWorker, Username: name, WorkerName: name}, nil + return &CallerIdentity{Role: RoleWorker, Username: name, WorkerName: name, ServiceAccountNamespace: saNamespace, ServiceAccountName: saName}, nil default: return nil, fmt.Errorf("unrecognized SA name pattern: %q", saName) } diff --git a/hiclaw-controller/internal/backend/agent_pod_template.go b/hiclaw-controller/internal/backend/agent_pod_template.go index f0175e3de..f44e9e457 100644 --- a/hiclaw-controller/internal/backend/agent_pod_template.go +++ b/hiclaw-controller/internal/backend/agent_pod_template.go @@ -8,6 +8,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/yaml" + + v1beta1 "github.com/hiclaw/hiclaw-controller/api/v1beta1" ) // AgentPodTemplateConfigMapKey is the data key inside the controller-scoped @@ -15,6 +17,16 @@ import ( // and only this key; any other keys in the same ConfigMap are ignored. const AgentPodTemplateConfigMapKey = "pod-template.yaml" +// AgentPodTemplateRemoteConfigMapKey is an optional data key inside the same +// ConfigMap. When the agent is being created in remote deploy mode +// (DeployMode=Remote) and this key is present and non-empty, the controller +// uses it instead of AgentPodTemplateConfigMapKey. This lets operators ship +// remote-cluster-specific scheduling fields (tolerations, nodeSelector, +// imagePullSecrets, etc.) without affecting in-cluster Pods. When the key is +// absent or empty in remote mode, the controller returns a zero-value +// PodTemplateSpec (no overlay) — it does NOT fall back to pod-template.yaml. +const AgentPodTemplateRemoteConfigMapKey = "pod-template-remote.yaml" + // PodOverlay carries every controller-computed field that ApplyPodTemplate // must force onto the final Pod. Anything NOT in this struct is either copied // verbatim from the PodTemplateSpec (the "user wins" side of the merge) or @@ -68,7 +80,14 @@ type PodOverlay struct { // - NotFound: V(1) debug — a common "no overlay configured" state. // - Parse failure: Error — the user's YAML is almost certainly wrong. // - Other API errors: Info — likely transient; next Create retries. -func LoadAgentPodTemplate(ctx context.Context, client K8sCoreClient, namespace, name string) corev1.PodTemplateSpec { +// +// When deployMode == v1beta1.DeployModeRemote, the loader only tries the +// AgentPodTemplateRemoteConfigMapKey ("pod-template-remote.yaml") within the +// same ConfigMap. If that key is missing or empty, it returns a zero-value +// PodTemplateSpec (no overlay) without falling back to pod-template.yaml. +// Any other deployMode value (including the empty string) only consults the +// standard key, preserving the original behaviour. +func LoadAgentPodTemplate(ctx context.Context, client K8sCoreClient, namespace, name, deployMode string) corev1.PodTemplateSpec { logger := log.FromContext(ctx).WithName("agent-pod-template") if client == nil || namespace == "" || name == "" { return corev1.PodTemplateSpec{} @@ -84,14 +103,33 @@ func LoadAgentPodTemplate(ctx context.Context, client K8sCoreClient, namespace, "namespace", namespace, "name", name, "err", err.Error()) return corev1.PodTemplateSpec{} } - raw, ok := cm.Data[AgentPodTemplateConfigMapKey] - if !ok || raw == "" { - return corev1.PodTemplateSpec{} + + // Resolve which key to consume. Remote deploy mode uses only the remote + // key; if absent or empty, no overlay is applied (no fallback to local). + var raw string + var usedKey string + if deployMode == v1beta1.DeployModeRemote { + v, ok := cm.Data[AgentPodTemplateRemoteConfigMapKey] + if !ok || v == "" { + logger.V(1).Info("remote pod template key not found or empty; using empty overlay", + "namespace", namespace, "name", name, "key", AgentPodTemplateRemoteConfigMapKey) + return corev1.PodTemplateSpec{} + } + raw = v + usedKey = AgentPodTemplateRemoteConfigMapKey + } else { + v, ok := cm.Data[AgentPodTemplateConfigMapKey] + if !ok || v == "" { + return corev1.PodTemplateSpec{} + } + raw = v + usedKey = AgentPodTemplateConfigMapKey } + var tmpl corev1.PodTemplateSpec if err := yaml.Unmarshal([]byte(raw), &tmpl); err != nil { logger.Error(err, "agent pod template YAML parse failed; using empty overlay", - "namespace", namespace, "name", name, "key", AgentPodTemplateConfigMapKey) + "namespace", namespace, "name", name, "key", usedKey) return corev1.PodTemplateSpec{} } return tmpl diff --git a/hiclaw-controller/internal/backend/agent_pod_template_test.go b/hiclaw-controller/internal/backend/agent_pod_template_test.go index eaca80495..9df3787ea 100644 --- a/hiclaw-controller/internal/backend/agent_pod_template_test.go +++ b/hiclaw-controller/internal/backend/agent_pod_template_test.go @@ -9,6 +9,8 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + v1beta1 "github.com/hiclaw/hiclaw-controller/api/v1beta1" ) // ── fixtures ───────────────────────────────────────────────────────────── @@ -405,7 +407,7 @@ func injectLoaderCM(fake *fakeK8sCoreClient, namespace, name, yaml string) { } func TestLoadAgentPodTemplate_NilClient(t *testing.T) { - got := LoadAgentPodTemplate(context.Background(), nil, loaderTestNS, loaderTestName) + got := LoadAgentPodTemplate(context.Background(), nil, loaderTestNS, loaderTestName, "") if !reflect.DeepEqual(got, corev1.PodTemplateSpec{}) { t.Fatalf("nil client: expected zero PodTemplateSpec, got %+v", got) } @@ -416,18 +418,18 @@ func TestLoadAgentPodTemplate_EmptyNameOrNamespace(t *testing.T) { injectLoaderCM(fake, loaderTestNS, loaderTestName, `metadata: {labels: {x: "y"}}`) // Empty name → no lookup, empty template. - if got := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, ""); !reflect.DeepEqual(got, corev1.PodTemplateSpec{}) { + if got := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, "", ""); !reflect.DeepEqual(got, corev1.PodTemplateSpec{}) { t.Fatalf("empty name: expected zero, got %+v", got) } // Empty namespace → no lookup, empty template. - if got := LoadAgentPodTemplate(context.Background(), fake, "", loaderTestName); !reflect.DeepEqual(got, corev1.PodTemplateSpec{}) { + if got := LoadAgentPodTemplate(context.Background(), fake, "", loaderTestName, ""); !reflect.DeepEqual(got, corev1.PodTemplateSpec{}) { t.Fatalf("empty namespace: expected zero, got %+v", got) } } func TestLoadAgentPodTemplate_ConfigMapNotFound(t *testing.T) { fake := newFakeK8sCoreClient() - got := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName) + got := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName, "") if !reflect.DeepEqual(got, corev1.PodTemplateSpec{}) { t.Fatalf("NotFound: expected zero PodTemplateSpec, got %+v", got) } @@ -446,7 +448,7 @@ spec: operator: Exists effect: NoSchedule `) - got := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName) + got := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName, "") if got.Labels["a"] != "x" { t.Fatalf("labels: %+v", got.Labels) } @@ -465,14 +467,14 @@ func TestLoadAgentPodTemplate_MissingOrEmptyDataKey(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: loaderTestName, Namespace: loaderTestNS}, Data: map[string]string{"something-else": "hello"}, }) - if got := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName); !reflect.DeepEqual(got, corev1.PodTemplateSpec{}) { + if got := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName, ""); !reflect.DeepEqual(got, corev1.PodTemplateSpec{}) { t.Fatalf("missing key: expected zero, got %+v", got) } // ConfigMap has the key but the value is empty string. fake2 := newFakeK8sCoreClient() injectLoaderCM(fake2, loaderTestNS, loaderTestName, "") - if got := LoadAgentPodTemplate(context.Background(), fake2, loaderTestNS, loaderTestName); !reflect.DeepEqual(got, corev1.PodTemplateSpec{}) { + if got := LoadAgentPodTemplate(context.Background(), fake2, loaderTestNS, loaderTestName, ""); !reflect.DeepEqual(got, corev1.PodTemplateSpec{}) { t.Fatalf("empty value: expected zero, got %+v", got) } } @@ -481,7 +483,7 @@ func TestLoadAgentPodTemplate_ParseFailure(t *testing.T) { fake := newFakeK8sCoreClient() // Invalid YAML that fails strict PodTemplateSpec unmarshal. injectLoaderCM(fake, loaderTestNS, loaderTestName, "not-a-podtemplate-at-all: : : :") - got := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName) + got := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName, "") if !reflect.DeepEqual(got, corev1.PodTemplateSpec{}) { t.Fatalf("parse failure must produce zero PodTemplateSpec, got %+v", got) } @@ -493,13 +495,13 @@ func TestLoadAgentPodTemplate_ParseFailure(t *testing.T) { func TestLoadAgentPodTemplate_LiveRead(t *testing.T) { fake := newFakeK8sCoreClient() injectLoaderCM(fake, loaderTestNS, loaderTestName, `metadata: {labels: {v: "1"}}`) - v1 := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName) + v1 := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName, "") if v1.Labels["v"] != "1" { t.Fatalf("v1 labels: %+v", v1.Labels) } injectLoaderCM(fake, loaderTestNS, loaderTestName, `metadata: {labels: {v: "2"}}`) - v2 := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName) + v2 := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName, "") if v2.Labels["v"] != "2" { t.Fatalf("v2 labels (live-read broken): %+v", v2.Labels) } @@ -511,8 +513,101 @@ func TestLoadAgentPodTemplate_LiveRead(t *testing.T) { func TestLoadAgentPodTemplate_GetError(t *testing.T) { fake := newFakeK8sCoreClient() fake.cmGetErr = errors.New("boom: API server unavailable") - got := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName) + got := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName, "") if !reflect.DeepEqual(got, corev1.PodTemplateSpec{}) { t.Fatalf("API error must produce zero PodTemplateSpec, got %+v", got) } } + +// TestLoadAgentPodTemplate_RemoteKeyPreferred validates that when deployMode +// is Remote and pod-template-remote.yaml is present, that key wins over +// pod-template.yaml. +func TestLoadAgentPodTemplate_RemoteKeyPreferred(t *testing.T) { + fake := newFakeK8sCoreClient() + fake.injectConfigMap(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: loaderTestName, Namespace: loaderTestNS}, + Data: map[string]string{ + AgentPodTemplateConfigMapKey: `metadata: {labels: {pick: "local"}}`, + AgentPodTemplateRemoteConfigMapKey: `metadata: {labels: {pick: "remote"}}`, + }, + }) + got := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName, v1beta1.DeployModeRemote) + if got.Labels["pick"] != "remote" { + t.Fatalf("remote mode must consume remote key, got %+v", got.Labels) + } +} + +// TestLoadAgentPodTemplate_RemoteKeyMissing_ReturnsEmpty validates that when +// deployMode is Remote but pod-template-remote.yaml is missing or empty, the +// loader returns a zero PodTemplateSpec (no fallback to pod-template.yaml). +func TestLoadAgentPodTemplate_RemoteKeyMissing_ReturnsEmpty(t *testing.T) { + // Remote key absent. + fake := newFakeK8sCoreClient() + fake.injectConfigMap(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: loaderTestName, Namespace: loaderTestNS}, + Data: map[string]string{ + AgentPodTemplateConfigMapKey: `metadata: {labels: {pick: "local"}}`, + }, + }) + got := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName, v1beta1.DeployModeRemote) + if !reflect.DeepEqual(got, corev1.PodTemplateSpec{}) { + t.Fatalf("missing remote key must return zero PodTemplateSpec, got %+v", got) + } + + // Remote key present but empty. + fake2 := newFakeK8sCoreClient() + fake2.injectConfigMap(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: loaderTestName, Namespace: loaderTestNS}, + Data: map[string]string{ + AgentPodTemplateConfigMapKey: `metadata: {labels: {pick: "local"}}`, + AgentPodTemplateRemoteConfigMapKey: "", + }, + }) + got2 := LoadAgentPodTemplate(context.Background(), fake2, loaderTestNS, loaderTestName, v1beta1.DeployModeRemote) + if !reflect.DeepEqual(got2, corev1.PodTemplateSpec{}) { + t.Fatalf("empty remote key must return zero PodTemplateSpec, got %+v", got2) + } +} + +// TestLoadAgentPodTemplate_NonRemoteIgnoresRemoteKey validates that any +// non-Remote deployMode (including the empty string and the explicit Local +// mode) ignores pod-template-remote.yaml even when it is present. +func TestLoadAgentPodTemplate_NonRemoteIgnoresRemoteKey(t *testing.T) { + fake := newFakeK8sCoreClient() + fake.injectConfigMap(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: loaderTestName, Namespace: loaderTestNS}, + Data: map[string]string{ + AgentPodTemplateConfigMapKey: `metadata: {labels: {pick: "local"}}`, + AgentPodTemplateRemoteConfigMapKey: `metadata: {labels: {pick: "remote"}}`, + }, + }) + for _, mode := range []string{"", v1beta1.DeployModeLocal} { + got := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName, mode) + if got.Labels["pick"] != "local" { + t.Fatalf("deployMode=%q must ignore remote key, got %+v", mode, got.Labels) + } + } +} + +// TestLoadAgentPodTemplate_RemoteKeyOnly validates the case where the +// ConfigMap only carries the remote key (no pod-template.yaml) and the +// caller is in Remote mode. +func TestLoadAgentPodTemplate_RemoteKeyOnly(t *testing.T) { + fake := newFakeK8sCoreClient() + fake.injectConfigMap(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: loaderTestName, Namespace: loaderTestNS}, + Data: map[string]string{ + AgentPodTemplateRemoteConfigMapKey: `metadata: {labels: {pick: "remote"}}`, + }, + }) + got := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName, v1beta1.DeployModeRemote) + if got.Labels["pick"] != "remote" { + t.Fatalf("remote-only ConfigMap must surface remote key, got %+v", got.Labels) + } + + // Same ConfigMap, but caller is non-remote: no usable key, expect zero. + got2 := LoadAgentPodTemplate(context.Background(), fake, loaderTestNS, loaderTestName, "") + if !reflect.DeepEqual(got2, corev1.PodTemplateSpec{}) { + t.Fatalf("non-remote with only remote key must produce zero PodTemplateSpec, got %+v", got2) + } +} diff --git a/hiclaw-controller/internal/backend/interface.go b/hiclaw-controller/internal/backend/interface.go index 1c934f5aa..40befbbd9 100644 --- a/hiclaw-controller/internal/backend/interface.go +++ b/hiclaw-controller/internal/backend/interface.go @@ -4,6 +4,7 @@ import ( "context" "errors" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -20,6 +21,7 @@ const ( StatusRunning WorkerStatus = "running" StatusReady WorkerStatus = "ready" StatusStopped WorkerStatus = "stopped" + StatusSleeping WorkerStatus = "sleeping" StatusStarting WorkerStatus = "starting" StatusNotFound WorkerStatus = "not_found" StatusUnknown WorkerStatus = "unknown" @@ -155,8 +157,24 @@ type CreateRequest struct { // OwnerReference via controllerutil.SetControllerReference, so that // deletion of the owning CR (Worker / Team / Manager) cascades to the // Pod via native K8s garbage collection. Docker backend ignores this - // field. + // field. Skipped when DeployMode is "Remote" (cross-cluster ownerRef + // is not possible). Owner metav1.Object `json:"-"` + + // DeployMode selects local (same cluster) or remote (different cluster) + // Pod placement. "Local" (default/empty) uses the backend's own client; + // "Remote" routes through RemoteClientProvider. + DeployMode string `json:"-"` + + // TargetClusterID identifies the remote cluster (Remote mode only). + TargetClusterID string `json:"-"` + + // TargetNamespace overrides the namespace in the remote cluster (Remote + // mode only). Empty means the backend falls back to its own namespace. + TargetNamespace string `json:"-"` + + // ServiceEnabled indicates whether a Service should be created for the Pod. + ServiceEnabled bool `json:"-"` } // Deployment modes returned by backends. @@ -165,6 +183,36 @@ const ( DeployCloud = "cloud" ) +// RemoteClientProvider abstracts access to remote cluster k8s clients. +// The remoteclient.Cache implements this interface. +type RemoteClientProvider interface { + ResolveClient(ctx context.Context, clusterID string) (K8sCoreClient, error) +} + +// K8sServiceClient is the minimal Service client surface needed by the backend. +type K8sServiceClient interface { + Get(ctx context.Context, name string, opts metav1.GetOptions) (*corev1.Service, error) + Create(ctx context.Context, svc *corev1.Service, opts metav1.CreateOptions) (*corev1.Service, error) + Update(ctx context.Context, svc *corev1.Service, opts metav1.UpdateOptions) (*corev1.Service, error) + Delete(ctx context.Context, name string, opts metav1.DeleteOptions) error +} + +// K8sNamespaceClient is the minimal Namespace client surface needed by the backend. +type K8sNamespaceClient interface { + Get(ctx context.Context, name string, opts metav1.GetOptions) (*corev1.Namespace, error) + Create(ctx context.Context, ns *corev1.Namespace, opts metav1.CreateOptions) (*corev1.Namespace, error) +} + +// ServiceBackend is an optional capability of a WorkerBackend that can +// provide Kubernetes Service lifecycle operations. Only K8sBackend implements +// this; Docker backend does not support Service management. +type ServiceBackend interface { + // ServiceClient returns a K8sServiceClient and resolved namespace for the + // appropriate cluster based on deploy mode. Callers use this to create, + // update, or delete Services alongside member pods. + ServiceClient(ctx context.Context, deployMode, targetClusterID, targetNamespace string) (K8sServiceClient, string, error) +} + // WorkerResult holds the result of a worker operation. type WorkerResult struct { Name string `json:"name"` diff --git a/hiclaw-controller/internal/backend/kubernetes.go b/hiclaw-controller/internal/backend/kubernetes.go index 9d33b1130..ad4354cf0 100644 --- a/hiclaw-controller/internal/backend/kubernetes.go +++ b/hiclaw-controller/internal/backend/kubernetes.go @@ -7,15 +7,19 @@ import ( "sort" "strings" + authenticationv1 "k8s.io/api/authentication/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + authenticationv1client "k8s.io/client-go/kubernetes/typed/authentication/v1" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + v1beta1 "github.com/hiclaw/hiclaw-controller/api/v1beta1" ) const defaultK8sNamespaceFile = "/var/run/secrets/kubernetes.io/serviceaccount/namespace" @@ -48,21 +52,47 @@ type K8sConfig struct { // K8sBackend manages worker lifecycle via Kubernetes Pods. type K8sBackend struct { client K8sCoreClient + remoteCache RemoteClientProvider // remote cluster client cache (may be nil) config K8sConfig containerPrefix string + // Fields for remote routing (set via WithRemoteTarget). + deployMode string + targetClusterID string + targetNamespace string + // scheme is used to resolve GVK for CreateRequest.Owner when stamping // the child Pod's controller OwnerReference via // controllerutil.SetControllerReference. A nil scheme means "callers // never supply Owner" — typical for unit tests that don't exercise // ownerRef behaviour. scheme *runtime.Scheme + + // namespace is a convenience alias for config.Namespace used by + // resolveClient to return the local namespace. + namespace string +} + +// K8sServiceAccountClient is the minimal ServiceAccount client surface needed. +type K8sServiceAccountClient interface { + Get(ctx context.Context, name string, opts metav1.GetOptions) (*corev1.ServiceAccount, error) + Create(ctx context.Context, sa *corev1.ServiceAccount, opts metav1.CreateOptions) (*corev1.ServiceAccount, error) + Delete(ctx context.Context, name string, opts metav1.DeleteOptions) error +} + +// K8sTokenReviewClient is the minimal TokenReview client surface needed for authentication. +type K8sTokenReviewClient interface { + Create(ctx context.Context, review *authenticationv1.TokenReview, opts metav1.CreateOptions) (*authenticationv1.TokenReview, error) } // K8sCoreClient is the minimal CoreV1 client surface needed by the backend. type K8sCoreClient interface { Pods(namespace string) K8sPodClient ConfigMaps(namespace string) K8sConfigMapClient + Services(namespace string) K8sServiceClient + Namespaces() K8sNamespaceClient + ServiceAccounts(namespace string) K8sServiceAccountClient + TokenReviews() K8sTokenReviewClient } // K8sPodClient is the minimal Pod client surface needed by the backend. @@ -81,7 +111,8 @@ type K8sConfigMapClient interface { // k8sCoreClientWrapper adapts *corev1client.CoreV1Client to K8sCoreClient. type k8sCoreClientWrapper struct { - client *corev1client.CoreV1Client + client *corev1client.CoreV1Client + authClient *authenticationv1client.AuthenticationV1Client } func (w *k8sCoreClientWrapper) Pods(namespace string) K8sPodClient { @@ -92,11 +123,36 @@ func (w *k8sCoreClientWrapper) ConfigMaps(namespace string) K8sConfigMapClient { return w.client.ConfigMaps(namespace) } +func (w *k8sCoreClientWrapper) Services(namespace string) K8sServiceClient { + return w.client.Services(namespace) +} + +func (w *k8sCoreClientWrapper) Namespaces() K8sNamespaceClient { + return w.client.Namespaces() +} + +func (w *k8sCoreClientWrapper) ServiceAccounts(namespace string) K8sServiceAccountClient { + return w.client.ServiceAccounts(namespace) +} + +func (w *k8sCoreClientWrapper) TokenReviews() K8sTokenReviewClient { + return w.authClient.TokenReviews() +} + // NewK8sBackend creates a Kubernetes backend using in-cluster config or kubeconfig. // scheme is used by Create to stamp CR-to-Pod controller OwnerReferences // (see CreateRequest.Owner); it must have all CR kinds that might appear as // Owner registered. func NewK8sBackend(config K8sConfig, containerPrefix string, scheme *runtime.Scheme) (*K8sBackend, error) { + return NewK8sBackendWithCache(config, containerPrefix, scheme, nil) +} + +// NewK8sBackendWithCache creates a Kubernetes backend using in-cluster config +// or kubeconfig, and wires in an optional remote-cluster client cache used +// to route operations against Workers/Managers deployed to remote clusters. +// remoteCache may be nil when no remote clusters are configured; in that +// case the backend behaves identically to NewK8sBackend. +func NewK8sBackendWithCache(config K8sConfig, containerPrefix string, scheme *runtime.Scheme, remoteCache RemoteClientProvider) (*K8sBackend, error) { restConfig, err := loadK8sRESTConfig() if err != nil { return nil, err @@ -105,7 +161,11 @@ func NewK8sBackend(config K8sConfig, containerPrefix string, scheme *runtime.Sch if err != nil { return nil, fmt.Errorf("create kubernetes client: %w", err) } - return NewK8sBackendWithClient(&k8sCoreClientWrapper{client: clientset}, config, containerPrefix, scheme), nil + authClient, err := authenticationv1client.NewForConfig(restConfig) + if err != nil { + return nil, fmt.Errorf("create authentication client: %w", err) + } + return NewK8sBackendWithRemote(&k8sCoreClientWrapper{client: clientset, authClient: authClient}, remoteCache, config, containerPrefix, scheme), nil } // NewK8sBackendWithClient creates a Kubernetes backend with a custom client. @@ -125,9 +185,18 @@ func NewK8sBackendWithClient(client K8sCoreClient, config K8sConfig, containerPr config: config, containerPrefix: containerPrefix, scheme: scheme, + namespace: config.Namespace, } } +// NewK8sBackendWithRemote creates a Kubernetes backend with remote cluster support. +// remoteCache may be nil if no remote clusters are configured. +func NewK8sBackendWithRemote(client K8sCoreClient, remoteCache RemoteClientProvider, config K8sConfig, containerPrefix string, scheme *runtime.Scheme) *K8sBackend { + b := NewK8sBackendWithClient(client, config, containerPrefix, scheme) + b.remoteCache = remoteCache + return b +} + // WithPrefix returns a shallow copy of the backend with a different container name prefix. // The returned backend shares the same client (safe — K8sCoreClient is stateless). // Use WithPrefix("") to disable prefix for containers that already have full names @@ -138,6 +207,67 @@ func (k *K8sBackend) WithPrefix(prefix string) *K8sBackend { return &cp } +// WithRemoteTarget returns a shallow copy of the backend configured to route +// Delete and Status operations to the specified remote cluster/namespace. +// Use this when the controller needs to operate on a pod in a remote cluster +// via the interface methods that only accept a name. +func (k *K8sBackend) WithRemoteTarget(deployMode, clusterID, namespace string) *K8sBackend { + cp := *k + cp.deployMode = deployMode + cp.targetClusterID = clusterID + cp.targetNamespace = namespace + return &cp +} + +// resolveClient returns the appropriate K8sCoreClient and namespace based on +// deploy mode. For Remote mode, it fetches the client from remoteCache. +func (k *K8sBackend) resolveClient(ctx context.Context, deployMode, targetClusterID, targetNamespace string) (K8sCoreClient, string, error) { + if deployMode == v1beta1.DeployModeRemote { + if k.remoteCache == nil { + return nil, "", fmt.Errorf("remote client cache not configured") + } + client, err := k.remoteCache.ResolveClient(ctx, targetClusterID) + if err != nil { + return nil, "", fmt.Errorf("failed to get remote client for cluster %s: %w", targetClusterID, err) + } + ns := targetNamespace + if ns == "" { + ns = k.namespace + } + return client, ns, nil + } + return k.client, k.namespace, nil +} + +// ServiceClient implements ServiceBackend. It returns a K8sServiceClient and +// resolved namespace for the appropriate cluster based on deploy mode. +func (k *K8sBackend) ServiceClient(ctx context.Context, deployMode, targetClusterID, targetNamespace string) (K8sServiceClient, string, error) { + client, ns, err := k.resolveClient(ctx, deployMode, targetClusterID, targetNamespace) + if err != nil { + return nil, "", err + } + return client.Services(ns), ns, nil +} + +// ensureRemoteNamespace ensures the target namespace exists in the remote cluster. +func (k *K8sBackend) ensureRemoteNamespace(ctx context.Context, client K8sCoreClient, namespace string) error { + _, err := client.Namespaces().Get(ctx, namespace, metav1.GetOptions{}) + if err == nil { + return nil + } + if !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to check namespace %s: %w", namespace, err) + } + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: namespace}, + } + _, err = client.Namespaces().Create(ctx, ns, metav1.CreateOptions{}) + if err != nil && !apierrors.IsAlreadyExists(err) { + return fmt.Errorf("failed to create namespace %s: %w", namespace, err) + } + return nil +} + func (k *K8sBackend) Name() string { return "k8s" } func (k *K8sBackend) DeploymentMode() string { return DeployCloud } func (k *K8sBackend) NeedsCredentialInjection() bool { return true } @@ -155,11 +285,24 @@ func (k *K8sBackend) Create(ctx context.Context, req CreateRequest) (*WorkerResu // HICLAW_DEFAULT_WORKER_RUNTIME for workers). req.Runtime = ResolveRuntime(req.Runtime, req.RuntimeFallback) + // Resolve the target client and namespace based on deploy mode. + targetClient, targetNS, err := k.resolveClient(ctx, req.DeployMode, req.TargetClusterID, req.TargetNamespace) + if err != nil { + return nil, fmt.Errorf("resolve client for create: %w", err) + } + + // Ensure the remote namespace exists before creating the Pod. + if req.DeployMode == v1beta1.DeployModeRemote { + if err := k.ensureRemoteNamespace(ctx, targetClient, targetNS); err != nil { + return nil, fmt.Errorf("ensure remote namespace: %w", err) + } + } + podName := req.ContainerName if podName == "" { podName = k.podName(req.NamePrefix, req.Name) } - if _, err := k.client.Pods(k.config.Namespace).Get(ctx, podName, metav1.GetOptions{}); err == nil { + if _, err := targetClient.Pods(targetNS).Get(ctx, podName, metav1.GetOptions{}); err == nil { return nil, fmt.Errorf("%w: pod %q", ErrConflict, podName) } else if !apierrors.IsNotFound(err) { return nil, fmt.Errorf("kubernetes get pod %s: %w", podName, err) @@ -218,6 +361,12 @@ func (k *K8sBackend) Create(ctx context.Context, req CreateRequest) (*WorkerResu } } + // Inject cluster ID so the worker CLI sends X-HiClaw-Cluster-ID header + // for remote TokenReview routing. + if req.DeployMode == v1beta1.DeployModeRemote && req.TargetClusterID != "" { + req.Env["HICLAW_CLUSTER_ID"] = req.TargetClusterID + } + image := req.Image if image == "" { switch { @@ -314,11 +463,11 @@ func (k *K8sBackend) Create(ctx context.Context, req CreateRequest) (*WorkerResu podLabels[k] = v } - tmpl := LoadAgentPodTemplate(ctx, k.client, k.config.Namespace, k.config.ControllerName) + tmpl := LoadAgentPodTemplate(ctx, k.client, k.config.Namespace, k.config.ControllerName, req.DeployMode) pod := ApplyPodTemplate(tmpl, PodOverlay{ Name: podName, - Namespace: k.config.Namespace, + Namespace: targetNS, Labels: podLabels, Annotations: nil, ServiceAccountName: saName, @@ -330,7 +479,9 @@ func (k *K8sBackend) Create(ctx context.Context, req CreateRequest) (*WorkerResu HostAliases: buildHostAliases(req.ExtraHosts), }) - if req.Owner != nil { + // Skip controller OwnerReference for remote pods — cross-cluster ownerRef + // is not possible. Local pods still get ownerRef for GC cascading. + if req.Owner != nil && req.DeployMode != v1beta1.DeployModeRemote { if k.scheme == nil { return nil, fmt.Errorf("kubernetes backend: scheme is required when CreateRequest.Owner is set") } @@ -339,7 +490,7 @@ func (k *K8sBackend) Create(ctx context.Context, req CreateRequest) (*WorkerResu } } - created, err := k.client.Pods(k.config.Namespace).Create(ctx, pod, metav1.CreateOptions{}) + created, err := targetClient.Pods(targetNS).Create(ctx, pod, metav1.CreateOptions{}) if err != nil { if apierrors.IsAlreadyExists(err) { return nil, fmt.Errorf("%w: pod %q", ErrConflict, podName) @@ -356,8 +507,12 @@ func (k *K8sBackend) Create(ctx context.Context, req CreateRequest) (*WorkerResu } func (k *K8sBackend) Delete(ctx context.Context, name string) error { + targetClient, targetNS, err := k.resolveClient(ctx, k.deployMode, k.targetClusterID, k.targetNamespace) + if err != nil { + return fmt.Errorf("resolve client for delete: %w", err) + } podName := k.workerPodName(name) - err := k.client.Pods(k.config.Namespace).Delete(ctx, podName, metav1.DeleteOptions{}) + err = targetClient.Pods(targetNS).Delete(ctx, podName, metav1.DeleteOptions{}) if apierrors.IsNotFound(err) { return nil } @@ -368,7 +523,11 @@ func (k *K8sBackend) Delete(ctx context.Context, name string) error { } func (k *K8sBackend) Start(ctx context.Context, name string) error { - pod, err := k.client.Pods(k.config.Namespace).Get(ctx, k.workerPodName(name), metav1.GetOptions{}) + targetClient, targetNS, err := k.resolveClient(ctx, k.deployMode, k.targetClusterID, k.targetNamespace) + if err != nil { + return fmt.Errorf("resolve client for start: %w", err) + } + pod, err := targetClient.Pods(targetNS).Get(ctx, k.workerPodName(name), metav1.GetOptions{}) if apierrors.IsNotFound(err) { return fmt.Errorf("%w: worker %q", ErrNotFound, name) } @@ -389,7 +548,11 @@ func (k *K8sBackend) Stop(ctx context.Context, name string) error { } func (k *K8sBackend) Status(ctx context.Context, name string) (*WorkerResult, error) { - pod, err := k.client.Pods(k.config.Namespace).Get(ctx, k.workerPodName(name), metav1.GetOptions{}) + targetClient, targetNS, err := k.resolveClient(ctx, k.deployMode, k.targetClusterID, k.targetNamespace) + if err != nil { + return nil, fmt.Errorf("resolve client for status: %w", err) + } + pod, err := targetClient.Pods(targetNS).Get(ctx, k.workerPodName(name), metav1.GetOptions{}) if apierrors.IsNotFound(err) { return &WorkerResult{Name: name, Backend: "k8s", Status: StatusNotFound}, nil } diff --git a/hiclaw-controller/internal/backend/kubernetes_test.go b/hiclaw-controller/internal/backend/kubernetes_test.go index b996f704b..357bf9001 100644 --- a/hiclaw-controller/internal/backend/kubernetes_test.go +++ b/hiclaw-controller/internal/backend/kubernetes_test.go @@ -81,6 +81,48 @@ func (f *fakeK8sCoreClient) ConfigMaps(namespace string) K8sConfigMapClient { } } +func (f *fakeK8sCoreClient) Services(_ string) K8sServiceClient { + return &fakeK8sServiceClient{} +} + +func (f *fakeK8sCoreClient) Namespaces() K8sNamespaceClient { + return &fakeK8sNamespaceClient{} +} + +func (f *fakeK8sCoreClient) ServiceAccounts(_ string) K8sServiceAccountClient { + panic("not implemented") +} + +func (f *fakeK8sCoreClient) TokenReviews() K8sTokenReviewClient { + panic("not implemented") +} + +// fakeK8sServiceClient is a no-op stub for tests that don't exercise Services. +type fakeK8sServiceClient struct{} + +func (f *fakeK8sServiceClient) Get(_ context.Context, _ string, _ metav1.GetOptions) (*corev1.Service, error) { + return nil, apierrors.NewNotFound(schema.GroupResource{Resource: "services"}, "") +} +func (f *fakeK8sServiceClient) Create(_ context.Context, svc *corev1.Service, _ metav1.CreateOptions) (*corev1.Service, error) { + return svc, nil +} +func (f *fakeK8sServiceClient) Update(_ context.Context, svc *corev1.Service, _ metav1.UpdateOptions) (*corev1.Service, error) { + return svc, nil +} +func (f *fakeK8sServiceClient) Delete(_ context.Context, _ string, _ metav1.DeleteOptions) error { + return nil +} + +// fakeK8sNamespaceClient is a no-op stub for tests that don't exercise Namespaces. +type fakeK8sNamespaceClient struct{} + +func (f *fakeK8sNamespaceClient) Get(_ context.Context, _ string, _ metav1.GetOptions) (*corev1.Namespace, error) { + return nil, apierrors.NewNotFound(schema.GroupResource{Resource: "namespaces"}, "") +} +func (f *fakeK8sNamespaceClient) Create(_ context.Context, ns *corev1.Namespace, _ metav1.CreateOptions) (*corev1.Namespace, error) { + return ns, nil +} + type fakeK8sConfigMapClient struct { namespace string store map[string]*corev1.ConfigMap @@ -907,3 +949,280 @@ func TestK8sCreate_DefaultSAFallback(t *testing.T) { t.Fatalf("SA = %q, want acme-worker-bob", pod.Spec.ServiceAccountName) } } + +// ── Cross-cluster routing tests ────────────────────────────────────────── + +// statefulNamespaceClient backs ensureRemoteNamespace tests; it tracks both +// stored namespaces and a configurable Get error to exercise the IsNotFound / +// IsAlreadyExists branches in the backend. +type statefulNamespaceClient struct { + store map[string]*corev1.Namespace + createErr error + getCalls int + createCalls int +} + +func (s *statefulNamespaceClient) Get(_ context.Context, name string, _ metav1.GetOptions) (*corev1.Namespace, error) { + s.getCalls++ + if ns, ok := s.store[name]; ok { + return ns.DeepCopy(), nil + } + return nil, apierrors.NewNotFound(schema.GroupResource{Resource: "namespaces"}, name) +} + +func (s *statefulNamespaceClient) Create(_ context.Context, ns *corev1.Namespace, _ metav1.CreateOptions) (*corev1.Namespace, error) { + s.createCalls++ + if s.createErr != nil { + return nil, s.createErr + } + if _, exists := s.store[ns.Name]; exists { + return nil, apierrors.NewAlreadyExists(schema.GroupResource{Resource: "namespaces"}, ns.Name) + } + cp := ns.DeepCopy() + s.store[cp.Name] = cp + return cp.DeepCopy(), nil +} + +// remoteFakeClient wraps fakeK8sCoreClient with a stateful Namespace client so +// ensureRemoteNamespace can be exercised end-to-end. +type remoteFakeClient struct { + *fakeK8sCoreClient + nsClient *statefulNamespaceClient +} + +func (r *remoteFakeClient) Namespaces() K8sNamespaceClient { return r.nsClient } + +func newRemoteFakeClient() *remoteFakeClient { + return &remoteFakeClient{ + fakeK8sCoreClient: newFakeK8sCoreClient(), + nsClient: &statefulNamespaceClient{ + store: map[string]*corev1.Namespace{}, + }, + } +} + +// fakeRemoteCache implements RemoteClientProvider by serving a single +// pre-built K8sCoreClient regardless of clusterID. Tests can also wire in +// a configurable error to exercise resolveClient failure paths. +type fakeRemoteCache struct { + client K8sCoreClient + resolveEr error + calls int + lastID string +} + +func (f *fakeRemoteCache) ResolveClient(_ context.Context, clusterID string) (K8sCoreClient, error) { + f.calls++ + f.lastID = clusterID + if f.resolveEr != nil { + return nil, f.resolveEr + } + return f.client, nil +} + +func TestK8sResolveClient_Local(t *testing.T) { + b := newTestK8sBackend() + + cases := []string{"", v1beta1.DeployModeLocal} + for _, mode := range cases { + t.Run("mode="+mode, func(t *testing.T) { + cli, ns, err := b.resolveClient(context.Background(), mode, "", "") + if err != nil { + t.Fatalf("resolveClient(%q): %v", mode, err) + } + if cli != b.client { + t.Errorf("expected local client to be returned for mode %q", mode) + } + if ns != b.namespace { + t.Errorf("namespace = %q, want %q", ns, b.namespace) + } + }) + } +} + +func TestK8sResolveClient_Remote(t *testing.T) { + remote := newRemoteFakeClient() + cache := &fakeRemoteCache{client: remote} + + b, _ := newTestK8sBackendWithFake(K8sConfig{}) + b.remoteCache = cache + + cli, ns, err := b.resolveClient(context.Background(), v1beta1.DeployModeRemote, "c-1", "team-a") + if err != nil { + t.Fatalf("resolveClient: %v", err) + } + if cli != remote { + t.Error("expected remote client from cache") + } + if ns != "team-a" { + t.Errorf("namespace = %q, want team-a", ns) + } + if cache.calls != 1 || cache.lastID != "c-1" { + t.Errorf("cache.ResolveClient invocation = (%d, %q), want (1, c-1)", cache.calls, cache.lastID) + } +} + +// TestK8sResolveClient_RemoteEmptyNamespaceFallsBack covers the helper's +// fallback from an empty TargetNamespace to the backend's local namespace. +func TestK8sResolveClient_RemoteEmptyNamespaceFallsBack(t *testing.T) { + remote := newRemoteFakeClient() + cache := &fakeRemoteCache{client: remote} + + b, _ := newTestK8sBackendWithFake(K8sConfig{}) + b.remoteCache = cache + + _, ns, err := b.resolveClient(context.Background(), v1beta1.DeployModeRemote, "c-1", "") + if err != nil { + t.Fatalf("resolveClient: %v", err) + } + if ns != b.namespace { + t.Errorf("namespace = %q, want backend default %q", ns, b.namespace) + } +} + +func TestK8sResolveClient_RemoteWithoutCache(t *testing.T) { + b, _ := newTestK8sBackendWithFake(K8sConfig{}) + // remoteCache intentionally left nil. + + if _, _, err := b.resolveClient(context.Background(), v1beta1.DeployModeRemote, "c-1", "team-a"); err == nil { + t.Fatal("expected error when DeployMode=Remote but remoteCache is nil") + } +} + +func TestK8sResolveClient_RemoteCacheError(t *testing.T) { + cache := &fakeRemoteCache{resolveEr: apierrors.NewServiceUnavailable("sts down")} + b, _ := newTestK8sBackendWithFake(K8sConfig{}) + b.remoteCache = cache + + if _, _, err := b.resolveClient(context.Background(), v1beta1.DeployModeRemote, "c-1", "team-a"); err == nil { + t.Fatal("expected error when remote cache returns error") + } +} + +func TestK8sEnsureRemoteNamespace_CreatesWhenAbsent(t *testing.T) { + b := newTestK8sBackend() + remote := newRemoteFakeClient() + + if err := b.ensureRemoteNamespace(context.Background(), remote, "team-a"); err != nil { + t.Fatalf("ensureRemoteNamespace: %v", err) + } + if remote.nsClient.createCalls != 1 { + t.Errorf("Namespaces().Create calls = %d, want 1", remote.nsClient.createCalls) + } + if _, ok := remote.nsClient.store["team-a"]; !ok { + t.Error("namespace must be stored after creation") + } +} + +func TestK8sEnsureRemoteNamespace_IdempotentExisting(t *testing.T) { + b := newTestK8sBackend() + remote := newRemoteFakeClient() + remote.nsClient.store["team-a"] = &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: "team-a"}, + } + + if err := b.ensureRemoteNamespace(context.Background(), remote, "team-a"); err != nil { + t.Fatalf("ensureRemoteNamespace (existing): %v", err) + } + if remote.nsClient.createCalls != 0 { + t.Errorf("Create must not be called when namespace already exists; calls=%d", remote.nsClient.createCalls) + } +} + +func TestK8sEnsureRemoteNamespace_AlreadyExistsRace(t *testing.T) { + b := newTestK8sBackend() + remote := newRemoteFakeClient() + // Simulate a race where Get returns NotFound but the subsequent Create + // loses to a concurrent creator. + remote.nsClient.createErr = apierrors.NewAlreadyExists(schema.GroupResource{Resource: "namespaces"}, "team-a") + + if err := b.ensureRemoteNamespace(context.Background(), remote, "team-a"); err != nil { + t.Fatalf("AlreadyExists must be tolerated, got %v", err) + } +} + +// TestK8sCreate_RemoteSkipsOwnerReference verifies that Create() skips +// stamping a controller OwnerReference when the request targets a remote +// cluster — cross-cluster ownerRefs are not possible. +func TestK8sCreate_RemoteSkipsOwnerReference(t *testing.T) { + scheme := runtime.NewScheme() + if err := v1beta1.AddToScheme(scheme); err != nil { + t.Fatalf("AddToScheme: %v", err) + } + + remote := newRemoteFakeClient() + cache := &fakeRemoteCache{client: remote} + + local := newFakeK8sCoreClient() + b := NewK8sBackendWithRemote(local, cache, K8sConfig{ + Namespace: "hiclaw", + WorkerImage: "hiclaw/worker-agent:latest", + }, "hiclaw-worker-", scheme) + + owner := &v1beta1.Worker{ + ObjectMeta: metav1.ObjectMeta{Name: "alice", Namespace: "hiclaw", UID: "u-1"}, + } + + if _, err := b.Create(context.Background(), CreateRequest{ + Name: "alice", + Owner: owner, + DeployMode: v1beta1.DeployModeRemote, + TargetClusterID: "c-1", + TargetNamespace: "team-a", + }); err != nil { + t.Fatalf("Create: %v", err) + } + + // Pod must land in the REMOTE client at the target namespace, not the local one. + pod, err := remote.Pods("team-a").Get(context.Background(), "hiclaw-worker-alice", metav1.GetOptions{}) + if err != nil { + t.Fatalf("remote pod lookup: %v", err) + } + if len(pod.OwnerReferences) != 0 { + t.Errorf("remote pod must have NO ownerReferences (cross-cluster ownerRef impossible), got %+v", pod.OwnerReferences) + } + + // Sanity: target namespace must have been ensured. + if _, ok := remote.nsClient.store["team-a"]; !ok { + t.Error("ensureRemoteNamespace must run before Create in Remote mode") + } + + // Local backend must NOT have received the pod. + if _, err := local.Pods("hiclaw").Get(context.Background(), "hiclaw-worker-alice", metav1.GetOptions{}); !apierrors.IsNotFound(err) { + t.Errorf("local backend must not store the remote pod, got err=%v", err) + } +} + +// TestK8sCreate_LocalKeepsOwnerReference is the complementary assertion: +// when DeployMode is empty (Local), Create still stamps the controller +// OwnerReference. This guards against a refactor that accidentally broadens +// the "skip ownerRef" branch beyond Remote mode. +func TestK8sCreate_LocalKeepsOwnerReference(t *testing.T) { + scheme := runtime.NewScheme() + if err := v1beta1.AddToScheme(scheme); err != nil { + t.Fatalf("AddToScheme: %v", err) + } + + client := newFakeK8sCoreClient() + b := NewK8sBackendWithClient(client, K8sConfig{ + Namespace: "hiclaw", + WorkerImage: "hiclaw/worker-agent:latest", + }, "hiclaw-worker-", scheme) + + owner := &v1beta1.Worker{ + ObjectMeta: metav1.ObjectMeta{Name: "bob", Namespace: "hiclaw", UID: "u-2"}, + } + if _, err := b.Create(context.Background(), CreateRequest{ + Name: "bob", + Owner: owner, + }); err != nil { + t.Fatalf("Create: %v", err) + } + pod, err := client.Pods("hiclaw").Get(context.Background(), "hiclaw-worker-bob", metav1.GetOptions{}) + if err != nil { + t.Fatalf("Get: %v", err) + } + if len(pod.OwnerReferences) != 1 { + t.Fatalf("expected 1 ownerReference for Local pod, got %+v", pod.OwnerReferences) + } +} diff --git a/hiclaw-controller/internal/controller/member_phase_test.go b/hiclaw-controller/internal/controller/member_phase_test.go new file mode 100644 index 000000000..a22f854e9 --- /dev/null +++ b/hiclaw-controller/internal/controller/member_phase_test.go @@ -0,0 +1,129 @@ +package controller + +import ( + "errors" + "testing" + + "github.com/hiclaw/hiclaw-controller/internal/backend" +) + +func TestComputeMemberPhase(t *testing.T) { + tests := []struct { + name string + currentPhase string + matrixUserID string + desiredState string + containerState string + reconcileErr error + want string + }{ + // Running path + { + name: "Running + running = Running", + desiredState: "Running", + containerState: string(backend.StatusRunning), + want: "Running", + }, + { + name: "Running + ready = Running", + desiredState: "Running", + containerState: string(backend.StatusReady), + want: "Running", + }, + { + name: "Running + starting = Starting", + desiredState: "Running", + containerState: string(backend.StatusStarting), + want: "Starting", + }, + { + name: "Running + not_found = Pending", + desiredState: "Running", + containerState: string(backend.StatusNotFound), + want: "Pending", + }, + { + name: "Running + stopped = Pending", + desiredState: "Running", + containerState: string(backend.StatusStopped), + want: "Pending", + }, + { + name: "Running + sleeping = Pending", + desiredState: "Running", + containerState: string(backend.StatusSleeping), + want: "Pending", + }, + { + name: "Running + unknown = Pending", + desiredState: "Running", + containerState: string(backend.StatusUnknown), + want: "Pending", + }, + // Sleeping path + { + name: "Sleeping + stopping = Stopping", + desiredState: "Sleeping", + containerState: "stopping", + want: "Stopping", + }, + { + name: "Sleeping + sleeping = Sleeping", + desiredState: "Sleeping", + containerState: string(backend.StatusSleeping), + want: "Sleeping", + }, + { + name: "Sleeping + not_found = Sleeping", + desiredState: "Sleeping", + containerState: string(backend.StatusNotFound), + want: "Sleeping", + }, + // Stopped path + { + name: "Stopped + stopping = Stopping", + desiredState: "Stopped", + containerState: "stopping", + want: "Stopping", + }, + { + name: "Stopped + not_found = Stopped", + desiredState: "Stopped", + containerState: string(backend.StatusNotFound), + want: "Stopped", + }, + // Error path + { + name: "Error + no matrixUserID = Failed", + matrixUserID: "", + desiredState: "Running", + reconcileErr: errors.New("provision failed"), + want: "Failed", + }, + { + name: "Error + matrixUserID + no phase = Pending", + matrixUserID: "@bot:example.com", + currentPhase: "", + desiredState: "Running", + reconcileErr: errors.New("transient"), + want: "Pending", + }, + { + name: "Error + matrixUserID + existing phase = preserve", + matrixUserID: "@bot:example.com", + currentPhase: "Running", + desiredState: "Running", + reconcileErr: errors.New("transient"), + want: "Running", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := computeMemberPhase(tt.currentPhase, tt.matrixUserID, tt.desiredState, tt.containerState, tt.reconcileErr) + if got != tt.want { + t.Errorf("computeMemberPhase() = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/hiclaw-controller/internal/controller/member_reconcile.go b/hiclaw-controller/internal/controller/member_reconcile.go index 9af711ca8..678acc76d 100644 --- a/hiclaw-controller/internal/controller/member_reconcile.go +++ b/hiclaw-controller/internal/controller/member_reconcile.go @@ -104,6 +104,19 @@ type MemberContext struct { // ModelProviderInfo is the resolved APIG Model API info when // spec.modelProvider is set. Nil when not set or on non-ai-gateway. ModelProviderInfo *gateway.ModelProviderInfo + + // DeployMode specifies where the member pod runs: "Local" (default) or + // "Remote". Sourced from spec.deployMode with a default of "Local". + DeployMode string + // TargetClusterID is the remote cluster ID when DeployMode is "Remote". + // Sourced from spec.targetCluster.id. + TargetClusterID string + // TargetNamespace is the namespace in the remote cluster when DeployMode + // is "Remote". Sourced from spec.targetCluster.namespace. + TargetNamespace string + // ServiceEnabled controls whether a ClusterIP Service is created + // alongside the member pod. Sourced from spec.serviceEnabled. + ServiceEnabled bool } // MemberState captures reconcile outputs that the caller writes back to the @@ -118,6 +131,18 @@ type MemberState struct { ProvResult *service.WorkerProvisionResult } +// resolveBackendForMember returns a backend configured for the member's +// deploy target. For remote members using the K8s backend, it applies +// WithRemoteTarget so that Status/Delete/Stop route to the correct cluster. +func resolveBackendForMember(wb backend.WorkerBackend, m MemberContext) backend.WorkerBackend { + if m.DeployMode == v1beta1.DeployModeRemote && m.TargetClusterID != "" { + if k8sBackend, ok := wb.(*backend.K8sBackend); ok { + return k8sBackend.WithRemoteTarget(m.DeployMode, m.TargetClusterID, m.TargetNamespace) + } + } + return wb +} + // MemberDeps aggregates the service-layer dependencies the member phases // invoke. Both WorkerReconciler and TeamReconciler build a MemberDeps once // and pass it through each phase. @@ -149,6 +174,24 @@ type MemberDeps struct { DefaultRuntime string } +// ValidateMemberDeployment checks the cross-cluster deployment fields on a +// MemberContext for consistency. Returns a non-nil error when: +// - DeployMode is not "Local" or "Remote" +// - DeployMode is "Remote" but TargetClusterID or TargetNamespace is empty +func ValidateMemberDeployment(m MemberContext) error { + switch m.DeployMode { + case v1beta1.DeployModeLocal, v1beta1.DeployModeRemote: + default: + return fmt.Errorf("invalid deployMode %q: must be \"Local\" or \"Remote\"", m.DeployMode) + } + if m.DeployMode == v1beta1.DeployModeRemote { + if m.TargetClusterID == "" || m.TargetNamespace == "" { + return fmt.Errorf("deployMode \"Remote\" requires targetCluster.id and targetCluster.namespace") + } + } + return nil +} + // ReconcileMemberInfra ensures Matrix account, Gateway consumer, MinIO user, // and DM room are provisioned (or credentials refreshed). Writes MatrixUserID, // RoomID, and ProvResult into state. @@ -279,9 +322,9 @@ func ReconcileMemberContainer(ctx context.Context, d MemberDeps, m MemberContext desired := m.Spec.DesiredState() switch desired { case "Stopped": - return ensureMemberContainerAbsent(ctx, d, m, true) + return ensureMemberContainerAbsent(ctx, d, m, true, state) case "Sleeping": - return ensureMemberContainerAbsent(ctx, d, m, false) + return ensureMemberContainerAbsent(ctx, d, m, false, state) default: return ensureMemberContainerPresent(ctx, d, m, state) } @@ -296,6 +339,7 @@ func ensureMemberContainerPresent(ctx context.Context, d MemberDeps, m MemberCon log.FromContext(ctx).Info("no worker backend available, member needs manual start", "name", m.Name) return reconcile.Result{}, nil } + wb = resolveBackendForMember(wb, m) logger := log.FromContext(ctx) result, err := wb.Status(ctx, m.Name) @@ -321,9 +365,13 @@ func ensureMemberContainerPresent(ctx context.Context, d MemberDeps, m MemberCon if err := wb.Delete(ctx, m.Name); err != nil && !errors.Is(err, backend.ErrNotFound) { return reconcile.Result{}, fmt.Errorf("delete container for recreate: %w", err) } - return createMemberContainer(ctx, d, m, state, wb) + res, err := createMemberContainer(ctx, d, m, state, wb) + if err == nil { + state.ContainerState = string(backend.StatusStarting) + } + return res, err - case backend.StatusStopped: + case backend.StatusStopped, backend.StatusSleeping: state.ContainerState = string(result.Status) if wb.Name() == "docker" && !specChanged { if err := wb.Start(ctx, m.Name); err != nil { @@ -334,10 +382,18 @@ func ensureMemberContainerPresent(ctx context.Context, d MemberDeps, m MemberCon if err := wb.Delete(ctx, m.Name); err != nil && !errors.Is(err, backend.ErrNotFound) { return reconcile.Result{}, fmt.Errorf("delete stale container: %w", err) } - return createMemberContainer(ctx, d, m, state, wb) + res, err := createMemberContainer(ctx, d, m, state, wb) + if err == nil { + state.ContainerState = string(backend.StatusStarting) + } + return res, err case backend.StatusNotFound: - return createMemberContainer(ctx, d, m, state, wb) + res, err := createMemberContainer(ctx, d, m, state, wb) + if err == nil { + state.ContainerState = string(backend.StatusStarting) + } + return res, err default: state.ContainerState = string(result.Status) @@ -349,7 +405,7 @@ func ensureMemberContainerPresent(ctx context.Context, d MemberDeps, m MemberCon } } -func ensureMemberContainerAbsent(ctx context.Context, d MemberDeps, m MemberContext, remove bool) (reconcile.Result, error) { +func ensureMemberContainerAbsent(ctx context.Context, d MemberDeps, m MemberContext, remove bool, state *MemberState) (reconcile.Result, error) { if d.Backend == nil { return reconcile.Result{}, nil } @@ -357,14 +413,34 @@ func ensureMemberContainerAbsent(ctx context.Context, d MemberDeps, m MemberCont if wb == nil { return reconcile.Result{}, nil } + wb = resolveBackendForMember(wb, m) + + result, err := wb.Status(ctx, m.Name) + if err != nil { + return reconcile.Result{}, fmt.Errorf("query status for absent: %w", err) + } + state.ContainerState = string(result.Status) + + // Already gone + if result.Status == backend.StatusNotFound { + return reconcile.Result{}, nil + } + if remove { + // Stopped: need full deletion regardless of current state if err := wb.Delete(ctx, m.Name); err != nil && !errors.Is(err, backend.ErrNotFound) { return reconcile.Result{}, fmt.Errorf("delete container: %w", err) } + state.ContainerState = "stopping" } else { + // Sleeping: skip if already sleeping/stopped + if result.Status == backend.StatusStopped || result.Status == backend.StatusSleeping { + return reconcile.Result{}, nil + } if err := wb.Stop(ctx, m.Name); err != nil && !errors.Is(err, backend.ErrNotFound) { return reconcile.Result{}, fmt.Errorf("stop container: %w", err) } + state.ContainerState = "stopping" } return reconcile.Result{}, nil } @@ -372,6 +448,15 @@ func ensureMemberContainerAbsent(ctx context.Context, d MemberDeps, m MemberCont func createMemberContainer(ctx context.Context, d MemberDeps, m MemberContext, state *MemberState, wb backend.WorkerBackend) (reconcile.Result, error) { logger := log.FromContext(ctx) + // Ensure remote ServiceAccount exists before creating the Pod in a + // remote cluster. The SA provides projected-token authentication back + // to the local controller via TokenReview routing. + if m.DeployMode == v1beta1.DeployModeRemote && m.TargetClusterID != "" { + if err := d.Provisioner.EnsureRemoteServiceAccount(ctx, m.Name, m.TargetClusterID, m.TargetNamespace); err != nil { + return reconcile.Result{}, fmt.Errorf("ensure remote SA for worker %s: %w", m.Name, err) + } + } + prov := state.ProvResult if prov == nil || prov.MatrixToken == "" { refreshResult, err := d.Provisioner.RefreshWorkerCredentials(ctx, m.Name, m.RuntimeName) @@ -417,6 +502,12 @@ func createMemberContainer(ctx context.Context, d MemberDeps, m MemberContext, s Resources: agentResourcesToBackend(m.Spec.Resources), Labels: labels, Owner: m.Owner, + // DeployMode / TargetClusterID / TargetNamespace route the Pod to + // the correct cluster. Without them, Remote workers would always + // be created in the local cluster. + DeployMode: m.DeployMode, + TargetClusterID: m.TargetClusterID, + TargetNamespace: m.TargetNamespace, } if wb.Name() != "k8s" { token, err := d.Provisioner.RequestSAToken(ctx, m.Name) @@ -442,6 +533,41 @@ func extraHostsForBackend(wb backend.WorkerBackend) []string { return nil } +// computeMemberPhase derives the lifecycle phase from desired state, +// observed container status, and reconcile outcome. Single source of truth +// for both Worker and Team member paths. +func computeMemberPhase(currentPhase, matrixUserID, desiredState, containerState string, reconcileErr error) string { + if reconcileErr != nil { + if matrixUserID == "" { + return "Failed" + } + if currentPhase == "" { + return "Pending" + } + return currentPhase + } + switch desiredState { + case "Sleeping": + if containerState == "stopping" { + return "Stopping" + } + return "Sleeping" + case "Stopped": + if containerState == "stopping" { + return "Stopping" + } + return "Stopped" + default: // Running + if containerState == string(backend.StatusRunning) || containerState == string(backend.StatusReady) { + return "Running" + } + if containerState == string(backend.StatusStarting) { + return "Starting" + } + return "Pending" + } +} + // ReconcileMemberExpose reconciles Higress port exposure for the member. // Non-fatal: logs and returns current state unchanged on failure. The returned // slice overwrites state.ExposedPorts on success. @@ -502,11 +628,15 @@ func ReconcileMemberDelete(ctx context.Context, d MemberDeps, m MemberContext) e // about, so this is the only reliable cleanup path. if d.Backend != nil { if wb := d.Backend.DetectWorkerBackend(ctx); wb != nil { + wb = resolveBackendForMember(wb, m) if err := wb.Delete(ctx, m.Name); err != nil && !errors.Is(err, backend.ErrNotFound) { logger.Error(err, "failed to delete member container (may already be removed)", "name", m.Name) } } } + if err := ensureServiceDeleted(ctx, &m, &d); err != nil { + logger.Error(err, "failed to delete member Service (non-fatal)", "name", m.Name) + } if err := d.Deployer.CleanupOSSData(ctx, m.RuntimeName); err != nil { logger.Error(err, "failed to clean up OSS agent data (non-fatal)", "name", m.Name, "runtimeName", m.RuntimeName) @@ -517,6 +647,13 @@ func ReconcileMemberDelete(ctx context.Context, d MemberDeps, m MemberContext) e if err := d.Provisioner.DeleteServiceAccount(ctx, m.Name); err != nil { logger.Error(err, "failed to delete ServiceAccount (non-fatal)", "name", m.Name) } + // Clean up remote ServiceAccount when the member was deployed to a + // remote cluster. Non-fatal: log and continue. + if m.DeployMode == v1beta1.DeployModeRemote && m.TargetClusterID != "" { + if err := d.Provisioner.DeleteRemoteServiceAccount(ctx, m.Name, m.TargetClusterID, m.TargetNamespace); err != nil { + logger.Error(err, "failed to delete remote ServiceAccount (non-fatal)", "name", m.Name, "cluster", m.TargetClusterID) + } + } // Every worker (standalone, team leader, team worker) owns a per-worker // comm room created by ProvisionWorker. Release its alias here so a // future Worker/Team CR with the same runtime identity can reclaim it diff --git a/hiclaw-controller/internal/controller/member_reconcile_service.go b/hiclaw-controller/internal/controller/member_reconcile_service.go new file mode 100644 index 000000000..b7243ac55 --- /dev/null +++ b/hiclaw-controller/internal/controller/member_reconcile_service.go @@ -0,0 +1,171 @@ +package controller + +import ( + "context" + "fmt" + "reflect" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "sigs.k8s.io/controller-runtime/pkg/log" + + "github.com/hiclaw/hiclaw-controller/internal/backend" +) + +// ReconcileMemberService ensures a ClusterIP Service exists (or is removed) +// based on ServiceEnabled. This phase runs after ReconcileMemberContainer so +// the Pod selector labels are guaranteed to be present on the target pod. +func ReconcileMemberService(ctx context.Context, mc *MemberContext, deps *MemberDeps) error { + if !mc.ServiceEnabled { + return ensureServiceDeleted(ctx, mc, deps) + } + return ensureServiceExists(ctx, mc, deps) +} + +// ensureServiceExists creates or updates a ClusterIP Service for the member pod. +func ensureServiceExists(ctx context.Context, mc *MemberContext, deps *MemberDeps) error { + logger := log.FromContext(ctx) + + // A Service without ports is useless; delete any stale Service from a + // previous expose config. + if len(mc.Spec.Expose) == 0 { + logger.V(1).Info("serviceEnabled is true but no ports declared in spec.expose, deleting stale Service if present", "name", mc.Name) + return ensureServiceDeleted(ctx, mc, deps) + } + + svcClient, ns, err := resolveServiceClient(ctx, mc, deps) + if err != nil { + return err + } + + svcName := serviceName(deps, mc) + selector := serviceSelector(mc) + desiredPorts := buildServicePorts(mc) + + existing, err := svcClient.Get(ctx, svcName, metav1.GetOptions{}) + if err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("get service %s/%s: %w", ns, svcName, err) + } + + if apierrors.IsNotFound(err) { + // Create + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: svcName, + Namespace: ns, + Labels: map[string]string{ + "hiclaw.io/worker": mc.Name, + "app": deps.ResourcePrefix.WorkerAppLabel(), + }, + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + Selector: selector, + Ports: desiredPorts, + }, + } + if _, err := svcClient.Create(ctx, svc, metav1.CreateOptions{}); err != nil { + if apierrors.IsAlreadyExists(err) { + // Lost the race; will reconcile on next pass. + return nil + } + return fmt.Errorf("create service %s/%s: %w", ns, svcName, err) + } + logger.Info("created ClusterIP Service for member", "name", mc.Name, "service", svcName, "namespace", ns) + return nil + } + + // Update if selector or ports differ. + needsUpdate := !reflect.DeepEqual(existing.Spec.Selector, selector) || + !reflect.DeepEqual(existing.Spec.Ports, desiredPorts) + if !needsUpdate { + return nil + } + + existing.Spec.Selector = selector + existing.Spec.Ports = desiredPorts + if _, err := svcClient.Update(ctx, existing, metav1.UpdateOptions{}); err != nil { + return fmt.Errorf("update service %s/%s: %w", ns, svcName, err) + } + logger.Info("updated ClusterIP Service for member", "name", mc.Name, "service", svcName, "namespace", ns) + return nil +} + +// ensureServiceDeleted removes the ClusterIP Service for the member pod. +func ensureServiceDeleted(ctx context.Context, mc *MemberContext, deps *MemberDeps) error { + svcClient, ns, err := resolveServiceClient(ctx, mc, deps) + if err != nil { + // If the backend doesn't support services (e.g. Docker), nothing to delete. + return nil + } + + svcName := serviceName(deps, mc) + + // Get first: skip deletion if Service does not exist. + _, err = svcClient.Get(ctx, svcName, metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + return nil + } + if err != nil { + return fmt.Errorf("check service %s/%s: %w", ns, svcName, err) + } + + // Service exists — delete it. + if err := svcClient.Delete(ctx, svcName, metav1.DeleteOptions{}); err != nil { + if apierrors.IsNotFound(err) { + return nil + } + return fmt.Errorf("delete service %s/%s: %w", ns, svcName, err) + } + log.FromContext(ctx).Info("deleted ClusterIP Service for member", "name", mc.Name, "service", svcName, "namespace", ns) + return nil +} + +// resolveServiceClient obtains a K8sServiceClient from the detected backend. +// Returns an error if no backend supports Service management. +func resolveServiceClient(ctx context.Context, mc *MemberContext, deps *MemberDeps) (backend.K8sServiceClient, string, error) { + if deps.Backend == nil { + return nil, "", fmt.Errorf("no backend registry available") + } + wb := deps.Backend.DetectWorkerBackend(ctx) + if wb == nil { + return nil, "", fmt.Errorf("no worker backend available") + } + sb, ok := wb.(backend.ServiceBackend) + if !ok { + return nil, "", fmt.Errorf("backend %q does not support Service management", wb.Name()) + } + return sb.ServiceClient(ctx, mc.DeployMode, mc.TargetClusterID, mc.TargetNamespace) +} + +// serviceName returns the K8s Service name for the member, using the same +// prefix + name convention as Pod names (e.g. "hiclaw-worker-alice"). +func serviceName(deps *MemberDeps, mc *MemberContext) string { + return deps.ResourcePrefix.WorkerNamePrefix() + mc.Name +} + +// serviceSelector builds the label selector that matches the member Pod. +// Uses the identity label stamped by createMemberContainer. +func serviceSelector(mc *MemberContext) map[string]string { + return map[string]string{ + "hiclaw.io/worker": mc.Name, + } +} + +// buildServicePorts converts spec.Expose entries into Kubernetes ServicePorts. +func buildServicePorts(mc *MemberContext) []corev1.ServicePort { + ports := make([]corev1.ServicePort, 0, len(mc.Spec.Expose)) + for _, ep := range mc.Spec.Expose { + proto := corev1.ProtocolTCP + name := fmt.Sprintf("port-%d", ep.Port) + ports = append(ports, corev1.ServicePort{ + Name: name, + Port: int32(ep.Port), + TargetPort: intstr.FromInt(ep.Port), + Protocol: proto, + }) + } + return ports +} diff --git a/hiclaw-controller/internal/controller/member_reconcile_service_test.go b/hiclaw-controller/internal/controller/member_reconcile_service_test.go new file mode 100644 index 000000000..6e62a8955 --- /dev/null +++ b/hiclaw-controller/internal/controller/member_reconcile_service_test.go @@ -0,0 +1,308 @@ +package controller + +import ( + "context" + "errors" + "testing" + + v1beta1 "github.com/hiclaw/hiclaw-controller/api/v1beta1" + authpkg "github.com/hiclaw/hiclaw-controller/internal/auth" + "github.com/hiclaw/hiclaw-controller/internal/backend" + "github.com/hiclaw/hiclaw-controller/test/testutil/mocks" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// fakeServiceClient is a stateful K8sServiceClient used by the +// member-Service reconcile tests. It records every call so tests can +// assert which Service operations fired. +type fakeServiceClient struct { + store map[string]*corev1.Service + + getCalls int + createCalls int + updateCalls int + deleteCalls int + + deleteErr error +} + +func newFakeServiceClient() *fakeServiceClient { + return &fakeServiceClient{store: map[string]*corev1.Service{}} +} + +func (f *fakeServiceClient) Get(_ context.Context, name string, _ metav1.GetOptions) (*corev1.Service, error) { + f.getCalls++ + if svc, ok := f.store[name]; ok { + return svc.DeepCopy(), nil + } + return nil, apierrors.NewNotFound(schema.GroupResource{Resource: "services"}, name) +} + +func (f *fakeServiceClient) Create(_ context.Context, svc *corev1.Service, _ metav1.CreateOptions) (*corev1.Service, error) { + f.createCalls++ + cp := svc.DeepCopy() + if _, exists := f.store[cp.Name]; exists { + return nil, apierrors.NewAlreadyExists(schema.GroupResource{Resource: "services"}, cp.Name) + } + f.store[cp.Name] = cp + return cp.DeepCopy(), nil +} + +func (f *fakeServiceClient) Update(_ context.Context, svc *corev1.Service, _ metav1.UpdateOptions) (*corev1.Service, error) { + f.updateCalls++ + cp := svc.DeepCopy() + f.store[cp.Name] = cp + return cp.DeepCopy(), nil +} + +func (f *fakeServiceClient) Delete(_ context.Context, name string, _ metav1.DeleteOptions) error { + f.deleteCalls++ + if f.deleteErr != nil { + return f.deleteErr + } + if _, exists := f.store[name]; !exists { + return apierrors.NewNotFound(schema.GroupResource{Resource: "services"}, name) + } + delete(f.store, name) + return nil +} + +// fakeServiceBackend implements both backend.WorkerBackend and +// backend.ServiceBackend, returning a controllable K8sServiceClient. +type fakeServiceBackend struct { + svc *fakeServiceClient + ns string +} + +func (f *fakeServiceBackend) Name() string { return "fake" } +func (f *fakeServiceBackend) DeploymentMode() string { return backend.DeployCloud } +func (f *fakeServiceBackend) Available(_ context.Context) bool { return true } +func (f *fakeServiceBackend) NeedsCredentialInjection() bool { return true } +func (f *fakeServiceBackend) Create(_ context.Context, _ backend.CreateRequest) (*backend.WorkerResult, error) { + return nil, nil +} +func (f *fakeServiceBackend) Delete(_ context.Context, _ string) error { return nil } +func (f *fakeServiceBackend) Start(_ context.Context, _ string) error { return nil } +func (f *fakeServiceBackend) Stop(_ context.Context, _ string) error { return nil } +func (f *fakeServiceBackend) Status(_ context.Context, _ string) (*backend.WorkerResult, error) { + return nil, nil +} +func (f *fakeServiceBackend) ServiceClient(_ context.Context, _, _, _ string) (backend.K8sServiceClient, string, error) { + return f.svc, f.ns, nil +} + +// noopWorkerBackend is a WorkerBackend that does NOT implement ServiceBackend +// — used to assert that ensureServiceDeleted gracefully tolerates such a +// backend (e.g. Docker) without erroring out. +type noopWorkerBackend struct{} + +func (n *noopWorkerBackend) Name() string { return "noop" } +func (n *noopWorkerBackend) DeploymentMode() string { return backend.DeployLocal } +func (n *noopWorkerBackend) Available(_ context.Context) bool { return true } +func (n *noopWorkerBackend) NeedsCredentialInjection() bool { return false } +func (n *noopWorkerBackend) Create(_ context.Context, _ backend.CreateRequest) (*backend.WorkerResult, error) { + return nil, nil +} +func (n *noopWorkerBackend) Delete(_ context.Context, _ string) error { return nil } +func (n *noopWorkerBackend) Start(_ context.Context, _ string) error { return nil } +func (n *noopWorkerBackend) Stop(_ context.Context, _ string) error { return nil } +func (n *noopWorkerBackend) Status(_ context.Context, _ string) (*backend.WorkerResult, error) { + return nil, nil +} + +func newServiceTestDeps(svc *fakeServiceClient) *MemberDeps { + return &MemberDeps{ + Backend: backend.NewRegistry([]backend.WorkerBackend{ + &fakeServiceBackend{svc: svc, ns: "hiclaw"}, + }), + ResourcePrefix: authpkg.DefaultResourcePrefix, + } +} + +func newServiceTestMember(name string, enabled bool, ports ...int) *MemberContext { + expose := make([]v1beta1.ExposePort, 0, len(ports)) + for _, p := range ports { + expose = append(expose, v1beta1.ExposePort{Port: p}) + } + return &MemberContext{ + Name: name, + Spec: v1beta1.WorkerSpec{Expose: expose}, + ServiceEnabled: enabled, + } +} + +func TestReconcileMemberService_CreatesWhenEnabled(t *testing.T) { + svc := newFakeServiceClient() + deps := newServiceTestDeps(svc) + mc := newServiceTestMember("alice", true, 8080, 9090) + + if err := ReconcileMemberService(context.Background(), mc, deps); err != nil { + t.Fatalf("ReconcileMemberService: %v", err) + } + + stored, ok := svc.store["hiclaw-worker-alice"] + if !ok { + t.Fatalf("expected Service hiclaw-worker-alice to be created; got %+v", svc.store) + } + if svc.createCalls != 1 { + t.Errorf("Create calls = %d, want 1", svc.createCalls) + } + if got := stored.Spec.Selector["hiclaw.io/worker"]; got != "alice" { + t.Errorf("Selector hiclaw.io/worker = %q, want alice", got) + } + if len(stored.Spec.Ports) != 2 { + t.Errorf("Ports len = %d, want 2", len(stored.Spec.Ports)) + } + if stored.Spec.Type != corev1.ServiceTypeClusterIP { + t.Errorf("Service.Spec.Type = %q, want ClusterIP", stored.Spec.Type) + } +} + +func TestReconcileMemberService_SkipsWhenDisabled(t *testing.T) { + svc := newFakeServiceClient() + deps := newServiceTestDeps(svc) + mc := newServiceTestMember("bob", false, 8080) + + if err := ReconcileMemberService(context.Background(), mc, deps); err != nil { + t.Fatalf("ReconcileMemberService: %v", err) + } + if svc.createCalls != 0 { + t.Errorf("expected no Service.Create calls when disabled, got %d", svc.createCalls) + } + // ServiceEnabled=false routes through ensureServiceDeleted which + // performs a Get first; the Service does not exist so the fake + // returns NotFound and the reconciler short-circuits without Delete. + if svc.getCalls != 1 { + t.Errorf("expected 1 Get attempt for existence check, got %d", svc.getCalls) + } + if svc.deleteCalls != 0 { + t.Errorf("expected 0 Delete calls (Get returned NotFound), got %d", svc.deleteCalls) + } +} + +// TestReconcileMemberService_DeletesWhenExposeEmpty covers the edge case +// where ServiceEnabled is true but the member exposes no ports. A portless +// Service is useless, and an existing Service from a previous expose config +// must be removed. +func TestReconcileMemberService_DeletesWhenExposeEmpty(t *testing.T) { + svc := newFakeServiceClient() + svc.store["hiclaw-worker-carol"] = &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "hiclaw-worker-carol", Namespace: "hiclaw"}, + } + deps := newServiceTestDeps(svc) + mc := newServiceTestMember("carol", true) // no ports + + if err := ReconcileMemberService(context.Background(), mc, deps); err != nil { + t.Fatalf("ReconcileMemberService: %v", err) + } + if svc.createCalls != 0 { + t.Errorf("Create calls = %d, want 0 when expose is empty", svc.createCalls) + } + if svc.deleteCalls != 1 { + t.Errorf("Delete calls = %d, want 1 when expose is empty and Service exists", svc.deleteCalls) + } + if _, ok := svc.store["hiclaw-worker-carol"]; ok { + t.Fatal("expected stale Service to be deleted when expose is empty") + } +} + +func TestReconcileMemberService_UpdatesWhenPortsDiffer(t *testing.T) { + svc := newFakeServiceClient() + // Pre-populate a stale Service that selects the right Pod but exposes + // the wrong ports. + svc.store["hiclaw-worker-dave"] = &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "hiclaw-worker-dave", Namespace: "hiclaw"}, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + Selector: map[string]string{"hiclaw.io/worker": "dave"}, + Ports: []corev1.ServicePort{ + {Name: "port-1111", Port: 1111, Protocol: corev1.ProtocolTCP}, + }, + }, + } + deps := newServiceTestDeps(svc) + mc := newServiceTestMember("dave", true, 8080) + + if err := ReconcileMemberService(context.Background(), mc, deps); err != nil { + t.Fatalf("ReconcileMemberService: %v", err) + } + if svc.updateCalls != 1 { + t.Errorf("Update calls = %d, want 1 (ports differed)", svc.updateCalls) + } +} + +// TestEnsureServiceDeleted_NotFoundIsOK verifies that ensureServiceDeleted +// swallows an apierrors.NotFound — the desired post-condition (no Service) +// is already satisfied. +func TestEnsureServiceDeleted_NotFoundIsOK(t *testing.T) { + svc := newFakeServiceClient() + // Configure the fake's Delete to surface NotFound directly. + svc.deleteErr = apierrors.NewNotFound(schema.GroupResource{Resource: "services"}, "hiclaw-worker-eve") + deps := newServiceTestDeps(svc) + mc := newServiceTestMember("eve", false) + + if err := ensureServiceDeleted(context.Background(), mc, deps); err != nil { + t.Fatalf("ensureServiceDeleted should swallow NotFound, got %v", err) + } +} + +// TestEnsureServiceDeleted_NoBackendIsTolerated covers the Docker-style +// case where the active worker backend does not implement ServiceBackend. +// ensureServiceDeleted treats the resolve-failure as a no-op so reconcile +// of a Worker with serviceEnabled=false does not block on that distinction. +func TestEnsureServiceDeleted_NoBackendIsTolerated(t *testing.T) { + deps := &MemberDeps{ + Backend: backend.NewRegistry([]backend.WorkerBackend{&noopWorkerBackend{}}), + ResourcePrefix: authpkg.DefaultResourcePrefix, + } + mc := newServiceTestMember("frank", false) + + if err := ensureServiceDeleted(context.Background(), mc, deps); err != nil { + t.Fatalf("ensureServiceDeleted must not error when backend lacks ServiceBackend, got %v", err) + } +} + +// TestEnsureServiceDeleted_PropagatesUnexpectedError ensures that errors +// other than NotFound bubble up so reconcile retries them. +func TestEnsureServiceDeleted_PropagatesUnexpectedError(t *testing.T) { + svc := newFakeServiceClient() + // Pre-populate so Get succeeds and Delete is actually attempted. + svc.store["hiclaw-worker-grace"] = &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "hiclaw-worker-grace", Namespace: "hiclaw"}, + } + svc.deleteErr = errors.New("boom") + deps := newServiceTestDeps(svc) + mc := newServiceTestMember("grace", false) + + if err := ensureServiceDeleted(context.Background(), mc, deps); err == nil { + t.Fatal("expected non-NotFound error to propagate") + } +} + +func TestReconcileMemberDelete_DeletesService(t *testing.T) { + svc := newFakeServiceClient() + svc.store["hiclaw-worker-heidi"] = &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "hiclaw-worker-heidi", Namespace: "hiclaw"}, + } + deps := newServiceTestDeps(svc) + deps.Provisioner = mocks.NewMockProvisioner() + deps.Deployer = mocks.NewMockDeployer() + member := MemberContext{ + Name: "heidi", + RuntimeName: "heidi", + Role: RoleStandalone, + } + + if err := ReconcileMemberDelete(context.Background(), *deps, member); err != nil { + t.Fatalf("ReconcileMemberDelete: %v", err) + } + if svc.deleteCalls != 1 { + t.Fatalf("Delete calls = %d, want 1", svc.deleteCalls) + } + if _, ok := svc.store["hiclaw-worker-heidi"]; ok { + t.Fatal("expected Service to be deleted during member finalizer cleanup") + } +} diff --git a/hiclaw-controller/internal/controller/member_reconcile_test.go b/hiclaw-controller/internal/controller/member_reconcile_test.go index a5c83bb94..30b14b4ef 100644 --- a/hiclaw-controller/internal/controller/member_reconcile_test.go +++ b/hiclaw-controller/internal/controller/member_reconcile_test.go @@ -5,6 +5,7 @@ import ( "testing" v1beta1 "github.com/hiclaw/hiclaw-controller/api/v1beta1" + "github.com/hiclaw/hiclaw-controller/internal/backend" "github.com/hiclaw/hiclaw-controller/internal/service" "github.com/hiclaw/hiclaw-controller/test/testutil/mocks" ) @@ -111,3 +112,111 @@ func equalStringSlices(a, b []string) bool { } return true } + +// mockBackend is a minimal WorkerBackend implementation used to test +// resolveBackendForMember when the backend is NOT a *backend.K8sBackend. +type mockBackend struct{} + +func (m *mockBackend) Name() string { return "mock" } +func (m *mockBackend) DeploymentMode() string { return "local" } +func (m *mockBackend) Available(_ context.Context) bool { return true } +func (m *mockBackend) NeedsCredentialInjection() bool { return false } +func (m *mockBackend) Create(_ context.Context, _ backend.CreateRequest) (*backend.WorkerResult, error) { + return nil, nil +} +func (m *mockBackend) Delete(_ context.Context, _ string) error { return nil } +func (m *mockBackend) Start(_ context.Context, _ string) error { return nil } +func (m *mockBackend) Stop(_ context.Context, _ string) error { return nil } +func (m *mockBackend) Status(_ context.Context, _ string) (*backend.WorkerResult, error) { + return nil, nil +} + +func TestResolveBackendForMember_LocalMode(t *testing.T) { + // When DeployMode is empty or "Local", the original backend is returned unchanged. + k8sB := backend.NewK8sBackendWithClient(nil, backend.K8sConfig{Namespace: "default"}, "hiclaw-worker-", nil) + + tests := []struct { + name string + deployMode string + }{ + {"empty deploy mode", ""}, + {"explicit Local mode", v1beta1.DeployModeLocal}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := MemberContext{ + Name: "worker-a", + DeployMode: tt.deployMode, + } + got := resolveBackendForMember(k8sB, m) + if got != k8sB { + t.Errorf("expected original backend pointer, got a different instance") + } + }) + } +} + +func TestResolveBackendForMember_RemoteMode(t *testing.T) { + // When DeployMode="Remote" and TargetClusterID is set with a K8sBackend, + // should return a new K8sBackend with remote target configured. + k8sB := backend.NewK8sBackendWithClient(nil, backend.K8sConfig{Namespace: "default"}, "hiclaw-worker-", nil) + + m := MemberContext{ + Name: "worker-b", + DeployMode: v1beta1.DeployModeRemote, + TargetClusterID: "cluster-remote-1", + TargetNamespace: "remote-ns", + } + + got := resolveBackendForMember(k8sB, m) + if got == k8sB { + t.Fatal("expected a new backend instance for remote mode, got same pointer") + } + + // Verify the returned value is still a *backend.K8sBackend. + remoteK8s, ok := got.(*backend.K8sBackend) + if !ok { + t.Fatalf("expected *backend.K8sBackend, got %T", got) + } + + // The remote backend should still report as "k8s". + if remoteK8s.Name() != "k8s" { + t.Errorf("remote backend Name() = %q, want %q", remoteK8s.Name(), "k8s") + } +} + +func TestResolveBackendForMember_NonK8sBackend(t *testing.T) { + // When DeployMode="Remote" but the backend is not a *K8sBackend, + // should return the original backend unchanged. + mb := &mockBackend{} + m := MemberContext{ + Name: "worker-c", + DeployMode: v1beta1.DeployModeRemote, + TargetClusterID: "cluster-remote-2", + TargetNamespace: "remote-ns", + } + + got := resolveBackendForMember(mb, m) + if got != mb { + t.Errorf("expected original mock backend, got a different instance") + } +} + +func TestResolveBackendForMember_EmptyClusterID(t *testing.T) { + // When DeployMode="Remote" but TargetClusterID is empty, + // should return the original backend unchanged. + k8sB := backend.NewK8sBackendWithClient(nil, backend.K8sConfig{Namespace: "default"}, "hiclaw-worker-", nil) + + m := MemberContext{ + Name: "worker-d", + DeployMode: v1beta1.DeployModeRemote, + TargetClusterID: "", + TargetNamespace: "remote-ns", + } + + got := resolveBackendForMember(k8sB, m) + if got != k8sB { + t.Errorf("expected original backend (empty clusterID), got a different instance") + } +} diff --git a/hiclaw-controller/internal/controller/team_controller.go b/hiclaw-controller/internal/controller/team_controller.go index c7925238e..a9e13746d 100644 --- a/hiclaw-controller/internal/controller/team_controller.go +++ b/hiclaw-controller/internal/controller/team_controller.go @@ -67,6 +67,9 @@ type TeamReconciler struct { // MemberContext.PodLabels → backend.CreateRequest.Labels. Empty in // embedded mode. ControllerName string + Namespace string + + RemoteWatchRegistrar RemoteWatchRegistrar // ResourcePrefix scopes team-member ServiceAccount and Pod names per // HiClaw tenant instance. Forwarded into MemberDeps.ResourcePrefix so @@ -314,9 +317,12 @@ func (r *TeamReconciler) reconcileTeamNormal(ctx context.Context, t *v1beta1.Tea } if err := r.reconcileMember(ctx, deps, m, ms); err != nil { logger.Error(err, "team member reconcile failed", "name", m.Name) + ms.Message = err.Error() + ms.Phase = computeMemberPhase(ms.Phase, ms.MatrixUserID, m.Spec.DesiredState(), ms.ContainerState, err) perMemberErrors = append(perMemberErrors, fmt.Sprintf("%s: %v", m.Name, err)) continue } + ms.Message = "" // Record the hash only after a full reconcile success so a failed // mid-phase attempt on the next pass still sees SpecChanged=true // and retries the container recreation. ms.Observed was already @@ -416,6 +422,11 @@ func (r *TeamReconciler) reconcileTeamNormal(ctx context.Context, t *v1beta1.Tea // see the Step 4 comment in reconcileTeamNormal for why post-infra failures // must not revoke observed status (token-rotation hazard). func (r *TeamReconciler) reconcileMember(ctx context.Context, deps MemberDeps, m MemberContext, ms *v1beta1.TeamMemberStatus) error { + // Validate cross-cluster deployment fields before entering phases. + if err := ValidateMemberDeployment(m); err != nil { + return err + } + state := &MemberState{} // Pre-populate ExistingMatrixUserID when we've already provisioned the @@ -447,6 +458,9 @@ func (r *TeamReconciler) reconcileMember(ctx context.Context, deps MemberDeps, m if _, err := ReconcileMemberContainer(ctx, deps, m, state); err != nil { return err } + if err := ReconcileMemberService(ctx, &m, &deps); err != nil { + return err + } _ = ReconcileMemberExpose(ctx, deps, m, state) if m.Role == RoleTeamWorker { @@ -474,13 +488,16 @@ func (r *TeamReconciler) summarizeBackendReadiness(ctx context.Context, t *v1bet return false, 0 } for _, m := range members { - result, err := wb.Status(ctx, m.Name) + mwb := resolveBackendForMember(wb, m) + result, err := mwb.Status(ctx, m.Name) if err != nil { continue } ready := result.Status == backend.StatusRunning || result.Status == backend.StatusReady if ms := t.Status.MemberByName(m.Name); ms != nil { ms.Ready = ready + ms.ContainerState = string(result.Status) + ms.Phase = computeMemberPhase(ms.Phase, ms.MatrixUserID, m.Spec.DesiredState(), ms.ContainerState, nil) } if m.Role == RoleTeamLeader { leaderReady = ready @@ -811,6 +828,12 @@ func buildDesiredMembers(t *v1beta1.Team, controllerName string) []MemberContext Every: every, } } + + var leaderServiceEnabled bool + if t.Spec.Leader.ServiceEnabled != nil { + leaderServiceEnabled = *t.Spec.Leader.ServiceEnabled + } + members = append(members, MemberContext{ Name: t.Spec.Leader.Name, RuntimeName: t.Spec.Leader.EffectiveWorkerName(), @@ -827,11 +850,19 @@ func buildDesiredMembers(t *v1beta1.Team, controllerName string) []MemberContext PodLabels: memberLabels(RoleTeamLeader, t.Spec.Leader.Labels), Owner: t, Heartbeat: leaderHeartbeat, + DeployMode: v1beta1.DeployModeLocal, + ServiceEnabled: leaderServiceEnabled, }) for _, w := range t.Spec.Workers { workerObserved := isObserved(w.Name) spec := teamWorkerSpecToWorkerSpec(t, w) + + var wServiceEnabled bool + if w.ServiceEnabled != nil { + wServiceEnabled = *w.ServiceEnabled + } + members = append(members, MemberContext{ Name: w.Name, RuntimeName: w.EffectiveWorkerName(), @@ -847,6 +878,8 @@ func buildDesiredMembers(t *v1beta1.Team, controllerName string) []MemberContext TeamCoordinatorIDs: teamCoordinatorIDs(t), PodLabels: memberLabels(RoleTeamWorker, w.Labels), Owner: t, + DeployMode: v1beta1.DeployModeLocal, + ServiceEnabled: wServiceEnabled, }) } return members @@ -1130,24 +1163,50 @@ func (r *TeamReconciler) SetupWithManager(mgr ctrl.Manager) error { if wb := r.Backend.DetectWorkerBackend(context.Background()); wb != nil && wb.Name() == "k8s" { bldr = bldr.Watches( &corev1.Pod{}, - handler.EnqueueRequestsFromMapFunc(func(_ context.Context, obj client.Object) []reconcile.Request { - teamName := obj.GetLabels()["hiclaw.io/team"] - if teamName == "" { - return nil - } - return []reconcile.Request{ - {NamespacedName: client.ObjectKey{ - Name: teamName, - Namespace: obj.GetNamespace(), - }}, - } - }), + teamPodEventHandler(""), builder.WithPredicates(podLifecyclePredicates("hiclaw.io/team", r.ControllerName)), ) } } - return bldr.Complete(r) + ctl, err := bldr.Build(r) + if err != nil { + return err + } + if r.RemoteWatchRegistrar != nil && r.Backend != nil { + if wb := r.Backend.DetectWorkerBackend(context.Background()); wb != nil && wb.Name() == "k8s" { + r.RemoteWatchRegistrar.RegisterWatch( + ctl, + &corev1.Pod{}, + teamPodEventHandler(r.Namespace), + podLifecyclePredicates("hiclaw.io/team", r.ControllerName), + ) + } + } + return nil +} + +func teamPodEventHandler(localNamespace string) handler.EventHandler { + return handler.EnqueueRequestsFromMapFunc(func(_ context.Context, obj client.Object) []reconcile.Request { + return teamPodRequests(obj, localNamespace) + }) +} + +func teamPodRequests(obj client.Object, localNamespace string) []reconcile.Request { + teamName := obj.GetLabels()["hiclaw.io/team"] + if teamName == "" { + return nil + } + namespace := localNamespace + if namespace == "" { + namespace = obj.GetNamespace() + } + return []reconcile.Request{ + {NamespacedName: client.ObjectKey{ + Name: teamName, + Namespace: namespace, + }}, + } } // --- Policy helpers (preserved from prior implementation) --- diff --git a/hiclaw-controller/internal/controller/worker_controller.go b/hiclaw-controller/internal/controller/worker_controller.go index 8456cd4de..58217a505 100644 --- a/hiclaw-controller/internal/controller/worker_controller.go +++ b/hiclaw-controller/internal/controller/worker_controller.go @@ -16,6 +16,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + ctrlcontroller "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -56,6 +57,9 @@ type WorkerReconciler struct { // hiclaw.io/controller label so multiple controller instances sharing a // namespace do not cross-watch each other's resources. ControllerName string + Namespace string + + RemoteWatchRegistrar RemoteWatchRegistrar } func (r *WorkerReconciler) Reconcile(ctx context.Context, req reconcile.Request) (retres reconcile.Result, reterr error) { @@ -71,6 +75,10 @@ func (r *WorkerReconciler) Reconcile(ctx context.Context, req reconcile.Request) patchBase := client.MergeFrom(worker.DeepCopy()) + // Shared MemberState captured by the defer so phase computation can + // observe the actual container state recorded during reconcile. + state := &MemberState{} + // Unified status patch at the end of every reconcile. ObservedGeneration // is only written when reconcile succeeds, preventing the infinite-loop // bug where a failed status write triggered re-reconcile with @@ -79,7 +87,7 @@ func (r *WorkerReconciler) Reconcile(ctx context.Context, req reconcile.Request) if !worker.DeletionTimestamp.IsZero() { return } - worker.Status.Phase = computeWorkerPhase(&worker, reterr) + worker.Status.Phase = computeWorkerPhase(&worker, state.ContainerState, reterr) if reterr == nil { worker.Status.ObservedGeneration = worker.Generation worker.Status.Message = "" @@ -107,14 +115,14 @@ func (r *WorkerReconciler) Reconcile(ctx context.Context, req reconcile.Request) } } - return r.reconcileNormal(ctx, &worker) + return r.reconcileNormal(ctx, &worker, state) } // reconcileNormal builds a MemberContext from the Worker CR, runs the shared // member reconcile phases, and writes runtime state back to Worker.Status. // Legacy Manager groupAllowFrom is updated here only for standalone workers; // team leaders are handled by TeamReconciler. -func (r *WorkerReconciler) reconcileNormal(ctx context.Context, w *v1beta1.Worker) (reconcile.Result, error) { +func (r *WorkerReconciler) reconcileNormal(ctx context.Context, w *v1beta1.Worker, state *MemberState) (reconcile.Result, error) { deps := MemberDeps{ Provisioner: r.Provisioner, Deployer: r.Deployer, @@ -124,7 +132,11 @@ func (r *WorkerReconciler) reconcileNormal(ctx context.Context, w *v1beta1.Worke DefaultRuntime: r.DefaultRuntime, GatewayClient: r.GatewayClient, } - mctx := r.workerMemberContext(w) + spec, err := effectiveWorkerSpecForTarget(w) + if err != nil { + return reconcile.Result{}, err + } + mctx := r.workerMemberContextWithSpec(w, spec) if w.Spec.ModelProvider != "" && r.GatewayClient != nil { info, err := r.GatewayClient.ResolveModelProvider(ctx, w.Spec.ModelProvider) @@ -134,7 +146,10 @@ func (r *WorkerReconciler) reconcileNormal(ctx context.Context, w *v1beta1.Worke mctx.ModelProviderInfo = info } - state := &MemberState{} + // Validate cross-cluster deployment fields before entering phases. + if err := ValidateMemberDeployment(mctx); err != nil { + return reconcile.Result{}, err + } if res, err := ReconcileMemberInfra(ctx, deps, mctx, state); err != nil || res.RequeueAfter > 0 { applyMemberStateToWorker(w, state) @@ -154,8 +169,16 @@ func (r *WorkerReconciler) reconcileNormal(ctx context.Context, w *v1beta1.Worke } if res, err := ReconcileMemberContainer(ctx, deps, mctx, state); err != nil || res.RequeueAfter > 0 { applyMemberStateToWorker(w, state) + if err == nil { + applyDeploymentTargetStatus(w, mctx) + } return res, err } + applyDeploymentTargetStatus(w, mctx) + if err := ReconcileMemberService(ctx, &mctx, &deps); err != nil { + applyMemberStateToWorker(w, state) + return reconcile.Result{}, err + } _ = ReconcileMemberExpose(ctx, deps, mctx, state) applyMemberStateToWorker(w, state) @@ -187,7 +210,8 @@ func (r *WorkerReconciler) reconcileDelete(ctx context.Context, w *v1beta1.Worke DefaultRuntime: r.DefaultRuntime, GatewayClient: r.GatewayClient, } - mctx := r.workerMemberContext(w) + spec := workerSpecWithAppliedDeploymentTarget(w.Spec, w.Status) + mctx := r.workerMemberContextWithSpec(w, spec) _ = ReconcileMemberDelete(ctx, deps, mctx) @@ -260,19 +284,39 @@ func (r *WorkerReconciler) reconcileLegacy(ctx context.Context, w *v1beta1.Worke // anything the user writes that collides (e.g. `hiclaw.io/controller`) // is silently overridden rather than rejected. func (r *WorkerReconciler) workerMemberContext(w *v1beta1.Worker) MemberContext { + return r.workerMemberContextWithSpec(w, w.Spec) +} + +func (r *WorkerReconciler) workerMemberContextWithSpec(w *v1beta1.Worker, spec v1beta1.WorkerSpec) MemberContext { role := roleForAnnotations(w.Annotations["hiclaw.io/role"], w.Annotations["hiclaw.io/team-leader"]) - runtimeName := w.Spec.EffectiveWorkerName(w.Name) + runtimeName := spec.EffectiveWorkerName(w.Name) + + // Cross-cluster deployment fields. + deployMode := v1beta1.DeployModeLocal + if spec.DeployMode != nil { + deployMode = *spec.DeployMode + } + var targetClusterID, targetNamespace string + if spec.TargetCluster != nil { + targetClusterID = spec.TargetCluster.ID + targetNamespace = spec.TargetCluster.Namespace + } + var serviceEnabled bool + if spec.ServiceEnabled != nil { + serviceEnabled = *spec.ServiceEnabled + } + return MemberContext{ Name: w.Name, RuntimeName: runtimeName, Namespace: w.Namespace, Role: role, - Spec: w.Spec, + Spec: spec, Generation: w.Generation, ObservedGeneration: w.Status.ObservedGeneration, PodLabels: mergeLabels( w.ObjectMeta.Labels, - w.Spec.Labels, + spec.Labels, map[string]string{ v1beta1.LabelController: r.ControllerName, "hiclaw.io/role": role.String(), @@ -296,7 +340,95 @@ func (r *WorkerReconciler) workerMemberContext(w *v1beta1.Worker) MemberContext ExistingRoomID: w.Status.RoomID, CurrentExposedPorts: w.Status.ExposedPorts, Owner: w, + DeployMode: deployMode, + TargetClusterID: targetClusterID, + TargetNamespace: targetNamespace, + ServiceEnabled: serviceEnabled, + } +} + +func effectiveWorkerSpecForTarget(w *v1beta1.Worker) (v1beta1.WorkerSpec, error) { + spec := *w.Spec.DeepCopy() + if w.Status.DeployMode == "" && w.Status.TargetCluster == nil { + return spec, nil + } + statusMode := w.Status.DeployMode + if statusMode == "" { + statusMode = v1beta1.DeployModeLocal + } + desiredMode, desiredTarget := workerSpecDeploymentTarget(spec) + if statusMode == desiredMode && sameTargetCluster(w.Status.TargetCluster, desiredTarget) { + return spec, nil + } + if spec.DesiredState() != "Stopped" { + return spec, fmt.Errorf("spec.deployMode/spec.targetCluster cannot be changed until the Worker is Stopped; current=%s, desired=%s", + deploymentTargetSummary(statusMode, w.Status.TargetCluster), + deploymentTargetSummary(desiredMode, desiredTarget)) + } + if w.Status.Phase != "Stopped" { + return workerSpecWithAppliedDeploymentTarget(spec, w.Status), nil } + return spec, nil +} + +func workerSpecWithAppliedDeploymentTarget(spec v1beta1.WorkerSpec, status v1beta1.WorkerStatus) v1beta1.WorkerSpec { + if status.DeployMode == "" && status.TargetCluster == nil { + return spec + } + mode := status.DeployMode + if mode == "" { + mode = v1beta1.DeployModeLocal + } + spec.DeployMode = &mode + if mode == v1beta1.DeployModeRemote && status.TargetCluster != nil { + spec.TargetCluster = status.TargetCluster.DeepCopy() + } else if mode == v1beta1.DeployModeLocal { + spec.TargetCluster = nil + } + return spec +} + +func applyDeploymentTargetStatus(w *v1beta1.Worker, m MemberContext) { + w.Status.DeployMode = m.DeployMode + if m.DeployMode == v1beta1.DeployModeRemote || m.TargetClusterID != "" || m.TargetNamespace != "" { + w.Status.TargetCluster = &v1beta1.TargetClusterSpec{ + ID: m.TargetClusterID, + Namespace: m.TargetNamespace, + } + } else { + w.Status.TargetCluster = nil + } +} + +func workerSpecDeploymentTarget(spec v1beta1.WorkerSpec) (string, *v1beta1.TargetClusterSpec) { + mode := v1beta1.DeployModeLocal + if spec.DeployMode != nil && *spec.DeployMode != "" { + mode = *spec.DeployMode + } + if mode != v1beta1.DeployModeRemote { + return mode, nil + } + if spec.TargetCluster == nil { + return mode, nil + } + return mode, spec.TargetCluster.DeepCopy() +} + +func sameTargetCluster(a, b *v1beta1.TargetClusterSpec) bool { + if a == nil || b == nil { + return a == nil && b == nil + } + return a.ID == b.ID && a.Namespace == b.Namespace +} + +func deploymentTargetSummary(mode string, target *v1beta1.TargetClusterSpec) string { + if mode == "" { + mode = v1beta1.DeployModeLocal + } + if mode != v1beta1.DeployModeRemote || target == nil { + return mode + } + return fmt.Sprintf("%s/%s/%s", mode, target.ID, target.Namespace) } // applyMemberStateToWorker copies runtime state into Worker.Status fields. @@ -321,20 +453,9 @@ func applyMemberStateToWorker(w *v1beta1.Worker, state *MemberState) { } // computeWorkerPhase determines the Worker status phase from the reconcile -// outcome. On success, phase reflects the desired lifecycle state. -func computeWorkerPhase(w *v1beta1.Worker, reconcileErr error) string { - if reconcileErr != nil { - if w.Status.MatrixUserID == "" { - return "Failed" - } - if w.Status.Phase == "" { - return "Pending" - } - // Keep the old Phase to avoid marking a healthy worker as Failed on a - // transient error; the error surfaces through Status.Message instead. - return w.Status.Phase - } - return w.Spec.DesiredState() +// outcome. Delegates to the shared computeMemberPhase function. +func computeWorkerPhase(w *v1beta1.Worker, containerState string, reconcileErr error) string { + return computeMemberPhase(w.Status.Phase, w.Status.MatrixUserID, w.Spec.DesiredState(), containerState, reconcileErr) } func (r *WorkerReconciler) SetupWithManager(mgr ctrl.Manager) error { @@ -345,29 +466,59 @@ func (r *WorkerReconciler) SetupWithManager(mgr ctrl.Manager) error { if wb := r.Backend.DetectWorkerBackend(context.Background()); wb != nil && wb.Name() == "k8s" { bldr = bldr.Watches( &corev1.Pod{}, - handler.EnqueueRequestsFromMapFunc(func(_ context.Context, obj client.Object) []reconcile.Request { - workerName := obj.GetLabels()["hiclaw.io/worker"] - if workerName == "" { - return nil - } - // Skip pods owned by a Team (those are reconciled via - // the Team controller's own pod watch). - if obj.GetLabels()["hiclaw.io/team"] != "" { - return nil - } - return []reconcile.Request{ - {NamespacedName: client.ObjectKey{ - Name: workerName, - Namespace: obj.GetNamespace(), - }}, - } - }), + workerPodEventHandler(""), builder.WithPredicates(podLifecyclePredicates("hiclaw.io/worker", r.ControllerName)), ) } } - return bldr.Complete(r) + ctl, err := bldr.Build(r) + if err != nil { + return err + } + if r.RemoteWatchRegistrar != nil && r.Backend != nil { + if wb := r.Backend.DetectWorkerBackend(context.Background()); wb != nil && wb.Name() == "k8s" { + r.RemoteWatchRegistrar.RegisterWatch( + ctl, + &corev1.Pod{}, + workerPodEventHandler(r.Namespace), + podLifecyclePredicates("hiclaw.io/worker", r.ControllerName), + ) + } + } + return nil +} + +type RemoteWatchRegistrar interface { + RegisterWatch(ctrlcontroller.Controller, client.Object, handler.EventHandler, ...predicate.Predicate) +} + +func workerPodEventHandler(localNamespace string) handler.EventHandler { + return handler.EnqueueRequestsFromMapFunc(func(_ context.Context, obj client.Object) []reconcile.Request { + return workerPodRequests(obj, localNamespace) + }) +} + +func workerPodRequests(obj client.Object, localNamespace string) []reconcile.Request { + workerName := obj.GetLabels()["hiclaw.io/worker"] + if workerName == "" { + return nil + } + // Skip pods owned by a Team (those are reconciled via + // the Team controller's own pod watch). + if obj.GetLabels()["hiclaw.io/team"] != "" { + return nil + } + namespace := localNamespace + if namespace == "" { + namespace = obj.GetNamespace() + } + return []reconcile.Request{ + {NamespacedName: client.ObjectKey{ + Name: workerName, + Namespace: namespace, + }}, + } } // podLifecyclePredicates filters Pod events to only trigger reconciliation on diff --git a/hiclaw-controller/internal/controller/worker_controller_test.go b/hiclaw-controller/internal/controller/worker_controller_test.go index d4d9b112d..93fa2668d 100644 --- a/hiclaw-controller/internal/controller/worker_controller_test.go +++ b/hiclaw-controller/internal/controller/worker_controller_test.go @@ -4,6 +4,8 @@ import ( "testing" v1beta1 "github.com/hiclaw/hiclaw-controller/api/v1beta1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // TestWorkerMemberContext_StampsControllerAndRoleLabels verifies that a @@ -148,3 +150,121 @@ func TestWorkerMemberContext_SpecChangedGate(t *testing.T) { }) } } + +func TestEffectiveWorkerSpecForTargetRejectsRunningChange(t *testing.T) { + remote := v1beta1.DeployModeRemote + w := &v1beta1.Worker{ + Spec: v1beta1.WorkerSpec{ + DeployMode: &remote, + TargetCluster: &v1beta1.TargetClusterSpec{ID: "cluster-b", Namespace: "agents"}, + }, + Status: v1beta1.WorkerStatus{ + Phase: "Running", + DeployMode: v1beta1.DeployModeRemote, + TargetCluster: &v1beta1.TargetClusterSpec{ + ID: "cluster-a", + Namespace: "agents", + }, + }, + } + + if _, err := effectiveWorkerSpecForTarget(w); err == nil { + t.Fatal("expected running target change to be rejected") + } +} + +func TestEffectiveWorkerSpecForTargetUsesAppliedTargetUntilStopped(t *testing.T) { + remote := v1beta1.DeployModeRemote + stopped := "Stopped" + w := &v1beta1.Worker{ + Spec: v1beta1.WorkerSpec{ + State: &stopped, + DeployMode: &remote, + TargetCluster: &v1beta1.TargetClusterSpec{ID: "cluster-b", Namespace: "agents-b"}, + }, + Status: v1beta1.WorkerStatus{ + Phase: "Stopping", + DeployMode: v1beta1.DeployModeRemote, + TargetCluster: &v1beta1.TargetClusterSpec{ + ID: "cluster-a", + Namespace: "agents-a", + }, + }, + } + + spec, err := effectiveWorkerSpecForTarget(w) + if err != nil { + t.Fatalf("effectiveWorkerSpecForTarget: %v", err) + } + if spec.TargetCluster == nil || spec.TargetCluster.ID != "cluster-a" || spec.TargetCluster.Namespace != "agents-a" { + t.Fatalf("TargetCluster=%+v, want applied cluster-a/agents-a", spec.TargetCluster) + } +} + +func TestEffectiveWorkerSpecForTargetAllowsChangeAfterStopped(t *testing.T) { + remote := v1beta1.DeployModeRemote + stopped := "Stopped" + w := &v1beta1.Worker{ + Spec: v1beta1.WorkerSpec{ + State: &stopped, + DeployMode: &remote, + TargetCluster: &v1beta1.TargetClusterSpec{ID: "cluster-b", Namespace: "agents-b"}, + }, + Status: v1beta1.WorkerStatus{ + Phase: "Stopped", + DeployMode: v1beta1.DeployModeRemote, + TargetCluster: &v1beta1.TargetClusterSpec{ + ID: "cluster-a", + Namespace: "agents-a", + }, + }, + } + + spec, err := effectiveWorkerSpecForTarget(w) + if err != nil { + t.Fatalf("effectiveWorkerSpecForTarget: %v", err) + } + if spec.TargetCluster == nil || spec.TargetCluster.ID != "cluster-b" || spec.TargetCluster.Namespace != "agents-b" { + t.Fatalf("TargetCluster=%+v, want desired cluster-b/agents-b", spec.TargetCluster) + } +} + +func TestWorkerPodRequests_RemoteWatchUsesLocalNamespace(t *testing.T) { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hiclaw-worker-alice", + Namespace: "remote-ns", + Labels: map[string]string{ + "hiclaw.io/worker": "alice", + }, + }, + } + + reqs := workerPodRequests(pod, "local-ns") + if len(reqs) != 1 { + t.Fatalf("requests=%v, want one", reqs) + } + if reqs[0].Name != "alice" || reqs[0].Namespace != "local-ns" { + t.Fatalf("request=%s/%s, want local-ns/alice", reqs[0].Namespace, reqs[0].Name) + } +} + +func TestTeamPodRequests_RemoteWatchUsesLocalNamespace(t *testing.T) { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hiclaw-worker-dev", + Namespace: "remote-ns", + Labels: map[string]string{ + "hiclaw.io/team": "team-a", + }, + }, + } + + reqs := teamPodRequests(pod, "local-ns") + if len(reqs) != 1 { + t.Fatalf("requests=%v, want one", reqs) + } + if reqs[0].Name != "team-a" || reqs[0].Namespace != "local-ns" { + t.Fatalf("request=%s/%s, want local-ns/team-a", reqs[0].Namespace, reqs[0].Name) + } +} diff --git a/hiclaw-controller/internal/credentials/sts_test.go b/hiclaw-controller/internal/credentials/sts_test.go index b0fac88a3..b40658b24 100644 --- a/hiclaw-controller/internal/credentials/sts_test.go +++ b/hiclaw-controller/internal/credentials/sts_test.go @@ -32,6 +32,12 @@ func (f *fakeProvider) Issue(_ context.Context, req credprovider.IssueRequest) ( return f.resp, nil } +// GetKubeconfig is a stub to satisfy the credprovider.Client interface; the +// credentials package tests do not exercise the kubeconfig path. +func (f *fakeProvider) GetKubeconfig(_ context.Context, _ string) (*credprovider.KubeconfigResponse, error) { + return nil, errors.New("not implemented") +} + const ns = "hiclaw" func newFakeK8sClient(t *testing.T, objs ...client.Object) client.Client { diff --git a/hiclaw-controller/internal/credprovider/client.go b/hiclaw-controller/internal/credprovider/client.go index 0f6fbdfc8..0febba9b9 100644 --- a/hiclaw-controller/internal/credprovider/client.go +++ b/hiclaw-controller/internal/credprovider/client.go @@ -18,6 +18,8 @@ type Client interface { // A non-nil error means either the HTTP call failed or the sidecar // returned a non-2xx status. Issue(ctx context.Context, req IssueRequest) (*IssueResponse, error) + // GetKubeconfig obtains a temporary kubeconfig for the given cluster. + GetKubeconfig(ctx context.Context, clusterID string) (*KubeconfigResponse, error) } // HTTPClient is an HTTP implementation of Client that talks to the sidecar diff --git a/hiclaw-controller/internal/credprovider/kubeconfig.go b/hiclaw-controller/internal/credprovider/kubeconfig.go new file mode 100644 index 000000000..d608237eb --- /dev/null +++ b/hiclaw-controller/internal/credprovider/kubeconfig.go @@ -0,0 +1,62 @@ +package credprovider + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "strings" +) + +// KubeconfigRequest is the request body for POST /api/v1/kubernetes/kubeconfig. +type KubeconfigRequest struct { + ClusterID string `json:"clusterId"` +} + +// KubeconfigResponse contains the kubeconfig and its expiration. +type KubeconfigResponse struct { + ClusterID string `json:"clusterId"` + Kubeconfig string `json:"kubeconfig"` + Expiration string `json:"expiration"` // RFC3339 +} + +// GetKubeconfig calls the STS provider to obtain a temporary kubeconfig for the given cluster. +func (c *HTTPClient) GetKubeconfig(ctx context.Context, clusterID string) (*KubeconfigResponse, error) { + if c.baseURL == "" { + return nil, errors.New("credprovider: base URL not configured (HICLAW_CREDENTIAL_PROVIDER_URL)") + } + body, err := json.Marshal(KubeconfigRequest{ClusterID: clusterID}) + if err != nil { + return nil, fmt.Errorf("marshal kubeconfig request: %w", err) + } + httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, + c.baseURL+"/api/v1/kubernetes/kubeconfig", bytes.NewReader(body)) + if err != nil { + return nil, fmt.Errorf("build request: %w", err) + } + httpReq.Header.Set("Content-Type", "application/json") + + resp, err := c.http.Do(httpReq) + if err != nil { + return nil, fmt.Errorf("call credential-provider: %w", err) + } + defer resp.Body.Close() + + respBody, _ := io.ReadAll(resp.Body) + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return nil, fmt.Errorf("credential-provider returned %d: %s", + resp.StatusCode, strings.TrimSpace(string(respBody))) + } + + var out KubeconfigResponse + if err := json.Unmarshal(respBody, &out); err != nil { + return nil, fmt.Errorf("parse kubeconfig response: %w", err) + } + if out.Kubeconfig == "" { + return nil, errors.New("credential-provider returned empty kubeconfig") + } + return &out, nil +} diff --git a/hiclaw-controller/internal/executor/nacos_client_test.go b/hiclaw-controller/internal/executor/nacos_client_test.go index 90214c970..033379d21 100644 --- a/hiclaw-controller/internal/executor/nacos_client_test.go +++ b/hiclaw-controller/internal/executor/nacos_client_test.go @@ -2,6 +2,7 @@ package executor import ( "context" + "fmt" "net/http" "net/http/httptest" "net/url" @@ -100,6 +101,12 @@ func (r *recordingIssueClient) Issue(ctx context.Context, req credprovider.Issue }, nil } +// GetKubeconfig is a stub to satisfy the credprovider.Client interface; this +// test only exercises the STS issue path. +func (r *recordingIssueClient) GetKubeconfig(_ context.Context, _ string) (*credprovider.KubeconfigResponse, error) { + return nil, fmt.Errorf("not implemented") +} + func TestNewNacosAIClient_STS_IssueUsesAIRegistryForNamespace(t *testing.T) { s := httptest.NewServer(http.NotFoundHandler()) t.Cleanup(s.Close) diff --git a/hiclaw-controller/internal/executor/nacos_credential_test.go b/hiclaw-controller/internal/executor/nacos_credential_test.go index 9f8d50268..e2ea1c565 100644 --- a/hiclaw-controller/internal/executor/nacos_credential_test.go +++ b/hiclaw-controller/internal/executor/nacos_credential_test.go @@ -2,6 +2,7 @@ package executor import ( "context" + "fmt" "net/http" "net/http/httptest" "net/url" @@ -73,6 +74,12 @@ func (s stubTokenIssuer) Issue(_ context.Context, _ credprovider.IssueRequest) ( return s.out, nil } +// GetKubeconfig is a stub to satisfy the credprovider.Client interface; the +// Nacos credential tests do not need cluster kubeconfigs. +func (s stubTokenIssuer) GetKubeconfig(context.Context, string) (*credprovider.KubeconfigResponse, error) { + return nil, fmt.Errorf("not implemented") +} + func stubCredForCredentialTest() credprovider.Client { return stubTokenIssuer{ out: &credprovider.IssueResponse{ diff --git a/hiclaw-controller/internal/executor/package_test.go b/hiclaw-controller/internal/executor/package_test.go index 82647e799..40f2c9b2f 100644 --- a/hiclaw-controller/internal/executor/package_test.go +++ b/hiclaw-controller/internal/executor/package_test.go @@ -3,6 +3,7 @@ package executor import ( "archive/zip" "context" + "fmt" "net/http" "net/http/httptest" "net/url" @@ -773,6 +774,12 @@ func (stubCredClient) Issue(ctx context.Context, req credprovider.IssueRequest) Expiration: "2099-01-01T00:00:00Z", }, nil } + +// GetKubeconfig is a stub to satisfy the credprovider.Client interface; this +// test only exercises Nacos Spas signing. +func (stubCredClient) GetKubeconfig(context.Context, string) (*credprovider.KubeconfigResponse, error) { + return nil, fmt.Errorf("not implemented") +} func newNacosAgentSpecCheckTestServer(t *testing.T, listStatus int, listBody string, getStatus int, getBody string) *httptest.Server { t.Helper() diff --git a/hiclaw-controller/internal/remoteclient/cache.go b/hiclaw-controller/internal/remoteclient/cache.go new file mode 100644 index 000000000..472c8e640 --- /dev/null +++ b/hiclaw-controller/internal/remoteclient/cache.go @@ -0,0 +1,395 @@ +package remoteclient + +import ( + "context" + "fmt" + "sync" + "time" + + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" + authenticationv1client "k8s.io/client-go/kubernetes/typed/authentication/v1" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" + ctrlcache "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/handler" + ctrllog "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/source" + + v1beta1 "github.com/hiclaw/hiclaw-controller/api/v1beta1" + "github.com/hiclaw/hiclaw-controller/internal/backend" + "github.com/hiclaw/hiclaw-controller/internal/credprovider" +) + +// maintenanceInterval is the interval between maintenance loop iterations. +const maintenanceInterval = 15 * time.Minute + +// renewThreshold is the remaining time before expiration at which +// the maintenance loop considers renewing a cluster's kubeconfig. +const renewThreshold = 2 * time.Hour + +// remoteCacheSyncTimeout bounds the time spent waiting for a remote +// cache to sync, so that a slow or unreachable cluster does not block +// reconcile indefinitely. +const remoteCacheSyncTimeout = 30 * time.Second + +// ClusterEntry holds a cached remote k8s client and its certificate expiration. +type ClusterEntry struct { + ClusterID string + Client backend.K8sCoreClient + RestConfig *rest.Config // used to start a controller-runtime cache against the remote cluster + Expiration time.Time + mu sync.Mutex + cancel context.CancelFunc // stops the remote cache for this cluster +} + +// CacheConfig holds configuration for creating a Cache. +type CacheConfig struct { + CredClient credprovider.Client + CtrlClient client.Client // controller-runtime client for querying CRs + Scheme *runtime.Scheme + ControllerName string // label filter applied to remote watches: hiclaw.io/controller= +} + +// remoteWatch describes a watch registration that should be applied +// against every remote cluster cache that this Cache opens. +type remoteWatch struct { + ctrl controller.Controller + object client.Object + handler handler.EventHandler + predicates []predicate.Predicate +} + +// Cache manages remote k8s clients keyed by cluster ID. +type Cache struct { + mu sync.RWMutex + entries map[string]*ClusterEntry + credClient credprovider.Client + scheme *runtime.Scheme + ctrlClient client.Client + controllerName string + watches []remoteWatch + logger logr.Logger + // skipHealthCheck disables the /healthz probe in buildEntry. For tests only. + skipHealthCheck bool +} + +// NewCache creates a new remote client cache. +func NewCache(cfg CacheConfig) *Cache { + return &Cache{ + entries: make(map[string]*ClusterEntry), + credClient: cfg.CredClient, + scheme: cfg.Scheme, + ctrlClient: cfg.CtrlClient, + controllerName: cfg.ControllerName, + logger: ctrllog.Log.WithName("remoteclient"), + } +} + +// RegisterWatch stores a watch configuration. It must be called at startup +// BEFORE any remote cluster is connected. When a remote cluster cache is +// created, all registered watches are applied via ctrl.Watch(source.Kind(...)). +func (c *Cache) RegisterWatch(ctrl controller.Controller, obj client.Object, h handler.EventHandler, preds ...predicate.Predicate) { + c.watches = append(c.watches, remoteWatch{ctrl: ctrl, object: obj, handler: h, predicates: preds}) +} + +// GetOrCreate returns the cached client for the cluster, or creates one +// by calling the STS provider. +func (c *Cache) GetOrCreate(ctx context.Context, clusterID string) (*ClusterEntry, error) { + // Fast path: read-lock, return if valid. + c.mu.RLock() + if entry, ok := c.entries[clusterID]; ok && time.Now().Before(entry.Expiration) { + c.mu.RUnlock() + return entry, nil + } + c.mu.RUnlock() + + // Slow path: acquire write lock and double-check. + c.mu.Lock() + defer c.mu.Unlock() + + if entry, ok := c.entries[clusterID]; ok && time.Now().Before(entry.Expiration) { + return entry, nil + } + + c.logger.Info("initializing remote cluster client", "clusterID", clusterID) + entry, err := c.buildEntry(ctx, clusterID) + if err != nil { + c.logger.Error(err, "failed to initialize remote cluster client", "clusterID", clusterID) + return nil, err + } + c.entries[clusterID] = entry + c.logger.Info("remote cluster client ready", "clusterID", clusterID, "expiration", entry.Expiration, "apiServer", entry.RestConfig.Host) + return entry, nil +} + +// Remove deletes a cached cluster entry and stops its informer. +func (c *Cache) Remove(clusterID string) { + c.mu.Lock() + defer c.mu.Unlock() + + if entry, ok := c.entries[clusterID]; ok { + c.logger.Info("removing remote cluster client", "clusterID", clusterID) + if entry.cancel != nil { + entry.cancel() + } + delete(c.entries, clusterID) + } +} + +// StartMaintenanceLoop starts a background goroutine that checks every 15 minutes: +// - For each entry where expiration − now > 2h: skip. +// - For each entry where expiration − now ≤ 2h: +// 1. Query if any Worker/Team CR has targetCluster.id pointing to this cluster. +// 2. If no workers deployed: Remove the entry. +// 3. If workers deployed: call STS to renew kubeconfig, rebuild client, update expiration. +func (c *Cache) StartMaintenanceLoop(ctx context.Context) { + go func() { + ticker := time.NewTicker(maintenanceInterval) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + c.maintain(ctx) + } + } + }() +} + +// maintain runs one maintenance pass over all cached entries. +func (c *Cache) maintain(ctx context.Context) { + // Snapshot current cluster IDs and expirations under read lock. + type snapshot struct { + clusterID string + expiration time.Time + } + c.mu.RLock() + entries := make([]snapshot, 0, len(c.entries)) + for id, e := range c.entries { + entries = append(entries, snapshot{clusterID: id, expiration: e.Expiration}) + } + c.mu.RUnlock() + + now := time.Now() + for _, s := range entries { + remaining := s.expiration.Sub(now) + if remaining > renewThreshold { + continue + } + + deployed, err := c.hasWorkersDeployed(ctx, s.clusterID) + if err != nil { + c.logger.Error(err, "failed to check workers for cluster", "clusterID", s.clusterID) + continue + } + if !deployed { + c.logger.Info("no workers deployed, removing cached client", "clusterID", s.clusterID) + c.Remove(s.clusterID) + continue + } + + // Renew kubeconfig for clusters that still have deployed workers. + if err := c.renewEntry(ctx, s.clusterID); err != nil { + c.logger.Error(err, "failed to renew kubeconfig", "clusterID", s.clusterID) + // Will be retried on next cycle. + } + } +} + +// renewEntry rebuilds the k8s client for the given cluster using a fresh +// kubeconfig from the STS provider. +func (c *Cache) renewEntry(ctx context.Context, clusterID string) error { + c.mu.Lock() + defer c.mu.Unlock() + + entry, ok := c.entries[clusterID] + if !ok { + return nil // removed between snapshot and renewal + } + + c.logger.Info("renewing remote cluster credentials", "clusterID", clusterID) + newEntry, err := c.buildEntry(ctx, clusterID) + if err != nil { + return err + } + + // Stop old informer before replacing. + if entry.cancel != nil { + entry.cancel() + } + + c.entries[clusterID] = newEntry + c.logger.Info("remote cluster credentials renewed", "clusterID", clusterID, "newExpiration", newEntry.Expiration) + return nil +} + +// buildEntry fetches a kubeconfig from the STS provider and constructs a +// ClusterEntry. Caller must hold c.mu (write). +func (c *Cache) buildEntry(ctx context.Context, clusterID string) (*ClusterEntry, error) { + resp, err := c.credClient.GetKubeconfig(ctx, clusterID) + if err != nil { + return nil, fmt.Errorf("get kubeconfig for cluster %s: %w", clusterID, err) + } + + restCfg, err := clientcmd.RESTConfigFromKubeConfig([]byte(resp.Kubeconfig)) + if err != nil { + return nil, fmt.Errorf("parse kubeconfig for cluster %s: %w", clusterID, err) + } + + coreClient, err := corev1client.NewForConfig(restCfg) + if err != nil { + return nil, fmt.Errorf("create core client for cluster %s: %w", clusterID, err) + } + + authClient, err := authenticationv1client.NewForConfig(restCfg) + if err != nil { + return nil, fmt.Errorf("create auth client for cluster %s: %w", clusterID, err) + } + + // Health check: verify remote cluster is reachable before caching. + if !c.skipHealthCheck { + healthCtx, healthCancel := context.WithTimeout(ctx, 10*time.Second) + defer healthCancel() + if _, err := coreClient.RESTClient().Get().AbsPath("/healthz").DoRaw(healthCtx); err != nil { + return nil, fmt.Errorf("health check failed for cluster %s: %w", clusterID, err) + } + } + + expiration, err := time.Parse(time.RFC3339, resp.Expiration) + if err != nil { + return nil, fmt.Errorf("parse expiration for cluster %s: %w", clusterID, err) + } + + // Derive a per-entry context so the remote cache stops when the entry + // is removed or replaced (Remove / renewEntry call entry.cancel). + entryCtx, cancel := context.WithCancel(context.Background()) + entry := &ClusterEntry{ + ClusterID: clusterID, + Client: &k8sCoreClientWrapper{client: coreClient, authClient: authClient}, + RestConfig: restCfg, + Expiration: expiration, + cancel: cancel, + } + + // Start the remote cache and apply registered watches. Failures here + // are non-fatal: remote watches are an enhancement on top of the + // basic client cache, and reconcile will still function via polling. + if err := c.startRemoteCache(entryCtx, restCfg, clusterID); err != nil { + c.logger.Error(err, "failed to start remote cache", "clusterID", clusterID) + } + + return entry, nil +} + +// startRemoteCache starts a controller-runtime cache against the remote +// cluster and applies every registered watch. The cache lifecycle is bound +// to ctx: when ctx is cancelled (via entry.cancel), the cache stops. +func (c *Cache) startRemoteCache(ctx context.Context, restCfg *rest.Config, clusterID string) error { + if len(c.watches) == 0 { + return nil + } + + c.logger.Info("starting remote cache", "clusterID", clusterID, "watches", len(c.watches)) + + labelSelector := labels.SelectorFromSet(labels.Set{ + "hiclaw.io/controller": c.controllerName, + }) + byObject := make(map[client.Object]ctrlcache.ByObject, len(c.watches)) + for _, w := range c.watches { + byObject[w.object] = ctrlcache.ByObject{Label: labelSelector} + } + + remoteCache, err := ctrlcache.New(restCfg, ctrlcache.Options{ByObject: byObject}) + if err != nil { + return fmt.Errorf("create remote cache: %w", err) + } + + go func() { + if err := remoteCache.Start(ctx); err != nil && ctx.Err() == nil { + c.logger.Error(err, "remote cache stopped unexpectedly", "clusterID", clusterID) + } + }() + + // Bound the wait so a slow or unreachable remote cluster does not + // block reconcile for an unbounded amount of time. + syncCtx, cancel := context.WithTimeout(ctx, remoteCacheSyncTimeout) + defer cancel() + if !remoteCache.WaitForCacheSync(syncCtx) { + return fmt.Errorf("remote cache sync failed or timed out after %s", remoteCacheSyncTimeout) + } + c.logger.Info("remote cache synced", "clusterID", clusterID) + + for _, w := range c.watches { + src := source.Kind(remoteCache, w.object, w.handler, w.predicates...) + if err := w.ctrl.Watch(src); err != nil { + return fmt.Errorf("register watch for %T: %w", w.object, err) + } + } + return nil +} + +// hasWorkersDeployed checks whether any Worker CR references the given cluster +// as a remote deployment target. +func (c *Cache) hasWorkersDeployed(ctx context.Context, clusterID string) (bool, error) { + // List Workers with spec.deployMode=Remote and spec.targetCluster.id=clusterID. + var workers v1beta1.WorkerList + if err := c.ctrlClient.List(ctx, &workers); err != nil { + return false, err + } + for _, w := range workers.Items { + if w.Spec.DeployMode != nil && *w.Spec.DeployMode == v1beta1.DeployModeRemote && + w.Spec.TargetCluster != nil && w.Spec.TargetCluster.ID == clusterID { + return true, nil + } + } + + return false, nil +} + +// ResolveClient returns the cached K8sCoreClient for the given cluster ID. +// This implements backend.RemoteClientProvider. +func (c *Cache) ResolveClient(ctx context.Context, clusterID string) (backend.K8sCoreClient, error) { + entry, err := c.GetOrCreate(ctx, clusterID) + if err != nil { + return nil, err + } + return entry.Client, nil +} + +// k8sCoreClientWrapper adapts *corev1client.CoreV1Client to backend.K8sCoreClient. +type k8sCoreClientWrapper struct { + client *corev1client.CoreV1Client + authClient *authenticationv1client.AuthenticationV1Client +} + +func (w *k8sCoreClientWrapper) Pods(namespace string) backend.K8sPodClient { + return w.client.Pods(namespace) +} + +func (w *k8sCoreClientWrapper) ConfigMaps(namespace string) backend.K8sConfigMapClient { + return w.client.ConfigMaps(namespace) +} + +func (w *k8sCoreClientWrapper) Services(namespace string) backend.K8sServiceClient { + return w.client.Services(namespace) +} + +func (w *k8sCoreClientWrapper) Namespaces() backend.K8sNamespaceClient { + return w.client.Namespaces() +} + +func (w *k8sCoreClientWrapper) ServiceAccounts(namespace string) backend.K8sServiceAccountClient { + return w.client.ServiceAccounts(namespace) +} + +func (w *k8sCoreClientWrapper) TokenReviews() backend.K8sTokenReviewClient { + return w.authClient.TokenReviews() +} diff --git a/hiclaw-controller/internal/remoteclient/cache_test.go b/hiclaw-controller/internal/remoteclient/cache_test.go new file mode 100644 index 000000000..b70d85f7e --- /dev/null +++ b/hiclaw-controller/internal/remoteclient/cache_test.go @@ -0,0 +1,358 @@ +package remoteclient + +import ( + "context" + "errors" + "sync/atomic" + "testing" + "time" + + v1beta1 "github.com/hiclaw/hiclaw-controller/api/v1beta1" + "github.com/hiclaw/hiclaw-controller/internal/credprovider" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +// testKubeconfig is a minimal-but-valid kubeconfig that clientcmd accepts +// and from which corev1client.NewForConfig can build a CoreV1Client. It +// never connects to the fake server because the cache tests only inspect +// metadata (entry.Expiration / entry.Client wrapper); no API calls fire. +const testKubeconfig = `apiVersion: v1 +clusters: +- cluster: + server: https://localhost:6443 + insecure-skip-tls-verify: true + name: test +contexts: +- context: + cluster: test + user: test + name: test +current-context: test +kind: Config +users: +- name: test + user: + token: test-token +` + +// fakeCredClient is a stub credprovider.Client whose GetKubeconfig +// returns a configurable response and counts invocations. +type fakeCredClient struct { + kubeconfig string + expiration string + err error + calls int32 +} + +func (f *fakeCredClient) Issue(ctx context.Context, req credprovider.IssueRequest) (*credprovider.IssueResponse, error) { + return nil, errors.New("not implemented") +} + +func (f *fakeCredClient) GetKubeconfig(ctx context.Context, clusterID string) (*credprovider.KubeconfigResponse, error) { + atomic.AddInt32(&f.calls, 1) + if f.err != nil { + return nil, f.err + } + return &credprovider.KubeconfigResponse{ + ClusterID: clusterID, + Kubeconfig: f.kubeconfig, + Expiration: f.expiration, + }, nil +} + +func (f *fakeCredClient) callCount() int { return int(atomic.LoadInt32(&f.calls)) } + +// newTestScheme builds a scheme containing the v1beta1 CRD types so the +// fake controller-runtime client can List Workers/Teams. +func newTestScheme(t *testing.T) *runtime.Scheme { + t.Helper() + s := runtime.NewScheme() + if err := v1beta1.AddToScheme(s); err != nil { + t.Fatalf("AddToScheme: %v", err) + } + return s +} + +// newTestCache returns a Cache wired with the supplied credprovider stub +// and an empty controller-runtime fake client (no Workers/Teams). +func newTestCache(t *testing.T, cred *fakeCredClient) *Cache { + t.Helper() + scheme := newTestScheme(t) + ctrl := fake.NewClientBuilder().WithScheme(scheme).Build() + c := NewCache(CacheConfig{ + CredClient: cred, + CtrlClient: ctrl, + Scheme: scheme, + }) + // Disable /healthz probe so unit tests do not require a reachable API server. + c.skipHealthCheck = true + return c +} + +// futureRFC3339 returns an RFC3339 timestamp d ahead of now. +func futureRFC3339(d time.Duration) string { + return time.Now().Add(d).UTC().Format(time.RFC3339) +} + +func TestCache_GetOrCreate_CreatesAndCaches(t *testing.T) { + cred := &fakeCredClient{ + kubeconfig: testKubeconfig, + expiration: futureRFC3339(24 * time.Hour), + } + c := newTestCache(t, cred) + + entry, err := c.GetOrCreate(context.Background(), "c-1") + if err != nil { + t.Fatalf("GetOrCreate: %v", err) + } + if entry == nil || entry.Client == nil { + t.Fatalf("expected non-nil entry/client") + } + if entry.ClusterID != "c-1" { + t.Errorf("ClusterID = %q, want c-1", entry.ClusterID) + } + if cred.callCount() != 1 { + t.Errorf("cred.GetKubeconfig calls = %d, want 1", cred.callCount()) + } + + // Second call returns cached entry without hitting credprovider. + entry2, err := c.GetOrCreate(context.Background(), "c-1") + if err != nil { + t.Fatalf("GetOrCreate (2nd): %v", err) + } + if entry2 != entry { + t.Errorf("expected same cached entry on second call") + } + if cred.callCount() != 1 { + t.Errorf("cred.GetKubeconfig calls after second GetOrCreate = %d, want 1", cred.callCount()) + } +} + +func TestCache_GetOrCreate_ExpiredTriggersRefresh(t *testing.T) { + cred := &fakeCredClient{ + kubeconfig: testKubeconfig, + expiration: futureRFC3339(24 * time.Hour), + } + c := newTestCache(t, cred) + + // First fetch — caches. + if _, err := c.GetOrCreate(context.Background(), "c-1"); err != nil { + t.Fatalf("initial GetOrCreate: %v", err) + } + + // Force the cached entry to be expired. + c.mu.Lock() + c.entries["c-1"].Expiration = time.Now().Add(-1 * time.Minute) + c.mu.Unlock() + + // Update credprovider to issue a fresh kubeconfig. + cred.expiration = futureRFC3339(48 * time.Hour) + + if _, err := c.GetOrCreate(context.Background(), "c-1"); err != nil { + t.Fatalf("refresh GetOrCreate: %v", err) + } + if cred.callCount() != 2 { + t.Errorf("cred.GetKubeconfig calls = %d, want 2 (refresh)", cred.callCount()) + } +} + +func TestCache_GetOrCreate_PropagatesProviderError(t *testing.T) { + cred := &fakeCredClient{err: errors.New("sts down")} + c := newTestCache(t, cred) + + if _, err := c.GetOrCreate(context.Background(), "c-bad"); err == nil { + t.Fatal("expected error when credprovider fails") + } + // No entry should be cached on failure. + c.mu.RLock() + _, exists := c.entries["c-bad"] + c.mu.RUnlock() + if exists { + t.Error("failed buildEntry must not leave an entry in the cache") + } +} + +func TestCache_Remove(t *testing.T) { + cred := &fakeCredClient{ + kubeconfig: testKubeconfig, + expiration: futureRFC3339(time.Hour), + } + c := newTestCache(t, cred) + if _, err := c.GetOrCreate(context.Background(), "c-1"); err != nil { + t.Fatalf("GetOrCreate: %v", err) + } + + c.Remove("c-1") + + c.mu.RLock() + _, exists := c.entries["c-1"] + c.mu.RUnlock() + if exists { + t.Error("Remove should delete entry") + } + + // Removing an absent key is a no-op (no panic). + c.Remove("never-existed") +} + +func TestCache_ResolveClient_DelegatesToGetOrCreate(t *testing.T) { + cred := &fakeCredClient{ + kubeconfig: testKubeconfig, + expiration: futureRFC3339(time.Hour), + } + c := newTestCache(t, cred) + + cli, err := c.ResolveClient(context.Background(), "c-1") + if err != nil { + t.Fatalf("ResolveClient: %v", err) + } + if cli == nil { + t.Fatal("expected non-nil K8sCoreClient") + } +} + +func TestCache_HasWorkersDeployed_WorkerCR(t *testing.T) { + mode := "Remote" + worker := &v1beta1.Worker{ + ObjectMeta: metav1.ObjectMeta{Name: "w1", Namespace: "default"}, + Spec: v1beta1.WorkerSpec{ + Model: "m", + DeployMode: &mode, + TargetCluster: &v1beta1.TargetClusterSpec{ID: "c-1", Namespace: "ns"}, + }, + } + scheme := newTestScheme(t) + ctrl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(worker).Build() + c := NewCache(CacheConfig{ + CredClient: &fakeCredClient{}, + CtrlClient: ctrl, + Scheme: scheme, + }) + + got, err := c.hasWorkersDeployed(context.Background(), "c-1") + if err != nil { + t.Fatalf("hasWorkersDeployed: %v", err) + } + if !got { + t.Error("expected hasWorkersDeployed=true for matching Worker CR") + } + + got, err = c.hasWorkersDeployed(context.Background(), "c-other") + if err != nil { + t.Fatalf("hasWorkersDeployed (other): %v", err) + } + if got { + t.Error("expected hasWorkersDeployed=false for non-matching cluster") + } +} + +// TestCache_Maintain_RemovesUnused exercises the maintenance loop logic +// directly (without StartMaintenanceLoop) — entries near expiration with +// no referencing Workers must be evicted. +func TestCache_Maintain_RemovesUnused(t *testing.T) { + cred := &fakeCredClient{ + kubeconfig: testKubeconfig, + expiration: futureRFC3339(time.Hour), + } + c := newTestCache(t, cred) + if _, err := c.GetOrCreate(context.Background(), "c-stale"); err != nil { + t.Fatalf("GetOrCreate: %v", err) + } + + // Force expiration to be within the renew threshold so maintain() + // considers it. With no Workers referencing c-stale, the entry must be + // removed. + c.mu.Lock() + c.entries["c-stale"].Expiration = time.Now().Add(30 * time.Minute) + c.mu.Unlock() + + c.maintain(context.Background()) + + c.mu.RLock() + _, exists := c.entries["c-stale"] + c.mu.RUnlock() + if exists { + t.Error("maintain() should remove entries with no referencing Worker CRs") + } +} + +// TestCache_Maintain_RenewsActive verifies that maintain() calls +// GetKubeconfig again to refresh entries that are still referenced by a +// CR but approaching expiration. +func TestCache_Maintain_RenewsActive(t *testing.T) { + cred := &fakeCredClient{ + kubeconfig: testKubeconfig, + expiration: futureRFC3339(time.Hour), + } + mode := "Remote" + worker := &v1beta1.Worker{ + ObjectMeta: metav1.ObjectMeta{Name: "w1", Namespace: "default"}, + Spec: v1beta1.WorkerSpec{ + Model: "m", + DeployMode: &mode, + TargetCluster: &v1beta1.TargetClusterSpec{ID: "c-active", Namespace: "ns"}, + }, + } + scheme := newTestScheme(t) + ctrl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(worker).Build() + c := NewCache(CacheConfig{ + CredClient: cred, + CtrlClient: ctrl, + Scheme: scheme, + }) + // Disable /healthz probe so unit tests do not require a reachable API server. + c.skipHealthCheck = true + + if _, err := c.GetOrCreate(context.Background(), "c-active"); err != nil { + t.Fatalf("GetOrCreate: %v", err) + } + beforeCalls := cred.callCount() + + // Force entry near expiration; renewal should kick in. + c.mu.Lock() + c.entries["c-active"].Expiration = time.Now().Add(30 * time.Minute) + c.mu.Unlock() + cred.expiration = futureRFC3339(48 * time.Hour) + + c.maintain(context.Background()) + + if cred.callCount() != beforeCalls+1 { + t.Errorf("expected renew to trigger one extra GetKubeconfig call, got %d -> %d", + beforeCalls, cred.callCount()) + } + c.mu.RLock() + _, exists := c.entries["c-active"] + c.mu.RUnlock() + if !exists { + t.Error("active entry must remain in the cache after renewal") + } +} + +// TestCache_Maintain_SkipsHealthy verifies that entries far from +// expiration are left untouched. +func TestCache_Maintain_SkipsHealthy(t *testing.T) { + cred := &fakeCredClient{ + kubeconfig: testKubeconfig, + expiration: futureRFC3339(24 * time.Hour), + } + c := newTestCache(t, cred) + if _, err := c.GetOrCreate(context.Background(), "c-healthy"); err != nil { + t.Fatalf("GetOrCreate: %v", err) + } + beforeCalls := cred.callCount() + + c.maintain(context.Background()) + + if cred.callCount() != beforeCalls { + t.Errorf("healthy entry should not trigger any credprovider call, got %d -> %d", + beforeCalls, cred.callCount()) + } + c.mu.RLock() + _, exists := c.entries["c-healthy"] + c.mu.RUnlock() + if !exists { + t.Error("healthy entry must remain in the cache") + } +} diff --git a/hiclaw-controller/internal/remoteclient/cache_watch_test.go b/hiclaw-controller/internal/remoteclient/cache_watch_test.go new file mode 100644 index 000000000..2d1d50a6a --- /dev/null +++ b/hiclaw-controller/internal/remoteclient/cache_watch_test.go @@ -0,0 +1,103 @@ +package remoteclient + +import ( + "context" + "reflect" + "testing" + + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + v1beta1 "github.com/hiclaw/hiclaw-controller/api/v1beta1" +) + +// TestRegisterWatch_StoresConfig verifies that RegisterWatch appends each +// invocation to c.watches in order and preserves the supplied object, +// handler, and predicates. +func TestRegisterWatch_StoresConfig(t *testing.T) { + c := newTestCache(t, &fakeCredClient{}) + + if len(c.watches) != 0 { + t.Fatalf("initial watches len = %d, want 0", len(c.watches)) + } + + worker := &v1beta1.Worker{} + team := &v1beta1.Team{} + h := handler.EnqueueRequestsFromMapFunc(func(_ context.Context, _ client.Object) []reconcile.Request { + return nil + }) + predA := predicate.NewPredicateFuncs(func(_ client.Object) bool { return true }) + predB := predicate.NewPredicateFuncs(func(_ client.Object) bool { return false }) + + // Pass a nil controller.Controller: RegisterWatch only stores values + // and never invokes them, so nil is acceptable here. + c.RegisterWatch(nil, worker, h, predA) + c.RegisterWatch(nil, team, h, predA, predB) + + if got, want := len(c.watches), 2; got != want { + t.Fatalf("watches len = %d, want %d", got, want) + } + + if c.watches[0].object != worker { + t.Errorf("watches[0].object = %T, want *v1beta1.Worker", c.watches[0].object) + } + if c.watches[1].object != team { + t.Errorf("watches[1].object = %T, want *v1beta1.Team", c.watches[1].object) + } + + if len(c.watches[0].predicates) != 1 { + t.Errorf("watches[0].predicates len = %d, want 1", len(c.watches[0].predicates)) + } + if len(c.watches[1].predicates) != 2 { + t.Errorf("watches[1].predicates len = %d, want 2", len(c.watches[1].predicates)) + } + + // Handler identity must be preserved across both registrations. + if !reflect.DeepEqual(c.watches[0].handler, h) { + t.Error("watches[0].handler not preserved") + } + if !reflect.DeepEqual(c.watches[1].handler, h) { + t.Error("watches[1].handler not preserved") + } +} + +// TestStartRemoteCache_NoWatches_Noop verifies that startRemoteCache +// returns nil immediately when no watches have been registered, without +// attempting any connection against the supplied rest.Config. +func TestStartRemoteCache_NoWatches_Noop(t *testing.T) { + c := newTestCache(t, &fakeCredClient{}) + + // Use a bogus REST config: if startRemoteCache were not a true + // no-op, building the cache against this config would fail or hang. + cfg := &rest.Config{Host: "https://invalid.local.test:1"} + + if err := c.startRemoteCache(context.Background(), cfg, "test-cluster"); err != nil { + t.Fatalf("startRemoteCache with no watches must be a no-op, got err: %v", err) + } +} + +// TestClusterEntry_HasRestConfigField guards the public ClusterEntry +// contract: callers (notably the remote cache wiring in buildEntry and +// future external consumers) rely on the RestConfig field being present +// and typed as *rest.Config. +func TestClusterEntry_HasRestConfigField(t *testing.T) { + cfg := &rest.Config{Host: "https://example.test"} + e := &ClusterEntry{RestConfig: cfg} + + if e.RestConfig != cfg { + t.Fatal("ClusterEntry.RestConfig must round-trip the assigned *rest.Config") + } + + // Reflect-level check ensures the field is exported and has the + // expected static type, catching accidental renames or retyping. + ft, ok := reflect.TypeOf(ClusterEntry{}).FieldByName("RestConfig") + if !ok { + t.Fatal("ClusterEntry has no exported RestConfig field") + } + if ft.Type != reflect.TypeOf((*rest.Config)(nil)) { + t.Errorf("ClusterEntry.RestConfig type = %s, want *rest.Config", ft.Type) + } +} diff --git a/hiclaw-controller/internal/service/interfaces.go b/hiclaw-controller/internal/service/interfaces.go index da07d6a31..b72d13b8f 100644 --- a/hiclaw-controller/internal/service/interfaces.go +++ b/hiclaw-controller/internal/service/interfaces.go @@ -17,6 +17,8 @@ type WorkerProvisioner interface { ReconcileExpose(ctx context.Context, workerName string, desired []v1beta1.ExposePort, current []v1beta1.ExposedPortStatus) ([]v1beta1.ExposedPortStatus, error) EnsureServiceAccount(ctx context.Context, workerName string) error DeleteServiceAccount(ctx context.Context, workerName string) error + EnsureRemoteServiceAccount(ctx context.Context, workerName, clusterID, namespace string) error + DeleteRemoteServiceAccount(ctx context.Context, workerName, clusterID, namespace string) error DeleteCredentials(ctx context.Context, workerName string) error DeleteWorkerCredentials(ctx context.Context, credentialName string) error RequestSAToken(ctx context.Context, workerName string) (string, error) diff --git a/hiclaw-controller/internal/service/provisioner.go b/hiclaw-controller/internal/service/provisioner.go index fba3e53c3..7d2232115 100644 --- a/hiclaw-controller/internal/service/provisioner.go +++ b/hiclaw-controller/internal/service/provisioner.go @@ -9,6 +9,7 @@ import ( v1beta1 "github.com/hiclaw/hiclaw-controller/api/v1beta1" authpkg "github.com/hiclaw/hiclaw-controller/internal/auth" + "github.com/hiclaw/hiclaw-controller/internal/backend" "github.com/hiclaw/hiclaw-controller/internal/gateway" "github.com/hiclaw/hiclaw-controller/internal/matrix" "github.com/hiclaw/hiclaw-controller/internal/oss" @@ -129,6 +130,10 @@ type ProvisionerConfig struct { // the manager; otherwise Conduwuit/Tuwunel returns HTTP 403 (it rejects // invites to non-existent local users). ManagerEnabled bool + + // RemoteCache resolves remote cluster clients for cross-cluster SA operations. + // May be nil when remote mode is not configured. + RemoteCache backend.RemoteClientProvider } // Provisioner orchestrates infrastructure provisioning and deprovisioning @@ -148,6 +153,7 @@ type Provisioner struct { adminUser string resourcePrefix authpkg.ResourcePrefix controllerName string + remoteCache backend.RemoteClientProvider managerPassword string managerGatewayKey string @@ -186,6 +192,7 @@ func NewProvisioner(cfg ProvisionerConfig) *Provisioner { managerEnabled: cfg.ManagerEnabled, aiGatewayURL: cfg.AIGatewayURL, managerModel: cfg.ManagerModel, + remoteCache: cfg.RemoteCache, } } diff --git a/hiclaw-controller/internal/service/provisioner_sa.go b/hiclaw-controller/internal/service/provisioner_sa.go index e4102732e..3da5fe36e 100644 --- a/hiclaw-controller/internal/service/provisioner_sa.go +++ b/hiclaw-controller/internal/service/provisioner_sa.go @@ -257,3 +257,78 @@ func (p *Provisioner) RequestSAToken(ctx context.Context, workerName string) (st } return result.Status.Token, nil } + +// EnsureRemoteServiceAccount creates the worker ServiceAccount on the remote +// cluster identified by clusterID. It is idempotent: if the SA already exists, +// it returns nil. +func (p *Provisioner) EnsureRemoteServiceAccount( + ctx context.Context, + workerName, clusterID, namespace string, +) error { + if p.remoteCache == nil { + return fmt.Errorf("remote client provider not configured") + } + + cli, err := p.remoteCache.ResolveClient(ctx, clusterID) + if err != nil { + return fmt.Errorf("resolve remote client for cluster %s: %w", clusterID, err) + } + + // Ensure the target namespace exists on the remote cluster before creating the SA. + if _, err := cli.Namespaces().Get(ctx, namespace, metav1.GetOptions{}); err != nil { + if !apierrors.IsNotFound(err) { + return fmt.Errorf("check remote namespace %s in cluster %s: %w", namespace, clusterID, err) + } + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: namespace}, + } + if _, err := cli.Namespaces().Create(ctx, ns, metav1.CreateOptions{}); err != nil && !apierrors.IsAlreadyExists(err) { + return fmt.Errorf("create remote namespace %s in cluster %s: %w", namespace, clusterID, err) + } + } + + saName := p.resourcePrefix.SAName(authpkg.RoleWorker, workerName) + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: saName, + Namespace: namespace, + Labels: map[string]string{ + "app.kubernetes.io/managed-by": "hiclaw-controller", + "hiclaw.io/role": "worker", + "hiclaw.io/worker": workerName, + }, + }, + } + + if _, err := cli.ServiceAccounts(namespace).Create(ctx, sa, metav1.CreateOptions{}); err != nil { + if apierrors.IsAlreadyExists(err) { + return nil + } + return fmt.Errorf("create remote SA %s in cluster %s namespace %s: %w", saName, clusterID, namespace, err) + } + return nil +} + +// DeleteRemoteServiceAccount deletes the remote worker SA. NotFound is ignored. +func (p *Provisioner) DeleteRemoteServiceAccount( + ctx context.Context, + workerName, clusterID, namespace string, +) error { + if p.remoteCache == nil { + return fmt.Errorf("remote client provider not configured") + } + + cli, err := p.remoteCache.ResolveClient(ctx, clusterID) + if err != nil { + return fmt.Errorf("resolve remote client for cluster %s: %w", clusterID, err) + } + + saName := p.resourcePrefix.SAName(authpkg.RoleWorker, workerName) + if err := cli.ServiceAccounts(namespace).Delete(ctx, saName, metav1.DeleteOptions{}); err != nil { + if apierrors.IsNotFound(err) { + return nil + } + return fmt.Errorf("delete remote SA %s in cluster %s namespace %s: %w", saName, clusterID, namespace, err) + } + return nil +} diff --git a/hiclaw-controller/internal/service/provisioner_sa_test.go b/hiclaw-controller/internal/service/provisioner_sa_test.go index 44b481ec6..83b5d037b 100644 --- a/hiclaw-controller/internal/service/provisioner_sa_test.go +++ b/hiclaw-controller/internal/service/provisioner_sa_test.go @@ -2,11 +2,17 @@ package service import ( "context" + "fmt" "testing" v1beta1 "github.com/hiclaw/hiclaw-controller/api/v1beta1" authpkg "github.com/hiclaw/hiclaw-controller/internal/auth" + "github.com/hiclaw/hiclaw-controller/internal/backend" + authenticationv1 "k8s.io/api/authentication/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" fakeclient "k8s.io/client-go/kubernetes/fake" ) @@ -44,3 +50,173 @@ func TestProvisioner_EnsureServiceAccount_StampsControllerLabel(t *testing.T) { t.Fatalf("manager SA: expected controller label ctl-b, got %q (labels=%v)", got, mgrSA.Labels) } } + +// --- Mock types for remote SA tests --- + +// fakeServiceAccountClient implements backend.K8sServiceAccountClient for testing. +type fakeServiceAccountClient struct { + created map[string]*corev1.ServiceAccount + deleted []string + createErr error + deleteErr error +} + +func newFakeServiceAccountClient() *fakeServiceAccountClient { + return &fakeServiceAccountClient{created: make(map[string]*corev1.ServiceAccount)} +} + +func (f *fakeServiceAccountClient) Get(_ context.Context, name string, _ metav1.GetOptions) (*corev1.ServiceAccount, error) { + if sa, ok := f.created[name]; ok { + return sa, nil + } + return nil, apierrors.NewNotFound(schema.GroupResource{Resource: "serviceaccounts"}, name) +} + +func (f *fakeServiceAccountClient) Create(_ context.Context, sa *corev1.ServiceAccount, _ metav1.CreateOptions) (*corev1.ServiceAccount, error) { + if f.createErr != nil { + return nil, f.createErr + } + f.created[sa.Name] = sa + return sa, nil +} + +func (f *fakeServiceAccountClient) Delete(_ context.Context, name string, _ metav1.DeleteOptions) error { + if f.deleteErr != nil { + return f.deleteErr + } + f.deleted = append(f.deleted, name) + delete(f.created, name) + return nil +} + +type fakeNamespaceClient struct { + created map[string]*corev1.Namespace +} + +func newFakeNamespaceClient() *fakeNamespaceClient { + return &fakeNamespaceClient{created: make(map[string]*corev1.Namespace)} +} + +func (f *fakeNamespaceClient) Get(_ context.Context, name string, _ metav1.GetOptions) (*corev1.Namespace, error) { + if ns, ok := f.created[name]; ok { + return ns, nil + } + return nil, apierrors.NewNotFound(schema.GroupResource{Resource: "namespaces"}, name) +} + +func (f *fakeNamespaceClient) Create(_ context.Context, ns *corev1.Namespace, _ metav1.CreateOptions) (*corev1.Namespace, error) { + f.created[ns.Name] = ns + return ns, nil +} + +// fakeCoreClient implements backend.K8sCoreClient for testing. +type fakeCoreClient struct { + saClient *fakeServiceAccountClient + nsClient *fakeNamespaceClient +} + +func (f *fakeCoreClient) Pods(_ string) backend.K8sPodClient { return nil } +func (f *fakeCoreClient) ConfigMaps(_ string) backend.K8sConfigMapClient { return nil } +func (f *fakeCoreClient) Services(_ string) backend.K8sServiceClient { return nil } +func (f *fakeCoreClient) Namespaces() backend.K8sNamespaceClient { return f.nsClient } +func (f *fakeCoreClient) ServiceAccounts(_ string) backend.K8sServiceAccountClient { return f.saClient } +func (f *fakeCoreClient) TokenReviews() backend.K8sTokenReviewClient { return nil } + +// fakeRemoteClientProvider implements backend.RemoteClientProvider for testing. +type fakeRemoteClientProvider struct { + clients map[string]backend.K8sCoreClient +} + +func (f *fakeRemoteClientProvider) ResolveClient(_ context.Context, clusterID string) (backend.K8sCoreClient, error) { + if cli, ok := f.clients[clusterID]; ok { + return cli, nil + } + return nil, fmt.Errorf("cluster %q not found", clusterID) +} + +// --- Remote SA tests --- + +func TestEnsureRemoteServiceAccount(t *testing.T) { + saClient := newFakeServiceAccountClient() + coreClient := &fakeCoreClient{saClient: saClient, nsClient: newFakeNamespaceClient()} + remoteProvider := &fakeRemoteClientProvider{ + clients: map[string]backend.K8sCoreClient{"cluster-a": coreClient}, + } + + p := NewProvisioner(ProvisionerConfig{ + Namespace: "hiclaw", + ResourcePrefix: authpkg.ResourcePrefix("hiclaw-"), + RemoteCache: remoteProvider, + }) + + // Test successful creation. + if err := p.EnsureRemoteServiceAccount(context.Background(), "bob", "cluster-a", "remote-ns"); err != nil { + t.Fatalf("EnsureRemoteServiceAccount: %v", err) + } + if _, ok := saClient.created["hiclaw-worker-bob"]; !ok { + t.Fatal("expected SA hiclaw-worker-bob to be created") + } + + // Test idempotency: AlreadyExists returns nil. + saClient.createErr = apierrors.NewAlreadyExists(schema.GroupResource{Resource: "serviceaccounts"}, "hiclaw-worker-bob") + if err := p.EnsureRemoteServiceAccount(context.Background(), "bob", "cluster-a", "remote-ns"); err != nil { + t.Fatalf("expected nil on AlreadyExists, got: %v", err) + } + + // Test error when remote cache is nil. + p2 := NewProvisioner(ProvisionerConfig{ + Namespace: "hiclaw", + ResourcePrefix: authpkg.ResourcePrefix("hiclaw-"), + }) + if err := p2.EnsureRemoteServiceAccount(context.Background(), "bob", "cluster-a", "remote-ns"); err == nil { + t.Fatal("expected error when remoteCache is nil") + } + + // Test error when cluster not found. + if err := p.EnsureRemoteServiceAccount(context.Background(), "bob", "unknown-cluster", "ns"); err == nil { + t.Fatal("expected error for unknown cluster") + } +} + +func TestDeleteRemoteServiceAccount(t *testing.T) { + saClient := newFakeServiceAccountClient() + saClient.created["hiclaw-worker-bob"] = &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: "hiclaw-worker-bob"}, + } + coreClient := &fakeCoreClient{saClient: saClient} + remoteProvider := &fakeRemoteClientProvider{ + clients: map[string]backend.K8sCoreClient{"cluster-a": coreClient}, + } + + p := NewProvisioner(ProvisionerConfig{ + Namespace: "hiclaw", + ResourcePrefix: authpkg.ResourcePrefix("hiclaw-"), + RemoteCache: remoteProvider, + }) + + // Test successful deletion. + if err := p.DeleteRemoteServiceAccount(context.Background(), "bob", "cluster-a", "remote-ns"); err != nil { + t.Fatalf("DeleteRemoteServiceAccount: %v", err) + } + if len(saClient.deleted) != 1 || saClient.deleted[0] != "hiclaw-worker-bob" { + t.Fatalf("expected hiclaw-worker-bob in deleted list, got %v", saClient.deleted) + } + + // Test idempotency: NotFound returns nil. + saClient.deleteErr = apierrors.NewNotFound(schema.GroupResource{Resource: "serviceaccounts"}, "hiclaw-worker-bob") + if err := p.DeleteRemoteServiceAccount(context.Background(), "bob", "cluster-a", "remote-ns"); err != nil { + t.Fatalf("expected nil on NotFound, got: %v", err) + } + + // Test error when remote cache is nil. + p2 := NewProvisioner(ProvisionerConfig{ + Namespace: "hiclaw", + ResourcePrefix: authpkg.ResourcePrefix("hiclaw-"), + }) + if err := p2.DeleteRemoteServiceAccount(context.Background(), "bob", "cluster-a", "remote-ns"); err == nil { + t.Fatal("expected error when remoteCache is nil") + } +} + +// Ensure unused imports are used. +var _ = authenticationv1.TokenReview{} diff --git a/hiclaw-controller/test/integration/controller/worker_test.go b/hiclaw-controller/test/integration/controller/worker_test.go index 2d861256c..3e45e01f5 100644 --- a/hiclaw-controller/test/integration/controller/worker_test.go +++ b/hiclaw-controller/test/integration/controller/worker_test.go @@ -354,9 +354,7 @@ func TestWorkerPodDeleted_Recreates(t *testing.T) { if err := k8sClient.Create(ctx, worker); err != nil { t.Fatalf("failed to create Worker CR: %v", err) } - t.Cleanup(func() { - _ = k8sClient.Delete(ctx, worker) - }) + t.Cleanup(func() { _ = deleteWorkerAndWait(t, worker) }) waitForRunning(t, worker) @@ -374,14 +372,8 @@ func TestWorkerPodDeleted_Recreates(t *testing.T) { return nil }) - // Phase should still be Running after recreation - var w v1beta1.Worker - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(worker), &w); err != nil { - t.Fatalf("failed to get Worker: %v", err) - } - if w.Status.Phase != "Running" { - t.Errorf("phase=%q after pod recreation, want Running", w.Status.Phase) - } + // Phase should converge back to Running after recreation. + waitForRunning(t, worker) } // --------------------------------------------------------------------------- @@ -397,9 +389,7 @@ func TestWorkerCreate_Idempotent_NoDoubleProvision(t *testing.T) { if err := k8sClient.Create(ctx, worker); err != nil { t.Fatalf("failed to create Worker CR: %v", err) } - t.Cleanup(func() { - _ = k8sClient.Delete(ctx, worker) - }) + t.Cleanup(func() { _ = deleteWorkerAndWait(t, worker) }) waitForRunning(t, worker) @@ -971,3 +961,20 @@ func assertEventually(t *testing.T, condFn func() error) { } t.Fatalf("condition not met within %v: %v", timeout, lastErr) } + +func deleteWorkerAndWait(t *testing.T, worker *v1beta1.Worker) error { + t.Helper() + if err := k8sClient.Delete(ctx, worker); err != nil { + return client.IgnoreNotFound(err) + } + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + var got v1beta1.Worker + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(worker), &got) + if err != nil { + return client.IgnoreNotFound(err) + } + time.Sleep(interval) + } + return fmt.Errorf("worker %s not deleted within timeout", worker.Name) +} diff --git a/hiclaw-controller/test/testutil/mocks/provisioner.go b/hiclaw-controller/test/testutil/mocks/provisioner.go index ff6e8eb89..5cc7b3b6e 100644 --- a/hiclaw-controller/test/testutil/mocks/provisioner.go +++ b/hiclaw-controller/test/testutil/mocks/provisioner.go @@ -12,24 +12,26 @@ import ( type MockProvisioner struct { mu sync.Mutex - ProvisionWorkerFn func(ctx context.Context, req service.WorkerProvisionRequest) (*service.WorkerProvisionResult, error) - DeprovisionWorkerFn func(ctx context.Context, req service.WorkerDeprovisionRequest) error - RefreshCredentialsFn func(ctx context.Context, workerName string) (*service.RefreshResult, error) - RefreshWorkerCredentialsFn func(ctx context.Context, credentialName, workerName string) (*service.RefreshResult, error) - EnsureWorkerGatewayAuthFn func(ctx context.Context, workerName, gatewayKey string) error - ReconcileExposeFn func(ctx context.Context, workerName string, desired []v1beta1.ExposePort, current []v1beta1.ExposedPortStatus) ([]v1beta1.ExposedPortStatus, error) - EnsureServiceAccountFn func(ctx context.Context, workerName string) error - DeleteServiceAccountFn func(ctx context.Context, workerName string) error - DeleteCredentialsFn func(ctx context.Context, workerName string) error - DeleteWorkerCredentialsFn func(ctx context.Context, credentialName string) error - RequestSATokenFn func(ctx context.Context, workerName string) (string, error) - LeaveAllWorkerRoomsFn func(ctx context.Context, workerName string) error - DeleteWorkerRoomFn func(ctx context.Context, roomID string) error - MatrixUserIDFn func(name string) string - LoginAsHumanFn func(ctx context.Context, username, password string) (string, error) - ProvisionTeamRoomsFn func(ctx context.Context, req service.TeamRoomRequest) (*service.TeamRoomResult, error) - DeleteTeamRoomAliasesFn func(ctx context.Context, teamName, leaderName string) error - DeleteWorkerRoomAliasFn func(ctx context.Context, workerName string) error + ProvisionWorkerFn func(ctx context.Context, req service.WorkerProvisionRequest) (*service.WorkerProvisionResult, error) + DeprovisionWorkerFn func(ctx context.Context, req service.WorkerDeprovisionRequest) error + RefreshCredentialsFn func(ctx context.Context, workerName string) (*service.RefreshResult, error) + RefreshWorkerCredentialsFn func(ctx context.Context, credentialName, workerName string) (*service.RefreshResult, error) + EnsureWorkerGatewayAuthFn func(ctx context.Context, workerName, gatewayKey string) error + ReconcileExposeFn func(ctx context.Context, workerName string, desired []v1beta1.ExposePort, current []v1beta1.ExposedPortStatus) ([]v1beta1.ExposedPortStatus, error) + EnsureServiceAccountFn func(ctx context.Context, workerName string) error + DeleteServiceAccountFn func(ctx context.Context, workerName string) error + EnsureRemoteServiceAccountFn func(ctx context.Context, workerName, clusterID, namespace string) error + DeleteRemoteServiceAccountFn func(ctx context.Context, workerName, clusterID, namespace string) error + DeleteCredentialsFn func(ctx context.Context, workerName string) error + DeleteWorkerCredentialsFn func(ctx context.Context, credentialName string) error + RequestSATokenFn func(ctx context.Context, workerName string) (string, error) + LeaveAllWorkerRoomsFn func(ctx context.Context, workerName string) error + DeleteWorkerRoomFn func(ctx context.Context, roomID string) error + MatrixUserIDFn func(name string) string + LoginAsHumanFn func(ctx context.Context, username, password string) (string, error) + ProvisionTeamRoomsFn func(ctx context.Context, req service.TeamRoomRequest) (*service.TeamRoomResult, error) + DeleteTeamRoomAliasesFn func(ctx context.Context, teamName, leaderName string) error + DeleteWorkerRoomAliasFn func(ctx context.Context, workerName string) error Calls struct { ProvisionWorker []service.WorkerProvisionRequest @@ -40,6 +42,8 @@ type MockProvisioner struct { ReconcileExpose []string EnsureServiceAccount []string DeleteServiceAccount []string + EnsureRemoteSA []string + DeleteRemoteSA []string DeleteCredentials []string DeleteWorkerCredentials []string RequestSAToken []string @@ -84,6 +88,8 @@ func (m *MockProvisioner) Reset() { m.ReconcileExposeFn = nil m.EnsureServiceAccountFn = nil m.DeleteServiceAccountFn = nil + m.EnsureRemoteServiceAccountFn = nil + m.DeleteRemoteServiceAccountFn = nil m.DeleteCredentialsFn = nil m.DeleteWorkerCredentialsFn = nil m.RequestSATokenFn = nil @@ -113,6 +119,8 @@ func (m *MockProvisioner) clearCallsLocked() { ReconcileExpose []string EnsureServiceAccount []string DeleteServiceAccount []string + EnsureRemoteSA []string + DeleteRemoteSA []string DeleteCredentials []string DeleteWorkerCredentials []string RequestSAToken []string @@ -236,6 +244,28 @@ func (m *MockProvisioner) DeleteServiceAccount(ctx context.Context, workerName s return nil } +func (m *MockProvisioner) EnsureRemoteServiceAccount(ctx context.Context, workerName, clusterID, namespace string) error { + m.mu.Lock() + m.Calls.EnsureRemoteSA = append(m.Calls.EnsureRemoteSA, workerName) + fn := m.EnsureRemoteServiceAccountFn + m.mu.Unlock() + if fn != nil { + return fn(ctx, workerName, clusterID, namespace) + } + return nil +} + +func (m *MockProvisioner) DeleteRemoteServiceAccount(ctx context.Context, workerName, clusterID, namespace string) error { + m.mu.Lock() + m.Calls.DeleteRemoteSA = append(m.Calls.DeleteRemoteSA, workerName) + fn := m.DeleteRemoteServiceAccountFn + m.mu.Unlock() + if fn != nil { + return fn(ctx, workerName, clusterID, namespace) + } + return nil +} + func (m *MockProvisioner) DeleteCredentials(ctx context.Context, workerName string) error { m.mu.Lock() m.Calls.DeleteCredentials = append(m.Calls.DeleteCredentials, workerName)