mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 05:25:14 +09:00
61bbf86755f69769ec133fb6305b0ca8e4fa044e
5884
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
61bbf86755 |
fix(kubernetes-nodes): scope the imageFactoryURL guard to the paths that read it
resolveOsImage assigns $factoryURL before it branches, and the builtin arm never reassigns it, so the http(s) pattern ran for every pool including one that clones. A builtin pool builds no source URL at all: its DataVolume carries source.pvc and no source.http, and the reconcile Job takes only the schematic and the version. The value was therefore refused on a path that does not consume it. The operator this landed on is the one the feature exists for. Somebody moving to osImage.builtin to stop depending on the Image Factory is exactly the person likely to blank imageFactoryURL, and the render then failed with a message naming a source URL that path never builds. The field is a bare string in values.schema.json and in the cozyrds openAPISchema -- no pattern, no minLength -- so an empty or malformed one reaches the render rather than being rejected at admission. The guard is unchanged for osImage.factory and for the no-osImage default, which are the two arms that interpolate it. Three cases: a builtin pool renders with the field blanked and with it malformed, and the omitted-osImage default still refuses a malformed one, so neither new case can be read as relaxing the arm a running cluster is already on. Forcing the new condition open reddens the two builtin cases; forcing it shut reddens the factory and default ones. Reviewed as [MINOR] on cozystack/cozystack#3294. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
35d7a3a746 |
fix(kubernetes-nodes): compare frozen StorageClasses for a pool that has cloned
Resolving an empty storageClass closed the corner where a cross-class clone passed unchecked, and opened a worse one: the two sides of the comparison stopped being the same kind of value. The golden's class came from its bound PVC and could not move; the pool's came from today's cluster default and could. An admin moving storageclass.kubernetes.io/is-default-class, which the resolver's own comment calls the normal state midway through a swap, then failed the whole KubernetesNodes release of a pool whose disks were all on the right class, which had changed nothing, and whose storageClass is immutable so it could not answer. A pool that has already cloned now reads its class from its own bound PVC, the same authority the golden side uses. Today's default applies only to a pool with no disk yet, which is the class its first clone will actually get. Gating the guard on the already-cloned flag instead, the way the two guards above it are gated, would have spared the same pool and then let its next worker take the host-assisted copy in silence, which is the transfer this feature exists to remove. The failure message named a cluster default the render had not consulted. It is computed only when a side is empty, and interpolated unconditionally, so a mismatch between two explicit classes reported resolving to . The sentence is now built only when a side really was resolved from the default. The size floor skipped whenever the DataVolume spec omitted a size, and a golden whose spec is silent still has a bound PVC stating the allocated one. The same authority argument as the class, and it makes the gate observable: previously it could be forced either way with no test noticing, which is how it survived a mutation pass that only ever forced conditions false. Six cases, including the reviewer's own reproduction. Three of them pin precedence contracts nothing held before: the golden's declared class and size beat its PVC, and a pool's declared class beats the PVC of its own disks. The PVC is a fallback, never an override. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
ceeec044ab |
fix(kubernetes-nodes): close five findings from the cross-check
The second model read the same diff and reached five defects the single-model pass did not, all in code this branch added. A sibling pool answered for a pool that had never cloned. The already-cloned gate matched worker disks by the name prefix disk-system-<cluster>-<pool>-, and a pool named md0-gpu produces disks carrying md0's prefix. With the golden absent or Failed, md0's guards were suppressed by md0-gpu's disk and md0's new workers pointed at a source that was gone -- silently, which is the outcome the guards exist to prevent. Each cloned disk now names its own pool in an annotation and the match is exact. This only touches the clone path, so no existing worker's template changes. A golden that declares no StorageClass sits on whatever default was in force when CDI bound its PVC, not on today's. Substituting the current default for it could wave through a cross-class copy or refuse a pool that does match. The class now comes from the PVC, which is where the concrete value lives. More than one StorageClass may carry the default annotation at once -- the normal state midway through swapping a cluster's default -- and Kubernetes resolves it by creation time. The helper emitted every match concatenated, producing a class name that does not exist and refusing a pool whose class matched the default actually in force. It now picks the most recently created, as Kubernetes does. A Talos version was accepted without its patch segment. Both regexes took vMAJOR.MINOR, which names an artifact the Image Factory does not publish and an installer tag that does not exist, so the failure landed in the guest instead of at render. And the remedy printed by the cross-class guard could not be followed. The previous commit pointed it at the catalog entry's storageClass; the immutability guard added by this same branch rejects that edit once the golden exists, which is the only state in which this branch runs. The message now names the two moves that work: a new (schematicID, version) entry on the right class with the pool repointed at it, or deleting the golden once nothing clones it. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
3db22e33aa |
docs(kubernetes-nodes): correct two claims the falsification lens executed
Both were introduced by this branch and both are the class of defect this PR has already been reviewed for twice: a description that promises something reality does not. The clone headroom arithmetic counted the golden once per worker in flight. The golden is a single shared PVC and a one-time cost for the cluster; what scales with concurrency is the temporary clone PVC beside each target. The number an operator would have sized against was therefore too large by one golden per concurrent clone. Found by running the sentence against the harness note it was derived from, which lists the golden, a temp clone PVC and the grown target as the peak for one clone in flight rather than as a per-worker multiple. The cross-class guard told the operator to point the pool at the golden's StorageClass. That field carries self == oldSelf in the values schema, so on an existing pool -- the only pool that can reach this guard -- it cannot be set. The message now names the reachable fix, the catalog entry's storageClass together with the platform values key that sets it, and says plainly that the pool-side match is a choice at creation rather than a remedy here. The same defect on the catalog half of this sentence was a review blocker; this is its other half. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
7660b9d0ba |
test(kubernetes-nodes): pin the guard terms a mutation pass found unasserted
Ran every new conditional through deletion -- force the condition false, and for an and-chain drop each conjunct -- then ran the suite against each mutant. A mutant that leaves the suite green is a term nothing asserts, so it can be removed in good faith by the next person to touch the file. Three survived, all in code this branch added: The presence check on each drift comparison in the catalog. Without it a golden whose spec carries no http source, no storage request or no class compares an empty string against the rendered value, reports drift that did not happen, and blocks every later upgrade of the catalog. The (not $poolCloned) conjunct on the Failed golden guard. The suite had a pool whose sibling had cloned and a pool whose own clone was unfinished, but not the case the gate exists for: a golden that went Failed after this pool was already whole. The $workerSC presence term. Nothing covered an empty pool storageClass with no default StorageClass to resolve it against, which is the arm where the comparison is skipped rather than guessed. One mutant still survives in this region, on the gate in front of the second support-matrix call. It is unreachable from any schema-valid render -- the matrix ships one row whose Kubernetes list is a superset of the version enum -- and it is covered structurally instead, by the greps in hack/check-worker-image-catalog-defaults.bats that assert both call sites exist and that the second is fed the resolved version. That is stated in the bats file rather than left to be rediscovered. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
0034da0181 |
fix(kubernetes-nodes): close IvanHunters' fourth review round
Four blockers, plus the immutability finding carried over from the second round and the minors alongside it. The optional-package helper emitted a Package carrying a variant and nothing else, and package_reconciler takes a component's HelmRelease values from the Package alone, so the worker image catalog documented four operator knobs that no supported path could set. That left the two scenarios the feature exists for -- air-gapped and non-replicated storage -- with a render guard naming a fix the operator could not apply. The helper now takes the same components argument its non-optional twin already had, iaas.yaml forwards a new platform-level kubernetesWorkerImage block, and the nine other optional packages get the same path. The transient headroom a clone needs was written down only in the CI harness. It is a property of the feature, so it belongs in diskSize and in the catalog's storage description, with the arithmetic spelled out. The catalog told operators to set nodeGroups.<name> on a Kubernetes CR, inert since the Phase 2 split, one paragraph above the query that gets it right. talos.version promised the matrix covers every pairing; the helper's own comment says the opposite three paragraphs above the code, and cozyvalues-gen copies the optimistic half into the schema, the README and the dashboard. Both now say what the guard does. An entry whose golden exists is now enforced immutable rather than merely documented so, because the documented recovery -- delete the golden -- runs into the deletion blast radius the same file warns about. Beside it the catalog refuses a duplicate (schematicID, version) and pattern-checks both fields, so it can no longer create a golden whose name a pool cannot reconstruct. The absence and Failed golden guards now fire only for a pool still waiting for its first disk. A pool that cloned months ago has no stake in the source, and aborting its whole HelmRelease on a periodic reconcile was the blast radius the mid-import case already refuses. An empty storageClass on either side now resolves against the cluster default before the cross-class comparison, which is the one corner that cannot be a clone and was passing unchecked. The image resolution is a named template both nodegroup.yaml and talos-reconcile-job.yaml call, carrying the format checks with it, so the boot disk and the in-guest installer cannot drift apart -- the argument this PR already made for the support matrix. The pool field is renamed image -> osImage. It landed one character from the existing images (chart-internal helper overrides) with an unrelated meaning and the same air-gapped motivation in its description. This is new CR surface that has not shipped, so the rename is free now and a migration later. Also: the schema enum drift guard iterated over a command substitution whose failure looked like an empty result, so a renamed property disarmed it; the e2e golden wait reported nothing when the outer timeout fired before the inner one; and ComputePlane's pool-shape allowlist now records the exclusion of osImage rather than leaving it merely absent. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
8bfa66962f |
test(kubernetes-nodes): move the heredoc test onto the guard that replaced escaping
hack/talos-reconcile-heredoc_test.bats renders the pool with hostile Talos image coordinates and runs the reconcile Job's unquoted heredoc through a real shell, to prove the escaping holds. The render guard added a commit ago refuses those values before they reach the heredoc, so the test could no longer get a Job to extract and failed with "render produced no Job command". The chart's own invariant gives a field two ways to be safe here, escaped or render-validated, and this change moved schematicID and version from the first to the second without moving the test with them. Both arms are now asserted rather than one: a case pins that hostile coordinates are refused at render, and the literal-through-a-real-shell case keeps proving the escaping on installerRepository, which stays free-form because it is not part of the DataVolume name. registryMirrors is untouched. The file header says which field is in which arm, so the next person editing either does not have to derive it. Verified by mutation: disabling the schematicID guard turns the refusal case red. Whole `make unit-tests` green, 1485 bats cases, which is what should have been run before the previous push instead of the suites this branch happened to touch. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
ea30f79c76 |
docs(kubernetes-nodes): correct four claims the code no longer supports
Second review pass, all four are prose that describes something the code beside it does not do. The talos.version description still said the support matrix is evaluated against that value only and that an image.* override is not checked against it. That was true when it was written and stopped being true one commit later, when the matrix moved into a named template and gained its second call site. cozyvalues-gen had propagated the stale sentence into values.schema.json, the README, the Go types and the ApplicationDefinition schema, so the operator and the dashboard both read it. The guard block claimed an empty lookup makes all of its guards no-ops under `helm template`. It does for the phase, StorageClass and size guards, which read fields of a golden that was found. For the absence guard an empty lookup IS the failure condition, so a builtin pool cannot be rendered by anything with no cluster to read, and its unit tests have to mock the golden. Worth knowing before someone runs helm lint against it and files a bug. The e2e golden wait promised a stall would surface "with the factory in the message". `kubectl wait` prints only "timed out waiting for the condition on datavolumes/<name>" on expiry, which names neither the URL nor the importer's error, so the step reported a stall about as legibly as the node-join timeout it exists to replace. It now describes the DataVolume and tails the importer Pod before failing; CDI writes the HTTP error and the source URL into the Running condition message. The catalog said a golden's StorageClass must be snapshot-capable. The platform pins cloneStrategyOverride: csi-clone, which needs no VolumeSnapshotClass, so that was an over-constraint on a value operators are told to change. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
6955a083c7 |
fix(kubernetes-nodes): close three findings from the branch review
Self-review of the two commits above, three blockers. The golden-deletion procedure was wrong for the case it was written for. It told the operator to query live worker DataVolumes and read an empty result as permission to delete. A pool sits at zero workers whenever the autoscaler has nothing to place -- minReplicas: 0 is the default -- and it still pins its golden, so that query answers "empty" for exactly the pool whose next scale-up breaks. The pool declarations are the authority now, with the live disks as the second read; both empty is the go-ahead. Nothing asserted that the e2e suites take the clone path. Dropping the image block from the KubernetesNodes heredoc, or landing it at the wrong indent, puts the pool back on the HTTP import while every suite still passes -- the catalog imports a golden nobody clones. That is the same silent-fallback shape this branch already had to fix once, so it gets a guard, mutation-proven by deleting the block and watching it fail. The catalog suite covered only accepted input. Both `images[]` fields are `required`, and what that refusal buys is the golden's name: an entry missing either renders `talos-worker--<version>` or `talos-worker-<id>-`, and two such entries collide on one DataVolume, so the second replaces the first and every pool pinning it clones the wrong OS. Two cases now assert the refusal. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
f489e78265 |
fix(kubernetes-nodes): close the review findings on the worker image source
Five findings from the #3294 review rounds, each with a guard that fails without the fix. [MAJOR] The Talos<->Kubernetes support matrix keyed only on talos.version, so a pool could keep a supported talos.version and still boot a Talos release outside the kubelet's support window through image.builtin.version or image.factory.version, with no render failure. The matrix moves into a named template in _helpers.tpl and both paths consult it -- the pool default at the top of nodegroup.yaml, the resolved version where the image is resolved. A Talos minor the table does not list still passes, unchanged, because failing closed on a hand-maintained table would block an operator moving to a newer Talos before the table is updated. [MINOR] schematicID, version and imageFactoryURL reach an unquoted YAML scalar in the KubevirtMachineTemplate, and a value carrying a newline or a YAML metacharacter injects keys into the pool's own template. The schema cannot narrow them -- the pinned cozyvalues-gen derives `pattern` from the value type and has no annotation for a custom one -- so they are pattern-checked at render, which is where this chart already validates the kubelet reservation fields against the same hazard. The clone-path DataVolume name is quoted too; the HTTP url deliberately is not, because quoting it changes the KMT content hash and rolls every live worker. [MINOR] A golden was referenced only from inside a VM dataVolumeTemplates source, so nothing told an operator which pools still pinned it -- and a pool on `builtin: {}` does not name it in its own CR either. Every cloned worker disk now annotates its source golden, and the catalog documents the one read that answers "is this golden still in use". [MINOR] An images[] entry is effectively immutable once its golden exists: the DataVolume is named from (schematicID, version) while storage, storageClass and the URL are rendered into a spec that cannot be patched in place. The catalog said the opposite, inviting a `storage` bump. It now says to add a new entry instead. [MAJOR, residual] Nothing tied the parent kubernetes chart's talos defaults to kubernetes-nodes'. Both declare "keep in sync", the pool chart's `make update` copies neither, and the catalog drift guard reads only one of the two files. A one-sided bump now trips three checks instead of none. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
5b673ce7b5 |
feat(kubernetes-nodes): boot worker disks from a shared golden Talos image (CDI clone)
Rebase of #3294 onto main after the Phase 2 KubernetesNodes split (#3315). The worker disk source and the in-guest installer pin move from packages/apps/kubernetes to packages/apps/kubernetes-nodes, where the pool objects now live; the per-node-group `nodeGroups.<name>.image` union becomes the pool-level `image` value of the KubernetesNodes chart. Adds packages/system/kubernetes-worker-image, an opt-in catalog that imports a golden Talos worker image into cozy-public once per (schematicID, version). A pool that sets image.builtin CDI-clones that golden instead of streaming the raw image over HTTP per worker, which is the per-worker Image Factory dependency behind the kubernetes-* node-join flake (#3231). image.factory keeps the HTTP path and lets a pool point at its own mirror/schematic/version. With `image` omitted the render is byte-identical to before -- the stored render snapshot is unchanged, so existing workers do not roll on upgrade. The e2e suites switch to the clone path: the per-worker talos-image-cache mirror and its manifest, helper and tests are removed, the catalog is enabled in the sandbox, and the run waits for the golden to import before creating the tenant. The ghcr.io kubelet-image mirror is untouched and still spliced into both tenant CRs. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
c0ac27c00d |
test(e2e): drop the ghcr.io pull-through mirror
The mirror was added on the hypothesis that tenant Kubernetes workers missed the node-join deadline because pulling ghcr.io/siderolabs/kubelet from public ghcr.io was rate-limited from the CI runner, and that caching those blobs in-sandbox would take that leg out of the budget. Work on #3513 since then has disproved it. The discriminator between a run whose workers join and a run that stalls is the host's kvm_amd vgif flag, not registry egress, and the stall reproduces with the mirror deployed and its warm-up Job complete. The mirror therefore bought nothing, and it was not free. It is a Deployment, a warm-up Job and a CiliumClusterwideNetworkPolicy applied to every e2e cluster before install, a registry:2 image pulled across the same throttled egress it exists to avoid, a tenant CR knob wired through talos_spec_block, and a failure-path collector competing for the node-join diagnostic budget against the guest serial console -- which is the capture that actually answers why a worker did not join. Removed whole rather than left dormant. The durable knob it exercised is the chart's own spec.talos.registryMirrors, which a production operator points at their own registry, so nothing here was a product artifact and a disabled copy would preserve only the hypothesis. talos_spec_block keeps its Talos OS image cache half (#3231). That is a separate mechanism against a separate failure and stays. The node-join deadline sweep in run-kubernetes-node-join_test.bats has its floor lowered from 18 to 15 because the removed collector quoted the deadline in several comments; the floor is there to catch a pattern that has gone stale, not to pin a count. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
67b4e23ca2 |
test(tests): cut the noise in unit and e2e job logs (#3973)
## What this PR does Unit test job log is 238,794 lines and 31MB, and 233,444 of those are `set -x` trace from `hack/cozytest.sh`. The one failing assertion sits at line 238,573. Nothing in that stream is needed there, `run_one` already tees raw output to `$log` and the fail handler dumps all of it for whichever test fails, so dropping the live stream loses output of passing tests only. `COZYTEST_TRACE` defaults to 1, so e2e call sites in `packages/core/testing/Makefile` keep the live stream. For a long e2e suite that stream is the only progress signal, and the only record at all when a step timeout or a dead runner kills the job before the fail handler runs. `bats-unit-tests` sets 0. Running the runner by hand stays verbose, that is the case where a human is actually watching. Second thing, previous-instance log capture now orders by restart recency. Container cap (`COZY_PREVLOG_MAX=12`) is spent in namespace order and "unrelated" meant only "in another namespace", time was never a key. So a container that restarted once during bring-up competed on equal footing with one that crash-looped during the test, and won whenever the pod list reached it first. Two measured e2e jobs spent 1034 and 1545 lines on `virt-api` and `cdi-apiserver`, both restarted once at install time by `Unable to create certwatcher`, hours before any app suite ran. On a live stand 178 containers carry a restart and their timestamps span 24 days, so the cap binds hard enough for order to decide everything. This orders, it does not filter. Install-time restart is the explanation when install is what broke, and the capture has no reference point for "when the test began" that would make a cutoff honest. Namespace stays primary key, because a stale restart in the namespace under test beats a fresh one three namespaces away. So it fixes cap displacement, not raw volume of a single run. `lastState.terminated.finishedAt` joins the row. The guard on it is three deep and the innermost one is load bearing, which I got wrong first time round and review caught: `finishedAt` is a metav1.Time behind `json:",omitempty"`, omitempty cannot omit a struct, and the zero value marshals to `null`. The kubelet does synthesize a terminated state with no time, ContainerStatusUnknown on a pod deleted mid-run, which also bumps restartCount and so clears the restart filter. kubectl renders that leaf as the literal `<no value>`, and `<` sorts above every digit, so under the reverse sort that row landed FIRST and displaced the most recent real restart. Exactly the inversion the ordering exists to prevent, plus a header reading `previous instance ended <no value>`. Fixed at both layers because they fail independently: the template guards the leaf so the string cannot be produced, and the sort normalises any field that is not shaped like a timestamp to empty so the ordering holds even if some other producer emits one. Worth naming how that slipped through. I probed the template against a live cluster, saw no `<no value>`, and read it as the guard being correct. It only meant no container on that cluster was in that state. Reading the field also fixes a latent bug it would otherwise have caused: `read -r ... restarts` folds a trailing field into the last variable, so the dump header would print `restarts=1|2026-08-22T09:06:00Z`. Now it names the time, which is the first thing telling a reader whether the dump predates the test. ### What quiet mode keeps and what it drops Two things are dropped, and the second one came out of review rather than out of me. First, xtrace and stdout of tests that pass. You cannot diff a passing test's trace against the failing one inside the same job log, and a warning printed by a test that passed is gone. Second, a test that is KILLED rather than failed loses its trace as well. The fail handler runs only after the pipeline returns, so a hang reaped by the job timeout never dumps anything, and quiet mode has by then dropped the live stream. Verbose mode never noticed, because the trace was already printed. Measured: verbose leaves the line the test blocked on, quiet leaves the `╭` and nothing else. So this one is a real regression on the unit lane, not a pre-existing gap. An INT/TERM trap is the obvious repair and it does not work. POSIX sh defers a trap until the current foreground command returns, so installing one stops the runner dying on the signal at all: a suite hung on `sleep 30` took 30s to exit with the trap against 2s under the default disposition. GitHub escalates INT, then TERM, then KILL, so the deferred handler gets SIGKILLed before it ever runs, and the grace period goes on waiting for the hang. Built it, measured it, reverted it, and the measurement is in the comment now so the next reader does not repeat it. Making it work needs the test pipeline moved into the background with `wait`, which is the interruptible one, and that is surgery on the path every suite in this repo takes for a partial trace in a rare case. What is left for that case is the escape hatch below, which for this lane costs 48s and no cluster. The same argument is why e2e keeps the stream on: re-running there is an hour and a live cluster, so the record has to be written as it goes. Everything else stays. The `╭`/`╰` pair per test with elapsed time, so which tests ran, in what order and how long each took is all still there. Full trace, stdout and stderr of the failing test, same bytes as before, because `run_one` tees to `$log` and the fail handler dumps it, and that path is untouched. Anything printed outside `run_one` as well, file level code, `.bats` parse errors, and the `--- running <file> ---` markers. A hang still shows up as `╭` with no matching `╰`. And `make unit-tests` stops at the first failing test anyway, so there was never more than one dump to read. Measured on a suite that is actually red (`hack/ghcr-mirror_test.bats` fails on my machine because it reads a live cluster): verbose 1472 lines, quiet 63 lines, and the post-failure dump is 23 lines and byte-identical in both modes. Getting the trace back, both paths checked: ``` COZYTEST_TRACE=1 make unit-tests # env prefix, 29200 trace lines make unit-tests COZYTEST_TRACE=1 # make cmdline, same hack/cozytest.sh hack/foo.bats # by hand, verbose by default ``` This lane needs no cluster and runs locally in 48s, so anyone who wants the trace just reruns it. That is exactly why the same argument does not extend to e2e, and why the default stays 1 there. Nothing else consumed that stream. Unit lane stdout goes only to the job log, `cozyreport.sh` folds in crust-gather snapshots and the trap writes those to disk independently, and the unit lane produces none of them. ### Verification Measured on this PR's own run: unit job went from 238,794 lines and 31MB with 233,444 trace lines, to 5,720 lines and 580KB with 0 trace lines and 1548 tests OK. Those two totals are not strictly like for like, the old run was red and this one is green, so the honest comparator is 233,444 trace lines down to 0. Quiet output is byte-for-byte the non-trace subset of verbose, and the fail handler dump is unaffected (checked with a deliberately failing suite). Verbose output unchanged against main, byte-identical sorted on `hack/bats-no-exit-trap.bats`. Other suites are not self-stable run to run (temp names, epochs, stdout/stderr interleaving) so that is the ceiling on that check, not a finding. Eight tests on the capture and seven on the switch. Every fix is mutation proven: reverting the template leaf guard, removing the sort key normalisation, dropping the read variable, dropping `-s`, silencing an e2e recipe, removing the fail handler dump, flipping the default, and making the switch a no-op are each caught. Three of those guards did not bite when first written, which review also found: both stability fixtures were pre-sorted ascending, which is what sort's last-resort whole-line compare reproduces with no `-s` at all, and no test fed a six-field row through the whole script, so deleting the read variable left the suite green at 39/39. Whole unit lane, 83 files, 1503 passed and 1 failed. The failure is `hack/ghcr-mirror_test.bats`, it reads a live cluster and fails identically under main's own runner. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map file by file against the diff. Nothing under `hack/` is moved or renamed, no make target changes what it does (`bats-unit-tests` runs the same files to the same verdict, only quieter), and `hack/package.mk`, `hack/common-envs.mk`, `hack/update-crd.sh` and `hack/e2e-prepare-cluster.bats` are untouched, which is what ccp, external-apps-example, talm and ansible-cozystack couple to. No package `values.yaml`, `values.schema.json`, CRD, namespace, label, annotation or metric name is involved. - [x] No downstream repository is affected by this change ### Release note ```release-note test(tests): bats unit test trace is now opt-in via COZYTEST_TRACE and quiet by default in `make unit-tests`, and the e2e previous-instance log capture now picks the most recently restarted containers first ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Improvements** - Test runs can suppress live trace output for passing tests while preserving full diagnostics for failures. - Trace visibility can be customized through the test configuration. - Previous-instance logs are prioritized by the most recent container termination within the preferred namespace. - Captured log headers now show when the previous instance ended. - Undated logs are retained and placed after dated entries. - **Tests** - Added coverage for trace controls, failure handling, and recent-log ordering. <!-- end of auto-generated comment: release notes by coderabbit.ai -->v1.7.0-alpha.1 |
||
|
|
5688b3c66a |
fix(ingress-nginx): re-pin to the segfault-free v1.11.5 rebuild (#4012)
Re-pins the ingress controller to the rebuilt `v1.11.5`. The digest pinned before this segfaults. `pb.c` in lua-protobuf violates strict aliasing and gcc 14 at `-O2` acts on it, so `pb.so` in the previously pinned build makes nginx `cache manager` and `cache loader` children segfault continuously, tens per second and it never stops. Workers are untouched, so the pod stays `Ready` with `restarts=0` and nothing surfaces until master itself dies, which is how it got here unnoticed. Fixed in cozystack/ingress-nginx-with-protobuf-exporter#7, tag `v1.11.5` re-pointed at that merge and both images rebuilt. Verified against the published image, not a local build: * `luajit -e 'require "protoc"'` exits 0, was 139. * 25 second nginx run on a real generated config: 0 `signal 11`, was 105. 0 emerg, all four lua modules load. * `luarocks` and `gcc` are gone from the runtime image now, so `apk` no longer upgrades musl underneath the nginx the base image already built. The exporter is re-pinned too, because one tag builds both images and the rebuild published a new exporter digest. Its source did not change. `release-1.5` and `release-1.6` pin v1.11.2 and were never affected, so nothing to backport. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated container image versions for the ingress controller and protobuf exporter to newer verified builds. * Existing image tags and configuration remain unchanged. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
127cc2ce9a |
fix(ingress-nginx): re-pin to the segfault-free v1.11.5 rebuild
The v1.11.5 tag in cozystack/ingress-nginx-with-protobuf-exporter now points at the merge of its #7, which builds lua-protobuf with -fno-strict-aliasing. The digest pinned here was the build before that, whose pb.so makes nginx's cache manager and cache loader children segfault continuously: tens per second, indefinitely. The workers are unaffected, so the pod stays Ready with no restarts and nothing surfaces until the master itself segfaults, which is how this reached main unnoticed. Verified against the published image rather than a local build. The one-line repro, luajit -e 'require "protoc"', exits 0 where it exited 139, and a 25 second nginx run on a generated ingress-nginx config produces no signal 11 where the pinned build produced 105. The exporter is re-pinned alongside it because one tag builds both images, so the rebuild published a new exporter digest too. Its source did not change. release-1.5 and release-1.6 pin v1.11.2 and were never affected. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
dc2e427262 |
docs(etcd): note that affinity is not applied to existing members (#3964)
## What this PR does Documentation follow-up to #3962. Both reviewers (CodeRabbit and @androndo/@IvanHunters) noted that the new `affinity` value is not applied retroactively: the etcd-operator sets `spec.affinity` only when it creates or replaces a member Pod and never reschedules running members. On a cluster that already has co-located members (the exact failure #3962 targets) the default soft rule — or a newly set hard rule — takes effect only as each member is replaced. This adds that caveat to the `affinity` parameter documentation in `values.yaml` and regenerates `values.schema.json`, `README.md`, and the embedded `openAPISchema` in `etcd-rd`. No template or behaviour change; `make test` passes (20/20). The other inline note (the soft podAntiAffinity default overlaps the soft `topologySpreadConstraints`) was explicitly flagged as "fine to keep as-is", so no code change is made for it. > Note: this PR is stacked on `fix/etcd-soft-antiaffinity` (#3962) and will retarget to `main` once that merges. ### Screenshots n/a — no UI changes. ### Downstream repositories Doc-only change to an app chart (regenerated artifacts). No package added/renamed/removed, `kind`/`plural` unchanged, no installer values/assets/variants touched. No downstream follow-up required. - [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 * **Documentation** * Clarified that etcd affinity rules apply when member Pods are created or replaced. * Noted that running members are not rescheduled retroactively; the rule takes effect as each member is replaced. * Updated the guidance consistently across configuration documentation and schema descriptions. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
c8d073fff7 |
fix(api): drop caller-supplied internal.cozystack.io labels in tenantsecret writes (#3599)
## What this PR does Labels and annotations under `internal.cozystack.io/` belong to the platform. The lineage webhook stamps `internal.cozystack.io/tenantresource` to decide whether a Secret is visible through this API at all, and the cacert controller stamps `internal.cozystack.io/tenant-ca`, which an `ApplicationDefinition` selects on in `spec.secrets` to hand a trust anchor to the tenant. The registry copied every caller-supplied key onto the backing Secret as is, so anyone with write access to `tenantsecrets` could plant either marker on a Secret of their choice, or strip one to take an object out of the platform's view. `tenantToSecret` is the only place the write verbs go through, PATCH included, because `rest.Patcher` is `Getter`+`Updater` and the endpoint installer serves PATCH out of `Update`. It now skips keys with that prefix. The update target starts as a copy of the stored Secret, so a platform key cannot be removed either. The guard is on the whole prefix, so a platform key added later is covered without anyone touching this code again. Reads are unchanged. A tenant can see the platform's verdicts, it just cannot write them. Keys are dropped silently instead of rejected, otherwise a GET, edit and PUT round trip would break `kubectl edit` for no gain. Nothing writes through this API today. Every `tenantsecrets` grant in `cozystack-basics` is `get`/`list`/`watch`, and the console only lists and watches. That read-only RBAC is what keeps the hole inert, and it lives in a different package from the registry that implements the write verbs, which is why I would rather close it here than keep relying on it. Two things I found next to the diff and did not fix. The `Patch` method on this type is never reached: its signature is the `client-go` shape and matches no interface the endpoint installer asserts, so no request lands there. I gave it the same guard instead of leaving the weaker of two doors, but deleting it is probably the better answer and I would rather that call was yours. A repair after the write is weaker than the conversion anyway: the forged key is live between the patch and the repair, and forcing the object back to a pre-patch snapshot drops a key that a controller stamped inside that window. Second one, and the reason none of this is easy to spot: `Update` passes `nil` where the handler hands the storage the current object, so `hasUID` reports false and the patch mechanism takes its create-new-object branch. For JSON Patch, merge patch and strategic merge patch that branch returns `NotFound`, so all three get a 404 against an existing TenantSecret. Apply is the exception. `applyPatcher.createNewObject` builds a fresh object of the kind and applies onto that, so an apply does reach `Update`, carrying only the fields the caller sent, and the merge in `tenantToSecret` is what keeps the stored labels. Passing `secretToTenant(previous)` instead of `nil` is the fix and it belongs in its own change, since it makes the other three patch types live. Tell me if you want issues for either. Closes #3464 ### 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: Walked the trigger map against the diff: two Go files under `pkg/registry/core/tenantsecret/`, no package added, no chart, values schema, CRD, `ApplicationDefinition` field, namespace, `hack/` target or release asset touched. Nothing in the map fires. ### Release note ```release-note fix(api): TenantSecret writes no longer accept caller-supplied `internal.cozystack.io/*` labels and annotations, so a principal with write access to the API can neither plant nor strip the platform markers that decide tenant visibility on the backing Secret. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Strengthened protection of platform-managed metadata during tenant-secret creation and updates. * Prevented tenant-provided platform-owned labels and annotations from overwriting protected values. * Preserved standard metadata while hiding sensitive resource markers. * Improved patch-based update handling and protected metadata consistency across changes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
df20f69e5b |
chore(ci): re-run with diagnostics enabled
Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
ef4f5b45ab |
fix(api): drop caller-supplied internal.cozystack.io keys in tenantsecret writes
Labels and annotations under internal.cozystack.io/ are platform verdicts, not tenant metadata. The lineage admission webhook stamps internal.cozystack.io/tenantresource to decide whether a Secret is visible through this API at all, and the cacert controller stamps internal.cozystack.io/tenant-ca, which an ApplicationDefinition selects on in spec.secrets to promote a trust anchor to the tenant. The registry copied every caller-supplied key onto the backing Secret verbatim, so a principal holding write access to tenantsecrets could plant either one on a Secret of its choosing, or strip one to take an object out of the platform's view. Only read-only tenant RBAC, which lives in a different package, stood between a caller and that path. tenantToSecret is the choke point every write verb goes through, PATCH included: rest.Patcher is Getter+Updater, so the endpoint installer serves PATCH out of Update. It now skips the platform keys, and since the update target starts as a copy of the stored Secret, ignoring the caller there also means a platform key cannot be removed. Guard on the whole prefix rather than on a list of keys, so a platform key added later is protected without anyone remembering to come back here. Delete the Patch method on this type. Its signature matches no interface the endpoint installer asserts, so no request has ever reached it, and as a write path it was strictly weaker than the conversion: the raw patch hit the backing Secret before any guard ran, leaving a forged key live in etcd -- visible to every controller and to List -- until a second write put the platform keys back, or for good when that second write failed. One door is better than two, and the surviving one commits once. Keys stay visible on read. A tenant may see the platform's verdicts, and failing a write that echoes them back would break a read-modify-write round trip for no gain, so they are dropped silently instead of rejected. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
c94a1da5bb |
feat(mongodb): add TLS support via PSMDB operator-managed certificates (#2692)
## What this PR does Enables operator-managed TLS for MongoDB and stops handing tenants any Secret that carries private key material. - Percona Server for MongoDB serves TLS unconditionally (`mode: preferTLS`), so this PR drops the misleading `tls.enabled` toggle rather than pretending the chart controls it. - The tenant no longer receives the operator's CA Secret (`<release>-ca-cert`, which holds `ca.key`) or the leaf (`<release>-ssl`). Both were previously reachable, and either one is a private key in tenant hands, the CA one is a full trust-chain compromise. - The chart declares a `TenantProjection` sentinel naming its CA source Secret (`<release>-ssl`, the operator-managed leaf, which carries `ca.crt`), and the ResourceDefinition selects the resulting key-free `<release>.tenant-ca` projection by the engine-agnostic `internal.cozystack.io/tenant-ca` label. The CA-extraction controller reads the sentinel and publishes the projection holding only `ca.crt`. The tenant gets what it needs to verify the server and nothing else. ### Why the include also carries an exclude list The trust-anchor selector matches by label with no name constraint, so any Secret in the namespace that acquired that label would be promoted with its data copied verbatim. The enumerable dangerous set is therefore excluded explicitly: the CA, both leaves, the rotation snapshots (`-ssl-old`, `-ssl-internal-old`, same server keys as the leaves, and unlike them they carry no owner reference, so an interrupted rotation can leave them behind after the release is gone), the system-user passwords, the at-rest encryption key, the keyfile, the object-storage credentials, and both forms of the operator's internal Secret. That backstop is bounded, and the comment says so: matching is by exact name with no globs, so a Secret a future operator version introduces, or a transient object an issuer creates mid-renewal, stays in scope of the label-only include. The per-user Secrets cannot be enumerated at all, since their names carry the username. The primary defence remains that the controller stamps only its own projection; this closes the enumerable set, not the whole surface. ### Release note ```release-note feat(mongodb): stop exposing TLS private key material to tenants and publish the CA as a key-free trust anchor ``` |
||
|
|
6fdb5e0840 |
docs(etcd): note that affinity is not applied to existing members
The operator applies spec.affinity only when it creates or replaces a member Pod; it does not reschedule running members. Document that on the `affinity` value so operators do not expect the default (or a newly set rule) to redistribute already co-located members until each is replaced. Addresses the non-blocking review notes on #3962. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
|
edd767ea0f |
feat(nats): publish ca.crt-only trust anchor to tenants (#3340)
## What this PR does Converges nats onto the key-free CA trust-anchor contract. A TLS-enabled nats release renders a `TenantProjection` sentinel naming the chart's cert-manager CA Secret as its source, and the nats ResourceDefinition selects the resulting `<release>.tenant-ca` object by label. The projection is built by the CA extraction controller in `internal/controller/cacert`, which parses the source certificate and re-encodes it into a single `ca.crt` key, so a tenant that enables TLS gets something to verify the server against without ever getting a private key. No key-bearing Secret is granted to a tenant, by name or by label. `packages/apps/nats/templates/certmanager.yaml` is not touched. An earlier draft of this change stamped publish labels onto the CA `Certificate`'s `secretTemplate`; the controller that shipped consumes a sentinel object instead, so the chart now declares its source rather than marking it. The functional change is one new template and one `matchLabels` entry in the ResourceDefinition. The sentinel is gated on the chart's effective TLS state, which is tri-state: left unset it follows `external`. That gate is the single divergence from the postgres implementation of the same contract, and it exists because the nats CA is only issued while TLS resolves true. An ungated sentinel would sit `Ready=False/SourceNotFound` forever on every plaintext release. The CA extraction controller is already on main, so this is live on merge rather than dormant until a follow-up lands. The `do-not-merge/hold` label predates that and no longer guards anything. ## Verified against a running cluster The premise this rests on is that cert-manager populates `ca.crt` on the Secret of a `selfSigned`, `isCA: true` Certificate. Nothing in this repository witnesses it: `hack/e2e-chainsaw/` has no nats suite, and nats appears in no e2e workflow, so every unit test here encodes the same assumption the chart encodes and the two cannot catch each other being wrong. I closed it against a live cluster instead, running cert-manager v1.20.2, the version this repo ships, on two independent nats releases: - `nats-<name>-ca`, the chart's CA Secret, holds `ca.crt`, `tls.crt` and `tls.key`. The declared source key exists. - `nats-<name>.tenant-ca`, the projection, holds exactly one key, `ca.crt`. - The projection's SHA-256 certificate fingerprint equals the source's, so it carries that CA and not some other certificate. - The projection carries `internal.cozystack.io/tenant-ca: "true"` and `internal.cozystack.io/tenantresource: "true"`. The second is stamped by the lineage webhook, computed from the ResourceDefinition selector this PR adds, so its presence means the whole chain held. - `kubectl get tenantsecret nats-<name>-ca` returns NotFound. The key-bearing Secret is not served through the tenant API. What is still missing is an automated regression witness for that premise. Adding one means a new chainsaw suite for nats, which has no e2e coverage of any kind today, and that is a larger change than this one. ## What the tests pin 61 helm-unittest cases and 3 bats cases. The sentinel's absence of `ownerReferences` (an owner would short-circuit the lineage walk before it reaches the Flux label the attribution depends on), the length of `spec.projections` as well as its single entry (the controller refuses a multi-entry sentinel, so a second entry is a silent loss of the anchor rather than a leak), both implied legs of the tri-state TLS gate, and `notExists` on either visibility label for both key-bearing Certificates. The bats guard covers the two grant surfaces nothing else asserts on. The ResourceDefinition is rendered, by the `nats-rd` chart inlining `cozyrds/*`, but that chart has neither a tests directory nor the `test` target `hack/helm-unit-tests.sh` looks for, and no `packages/system/*-rd` package does. The dashboard Role is rendered by a chart that has both, and no suite under `packages/apps/nats/tests/` asserts on it. Both are compared whole rather than filtered down to the entries that mention a Secret, because every such filter admits the spelling it did not anticipate. A name whitelist passes an added label selector, and `controller.cert-manager.io/fao` is stamped on both key-bearing Secrets, so a selector on it is a one-line grant of the CA private key. On the Role side, a rule with no `resourceNames` names no Secret, a rule with `resources: ["*"]` names no resource, and a ClusterRole is not a Role, while all three grant read access to the CA key. Comparing the rendered output whole needs no filter and leaves no spelling to find. ## Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file. No trigger fires, and no follow-up PR is owed. `packages/apps/nats/templates/tenant-projection.yaml` is a new template and `packages/apps/nats/tests/` holds chart tests. No package is added, renamed or removed; `values.yaml` and `values.schema.json` are untouched, so neither the hand-written Terraform schema, its version enums, nor its restated defaults have anything to track. `kind`, `plural` and `release.prefix` are unchanged, and no output Secret or Service is renamed, so the provider's concatenated name lookups still resolve. `packages/system/nats-rd/cozyrds/nats.yaml` is edited in place rather than renamed, so the ccp external-app skill's dependency-contract anchor still resolves. The provider offers no typed access to the new projection, which is the position it already holds for every Secret it does not model, and not a break. `packages/apps/nats/README.md` is covered by the one automated path that does exist: `nats` is present in the website `Makefile` `APPS` list, so the docs bot regenerates the managed-apps reference page from this `README.md` on the next stable tag. This is the silent-skip failure mode the trigger map warns about, so I checked the list rather than assuming; an app missing from it gets no page at all, which is what happened to `opensearch`. `hack/check-nats-rd-secrets.bats` is a new file, not a move or a rename, and it changes no make target's contract: `Makefile` globs `hack/*.bats` into `BATS_UNIT_FILES` automatically. The ccp skills anchor on `hack/package.mk` and `hack/common-envs.mk`, neither of which 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: Part of #2814. ### Release note ```release-note feat(nats): expose the CA certificate to tenants as a key-free `<release>.tenant-ca` Secret ``` |
||
|
|
65e700486b |
feat(qdrant): publish ca.crt-only trust anchor to tenants (#3341)
## What this PR does Converges qdrant onto the key-free CA trust-anchor contract (#2814), the same way #3340 does it for nats. The chart declares a `TenantProjection` sentinel naming its cert-manager CA Secret. The CA-extraction controller reads that declaration and publishes a key-free copy, `ca.crt` and nothing else, as `<release>.tenant-ca`. The qdrant ResourceDefinition selects that projection by label so it shows up in the tenant's secret list. No key-bearing Secret gains a tenant-visible label. The sentinel is gated on the chart's TLS condition. Engines whose operator mints a CA unconditionally do not need that, but qdrant only has a CA while TLS is on, and an ungated sentinel would sit `Ready=False`/`SourceNotFound` on every plaintext release. This was held while the CA-extraction controller was still open. That controller landed in #3407, so the sentinel is honoured as soon as this merges and the hold is gone. Tests pin the invariants that otherwise fail silently. The sentinel carries no `ownerReferences`, so the lineage walk falls through to the Flux release label. Neither key-bearing Certificate stamps a tenant-visible label on its Secret. The two objects that decide which Secrets a tenant may actually read, the dashboard Role with its binding and the ResourceDefinition, are pinned whole rather than checked for known-bad names: a rule granting secrets with no `resourceNames` is unrestricted rather than narrow, and a label selector can reach a key-bearing Secret while every name in the file stays correct, so neither adds a name for a probe to notice. Part of #2814. ### Follow-ups What this PR does not do, and where each belongs instead. - nats converges the same contract in #3340 and has no `dashboard-resourcemap_test.yaml`, so the dashboard Role pin belongs there too. Tracked in #3701 along with postgres, which already carries the label selector with no guard behind it. - The qdrant e2e fixture runs plaintext (`hack/e2e-chainsaw/qdrant/qdrant.yaml` sets `external: false`), so nothing in CI walks the path from the sentinel to `tenantsecrets`. postgres has a `verify-tenant-ca-projection` step to copy. - `hack/check-qdrant-rd-secrets.bats` is engine-agnostic apart from two literals, the chart path and the expected Secret set. The third copy of it is the point to parameterise over `packages/system/*-rd/cozyrds/` instead of copying again. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file. Nothing needs a manual follow-up, but not for the reason a values-only walk would give: the website reference page for an app is regenerated from that package's `README.md`, and this PR does change it. `qdrant` is in the app list hardcoded in the website `Makefile`, so the docs bot picks the new section up on the next stable tag and there is nothing to open by hand. The Terraform provider's schema is hand-written against `values.schema.json`, which is untouched, and the provider offers no typed access to the new projection, the same position it holds for every other Secret it does not model. No package is added, renamed or removed, and the `cozyrds` file is edited in place rather than renamed, so the ccp dependency-contract anchor still resolves. User-facing documentation for consuming the trust anchor ships with the chart README, matching #3340. - [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(qdrant): expose the CA certificate to tenants as a key-free `<release>.tenant-ca` Secret ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Tenant environments can securely access the Qdrant TLS CA certificate without receiving the private key. * TLS CA access is automatically enabled when TLS or external access is configured. * **Documentation** * Expanded TLS guidance explains certificate validation and provides commands for retrieving the tenant CA certificate. * **Tests** * Added coverage for TLS projections, secret visibility, dashboard resources, and secure fail-closed secret exposure. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
8d386c2cbc |
docs(mongodb): name the trust anchor as the tenant sees it
Every object this chart creates is named after the Helm release, not after the MongoDB resource, so a MongoDB named catest yields mongodb-catest.tenant-ca. The retrieval commands used a bare <release> placeholder, which reads as the resource name and produces a command that returns nothing. Spell the prefix out in the two commands meant to be copied, and define the placeholder once for the prose. Split the sentinel's shape test in two. The absence of ownerReferences is the invariant — it breaks the walk from the projection back to the release, and the tenant stops receiving the anchor. An explicit namespace or a chart-added label breaks nothing by itself; grouping all three under one name presented them as equally load-bearing. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
04112dacdf |
test(mongodb): stop the tenant-Role guards passing on the wrong document
The assertions withholding the key-bearing TLS Secrets from the tenant Role addressed it as document 0 and, in two cases, asserted only with notContains. Neither survives a rearrangement. notContains passes when its path does not resolve, so a test built from it alone goes green against the wrong document. Reversing the order of the two documents in the template moved the RoleBinding into position 0, and "does not grant the key-bearing TLS secrets when external is enabled" and "does not grant the tenant-ca projection by name" both stayed green while the Role granted <release>-ca-cert by name — the Secret holding the CA private key. Select the Role by kind, and pair every notContains with a positive assertion on the same path so a rule reordering that moves rules[1] reddens too. Indexing survives neither rearrangement; selecting survives the first; the positive anchor catches the second. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
8b6867dc47 |
fix(mongodb): correct the trust-anchor prose and two e2e assertions
The name-clash note claimed the shared TLS helper documents <release>-ca-cert as the conventional key-free anchor. It documents the opposite: the canonical name is <release>.tenant-ca, and it names this engine as the reason, because MongoDB already claims <release>-ca-cert with a key-bearing Secret. There is no clash to warn about and the chart follows the convention rather than deviating from it. The paragraph above already says <release>-ca-cert holds the CA private key, so drop the note instead of inverting it. Guard the ownerReference assertions against an absent list. length() of null is a type error rather than a failed comparison, and the harness returns it as an error, which aborts the poll instead of retrying — so the timeout never applies while the operator is still patching the reference on. Restore the two-variable form of the fingerprint comparison. Inlining both command substitutions into the test swallows a non-zero exit: with no openssl on PATH both sides are empty, they compare equal, and the check passes having compared nothing. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
01cfb5fda0 |
fix(mongodb): correct what a missing extraction controller does
The README said the sentinel would sit inert with nothing else affected. The CRD is a template of the controller's own chart, so a cluster without the controller has no TenantProjection kind and the release fails to render at all. The caveat also guarded a state that cannot occur, since the controller is a mandatory platform component — drop it rather than restate it, matching the postgres chart, and keep the paragraph explaining the choice of source Secret. Enumerate all three key-bearing Secrets in the registry negative. Two of the three were listed, which leaves the third free to acquire the label and leak a key, the exact reasoning the step above it already spells out. Collect TenantProjection status on failure. The controller reports SourceNotFound and its siblings there, and without it a failed projection surfaces only as an absent Secret with no reason given. Rename the source-shape step, which no longer looks at the CA Secret, and cross-reference the two halves of the leaf Secret's name. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
fc57e928e1 |
test(mongodb): drop a document count the sentinel invalidates
The cert-manager exclusion suite justified its `any: true` requirement with a specific number of rendered documents. The argument only needs "more than one"; the number was already tied to one value set and the sentinel adds another document. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
c0b2ddcdf7 |
test(mongodb): assert the published trust anchor end-to-end
The existing steps only assert that key-bearing Secrets are NOT promoted to the tenant, and every one of those is satisfied by an absent label — they hold just as well if nothing was ever published. Add the positive: the projection exists, carries exactly ca.crt, matches the source certificate by fingerprint, and is reachable through the tenantsecrets registry rather than only as a raw Secret. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
29d2955241 |
fix(mongodb): declare the trust anchor with a TenantProjection
A tenant verifying a MongoDB endpoint needs the CA certificate, and every Secret that holds one also holds a private key: the operator's <release>-ca-cert carries the CA key, the leaf <release>-ssl the server key. Granting either would hand over the key along with the anchor. Render a TenantProjection naming the source, so the CA-extraction controller can publish the key-free <release>.tenant-ca, and select that projection by label from the managed-service definition. The sentinel carries no ownerReferences: it is resolved back to its release through the label Flux stamps, and an owner reference short-circuits that walk and silently costs the tenant its anchor. The sentinel is ungated. The operator issues the certificate chain whatever the chart asks for, so the source exists for every release and there is no plaintext leg to skip. The source is <release>-ssl rather than <release>-ca-cert. It is the name this chart itself sets in spec.secrets.ssl, so the reference cannot drift from the CR; the operator writes it on both its cert-manager path and the self-signed fallback it uses when cert-manager is absent, where no <release>-ca-cert exists at all; and it holds the merged old and new CA through a rotation, where <release>-ca-cert holds only the new one and would stop verifying pods still serving the previous leaf. Withhold the key-bearing Secrets from the tenant explicitly as well. The label selector matches on no name, so a single mis-stamp would promote a private key; the exclude list is evaluated first and closes the enumerable set. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
4092f59528 |
docs(mongodb): scope the upgrade-availability claim to quorum-capable sizes
The upgrade-restart note said the replica set stays available through the rollout. That holds at three members and fails below them, and the chart permits both smaller sizes: replicas 1 and 2 render unsafeFlags.replsetSize and are not excluded from the SAN change, so they gain horizons and roll on upgrade exactly as a three-member release does. At one replica the rollout takes the only mongod down; at two the write majority is still two, so restarting either member stalls writes until it returns. An operator running a single-replica external release would have read the old text and declined to book a window. Cover the two replicas case, which was untested, and assert horizons and the sub-quorum marker together at both sizes, so gating horizons to sizes that can absorb a restart turns the tests red and sends someone back to this paragraph. The default-size case carries the pair as a positive control: the marker proves nothing unless it is absent where a restart is survivable. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
7ef09cc963 |
docs(mongodb): show how to verify a connection against the trust anchor
The CA retrieval command was documented without the other half: what to do with the bundle once it is in hand. A tenant reading only the retrieval step has the certificate and no worked example of a verified connection. Pass the two TLS inputs as mongosh flags rather than folding tls=true into the connection string, because the correct separator differs by topology -- the replica-set URI already carries a ?replicaSet=rs0 query and the sharded one carries none, so a single copyable form would be wrong for one of them. Call out that omitting the flags is silent. Under preferTLS the server accepts the plaintext connection and nothing reports the downgrade, so a working connection is not evidence that TLS was negotiated. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
8af39eea22 |
docs(mongodb): correct the external SAN version gate and the leaf lifetime
The external-hostname note claimed the operator and the CR version were "both shipped by this chart". Only crVersion is: the PSMDB operator is a separate platform component with its own lifecycle, and the SAN fold is gated on both. On a platform carrying an older operator nothing reports the unmet gate — the chart still renders splitHorizons, the CR is still accepted and the cluster still comes up healthy, while the external hostnames never enter the SAN list and a verifying client fails the handshake. State which half the chart owns and point at the operator version as the thing to check. The leaf certificate lifetime was stated as a flat 90 days. It is an operator default this chart deliberately does not pin: nothing in the tenant contract keys off it, the operator reissues without client action, and pinning would freeze the chart against upstream shortening it. Attribute the number to the operator version and pin the field's absence, so "inherited rather than chosen" stays falsifiable — the same treatment allowInvalidCertificates already gets. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
90eb5c6815 |
test(mongodb): make the cert-manager exclusion suite discriminate
Every assertion in the suite passed while checking nothing. containsDocument without `any: true` asserts that EVERY rendered document matches, so negating it asks whether "not every document is a Certificate" — true of any chart rendering more than one object, and this one renders eight. A cert-manager Certificate appended to templates/mongodb.yaml, the most plausible place for one to appear, left the suite fully green. Add `any: true` to each containsDocument so the negation means "no document anywhere is of this kind". Unscoped assertions are ANDed across template files, which is what makes the negatives correct: they go red if any file renders the forbidden kind. Add a positive control asserting the chart does render the PerconaServerMongoDB CR through the same matcher shape, scoped to the one template that emits it. The four negatives are all satisfied by a matcher that never matches, which is exactly how this suite failed; the control fails if that returns. Verified by injecting a Certificate into the chart: the suite now fails and previously passed. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
ea7bc00016 |
docs(mongodb): correct the TLS secret ownership and external SAN claims
The uninstall section promised that no manual cleanup of the TLS Secrets is needed, reversing the instruction it replaced. That promise rests on an ownerReference the operator patches on at reconcile time rather than sets at creation, and two paths skip the patch: a dry-run reconcile returns before it, and a Secret already present under the same name with a mismatched cert-manager.io/certificate-name annotation is counted toward success and returned on unpatched. The deletion finalizer does not cover the gap — it enumerates only the users, internal-<release>, internal-<release>-users and encryption-key Secrets. So a Secret predating the release at one of these names can stay unowned and survive uninstall, and for <release>-ca-cert that orphans the CA private key. State the reconcile-time mechanism and tell operators to confirm the CA Secret is gone. Pin the outcome with a chainsaw step asserting the CR owns all three TLS Secrets on a normal install, so the claim goes red if the operator stops patching instead of rotting as prose. The external-hostname note described a permanently empty _namespace.host as a bootstrap window that self-heals. A tenant with no host of its own and no ancestor supplying one resolves to an empty host for good, leaving external TLS hostname verification silently unavailable. Name that case alongside the transient one, and say why the render is not failed closed: it would break existing hostless tenants already running external access over IP. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
5be4e417b3 |
docs(mongodb): correct the operator-behaviour claims and the upgrade impact
The TLS notes made several claims the operator's own behaviour contradicts. The CA is issued for a year and renewed about a month early, not for a decade with rotation disabled — so a bundle copied once stops verifying, and the retrieval steps now say to re-read it. Sharded mongos SANs are complete, because the operator emits them for every topology; the real gap is that no SAN covers the single external LoadBalancer fronting mongos, so external hostname verification is unsupported there. preferTLS is the operator's own default rather than a posture adopted to accommodate backups, which run under the same certificates. External access in replica set mode is one LoadBalancer per member with driver-side primary discovery, not a single load-balanced endpoint — the old description contradicted the external SAN note. And the posture section named only one of its two inherited weaknesses: unverified clients are accepted as well as plaintext ones. The lifecycle section was wrong in both directions. The operator owns its TLS Secrets and its cert-manager objects through the CR, so they are collected on uninstall rather than left behind. The rotation snapshots are the opposite of what was written: the operator deletes them itself once the merged CA has landed, and reads the previous CA back out of them while a rotation is in flight — so the advice to delete them by hand could destroy the only copy of the old CA. Only an interrupted rotation strands them. Document what an upgrade actually does to a release already running with external access: the new hostnames become certificate SANs, the leaf is reissued, and the replica set rolls. Gate the retrieval steps on the component that publishes the trust anchor and on cert-manager, without which the declared source does not exist and the command returns nothing. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
2670f7efc1 |
build(mongodb): run the chart's unit tests
The unit-test runner discovers charts by probing for a test target, and this chart had none — so its suites had never run in CI. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
612ae08e5f |
fix(mongodb): omit splitHorizons where it would render empty
With no external hostname to advertise — zero replicas, or a tenant host the controller has not populated yet — the horizons range produced nothing and the key was emitted with a null value. The CRD types it as an object without nullable, so the API server rejected the whole PerconaServerMongoDB and the HelmRelease with it. The values schema permits zero replicas, so this was reachable. Guard on both and omit the key entirely. Pin the surrounding invariants, which were asserted in comments and enforced nowhere. The chart must render no cert-manager objects of its own, since the operator owns the chain and anything rendered here would be orphaned while looking like it participated. It must set neither of the two fields that switch TLS off at defaulting time while the rendered mode still reads preferTLS. And the external-access case now supplies a tenant host, so it isolates the flag it names rather than passing on the host guard. Correct the header comment, which fused two false claims: that setting spec.secrets.ssl is what keeps TLS on, and that it is what engages the cert-manager integration. The operator defaults that name unconditionally and runs the cert-manager path regardless. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
27f71bfbe9 |
fix(mongodb): deliver the TLS trust anchor without private key material
The tenant-facing paths handed over both Secrets the PSMDB operator creates for TLS. <release>-ca-cert holds the CA private key, so read access is a full trust-chain compromise; <release>-ssl holds the server private key, so read access allows impersonation. A client verifying the endpoint needs ca.crt and nothing else, so neither object belongs on a tenant-facing path. Declare the trust anchor by name instead. The operator creates its CA Secret itself and offers no way to label it, so the ApplicationDefinition names it and the CA-extraction controller lifts ca.crt alone into the key-free projection. The include selector matches that projection by label, and the dashboard Role drops both key-bearing names without gaining the projection by name: a name grant conveys whatever occupies the name, while the label path conveys only what the platform vouched for. Witness both delivery paths against a running cluster, since the two that matter are separate. The Role is asserted as an exact grant with bounded rule counts, because a rule with no resourceNames — or one granting every resource — would otherwise pass an absence check while conveying every Secret in the namespace. The label path is asserted as a negative on all three key-bearing Secrets. A bats check pins the declaration's names and keys, which nothing else covers: the field is pruned at admission until the controller ships, so drift there would surface as an empty trust anchor with nothing red. Drop the mongodb-rd unit test. That package has no test target, so the file never ran, and what it asserted was the leak. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
7f8f32b1ba |
fix(mongodb): always enable operator-managed TLS, drop tls.enabled field
Address review feedback from lexfrei (B3): the tri-state tls.enabled field could not actually disable TLS. The PSMDB operator defaults spec.tls to preferTLS whenever it is unset, so tls.enabled: false rendered no spec.tls while the operator still served TLS — a misleading no-op. Remove the field and unconditionally render spec.secrets.ssl and spec.tls.mode: preferTLS to match operator behaviour. splitHorizons SAN injection now keys off external alone. Also drop the Makefile jq patch that re-injected the tls schema property. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Arsolitt <arsolitt@gmail.com> |
||
|
|
e9f276195f |
test(mongodb): cover TLS modes, splitHorizons, and RBAC
Signed-off-by: Arsolitt <arsolitt@gmail.com> Assisted-By: Claude <noreply@anthropic.com> |
||
|
|
f6816e1f44 |
feat(mongodb): add operator-managed TLS with external hostname SANs
Signed-off-by: Arsolitt <arsolitt@gmail.com> Assisted-By: Claude <noreply@anthropic.com> |
||
|
|
e84a446893 |
chore(keycloak): bump KMS proxy to 0.2.2 (#3995)
## What this PR does This PR updates keycloak-kms-proxy to v0.2.2 and pins image to published digest. New version fixes decryption for reads whose result set spans a join. ### Screenshots Not applicable. ### 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: ### Release note ```release-note chore(keycloak): update keycloak-kms-proxy to v0.2.2 with a fix for decrypting reads whose result set spans a join ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated the Keycloak encryption component to a newer verified image version. * Improved deployment reliability by pinning the image to a specific integrity digest. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
6d9444056d |
chore(keycloak): bump KMS proxy to 0.2.2
Pin the released image by digest so package rendering is reproducible. Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
b9a95a0c4a |
fix(postgres): let in-place restore re-bootstrap instead of wedging (#3959)
## What this PR does An in-place `RestoreJob` (one whose `targetApplicationRef` is omitted, so the app is restored back into itself) re-bootstraps the Postgres app under its own name. On stock v1.6.2 it never comes back up — the cluster is deleted and then hangs in `Setting up primary` with `*-full-recovery-*` pods crash-looping, or the HelmRelease upgrade fails outright. There are two independent causes, both in the same restore patch, and both surface only in place because a to-copy restore targets a different app. **1. Archive-vs-recovery `serverName` collision.** The driver set the restored cluster's WAL-archive `serverName` equal to the recovery source (`bootstrap.serverName = bootstrap.oldName = sourceServerName`), while the chart pinned the archiver `serverName` to the release name. In place those coincide, so the cluster archived onto the source's own WAL prefix and `barman-cloud-check-wal-archive` refused to start with `Expected empty archive`. Fixed by giving the restored cluster a fresh `bootstrap.newServerName`, keyed by the `RestoreJob` UID (unique per restore, stable across reconciles); the chart archives there while `externalClusters` still recovers from `bootstrap.serverName`. This is what manual `CNPG` strategies hit (their `serverName` defaults to the cluster name, including the shipped `examples/backups/postgres/10-cnpg-strategy.yaml`); the `cozy-default` flow dodged it because its `serverName` is `<namespace>-<app>`. **2. `useSystemBucket` guard.** The restore populates explicit S3 coordinates (`destinationPath` / `s3CredentialsSecret` / `endpointCA`) but left `backup.useSystemBucket=true`, so the chart's `bootstrap.enabled is incompatible with useSystemBucket` guard failed the HelmRelease upgrade and the cluster never re-rendered. This is what the default `cozy-default` (system-bucket) flow hits on an in-place restore. Fixed by clearing `useSystemBucket` in the restore patch, since the app is now on the explicit-creds path. Verified live on a v1.6.2 cluster: with `useSystemBucket` cleared, the same in-place restore re-bootstraps to a healthy cluster and the sentinel row survives. Both are exercised only by an in-place restore; the postgres backup e2e drove only to-copy, so neither surfaced in CI. A step 50 in-place restore now guards cause 1 (manual-strategy flow); cause 2 is covered by a Go unit assertion (a system-bucket in-place e2e needs the platform bucket wired in CI and is left as a follow-up). This is a bug in released v1.6.2 (chart and driver are byte-identical on `main`), so it is a backport candidate. ### Screenshots Not applicable (no UI changes). ### Downstream repositories The only externally visible schema surface is one new field, `spec.bootstrap.newServerName`, in the postgres `values.schema.json` / CRD; it is set by the CNPG backup driver on restore and never authored by tenants, exactly like the existing `bootstrap.serverName` / `bootstrap.oldName`. (The `useSystemBucket` clearing is internal to the Go driver — `backup.useSystemBucket` already exists in the schema.) The chart `README.md` is regenerated from `values.yaml` automatically by the website docs bot on release, and `terraform-provider-cozystack` models user-authored spec (its guard tracks top-level fields, not driver-set nested ones), so neither needs a change in practice. Leaving every box unticked per the trigger-map guidance so a maintainer confirms rather than acting on a speculative follow-up. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note fix(postgres): in-place restore no longer wedges — the restored cluster archives WAL under a fresh serverName distinct from the recovery source, and a system-bucket app is switched to explicit credentials so the bootstrap/useSystemBucket guard no longer fails its HelmRelease. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for specifying a distinct WAL archive destination when restoring PostgreSQL clusters. - In-place restores now automatically use a fresh archive location while continuing to recover from the original source. - Added configuration and schema support for the optional `bootstrap.newServerName` setting. - **Bug Fixes** - Prevented archive conflicts that could cause in-place restores to fail. - **Documentation** - Documented the new restore archive configuration and behavior. - **Tests** - Added coverage for unique restore destinations and in-place restore validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
ac00577eb7 |
fix(clickhouse): scheme the backup S3_ENDPOINT on the system-bucket flow (#3963)
## What this PR does On the default `cozy-default` / `useSystemBucket` backup flow, the ClickHouse chart pointed the `clickhouse-backup` sidecar's `S3_ENDPOINT` straight at the projected `cozy-backups-creds.endpoint`, which the backup credentials projector stores as a **bare host** (no scheme) by design. `altinity/clickhouse-backup` runs `S3_ENDPOINT` through the aws-sdk-go-v2 endpoint resolver, which rejects a scheme-less value with `Custom endpoint 's3.host' was not a valid URI` — so every ClickHouse backup and restore on the default flow failed. Reproduced live on a v1.6.2 cluster: a `cozy-default` `BackupJob` against a `useSystemBucket: true` ClickHouse hangs, and the sidecar logs `S3 ResolveEndpoint ... was not a valid URI`. The fix prepends `https://` on the `useSystemBucket` path via a helper env plus `$(VAR)` expansion (a `secretKeyRef` value can't be string-interpolated), mirroring the platform's `backupstrategy-controller.endpoint` helper which force-prepends `https://` for the always-TLS external S3 ingress. The legacy and bring-your-own-Secret paths keep reading `endpoint` verbatim, since that value is tenant-supplied and already carries its own scheme. ClickHouse was the only `cozy-default` app hit by this. I audited every default-flow app: the CNPG/postgres, etcd and velero strategies already get a scheme'd URL from the endpoint helper, and mariadb and foundationdb deliberately pair a bare host with a separate TLS flag; mongodb reads a full URL from its own chart values. So this is the single scheme-class fix needed. Not addressed here (separate, non-scheme gaps surfaced during the audit, flagged for follow-up): MongoDB has no `useSystemBucket` wiring at all (its `cozy-default` strategy assumes a manually-configured `backup.endpointURL` + per-app S3 creds), and FoundationDB's self-signed-CA trust to the operator pod is a known gap. ### Screenshots Not applicable (no UI changes). ### Downstream repositories No schema or API change (only a chart template and its helm-unittest). `values.yaml` / `values.schema.json` / the CRD are untouched, so nothing downstream is affected. Leaving every box unticked per the trigger-map guidance. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note fix(clickhouse): backup and restore now work on the default system-bucket (useSystemBucket) flow — the clickhouse-backup sidecar receives a scheme'd https:// S3_ENDPOINT instead of the projected bare host, which the S3 client rejected as an invalid URI. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Corrected ClickHouse backup endpoint configuration when using the system bucket. * Ensured backup connections consistently use the required HTTPS endpoint format. * Improved handling of backup credentials across system bucket, legacy, and custom-secret configurations. * **Tests** * Added coverage validating endpoint and credential wiring for supported backup configurations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
c88da18139 |
test(tests): keep trace switch suite ASCII-only
Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
e949f39800 |
fix(e2e): preserve previous-log cap slots
Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
76d397523b |
docs(tests): record why an INT/TERM trap cannot cover a killed test
The comment added with the trace switch pointed at a signal trap as the fix for the one case quiet mode does not cover: a test killed rather than failed never reaches the fail handler, so its trace is gone rather than already printed. That advice is wrong, and wrong in a way that costs whoever follows it. POSIX sh defers a trap until the current foreground command returns, so installing an INT/TERM handler stops the runner dying on the signal at all. Measured: a suite hung on `sleep 30` exits in 30s with the trap installed against 2s under the default disposition. GitHub's cancellation escalates INT, TERM, then KILL, so the deferred handler gets SIGKILLed before it runs -- no dump, and the grace period spent waiting for a hang. Built it, measured it, reverted it. Making it work means running the test pipeline in the background and waiting on it, because `wait` is the interruptible one. That is surgery on the path every suite in this repo takes, unit and e2e, to recover a partial trace in a rare case that already has a 48-second escape hatch. Not taken; recorded so the next reader does not spend the afternoon finding out the same way. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
a5c90d984e |
fix(tests): guard the finishedAt leaf and make three guards actually bite
Findings from two independent adversarial reviews of the preceding two
commits. Each was reproduced locally before being fixed.
The ordering inverted itself on a shape I had not probed. `finishedAt`
is a metav1.Time behind `json:",omitempty"`, which cannot omit a
struct, and its zero value marshals to `null`; the kubelet does
synthesize a terminated state with no time (ContainerStatusUnknown on a
pod deleted mid-run, which also bumps restartCount and so clears the
restart filter). Two guards deep left the leaf exposed, and kubectl
renders it as the literal `<no value>` -- which sorts ABOVE every digit,
so that row took the front of the queue from the most recent real
restart, and the header printed `previous instance ended <no value>`, a
claim about a time nothing observed. Verified with a text/template probe
over seven lastState shapes: `null` and a missing key both produced it.
Fixed at both layers, because they fail independently. The template now
guards the leaf, so the string cannot be produced. And the sort
normalises any field-6 value not shaped like a timestamp to empty, so
the ordering stays correct and the header stays quiet even if some other
producer emits one. Earlier verification against a live cluster showed
no `<no value>` and I read that as the guard being right; it only meant
no container on that cluster was in that state.
Three guards did not bite:
- both stability fixtures were pre-sorted ascending, which is exactly
what sort's last-resort whole-line compare reproduces with no -s at
all, so the tests written for -s passed without it. Reversed; they
now fail when -s is dropped.
- no test fed a six-field row through the whole script, so the
timestamp reached the dump header uncovered -- deleting the read
variable left the suite green at 39/39. One integration test now
asserts the header names the time.
- the e2e guard matched any mention of COZYTEST_TRACE, so documenting
the design over there would have turned it red. It now matches an
assignment, and tolerating a comment is verified too.
Also: a failed sort no longer discards a whole namespace group. The
caller compares row counts to report the cap, so losing a group silently
became "0 of N captured, N more not captured" and sent the reader after
a cap that never fired. It falls back to discovery order and says so.
LC_ALL=C on the sort, since the key is fixed-format ASCII.
And the claim itself was too strong. "Nothing diagnostic is lost" holds
only for a test that RETURNS: the fail handler runs after the pipeline,
and there is no INT/TERM trap, so a test killed rather than failed -- a
hang reaped by the job timeout -- never dumps its log, and under quiet
mode its trace is gone rather than already printed. The comments in
hack/cozytest.sh and the Makefile now say so and name the escape hatch.
A trap that flushed the tail of the log would close it; left out
deliberately rather than bundled in here.
Every fix is mutation-proven: reverting the leaf guard, removing the
key normalisation, dropping the read variable, dropping -s, and
silencing an e2e recipe are each caught. Unit lane 1513 passed, 1 failed
(hack/ghcr-mirror_test.bats, which reads a live cluster and fails the
same way under main's runner).
Assisted-By: Claude <noreply@anthropic.com>
Assisted-By: GPT-5 <noreply@openai.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|