Repository navigation
[docs] Add GitOps configuration surfaces design proposal - #16
myasnikovdaniil wants to merge 6 commits into
Conversation
Proposes a single GitOps primitive (GitRepository + Kustomization + a scoped ServiceAccount) instantiated at two trust tiers: platform GitOps for admins (cluster scope) and tenant GitOps for tenants (namespace scope), plus a packageOverrides admin override layer. Generalizes cozystack/cozystack#2731 as the first increment. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a design proposal for GitOps Configuration Surfaces in Cozystack, defining a unified primitive to enable platform GitOps for admins and tenant GitOps for users. The review feedback highlights several critical areas for improvement: first, Helm's default list-merging behavior could inadvertently overwrite default lists (like VM images) when applying packageOverrides; second, a potential security bypass exists where tenants could omit or modify the sharding label on Kustomization resources to run them on the unrestricted global controller; and third, restricting the customizer's write access to cozy-system to protect the supply chain would conflict with the need for admins to manage system-level configurations (like the cozystack-values Secret) in that namespace.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| packageOverrides: | ||
| linstor: | ||
| autoDiskful: | ||
| minutes: 10 | ||
| vm-default-images: | ||
| images: | ||
| - name: ubuntu-24.04 | ||
| url: https://example/custom.img | ||
| ``` |
There was a problem hiding this comment.
When implementing the packageOverrides merge logic in the platform chart, keep in mind that standard Helm merge functions (like merge or mustMerge) overwrite entire lists/arrays (such as the images list under vm-default-images) rather than merging them by key or appending to them. If an admin only wants to override or add a single image, they might inadvertently wipe out all other default images. It would be beneficial to specify in the design how list-type overrides are handled or if custom template merging logic will be used.
There was a problem hiding this comment.
The packageOverrides example this comment sits on has been dropped — PackageValues is now the single override layer. The list-merge concern is addressed head-on: the Admin override layer section pins the policy as maps deep-merge, lists replace wholesale (standard Helm semantics), and calls out the vm-default-images images list as exactly this footgun — overriding one image replaces the whole list, so adding one means restating it; keyed/append-merge is an explicit per-package opt-in. (1620d3b)
|
|
||
| A tenant must not be able to (a) reconcile into another namespace, or (b) borrow a more privileged SA. Two options, recommendation first: | ||
|
|
||
| 1. **Dedicated tenant kustomize-controller shard** (recommended) — mirror the existing `flux-tenants` helm-controller shard. Run a kustomize-controller selecting `sharding.fluxcd.io/key: tenants`, started with `--no-cross-namespace-refs` and a default-service-account lockdown, so the restrictions apply *only* to tenant reconciliation and platform Kustomizations are unaffected. This matches a pattern Cozystack already uses. |
There was a problem hiding this comment.
Using a dedicated tenant kustomize-controller shard that filters by a sharding label (e.g., sharding.fluxcd.io/key: tenants) introduces a potential security bypass. If a tenant can create or modify Kustomization resources directly, they could omit or change this label to have their Kustomization reconciled by the default, unrestricted global kustomize-controller instead. To make this isolation robust, the design should include an admission policy (such as a ValidatingAdmissionPolicy or webhook) that enforces the sharding label on all tenant-created Kustomization resources, or configure the global controller to explicitly ignore resources in tenant namespaces.
There was a problem hiding this comment.
Reworked in 771ebec. The legacy flux-tenants shard this described is retired (flux-shard-operator now owns helm-controller sharding), so tenant isolation no longer "mirrors" it. Tool 2 now provisions a dedicated per-tenant kustomize-controller + SA (a separate sharding path from the helm pool), plus a fail-closed mutating webhook that stamps the shard label + serviceAccountName and a ValidatingAdmissionPolicy backstop — so a label-less or foreign-SA Kustomization is rejected at admission and never reaches the unrestricted main controller. The enforcement ships with the surface, not as deferred hardening.
| - **Tool 1 (admin):** repo write == cluster-admin, by design and by documentation. The curated ClusterRole in the prototype (patch-not-delete on Packages, no CRDs, etc.) is *defense against accident, not against intent* — anyone with repo write can escalate, so the docs must not over-claim a boundary. New trust surface introduced: an admin git repo + its credentials (protect like a kubeconfig), and a standing, self-healing reconciling credential whose blast radius equals an admin's. | ||
| - **Tool 1 footguns to narrow** (they cost nothing functionally): source access should not let the customizer rewrite the platform's *own* supply chain (the `cozystack-packages` OCIRepository in `cozy-system`), so scope write away from `cozy-system`; and field ownership on downstream Packages should eventually be enforced by a `ValidatingAdmissionPolicy` scoped to the customizer SA that rejects changes to fields currently managed by helm-controller (detected via `managedFields`), leaving the hand-owned bootstrap Package free. Until then, the `packageOverrides` layer keeps admins off chart-owned objects entirely, so the contract is mostly avoided rather than merely documented. | ||
| - **Tool 2 (tenant):** the surface runs as the tenant's own SA, so it can do nothing the tenant cannot already do via the API. The new risk is **cross-namespace / SA-borrowing**, closed by the tenant shard lockdown (`--no-cross-namespace-refs`, forced `serviceAccountName`). Tenant-supplied input = the contents of a tenant-owned repo, bounded by `cozy:tenant:*`. |
There was a problem hiding this comment.
There is a potential conflict between scoping the customizer's write access away from cozy-system (to protect the platform's supply chain like cozystack-packages) and the need for admins to manage system-level configurations. As noted in the Context section, critical admin configurations (such as cert-manager issuers or other platform values) are driven by resources residing in cozy-system (e.g., the cozystack-values Secret). If write access to cozy-system is completely blocked, admins won't be able to manage these via GitOps. Consider refining this to use fine-grained RBAC or a ValidatingAdmissionPolicy that specifically protects the supply chain resources (like OCIRepository or specific HelmReleases) while still allowing the customizer to manage other configurations in cozy-system.
There was a problem hiding this comment.
Good catch on the tension — resolved deliberately the other way. The customizer is a cluster-admin-equivalent admin tool, so it manages cozy-system (cert-manager issuers, the cozystack-values Secret, etc.) the same way Flux manages its own manifests — intentional, not a boundary to wall off. So rather than scope write away from cozy-system, any supply-chain protection (e.g. the cozystack-packages OCIRepository) is an opt-out guard, not a default exclusion. Updated the Security posture in 1620d3b.
Revising Tool 1: override layer as a
|
PackageValues (proposed) |
packageOverrides (alternative) |
|
|---|---|---|
| API change | new optional CRD + reconciler merge | none (chart-only) |
| Field-ownership race | eliminated (separate object) | sidestepped (funnel through chart) |
| RBAC delegation | per-package (write PackageValues, read Package) |
all-or-nothing (sub-key of one object) |
List/map merge (e.g. vm-default-images.images is a 16-entry list) |
one tested merge in the operator | per-package Go-template logic, untested |
| Override blob | one small reviewable resource per package | one god-object for all packages |
| Schema validation | validatable per package later | free-form JSON |
packageOverrides is a reasonable lighter-weight alternative — no Go, ships purely as chart templating — and is essentially PackageValues crammed into one chart-rendered blob. The tradeoff is that it sidesteps the field-ownership race instead of removing it, can't do per-package RBAC delegation, and pushes list-merge correctness into per-package templates. Since PackageValues is additive, shipping packageOverrides first wouldn't block it — but it would make the packageOverrides key throwaway.
Feedback welcome on the precedence chain (chart defaults < cozystack-values < Package < PackageValues) and the RBAC split before I flesh out the CRD.
…layer Recast Tool 1's override layer from a `packageOverrides` map on the bootstrap Package (merged into downstream Packages by the platform chart) to a dedicated `PackageValues` API kind that the operator merges into each HelmRelease. `packageOverrides` is kept as a lighter-weight chart-only alternative/interim. PackageValues removes the helm-controller force-ownership race structurally (the chart never writes PackageValues; the admin never writes the chart-owned Package), makes ownership enforceable by object-level RBAC, and is fully additive — absent a PackageValues, the operator builds HelmReleases as today. Updates the admin override layer, user-facing / upgrade / security / failure / testing / rollout sections, open question 3 (now PackageValues precedence and binding), and alternatives considered. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a design proposal for Cozystack GitOps surfaces covering admin and tenant trust tiers, override handling with ChangesGitOps Configuration Surfaces Proposal
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@design-proposals/gitops-surfaces/README.md`:
- Around line 130-131: The proposal currently presents RBAC enforcement as a
mechanism that prevents field-level edits, but this is actually a
human-discipline contract with no API-level enforcement. Revise lines 130-131 to
clarify that the split design structurally avoids the contract violation (by
separating concerns into PackageValues and Package) rather than enforcing it via
RBAC alone. Additionally, update the text to explicitly state that the admission
policy is a prerequisite for production use, not an optional enhancement. Make
clear that without field-ownership enforcement at the API level, admins must
manually follow the documented protocol, and any deviation recreates the
helm-controller race conditions the PackageValues design was meant to prevent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d7a0a413-b6cb-49ef-ba12-c1bd9336da65
📒 Files selected for processing (1)
design-proposals/gitops-surfaces/README.md
IvanHunters
left a comment
IvanHunters
left a comment
There was a problem hiding this comment.
Requesting changes — solid proposal, but the tenant-tier isolation argument has a real hole that should be resolved before this leaves Draft.
- The tenant shard is not self-securing. Flux's main controllers run with
--watch-label-selector=!sharding.fluxcd.io/key(confirmed in cozystack's own fluxcd manifests), so a Kustomization created without the shard label is reconciled by the main, unrestricted controller — no cross-namespace-ref block, no forced SA. A tenant withcozy:tenant:admincan create Kustomizations and evade the shard by omitting the label or setting a foreignspec.serviceAccountName. The shard needs an admission policy enforcing the label + SA on tenant-namespace Kustomizations as a prerequisite, not deferred hardening. (A tenant confined to its own SA is still bounded by that SA's RBAC — but nothing forces it onto its own SA once the main controller is in play.) - The
PackageValuesfield-ownership contract is discipline, not enforcement, until the ValidatingAdmissionPolicy lands: kustomize-controller always applies withclient.ForceOwnership(not gated byspec.force), so an admin editing a chart-owned Package field silently steals it from helm-controller. The doc reads as "structurally removed" in one section and "advisory until policy lands" in another — please make that consistent and treat the admission policy as a production prerequisite. - Open Question 1 is load-bearing: both the operator-embedded
internal/fluxinstallcontrollers and a flux-operator-managedFluxInstanceexist in the tree, and the legacy flux-tenants shard is being actively retired by a new flux-shard-operator — so "mirror the flux-tenants pattern" mirrors something on its way out. Which instance serves tenant Kustomizations, and where the lockdown flags live, needs resolving before Tool 2 is implementable.
All Flux/Helm behavior claims I checked are accurate. Minor: the cozy-system write-scoping tension (admin config lives there) deserves an explicit answer, and the list-vs-map merge policy (Open Q3) needs pinning down.
Dismissing the review verdict: converting this to non-blocking feedback since this is a design proposal, not code. The technical points in the review comment still stand as suggestions to reconcile before implementation.
…ator The flux-tenants Deployment is retired; helm-controller sharding is now driven by flux-shard-operator (an auto-scaled shard pool), and there is no kustomize-controller sharding. Rework Tool 2 and Context accordingly: - Context: describe the embedded flux-aio control plane and flux-shard-operator; note tenants hold no Flux-CRD RBAC today, so Tool 2 is what creates the surface (and thus the isolation problem). - Tool 2 isolation: a dedicated kustomize-controller + SA per gitops-enabled tenant (a separate sharding path, not the helm-controller pool), a fail-closed mutating webhook that stamps the shard label + serviceAccountName, and a ValidatingAdmissionPolicy backstop. Enforced by construction and shipped with the surface, not deferred hardening. - Resolve Open Question 1: management-plane tenant reconciliation is the embedded flux-aio; the FluxInstance/multitenant lever governs child Kubernetes clusters only (migration 21 retired it here). - Disambiguate the deferred Tool 1 Package field-ownership VAP from the in-scope Tool 2 Kustomization admission enforcement. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
…verrides Address the remaining review points as deliberate design decisions: - Package field-ownership: the direct-edit path on chart-owned Packages stays open by design (it is a cluster-admin tool; locking it risks removing tuning knobs admins can reach today). PackageValues is the recommended non-racing path, not a cage; the field-ownership VAP is an optional opt-out guard, never a prerequisite. Reconcile the "structurally removed" vs "advisory" wording toward this. - cozy-system: do not scope customizer writes away from cozy-system — same logic as Flux managing its own manifests. Supply-chain guards are opt-out, not a default namespace exclusion. - PackageValues merge policy fixed (maps deep-merge, lists replace wholesale; the vm-default-images image list is the footgun). packageOverrides is dropped in favor of PackageValues as the single override mechanism. - Resolve Open Question 4: tenant GitOps ships as a catalog app, not a Tenant CRD field. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
…ted apps Tenants keep the curated Application abstraction: the GitOps surface grants the source/kustomize authoring needed to reconcile, but not raw HelmRelease or arbitrary-CR create rights. Since the per-tenant kustomize-controller runs as the tenant SA, "BYO from git" means Application CRs from git, not arbitrary HelmReleases — the tenant trust model is identical to today. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@design-proposals/gitops-surfaces/README.md`:
- Around line 206-207: The PackageValues binding contract is still ambiguous
because this section reopens the choice between 1:1-by-name and spec.packageRef
while the earlier example and failure mode already assume name binding. Update
the design text in the PackageValues section to make the binding rule explicit
and consistent, or mark the earlier example as provisional, so the README does
not imply two different contracts. Use the PackageValues heading and the binding
discussion around packageOverrides as the anchor for the edit.
- Line 184: The current guidance in the PackageValues behavior is to treat an
orphaned PackageValues object as a no-op, but this should be changed to surface
an explicit failure instead. Update the reconcile/validation handling around
PackageValues and the Package merge path so a missing target Package records a
status/validation error or fails reconciliation rather than silently succeeding.
Use the PackageValues and Package reconciliation logic to locate the
orphaned-object check and replace the no-op behavior with an explicit error
path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e932be38-f262-4573-b0d1-8f953447b8a4
📒 Files selected for processing (1)
design-proposals/gitops-surfaces/README.md
|
IvanHunters thanks — this was the review that reshaped the proposal. Pushed three commits ( #1 (tenant shard not self-securing) & #3 (Open Q1 / flux-tenants retiring): fully reworked. You're right that #2 (PackageValues = discipline, not enforcement): you're right the wording contradicted itself, and I reconciled it. But I've landed it the other way from "VAP as prerequisite": for the admin Package path we deliberately keep direct edits on chart-owned Packages open — it's a cluster-admin-equivalent tool, and fencing those fields by default risks silently removing tuning knobs admins reach today. So Minors: Also resolved Open Q2 (tenants stay on curated Applications — no raw HelmRelease/CR rights) and Q4 (catalog app). Thanks again for the flux-shard-operator pointer — that was the load-bearing correction. |
Andrei Kvapil (kvaps)
left a comment
Andrei Kvapil (kvaps)
left a comment
There was a problem hiding this comment.
Thanks myasnikovdaniil — this is a well-researched proposal. I verified the factual claims against the codebase (fluxinstall label selectors, migrations 21/44, package_reconciler.go values handling, tenant RBAC) — all accurate.
PackageValues is the right call, and it solves more than the doc claims. Users running their own GitOps operators (ArgoCD, plain Flux) already hit this today: the only way to override a bundle-shipped Package is delete-and-recreate it from their repo. PackageValues fixes exactly that, and — since the merge happens in the cozystack-operator — it works with any GitOps tooling, independent of the customizer. Please add this case to the problem statement and state that independence explicitly.
1. Materialize the override into the Package itself (single source of truth, loud conflicts). Rather than merging PackageValues invisibly at HelmRelease build time, have the operator SSA-apply its content onto the matching Package's spec.components.*.values under a dedicated field manager, without force. This buys several things at once:
Packagebecomes the single materialized source of truth — the HR build path keeps reading Package only, no hidden merge layer;- in the common case the chart renders Packages with no values at all (
_helpers.tplemits onlyvariant), so the operator owns the keys cleanly; where the chart does render a key, ownership is tracked per map key, so chart-owned and admin-owned keys coexist in one object; - a genuine collision on the same key fails the
PackageValuesreconcile with an SSA conflict surfaced as a status condition — chart-rendered intent wins, the admin gets a loud error instead of a silent race in either direction (note this inverts the currently fixedPackage < PackageValuesprecedence — deliberately); - deleting a
PackageValues(or a key from it) prunes the operator-owned fields and the Package reverts to chart state; an orphanedPackageValues(typo, missing Package) fails the apply and surfaces the same way — no silent no-op.
List semantics stay as already pinned (untyped JSON lists are atomic under SSA — replace wholesale). One caveat to document: helm-controller writes via helm's three-way merge, not SSA, so "chart wins" materializes as helm overwriting a contested key on a chart change and the operator's next apply failing loudly — an acceptable, self-announcing path.
2. Document the admin repo layout. The platform only consumes customizer.source.* + kustomization.path; everything inside the path is unspecified. Please add a reference repo structure to the proposal (and ideally an example repo before GA) — otherwise every operator invents their own, and the "environment overlays" use case stays theoretical.
3. Tenant surface: make it a parent-gated singleton, not a catalog app. Open Q4 considered "catalog app vs Tenant CRD field" — there's a middle ground that fits existing machinery: an extra module with singularResource: true (like monitoring/etcd/seaweedfs), enabled by the parent tenant (as with etcd: true today), with the repo settings then edited on the deployed instance. This gives a clean 1:1 between tenant and its dedicated kustomize-controller, avoids "5 GitOps instances = 5 controllers?" ambiguity, and puts controller-count under parent/admin control — which also answers the capacity question for large installations. Multiple repos, if ever needed, can be a list inside the single instance.
4. Bind the tenant surface to a dedicated narrow SA. Rather than a SA "carrying cozy:tenant:*", a dedicated <tenant>-gitops SA with write only on apps.cozystack.io (+ read for status) — the git path then has a strictly narrower surface than an interactive user, and the "git can only produce Application CRs" guarantee becomes explicit RBAC rather than a property of the current role set.
5. Define the dashboard ↔ git story. The surface is one-way: once an Application is git-managed, a dashboard edit gets silently reverted by drift correction on the next reconcile — a confusing UX the doc doesn't mention. Minimum: the dashboard marks git-managed Applications read-only (detectable via field manager or a label). Better: consider a two-way model where dashboard edits become commits/PRs to the tenant repo — the ReverseGitOps approach Simon Koudijs (@sunib) demoed at the last Cozystack meetup (reversegitops.dev). Simon, curious about your take here.
6. Gap: the source path is outside the isolation model (blocking for Tool 2). The dedicated-controller design covers kustomize-controller only. Tenant GitRepository objects, being label-less for source-controller, land on the shared main source-controller in cozy-fluxcd: (a) a platform component outside tenant NetworkPolicies now fetches tenant-supplied URLs (SSRF surface); (b) shared-controller DoS via large repos / aggressive intervals; (c) most importantly, source-controller serves artifacts over unauthenticated HTTP — so the per-tenant kustomize-controller's placement matters: if it runs inside the tenant namespace, the NetworkPolicy opening it to artifact storage opens every tenant pod to all artifacts, including platform manifests and other tenants' repos. The proposal should state who reconciles tenant sources, where the dedicated controller runs (platform namespace, I'd argue), and how artifact storage stays closed to tenant workloads.
7. Admission details to pin. The webhook/VAP pair must match CREATE and UPDATE (create-valid-then-strip-the-label is otherwise an escape), and the VAP must validate the shard label value equals the tenant's own shard — not mere presence — or tenant A can point its Kustomization at tenant B's controller.
8. Prune cascade on uninstall. Deleting the tenant GitOps instance deletes its Kustomization; with prune enabled, the finalizer then deletes everything it created — i.e. the tenant's Application CRs, i.e. their databases and VMs. Please add this to Failure and edge cases; I'd default the tenant surface to deletionPolicy: Orphan.
Overall: direction is right, Tool 1 + PackageValues can proceed independently (as the rollout already stages them); for Tool 2 I'd treat #6 and #7 as part of the "enforcement ships with the surface" package the doc itself commits to.
… isolation, PackageValues visibility Incorporate @kvaps review feedback: - Bind the tenant surface to a dedicated narrow `<tenant>-gitops` SA (write on apps.cozystack.io only) instead of reusing cozy:tenant:*. - Close the source-controller gap: run the per-tenant kustomize-controller platform-side and deny tenant egress to the unauthenticated artifact endpoint; bound shared-source DoS via interval/size limits. - Tighten admission: webhook + VAP match CREATE and UPDATE, and the VAP validates the shard-label value equals the tenant's own shard. - Default the tenant Kustomization to deletionPolicy: Orphan so uninstall does not cascade-delete tenant apps. - Add dashboard read-only for git-managed Applications (one-way drift). - PackageValues: keep build-time merge + admin precedence; add Package status visibility and loud handling of orphaned PackageValues; record the SSA materialize-onto-Package variant in Alternatives. - Keep tenant delivery as a self-service catalog app with multi-source; record the parent-gated extra-module singleton in Alternatives. - Add the bring-your-own-GitOps (ArgoCD / plain Flux) case to the problem statement; note PackageValues is operator-side and tooling-independent. - Document a reference admin-repo layout deliverable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
Thanks Andrei Kvapil (@kvaps) — extremely useful review. I verified each mechanism against the tree and Flux 2.8 before responding; most of this is now in the doc (pushed in the latest commit). One factual update up front that strengthens your #1: helm-controller v1.5.0 does default to server-side apply (install #1 — materialize onto Package / precedence. Agreed it's clean, and I've recorded it in Alternatives as a first-class option. I'm keeping the build-time merge as the default, though, for one reason: "no-force SSA onto the Package" necessarily makes the chart win any genuine overlap, which pushes the admin back onto the racy direct-edit path for exactly the keys they were trying to override; making the admin win while materializing would require #2 — repo layout. Added; a reference layout + example repo is a pre-GA deliverable. #3 — parent-gated singleton extra module. I'd like to keep it a self-service catalog app rather than a parent-gated module. The capacity concern behind the singleton is real, but it's dissolved by letting a single instance take a list of sources — one instance ⇒ one dedicated controller per gitops-enabled tenant, with a #4 — narrow SA. Adopted. The surface now binds a dedicated #5 — dashboard ↔ git. Added: git-managed Applications go read-only in the dashboard (detected via field manager/label); the two-way ReverseGitOps model is noted as the intended future direction. #6 — source path (blocking). Agreed this was the gap. Added: tenant #7 — admission. Fixed: webhook and VAP both match CREATE and UPDATE, and the VAP validates the shard-label value equals the tenant's own shard (presence alone would let A point at B's controller). #8 — prune cascade. Agreed — tenant Net: #4–#8 + the BYO-GitOps problem framing are in; #1 and #3 are addressed as deliberate design choices with the alternatives recorded. Tool 1 + |
|
Thanks for the mention and including Reverse GitOps into future plans! And sorry for the late response. Is there any new insights on this? Let me draft a more elaborate response tomorrow. |
|
My implemention for reversegitops.dev is the open source ConfigButler/gitops-reverser. I think that it can really help this proposal, but I do look at some things in a different way. I did read the proposal, and I'm curious to what you think of this response. Intent SurfaceThe first question I would lead with is: what is the primary intent you want customers to My current read is that the proposal points toward this answer, but I would want to confirm the
The scope question I am less sure about is whether Cozystack wants this GitOps surface to stop at %%{init: {"theme": "base", "themeVariables": {"primaryTextColor": "#0f172a", "lineColor": "#475569", "tertiaryColor": "#f8fafc"}}}%%
flowchart TD
Admin["Admin intent<br/>PackageValues"] --> AdminGit["admin git repo"]
Admin --> API["Cozystack API"]
Tenant["Tenant intent<br/>Application CRs"] --> TenantGit["tenant git repo"]
Tenant --> API
API --> Derived["derived state<br/>Packages / HelmReleases / engine CRs"]
Derived -.-> NotSurface["not the primary GitOps surface"]
classDef intent fill:#e0f2fe,stroke:#0284c7,color:#0f172a
classDef git fill:#dcfce7,stroke:#16a34a,color:#052e16
classDef api fill:#fef3c7,stroke:#d97706,color:#451a03
classDef derived fill:#ede9fe,stroke:#7c3aed,color:#2e1065
classDef avoid fill:#fee2e2,stroke:#dc2626,color:#450a0a
class Admin,Tenant intent
class AdminGit,TenantGit git
class API api
class Derived derived
class NotSurface avoid
Source Of TruthEnabling GitOps costs the tenant the dashboard (as it becomes read-only). For Cozystack that feels backwards. Cozystack is already Kubernetes API-first: tenants can Wouldnt it be better to start with Git as audit backend? And that you write all intent back into git without supporting bi-directional behaviour? That does already make a nice project: and still has enough hard topics to overcome. It already gives everybody the time to get used to it and to setup Git. Bootstrappinggitops-reverser can reconcile current cluster state into git, so existing dashboard-created Applications do not need to be manually reconstructed before a tenant enables GitOps. gitops-reverser already ships brownfield discovery/import for this (it even can already bootstrap a %%{init: {"theme": "base", "themeVariables": {"actorBkg": "#e0f2fe", "actorBorder": "#0284c7", "actorTextColor": "#0f172a", "actorLineColor": "#94a3b8", "signalColor": "#334155", "signalTextColor": "#0f172a", "noteBkgColor": "#fff7ed", "noteBorderColor": "#f97316", "activationBkgColor": "#dcfce7", "activationBorderColor": "#16a34a"}}}%%
sequenceDiagram
title Brownfield adoption
participant User as Tenant
participant UI as Dashboard
participant API as Cozystack API
participant R as gitops-reverser
participant Git as tenant repo
participant Flux
User->>UI: Enable GitOps for existing namespace
UI->>API: Request adoption
R->>API: List selected intent objects
R->>R: Normalize, drop runtime fields, apply secret policy
R->>Git: Create initial commit / PR
User->>Git: Review and merge
Git-->>Flux: First reconciled revision
Flux->>API: Apply same API state
Note over API,Flux: Existing objects are adopted, not recreated by hand
I did not see the repo bootstrap flow specified yet: who creates the repo, who writes the first commit, whether existing dashboard-created Applications are exported automatically, and how generated defaults and secrets are handled during that first export. I also could not find an actual example repo yet. The proposal lists a reference admin-repo layout/example repo as a pre-GA deliverable, while the current implementation PR appears to add the customizer chart rather than a sample repo. Secret ContractThe other boundary I would define before GitOps becomes a write path is secrets. This is a generic GitOps/API design problem, not just a Cozystack implementation detail: once API objects can also become git commits, the API needs a clean boundary between readable intent and secret material. Some app surfaces expose password-shaped fields in the Application spec:
In my opinion this way of handling secrets is not a good idea: readable intent goes in git, secret material goes in gitops-reverser handles this through explicit policy rather than writing sensitive material into git, including SOPS + age encryption for opted-in secret resources and bootstrapping key material during initial repo creation. I would not build the tenant story around "encrypt arbitrary Application specs because they contain inline passwords." You could enable it with gitops-reverser btw. Mirroring LayersAnother product question is whether mirroring should be opt-in or default-on. The proposal treats
The read-only debate is mostly about layer 3, but the proposal makes tenants buy all three layers at Bi-directional GitOpsThe support for this actually is betten than I thought: it's a bit to much to dive in now, but my repo has an elaborate document (and e2e test) on it. I'm quite positive that it's not to hard to get going with Cozystack (as I did this aleady). I think that this is already long enough for a first response: I'm more than happy to setup a call to talk some things through. Please do note that I will be on holiday comming weeks, so don't expect fast responses. |
|
Andrei Kvapil (@kvaps) Timofei Larkin (@lllamnyp) PackageValues stays merged at HelmRelease build time, admin wins an overlap. Materialize onto Package with no-force SSA is in Alternatives now, but it makes the chart win exactly the keys admin tried to override, so admin goes back to direct edit on chart-owned Package and we are back to the ForceOwnership race this proposal exists to remove. It is described in the Admin override layer section and I don't want to keep it as open question, so I need explicit ack from you two as api owners. Tenant surface is catalog app, not a Tenant CRD field and not parent-gated singleton. Binding is the only thing left open, 1:1 by name as drafted or explicit |
|
Simon Koudijs (@sunib) thanks, this is a lot of useful detail, and gitops-reverser is clearly further along than I expected, brownfield import especially. You are right that repo bootstrap flow is not specified in the proposal, who creates the repo, who writes first commit, how existing dashboard-created apps get exported. Fair catch. I read your bi-directional guide. Your model keeps kubernetes api as the write path and demotes reconciler to triggered applier that applies exact sha, this proposal goes other way, git is source of truth and per-tenant kustomize-controller reconciles always-on with drift correction. Same path cant be owned by both, so it is different feature and not a variant of this one, and live to git direction is not something I plan to implement here. I would rather see it as its own design proposal in this repo, and I will review it. Then we can look how both designs coexist, and if mirroring lands as separate surface it can reuse the same tenant repo and credentials. |
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
myasnikovdaniil the Flux, Package and tenant RBAC descriptions hold on cozystack main (785cbc391b7c), with one exception inline. What blocks is the commit history, the references to another PR's state, and Tool 2's isolation model: what the narrow SA can apply, the remote fetch path, the network containment, and which way the Tool 1 guards default.
All six commit messages land in main as written, since community merges with rebase. Five carry Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> and none carries Assisted-by: LLM. 6afe9a5 and 1620d3b describe the review instead of the change ("address review", "Incorporate Andrei Kvapil (@kvaps) review feedback"), and the first subject is [docs] ... instead of docs(scope): .... Please squash into one docs(gitops-surfaces): commit with Assisted-by: LLM.
The text still presents cozystack/cozystack#2731 as the first increment and "in review" (lines 16, 86, 199, 203, 207). That PR was closed unmerged on 2026-07-29. A proposal should describe the design, not the state of other PRs, so I'd describe Phase 1 by what it ships and drop the "already done in the prototype" lines. Lines 215 and 218 ("Resolved (during review)") have the same problem.
On kvaps's change request, as I read it at this head: points 4 to 8 are in the text. Points 1 (materialize onto Package) and 3 (parent-gated singleton) are declined with alternatives recorded, and still need his ack. Point 2 asked for a reference repo layout in the proposal itself, and line 168 only promises one in the docs later, so it is still open. IvanHunters' review is dismissed and his points look addressed.
The rest is inline.
| - **Downstream Packages** (`cozystack.linstor`, `cozystack.cilium`, …) are **rendered by the platform chart** (`packages/core/platform`) with `helm.sh/resource-policy: keep` and are owned by helm-controller. | ||
| - **Most admin config is Package values, not raw resources.** VM golden images live in `packages/system/vm-default-images` values; LINSTOR parameters in `packages/system/linstor` values; cert-manager `ClusterIssuer`s are driven by `packages/core/platform` values → the `cozystack-values` Secret in `cozy-system` → the `cert-manager-issuers` chart. | ||
| - **Management-plane Flux is the embedded flux-aio, and helm-controller is already sharded by an operator.** The management cluster runs a single all-in-one `flux` Deployment installed from `internal/fluxinstall`; its source-, kustomize-, and helm-controllers carry `--watch-label-selector=!sharding.fluxcd.io/key`, so any object *labelled* with that key is skipped by the unrestricted main plane. **flux-shard-operator** provisions auto-scaled `helm-controller-shard<i>` Deployments (each `--watch-label-selector=sharding.fluxcd.io/key=shard<i>`), assigns each tenant to a shard, and, via a mutating webhook, rewrites the birth label `sharding.fluxcd.io/key: tenants` on tenant HelmReleases to the assigned `shard<i>`. It has **retired the old hand-rolled `flux-tenants` Deployment** (migration 44) — so tenant HelmReleases are no longer one-controller-per-tenant but packed onto a shared, auto-scaled pool. Crucially, **there is no kustomize-controller sharding**: a `Kustomization` *without* the shard label is reconciled by the unrestricted main kustomize-controller. | ||
| - **Tenants cannot self-GitOps.** A `Tenant` (`apps.cozystack.io`) produces a `tenant-<name>` namespace, a ServiceAccount named `<tenant-name>`, and `cozy:tenant:*` ClusterRoles. Tenants create curated **Application** CRs (e.g. Postgres, Kafka, VM) which the aggregated apiserver converts into the HelmReleases above. Tenants have **no RBAC for `source`/`kustomize`/`helm` toolkit CRDs** and there is **no tenant-scoped source/kustomize reconciliation** — the only isolation a tenant has today is precisely that it cannot author Flux objects at all. (The flux-operator `FluxInstance` with `multitenant: false` in `packages/system/fluxcd` governs **child Kubernetes clusters**, not this management plane — migration 21 retired the FluxInstance path here in favor of the embedded flux-aio. This resolves Open Question 1 below.) |
There was a problem hiding this comment.
packages/system/fluxcd no longer exists on main, so there is no FluxInstance with multitenant: false in the tree. The conclusion still holds, since the management plane is the embedded flux-aio (internal/fluxinstall). Please drop the FluxInstance reference here and on line 215.
| linstor: | ||
| values: | ||
| autoDiskful: | ||
| minutes: 10 |
There was a problem hiding this comment.
The linstor chart reads linstor.autoDiskful.minutes (packages/system/linstor/values.yaml:58-60), and component values go straight into the HelmRelease values. So autoDiskful.minutes: 10 here does nothing; it needs linstor: {autoDiskful: {minutes: 10}}.
|
|
||
| ### Tool 2 — Tenant GitOps (tenant, namespace scope) | ||
|
|
||
| Delivered as a **catalog app**, not a `Tenant` API change and not a parent-gated "extra module" — it is opt-in per tenant, self-service, and fits the existing app model. A single instance takes a **list of sources** (one or more `GitRepository` + `Kustomization` pairs), so a tenant never needs to instantiate the app more than once: the natural shape is **one instance ⇒ one dedicated controller per gitops-enabled tenant**, which dissolves the "5 instances = 5 controllers?" capacity worry without gating the feature behind the parent. If an admin wants a hard cap, a per-namespace `ResourceQuota` (or a one-instance-per-namespace admission check) bounds it — a restriction added where needed, rather than trading away tenant self-service by default (see Alternatives). Unlike Tool 1 it is *not* a no-op on the control plane: tenants hold **no** RBAC for Flux CRDs today, so the surface must grant Kustomization write, and granting it is exactly what opens the isolation problem below. The enforcement therefore ships **with** the surface, not as later hardening. |
There was a problem hiding this comment.
I don't think a ResourceQuota can cap instances. Application objects are served by the aggregated cozystack-api, which runs the generic apiserver admission chain (pkg/cmd/server/start.go) without the ResourceQuota plugin, so count/<kind>.apps.cozystack.io is never enforced on create. #39 proposes cardinality: {scope: tenant, max: 1} for exactly this, so I'd reference it here and in the singularResource alternative. Please also say whether the dedicated controller is per tenant or per instance, since nothing stops a second instance.
| Delivered as a **catalog app**, not a `Tenant` API change and not a parent-gated "extra module" — it is opt-in per tenant, self-service, and fits the existing app model. A single instance takes a **list of sources** (one or more `GitRepository` + `Kustomization` pairs), so a tenant never needs to instantiate the app more than once: the natural shape is **one instance ⇒ one dedicated controller per gitops-enabled tenant**, which dissolves the "5 instances = 5 controllers?" capacity worry without gating the feature behind the parent. If an admin wants a hard cap, a per-namespace `ResourceQuota` (or a one-instance-per-namespace admission check) bounds it — a restriction added where needed, rather than trading away tenant self-service by default (see Alternatives). Unlike Tool 1 it is *not* a no-op on the control plane: tenants hold **no** RBAC for Flux CRDs today, so the surface must grant Kustomization write, and granting it is exactly what opens the isolation problem below. The enforcement therefore ships **with** the surface, not as later hardening. | ||
|
|
||
| - New app package `packages/apps/gitops` + ApplicationDefinition in `packages/system/gitops-rd`. When a tenant instantiates it, the chart renders, in the tenant namespace, one or more `GitRepository` + `Kustomization` pairs bound to a dedicated **`<tenant>-gitops` `ServiceAccount`** (see *A dedicated, narrow GitOps SA* below), and the tenant is provisioned **its own dedicated kustomize-controller** (running platform-side — see the source-path isolation point). | ||
| - Tenants point the surface at a repo of Application CRs and namespaced manifests. Because every reconcile runs as the `<tenant>-gitops` SA, a tenant can only create `apps.cozystack.io` Application CRs inside its own namespace subtree — but that guarantee holds only if nothing lets a tenant Kustomization escape onto the unrestricted main controller or borrow a foreign SA (see below). |
There was a problem hiding this comment.
This contradicts the narrow SA. Here, in the diagram and in User-facing changes, the tenant repo carries "Application CRs and namespaced manifests", but line 151 binds reconciliation to <tenant>-gitops with write on apps.cozystack.io only, and kustomize-controller applies the whole set as that SA. Any ConfigMap or Secret in the repo then fails the reconcile, including the SOPS secrets from line 160, whose output is a Secret. Please narrow the promise to Application CRs, or list the extra kinds the SA gets.
|
|
||
| **A dedicated, narrow GitOps SA.** The surface does **not** reuse the interactive tenant SA. The default tenant SA carries `cozy:tenant:base`, which grants more than app management — full write on `sdn.cozystack.io/securitygroups` plus assorted cluster reads — so binding the git path to it would hand a repo more than it needs. Instead the app binds a purpose-built **`<tenant>-gitops` ServiceAccount** with write on `apps.cozystack.io` only (plus read for status). The git path is therefore *strictly narrower* than an interactive user, and the "git can only produce Application CRs" guarantee (Open Question 2) becomes **explicit RBAC** rather than an emergent property of the current role set. | ||
|
|
||
| 1. **A dedicated kustomize-controller per gitops-enabled tenant.** Enabling the app provisions a kustomize-controller scoped to that tenant, running **in the platform namespace (`cozy-fluxcd`)** — not the tenant namespace (see the source-path point) — watching only that tenant's shard label, started with `--no-cross-namespace-refs`, pinned so every Kustomization reconciles as the `<tenant>-gitops` SA. It is opt-in, so the controller count tracks only the tenants who use GitOps — not every tenant — which is what makes a per-tenant (rather than pooled) controller affordable here. |
There was a problem hiding this comment.
The lockdown flags have --no-cross-namespace-refs but not --no-remote-bases, which defaults to false. With remote bases allowed, a kustomization.yaml in a tenant repo makes this controller fetch arbitrary git URLs from inside cozy-fluxcd, the same fetch surface point 4 closes for the source path. Please add the flag here and to the isolation tests.
| 1. **A dedicated kustomize-controller per gitops-enabled tenant.** Enabling the app provisions a kustomize-controller scoped to that tenant, running **in the platform namespace (`cozy-fluxcd`)** — not the tenant namespace (see the source-path point) — watching only that tenant's shard label, started with `--no-cross-namespace-refs`, pinned so every Kustomization reconciles as the `<tenant>-gitops` SA. It is opt-in, so the controller count tracks only the tenants who use GitOps — not every tenant — which is what makes a per-tenant (rather than pooled) controller affordable here. | ||
| 2. **A separate mutating webhook** (distinct from the helm-controller shard webhook) stamps the tenant's shard label **and** forces `spec.serviceAccountName` on Kustomizations created in tenant namespaces, on **CREATE and UPDATE**. Because the surface grants tenants Kustomization write, this webhook is a **security boundary**, so it runs **fail-closed** (`failurePolicy: Fail`) — unlike the helm-controller webhook, which can fail-open safely since tenants cannot author HelmReleases. | ||
| 3. **A `ValidatingAdmissionPolicy` as an independent backstop.** On **CREATE and UPDATE** (so create-valid-then-strip-the-label is not an escape) the VAP rejects any tenant-namespace Kustomization whose shard-label **value** is not the tenant's own shard — mere *presence* is insufficient, since a wrong value would route the Kustomization to another tenant's controller — or that names a foreign SA. Webhook (mutate → correct) plus VAP (validate → reject) close the "omit/alter the label to reach another controller" and "borrow a privileged SA" evasions **by construction**, rather than documenting them as advisory. | ||
| 4. **The source path stays platform-side and closed to tenant workloads.** A tenant `GitRepository` carries no shard label, so it is reconciled by the shared main **source-controller** in `cozy-fluxcd` (there is no source-controller sharding) — a *platform* component now fetching tenant-supplied URLs (an SSRF surface) and a shared DoS target (large repos / aggressive intervals). Three decisions contain this: (a) the per-tenant kustomize-controller runs in **`cozy-fluxcd`**, not the tenant namespace, so opening it to the artifact endpoint does not open every tenant pod to it; (b) the source-controller artifact endpoint — served **unauthenticated over HTTP** at `flux.cozy-fluxcd` — is reachable only from the flux control plane; tenant namespaces get **no** egress to it (Cilium policy), so a tenant workload can never pull another tenant's or the platform's artifacts; (c) tenant `GitRepository` interval/size limits bound the shared-source DoS. A per-tenant source-controller is possible but not warranted at first; the placement + closed-artifact contract is what must be stated (this is the part of the isolation model that was previously specified only for kustomize-controller). |
There was a problem hiding this comment.
This relies on Cilium policy, but the tenant chart doesn't always render it. tenant.ciliumPolicies is false on isp-hosted and isp-hosted-slim when the host CNI lacks the Cilium CRDs (packages/apps/tenant/templates/_helpers.tpl:109-115), and then tenant pods reach the source-controller with no restriction. Since this containment is a prerequisite, please say what happens there: refuse to enable the app, or ship a plain NetworkPolicy.
| This is the heart of the proposal; each tier has a distinct, **explicit** trust boundary. | ||
|
|
||
| - **Tool 1 (admin):** repo write == cluster-admin, by design and by documentation. The curated ClusterRole in the prototype (patch-not-delete on Packages, no CRDs, etc.) is *defense against accident, not against intent* — anyone with repo write can escalate, so the docs must not over-claim a boundary. New trust surface introduced: an admin git repo + its credentials (protect like a kubeconfig), and a standing, self-healing reconciling credential whose blast radius equals an admin's. | ||
| - **Tool 1 posture (deliberately permissive — it is an admin tool):** the customizer is a cluster-admin-equivalent credential, so by design it manages `cozy-system` and the platform's own supply chain the same way Flux manages its own manifests — this is intentional, not a boundary to wall off. Optional, **opt-out** guards exist for teams that want them: a `ValidatingAdmissionPolicy` scoped to the customizer SA could protect specific supply-chain objects (the `cozystack-packages` OCIRepository) or reject changes to helm-controller-managed `Package` fields (detected via `managedFields`). These are **defense against accident, not prerequisites** — locking chart-owned `Package` fields by default would risk removing tuning knobs admins can reach today, so the direct-edit path stays open and `PackageValues` is the recommended non-racing path rather than a wall. |
There was a problem hiding this comment.
The default isn't clear. This line calls the guards "opt-out" but also "for teams that want them", and line 207 ships the permissive posture first. Please pick one. I'd argue the guard on the cozystack-packages OCIRepository should be on by default: rewriting that one object redirects every PackageSource, which is the same accident the curated role already guards against by excluding CRDs and Package delete.
Summary
Adds a design proposal — GitOps Configuration Surfaces — under
design-proposals/gitops-surfaces/.It defines a single GitOps primitive (
GitRepository+Kustomization+ a scopedServiceAccount) and instantiates it at two trust tiers:packageOverridesadmin override layer so component-parameter changes don't race helm-controller for field ownership.gitopsapp bound to the tenantServiceAccount, isolated via a dedicated tenant kustomize-controller shard.Covers admin package/parameter/golden-image configuration, tenant self-service GitOps, and arbitrary cluster resources outside the core API, plus cross-cutting secrets-as-code, DR/reproducibility, and policy-as-code.
Status
Draft — opening for community feedback per the design-proposal process. The first increment (admin surface) is already prototyped in cozystack/cozystack#2731.
Summary by CodeRabbit