mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 22:13:18 +09:00
a18cbc581c4829b657ebef0095f4ef0afc4cfc12
460
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Mattia Eleuteri
|
1cefadcef6 |
docs(backups): say when a base backup gone from the bucket is still pinned
A restore leaves the base backup to the plugin only once the cnpg.io/Backup is gone. A base backup removed from the bucket by a lifecycle rule or by hand keeps its cnpg.io/Backup until the plugin's next maintenance pass on the source cluster, and indefinitely once that cluster no longer runs under the same UID. A restore in that window fails after the purge and on every retry, with nothing on the RestoreJob to unpin it; deleting the cnpg.io/Backup is the way out and does not touch the object store. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
64e837727c |
docs(backups): say what recovery does when the latest timeline is a sibling
recovery_target_timeline latest targets the highest consecutively numbered history file, without checking that it descends from the base backup. On an archive holding sibling branches that can be another branch, and recovery then stops with "requested timeline N is not a child of this server's history" rather than following the backup's own line of descent. The page also names the second case where a restore does not pin its base backup: a cnpg.io/Backup removed by retention. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
6999609585 |
docs(backups): bound Postgres PITR by the last archived transaction
The recoverable window was described as ending at the newest archived WAL segment, and the safe fallback as "at/before the most recent completed backup". A time-targeted recovery ends just before the first transaction after the target, and a segment switch or a backup is not one, so on a database at rest every instant after its last write fails, including the end of a later backup. Aiming at a backup's stoppedAt also misses it: stoppedAt is truncated to the second and the plugin compares barman's end time to the microsecond. Describe the bound the recovery actually has, how to read it from the archive, the one-second margin on stoppedAt, what pinning the base backup changes, and why archives with several timelines need it. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Aleksei Sviridkin
|
cb035f8eac |
docs(agents): stop calling the image-ref known gaps unfixed
Two of the gaps listed there are handled in the tree now, and each entry already says whether it is. The heading sentence still claimed all of them were open. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
79854b16fb |
Merge remote-tracking branch 'origin/fix/build-list-orphan-images' into feat/arm64-image-builds
Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2f08832392 |
Merge remote-tracking branch 'origin/feat/talos-arm64' into feat/arm64-image-builds
Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
6d759cc73e |
feat(build): build package images for amd64 and arm64 (#4498)
## What this PR does
Package images can now be built for amd64 and arm64 in one `make image`,
and a published build is multi-arch unless `PLATFORM` says otherwise.
This is the build half of arm64 support from #1961 and #903. CI still
publishes amd64 only until it gets a native arm64 leg, which comes in a
separate PR.
Builder stages now run on the build platform and compile for the target
one, so Go never runs under emulation. Under emulation it is slow and it
crashes: the migration-controller Dockerfile already records a "bad
pointer in Go heap" from exactly that. On top of this, `PLATFORM`
defaults to `linux/amd64,linux/arm64`. A build on an arm64 workstation
can't publish an arm64-only digest any more. That happened once already,
the cilium pin was arm64-only until
|
||
|
cozystack-ci[bot]
|
59faddc98a |
docs: add changelog for v1.6.4
Signed-off-by: cozystack-ci[bot] <274107086+cozystack-ci[bot]@users.noreply.github.com> |
||
|
Aleksei Sviridkin
|
7753394bfe |
build: rebuild every package that has an image target
flux-plunger, keycloak-operator, kilo and migration-controller have image targets but were missing from the root build list, which the build matrix, the main build and release preparation all iterate. No CI path rebuilt them, so their pins stayed on images pushed by hand while Dockerfile, patch and base-image changes never shipped, and the multi-arch pre-release build would find single-arch pins it cannot replace. migration-controller shipped a bare :latest with no digest. Add the four packages, and a test that derives the set from the package Makefiles so a new image target cannot be left out again. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
f02963a2c7 |
fix(keycloak-operator): keep the image host inside repository
The image target stamped only `tag`, leaving the committed `registry: ghcr.io` in place, so a build against the CI registry would render the public host with a digest that exists only in the build registry. Stamping the build host into `registry` instead would split the source registry across two keys, where the nightly mirror's substring host rewrite cannot reach it and the published tree would keep pointing at the private registry. Set `registry: ""` and keep the whole host in `repository`, stamped by the image target. The upstream chart joins the two keys, so the rendered image string is unchanged, and the ref now has the same shape as every other stamped reference the mirror and promotion already handle. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
6907c22302 |
docs(agents): describe the talos package as multi-arch
The build notes said the talos package pins linux/amd64, which no longer holds and would steer an agent into re-adding the pin. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
084ac47237 |
feat(build): build published images for amd64 and arm64 by default
PLATFORM defaulted to empty, so buildx built for the host alone. On an arm64 workstation `make image` published an arm64-only digest, which amd64 nodes then fail to pull; the cilium pin shipped exactly that until it was rebuilt by hand. A build now targets linux/amd64 and linux/arm64 unless PLATFORM is set, and the digest a package stamps is the index digest. LOAD=1 keeps building for the host only. The classic docker image store cannot load a multi-platform index, and loading is the documented local workflow. This changes local pushes: a `make image` on the default docker driver without the containerd image store now fails with "Multi-platform build is not supported for the docker driver", where it used to build for the host. Such a build needs a docker-container builder (BUILDER), or LOAD=1 or an explicit PLATFORM for a single architecture. talos and testing pin linux/amd64 with `override`, placed before the include because common-envs.mk expands BUILDX_ARGS immediately. The Talos assets are amd64: matchbox bakes the kernel and initramfs into its image, and the e2e sandbox boots the disk under qemu-system-x86_64. An arm64 variant of either would still serve or boot amd64 files. CI pins linux/amd64 on every image build. Nothing there sets up QEMU, fork builds export a single-platform archive, and tags.yaml builds on the default docker driver, so the default would break or emulate those jobs. Native arm64 builds belong in a leg of their own. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
368d00b5d4 |
test(e2e): remove unused monitoring suites and audit BATS runners (#4019)
## What this PR does I removed the two monitoring OIDC BATS suites that had no runner after the Chainsaw migration, and brought the runner audit and wrapper OIDC test from [#3778](https://github.com/cozystack/cozystack/pull/3778). Also added checks for audience mapper and `OIDC_MODE=System`. The named `KeycloakRealmGroup` test documents coverage the document-count and kind assertions already provided. Runner audit reports unused BATS suites, missing files in runner paths, and unresolved Make variables (including `$(E2E_SUITE)`). Live Monitoring API admission, Grafana operator reconciliation and mode transitions are not tested here. ### Validation - `make -j4 unit-tests SHELL=/usr/bin/dash` passed. - Monitoring Helm tests: 53 system-chart tests and 10 wrapper-chart tests passed. - Runner audit: 12 cases passed under dash and Bats 1.14.0. The full shell syntax and BATS sweep passed under dash too. - Three chart mutations and seven runner mutations failed as expected. ### Downstream repositories - [x] No downstream repository is affected by this change ### Release note ```release-note NONE ``` |
||
|
scooby87
|
03e2da5805 |
feat(postgres,mariadb)!: drop plaintext user passwords (#4078)
## What this PR does Removes the plaintext `users[].password` field from the `apps/postgres` and `apps/mariadb` charts (and the mariadb `root` account), so managed-database passwords can no longer be pinned in tenant values or committed to git. Passwords are always chart-generated into the `<release>-credentials` Secret and preserved across reconciles via `lookup`, matching the model `redis`/`valkey` already use. This closes the "User passwords" item of the portal upstream epic (aenix-org/cozyportal#1060). The generated password never lands in the init-script Secret or the SQL it runs: the postgres init-job reads each user's password from the mounted `<release>-credentials` Secret at run time and applies it through a psql variable (`ALTER ROLE … WITH PASSWORD :'pw'`), so it does not linger in the init-script manifest a tenant can read. It does live in cleartext `stringData` in the `<release>-credentials` Secret itself, which is a chart manifest and therefore kept in Helm release history for `MaxHistory` revisions like any chart-generated secret — that residue is exactly why rotation cannot be built on the chart (see below). A `password` or a `passwordRotation` key left over in a tenant's values is accepted-but-inert and draws an admission warning, so an operator editing it is told it has no effect rather than getting a silent 200. Backup/restore into a copy keeps the target's chart-managed credentials working: the CNPG driver clears `bootstrap.enabled` once recovery is healthy so the init-job re-applies the generated passwords onto the recovered roles, and the MariaDB driver excludes `mysql.global_priv` from the dump (`ignoreGlobalPriv`) for backups captured from this release onward, so restoring one of those into a copy does not clobber the target's chart-managed users or `root`. Excluding the grant table has a flip side, and it applies to every MariaDB restore, not only a to-copy one: only accounts declared through `User` CRs (the chart's `users` map) are captured, so an account created out of band directly in MySQL is in no backup and a restore brings back its data and `mysql.db` grants but not the account itself. This is also a backup-side fix only: a restore from a backup taken **before** this change still replays the source's `mysql.global_priv` and overwrites the target's users and `root`, and `root` then needs an out-of-band reset (the operator applies `rootPasswordSecretKeyRef` only at datadir bootstrap, so there is no in-band path). See `docs/operations/backup-classes.md` for the convergence window, the out-of-band-account limitation, and this old-backup caveat. Password **rotation** is deliberately **not** in this PR. A chart-rendered credentials Secret keeps its cleartext in Helm release history for `MaxHistory` revisions, so a rotation built on it cannot revoke a leaked password; rotation is being reworked as a controller that owns the Secret — design proposal in cozystack/community#72. Migration is seamless: existing releases keep their current passwords on upgrade (the value already lives in `<release>-credentials`, which `lookup` re-reads and preserves); a `password` still present in a tenant's values is ignored, so no existing HelmRelease fails validation. Verification: `helm unittest` passes for both charts (postgres 87, mariadb 126, including render-time username guards that reject any name k8s would later reject as an invalid Secret key or User resource name); `go build ./...`, `go vet`, the backup-controller tests and the apps-apiserver tests are green; `make generate` was run with the CI-pinned `cozyvalues-gen` v1.6.0 so `values.schema.json`, the READMEs, the API types, and the `cozyrds` ApplicationDefinitions are all in sync (the branch was rebased onto `main`, which reconciled cleanly with the read-replica autoscaling feature that landed there). ### Screenshots N/A — no UI change. ### Downstream repositories - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: affected — `content/en/docs/next/cozystack-api/go-types.md` is hand-written and still shows `Password:` in the Go examples, and the managed-app reference pages regenerate from these chart READMEs, so both document `users[name].password` until refreshed (not yet filed). - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: affected — the provider hand-maintains the postgres/mariadb schemas and per-field defaults; this PR removes `users[].password`. A follow-up issue/PR is needed there (not yet filed). `terraform-provider-cozystack` and `website` are reached by this diff; both boxes are left unticked because the follow-ups are not yet filed. Same-repo future work this change suggests: the controller-owned password rotation (design in cozystack/community#72), and extending the "passwords only in the Secret" model to the other engines that still accept plaintext (`clickhouse`, `mongodb`, `nats`, `opensearch`, `rabbitmq`, `vpn`). ### Release note ```release-note feat(postgres,mariadb)!: passwords for managed PostgreSQL and MariaDB users can no longer be set via `users[].password` — they are always auto-generated and stored only in the `<release>-credentials` Secret, never rendered into the init-script or the SQL it runs. Existing releases keep their current passwords on upgrade; a `password` (or a `passwordRotation`) left in values is ignored with an admission warning. Password rotation is deferred to a follow-up controller design (cozystack/community#72). MariaDB backups now exclude the `mysql.global_priv` grant table (`ignoreGlobalPriv`), so a restore recreates only accounts declared through the chart's `users` map — an account created out of band directly in MySQL is not captured and must be recreated after a restore. ``` |
||
|
myasnikovdaniil
|
0cb37dba9e |
docs(release): add changelog for v1.6.3 (#4077)
This PR adds the changelog for release `v1.6.3`.
✅ Changelog has been automatically generated in
`docs/changelogs/v1.6.3.md`.
|
||
|
Aleksei Sviridkin
|
564f540f46 |
fix(kubernetes): render the OIDC UID header only from v1.33 (#4407)
## What this PR does Stacked on #4398; review the diff against `fix/kamaji-pin-hash-reset`. The tenant Kubernetes chart now renders `--requestheader-uid-headers=X-Remote-Uid` only from an effective Kubernetes version of v1.33, where `RemoteRequestHeaderUID` is beta and on by default. It no longer renders a `--feature-gates` term of its own on any version. On v1.32 it used to add a term enabling that gate, which is alpha there, whenever it could read the operator's entries and they left the gate unset; no release tag contains that path, so dropping it now changes nothing a user has run. The chart still reads `--emulated-version`, because emulation moves the effective version: `--emulated-version=1.32` on a v1.35 tenant puts the gate back to alpha-and-off, and the header flag is not rendered there. It still reads the operator's `--feature-gates` entries for an opt-out (`RemoteRequestHeaderUID=false`, or `AllBeta=false` with no explicit `=true`), so the header flag never lands next to a disabled gate. `AllAlpha` is no longer read, since it says nothing about a beta gate. An unquoted `--emulated-version=value` entry whose kube version has a non-zero patch component now fails the render and names the entry. The rule mirrors upstream rather than the regex in #4396: `toVersionMap` parses each value with `version.Parse` and refuses only `ver.Patch() != 0` (`staging/src/k8s.io/component-base/compatibility/registry.go` lines 320-326 at v1.35.0, the same code in `staging/src/k8s.io/apiserver/pkg/util/version/registry.go` at v1.31.0). `version.Parse` takes `^\s*v?([0-9]+(?:\.[0-9]+)*)(.*)*$` (`versionMatchRE` in `k8s.io/apimachinery/pkg/util/version/version.go`, identical at v0.31.1 and v0.35.0). So `1.32`, `v1.32`, `1.32-rc`, `1.32.0` and `1.32.0-rc` all start an apiserver emulating 1.32, and `1.32.4` and `1.32.4-rc` do not. A `^[0-9]+\.[0-9]+$` check would have refused the first four. The chart extracts the numeric run with the same pattern, reads the minor from it, and fails only on a non-zero third component. A quoted value is still left unread. Two deliberate narrowings: - The chart no longer refuses a mix of bare and `kube:`-prefixed `--feature-gates` terms. Once the chart writes no gate term, the spelling of the operator's terms no longer concerns anything it renders. The apiserver still refuses the mix (`component-base/compatibility/registry.go:405` at v1.35.0), and the chart now passes it through like the other fatal gate values it already leaves to the apiserver: unknown names, unparseable values, quoted terms. - On v1.32 the chart now neither renders nor judges the UID flags, as it already did on v1.31. An operator's own `--requestheader-uid-headers` entry without the gate reaches an apiserver that refuses it, which is how every released chart version behaved. A cluster built from `main` that already runs a v1.32 tenant with OIDC on loses the chart's `--requestheader-uid-headers` and gate term on the next render; no release carried them. That holds only while no release is cut from `main` before this merges; a release that carries the v1.32 gate term first would turn this into a breaking change for v1.32 tenants, whose migration is to write both flags into `extraArgs` themselves. An element such as `kube=1.32=x` is now split at its first `=` only, the way `toVersionMap` splits it with `SplitN(compVer, "=", 2)`. The chart used to skip that element, which the apiserver reads as 1.32, and so rendered the header flag against an emulated 1.32. That gap predates this branch. Rebased onto #4398 at `fb2e57c3b`, which reads a two-element `--feature-gates` and `--emulated-version` separately. That behaviour carries over unchanged under the v1.33 floor: behind a hidden `--emulated-version` only an explicit `RemoteRequestHeaderUID=false` is refused beside a header entry, while a blanket `AllBeta=false` waits for a reading, because a lower emulated version can make the gate alpha again; nothing is refused as an opt-out behind a hidden `--feature-gates` value; the empty-element refusal applies behind both; and behind either one `X-Remote-Uid` is added only beside a uid-headers list of the operator's. The README breaking-changes note from #4398 now says the chart refuses neither the mixed gate spelling nor two kube emulated versions. In `docs/oidc-tenant.md` the paragraphs that list refusals now name the managed-flag move and refusal from #4398 and no longer carry a count, and the two-element paragraph now says what the chart renders behind a hidden value. Tests: `helm unittest` goes from 308 on #4398 to 298. 26 cases are deleted. 25 of them pinned the v1.32 gate term, `AllAlpha` handling or the mixed-spelling refusal. One emulated a version below 1.32, which the below-floor emulation case now covers. v1.32 cases that tested parsing rather than the gate are retargeted to v1.34. Sixteen cases are added: the patch-level refusal, the version parsing below the floor and above it (on v1.35 emulating `1.33-rc`, `v1.33`, `1.33.0` and `kube=1.33=x`, where only a correct minor renders the header), the first-`=` split, a pin that a mixed gate spelling now passes through, emulation landing exactly on 1.33, and a v1.32 opt-out beside a header entry passing through untouched. Coverage notes, not applied: no case pins that the patch-level refusal stays off with `spec.oidc.mode: None`, where the block that holds it does not run. The case "splits a component element at its first equals sign only" pins that the kube element of `kube=1.32=x` is read. It does not pin the split itself, because the version pattern ignores what follows the numbers either way. The `extraArgs` field description still calls the field the escape hatch for apiserver flags unrelated to OIDC. It is unchanged here, and it stays accurate for the v1.32 advice in the guide, because the UID flags belong to the aggregation layer rather than to OIDC. Optional, not applied: on v1.32 the chart could refuse an operator's `--requestheader-uid-headers` entry that comes without the gate, since the gate is off by default there and the apiserver will not start. The same skip also drops two refusals the branch history had at 1.32, a header entry beside `RemoteRequestHeaderUID=false` and a header list ending in a comma; both now reach an apiserver that refuses them. It stays out so that v1.32 matches v1.31 and the released charts, where the chart judges none of these flags. Not applied: for a value with a zero-prefixed major such as `01.32.4`, the patch refusal suggests `01.32`. The apiserver refuses both values over the leading zero, so the configuration fails either way and only the suggested fix is off. Why the patch refusal stays while the mixed-spelling refusal goes: the patch refusal is about `--emulated-version`, which the chart reads itself to decide what to render. The mixed spelling concerns `--feature-gates`, which the chart no longer writes to. Not split out: the `--emulated-version` parsing and the patch-level refusal share the fix commit with the floor move, because both change how the same value is read. The change also removes a comment from `hack/e2e-chainsaw/_lib/run-kubernetes.sh` that became false, so select-e2e runs the full suite for this PR. Default path: renders of the parent and this branch through `tests/values/common.yaml` and `tests/values/oidc-system.yaml` at v1.31 to v1.35 are byte-identical, once the randomly generated Kamaji token fields are masked, except for OIDC System at v1.32, which loses exactly the two UID lines. ### Screenshots Not a UI change. ### Downstream repositories - [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: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The only field that changes is the description of `version`, which returns to its earlier wording. The website's kubernetes reference page is regenerated from the README on the next tag, and the Terraform provider keeps its own copy of that description, which already reads "Kubernetes major.minor version to deploy.". Closes #4373 Closes #4396 ### Release note ```release-note fix(kubernetes): the tenant chart renders the aggregation-layer UID header flag with OIDC on only from an effective Kubernetes v1.33, and never renders a --feature-gates term of its own. v1.32 tenants render no UID flags, which matches every released version, so nothing regresses. This supersedes the v1.32 clause of the #3539 release note. An unquoted --emulated-version entry with a patch-level kube version such as 1.32.4, which the kube-apiserver refuses, now fails the render and names the entry. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * OIDC-enabled clusters on Kubernetes 1.33 and later now receive the UID request-header setting without an added feature-gate flag. * Chart rendering now checks Kubernetes and Talos version compatibility and prevents rendering when worker deployments do not meet ownership or retention requirements. * **Bug Fixes** * Emulated Kubernetes versions with non-zero patch components now fail with a clear error; supported version formats are recognized more consistently. * **Documentation** * Updated version and OIDC guidance to reflect current chart behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
339cffe70f |
feat(kamaji)!: update to 26.9.4-edge and read every apiServer extraArgs entry (#4398)
## What this PR does BREAKING CHANGE: a Kubernetes application whose `controlPlane.apiServer.extraArgs` names a flag the tenant control plane now owns fails to render until that entry is removed, except that an `--enable-admission-plugins=` entry is moved into the new `controlPlane.apiServer.admissionControllers` and a `--kubelet-preferred-address-types=` entry into the new `controlPlane.kubelet.preferredAddressTypes`. Closes #3541. Updates the Kamaji build this repo ships and rewrites the part of the tenant Kubernetes chart whose correctness rested on the old one. The new pin is `26.9.4-edge`, which is what `make update` picks. Two upstream changes reach us. The tenant control plane no longer folds the kube-apiserver argument list into a map keyed by flag name, so entries supplied through the control plane spec arrive at the apiserver verbatim, duplicates included. And the reset that rewrites those arguments from scratch keys on a hash of the list rather than on its length, so swapping one flag for another no longer strands the old one on the running Deployment. The OIDC block in the tenant chart read only the last entry of each flag. That was right while earlier entries were dropped before the apiserver saw them, and it is wrong now. An operator whose `extraArgs` carried `--feature-gates=RemoteRequestHeaderUID=false` followed by an unrelated `--feature-gates` entry got `--requestheader-uid-headers=X-Remote-Uid` rendered next to a gate the apiserver has switched off, and the control plane would not start. A blanket `AllBeta=false` has the same shape, and so does an `--emulated-version` entry naming kube followed by one naming another component. The chart now reads every entry with the accumulation each flag performs. Occurrences of `--requestheader-uid-headers` append into one list, so where a list of the operator's own omits `X-Remote-Uid` the chart adds its own element beside it instead of refusing to render: the union is what the apiserver validates, and their header keeps working. Feature-gate terms collect per component and are applied in one pass, so the chart carries only its own term, placed last where it wins. Naming the kube component both bare and `kube:`-prefixed now fails the render, because the apiserver refuses that configuration and nothing the chart renders repairs it. `--emulated-version` accumulates too, and two entries naming kube stop the apiserver outright, so the chart renders nothing rather than pick a version that will not exist. The same upstream change narrows what `extraArgs` can do, and the chart now refuses the part that stopped working. Kamaji 26.9.4-edge sets a list of kube-apiserver flags itself and drops any `extraArgs` entry that names one before the apiserver sees the list, where `26.3.6-edge` merged `extraArgs` last and let the entry win. A tenant with `--enable-admission-plugins=NodeRestriction,AlwaysPullImages` loses both plugins on the first reconcile after the upgrade, with nothing to say so. The drop happens on the running control plane whatever the chart renders, so the chart cannot prevent it; what it can do is refuse to render such an entry, in every `oidc.mode`, and name it. The list mirrors `buildKubeAPIServerCommand` at the pinned tag for the control plane this chart renders (an etcd DataStore, konnectivity on, no `dataStoreOverrides`), and the `controlPlane.apiServer.extraArgs` description in the chart README carries it. A flag the control plane only defaults, such as `--authorization-mode` or `--requestheader-group-headers`, is still yours to set. `--enable-admission-plugins` gets a replacement: the new `controlPlane.apiServer.admissionControllers` passes a list through to the KamajiControlPlane `spec.admissionControllers`, which the control plane renders into that flag, and the application API checks each name against the enum the KamajiControlPlane CRD declares. The list is rendered only when non-empty, so a tenant that sets nothing renders exactly as before, and a non-empty list replaces the control plane's default set, as the `extraArgs` entry did under the previous build. `--kubelet-preferred-address-types` gets the same treatment: the list the chart used to hardcode on the KamajiControlPlane is now `controlPlane.kubelet.preferredAddressTypes`, which renders that list when left empty. Neither entry is refused where it can be moved: an `--enable-admission-plugins=` or `--kubelet-preferred-address-types=` entry is moved into its field while the field is empty, on every render, and left out of the rendered `extraArgs`, so a tenant carrying one keeps its setting across the Kamaji update with no edit and no ordering to get right. The entry is refused beside a non-empty field, when it names nothing, and in the two-entry spelling. A separate defect in the same package is fixed here. The TenantControlPlane schema reaches existing clusters through the `kamaji-crds` subchart's templated CRDs, which every release upgrade applies; `crds/` holds the copies a fresh install lays down first. Directory-based renders lost that schema entirely, `helm template`, `lint` and `package` alike, because the parent `.helmignore` matched the subchart's `hack/` directory at any depth and that is where the templates keep the schema they read with `.Files.Get`. Helm honours `.helmignore` for a directory and skips it for an archive, so the cluster never saw the difference and no local check could catch a schema defect. The patterns are anchored to the chart root and a unit test asserts the rendered CRDs carry their versions. The bump is not cleanly reversible, and that is the one thing to know before rolling it out. The args annotation holds a checksum once the new build has written it, and the old build reads that annotation with `strconv.Atoi`. The parse fails and `resetKubeAPIServerFlags` returns early, ahead of both the args reset and the annotation write that would put a number back. So after a rollback to `26.3.6-edge` the count-keyed reset is disabled on every TenantControlPlane: a later removal of an `extraArgs` entry no longer clears the flag from the running Deployment, and it stays that way until someone writes a number into `kube-apiserver.kamaji.clastix.io/args` by hand. Findings from a live upgrade on a dev cluster, none of them caused by the diff. The DNS-service-IP check moved out of a CEL rule on the CRD into the admission webhook (`internal/webhook/handlers/tcp_dns.go`), so it now depends on that webhook being reachable where the apiserver used to enforce it by itself. The manager's readiness probe targets port 8081 while the webhook serves on 9443, so a Ready pod does not prove 9443 accepts connections; with `failurePolicy: Fail` on the TenantControlPlane webhook, that window rejects writes rather than letting them through unvalidated. The readiness probe this build adds to kube-scheduler and kube-controller-manager survives a rollback, since the old build neither sets nor clears those two; that one is benign. Seven limits worth naming rather than leaving to be found. The mixed-spelling refusal quotes the entry carrying each spelling, so one entry holding both, `--feature-gates=Foo=true,kube:Bar=true`, is quoted twice in its own message; the refusal is right and the message reads oddly. The unit test that pins the rendered CRD schemas asserts on a count, so a CRD missing from the render altogether fails on an empty comparison rather than naming the CRD that went missing. There is one window during the upgrade itself: the chart used to fold an operator's own `--feature-gates` terms into the entry it rendered, because the old build would otherwise drop them, and it now renders only its own term. Nothing orders a tenant release after the Kamaji operator has rolled, so a `v1.32` tenant that sets gate terms of its own with OIDC on can lose them until the new operator reconciles it, which it then does on its own. The same window takes a `--requestheader-uid-headers` list of the operator's own that lacks `X-Remote-Uid`: the chart now renders its own entry beside it instead of refusing, and the old operator keeps one value per flag name, the chart's, so the operator's header is dropped until the new operator reconciles. Which entry an error message quotes is pinned only where one entry is eligible. Three variables choose between equally eligible entries, the gate dictionary taking the last setting for a name and the uid-headers ones the first, and no test distinguishes first from last when two entries qualify, so a future edit could make a message name the wrong entry of the two without a test noticing. What the chart renders and what it refuses is pinned exhaustively; it is the provenance in three message strings that is not. The block is deliberately inconsistent about two shapes that stop the apiserver equally hard: two `--emulated-version` entries naming kube make it withhold silently, while a kube-prefixed and a bare gate spelling in the same configuration make it refuse the render. The refusal is right where the apiserver rejects the configuration whatever the chart does; the withholding is right where the chart cannot read a version it must render against. They are two rules, not one, and a third that covers both is fair to ask for. Moving the refusal out from under those conditions so it fires everywhere is the obvious third rule and it is not free: the apiserver refuses the mix on `v1.31` too, but from `k8s.io/apiserver/pkg/util/version` rather than from the component-base `componentGlobalsRegistry.Set()` the message names, so hoisting it means rewriting that sentence as well. Reading a flag and its value written as two entries would be better than withholding for it, and is a change of behaviour rather than a correction, so it is not here. And two sentences about `--emulated-version` in `docs/oidc-tenant.md` sit thirty lines apart and read as a retraction in sequence, though each is true at its own layer: `toVersionMap` accepts an element naming a foreign component, and the registry rejects it later. What this leaves open, by name. The guard for a valueless flag at the end of `extraArgs` covers `--requestheader-uid-headers`, `--feature-gates` and `--emulated-version` only; any other flag that takes a value swallows the appended `--authentication-config` the same way, which is an input mistake the chart does not look for. The managed-flag refusal matches names as written, as Kamaji does: an underscore spelling such as `--secure_port` passes both, and the apiserver normalises it to `--secure-port`, where the operator's value wins as the later one, the same as before this change. Rendering `--authentication-config` ahead of the operator's entries would close that class for OIDC, since only the UID flags would then follow them; it changes the rendered order, and so rolls every OIDC tenant that has `extraArgs` of its own, and belongs in a separate change after the OIDC floor move. One pre-existing comment needs checking: the note above `extraArgs` in `templates/cluster.yaml` says that omitting the field would leave the live array untouched under JSON merge patch, which reads against Helm's `createPatch`, where a key the previous manifest carried and the new one does not is patched to null; what that comment was guarding against is still to be verified. The KamajiControlPlane CRD accepts only the plugin names it lists, so a plugin outside that list that the apiserver leaves off by default can no longer be enabled; that limit is Kamaji's, since it owns the flag and restricts the field. A name moved out of an `extraArgs` entry is not checked against that list by the chart, so one outside it fails when helm-controller applies the KamajiControlPlane, with an error that names it, rather than at render; the chart keeps no copy of the list to check it with. A repeated address type in a moved entry is dropped, keeping the first place it appears, because the field is a set. With `oidc.mode: None` the chart does not read `extraArgs` for the OIDC flags at all, and two shapes regress with the bump there: a kube feature-gate term written both bare and `kube:`-prefixed across entries, and two `--emulated-version` entries naming kube. The old build kept one entry per flag, the new one passes both, and the apiserver refuses both at startup. Next to a flag and value written as two entries the chart keeps the refusals that hold whatever the hidden value is, but it does not read positions: a hidden `--feature-gates` term keeps the opt-out refusal off even when it sits before the explicit opt-out and so cannot undo it. What the live run could not exercise: the konnectivity datapath, because the stand had no tenant workers, and `--requestheader-uid-headers`, which comes from the tenant Kubernetes chart and no image swap reaches. Both are covered by render and unit tests only. One shape the old collapse was quietly repairing, with OIDC on, needs a guard now that it is gone. A bare `--requestheader-uid-headers` in `extraArgs` used to be overwritten by the entry of the same name the chart renders, because the collapse kept one value per flag name; the same held for a bare `--feature-gates` on `v1.32`, where the chart renders a gate entry too. A bare `--emulated-version`, or a bare `--feature-gates` on any other version, had no entry of the chart's to be overwritten by and was emitted bare in sorted position, where it took the next flag as its value, so it never worked. Without it those entries reach the apiserver as written, and the apiserver gives a valueless long flag the next argument whatever that argument looks like. What follows the operator's last entry is the `--authentication-config` the chart appends, so the flag swallows it and the AuthenticationConfiguration is never applied. On `--requestheader-uid-headers` the path is accepted as a header name and the control plane comes up with OIDC silently off, failing every login for that tenant with nothing in the cluster to say why; the other two reject it and the apiserver exits. The chart refuses to render that shape and says which entry to fix. Two things in the image build fall out of the bump. The datastore deletion fix carried here as a patch landed upstream verbatim, so its patch no longer applies, and one failing patch fails the whole apply of the directory. The Go module overlay is obsolete for the opposite reason: this tag already carries `golang.org/x/net`, `golang.org/x/crypto` and `go.opentelemetry.io/otel/sdk` above the versions the overlay forced, so keeping it would downgrade all three. What I checked: `helm unittest` passes at 307 cases for the package, with the OIDC suite carrying every shape above, and the image builder stage builds end to end at the new tag with the one remaining patch. Kamaji's own support for Kubernetes 1.36 and 1.37 does not move the tenant version enum here; that is a separate update of the `kubernetes` package. No image reference in `packages/system/kamaji/values.yaml` is touched, since CI stamps that digest. ### Screenshots No UI changes. ### Downstream repositories I walked the trigger map file by file against the diff. The chart templates, the vendored Kamaji chart, the image build and the docs reach nothing downstream. The one generated file that reaches downstream is `packages/apps/kubernetes/values.schema.json`, which gains `controlPlane.apiServer.admissionControllers` and `controlPlane.kubelet.preferredAddressTypes` and a changed `extraArgs` description; both new fields are optional with defaults, so a stored Kubernetes object needs no change, no field is removed or renamed and no version enum moves. The new fields are the one trigger the map fires on: the Terraform provider keeps a hand-written schema. `packages/apps/kubernetes/README.md` feeds the website's managed-apps reference pages, which a bot regenerates on a stable tag. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: model `controlPlane.apiServer.admissionControllers` and `controlPlane.kubelet.preferredAddressTypes` on the kubernetes resource (schema, model, expand/flatten). - [ ] [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: ### Release note ```release-note feat(kamaji): update Kamaji to 26.9.4-edge. Tenant kube-apiserver arguments are now derived from a hash of the control plane's extraArgs instead of from its length, so on the first reconcile after the upgrade every tenant control plane rewrites those arguments from scratch and rolls once. The rebuilt list also orders them differently, Kamaji's own flags sorted first and the operator's kept verbatim after them, so the roll happens whether or not the tenant sets any extraArgs. A tenant with OIDC enabled takes a second roll when the chart's own flags land. Rolling the Kamaji image back to an earlier build does not restore the old behaviour: the older code reads that annotation as a number, gives up when it finds a checksum, and from then on never clears a flag you remove from extraArgs, on any TenantControlPlane, until the annotation is set back to a number by hand. The tenant Kubernetes chart reads every extraArgs entry the way the apiserver accumulates them: a --requestheader-uid-headers list that omits X-Remote-Uid now gets the chart's own element beside it instead of failing the render, a feature gate set in an earlier entry is honoured, and naming the kube feature-gates component both bare and kube:-prefixed fails the render, because the apiserver refuses that configuration outright. Writing --feature-gates or --emulated-version as a flag and a value in two separate entries now keeps the chart from rendering UID flags on its own, because an entry the chart cannot read may hide an opt-out; a --requestheader-uid-headers list of your own still gets X-Remote-Uid beside it, and write --flag=value to get the rest back. Action required for tenants that pass their own apiserver flags: Kamaji now sets a list of flags itself and drops a controlPlane.apiServer.extraArgs entry naming one, where the previous build let that entry win, and the Kubernetes application now refuses to render such an entry, except for the two it moves into a field. A tenant carrying one of the others stops reconciling after the upgrade, and no later change to that tenant applies, until the entry is removed; the entry no longer has an effect either way. The list is in the description of controlPlane.apiServer.extraArgs. An --enable-admission-plugins=value entry is moved into the new controlPlane.apiServer.admissionControllers, a list that replaces the control plane's default set as the entry did, and a --kubelet-preferred-address-types=value entry into the new controlPlane.kubelet.preferredAddressTypes, while that field is empty; nothing has to be edited for them. Before upgrading, list the affected entries with: kubectl get kuberneteses.apps.cozystack.io --all-namespaces --output json | jq --raw-output '["--advertise-address","--client-ca-file","--enable-admission-plugins","--service-cluster-ip-range","--kubelet-client-certificate","--kubelet-client-key","--kubelet-preferred-address-types","--proxy-client-cert-file","--proxy-client-key-file","--requestheader-allowed-names","--requestheader-client-ca-file","--secure-port","--service-account-key-file","--service-account-signing-key-file","--tls-cert-file","--tls-private-key-file","--etcd-servers","--etcd-cafile","--etcd-certfile","--etcd-keyfile","--etcd-prefix","--etcd-compaction-interval","--egress-selector-config-file"] as $owned | .items[] | .metadata as $m | (.spec.controlPlane.apiServer.extraArgs // [])[] | select((split("=")[0]) as $f | $owned | index($f)) | "\($m.namespace)/\($m.name): \(.)"' Entries it lists for --enable-admission-plugins or --kubelet-preferred-address-types need a change only when written as two entries, when they name nothing, or next to a non-empty field of their own. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Configure tenant control-plane admission controllers and kubelet preferred address types. - Choose a cert-manager issuer for webhook certificates; self-signed certificates remain the default. - **Bug Fixes** - Improved OIDC argument handling for repeated, split, quoted, and conflicting settings, with clearer render-time errors. - DataStores are recognized as unused when no tenant control planes reference them. - **Validation** - DataStore endpoints are checked for length, count, and IPv6 address formatting. - Chart-managed tenant control-plane arguments are rejected when supplied through `extraArgs`. - **Documentation** - Updated Kubernetes OIDC and certificate issuer guidance. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Alexey Artamonov
|
ce74fdeea9 |
docs(backups): describe restored credentials and pre-upgrade residue
Say that a restored copy converges its application passwords shortly after recovery, that MariaDB backups leave out the grant table, and that dropping the password field does not rewrite copies made before the upgrade: old Backup snapshots and Helm revisions still hold the password that stays live. The credentials projector comment counts only the engines that still take a password from values. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
a92e89e969 |
docs(kubernetes): describe the OIDC UID header from v1.33
The tenant OIDC guide, the version field description and the e2e helper comment still described a --feature-gates term rendered on v1.32. Left as they were, they would tell an operator that the chart switches that alpha gate on, which it no longer does. The guide now says what a v1.32 tenant has to write itself to get the UID, and which --emulated-version values fail the render, so an operator who meets that refusal finds it explained. The breaking-changes note in the README said the chart refuses the mixed gate spelling while OIDC is on; with that refusal gone it would promise a render error that no longer comes. The version description returns to its earlier wording in values.yaml and in the update-versions.sh template, because that template rewrites the line on every version bump and would otherwise bring the old text back. make generate carries it into the schema, README, API types and ApplicationDefinition. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
4e9f83ac0d |
fix(kubernetes)!: move or refuse extraArgs flags the control plane owns
From 26.9.4-edge the tenant control plane sets a list of kube-apiserver flags itself and drops any extraArgs entry naming one before the apiserver sees the list; 26.3.6-edge let such an entry win. An existing tenant that set, for example, its own admission plugins through extraArgs would lose them on the first reconcile after the upgrade with nothing to say so, and a flag written with its value in a separate entry leaves that value behind as a positional argument the apiserver refuses. Two of those flags have a KamajiControlPlane field the control plane renders them from. An --enable-admission-plugins=value or --kubelet-preferred-address-types=value entry is moved into that field while the field is empty, and left out of the rendered extraArgs, so the tenant keeps its setting across the upgrade without an edit. The move is computed from the values on every render; the last entry wins, as it did while the control plane kept one entry per flag name, and a repeated address type is dropped because that field is a set. The post-upgrade Job that patches extraArgs onto the KamajiControlPlane reads the same list. Every other entry naming an owned flag is refused at render time in every OIDC mode, since the chart cannot stop the drop and the refusal is where the operator learns of it. So are the two moved flags beside a non-empty field, with a value that names nothing or is quoted, and in the two-entry spelling. The list mirrors the Kamaji builder for the shape of control plane this chart renders, and the extraArgs description names the same flags. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
88264216b7 |
docs(kubernetes): restate the OIDC UID flags for the merged reading
The migration note told operators to move the extraArgs entry count in the same edit so the running control plane would rewrite its apiserver arguments; the control plane compares a hash of the list now, so any change already forces the rebuild and no second edit is owed in either direction. The paragraphs describing a value that survives only as the last entry of its flag, and an empty value reaching the apiserver as a bare flag, describe a build that is no longer shipped. What replaces them is the accumulation each flag actually performs: a uid-headers list the apiserver validates as the union of every entry, feature gates settled across entries with the last term winning and one spelling of the kube component throughout, and an emulated version that two entries cannot both name. One contract the earlier text sold is narrowed to what the chart keeps. The render checks run only where the chart renders something, so the three readings it refuses to guess at switch them off as well, and a shape that would otherwise be refused passes through to a control plane that does not start. A flag and value written as two entries is a narrower case, and the page says which checks hold next to one. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov Daniil
|
c55ab268e2 |
test(monitoring): remove orphaned OIDC BATS suites
The two pre-Chainsaw monitoring OIDC suites have had no invocation site since the app suite migrated to Chainsaw. Remove them instead of carrying dead tests that force the selector to run every live suite. Keep the existing selector and capture guards for the historical path shape, and update their comments now that the directory is empty. Retain a named chart assertion for the no-KeycloakRealmGroup ownership invariant; the existing count and kind assertions already enforce the same behavior. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Timofei Larkin
|
180030be72 |
fix(api): refuse only what a name declaration actually breaks
Three cases refused creates of a kind, or were documented as refused, without the declaration being at fault. A schema that is not a JSON object was reported as an invalid x-cozystack-name, although such a schema has no declaration to blame, and the spec schema's own loader already reports it. It now declares nothing, so creates keep working and a kind with a legacy cap keeps it. Any unknown key inside a declaration took the kind offline, so a generator adding a title or an example would do the same. Keywords that constrain nothing, the JSON Schema annotations and x- extensions, are now ignored. A constraint keyword the server does not implement is still refused, because accepting it would drop it silently. The docs said a declaration no name satisfies is refused. That holds for lengths only: a pattern is checked to compile, not to match anything. The docs now say so. The parser moves to pkg/apis/apps/validation so the API review gate can reuse it rather than keep a copy of its rules. Assisted-by: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
146e06e75e |
feat(api): take Application name limits from the application's own schema (#4367)
## What this PR does
The aggregated API server capped two kinds' names by knowing what their
charts render. A `Kubernetes` cluster name was held to 32 characters
because every worker pool is a `KubernetesNodes` release named
`kubernetes-nodes-<name>-<pool>`, and the default `md0` pool has to fit
the 53-character Helm release name limit. A `Kafka` name was held to 44
because the KRaft controller pod `kafka-<name>-c-<hash>-<id>` has to fit
the 63-character DNS-1123 label limit. The arithmetic is right, but it
is the charts' knowledge living in the platform: the next application
with a sub-resource budget needs another `if r.kindName == ...` branch,
and an application served from outside this repository — the whole point
of `ApplicationDefinition` — cannot get one at all.
An application now declares the limits on its own name in its own
schema, and the server applies whatever it finds there for whatever kind
the definition describes. The chart's `values.yaml` carries a `## @name`
annotation, `cozyvalues-gen` v1.7.0 publishes it at the root of
`values.schema.json` as `x-cozystack-name`, and `hack/update-crd.sh`
carries it into the `ApplicationDefinition`:
```yaml
## @name {string} - Cluster name. Every worker pool is a separate KubernetesNodes release named `kubernetes-nodes-<name>-<pool>`, so the name must leave room for the default `md0` pool inside the 53-character Helm release name limit.
## @maxLength 32
```
```json
"x-cozystack-name": {
"type": "string",
"description": "Cluster name. Every worker pool is a separate KubernetesNodes release named `kubernetes-nodes-<name>-<pool>`, so the name must leave room for the default `md0` pool inside the 53-character Helm release name limit.",
"maxLength": 32
}
```
`maxLength`, `minLength` and `pattern` are honoured; `pattern` is
unanchored, as JSON Schema specifies. A declaration only tightens:
`prefix + name` must still fit the 53-character Helm limit. A rejected
name is answered with the chart's own description, or with "limit
declared by the <Kind> application" when the chart gives none.
The schema is untrusted input — definitions from other repositories are
written by hand — so a declaration that cannot be enforced as written
(not an object, a type error, an unknown keyword such as a misspelled
`maxLenght`, `maxLength` below 1, a negative `minLength`, `minLength`
above `maxLength` or above the Helm budget, a pattern that is not RE2)
refuses every create of that kind with `configuration error: the <Kind>
ApplicationDefinition declares an invalid x-cozystack-name: …`, and logs
the same at start-up. Dropping the declaration would admit names the
chart cannot render; applying it would reject every name and blame the
user. Other kinds are unaffected, and existing objects of the affected
kind can still be read, updated and deleted.
An upgrade brings `cozystack-api` up before `kubernetes-rd` and
`kafka-rd` re-render (`kubernetes-rd` depends on `cozystack-engine`), so
the new server can briefly read a definition that predates its
declaration. For exactly that case — a definition that declares nothing
— the two previous caps stay in `legacyNameCaps` as a floor, to be
removed once no supported upgrade starts from such a definition. That is
the only per-kind knowledge left in the name checks.
Also in this PR:
- The `pre-commit` job pins `cozyvalues-gen` v1.7.0, the first release
that understands `@name`
([cozystack/cozyvalues-gen#33](https://github.com/cozystack/cozyvalues-gen/pull/33)).
- The API review gate now diffs the root `x-cozystack-name` of every
definition as `metadata.name`, so a chart that lowers its cap or adds a
pattern is reported like any other tightened field. On this PR it
reports the Kubernetes and Kafka caps as added; their effective limits
are unchanged.
- The migration 56 comment that pointed at the removed
`maxKubernetesClusterName` now points at the chart's declaration.
Not in scope, because none of it is a static limit a chart can declare:
the `Tenant` name pattern in `pkg/apis/apps/validation`, the computed
tenant-namespace length check (it depends on the parent namespace at
request time), the tenant quota hierarchy, and the Phase 2 removed-field
warning for `Kubernetes`.
### How it is tested
`TestNameCapComesFromTheShippedSchema` reads the shipped `kubernetes`
and `kafka` ApplicationDefinitions off disk, builds storage through
`NewREST` as the server does, and checks the cap and the chart's
explanation; it then rewrites `maxLength` inside the shipped schema and
checks the limit moves with it, which a cap held in code would not.
`TestInvalidNameSchemaFailsClosed` covers each malformed declaration
through the same path, `TestLegacyNameCapsCoverUndeclaredDefinitions`
pins the upgrade floor and that a declaration replaces it, and
`TestClassifyNameDeclaration` covers the gate.
### Downstream repositories
Two repositories are affected and neither follow-up is filed yet, so no
box is ticked.
- `cozystack/website` — `content/en/docs/next/development.md` documents
the `values.yaml` annotations and needs `@name`;
`content/en/docs/next/cozystack-api/application-definitions.md`
describes `openAPISchema` and needs `x-cozystack-name`.
- `cozystack/external-apps-example` — it pins no `cozyvalues-gen`
version and regenerates against whatever is current; nothing in it
declares a name constraint, so it is unaffected until it chooses to.
- [ ] 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:
### Release note
```release-note
feat(api): Application name limits now come from the application's own schema instead of being hardcoded per kind in the aggregated API server. A chart declares them with a `## @name` annotation in `values.yaml`, which cozyvalues-gen v1.7.0 publishes as `x-cozystack-name`; the API server enforces `maxLength`, `minLength` and `pattern` on `metadata.name` for any kind, answers a rejected name with the chart's own explanation, and refuses creates of a kind whose declaration it cannot enforce with a configuration error. The Kubernetes (32) and Kafka (44) name caps are unchanged in effect and are now declared by their charts.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Applications can declare minimum and maximum name lengths and
formatting patterns.
* Kubernetes cluster names are limited to 32 characters, and Kafka
cluster names to 44 characters, to meet generated resource naming
limits.
* Names that violate declared constraints are rejected with an
explanatory message. Tighter name constraints are also reported as
breaking changes.
* Declared maximum lengths cannot exceed the platform’s existing Helm
release-name budget.
* **Documentation**
* Added guidance on declaring and enforcing chart-specific resource-name
limits.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
Aleksei Sviridkin
|
1f30ac4598 |
fix(tests): let the mariadb waits cover a first boot (#3769)
## What this PR does Closes #3741 Several waits for a database HelmRelease to become Ready gave up before the release could: the mariadb, mongodb, rabbitmq and clickhouse backup walkthroughs waited 300 seconds, and both mariadb Chainsaw suites gave their HelmRelease-ready assert 6 minutes. A generated release carries a 10-minute install timeout (`pkg/cmd/server/start.go:91`), so a failed install was reported as a bare timeout, because the release had not yet recorded why it failed, and for mariadb a first boot the startup probe was still willing to wait for failed the harness instead. A wait on such a release has two floors and has to beat the larger. The first is the app's own first boot: cozystack-api leaves `spec.waitStrategy` unset and helm-controller defaults it to `poller`, so the install waits through kstatus, and kstatus maps the operator CR's `Ready=False` onto `InProgress`. For mariadb that boot may take the chart's 310-second startup budget (`startupProbe.failureThreshold: 30` in `packages/apps/mariadb/templates/mariadb.yaml` with the operator's probe defaults of 20 and 10); mongodb, rabbitmq and clickhouse set no startup probe. The second is the install timeout. At or below it the waiter is the one that expires, because its clock starts when the step runs and the install's only once the Application has been reconciled and the chart artifact fetched. The install timeout binds in every case here, so the walkthrough waits go to 660 seconds and the mariadb asserts to 11 minutes. Raising a wait inside a walkthrough spends the Chainsaw op that runs the script, and an op that runs out SIGKILLs the process group and loses the script's error. Each op therefore grows by exactly what the waits it reaches gained: mariadb's round-trip from 30 to 42 minutes, rabbitmq's from 30 to 42, mongodb's from 30 to 36 (its round-trip runs with `SKIP_RESTORE`, so only the source wait counts), and clickhouse's from 60 to 72, which still sits above the sum of every inner ceiling. That bounds a failure at a single wait, not a run that is slow at every stage and then fails late; bounding that needs a phase budget, not a larger literal. postgres keeps its 360-second waits and every walkthrough keeps its 300-second bucket wait: whether a suite pays the extra minutes on every red run is a per-suite call. The mariadb endpoint asserts keep their job. A freshly applied CR carries no conditions and kstatus reads it as current, so the install wait can end before any pod is up; an endpoint address closes that window. That is also why the `wait_for_field` on the CR stays in the walkthrough. `wait_hr_ready` now dumps every condition and the release history on timeout and on `Stalled=True`, instead of the single Ready message a scheduled retry can overwrite before anyone reads it. It also stops printing one `(none recorded)` line for different answers: a release that never appeared, a release deleted during the wait, a lookup kubectl could not answer, a failed history read and an empty history each say so, with kubectl's own stderr kept, and a release seen earlier still gets its dump when only the last lookup failed. The change goes to every byte-identical copy of the helper, in `examples/backups/{clickhouse,mariadb,mongodb,postgres,rabbitmq}/00-helpers.sh`, and they stay identical. `hack/mariadb-first-boot-waits.bats` pins the mariadb budgets against the tree. It derives both floors from their sources instead of pinning literals, so raising `failureThreshold` past the install timeout, or changing the install-timeout default, reds it; each source has a test that refuses to proceed when it cannot be read, including the ApplicationDefinition, whose two override annotations would each invalidate the derivation. It holds the round-trip op at a fixed remainder over the ceilings up to and including the source wait, bucket stage and S3 preflight included, and at a fixed slack once both mariadb waits are paid, so raising a wait without the op reds. The extractors are also tested against fixtures, because the tree cannot tell a correct extractor from a broken one. Beyond mariadb, the file runs every exit branch of `wait_hr_ready` in each copy against a stubbed kubectl, and pins the copies byte-identical. The population is every definition in the tree found by grep, minus two named exceptions held to still differ, so a new copy joins without editing the test and an exception whose copy converges goes red: - `examples/backups/redis/00-helpers.sh` is written in the compact one-line style of the rest of that file, so converging it rewrites its shape rather than applying the shared body. - `examples/backups/kafka-metadata/00-helpers.sh` is a helper of its own with the same diagnostic gap on timeout (it prints no reason at all), and closing that is a change to kafka-metadata. `docs/agents/e2e-testing.md` replaces the reviewer checklist's 5-6m standard and its list of exceptions, which the tree had outgrown, with the floor a wait has to beat and the rule that a longer wait carries its reason in-line. The derivation gets a section of its own, opening with the rule in one line. For a release an app chart renders itself the floor is `spec.install.timeout`, then `spec.timeout`, then helm-controller's 5m default, and the nested `<name>-system` releases never take cozystack-api's 10m. The harbor comments name the floor each of their asserts sits against, and the opensearch comment stops citing the removed standard. Adding `hack/mariadb-first-boot-waits.bats` does not escalate the E2E selection: `hack/[^/]+\.bats$` sits in `inert_config_pattern` in `hack/select-e2e.sh`, and `full_suite_pattern` escalates only `hack/e2e-*.bats`. On this changeset `hack/select-e2e.sh` selects `clickhouse harbor mariadb mongodb opensearch postgres rabbitmq`. What this costs is paid only on a failing run; the raised numbers are ceilings. Summing the timeouts stated on each suite's own steps, mariadb goes from 92m to 114m, clickhouse from 116m to 128m, rabbitmq from 67m to 79m and mongodb from 95m to 101m, before the untimed steps that fall back to the defaults in `.chainsaw.yaml`. clickhouse is the most expensive suite by that sum. All of them compete for the 215m `timeout-minutes` of the whole `e2e` job, which also covers image download, sandbox preparation and the Cozystack install, and a job cancelled at the cap loses both its log and `cozyreport.tgz`. This does not fix the failure that led here. In the run that prompted the investigation the mariadb replica never opened port 3306 at all, spending its whole 310-second budget inside `mariadb-install-db` before being killed, and no wait budget would have saved it. That defect is #3744. What this fixes is a second, independent defect found while reading the same code. One thing here is argued rather than measured: the mechanism is checked against helm-controller and kstatus, and every number is derived from the tree, but no run ID pins that it is what any particular red run hit. Both floors justify 660s whatever the cause was, so being wrong about the cause leaves slack rather than breakage. ### Not in this PR - This PR retires the mariadb, mongodb, rabbitmq and clickhouse walkthrough waits and the two mariadb asserts. The rest is tracked in #4404: the other Chainsaw HR-Ready asserts under their floor, the harbor asserts, the 300-second bucket waits, the postgres waits, and the redis and kafka-metadata diagnostic gap. - The clickhouse round-trip comment says every wait fails fast, `wait_hr_ready` on `Stalled=True`, "so a genuinely broken backup reports in seconds". A release whose install is still running is neither Ready nor Stalled, so that holds for a stalled release and not for an install that has not finished. The sentence predates this change. - `hack/mariadb-first-boot-waits.bats` is large for what it guards (two floors, two ratchets on the op, and the helper identity pin), with many fixture tests on its own extractors. Trimming it to that core is a separate decision. - Two comments in that file state facts about the mariadb suite that are true today and that nothing checks: three script ops carry a minute timeout, and the only one ahead of the round-trip is the 8m pre-clean. - The mariadb op comment says a bound on the whole flow "needs a phase budget (docs/agents/e2e-testing.md)"; the doc describes it as bounding the collection phase as a phase (`COZY_DIAG_PHASE_BUDGET`), so the pointer would resolve faster if it named that bullet. - Cosmetic: the behavioural test's loop body is not indented under its `for`, one line of the bats header runs past the width of the rest, and the checklist's sentence under the example assert ("what this block demonstrates least") reads awkwardly. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file, for all eighteen files. The five `examples/backups/*/00-helpers.sh` files change the diagnostics of `wait_hr_ready`, and the mariadb, mongodb, rabbitmq and clickhouse walkthrough scripts change wait budgets; the manifests the walkthroughs apply and the flows they document are unchanged, so no website page restates anything that moved. Four Chainsaw suites (mariadb, clickhouse, mongodb, rabbitmq) change timeouts and comments, and the harbor and opensearch suites change comments only. `docs/agents/e2e-testing.md` changes the reviewer checklist entry and adds the sizing section. `hack/mariadb-first-boot-waits.bats` is a new file picked up by the existing `BATS_UNIT_FILES` wildcard. `hack/package.mk`, `hack/common-envs.mk` and `hack/update-crd.sh` are untouched, and no `values.schema.json`, `values.yaml` default, version enum, `ApplicationDefinition`, CRD, namespace or release asset is touched. - [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: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(tests): the MariaDB, MongoDB, RabbitMQ and ClickHouse backup examples and the MariaDB Chainsaw suites now wait for a HelmRelease past the release's own install timeout, so a failed install reports its own reason instead of a bare timeout, and the examples' HelmRelease wait now tells a release that never appeared, one deleted mid-wait and a kubectl that could not answer apart. ``` |
||
|
Aleksei Sviridkin
|
b1519acfe7 |
fix(e2e): cap the bucket S3 reads so the verify step honors its timeout (#4281)
## What this PR does Refs #4268 — a plain reference, not `Fixes`: the issue asks for a `script` op to be cut off at its declared ceiling, and that is Chainsaw behaviour this repo cannot change. Chainsaw runs the op's shell under Go's `exec.CommandContext`, whose default cancel SIGKILLs the shell PID alone and then blocks on the inherited stdout/stderr pipe until an orphaned `mc` closes it, so the op returns only when `mc` exits on its own rather than at the ceiling. This PR mitigates that where it bit — per-command caps so no descendant outlives its own budget — but the op-timeout gap itself lives in Chainsaw, so the link stays a reference rather than a `Fixes`. The sibling step that produced the 606s, `verify-every-bucket-takes-a-write` in the `bucket-1-capacity` Test, is on `main` and already carries its own per-call caps; this PR applies the same shape to `verify-s3-access`. On run 34948279363 the bucket suite's `verify-every-bucket-takes-a-write` step declared `timeout: 5m` but ran 606s and ended on `mc`'s own non-zero status rather than being cut at the ceiling. `verify-s3-access` in `hack/e2e-chainsaw/bucket/chainsaw-test.yaml` is the same-shaped step in this suite. This PR wraps every S3 (`mc`) and apiserver (`kubectl`) read in `verify-s3-access` in its own `timeout`, so a hung read dies at a known budget instead of holding the step open, and corrects the two places (`hack/e2e-chainsaw/.chainsaw.yaml` and `docs/agents/e2e-testing.md`) that explained the op ceiling with a mechanism that does not hold. It caps `verify-s3-access` for sharing the shape, not for producing the 606s number. ### Which of the issue's two candidate mechanisms holds The issue named two candidates and stated the log could not separate them. The Chainsaw v0.2.15 source separates them, and it is the second one, with the exact refinement the issue proposed. **Candidate 1 — "an operation `timeout` does not bound a `script` op" — is refuted.** The op timeout is applied and does reach the process: `pkg/runner/operations/script.go:24` copies the step's `timeout` into `Timeouts.Exec`, and `:38` wraps the op context with `ctx, cancel := context.WithTimeout(ctx, tc.Timeouts().Exec)` before calling `op.Exec(ctx, …)`. So a 5m step produces a real 300s deadline on the context the command runs under. **Candidate 2 — "it bounds it and kills the shell, but Chainsaw waits on a pipe a grandchild (`mc`) still holds open" — is confirmed.** In `pkg/engine/operations/script/operation.go` the command is built with `exec.CommandContext(ctx, shell, …)` (`:105`), and in `execute` its stdout/stderr are set to `*bytes.Buffer` (`:128`–`:129`, the buffers are `internal.CommandOutput` in `pkg/engine/operations/internal/command_output.go:12`–`13`) and run via `cmd.Run()` (`:130`). Two Go `os/exec` properties then produce the overrun: `exec.CommandContext`'s default cancel is `Process.Kill()` on the shell PID alone — not a process-group kill, and the operation sets no `SysProcAttr.Setpgid` — and because stdout/stderr are not `*os.File`, `os/exec` allocates OS pipes and copier goroutines, so `cmd.Wait()` blocks until those pipes reach EOF. EOF needs every inherited write-end closed. When the shell forks `mc`, `mc` inherits those fds; at 300s the deadline SIGKILLs the shell but `mc` is orphaned, keeps retrying the S3 PUT, and holds the pipe, so the op does not return until `mc` exits on its own at 606s. `cmd.Run()` then returns `signal: killed`, which is stored in the `$error` binding and never printed — only the `($error == null): true` check failure is logged — which is why grepping the run for `signal: killed` found nothing. **Empirical confirmation.** A standalone Go program reproduces it exactly. `exec.CommandContext` with a 1s timeout, `*bytes.Buffer` stdout/stderr, running `sh -c 'sleep 2 & sleep 4'` (a descendant that inherits the pipe): `cmd.Run()` returns after ~4s, not 1s, with `err=signal: killed` and `ctx.Err()=context deadline exceeded` — the ceiling fired at 1s, control came back only when the descendant closed the pipe. Redirecting the descendants' stdout/stderr away from the pipe (`… >/dev/null 2>&1`) returns at 1.00s. That is the whole mechanism: the ceiling cuts the shell but not a descendant that still holds the pipe. ### The fix Per-command caps, the remedy the issue itself points to ("every read carries its own cap") and the shape already used elsewhere in the suite. The bucket step now defines a `k()` wrapper (`timeout 30 kubectl`) and an `mc()` wrapper (`timeout 15 mc`) for the reads, and calls `timeout 45 mc cp` directly for the two PUTs; a hung read is killed at its own budget, releasing the inherited pipe promptly, so even when the op ceiling fires the residual is one inner cap rather than `mc`'s full retry exhaustion. The terminal precedents for these budgets are the redis suite's terminal `k()` (30s, `redis/chainsaw-test.yaml:298`) and the sibling `verify-every-bucket-takes-a-write` step's terminal `mc cp` (45s, `:390`, sized for a volume grow); the 15s cap on `mc` in `hack/e2e-chainsaw/_lib/backup-access-preflight.sh` is not a precedent, because it sits inside a `while :;` retry loop where a 124 costs one attempt, whereas here under `set -e` a 124 ends the step. The inner budgets — 2×30 for the two `k` reads + 2×45 for the two `mc cp` PUTs + at most 7×15 for the other `mc` calls (eight other call sites, but the two cleanup `mc rm` are mutually exclusive) + the 30s port-forward readiness wait = at most 285s — sit under the 5m op ceiling, which stays as a backstop, per the "op above the sum of inner budgets" rule in `docs/agents/e2e-testing.md`. The long-lived `kubectl port-forward` keeps no cap by design (it must persist for the whole step, and its stdout/stderr are already redirected to `/dev/null`, so it cannot hold the step's pipe). The `mc()` wrapper is non-recursive because `timeout` execs its argument through a PATH lookup that bypasses shell functions — verified in `dash`, `bash` and `sh`. The `catch` block's diagnostic read gets the same treatment — `timeout 30 kubectl` under a 1m op `timeout` — for the same pipe reason. ### The readonly deny probe fails on a timeout The readonly-upload probe used to branch on `mc` directly (`if mc cp …; then`), which suspends `set -e`, so a `timeout` there (exit 124) was indistinguishable from the access-denied non-zero the probe expects: a 45s hang on the deny path would exit 124, take the false branch, reach cleanup and exit 0 — a long hang turned green, the exact window this PR exists to bound. This is the first `ro-user` write in the step, so no earlier call proved that path healthy. The probe now captures the status: exit 0 is the readonly regression, 124 is a timed-out probe, both fail; any other non-zero is the expected denial. ### No unit surface; why the evidence is the proof This is an e2e-config and doc-correctness change. The embedded Chainsaw script is not extracted into a unit harness anywhere in the tree, so there is no red-to-green unit test to add without inventing a bespoke script-linter (which would be a new checker standing in for the fix). The regression is instead pinned by the mechanism proof above: the source citations show the ceiling cannot cut a descendant that holds the pipe, and the caps remove the only way a descendant outlives its budget. The deny-probe status handling and the pipe-vs-file secret reads were exercised against stub exit codes (0 → regression-fail, 124 → timeout-fail, other non-zero → expected denial) and the whole script passes `sh -n` and `bash -n`. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. The change touches only `hack/e2e-chainsaw/` suite content and `docs/agents/e2e-testing.md`; it moves or renames nothing under `hack/`, and touches no node-prerequisite or `package.mk` path. No downstream repository is reached. - [x] No downstream repository is affected by this change ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved end-to-end S3 access verification so stalled Kubernetes API and S3 operations fail promptly. - Correctly distinguishes timed-out access checks from expected permission denials. - Preserves partial command output and allows subsequent test steps to continue after individual timeouts. - Applies separate time limits to bucket reads, uploads, and access checks for more predictable test behavior. - **Documentation** - Clarified operation timeout behavior and recommended per-command limits within the overall operation limit. - Documented timeout handling for orphaned processes and updated worst-case timing guidance. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
b96c822bde |
docs(agents): give the HR-Ready budget derivation its own section
The checklist entry had grown into a derivation and buried the rule it states. The rule stays in the checklist and the derivation moves to a section of its own. The derivation under-budgeted in two ways. It stated the 10m install timeout as universal, while releases an app chart renders itself take their template's spec.install.timeout, then its spec.timeout, or helm-controller's 5m where neither is set. And it restated which kinds kstatus gates on, which is easy to get wrong, so it now points at the upstream table instead. The harbor comments now name the floor each of their asserts actually sits against, and the opensearch comment stops citing a 5-6m standard the checklist no longer states. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Timofei Larkin
|
a3911811c7 |
feat(api): take Application name limits from the application's schema
The API server capped Kubernetes and Kafka names by knowing what those two charts render: worker pool releases under a 17-character prefix, controller pods with a 13-character suffix. That is the charts' knowledge sitting in the platform, and it does not generalise. The next application with a sub-resource budget needs another branch on Kind, and an application served from outside this repository cannot get one at all. An application now declares the limits on its own name in its schema, under x-cozystack-name, and the server applies whatever it finds there for whatever kind the ApplicationDefinition describes. A declaration only tightens: prefix + name must still fit the 53-character Helm release name limit. A rejected name is answered with the chart's own explanation of the limit. The schema is untrusted input, and definitions from other repositories are written by hand. A declaration that cannot be enforced as written, whether an unknown keyword, a wrong type, a limit no name satisfies or a pattern Go cannot compile, refuses creates of that kind with a configuration error. Dropping it would admit names the chart cannot render; applying it would reject every name and blame the user. An upgrade brings cozystack-api up before the definitions it serves are re-rendered, so for a while the new server can read a Kubernetes or Kafka definition that predates its declaration. The two old caps stay as a floor for exactly that case, a definition that declares nothing, until no supported upgrade starts from one. Assisted-by: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Aleksei Sviridkin
|
1071802e3f |
test(e2e): pin the release-history read against the upstream type (#3607)
## What this PR does
`hack/remediation-guard.bats` carried a test called "status.history
extraction pins HR v2 status.history shape". It wrote a HelmRelease
document containing the field names it expected, then read that same
document back with the same jsonpath through `yq`. What it established
is that `yq` can read a fixture the test itself composed.
Its own comment named the failure it was there to catch: a future Flux
release renames the field, the jsonpath matches nothing, the guard
reports no remediation cycle, and real cycles pass the e2e assertion. A
rename moves neither the fixture nor the expression, so the test stays
green through exactly that event. The comment also named where the truth
lives, `github.com/fluxcd/helm-controller/api/v2`, and then pinned a
copy of the author's expectations instead.
`internal/fluxcontract` supplies neither half. The expression is read
out of `hack/e2e-chainsaw/_lib/remediation-guard.sh` at test time, so it
is the one that ships rather than a copy. The object is a
`helmv2.HelmRelease` built from the upstream type at the version
`go.mod` holds, serialized and matched through
`k8s.io/client-go/util/jsonpath`, which is the engine kubectl's `-o
jsonpath` uses. A renamed or retyped field is then a compile error or a
failed match, and a guard that stops reading history fails the lookup
with a message saying so.
That is one test. Two more exist so that it reads the expression kubectl
actually receives, because the expression now lives in a named
assignment the test sources and the caller expands: between the value
checked here and the value that runs there is a shell variable, and each
step of that indirection can fail while the suite stays green. The first
follows the caller's own call: it reads the local handed to
`helmrelease_has_remediation_cycle`, requires that local to be assigned
exactly once, and requires that assignment to take the expansion of the
shared name rather than the bare name, which matters because kubectl
echoes a brace-less template back verbatim, so a read that lost its `$`
returns its own text, clears the script's empty-history check, and
matches no status. Starting from the call rather than from the text is
what makes the pin describe the read that runs: asking only whether a
well formed read exists somewhere in the file is satisfied by a second
assignment overwriting the same local, and by a read filling a local the
guard never consumes. The second holds that no shell library but the one
that owns the name assigns it, which does not follow from the first: an
assignment beside the read satisfies the first and still replaces the
value after this file has approved the one the library exported. It
covers every library in that directory rather than the caller alone,
because the caller sources three of them and the owning one first, so an
assignment in any of the others lands later and wins.
Those two match text with a pattern, and each carries both halves of its
contract. A refuse table lists the forms the pattern must reject, and an
accept table lists the forms it must keep accepting, one case per
freedom the code claims: for the call, the `if` prefix, the negation, an
expansion with or without braces; for the read, the spelling of `-o`,
the quoting of the substitution, an expansion with or without braces,
and the local's name, which stays free because the call and the read are
renamed together. Without the accept side those freedoms survive only
until someone tightens the pattern, and nothing reddens when they go.
Two boundaries, named rather than left to be found. The pin holds the
shape of the read, not the vocabulary of the answer: the guard greps for
`failed` and `uninstalled`, and the test supplies those two strings
itself, so a release status renamed upstream leaves the jsonpath
returning strings, the test green because it wrote them, and the shell
grep silently matching nothing. Those values come from Helm's release
status rather than the Flux type, so pinning them reaches past what this
change binds, and there is more than one way to do it. Second, a literal
history read that fills a local the guard never consumes is not caught,
and on its own that is harmless: the guard still runs the shared
expression, and the moment it stops doing so the read filling the
consumed local no longer matches and the caller test fails.
The caller-side half is a heuristic and is meant to be read as one. It
decides whether a piece of text is the read by matching it, and a
matcher that reasons about shell is wrong at some spelling by
construction, which is the same objection the file raises against
reading the library's assignment as text instead of sourcing it.
Sourcing is available for the library and not for the caller, so what is
left is to make the imprecision run one way: the count is of
occurrences, so an extra one reddens and a missed one cannot pass, and a
form outside the accepted set fails with the pattern printed rather than
quietly stopping asserting.
That direction is the whole argument for keeping it. A false red is a
broken build on a legitimate rewrite, and the failure names what it did
not recognise, so it costs a minute. A silent green is a guard that
asserts nothing for as long as nobody looks, which is the state this
branch exists to end. The ways the caller can break without anyone
noticing are worth that trade. A read that lost its `$` hands kubectl a
template with no braces, which comes back as the variable's own name; a
hand-written path that is valid but names the wrong field comes back as
that field's values. Both are non-empty, so the script's own
empty-history backstop stays quiet and the cycle grep matches nothing. A
deleted call is quieter still, since there is then no assertion to fail.
None of the three is visible to the library-side pin, which judges the
value the library hands out and never learns whether it reached kubectl.
What does fail loudly is an expression kubectl cannot use at all, and
that is the one case needing no pin.
The shape the fixed test now has already exists in this tree, in
`hack/seaweedfs-naming-audit.bats`, under "the audit's reconstruction
agrees with the chart helper it mirrors". That one also writes a
heredoc, which is what makes the pair worth comparing: it copies the
real `_naming.tpl` out of the chart, renders it through `helm`, and
checks the audit script's output against the render. The heredoc is a
harness for calling the real helper, not a record of what the author
expected it to say, and the two sides have different authors. A test
that writes both sides proves only that it is self-consistent.
Nothing is lost by the deletion. The removed test's tail fed extracted
statuses into `helmrelease_has_remediation_cycle`, and the seven tests
remaining in the bats file `hack/remediation-guard.bats` still cover
that, including `uninstalled then deployed still returns detected`. The
extraction half is now pinned more strictly than before: the old test
asserted only that the output was non-empty and contained one line, the
new one asserts the exact two bare tokens, so dropping the `{"\n"}`
separator also goes red.
CI runs it. `make test-controllers` is its own step in
`.github/workflows/pull-requests.yaml` and runs `go test ./internal/...
-count=1`, which picks the new package up without registration.
### Screenshots
Not a UI change.
### Downstream repositories
Walked the trigger map against the diff. It adds a Go test package under
`internal/`, deletes one test from `hack/remediation-guard.bats`, adds a
variable to `hack/e2e-chainsaw/_lib/remediation-guard.sh`, points the
kubectl invocation in `hack/e2e-chainsaw/_lib/run-kubernetes.sh` at that
variable, adds a sentence to `docs/agents/e2e-testing.md`, and records
`make test-controllers` in `docs/agents/overview.md`. No package is
added, renamed or removed, and nothing touches a `values.schema.json`, a
version enum, a `values.yaml` default, an `ApplicationDefinition`, a
CRD, a namespace, an annotation or a telemetry metric.
`hack/e2e-prepare-cluster.bats` is untouched, so the node contract that
`talm` and `ansible-cozystack` restate by hand does not move.
The closest call is `ccp`, whose skills anchor on files under `hack/`.
Its trigger is a move, a rename, or a make target that does something
different. This changes the contents of three files, not their names,
and no target behaves differently; `hack/package.mk` and
`hack/common-envs.mk` are untouched.
- [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
NONE
```
|
||
|
Aleksei Sviridkin
|
97cb2781db |
test(e2e): pin the release-history read against the upstream type
The test that claimed to pin the HelmRelease v2 history shape wrote a HelmRelease document containing the field names it expected, then read that same document back with the same jsonpath. It established that yq can read a fixture the test itself composed. Its own comment described the failure it was meant to catch: a future Flux release renames the field, the jsonpath matches nothing, the guard reports no remediation cycle, and real cycles pass the e2e assertion. A rename moves neither the fixture nor the expression, so the test stays green through exactly that event. The comment also named where the truth lives, github.com/fluxcd/helm-controller/api/v2, and pinned a copy of the author's expectations instead. Replace it with a Go test that supplies neither half. The expression is read out of the shell library that ships it, and the object is built from the upstream type at the version go.mod holds, then serialized and matched the way kubectl does. A renamed or retyped field is a compile error or a failed match; a guard that stops reading history fails the lookup with a message saying so. Both directions were confirmed by mutation. Locating the expression by searching the file is not enough on its own. Inline in a long script, locating it means deciding which occurrences are code and which are prose, and every way of getting that wrong fails green: an example quoted near a broken read satisfies the pin while the guard reads nothing. So the expression moves into a named assignment in remediation-guard.sh, which the guard already sources, and the test sources that library and takes the value the shell is left holding. Deciding which text is the assignment is then not the test's problem: an `export`, an indent, a second assignment later in the file all resolve the way the guard will resolve them, because the thing resolving them is the shell. Sourcing borrows the environment along with the shell, so the snippet unsets the name before it sources. `go test` hands the child its own environment, and a shell that had exported this name would answer in place of a library that stopped assigning it: the value comes back non-empty, the empty check stays quiet, and the pin passes over a guard that reads nothing. That is this file's own subject reaching it through the environment rather than through the source. Moving the expression out of the call site puts a step between what is pinned and what runs, so the call site is checked too, and the check starts from the call rather than from the text: the local the guard is handed must be assigned exactly once, by an anchored assignment whose jsonpath is the expansion of the shared name. That a well-formed read exists somewhere in the file is a weaker claim than it reads as, since a second assignment to the same local, or a read filling another one, satisfies it while the guard runs a literal nothing here pins. The expansion and not the name: kubectl echoes a braceless template back verbatim, so a read that dropped the `$` would return its own text, clear the script's empty-history backstop, and carry no status the cycle check matches. The library is checked by sourcing rather than by reading, so an assignment the shell would run is resolved by the shell instead of by a guess about which text the shell would pick. Each rule is pinned by its own test, so a regression names which one it opened rather than only that extraction broke. How kubectl's printer treats an absent path is its own configuration rather than part of this contract, so it is left unasserted: guessing at it would put the expectation and the fixture back in the same hands. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
cbf297fa65 |
docs(kubernetes): describe the UID System-mode OIDC users carry
The aggregation-layer section said System-mode OIDC users have no UID for the published header to carry, and that mapping a claim for them was somebody else's change. Both are now false: the chart maps the sub claim. An operator upgrading a running cluster needs to know the identity of those users gained a field. Tenant RBAC never matched on it, but audit records and kubectl auth whoami show it, and an in-tenant webhook or extension server that read the empty value now reads the subject. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
4bf95815a8 |
fix(kubernetes): render the aggregation-layer UID header for tenant OIDC (#3539)
## What this PR does Tenant clusters with `spec.oidc.mode: System` or `CustomConfig` authenticate users fine, but a UID does not survive the hop to an aggregated API server. The tenant kube-apiserver publishes `requestheader-uid-headers` into the `extension-apiserver-authentication` ConfigMap only when the flag is set, and the chart never set it. The aggregation layer still sends `X-Remote-Uid`, but the extension server trusts only the header names published in that ConfigMap, so it ignores the one it gets and the call arrives without a UID. The field documentation even told operators to add the flag by hand via `controlPlane.apiServer.extraArgs`. It forwards a UID rather than creating one. ServiceAccount tokens carry one, and for those the aggregation hop starts working. Users authenticated by `spec.oidc.mode: System` still have none, because the generated AuthenticationConfiguration maps `username` and `groups` only; giving them one means mapping a claim, which changes the identity of users who have none today, and is tracked separately. The chart now renders `--requestheader-uid-headers=X-Remote-Uid` on v1.32 and newer, and adds `--feature-gates=RemoteRequestHeaderUID=true` on v1.32, where the gate is still alpha and the apiserver rejects the flag without it. From v1.33 the gate is beta and on by default, so only the header flag is rendered. v1.31 gets nothing: the flag does not exist there, and writing it by hand stops the apiserver from starting. `--emulated-version` in the same passthrough moves that decision, because the gate resolves through the versioned spec at the emulated version. Rendering the gate on every version from v1.32 rather than on v1.32 alone was the alternative, and it would remove the migration step entirely, because an opt-out would then drop two entries instead of one and move the entry count by itself. It loses on what it leaves behind: a redundant gate term on every cluster, which stops the apiserver from starting on the day the gate is retired from the known list. The design also prefers rendering over failing for the same reason the fail branches are narrow, since a cluster stranded on a HelmRelease that no longer converges is a worse outcome than one that never gets the UID fix. The chart reads an operator entry the way the apiserver reads it: header list elements matched whole and untrimmed and folded as `EqualFold` folds them, gate names compared exactly, gate values resolved as `strconv.ParseBool` resolves them, `AllAlpha` and `AllBeta` blankets honoured, an empty value treated as no entry. On v1.32 an entry that says nothing about `RemoteRequestHeaderUID` gets a second one carrying the operator's own terms plus `RemoteRequestHeaderUID=true`. Carrying those terms into the merged copy is what makes it land correctly whether the tenant control plane collapses duplicate flags into the last one or hands both to the apiserver, which merges them in the order it reads them. Tests cover the render matrix over mode, version and operator input: every fail path with its exact message, the ownership and merge paths, duplicate entries in both orders, blanket gates, empty values, and case variants on the header name and on the gate value. The latest-version tenant lane creates its tenant with `spec.oidc.mode: System` and asserts the flag on the rendered KamajiControlPlane, so the tenant apiserver starts with the flag in CI. Nothing asserts the UID arriving at an aggregated apiserver; that needs an extension server inside the tenant and is not in this change. The layer where the count-keyed reset actually bites is the tenant Deployment's arguments, and asserting there needs a live tenant control plane, so it is not in this change either. The rule this follows is that a configuration which renders and boots today keeps rendering. Both flags are new, so an `extraArgs` shape reaching this code was rendering before it existed, and stranding a running tenant on a HelmRelease that no longer converges is a worse outcome than one that never gets the fix. A few shapes do stop rendering, and reaching any of them takes writing `--requestheader-uid-headers` into the passthrough by hand, attempting exactly what the chart now does and getting it wrong; in each the kube-apiserver fails option validation and never starts, so nothing that was working is lost. `docs/oidc-tenant.md` covers these shapes. One known limitation is in there and worth naming here: on v1.33 and newer, adding the documented opt-out is count-neutral in `extraArgs`, and so is removing it again, and on v1.33 and v1.34 adding or removing `--emulated-version=1.31`. The pinned Kamaji build rewrites a running apiserver's arguments only when that count changes and otherwise keeps every flag that has left the list, so either the header flag stays beside a now-disabled gate or the disabled gate stays beside the header flag, and the apiserver refuses to start. Changing the entry count in the same edit is the mitigation, and the fix belongs to the pin rather than to this chart, tracked in #3541. The paths that need no operator edit are clear: turning the feature on for a default tenant moves the count, and a tenant already carrying the opt-out renders byte-identically before and after. Out of scope: `mode: None` still publishes no UID header even though ServiceAccount tokens carry UIDs. That is pre-existing, and widening the condition changes the render for every default tenant. Tracked separately, along with the claim mapping above. Closes #3377 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change Checked the trigger map against the diff: no package added or removed, no schema field, enum or default changed (description text only), no ApplicationDefinition surface touched, no hack/ layout moved. The managed-apps reference page picks up the new field description through release-time README regeneration. ### Release note ```release-note fix(kubernetes): tenant clusters with `spec.oidc.mode` set to `System` or `CustomConfig` now render `--requestheader-uid-headers=X-Remote-Uid` on the tenant kube-apiserver, plus the `RemoteRequestHeaderUID` feature gate on v1.32, so that an aggregated API server can read the caller's UID out of the request headers. Without that flag the tenant apiserver never publishes `requestheader-uid-headers`, and an aggregated API server ignores the UID header even for callers that have one, ServiceAccount tokens among them. It forwards a UID rather than creating one: users authenticated by `spec.oidc.mode: System` still have none, because the generated AuthenticationConfiguration maps `username` and `groups` only, and that is tracked separately. Adding the flag by hand through `controlPlane.apiServer.extraArgs` is no longer necessary, and an entry of your own is respected: the chart steps aside when yours already carries `X-Remote-Uid` or already enables the gate, merges `RemoteRequestHeaderUID=true` into a copy of a v1.32 `--feature-gates` entry that says nothing about the gate, and renders neither flag when you switch the gate off. The render fails where a `--requestheader-uid-headers` list of your own would stop the kube-apiserver from starting anyway: one that omits `X-Remote-Uid`, one with an empty element, and a list of your own while the gate is off. On v1.33 and newer there is a migration step, and it applies in both directions: the pinned Kamaji build rewrites a running apiserver's arguments only when the number of `extraArgs` entries changes, and otherwise keeps every flag that has left the list. Adding the documented opt-out keeps that number the same, so the header flag already on the Deployment stays beside the now-disabled gate. Removing the opt-out later keeps it the same too, so the disabled gate stays on the Deployment beside the header flag the chart renders again, although your spec no longer shows it. Adding or removing `--emulated-version=1.31` has the same shape both ways on v1.33 and v1.34. The apiserver refuses to start in each case, so change the entry count in the same edit. ``` |
||
|
scooby87
|
7d07bbe3ce |
[kafka] Migrate from ZooKeeper to KRaft mode (#2352)
## Summary - Replace ZooKeeper-based Kafka deployments with KRaft (Kafka Raft) metadata mode - Existing ZooKeeper instances migrate automatically and in place via a Helm `pre-install,pre-upgrade` hook Job that drives Strimzi's ZK-to-KRaft migration state machine - New instances deploy directly in KRaft mode with broker and controller `KafkaNodePool`s - The node pools are owned by the migration hook, not by regular chart manifests (see "Why the pools are hook-managed") - Kafka version is user-configurable (v3.9 default, v3.8 available) - Strimzi operator is unchanged in this PR (stays at 0.45.1-rc1); 0.45.x is the last line that supports ZooKeeper→KRaft migration - ZooKeeper removed from the dashboard UI (ApplicationDefinition) ## Migration strategy Both fresh installs and ZooKeeper→KRaft migrations are driven by a single Helm hook Job (`templates/migration-hook.yaml`, `pre-install,pre-upgrade`) that owns the `KafkaNodePool`s. On every install and upgrade the Job: 1. Ensures both node pools exist (idempotent): a broker pool named exactly `kafka` for a migrating cluster — so Strimzi adopts the existing `<cluster>-kafka-N` brokers and their PVCs in place — or `b-<hash>` for a fresh cluster, plus a controller pool `c-<hash>`. The controller pool pins `strimzi.io/next-node-ids` above the broker range so the broker pool keeps node IDs `[0..N-1]` and Strimzi cannot hand the existing broker IDs to the fresh controllers. The shared `kafka` name is claimed atomically (via `kubectl create`) so only one migration per namespace can win it. 2. Inspects the Kafka CR: no CR → fresh install (pools ensured; the main apply then creates the KRaft Kafka CR, nothing to migrate); already `strimzi.io/kraft=enabled` or `status.kafkaMetadataState: KRaft` → done; any other read error → fail closed. Otherwise (a ZooKeeper cluster) it migrates. 3. Annotates `strimzi.io/node-pools=enabled` then `strimzi.io/kraft=migration`, polls `status.kafkaMetadataState` through `ZooKeeper → KRaftMigration → KRaftDualWriting → KRaftPostMigration → PreKRaft → KRaft`, and only once a safe state is reached flips `strimzi.io/kraft=enabled`. If it times out in an intermediate state it aborts without finalising, so Helm fails the upgrade and the next reconcile retries from where it left off. Data is migrated in place: brokers keep their node IDs and reuse their existing PVCs, nothing is copied. Migration is one-way — Strimzi has no supported KRaft→ZooKeeper rollback once complete, so lowering the `version` field or rolling the app back to a pre-KRaft chart is unsupported (take a backup first). ### Why the pools are hook-managed The hook must create the pools and walk the migration state machine *before* the main release apply. A resource a hook creates cannot also be a regular chart manifest across the ZK→KRaft boundary: Helm's main apply finds the pool live in the cluster but absent from the previous (ZooKeeper) release revision and aborts with `original object KafkaNodePool ... not found`, leaving the release `failed` even though the cluster migrated fine (reproduced under server-side apply, the helm-controller default). Managing the pools only from the hook avoids that, so `templates/kafkanodepools.yaml` and the deployed-version gate ConfigMap (`templates/version.yaml`) are removed and the pools are applied idempotently by the Job on every install and upgrade. ## Bug fixes included - ZooKeeper `metricsConfig` referenced wrong ConfigMap key (`kafka-metrics-config.yml` instead of `zookeeper-metrics-config.yml`) - ZooKeeper `storageClass` used `kafka.storageClass` instead of `zookeeper.storageClass` - WorkloadMonitor used `.Values.replicas` instead of `.Values.kafka.replicas` - `validateNameLength` now caps a Kafka app name at the KRaft controller pod-hostname budget (`<release>-c-<hash>-<id>` must fit 63 chars), instead of admitting names that later fail the chart render forever ## What's next A follow-up PR will upgrade Strimzi to 0.51.0 and Kafka to 4.2 once all instances are confirmed migrated to KRaft. That PR is already prepared on branch `KRaft-upgrade-strimzi-051`. ## Test plan - [x] `helm template` renders correctly for new KRaft instances (no ZooKeeper refs) - [x] `helm template` with custom values (1 replica, external, topics, v3.8) - [x] Version validation rejects unsupported versions via JSON schema - [x] helm-unittest: 86/86 across 9 suites - [x] `hack/kafka-migration-hook.bats`: 12/12 — the real hook script executed against a stub kubectl (resolution branches, fail-closed reads, atomic collision guard, finalize guard) - [x] Operator template renders with the unchanged Strimzi 0.45.1-rc1 - [x] Live migration on a disposable cluster under server-side apply: fresh install → healthy KRaft, release `deployed`; in-place migration of a 20000-message ZooKeeper cluster → release `deployed`, broker PVCs adopted in place (same UIDs), and the KRaft data byte-for-byte identical to the pre-migration ZooKeeper data (matching sha256 over every `partition:offset:key:value`) ### Release note ```release-note [kafka] Enable KRaft metadata mode with automatic, in-place ZooKeeper-to-KRaft migration. Kafka version is now configurable (v3.9, v3.8). The Strimzi operator is unchanged (0.45.1-rc1). ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added chart-level Kafka version selector (default v3.9) and KRaft-aware node pool rendering with controller storage sizing. * **Updates** * Default Kafka appVersion updated to 3.9.0; operator and image tag defaults aligned to 0.45.0. * Templates now omit ZooKeeper resources and metrics when KRaft is in use. * **Tests** * New Helm unit tests for Kafka CR, node pools, metrics, topics, dashboard mappings, and workload monitors. * **Documentation** * Values, schema, and README updated to document new version and controller storage options. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
97832f3a40 |
docs(kubernetes): explain the aggregation-layer UID hop for tenant OIDC
A tenant cluster with OIDC on authenticates users fine while every call through an aggregated API server reaches it without the caller's UID, and nothing in the operator guide connected the two. Describe the hop, what the chart now renders per Kubernetes version to make it work, and how a hand-written flag in the extraArgs passthrough interacts with it. The migration step is described in both directions. The pinned tenant control plane rewrites a running apiserver's arguments only when their count changes and otherwise keeps every flag that has left extraArgs, so removing an opt-out or an --emulated-version entry strands a flag exactly as adding one does, and the operator can no longer see it in their own spec. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
9efe78ce44 |
test(e2e): hold the tenant-cluster install wait to an ordering rule (#3802)
A test that creates a tenant `Kubernetes` cluster must not end while that release is still installing. The kind drops the install wait on its main resources, but the chart's blocking post-install hook still holds the install open, so a test that asserts and ends hands the rest of it to teardown. Teardown then reds on a step that tests nothing. Reading the cluster mid-install is fine. Ending mid-install is not. `hack/e2e-chainsaw/_lib/run-kubernetes.sh` already waits for the parent release before its tenant-cluster assertions, and that wait is older than this change. Nothing held it there. This adds the guard that does. The guard reads the script, not Chainsaw fixtures: the cluster comes from a heredoc, so a guard walking `apply.file` would find nothing to check and pass. It requires the CR, then a readiness wait on the parent release, then the assertions the test ends on. The wait has to block, and it has to be unconditional in the body that holds it. One more case holds this script to being the only creator of that kind, since a second one would be governed by no rule here. The ordering is read within one function body, so splitting `run_kubernetes_test` reds the unit lane until the rule is pointed at the new shape. Timeout length stays with review. The floor depends on where the wait sits, and the script text does not carry that. Which spellings each matcher takes, which rewrites it refuses instead of judging, and the blind spots it names are written in the guard's header next to the code, each with its mechanism and whether it fails loudly or quietly. Fixtures mark their own lines, so a probe resolves by name instead of by a line number that drifts. No runtime code here: the guard, its fixtures, and the conventions that state the rule. ```release-note NONE ``` |
||
|
myasnikovdaniil
|
41ef4a6437 |
fix(ci): teach the overlay to repair an unstamped images/*.tag ref (#4362)
## What `hack/overlay-main-images.sh` walks `images/*.tag`, but every branch of its line classifier needs either a YAML key or an `@sha256:`. A `.tag` file holds one bare reference and nothing else, so until a build stamps it the line has neither, the drift guard reads it as a non-ref change, and the committed reference survives. For a package onboarded with the `:v0.0.0` placeholder this repo already uses for a new image, that surviving reference is one no registry serves. The overlay is the only thing that would replace it for a PR that does not rebuild the package, so once such a package is on main every full-suite run that skips its build keeps the placeholder. The same placeholder inside a `values.yaml` repairs itself, because there it sits under an `image:` key the classifier recognises. The difference is the file shape, not the placeholder. ## Reproduction A fixture holding a committed bare tag against an artifact copy carrying a stamped one: ``` $ cd fixture/fakerepo && sh ../../hack/overlay-main-images.sh ../mainpkgs '[]' '' overlay: packages/system/x/values.yaml -> current-main drift (non-ref change) in packages/apps/x/images/x.tag -> keeping committed ref ``` Both files differ only in their reference. The `values.yaml` one is repaired; the `.tag` one is not. Adding any digest to the same placeholder makes it pass, which is what narrows this to the classifier rather than the file. ## Why it has not bitten yet 17 of the 19 `.tag` files in the tree carry `@sha256:` and pass on that branch alone. The only other digest-less one is `packages/extra/seaweedfs/images/seaweedfs-cosi-driver.tag`, and it survives only because it is third-party: `build-main` never rebuilds it, the artifact copy is byte-identical, and the classifier is never reached. The first digest-less first-party `.tag` is what turns the gap live, and one is coming in #3426. ## The fix `hack/promote-rewrite-tags.sh` already carries a second match branch for exactly this shape, with a comment saying why ("the second branch is what catches a bare `.tag` file, whose content has no YAML key at all"). This gives the overlay the same one, scoped to `*.tag` so an unkeyed line in a `values.yaml` still counts as drift, because only in a `.tag` file is a whole line known to be a reference and nothing else. Two tests in `hack/overlay-main-images_test.bats`: the unstamped placeholder is overlaid, and a `.tag` whose non-reference line differs still counts as drift, so the rule cannot quietly widen into "every line in a `.tag` file is a reference". Removing the fix fails the first one. ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Fixed image overlay handling for `.tag` files containing bare image references, allowing current references to be applied correctly. - Preserved drift detection when `.tag` files also contain unrelated, non-reference changes. - **Documentation** - Clarified how image-reference tooling handles bare references and package-tree processing. - **Tests** - Added coverage for bare-reference overlays and mixed reference/comment changes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Alexey Artamonov
|
ac92b6b95c |
fix(e2e): cap the bucket verify S3 reads so the step honors its timeout
A Chainsaw `script` op runs its shell under exec.CommandContext with stdout and stderr wired to in-process buffers. When the op timeout fires it SIGKILLs the shell process alone, not its descendants, and cmd.Wait() does not return until every process that inherited the pipe fds closes them. An mc that keeps retrying an S3 call after the shell is gone holds the pipe open, so the op runs to mc's own exhaustion instead of stopping at its ceiling. This is why the bucket suite's verify-every-bucket-takes-a-write step ran 606s under a 300s ceiling. verify-s3-access shares that shape, so wrap each S3 and apiserver read in its own `timeout`: a hung read dies at a known budget and the op timeout stays a backstop. The 30s kubectl cap matches the terminal `k` wrapper the redis suite carries. The two `mc cp` PUTs take 45s, the budget verify-every-bucket-takes-a-write already gives a write, because a working PUT can still wait while the master grows a volume for a freshly created bucket; 15s left too little headroom on that grow and could turn a slow-but-healthy write red on an unrelated PR. The 15s cap on the mc reads has no terminal precedent: the backup-access preflight caps `mc cp` at 15s too, but inside a retry loop where a 124 costs one attempt rather than the step, so it is not one. Land each secret's encoded value in a file before base64 -d so a timed-out kubectl read (exit 124) stops the step under set -e rather than masking as an empty-credential mc alias set further down. The readonly-upload deny probe branched on mc directly (if mc cp ...), which suspends set -e, so a timeout there (exit 124) was indistinguishable from the access-denied non-zero the probe expects: a hung deny path would exit 124, take the false branch, and reach a green cleanup, the exact window this change bounds. Capture the status instead: exit 0 is the readonly regression, 124 is a timed-out probe, both fail; any other non-zero is the expected denial. The suite's own catch read (the `kubectl get pods` in namespace cozy-objectstorage-controller) carried neither an op timeout nor an inner cap, so it inherits timeouts.exec (5m) and can hold the pipe the same way; give it a 30s inner cap and a 1m op timeout so the rule the rest of the file follows holds for it too. The .chainsaw.yaml notes still read the op ceiling as a reliable cut, the old SIGKILL as a de-facto cap and a smaller timeout as guaranteeing a kill; correct them, since the shell-only kill plus the pipe wait are why an inner cap on every read is required rather than optional. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Timur Tukaev
|
e32353e839 |
docs(agents): correct the labeler's trigger list (#4090)
## What this PR does The PR Title Auto-Labeling section still lists `edited` among the events `.github/workflows/pr-labeler.yaml` runs on. That trigger was dropped in #3211 — the workflow carries the note at the top of its `on:` block, and the v1.6.0 changelog records the change — so the document and the workflow have disagreed since. I hit it the way the document's intended reader would. Three PRs opened with GitHub's branch-derived titles, corrected afterwards to Conventional Commits, and the labels did not follow: `area/uncategorized` on all three and no `kind/feature`, because nothing re-ran. The second paragraph this PR adds is that consequence, which is the part a contributor actually needs: a corrected title does not re-label until the next push or a reopen, and since the labeler only ever adds, the `area/uncategorized` left by the first title stays until a human removes it. ### Screenshots Not applicable — no UI change. ### Downstream repositories I walked the trigger map in `docs/agents/contributing.md` against this diff. The only file it touches is that document, and the two triggers pointing at it are narrower than this change: `cozystack/community` restates the **commit convention** from it (types, scopes, sign-off, the `Assisted-by: LLM` trailer) and the **PR template and downstream checklist**, and `cozystack/ccp` gates its `package-bump` skill on the same commit convention. Neither restates the labeler's event list, which is what changes here. Nothing else in the map reaches a docs-only change. - [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: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Clarified when pull request title auto-labeling runs. - Documented that title edits do not trigger relabeling. - Explained that existing labels remain until a push or reopen, while additional uncategorized labels require manual removal. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
scooby87
|
6cd78b0e30 |
feat(platform): make the Keycloak realm name configurable (#4237)
## What this PR does Fixes #4216. The Keycloak realm name `cozy` was hardcoded across every SSO component, so operators could not rename the realm without patching vendored templates that `make update` reverts. The kube-apiserver OIDC issuer is already configurable out-of-band via talm `oidcIssuerUrl`, which made the mismatch worse: pointing the apiserver at a custom realm left the dashboard, linstor-gui and the generated kubeconfig on `cozy`, so SSO broke with an issuer mismatch. This adds a single `authentication.oidc.realmName` (default `cozy`) to the platform Package values and threads it through the `_cluster` channel so every consumer resolves the same realm: keycloak-configure (realm, clients and `cozystack-cluster-admin` roles), the dashboard and linstor-gui gatekeepers (issuer/login/logout URLs), the info kubeconfig, and — beyond the components listed in the issue — the Grafana and managed-Kubernetes apiserver OIDC issuers, which carried the same hardcoded `/realms/cozy` in their `_helpers.tpl` and would otherwise have left SSO half-broken. `keycloakInternalUrl` is now derived from `realmName` when left unset, so the realm is named in one place. Its default changed from the literal in-cluster URL to `null` with a documented tri-state: `null` derives `http://keycloak-http.cozy-keycloak.svc:8080/realms/<realmName>`; `""` keeps the existing escape hatch that restores external OIDC discovery; any explicit value is used verbatim. A stock cluster renders the byte-identical `keycloak-internal-url` it did before, so existing clusters are unaffected on upgrade. The Kubernetes CR object names (`keycloakrealm-cozy`, `keycloak-cozy`) and the `realmRef` that clients point at are internal resource names, not the realm's technical name, and stay fixed — the EDP operator resolves clients into the renamed realm through the unchanged `realmRef`. Covered by helm-unittest across all seven affected packages, written test-first (red → green, mutation-verified): default stays `cozy`, a custom realm reaches every URL, the derived internal URL follows `realmName`, the `""` escape hatch survives a custom realm, and a numeric/reserved realm name stays a string. `packages/extra/info` had no unit-test target, so one was added. ### Downstream repositories `cozystack/website` needs a follow-up: `content/en/docs/next/operations/configuration/platform-package.md` is a hand-written table of the `spec.components.platform.values.*` keys and does not yet list `authentication.oidc.realmName`. Leaving the box unticked for a maintainer to action rather than filing a speculative PR. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: document `authentication.oidc.realmName` in `platform-package.md` - [ ] [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: ### Release note ```release-note feat(platform): the Keycloak realm name is now configurable via `authentication.oidc.realmName` (default `cozy`), threaded consistently through the apiserver OIDC issuer, dashboard, linstor-gui, Grafana, the managed-Kubernetes apiserver and the generated kubeconfig. Set it to the same realm as the talm `oidcIssuerUrl`. The `authentication.oidc.keycloakInternalUrl` default changed from the literal in-cluster URL to `null`, which now derives `http://keycloak-http.cozy-keycloak.svc:8080/realms/<realmName>`; clusters that leave it unset render the identical URL as before, and `""` still selects external OIDC discovery. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configurable Keycloak realm support across OIDC integrations, including Kubernetes, dashboards, monitoring, kubeconfig, and authentication services. * The realm defaults to `cozy` and can be customized through platform authentication settings. * Internal Keycloak URLs are derived from the configured realm when not explicitly provided. * Added realm-name validation and support for explicitly disabling derived internal URLs. * **Documentation** * Updated OIDC guidance to describe configurable realm behavior and defaults. * **Tests** * Added coverage for custom realms, defaults, validation, and generated OIDC URLs. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
myasnikovdaniil
|
a1b32f2e5a |
ci(workflows): build Talos only when its sources changed (#4021)
Stacked on #4020, not independently mergeable. Nothing on the same-repo PR path consumes either half of what `build-talos` produces. `image-talos` pushes `${REGISTRY}/talos:pr-N-sha`, which no chart in the tree references and which is not even version-tagged here (`PUBLISH_VERSIONED` and `PUBLISH_FLOATING` are both 0). `image-matchbox` stamps `packages/extra/bootbox/images/matchbox.tag`, and that one line is the whole content of `pr-patch-fragment-talos` - checked against the artifact from a real run - but bootbox is opt-in and the E2E platform Package does not enable it, so no run pulls the image. Unstamped tag leaves installer artifact pointing at released matchbox, which is a valid ref nothing on this path resolves. So the job ran a privileged siderolabs imager build on every PR for two artifacts with no reader. Now it runs when the PR touched `packages/core/talos`, and `finalize` accepts a skipped result as well as a successful one. Without that second half skipping it would block finalize, and therefore e2e, on nearly every PR. Forks keep the unconditional build, and the reason is not about talos. `e2e-fork.yaml` publish job requires at least one OCI archive to exist, an invariant that holds today only because this job always ran, so a fork PR touching nothing under `packages/` would arrive with an empty export and fail a legitimate run. Teaching that guard what `plan` expected to build is work in the fork allowlist path and belongs in its own change. Saving is about 1.5-2 min of critical path. Main point is removing a privileged image build that nobody reads from every pull request. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **CI Improvements** - Pull request validation now runs Talos builds only when relevant changes require them, while retaining full validation for fork-based contributions. - Finalization correctly handles skipped Talos builds and preserves generated boot artifacts when builds succeed. - Changed-file detection now accounts for renames, deletions, and shared build inputs. - **Documentation** - Updated testing and build guidance to reflect the current validation behavior. - **Tests** - Added comprehensive coverage for build classification, workflow outputs, skipped jobs, and artifact handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
1118b340a7 |
fix(ci): teach the overlay to repair an unstamped images/*.tag ref
hack/overlay-main-images.sh walks images/*.tag, but every branch of its line classifier needs either a YAML key or an @sha256:. A .tag file holds one bare reference and nothing else, so until a build stamps it the line has neither, the drift guard reads it as a non-ref change, and the committed reference survives. For a package onboarded with the :v0.0.0 placeholder this repo already uses for a new image, that surviving reference is one no registry serves. The overlay is the only thing that would replace it for a PR that does not rebuild the package, so once such a package is on main every full-suite run that skips its build keeps the placeholder. The same placeholder inside a values.yaml repairs itself, because there it sits under an `image:` key the classifier recognises: the difference is the file shape, not the placeholder. The gap is latent rather than new. 17 of the 19 .tag files in the tree carry @sha256: and pass on that branch alone; the only other digest-less one, extra/seaweedfs/images/seaweedfs-cosi-driver.tag, is third-party, so build-main never rebuilds it, the artifact copy is byte-identical and the classifier is never reached. The first digest-less first-party .tag is what turns it live. hack/promote-rewrite-tags.sh already carries a second match branch for this shape and states why. This is that branch, scoped to *.tag so an unkeyed line in a values.yaml still counts as drift. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Alexey Artamonov
|
ff637ed9a2 |
fix(platform): validate the realm-name charset and regenerate kubernetes artifacts
The realm-name guard checked only the type and echoed the parsed value back, so its hint told an operator who wrote `on` to set realmName: "true" — the token YAML had already discarded. Drop the echo and add a slug-charset check: the name is spliced verbatim into URL path segments and a shell token, so a "/" or a space must be rejected at the source rather than split a URL or the hook script downstream. The description edit in the previous commit was hand-applied to the generated kubernetes README and never reached the Go types, leaving pre-commit red; regenerate both with cozyvalues-gen. Finish the oidc-tenant System-mode prose that still read cozy as a fixed name. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
80d32b3ab7 |
fix(keycloak): complete realm-name coverage and guard non-string names
The enable-update-email hook Job in keycloak-configure still pinned REALM=cozy, so on a custom realmName it waited on a realm the chart never provisions and the cold install timed out — the one seventh consumer left behind, and the only value nobody could set without breaking the install. A bare-integer or YAML-reserved realm name reached the printf %s that builds the derived keycloakInternalUrl as a Go int, rendering a "%!s(...)" format error into the URL; coerce the name to a string once at the source. Document that renaming the realm on a running cluster provisions a fresh empty realm and orphans the accounts in the old one, and note in the OIDC tenant and grafana guides that the realm name is now the configurable default. info/Makefile carried NAME=etcd, so cozyhr drove the wrong release from that package; correct it while touching the file. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
8ef230f0bf |
Merge remote-tracking branch 'origin/main' into kafka-multi-instance
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> # Conflicts: # docs/storage-immutability.md # packages/apps/kafka/tests/topics_test.yaml # packages/system/kafka-rd/cozyrds/kafka.yaml |
||
|
Timofei Larkin
|
c398e0e116 |
Merge branch 'main' into chore/drop-http-cache
main took migration 57 and moved migrations.targetVersion to 58 while this branch was open, so that line is the one conflict: 57 at the merge base, 58 on main, 59 here. Resolved as 59, which is what this branch's migration 58 requires, since run-migrations.sh loops up to targetVersion - 1. Merged rather than rebased because the branch carries commits that are not mine to rewrite. Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
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> |
||
|
Alexey Artamonov
|
710cdd18bb |
fix(kafka): address KRaft-migration review — broker pool adoption, hook safety, tests
Addresses the branch review on the ZK->KRaft migration path. CRITICAL — broker node pool adoption. Strimzi derives broker/PVC names as <cluster>-<pool>-<id>; a pre-node-pool ZooKeeper cluster has brokers <cluster>-kafka-N, so the adopting broker pool must be named exactly "kafka" or the operator provisions fresh empty brokers and orphans the tenant's data. A new kafka.brokerPoolName helper resolves the broker pool to "kafka" for an existing (migrating) cluster and keeps the release-scoped "<release>-broker" for fresh installs, so multiple Kafka instances still coexist in one namespace. The migration hook creates the pool as "kafka" and fails closed if a "kafka" pool already belongs to another cluster (node pool names are namespace-unique, so in-place migration of co-namespaced ZK clusters is unsupported). Wired through kafkanodepools.yaml, workloadmonitor.yaml and the hook. MAJOR — migration robustness. - Raise the HelmRelease timeout: the migration Job can run tens of minutes but the release timeout defaults to 10m, so helm-controller aborted and thrashed the pre-upgrade hook. Add release.cozystack.io/helm-install-timeout=45m on the kafka ApplicationDefinition, above the Job's activeDeadlineSeconds (2400s). - Idempotent migration: do not re-issue kraft=migration once the cluster is past the ZooKeeper phase (an invalid backward transition Strimzi rejects on retry). - Fail closed on the decision gates: distinguish a genuine NotFound (skip) from any other kubectl error (abort), and never treat an empty kafkaMetadataState on an existing CR as "migrated". MINOR — version downgrade documented as unsupported; integer (not lexical) compare in the version gate; deleteClaim/PVC-retention and the one-per-namespace migration limitation documented in the README; docs/storage-immutability.md updated for KRaft. Tests — helm-unittest coverage for the broker pool name, the hook guards and the idempotent/fail-closed logic; a bats check for the helm-install-timeout budget ordering; and a Chainsaw ZK->KRaft migration suite that seeds a ZooKeeper cluster, drives the migration, and asserts the original broker PVC survives (not recreated) and the cluster reaches KRaft. The migration sequence and the Chainsaw assertions were validated against a live cluster. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
a2db2aeb8c |
fix(seaweedfs): raise the number of buckets a volume server can serve
A volume server started with -max 0 takes a slot count derived from disk rather than a fixed one, replication 001 spends a slot on each of the two servers per logical volume, and a collection claims COPY_2 volumes whenever it grows. So the bucket ceiling is set by the product of the size limit and COPY_2, and moving either number alone moves it. The mechanism is upstream in MaybeAdjustVolumeMax (weed/storage/store.go) and findVolumeCount (weed/topology/volume_growth.go). Do not divide these out of the nominal PVC size, because the count follows the space the filesystem makes available and not the volume's advertised size: the shipped 10Gi volume PVC was seen to give nine slots per server at the upstream 1000 and 19 at 500, which puts 400 at 23 to 24 by the same division, or eight collections against two. The span is the observation's own, since the free space behind 19 at 500 fits more than one filesystem geometry and those divide differently at 400; eight either way, because a collection that cannot get a whole batch of COPY_2 keeps the volumes it did get and writes into them. The division itself is measured: on a 1 GB image the master reports max as the filesystem's available space over the limit in every run, 25 slots and nine collections at the 40 and 3 that scale to this pair. Two is what the platform's own cozy-backups bucket and a single tenant bucket already use. Past that every S3 PUT to a new bucket times out while the disk still reads as nearly empty, and no COSI object reports anything wrong, because the claim and the access are both granted. 400 and 3 make the budget 1200 MB per collection. Splitting the reduction across both numbers rather than taking it out of the size limit alone keeps the cost off the curve that grows: open descriptors track stored bytes divided by the limit, so a 10 TiB server keeps roughly 52k of them at this limit against 21k upstream, where moving the size limit alone would have put it at 105k. A .dat and an .idx stay open per volume; the .vif beside them is written and closed. The ceiling reached is the same either way. Both figures describe the shipped replicationFactor of 2. The master picks its growth count from the replication the filer asks for, so at 1 and 3 copies it reads COPY_1 and COPY_3 instead and this COPY_2 does not apply; lowering the size limit still raises the ceiling there. The size limit was out of an operator's reach: it lived in the data-plane chart, which is pulled by ExternalArtifact from a component that declares no install section, so a Package-level values override for it is dropped before any merge. The app chart now declares master.volumeSizeLimitMB and sends it on every release, and 400 is set there. The data-plane chart keeps upstream's 1000 as its fallback on purpose. A platform upgrade can re-render that release from its artifact before the parent has sent anything new, and a lower fallback would then reach the master first and drop the limit ahead of an operator's own setting. COPY_2 is sent by nothing and lives in the data-plane chart, so it does land in that window, where a grow costs 3000 MB against the 6000 MB it cost before. The app chart also writes maxVolumes into dataDirs on every release, and because dataDirs is a list Helm replaces the data-plane chart's copy wholesale, so the zero that makes the slot count auto-derived at all comes from there. Losing it drops the server to the subchart's fixed seven without failing any render. The field is bounded at both ends, because either end takes an instance down. Zero is not a smaller limit but an off switch: the volume server leaves its slot calculation untouched at zero, keeps reporting no capacity, and nothing is ever grown. Past 30000 the master does not start at all. The bound sits in the values schema rather than in a template guard, so Helm refuses the value before any template runs. The apps API does not: it reads that schema only for defaults, so it accepts a value out of range, and the release then fails to render. An explicit null gets past both bounds, because Helm drops the key before it validates, and would render an empty -volumeSizeLimitMB= that the master exits on at start, measured on the shipped image, so the template requires the value. No bound against the disk goes with it, though the field invites one. A limit at or above the volume's own size does not break an instance: MaybeAdjustVolumeMax floors the slot count at one, measured on 4.31 with a 255 MiB disk against a 1000 MB limit, which reported Max 1 and served a write it then read back. Such a server is capped at a single slot rather than dead, so refusing it at render would be a capacity policy that stops a working release on every reconcile, not only on the upgrade. The field's description carries the relation instead, where the value is chosen. The growth count is deliberately not exposed beside the limit. The master picks which of its four growth counts to apply from the replication the filer requests, and the filer takes that from replicationFactor, so a field wired to COPY_2 would do nothing at 1 or 3 copies while still appearing to be a setting. A knob that is silently inert for two of the three supported values is worse than no knob. Two further prices. A collection absorbs the budget before it needs another grow, so growth requests arrive five times as often as at the upstream pair. A collection also starts with half as many writable volumes, and a volume server takes the writes into each volume one at a time (syncWrite holds that volume's dataFileAccessLock), so a heavily parallel writer spreads over fewer volumes until the collection grows again; that holds for instances kept at 1000 too. And lowering the limit on an instance that already holds data carries the other one: its volumes between the new limit and the old go oversized on the next heartbeat and leave the master's writable set, and each affected collection needs one successful grow before its next write lands. Nothing is lost and the volumes stay readable; the room left inside them for further writes is given up and does not return as capacity. Those grows compete for one pool, and the pool is smaller than it looks. An oversized volume is charged no headroom, while one below the limit is charged all of its unfilled room; a grow then claims a whole COPY_2 batch and takes as much of it as is there, and the volumes it creates are empty and charge their headroom straight back. Measured on a local SeaweedFS 4.31 setup: two oversized collections with two free slots between them, the first to write took both, and the second stayed refused with the disk still reporting a quarter of itself free. Collections that need no grow are not free either, since their volumes below the new limit were created six at a time and each can hold back a whole limit. Also measured there, with the limit and batch scaled down: a server with free space for one batch per collection refused the grow its one affected collection needed, because a second, nearly empty collection held the room, and the same free space without it let the grow through. The minFreeSpacePercent reserve only comes into it on a server whose writable volumes have all passed the limit, where nothing is charged headroom: from 8000 MiB of filesystem upward, a 10Gi PVC measuring 9979, five percent is at least 400 MB, and such a server then refuses a grow only once its disk is down to the reserve. Everywhere else the headroom charge decides, and a refusal can arrive with the disk far from full whatever the reserve. Deleting a collection gives the room back a slot per volume, however little it stored, since the headroom charged to those volumes leaves with them. So, on a variant that runs platform migrations, the upgrade does not change the limit on an existing instance unless its operator already set one, which the previous chart accepted and ignored. Platform migration 57 writes master.volumeSizeLimitMB: 1000 into every existing SeaweedFS instance that has not chosen a limit, and 400 applies only to instances created from here on. It leaves alone only an instance whose master is not an object, which no v1 chart renders, and names that one in its log. The default variant runs no platform migrations, so a cluster there that installed SeaweedFS by hand gets no pin, and the document below gives the same patch to apply before upgrading. A chosen limit is a whole number the new chart accepts; anything else counts as not chosen and is replaced with 1000, which is what the previous chart ran in its place, since it ignored the key and the new chart would refuse to render it. A valid value already set is kept, and since the previous chart ignored it, the upgrade is when it starts to apply; the document below lists the ones that change the limit ahead of it. Raised values are listed as well: existing buckets keep writing, but every volume below the new limit is charged up to it, and on a local SeaweedFS 4.31 setup seven nearly empty volumes left no free slot once the limit went up tenfold, so a new bucket was refused. The value goes into the application's HelmRelease, whose spec.values is the application spec, for an instance created through the API and for one the tenant module renders alike. The tenant module never renders spec.values on that release, and a Helm upgrade leaves a key alone that neither manifest carries: measured with Helm 4.3.0 on client-side, server-side and forced server-side apply, and read in createPatch at the v4.1.1 that helm-controller v1.5.0 runs. Where the migration runs, it is a pre-upgrade hook of the platform release, and the cozystack-packages source every application chart comes from is a manifest of that same release, applied only after its hooks; so no instance meets the new chart unstamped, and a failed migration stops the platform upgrade before that source moves. That holds while the platform source is pinned by digest, as it is out of the box; one that follows a tag, a semver range or a branch lets source-controller move cozystack-packages on its own schedule. Until cozystack-packages moves, each patched release keeps its old chart, which ignores the key, since no v1 release reads it or closes the master object in its schema. An instance kept at 1000 still gains, because at a replication factor of 2 a grow now claims three volumes rather than six, so it has room for roughly twice as many new buckets. Lowering an existing instance is left to its operator, and docs/operations/seaweedfs-lowering-the-volume-size-limit.md is the check for it. It lists each server's volume files and adds one limit for every volume below the limit and a batch for every collection with no volume at or below 90 percent of it, since the master grows such a collection on its own schedule without a write; measured the same way, a collection with every volume in that band and no writes got two new volumes per server within one pass. Where each disk type is served by as many volume servers as the replication factor, as the shipped two at a factor of 2 are, each collection's volumes sit on every server of that type, and that bounds the claim from above without assuming how the collections grew. The master places a new volume on any server of the requested disk type with room rather than where its collection already lives, whichever group or pool that server belongs to, and it reads an empty disk type as hdd; under MultiZone it also grows each zone on its own. So the document keeps zoned instances, and any disk type served by more servers than copies, at 1000. BREAKING CHANGE: on the default variant, which runs no platform migrations, set master.volumeSizeLimitMB to 1000 before upgrading on every SeaweedFS instance that has no valid value and whose master is an object, with the step in docs/operations/seaweedfs-lowering-the-volume-size-limit.md. On the variants that run platform migrations, migration 57 does this. If the step is skipped, every such instance drops to 400 on the upgrade: each of its volumes at or above 400 MB leaves the master's writable set, and a bucket whose volumes have all crossed that line takes no writes until it can grow. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
f9ea0da3de |
feat(backups): Kafka topic-metadata backup via a dedicated strategy driver (#4000)
## What this PR does Adds a dedicated `strategy.backups.cozystack.io/Kafka` Go driver that backs up the **topic metadata** of a Cozystack-managed `Kafka` application and wires it into the platform `cozy-default` flow — the counterpart to the app-specific drivers (CNPG, MariaDB, Altinity, MongoDB). It is a typed variant of the generic Job strategy: the controller renders a one-shot batch Job that talks to the Kafka Admin API, and adds on top an applicationRef Kind gate, a `Ready` precondition on the Strimzi `Kafka` cluster, and a per-run object key via `.BackupName` so distinct runs never overwrite each other. What it captures: every non-internal topic's partition count, replication factor and non-default configs (via `kafka-topics --describe`), stored as one small object per run in the system bucket; restore recreates them with `kafka-topics --create` + `kafka-configs --alter`. Deliberately **out of scope**: message payloads (a volume-snapshot strategy's job), consumer-group offsets (a log position, only meaningful with a byte-identical data restore), ACLs, quotas, and `KafkaUser`s. No PITR. Restored topics are created directly through the Admin API, so on the target they are unmanaged (no `KafkaTopic` CR) — the accepted tradeoff of an Admin-API-only driver. Both restore flows are implemented: **in-place** recreates dropped topics into the source, and **to-copy** applies the source's topics onto a freshly-bootstrapped empty `Kafka` (the "bootstrap a cluster, then apply the metadata state" flow). Contents: - Driver: `api/backups/strategy/v1alpha1/kafka_types.go`, `internal/backupcontroller/kafkastrategy_controller.go` (+ unit tests for its pure helpers), `internal/backupcontroller/kafkatypes/` (Strimzi `Kafka` readiness gate), dispatch cases in both controllers + `supportedBackupStrategyKinds()`, scheme registration, generated CRD + deepcopy. - Chart: `strategy-kafka-default.yaml` (gated on the bucket name), a `cozy-default` binding, read RBAC on `kafka.strimzi.io/kafkas`, and the Strimzi Kafka client image value. - Docs: a Kafka section in `docs/operations/backup-classes.md`. - Example + e2e: `examples/backups/kafka-metadata/` and `hack/e2e-chainsaw/kafka-metadata/` (`kafka-3-metadata-roundtrip`), an honest round-trip that proves a per-run `retention.ms` sentinel survives both restores. The original `packages/apps/kafka` chart is left untouched (it has no legacy backup mechanism). Validated: `go build ./...`, `go vet`, `go test ./internal/backupcontroller/...` (pure helpers plus a reconcile Kind-gate test and the RestoreJob cleanup regression test), `helm unittest` on the package (which now renders `strategy-kafka-default.yaml` and asserts its kind/name/`artifactURITemplate`), the `select-install`/`select-e2e` bats suites, `helm template` of the strategy CR, `bash -n` of the rendered script, and `make generate` (clean). The end-to-end round-trip (`kafka-3-metadata-roundtrip`) is enabled and mapped like the sibling backup suites, so CI builds this branch's controller and runs it live against a fresh sandbox; it has not been merged-and-run before, so that first live run must go green before merge. ### Screenshots N/A — no UI changes. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. This PR adds a new `strategy.backups.cozystack.io/Kafka` CRD, a Go driver, chart wiring, docs and examples. It does not add or rename a `packages/apps`/`packages/extra` package, does not change any `ApplicationDefinition`, `values.schema.json`, version enum, `release.prefix`, output Secret/Service, the `Package`/`Plan`/`RestoreJob` CRDs the provider types by hand, any `hack/*.mk` anchor or make-target behaviour, node prerequisites, labels/annotations, or telemetry metrics. A brand-new `strategy.backups.cozystack.io` CRD travels to the Terraform provider as opaque JSON (it types only `Plan`/`RestoreJob`/`Package`), so it keeps working untouched. None of the downstream triggers match. - [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 feat(backups): add a Kafka topic-metadata backup strategy bound by cozy-default — backs up topic definitions and configs (partitions, replication factor, non-default configs) via the Kafka Admin API, with in-place and to-copy restore; message data, consumer offsets, ACLs, quotas and KafkaUsers are out of scope ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added Kafka topic-metadata backup and restore support, excluding message data. - Supports ad-hoc and scheduled backups, in-place restoration, and restoration to a separate Kafka cluster. - Recreates missing topics and restores compatible configurations while validating partition and replication-factor constraints. - Waits for Kafka readiness and cleans up backup artifacts after completion. - Backup and restore jobs now enforce configurable deadlines to prevent stalled operations. - **Documentation** - Added guidance covering Kafka metadata backups, restoration limitations, and S3 configuration. - **Tests** - Added end-to-end coverage for backup, restoration, validation, and cleanup scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrei Kvapil
|
d7c0332d5e |
chore(dashboard,api): drop the HTTPCache catalog entry and values type
The dashboard no longer registers a brand icon for the kind, and the ServicesTab fixture moves to TCPBalancer, which the catalog still ships. An instance that survives the removal still renders, on the generic sidebar icon. api/apps/v1alpha1/httpcache goes too. types.go is generated by values-gen out of packages/apps/http-cache, which is gone, so the file could never be regenerated, and no Go file in this repository imports it. Surviving instances do not depend on it: the aggregated API serves the schema recorded in the in-cluster ApplicationDefinition. The terraform provider's cozystack_httpcache resource is hand-written and does not import it either; only its per-kind schema-guard test does, and that goes away with the resource. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |