diff --git a/.fossa.yml b/.fossa.yml new file mode 100644 index 00000000..cb23eca0 --- /dev/null +++ b/.fossa.yml @@ -0,0 +1,5 @@ +version: 3 + +paths: + exclude: + - external-sources \ No newline at end of file diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 51a0f5c9..c7adb6b0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -779,10 +779,10 @@ jobs: k3d_agent_count: "0" - name: full-core script: "task test-e2e" - # `bi-directional` moved out to its own leg (it needs an Argo CD - # install that `task prepare-e2e` does not perform), so this filter is - # now a strict subset of what it used to run. - e2e_label_filter: "!manager && !image-refresh && !bi-directional" + # `bi-directional` and `source-cluster` moved out to their own legs (they + # install Argo CD / kcp that `task prepare-e2e` does not perform), so this + # filter is now a strict subset of what it used to run. + e2e_label_filter: "!manager && !image-refresh && !bi-directional && !source-cluster" e2e_report_name: "full-core" needs_artifact: false coverage: "1" @@ -805,6 +805,23 @@ jobs: needs_artifact: false coverage: "1" k3d_agent_count: "0" + # The source-cluster corner: the only leg that installs kcp, and the only + # place gitops-reverser mirrors REMOTE clusters (GitTarget.spec.kubeConfig). + # kcp workspaces are cheap logical clusters, so its own runner installs a + # small kcp control plane and provisions three workspaces as source clusters. + # docs/design/config-plane-split.md. + # + # `task test-e2e-source-cluster` pins its own label filter and Ginkgo procs + # (SOURCE_CLUSTER_GINKGO_PROCS=1 — the specs share one kcp port-forward), so + # e2e_label_filter and e2e_ginkgo_procs are deliberately absent here. It uses + # the default config-dir install, so it carries the GOCOVERDIR overlay and its + # coverage unions with the others'. + - name: source-cluster + script: "task test-e2e-source-cluster" + e2e_report_name: "source-cluster" + needs_artifact: false + coverage: "1" + k3d_agent_count: "0" # Quickstart chain, sharded across two runners (was the single # `quickstart` lane). quickstart-install runs the two install-mode # validations on one cluster (cleanup-installs.sh resets the namespace diff --git a/api/v1alpha3/gittarget_types.go b/api/v1alpha3/gittarget_types.go index ec3a5fb1..dc5cf337 100644 --- a/api/v1alpha3/gittarget_types.go +++ b/api/v1alpha3/gittarget_types.go @@ -3,6 +3,7 @@ package v1alpha3 import ( + meta "github.com/fluxcd/pkg/apis/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -45,6 +46,20 @@ type GitProviderReference struct { // +kubebuilder:validation:XValidation:rule="self.providerRef == oldSelf.providerRef",message="spec.providerRef is immutable; delete and recreate the GitTarget to change its destination" // +kubebuilder:validation:XValidation:rule="self.branch == oldSelf.branch",message="spec.branch is immutable; delete and recreate the GitTarget to change its destination" // +kubebuilder:validation:XValidation:rule="self.path == oldSelf.path",message="spec.path is immutable; delete and recreate the GitTarget to change its destination" +// +// spec.kubeConfig is immutable — the source of a folder's content is destination identity, like +// providerRef/branch/path above. Delete and recreate to change the cluster a GitTarget mirrors. +// +kubebuilder:validation:XValidation:rule="has(self.kubeConfig) == has(oldSelf.kubeConfig) && (!has(self.kubeConfig) || self.kubeConfig == oldSelf.kubeConfig)",message="spec.kubeConfig is immutable; delete and recreate the GitTarget to change the cluster it mirrors" +// +// configMapRef (provider / workload-identity auth) is present in meta.KubeConfigReference's schema +// but not yet implemented here; reject it at admission so the v1alpha3 contract is "secretRef only". +// Deleting this one rule, plus wiring the provider path in the resolver, is the whole future enablement. +// +kubebuilder:validation:XValidation:rule="!has(self.kubeConfig) || !has(self.kubeConfig.configMapRef)",message="spec.kubeConfig.configMapRef (provider auth) is not yet supported; use secretRef" +// +// secretRef.name comes from the external meta.KubeConfigReference schema, which marks it required but +// permits the empty string; an empty name is meaningless (it can never resolve a Secret), so reject it +// at admission rather than letting it surface later as a Validated=False "Secret not found". +// +kubebuilder:validation:XValidation:rule="!has(self.kubeConfig) || !has(self.kubeConfig.secretRef) || size(self.kubeConfig.secretRef.name) > 0",message="spec.kubeConfig.secretRef.name must not be empty" type GitTargetSpec struct { // ProviderRef references the GitProvider that backs this target. // Immutable: delete and recreate the GitTarget to change its destination. @@ -80,6 +95,19 @@ type GitTargetSpec struct { // change only affects resources created after the change. // +optional Placement *GitTargetPlacementSpec `json:"placement,omitempty"` + + // KubeConfig names the SOURCE CLUSTER this GitTarget mirrors FROM: the kubeconfig + // determines both the cluster and the credentials to reach it. Omitted means the cluster + // the operator runs in, the single-cluster default that behaves exactly as before. Its + // Secret is read from the GitTarget's OWN namespace, on the cluster the operator runs in — + // the credential for a cluster never has to live on that cluster. When SecretRef.Key is + // empty the resolver reads "value" then "value.yaml" (Flux's order). Immutable: the source + // of a folder's content is part of what the folder means; delete and recreate to change it. + // Only kubeConfig.secretRef is honored in v1alpha3 (configMapRef is rejected at admission); + // unsafe kubeconfigs (exec auth providers, insecure-skip-tls-verify) are rejected by the + // controller with a legible Validated=False reason unless the operator opts in via flags. + // +optional + KubeConfig *meta.KubeConfigReference `json:"kubeConfig,omitempty"` } // GitTargetPlacementSpec declares where NEW resources are written when no document @@ -180,6 +208,8 @@ type GitTargetStreamsStatus struct { // +kubebuilder:printcolumn:name="GitPathAccepted",type=string,JSONPath=`.status.conditions[?(@.type=="GitPathAccepted")].status`,priority=1 // +kubebuilder:printcolumn:name="RenderMatchesLive",type=string,JSONPath=`.status.conditions[?(@.type=="RenderMatchesLive")].status`,priority=1 // +kubebuilder:printcolumn:name="StreamsRunning",type=string,JSONPath=`.status.conditions[?(@.type=="StreamsRunning")].status`,priority=1 +// +kubebuilder:printcolumn:name="SourceReachable",type=string,JSONPath=`.status.conditions[?(@.type=="SourceClusterReachable")].reason`,priority=1 +// +kubebuilder:printcolumn:name="ProviderReady",type=string,JSONPath=`.status.conditions[?(@.type=="GitProviderReady")].status`,priority=1 // +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].message`,priority=1 // +kubebuilder:printcolumn:name="Encryption",type=string,JSONPath=`.spec.encryption.provider`,priority=1 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` @@ -201,6 +231,24 @@ type GitTarget struct { Status GitTargetStatus `json:"status,omitempty,omitzero"` } +// SourceClusterID renders the identity the watch data plane keys a GitTarget's source +// cluster on: "//", where namespace/name locate the kubeconfig Secret +// in the GitTarget's own (config-plane) namespace and key is the SecretRef key AS WRITTEN in +// spec — an empty key is its own identity, distinct from an explicit one, because the +// resolver's value→value.yaml fallback only runs when the key is omitted. A GitTarget with no +// spec.kubeConfig (or no secretRef) mirrors the cluster the operator runs in and returns "", +// the local-cluster id every source-cluster-unaware code path already lands on. +// +// Neither a namespace, a Secret name, nor a Secret data key may contain "/", so the three +// segments are unambiguous; the resolver splits them back with SplitN(id, "/", 3). +func (g *GitTarget) SourceClusterID() string { + if g.Spec.KubeConfig == nil || g.Spec.KubeConfig.SecretRef == nil { + return "" + } + ref := g.Spec.KubeConfig.SecretRef + return g.Namespace + "/" + ref.Name + "/" + ref.Key +} + // +kubebuilder:object:root=true // GitTargetList contains a list of GitTarget. diff --git a/api/v1alpha3/zz_generated.deepcopy.go b/api/v1alpha3/zz_generated.deepcopy.go index 86340c09..70363237 100644 --- a/api/v1alpha3/zz_generated.deepcopy.go +++ b/api/v1alpha3/zz_generated.deepcopy.go @@ -7,6 +7,7 @@ package v1alpha3 import ( + "github.com/fluxcd/pkg/apis/meta" "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -616,6 +617,11 @@ func (in *GitTargetSpec) DeepCopyInto(out *GitTargetSpec) { *out = new(GitTargetPlacementSpec) (*in).DeepCopyInto(*out) } + if in.KubeConfig != nil { + in, out := &in.KubeConfig, &out.KubeConfig + *out = new(meta.KubeConfigReference) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GitTargetSpec. diff --git a/cmd/main.go b/cmd/main.go index fdf8cc23..ad721c6f 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -40,6 +40,7 @@ import ( configbutleraiv1alpha3 "github.com/ConfigButler/gitops-reverser/api/v1alpha3" "github.com/ConfigButler/gitops-reverser/internal/controller" "github.com/ConfigButler/gitops-reverser/internal/git" + "github.com/ConfigButler/gitops-reverser/internal/kubeconfig" "github.com/ConfigButler/gitops-reverser/internal/queue" "github.com/ConfigButler/gitops-reverser/internal/rulestore" "github.com/ConfigButler/gitops-reverser/internal/telemetry" @@ -64,6 +65,11 @@ const ( defaultAuditIdleTimeout = 60 * time.Second defaultAuditShutdownTimeout = 10 * time.Second defaultBranchBufferMaxSizeStr = "8Mi" + // defaultSourceClusterQPS / -Burst are the client-side throttle for a remote source + // cluster reached via GitTarget.spec.kubeConfig — a conservative default since a remote is + // reached over a network the in-cluster config is not, and is only read (list/watch/get). + defaultSourceClusterQPS = 20.0 + defaultSourceClusterBurst = 30 ) func init() { @@ -137,6 +143,11 @@ func main() { RuleStore: ruleStore, EventRouter: nil, // Will be set below SensitiveResources: cfg.sensitiveResources, + // Resolve a GitTarget.spec.kubeConfig into a rest.Config by reading its Secret from the + // config plane. The manager client bypasses its cache for Secrets, so a rotated + // kubeconfig is seen without a Secret informer. + SourceClusters: watch.NewSecretSourceClusterResolver( + mgr.GetClient(), cfg.kubeConfigSafety, float32(cfg.sourceClusterQPS), cfg.sourceClusterBurst), } // Initialize EventRouter with all dependencies. The streaming-snapshot resync @@ -154,8 +165,12 @@ func main() { // Inject the live followability registry into the writer, so a GVR-only DELETE // event resolves to a manifest moved off its canonical path (M6 in the writer). - // The registry is a stable pointer the watch manager refreshes in place. + // The registry is a stable pointer the watch manager refreshes in place. SetMapper is + // the LOCAL cluster's resolver; SetClusterMapper gives the writer each SOURCE cluster's + // registry so a folder mirroring a remote resolves its documents' GVK->GVR against that + // remote — never a union of all clusters. workerManager.SetMapper(watchMgr.TypeRegistry()) + workerManager.SetClusterMapper(watchMgr.ClusterTypeLookup) // Give the workers a way to surface a refused live write plan. Live events are committed // off a timer with no result channel, so without this a refusal (acceptance gate or a @@ -257,10 +272,11 @@ func main() { os.Exit(1) } if err := (&controller.GitTargetReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - WorkerManager: workerManager, - EventRouter: eventRouter, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + WorkerManager: workerManager, + EventRouter: eventRouter, + KubeConfigSafety: cfg.kubeConfigSafety, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "GitTarget") os.Exit(1) @@ -357,7 +373,16 @@ type appConfig struct { branchBufferMaxBytes int64 sensitiveResources types.SensitiveResourcePolicy sshHostKeys git.SSHHostKeyConfig - zapOpts zap.Options + // sourceClusterQPS / sourceClusterBurst bound the rate at which the operator talks to a + // source cluster reached through a GitTarget.spec.kubeConfig. A remote is reached over a + // network the in-cluster config is not, so it carries client-side throttling by default. + sourceClusterQPS float64 + sourceClusterBurst int + // kubeConfigSafety is the exec / insecure-TLS opt-in for source-cluster kubeconfigs. Both + // default OFF: an operator-supplied kubeconfig is attacker-adjacent input, so unsafe + // kubeconfigs are REJECTED (a legible Validated=False), diverging from Flux's silent strip. + kubeConfigSafety kubeconfig.SafetyPolicy + zapOpts zap.Options } // parseFlags parses CLI flags and returns the application configuration. @@ -470,6 +495,16 @@ func parseFlagsWithArgs(fs *flag.FlagSet, args []string) (appConfig, error) { "", "Comma-separated additional sensitive resources in resource or group/resource form.", ) + fs.Float64Var(&cfg.sourceClusterQPS, "source-cluster-qps", defaultSourceClusterQPS, + "Client-side QPS limit for talking to a source cluster reached via GitTarget.spec.kubeConfig.") + fs.IntVar(&cfg.sourceClusterBurst, "source-cluster-burst", defaultSourceClusterBurst, + "Client-side burst limit for talking to a source cluster reached via GitTarget.spec.kubeConfig.") + fs.BoolVar(&cfg.kubeConfigSafety.AllowExec, "insecure-kubeconfig-exec", false, + "Allow a source-cluster kubeconfig to use an exec auth provider (runs a binary in the "+ + "operator Pod). Rejected by default; enabling this is a deliberate trust decision.") + fs.BoolVar(&cfg.kubeConfigSafety.AllowInsecureTLS, "insecure-kubeconfig-tls", false, + "Allow a source-cluster kubeconfig to set insecure-skip-tls-verify (disables server cert "+ + "validation). Rejected by default; enabling this is a deliberate trust decision.") fs.StringVar(&cfg.sshHostKeys.DefaultKnownHostsConfigMap, "default-known-hosts-configmap", "", "Optional install-level ConfigMap (in the controller's namespace) supplying SSH known_hosts "+ "for Git hosts when neither the credentials Secret nor the GitProvider's knownHostsRef does.") diff --git a/config/crd/bases/configbutler.ai_gittargets.yaml b/config/crd/bases/configbutler.ai_gittargets.yaml index 0bc29913..d0a7c698 100644 --- a/config/crd/bases/configbutler.ai_gittargets.yaml +++ b/config/crd/bases/configbutler.ai_gittargets.yaml @@ -45,6 +45,14 @@ spec: name: StreamsRunning priority: 1 type: string + - jsonPath: .status.conditions[?(@.type=="SourceClusterReachable")].reason + name: SourceReachable + priority: 1 + type: string + - jsonPath: .status.conditions[?(@.type=="GitProviderReady")].status + name: ProviderReady + priority: 1 + type: string - jsonPath: .status.conditions[?(@.type=="Ready")].message name: Status priority: 1 @@ -154,6 +162,86 @@ spec: required: - provider type: object + kubeConfig: + description: |- + KubeConfig names the SOURCE CLUSTER this GitTarget mirrors FROM: the kubeconfig + determines both the cluster and the credentials to reach it. Omitted means the cluster + the operator runs in, the single-cluster default that behaves exactly as before. Its + Secret is read from the GitTarget's OWN namespace, on the cluster the operator runs in — + the credential for a cluster never has to live on that cluster. When SecretRef.Key is + empty the resolver reads "value" then "value.yaml" (Flux's order). Immutable: the source + of a folder's content is part of what the folder means; delete and recreate to change it. + Only kubeConfig.secretRef is honored in v1alpha3 (configMapRef is rejected at admission); + unsafe kubeconfigs (exec auth providers, insecure-skip-tls-verify) are rejected by the + controller with a legible Validated=False reason unless the operator opts in via flags. + properties: + configMapRef: + description: |- + ConfigMapRef holds an optional name of a ConfigMap that contains + the following keys: + + - `provider`: the provider to use. One of `aws`, `azure`, `gcp`, or + `generic`. Required. + - `cluster`: the fully qualified resource name of the Kubernetes + cluster in the cloud provider API. Not used by the `generic` + provider. Required when one of `address` or `ca.crt` is not set. + - `address`: the address of the Kubernetes API server. Required + for `generic`. For the other providers, if not specified, the + first address in the cluster resource will be used, and if + specified, it must match one of the addresses in the cluster + resource. + If audiences is not set, will be used as the audience for the + `generic` provider. + - `ca.crt`: the optional PEM-encoded CA certificate for the + Kubernetes API server. If not set, the controller will use the + CA certificate from the cluster resource. + - `audiences`: the optional audiences as a list of + line-break-separated strings for the Kubernetes ServiceAccount + token. Defaults to the `address` for the `generic` provider, or + to specific values for the other providers depending on the + provider. + - `serviceAccountName`: the optional name of the Kubernetes + ServiceAccount in the same namespace that should be used + for authentication. If not specified, the controller + ServiceAccount will be used. + + Mutually exclusive with SecretRef. + properties: + name: + description: Name of the referent. + type: string + required: + - name + type: object + secretRef: + description: |- + SecretRef holds an optional name of a secret that contains a key with + the kubeconfig file as the value. If no key is set, the key will default + to 'value'. Mutually exclusive with ConfigMapRef. + It is recommended that the kubeconfig is self-contained, and the secret + is regularly updated if credentials such as a cloud-access-token expire. + Cloud specific `cmd-path` auth helpers will not function without adding + binaries and credentials to the Pod that is responsible for reconciling + Kubernetes resources. Supported only for the generic provider. + properties: + key: + description: Key in the Secret, when not specified an implementation-specific + default key is used. + type: string + name: + description: Name of the Secret. + type: string + required: + - name + type: object + type: object + x-kubernetes-validations: + - message: exactly one of spec.kubeConfig.configMapRef or spec.kubeConfig.secretRef + must be specified + rule: has(self.configMapRef) || has(self.secretRef) + - message: exactly one of spec.kubeConfig.configMapRef or spec.kubeConfig.secretRef + must be specified + rule: '!has(self.configMapRef) || !has(self.secretRef)' path: description: |- Path within the repository to write resources to, relative to the repository @@ -235,6 +323,16 @@ spec: - message: spec.path is immutable; delete and recreate the GitTarget to change its destination rule: self.path == oldSelf.path + - message: spec.kubeConfig is immutable; delete and recreate the GitTarget + to change the cluster it mirrors + rule: has(self.kubeConfig) == has(oldSelf.kubeConfig) && (!has(self.kubeConfig) + || self.kubeConfig == oldSelf.kubeConfig) + - message: spec.kubeConfig.configMapRef (provider auth) is not yet supported; + use secretRef + rule: '!has(self.kubeConfig) || !has(self.kubeConfig.configMapRef)' + - message: spec.kubeConfig.secretRef.name must not be empty + rule: '!has(self.kubeConfig) || !has(self.kubeConfig.secretRef) || size(self.kubeConfig.secretRef.name) + > 0' status: description: status defines the observed state of GitTarget properties: diff --git a/docs/INDEX.md b/docs/INDEX.md index d57709bf..d2ae5389 100644 --- a/docs/INDEX.md +++ b/docs/INDEX.md @@ -66,14 +66,15 @@ says what we support and refuse** — and then its kustomize field taxonomy, the write boundary, the orchestrator/expansion line, and how secrets are handled. -Nine other open items: +Ten other open items: | Doc | Open question | |---|---| +| [`config-plane-split.md`](design/config-plane-split.md) | remote-cluster mirroring via an inline, immutable `GitTarget.spec.kubeConfig` (Flux's `meta.KubeConfigReference`) — **redesign of #220's #1, built** | | [`watch-and-catalog-architecture.md`](design/watch-and-catalog-architecture.md) | the target three-layer watch model — **needs a human call before building** | | [`metrics-observability-plan.md`](design/metrics-observability-plan.md) | the watch-stage metrics do not exist yet | | [`reconcile-triggering.md`](design/reconcile-triggering.md) | which controllers still fail to wake up | -| [`multi-cluster-audit-ingestion-implications.md`](design/multi-cluster-audit-ingestion-implications.md) | there is still no CRD for remote cluster connectivity | +| [`multi-cluster-audit-ingestion-implications.md`](design/multi-cluster-audit-ingestion-implications.md) | §5's `SourceCluster` CRD is **superseded** by [`config-plane-split.md`](design/config-plane-split.md); the rest (per-cluster audit ingestion) is still open | | [`release-image-reuse-plan.md`](design/release-image-reuse-plan.md) | PRs 2–5 unstarted | | [`e2e-coverage-gaps-and-improvements-plan.md`](design/e2e-coverage-gaps-and-improvements-plan.md) | tests A/B/C still proposals | | [`e2e-finish-plan.md`](design/e2e-finish-plan.md) | remaining e2e harness work | diff --git a/docs/design/config-plane-split.md b/docs/design/config-plane-split.md new file mode 100644 index 00000000..43ef46ac --- /dev/null +++ b/docs/design/config-plane-split.md @@ -0,0 +1,944 @@ +# Separating the config plane from the watched cluster + +> **design** — **built** (this PR). Index: [`../INDEX.md`](../INDEX.md) +> Supersedes the `SourceCluster` CRD proposal in +> [`multi-cluster-audit-ingestion-implications.md`](multi-cluster-audit-ingestion-implications.md) §5. +> Redesign of feature #1 from the closed multi-tenant PR (#220), shipped on its own. + +## One sentence + +A `GitTarget` may name the cluster it mirrors *from* — an immutable, optional +`spec.kubeConfig`, Flux's `meta.KubeConfigReference` verbatim (the same field +`Kustomization.spec.kubeConfig` uses) — so the operator can read its own config and +Git credentials from the cluster it runs in while watching resources on another, +and one operator can mirror many clusters. + +## Problem + +GitOps Reverser builds exactly one client: + +```go +mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ … }) +``` + +That single `rest.Config` serves two jobs that are conceptually unrelated: + +- **the config plane** — where `GitProvider`, `GitTarget`, `WatchRule` and the + `secretRef` Git credentials are read; +- **the watched cluster** — where the resources it mirrors to Git actually live. + +Because `GitProvider.spec.secretRef` is a local reference and +`WatchRule.spec.targetRef` must name a same-namespace `GitTarget`, all four +objects have to sit in one namespace **on the cluster being watched**. Nothing +chose this; it fell out of having one kubeconfig. + +For an operator mirroring a cluster they also hand to a tenant, that means a Git +write credential — often scoped far more broadly than the one repository a +`GitTarget` names — lives one RBAC rule away from whoever can read Secrets in +that namespace. The isolation rests on a policy decision rather than on a +boundary. Splitting the two planes turns the policy into a boundary: the +credential for a cluster never has to live on that cluster. + +## Decision + +Add an **inline, optional, immutable** `spec.kubeConfig` to `GitTarget` — Flux's +`meta.KubeConfigReference`, verbatim and inline, exactly like +`Kustomization.spec.kubeConfig`. Omitted means "the cluster I run in" — the +single-cluster default, which needs no configuration and behaves exactly as today. + +```yaml +apiVersion: configbutler.ai/v1alpha3 +kind: GitTarget +spec: + providerRef: { name: acme } + branch: main + path: clusters/acme + kubeConfig: # NEW — omit for "the cluster I run in" + secretRef: + name: acme-kubeconfig + # key optional; when empty the operator reads "value" then + # "value.yaml" — Flux's resolution order. +``` + +The Secret is read from the `GitTarget`'s **own** namespace, on the cluster the +operator runs in. Its value is an ordinary kubeconfig, which determines both the +source cluster and the credentials to reach it. The watched cluster then holds +nothing but the watched resources — no Secret, no `configbutler.ai` CRDs at all. + +Three decisions were taken deliberately and are argued below: + +- **Inline on `GitTarget`, not a dedicated CRD, and not on `WatchRule`.** +- **A bare `spec.kubeConfig`, not a `spec.sourceCluster` wrapper.** +- **Immutable, and therefore fully decoupled from retarget** — unlike the closed + PR, which coupled it into a mutable-destination lifecycle and paid for it. + +### Why `GitTarget`, not `WatchRule` + +A `GitTarget` already owns exactly one materialization: one +(provider, branch, folder). Adding the source cluster makes it one +(**cluster**, provider, branch, folder) — still one owner, one folder, one +desired state. The watch data plane is *already* keyed by `GitTarget` +([`materialization.go`](../../internal/watch/materialization.go): `DeclareForGitTarget` +takes a `gitDest`), so the cluster comes along for free. + +Putting it on `WatchRule` would let two rules point at different clusters and +feed one folder, and the mark-and-sweep would then alternately delete each +cluster's objects. That is not a configuration anyone should be able to write. + +`WatchRule` keeps its meaning: it watches the namespace **it lives in**, resolved +on the `GitTarget`'s source cluster — a `WatchRule` in config-plane namespace +`team-a` watches namespace `team-a` on the remote. A `ClusterWatchRule` watches +the whole source cluster. + +### Why `spec.kubeConfig`, not a `spec.sourceCluster` wrapper + +An earlier draft nested the reference as `spec.sourceCluster.kubeConfig`. It is +collapsed to a bare `spec.kubeConfig`, because in v1 the wrapper would hold a +**single** field and the kubeconfig **already determines** the remote cluster and +its credentials — the nesting adds no contract value, only depth. Collapsing it +also makes the field *genuinely* Flux-shaped: `GitTarget.spec.kubeConfig`, the same +path as `Kustomization.spec.kubeConfig`, not a one-deeper cousin. + +"Source cluster" stays the **domain / internal** term — the derived cluster-context +id (`SourceClusterID`), the `SourceClusterResolver`, the `SourceClusterUnreachable` +condition — but it is not an API object. + +A wrapper would only earn its place by gaining a **real second property now**, and +none exists: per-target qps/burst live on flags, a friendly cluster name is not +needed for correctness, and a future platform-owned reference does **not** justify +it — that arrives as a *mutually-exclusive sibling*, `spec.sourceClusterRef` +alongside `spec.kubeConfig` (see *Future shape*), not as a field nested under a +wrapper today. + +### Why inline, not a dedicated `SourceCluster` CRD + +[`multi-cluster-audit-ingestion-implications.md`](multi-cluster-audit-ingestion-implications.md) +§5 proposed a dedicated `SourceCluster` CRD fusing **audit-ingress identity** and +**kube-API connectivity** into one onboarding object. Its load-bearing rationale +is gone: `main` **removed** the `/audit-webhook/` path — the handler +now rejects any cluster-id segment +([`audit_handler.go`](../../internal/webhook/audit_handler.go): `validateAuditWebhookPath`, +*"audit webhook path must not include a cluster ID"*). Multi-cluster is now +purely a **kube-API** story (discovery + snapshot + live watches over a +kubeconfig), and that is exactly the shape Flux uses inline. A dedicated CRD's +remaining benefits — reuse across many `GitTarget`s, platform-admin RBAC +ownership, a home for connectivity status — are real but not needed for the first +version, and are not what Flux reached for. The inline field leaves a clean +migration path (see *Future shape*). + +## Scope: what the first PR ships (and what it deliberately does not) + +This is intentionally the **smallest useful cut**. The first PR does **one** thing: +let a `GitTarget` mirror a remote cluster reached by a **kubeconfig Secret**. Two +related capabilities are **designed in this document but explicitly NOT built in +this PR** — the plan is written down so the shape is settled, not so it ships now. + +| | Mechanism | First PR | +|---|---|---| +| ✅ **In** | `kubeConfig.secretRef` — literal kubeconfig Secret | **built** | +| ⛔ **Out** | `kubeConfig.configMapRef` — cloud / OIDC **workload identity** | **CEL-rejected as reserved**; planned — see *Remote auth mechanisms* | +| ⛔ **Out** | `serviceAccountName` — remote **ServiceAccount impersonation** | not in the API at all; planned — see *Remote auth mechanisms* | + +Concretely, in this first PR: + +- The **only** Flux import taken is `github.com/fluxcd/pkg/apis/meta` (the type + its + CEL), pinned to **v1.31.0** (matches this repo's Go 1.26 / Kubernetes 0.36 + dependencies). There is **no** `fluxcd/pkg/runtime` and **no** `fluxcd/pkg/auth` + import — those arrive only with workload identity, as a separate feature. +- `configMapRef` is **rejected at admission** by a CEL guard, so setting it is a + legible "not yet supported", never a silent no-op or a half-working path. +- There is **no** remote-impersonation surface at all: a remote `GitTarget` connects + as the kubeconfig's own identity, which the operator should be given narrow + read-only RBAC on the remote (see *Security*). + +**Why both are deferred, not just unbuilt.** Workload identity turns the operator's +cloud/OIDC identity into a powerful access broker, and remote impersonation lets the +spec author choose the identity requests run as — both raise authorization questions +that the **credential-reference boundary** (*Security*, below) has to answer first. +`secretRef` plus a narrowly-scoped read-only remote identity is the simpler, safer +v1, and nothing about it forecloses either follow-up. + +## What Flux does, and what we copy + +Flux is the reference implementation of "optional remote cluster via kubeconfig", +so we read it as ground truth. The findings that shaped this design: + +- **The type is shared, not per-controller.** `Kustomization.spec.kubeConfig` + and `HelmRelease.spec.kubeConfig` both embed one type, + `meta.KubeConfigReference` (`fluxcd/pkg/apis/meta/reference_types.go`). There + is one canonical shape to copy. +- **It is an all-optional object with two mutually-exclusive refs:** `secretRef` + (a literal kubeconfig, a `SecretKeyReference` = name + optional key) **or** + `configMapRef` (a `LocalObjectReference` naming a ConfigMap that drives + cloud **workload identity**: `provider` ∈ {aws,azure,gcp,generic}, `cluster`, + `address`, `ca.crt`, `audiences`, `serviceAccountName`). The exactly-one-of + constraint is enforced by **paired CEL `XValidation`**, not a webhook. +- **The default key is resolved in code, not the schema.** There is no + `+kubebuilder:default`. `getRESTConfigFromSecret` + (`fluxcd/pkg/runtime/client/impersonator.go`) resolves: explicit `key` wins → + else `value` → else `value.yaml` → else error. +- **Kubeconfigs are sanitized — by *stripping*, silently.** Flux's client builder + **drops** `exec` auth providers and `insecure-skip-tls-verify` from the kubeconfig + unless `--insecure-kubeconfig-exec` / `--insecure-kubeconfig-tls` are set + (`fluxcd/pkg/runtime/client/kubeconfig.go`). It neutralizes to a safe subset; it + does not reject. (We diverge here — see below.) +- **`kubeConfig` composes with `spec.serviceAccountName`** to *impersonate* a + ServiceAccount on the remote (`system:serviceaccount::`), with a + controller-level `--default-service-account` fallback. +- **Flux does not make `kubeConfig` immutable**, and has **no dedicated + remote-connectivity condition** — failures surface as generic `Ready=False`. +- **The Flux Operator itself carries no kubeconfig field at all.** It reconciles + locally and *delegates* remote targeting to the Kustomization/HelmRelease + objects it installs. This confirms the placement: the kubeconfig belongs on the + object that owns one materialization, not on an install-wide object. + +The one thing we **reuse as code** is the *type*: `spec.kubeConfig` +is Flux's `meta.KubeConfigReference` verbatim (see *Reuse*, next), which brings the +`secretRef`/`configMapRef` CEL with it. Everything else is our own — the +`value`→`value.yaml` key resolution, the kubeconfig safety check, the immutability, +and a legibility condition Flux does without. On the safety check we **diverge on +purpose**: where Flux *silently strips* `exec`/insecure-TLS to a safe subset, we +**reject** a kubeconfig that carries them, so the failure is legible (a +`Validated=False` reason) rather than a quietly-neutered credential that then fails +to connect for reasons the operator cannot see. + +## Reuse — one type module in; everything else our own + +Both Flux packages are public Go modules under Apache-2.0, and this repo is +Apache-2.0, so reuse is clean (keep upstream attribution / `NOTICE`). We depend on +no `fluxcd` module today. For the first PR **exactly one** Flux import is taken — +`apis/meta` — and nothing else. + +**Import the API types — `github.com/fluxcd/pkg/apis/meta`.** This is the exact +`KubeConfigReference` / `SecretKeyReference` / `LocalObjectReference` we would +otherwise hand-write. Reusing it is strictly better: + +- **Near-zero weight, and version-aligned.** Its only real dependency is + `k8s.io/apimachinery`, which we already have. **Pin `v1.31.0`** — it targets Go + 1.26 and Kubernetes 0.36, matching this repo's own dependencies, and its CEL is + the same generated markers proven in Flux's shipping CRDs. +- **It embeds in our CRD.** It ships `DeepCopy`, and the types are import-free pure + structs, so controller-gen embeds them and **emits Flux's CEL markers into our + CRD** — the exactly-one-of `secretRef`/`configMapRef` rule and the key contract + come from upstream, and a Flux kubeconfig Secret works unchanged. +- **It realizes "shaped for later" by construction** — `configMapRef` is already in + the type, so provider auth becomes a code-only change, never a schema break. + +**Do *not* import `github.com/fluxcd/pkg/runtime` — not even for the sanitizer.** +The `client` package holds the real "connect to an external kube-api" logic (the +`Impersonator`/`GetClient` builder and the `KubeConfig()` sanitizer), but: + +- **It is heavy** — it drags in `fluxcd/cli-utils`, `fluxcd/pkg/apis/{acl,event, + kustomize}`, `cel-go`, `prometheus/client_golang`, and couples our build to + Flux's release cadence. +- **Its shape is not ours.** It builds a *controller-runtime client* around + impersonation and provider fetchers; we build a **dynamic client + discovery** + per cluster, cache/rotate by `resourceVersion`, and deliberately don't dial. +- **Its sanitizer strips; we reject.** The one piece we might have lifted, the + `KubeConfig()` sanitizer, *silently neutralizes* `exec`/insecure-TLS. We want a + legible **rejection** instead — a different action, so there is nothing to reuse. + Our check is a few lines over the parsed kubeconfig in `internal/watch`, takes no + Flux import, and is documented as a deliberate divergence. + +The **one** thing that later justifies importing `pkg/runtime` is **cloud workload +identity** — the `configMapRef` provider path — and it is *two* modules, not one: +`pkg/runtime/client` supplies only the injection **plumbing** (a +`ProviderRESTConfigFetcher` function type; it mints nothing and does not even +depend on the auth module), while `fluxcd/pkg/auth` supplies the actual token +**minting**. What that buys — and the cloud-SDK cost — is spelled out in +*[Remote auth mechanisms](#remote-auth-mechanisms--impersonation-vs-workload-identity)* +below. The other two auth mechanisms are **not** triggers for either import and +should not be conflated with them: + +- **`secretRef`** (v1) needs only `clientcmd` + our own reject check — no Flux + import beyond the `apis/meta` type. +- **`spec.serviceAccountName` impersonation** is ~three lines of client-go — + `restConfig.Impersonate = rest.ImpersonationConfig{UserName: + "system:serviceaccount::"}`. Flux's `Impersonator` wraps it, but the + mechanism is trivial; port it, don't import for it. + +So: import `pkg/runtime` only for the *provider/workload-identity* path, not for +impersonation. + +## What this redesign changes vs the closed PR (#220) + +The closed PR shipped a working version of this feature. It was folded into a +seven-feature branch and coupled to feature #6 (a *mutable, retargetable* +destination). This redesign keeps the good structure and removes the coupling. + +| Closed PR (#220) | This redesign | Why | +|---|---|---| +| `sourceCluster` **mutable**, part of the retarget lifecycle (`observedDestination`, `retargetingTo`, teardown-before-validation). | `spec.kubeConfig` **immutable**, exactly like `providerRef`/`branch`/`path` are on `main` today. | The source of a folder's content is destination identity. On `main` the destination is already immutable; extending it is the minimal, coherent change. Mutability is retarget's problem (#6), and can subsume it later. | +| Source cluster **stamped onto every `CompiledRule`**; a `spec` change raced rule recompilation, needing `CompiledSourceClusters`, `sourceClusterRulesCaughtUp`, and a "rules disagree → watch nothing" state. | Source cluster is a **GitTarget property captured on `Declare`**, the same way `gitTargetUIDs` already is. | It *is* a GitTarget property; normalize it as one. Because it is immutable, there is no "spec changed, rules haven't caught up" window — the entire race apparatus disappears. | +| `key` **schema-defaulted to `value.yaml`**. | `key` optional, **no schema default**; resolver reads `value` then `value.yaml`. | `value.yaml` alone would **fail** on a standard Flux kubeconfig Secret, whose key is `value`. Matching Flux's fallback order is simpler *and* more compatible. | +| Resolver did a bare `RESTConfigFromKubeConfig(raw)`. | Resolver **rejects** a kubeconfig carrying `exec`/insecure-TLS (legible `Validated=False`), unless flag-opted-in. | An operator-supplied kubeconfig is attacker-adjacent input; an `exec` stanza runs a binary in the operator Pod. Flux *strips* these silently; we reject for legibility. | +| `spec.sourceCluster.kubeConfigSecretRef` (a bespoke wrapper + bare secret ref). | `spec.kubeConfig` — Flux's `meta.KubeConfigReference`, imported inline. | Exact Flux parity (`Kustomization.spec.kubeConfig`), and `kubeConfig.configMapRef` (provider/workload-identity) is already in the imported schema — **no wrapper, no schema break** later. | + +Most of the PR's internals are sound and carried forward: the per-cluster +`clusterContext` (now with refcounted teardown), credential rotation on the +refresh cadence, and the parse-don't-dial legibility gate. Two things are **not** +carried over: its bare unsanitized resolver (we reject unsafe kubeconfigs, above) +and its **union GVK→GVR lookup**, which is replaced by target-scoped resolution +because a union is a correctness bug across clusters (see *Architecture*). + +## API shape + +No wrapper: `spec.kubeConfig` is Flux's own type, **imported, not re-declared** +(see *Reuse*), inline on `GitTargetSpec` — `GitTarget.spec.kubeConfig`, exactly like +`Kustomization.spec.kubeConfig`. Embedding `meta.KubeConfigReference` means the +schema, the `secretRef`/`configMapRef` CEL, and the `value`→`value.yaml` key +contract all come from upstream, and a Secret produced for a Flux `Kustomization` +works here unchanged. + +```go +import meta "github.com/fluxcd/pkg/apis/meta" + +// spec.kubeConfig is immutable — the source of a folder's content is destination identity. +// +kubebuilder:validation:XValidation:rule="has(self.kubeConfig) == has(oldSelf.kubeConfig) && (!has(self.kubeConfig) || self.kubeConfig == oldSelf.kubeConfig)",message="spec.kubeConfig is immutable; delete and recreate the GitTarget to change the cluster it mirrors" +// +// configMapRef (provider / workload-identity auth) is in meta's schema but not yet +// implemented; reject it at admission so the v1alpha3 contract is "secretRef only". Deleting +// this one rule, plus wiring the provider path, is the whole future enablement. +// +kubebuilder:validation:XValidation:rule="!has(self.kubeConfig) || !has(self.kubeConfig.configMapRef)",message="spec.kubeConfig.configMapRef (provider auth) is not yet supported; use secretRef" +type GitTargetSpec struct { + // … providerRef, branch, path (unchanged, still immutable) … + + // KubeConfig names the SOURCE CLUSTER this GitTarget mirrors FROM: the kubeconfig + // determines both the cluster and the credentials to reach it. Omitted means the cluster + // the operator runs in, the single-cluster default. Its Secret is read from the + // GitTarget's OWN namespace, on the cluster the operator runs in — the credential for a + // cluster never has to live on that cluster. When SecretRef.Key is empty the resolver + // reads "value" then "value.yaml", Flux's order (see Resolver, below). Immutable: the + // source of a folder's content is part of what the folder means; delete and recreate to + // change it. Only kubeConfig.secretRef is honored in v1alpha3. + // +optional + KubeConfig *meta.KubeConfigReference `json:"kubeConfig,omitempty"` +} +``` + +The two CEL rules live at *our* `GitTargetSpec` level — Flux's type carries neither +(no immutability, no provider guard), so nothing upstream is forked — and both use +`has()` guards because `kubeConfig` is optional. `meta.KubeConfigReference` already +carries the paired "exactly one of `configMapRef`/`secretRef`" CEL, so with our +guard blocking `configMapRef`, `secretRef` is effectively required: an empty +`kubeConfig` is rejected at admission, not discovered at watch time. + +## Architecture + +The changes are confined to the watch manager and the `GitTarget` controller. The +git write path is unchanged except for making GVK→GVR resolution source-cluster +scoped. + +### The cluster context + +The watch manager grows a **cluster context**: the set of things that were +Manager-wide singletons and are in fact properties of *one* cluster — its API +surface, the followability decisions derived from it, the clients that reach it, +and the informers that report its surface moved. + +```go +type clusterContext struct { + id string // "" for the local cluster, else "//" + catalog *APIResourceCatalog + registry *typeset.Registry + restConfig *rest.Config + configVersion string // the kubeconfig Secret's resourceVersion + dynamicClient dynamic.Interface + discovery apiResourceDiscovery + triggerFactory dynamicinformer.DynamicSharedInformerFactory + // … per-cluster edge-triggered logging state … +} +``` + +- `Manager.clusters map[string]*clusterContext`. A zero-value Manager (unit + tests, and **every** single-cluster install) creates exactly one, keyed + `LocalClusterID = ""`. Nothing that does not know about source clusters changes + behavior — it all lands on the local context. +- The catalog refresh runs for every **active** cluster — the local one plus every + cluster some `GitTarget` currently points at. It returns the *local* cluster's + error only: a remote that cannot be reached fails **its own** `GitTarget`s + (through their unready registries), never the local cluster's. +- Watch and list opens take the cluster id, so each `(GitTarget, GVR, namespace)` + watch runs against the right cluster. +- A `GitTarget`'s rules resolve against **its own cluster's** type registry. A CRD + installed only on the remote is followable only there — and, more importantly, a + type served only *locally* never resolves for a remote target. Mirroring the + wrong cluster into a folder is worse than mirroring none. +- **Contexts are reference-counted and torn down.** A `clusterContext` is created on + first use by a `GitTarget` that names it, and **torn down when the last such + `GitTarget` is gone** — its trigger informers stopped, its discovery/dynamic + clients closed, its catalog and registry dropped. The local context is never torn + down. Without refcounted teardown a deleted remote `GitTarget` would leak a + discovery stream, an informer factory, and a client set for the life of the + process; this is called out because PR #220 created contexts lazily but is worth + auditing for the symmetric teardown. It needs a test (see *Build order*). + +Because `spec.kubeConfig` is immutable, the manager learns a `GitTarget`'s cluster +once, on `DeclareForGitTarget`, and stores it keyed by `GitTarget` — the existing +`gitTargetUIDs` pattern. No per-rule propagation, no cross-rule disagreement +window. + +### Cluster identity and keying + +The cluster id the data plane keys on is `//` — the +config-plane namespace, the Secret name, and the Secret key **as written in spec** +(empty is its own identity). The key is part of the identity because two +`GitTarget`s naming one Secret under different keys are pointed at different +kubeconfigs, and so at different clusters. Two `GitTarget`s naming the *same* +Secret+key share one `clusterContext` — one catalog, one client set, one discovery +stream — which is the efficient outcome. + +Identity is the *reference*, not the kubeconfig *contents*. Consequence: two +different Secrets that happen to point at the same physical cluster get two +contexts (duplicate discovery). This is accepted — each context is independent and +correct; deriving identity from contents (server URL, cluster UID) is fragile +under HA and rotation. Rotating a Secret's **contents** is transparent (same id, +fresh credential); renaming the Secret is a new cluster and requires recreating +the `GitTarget`. + +### Credential resolution, rotation, and sanitization + +A `SourceClusterResolver` turns a cluster id into a `rest.Config` by reading the +named Secret from the config plane: + +1. Parse the id back to `{namespace, name, key}`. +2. `Get` the Secret (cache-bypassing read, so a rotation is seen without a Secret + informer — same reasoning as the existing SOPS-key reads). +3. Select the kubeconfig bytes: explicit `key` → else `value` → else `value.yaml` + → else error (Flux's order). +4. **Reject** an unsafe kubeconfig before building the config. Flux's + `pkg/runtime/client` *silently strips* `exec` auth providers and + `insecure-skip-tls-verify`; we instead **fail** a kubeconfig that carries them, + with a legible `Validated=False` reason, unless the operator opts in with + `--insecure-kubeconfig-exec` / `--insecure-kubeconfig-tls`. Explicit rejection + over silent stripping is a **deliberate divergence** — a stripped-but-accepted + kubeconfig that then fails to connect is exactly the illegible failure the + legibility gate exists to prevent. This is our own check (not a port), so it + takes no Flux import. +5. Build the `rest.Config`, apply `--source-cluster-qps` / `--source-cluster-burst` + (a remote is reached over a network the local one is not), **drop the bytes**, + and remember only the Secret's `resourceVersion` as the version token. + +Rotation is picked up on the catalog-refresh cadence (every 30s and on every rule +change), not on the hot watch-reconnect path: one Secret read per cluster per +refresh, not one per watch. When the token changes, the cached clients are +dropped and the next use rebuilds them; a watch already streaming on the old +credential keeps working until that credential stops being accepted, and the +reconnect that follows picks up the rebuilt client. + +### GVK→GVR resolution is scoped to the source cluster (not a union) + +When a branch worker scans the manifests already in a Git folder to answer "what +resource is this document?", it must resolve GVK→GVR against **the source cluster +that folder mirrors** — not a union of all clusters. + +PR #220 used a single **union** lookup (local first, then remotes; first answer +wins), reasoning that GVK→GVR is stable across clusters. **That is not safe.** Two +clusters can validly serve the same GVK under **different plural resources or +scopes** — two independently-authored CRDs both defining `example.io/v1 Widget` +with different `.spec.names.plural`, or one namespaced and one cluster-scoped. +"First answer wins" would then index or sweep a folder's documents against the +wrong cluster's mapping, **mis-filing or deleting** manifests. This is a +correctness bug, not an efficiency one, so the union is dropped. + +The mapping is **target-scoped**. A folder is owned by exactly one `GitTarget` (one +materialization), so folder → `GitTarget` → source cluster is determined, and the +worker resolves each document against **that** cluster's `typeset.Registry`. This +costs the plumbing the union was avoiding: a branch worker is keyed by +(provider, branch) and may serve several `GitTarget`s with different source +clusters, so the resolver cannot be one worker-wide field. Instead each pending +write and each folder scan carries its originating `GitTarget`'s source-cluster id, +and the worker looks up the mapping for that id — the extra threading is the price +of correctness, and is paid deliberately. There is **no** union and **no** +first-wins fallback; a document whose owning cluster's registry does not know its +type is refused by the acceptance gate, exactly as in the single-cluster case. In a +single-cluster install every target resolves against the one local registry, +unchanged. + +### The legibility gate + +The `GitTarget` controller reads and parses the kubeconfig before any watch opens +against it, and reports **`Validated=False`** with a specific `KubeConfig*` reason +(*Status and conditions*) when the Secret is missing, empty at its key, unparseable, +or fails the exec/TLS safety policy. This is a **legibility** gate, not a security +one, and it diverges from Flux (which surfaces this only as a generic `Ready=False`): +without it, a typo'd Secret name surfaces only as a stalled data plane and a +repeating log line; with it, the `GitTarget` says exactly which input was wrong. + +It deliberately does **not** dial the cluster — so it only ever sets `Validated`, +never reachability. Whether the API server can actually be *contacted* is a runtime +observation the data plane records on `SourceClusterReachable` after real discovery; +an unreachable-right-now cluster is a transient it retries, and a controller that +blocked on a network round trip would stall every other `GitTarget` behind it. This +split — inputs on `Validated`, reachability on `SourceClusterReachable` — is the one +the earlier draft got wrong by folding both into `Validated=False / +SourceClusterUnreachable`. + +## Security and RBAC + +- The operator needs `get` on Secrets in the namespaces where `GitTarget`s live — + which it already has, for `GitProvider.spec.secretRef` and the SOPS age keys. +- The credential for a remote cluster lives **only** in the config plane. The + watched cluster holds nothing but the watched resources. +- On the remote cluster, the kubeconfig's identity needs only **read** access to + the mirrored types (`get`/`list`/`watch`), plus `apiextensions`/`apiregistration` + read for discovery. Least privilege is the operator's to grant on the remote. The + minimal ClusterRole the kubeconfig's identity must be bound to on the **source** + cluster (bind it to whichever ServiceAccount/user the kubeconfig authenticates as): + + ```yaml + apiVersion: rbac.authorization.k8s.io/v1 + kind: ClusterRole + metadata: + name: gitops-reverser-source-read + rules: + # Discovery: what types does this cluster serve, and are they followable? + - apiGroups: ["apiextensions.k8s.io"] + resources: ["customresourcedefinitions"] + verbs: ["get", "list", "watch"] + - apiGroups: ["apiregistration.k8s.io"] + resources: ["apiservices"] + verbs: ["get", "list", "watch"] + # The mirrored types. `["*"]` grants read on every type; narrow it to exactly the + # groups/resources the WatchRules select for a true least-privilege mirror. + - apiGroups: ["*"] + resources: ["*"] + verbs: ["get", "list", "watch"] + ``` + + The operator never writes to the source cluster, so no write verb is ever needed; + narrowing the wildcard rule to the selected types is the recommended hardening. +- Kubeconfig **rejection** (above) closes the `exec`-provider and insecure-TLS holes + an operator-supplied kubeconfig would otherwise open. +- Remote clients carry client-side throttling the in-cluster config does not by + default. + +### Credential-reference authorization — the namespace is the boundary + +The operator reads the kubeconfig Secret with **its own** credentials, not the +spec author's. Under ordinary namespace RBAC that is harmless: `spec.kubeConfig.secretRef` +is resolved **only from the GitTarget's own namespace** (there is no cross-namespace +Secret reference), and whoever can create a `GitTarget` in namespace *N* can already +read Secrets in *N* — so naming one grants nothing new. **The namespace is the trust +boundary**, exactly as it already is for `GitProvider.spec.secretRef` (Git credentials), +which has lived in one trust zone since day one. This is the model the feature ships with, +and it is picked up entirely on the reconcile loop — no admission webhook. + +The one residual case this does *not* close is a **fine-grained intra-namespace RBAC +split**: a subject granted create/update on `GitTarget` in namespace *N* but **denied** +Secret-read in that same namespace could name a privileged kubeconfig Secret they cannot +themselves read, and the operator — which can — would mirror that remote cluster's state +into a Git destination the subject controls (a confused-deputy read escalation). This +requires an unusual RBAC posture (write-GitTarget-yes, read-Secret-no, same namespace); a +namespace is normally a single tenant's compartment, so the split rarely exists in +practice. **It is deliberately not closed by an admission webhook.** An earlier revision of +this design guarded it with a fail-closed `SubjectAccessReview` at admission on the +requesting user's `get` of the named Secret; that was removed because the product model is +**all configuration is picked up on the reconcile loop, like every other resource** — and +a reconcile runs as the operator ServiceAccount, with no requesting-user identity to review. + +If self-service multi-tenant remote clusters with per-subject Secret isolation *inside* one +namespace ever becomes a real requirement, the right shape is **not** a per-field admission +webhook but a platform-owned `ClusterConnection`/`SourceCluster` CRD that only platform +admins may create (RBAC on the *object*, which the reconcile loop can honor natively), +referenced by name from the `GitTarget`. That keeps the "everything reconciles" model intact +while moving the credential out of the tenant's write reach. Deferring workload identity and +impersonation (see *Remote auth mechanisms*) is partly downstream of this: both widen what a +chosen credential/identity can reach, so neither should land before that CRD, if it is ever +needed. + +## Explicitly out of scope (and why it's safe to defer) + +- **Author attribution across clusters.** Live object *state* comes from watches, + which work against a remote cluster. But *who* made a change is joined from + apiserver **audit** events, and the audit webhook is now local-only (the + cluster-id path was removed). So a remote-cluster mirror commits as the + configured committer, not the human author — correct, just less rich. + Per-cluster audit ingestion is a separate feature; this design does not block + it, and notes the boundary rather than pretending to solve it. +- **Provider / workload-identity auth (`kubeConfig.configMapRef`).** Deferred, and + the schema already carries it (rejected by CEL for now). It is genuinely useful + later — no long-lived kubeconfigs — but it turns the operator's cloud/OIDC + identity into a powerful access broker, so it is **gated on the + credential-reference authorization model** (*Security*) landing first. +- **ServiceAccount impersonation (`spec.serviceAccountName`).** Flux's remote + impersonation, and **not in the v1 API at all**. A read-only mirror does not need + it — a kubeconfig identity with narrow remote RBAC is simpler and safer — and + letting the spec author choose the impersonated ServiceAccount can itself be an + **escalation path** (the connecting identity must hold remote `impersonate`, and + the author picks who requests run as). Deferred until there is a concrete need and + the authorization model above. +- **A mutable / retargetable source cluster.** Owned by feature #6 (retarget). + When that lands and makes the destination mutable, it can generalize to cover + `spec.kubeConfig` too. Until then, immutable is the honest, simple contract. + +## Status and conditions + +`GitTarget` already carries a kstatus-shaped set — `Ready` (aggregate), +`Reconciling`/`Stalled`, `Validated`, `EncryptionConfigured`, `GitPathAccepted`, +`RenderMatchesLive`, `StreamsRunning` +([`constants.go`](../../internal/controller/constants.go), +[`gittarget_controller.go`](../../internal/controller/gittarget_controller.go)). The +source-cluster split **slots into it**, and two principles keep it legible: + +1. **Input validation and network reachability are different conditions.** A missing + or malformed kubeconfig is a *spec* problem the controller sees without touching + the network; an unreachable API server is a *runtime* observation. The earlier + draft folded both into `Validated=False / SourceClusterUnreachable` — that + conflation is corrected here. +2. **`SourceClusterUnreachable` is a *Reason*, not a condition *Type*.** The type is + **`SourceClusterReachable`**, which names the current state without implying a + held-open connection. + +### The condition set + +| Condition | Meaning | Change | +|---|---|---| +| `Validated` | Spec + **directly readable** inputs valid: provider/branch/path policy, and now — kubeconfig Secret & key exist, the kubeconfig parses, and it passes the exec/TLS safety policy. **No network dial.** | extended | +| `SourceClusterReachable` | The controller can actually reach the configured source API. `True` (reason `LocalCluster`) when `kubeConfig` is omitted; `Unknown` before first discovery; `False` after a real failed attempt. | **new** | +| `StreamsRunning` | Every selected source type has completed initial replay and its watch is healthy — strictly stronger than mere reachability. | existing | +| `GitProviderReady` | The referenced `GitProvider` is currently ready (its own credential/repository check), **projected** onto the target so one `kubectl get gittarget` separates source-side from destination-side failure. | **new (projection)** | +| `GitPathAccepted` | Git-tree / worktree write-safety state. | existing | +| `EncryptionConfigured` / `RenderMatchesLive` | Encryption-setup and render-fidelity, where enabled. | existing | +| `Ready` | Aggregate of the required conditions above, plus encryption/render where enabled. | extended | +| `Reconciling` / `Stalled` | Generic kstatus progress / needs-action, as today. | existing | + +### Reasons, grouped by the condition they set + +- **`Validated=False`** (input, no dial): `KubeConfigSecretNotFound`, + `KubeConfigKeyNotFound`, `KubeConfigInvalid`, `KubeConfigExecNotAllowed`, + `KubeConfigInsecureTLSNotAllowed` — alongside the existing `ProviderNotFound`, + `BranchNotAllowed`, `TargetConflict`, … +- **`SourceClusterReachable=False`** (after a real API attempt): + `SourceClusterUnreachable` (DNS/TCP/TLS/timeout), `SourceClusterAuthenticationFailed` + (401), `SourceClusterAccessDenied` (403 during discovery). `True` reason + `LocalCluster` for the omitted-`kubeConfig` case. +- **`StreamsRunning=False`**: `InitialReplay`, `WatchError`, `WatchNotPermitted`. +- **`GitProviderReady=False`**: `GitProviderNotReady`. + +The load-bearing distinction: a missing/malformed kubeconfig is **`Validated=False`**; +an otherwise-valid kubeconfig whose API server cannot be contacted is +**`SourceClusterReachable=False` / `SourceClusterUnreachable`**. + +### Where each is set + +- `Validated` (+ the `KubeConfig*` reasons) is set by the **GitTarget controller** + during reconcile — it reads/parses the Secret and applies the exec/TLS policy but + **does not dial** (this is the legibility gate in *Architecture*, now correctly + scoped to inputs only). +- `SourceClusterReachable` is set by the **watch data plane** on the catalog-refresh + path — discovery is the first thing that actually talks to the source API. It + starts `Unknown`, goes `True` on first successful discovery, and `False` with the + reason matching the failure class. The controller never blocks on a dial to + compute it. +- `StreamsRunning` is set as today, from per-type watch replay, computed against the + **source cluster's** registry. +- `GitProviderReady` reuses the projection pattern the codebase already has for + `WatchRule` → `GitTargetReady` + ([`ConditionTypeGitTargetReady`](../../internal/controller/constants.go)): the + GitTarget reads the referenced GitProvider's `Ready` and mirrors it. It requires + **reconciling the GitTarget when the GitProvider's status changes** — a + `Watches(&GitProvider{}, …)` mapping (as `WatchRule` already watches `GitTarget`), + with the 5-minute periodic reconcile as the fallback. + +### Git side: owned by GitProvider, projected here + +Repository connectivity stays **owned by `GitProvider`** — its `Ready` already means +"the periodic repository-connectivity check passes". The GitTarget does not re-check +the repo; it **projects** that readiness as `GitProviderReady` and folds it into +`Ready`, so one `kubectl get gittarget` says whether a stall is source-side +(`SourceClusterReachable=False`) or destination-side (`GitProviderReady=False`). + +**Caveat, and a deliberate deferral.** `GitProviderReady=True` means the provider's +periodic check succeeds; it does **not** prove every individual push lands. If +`Ready` should later mean "writes are actively succeeding", that is a separate +`GitDeliveryHealthy` condition driven by the branch worker — explicitly **out of +scope** here, so `Ready`'s meaning stays honest ("inputs valid, source reachable, +streams replaying, provider healthy") rather than overclaimed. + +No `observedDestination` / `retargetingTo` — those belong to #6. + +## Migration and compatibility + +- `spec.kubeConfig` is a new optional field. Existing installs are unaffected: + absent → local cluster → today's behavior, byte for byte. +- The stale [`multi-cluster-audit-ingestion-implications.md`](multi-cluster-audit-ingestion-implications.md) + should have its §5 `SourceCluster` CRD proposal marked superseded by this doc, + and its INDEX line updated from "there is still no CRD for remote cluster + connectivity" to point here (the connectivity model is now inline, by choice). + +## Future shape (designed-in, not built) + +Because we embed `meta.KubeConfigReference` whole, the `configMapRef` arm is +**already in the shipped schema**; enabling provider / workload-identity access is +not a type change at all. It is: + +1. **delete the one `configMapRef`-unsupported CEL guard** on `GitTargetSpec`; and +2. **wire the provider path in the resolver, `generic` first** — import + `pkg/auth/generic` **directly** (no cloud SDKs — see *Remote auth mechanisms*), + mint the token, and build the `rest.Config` in our own resolver from the + ConfigMap's `address`/`ca.crt`. `pkg/runtime/client`'s `ProviderRESTConfigFetcher` + wiring is *optional* here: it fits flux's Impersonator, but a from-scratch + resolver can call the auth provider directly and skip that import. Add each cloud + (`pkg/auth/aws`, …) à la carte afterwards. A v1alpha3 object using `secretRef` + stays valid across the change. + +`spec.serviceAccountName` (Flux's remote impersonation) composes on top +independently — it is a few lines of client-go (`rest.ImpersonationConfig`), needs +no `pkg/runtime` import, and can be added whether or not the provider path ever is. + +And if reuse across many `GitTarget`s or platform-admin ownership ever justifies a +platform-owned connection object, it arrives as a **sibling** of `spec.kubeConfig`, +not by reintroducing a wrapper: a `spec.sourceClusterRef` naming a +`ClusterConnection`/`SourceCluster` CRD, **mutually exclusive** with `spec.kubeConfig` +via one CEL rule. The inline `kubeConfig` path stays; the ref is additive. This is +the concrete reason the v1 wrapper was not worth keeping — the anticipated second +option is a peer of `kubeConfig`, never something nested beside it. + +```go +type GitTargetSpec struct { + // … one of kubeConfig or sourceClusterRef (future) … + // +optional + KubeConfig *meta.KubeConfigReference `json:"kubeConfig,omitempty"` + // +optional + SourceClusterRef *ClusterConnectionReference `json:"sourceClusterRef,omitempty"` // future +} +// +kubebuilder:validation:XValidation:rule="!(has(self.kubeConfig) && has(self.sourceClusterRef))",message="set at most one of spec.kubeConfig or spec.sourceClusterRef" +``` + +## Remote auth mechanisms — impersonation vs workload identity + +Two mechanisms sit beyond the v1 `secretRef`. They are **different axes**, not two +flavors of one thing, and keeping them straight is what makes deferring both safe. + +- **Workload identity** (`kubeConfig.configMapRef`) answers *"how does the operator + **authenticate** to the remote apiserver without a stored kubeconfig?"* The + operator's own pod cloud identity (EKS/GKE/AKS, or any OIDC issuer) is exchanged + for a short-lived token to the named cluster. It **replaces the Secret** as the + credential source. +- **ServiceAccount impersonation** (`spec.serviceAccountName`) answers *"once + connected, whose **RBAC** do my requests run under?"* It adds an `Impersonate-User` + header so the apiserver evaluates permissions as `system:serviceaccount::` + instead of the connecting identity. It **narrows** whatever credential you used; it + is not a credential itself. + +They compose: `kubeConfig` (secret or provider) sets *who you are*, optional +`serviceAccountName` sets *who you act as*. + +**Why they matter differently to us.** We only ever `list`/`watch`/`get` the source +cluster — we never write to it. + +- **Workload identity is a real win**: it lets an operator mirror managed clusters + with zero stored kubeconfigs. It is the path worth an import when we build it — + but it makes the operator's cloud/OIDC identity a **powerful access broker**, so + it is gated on the credential-reference authorization model (*Security*) first. +- **Impersonation is mostly redundant for us, and carries its own escalation risk**: + Flux needs it to scope *writes* on behalf of tenants; a read-only mirror gets the + same containment by simply giving the kubeconfig's own identity a read-only + ClusterRole. Worse, allowing the spec author to choose the impersonated + ServiceAccount is itself an escalation surface (the connecting identity must hold + remote `impersonate`, and the author picks whom requests run as). It stays a + defence-in-depth nicety at best, never a requirement. + +### What reusing Flux buys for the provider path (and what it doesn't) + +The provider path is **two** modules — worth being exact about before signing up for +either: + +- **`pkg/runtime/client` is only plumbing.** It defines the + `ProviderRESTConfigFetcher` function *type* and an injection point on its + impersonator; it mints no tokens and does not depend on the auth module. Importing + it alone gives you a socket, not a provider. +- **`fluxcd/pkg/auth` is where the minting lives**, and its `utils.ProviderByName` + switch implements exactly **four** cluster-auth providers: **`aws`, `azure`, `gcp`, + and `generic`**. So it is *not only the big three* — but it is a fixed set of four + in a `switch`, not an open plugin ecosystem. +- **`generic` is the open one.** It is cloud-agnostic OIDC: it exchanges a standard + Kubernetes ServiceAccount projected token (via `coreos/go-oidc`) for access to any + apiserver that trusts a conformant OIDC issuer — self-hosted, on-prem, or a managed + cluster outside the big three. That is the "more open providers" answer: the escape + hatch from the three clouds is `generic`, not a fourth vendor. +- **`pkg/auth`'s other providers are a different axis.** It also ships `githubapp`, + `actionsoidc`, and `jwt` — but those authenticate to **git hosts / CI**, not to a + remote kube-apiserver, so we would get no remote-cluster auth "for free" from them + (they could only ever matter to our *GitProvider* surface, a separate feature). +- **The cost is a cloud-SDK dependency surface — but it is opt-in per provider.** + `pkg/auth`'s `go.mod` requires the AWS SDK v2, the Azure SDK, and the Google Cloud + SDK, but those are reachable **only** through the `aws`/`azure`/`gcp` subpackages + and the `utils` registry that switches over all four. Verified: the production + `pkg/auth/generic` package (and the `pkg/auth` root it imports) pull **no cloud + SDK at all** — only `golang-jwt/jwt/v5` and `k8s.io/*` / `controller-runtime` + packages we already have. + +**This makes a `generic`-first rollout the recommended entry point**, and clean: +import `pkg/auth/generic` **directly** and write a one-case +`ProviderRESTConfigFetcher` (for `provider: generic`) — do **not** import +`pkg/auth/utils`, which is the `switch` that references all four providers and so +drags in every cloud SDK. Because the providers are independent subpackages, each +cloud is then addable **à la carte** later: importing `pkg/auth/aws` accepts exactly +the AWS SDK and nothing else. So the staged path is `secretRef` → `generic` OIDC +(near-zero added weight) → individual clouds as demand appears, and the `provider` +value lives in the referenced ConfigMap (not a CRD field), so an unsupported +provider is a clear resolver error, not a schema change. + +## Build order + +Each step leaves the system correct and is independently reviewable. + +1. **API types + CRD** — add **only** `github.com/fluxcd/pkg/apis/meta` (pin + **v1.31.0**) to `go.mod`; `spec.kubeConfig *meta.KubeConfigReference` **inline on + `GitTargetSpec`** (no wrapper); the immutability + `configMapRef`-guard CEL; + `task manifests`. (Confirm controller-gen emits meta's embedded CEL into our CRD; + it should, since meta ships DeepCopy and the markers travel with the type.) +2. **`clusterContext`** — extract the Manager-wide catalog/registry/clients/triggers + into a per-cluster context keyed `LocalClusterID`; pure refactor, no behavior + change, fully unit-testable. Include **refcounted teardown** — the last + `GitTarget` leaving a cluster stops its informers and closes its clients. +3. **Resolver** — `SourceClusterResolver` (parse id → read Secret → key fallback → + **reject unsafe** → build config → drop bytes → version token). The reject check + is our own code in `internal/watch` (no Flux import); add the + `--source-cluster-qps/-burst` and `--insecure-kubeconfig-exec/-tls` flags. +4. **Wire the cluster into declare** — capture a `GitTarget`'s source cluster on + `DeclareForGitTarget` (the `gitTargetUIDs` pattern); resolve rules and open + watches against that cluster's context; **target-scoped GVK→GVR resolution** for + the writer (each write carries its cluster id — *not* a union). +5. **Credential-reference authorization** — the namespace is the boundary + (*Security*): `spec.kubeConfig.secretRef` resolves only from the GitTarget's own + namespace, so the model is the same one `GitProvider.spec.secretRef` already uses. + No admission webhook — all validation is on the reconcile loop. +6. **Status conditions** (*Status and conditions*) — `Validated` extended with the + `KubeConfig*` reasons in the controller (inputs, no dial); the new + `SourceClusterReachable` set from the data plane's discovery (`Unknown` → + `True`/`False`); and `GitProviderReady` projected from the referenced provider, + with a `Watches(&GitProvider{})` trigger, folded into `Ready`. +7. **Docs + tests** — the minimal remote-read ClusterRole; the e2e/integration + scenarios below; retire the stale CRD proposal. + +## E2E and integration test plan + +The e2e harness ([`test/e2e/`](../../test/e2e/)) is kubectl-driven — CRs are rendered +YAML applied to the cluster. The source-cluster corner is its own gated leg +(`task test-e2e-source-cluster`, label `source-cluster`), the same idiom as +`E2E_ENABLE_BI_DIRECTIONAL`. Two consequences shape this plan: + +- **The reachability/validation scenarios need no separate cluster.** Input validation + and the `Validated`/`SourceClusterReachable` split exercise the source path against a + kubeconfig Secret whose server is unreachable — no genuinely remote cluster required. +- **The mirror and GVK→GVR scenarios use real REMOTE clusters — kcp workspaces.** + Rather than provision a second k3d cluster, the corner installs kcp + ([`test/e2e/setup/kcp`](../../test/e2e/setup/kcp)) **by Flux**, like every other e2e + dependency, and mirrors kcp *workspaces*: cheap logical clusters, each a real + Kubernetes API reached at + `frontproxy-front-proxy.kcp.svc.cluster.local:6443/clusters/` over verifiable + TLS. Because kcp allows isolated copies of the same Kind across workspaces, three + workspaces holding the *same* namespace + resource is the cheapest possible proof that + state is keyed by source cluster — no second k3d, no two disagreeing CRDs to hand-wire. + The harness lives in + [`test/e2e/kcp_workspace_test.go`](../../test/e2e/kcp_workspace_test.go); the specs + Skip when kcp is absent, so a default `task test-e2e` never runs them. + +### The scenarios + +1. **Input validation is legible, and does not dial** (single cluster). For each bad + input, apply a `GitTarget` whose `spec.kubeConfig.secretRef` names it and assert + `Validated=False` with the exact reason: Secret absent → `KubeConfigSecretNotFound`; + key absent → `KubeConfigKeyNotFound`; not a kubeconfig → `KubeConfigInvalid`; an + `exec` auth provider → `KubeConfigExecNotAllowed`; `insecure-skip-tls-verify` → + `KubeConfigInsecureTLSNotAllowed`. Guards: the reject-not-strip posture, and that + the controller never blocks on the network to reach these verdicts. + +2. **The `Validated` vs `SourceClusterReachable` split** (single cluster). Give a + Secret a **valid** kubeconfig whose server is unroutable (`https://192.0.2.1:6443`, + RFC 5737 TEST-NET). Assert `Validated=True` (inputs are fine) **and** + `SourceClusterReachable=False` / `SourceClusterUnreachable`. This is the exact + conflation the earlier draft had, pinned as a test so it cannot regress. + +3. **Omitted `kubeConfig` is unchanged local behavior** (single cluster). A + `GitTarget` with no `kubeConfig` mirrors as today, and reports + `SourceClusterReachable=True` reason `LocalCluster`. The single-cluster + compatibility guard. + +4. **Remote round-trip through a kcp workspace** (real remote). Create a kcp workspace, + put a ConfigMap in it, point a `GitTarget`'s `kubeConfig` at that workspace, wire a + `WatchRule` for ConfigMaps, and assert it mirrors to Git with + `SourceClusterReachable=True` and `StreamsRunning=True`. Exercises the full resolver → + `clusterContext` → per-cluster discovery → watch → target-scoped writer path against an + actually-remote API (a workspace is a distinct logical cluster), not a self-referencing + in-cluster fake — so it can tell a mistakenly-local watch from a remote one. + +5. **Credential rotation is transparent** (deferred). Rotate the Secret's contents (new + token, same cluster). Assert mirroring continues, the clients rebuild once, and the + cluster identity is **unchanged** — no retarget, same folder. Not yet implemented as a + spec; the rotation-on-refresh-cadence path is unit-tested + (`refreshClusterCredentials`). + +6. **Credential-reference authorization is namespace-scoped** (single cluster). + `spec.kubeConfig.secretRef` resolves only from the GitTarget's own namespace — a + Secret named in another namespace is never read. Assert a same-namespace reference + validates and a cross-namespace name does not resolve. The boundary is namespace RBAC + (whoever can write GitTargets in *N* can already read Secrets in *N*), not an + admission check (*Security*). + +7. **`GitProviderReady` projection** (single cluster). Make the referenced + `GitProvider` un-ready (bad repo URL). Assert the `GitTarget` reflects + `GitProviderReady=False` and `Ready=False`, and recovers when the provider does — + which also exercises the `Watches(&GitProvider{})` reconcile trigger. + +8. **Source-cluster identity is load-bearing — the centerpiece** (three kcp workspaces). + Three workspaces each hold the **same namespace + the same resource name** + (``/`ConfigMap`/`shared`) with **different content**; three `GitTarget`s mirror + them into three folders. **Assert three distinct files, each carrying its own + workspace's value.** If the operator keyed state by `(namespace, GVR)` alone — a union + / first-wins lookup (PR #220) — the three identical identities would collapse into one; + that they land as three distinct, correctly-valued files is the proof that everything is + keyed by source cluster. This is the same invariant the original two-disagreeing-CRDs + plan targeted, proven more directly and far more cheaply with kcp's isolated-per-workspace + API surfaces. Verified sound at the code level too: + [`typeset`](../../internal/typeset/observe.go) derives the GVR from the served resource + name and the scope from `Namespaced`, and refuses a *within-registry* GVK→two-GVR clash + ([`funnel.go`](../../internal/typeset/funnel.go) `ReasonGVKNotUnique`) — so a clash can + only arise from a cross-cluster union, which the scoped resolver rules out. + +### Unit / integration coverage (faster, no cluster) + +- **Target-scoped resolver** — the load-bearing correctness fix deserves a unit test + too: build two `typeset.Registry` instances that map one GVK to different GVRs/scopes, + and assert the writer's GVK→GVR resolution returns each target's *own* cluster's + answer, and that there is no union/first-wins path. This reproduces #8's core in + milliseconds and pins the invariant independent of the two-cluster e2e. +- **Resolver** — key fallback (`value` → `value.yaml`), exec/insecure-TLS rejection, + bytes-dropped-after-build, `resourceVersion` version token. +- **`clusterContext` refcount** — creation on first use, teardown (informers stopped, + clients closed) when the last referencing `GitTarget` leaves. + +## Open questions + +1. **Key in the identity vs. resolved key.** Identity uses the spec key verbatim + (empty is its own id), while the resolver falls back `value`→`value.yaml`. Two + `GitTarget`s — one with `key: value`, one with the key omitted — reach the same + kubeconfig but get two contexts. Accept the duplicate, or canonicalize the id + after the first successful read? (Proposal: accept it; canonicalizing couples + identity to a network read.) +2. **Credential-reference authorization shape** (*Security*). **Decided:** the + namespace is the boundary — `secretRef` resolves only from the GitTarget's own + namespace, same as `GitProvider.spec.secretRef`, and everything is picked up on the + reconcile loop (no admission webhook). The fine-grained intra-namespace RBAC split is + left open by design; if per-subject Secret isolation inside one namespace is ever a + real requirement, the answer is a platform-owned `ClusterConnection` CRD (RBAC on the + object) referenced by name — never a per-field admission webhook. +3. **Unsafe-kubeconfig default.** Ship `exec`/insecure-TLS **rejected by default** + (proposed, diverging from Flux's silent strip) with opt-in flags, or warn-and- + allow? Rejecting is the safe default; confirm it will not surprise operators who + rely on an `exec` auth plugin in a kubeconfig. +4. **Least-privilege ClusterRole shape on the remote.** Ship a documented broad + read ClusterRole, or require the operator to grant per-type read and degrade + unobservable cells gracefully (ties into + [`watch-and-catalog-architecture.md`](watch-and-catalog-architecture.md) §1.7)? +5. **`GitProviderReady` projection trigger and `Ready` gating.** Wire a + `Watches(&GitProvider{})` so a provider going un-ready promptly re-reconciles its + GitTargets (best, but adds an edge to the set that + [`reconcile-triggering.md`](reconcile-triggering.md) tracks), or lean on the + 5-minute periodic reconcile (simpler, laggier)? And does + `SourceClusterReachable=Unknown` (pre-first-discovery) hold `Ready` at `Unknown`, + or is `Ready` allowed to settle on the other axes first? (Proposal: `Watches`, and + `Unknown` holds `Ready=Unknown` — an unconfirmed source is not yet Ready.) diff --git a/docs/design/multi-cluster-audit-ingestion-implications.md b/docs/design/multi-cluster-audit-ingestion-implications.md index b104b0e8..71f2d2a2 100644 --- a/docs/design/multi-cluster-audit-ingestion-implications.md +++ b/docs/design/multi-cluster-audit-ingestion-implications.md @@ -1,6 +1,15 @@ # Multi-Cluster Audit Ingestion: Implications and Configuration Mapping > **design** — open, not yet built. Index: [`../INDEX.md`](../INDEX.md) +> +> **Partly superseded.** §5's dedicated `SourceCluster` CRD proposal is replaced by +> [`config-plane-split.md`](config-plane-split.md), which puts remote-cluster +> connectivity **inline on `GitTarget`** (Flux's idiom) rather than in a separate +> onboarding CRD. That proposal's load-bearing rationale — fusing per-cluster +> **audit identity** with kube-API connectivity — no longer holds: the +> `/audit-webhook/` path was removed, so multi-cluster is now purely a +> kube-API story. The rest of this document (per-cluster **audit ingestion**, +> routing, fairness, quotas) remains an open, separate workstream. ## 1. Purpose diff --git a/go.mod b/go.mod index 8bcd4fb1..8360a8bb 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( filippo.io/age v1.3.1 github.com/alicebob/miniredis/v2 v2.38.0 github.com/cespare/xxhash/v2 v2.3.0 + github.com/fluxcd/pkg/apis/meta v1.31.0 github.com/go-git/go-billy/v5 v5.9.0 github.com/go-git/go-git/v5 v5.19.1 github.com/go-logr/logr v1.4.3 diff --git a/go.sum b/go.sum index 027002a0..26d52a69 100644 --- a/go.sum +++ b/go.sum @@ -56,6 +56,8 @@ github.com/evanphx/json-patch/v5 v5.9.11 h1:/8HVnzMq13/3x9TPvjG08wUGqBTmZBsCWzjT github.com/evanphx/json-patch/v5 v5.9.11/go.mod h1:3j+LviiESTElxA4p3EMKAB9HXj3/XEtnUf6OZxqIQTM= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= +github.com/fluxcd/pkg/apis/meta v1.31.0 h1:5niQvTirK0wTE0TfRjnUSdmu6GTSbAFzrdnovtZ9rJ8= +github.com/fluxcd/pkg/apis/meta v1.31.0/go.mod h1:Gx+YRq26a+mTbCjotSXC7/6kSSyo0zXQ8JnsEXf2vVk= github.com/fsnotify/fsnotify v1.10.1 h1:b0/UzAf9yR5rhf3RPm9gf3ehBPpf0oZKIjtpKrx59Ho= github.com/fsnotify/fsnotify v1.10.1/go.mod h1:TLheqan6HD6GBK6PrDWyDPBaEV8LspOxvPSjC+bVfgo= github.com/fxamacker/cbor/v2 v2.9.2 h1:X4Ksno9+x3cz0TZv69ec1hxP/+tymuR8PXQJyDwfh78= diff --git a/hack/e2e/inject-webhook-tls.sh b/hack/e2e/inject-webhook-tls.sh index 7fee488a..b0dfe8b4 100755 --- a/hack/e2e/inject-webhook-tls.sh +++ b/hack/e2e/inject-webhook-tls.sh @@ -22,7 +22,11 @@ NAMESPACE="${NAMESPACE:-gitops-reverser}" WEBHOOK_CONFIG="${WEBHOOK_CONFIG:-test/e2e/cluster/audit/webhook-config.yaml}" CONTROLLER_DEPLOY_SELECTOR="${CONTROLLER_DEPLOY_SELECTOR:-app.kubernetes.io/part-of=gitops-reverser}" CERT_READY_TIMEOUT="${CERT_READY_TIMEOUT:-120s}" -MANAGER_ROLLOUT_TIMEOUT="${MANAGER_ROLLOUT_TIMEOUT:-180s}" +# 300s (was 180s): restarting the k3d SERVER node reschedules the manager pod, and on a +# single-node CI runner that means the whole control plane comes back at once — heavier than the +# initial deploy, which already gets 300s. The extra headroom only affects a slow recovery; a +# healthy manager is back in well under a minute and never reaches the timeout. +MANAGER_ROLLOUT_TIMEOUT="${MANAGER_ROLLOUT_TIMEOUT:-300s}" AUDIT_WARMUP_TIMEOUT="${AUDIT_WARMUP_TIMEOUT:-150}" SERVER_CONTAINER="k3d-${CLUSTER_NAME}-server-0" WARMUP_NS="gitops-reverser-audit-warmup" @@ -63,8 +67,17 @@ resolve_manager_deploy() { # killed has rolled a Ready pod back out, so its audit ingress endpoint is up. wait_for_manager_rollout() { echo "⏳ Waiting for the manager (${MANAGER_DEPLOY}) to recover after the node restart..." - kubectl --context "${CTX}" -n "${NAMESPACE}" rollout status "${MANAGER_DEPLOY}" \ - --timeout="${MANAGER_ROLLOUT_TIMEOUT}" + if kubectl --context "${CTX}" -n "${NAMESPACE}" rollout status "${MANAGER_DEPLOY}" \ + --timeout="${MANAGER_ROLLOUT_TIMEOUT}"; then + return 0 + fi + echo "❌ Manager did not recover after the node restart; dumping diagnostics." >&2 + kubectl --context "${CTX}" -n "${NAMESPACE}" get pods -o wide >&2 || true + kubectl --context "${CTX}" -n "${NAMESPACE}" describe "${MANAGER_DEPLOY}" >&2 || true + kubectl --context "${CTX}" -n "${NAMESPACE}" logs "${MANAGER_DEPLOY}" --tail=80 >&2 || true + kubectl --context "${CTX}" get events -n "${NAMESPACE}" \ + --sort-by=.lastTimestamp 2>/dev/null | tail -30 >&2 || true + return 1 } # warmup_audit_path drives a throwaway audited write on every iteration and waits diff --git a/hack/e2e/setup-kcp.sh b/hack/e2e/setup-kcp.sh new file mode 100755 index 00000000..777f07fa --- /dev/null +++ b/hack/e2e/setup-kcp.sh @@ -0,0 +1,68 @@ +#!/usr/bin/env bash +# Install the kcp control plane into the e2e cluster for the source-cluster corner. +# +# Two phases, because the instance CRs (RootShard/FrontProxy/Kubeconfig) are typed by CRDs +# the kcp-operator HelmRelease installs: +# 1. apply the base (namespace, etcd, PKI Issuer, kcp-operator HelmRepository+HelmRelease), +# then wait for the operator + its CRDs + etcd; +# 2. apply the instance CRs, then wait for the root shard, the front-proxy, and the admin +# kubeconfig Secret the operator mints. +# +# kcp-operator is installed BY Flux (a HelmRelease), like every other e2e dependency; this +# script only applies the manifests and waits, it does not imperatively install kcp itself. +# +# Env: CTX (kube context, required). Idempotent — safe to re-run against a warm cluster. +set -euo pipefail + +CTX="${CTX:?CTX (kube context) must be set}" +KCP_DIR="${KCP_DIR:-test/e2e/setup/kcp}" +KCP_WAIT_TIMEOUT="${KCP_WAIT_TIMEOUT:-300s}" + +kc() { kubectl --context "${CTX}" "$@"; } + +echo "⬇️ Applying kcp base (etcd, PKI issuer, kcp-operator HelmRelease)…" +kc apply -k "${KCP_DIR}/base" + +echo "⏳ Waiting for the kcp-operator HelmRelease to become Ready…" +if ! kc -n flux-system wait helmrelease/kcp-operator --for=condition=Ready --timeout="${KCP_WAIT_TIMEOUT}"; then + echo "ERROR: kcp-operator HelmRelease did not become Ready." >&2 + kc -n flux-system get helmrelease kcp-operator -o yaml | sed -n '/status:/,$p' >&2 || true + kc -n kcp-operator get pods >&2 || true + exit 1 +fi + +echo "⏳ Waiting for the kcp-operator CRDs to be Established…" +kc wait --for=condition=Established --timeout=120s \ + crd/rootshards.operator.kcp.io \ + crd/frontproxies.operator.kcp.io \ + crd/kubeconfigs.operator.kcp.io + +echo "⏳ Waiting for etcd…" +kc -n kcp rollout status statefulset/etcd --timeout=120s + +echo "⬇️ Applying the kcp instance (RootShard / FrontProxy / admin Kubeconfig)…" +kc apply -f "${KCP_DIR}/instance.yaml" + +echo "⏳ Waiting for the root shard and front-proxy Deployments…" +# The operator creates the Deployments a few seconds after the CRs are accepted; poll until +# they exist, then let rollout status block on availability. +for dep in root-kcp frontproxy-front-proxy; do + for _ in $(seq 1 40); do + kc -n kcp get deployment "${dep}" >/dev/null 2>&1 && break + sleep 3 + done + kc -n kcp rollout status "deployment/${dep}" --timeout="${KCP_WAIT_TIMEOUT}" +done + +echo "⏳ Waiting for the admin kubeconfig Secret (kcp/kcp-admin-kubeconfig)…" +for _ in $(seq 1 40); do + kc -n kcp get secret kcp-admin-kubeconfig >/dev/null 2>&1 && break + sleep 3 +done +kc -n kcp get secret kcp-admin-kubeconfig >/dev/null 2>&1 || { + echo "ERROR: kcp admin kubeconfig Secret was never minted." >&2 + kc -n kcp get kubeconfig admin -o yaml | sed -n '/status:/,$p' >&2 || true + exit 1 +} + +echo "✅ kcp control plane is ready (front-proxy: frontproxy-front-proxy.kcp.svc.cluster.local:6443)." diff --git a/internal/controller/gittarget_controller.go b/internal/controller/gittarget_controller.go index f16b8e57..1a1e3afb 100644 --- a/internal/controller/gittarget_controller.go +++ b/internal/controller/gittarget_controller.go @@ -29,6 +29,7 @@ import ( configbutleraiv1alpha3 "github.com/ConfigButler/gitops-reverser/api/v1alpha3" "github.com/ConfigButler/gitops-reverser/internal/git" + "github.com/ConfigButler/gitops-reverser/internal/kubeconfig" "github.com/ConfigButler/gitops-reverser/internal/reconcile" "github.com/ConfigButler/gitops-reverser/internal/types" "github.com/ConfigButler/gitops-reverser/internal/watch" @@ -106,6 +107,10 @@ type GitTargetReconciler struct { Scheme *runtime.Scheme WorkerManager *git.WorkerManager EventRouter *watch.EventRouter + // KubeConfigSafety is the exec / insecure-TLS opt-in applied by the Validated gate when a + // GitTarget names spec.kubeConfig. It mirrors the resolver's policy so the controller's + // legibility verdict and the data plane's dial never disagree on what is safe. + KubeConfigSafety kubeconfig.SafetyPolicy } // +kubebuilder:rbac:groups=configbutler.ai,resources=gittargets,verbs=get;list;watch;create;update;patch;delete @@ -134,6 +139,13 @@ func (r *GitTargetReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, validationErr } if !validated { + // A remote-source GitTarget whose kubeConfig stopped validating (Secret deleted, key gone, + // or contents now unsafe/unparseable) must have its data plane STOPPED, not merely marked + // blocked. Otherwise its watches keep mirroring the remote on the cached credential while + // status claims Validated=False. Forgetting the declaration cancels those watches and + // releases the source-cluster context; a later recovery re-declares and re-snapshots. A + // local GitTarget names no source cluster, so its streams are left untouched. + r.stopSourceClusterMirror(&target) r.setCondition( &target, GitTargetConditionEncryptionConfigured, @@ -210,6 +222,7 @@ func (r *GitTargetReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if declareErr := r.EventRouter.WatchManager.DeclareForGitTarget( ctx, gitDest, + target.SourceClusterID(), gitPathWasRefused, ); declareErr != nil { log.V(1).Info("stream declaration skipped; surface not observable", @@ -230,6 +243,24 @@ func (r *GitTargetReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( r.applyDataPlaneConditions(&target, streams, gitPath, renderFidelity) + // Project source-cluster reachability (runtime) and GitProvider readiness (destination-side) + // and fold both into Ready. This runs AFTER applyDataPlaneConditions so it can only downgrade + // Ready — a source/provider problem holds the target below Ready, but a healthy pair never + // overrides a still-replaying stream. + // A GitTarget with no source cluster mirrors the cluster the operator runs in, which is + // reachable by definition — so the default (used when no watch manager is wired, e.g. in + // tests) is True/LocalCluster for a local target and Unknown only for a remote one. + sourceReach := watch.SourceClusterReachableStatus{State: "True", Reason: "LocalCluster"} + if target.SourceClusterID() != "" { + sourceReach = watch.SourceClusterReachableStatus{State: "Unknown", Reason: "AwaitingDiscovery"} + } + if r.EventRouter != nil && r.EventRouter.WatchManager != nil { + sourceReach = r.EventRouter.WatchManager.SourceClusterReachable(target.SourceClusterID()) + } + providerStatus, providerReason, providerMessage := r.gitProviderReadiness(ctx, &target, providerNS) + r.projectSourceAndProvider(&target, sourceReach, providerStatus, providerReason, providerMessage) + streamsSettling = streamsSettling || sourceReach.State != "True" || providerStatus != metav1.ConditionTrue + if err := r.updateStatusWithRetry(ctx, &target); err != nil { return ctrl.Result{}, err } @@ -274,12 +305,25 @@ func (r *GitTargetReconciler) evaluateValidatedGate( return false, fmt.Sprintf("Validated gate failed: %s", GitTargetReasonInvalidConfig), nil, nil } + // spec.kubeConfig legibility gate: read/parse the kubeconfig Secret and apply the exec/TLS + // safety policy WITHOUT dialing. Reachability is a runtime observation the data plane records + // on SourceClusterReachable, so this only ever fails on a directly-readable input problem. + kcOK, kcReason, kcMsg, kcErr := r.validateKubeConfig(ctx, target) + if kcErr != nil { + return false, "", nil, kcErr + } + if !kcOK { + r.setCondition(target, GitTargetConditionValidated, metav1.ConditionFalse, kcReason, kcMsg) + result := ctrl.Result{RequeueAfter: RequeueSteadyInterval} + return false, fmt.Sprintf("Validated gate failed: %s", kcReason), &result, nil + } + r.setCondition( target, GitTargetConditionValidated, metav1.ConditionTrue, GitTargetReasonOK, - "Provider, branch, and placement policy validation passed", + "Provider, branch, placement, and kubeconfig validation passed", ) return true, "", nil, nil } @@ -353,6 +397,21 @@ func (r *GitTargetReconciler) evaluateWorkerWiringGate( // setBlockedDataPlane marks stream readiness as not-yet-evaluated when a control-plane gate // blocked the reconcile before watches could be declared. +// stopSourceClusterMirror tears down the data plane of a remote-source GitTarget that is no longer +// Validated, so a dead credential can never keep an active mirror running behind a Validated=False +// status. It is a no-op for a local GitTarget (nothing remote to stop) and idempotent across +// requeues while the target stays blocked. A later recovery re-declares the watches and re-snapshots. +func (r *GitTargetReconciler) stopSourceClusterMirror(target *configbutleraiv1alpha3.GitTarget) { + if target.SourceClusterID() == "" { + return + } + if r.EventRouter == nil || r.EventRouter.WatchManager == nil { + return + } + gitDest := types.NewResourceReference(target.Name, target.Namespace) + r.EventRouter.WatchManager.ForgetGitTargetDeclaration(gitDest) +} + func (r *GitTargetReconciler) setBlockedDataPlane(target *configbutleraiv1alpha3.GitTarget) { r.setCondition( target, diff --git a/internal/controller/gittarget_source_cluster.go b/internal/controller/gittarget_source_cluster.go new file mode 100644 index 00000000..3b468fe7 --- /dev/null +++ b/internal/controller/gittarget_source_cluster.go @@ -0,0 +1,199 @@ +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "context" + "fmt" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8stypes "k8s.io/apimachinery/pkg/types" + + configbutleraiv1alpha3 "github.com/ConfigButler/gitops-reverser/api/v1alpha3" + "github.com/ConfigButler/gitops-reverser/internal/kubeconfig" + "github.com/ConfigButler/gitops-reverser/internal/watch" +) + +const ( + // GitTargetConditionSourceClusterReachable is the RUNTIME reachability of the source + // cluster a GitTarget mirrors from: True (reason LocalCluster) when kubeConfig is omitted, + // Unknown before the data plane's first discovery, False after a real failed attempt. It is + // distinct from Validated: a missing/malformed kubeconfig is Validated=False (an input the + // controller reads without a network dial); an otherwise-valid kubeconfig whose API server + // cannot be contacted is SourceClusterReachable=False. + GitTargetConditionSourceClusterReachable = "SourceClusterReachable" + // GitTargetConditionGitProviderReady projects the referenced GitProvider's Ready onto the + // GitTarget, so one `kubectl get gittarget` separates source-side (SourceClusterReachable) + // from destination-side (GitProviderReady) failure. + GitTargetConditionGitProviderReady = "GitProviderReady" + + // GitTargetReasonGitProviderNotReady is the GitProviderReady=False reason. + GitTargetReasonGitProviderNotReady = "GitProviderNotReady" + // GitTargetReasonGitProviderReady is the GitProviderReady=True reason. + GitTargetReasonGitProviderReady = "GitProviderReady" +) + +// validateKubeConfig is the legibility gate for spec.kubeConfig (extends Validated). It reads +// and parses the kubeconfig Secret from the config plane and applies the exec/TLS safety +// policy, but deliberately does NOT dial the cluster — reachability is a runtime observation +// the data plane records on SourceClusterReachable. It returns ok=false with a typed +// KubeConfig* reason and a legible message when an input is wrong; a non-NotFound read error is +// returned as err so the reconcile requeues rather than falsely reporting a bad input. An +// omitted kubeConfig is trivially valid — the local cluster. +func (r *GitTargetReconciler) validateKubeConfig( + ctx context.Context, + target *configbutleraiv1alpha3.GitTarget, +) (bool, string, string, error) { + if target.Spec.KubeConfig == nil || target.Spec.KubeConfig.SecretRef == nil { + return true, "", "", nil + } + ref := target.Spec.KubeConfig.SecretRef + secretKey := k8stypes.NamespacedName{Namespace: target.Namespace, Name: ref.Name} + + var secret corev1.Secret + if getErr := r.Get(ctx, secretKey, &secret); getErr != nil { + if apierrors.IsNotFound(getErr) { + return false, kubeconfig.ReasonSecretNotFound, fmt.Sprintf( + "spec.kubeConfig.secretRef names Secret %s, which does not exist in this namespace", + secretKey), nil + } + return false, "", "", fmt.Errorf("read kubeconfig Secret %s: %w", secretKey, getErr) + } + + raw, usedKey, present := kubeconfig.ResolveKey(secret.Data, ref.Key) + if !present { + return false, kubeconfig.ReasonKeyNotFound, fmt.Sprintf( + "kubeconfig Secret %s has no kubeconfig under key %q (set spec.kubeConfig.secretRef.key)", + secretKey, describeKubeConfigKey(ref.Key)), nil + } + if rej := kubeconfig.Check(raw, r.KubeConfigSafety); rej != nil { + // A RejectionError is a validation VERDICT, not a reconcile error: it is surfaced as + // the Validated=False reason/message, and reconcile requeues normally (err == nil). + //nolint:nilerr + return false, rej.Reason, fmt.Sprintf("kubeconfig Secret %s key %q: %s", secretKey, usedKey, rej.Message), nil + } + return true, "", "", nil +} + +// describeKubeConfigKey renders the resolved-key hint for a "key not found" message. +func describeKubeConfigKey(specKey string) string { + if specKey == "" { + return "value or value.yaml" + } + return specKey +} + +// gitProviderReadiness reads the referenced GitProvider's Ready condition and projects it. It +// runs only after the Validated gate has confirmed the provider exists. It returns Unknown — +// which does NOT downgrade Ready — when the provider's readiness cannot be observed (a transient +// read error, or a provider that has not reported a Ready condition yet), so a not-yet-reconciled +// provider never blocks its GitTarget; only an EXPLICIT Ready=False downgrades. +func (r *GitTargetReconciler) gitProviderReadiness( + ctx context.Context, + target *configbutleraiv1alpha3.GitTarget, + providerNS string, +) (metav1.ConditionStatus, string, string) { + var gp configbutleraiv1alpha3.GitProvider + key := k8stypes.NamespacedName{Name: target.Spec.ProviderRef.Name, Namespace: providerNS} + if err := r.Get(ctx, key, &gp); err != nil { + return metav1.ConditionUnknown, GitTargetReasonGitProviderNotReady, + fmt.Sprintf("referenced GitProvider %s readiness not observed: %v", key, err) + } + c := findCondition(gp.Status.Conditions, ConditionTypeReady) + switch { + case c == nil: + return metav1.ConditionUnknown, GitTargetReasonGitProviderNotReady, + fmt.Sprintf("referenced GitProvider %s has not reported readiness yet", key) + case c.Status == metav1.ConditionTrue: + return metav1.ConditionTrue, GitTargetReasonGitProviderReady, + fmt.Sprintf("referenced GitProvider %s is Ready", key) + default: + msg := fmt.Sprintf("referenced GitProvider %s is not Ready", key) + if c.Message != "" { + msg = fmt.Sprintf("referenced GitProvider %s is not Ready: %s", key, c.Message) + } + return metav1.ConditionFalse, GitTargetReasonGitProviderNotReady, msg + } +} + +// findCondition returns the named condition, or nil. +func findCondition(conditions []metav1.Condition, conditionType string) *metav1.Condition { + for i := range conditions { + if conditions[i].Type == conditionType { + return &conditions[i] + } + } + return nil +} + +// projectSourceAndProvider sets the SourceClusterReachable and GitProviderReady conditions and +// folds them into the aggregate. It only ever DOWNGRADES Ready — a source-side or +// destination-side problem holds the target below Ready, but a healthy pair never overrides a +// data-plane stall (e.g. a still-replaying stream). Precedence: destination first +// (GitProviderReady, a stall the operator must fix), then source reachability (a transient the +// data plane retries — False is progressing, Unknown holds Ready at Unknown). +func (r *GitTargetReconciler) projectSourceAndProvider( + target *configbutleraiv1alpha3.GitTarget, + sourceReach watch.SourceClusterReachableStatus, + providerStatus metav1.ConditionStatus, + providerReason, providerMessage string, +) { + reachStatus := conditionStatusFromString(sourceReach.State) + r.setCondition( + target, + GitTargetConditionSourceClusterReachable, + reachStatus, + sourceReach.Reason, + sourceReach.Message, + ) + r.setCondition(target, GitTargetConditionGitProviderReady, providerStatus, providerReason, providerMessage) + + switch { + case providerStatus == metav1.ConditionFalse: + // Destination-side: the provider's own periodic check is failing. Wait for it to + // recover (progressing), which the Watches(&GitProvider{}) trigger promptly re-runs. + r.downgradeReady(target, metav1.ConditionFalse, providerReason, providerMessage, true) + case reachStatus == metav1.ConditionFalse: + // Source-side: an otherwise-valid kubeconfig whose API server cannot be contacted. A + // transient the data plane retries, so this is progressing, not stalled. + r.downgradeReady(target, metav1.ConditionFalse, sourceReach.Reason, sourceReach.Message, true) + case reachStatus == metav1.ConditionUnknown: + // An unconfirmed source is not yet Ready; hold Ready at Unknown until first discovery. + r.downgradeReady(target, metav1.ConditionUnknown, sourceReach.Reason, sourceReach.Message, true) + } +} + +// downgradeReady lowers the aggregate below Ready without ever raising it: it is called only on +// a source/provider problem, and rewrites Ready plus the kstatus Reconciling/Stalled pair to +// match. progressing=true means the condition is expected to clear on its own (Reconciling), +// false means it needs action (Stalled). +func (r *GitTargetReconciler) downgradeReady( + target *configbutleraiv1alpha3.GitTarget, + readyStatus metav1.ConditionStatus, + reason, message string, + progressing bool, +) { + r.setCondition(target, GitTargetConditionReady, readyStatus, reason, message) + if progressing { + r.setCondition(target, GitTargetConditionReconciling, metav1.ConditionTrue, reason, message) + r.setCondition(target, GitTargetConditionStalled, metav1.ConditionFalse, ReasonProgressing, + "Reconciliation is making progress") + return + } + r.setCondition(target, GitTargetConditionReconciling, metav1.ConditionFalse, reason, "Reconciliation is stalled") + r.setCondition(target, GitTargetConditionStalled, metav1.ConditionTrue, reason, message) +} + +// conditionStatusFromString maps the watch layer's "True"/"False"/"Unknown" onto the API type. +func conditionStatusFromString(state string) metav1.ConditionStatus { + switch state { + case "True": + return metav1.ConditionTrue + case "False": + return metav1.ConditionFalse + default: + return metav1.ConditionUnknown + } +} diff --git a/internal/controller/gittarget_source_cluster_test.go b/internal/controller/gittarget_source_cluster_test.go new file mode 100644 index 00000000..f5c41e5c --- /dev/null +++ b/internal/controller/gittarget_source_cluster_test.go @@ -0,0 +1,187 @@ +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "context" + "testing" + + meta "github.com/fluxcd/pkg/apis/meta" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + configbutleraiv1alpha3 "github.com/ConfigButler/gitops-reverser/api/v1alpha3" + "github.com/ConfigButler/gitops-reverser/internal/kubeconfig" +) + +const scValidKubeConfig = `apiVersion: v1 +kind: Config +clusters: +- {name: c, cluster: {server: https://192.0.2.1:6443, certificate-authority-data: dGVzdA==}} +contexts: +- {name: c, context: {cluster: c, user: u}} +current-context: c +users: +- {name: u, user: {token: t}} +` + +const scExecKubeConfig = `apiVersion: v1 +kind: Config +clusters: +- {name: c, cluster: {server: https://192.0.2.1:6443}} +contexts: +- {name: c, context: {cluster: c, user: u}} +current-context: c +users: +- name: u + user: + exec: {apiVersion: client.authentication.k8s.io/v1, command: /bin/echo, interactiveMode: Never} +` + +const scInsecureKubeConfig = `apiVersion: v1 +kind: Config +clusters: +- {name: c, cluster: {server: https://192.0.2.1:6443, insecure-skip-tls-verify: true}} +contexts: +- {name: c, context: {cluster: c, user: u}} +current-context: c +users: +- {name: u, user: {token: t}} +` + +func scScheme(t *testing.T) *runtime.Scheme { + t.Helper() + s := runtime.NewScheme() + require.NoError(t, configbutleraiv1alpha3.AddToScheme(s)) + require.NoError(t, corev1.AddToScheme(s)) + return s +} + +func gitTargetWithKubeConfig(secretName, key string) *configbutleraiv1alpha3.GitTarget { + t := &configbutleraiv1alpha3.GitTarget{ + ObjectMeta: metav1.ObjectMeta{Name: "gt", Namespace: "team-a"}, + } + if secretName != "" { + t.Spec.KubeConfig = &meta.KubeConfigReference{ + SecretRef: &meta.SecretKeyReference{Name: secretName, Key: key}, + } + } + return t +} + +func TestValidateKubeConfig_AllScenarios(t *testing.T) { + tests := []struct { + name string + secretName string + key string + secretData map[string][]byte + safety kubeconfig.SafetyPolicy + wantOK bool + wantReason string + }{ + {name: "omitted is valid (local cluster)", secretName: "", wantOK: true}, + {name: "missing Secret", secretName: "absent", wantOK: false, wantReason: kubeconfig.ReasonSecretNotFound}, + { + name: "missing key", secretName: "kc", key: "value", + secretData: map[string][]byte{"elsewhere": []byte(scValidKubeConfig)}, + wantOK: false, wantReason: kubeconfig.ReasonKeyNotFound, + }, + { + name: "unparseable", secretName: "kc", + secretData: map[string][]byte{"value": []byte("not a kubeconfig")}, + wantOK: false, wantReason: kubeconfig.ReasonInvalid, + }, + { + name: "exec rejected", secretName: "kc", + secretData: map[string][]byte{"value": []byte(scExecKubeConfig)}, + wantOK: false, wantReason: kubeconfig.ReasonExecNotAllowed, + }, + { + name: "insecure TLS rejected", secretName: "kc", + secretData: map[string][]byte{"value": []byte(scInsecureKubeConfig)}, + wantOK: false, wantReason: kubeconfig.ReasonInsecureTLSNotAllowed, + }, + { + name: "valid via value.yaml fallback", secretName: "kc", + secretData: map[string][]byte{"value.yaml": []byte(scValidKubeConfig)}, + wantOK: true, + }, + { + name: "exec allowed when opted in", secretName: "kc", + secretData: map[string][]byte{"value": []byte(scExecKubeConfig)}, + safety: kubeconfig.SafetyPolicy{AllowExec: true}, + wantOK: true, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + builder := fake.NewClientBuilder().WithScheme(scScheme(t)) + if tc.secretData != nil { + builder = builder.WithObjects(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "kc"}, + Data: tc.secretData, + }) + } + r := &GitTargetReconciler{Client: builder.Build(), KubeConfigSafety: tc.safety} + ok, reason, msg, err := r.validateKubeConfig( + context.Background(), + gitTargetWithKubeConfig(tc.secretName, tc.key), + ) + require.NoError(t, err) + assert.Equal(t, tc.wantOK, ok) + if !tc.wantOK { + assert.Equal(t, tc.wantReason, reason) + assert.NotEmpty(t, msg) + } + }) + } +} + +func TestGitProviderReadiness_AllScenarios(t *testing.T) { + provider := func(conds []metav1.Condition) *configbutleraiv1alpha3.GitProvider { + return &configbutleraiv1alpha3.GitProvider{ + ObjectMeta: metav1.ObjectMeta{Name: "prov", Namespace: "team-a"}, + Status: configbutleraiv1alpha3.GitProviderStatus{Conditions: conds}, + } + } + ready := metav1.Condition{Type: ConditionTypeReady, Status: metav1.ConditionTrue, Reason: "OK"} + notReady := metav1.Condition{ + Type: ConditionTypeReady, + Status: metav1.ConditionFalse, + Reason: "BadRepo", + Message: "no repo", + } + + tests := []struct { + name string + gp *configbutleraiv1alpha3.GitProvider + want metav1.ConditionStatus + }{ + {"ready", provider([]metav1.Condition{ready}), metav1.ConditionTrue}, + {"not ready -> False (downgrades)", provider([]metav1.Condition{notReady}), metav1.ConditionFalse}, + {"no condition -> Unknown (does not downgrade)", provider(nil), metav1.ConditionUnknown}, + {"absent provider -> Unknown", nil, metav1.ConditionUnknown}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + builder := fake.NewClientBuilder().WithScheme(scScheme(t)) + if tc.gp != nil { + builder = builder.WithObjects(tc.gp) + } + r := &GitTargetReconciler{Client: builder.Build()} + target := &configbutleraiv1alpha3.GitTarget{ + ObjectMeta: metav1.ObjectMeta{Name: "gt", Namespace: "team-a"}, + Spec: configbutleraiv1alpha3.GitTargetSpec{ + ProviderRef: configbutleraiv1alpha3.GitProviderReference{Name: "prov"}, + }, + } + status, _, msg := r.gitProviderReadiness(context.Background(), target, "team-a") + assert.Equal(t, tc.want, status) + assert.NotEmpty(t, msg) + }) + } +} diff --git a/internal/git/branch_worker.go b/internal/git/branch_worker.go index 21243645..326bbe54 100644 --- a/internal/git/branch_worker.go +++ b/internal/git/branch_worker.go @@ -75,10 +75,18 @@ type BranchWorker struct { Log logr.Logger contentWriter *contentWriter // mapper resolves manifest GVKs into resource identities while building the - // GitTarget inventory. A nil mapper keeps the writer structure-only, so - // object-less deletes have no resource index to target. + // GitTarget inventory, for the LOCAL cluster and as the fallback when no source + // cluster is named. A nil mapper keeps the writer structure-only, so object-less + // deletes have no resource index to target. mapper typeset.Lookup + // clusterMapper resolves the GVK->GVR lookup for a NAMED source cluster: a folder that + // mirrors a remote resolves its documents against that remote's type registry, never a + // union of all clusters (two clusters can serve one GVK under different GVRs/scopes). Nil + // in the CLI and in tests, which fall back to `mapper`; wired at startup by the + // WorkerManager from the watch manager's ClusterTypeLookup. + clusterMapper func(clusterID string) typeset.Lookup + // sshHostKeys configures SSH host-key resolution (install-level default ConfigMap and the // dev-only missing-key opt-out) for this worker's credential reads. Set by the WorkerManager // before Start, on the same goroutine the event loop reads it from. diff --git a/internal/git/pending_writes.go b/internal/git/pending_writes.go index 0d481d52..c0f2f89c 100644 --- a/internal/git/pending_writes.go +++ b/internal/git/pending_writes.go @@ -168,6 +168,7 @@ func (w *BranchWorker) resolveTargetMetadata( BootstrapOptions: buildBootstrapOptions(encryptionConfig), EncryptionConfig: encryptionConfig, Placement: resolvePlacementPolicy(target.Spec.Placement), + SourceClusterID: target.SourceClusterID(), }, nil } diff --git a/internal/git/placement_test.go b/internal/git/placement_test.go index c2cb642c..1935744e 100644 --- a/internal/git/placement_test.go +++ b/internal/git/placement_test.go @@ -682,7 +682,7 @@ func TestPlacement_ColdBundleCollision_ViaResync(t *testing.T) { } w := &BranchWorker{contentWriter: newContentWriter(types.SensitiveResourcePolicy{}), mapper: configMapMapper()} - _, changed, err := w.applyResyncToWorktree(context.Background(), worktree, "", desired, nil, policy) + _, changed, err := w.applyResyncToWorktree(context.Background(), worktree, "", "", desired, nil, policy) require.NoError(t, err) assert.True(t, changed) diff --git a/internal/git/plan_flush.go b/internal/git/plan_flush.go index 27adb2ab..498441a1 100644 --- a/internal/git/plan_flush.go +++ b/internal/git/plan_flush.go @@ -40,6 +40,30 @@ import ( // a single-identity intent — an upsert (create/patch/replace) for an object-bearing // event, or a delete-document for a DELETE — and the writer NEVER mark-and-sweeps a // batch. Whole-folder mark-and-sweep is the resync mechanism (M8), not steady state. +// mapperForCluster returns the GVK->GVR lookup for a source cluster: the per-cluster registry +// when a cluster is named and a cluster resolver is wired, else the default (local) mapper. +// The CLI and tests leave clusterMapper nil, so they always resolve against `mapper`. +func (w *BranchWorker) mapperForCluster(clusterID string) typeset.Lookup { + if clusterID != "" && w.clusterMapper != nil { + if lk := w.clusterMapper(clusterID); lk != nil { + return lk + } + } + return w.mapper +} + +// clusterIDForEvents returns the source cluster the events in one base belong to. Events in a +// single flush share a GitTarget (they are grouped by base), so they share a cluster; the +// first non-empty id wins, and an all-empty set is the local cluster. +func clusterIDForEvents(events []Event) string { + for _, ev := range events { + if ev.SourceClusterID != "" { + return ev.SourceClusterID + } + } + return "" +} + func (w *BranchWorker) flushEventsToWorktree( ctx context.Context, worktree *gogit.Worktree, @@ -53,7 +77,10 @@ func (w *BranchWorker) flushEventsToWorktree( return false, err } - batch := newWriteBatch(ctx, w.contentWriter, w.mapper, scoped.scan, policy, scoped.writeSubdir) + // Every event in a base shares one GitTarget (events are grouped by base), so they share + // one source cluster; resolve this subtree's GVK->GVR against that cluster's registry. + mapper := w.mapperForCluster(clusterIDForEvents(events)) + batch := newWriteBatch(ctx, w.contentWriter, mapper, scoped.scan, policy, scoped.writeSubdir) if err := batch.refusal(); err != nil { return false, err } diff --git a/internal/git/render_fidelity_test.go b/internal/git/render_fidelity_test.go index 93668bd3..b489e27f 100644 --- a/internal/git/render_fidelity_test.go +++ b/internal/git/render_fidelity_test.go @@ -67,7 +67,7 @@ func TestRenderFidelityRefusal_BlocksLiveAndResyncWrites(t *testing.T) { name: "scoped resync", run: func(worker *BranchWorker, worktree *gogit.Worktree) error { _, _, err := worker.applyResyncToWorktree( - context.Background(), worktree, "", + context.Background(), worktree, "", "", []manifestanalyzer.DesiredResource{{ Resource: postBuildTokenEvent().Identifier, Object: postBuildTokenEvent().Object, diff --git a/internal/git/resync_flush.go b/internal/git/resync_flush.go index 630932bd..b272f090 100644 --- a/internal/git/resync_flush.go +++ b/internal/git/resync_flush.go @@ -230,7 +230,7 @@ func (w *BranchWorker) executeResyncPendingWrite( target := pendingWrite.Target() base := sanitizePath(target.Path) - if err := w.refuseUnsafeWorktree(ctx, worktree, base); err != nil { + if err := w.refuseUnsafeWorktree(ctx, worktree, base, target.SourceClusterID); err != nil { return 0, err } @@ -248,7 +248,7 @@ func (w *BranchWorker) executeResyncPendingWrite( } stats, anyChanges, err := w.applyResyncToWorktree( - ctx, worktree, base, pendingWrite.Desired, pendingWrite.ScopeGVR, target.Placement, + ctx, worktree, base, target.SourceClusterID, pendingWrite.Desired, pendingWrite.ScopeGVR, target.Placement, ) if err != nil { return 0, err @@ -289,14 +289,18 @@ func (w *BranchWorker) executeResyncPendingWrite( return 1, nil } -func (w *BranchWorker) refuseUnsafeWorktree(ctx context.Context, worktree *gogit.Worktree, base string) error { +func (w *BranchWorker) refuseUnsafeWorktree( + ctx context.Context, + worktree *gogit.Worktree, + base, clusterID string, +) error { root := worktree.Filesystem.Root() scoped, err := scanRenderScope(root, base) if err != nil { return err } // The acceptance gate never places a resource, so no placement policy is needed here. - batch := newWriteBatch(ctx, w.contentWriter, w.mapper, scoped.scan, nil, scoped.writeSubdir) + batch := newWriteBatch(ctx, w.contentWriter, w.mapperForCluster(clusterID), scoped.scan, nil, scoped.writeSubdir) return batch.refusal() } @@ -323,7 +327,7 @@ func (w *BranchWorker) refuseUnsafeWorktree(ctx context.Context, worktree *gogit func (w *BranchWorker) applyResyncToWorktree( ctx context.Context, worktree *gogit.Worktree, - base string, + base, clusterID string, desired []manifestanalyzer.DesiredResource, scopeGVR *schema.GroupVersionResource, policy *manifestanalyzer.PlacementPolicy, @@ -334,7 +338,7 @@ func (w *BranchWorker) applyResyncToWorktree( return ResyncStats{}, false, err } - batch := newWriteBatch(ctx, w.contentWriter, w.mapper, scoped.scan, policy, scoped.writeSubdir) + batch := newWriteBatch(ctx, w.contentWriter, w.mapperForCluster(clusterID), scoped.scan, policy, scoped.writeSubdir) // First materialization is the adoption gate: refuse a subtree that holds content the // operator cannot safely manage (unsupported kustomization, duplicate identity, impure // or non-KRM files, foreign content, a catastrophic .gittargetignore) and commit nothing, diff --git a/internal/git/resync_flush_test.go b/internal/git/resync_flush_test.go index e3c8c34b..2d231a4e 100644 --- a/internal/git/resync_flush_test.go +++ b/internal/git/resync_flush_test.go @@ -65,7 +65,7 @@ func applyResyncViaWorktree( ) (ResyncStats, bool) { t.Helper() w := &BranchWorker{contentWriter: writer, mapper: mapper} - stats, changed, err := w.applyResyncToWorktree(context.Background(), worktree, "", desired, nil, nil) + stats, changed, err := w.applyResyncToWorktree(context.Background(), worktree, "", "", desired, nil, nil) require.NoError(t, err) return stats, changed } @@ -222,7 +222,7 @@ func TestResync_ScopedSweepDropsOnlyTargetType(t *testing.T) { w := &BranchWorker{contentWriter: writer, mapper: twoTypeMapper()} scope := &schema.GroupVersionResource{Group: "", Version: "v1", Resource: "configmaps"} - stats, changed, err := w.applyResyncToWorktree(context.Background(), worktree, "", nil, scope, nil) + stats, changed, err := w.applyResyncToWorktree(context.Background(), worktree, "", "", nil, scope, nil) require.NoError(t, err) require.True(t, changed, "the removed type's document is swept") assert.Equal(t, 1, stats.Deleted, "exactly the configmap is swept, not the secret") @@ -314,7 +314,7 @@ func TestResync_UnsafePlacementCountsAsPlacementSkipped(t *testing.T) { } w := &BranchWorker{contentWriter: writer, mapper: twoTypeMapper()} - stats, _, err := w.applyResyncToWorktree(context.Background(), worktree, "", desired, nil, policy) + stats, _, err := w.applyResyncToWorktree(context.Background(), worktree, "", "", desired, nil, policy) require.NoError(t, err) assert.Equal(t, 1, stats.PlacementSkipped, @@ -363,6 +363,7 @@ func TestResync_SensitiveUpdateCountsAsUpdatedNotSkipped(t *testing.T) { context.Background(), worktree, "", + "", []manifestanalyzer.DesiredResource{desired}, nil, nil, diff --git a/internal/git/types.go b/internal/git/types.go index 451dc84f..d380944d 100644 --- a/internal/git/types.go +++ b/internal/git/types.go @@ -158,6 +158,11 @@ type ResolvedTargetMetadata struct { // from spec.placement. Nil when the GitTarget declares none, in which case new // resources are placed by sibling inference and then the canonical path. Placement *manifestanalyzer.PlacementPolicy + // SourceClusterID is the id of the cluster the GitTarget mirrors from — + // (api/v1alpha3).GitTarget.SourceClusterID(), "" for the local cluster. The resync + // mark-and-sweep resolves this subtree's documents' GVK->GVR against that cluster's + // registry, so a folder mirroring a remote is swept against the right cluster's mapping. + SourceClusterID string } // PendingWrite is the unit retained until a push succeeds. @@ -336,6 +341,12 @@ type Event struct { // GitTargetNamespace is the namespace of the target owning this event. GitTargetNamespace string + // SourceClusterID is the id of the cluster this object was watched on — + // (api/v1alpha3).GitTarget.SourceClusterID(), "" for the local cluster. The writer + // resolves this document's GVK->GVR against that cluster's type registry, so a folder + // mirroring a remote is never indexed against the local cluster's mapping. + SourceClusterID string + // BootstrapOptions controls path-scoped bootstrap file staging for this event. BootstrapOptions pathBootstrapOptions } diff --git a/internal/git/worker_manager.go b/internal/git/worker_manager.go index 61e8ea60..738da226 100644 --- a/internal/git/worker_manager.go +++ b/internal/git/worker_manager.go @@ -35,8 +35,14 @@ type WorkerManager struct { ctx context.Context // mapper is the GVK->GVR resolver injected into every worker so store scans build a // resource-identity inventory. It is set once at startup (SetMapper) before any - // worker is created; a nil mapper keeps workers structure-only. + // worker is created; a nil mapper keeps workers structure-only. It is the LOCAL cluster's + // resolver and the fallback when a GitTarget names no source cluster. mapper typeset.Lookup + // clusterMapper resolves the GVK->GVR lookup for a NAMED source cluster, so a worker + // serving a GitTarget that mirrors a remote resolves that folder against the remote's + // registry — never a union. Set once at startup (SetClusterMapper); nil in the CLI and + // in tests, which fall back to `mapper`. + clusterMapper func(clusterID string) typeset.Lookup // sshHostKeys configures SSH host-key resolution for every worker's credential reads. Set // once at startup (SetSSHHostKeyConfig) before any worker is created. @@ -91,6 +97,15 @@ func (m *WorkerManager) SetMapper(mapper typeset.Lookup) { m.mapper = mapper } +// SetClusterMapper injects the per-source-cluster GVK->GVR resolver used by every worker's +// store scan when a GitTarget names a source cluster. Like SetMapper, it is called once at +// startup before any worker is created, so each worker created by EnsureWorker carries it. +func (m *WorkerManager) SetClusterMapper(resolver func(clusterID string) typeset.Lookup) { + m.mu.Lock() + defer m.mu.Unlock() + m.clusterMapper = resolver +} + // SetSSHHostKeyConfig injects the SSH host-key resolution config used by every worker's credential // reads. Like SetMapper, it is called once at startup before any worker is created. func (m *WorkerManager) SetSSHHostKeyConfig(cfg SSHHostKeyConfig) { @@ -164,6 +179,7 @@ func (m *WorkerManager) EnsureWorker( // goroutine Start spawns, so setting it here (under m.mu, before that goroutine // exists) is race-free. worker.mapper = m.mapper + worker.clusterMapper = m.clusterMapper worker.sshHostKeys = m.sshHostKeys worker.pathRefusal = m.pathRefusal worker.renderFidelityGate = m.renderFidelityGate diff --git a/internal/kubeconfig/kubeconfig.go b/internal/kubeconfig/kubeconfig.go new file mode 100644 index 00000000..5fa8d5ee --- /dev/null +++ b/internal/kubeconfig/kubeconfig.go @@ -0,0 +1,202 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Package kubeconfig resolves and safety-checks a source-cluster kubeconfig held in a Secret, +// shared by the watch data plane's SourceClusterResolver and the GitTarget controller's +// Validated gate so both apply exactly one contract: Flux's value→value.yaml key order, and a +// REJECT — not Flux's silent strip — of exec auth providers and insecure-skip-tls-verify. +// +// It depends only on client-go's clientcmd (which this repo already has) and never dials: the +// safety check is a few lines over the parsed kubeconfig, deliberately not a Flux import. +package kubeconfig + +import ( + "errors" + "fmt" + + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" +) + +// Reason strings map one-to-one onto the GitTarget Validated=False reasons the controller +// reports and the e2e plan asserts. They live here so the resolver's errors and the +// controller's condition reasons can never drift. +const ( + // ReasonSecretNotFound: spec.kubeConfig.secretRef names a Secret that does not exist. + ReasonSecretNotFound = "KubeConfigSecretNotFound" + // ReasonKeyNotFound: the Secret exists but has no kubeconfig under the resolved key. + ReasonKeyNotFound = "KubeConfigKeyNotFound" + // ReasonInvalid: the bytes are not a parseable kubeconfig. + ReasonInvalid = "KubeConfigInvalid" + // ReasonExecNotAllowed: the kubeconfig carries an exec auth provider (runs a binary in + // the operator Pod) and --insecure-kubeconfig-exec is not set. + ReasonExecNotAllowed = "KubeConfigExecNotAllowed" + // ReasonInsecureTLSNotAllowed: the kubeconfig sets insecure-skip-tls-verify and + // --insecure-kubeconfig-tls is not set. + ReasonInsecureTLSNotAllowed = "KubeConfigInsecureTLSNotAllowed" + // ReasonFileReferenceNotAllowed: the kubeconfig names a credential or CA by file PATH + // (tokenFile, client-certificate, client-key, or certificate-authority) instead of embedding + // it. client-go reads those paths from the operator Pod's OWN filesystem when it builds the + // REST config, so an operator-supplied (attacker-adjacent) kubeconfig could point at in-Pod + // Secrets and ship them to a remote API server it names. Always rejected — there is no safe + // opt-in, unlike exec/insecure-TLS: require the embedded *-data (or inline token) forms. + ReasonFileReferenceNotAllowed = "KubeConfigFileReferenceNotAllowed" +) + +// SafetyPolicy is the operator's opt-in to the two footguns this package rejects by default. +// Both default to false — the safe posture, diverging from Flux's silent strip. +type SafetyPolicy struct { + // AllowExec permits exec auth providers (--insecure-kubeconfig-exec). + AllowExec bool + // AllowInsecureTLS permits insecure-skip-tls-verify (--insecure-kubeconfig-tls). + AllowInsecureTLS bool +} + +// RejectionError is a typed, legible reason a kubeconfig was not accepted. It carries both the +// stable Reason (for a condition) and a human Message (for its text). +type RejectionError struct { + Reason string + Message string +} + +// Error implements error so a RejectionError can flow through the resolver's error return. +func (r *RejectionError) Error() string { return r.Message } + +// ResolveKey selects the kubeconfig bytes from a Secret's data, following Flux's order: +// an explicit spec key wins; otherwise "value" then "value.yaml". ok is false when no +// candidate key holds a non-empty value — the caller reports ReasonKeyNotFound. usedKey is +// the key the bytes came from, for legible messages. +func ResolveKey(data map[string][]byte, specKey string) ([]byte, string, bool) { + candidates := []string{specKey} + if specKey == "" { + candidates = []string{"value", "value.yaml"} + } + for _, key := range candidates { + if v, present := data[key]; present && len(v) > 0 { + return v, key, true + } + } + return nil, "", false +} + +// Check parses raw kubeconfig bytes and applies the safety policy WITHOUT dialing. A nil +// return means the kubeconfig is well-formed and permitted; a non-nil *RejectionError names exactly +// which input was wrong (Invalid / ExecNotAllowed / InsecureTLSNotAllowed). +func Check(raw []byte, policy SafetyPolicy) *RejectionError { + cfg, err := clientcmd.Load(raw) + if err != nil { + return &RejectionError{Reason: ReasonInvalid, Message: fmt.Sprintf("not a parseable kubeconfig: %v", err)} + } + return checkParsed(cfg, policy) +} + +// checkParsed is the safety half of Check, over an already-parsed config. +func checkParsed(cfg *clientcmdapi.Config, policy SafetyPolicy) *RejectionError { + if !policy.AllowExec { + for name, auth := range cfg.AuthInfos { + if auth != nil && auth.Exec != nil { + return &RejectionError{ + Reason: ReasonExecNotAllowed, + Message: fmt.Sprintf( + "kubeconfig user %q uses an exec auth provider, which runs a binary in the operator "+ + "Pod; rejected. Set --insecure-kubeconfig-exec to allow it, or use a token/cert credential.", + name), + } + } + } + } + if !policy.AllowInsecureTLS { + for name, cluster := range cfg.Clusters { + if cluster != nil && cluster.InsecureSkipTLSVerify { + return &RejectionError{ + Reason: ReasonInsecureTLSNotAllowed, + Message: fmt.Sprintf( + "kubeconfig cluster %q sets insecure-skip-tls-verify, which disables server certificate "+ + "validation; rejected. Set --insecure-kubeconfig-tls to allow it, or embed the CA.", + name), + } + } + } + } + if rej := checkNoFileReferences(cfg); rej != nil { + return rej + } + return nil +} + +// checkNoFileReferences rejects any credential or CA named by file PATH. client-go resolves +// tokenFile / client-certificate / client-key / certificate-authority against the process's own +// filesystem when it builds the REST config, so a remote kubeconfig that named an in-Pod path +// would make the operator read its own Secrets and send them to the server the kubeconfig points +// at. There is no legitimate reason for an operator-supplied remote kubeconfig to reference the +// operator Pod's files, and no safe opt-in — so this is unconditional (unlike exec/insecure-TLS). +func checkNoFileReferences(cfg *clientcmdapi.Config) *RejectionError { + for name, auth := range cfg.AuthInfos { + if auth == nil { + continue + } + if field := fileBackedAuthField(auth); field != "" { + return &RejectionError{ + Reason: ReasonFileReferenceNotAllowed, + Message: fmt.Sprintf( + "kubeconfig user %q names %s by file path, which client-go would read from the operator "+ + "Pod's filesystem; rejected. Embed the credential with its *-data field (client-"+ + "certificate-data / client-key-data) or use an inline token.", name, field), + } + } + } + for name, cluster := range cfg.Clusters { + if cluster != nil && cluster.CertificateAuthority != "" { + return &RejectionError{ + Reason: ReasonFileReferenceNotAllowed, + Message: fmt.Sprintf( + "kubeconfig cluster %q names certificate-authority by file path, which client-go would "+ + "read from the operator Pod's filesystem; rejected. Embed it with "+ + "certificate-authority-data.", name), + } + } + } + return nil +} + +// fileBackedAuthField reports the first file-path credential field an AuthInfo sets, or "". +func fileBackedAuthField(auth *clientcmdapi.AuthInfo) string { + switch { + case auth.ClientCertificate != "": + return "client-certificate" + case auth.ClientKey != "": + return "client-key" + case auth.TokenFile != "": + return "tokenFile" + } + return "" +} + +// BuildRESTConfig parses raw kubeconfig bytes, applies the safety policy, and returns the +// rest.Config to reach the cluster. It is the resolver's one call: parse → reject-unsafe → +// build. A *RejectionError is returned (as error) when the bytes are unusable, so the caller can +// surface the same typed reason the controller's Validated gate does. It never dials. +func BuildRESTConfig(raw []byte, policy SafetyPolicy) (*rest.Config, error) { + if rej := Check(raw, policy); rej != nil { + return nil, rej + } + cfg, err := clientcmd.RESTConfigFromKubeConfig(raw) + if err != nil { + // Check already parsed successfully, so this is unusual; classify it as Invalid anyway. + return nil, &RejectionError{ + Reason: ReasonInvalid, + Message: fmt.Sprintf("build REST config from kubeconfig: %v", err), + } + } + return cfg, nil +} + +// AsRejection extracts the *RejectionError from an error chain, so a caller can read the typed +// Reason. It returns false for any other error. +func AsRejection(err error) (*RejectionError, bool) { + var rej *RejectionError + if errors.As(err, &rej) { + return rej, true + } + return nil, false +} diff --git a/internal/kubeconfig/kubeconfig_test.go b/internal/kubeconfig/kubeconfig_test.go new file mode 100644 index 00000000..586de5af --- /dev/null +++ b/internal/kubeconfig/kubeconfig_test.go @@ -0,0 +1,184 @@ +// SPDX-License-Identifier: Apache-2.0 + +package kubeconfig + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const validKubeConfig = `apiVersion: v1 +kind: Config +clusters: +- name: c + cluster: + server: https://192.0.2.1:6443 + certificate-authority-data: dGVzdA== +contexts: +- name: c + context: {cluster: c, user: u} +current-context: c +users: +- name: u + user: + token: dummy-token +` + +const execKubeConfig = `apiVersion: v1 +kind: Config +clusters: +- name: c + cluster: {server: https://192.0.2.1:6443} +contexts: +- name: c + context: {cluster: c, user: u} +current-context: c +users: +- name: u + user: + exec: + apiVersion: client.authentication.k8s.io/v1 + command: /bin/echo +` + +const insecureKubeConfig = `apiVersion: v1 +kind: Config +clusters: +- name: c + cluster: + server: https://192.0.2.1:6443 + insecure-skip-tls-verify: true +contexts: +- name: c + context: {cluster: c, user: u} +current-context: c +users: +- name: u + user: + token: dummy-token +` + +// tokenFileKubeConfig names a token by file path — client-go would read it from the operator +// Pod's filesystem. The CA is embedded so ONLY the file reference is the fault under test. +const tokenFileKubeConfig = `apiVersion: v1 +kind: Config +clusters: +- name: c + cluster: + server: https://192.0.2.1:6443 + certificate-authority-data: dGVzdA== +contexts: +- name: c + context: {cluster: c, user: u} +current-context: c +users: +- name: u + user: + tokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token +` + +// caFileKubeConfig names the cluster CA by file path — another in-Pod read vector. +const caFileKubeConfig = `apiVersion: v1 +kind: Config +clusters: +- name: c + cluster: + server: https://192.0.2.1:6443 + certificate-authority: /etc/ssl/certs/ca.crt +contexts: +- name: c + context: {cluster: c, user: u} +current-context: c +users: +- name: u + user: + token: dummy-token +` + +func TestResolveKey_ExplicitKeyWins(t *testing.T) { + data := map[string][]byte{"value": []byte("a"), "custom": []byte("b")} + raw, used, ok := ResolveKey(data, "custom") + require.True(t, ok) + assert.Equal(t, "custom", used) + assert.Equal(t, "b", string(raw)) +} + +func TestResolveKey_FallsBackValueThenValueYaml(t *testing.T) { + // value wins over value.yaml. + raw, used, ok := ResolveKey(map[string][]byte{"value": []byte("a"), "value.yaml": []byte("b")}, "") + require.True(t, ok) + assert.Equal(t, "value", used) + assert.Equal(t, "a", string(raw)) + + // value absent -> value.yaml (Flux's Kustomization Secret shape). + raw, used, ok = ResolveKey(map[string][]byte{"value.yaml": []byte("b")}, "") + require.True(t, ok) + assert.Equal(t, "value.yaml", used) + assert.Equal(t, "b", string(raw)) +} + +func TestResolveKey_MissingAndEmptyAreNotOK(t *testing.T) { + _, _, ok := ResolveKey(map[string][]byte{"elsewhere": []byte("x")}, "value") + assert.False(t, ok, "explicit key absent") + + _, _, ok = ResolveKey(map[string][]byte{"value": {}}, "") + assert.False(t, ok, "empty value is treated as absent") + + _, _, ok = ResolveKey(nil, "") + assert.False(t, ok, "no data") +} + +func TestCheck_RejectsUnsafeByDefault(t *testing.T) { + tests := []struct { + name string + raw string + wantReason string + }{ + {"garbage", "this is not a kubeconfig", ReasonInvalid}, + {"exec", execKubeConfig, ReasonExecNotAllowed}, + {"insecureTLS", insecureKubeConfig, ReasonInsecureTLSNotAllowed}, + {"tokenFile", tokenFileKubeConfig, ReasonFileReferenceNotAllowed}, + {"caFile", caFileKubeConfig, ReasonFileReferenceNotAllowed}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + rej := Check([]byte(tc.raw), SafetyPolicy{}) + require.NotNil(t, rej, "must reject") + assert.Equal(t, tc.wantReason, rej.Reason) + assert.NotEmpty(t, rej.Error()) + }) + } +} + +func TestCheck_AllowsSafeAndOptedIn(t *testing.T) { + assert.Nil(t, Check([]byte(validKubeConfig), SafetyPolicy{}), "a token kubeconfig is safe") + assert.Nil(t, Check([]byte(execKubeConfig), SafetyPolicy{AllowExec: true}), "exec opted in") + assert.Nil(t, Check([]byte(insecureKubeConfig), SafetyPolicy{AllowInsecureTLS: true}), "insecure TLS opted in") +} + +// A file-backed reference is a pod-filesystem read vector with no legitimate use for a remote +// kubeconfig, so it is rejected UNCONDITIONALLY — even with every safety opt-in enabled. +func TestCheck_FileReferencesRejectedEvenWhenOptedIn(t *testing.T) { + all := SafetyPolicy{AllowExec: true, AllowInsecureTLS: true} + for name, raw := range map[string]string{"tokenFile": tokenFileKubeConfig, "caFile": caFileKubeConfig} { + rej := Check([]byte(raw), all) + require.NotNil(t, rej, "%s must be rejected regardless of policy", name) + assert.Equal(t, ReasonFileReferenceNotAllowed, rej.Reason) + } +} + +func TestBuildRESTConfig(t *testing.T) { + cfg, err := BuildRESTConfig([]byte(validKubeConfig), SafetyPolicy{}) + require.NoError(t, err) + require.NotNil(t, cfg) + assert.Equal(t, "https://192.0.2.1:6443", cfg.Host) + + // A rejected kubeconfig surfaces the typed Rejection through the error chain. + _, err = BuildRESTConfig([]byte(execKubeConfig), SafetyPolicy{}) + require.Error(t, err) + rej, ok := AsRejection(err) + require.True(t, ok) + assert.Equal(t, ReasonExecNotAllowed, rej.Reason) +} diff --git a/internal/watch/cluster_context.go b/internal/watch/cluster_context.go new file mode 100644 index 00000000..fd811063 --- /dev/null +++ b/internal/watch/cluster_context.go @@ -0,0 +1,628 @@ +// SPDX-License-Identifier: Apache-2.0 + +package watch + +import ( + "context" + "fmt" + "sort" + "strings" + "sync" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/discovery" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" + + "github.com/ConfigButler/gitops-reverser/internal/kubeconfig" + "github.com/ConfigButler/gitops-reverser/internal/types" + "github.com/ConfigButler/gitops-reverser/internal/typeset" +) + +// LocalClusterID identifies the cluster the operator runs in — the config plane, which is +// also the watched cluster in a single-cluster install. It is the zero value on purpose: +// every code path that does not know about source clusters lands on it. It matches +// (api/v1alpha3).GitTarget.SourceClusterID()'s "" return for an omitted spec.kubeConfig. +const LocalClusterID = "" + +// SourceClusterResolver turns a GitTarget's source-cluster id into a rest.Config by reading +// the kubeconfig Secret it names from the config plane. It is an interface so the watch +// manager grows no Kubernetes client of its own for this, and so tests can stand up a +// remote cluster without a Secret. The concrete implementation lives in +// source_cluster_resolver.go. +type SourceClusterResolver interface { + // ResolveSourceCluster returns the rest.Config for a cluster id, and an opaque version + // token that changes when the underlying kubeconfig changes (the Secret's + // resourceVersion). An unknown or unreadable id is an error: mirroring the wrong + // cluster into a folder is worse than mirroring none. + ResolveSourceCluster(ctx context.Context, clusterID string) (cfg *rest.Config, version string, err error) +} + +// clusterContext holds everything that used to be a Manager-wide singleton and is in fact a +// property of ONE cluster: its API surface catalog, the followability registry derived from +// it, and the clients that reach it. +// +// A single-cluster install has exactly one, keyed by LocalClusterID, and behaves exactly as +// before — the trigger informers, the catalog refresh, and the type registry all land on it. +// A GitTarget that names a source cluster (spec.kubeConfig) gets its own, created on first +// use and torn down when the last such GitTarget is gone. +type clusterContext struct { + id string + + // catalog and registry are the "Scan -> Registry" pipeline for this cluster. A CRD + // installed only on the remote is followable only there — and, more importantly, a type + // served only locally never resolves for a remote target. + catalog *APIResourceCatalog + registry *typeset.Registry + + // clientsMu guards the client/config fields below. It is PER CLUSTER: resolving a + // remote cluster's kubeconfig reads a Secret from the config plane, and a slow apiserver + // must not block client construction for every other cluster behind one global lock. + clientsMu sync.Mutex + // restConfig is nil until the cluster is first reached. configVersion is the version + // token restConfig was built from; when it changes (a rotated kubeconfig Secret), the + // cached clients are dropped so the next use rebuilds them. The kubeconfig bytes are + // never retained — only the built rest.Config and the opaque version token survive. + restConfig *rest.Config + configVersion string + dynamicClient dynamic.Interface + discovery apiResourceDiscovery + + // Logging state, edge-triggered per cluster so a degraded remote does not silence the + // local cluster's transitions and vice versa. catalogReadyOnce synchronizes itself; the + // two maps are guarded by Manager.resourceCatalogMu. + catalogReadyOnce sync.Once + catalogDegradedLogged map[schema.GroupVersion]struct{} + typeRefusalsLogged map[string]string + + // reachable is the runtime reachability the data plane records after a real discovery + // attempt, projected onto every GitTarget on this cluster as SourceClusterReachable + // (see stream_readiness.go). It starts Unknown and is guarded by Manager.clustersMu. + reachable sourceClusterReachability +} + +// sourceClusterReachability is the tri-state a source cluster's SourceClusterReachable +// condition projects: Unknown before the first discovery attempt, then True/False after one. +type sourceClusterReachability struct { + state sourceClusterReachState + reason string + message string +} + +type sourceClusterReachState int + +const ( + // reachUnknown is the pre-first-discovery state. + reachUnknown sourceClusterReachState = iota + // reachTrue means the last discovery attempt reached the source API. + reachTrue + // reachFalse means the last discovery attempt failed to reach the source API. + reachFalse +) + +// SourceClusterReachable reasons, grouped by the state they set (see the design doc's +// "Status and conditions"). The local cluster is reachable by definition; a remote failure +// is classified by what the discovery attempt hit. +const ( + // reasonLocalCluster is the SourceClusterReachable=True reason when kubeConfig is omitted. + reasonLocalCluster = "LocalCluster" + // reasonSourceClusterReachable is the SourceClusterReachable=True reason for a remote whose + // API discovery succeeded. + reasonSourceClusterReachable = "SourceClusterReachable" + // reasonSourceClusterUnreachable is DNS/TCP/TLS/timeout — the API server could not be contacted. + reasonSourceClusterUnreachable = "SourceClusterUnreachable" + // reasonSourceClusterAuthFailed is a 401 during discovery — the credential was rejected. + reasonSourceClusterAuthFailed = "SourceClusterAuthenticationFailed" + // reasonSourceClusterAccessDenied is a 403 during discovery — the identity lacks read access. + reasonSourceClusterAccessDenied = "SourceClusterAccessDenied" +) + +func newClusterContext(id string) *clusterContext { + return &clusterContext{ + id: id, + catalog: NewAPIResourceCatalog(), + registry: typeset.NewRegistry(), + catalogDegradedLogged: map[schema.GroupVersion]struct{}{}, + typeRefusalsLogged: map[string]string{}, + } +} + +// isLocal reports whether this context is the cluster the operator runs in. +func (c *clusterContext) isLocal() bool { return c.id == LocalClusterID } + +// describeCluster renders a cluster id for logs. The local cluster has no name of its own. +func describeCluster(id string) string { + if id == LocalClusterID { + return "local" + } + return id +} + +// cluster returns the context for a cluster id, creating it on first use. The local context +// is created lazily too, which is what lets a zero-value Manager work in tests: its catalog +// is seeded from m.resourceCatalog when a test injected one, so the existing catalog-driven +// resolution keeps working unchanged. +func (m *Manager) cluster(id string) *clusterContext { + m.clustersMu.Lock() + defer m.clustersMu.Unlock() + if m.clusters == nil { + m.clusters = map[string]*clusterContext{} + } + if cc := m.clusters[id]; cc != nil { + return cc + } + cc := newClusterContext(id) + if id == LocalClusterID { + m.seedLocalClusterLocked(cc) + } else { + m.Log.Info("source cluster registered", "clusterID", id) + } + m.clusters[id] = cc + m.publishClusterOrderLocked() + return cc +} + +// seedLocalClusterLocked wires the local context's catalog to m.resourceCatalog (a test may +// have injected one; otherwise the two are aliased so apiResourceCatalog() and the context see +// the same object) and marks it reachable — the operator runs in it. Must hold clustersMu. +func (m *Manager) seedLocalClusterLocked(cc *clusterContext) { + if m.resourceCatalog != nil { + cc.catalog = m.resourceCatalog + } else { + m.resourceCatalog = cc.catalog + } + cc.reachable = sourceClusterReachability{state: reachTrue, reason: reasonLocalCluster} +} + +// localCluster is the config plane, and the watched cluster of every GitTarget that does +// not name a source. +func (m *Manager) localCluster() *clusterContext { return m.cluster(LocalClusterID) } + +// registryForGitTarget returns the followability registry of the cluster a GitTarget mirrors +// from — its OWN cluster's surface, never a union. A single-cluster GitTarget resolves against +// the one local registry, unchanged. +func (m *Manager) registryForGitTarget(gitDest types.ResourceReference) *typeset.Registry { + return m.cluster(m.clusterIDForGitTarget(gitDest)).registry +} + +// ClusterTypeLookup returns the GVK->GVR resolver the git writer scans a folder's manifests +// with, scoped to ONE source cluster — its own registry. A folder is owned by exactly one +// GitTarget (one materialization), so the writer resolves each document against that +// GitTarget's cluster, never a union: two clusters can validly serve one GVK under different +// GVRs/scopes, and a first-wins union would mis-file or delete manifests. In a single-cluster +// install every target resolves against the one local registry, unchanged. An unknown cluster +// id yields the (possibly unready) context registry, which fails closed via the acceptance gate. +func (m *Manager) ClusterTypeLookup(clusterID string) typeset.Lookup { + if cc := m.clusterContextByID(clusterID); cc != nil { + return cc.registry + } + return m.cluster(clusterID).registry +} + +// activeClusterIDs is every cluster some GitTarget currently mirrors from, plus the local +// one. The local cluster is always active: the operator's own CRs live there, and a +// rule-less install still refreshes its catalog. The remote ids come from the Declare-time +// capture (gitTargetClusters), not from the rules — spec.kubeConfig is immutable and a +// GitTarget property, so there is no rules-disagree window to reconcile. +func (m *Manager) activeClusterIDs() []string { + seen := map[string]struct{}{LocalClusterID: {}} + m.gitTargetClustersMu.Lock() + for _, id := range m.gitTargetClusters { + seen[id] = struct{}{} + } + m.gitTargetClustersMu.Unlock() + out := make([]string, 0, len(seen)) + for id := range seen { + out = append(out, id) + } + sort.Strings(out) + return out +} + +// rememberGitTargetCluster captures the source cluster a GitTarget mirrors from, keyed by +// GitTarget — the same capture-on-Declare pattern as rememberGitTargetUID. Because +// spec.kubeConfig is immutable, this is learned once and never changes for a given +// GitTarget, so there is no per-rule propagation and no cross-rule disagreement window. +func (m *Manager) rememberGitTargetCluster(gitDest types.ResourceReference, clusterID string) { + m.gitTargetClustersMu.Lock() + defer m.gitTargetClustersMu.Unlock() + if m.gitTargetClusters == nil { + m.gitTargetClusters = map[string]string{} + } + m.gitTargetClusters[gitDest.Key()] = clusterID +} + +// clusterIDForGitTarget resolves the source cluster of a GitTarget from the Declare-time +// capture, defaulting to the local cluster for a GitTarget that has not declared yet (a +// status read racing the first Declare) or that names no source cluster. +func (m *Manager) clusterIDForGitTarget(gitDest types.ResourceReference) string { + m.gitTargetClustersMu.Lock() + defer m.gitTargetClustersMu.Unlock() + return m.gitTargetClusters[gitDest.Key()] +} + +// forgetGitTargetCluster drops a deleted GitTarget's captured cluster and tears down that +// cluster's context when it was the last GitTarget mirroring from it. The local context is +// never torn down. Without this a deleted remote GitTarget would leak a discovery client and +// a dynamic client set for the life of the process. +func (m *Manager) forgetGitTargetCluster(gitDest types.ResourceReference) { + m.gitTargetClustersMu.Lock() + clusterID, had := m.gitTargetClusters[gitDest.Key()] + if had { + delete(m.gitTargetClusters, gitDest.Key()) + } + stillReferenced := false + for _, id := range m.gitTargetClusters { + if id == clusterID { + stillReferenced = true + break + } + } + m.gitTargetClustersMu.Unlock() + + if !had || clusterID == LocalClusterID || stillReferenced { + return + } + m.teardownCluster(clusterID) +} + +// teardownCluster drops a source cluster's context once no GitTarget references it: its +// clients and catalog/registry are released so a deleted remote GitTarget leaks nothing. The +// local cluster is never torn down. +func (m *Manager) teardownCluster(clusterID string) { + if clusterID == LocalClusterID { + return + } + m.clustersMu.Lock() + defer m.clustersMu.Unlock() + if _, ok := m.clusters[clusterID]; !ok { + return + } + delete(m.clusters, clusterID) + m.publishClusterOrderLocked() + m.Log.Info("source cluster torn down; no GitTarget mirrors from it", "clusterID", clusterID) +} + +// recordClusterReachability updates a source cluster's SourceClusterReachable state from the +// outcome of a discovery attempt. The local cluster is always reachable — the operator runs in +// it — so it is never touched here. A remote's failure class (unreachable / auth / access +// denied) is derived by classifySourceClusterReachFailure. +func (m *Manager) recordClusterReachability(cc *clusterContext, err error) { + if cc.isLocal() { + return + } + m.clustersMu.Lock() + defer m.clustersMu.Unlock() + if err == nil { + cc.reachable = sourceClusterReachability{state: reachTrue, reason: reasonSourceClusterReachable} + return + } + cc.reachable = classifySourceClusterReachFailure(err) +} + +// classifySourceClusterReachFailure maps a discovery-attempt error onto the +// SourceClusterReachable reason it should surface. A 401 is an authentication failure (the +// credential was rejected), a 403 is access denied (the identity lacks read on discovery), and +// everything else — DNS, TCP, TLS, timeout — is "unreachable". The message carries the raw +// error so the human fix is legible. +func classifySourceClusterReachFailure(err error) sourceClusterReachability { + reason := reasonSourceClusterUnreachable + switch { + case apierrors.IsUnauthorized(err): + reason = reasonSourceClusterAuthFailed + case apierrors.IsForbidden(err): + reason = reasonSourceClusterAccessDenied + } + return sourceClusterReachability{state: reachFalse, reason: reason, message: err.Error()} +} + +// SourceClusterReachableStatus is the kstatus-shaped projection of a source cluster's +// reachability, for the GitTarget controller to set as the SourceClusterReachable condition. +// State is "True" | "False" | "Unknown"; the controller maps it to metav1.ConditionStatus. +type SourceClusterReachableStatus struct { + State string + Reason string + Message string +} + +// reasonAwaitingDiscovery is the SourceClusterReachable=Unknown reason before the data plane +// has made its first discovery attempt against a remote source cluster. +const reasonAwaitingDiscovery = "AwaitingDiscovery" + +// SourceClusterReachable projects a source cluster's runtime reachability for a GitTarget's +// SourceClusterReachable condition. The local cluster is always reachable; a remote is Unknown +// until the data plane's first discovery attempt, then True or False with a classified reason. +func (m *Manager) SourceClusterReachable(clusterID string) SourceClusterReachableStatus { + r := m.clusterReachability(clusterID) + switch r.state { + case reachTrue: + reason := r.reason + if reason == "" { + reason = reasonSourceClusterReachable + } + return SourceClusterReachableStatus{State: "True", Reason: reason, Message: r.message} + case reachFalse: + return SourceClusterReachableStatus{State: "False", Reason: r.reason, Message: r.message} + case reachUnknown: + return SourceClusterReachableStatus{ + State: "Unknown", + Reason: reasonAwaitingDiscovery, + Message: "source cluster not yet reached; awaiting first discovery", + } + default: + return SourceClusterReachableStatus{State: "Unknown", Reason: reasonAwaitingDiscovery} + } +} + +// clusterReachability returns a snapshot of a cluster's SourceClusterReachable state for +// projection onto its GitTargets. An unknown id is reported Unknown. +func (m *Manager) clusterReachability(clusterID string) sourceClusterReachability { + if clusterID == LocalClusterID { + return sourceClusterReachability{state: reachTrue, reason: reasonLocalCluster} + } + m.clustersMu.Lock() + defer m.clustersMu.Unlock() + if cc, ok := m.clusters[clusterID]; ok { + return cc.reachable + } + return sourceClusterReachability{state: reachUnknown} +} + +// clusterRESTConfigLocked returns a cluster's rest.Config, resolving it on first use. +// +// It does NOT re-read the kubeconfig Secret when a config is already cached — that read is a +// config-plane API call, and this runs on the watch reconnect path, once per (GitTarget, GVR, +// scope) watch. Rotation is picked up by refreshClusterCredentials on the catalog-refresh +// cadence instead. Must be called with cc.clientsMu held. +func (m *Manager) clusterRESTConfigLocked(ctx context.Context, cc *clusterContext) (*rest.Config, error) { + if cc.restConfig != nil { + return cc.restConfig, nil + } + if cc.isLocal() { + cfg, err := ctrl.GetConfig() + if err != nil { + return nil, fmt.Errorf("no REST config for the local cluster: %w", err) + } + cc.restConfig = cfg + return cfg, nil + } + cfg, version, err := m.resolveRemoteConfig(ctx, cc) + if err != nil { + return nil, err + } + cc.restConfig = cfg + cc.configVersion = version + return cfg, nil +} + +// resolveRemoteConfig reads a remote cluster's kubeconfig Secret from the config plane. +func (m *Manager) resolveRemoteConfig(ctx context.Context, cc *clusterContext) (*rest.Config, string, error) { + if m.SourceClusters == nil { + return nil, "", fmt.Errorf("cannot reach source cluster %q: no source-cluster resolver configured", cc.id) + } + cfg, version, err := m.SourceClusters.ResolveSourceCluster(ctx, cc.id) + if err != nil { + return nil, "", fmt.Errorf("resolve source cluster %q: %w", cc.id, err) + } + if cfg == nil { + return nil, "", fmt.Errorf("resolve source cluster %q: nil REST config", cc.id) + } + return cfg, version, nil +} + +// refreshClusterCredentials re-reads a remote cluster's kubeconfig Secret on the catalog-refresh +// cadence (every 30s and on every rule change), never on the hot watch reconnect path. It is the +// data plane's half of the reconcile model: it does not watch Secrets, it RE-CHECKS them, and a +// changed or vanished value is the moment to clean up. On a value CHANGE (rotation, or a repoint at +// a different server) it rebuilds the cached clients and invalidates the active watches so they +// re-establish on the fresh client; on a definitive LOSS (Secret deleted, key gone, or contents now +// unsafe/unparseable) it drops the clients fail-closed and invalidates the watches so mirroring +// stops — the enqueued reconcile then holds each GitTarget Validated=False. A transient read error +// (slow apiserver, momentary blip) changes nothing: the next refresh retries. +func (m *Manager) refreshClusterCredentials(ctx context.Context, cc *clusterContext) { + if cc.isLocal() { + return + } + cfg, version, err := m.resolveRemoteConfig(ctx, cc) + if err != nil { + if isDefinitiveCredentialFailure(err) && m.dropClusterClients(cc) { + m.invalidateClusterWatches(cc.id) + } + return + } + + cc.clientsMu.Lock() + if cc.restConfig != nil && version == cc.configVersion { + cc.clientsMu.Unlock() + return + } + rotated := cc.restConfig != nil + if rotated { + m.Log.Info("source cluster kubeconfig changed; rebuilding clients and invalidating watches", + "clusterID", cc.id, "version", version) + } + cc.restConfig = cfg + cc.configVersion = version + cc.dynamicClient = nil + cc.discovery = nil + cc.clientsMu.Unlock() + + if rotated { + m.invalidateClusterWatches(cc.id) + } +} + +// invalidateClusterWatches cancels the active watches of every GitTarget mirroring from a cluster +// and enqueues those targets for reconcile. It is how a credential CHANGE or LOSS is cleaned up on +// the refresh cadence instead of waiting for a chance disconnect. The GitTarget->cluster mapping is +// kept (only the watches are cancelled), so the enqueued reconcile re-declares each target — which +// rebuilds its watch on the freshly-rebuilt client for a rotation, or holds it Validated=False for a +// revocation. No Secret watch is involved; this rides the existing catalog-refresh loop. +func (m *Manager) invalidateClusterWatches(clusterID string) { + m.gitTargetClustersMu.Lock() + affected := make([]types.ResourceReference, 0) + for key, id := range m.gitTargetClusters { + if id == clusterID { + affected = append(affected, resourceReferenceFromKey(key)) + } + } + m.gitTargetClustersMu.Unlock() + for _, gitDest := range affected { + m.forgetGitTargetWatches(gitDest) + m.enqueueGitPathChange(gitDest) + } +} + +// resourceReferenceFromKey reconstructs the ResourceReference a gitTargetClusters key encodes. The +// key is ResourceReference.Key() == "namespace/name"; neither a namespace nor a name can contain "/". +func resourceReferenceFromKey(key string) types.ResourceReference { + namespace, name, _ := strings.Cut(key, "/") + return types.NewResourceReference(name, namespace) +} + +// isDefinitiveCredentialFailure reports whether a source-cluster resolve error means the +// credential is now gone or unusable — as opposed to a transient read error worth retrying. A +// deleted Secret (NotFound) and any kubeconfig RejectionError (key not found, unsafe, or +// unparseable content) are definitive: retrying cannot make them succeed, so the cached clients +// must be dropped rather than reused. Both are unwrapped through the resolver's fmt.Errorf wrapping. +func isDefinitiveCredentialFailure(err error) bool { + if apierrors.IsNotFound(err) { + return true + } + _, rejected := kubeconfig.AsRejection(err) + return rejected +} + +// dropClusterClients releases a source cluster's cached REST/dynamic/discovery clients under the +// per-cluster lock, forcing the next use to re-resolve the credential and rebuild them. It is the +// fail-closed half of the credential refresh: called when the credential a cluster's clients were +// built from is definitively gone, so a watch reconnect cannot silently reuse a revoked credential. +// It reports whether it actually dropped anything (clients were cached), so the caller invalidates +// the watches only on the transition to gone — not on every subsequent refresh of a still-broken +// cluster. +func (m *Manager) dropClusterClients(cc *clusterContext) bool { + cc.clientsMu.Lock() + defer cc.clientsMu.Unlock() + if cc.restConfig == nil { + return false + } + m.Log.Info("source cluster credential no longer resolvable; dropping cached clients (fail-closed)", + "clusterID", cc.id) + cc.restConfig = nil + cc.configVersion = "" + cc.dynamicClient = nil + cc.discovery = nil + return true +} + +// clusterDynamicClient returns the dynamic client a cluster's watches and lists run on. +func (m *Manager) clusterDynamicClient(ctx context.Context, clusterID string) (dynamic.Interface, error) { + cc := m.cluster(clusterID) + + // Tests inject a fake client for the local cluster without a REST config at all. + if cc.isLocal() && m.dynamicClient != nil { + return m.dynamicClient, nil + } + + cc.clientsMu.Lock() + defer cc.clientsMu.Unlock() + if cc.dynamicClient != nil { + return cc.dynamicClient, nil + } + cfg, err := m.clusterRESTConfigLocked(ctx, cc) + if err != nil { + return nil, err + } + dc, err := dynamic.NewForConfig(cfg) + if err != nil { + return nil, fmt.Errorf("build dynamic client for cluster %q: %w", describeCluster(clusterID), err) + } + cc.dynamicClient = dc + return dc, nil +} + +// clusterDiscovery returns the discovery client backing a cluster's API-resource catalog. +func (m *Manager) clusterDiscovery(ctx context.Context, clusterID string) (apiResourceDiscovery, error) { + cc := m.cluster(clusterID) + + // Tests inject discovery for the local cluster without a REST config. + if cc.isLocal() && m.discoveryClient != nil { + return m.discoveryClient() + } + + cc.clientsMu.Lock() + defer cc.clientsMu.Unlock() + if cc.discovery != nil { + return cc.discovery, nil + } + cfg, err := m.clusterRESTConfigLocked(ctx, cc) + if err != nil { + return nil, err + } + discoCfg := cfg + if !cc.isLocal() { + // Discovery uses the legacy, non-context ServerGroupsAndResources(), and a remote config + // deliberately carries no request-level timeout (its watches must stay open). Bound the + // finite discovery call with a request timeout on a COPY, so a remote that accepts the + // connection but hangs on the response cannot stall the catalog refresh — without ever + // deadlining a watch built from the shared config. + discoCfg = rest.CopyConfig(cfg) + discoCfg.Timeout = sourceClusterDialTimeout + } + disco, err := discovery.NewDiscoveryClientForConfig(discoCfg) + if err != nil { + return nil, fmt.Errorf("create discovery client for cluster %q: %w", describeCluster(clusterID), err) + } + cc.discovery = disco + return disco, nil +} + +// orderedClusters returns the live cluster contexts, local first and then remotes sorted by +// id, so per-cluster iteration is deterministic across reconciles. +// +// It reads a snapshot published whenever the cluster set changes, rather than taking +// clustersMu and rebuilding a slice: the git writer's cluster-scoped GVK lookup calls +// clusterContextByID once per document it scans out of a folder, on the branch-worker +// goroutine, and that is no place for a mutex the reconcile loop also holds. Cluster contexts +// are created once and never mutated in place by that path, so handing out the slice is safe. +func (m *Manager) orderedClusters() []*clusterContext { + if snapshot := m.clusterOrder.Load(); snapshot != nil { + return *snapshot + } + // No cluster has been created yet: force the local one into existence, which publishes + // the first snapshot. + m.localCluster() + if snapshot := m.clusterOrder.Load(); snapshot != nil { + return *snapshot + } + return nil +} + +// clusterContextByID returns the live context for a cluster id from the published snapshot, +// without taking clustersMu — the read the git writer's cluster-scoped GVK lookup makes. It +// returns nil for an unknown id, which the caller treats as an unready lookup (fail closed). +func (m *Manager) clusterContextByID(id string) *clusterContext { + for _, cc := range m.orderedClusters() { + if cc.id == id { + return cc + } + } + return nil +} + +// publishClusterOrderLocked recomputes the ordered snapshot. Must be called with clustersMu +// held, from the two places that add or remove a cluster. +func (m *Manager) publishClusterOrderLocked() { + ids := make([]string, 0, len(m.clusters)) + for id := range m.clusters { + ids = append(ids, id) + } + sort.Strings(ids) // LocalClusterID is "" and sorts first. + out := make([]*clusterContext, 0, len(ids)) + for _, id := range ids { + out = append(out, m.clusters[id]) + } + m.clusterOrder.Store(&out) +} diff --git a/internal/watch/cluster_context_test.go b/internal/watch/cluster_context_test.go new file mode 100644 index 00000000..64c53e56 --- /dev/null +++ b/internal/watch/cluster_context_test.go @@ -0,0 +1,132 @@ +// SPDX-License-Identifier: Apache-2.0 + +package watch + +import ( + "errors" + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + + "github.com/ConfigButler/gitops-reverser/internal/types" +) + +func gd(name string) types.ResourceReference { + return types.NewResourceReference(name, "team-a") +} + +func TestLocalCluster_SeededAndReachable(t *testing.T) { + m := &Manager{Log: logr.Discard()} + local := m.localCluster() + assert.True(t, local.isLocal()) + assert.Same(t, local, m.cluster(LocalClusterID), "the local id is stable") + assert.Same(t, local.catalog, m.apiResourceCatalog(), "apiResourceCatalog() is the local catalog") + + reach := m.clusterReachability(LocalClusterID) + assert.Equal(t, reachTrue, reach.state) + assert.Equal(t, reasonLocalCluster, reach.reason) +} + +func TestActiveClusterIDs_FromDeclareCapture(t *testing.T) { + m := &Manager{Log: logr.Discard()} + assert.Equal(t, []string{LocalClusterID}, m.activeClusterIDs(), "only local before any declare") + + m.rememberGitTargetCluster(gd("a"), "team-a/kc/value") + m.rememberGitTargetCluster(gd("b"), "team-a/kc/value") // same remote, shared + m.rememberGitTargetCluster(gd("c"), "team-b/kc2/value") + + assert.ElementsMatch(t, + []string{LocalClusterID, "team-a/kc/value", "team-b/kc2/value"}, + m.activeClusterIDs(), + "active ids are the deduped Declare-captured remotes plus local") + + assert.Equal(t, "team-a/kc/value", m.clusterIDForGitTarget(gd("a"))) + assert.Equal(t, LocalClusterID, m.clusterIDForGitTarget(gd("never-declared"))) +} + +func TestRefcountedTeardown(t *testing.T) { + m := &Manager{Log: logr.Discard()} + const remote = "team-a/kc/value" + + // Two GitTargets mirror the same remote; create its context. + m.rememberGitTargetCluster(gd("a"), remote) + m.rememberGitTargetCluster(gd("b"), remote) + require.NotNil(t, m.cluster(remote)) + assert.NotNil(t, m.clusterContextByID(remote), "context exists after first use") + + // Forgetting one leaves the context: the other still mirrors from it. + m.forgetGitTargetCluster(gd("a")) + assert.NotNil(t, m.clusterContextByID(remote), "still referenced by b") + + // Forgetting the last tears it down. + m.forgetGitTargetCluster(gd("b")) + assert.Nil(t, m.clusterContextByID(remote), "last referencing GitTarget gone -> torn down") + + // The local cluster is never torn down by a forget, even when the forgotten GitTarget + // mapped to it. + require.NotNil(t, m.localCluster()) + m.rememberGitTargetCluster(gd("c"), LocalClusterID) + m.forgetGitTargetCluster(gd("c")) + assert.NotNil(t, m.clusterContextByID(LocalClusterID)) +} + +func TestRegistryForGitTarget_PerCluster(t *testing.T) { + m := &Manager{Log: logr.Discard()} + m.rememberGitTargetCluster(gd("remote"), "team-a/kc/value") + + localReg := m.registryForGitTarget(gd("local-target")) + remoteReg := m.registryForGitTarget(gd("remote")) + assert.Same(t, m.localCluster().registry, localReg) + assert.NotSame(t, localReg, remoteReg, "a remote GitTarget resolves against its own registry, not local") +} + +func TestClusterTypeLookup(t *testing.T) { + m := &Manager{Log: logr.Discard()} + // Local lookup is the local registry. + assert.Same(t, m.localCluster().registry, m.ClusterTypeLookup(LocalClusterID)) + // An unknown remote yields a (fresh, unready) registry that fails closed, never nil. + lk := m.ClusterTypeLookup("team-a/kc/value") + require.NotNil(t, lk) + assert.False(t, lk.Ready(), "an unobserved remote registry is not ready — the writer falls closed") +} + +func TestClassifySourceClusterReachFailure(t *testing.T) { + gvr := schema.GroupResource{Resource: "pods"} + tests := []struct { + name string + err error + reason string + }{ + {"unauthorized", apierrors.NewUnauthorized("bad token"), reasonSourceClusterAuthFailed}, + {"forbidden", apierrors.NewForbidden(gvr, "x", errors.New("nope")), reasonSourceClusterAccessDenied}, + {"other", errors.New("dial tcp: i/o timeout"), reasonSourceClusterUnreachable}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := classifySourceClusterReachFailure(tc.err) + assert.Equal(t, reachFalse, got.state) + assert.Equal(t, tc.reason, got.reason) + assert.NotEmpty(t, got.message) + }) + } +} + +func TestRecordClusterReachability(t *testing.T) { + m := &Manager{Log: logr.Discard()} + const remote = "team-a/kc/value" + cc := m.cluster(remote) + + assert.Equal(t, reachUnknown, m.clusterReachability(remote).state, "unknown before first attempt") + + m.recordClusterReachability(cc, nil) + assert.Equal(t, reachTrue, m.clusterReachability(remote).state) + + m.recordClusterReachability(cc, apierrors.NewUnauthorized("bad")) + got := m.clusterReachability(remote) + assert.Equal(t, reachFalse, got.state) + assert.Equal(t, reasonSourceClusterAuthFailed, got.reason) +} diff --git a/internal/watch/config_plane_split_review_fixes_test.go b/internal/watch/config_plane_split_review_fixes_test.go new file mode 100644 index 00000000..845e376e --- /dev/null +++ b/internal/watch/config_plane_split_review_fixes_test.go @@ -0,0 +1,345 @@ +// SPDX-License-Identifier: Apache-2.0 + +package watch + +import ( + "context" + "errors" + "fmt" + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + dynamicfake "k8s.io/client-go/dynamic/fake" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/event" + + configv1alpha3 "github.com/ConfigButler/gitops-reverser/api/v1alpha3" + "github.com/ConfigButler/gitops-reverser/internal/kubeconfig" + "github.com/ConfigButler/gitops-reverser/internal/rulestore" + "github.com/ConfigButler/gitops-reverser/internal/types" +) + +// These tests cover the three code-review findings on the config-plane split: +// P1(a) — a dead credential must stop the mirror, not merely mark it blocked. +// P1(b) — a delete/recreate retarget must never reuse the previous cluster's watched-type table. +// P2 — a rule's ResourcesResolved status resolves against the GitTarget's source cluster. + +// stubSourceClusterResolver returns a fixed verdict for every id, so the credential-refresh +// fail-closed path can be driven without an apiserver. +type stubSourceClusterResolver struct { + cfg *rest.Config + version string + err error +} + +func (s stubSourceClusterResolver) ResolveSourceCluster( + context.Context, string, +) (*rest.Config, string, error) { + return s.cfg, s.version, s.err +} + +// seedClusterCatalog gives a cluster context a ready registry from one discovery scan, so two +// remote clusters can be seeded to EQUAL registry revisions (each is one UpdateFromScan). +func seedClusterCatalog(t *testing.T, m *Manager, id string, disco staticCatalogDiscovery) *clusterContext { + t.Helper() + cc := m.cluster(id) + _, err := cc.catalog.Refresh(disco) + require.NoError(t, err) + m.refreshClusterTypeRegistry(cc) + require.True(t, cc.registry.Ready(), "seeded remote registry should be ready") + return cc +} + +// oneResourceDiscovery builds a discovery serving exactly one namespaced v1 resource, so two +// remote clusters can be made to legitimately disagree on what they serve. An empty group is the +// core group. +func oneResourceDiscovery(group, name, kind string) staticCatalogDiscovery { + listWatch := metav1.Verbs{"get", "list", "watch"} + groupVersion := "v1" + if group != "" { + groupVersion = group + "/v1" + } + return staticCatalogDiscovery{ + groups: []*metav1.APIGroup{testAPIGroup(group, "v1")}, + resources: []*metav1.APIResourceList{{ + GroupVersion: groupVersion, + APIResources: []metav1.APIResource{{Name: name, Kind: kind, Namespaced: true, Verbs: listWatch}}, + }}, + } +} + +// --- P1(a) ----------------------------------------------------------------------------------- + +func TestIsDefinitiveCredentialFailure_ClassifiesCredentialDeath(t *testing.T) { + notFound := apierrors.NewNotFound(schema.GroupResource{Resource: "secrets"}, "kc") + tests := []struct { + name string + err error + want bool + }{ + { + "secret deleted is definitive", + fmt.Errorf("resolve source cluster %q: %w", "team-a/kc/value", notFound), + true, + }, + { + "invalid kubeconfig content is definitive", + fmt.Errorf("wrap: %w", &kubeconfig.RejectionError{Reason: kubeconfig.ReasonInvalid, Message: "bad"}), + true, + }, + { + "missing key is definitive", + &kubeconfig.RejectionError{Reason: kubeconfig.ReasonKeyNotFound, Message: "no key"}, + true, + }, + {"a dial timeout is transient", errors.New("dial tcp: i/o timeout"), false}, + { + "a 403 reading the Secret is transient (RBAC can be fixed)", + apierrors.NewForbidden(schema.GroupResource{Resource: "secrets"}, "kc", errors.New("nope")), + false, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, isDefinitiveCredentialFailure(tc.err)) + }) + } +} + +func TestRefreshClusterCredentials_DropsClientsWhenCredentialGone(t *testing.T) { + notFound := apierrors.NewNotFound(schema.GroupResource{Resource: "secrets"}, "kc") + m := &Manager{Log: logr.Discard(), SourceClusters: stubSourceClusterResolver{err: notFound}} + cc := m.cluster("team-a/kc/value") + cc.restConfig = &rest.Config{Host: "https://192.0.2.1:6443"} + cc.dynamicClient = dynamicfake.NewSimpleDynamicClient(runtime.NewScheme()) + cc.configVersion = "7" + + m.refreshClusterCredentials(context.Background(), cc) + + assert.Nil(t, cc.restConfig, "a deleted kubeconfig Secret drops the cached REST config (fail-closed)") + assert.Nil(t, cc.dynamicClient, "and the dynamic client, so a reconnect cannot reuse a revoked credential") + assert.Empty(t, cc.configVersion) +} + +func TestRefreshClusterCredentials_KeepsClientsOnTransientError(t *testing.T) { + m := &Manager{ + Log: logr.Discard(), + SourceClusters: stubSourceClusterResolver{err: errors.New("dial tcp: i/o timeout")}, + } + cc := m.cluster("team-a/kc/value") + cc.restConfig = &rest.Config{Host: "https://192.0.2.1:6443"} + cc.dynamicClient = dynamicfake.NewSimpleDynamicClient(runtime.NewScheme()) + cc.configVersion = "7" + + m.refreshClusterCredentials(context.Background(), cc) + + assert.NotNil(t, cc.restConfig, "a transient resolve error must not kill a healthy stream") + assert.NotNil(t, cc.dynamicClient) + assert.Equal(t, "7", cc.configVersion, "the version token is untouched on a transient error") +} + +func TestResourceReferenceFromKey_RoundTrips(t *testing.T) { + for _, ref := range []types.ResourceReference{ + types.NewResourceReference("t", "team-a"), + types.NewResourceReference("cluster-scoped", ""), + } { + assert.Equal(t, ref, resourceReferenceFromKey(ref.Key())) + } +} + +// installFakeWatch records a cancellable watch set for a GitTarget and returns a pointer that flips +// true when its context is cancelled (forgetGitTargetWatches calls set.cancel()). +func installFakeWatch(m *Manager, ref types.ResourceReference) *bool { + cancelled := new(bool) + m.targetWatchesMu.Lock() + if m.targetWatches == nil { + m.targetWatches = map[string]*targetWatchSet{} + } + m.targetWatches[ref.Key()] = &targetWatchSet{ + cancel: func() { *cancelled = true }, + specs: map[targetWatchKey]string{}, + } + m.targetWatchesMu.Unlock() + return cancelled +} + +func drainEnqueuedNames(ch <-chan event.GenericEvent, limit int) []string { + var names []string + for range limit { + select { + case e := <-ch: + names = append(names, e.Object.GetName()) + default: + return names + } + } + return names +} + +func TestInvalidateClusterWatches_CancelsAndEnqueuesOnlyThatClusterTargets(t *testing.T) { + m := &Manager{Log: logr.Discard()} + const clusterA, clusterB = "team-a/kc/value", "team-b/kc/value" + m.rememberGitTargetCluster(gd("a1"), clusterA) + m.rememberGitTargetCluster(gd("a2"), clusterA) + m.rememberGitTargetCluster(gd("b1"), clusterB) + a1 := installFakeWatch(m, gd("a1")) + a2 := installFakeWatch(m, gd("a2")) + b1 := installFakeWatch(m, gd("b1")) + ch := m.GitPathEvents() + + m.invalidateClusterWatches(clusterA) + + assert.True(t, *a1, "cluster A target watch cancelled") + assert.True(t, *a2, "cluster A target watch cancelled") + assert.False(t, *b1, "cluster B target is untouched") + + m.targetWatchesMu.Lock() + _, a1Present := m.targetWatches[gd("a1").Key()] + _, b1Present := m.targetWatches[gd("b1").Key()] + m.targetWatchesMu.Unlock() + assert.False(t, a1Present, "cancelled watch set is removed") + assert.True(t, b1Present, "cluster B's watch set is kept") + + assert.ElementsMatch(t, []string{"a1", "a2"}, drainEnqueuedNames(ch, 4), + "only cluster A's targets are enqueued for reconcile") + + // The cluster->GitTarget mapping is KEPT so the enqueued reconcile can re-declare. + assert.Equal(t, clusterA, m.clusterIDForGitTarget(gd("a1"))) +} + +func TestRefreshClusterCredentials_InvalidatesWatchesOnRotation(t *testing.T) { + m := &Manager{ + Log: logr.Discard(), + SourceClusters: stubSourceClusterResolver{cfg: &rest.Config{Host: "h"}, version: "v2"}, + } + const cluster = "team-a/kc/value" + cc := m.cluster(cluster) + cc.restConfig = &rest.Config{Host: "h"} + cc.configVersion = "v1" // the watches were built on the old version + m.rememberGitTargetCluster(gd("t"), cluster) + cancelled := installFakeWatch(m, gd("t")) + ch := m.GitPathEvents() + + m.refreshClusterCredentials(context.Background(), cc) + + assert.Equal(t, "v2", cc.configVersion, "clients rebuilt to the new version") + assert.Nil(t, cc.dynamicClient, "cached dynamic client dropped so the next use rebuilds it") + assert.True(t, *cancelled, "a rotated Secret invalidates the active watch so it re-establishes") + assert.Equal(t, []string{"t"}, drainEnqueuedNames(ch, 2)) +} + +// --- P1(b) ----------------------------------------------------------------------------------- + +func TestClusterMappingFingerprint_MovesOnRetarget(t *testing.T) { + m := &Manager{Log: logr.Discard()} + base := m.clusterMappingFingerprint() + + m.rememberGitTargetCluster(gd("t"), "team-a/kc/value") + afterA := m.clusterMappingFingerprint() + assert.NotEqual(t, base, afterA, "capturing a source cluster moves the fingerprint") + assert.Equal(t, afterA, m.clusterMappingFingerprint(), "stable while the mapping is unchanged") + + m.rememberGitTargetCluster(gd("t"), "team-b/kc2/value") + afterB := m.clusterMappingFingerprint() + assert.NotEqual(t, afterA, afterB, "a GitTarget switching source clusters must move the fingerprint") +} + +// The gate must re-project when ONLY the cluster mapping changes — the summed registry revision +// and the rules fingerprint are both blind to a delete/recreate retarget between two clusters +// with equal registry revisions. Without the cluster mapping fingerprint the recreated GitTarget +// would keep the previous cluster's GVR table. +func TestRefreshWatchedTypeTables_RetargetReResolvesAtEqualRevisions(t *testing.T) { + store := rulestore.NewStore() + m := &Manager{Log: logr.Discard(), RuleStore: store, resourceCatalog: newCommonTestCatalog(t)} + + const clusterA, clusterB = "team-a/kc/value", "team-b/kc/value" + ccA := seedClusterCatalog(t, m, clusterA, oneResourceDiscovery("", "configmaps", "ConfigMap")) + ccB := seedClusterCatalog(t, m, clusterB, oneResourceDiscovery("", "secrets", "Secret")) + require.Equal(t, ccA.registry.Revision(), ccB.registry.Revision(), + "both remotes are one scan in, so their registry revisions are equal on purpose") + + store.AddOrUpdateClusterWatchRule( + clusterRuleForResource("rule-1", "configmaps"), + "t", "test-ns", "test-provider", "test-ns", "main", "test-path", + ) + m.rememberGitTargetCluster(gitDestRef("t"), clusterA) + m.refreshWatchedTypeTables() + first, ok := m.watchedTypeTableForGitDest(gitDestRef("t")) + require.True(t, ok) + require.Len(t, first.Types, 1, "cluster A serves configmaps") + assert.Equal(t, "ConfigMap", first.Types[0].GVK.Kind) + + // Retarget to B (equal revision, unchanged rule): only the cluster mapping fingerprint moved. + m.forgetGitTargetCluster(gitDestRef("t")) // last referencer of A -> A is torn down + m.rememberGitTargetCluster(gitDestRef("t"), clusterB) + m.refreshWatchedTypeTables() + + second, ok := m.watchedTypeTableForGitDest(gitDestRef("t")) + require.True(t, ok) + assert.Empty(t, second.Types, + "after retargeting to a cluster that does not serve configmaps the table must be "+ + "re-resolved to empty, not keep cluster A's GVRs") +} + +// --- P2 -------------------------------------------------------------------------------------- + +func TestResolveWatchRuleResources_ResolvesAgainstSourceCluster(t *testing.T) { + m := &Manager{Log: logr.Discard(), resourceCatalog: newCommonTestCatalog(t)} + const remote = "team-a/kc/value" + seedClusterCatalog(t, m, remote, oneResourceDiscovery("example.com", "widgets", "Widget")) + m.rememberGitTargetCluster(types.NewResourceReference("t", "test-ns"), remote) + + remoteOnly := configv1alpha3.WatchRule{ + ObjectMeta: metav1.ObjectMeta{Name: "r", Namespace: "test-ns"}, + Spec: configv1alpha3.WatchRuleSpec{ + TargetRef: configv1alpha3.LocalTargetReference{Name: "t"}, + Rules: []configv1alpha3.ResourceRule{ + {APIGroups: []string{"example.com"}, Resources: []string{"widgets"}}, + }, + }, + } + resolved, message := m.ResolveWatchRuleResources(context.Background(), remoteOnly) + assert.True(t, resolved) + assert.Equal(t, "watching 1 resource type(s)", message, + "a remote-only CRD is watched, resolved against the source cluster's registry") + + localOnly := configv1alpha3.WatchRule{ + ObjectMeta: metav1.ObjectMeta{Name: "r2", Namespace: "test-ns"}, + Spec: configv1alpha3.WatchRuleSpec{ + TargetRef: configv1alpha3.LocalTargetReference{Name: "t"}, + Rules: []configv1alpha3.ResourceRule{{Resources: []string{"deployments"}}}, + }, + } + resolved, message = m.ResolveWatchRuleResources(context.Background(), localOnly) + assert.True(t, resolved) + assert.Equal(t, "watching 0 resource type(s)", message, + "a type served only on the local cluster is not watched by a remote-source GitTarget") +} + +func TestResolveClusterWatchRuleResources_ResolvesAgainstSourceCluster(t *testing.T) { + m := &Manager{Log: logr.Discard(), resourceCatalog: newCommonTestCatalog(t)} + const remote = "team-a/kc/value" + seedClusterCatalog(t, m, remote, oneResourceDiscovery("example.com", "widgets", "Widget")) + m.rememberGitTargetCluster(types.NewResourceReference("t", "test-ns"), remote) + + rule := configv1alpha3.ClusterWatchRule{ + Spec: configv1alpha3.ClusterWatchRuleSpec{ + TargetRef: configv1alpha3.NamespacedTargetReference{Name: "t", Namespace: "test-ns"}, + Rules: []configv1alpha3.ClusterResourceRule{{ + APIGroups: []string{"example.com"}, + Resources: []string{"widgets"}, + Scope: configv1alpha3.ResourceScopeNamespaced, + }}, + }, + } + resolved, message := m.ResolveClusterWatchRuleResources(context.Background(), rule) + assert.True(t, resolved) + assert.Equal(t, "watching 1 resource type(s)", message, + "the ClusterWatchRule resolves against its GitTarget's source cluster") +} diff --git a/internal/watch/manager.go b/internal/watch/manager.go index 1ca6048c..b293320f 100644 --- a/internal/watch/manager.go +++ b/internal/watch/manager.go @@ -9,6 +9,7 @@ package watch import ( "context" "sync" + "sync/atomic" "time" "github.com/go-logr/logr" @@ -27,7 +28,6 @@ import ( "github.com/ConfigButler/gitops-reverser/internal/rulestore" "github.com/ConfigButler/gitops-reverser/internal/telemetry" "github.com/ConfigButler/gitops-reverser/internal/types" - "github.com/ConfigButler/gitops-reverser/internal/typeset" ) // The API-resource catalog reads the two resources that describe the API surface itself, so @@ -102,10 +102,36 @@ type Manager struct { // against what git already holds. See routeLiveTargetWatchEvent. liveContentDedup sync.Map - // resourceCatalog is the shared discovery-backed API surface used by rule planning. + // SourceClusters resolves a GitTarget's source-cluster id into a rest.Config by reading + // the kubeconfig Secret it names from the config plane. Nil is the single-cluster + // install: every GitTarget mirrors the cluster the operator runs in. + SourceClusters SourceClusterResolver + + // clusters holds one clusterContext per distinct source cluster — its API catalog, type + // registry, and clients. LocalClusterID is the cluster the operator runs in, and the only + // one a single-cluster install ever creates. See cluster_context.go. + clustersMu sync.Mutex + clusters map[string]*clusterContext + // clusterOrder is the published, ordered snapshot of clusters (local first). The git + // writer's cluster-scoped GVK lookup reads it once per document it scans out of a Git + // folder, on the branch-worker goroutine, so it must not contend on clustersMu with the + // reconcile loop. + clusterOrder atomic.Pointer[[]*clusterContext] + // gitTargetClusters maps a GitTarget key to the source-cluster id it mirrors from, + // captured on Declare (the gitTargetUIDs pattern). Because spec.kubeConfig is immutable + // this is learned once and never changes — no per-rule propagation, no disagreement + // window. Guarded by gitTargetClustersMu. + gitTargetClustersMu sync.Mutex + gitTargetClusters map[string]string + + // resourceCatalogMu guards every clusterContext's catalog/registry edge-triggered + // logging state (catalogDegradedLogged, typeRefusalsLogged). resourceCatalogMu sync.Mutex - resourceCatalog *APIResourceCatalog - // discoveryClient overrides REST-config discovery construction in tests. + // resourceCatalog seeds the LOCAL cluster's API-resource catalog. Tests set it on a + // zero-value Manager to drive resolution without an API server; production leaves it nil + // and the local cluster context builds its own. Aliased to localCluster().catalog. + resourceCatalog *APIResourceCatalog + // discoveryClient overrides REST-config discovery construction for the LOCAL cluster in tests. discoveryClient func() (apiResourceDiscovery, error) // catalogRefreshCh coalesces API-surface trigger watch events into manager reconciliation. catalogRefreshCh chan struct{} @@ -130,14 +156,6 @@ type Manager struct { // triggersForbiddenLogged records which trigger resources RBAC has already denied, so a // permanently unauthorized resource produces one line per denial, not one per retry. triggersForbiddenLogged map[schema.GroupVersionResource]struct{} - // catalogReadyOnce guards the one-time "catalog ready" log line, matching the - // firstMessage/firstGroupReady sync.Once pattern used by the audit consumer. - catalogReadyOnce sync.Once - // catalogDegradedLogged is the degraded group/version set last reflected in - // the log; logCatalogTransitions diffs against it to log appear/clear - // transitions (degradation can recur, so this is not a one-shot). Guarded by - // resourceCatalogMu. - catalogDegradedLogged map[schema.GroupVersion]struct{} // watchedTypes is the resident, per-GitTarget watched-type table set: the single // source of "what each GitTarget watches", a projection of the type registry's @@ -179,18 +197,6 @@ type Manager struct { gitTargetUIDsMu sync.Mutex gitTargetUIDs map[string]string - // typeRegistry is the followability decision surface (see - // docs/spec/type-followability.md): one typeset.TypeRecord - // per served type, refreshed from the catalog scan on every catalog refresh. It - // is the inventory/status surface ("is this type followable, and if not, why?"); - // typeRegistryInit guards its lazy construction for zero-value Managers in tests. - typeRegistryInit sync.Once - typeRegistry *typeset.Registry - // typeRefusalsLogged is the GVK->summary of every type the registry currently - // refuses, so the central "why is this not followable?" log is edge-triggered: a - // stable refusal is logged once, not on every refresh. Guarded by resourceCatalogMu. - typeRefusalsLogged map[string]string - // declaredGVRsMu guards declaredGVRs: the type-set each GitTarget last Declared. The watch-first // data plane reads it to drive the per-(GitTarget, type) watch set; re-declaring is idempotent. declaredGVRsMu sync.Mutex @@ -274,26 +280,6 @@ func (m *Manager) NeedLeaderElection() bool { return true } -// dynamicClientFromConfig builds a dynamic client from the controller's REST config. -// If m.dynamicClient is set (e.g. in tests) it is returned directly. It is used by the -// per-type checkpoint fill (mirrorTypeObjects) — the only API touch on a schedule. -func (m *Manager) dynamicClientFromConfig(log logr.Logger) dynamic.Interface { - if m.dynamicClient != nil { - return m.dynamicClient - } - cfg := m.restConfig() - if cfg == nil { - log.Info("skipping seed - no rest config available") - return nil - } - dc, err := dynamic.NewForConfig(cfg) - if err != nil { - log.Error(err, "failed to construct dynamic client for seed") - return nil - } - return dc -} - // ReconcileForRuleChange refreshes the trusted API catalog and the resident watched-type // tables when rules change or a CRD is installed/removed. It no longer starts object // informers or gathers a whole-GitTarget snapshot (R3): the catalog refresh drives the diff --git a/internal/watch/manager_catalog.go b/internal/watch/manager_catalog.go index 6da271ce..3c7bf165 100644 --- a/internal/watch/manager_catalog.go +++ b/internal/watch/manager_catalog.go @@ -4,10 +4,10 @@ package watch import ( "context" - "errors" "fmt" "math" "sort" + "sync" "time" "github.com/go-logr/logr" @@ -16,7 +16,6 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/discovery" "k8s.io/client-go/dynamic" "k8s.io/client-go/dynamic/dynamicinformer" "k8s.io/client-go/rest" @@ -25,6 +24,7 @@ import ( configv1alpha3 "github.com/ConfigButler/gitops-reverser/api/v1alpha3" "github.com/ConfigButler/gitops-reverser/internal/telemetry" + "github.com/ConfigButler/gitops-reverser/internal/types" "github.com/ConfigButler/gitops-reverser/internal/typeset" ) @@ -57,47 +57,137 @@ func apiServiceTriggerGVR() schema.GroupVersionResource { } } -// RefreshAPIResourceCatalog refreshes trusted catalog data from Kubernetes discovery. +// RefreshAPIResourceCatalog refreshes trusted catalog data from Kubernetes discovery, for the +// local cluster and every source cluster a GitTarget currently mirrors from. It returns the +// LOCAL cluster's error only: a remote that cannot be reached fails its OWN GitTargets (through +// their unready registries and a SourceClusterReachable=False projection), never the local +// cluster's reconcile. A remote rotation is also picked up here, on the refresh cadence. func (m *Manager) RefreshAPIResourceCatalog(ctx context.Context) error { - catalog := m.apiResourceCatalog() - disco, err := m.apiResourceDiscovery() + var ( + localErr error + remotes []*clusterContext + ) + for _, id := range m.activeClusterIDs() { + cc := m.cluster(id) + if cc.isLocal() { + // The local cluster stays on the caller's goroutine: it owns localErr (the only error + // returned) and re-arms the API-surface trigger informers, and it is never slow. + localErr = m.refreshClusterCatalog(ctx, cc) + m.recordClusterReachability(cc, localErr) + continue + } + remotes = append(remotes, cc) + } + m.refreshRemoteCatalogsConcurrently(ctx, remotes) + return localErr +} + +// maxConcurrentCatalogRefreshes bounds how many remote source clusters refresh at once, so a +// large tenant fan-out cannot open an unbounded number of simultaneous discovery connections. +const maxConcurrentCatalogRefreshes = 8 + +// refreshRemoteCatalogsConcurrently refreshes each remote source cluster's credentials and catalog +// independently, with bounded concurrency. Serial refresh made total latency grow as +// remoteCount × the discovery timeout — one unreachable remote could burn the full timeout before +// the next even started, delaying every other tenant. Each clusterContext has its own client lock +// and registry and shares no mutable state, so the refreshes are safe to run in parallel; the only +// shared structures they touch (the reachability field and the per-cluster log-dedup maps) are +// guarded by their existing manager-wide locks. +func (m *Manager) refreshRemoteCatalogsConcurrently(ctx context.Context, remotes []*clusterContext) { + if len(remotes) == 0 { + return + } + sem := make(chan struct{}, maxConcurrentCatalogRefreshes) + var wg sync.WaitGroup + for _, cc := range remotes { + wg.Add(1) + sem <- struct{}{} + go func(cc *clusterContext) { + defer wg.Done() + defer func() { <-sem }() + m.refreshClusterCredentials(ctx, cc) + err := m.refreshClusterCatalog(ctx, cc) + m.recordClusterReachability(cc, err) + }(cc) + } + wg.Wait() +} + +// refreshClusterForDeclare refreshes ONE source cluster's credentials, catalog, and reachability — +// the on-declare path for a single GitTarget. It deliberately touches only that GitTarget's own +// cluster: refreshing every active cluster on the declare path (which runs on the single GitTarget +// controller worker) blocks on any UNREACHABLE cluster's full dial timeout, starving healthy +// targets. The background RefreshAPIResourceCatalog loop keeps every OTHER cluster fresh. A remote's +// refresh error is not returned — the caller gates on registryForGitTarget().Ready() instead, so an +// unreachable remote simply leaves its own target "not observed yet" without failing the declare. +func (m *Manager) refreshClusterForDeclare(ctx context.Context, clusterID string) error { + cc := m.cluster(clusterID) + if !cc.isLocal() { + m.refreshClusterCredentials(ctx, cc) + } + err := m.refreshClusterCatalog(ctx, cc) + m.recordClusterReachability(cc, err) + if cc.isLocal() { + return err + } + return nil +} + +// refreshClusterCatalog refreshes ONE cluster's discovery-backed catalog and republishes its +// type registry — the per-cluster body of what used to be a single manager-wide refresh. The +// refresh metrics and the API-surface trigger informers stay local-cluster only: the metrics +// carry no cluster label, and the trigger informers (a "refresh sooner than the 30s tick" +// latency optimization) run against the config plane, so a remote cluster's catalog freshness +// rides the periodic refresh instead. +func (m *Manager) refreshClusterCatalog(ctx context.Context, cc *clusterContext) error { + disco, err := m.clusterDiscovery(ctx, cc.id) if err != nil { return err } start := time.Now() - changed, refreshErr := catalog.Refresh(disco) - recordCatalogRefresh(ctx, changed, refreshErr, time.Since(start)) - if refreshErr == nil { - // Re-derive the followability records from the fresh scan before logging, so - // the ready line can report how many served types are followable. - m.refreshTypeRegistry() - stats := catalog.Stats() + changed, refreshErr := cc.catalog.Refresh(disco) + if cc.isLocal() { + recordCatalogRefresh(ctx, changed, refreshErr, time.Since(start)) + } + if refreshErr != nil { + return refreshErr + } + // Re-derive the followability records from the fresh scan before logging, so the ready + // line can report how many served types are followable. + m.refreshClusterTypeRegistry(cc) + stats := cc.catalog.Stats() + if cc.isLocal() { recordCatalogStats(ctx, stats) - m.logCatalogTransitions(catalog, stats) + } + m.logCatalogTransitions(cc, stats) + if cc.isLocal() { // The fresh scan is the only source of truth for which trigger resources this API // server actually serves, so trigger informers are (re-)armed here rather than once // at startup. An aggregation layer installed later is picked up on its refresh. m.ensureAPISurfaceTriggerInformers(m.Log.WithName("catalog-triggers")) } - return refreshErr + return nil } // logCatalogTransitions emits an Info line on edge-triggered catalog changes // only: the first successful build, and when the set of group/versions that // discovery cannot serve appears or clears. Steady-state refreshes - which run // on every rule change, periodic tick, and CRD/APIService event - stay silent. -func (m *Manager) logCatalogTransitions(catalog *APIResourceCatalog, stats CatalogStats) { +func (m *Manager) logCatalogTransitions(cc *clusterContext, stats CatalogStats) { log := m.Log.WithName("catalog") + if !cc.isLocal() { + log = log.WithValues("clusterID", cc.id) + } - if catalog.Ready() { - m.catalogReadyOnce.Do(func() { + if cc.catalog.Ready() { + cc.catalogReadyOnce.Do(func() { log.Info("API resource catalog ready", "allowedResources", stats.AllowedResources, "excludedResources", stats.ExcludedResources, "trustedGroupVersions", stats.TrustedGroupVersions, "degradedGroupVersions", stats.DegradedGroupVersions, - "followableTypes", len(m.FollowableTypeRecords()), - "knownTypes", len(m.TypeRecords()), + "followableTypes", len(cc.registry.Followable()), + "knownTypes", len(cc.registry.All()), "generation", stats.Generation) }) } @@ -107,19 +197,19 @@ func (m *Manager) logCatalogTransitions(catalog *APIResourceCatalog, stats Catal current := make(map[schema.GroupVersion]struct{}) var appeared []schema.GroupVersion - for _, gv := range catalog.DegradedGroupVersions() { + for _, gv := range cc.catalog.DegradedGroupVersions() { current[gv] = struct{}{} - if _, known := m.catalogDegradedLogged[gv]; !known { + if _, known := cc.catalogDegradedLogged[gv]; !known { appeared = append(appeared, gv) } } var cleared []schema.GroupVersion - for gv := range m.catalogDegradedLogged { + for gv := range cc.catalogDegradedLogged { if _, still := current[gv]; !still { cleared = append(cleared, gv) } } - m.catalogDegradedLogged = current + cc.catalogDegradedLogged = current if len(appeared) > 0 { log.Info("API discovery degraded - the cluster cannot serve these group/versions; "+ @@ -195,44 +285,36 @@ func recordCatalogStats(ctx context.Context, stats CatalogStats) { } } +// apiResourceCatalog returns the LOCAL cluster's discovery-backed API surface catalog. It is +// the back-compatible accessor every source-cluster-unaware caller uses; per-cluster callers +// read cc.catalog directly. func (m *Manager) apiResourceCatalog() *APIResourceCatalog { - m.resourceCatalogMu.Lock() - defer m.resourceCatalogMu.Unlock() - if m.resourceCatalog == nil { - m.resourceCatalog = NewAPIResourceCatalog() - } - return m.resourceCatalog + return m.localCluster().catalog } -// typeRegistryInstance returns the lazily-built followability registry, so a -// zero-value Manager (used widely in tests) needs no explicit setup. +// typeRegistryInstance returns the LOCAL cluster's followability registry, so a zero-value +// Manager (used widely in tests) needs no explicit setup. Per-cluster callers read +// cc.registry directly. func (m *Manager) typeRegistryInstance() *typeset.Registry { - m.typeRegistryInit.Do(func() { - if m.typeRegistry == nil { - m.typeRegistry = typeset.NewRegistry() - } - }) - return m.typeRegistry + return m.localCluster().registry } -// refreshTypeRegistry publishes the catalog's latest normalized scan to the typeset -// registry, which owns ALL cross-scan judgement (retain-on-error, the removal grace -// for omissions — docs/spec/typeset-owns-discovery-grace.md). It runs after every -// catalog refresh, so the registry tracks discovery and its grace clocks advance on -// the same cadence the catalog scans do. It is the "Scan -> Registry" pipeline of -// docs/spec/type-followability.md. -func (m *Manager) refreshTypeRegistry() { - // Only publish once the catalog holds trusted data, so the registry's readiness - // tracks the catalog's: an unready catalog must leave the registry unready, which - // is what makes the live mapper fall closed (CatalogUnavailable) rather than treat - // an empty scan as a trusted "nothing is served". - scan, ok := m.apiResourceCatalog().Scan(m.SensitiveResources) +// refreshClusterTypeRegistry publishes one cluster's catalog scan to its typeset registry, +// which owns ALL cross-scan judgement (retain-on-error, the removal grace for omissions — +// docs/spec/typeset-owns-discovery-grace.md). It runs after every catalog refresh, so the +// registry tracks discovery and its grace clocks advance on the same cadence the catalog +// scans do. It is the "Scan -> Registry" pipeline of docs/spec/type-followability.md. +func (m *Manager) refreshClusterTypeRegistry(cc *clusterContext) { + // Only publish once the catalog holds trusted data, so the registry's readiness tracks the + // catalog's: an unready catalog must leave the registry unready, which is what makes the + // live mapper fall closed (CatalogUnavailable) rather than treat an empty scan as a + // trusted "nothing is served". + scan, ok := cc.catalog.Scan(m.SensitiveResources) if !ok { return } - reg := m.typeRegistryInstance() - reg.UpdateFromScan(scan) - m.logTypeRefusals(reg) + cc.registry.UpdateFromScan(scan) + m.logTypeRefusals(cc, cc.registry) } // logTypeRefusals is the single central place that explains why a served type is not @@ -240,9 +322,12 @@ func (m *Manager) refreshTypeRegistry() { // summary, so a stable refusal (a policy-excluded kind, a verb-poor type) is logged // once rather than on every refresh. The full machine-readable answer always lives on // the registry record (TypeRecords / FollowableTypeRecords), so callers that need it -// read there rather than parse logs. -func (m *Manager) logTypeRefusals(reg *typeset.Registry) { +// read there rather than parse logs. The edge-trigger state is per cluster. +func (m *Manager) logTypeRefusals(cc *clusterContext, reg *typeset.Registry) { log := m.Log.WithName("followability") + if !cc.isLocal() { + log = log.WithValues("clusterID", cc.id) + } m.resourceCatalogMu.Lock() defer m.resourceCatalogMu.Unlock() current := map[string]string{} @@ -252,47 +337,32 @@ func (m *Manager) logTypeRefusals(reg *typeset.Registry) { } key := rec.Identity.GVK.String() current[key] = rec.Followability.Summary - if prev, known := m.typeRefusalsLogged[key]; !known || prev != rec.Followability.Summary { + if prev, known := cc.typeRefusalsLogged[key]; !known || prev != rec.Followability.Summary { log.V(1).Info("type is not followable", "gvk", key, "gvr", rec.Identity.GVR.String(), "reason", rec.Followability.Summary) } } - m.typeRefusalsLogged = current + cc.typeRefusalsLogged = current } -// TypeRegistry returns the live followability registry, the single decision surface -// (a typeset.Lookup). The git worker reads it to resolve manifest GVKs; the manager -// refreshes it in place, so the returned pointer tracks discovery updates. +// TypeRegistry returns the LOCAL cluster's followability registry. Retained for the git +// writer's manager-wide mapper wiring (cmd/main.go); the cluster-scoped writer lookup uses +// ClusterTypeLookup instead (see gvr.go / Step 4). func (m *Manager) TypeRegistry() *typeset.Registry { - return m.typeRegistryInstance() + return m.localCluster().registry } -// FollowableTypeRecords returns every currently-followable type record (verdict -// followable or retained), sorted by identity. It is the inventory the status and -// visibility surfaces read; it never recomputes followability. +// FollowableTypeRecords returns the LOCAL cluster's currently-followable type records (verdict +// followable or retained), sorted by identity. It is the inventory the status and visibility +// surfaces read; it never recomputes followability. func (m *Manager) FollowableTypeRecords() []typeset.TypeRecord { - return m.typeRegistryInstance().Followable() + return m.localCluster().registry.Followable() } -// TypeRecords returns every known type record — followable, retained, and refused — -// for inventory and "why is this type not picked up?" views. +// TypeRecords returns every known type record — followable, retained, and refused — for the +// LOCAL cluster, for inventory and "why is this type not picked up?" views. func (m *Manager) TypeRecords() []typeset.TypeRecord { - return m.typeRegistryInstance().All() -} - -func (m *Manager) apiResourceDiscovery() (apiResourceDiscovery, error) { - if m.discoveryClient != nil { - return m.discoveryClient() - } - cfg := m.restConfig() - if cfg == nil { - return nil, errors.New("no REST config available for API resource discovery") - } - disco, err := discovery.NewDiscoveryClientForConfig(cfg) - if err != nil { - return nil, fmt.Errorf("create API resource discovery client: %w", err) - } - return disco, nil + return m.localCluster().registry.All() } // ruleResourceSelector is one rule's (apiGroups, apiVersions, resources, scope) tuple, @@ -304,7 +374,10 @@ type ruleResourceSelector struct { } // ResolveWatchRuleResources reports one WatchRule's resource-resolution status for -// controller feedback. See resolveRuleResourceStatus. +// controller feedback. See resolveRuleResourceStatus. A WatchRule's GitTarget lives in the +// WatchRule's own namespace, and the status resolves against THAT GitTarget's source cluster — +// so a remote-only CRD is reported as watched, and a local-only CRD selected by a remote target +// is not, instead of both being answered from the local registry. func (m *Manager) ResolveWatchRuleResources( _ context.Context, rule configv1alpha3.WatchRule, @@ -316,11 +389,14 @@ func (m *Manager) ResolveWatchRuleResources( scope: configv1alpha3.ResourceScopeNamespaced, }) } - return m.resolveRuleResourceStatus(selectors) + gitDest := types.NewResourceReference(rule.Spec.TargetRef.Name, rule.Namespace) + return m.resolveRuleResourceStatus(gitDest, selectors) } // ResolveClusterWatchRuleResources reports one ClusterWatchRule's resource-resolution -// status for controller feedback. See resolveRuleResourceStatus. +// status for controller feedback. See resolveRuleResourceStatus. A ClusterWatchRule is +// cluster-scoped, so its targetRef names both the GitTarget and its namespace; the status +// resolves against that GitTarget's source cluster. func (m *Manager) ResolveClusterWatchRuleResources( _ context.Context, rule configv1alpha3.ClusterWatchRule, @@ -331,7 +407,8 @@ func (m *Manager) ResolveClusterWatchRuleResources( groups: rr.APIGroups, versions: rr.APIVersions, resources: rr.Resources, scope: rr.Scope, }) } - return m.resolveRuleResourceStatus(selectors) + gitDest := types.NewResourceReference(rule.Spec.TargetRef.Name, rule.Spec.TargetRef.Namespace) + return m.resolveRuleResourceStatus(gitDest, selectors) } // resolveRuleResourceStatus reports a rule's resource-resolution status from the type @@ -340,9 +417,18 @@ func (m *Manager) ResolveClusterWatchRuleResources( // explain why an individual selector matched nothing: absent, refused, and not-yet-served // are all the same to a mirror. Status only reports catalog readiness and how many distinct // followable types the rule currently watches. -func (m *Manager) resolveRuleResourceStatus(selectors []ruleResourceSelector) (bool, string) { - m.refreshTypeRegistry() - reg := m.typeRegistryInstance() +func (m *Manager) resolveRuleResourceStatus( + gitDest types.ResourceReference, + selectors []ruleResourceSelector, +) (bool, string) { + // Resolve against the GitTarget's OWN source cluster, not a manager-wide union: a WatchRule + // scoped to a remote GitTarget must report the remote's followable set. clusterIDForGitTarget + // is the Declare-time capture (LocalClusterID until the first Declare, so a status read racing + // bootstrap falls back to local — the same fallback the watched-type tables use). The registry + // republish reads the already-scanned catalog and never dials. + cc := m.cluster(m.clusterIDForGitTarget(gitDest)) + m.refreshClusterTypeRegistry(cc) + reg := cc.registry if !reg.Ready() { return false, "API resource catalog is not ready" } diff --git a/internal/watch/manager_snapshot_test.go b/internal/watch/manager_snapshot_test.go index da2bd29c..76a8b54a 100644 --- a/internal/watch/manager_snapshot_test.go +++ b/internal/watch/manager_snapshot_test.go @@ -132,7 +132,7 @@ func TestRetainedWatchedTypes_NoneWhenAllServed(t *testing.T) { m.refreshWatchedTypeTables() table := m.residentWatchedTypeTable(myTargetRef()) require.NotEmpty(t, table.Types) - assert.Empty(t, m.retainedWatchedTypes(table), "served types are not retained") + assert.Empty(t, m.retainedWatchedTypes(table.GitDest, table), "served types are not retained") } func TestGVKListSummary(t *testing.T) { diff --git a/internal/watch/materialization.go b/internal/watch/materialization.go index fea3d611..7cc5a915 100644 --- a/internal/watch/materialization.go +++ b/internal/watch/materialization.go @@ -8,28 +8,37 @@ import ( "github.com/ConfigButler/gitops-reverser/internal/types" ) -// DeclareForGitTarget ensures the GitTarget's watch-first data plane is running. +// DeclareForGitTarget ensures the GitTarget's watch-first data plane is running against the +// source cluster it mirrors from. clusterID is (api/v1alpha3).GitTarget.SourceClusterID() — "" +// for the cluster the operator runs in. It is captured here, the same capture-on-Declare +// pattern as the UID: because spec.kubeConfig is immutable it is learned once and never +// changes, so there is no per-rule propagation and no cross-rule disagreement window. func (m *Manager) DeclareForGitTarget( ctx context.Context, gitDest types.ResourceReference, + clusterID string, forceRecheck ...bool, ) error { - // Capture the UID before starting watches: the data plane keys its resume cursors - // by GitTarget UID, which the rule-derived watch tables do not carry. + // Capture the UID and the source cluster before starting watches: the data plane keys its + // resume cursors by GitTarget UID, and resolves rules/opens watches against the captured + // cluster's context — neither of which the rule-derived watch tables carry. m.rememberGitTargetUID(gitDest) + m.rememberGitTargetCluster(gitDest, clusterID) force := len(forceRecheck) > 0 && forceRecheck[0] if err := m.EnsureGitTargetWatches(ctx, gitDest, force); err != nil { m.Log.Info("watch-first declare skipped; surface not observable", - "gitDest", gitDest.String(), "err", err.Error()) + "gitDest", gitDest.String(), "clusterID", describeCluster(clusterID), "err", err.Error()) return err } return nil } -// ForgetGitTargetDeclaration drops in-memory watch state for a deleted GitTarget. +// ForgetGitTargetDeclaration drops in-memory watch state for a deleted GitTarget, and tears +// down its source cluster's context when it was the last GitTarget mirroring from it. func (m *Manager) ForgetGitTargetDeclaration(gitDest types.ResourceReference) { m.forgetGitTargetWatches(gitDest) m.forgetGitTargetUID(gitDest) + m.forgetGitTargetCluster(gitDest) m.declaredGVRsMu.Lock() defer m.declaredGVRsMu.Unlock() delete(m.declaredGVRs, gitDest.String()) diff --git a/internal/watch/scope_resolve.go b/internal/watch/scope_resolve.go index 9e481a34..029f455c 100644 --- a/internal/watch/scope_resolve.go +++ b/internal/watch/scope_resolve.go @@ -62,7 +62,8 @@ func (m *Manager) resolveSnapshotGVRForType( } m.refreshWatchedTypeTables() - if !m.typeRegistryInstance().Ready() { + reg := m.registryForGitTarget(gitDest) + if !reg.Ready() { return snapshotGVR{}, false, fmt.Errorf( "aborting per-type reconcile for %s: the cluster API surface has not been observed yet", gitDest.String()) @@ -80,7 +81,7 @@ func (m *Manager) resolveSnapshotGVRForType( return snapshotGVR{}, false, nil } - if m.typeWobbling(gvr) { + if typeWobbling(reg, gvr) { return snapshotGVR{}, false, fmt.Errorf( "aborting per-type reconcile for %s: %s within the removal grace (currently unserved); "+ "refusing to reconcile a reduced view", @@ -106,7 +107,7 @@ func (m *Manager) resolveSnapshotGVRs( } m.refreshWatchedTypeTables() - if !m.typeRegistryInstance().Ready() { + if !m.registryForGitTarget(gitDest).Ready() { return nil, fmt.Errorf( "aborting scope resolution for %s: the cluster API surface has not been observed yet; "+ "refusing to reconcile a partial cluster view", @@ -118,7 +119,7 @@ func (m *Manager) resolveSnapshotGVRs( // A watched type the registry holds as `retained` is followable under the removal grace but // is not actually served right now (a discovery wobble). Reconciling it would sweep a reduced // view and delete a still-valid mirror, so fail closed until the wobble resolves. - if retained := m.retainedWatchedTypes(table); len(retained) > 0 { + if retained := m.retainedWatchedTypes(gitDest, table); len(retained) > 0 { return nil, fmt.Errorf( "aborting scope resolution for %s: %s within the removal grace (currently unserved); "+ "refusing to sweep a reduced cluster view", @@ -129,23 +130,29 @@ func (m *Manager) resolveSnapshotGVRs( } // retainedWatchedTypes returns the GVKs of the target's watched types the registry currently -// holds as `retained` (followable under the grace, but not served right now). -func (m *Manager) retainedWatchedTypes(table WatchedTypeTable) []schema.GroupVersionKind { +// holds as `retained` (followable under the grace, but not served right now), resolved against +// the GitTarget's OWN source cluster's registry. +func (m *Manager) retainedWatchedTypes( + gitDest types.ResourceReference, + table WatchedTypeTable, +) []schema.GroupVersionKind { + reg := m.registryForGitTarget(gitDest) var out []schema.GroupVersionKind for _, wt := range table.Types { - if m.typeWobbling(wt.GVR) { + if typeWobbling(reg, wt.GVR) { out = append(out, wt.GVK) } } return out } -// typeWobbling reports whether the registry currently holds gvr as `retained` — followable under +// typeWobbling reports whether a registry currently holds gvr as `retained` — followable under // the removal grace, but not actually served right now (a discovery wobble). It is the single // "do not reconcile or sweep this type" predicate, shared by the whole-GitTarget scope resolve -// and the per-type gate, so both fail closed on exactly the same registry verdict. -func (m *Manager) typeWobbling(gvr schema.GroupVersionResource) bool { - rec, ok := m.typeRegistryInstance().ByGVR(gvr) +// and the per-type gate, so both fail closed on exactly the same registry verdict. The registry +// is the GitTarget's own source cluster's, so a wobble on one cluster never sweeps another's. +func typeWobbling(reg *typeset.Registry, gvr schema.GroupVersionResource) bool { + rec, ok := reg.ByGVR(gvr) return ok && rec.Followability.Verdict == typeset.VerdictRetained } diff --git a/internal/watch/source_cluster_resolver.go b/internal/watch/source_cluster_resolver.go new file mode 100644 index 00000000..97bbfc76 --- /dev/null +++ b/internal/watch/source_cluster_resolver.go @@ -0,0 +1,144 @@ +// SPDX-License-Identifier: Apache-2.0 + +package watch + +import ( + "context" + "fmt" + "net" + "strings" + "time" + + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/ConfigButler/gitops-reverser/internal/kubeconfig" +) + +// sourceClusterDialTimeout bounds CONNECTION ESTABLISHMENT to a remote source cluster (the TCP +// dial + TLS handshake), not the whole request. A remote is reached over a network the in-cluster +// config is not, so an unreachable API server must surface as SourceClusterReachable=False +// promptly instead of hanging the catalog-refresh loop. It is applied as a dialer timeout (not +// rest.Config.Timeout), so a long-lived watch built from the same config is NEVER cut off after +// this interval; finite discovery calls get it as a request timeout on a separate config copy +// (see clusterDiscovery). +const sourceClusterDialTimeout = 15 * time.Second + +// sourceClusterKeepAlive is the TCP keep-alive on the dialer above, so an idle watch connection +// to a remote is kept healthy rather than silently half-open. +const sourceClusterKeepAlive = 30 * time.Second + +// secretSourceClusterResolver resolves a source-cluster id — "//", as +// (api/v1alpha3).GitTarget.SourceClusterID() renders it — into a rest.Config, by reading that +// Secret from the CONFIG PLANE: the cluster the operator runs in. +// +// This is the whole point of the split. The credential for a cluster never has to live on +// that cluster, and the watched cluster holds nothing but the watched resources — no Secret, +// no configbutler.ai CRDs at all. +type secretSourceClusterResolver struct { + // client reads Secrets from the config plane. Secret reads bypass the controller-runtime + // cache (the manager client sets Cache.DisableFor on Secrets), so a rotated kubeconfig is + // seen without a Secret informer — the same reasoning as the SOPS age-key reads. + client client.Client + + // safety is the operator's exec / insecure-TLS opt-in. Both default off: an + // operator-supplied kubeconfig is attacker-adjacent input, so we REJECT rather than + // silently strip (diverging from Flux) unless a flag opts in. + safety kubeconfig.SafetyPolicy + + // qps and burst bound the rate at which the operator talks to a source cluster. A remote + // cluster is reached over a network the local one is not, so it gets client-side + // throttling the in-cluster config does not carry by default. + qps float32 + burst int +} + +// NewSecretSourceClusterResolver builds the production source-cluster resolver. +func NewSecretSourceClusterResolver( + c client.Client, + safety kubeconfig.SafetyPolicy, + qps float32, + burst int, +) SourceClusterResolver { + return &secretSourceClusterResolver{client: c, safety: safety, qps: qps, burst: burst} +} + +// sourceClusterRef is a source-cluster id parsed back into the Secret it names. +type sourceClusterRef struct { + Namespace string + Name string + Key string +} + +// sourceClusterIDSegments is the number of "/"-separated parts in a source-cluster id. +const sourceClusterIDSegments = 3 + +// parseSourceClusterID splits the id GitTarget.SourceClusterID() produces. It is a private +// encoding, never a user input: a GitTarget's namespace cannot contain "/" and neither can a +// Secret name, so the first two segments are unambiguous and the rest is the data key. The KEY +// segment MAY be empty — an omitted spec key is its own identity, and the resolver then falls +// back value→value.yaml. +func parseSourceClusterID(id string) (sourceClusterRef, error) { + parts := strings.SplitN(id, "/", sourceClusterIDSegments) + if len(parts) != sourceClusterIDSegments || parts[0] == "" || parts[1] == "" { + return sourceClusterRef{}, fmt.Errorf("malformed source cluster id %q, want //", id) + } + return sourceClusterRef{Namespace: parts[0], Name: parts[1], Key: parts[2]}, nil +} + +func (r *secretSourceClusterResolver) ResolveSourceCluster( + ctx context.Context, + clusterID string, +) (*rest.Config, string, error) { + ref, err := parseSourceClusterID(clusterID) + if err != nil { + return nil, "", err + } + + var secret corev1.Secret + if err := r.client.Get(ctx, client.ObjectKey{Namespace: ref.Namespace, Name: ref.Name}, &secret); err != nil { + return nil, "", fmt.Errorf("read kubeconfig Secret %s/%s: %w", ref.Namespace, ref.Name, err) + } + raw, usedKey, ok := kubeconfig.ResolveKey(secret.Data, ref.Key) + if !ok { + return nil, "", &kubeconfig.RejectionError{ + Reason: kubeconfig.ReasonKeyNotFound, + Message: fmt.Sprintf("kubeconfig Secret %s/%s has no kubeconfig under key %q", + ref.Namespace, ref.Name, describeKey(ref.Key)), + } + } + + // Parse and REJECT unsafe kubeconfigs before building the config — a legible failure that + // the controller's Validated gate reports with the same typed reason. Never dials. + cfg, err := kubeconfig.BuildRESTConfig(raw, r.safety) + if err != nil { + return nil, "", fmt.Errorf("kubeconfig Secret %s/%s key %q: %w", + ref.Namespace, ref.Name, usedKey, err) + } + if r.qps > 0 { + cfg.QPS = r.qps + cfg.Burst = r.burst + } + // Bound CONNECTION SETUP so an unreachable remote surfaces as SourceClusterReachable=False + // promptly — but do NOT set rest.Config.Timeout, which applies to the full HTTP request and + // would cut off a long-lived watch every interval and churn reconnects. A dialer timeout bounds + // the TCP/TLS handshake only; the established watch stream then stays open indefinitely. Finite + // discovery/list calls are bounded separately (clusterDiscovery copies the config with a request + // timeout; list callers pass a context deadline). + cfg.Dial = (&net.Dialer{Timeout: sourceClusterDialTimeout, KeepAlive: sourceClusterKeepAlive}).DialContext + + // The Secret's resourceVersion is the version token: it changes on every rotation, and on + // nothing else. The kubeconfig bytes themselves are dropped here — only the built + // rest.Config survives the call. + return cfg, secret.ResourceVersion, nil +} + +// describeKey renders the resolved-key hint for a "key not found" message: an omitted spec key +// tried the value→value.yaml fallback. +func describeKey(specKey string) string { + if specKey == "" { + return "value or value.yaml" + } + return specKey +} diff --git a/internal/watch/source_cluster_resolver_test.go b/internal/watch/source_cluster_resolver_test.go new file mode 100644 index 00000000..fe40c033 --- /dev/null +++ b/internal/watch/source_cluster_resolver_test.go @@ -0,0 +1,136 @@ +// SPDX-License-Identifier: Apache-2.0 + +package watch + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/ConfigButler/gitops-reverser/internal/kubeconfig" +) + +const resolverKubeConfig = `apiVersion: v1 +kind: Config +clusters: +- name: c + cluster: + server: https://192.0.2.1:6443 + certificate-authority-data: dGVzdA== +contexts: +- name: c + context: {cluster: c, user: u} +current-context: c +users: +- name: u + user: + token: dummy-token +` + +const resolverExecKubeConfig = `apiVersion: v1 +kind: Config +clusters: +- name: c + cluster: {server: https://192.0.2.1:6443} +contexts: +- name: c + context: {cluster: c, user: u} +current-context: c +users: +- name: u + user: + exec: + apiVersion: client.authentication.k8s.io/v1 + command: /bin/echo + interactiveMode: Never +` + +func TestParseSourceClusterID(t *testing.T) { + ref, err := parseSourceClusterID("team-a/kc/value") + require.NoError(t, err) + assert.Equal(t, sourceClusterRef{Namespace: "team-a", Name: "kc", Key: "value"}, ref) + + // An empty key segment is valid — an omitted spec key is its own identity. + ref, err = parseSourceClusterID("team-a/kc/") + require.NoError(t, err) + assert.Equal(t, sourceClusterRef{Namespace: "team-a", Name: "kc", Key: ""}, ref) + + for _, bad := range []string{"", "onlyone", "ns/name", "/name/key", "ns//key"} { + _, err := parseSourceClusterID(bad) + assert.Error(t, err, "malformed id %q must error", bad) + } +} + +func newResolver(t *testing.T, secret *corev1.Secret, safety kubeconfig.SafetyPolicy) SourceClusterResolver { + t.Helper() + builder := fake.NewClientBuilder() + if secret != nil { + builder = builder.WithObjects(secret) + } + return NewSecretSourceClusterResolver(builder.Build(), safety, 20, 30) +} + +func kubeconfigSecret(key, body string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "kc"}, + Data: map[string][]byte{key: []byte(body)}, + } +} + +func TestResolveSourceCluster_ValidAppliesThrottleAndVersion(t *testing.T) { + secret := kubeconfigSecret("value", resolverKubeConfig) + r := newResolver(t, secret, kubeconfig.SafetyPolicy{}) + + cfg, version, err := r.ResolveSourceCluster(context.Background(), "team-a/kc/value") + require.NoError(t, err) + require.NotNil(t, cfg) + assert.Equal(t, "https://192.0.2.1:6443", cfg.Host) + assert.InDelta(t, 20.0, cfg.QPS, 0.001, "source-cluster QPS applied") + assert.Equal(t, 30, cfg.Burst, "source-cluster burst applied") + assert.NotEmpty(t, version, "the Secret resourceVersion is the version token") +} + +func TestResolveSourceCluster_KeyFallbackValueYaml(t *testing.T) { + // Secret stores under value.yaml (Flux Kustomization shape); id has an empty key segment. + secret := kubeconfigSecret("value.yaml", resolverKubeConfig) + r := newResolver(t, secret, kubeconfig.SafetyPolicy{}) + + cfg, _, err := r.ResolveSourceCluster(context.Background(), "team-a/kc/") + require.NoError(t, err) + assert.Equal(t, "https://192.0.2.1:6443", cfg.Host) +} + +func TestResolveSourceCluster_MissingSecretAndKey(t *testing.T) { + r := newResolver(t, nil, kubeconfig.SafetyPolicy{}) + _, _, err := r.ResolveSourceCluster(context.Background(), "team-a/absent/value") + require.Error(t, err, "an absent Secret is an error, not a nil config") + + // Secret present but no kubeconfig under the resolved key -> typed KeyNotFound. + secret := kubeconfigSecret("elsewhere", resolverKubeConfig) + r = newResolver(t, secret, kubeconfig.SafetyPolicy{}) + _, _, err = r.ResolveSourceCluster(context.Background(), "team-a/kc/value") + require.Error(t, err) + rej, ok := kubeconfig.AsRejection(err) + require.True(t, ok) + assert.Equal(t, kubeconfig.ReasonKeyNotFound, rej.Reason) +} + +func TestResolveSourceCluster_RejectsUnsafe(t *testing.T) { + secret := kubeconfigSecret("value", resolverExecKubeConfig) + r := newResolver(t, secret, kubeconfig.SafetyPolicy{}) + _, _, err := r.ResolveSourceCluster(context.Background(), "team-a/kc/value") + require.Error(t, err) + rej, ok := kubeconfig.AsRejection(err) + require.True(t, ok) + assert.Equal(t, kubeconfig.ReasonExecNotAllowed, rej.Reason) + + // Opting in (a deliberate trust decision) lets it through. + r = newResolver(t, secret, kubeconfig.SafetyPolicy{AllowExec: true}) + _, _, err = r.ResolveSourceCluster(context.Background(), "team-a/kc/value") + require.NoError(t, err) +} diff --git a/internal/watch/stream_readiness.go b/internal/watch/stream_readiness.go index c6c3c265..b3a01cb4 100644 --- a/internal/watch/stream_readiness.go +++ b/internal/watch/stream_readiness.go @@ -128,10 +128,13 @@ func (m *Manager) StreamSummaryForGitTarget(gitDest types.ResourceReference) Str return m.streamSummaryForExpectedKeys(gitDest, sortedTargetWatchSpecKeys(specs), names) } -// StreamSummaryForWatchRule reports stream readiness for one namespaced WatchRule. +// StreamSummaryForWatchRule reports stream readiness for one namespaced WatchRule, resolved +// against the source cluster its GitTarget mirrors from. func (m *Manager) StreamSummaryForWatchRule(rule configv1alpha3.WatchRule) StreamSummary { - m.refreshTypeRegistry() - records := m.typeRegistryInstance().Followable() + gitDest := types.NewResourceReference(rule.Spec.TargetRef.Name, rule.Namespace) + reg := m.registryForGitTarget(gitDest) + m.refreshClusterTypeRegistry(m.cluster(m.clusterIDForGitTarget(gitDest))) + records := reg.Followable() var keys []targetWatchKey names := map[schema.GroupVersionResource]string{} for _, rr := range rule.Spec.Rules { @@ -143,17 +146,16 @@ func (m *Manager) StreamSummaryForWatchRule(rule configv1alpha3.WatchRule) Strea names[rec.Identity.GVR] = streamDisplayName(rec.Identity.GVR) } } - return m.streamSummaryForExpectedKeys( - types.NewResourceReference(rule.Spec.TargetRef.Name, rule.Namespace), - deduplicateTargetWatchKeys(keys), - names, - ) + return m.streamSummaryForExpectedKeys(gitDest, deduplicateTargetWatchKeys(keys), names) } -// StreamSummaryForClusterWatchRule reports stream readiness for one ClusterWatchRule. +// StreamSummaryForClusterWatchRule reports stream readiness for one ClusterWatchRule, resolved +// against the source cluster its GitTarget mirrors from. func (m *Manager) StreamSummaryForClusterWatchRule(rule configv1alpha3.ClusterWatchRule) StreamSummary { - m.refreshTypeRegistry() - records := m.typeRegistryInstance().Followable() + gitDest := types.NewResourceReference(rule.Spec.TargetRef.Name, rule.Spec.TargetRef.Namespace) + reg := m.registryForGitTarget(gitDest) + m.refreshClusterTypeRegistry(m.cluster(m.clusterIDForGitTarget(gitDest))) + records := reg.Followable() var keys []targetWatchKey names := map[schema.GroupVersionResource]string{} for _, rr := range rule.Spec.Rules { @@ -163,11 +165,7 @@ func (m *Manager) StreamSummaryForClusterWatchRule(rule configv1alpha3.ClusterWa names[rec.Identity.GVR] = streamDisplayName(rec.Identity.GVR) } } - return m.streamSummaryForExpectedKeys( - types.NewResourceReference(rule.Spec.TargetRef.Name, rule.Spec.TargetRef.Namespace), - deduplicateTargetWatchKeys(keys), - names, - ) + return m.streamSummaryForExpectedKeys(gitDest, deduplicateTargetWatchKeys(keys), names) } func (m *Manager) streamSummaryForExpectedKeys( diff --git a/internal/watch/target_watch.go b/internal/watch/target_watch.go index 8e43c779..010c17aa 100644 --- a/internal/watch/target_watch.go +++ b/internal/watch/target_watch.go @@ -73,17 +73,22 @@ func (m *Manager) EnsureGitTargetWatches( if m.EventRouter == nil { return nil } - if err := m.RefreshAPIResourceCatalog(ctx); err != nil { + // Refresh ONLY this GitTarget's own source cluster on the declare path — never every active + // cluster. Refreshing all of them here means a healthy target's declare (which runs on the + // single GitTarget controller worker) blocks on an UNREACHABLE other cluster's full dial + // timeout, starving that healthy target's status. Cross-cluster catalog freshness and every + // cluster's SourceClusterReachable ride the background RefreshAPIResourceCatalog loop instead. + if err := m.refreshClusterForDeclare(ctx, m.clusterIDForGitTarget(gitDest)); err != nil { return fmt.Errorf("refresh API resource catalog for %s: %w", gitDest.String(), err) } m.refreshWatchedTypeTables() - if !m.typeRegistryInstance().Ready() { + if !m.registryForGitTarget(gitDest).Ready() { return fmt.Errorf("aborting watch setup for %s: the cluster API surface has not been observed yet", gitDest.String()) } table := m.residentWatchedTypeTable(gitDest) - if retained := m.retainedWatchedTypes(table); len(retained) > 0 { + if retained := m.retainedWatchedTypes(gitDest, table); len(retained) > 0 { return fmt.Errorf("aborting watch setup for %s: %s within the removal grace (currently unserved)", gitDest.String(), gvkListSummary(retained)) } @@ -343,7 +348,7 @@ func (m *Manager) targetWatchReplayAndStream( "target watch replay in progress", ) replaying := true - w, err := m.openTargetWatch(ctx, key.GVR, key.Namespace, opts) + w, err := m.openTargetWatch(ctx, m.clusterIDForGitTarget(gitDest), key.GVR, key.Namespace, opts) if err != nil { if watchListUnsupported(err) { log.Error(err, "WARNING: sendInitialEvents unsupported; falling back to LIST plus buffered WATCH", @@ -392,7 +397,7 @@ func (m *Manager) targetWatchResumeAndStream( ops OperationSet, cursor string, ) error { - w, err := m.openTargetWatch(ctx, key.GVR, key.Namespace, metav1.ListOptions{ + w, err := m.openTargetWatch(ctx, m.clusterIDForGitTarget(gitDest), key.GVR, key.Namespace, metav1.ListOptions{ ResourceVersion: cursor, AllowWatchBookmarks: true, }) @@ -434,7 +439,8 @@ func (m *Manager) targetWatchListAndStream( key targetWatchKey, ops OperationSet, ) error { - w, err := m.openTargetWatch(ctx, key.GVR, key.Namespace, metav1.ListOptions{ + clusterID := m.clusterIDForGitTarget(gitDest) + w, err := m.openTargetWatch(ctx, clusterID, key.GVR, key.Namespace, metav1.ListOptions{ AllowWatchBookmarks: true, }) if err != nil { @@ -455,7 +461,7 @@ func (m *Manager) targetWatchListAndStream( buffered := make(chan watch.Event, targetWatchBufferCapacity) go bufferTargetWatchEvents(ctx, w.ResultChan(), buffered) - list, err := m.openTargetList(ctx, key.GVR, key.Namespace, metav1.ListOptions{}) + list, err := m.openTargetList(ctx, clusterID, key.GVR, key.Namespace, metav1.ListOptions{}) if err != nil { if ctx.Err() != nil { return nil @@ -689,6 +695,9 @@ func (m *Manager) routeLiveTargetWatchEvent( return rv, nil } event := targetWatchGitEvent(key.GVR, u, op) + // Carry the source cluster so the git writer resolves this document's GVK->GVR + // against the cluster it was watched on, never a union of all clusters. + event.SourceClusterID = m.clusterIDForGitTarget(gitDest) // Drop a no-op UPDATE before it reaches the worker: a /status-only change // sanitizes to identical git content but ships unattributed (its /status audit // is dropped), so routing it would split an open commit window on the author @@ -856,8 +865,13 @@ func (s OperationSet) Match(op string) bool { return ok } +// openTargetWatch opens a watch against the cluster the GitTarget mirrors from. clusterID is +// LocalClusterID for a single-cluster GitTarget, which resolves to the in-cluster dynamic +// client exactly as before; a remote id resolves to that source cluster's dynamic client, +// built from its kubeconfig Secret. func (m *Manager) openTargetWatch( ctx context.Context, + clusterID string, gvr schema.GroupVersionResource, namespace string, opts metav1.ListOptions, @@ -865,9 +879,9 @@ func (m *Manager) openTargetWatch( if m.targetWatchOpen != nil { return m.targetWatchOpen(ctx, gvr, namespace, opts) } - dc := m.dynamicClientFromConfig(m.Log) - if dc == nil { - return nil, errors.New("no dynamic client for target watch") + dc, err := m.clusterDynamicClient(ctx, clusterID) + if err != nil { + return nil, err } resource := dc.Resource(gvr) if namespace != "" { @@ -878,6 +892,7 @@ func (m *Manager) openTargetWatch( func (m *Manager) openTargetList( ctx context.Context, + clusterID string, gvr schema.GroupVersionResource, namespace string, opts metav1.ListOptions, @@ -885,9 +900,9 @@ func (m *Manager) openTargetList( if m.targetWatchList != nil { return m.targetWatchList(ctx, gvr, namespace, opts) } - dc := m.dynamicClientFromConfig(m.Log) - if dc == nil { - return nil, errors.New("no dynamic client for target watch list") + dc, err := m.clusterDynamicClient(ctx, clusterID) + if err != nil { + return nil, err } resource := dc.Resource(gvr) if namespace != "" { diff --git a/internal/watch/target_watch_test.go b/internal/watch/target_watch_test.go index f3c97729..ce4550cd 100644 --- a/internal/watch/target_watch_test.go +++ b/internal/watch/target_watch_test.go @@ -504,6 +504,7 @@ func TestOpenTargetWatch_UsesConfiguredHook(t *testing.T) { w, err := manager.openTargetWatch( context.Background(), + LocalClusterID, configmapsGVR, "apps", metav1.ListOptions{ResourceVersion: "42"}, diff --git a/internal/watch/watched_type_resolver.go b/internal/watch/watched_type_resolver.go index 3eeaadba..ffb3cd28 100644 --- a/internal/watch/watched_type_resolver.go +++ b/internal/watch/watched_type_resolver.go @@ -36,8 +36,17 @@ type watchedTypeStore struct { refreshMu sync.Mutex mu sync.Mutex tables map[string]WatchedTypeTable - revision uint64 - rulesFP uint64 + // registriesFP fingerprints each active cluster's id paired with its registry revision, so a + // discovery change on any source cluster re-projects. Unlike a naked sum of revisions it also + // moves when the SET of active clusters changes even if the totals coincide. + registriesFP uint64 + rulesFP uint64 + // clusterFP fingerprints the GitTarget->source-cluster mapping. It is the third leg of the + // re-projection gate: registriesFP knows the active clusters but not which GitTarget maps to + // which, so it cannot see two GitTargets swapping clusters at a fixed active set; and the rules + // fingerprint knows nothing about clusters at all. Without it a retargeted GitTarget could keep + // the previous cluster's GVR table. + clusterFP uint64 resolved bool } @@ -56,33 +65,43 @@ func (m *Manager) refreshWatchedTypeTables() { m.watchedTypes.refreshMu.Lock() defer m.watchedTypes.refreshMu.Unlock() - // Lazily populate the registry the first time (unit tests drive this path without - // RefreshAPIResourceCatalog); in production the catalog refresh keeps it current, so - // the heavy scan→registry rebuild stays off this path. - if !m.typeRegistryInstance().Ready() { - m.refreshTypeRegistry() + // Lazily populate each active cluster's registry the first time (unit tests drive this + // path without RefreshAPIResourceCatalog); in production the catalog refresh keeps them + // current, so the heavy scan→registry rebuild stays off this path. + for _, id := range m.activeClusterIDs() { + cc := m.cluster(id) + if !cc.registry.Ready() { + m.refreshClusterTypeRegistry(cc) + } } - reg := m.typeRegistryInstance() - revision := reg.Revision() + // registriesFP spans every active cluster's (id, revision), so a remote CRD change — or a + // change in the set of active clusters — re-projects the tables even when the local registry + // is unchanged. + registriesFP := m.activeRegistriesFingerprint() fingerprint := m.rulesFingerprint() + // The cluster mapping fingerprint catches a GitTarget switching source clusters at a fixed + // active set, which neither registriesFP nor the rules fingerprint can see. + clusterFP := m.clusterMappingFingerprint() m.watchedTypes.mu.Lock() upToDate := m.watchedTypes.resolved && - m.watchedTypes.revision == revision && - m.watchedTypes.rulesFP == fingerprint + m.watchedTypes.registriesFP == registriesFP && + m.watchedTypes.rulesFP == fingerprint && + m.watchedTypes.clusterFP == clusterFP m.watchedTypes.mu.Unlock() if upToDate { return } - tables := m.resolveWatchedTypeTables(reg.Generation()) + tables := m.resolveWatchedTypeTables() m.watchedTypes.mu.Lock() previous := m.watchedTypes.tables m.watchedTypes.tables = tables - m.watchedTypes.revision = revision + m.watchedTypes.registriesFP = registriesFP m.watchedTypes.rulesFP = fingerprint + m.watchedTypes.clusterFP = clusterFP m.watchedTypes.resolved = true m.watchedTypes.mu.Unlock() @@ -177,15 +196,62 @@ type targetSelections struct { selections []watchSelection } -// resolveWatchedTypeTables projects every GitTarget's rules onto the type registry's -// followable set: a WatchRule scopes its records to its own namespace, a ClusterWatchRule -// streams them cluster-wide. A GitTarget whose rules select nothing followable is kept as -// an empty table so a transient discovery gap does not look like rule removal. -func (m *Manager) resolveWatchedTypeTables(generation uint64) map[string]WatchedTypeTable { +// activeRegistriesFingerprint hashes each active cluster's id paired with its registry revision, +// so the watched-type re-projection gate fires whenever ANY source cluster's discovery changes — +// and, unlike a naked SUM of revisions, it also moves when the SET of active clusters changes even +// if the totals happen to coincide (a cluster added and another removed at the same revision). +// Registry revisions are monotonic per cluster, so (sorted id, revision) pairs are a sound change +// detector. In a single-cluster install it is just the local registry's (id, revision). +func (m *Manager) activeRegistriesFingerprint() uint64 { + ids := m.activeClusterIDs() // already sorted + parts := make([]string, 0, len(ids)) + for _, id := range ids { + parts = append(parts, fmt.Sprintf("%s=%d", id, m.cluster(id).registry.Revision())) + } + return xxhash.Sum64String(strings.Join(parts, "\x00")) +} + +// clusterMappingFingerprint hashes the GitTarget->source-cluster mapping so the watched-type +// re-projection gate fires whenever a GitTarget's source cluster changes. The combined registry +// revision is a sum across active clusters and the rules fingerprint carries no cluster identity, +// so neither notices a GitTarget being retargeted (delete + recreate against a different cluster) +// when the old and new clusters happen to have equal registry revisions. Folding this in means a +// recreated GitTarget always re-resolves its table against the cluster it now mirrors, never +// keeping the previous cluster's GVRs. In a single-cluster install the mapping is empty (every +// target is local) and this is a constant, so the gate behaves exactly as before. +func (m *Manager) clusterMappingFingerprint() uint64 { + m.gitTargetClustersMu.Lock() + parts := make([]string, 0, len(m.gitTargetClusters)) + for gitTargetKey, clusterID := range m.gitTargetClusters { + parts = append(parts, gitTargetKey+"\x1f"+clusterID) + } + m.gitTargetClustersMu.Unlock() + sort.Strings(parts) + return xxhash.Sum64String(strings.Join(parts, "\x00")) +} + +// resolveWatchedTypeTables projects every GitTarget's rules onto ITS OWN source cluster's +// followable set — never a union: a WatchRule scopes its records to its own namespace, a +// ClusterWatchRule streams them cluster-wide, and a GitTarget that mirrors a remote resolves +// against that remote's registry. A GitTarget whose rules select nothing followable is kept as +// an empty table so a transient discovery gap does not look like rule removal. The per-target +// table is stamped with its own cluster's registry generation. +func (m *Manager) resolveWatchedTypeTables() map[string]WatchedTypeTable { if m.RuleStore == nil { return map[string]WatchedTypeTable{} } - records := m.typeRegistryInstance().Followable() + + // Followable records are resolved per source cluster and cached, so several GitTargets + // sharing one cluster fold against one snapshot. + recordsByCluster := map[string][]typeset.TypeRecord{} + recordsFor := func(clusterID string) []typeset.TypeRecord { + if r, ok := recordsByCluster[clusterID]; ok { + return r + } + r := m.cluster(clusterID).registry.Followable() + recordsByCluster[clusterID] = r + return r + } byTarget := map[string]*targetSelections{} get := func(ref types.ResourceReference, providerNS, provider, branch, path string) *targetSelections { @@ -199,11 +265,12 @@ func (m *Manager) resolveWatchedTypeTables(generation uint64) map[string]Watched return ts } - m.collectWatchRuleSelections(records, get) - m.collectClusterWatchRuleSelections(records, get) + m.collectWatchRuleSelections(recordsFor, get) + m.collectClusterWatchRuleSelections(recordsFor, get) tables := make(map[string]WatchedTypeTable, len(byTarget)) for key, ts := range byTarget { + generation := m.cluster(m.clusterIDForGitTarget(ts.gitDest)).registry.Generation() table := buildWatchedTypeTable(ts.gitDest, generation, ts.selections) table.Dest = ts.dest tables[key] = table @@ -211,17 +278,17 @@ func (m *Manager) resolveWatchedTypeTables(generation uint64) map[string]Watched return tables } -// collectWatchRuleSelections folds every namespaced WatchRule into its GitTarget's -// selected records, scoping each record to the rule's own namespace. +// collectWatchRuleSelections folds every namespaced WatchRule into its GitTarget's selected +// records, scoping each record to the rule's own namespace and resolving against the +// GitTarget's own source cluster's followable set. func (m *Manager) collectWatchRuleSelections( - records []typeset.TypeRecord, + recordsFor func(clusterID string) []typeset.TypeRecord, get func(types.ResourceReference, string, string, string, string) *targetSelections, ) { for _, rule := range m.RuleStore.SnapshotWatchRules() { - ts := get( - types.NewResourceReference(rule.GitTargetRef, rule.GitTargetNamespace), - rule.GitProviderNamespace, rule.GitProviderRef, rule.Branch, rule.Path, - ) + targetRef := types.NewResourceReference(rule.GitTargetRef, rule.GitTargetNamespace) + records := recordsFor(m.clusterIDForGitTarget(targetRef)) + ts := get(targetRef, rule.GitProviderNamespace, rule.GitProviderRef, rule.Branch, rule.Path) for _, rr := range rule.ResourceRules { matched := matchFollowableRecords( records, rr.APIGroups, rr.APIVersions, rr.Resources, configv1alpha3.ResourceScopeNamespaced) @@ -234,17 +301,16 @@ func (m *Manager) collectWatchRuleSelections( } } -// collectClusterWatchRuleSelections folds every ClusterWatchRule into its GitTarget's -// selected records as cluster-wide streams. +// collectClusterWatchRuleSelections folds every ClusterWatchRule into its GitTarget's selected +// records as cluster-wide streams, resolving against the GitTarget's own source cluster. func (m *Manager) collectClusterWatchRuleSelections( - records []typeset.TypeRecord, + recordsFor func(clusterID string) []typeset.TypeRecord, get func(types.ResourceReference, string, string, string, string) *targetSelections, ) { for _, rule := range m.RuleStore.SnapshotClusterWatchRules() { - ts := get( - types.NewResourceReference(rule.GitTargetRef, rule.GitTargetNamespace), - rule.GitProviderNamespace, rule.GitProviderRef, rule.Branch, rule.Path, - ) + targetRef := types.NewResourceReference(rule.GitTargetRef, rule.GitTargetNamespace) + records := recordsFor(m.clusterIDForGitTarget(targetRef)) + ts := get(targetRef, rule.GitProviderNamespace, rule.GitProviderRef, rule.Branch, rule.Path) for _, rr := range rule.Rules { matched := matchFollowableRecords(records, rr.APIGroups, rr.APIVersions, rr.Resources, rr.Scope) for _, rec := range matched { diff --git a/internal/watch/watched_type_resolver_test.go b/internal/watch/watched_type_resolver_test.go index eb043635..f79384c4 100644 --- a/internal/watch/watched_type_resolver_test.go +++ b/internal/watch/watched_type_resolver_test.go @@ -97,7 +97,7 @@ func TestRefreshWatchedTypeTables_RuleChangeReResolves(t *testing.T) { func TestResolveWatchedTypeTables_NilRuleStoreIsEmpty(t *testing.T) { m := &Manager{Log: logr.Discard()} - assert.Empty(t, m.resolveWatchedTypeTables(0)) + assert.Empty(t, m.resolveWatchedTypeTables()) } func TestRefreshWatchedTypeTables_NoChangeReusesResolvedTables(t *testing.T) { @@ -108,14 +108,14 @@ func TestRefreshWatchedTypeTables_NoChangeReusesResolvedTables(t *testing.T) { ) manager.refreshWatchedTypeTables() manager.watchedTypes.mu.Lock() - firstRev := manager.watchedTypes.revision + firstRegFP := manager.watchedTypes.registriesFP firstFP := manager.watchedTypes.rulesFP manager.watchedTypes.mu.Unlock() // A second refresh with no rule or registry change is a no-op gate hit. manager.refreshWatchedTypeTables() manager.watchedTypes.mu.Lock() - assert.Equal(t, firstRev, manager.watchedTypes.revision) + assert.Equal(t, firstRegFP, manager.watchedTypes.registriesFP) assert.Equal(t, firstFP, manager.watchedTypes.rulesFP) manager.watchedTypes.mu.Unlock() } diff --git a/test/e2e/Taskfile.yml b/test/e2e/Taskfile.yml index 2a6d6ee7..6b2b463a 100644 --- a/test/e2e/Taskfile.yml +++ b/test/e2e/Taskfile.yml @@ -149,6 +149,13 @@ vars: ARGOCD_DIR: '{{.ARGOCD_DIR | default "test/e2e/setup/argocd"}}' ARGOCD_PORT: '{{.ARGOCD_PORT | default "18080"}}' ARGOCD_WAIT_TIMEOUT: '{{.ARGOCD_WAIT_TIMEOUT | default "300s"}}' + # kcp — installed ONLY for the source-cluster corner (docs/design/config-plane-split.md), + # the same way Argo CD hangs off the bi-directional corner. kcp-operator arrives as a Flux + # HelmRelease (test/e2e/setup/kcp/base), like every other e2e dependency; the source-cluster + # specs mirror kcp *workspaces* (cheap logical clusters) as remote source clusters. Not a + # dependency of prepare-e2e, so the other CI legs never pay for it. + KCP_DIR: '{{.KCP_DIR | default "test/e2e/setup/kcp"}}' + KCP_WAIT_TIMEOUT: '{{.KCP_WAIT_TIMEOUT | default "300s"}}' DEMO_MANIFESTS_DIR: 'test/e2e/setup/demo-only' DEMO_TUNNEL_CREDENTIALS: '{{.DEMO_MANIFESTS_DIR}}/cloudflared-public/tunnel-credentials.yaml' DEMO_PULL_SECRET: '{{.DEMO_MANIFESTS_DIR}}/vote/pull-secret.yaml' @@ -244,7 +251,7 @@ tasks: # E2E_REPORT_NAME keeps each shard's Ginkgo JSON report distinct. go run github.com/onsi/ginkgo/v2/ginkgo \ --procs={{.E2E_GINKGO_PROCS}} --timeout="{{.E2E_FULL_TIMEOUT}}" -v \ - --label-filter='{{.E2E_LABEL_FILTER | default "!image-refresh && !bi-directional"}}' \ + --label-filter='{{.E2E_LABEL_FILTER | default "!image-refresh && !bi-directional && !source-cluster"}}' \ --output-dir="{{.CS}}/{{.NAMESPACE}}" \ --json-report=ginkgo-report-{{.E2E_REPORT_NAME | default "full"}}.json \ ./test/e2e/ @@ -322,6 +329,35 @@ tasks: --json-report=ginkgo-report-{{.E2E_REPORT_NAME | default "bi-directional"}}.json \ ./test/e2e/ + test-e2e-source-cluster: + desc: >- + Run the source-cluster corner: kcp workspaces as remote source clusters + (GitTarget.spec.kubeConfig). Installs kcp into the cluster. + vars: + # One process: the specs share the kcp front-proxy port-forward and the workspaces + # they provision, and the suite is Ordered — parallel processes would contend on the + # local forward port and the shared control plane. + SOURCE_CLUSTER_GINKGO_PROCS: '{{.SOURCE_CLUSTER_GINKGO_PROCS | default "1"}}' + SOURCE_CLUSTER_TIMEOUT: '{{.SOURCE_CLUSTER_TIMEOUT | default "25m"}}' + cmds: + # Sequential, not deps: _prepare-e2e-ready restarts the k3d server node when it injects + # the webhook TLS cert, which would disrupt a kcp install running concurrently — the + # same reason test-e2e-bi-directional chains rather than fans. + - task: prepare-e2e + - task: _kcp-ready + - | + export CTX="{{.CTX}}" + export INSTALL_MODE="{{.INSTALL_MODE}}" + export NAMESPACE="{{.NAMESPACE}}" + export E2E_AGE_KEY_FILE="{{.CS}}/age-key.txt" + export E2E_ENABLE_SOURCE_CLUSTER=true + go run github.com/onsi/ginkgo/v2/ginkgo \ + --procs={{.SOURCE_CLUSTER_GINKGO_PROCS}} --timeout="{{.SOURCE_CLUSTER_TIMEOUT}}" -v \ + --label-filter='source-cluster' \ + --output-dir="{{.CS}}/{{.NAMESPACE}}" \ + --json-report=ginkgo-report-{{.E2E_REPORT_NAME | default "source-cluster"}}.json \ + ./test/e2e/ + argocd-ui: desc: Install Argo CD if needed, port-forward its UI, and print the admin password cmds: @@ -960,6 +996,31 @@ tasks: # a bump or a repo/mirror switch. printf '%s\n%s\n' "{{.ARGOCD_CHART_VERSION}}" "{{.ARGOCD_HELM_REPO}}" > "{{.CS}}/argocd.installed" + _kcp-ready: + # kcp for the source-cluster corner. NOT a dependency of prepare-e2e — only + # test-e2e-source-cluster pulls it in, so the other CI legs never install kcp. + # See docs/design/config-plane-split.md. Modeled on _argocd-installed. + # + # _cluster-ready + _flux-setup-ready, because the kcp-operator HelmRelease needs Flux + # and its dependsOn: cert-manager (both arrive via the Flux setup). + method: timestamp + deps: + - _flux-setup-ready + sources: + - '{{.CS}}/flux-setup.ready' + - '{{.KCP_DIR}}/**/*' + - 'hack/e2e/setup-kcp.sh' + generates: + - '{{.CS}}/kcp.ready' + status: + - test -f "{{.CS}}/kcp.ready" + - test -z "$(find "{{.KCP_DIR}}" hack/e2e/setup-kcp.sh -type f -newer "{{.CS}}/kcp.ready" -print -quit)" + - | + kubectl --context "{{.CTX}}" -n kcp get secret kcp-admin-kubeconfig >/dev/null 2>&1 + cmds: + - CTX="{{.CTX}}" KCP_DIR="{{.KCP_DIR}}" KCP_WAIT_TIMEOUT="{{.KCP_WAIT_TIMEOUT}}" bash hack/e2e/setup-kcp.sh + - touch "{{.CS}}/kcp.ready" + _aggregated-api-ready: method: timestamp deps: diff --git a/test/e2e/kcp_workspace_test.go b/test/e2e/kcp_workspace_test.go new file mode 100644 index 00000000..e7257cb3 --- /dev/null +++ b/test/e2e/kcp_workspace_test.go @@ -0,0 +1,245 @@ +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "context" + "encoding/base64" + "fmt" + "net" + "os" + "os/exec" + "path/filepath" + "strings" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/client-go/tools/clientcmd" + + "github.com/ConfigButler/gitops-reverser/test/utils" +) + +// This file is the kcp harness for the source-cluster corner. kcp (installed by Flux via +// test/e2e/setup/kcp) gives us cheap LOGICAL clusters — workspaces — each a real Kubernetes +// API with its OWN namespaces and CRDs. A GitTarget mirrors a workspace exactly like any +// remote cluster: its spec.kubeConfig.secretRef names a kubeconfig that points at the kcp +// front-proxy at /clusters/. Because kcp allows isolated copies of the same +// Kind across workspaces, three workspaces serving the same namespace + resource is the +// cheapest possible proof that gitops-reverser keys everything by SOURCE CLUSTER. +// +// Two forms of every workspace kubeconfig: +// - test-runner form — server https://127.0.0.1:/clusters/, reached through a +// port-forward, with tls-server-name set to the front-proxy's real SAN. Used by the specs +// to provision workspaces and create resources inside them. +// - operator form — server https://frontproxy-front-proxy.kcp.svc.cluster.local:6443/ +// clusters/, the in-cluster Service DNS the operator Pod dials directly (its serving +// cert carries that SAN, so TLS verifies with kcp's real CA — no insecure-skip-tls-verify, +// no file-path credentials: the admin kubeconfig is fully embedded and passes the operator's +// kubeconfig safety checks unchanged). This is what goes into the GitTarget's Secret. + +const ( + kcpNamespace = "kcp" + kcpFrontProxyService = "frontproxy-front-proxy" + kcpFrontProxySNI = "frontproxy-front-proxy.kcp.svc.cluster.local" + kcpFrontProxyInCluster = "https://frontproxy-front-proxy.kcp.svc.cluster.local:6443" + kcpAdminSecret = "kcp-admin-kubeconfig" + // kcpLocalPort is the fixed local port the source-cluster suite forwards the front-proxy to. + // Fixed because the suite is Ordered and runs single-process (SOURCE_CLUSTER_GINKGO_PROCS=1). + kcpLocalPort = 16443 +) + +// kcpAvailable reports whether the kcp control plane is installed (the source-cluster corner's +// prepare ran). The suite skips rather than fails when it is absent, so a default `task test-e2e` +// (which does not install kcp) does not turn red on this corner. +func kcpAvailable() bool { + _, err := kubectlRun("-n", kcpNamespace, "get", "secret", kcpAdminSecret) + return err == nil +} + +// kcpTunnel is a persistent port-forward to the kcp front-proxy plus the derived kubeconfigs the +// test runner drives kcp with. Created once per source-cluster suite (BeforeAll), stopped in +// AfterAll. +type kcpTunnel struct { + cmd *exec.Cmd + cancel context.CancelFunc + // admin is the raw admin kubeconfig from Secret kcp/kcp-admin-kubeconfig (embedded certs). + admin []byte + // rootKubeconfig points at 127.0.0.1:/clusters/root — where Workspaces are created. + rootKubeconfig string + workdir string +} + +// startKcpTunnel reads the admin kubeconfig, opens the front-proxy port-forward, and derives the +// root-workspace kubeconfig. +func startKcpTunnel() *kcpTunnel { + GinkgoHelper() + encoded, err := kubectlRun("-n", kcpNamespace, "get", "secret", kcpAdminSecret, + "-o", "jsonpath={.data.kubeconfig}") + Expect(err).NotTo(HaveOccurred(), "read kcp admin kubeconfig Secret") + admin, err := base64.StdEncoding.DecodeString(strings.TrimSpace(encoded)) + Expect(err).NotTo(HaveOccurred(), "decode kcp admin kubeconfig") + + workdir, err := os.MkdirTemp("", "kcp-e2e-*") + Expect(err).NotTo(HaveOccurred()) + + cmd, cancel := startKcpPortForward() + + t := &kcpTunnel{cmd: cmd, cancel: cancel, admin: admin, workdir: workdir} + t.rootKubeconfig = t.writeDerivedKubeconfig("root", + fmt.Sprintf("https://127.0.0.1:%d/clusters/root", kcpLocalPort), kcpFrontProxySNI) + return t +} + +// startKcpPortForward forwards the front-proxy Service to the fixed local port and waits for it +// to accept connections. +func startKcpPortForward() (*exec.Cmd, context.CancelFunc) { + GinkgoHelper() + ctx, cancel := context.WithCancel(context.Background()) + args := kubectlArgs("-n", kcpNamespace, "port-forward", + "svc/"+kcpFrontProxyService, fmt.Sprintf("%d:6443", kcpLocalPort)) + cmd := exec.CommandContext(ctx, "kubectl", args...) + cmd.Stdout = GinkgoWriter + cmd.Stderr = GinkgoWriter + Expect(cmd.Start()).To(Succeed(), "start kcp front-proxy port-forward") + + Eventually(func() error { + conn, dialErr := net.DialTimeout("tcp", fmt.Sprintf("127.0.0.1:%d", kcpLocalPort), time.Second) + if dialErr == nil { + _ = conn.Close() + } + return dialErr + }).WithTimeout(30*time.Second).WithPolling(500*time.Millisecond). + Should(Succeed(), "kcp front-proxy port-forward never became reachable") + + return cmd, cancel +} + +func (t *kcpTunnel) stop() { + if t == nil { + return + } + if t.cancel != nil { + t.cancel() + } + if t.cmd != nil && t.cmd.Process != nil { + _ = t.cmd.Process.Kill() + _ = t.cmd.Wait() + } + _ = os.RemoveAll(t.workdir) +} + +// createWorkspace applies a universal-type Workspace under root, waits for it to be Ready, and +// returns its logical-cluster hash — the id both kubeconfig forms address as /clusters/. +func (t *kcpTunnel) createWorkspace(name string) string { + GinkgoHelper() + manifest := fmt.Sprintf(`apiVersion: tenancy.kcp.io/v1alpha1 +kind: Workspace +metadata: + name: %s +spec: + type: + name: universal + path: root +`, name) + _, err := kubectlWithKubeconfig(t.rootKubeconfig, manifest, "apply", "-f", "-") + Expect(err).NotTo(HaveOccurred(), "create kcp workspace %q", name) + + var hash string + Eventually(func(g Gomega) { + out, getErr := kubectlWithKubeconfig(t.rootKubeconfig, "", "get", "workspace", name, + "-o", `jsonpath={.status.phase}{" "}{.spec.cluster}`) + g.Expect(getErr).NotTo(HaveOccurred()) + fields := strings.Fields(out) + g.Expect(fields).To(HaveLen(2), "workspace %q not yet assigned a cluster (%q)", name, out) + g.Expect(fields[0]).To(Equal("Ready"), "workspace %q phase", name) + hash = fields[1] + }).WithTimeout(90*time.Second).WithPolling(2*time.Second). + Should(Succeed(), "kcp workspace %q never became Ready", name) + return hash +} + +// deleteWorkspace best-effort removes a workspace on suite teardown. +func (t *kcpTunnel) deleteWorkspace(name string) { + _, _ = kubectlWithKubeconfig(t.rootKubeconfig, "", "delete", "workspace", name, "--ignore-not-found") +} + +// cleanupWorkspaceTarget tears down everything a workspace-mirror spec created, in dependency +// order: the WatchRule and GitTarget FIRST (so the operator forgets the source cluster and stops +// watching before its workspace disappears), then the Secret, then the workspace. Deleting these +// between specs matters — a GitTarget left pointing at a deleted workspace churns the operator +// (failing discovery every reconcile) and can starve a following spec's initial resync. +func (t *kcpTunnel) cleanupWorkspaceTarget(ns, ws string) { + _, _ = kubectlRunInNamespace(ns, "delete", "watchrule", ws+"-rule", "--ignore-not-found") + _, _ = kubectlRunInNamespace(ns, "delete", "gittarget", ws+"-target", "--ignore-not-found") + _, _ = kubectlRunInNamespace(ns, "delete", "secret", ws+"-kubeconfig", "--ignore-not-found") + t.deleteWorkspace(ws) +} + +// wsKubectl runs kubectl against a workspace (test-runner form), lazily writing that workspace's +// runner kubeconfig on first use. +func (t *kcpTunnel) wsKubectl(hash string, args ...string) (string, error) { + kc := filepath.Join(t.workdir, "ws-"+hash+".kubeconfig") + if _, err := os.Stat(kc); err != nil { + t.writeDerivedKubeconfigAt(kc, + fmt.Sprintf("https://127.0.0.1:%d/clusters/%s", kcpLocalPort, hash), kcpFrontProxySNI) + } + return kubectlWithKubeconfig(kc, "", args...) +} + +// operatorKubeConfig returns the kubeconfig the OPERATOR uses to watch a workspace: the in-cluster +// front-proxy Service DNS at /clusters/, with the credential embedded. It is what a +// GitTarget's spec.kubeConfig.secretRef Secret carries. +func (t *kcpTunnel) operatorKubeConfig(hash string) string { + GinkgoHelper() + raw, err := deriveKubeConfig(t.admin, kcpFrontProxyInCluster+"/clusters/"+hash, "") + Expect(err).NotTo(HaveOccurred(), "derive operator kubeconfig for workspace %q", hash) + return string(raw) +} + +func (t *kcpTunnel) writeDerivedKubeconfig(name, server, sni string) string { + GinkgoHelper() + path := filepath.Join(t.workdir, name+".kubeconfig") + t.writeDerivedKubeconfigAt(path, server, sni) + return path +} + +func (t *kcpTunnel) writeDerivedKubeconfigAt(path, server, sni string) { + GinkgoHelper() + raw, err := deriveKubeConfig(t.admin, server, sni) + Expect(err).NotTo(HaveOccurred(), "derive kubeconfig for %s", server) + Expect(os.WriteFile(path, raw, 0o600)).To(Succeed()) +} + +// deriveKubeConfig rewrites every cluster entry's server (and, when sni is non-empty, its +// tls-server-name) in a kcp kubeconfig, preserving the embedded credential. It is the Go +// equivalent of the reference repo's kcp-kubectl.sh rewrite — done in-process so the harness +// needs no python. +func deriveKubeConfig(adminRaw []byte, server, sni string) ([]byte, error) { + cfg, err := clientcmd.Load(adminRaw) + if err != nil { + return nil, fmt.Errorf("load kcp admin kubeconfig: %w", err) + } + for _, cluster := range cfg.Clusters { + cluster.Server = server + if sni != "" { + cluster.TLSServerName = sni + } + } + out, err := clientcmd.Write(*cfg) + if err != nil { + return nil, fmt.Errorf("write derived kubeconfig: %w", err) + } + return out, nil +} + +// kubectlWithKubeconfig runs kubectl against an explicit kubeconfig file (a kcp workspace), +// bypassing the k3d --context the other e2e helpers inject. +func kubectlWithKubeconfig(kubeconfigPath, stdin string, args ...string) (string, error) { + full := append([]string{"--kubeconfig=" + kubeconfigPath}, args...) + cmd := exec.CommandContext(context.Background(), "kubectl", full...) + if stdin != "" { + cmd.Stdin = strings.NewReader(stdin) + } + return utils.Run(cmd) +} diff --git a/test/e2e/setup/kcp/base/etcd.yaml b/test/e2e/setup/kcp/base/etcd.yaml new file mode 100644 index 00000000..0bc2ab95 --- /dev/null +++ b/test/e2e/setup/kcp/base/etcd.yaml @@ -0,0 +1,65 @@ +# Backing store for the kcp root shard. kcp-operator does not embed etcd — the RootShard +# references this external endpoint (instance.yaml -> spec.etcd.endpoints). A single-node +# etcd is right for an e2e control plane; it is a small, well-understood component, so a +# plain StatefulSet is the correct tool here (kcp itself is managed by the operator). +apiVersion: v1 +kind: Service +metadata: + name: etcd + namespace: kcp + labels: { app: etcd } +spec: + clusterIP: None + selector: { app: etcd } + ports: + - { name: client, port: 2379 } + - { name: peer, port: 2380 } +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: etcd + namespace: kcp + labels: { app: etcd } +spec: + serviceName: etcd + replicas: 1 + selector: + matchLabels: { app: etcd } + template: + metadata: + labels: { app: etcd } + spec: + containers: + - name: etcd + image: gcr.io/etcd-development/etcd:v3.5.17 + command: ["/usr/local/bin/etcd"] + args: + - --name=etcd0 + - --data-dir=/var/run/etcd/data + - --listen-client-urls=http://0.0.0.0:2379 + - --advertise-client-urls=http://etcd.kcp.svc.cluster.local:2379 + - --listen-peer-urls=http://0.0.0.0:2380 + - --initial-advertise-peer-urls=http://etcd.kcp.svc.cluster.local:2380 + - --initial-cluster=etcd0=http://etcd.kcp.svc.cluster.local:2380 + - --initial-cluster-state=new + - --auto-compaction-retention=1 + ports: + - { name: client, containerPort: 2379 } + - { name: peer, containerPort: 2380 } + readinessProbe: + httpGet: { path: /health, port: 2379 } + initialDelaySeconds: 5 + periodSeconds: 10 + resources: + requests: { cpu: 100m, memory: 128Mi } + limits: { memory: 512Mi } + volumeMounts: + - { name: data, mountPath: /var/run/etcd } + volumeClaimTemplates: + - metadata: + name: data + spec: + accessModes: ["ReadWriteOnce"] + resources: + requests: { storage: 1Gi } diff --git a/test/e2e/setup/kcp/base/issuer.yaml b/test/e2e/setup/kcp/base/issuer.yaml new file mode 100644 index 00000000..0569b923 --- /dev/null +++ b/test/e2e/setup/kcp/base/issuer.yaml @@ -0,0 +1,12 @@ +# The trust root for kcp's PKI. kcp-operator builds the shard + front-proxy CA/certs from +# this cert-manager Issuer. A self-signed issuer is what kcp-operator's own quickstart uses; +# cert-manager (already installed in the e2e cluster for gitops-reverser) does the rest, +# giving the front-proxy a real serving cert whose SAN is the in-cluster Service DNS the +# operator dials — so a GitTarget mirroring a workspace gets a verifiable TLS connection. +apiVersion: cert-manager.io/v1 +kind: Issuer +metadata: + name: kcp-pki + namespace: kcp +spec: + selfSigned: {} diff --git a/test/e2e/setup/kcp/base/kcp-operator.yaml b/test/e2e/setup/kcp/base/kcp-operator.yaml new file mode 100644 index 00000000..b52e1108 --- /dev/null +++ b/test/e2e/setup/kcp/base/kcp-operator.yaml @@ -0,0 +1,53 @@ +# kcp-operator runs kcp declaratively: the RootShard / FrontProxy / Kubeconfig CRs in +# instance.yaml describe the control plane and the operator reconciles the Deployments, PKI +# (via the kcp-pki cert-manager Issuer), and the front-proxy. It is installed BY Flux from +# the official chart — the same pattern as every other e2e dependency (cert-manager, gitea, +# valkey, prometheus-operator in ../flux/releases). We use the operator rather than a +# hand-rolled StatefulSet or the plain kcp chart because it mints the admin kubeconfig +# Secret declaratively (a Kubeconfig CR) and issues cert-manager serving certs with the +# right SAN — exactly what a GitTarget needs to dial a workspace over verifiable TLS. +apiVersion: source.toolkit.fluxcd.io/v1 +kind: HelmRepository +metadata: + name: kcp + namespace: flux-system +spec: + interval: 30m + url: https://kcp-dev.github.io/helm-charts +--- +apiVersion: helm.toolkit.fluxcd.io/v2 +kind: HelmRelease +metadata: + name: kcp-operator + namespace: flux-system +spec: + interval: 30m + timeout: 5m + releaseName: kcp-operator + # cert-manager is a hard prerequisite (the operator issues shard/front-proxy PKI via it). + # It is the first HelmRelease in the e2e setup to declare a dependsOn — the others + # reconcile in parallel, but kcp-operator's CRDs/certs must not race cert-manager's webhook. + dependsOn: + - name: cert-manager + namespace: cert-manager + chart: + spec: + # Pinned for reproducibility (the gitops-api reference leaves it floating). kcp-operator + # 0.7.7 deploys kcp v0.8.x, which serves tenancy.kcp.io/v1alpha1 Workspaces — all this + # harness needs. + chart: kcp-operator + version: "0.7.7" + sourceRef: + kind: HelmRepository + name: kcp + namespace: flux-system + targetNamespace: kcp-operator + install: + createNamespace: true + crds: Create + remediation: + retries: 3 + upgrade: + crds: CreateReplace + remediation: + retries: 3 diff --git a/test/e2e/setup/kcp/base/kustomization.yaml b/test/e2e/setup/kcp/base/kustomization.yaml new file mode 100644 index 00000000..c2257685 --- /dev/null +++ b/test/e2e/setup/kcp/base/kustomization.yaml @@ -0,0 +1,11 @@ +# The kcp control plane's Flux-installable base: namespace, etcd, the PKI Issuer, and the +# kcp-operator HelmRepository + HelmRelease. Applied by hack/e2e/setup-kcp.sh, which then +# waits for the operator + its CRDs before applying ../instance.yaml (the RootShard / +# FrontProxy / Kubeconfig CRs need the operator.kcp.io CRDs to exist first). +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: + - namespace.yaml + - etcd.yaml + - issuer.yaml + - kcp-operator.yaml diff --git a/test/e2e/setup/kcp/base/namespace.yaml b/test/e2e/setup/kcp/base/namespace.yaml new file mode 100644 index 00000000..544ac7f2 --- /dev/null +++ b/test/e2e/setup/kcp/base/namespace.yaml @@ -0,0 +1,8 @@ +# The kcp control plane's namespace. Holds etcd, the root shard (root-kcp), and the +# front-proxy (frontproxy-front-proxy) that GitTargets reach a workspace through. The +# kcp-operator itself installs into its own namespace (kcp-operator), created by the +# HelmRelease. +apiVersion: v1 +kind: Namespace +metadata: + name: kcp diff --git a/test/e2e/setup/kcp/instance.yaml b/test/e2e/setup/kcp/instance.yaml new file mode 100644 index 00000000..17e67b88 --- /dev/null +++ b/test/e2e/setup/kcp/instance.yaml @@ -0,0 +1,82 @@ +# The kcp control-plane instance, declared as kcp-operator CRs. Applied AFTER the operator +# and its CRDs are Established (see hack/e2e/setup-kcp.sh). The operator reconciles the +# root-shard + front-proxy Deployments and all PKI (via the kcp-pki Issuer), and mints the +# admin kubeconfig into Secret kcp/kcp-admin-kubeconfig. +# +# external.hostname is the front-proxy's IN-CLUSTER Service DNS on purpose: the gitops-reverser +# operator dials a workspace at https://frontproxy-front-proxy.kcp.svc.cluster.local:6443/ +# clusters/, and the operator-issued serving cert carries that name as a SAN, so the +# GitTarget's kubeconfig verifies TLS with the real kcp CA (no insecure-skip-tls-verify). +# +# No audit stream and no per-workspace OIDC here (unlike the gitops-api reference): the +# reverser mirrors a workspace by WATCHING it, and the e2e mints an admin-derived kubeconfig +# per workspace rather than using Dex. So the WorkspaceAuthentication feature gate, the audit +# ConfigMap/Secret, and the WorkspaceType/WorkspaceAuthenticationConfiguration are all omitted. +apiVersion: operator.kcp.io/v1alpha1 +kind: RootShard +metadata: + name: root + namespace: kcp +spec: + # Single replica: this is an e2e control plane on a single-node runner shared with the whole + # gitops-reverser stack (gitea, valkey, prometheus, flux, cert-manager). The operator defaults + # the shard and front-proxy to 2 each; HA is not what these specs test, so halve the footprint. + replicas: 1 + external: + hostname: frontproxy-front-proxy.kcp.svc.cluster.local + port: 6443 + certificates: + issuerRef: + group: cert-manager.io + kind: Issuer + name: kcp-pki + cache: + embedded: + enabled: true + etcd: + endpoints: + - http://etcd.kcp.svc.cluster.local:2379 + # ServiceAccount auth lets in-cluster clients reach workspaces with a mounted SA; harmless + # here (the GitTarget uses the admin-derived kubeconfig) and matches the reference topology. + auth: + serviceAccount: + enabled: true +--- +apiVersion: operator.kcp.io/v1alpha1 +kind: FrontProxy +metadata: + name: frontproxy + namespace: kcp +spec: + replicas: 1 + rootShard: + ref: + name: root + serviceTemplate: + spec: + # ClusterIP: reached in-cluster by the operator; no cloud LoadBalancer in e2e. + type: ClusterIP + auth: + serviceAccount: + enabled: true +--- +# A declarative admin kubeconfig (system:kcp:admin) written to Secret kcp/kcp-admin-kubeconfig. +# The e2e derives each workspace's GitTarget kubeconfig from this one by re-pointing the +# server at /clusters/. It is a self-contained client-certificate kubeconfig +# (embedded CA + cert + key, no file paths, no insecure-skip-tls-verify), so it passes the +# operator's kubeconfig safety checks unchanged. +apiVersion: operator.kcp.io/v1alpha1 +kind: Kubeconfig +metadata: + name: admin + namespace: kcp +spec: + username: kcp-admin + groups: + - system:kcp:admin + validity: 8766h + secretRef: + name: kcp-admin-kubeconfig + target: + frontProxyRef: + name: frontproxy diff --git a/test/e2e/source_cluster_e2e_test.go b/test/e2e/source_cluster_e2e_test.go new file mode 100644 index 00000000..f05b7ab2 --- /dev/null +++ b/test/e2e/source_cluster_e2e_test.go @@ -0,0 +1,454 @@ +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// This file is the source-cluster corner for the config-plane split +// (docs/design/config-plane-split.md): a GitTarget may name the cluster it mirrors FROM via +// spec.kubeConfig (Flux's meta.KubeConfigReference). +// +// Two kinds of spec live here: +// - Input-validation / reachability specs (Scenarios 1-3) need no remote cluster: they +// assert the controller's Validated / SourceClusterReachable projection from bad, valid- +// but-unroutable, and omitted kubeconfigs. +// - Remote-mirror specs (Scenarios 4, 8) mirror real REMOTE clusters — kcp WORKSPACES, +// cheap logical clusters installed by Flux (test/e2e/setup/kcp, see kcp_workspace_test.go). +// Scenario 8 is the centerpiece: three workspaces holding the SAME namespace + resource +// with different content, proving state is keyed by source cluster, not (namespace, GVR). +// +// The whole suite is gated by skipUnlessSourceClusterEnabled() (env E2E_ENABLE_SOURCE_CLUSTER), +// and the kcp specs additionally Skip when kcp is not installed — so a default `task test-e2e` +// (which installs no kcp) never turns them red. Run the corner with `task test-e2e-source-cluster`. + +const ( + sourceClusterEnabledEnv = "E2E_ENABLE_SOURCE_CLUSTER" + // unreachableAPIServer is an RFC 5737 TEST-NET-1 address: syntactically a valid + // server, guaranteed not to route, so a kubeconfig pointing at it parses cleanly + // (Validated=True) yet can never be dialed (SourceClusterReachable=False). + unreachableAPIServer = "https://192.0.2.1:6443" +) + +func sourceClusterEnabled() bool { + v := strings.ToLower(strings.TrimSpace(os.Getenv(sourceClusterEnabledEnv))) + return v == "1" || v == "true" || v == "yes" +} + +func skipUnlessSourceClusterEnabled() { + GinkgoHelper() + if !sourceClusterEnabled() { + Skip(fmt.Sprintf( + "source-cluster corner is disabled; run `task test-e2e-source-cluster` "+ + "(sets %s=true and installs kcp)", sourceClusterEnabledEnv)) + } +} + +// rawKubeConfigWithServer returns the current cluster's real, self-contained kubeconfig +// (embedded CA + client credential) with only the API server address swapped. Swapping to an +// unroutable address yields a "valid but unreachable" kubeconfig — Validated=True yet +// SourceClusterReachable=False — which is what the reachability specs assert. (Real remote +// mirroring is exercised against kcp workspaces, not a server swap; see kcp_workspace_test.go.) +func rawKubeConfigWithServer(server string) string { + GinkgoHelper() + raw, err := kubectlRun("config", "view", "--raw", "--minify", "-o", "yaml") + Expect(err).NotTo(HaveOccurred(), "failed to read current kubeconfig") + out := make([]string, 0, strings.Count(raw, "\n")+1) + for _, line := range strings.Split(raw, "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "server:") { + indent := line[:len(line)-len(strings.TrimLeft(line, " "))] + out = append(out, indent+"server: "+server) + continue + } + out = append(out, line) + } + return strings.Join(out, "\n") +} + +// insecureKubeConfig is structurally valid but disables TLS verification — the operator +// must reject it (KubeConfigInsecureTLSNotAllowed), diverging from Flux's silent strip. +func insecureKubeConfig() string { + return `apiVersion: v1 +kind: Config +clusters: +- name: c + cluster: + server: ` + unreachableAPIServer + ` + insecure-skip-tls-verify: true +contexts: +- name: c + context: {cluster: c, user: u} +current-context: c +users: +- name: u + user: + token: dummy-token +` +} + +// execKubeConfig is structurally valid but carries an exec auth provider — the operator +// must reject it (KubeConfigExecNotAllowed): an exec stanza runs a binary in the Pod. +func execKubeConfig() string { + return `apiVersion: v1 +kind: Config +clusters: +- name: c + cluster: + server: ` + unreachableAPIServer + ` +contexts: +- name: c + context: {cluster: c, user: u} +current-context: c +users: +- name: u + user: + exec: + apiVersion: client.authentication.k8s.io/v1 + command: /bin/echo + args: ["token"] +` +} + +// fileReferenceKubeConfig is structurally valid but names its token by FILE PATH — client-go +// would read that path from the operator Pod's own filesystem, so the operator must reject it +// (KubeConfigFileReferenceNotAllowed) rather than let a remote kubeconfig exfiltrate in-Pod files. +func fileReferenceKubeConfig() string { + return `apiVersion: v1 +kind: Config +clusters: +- name: c + cluster: + server: ` + unreachableAPIServer + ` + certificate-authority-data: dGVzdA== +contexts: +- name: c + context: {cluster: c, user: u} +current-context: c +users: +- name: u + user: + tokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token +` +} + +// writeKubeConfigSecret applies a Secret holding a kubeconfig under the given key. +func writeKubeConfigSecret(ns, name, key, kubeconfig string) { + GinkgoHelper() + f, err := os.CreateTemp("", "e2e-kubeconfig-*.yaml") + Expect(err).NotTo(HaveOccurred()) + defer func() { _ = os.Remove(f.Name()) }() + _, err = f.WriteString(kubeconfig) + Expect(err).NotTo(HaveOccurred()) + Expect(f.Close()).To(Succeed()) + + manifest, err := kubectlRunInNamespace(ns, "create", "secret", "generic", name, + "--from-file="+key+"="+f.Name(), "--dry-run=client", "-o", "yaml") + Expect(err).NotTo(HaveOccurred(), "failed to render kubeconfig Secret manifest") + _, err = kubectlRunWithStdin(ns, manifest, "apply", "-f", "-") + Expect(err).NotTo(HaveOccurred(), "failed to apply kubeconfig Secret") +} + +// applyGitTargetWithKubeConfig applies a GitTarget whose spec.kubeConfig.secretRef names a +// kubeconfig Secret. It returns the kubectl error so a spec can assert the apply succeeded (the +// spec is well-formed; the kubeconfig, not the CR, is what a validation case makes bad). +// +//nolint:unparam // provider is kept an explicit argument for readability; the corner uses one. +func applyGitTargetWithKubeConfig(ns, name, provider, path, secretName, key string) (string, error) { + keyLine := "" + if key != "" { + // key: must be a sibling of name: (6-space indent, under secretRef:), not nested + // under it — an 8-space indent produces "mapping values are not allowed here". + keyLine = "\n key: " + key + } + manifest := fmt.Sprintf(`apiVersion: configbutler.ai/v1alpha3 +kind: GitTarget +metadata: + name: %s + namespace: %s +spec: + providerRef: + kind: GitProvider + name: %s + branch: main + path: %s + kubeConfig: + secretRef: + name: %s%s +`, name, ns, provider, path, secretName, keyLine) + return kubectlRunWithStdin(ns, manifest, "apply", "-f", "-") +} + +// findFileByBasename walks a checkout and returns the first path whose basename matches, +// so a mirror assertion need not hard-code the exact placement path. +func findFileByBasename(root, basename string) string { + var hit string + _ = filepath.Walk(root, func(p string, info os.FileInfo, err error) error { + if err == nil && info != nil && !info.IsDir() && filepath.Base(p) == basename { + hit = p + } + return nil + }) + return hit +} + +var _ = Describe("Manager source cluster / config-plane split", Label("source-cluster"), Ordered, func() { + const providerName = "sc-provider" + + var ( + testNs string + repo *RepoArtifacts + // kcp is the workspace harness for the remote-mirror specs. It is nil when kcp is not + // installed (a default `task test-e2e` run) — those specs then Skip rather than fail. + kcp *kcpTunnel + ) + + BeforeAll(func() { + skipUnlessSourceClusterEnabled() + + testNs = testNamespaceFor("source-cluster") + _, _ = kubectlRun("create", "namespace", testNs) + + repo = SetupRepo(resolveE2EContext(), testNs, fmt.Sprintf("e2e-source-cluster-%d", GinkgoRandomSeed())) + _, err := kubectlRunInNamespace(testNs, "apply", "-f", repo.SecretsYAML) + Expect(err).NotTo(HaveOccurred(), "failed to apply repo secrets") + + createReadyGitProvider(providerName, testNs, repo.GitSecretHTTP, repo.RepoURLHTTP) + + if kcpAvailable() { + kcp = startKcpTunnel() + } + }) + + AfterAll(func() { + if kcp != nil { + kcp.stop() + } + cleanupNamespace(testNs) + }) + + SetDefaultEventuallyTimeout(60 * time.Second) + SetDefaultEventuallyPollingInterval(2 * time.Second) + + // Scenario 1 — input validation is legible, and never dials. + inputCases := []struct { + name string + reason string + setup func(ns string) (secretName, key string) + }{ + { + name: "a missing Secret", + reason: "KubeConfigSecretNotFound", + setup: func(_ string) (string, string) { return "sc-absent", "" }, + }, + { + name: "a missing key", + reason: "KubeConfigKeyNotFound", + setup: func(ns string) (string, string) { + kubeconfig := rawKubeConfigWithServer(unreachableAPIServer) + writeKubeConfigSecret(ns, "sc-wrongkey", "somewhere-else", kubeconfig) + return "sc-wrongkey", "value" + }, + }, + { + name: "an unparseable kubeconfig", + reason: "KubeConfigInvalid", + setup: func(ns string) (string, string) { + writeKubeConfigSecret(ns, "sc-garbage", "value", "this is not a kubeconfig") + return "sc-garbage", "" + }, + }, + { + name: "an exec auth provider", + reason: "KubeConfigExecNotAllowed", + setup: func(ns string) (string, string) { + writeKubeConfigSecret(ns, "sc-exec", "value", execKubeConfig()) + return "sc-exec", "" + }, + }, + { + name: "insecure TLS", + reason: "KubeConfigInsecureTLSNotAllowed", + setup: func(ns string) (string, string) { + writeKubeConfigSecret(ns, "sc-insecure", "value", insecureKubeConfig()) + return "sc-insecure", "" + }, + }, + { + name: "a file-path credential", + reason: "KubeConfigFileReferenceNotAllowed", + setup: func(ns string) (string, string) { + writeKubeConfigSecret(ns, "sc-filepath", "value", fileReferenceKubeConfig()) + return "sc-filepath", "" + }, + }, + } + for _, tc := range inputCases { + It("fails Validated (no dial) for "+tc.name+" with reason "+tc.reason, func() { + secretName, key := tc.setup(testNs) + target := "sc-input-" + strings.ToLower(tc.reason) + path := "clusters/input/" + tc.reason + // The GitTarget spec is well-formed (the kubeconfig is bad, not the CR), so the apply + // itself must succeed — the controller then reports the typed reason on Validated. A + // discarded apply error would hide a real rejection as a 90s "condition not found". + _, err := applyGitTargetWithKubeConfig(testNs, target, providerName, path, secretName, key) + Expect(err).NotTo(HaveOccurred()) + verifyResourceCondition("gittarget", target, testNs, "Validated", "False", tc.reason, "") + }) + } + + // Scenario 2 — a valid kubeconfig that cannot be dialed: Validated=True, reachability=False. + It("separates Validated (inputs) from SourceClusterReachable (runtime)", func() { + writeKubeConfigSecret(testNs, "sc-unreachable", "value", rawKubeConfigWithServer(unreachableAPIServer)) + target := "sc-unreachable-target" + _, err := applyGitTargetWithKubeConfig( + testNs, target, providerName, "clusters/unreachable", "sc-unreachable", "") + Expect(err).NotTo(HaveOccurred()) + + verifyResourceCondition("gittarget", target, testNs, "Validated", "True", "OK", "") + verifyResourceCondition("gittarget", target, testNs, + "SourceClusterReachable", "False", "SourceClusterUnreachable", "", "150s") + }) + + // Scenario 3 — omitted kubeConfig is unchanged local behavior. + It("treats an omitted kubeConfig as the local cluster", func() { + target := "sc-local-target" + manifest := fmt.Sprintf(`apiVersion: configbutler.ai/v1alpha3 +kind: GitTarget +metadata: {name: %s, namespace: %s} +spec: + providerRef: {kind: GitProvider, name: %s} + branch: main + path: clusters/local +`, target, testNs, providerName) + _, err := kubectlRunWithStdin(testNs, manifest, "apply", "-f", "-") + Expect(err).NotTo(HaveOccurred()) + + verifyResourceCondition("gittarget", target, testNs, + "SourceClusterReachable", "True", "LocalCluster", "") + }) + + // Scenario 4 — a REAL remote cluster: mirror a ConfigMap out of one kcp workspace. Proves + // the whole remote path end to end (resolver -> per-cluster clientContext -> per-cluster + // discovery -> per-cluster watch -> target-scoped writer) against an actually-remote API, + // not the self-referencing in-cluster server the scaffold used to fake it with. + It("mirrors a ConfigMap from a kcp workspace", func() { + if kcp == nil { + Skip("kcp is not installed; run this corner via `task test-e2e-source-cluster`") + } + const ws = "sc-mirror" + hash := kcp.createWorkspace(ws) + DeferCleanup(func() { kcp.cleanupWorkspaceTarget(testNs, ws) }) + + // A WatchRule watches its OWN namespace name on the source cluster, so the ConfigMap must + // live under that namespace (testNs) IN the workspace — a separate cluster, so creating a + // namespace of that name there is independent of the local one. + _, err := kcp.wsKubectl(hash, "create", "namespace", testNs) + Expect(err).NotTo(HaveOccurred(), "create watched namespace in the workspace") + _, err = kcp.wsKubectl(hash, "-n", testNs, "create", "configmap", "sc-remote-cm", + "--from-literal=hello=from-kcp") + Expect(err).NotTo(HaveOccurred(), "create ConfigMap in the workspace") + + writeKubeConfigSecret(testNs, ws+"-kubeconfig", "value", kcp.operatorKubeConfig(hash)) + target := ws + "-target" + _, err = applyGitTargetWithKubeConfig(testNs, target, providerName, "clusters/kcp", ws+"-kubeconfig", "") + Expect(err).NotTo(HaveOccurred()) + verifyResourceCondition("gittarget", target, testNs, "SourceClusterReachable", "True", "", "", "180s") + + rule := fmt.Sprintf(`apiVersion: configbutler.ai/v1alpha3 +kind: WatchRule +metadata: {name: %s-rule, namespace: %s} +spec: + targetRef: {kind: GitTarget, name: %s} + rules: + - resources: ["configmaps"] +`, ws, testNs, target) + _, err = kubectlRunWithStdin(testNs, rule, "apply", "-f", "-") + Expect(err).NotTo(HaveOccurred()) + waitForStreamsRunning(target, testNs) + + Eventually(func(g Gomega) { + pullLatestRepoState(g, repo.CheckoutDir) + hit := findFileByBasename(filepath.Join(repo.CheckoutDir, "clusters/kcp"), "sc-remote-cm.yaml") + g.Expect(hit).NotTo(BeEmpty(), "expected the ConfigMap mirrored from the kcp workspace") + content, readErr := os.ReadFile(hit) + g.Expect(readErr).NotTo(HaveOccurred()) + g.Expect(string(content)).To(ContainSubstring("from-kcp")) + }).WithTimeout(180 * time.Second).Should(Succeed()) + }) + + // Scenario 8 — the centerpiece: source-cluster identity is load-bearing. Three workspaces + // each hold the SAME namespace + the SAME resource name (demo/ConfigMap "shared") with + // DIFFERENT content, mirrored by three GitTargets into three folders. If the operator keyed + // state by (namespace, GVR) alone — a union / first-wins lookup — the three identical + // identities would collapse into one; that they land as three distinct files, each carrying + // its own workspace's value, is the proof that everything is keyed by SOURCE CLUSTER. + It("mirrors identical resources from three workspaces as distinct GitOps state", func() { + if kcp == nil { + Skip("kcp is not installed; run this corner via `task test-e2e-source-cluster`") + } + type wsCase struct{ ws, folder, value string } + cases := []wsCase{ + {"sc-ws-a", "clusters/a", "alpha"}, + {"sc-ws-b", "clusters/b", "beta"}, + {"sc-ws-c", "clusters/c", "gamma"}, + } + // Stand up all three mirrors, then assert on the OUTCOME (the mirrored files). We do not + // gate on each target's StreamsRunning in the loop: that condition can lag its stream, and + // the load-bearing claim here is what lands in Git, not the intermediate status. The final + // Eventually is generous because three remote initial-snapshots run concurrently. + for i := range cases { + c := cases[i] + hash := kcp.createWorkspace(c.ws) + DeferCleanup(func() { kcp.cleanupWorkspaceTarget(testNs, c.ws) }) + + // The SAME namespace name (testNs) and the SAME resource name (shared) in every + // workspace; only the value differs. A WatchRule watches its own namespace name on the + // source cluster, so testNs is created inside each workspace (a distinct cluster). + _, err := kcp.wsKubectl(hash, "create", "namespace", testNs) + Expect(err).NotTo(HaveOccurred(), "create namespace in %s", c.ws) + _, err = kcp.wsKubectl(hash, "-n", testNs, "create", "configmap", "shared", + "--from-literal=which="+c.value) + Expect(err).NotTo(HaveOccurred(), "create ConfigMap shared in %s", c.ws) + + writeKubeConfigSecret(testNs, c.ws+"-kubeconfig", "value", kcp.operatorKubeConfig(hash)) + target := c.ws + "-target" + _, err = applyGitTargetWithKubeConfig(testNs, target, providerName, c.folder, c.ws+"-kubeconfig", "") + Expect(err).NotTo(HaveOccurred()) + verifyResourceCondition("gittarget", target, testNs, "SourceClusterReachable", "True", "", "", "180s") + + rule := fmt.Sprintf(`apiVersion: configbutler.ai/v1alpha3 +kind: WatchRule +metadata: {name: %s-rule, namespace: %s} +spec: + targetRef: {kind: GitTarget, name: %s} + rules: + - resources: ["configmaps"] +`, c.ws, testNs, target) + _, err = kubectlRunWithStdin(testNs, rule, "apply", "-f", "-") + Expect(err).NotTo(HaveOccurred()) + } + + Eventually(func(g Gomega) { + pullLatestRepoState(g, repo.CheckoutDir) + for _, c := range cases { + hit := findFileByBasename(filepath.Join(repo.CheckoutDir, c.folder), "shared.yaml") + g.Expect(hit).NotTo(BeEmpty(), "expected shared mirrored under %s", c.folder) + content, readErr := os.ReadFile(hit) + g.Expect(readErr).NotTo(HaveOccurred()) + g.Expect(string(content)).To(ContainSubstring("which: "+c.value), + "folder %s must carry workspace %s's own value, not another workspace's", + c.folder, c.ws) + } + // Generous: the initial snapshot writes each mirror promptly, but the ceiling also + // covers one RequeueSteadyInterval (5m) periodic resync as a fallback. + }).WithTimeout(420 * time.Second).WithPolling(5 * time.Second).Should(Succeed()) + }) +})