mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 22:13:18 +09:00
docs/api-stability-policy
24
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 IvanHunters
|
f05718b2de |
feat(redis)!: refuse external access without TLS at admission (#4639)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does An admission policy now refuses a Redis release that would expose a plaintext endpoint outside the cluster. Redis never turns TLS on by itself, so `external: true` without `tls.enabled: true` is exactly that case. The error names the release and a remedy that works. The rule has three parts, because TLS is fixed when a Redis instance is created: the RedisFailover schema refuses a change, while the Redis API accepts one and the HelmRelease then fails. A spec's `tls.enabled` is therefore a request, not proof of what the instance runs. - On create, `external: true` needs `tls.enabled: true`. - On update, turning `external` on is refused whatever `tls.enabled` says. - While `external` stays on, `tls.enabled` cannot change. Before the first reconcile nothing else stops TLS being dropped from a release admitted as external with TLS. Withdrawing `external` in the same edit lifts the freeze, and the callers trusted by `cozystack-tenant-host-policy` are exempt, so an instance whose TLS was flipped earlier can still be unblocked. Edits to a release that was already external pass, so releases exposed in plaintext before this policy keep working. The guarantee covers the Redis app. A generic publisher such as `tcp-balancer` can still forward to a plaintext Redis, by design. This diverges from the Security section of the `external-database-exposure` proposal, which refuses only an explicit `tls.enabled: false` and covers every engine. That rule was written before redis had TLS and matches only postgres. Postgres is left out here on purpose: CloudNativePG keeps `ssl=on` and its default pg_hba accepts plaintext from any client that asks, whatever the chart's `tls.enabled` says, so an admission rule on that field closes nothing. That gap is #4619. The proposal update and a decision record are cozystack/community#91. The policy follows `cozystack-tenant-host-policy`: a ValidatingAdmissionPolicy in `cozystack-basics`, enforced inside `cozystack-api` for the aggregated kind. Its decisions are tested in Go by compiling the shipped template with the apiserver's own CEL compiler and running Redis objects through it, including the bypass sequences found in review. A structural chart test can't show what the policy admits. The redis README, the `external` field description and the threat model describe the new rule. Related: #4622 (valkey `external` is always plaintext), #4624 (the redis `tls.enabled` description implies a live switch). ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: I walked the trigger map against the diff and nothing in it fires, but I left the boxes empty because cozystack/community is reached anyway: the `external-database-exposure` proposal still states the old admission rule. Its update and a decision record are cozystack/community#91. ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note feat(redis)!: a Redis release can be exposed outside the cluster only when it is created with `tls.enabled: true`. Creating one with `external: true` and no TLS, turning `external` on for an existing instance, or changing `tls.enabled` while `external` is on is refused at admission. Instances already external without TLS keep working and stay editable. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Redis admission checks now require TLS when external access is enabled at instance creation. They block enabling external access on an existing instance and changing TLS while external access remains enabled. Disabling external access in the same update allows TLS changes. * Existing instances that already had external access without TLS before the policy was introduced remain editable. * **Documentation** * Updated Redis configuration and security guidance to explain these restrictions and TLS migration behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
800bb0369d |
feat(redis)!: refuse external access without TLS at admission
A Redis release with external: true and TLS off publishes a plaintext endpoint outside the cluster, and nothing stopped a tenant from asking for one. The chart never infers TLS from external, because enabling it moves Redis to a TLS-only listener, so external: true without an explicit tls.enabled: true is exactly that case. A ValidatingAdmissionPolicy on the Redis kind now refuses it with a message naming the release and a remedy that works. TLS is fixed when a Redis instance is created: the operator's RedisFailover schema refuses a change, but the Redis API accepts it. A spec's tls.enabled is therefore a request, not the instance's state, and cannot prove on UPDATE that an instance has TLS; trusting it would let a tenant flip it, expose the instance, and remove it again. So external access can be enabled only at creation, where tls.enabled does decide the instance, and any UPDATE turning it on is refused. The RedisFailover schema does not exist before the first reconcile, so a release admitted as external with TLS could have TLS dropped before the instance is created. tls.enabled is therefore frozen while external access stays on. Withdrawing external access in the same edit lifts the freeze, which leaves a tenant a way to unblock an instance whose TLS was flipped earlier, and the callers trusted by the tenant host policy are exempt so an administrator can repair one in place. Other edits to an instance that is already external pass, so releases exposed in plaintext before the policy keep working. Postgres is left out on purpose. CNPG pins ssl=on, and its default pg_hba accepts plaintext from any client that asks, whatever the chart's tls.enabled says; that field only decides whether the external hostname goes into the certificate. A policy keyed on it would close nothing. The decisions are tested by compiling the shipped template with the apiserver's own CEL compiler, since a structural chart test cannot show what the policy admits. BREAKING CHANGE: creating a Redis with external: true and without tls.enabled: true, which applied before, is now refused, including a GitOps re-apply of such a manifest into a fresh cluster. Turning external on for an existing Redis is refused as well, so exposing an instance means creating a new one with TLS, and tls.enabled can no longer be changed on an instance with external access. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
0ae50c6d66 |
fix(dashboard): read vm-disk images from the image option source
The vm-disk source picker listed vm-default-images-* PVCs in cozy-public directly, as the tenant, so it kept offering goldens whose import failed even once the image option source leaves them out. The Option list is what the other dynamic dropdowns already read, and it shares their request. With the console no longer listing those PVCs, the tenant dashboard role's read on cozy-public PVCs has no user left, so it goes too. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Timofei Larkin
|
2cbc02bd10 |
fix(platform): freeze HTTPCache on the artifact the cluster runs
Pinning the frozen source to a digest written into the migration picks one release for every cluster, and v1.6.0 is not even the last one that ships http-cache: v1.6.1 through v1.6.3 and v1.7.0-alpha.1 all do, and 1.6 patch releases may keep doing so. A cluster on any of those would be rolled back to an older chart by the upgrade that was meant to leave its instances alone, and an air-gapped mirror resolves the constant only if someone mirrored that exact manifest. The hook runs before the upgrade re-points cozystack-packages, so the live object still names the release the instances render from, in whatever registry the operator serves it from and with whatever pull secret and verification they configured. Reading the digest off it makes the freeze a no-op by construction and needs no knowledge of which tag was last. spec.ref.digest is used when the installer pinned one; a tag or semver ref goes through status.artifact.revision, whose digest half is the manifest digest. status.artifact.digest is the checksum of the downloaded tarball and cannot stand in. A source with neither has nothing to freeze on and the migration stops there. The copy also carries the no-delete label the platform chart puts on the original, because it is the only chart source those instances have left. The tenant grant travels with the freeze as well. cozy:tenant:admin:base enumerates the apps.cozystack.io resources a tenant owner may write, and an application that has left the catalog has no business in that list: the next sweep over it takes the entry as dead config, and the failure is silent and remote, a Forbidden for the one person who still needs to remove what they have. The chart drops httpcaches, and a cluster that freezes gets a ClusterRole under the tenant admin aggregation label instead, applied before the upgrade drops the rule so the access never lapses and carrying the verb set the chart had so nothing changes for the owner. A cluster with no instance is granted nothing. The Package delete on the no-instance path has to block. The operator reconciles off an informer cache, so a reconcile in flight against the pre-delete entry re-creates the release it owns, and with it the ApplicationDefinition and the Helm storage the following lines remove. Returning immediately made the ordering a property of the command log rather than of the cluster. The preset check's SIGPIPE note named http-cache-rd as its example of a large enum, and that chart goes away here; kubernetes-rd carries the same enums. Numbered 58 rather than 57: main already carries a migration 57, the SeaweedFS volume-size-limit one, and its targetVersion is already 58. The whole shape of this retirement was worked out here rather than anywhere reusable, so docs/deprecating-applications.md writes it down as a procedure: how to pick between migrating onto a successor and freezing, what the prune takes with it, and what the migration owes a cluster on each branch. The next application to go should not have to rediscover it. Assisted-by: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
scooby87
|
afc003c464 |
feat(postgres): database read-replica autoscaling via KEDA (#3954)
## What this PR does Adds automatic horizontal autoscaling of a PostgreSQL database's **read replicas** in response to load, implementing the merged design proposal [Database Horizontal Autoscaler](https://github.com/cozystack/community/tree/main/design-proposals/database-horizontal-autoscaling) (community#44). The mechanism is entirely stock — no bespoke operator, no new CRD: - **KEDA as an optional platform component** (`packages/system/keda`, wired into `core/platform` as a source + system-bundle entry). It claims the `external.metrics.k8s.io` APIService that nothing served before. - **`cozy-lib.keda.scaledObject` helper** (`packages/library/cozy-lib/templates/_keda.tpl`) renders a KEDA `ScaledObject`, fail-closed on missing/invalid arguments. - **`packages/apps/postgres` autoscaling** — an `autoscaling` values block (`enabled`, `minReplicas`, `maxReplicas`, `metric`, `target`, `maxReplicationLagSeconds`, `dryRun`, `transition`) validated by `values.schema.json`; under active autoscaling the chart renders a `ScaledObject` and seeds `spec.instances` to a constant floor (`max(replicas, effectiveMin)`), while the postgres release is forced to client-side (two-way) Helm apply so the seed is a merge no-op on upgrade and the KEDA-managed HPA — the runtime writer via the CNPG `Cluster` scale subresource — is not reverted. - **Single-value metric (§1)** — one PromQL query returns `Σ(read load over standbys) + target`, so a stock HPA computes `desired = ceil((Σ+target)/target) = 1 + ceil(Σ/target) = primary + desiredRead`, with no per-pod emission. - **The four static brakes (§5)** — quorum floor `max(minReplicas, maxSyncReplicas+1, 2)` (quorum wins over `maxReplicas`), scale-down pacing (`Pods/1/600s`), a replication-lag clamp folded into the query (freeze both directions while lag exceeds the threshold and the primary is writing, with `max_over_time` hysteresis), and dry-run (render paused). - **Two-phase enable/disable migration** via a `transition` sub-flag: phase 1 stages `spec.instances` to the live count and brings KEDA up with its HPA paused in both directions; flipping to phase 2 hands the runtime count to KEDA, and because the seed re-renders the same count the flip is a merge no-op with no dip. - **Observability** — a database-autoscaling Grafana dashboard and PrometheusRule alerts. **Live validation (dev cluster, CNPG 1.30.0).** The full path works end-to-end: the CNPG `Cluster` scale subresource serves `status.selector`, KEDA renders the HPA, the metric reaches it (`ScalingActive=ValidMetricFound`, `TARGETS 151/150`, `desiredReplicas=2 = 1 primary + 1 replica` — the §1 arithmetic), and the lag-clamp query returns the base value on live VictoriaMetrics. Two packaging fixes were needed and are included: KEDA must run on the cozystack `cozy.local` cluster domain (the upstream `cluster.local` default makes the metrics gRPC target NXDOMAIN and every HPA hangs on `FailedGetExternalMetric`), and the operator's client-go throttle is raised so a single-replica operator does not lose its leader lease under scale-subresource polling. Implements #3953. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. Two repositories are affected; both are follow-ups to open once this lands (and after its dependency #3951 merges and the values surface is final), left unticked here for a maintainer to confirm rather than claimed with a speculative link: - **cozystack/terraform-provider-cozystack** — this adds the nested `autoscaling` block to `packages/apps/postgres/values.schema.json`. The provider hand-models each app's fields (schema + model + expand/flatten), so it needs the new block to expose it in Terraform. - **cozystack/website** — KEDA is a new platform component (`guides/platform-stack`), and the postgres reference page gains the `autoscaling` params. The managed-app reference is regenerated from `README.md` by the release bot; the platform-stack component list is hand-maintained. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: to open (new platform component + postgres autoscaling params) - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: to open (new `autoscaling` block in postgres values.schema.json) ### Depends on - #3951 — bump CloudNativePG to 1.30.0. The HPA can only drive the CNPG `Cluster` scale subresource on CNPG ≥ 1.28.4, where it serves `status.selector`; without it the HPA fails with `InvalidSelector`. This PR should not merge before #3951. ### Release note ```release-note feat(postgres): add optional horizontal autoscaling of read replicas via KEDA, configured through the new `autoscaling` values block; KEDA ships as a new optional platform component. Requires CloudNativePG >= 1.28.4 for the Cluster scale subresource selector. Note: every Postgres release now applies client-side (`release.cozystack.io/helm-server-side-apply: "false"`) so the autoscaler's live `/scale` count is not reverted by Helm; a kind-wide side effect is that chart-rendered fields on Postgres releases are no longer re-asserted by server-side-apply drift correction (driftDetection is unset), a documented trade-off with a per-instance apply strategy as follow-up. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configurable PostgreSQL read-replica autoscaling with KEDA. * Added CPU and read-connection metrics, replica bounds, replication-lag safeguards, dry-run mode, and transition handling. * Added optional KEDA platform support, Grafana autoscaling dashboard, and Prometheus alerts. * **Documentation** * Clarified integer thresholds, prerequisites, configuration, and transition guidance. * **Bug Fixes** * Improved validation and actionable errors for unavailable KEDA or invalid settings. * **Tests** * Expanded coverage for autoscaling, observability, package selection, and validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Alexey Artamonov
|
1979bd3f5c |
fix(cozystack-basics): let tenants read their own autoscaler objects
The autoscaling chart renders a ScaledObject into the tenant namespace and KEDA creates a keda-hpa-<release> HorizontalPodAutoscaler beside it, but the cozy:tenant:view aggregation granted no read on keda.sh or autoscaling. So the exact failure the feature documents - a monitoring: true tenant whose derived vmselect carries no kube_pod_labels, whose trigger never resolves - lands as a TriggerError ScaledObject and a FailedGetExternalMetric HPA the tenant cannot get, surfaced only in a root-scoped alert they cannot reach. A tenant who flips the knob and sees nothing happen has no in-band way to find out why. Grant get/list/watch on keda.sh/scaledobjects and autoscaling/horizontalpodautoscalers in cozy:tenant:view:base. Read verbs only: the objects are managed by the chart and KEDA, not edited by the tenant. Pinned by clusterroles-options unittest. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
d685fb30ec |
fix(gateway)!: require tenant routes to declare their hostnames (#3475)
## What this PR does The tenant route hostname policy skipped the shape it most needed to check. Its CEL opened with `!has(object.spec.hostnames) ||`, so an HTTPRoute or TLSRoute declaring no hostnames satisfied the first operand and got admitted with nothing compared against the tenant apex. An empty list passed too: `has()` is true on `[]`, and `all()` over an empty list is vacuously true. Such a route is not inert. Under Gateway API it inherits the hostnames of every listener it attaches to, and `parentRefs` can name a Gateway in another namespace, so it covers whatever that listener covers. Reported in #3467. The predicate now requires the field to be present and non-empty before validating it. Both policies rendered from that template share one CEL string, so HTTPRoute and TLSRoute get the change together, and both match `CREATE` and `UPDATE`, so an existing route that drops its hostnames is denied exactly as a new one is. The issue is titled after TLSRoute, but the reachable shape is mostly HTTPRoute. TLSRoute allows a hostname-less route only on the deprecated `v1alpha2`; at `v1` and `v1alpha3` the bundle this repo vendors marks `spec.hostnames` required and rejects an empty list on `minItems: 1`, while `v1alpha2` carries neither constraint. HTTPRoute `v1` carries neither either, so it accepts both shapes. All four are checkable in `packages/system/gateway-api-crds/templates/crds-experimental.yaml`. Checking that turned up a stale claim in the template header, which said TLSRoute had no `v1` in Gateway API 1.4 to 1.5 and used it to justify the rule naming only `v1alpha2`. Gateway API 1.5.1, which this repo vendors, serves TLSRoute at `v1` as the storage version. The rule still covers every served version because `matchConstraints.matchPolicy` defaults to `Equivalent`, and that default is now written down, because setting `matchPolicy: Exact` reads like a tightening and would silently stop the policy seeing `v1` and `v1alpha3` TLSRoutes. I corrected the text and left the coverage alone: the wrong sentence predates this branch, but my change is what makes the file contradict itself, so the text is mine to fix while widening version coverage is a separate job. A `ValidatingAdmissionPolicy` has no cross-object lookup, so it cannot resolve the parent Gateway to see which hostnames a route would inherit. Requiring the route to name its own is what keeps every admitted hostname comparable against the apex. That has a cost: a route relying on listener inheritance now has to spell its hostnames out, and a tenant that hand-writes one gets a denial where it used to get an admission. ### The controller change belongs in the same PR `renderHTTPRedirect` built the http-to-https redirect HTTPRoute with no hostnames on purpose, to catch every host on port 80. It lands in a `tenant-*` namespace, so the stricter policy denies it, `reconcileHTTPToHTTPSRedirect` returns that error, and since it runs before the status update the TenantGateway never reaches Ready. That redirect is what keeps app routes attaching by hostname from serving plaintext on port 80. Neither half of this change is safe alone. The redirect now names the tenant apex and its wildcard. Both entries are needed: a wildcard hostname is a suffix match over any number of labels, so `*.<apex>` covers subdomains at every depth but not the bare `<apex>`. `renderWildcardCertificate` already builds the same pair. Coverage narrows in one direction. A request on port 80 with a Host outside the tenant apex now falls through to a 404 instead of a 301. That narrowing costs no covered host: the port-80 listener admits routes only from the Gateway's own namespace and cert-manager's, and the tightened policy requires every route in the tenant namespace to declare its hostnames inside the apex, so any host published from this namespace already falls under the apex or its wildcard. An inheriting child tenant does not widen that set either. Its hostnames would reach port 443 through a listener on this same Gateway, and `cozystack-gateway-hostname-policy` compares every listener against the host label of the Gateway's own namespace, so a child apex outside that label never gets its listener: the Gateway write carrying it is refused at admission whole, in `reconcileGateway` and before the redirect is rendered. A child apex under this one passes that check and the wildcard already covers it. That holds at merge base as well, so it is not something this branch changes. Deriving hostnames from `spec.apex` makes that field load-bearing for admission, so it gains `MinLength=1`. An empty apex used to reconcile to Ready, since the redirect declared no hostnames at all; it would now render `""` and `"*."` and be rejected on the HTTPRoute hostname schema, surfacing as a validation error naming the route rather than the field. The renderer checks it too, because the CRD and the controller binary roll out separately and an older CRD in the cluster will not carry the bound. That schema bound is a breaking change to an existing field and the API review gate flags it as one, so here is what it actually invalidates. The chart that renders these CRs already fails on an empty `_namespace.host`, and in DNS-01 or existingSecret mode an empty apex already broke earlier, in `renderGateway`, on a listener with `hostname: "*."`. Nothing that works today becomes invalid. What the bound rejects is a TenantGateway written by hand, past the chart, with an empty apex, and I would rather that be refused at admission naming the field than at reconcile time in controller logs, which is why the bound sits in the CRD as well as the renderer. Naming those hostnames also meant excluding the redirect from `collectHostnameClaims`. A route that attaches to the Gateway and declares hostnames otherwise reads as an app publishing them, and each claimed hostname becomes an HTTPS listener named after its first label. The first label of `*.<apex>` is `*`, which the Gateway CRD's listener-name pattern rejects, so the Gateway write fails and the reconcile stops before anything downstream. The exclusion matches on namespace, name and controller ownership rather than on the labels the renderer stamps: those labels are writable by anyone who can create a route, and the claim set also drives conflict resolution, so a label-keyed exception would let any route opt out of being marked `Accepted=False` when it loses a hostname race. Name and ownership are each required and each is pinned by its own test, since ownership alone would exclude any other controller-owned route and name alone would extend the exception to a route planted under that name by someone else. ### Radius Only namespaces starting with `tenant-` are matched. The three first-party TLSRoutes the platform ships, `kubernetes-api` in `default` plus `cdi-uploadproxy` and `vm-exportproxy`, set hostnames explicitly and none sits in a `tenant-*` namespace, so nothing changes for them. I went through every producer of routes in the tree, in three places rather than one. No Helm template emits an absent or empty hostname list in any configuration a tenant can reach: the vendored openbao, harbor, opencost and flux-operator route templates can render one, but each is behind a flag that defaults off with no values path to turn it on, and openbao would fail CRD validation first because its default route apiVersion requires hostnames. Exactly one Go site builds a route, the redirect fixed here. And one e2e fixture did rely on the old behaviour: the Gateway suite's parentRef probe declared no hostnames and is applied into `tenant-test`, which the policy matches, so it now reads the apex from the namespace label and names a hostname under it, the same way the neighbouring cases in that suite already did. cert-manager also writes routes into tenant namespaces, since it creates the Challenge in the Certificate's own namespace and the HTTP-01 solver route beside it. That route declares the hostname it is validating, so the policy admits it. ### Related issues Two open issues have a text half that this PR happens to settle, and neither of them closes here. #3788 reports that the port-80 allow-list contains the namespace its own comment says it excludes; the comment and the README paragraph now describe what that filter actually does, while the behavioural exit it names stays open. #3789 reports that the TLSRoute rule names only the deprecated API version; that premise is now written down in the template header, including what has to change before a bundle drops `v1alpha2`, but the rule still names `v1alpha2` alone. #3787 covers the wildcard-hostname listener name that the apiserver rejects, which is what makes the claim-set exclusion here load-bearing rather than tidy. ### Screenshots N/A (no UI changes). ### Downstream repositories I walked the trigger map in `docs/agents/contributing.md` against the diff, file by file. No package is added, renamed or removed; no `values.yaml` key, `values.schema.json` field, version enum or default moves; no `kind` or `plural` changes; no namespace or release-asset name changes; nothing under `hack/` moves, is renamed, or changes what a make target does. One row deserves a sentence rather than a blanket denial, because the diff does change a CRD schema: `spec.apex` on `gateway.cozystack.io/v1alpha1 TenantGateway` gains a `minLength` bound, which lands in the generated `gateway.cozystack.io_tenantgateways.yaml`. The provider hand-types `Package` under `cozystack.io` and `Plan`/`RestoreJob` under `backups.cozystack.io`; `gateway.cozystack.io` is not among them, and a tightening on an existing field adds no new field for it to offer typed access to. The `packages/extra/gateway/README.md` edit sits in the `## Security model` section, below the `## Parameters` block that `cozyvalues-gen` regenerates, so it survives `make generate`. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note fix(gateway)!: the tenant route hostname policy now requires an HTTPRoute or TLSRoute in a `tenant-*` namespace to declare a non-empty `spec.hostnames`. A route that omitted the field, or set it to an empty list, was previously admitted without any hostname being checked against the tenant apex, even though such a route inherits the hostnames of the listener it attaches to rather than matching nothing. A route in a `tenant-*` namespace relying on that inheritance is now rejected and has to name its hostnames. The controller-owned http-to-https redirect route names the tenant apex and its wildcard accordingly, so a request reaching port 80 with a Host outside the tenant apex now gets a 404 instead of a 301. `TenantGateway.spec.apex` additionally gains a `minLength: 1` bound, because the redirect derives its hostnames from that field; a TenantGateway written past the chart with an empty apex is now rejected at admission instead of failing later in the controller. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added edge-terminated gateway support with plain-HTTP listeners and TLS passthrough handling. - HTTP-to-HTTPS redirects now explicitly cover the tenant apex and subdomains. - Gateway configurations require a non-empty apex hostname. - HTTPRoute, TLSRoute, and GRPCRoute resources must declare hostnames. - **Bug Fixes** - Invalid hostname-less routes are now rejected consistently. - Controller-managed redirects no longer trigger unnecessary hostname claims, listeners, or certificates. - Switching certificate modes now cleans up obsolete ACME resources and route status. - **Documentation** - Updated security and gateway guidance for hostname validation, edge termination, route compatibility, and certificate handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
0c90abcedd |
test(rbac): cover the marketplace tap grants in the pinned RBAC suites
The tap flow added a packagesources delete grant and an ocirepositories read grant to the cozystack-api ClusterRole, and taps to the tenant core.cozystack.io grants. Both suites pin those lists on purpose, so move the count bound and assert the new grants by name instead of leaving them uncovered. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
900711567a |
fix(cozystack-basics): require route hostnames in tenant namespaces
The route hostname policy short-circuited on an absent field. Its CEL opened with `!has(object.spec.hostnames) ||`, so an HTTPRoute, TLSRoute or GRPCRoute declaring no hostnames satisfied the first operand and was admitted without a single hostname compared against the tenant apex. An explicitly empty list reached the same outcome by a different path, because `all()` over an empty list is vacuously true. A route that names no hostname does not match nothing. Under Gateway API it inherits the hostnames of every listener it attaches to, so it covers whatever that listener covers. Require the field to be present and non-empty before validating it. All three policies rendered by the template share one CEL string, so HTTPRoute, TLSRoute and GRPCRoute pick the change up together. The denial message now names the missing-hostnames case, and the Gateway e2e grep moves to the part of that message common to every denial it issues. The Gateway e2e suite carried the one fixture that relied on the old behaviour: its parentRef probe declared no hostnames and is applied into the suite's tenant namespace. It now reads the apex from the namespace label and names a hostname under it, as the neighbouring cases in that suite already did. Radius is unchanged: the policy still matches only tenant-* namespaces. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
19eaff6a30 |
test(cozystack-basics): pin which policy each route binding names
A ValidatingAdmissionPolicyBinding whose policyName points at another policy renders without complaint and leaves its own policy inert. The suite pinned validationActions on the bindings but never policyName, so pointing the GRPCRoute binding at the HTTPRoute policy kept all 56 cases green while the GRPCRoute control did nothing. uncovered() assigned to the same variable names its caller reads back when printing the mismatch, so a future call site with the arguments swapped would print a diagnostic naming the wrong side. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
70a675fd6b |
feat(cozystack-basics): gate GRPCRoute hostnames in tenant namespaces
The route hostname policy covered HTTPRoute and TLSRoute. GRPCRoute carries spec.hostnames too, and it can reach a tenant Gateway: the port-443 listeners pin allowedRoutes.kinds to HTTPRoute and TLSRoute, but the port-80 listener leaves kinds unset, and an unset list resolves to the kinds the listener protocol supports, which for HTTP includes GRPCRoute. A GRPCRoute in a tenant namespace could therefore claim any hostname on the plaintext listener with nothing checking it. Tenants hold no gateway.networking.k8s.io RBAC, so this is the same defence-in-depth against a chart bug that the other two rules are, not a tenant-user control. The new rule is the HTTPRoute rule with grpcroutes in place of httproutes and the single served version the bundle provides; the coverage guard pins that list to the bundle like the others. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
9311a48f6a |
test(cozystack-basics): make the version-coverage guard prove it still compares
The guard added alongside the TLSRoute version fix only ever ran against a tree that already satisfied it. A passing check that is checking nothing looks exactly like a passing check that is checking something, so a later refactor could empty out the comparison and the suite would stay green forever. That is the same shape as the negated assertions that cannot fail elsewhere in this repository: the assertion survives, the guarantee does not. Drive the checker with synthetic input instead of only with the live tree, the way hack/bats-no-exit-trap.bats and hack/md-no-hardwrap.bats feed their checkers a broken sample and require a complaint. Two layers need it separately. The comparison itself moves into uncovered() so a fixture can call it with a served set the named set does not cover. Above that, assert_covers runs in a subshell with both readers replaced, so the wiring, the empty-set guards and the exit status are exercised too -- covering only the inner function leaves the outer one free to become a no-op while every other case in the file stays green, because the live-tree cases are its only callers and the tree they read is correct. Each fixture requires the complaint it expects rather than any non-zero exit. A renamed helper exits 127 and an unbound variable under set -u exits 1, and either would read as a successful rejection if only the status were checked; a rejection for the wrong reason is the same false comfort as no fixture, and harder to notice because the red looks earned. Each way of breaking the guard now fails at its own case. Emptying uncovered(), reducing assert_covers to a no-op, and deleting the empty-set guards each turn a different fixture red while the live-tree cases stay green, and changing only the wording of assert_covers' complaint -- which still rejects, just for an unstated reason -- turns the omission fixture red on the message rather than passing on the exit status. Select a resourceRules entry by comparing each resource name for equality, and match versions as fixed strings rather than patterns. yq's contains() compares array elements by substring, so a rule scoped to a subresource would have satisfied a search for the parent: a rule naming tlsroutes/status answers a query for tlsroutes and donates its apiVersions to the set the guard checks coverage against. That direction is the wrong one for a fail-closed check -- it makes coverage succeed where the real rule falls short. No rule in this policy is scoped that way today. Also pin apiGroups on the TLSRoute document in the chart suite. The HTTPRoute document has always had that assertion; the asymmetry meant the case named after both kinds only checked the group for one of them. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
7ba22369d4 |
fix(cozystack-basics): match every served TLSRoute version in the hostname policy
The tenant route hostname policy names v1alpha2 alone while the vendored Gateway API bundle serves tlsroutes at v1, v1alpha2 and v1alpha3: three served versions against one named. The gap is not new. The v1.5.1 bundle this package shipped before and the v1.6.1 one it ships now serve the same three versions, with the same two marked deprecated, so nothing in this change created or widened it -- a bundle bump is simply when someone last opened the file. Coverage of the two unnamed versions rested entirely on request conversion. matchConstraints.matchPolicy is unset on both rules in this file and so defaults to Equivalent, which rewrites a request submitted under any served version into the one the rule names before the policy sees it. That is why there is no bypass to demonstrate, and equally why the arrangement is brittle: the conversion has nothing left to convert into once no named version is served, and a rule that selects nothing is not caught by failurePolicy: Fail, because nothing failed -- there was simply no match. Name all three served versions, which is the shape the HTTPRoute rule directly above already uses. The defect is a version list drifting from the bundle it must track, so a longer list on its own would not catch the next drift: add a check that reads the served versions out of the vendored bundle and fails when a policy does not cover every one of them, in either direction. Both rules get an exact-value assertion in the chart's own suite, so the coverage check and the pinned value fail for different reasons and neither stands alone. The header comment being replaced claimed TLSRoute stays at v1alpha2 through Gateway API v1.4-v1.5 and that its promotion to v1 was still upstream-only. TLSRoute has been served at v1 since v1.5.0, so that was already false, and false in the direction that hid this gap: a reader who believed it had no reason to look at the version list at all. Widening the rule then falsified every remaining place that restated which versions it matches -- the template's own header, the security-model layer list in packages/extra/gateway/README.md, and the name of the test case that asserts the value. Rather than resync three copies of a list that will move again, they now say which kinds are gated and point at the rule and its guard for the versions. A comment in the same test file now states only the behaviour it guards: an earlier form of the CEL used `... ? true : (...)`, which allowed routes in any tenant-* namespace whose host label was missing or scrubbed. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
ba15b3817a |
fix(cozystack-basics): normalize tenant hostname VAP comparisons with lowerAscii
The Ingress, Gateway, and Route hostname ValidatingAdmissionPolicies compare tenant hostnames against the tenant apex with case-sensitive CEL. The resource-side host is already lowercase — the apiserver's hostname validation permits nothing else — but the apex operand is the namespace.cozystack.io/host label value, which Kubernetes permits to carry uppercase, and the Ingress policy additionally compares against the templated root host, a free-form platform value that nothing validates for case. A mixed-case apex label wrongly denies a tenant its own legitimate lowercase in-apex hostname. An uppercase root host is the dangerous direction: the Ingress outside-root exclusion compares a lowercase host against the uppercase literal, never matches, so the exclusion stops excluding and the policy degenerates into admitting any non-wildcard host — every non-wildcard platform-apex name a tenant asks for, including the root apex itself. Only the anti-wildcard gate still holds. Wrap both operands of every comparison in .lowerAscii() so the match follows DNS case semantics — no behavior change for the all-lowercase default. Each policy reads a label it does not own: the namespace host label is settable out-of-band by a trusted caller, so none of them may assume the apex arrives normalized. The Gateway policy admits any listener hostname when the host label is absent or empty, unlike its two siblings. That is the platform's one documented fail-open exception, recorded in the threat model together with its compensating control, so changing the posture is a security-model decision rather than a side effect of normalizing case. This leaves the behavior untouched and pins its current shape in the tests. Covered by helm-unittest assertions pinning each comparison as a whole operand pair, so dropping the normalization from any single operand is caught; a rendering case that pins the uppercase-root-host escape shut; and e2e probes that set a mixed-case apex on an isolated namespace and assert both halves of the match — a lowercase in-apex HTTPRoute and Ingress are still admitted, while a foreign apex and a hostname that ends with the apex without a dot boundary are still denied. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
267ea513ad |
fix(cozystack-basics): gate the hostname VAP policies on the VAP API
Render the route, gateway and ingress hostname ValidatingAdmissionPolicies only where the admissionregistration.k8s.io/v1/ValidatingAdmissionPolicy API is served, matching the deletion-protection guard, so a cluster without that API does not receive an unrenderable resource. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Alexey Artamonov
|
f9afc73096 |
fix(monitoring): surface missing VictoriaLogs and gate dead-end vlinsert ExternalName (#3181)
Platform log ingestion could fail silently: fluent-bit shipped to vlinsert-generic while the monitoring HelmRelease reported Ready, and a vlinsert-generic ExternalName in cozy-monitoring dead-ended at a tenant-root VictoriaLogs that did not exist when the root tenant had monitoring disabled. Logs were dropped with no failing signal. Three complementary changes: - Readiness (do not stay silently Ready): the monitoring-system HelmRelease now uses a `poller` waitStrategy and a healthCheckExpr on its VLCluster (status.updateStatus), so a missing/unhealthy log store makes the release not-ready instead of flipping Ready as soon as the CR is applied. Mirrors the seaweedfs CNPG pattern. - No empty-log-store hole: system/monitoring rejects an empty logsStorages at render time (helm fail). With no entry no VLCluster is created, the readiness gate has nothing to match, and fluent-bit would silently drop logs — so this is made a loud misconfiguration rather than a documented footgun. - No silent fallback: the cozy-monitoring -> tenant-root vlinsert/vminsert ExternalName redirects in cozystack-basics are now created only when the root tenant hosts monitoring, gated on _cluster.monitoring-enabled — fed from a new platform value monitoring.rootEnabled (default true, preserving the historical always-create behaviour). monitoring.rootEnabled is the single render-time source of truth: changing it re-renders cozystack-basics (helm-controller watches the cozystack-values Secret it flows into). A live `lookup` of Tenant/root spec.monitoring is deliberately NOT used: cozystack-basics is an operator-generated HelmRelease with drift detection off, so it re-renders only on install/upgrade/values-change — never on a reconcile tick or when a lookup result changes — and a lookup would go stale in both directions. Operators must keep monitoring.rootEnabled in sync with Tenant/root spec.monitoring. Covered by helm-unittest across all four charts: readiness gate fields, empty-vs-populated logsStorages contract, ExternalName present/absent and null-guard contract, and the platform wiring into the cozystack-values Secret. The CEL health predicates are exercised live, not in helm-unittest (which cannot evaluate CEL). Adds the missing `test:` target to packages/extra/monitoring/Makefile so its suite runs in CI. Known limitation (follow-up): the legacy cozystack.monitoring PackageSource that installs system/monitoring directly into cozy-monitoring cannot carry the readiness gate, because the engine's ComponentInstall/HelmRelease builder and the vendored helm-controller API v1.4.3 lack waitStrategy/healthCheckExprs. That source is not referenced by any active bundle; full coverage needs a helm-controller bump plus engine support. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
d89ad70698 |
fix(cozystack-basics): validate legacy Ingress hostnames against the tenant apex
The Gateway (cozystack-gateway-hostname-policy) and Route (cozystack-route-hostname-policy) admission policies constrain tenant hostnames on the Gateway API path. Gateway API is opt-in and off by default, so in the default configuration tenant applications publish through a legacy networking.k8s.io/Ingress on the shared ingress-nginx, which had no equivalent policy. Add a fail-closed ValidatingAdmissionPolicy + Binding for Ingress in tenant-* namespaces. A hostname on spec.rules[].host or spec.tls[].hosts[] is allowed when it is within the namespace's own apex (namespace.cozystack.io/host), or when it is a concrete (non-wildcard) name entirely outside the platform root apex (_cluster.root-host); a hostname under the platform root apex but outside the namespace's own apex, and a wildcard taken through the outside-root path (which could match the platform apex itself), are rejected. The outside-root branch keeps the kubernetes app's Proxied exposeMethod working, since it routes a user-supplied external domain to a nested cluster. A rule with no host (absent or empty, an unbounded catch-all) and spec.defaultBackend are also rejected. The policy templates the platform root apex into its CEL and renders only when _cluster.root-host is set, so it is never shipped in a state that would admit every hostname. A tenant-* namespace missing its own apex label is denied. Cover the policy with helm-unittest (render, including the root-host, empty-host, and wildcard guards) and behavioral admission tests against the live apiserver (in-apex allow, external-domain allow, own-apex-wildcard allow, external-tls allow, under-root-outside-apex deny, prefix-adjacent-under-root deny, wildcard-matching-apex deny, hostless deny, empty-string-host deny, defaultBackend deny), and document it as an additional layer in the gateway security model. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei SviridkinandTimofei Larkin
|
8ac68b24e9 |
feat(cozystack-api): register sdn APIService and SecurityGroup RBAC
Register the v1alpha1.sdn.cozystack.io aggregated APIService, grant the cozystack-api ServiceAccount full access to CiliumNetworkPolicy (the backing object for SecurityGroup), and let tenants self-service their SecurityGroups: full access in the base tenant role and read access in the view role, mirroring how apps.cozystack.io resources are exposed. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> Co-authored-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Myasnikov Daniil
|
bb322138ec |
fix(dashboard): grant tenant dashboard read on cozy-public PVCs
The vm-disk source image dropdown in the console lists PersistentVolumeClaims in cozy-public (the vm-default-images-* golden images). The browser issues that list as the tenant identity (e.g. system:serviceaccount:tenant-root:tenant-root), but the cozy:tenant:dashboard Role only granted read on Flux helmrepositories and helmcharts. The list returned 403 and the dropdown stayed empty even when the images existed. Add get/list/watch on persistentvolumeclaims to the Role. cozy-public is a shared public namespace, so read on its PVCs is safe to expose to every tenant dashboard subject. A helm-unittest regression pins the grant. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
e59f93605a |
test(cozystack-basics): pin tenant view grant on core.cozystack.io options
The tenant view aggregation grants read on the core.cozystack.io/options virtual resource so dashboard dropdowns can be populated. Add a helm-unittest on the cozy:tenant:view:base ClusterRole so a silent RBAC regression on the tenant side is caught, mirroring the apiserver-side rbac test. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
32241afe03 |
feat(gateway): drop tenant-* ban on gateway-attached-namespaces
Removes two gates that prevented child tenants from attaching to a parent's publishing Gateway: 1. Render-time fail() in templates/gateway-hostname-policy.yaml that refused to render the chart when _cluster.gateway-attached- namespaces contained any tenant-* entry. 2. cozystack-gateway-attached-namespaces-policy VAP that enforced the same rule at Package CR admission time. The original rationale — 'a tenant in attachedNamespaces can hijack the publishing tenant's hostnames' — assumed a static-name allowedRoutes whitelist where adding tenant-* literally widened the attach surface for arbitrary hostnames. Under the inheritance refactor (Stage 1/3/5) the attach surface is governed by the namespace.cozystack.io/gateway label selector, not by entries in AttachedNamespaces; and hostname hijack is independently blocked by three other layers: * Layer 4 — cozystack-tenant-host-policy VAP on Tenant.spec.host * Layer 5 — cozystack-namespace-host-label-policy VAP on the namespace label * Layer 7 — cozystack-route-hostname-policy VAP on HTTPRoute and TLSRoute hostnames Those three defend against the hijack vector regardless of what sits in attachedNamespaces, and they apply uniformly to every tenant (inheriting or owning a Gateway). The dropped gates added no extra defence, while their presence broke the inheritance flow kvaps reasonably asks for in the May 25 review. Doc count for gateway-hostname-policy.yaml drops from 8 to 6 (one VAP + one Binding removed); helm-unittest fixtures updated to match, with a regression-guard test asserting the removed VAP name no longer appears in any rendered document. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
30302eabe4 |
feat(tenantgateway): switch Gateway allowedRoutes to label selector
The Gateway's HTTPS / TLS-passthrough listeners now match attached namespaces by the namespace.cozystack.io/gateway label keyed on the TenantGateway's own namespace, instead of a static kubernetes.io/metadata.name In [list] whitelist. This restores inheritance parity with the legacy ingress flow: child tenants of a Gateway-owning ancestor publish through the ancestor's Gateway by default — without their own LB IP / Certificate, and without operator intervention to add each child by name to platform values. Two writers populate the label: * apps/tenant chart namespace.yaml — each tenant namespace gets a label pointing at the nearest ancestor that owns a Gateway (self if owning, inherited otherwise). The tenant Helm chart is the source of truth for tenant-tree labels; the controller never touches them (annotation-gated GC). * cozystack-controller (ensureNamespaceLabels) — patches the label onto every namespace in tgw.Spec.AttachedNamespaces, so cozy-* system namespaces (cert-manager, monitoring, harbor, …) reach the publishing Gateway alongside the tenant tree. Tracks its own writes via the cozystack.io/gateway-attached-by annotation; on a reconcile where an entry is removed from AttachedNamespaces, the label is garbage-collected (annotation present + namespace no longer expected → strip both label and annotation). Also adds the gateway-attach label to tenant-root itself via cozystack-basics/templates/tenant-root.yaml when _cluster.gateway-enabled is true, so the Gateway in tenant-root's own namespace passes its own selector for the controller-owned http→https redirect HTTPRoute. Tests (TDD, red→green): * TestReconcile_HTTPSListenerUsesGatewayLabelSelector pins the new MatchLabels shape on HTTPS listeners. * TestReconcile_LabelsAttachedNamespaces pins the patch path: every namespace in spec.AttachedNamespaces (plus tgw.Namespace) carries namespace.cozystack.io/gateway=<owner> after reconcile. * TestReconcile_LabelGCRemovesDroppedAttachedNamespaces pins the GC path: removing an entry between reconciles strips the label. * TestReconcile_LabelGCDoesNotStripHelmOwnedLabels pins the safety contract: labels lacking the controller annotation (Helm-owned tenant labels) are left alone. * tenant-root_test.yaml adds the positive case for the conditional label render under _cluster.gateway-enabled=true. The HTTP listener (port 80) keeps the narrower static-name selector — it allows only the tenant namespace plus cozy-cert-manager (for HTTP-01 ACME challenges) and must NOT accept routes from app namespaces or child tenants, since unattended routes on the HTTP listener serve plaintext and leak credentials. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
039924f59d |
feat(gateway): align _namespace.gateway propagation with tenant.gatewayEffective
Two related fixes for the gap where the tenant chart's gateway HelmRelease materialises but child apps (harbor, bucket) silently fall back to ingress-nginx: - packages/apps/tenant/templates/namespace.yaml: switch the _namespace.gateway resolution from raw .Values.gateway to tenant.gatewayEffective. This is the same helper consulted by gateway.yaml (HelmRelease render), so per-tenant Gateway materialisation and child-app HTTPRoute / Ingress selection now stay in lockstep — including the auto-default for derived-apex tenants. - packages/system/cozystack-basics/templates/cozystack-values-secret.yaml: add a conditional gateway field to the tenant-root cozystack-values Secret. cozystack-basics is the only writer of _namespace.* for tenant-root (apps/tenant chart skips tenant-root via the existing ne 'tenant-root' gate). Without this line, tenant-root apps read _namespace.gateway as empty and route to ingress even when the platform's gateway.enabled flag is on. Conditional on _cluster.gateway-enabled='true' so non-Gateway clusters keep the empty value. Test fixture in cozystack-basics/tests/cozystack-values-secret_test.yaml pins the conditional and the regression-guard that the other four _namespace keys (etcd, ingress, monitoring, seaweedfs) stay unconditional. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
d2e851f63b |
feat: Gateway API support via Cilium
Replace ingress-nginx as the default ingress layer for cozystack-native
services with Cilium Gateway API, materialised per-tenant via a new
TenantGateway CRD reconciled by cozystack-controller.
The extra/gateway chart renders one gateway.cozystack.io/v1alpha1
TenantGateway per tenant; the controller owns the resulting Gateway,
per-tenant Issuer, Certificate(s), and the http->https redirect
HTTPRoute. This avoids the helm-vs-controller race on
Gateway.spec.listeners when route-driven listeners are added.
publishing.certificates.solver selects HTTP-01 (default; per-app HTTPS
listener + per-app Certificate) or DNS-01 (opt-in; single wildcard
Certificate per apex; providers: cloudflare, route53, digitalocean,
rfc2136). HTTP-01 keeps zero-config per-app onboarding; DNS-01 saves
Let's Encrypt budget on apex-shared clusters.
Security model -- seven independent layers, all admission-time gates
fail-closed:
1. Listener allowedRoutes namespace whitelist via the
kube-apiserver-written kubernetes.io/metadata.name label;
plain-HTTP listener is strictly narrower than HTTPS, and HTTPS /
passthrough listeners restrict allowedRoutes.kinds to
HTTPRoute / TLSRoute respectively.
2. cozystack-gateway-hostname-policy VAP on Gateway.
3. cozystack-gateway-attached-namespaces-policy VAP on Package --
blocks tenant-* in gateway.attachedNamespaces.
4. cozystack-tenant-host-policy VAP on Tenant -- blocks spec.host
writes from non-trusted callers.
5. cozystack-namespace-host-label-policy VAP on Namespace -- blocks
writes to namespace.cozystack.io/host from non-trusted callers.
6. Render-time fail in cozystack-basics for tenant-* in
gateway.attachedNamespaces.
7. cozystack-route-hostname-policy VAP on HTTPRoute / TLSRoute,
scoped to tenant-* namespaces.
Within-apex cross-namespace hostname conflict is resolved at reconcile
time by a HostnameConflict condition (cozy-* namespace wins).
Five reconcile paths refuse to rewrite pre-existing objects with the
controller-derived name but no OwnerReference back to the
TenantGateway: Gateway, redirect HTTPRoute, per-tenant Issuer, wildcard
Certificate, per-listener Certificate.
Derived-apex tenants (tenants whose host is a subdomain of a parent
tenant's) auto-enable Gateway when the platform-level gateway.enabled
is on.
Bonus fix: pkg/registry/apps/application/rest.go was ignoring the
createValidation and deleteValidation callbacks that genericapiserver
hands to every storage method, silently bypassing admission for Create
and Delete on every apps.cozystack.io resource (Tenant, MariaDB,
Postgres, Kubernetes, ...). Aligned with the existing updateValidation
pattern.
Always-on changes: cilium.envoy.enabled + cilium.gatewayAPI.enabled
flip to true (extra cilium-envoy DaemonSet, ~100 MB RAM/node idle),
and the cozystack-api admission-chain fix. Everything else is gated
behind gateway.enabled=false and tenant.spec.gateway=false; existing
clusters see no behavioural change until an operator opts in.
Tests: 60+ Go cases in internal/controller/tenantgateway/, admission-
chain regression tests in pkg/registry/apps/application/, helm-unittest
across 7 charts (extra/gateway, core/platform, apps/tenant,
system/cozystack-basics, system/cert-manager-issuers,
system/cozystack-api, system/dashboard), and 14 e2e bats scenarios in
hack/e2e-apps/gateway.bats. New make test-controllers target wired
into pull-requests.yaml.
Deps: sigs.k8s.io/gateway-api v1.4.1, cert-manager v1.17.4.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
|