feat(ha): mirror worker credentials (Secrets/RBAC) to the Standby - #416
Open
sumanthd032 wants to merge 20 commits into
Open
feat(ha): mirror worker credentials (Secrets/RBAC) to the Standby#416sumanthd032 wants to merge 20 commits into
sumanthd032 wants to merge 20 commits into
Conversation
Introduce pkg/ha, the foundation for cross-cluster (Active/Standby) high availability per ADR kubeslice#293 (issue kubeslice#294). - HAMode (active|standby|standalone) with fail-safe parsing: empty or unknown input maps to standalone so a misconfig never disables writes. - Lease helpers over coordination.k8s.io/v1: acquire/renew (bumping leaseTransitions on takeover), get, and a leaseDuration+padding staleness check. - ClusterLeaderElector: IsLeader() reads an atomic flag kept current by background loops, so it is cheap enough to call at the top of every Reconcile and always reflects live leadership. StartLeaseRenewal (Active) renews the local Lease and releases leadership once the renew deadline is exceeded (natural fencing). WatchRemoteLease (Standby) reads the Active's Lease and logs staleness but does not promote; promotion is issue kubeslice#297. Standalone is the default and is always the leader, preserving today's single-hub behaviour (no regression). Unit tests are race-clean and cover leadership by mode, renew-deadline loss, lease staleness, and the standby-never-promotes boundary. vendor: add controller-runtime fake client + interceptor packages (test-only) via go mod vendor. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Wire pkg/ha into the controller so only the Active hub writes (issue kubeslice#294). - Add a LeaderElector field to all nine reconcilers and a per-call guard at the top of every Reconcile: a Standby logs "standby mode, skipping reconcile" and returns without writing. The guard is nil-safe, so a reconciler built without an elector keeps today's behaviour. - main.go: add --ha-mode, --ha-identity, --ha-active-kubeconfig, --ha-lease-duration, --ha-renew-deadline, --ha-retry-period and --ha-padding-seconds; construct the elector (building a remote client from the mounted Active kubeconfig in standby mode); start StartLeaseRenewal (active) or WatchRemoteLease (standby) and pass the shared signal-handler context to the manager. - Add coordination.k8s.io/leases RBAC for the Lease. --ha-mode=standalone is the default and is always the leader, so existing single-hub deployments are unaffected (no regression). The existing --leader-elect (in-cluster pod election) is left untouched. A controller test asserts the Standby skips and logs on every call. vendor: add go.uber.org/zap/zaptest/observer (test-only). Signed-off-by: Sumanth D <sumanthd032@gmail.com>
The HA leader-election Lease lives in the controller's own namespace and is already covered by the existing leader-election Role's leases grant (config/rbac/leader_election_role.yaml). The kubebuilder marker added a redundant cluster-wide grant and left the generated manifests out of sync (make manifests was not run). Replace it with a note pointing at the role that provides the permission, keeping markers and manifests consistent. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
…hutdown - acquireOrRenewLease rounds LeaseDurationSeconds up to whole seconds and clamps to a minimum of 1, so a sub-second --ha-lease-duration is not truncated to 0 (invalid, and skews staleness checks). - StartLeaseRenewal and WatchRemoteLease return nil instead of ctx.Err() on context cancellation, so a graceful shutdown is not logged as an error. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
The Lease namespace defaulted to a hard-coded constant, but the controller runs in a namespace injected at runtime via KUBESLICE_CONTROLLER_MANAGER_NAMESPACE (downward API), and the leader-election Role that grants leases is namespaced to that deploy namespace. Deploying into any other namespace would create the Lease where the controller has no leases permission, so the Active could not renew it and would fence itself permanently. Add --ha-lease-namespace defaulting to KUBESLICE_CONTROLLER_MANAGER_NAMESPACE so the Lease lands in the controller's own namespace. Empty (local runs) falls back to the pkg/ha default. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
NewClusterLeaderElector fell back to the hard-coded DefaultLeaseNamespace whenever LeaseNamespace was empty, independent of main.go's flag default. Since the controller already exposes KUBESLICE_CONTROLLER_MANAGER_NAMESPACE to represent its actual runtime namespace, check that env var first so the package resolves correctly on its own, not only by accident of how main.go wires the --ha-lease-namespace flag default. Only falls back to DefaultLeaseNamespace when the env var is unset too (e.g. running outside a pod). Regression tests included. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
StartLeaseRenewal/WatchRemoteLease returning nil (not ctx.Err()) on context cancellation was fixed in 0da6d82 but never actually got a regression test. Also add coverage for: the mode-guard no-op branches, WatchRemoteLease failing fast without a remote client, getLease/ checkRemoteLeaseOnce propagating a missing-Lease error instead of reporting fresh, renewOnce keeping leadership on a transient failure within renewDeadline, and setLeader logging Acquired/Lost exactly once per transition (F4 in 294-evaluation.md). No production code changes. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
…uard, ownerRef strip, status mirroring) Part of kubeslice#295 — the write path RemoteSyncer's workqueue will drive (next commit) to mirror KubeSlice CRDs from the Active hub onto the Standby. - MirroredResource + CRDMirrorSet (pkg/ha/mirror_set.go): the hub-side resource table (Project, Cluster, SliceConfig, ServiceExportConfig, SliceQoSConfig, VpnKeyRotation, WorkerSliceConfig, WorkerSliceGateway, WorkerServiceImport, plus core Namespace). Deliberately does not match issue kubeslice#295's own CRD table, which names Slice/SliceGateway/ ServiceExport — none of those types exist in this repo; they are worker-cluster data-plane CRDs owned by the separate worker-operator repo, irrelevant to hub-to-hub mirroring. - mirrorCreateOrUpdate/mirrorDelete (pkg/ha/mirror.go): strip resourceVersion/uid/managedFields/finalizers (+ ownerReferences only for VpnKeyRotation, the one mirrored type that carries one) before writing, label mirrored objects ha.kubeslice.io/synced-from=active, and only ever overwrite or delete a target object that already carries that label — the conflict guard that keeps the syncer off anything the Standby's own reconcilers or an operator created directly, including pre-existing namespaces like kube-system/default now that Namespace is an ordinary mirrored type rather than a special-cased cold-start step. - Every mirrored type has a status subresource. A plain Update() never touches .status once one is registered, so mirrorCreateOrUpdate always follows up with an explicit Status().Update() when the source object has a non-empty status — matching this repo's own UpdateStatus/CleanupUpdateStatus convention. Regression-tested. Fake-client unit tests cover create, update-of-existing, the conflict guard on both update and delete, delete-idempotent-on-NotFound, StripOwnerRefs true/false, and status mirroring. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Part of kubeslice#295. Registered via prometheus.NewHistogramVec/NewCounterVec + prometheus.MustRegister in pkg/ha's own init(), the same self-registration idiom metrics/prometheus.go already uses for KubeSliceEventsCounter — not routed through metrics.StartMetricsCollector, whose default labels are slice-specific and don't apply to a cross-cluster mirror. - ha_sync_lag_seconds{kind,operation}: time.Now() minus CreationTimestamp for creates; minus first-enqueue time for update/delete (RemoteSyncer's workqueue coalesces repeated events for the same object, so there's no single "delivery time" once a retry has backed off a few times — first-enqueue is the more useful number to alert on, since it reflects total time since the triggering change). - ha_sync_errors_total{kind,operation}: counts mirror failures. The syncer keeps running and retries via its workqueue on every increment; this metric never indicates a crash. vendor: add prometheus/client_golang/prometheus/testutil (test-only) via go mod vendor, used to assert metric samples/labels directly. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Part of kubeslice#295. RemoteSyncer runs only in standby mode: builds a controller-runtime cache.Cache against the Active hub's rest.Config, registers one informer per CRDMirrorSet entry, and starts a small worker pool draining a rate-limited workqueue.TypedRateLimitingInterface [syncKey] — the same primitive controller-runtime's own Controller uses internally (internal/controller.Controller). Informer callbacks (handlersFor) only enqueue a syncKey; no mirror logic runs on the informer's own goroutine. A burst of Update events for the same object coalesces into one queued item, and a worker determines the real action at dequeue time by re-reading the Active cache (found -> mirrorCreateOrUpdate, NotFound -> mirrorDelete) — the same way a Reconcile call would. This is the commit that satisfies issue kubeslice#295's own acceptance criterion that the syncer "retries without crashing": on any mirror failure, processOnce calls queue.AddRateLimited instead of dropping the key. The concrete failure mode this fixes: a namespaced object (e.g. a new Project's Cluster) created on Active after the Standby's initial sync has already completed previously had no path to retry — nothing fires again for an object that didn't change on Active once its first mirror attempt failed on "namespace not found". Now it just gets retried a few seconds later once the Namespace mirror (an ordinary CRDMirrorSet row, no special-casing needed) has landed. Start(ctx) delegates its blocking wait directly to remoteCache.Start(ctx), which itself blocks on <-ctx.Done() and returns nil — giving the "return nil, not ctx.Err(), on graceful shutdown" contract StartLeaseRenewal/WatchRemoteLease already use, for free. remoteGetFunc is a small seam (defaulted to a real cache.Cache.Get in the constructor) so the retry engine's tests exercise real workqueue backoff/redelivery behaviour without a *rest.Config or live cluster. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Part of kubeslice#295. Constructs ha.RemoteSyncer alongside the existing ClusterLeaderElector, reusing the same remote *rest.Config and local client the elector already builds rather than loading the Active kubeconfig twice — hoisted the standby block's remoteCfg to an outer remoteHACfg variable so both consumers can see it. Starts remoteSyncer.Start(ctx) in its own goroutine alongside the existing leaderElector.WatchRemoteLease(ctx) in the ha.ModeStandby switch case; a no-op in any other mode, matching RemoteSyncer's own mode check. New flag: --ha-sync-workers (default ha.DefaultSyncWorkers), matching the existing --ha-* flag style. --ha-sync-interval is intentionally not added yet — it belongs to the periodic prune backstop (pkg/ha/ prune.go), which is out of scope for this PR and would otherwise be a flag with no consumer. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
…mote read identity Part of kubeslice#295. --ha-active-kubeconfig's RBAC scope on the Active cluster has been undefined since kubeslice#294 introduced the flag — the dev demo uses a full-admin kubeconfig, and config/rbac/leader_election_role.yaml only ever granted configmaps/leases, nothing for the CRD/Namespace reads RemoteSyncer now needs. The ADR (kubeslice#293) itself names this as an unaddressed deployment-level boundary without resolving it. config/ha/active-cluster-clusterrole.yaml: a read-only (get/list/watch) ClusterRole covering Namespace plus every pkg/ha.CRDMirrorSet type, plus a ClusterRoleBinding template (subject left as a placeholder — it's deployment-specific, either a ServiceAccount for in-cluster dialing or a client-cert User for a flattened kubeconfig, and can't be hardcoded). config/ha/README.md explains this must be applied on the Active cluster by whoever provisions it, not through this repo's own deploy/kustomize flow — confirmed neither config/rbac/kustomization.yaml nor config/default/kustomization.yaml reference this directory, so it can never be accidentally auto-applied to the Standby's own cluster, where it would be meaningless. Also documents, ahead of time, that a later credential-mirroring PR appending Secrets to this grant exposes every Secret in the project namespaces on Active, not just the ones actually mirrored — RBAC can't scope Secrets by .type — a real tradeoff to weigh when that lands. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Start gave up permanently on the first GetInformer/AddEventHandler error instead of retrying, unlike StartLeaseRenewal/WatchRemoteLease. Split setup into registerInformers (retries with backoff) wrapping registerInformersOnce (one attempt). A naive retry would double-register handlers on resources that already succeeded, since AddEventHandlerWithResyncPeriod isn't idempotent. Added a handlerRegistered map so retries skip resources already done. CRDMirrorSet's Namespace entry had no filter, so it mirrored every namespace on the Active hub, not just kubeslice ones — and mirrorDelete's label-only guard meant an unrelated Active-side delete could cascade-delete one of those on the Standby. Scoped the Namespace informer to util.LabelsKubeSliceController via cache.Options.ByObject. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
An Active-side object mid-Terminating (deletionTimestamp set, contents still being garbage-collected) is delivered by the informer as an ordinary Update, not a Delete yet. mirrorCreateOrUpdate's payload carried deletionTimestamp straight through, and updating the Standby's non-terminating mirror with it set fails real API server immutable- field validation. Stripping it surfaced a second, related failure: the status-mirror block was still copying a Terminating status onto that now-non-terminating payload, which a real server also rejects (status.Phase may only be Terminating if deletionTimestamp is set). Both are stripped now: deletionTimestamp/deletionGracePeriodSeconds unconditionally alongside the other identity fields already cleared there, and status explicitly via delete(payload.Object, "status") rather than relying on a real API server silently ignoring .status on the main resource endpoint for subresource-registered types — a fake client does not replicate that behavior, so the code's correctness would otherwise depend on which client it's running against. The Standby still converges correctly once Active reports NotFound and mirrorDelete takes over; there's nothing useful to reflect about the in-between Terminating state. New tests: TestMirrorCreateOrUpdate_StripsDeletionTimestampFromTerminatingSource, TestMirrorCreateOrUpdate_SkipsStatusMirrorWhenSourceIsTerminating. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Dockerfile never copied pkg/ into the build context, so no image has been buildable since pkg/ha was introduced — go build works directly, docker build did not. Added COPY pkg/ pkg/ alongside the other source directories. config/rbac/role.yaml never granted namespaces/status: every other CRDMirrorSet type already has an explicit <kind>/status rule, but Namespace never needed one before it became a mirrored type with explicit status mirroring. Under real RBAC this permanently fails the Namespace status write. Added the +kubebuilder:rbac marker in main.go next to the existing namespaces rule and regenerated via make manifests. config/ha/active-cluster-clusterrole.yaml never granted coordination.k8s.io/leases: it was scoped only to what RemoteSyncer itself reads (Namespace + CRDMirrorSet), but the same --ha-active-kubeconfig identity is also used by WatchRemoteLease to read the Active's own Lease directly. Under a least-privilege identity using exactly this sample, the Standby could mirror correctly but never observe Lease staleness at all. Added the leases rule and updated the README to describe both consumers of this grant. Signed-off-by: Sumanth D <sumanthd032@gmail.com>
Informers self-heal missed updates via periodic resync and the workqueue
owns retry-on-failure, but neither can remove a mirror whose Active-side
original was deleted while the Standby wasn't watching (e.g. between two
Standby runs): cold-start informers only deliver what currently exists,
so such an orphan would survive forever.
Add a prune loop to RemoteSyncer that periodically lists Standby objects
carrying the ha.kubeslice.io/synced-from label per mirrored type, diffs
them against the remote cache, and enqueues anything no longer present
on the Active hub onto the ordinary mirror workqueue. The worker re-reads
Active at dequeue time, so the existing conflict guard, NotFound->delete
semantics, and rate-limited retry apply unchanged, and no second write
path races the workers.
Fail-safe choices:
- The first pass waits for the remote cache to sync; an unsynced cache
lists empty, which would otherwise read as "everything was deleted"
and prune every mirror on the Standby.
- A failed list (remote or local) skips that kind for the round and
increments ha_sync_errors_total{kind,"prune"} instead of pruning on
partial information.
Configurable via --ha-sync-interval (default 60s).
Part of kubeslice#295
Signed-off-by: Sumanth D <sumanthd032@gmail.com>
…lure Surface mirror failures as Kubernetes events on the Standby, attached to the object that failed to sync, using the EventRecorder main.go already builds for the reconcilers. The entry lives in config/events/controller.yaml with the generated map and config-map output from make generate-events committed alongside — RecordEvent hard-fails for any EventName missing from the generated EventsMap, so skipping that step would silently no-op the whole feature. A regression test pins the entry's presence in the generated map (and that a missing entry errors loudly), so an accidental revert of the generated code fails in go test rather than at runtime. One event per failure episode, not per retry attempt: NumRequeues is 0 only on the first failure since the last success, and early workqueue backoff retries arrive milliseconds apart — although the recorder aggregates repeats into one Event's Count, every call is still an API-server write. ha_sync_errors_total continues to count every attempt. The recorder is called directly rather than through util.RecordEvent: that helper logs via util.CtxLogger, which panics on any context that didn't pass through a reconciler's request-context setup — true for the syncer's own context (main.go's signal-handler context). Caught by the new event-emission test before it could crash a live Standby. Part of kubeslice#295 Signed-off-by: Sumanth D <sumanthd032@gmail.com>
…Account, Role, RoleBinding)
Mirror the credential objects a promoted Standby needs to serve its
worker clusters without manual re-provisioning: worker-identity RBAC
(Role/RoleBinding/ServiceAccount — the only RBAC kinds
access_control_service.go ever creates; no ClusterRole or
ClusterRoleBinding exists to mirror, despite ADR Decision 6's broader
wording) and Secrets such as the gateway certificates the ovpn job
generates.
kubernetes.io/service-account-token Secrets are excluded: SA tokens are
signed by the issuing cluster's service-account key, so an Active-minted
token is cryptographically invalid on the Standby. Mirroring the
ServiceAccount is what matters — the Standby's own token controller
mints a locally-valid token for it.
Unlike the CRD set, these are core types that exist cluster-wide, so
every row is scoped in two layers:
- Server-side, the remote informers are scoped in cache.Options:
ServiceAccount/Role/RoleBinding by the same
util.LabelsKubeSliceController selector the Namespace informer already
uses (util.GetOwnerLabel embeds that exact key/value pair on every
credential object the controller creates), and Secret — which cannot
be label-scoped, the cert-generator job creates its Secrets unlabeled
— by a field selector excluding the SA-token type.
- Client-side, every row sets RequireMirroredNamespace: the object only
mirrors if its namespace is in the remote cache's label-scoped
Namespace view, i.e. a namespace the syncer itself mirrors. The
boundary is deliberately NOT name-based: under the Helm chart's
real-world --project-namespace-prefix ("kubeslice-"), the
controller's own kubeslice-controller namespace matches the
project-namespace naming pattern, and a prefix rule would have
mirrored its webhook TLS key, image-pull credentials, and Helm
release Secrets onto the Standby — found by running the Standby
against a Helm-installed Active hub. The label boundary is the one
ReconcileProjectNamespace actually maintains. A transient failure
reading the namespace surfaces as an error (workqueue retry), never
as a silent skip.
StripOwnerRefs is set on every row: ownerReferences resolve by UID,
which never survives a cross-cluster copy, and credential objects are
also written by actors outside this repo (token controller, cert job),
so the CRD set's audited only-VpnKeyRotation-needs-it reasoning cannot
hold here.
Verified live against the real two-hub Kind topology: worker SAs,
Roles, RoleBindings, and dashboard Secrets in both project namespaces
mirror with the sync label; SA-token Secrets and everything in
kubeslice-controller and kube-system stay off the Standby.
Part of kubeslice#295
Signed-off-by: Sumanth D <sumanthd032@gmail.com>
The prune loop so far only walked one direction — Standby mirrors whose Active-side original disappeared. Walk the other direction too: an Active-side object with no Standby mirror gets re-enqueued onto the ordinary workqueue, where the worker re-reads Active and runs the full Skip/namespace/conflict-guard chain. Three real cases produce that state, none of which the forward pass or the informers handle promptly: - a mirror someone deleted directly on the Standby (previously healed only by the informer's resync period, default 10 minutes); - an object whose RequireMirroredNamespace verdict was decided before its namespace's informer had delivered on cold start — a skip is terminal for that queue item, so without this it also waited for resync; - a key stuck deep in rate-limiter backoff after repeated failures (observed live: a fixed failure cause still took minutes to heal because the next retry was scheduled ~164s out; enqueue bypasses the rate limiter's delay, so recovery lands within one prune tick). Re-enqueueing is always safe: objects that should not mirror simply no-op through the guards again. Verified live: a mirrored Secret deleted directly on the Standby was re-created 15s later, within one --ha-sync-interval, with zero errors logged. Part of kubeslice#295 Signed-off-by: Sumanth D <sumanthd032@gmail.com>
…C sample The Standby now mirrors ha.FullMirrorSet — CRDMirrorSet plus CredentialMirrorSet. No new configuration: the mirrored-namespace gate derives entirely from the label boundary the controller already maintains, so the syncer needs no knowledge of the deployment's --project-namespace-prefix. config/ha/active-cluster-clusterrole.yaml gains read access to secrets/serviceaccounts (core) and roles/rolebindings (rbac), and the README's forward-looking credential-mirroring note becomes present-tense documentation. The Secret grant's security tradeoff is stated where it will be read rather than buried: RBAC cannot scope Secrets by .type or namespace label, and a ClusterRole binding is cluster-wide, so the Standby's identity can read every Secret on the Active hub — the syncer's field selector and mirrored-namespace gate narrow what gets copied, not what the identity could read. The narrower per-namespace RoleBinding alternative is documented alongside, with its maintenance cost. Grant coverage verified live via an impersonation can-i matrix: get/list/watch allowed for every mirrored type plus the HA Lease, writes and unrelated resources denied. Fixes kubeslice#295 Signed-off-by: Sumanth D <sumanthd032@gmail.com>
7 tasks
There was a problem hiding this comment.
Pull request overview
This PR extends the HA Standby “state mirror” so a promoted Standby can serve worker clusters without manual credential re-provisioning. It adds credential mirroring (Secrets + worker-identity RBAC), a reverse-diff anti-entropy pass to recreate missing mirrors, and wires HA leader fencing + the full mirror set into the controller startup flow.
Changes:
- Add
CredentialMirrorSetand wireha.FullMirrorSet()into the Standby mirror pipeline (including Secret filtering + namespace gating). - Add reverse-diff anti-entropy in the prune loop to re-enqueue Active objects missing locally, plus metrics and event emission for sync failures.
- Wire HA mode/lease configuration into
main.go, extend RBAC for status mirroring, and vendor in new test/runtime dependencies.
Reviewed changes
Copilot reviewed 35 out of 53 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| vendor/sigs.k8s.io/controller-runtime/pkg/internal/objectutil/objectutil.go | Vendored controller-runtime internal helper used by dependencies/tests. |
| vendor/sigs.k8s.io/controller-runtime/pkg/client/interceptor/intercept.go | Vendored controller-runtime interceptor client support. |
| vendor/sigs.k8s.io/controller-runtime/pkg/client/fake/doc.go | Vendored controller-runtime fake client docs. |
| vendor/modules.txt | Vendor manifest updates for newly vendored packages. |
| vendor/k8s.io/apimachinery/pkg/util/rand/rand.go | Vendored apimachinery rand utilities. |
| vendor/go.uber.org/zap/zaptest/observer/observer.go | Vendored zap test observer. |
| vendor/go.uber.org/zap/zaptest/observer/logged_entry.go | Vendored zap test observer support type. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/testutil.go | Vendored Prometheus test utilities. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/promlint/validations/units.go | Vendored Prometheus metric linting validation. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/promlint/validations/histogram_validations.go | Vendored Prometheus metric linting validation. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/promlint/validations/help_validations.go | Vendored Prometheus metric linting validation. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/promlint/validations/generic_name_validations.go | Vendored Prometheus metric linting validation. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/promlint/validations/counter_validations.go | Vendored Prometheus metric linting validation. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/promlint/validation.go | Vendored Prometheus lint validation wiring. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/promlint/promlint.go | Vendored Prometheus metric linter. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/promlint/problem.go | Vendored Prometheus lint problem type. |
| vendor/github.com/prometheus/client_golang/prometheus/testutil/lint.go | Vendored Prometheus testutil lint entrypoints. |
| pkg/ha/remote_syncer.go | Implements Standby remote cache + workqueue mirroring for full mirror set (CRDs + credentials). |
| pkg/ha/remote_syncer_test.go | Unit tests for remote syncer workqueue + informer registration retry semantics. |
| pkg/ha/prune.go | Anti-entropy prune loop (forward prune + reverse diff re-enqueue). |
| pkg/ha/prune_test.go | Unit tests for prune forward/backward diff behavior and cache-sync safety. |
| pkg/ha/mode.go | HA mode parsing and validation. |
| pkg/ha/mode_test.go | Unit tests for HA mode parsing/validation. |
| pkg/ha/mirror.go | Core mirror create/update/delete logic with conflict guard + status handling. |
| pkg/ha/mirror_test.go | Unit tests for mirror conflict guard, ownerRef stripping, status mirroring, and termination handling. |
| pkg/ha/mirror_set.go | Defines CRD + credential mirror sets and helper filters. |
| pkg/ha/metrics.go | Adds HA sync lag/error Prometheus metrics and registers them. |
| pkg/ha/metrics_test.go | Tests metric collection/labeling via Prometheus testutil. |
| pkg/ha/lease.go | Lease acquire/renew and staleness checks for HA leadership coordination. |
| pkg/ha/lease_test.go | Unit tests for lease lifecycle and timing edge cases. |
| pkg/ha/leader_elector.go | Active/Standby leader elector with natural fencing and remote lease watch. |
| pkg/ha/leader_elector_test.go | Unit tests for leader elector behaviors and logging transitions. |
| pkg/ha/events_test.go | Tests “HAMirrorSyncFailed” event emission semantics and generated-map registration. |
| pkg/ha/credential_set_test.go | Tests credential mirror set scoping, SA-token exclusion, and cache selector behavior. |
| main.go | Wires HA mode flags, leader elector, remote syncer startup, and adds RBAC markers. |
| events/events_generated.go | Registers new HAMirrorSyncFailed event in generated event map. |
| Dockerfile | Includes pkg/ in build context for new HA package. |
| controllers/worker/workerslicegateway_controller.go | Adds HA write fence (leader gating) to prevent Standby writes. |
| controllers/worker/workersliceconfig_controller.go | Adds HA write fence (leader gating) to prevent Standby writes. |
| controllers/worker/workerserviceimport_controller.go | Adds HA write fence (leader gating) to prevent Standby writes. |
| controllers/controller/vpnkey_rotation_controller.go | Adds HA write fence (leader gating) to prevent Standby writes. |
| controllers/controller/sliceqosconfig_controller.go | Adds HA write fence (leader gating) to prevent Standby writes. |
| controllers/controller/sliceconfig_controller.go | Adds HA write fence (leader gating) to prevent Standby writes. |
| controllers/controller/serviceexportconfig_controller.go | Adds HA write fence (leader gating) to prevent Standby writes. |
| controllers/controller/project_controller.go | Adds HA write fence (leader gating) to prevent Standby writes. |
| controllers/controller/cluster_controller.go | Adds HA write fence (leader gating) to prevent Standby writes. |
| controllers/controller/leader_gate_test.go | Verifies Standby reconcile no-op behavior for leader fence. |
| config/rbac/role.yaml | Adds namespaces/status permissions needed for status mirroring. |
| config/ha/README.md | Documents Active-cluster RBAC for Standby remote reads and Secret-read tradeoff. |
| config/ha/active-cluster-clusterrole.yaml | Provides template ClusterRole(+Binding) to grant Standby read access on Active. |
| config/events/events_config_map.yaml | Enables the new HA mirror failure event in the config map. |
| config/events/controller.yaml | Adds HAMirrorSyncFailed event schema for generation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+83
to
+86
| // with a field selector excluding SA-token Secrets (the API server | ||
| // supports "type" as a Secret field selector), the highest-volume | ||
| // class; the project-namespace boundary stays client-side in | ||
| // CredentialMirrorSet's Skip predicate. |
Comment on lines
+125
to
+129
| for key := range activeKeys { | ||
| if _, mirrored := localKeys[key]; !mirrored { | ||
| s.enqueue(key) | ||
| } | ||
| } |
| flag.DurationVar(&haRetryPeriod, "ha-retry-period", ha.DefaultRetryPeriod, "Interval between Lease renew/watch attempts.") | ||
| flag.DurationVar(&haPaddingSeconds, "ha-padding-seconds", ha.DefaultPaddingSeconds, "Extra buffer a Standby waits before treating the Active Lease as stale.") | ||
| flag.IntVar(&haSyncWorkers, "ha-sync-workers", ha.DefaultSyncWorkers, "Number of workers draining the Standby's remote-mirror workqueue.") | ||
| flag.DurationVar(&haSyncInterval, "ha-sync-interval", ha.DefaultPruneInterval, "How often the Standby prunes mirrored objects that no longer exist on the Active hub.") |
Comment on lines
+57
to
+60
| if t.LeaderElector != nil && !t.LeaderElector.IsLeader() { | ||
| t.Log.Info("standby mode, skipping reconcile") | ||
| return ctrl.Result{}, nil | ||
| } |
Comment on lines
+57
to
+60
| if c.LeaderElector != nil && !c.LeaderElector.IsLeader() { | ||
| c.Log.Info("standby mode, skipping reconcile") | ||
| return ctrl.Result{}, nil | ||
| } |
Comment on lines
+58
to
+61
| if r.LeaderElector != nil && !r.LeaderElector.IsLeader() { | ||
| r.Log.Info("standby mode, skipping reconcile") | ||
| return ctrl.Result{}, nil | ||
| } |
Comment on lines
+51
to
+54
| if r.LeaderElector != nil && !r.LeaderElector.IsLeader() { | ||
| r.Log.Info("standby mode, skipping reconcile") | ||
| return ctrl.Result{}, nil | ||
| } |
Comment on lines
+58
to
+61
| if c.LeaderElector != nil && !c.LeaderElector.IsLeader() { | ||
| c.Log.Info("standby mode, skipping reconcile") | ||
| return ctrl.Result{}, nil | ||
| } |
Comment on lines
+51
to
+54
| if r.LeaderElector != nil && !r.LeaderElector.IsLeader() { | ||
| r.Log.Info("standby mode, skipping reconcile") | ||
| return ctrl.Result{}, nil | ||
| } |
Comment on lines
+38
to
+41
| core, logs := observer.New(zapcore.InfoLevel) | ||
| logger := zap.New(core).Sugar() | ||
|
|
||
| standby := ha.NewClusterLeaderElector(nil, nil, ha.Options{ |
pnavali
requested review from
Rahul-D78,
gourishkb and
rajendra-avesha
and
a lite review from Copilot
and removed request for
that-backend-guy
August 18, 2026 15:08
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 53 changed files in this pull request and generated no new comments.
Suppressed comments (6)
pkg/ha/remote_syncer.go:1
- The SPDX identifier line appears malformed (
# # SPDX-License-Identifier: ...) and may not be detected by license scanners. Update the header comment format to a standard SPDX form (and apply consistently across the newly added files) so automated tooling can correctly recognize the license.
pkg/ha/remote_syncer.go:1 time.Afterallocates a new timer on every retry iteration. For a long-lived retry loop (especially during prolonged Active-hub outages), this creates unnecessary allocations/timers. Prefer using a reusabletime.Timer(reset each loop) ortime.Sleepwith a ctx check to reduce overhead.
pkg/ha/remote_syncer.go:1- In standby mode,
schemeis effectively required for watching/caching the mirrored types (especially CRDs). Consider explicitly validatingscheme != niland returning a clear error when it’s missing, to avoid harder-to-debug runtime behavior where the cache cannot construct informers for expected types.
controllers/controller/project_controller.go:58 - Logging an Info message on every skipped reconcile in standby mode can create very high log volume (especially for frequently-updating resources), which can increase costs and make real issues harder to find. Consider lowering this to debug-level, sampling/rate-limiting it, or logging only on leadership transitions instead of per reconcile.
// HA write fence: only the Active hub (or a standalone controller) writes.
// A Standby evaluates this on every call and no-ops.
if t.LeaderElector != nil && !t.LeaderElector.IsLeader() {
t.Log.Info("standby mode, skipping reconcile")
return ctrl.Result{}, nil
}
controllers/controller/project_controller.go:60
- The HA write-fence block is duplicated across multiple reconcilers in this PR. To reduce the chance of future drift (e.g., changing log level/message or adding metrics), consider factoring this into a small shared helper (or an embedded struct method) used by all reconcilers.
if t.LeaderElector != nil && !t.LeaderElector.IsLeader() {
t.Log.Info("standby mode, skipping reconcile")
return ctrl.Result{}, nil
}
config/ha/active-cluster-clusterrole.yaml:82
- This manifest grants cluster-wide
get/list/watchon Secrets, which is a significant privilege escalation if the--ha-active-kubeconfigcredential is compromised. Since this file is meant as a template, consider splitting the Secret rule into an explicitly opt-in variant (separate file or clearly commented block), and/or add a concrete example of the per-namespace RoleBinding approach in the README to make the safer deployment path easier to follow.
# Credential mirroring (pkg/ha.CredentialMirrorSet). SECURITY TRADEOFF,
# named rather than buried: RBAC cannot scope Secret access by .type or by
# namespace *label*, and a ClusterRole+ClusterRoleBinding grants reads
# CLUSTER-WIDE — so this rule lets the Standby's identity read every
# Secret on the Active hub, not just the gateway certificates RemoteSyncer
# actually mirrors (the syncer's own field selector and mirrored-namespace
# gate narrow what is COPIED, but not what this identity COULD read).
# Treat the --ha-active-kubeconfig credential accordingly. To narrow the
# grant itself, replace this rule with per-namespace RoleBindings in each
# kubeslice project namespace — at the cost of maintaining them as
# projects come and go, which is cross-cluster provisioning tooling this
# repo deliberately does not ship.
- apiGroups:
- ""
resources:
- secrets
- serviceaccounts
verbs:
- get
- list
- watch
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds
CredentialMirrorSet: mirrors worker RBAC (ServiceAccount/Role/RoleBinding) and Secrets (e.g. gateway certs) to the Standby, so a promoted hub can serve workers without manual re-provisioning. Service-account token Secrets are excluded (a token minted by the Active is invalid on the Standby).Also adds a reverse-diff anti-entropy pass: an Active-side object with no Standby mirror gets re-enqueued, covering mirrors deleted by hand or missed during namespace sync.
Wires
ha.FullMirrorSetintomain.goand extends the sampleClusterRolewith the needed read grants.Stacked on #415. Only 3 commits are new here.
Fixes #295
How Has This Been Tested?
go build,go vet,gofmtclean.go test -race -count=1 ./pkg/ha/.... Verified live on a two-hub Kind topology: RBAC and Secrets mirror correctly, token Secrets and controller-internal namespaces stay excluded.Checklist:
Does this PR introduce a breaking change for other components like worker-operator?
No. Additive mirroring on the Standby side only.