mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-29 08:07:33 +09:00
11bac54f931682dd94de4081bb9e8a7eef60bef1
6613
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Andrey Kolkov
|
11bac54f93 |
test(backups): pin both sides of the Velero rename guard
The guard's regression cases only exercised renames, so dropping the IsRenamed conjunct (failing every non-VMInstance restore) or keying the check on targetApplicationRef.kind instead of the backup's kind left the package green. Add the inputs the guard must let through, a VMDisk copy that keeps its name and a VMDisk in-place restore without targetApplicationRef, and a VMDisk backup restored with a VMInstance target kind, which must still be rejected by the source kind. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
88073bb209 |
fix(backups): fail Velero RestoreJob rename for non-VMInstance kinds
A RestoreJob that set targetApplicationRef.name to a new name and options.targetNamespace to another namespace reported Succeeded for any Velero-backed application, but only a VMInstance was actually renamed. postRestoreRename looks up the HelmRelease under the vm-instance- prefix, gets NotFound for every other kind and returns nil, so a Postgres, MariaDB, ClickHouse, MongoDB or standalone VMDisk copy came up under the source name with no error surfaced. Renaming those kinds through the Velero strategy cannot work: DataUpload restores volumes under their original names, and only a VMInstance keeps using them after its HelmRelease is renamed, because its disks keep their names and CDI adopts the restored PVCs. A renamed database Cluster or VMDisk renders fresh volumes and comes up empty. Reject the rename up front for every kind other than VMInstance, before the RestoreJob enters Running, with a message that points at omitting targetApplicationRef.name or using the engine-native strategy. The existing same-namespace guard keeps precedence, so its message is unchanged. Closes #3537 Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Aleksei Sviridkin
|
45f5698bce |
fix(cdi): validate kubevirt-cdi values against a schema (#4417)
## What this PR does This gives the kubevirt-cdi chart a values schema, so a bad clone strategy fails when the chart renders instead of when the CDI object is applied. Today a typo like `csi_clone` renders fine, the CDI CRD refuses it on apply, and the operator finds out from a failed HelmRelease. The schema is generated by `cozyvalues-gen` from annotations in `values.yaml`, through a new `make generate` target, the same way `talos-log-collector` does it. There is no README here, so the target only writes the schema. The clone strategy is checked with a pattern, not with the CRD's enum. The chart uses the empty string to omit the field and let CDI pick a strategy per StorageProfile, and the CRD enum has no member for that. `cozyvalues-gen` also can't emit an empty enum value. So the accepted values are `copy`, `snapshot`, `csi-clone` and `""`. A null still works as before. The key has a chart default, so Helm drops a null for it before validation, and the template omits the field. `uploadProxyURL` only has to be a string. The new test suite refuses `csi_clone` and values of the wrong type, and accepts each of the allowed values and null. Closes #4141 ### Screenshots No UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(cdi): kubevirt-cdi now validates its values against a schema, so an invalid `cloneStrategyOverride` such as `csi_clone` fails at render time instead of when the CDI object is applied. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Configuration** * Chart values are now validated to ensure the upload proxy URL is a string, importer resources are provided as an object, and clone strategy uses a supported value: `copy`, `snapshot`, `csi-clone`, or an empty value. * Existing defaults for these settings remain unchanged. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
5e460605c2 |
fix(cdi): drop the ExpandDisks gate from the CDI object (#4415)
## What this PR does This drops a feature gate from the CDI object that CDI never reads. `ExpandDisks` sits in `spec.config.featureGates` in `packages/system/kubevirt-cdi/templates/cdi-cr.yaml`, but it is a KubeVirt gate. CDI v1.66.1 has no gate with that name. It looks a gate up by exact name, so an extra entry is read and ignored. The CRD types the list as plain strings, so nothing rejects it either. Disk expansion comes from KubeVirt, where `ExpandDisks` is GA at v1.9.0, the release the platform ships. I left the KubeVirt CR as is; #4029 tracks the GA gates in that list. The gate list now has its own test case, pinned whole to what upstream's `cdi-cr.yaml` ships. Adding `ExpandDisks` back turns it red. `make update` only checks the upstream file against its pinned hash and writes nothing under `templates/`, so it can't bring the line back. Closes #4112 ### Screenshots No UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(cdi): stop listing the KubeVirt `ExpandDisks` feature gate on the CDI object, where CDI ignored it. Disk expansion is unaffected. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Configuration** * CDI now enables only the `HonorWaitForFirstConsumer` feature gate. The `ExpandDisks` gate is no longer included in the configuration. * **Tests** * Updated configuration checks to verify the exact feature-gate list and ensure it matches the gate CDI supports. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
1f30ac4598 |
fix(tests): let the mariadb waits cover a first boot (#3769)
## What this PR does Closes #3741 Several waits for a database HelmRelease to become Ready gave up before the release could: the mariadb, mongodb, rabbitmq and clickhouse backup walkthroughs waited 300 seconds, and both mariadb Chainsaw suites gave their HelmRelease-ready assert 6 minutes. A generated release carries a 10-minute install timeout (`pkg/cmd/server/start.go:91`), so a failed install was reported as a bare timeout, because the release had not yet recorded why it failed, and for mariadb a first boot the startup probe was still willing to wait for failed the harness instead. A wait on such a release has two floors and has to beat the larger. The first is the app's own first boot: cozystack-api leaves `spec.waitStrategy` unset and helm-controller defaults it to `poller`, so the install waits through kstatus, and kstatus maps the operator CR's `Ready=False` onto `InProgress`. For mariadb that boot may take the chart's 310-second startup budget (`startupProbe.failureThreshold: 30` in `packages/apps/mariadb/templates/mariadb.yaml` with the operator's probe defaults of 20 and 10); mongodb, rabbitmq and clickhouse set no startup probe. The second is the install timeout. At or below it the waiter is the one that expires, because its clock starts when the step runs and the install's only once the Application has been reconciled and the chart artifact fetched. The install timeout binds in every case here, so the walkthrough waits go to 660 seconds and the mariadb asserts to 11 minutes. Raising a wait inside a walkthrough spends the Chainsaw op that runs the script, and an op that runs out SIGKILLs the process group and loses the script's error. Each op therefore grows by exactly what the waits it reaches gained: mariadb's round-trip from 30 to 42 minutes, rabbitmq's from 30 to 42, mongodb's from 30 to 36 (its round-trip runs with `SKIP_RESTORE`, so only the source wait counts), and clickhouse's from 60 to 72, which still sits above the sum of every inner ceiling. That bounds a failure at a single wait, not a run that is slow at every stage and then fails late; bounding that needs a phase budget, not a larger literal. postgres keeps its 360-second waits and every walkthrough keeps its 300-second bucket wait: whether a suite pays the extra minutes on every red run is a per-suite call. The mariadb endpoint asserts keep their job. A freshly applied CR carries no conditions and kstatus reads it as current, so the install wait can end before any pod is up; an endpoint address closes that window. That is also why the `wait_for_field` on the CR stays in the walkthrough. `wait_hr_ready` now dumps every condition and the release history on timeout and on `Stalled=True`, instead of the single Ready message a scheduled retry can overwrite before anyone reads it. It also stops printing one `(none recorded)` line for different answers: a release that never appeared, a release deleted during the wait, a lookup kubectl could not answer, a failed history read and an empty history each say so, with kubectl's own stderr kept, and a release seen earlier still gets its dump when only the last lookup failed. The change goes to every byte-identical copy of the helper, in `examples/backups/{clickhouse,mariadb,mongodb,postgres,rabbitmq}/00-helpers.sh`, and they stay identical. `hack/mariadb-first-boot-waits.bats` pins the mariadb budgets against the tree. It derives both floors from their sources instead of pinning literals, so raising `failureThreshold` past the install timeout, or changing the install-timeout default, reds it; each source has a test that refuses to proceed when it cannot be read, including the ApplicationDefinition, whose two override annotations would each invalidate the derivation. It holds the round-trip op at a fixed remainder over the ceilings up to and including the source wait, bucket stage and S3 preflight included, and at a fixed slack once both mariadb waits are paid, so raising a wait without the op reds. The extractors are also tested against fixtures, because the tree cannot tell a correct extractor from a broken one. Beyond mariadb, the file runs every exit branch of `wait_hr_ready` in each copy against a stubbed kubectl, and pins the copies byte-identical. The population is every definition in the tree found by grep, minus two named exceptions held to still differ, so a new copy joins without editing the test and an exception whose copy converges goes red: - `examples/backups/redis/00-helpers.sh` is written in the compact one-line style of the rest of that file, so converging it rewrites its shape rather than applying the shared body. - `examples/backups/kafka-metadata/00-helpers.sh` is a helper of its own with the same diagnostic gap on timeout (it prints no reason at all), and closing that is a change to kafka-metadata. `docs/agents/e2e-testing.md` replaces the reviewer checklist's 5-6m standard and its list of exceptions, which the tree had outgrown, with the floor a wait has to beat and the rule that a longer wait carries its reason in-line. The derivation gets a section of its own, opening with the rule in one line. For a release an app chart renders itself the floor is `spec.install.timeout`, then `spec.timeout`, then helm-controller's 5m default, and the nested `<name>-system` releases never take cozystack-api's 10m. The harbor comments name the floor each of their asserts sits against, and the opensearch comment stops citing the removed standard. Adding `hack/mariadb-first-boot-waits.bats` does not escalate the E2E selection: `hack/[^/]+\.bats$` sits in `inert_config_pattern` in `hack/select-e2e.sh`, and `full_suite_pattern` escalates only `hack/e2e-*.bats`. On this changeset `hack/select-e2e.sh` selects `clickhouse harbor mariadb mongodb opensearch postgres rabbitmq`. What this costs is paid only on a failing run; the raised numbers are ceilings. Summing the timeouts stated on each suite's own steps, mariadb goes from 92m to 114m, clickhouse from 116m to 128m, rabbitmq from 67m to 79m and mongodb from 95m to 101m, before the untimed steps that fall back to the defaults in `.chainsaw.yaml`. clickhouse is the most expensive suite by that sum. All of them compete for the 215m `timeout-minutes` of the whole `e2e` job, which also covers image download, sandbox preparation and the Cozystack install, and a job cancelled at the cap loses both its log and `cozyreport.tgz`. This does not fix the failure that led here. In the run that prompted the investigation the mariadb replica never opened port 3306 at all, spending its whole 310-second budget inside `mariadb-install-db` before being killed, and no wait budget would have saved it. That defect is #3744. What this fixes is a second, independent defect found while reading the same code. One thing here is argued rather than measured: the mechanism is checked against helm-controller and kstatus, and every number is derived from the tree, but no run ID pins that it is what any particular red run hit. Both floors justify 660s whatever the cause was, so being wrong about the cause leaves slack rather than breakage. ### Not in this PR - This PR retires the mariadb, mongodb, rabbitmq and clickhouse walkthrough waits and the two mariadb asserts. The rest is tracked in #4404: the other Chainsaw HR-Ready asserts under their floor, the harbor asserts, the 300-second bucket waits, the postgres waits, and the redis and kafka-metadata diagnostic gap. - The clickhouse round-trip comment says every wait fails fast, `wait_hr_ready` on `Stalled=True`, "so a genuinely broken backup reports in seconds". A release whose install is still running is neither Ready nor Stalled, so that holds for a stalled release and not for an install that has not finished. The sentence predates this change. - `hack/mariadb-first-boot-waits.bats` is large for what it guards (two floors, two ratchets on the op, and the helper identity pin), with many fixture tests on its own extractors. Trimming it to that core is a separate decision. - Two comments in that file state facts about the mariadb suite that are true today and that nothing checks: three script ops carry a minute timeout, and the only one ahead of the round-trip is the 8m pre-clean. - The mariadb op comment says a bound on the whole flow "needs a phase budget (docs/agents/e2e-testing.md)"; the doc describes it as bounding the collection phase as a phase (`COZY_DIAG_PHASE_BUDGET`), so the pointer would resolve faster if it named that bullet. - Cosmetic: the behavioural test's loop body is not indented under its `for`, one line of the bats header runs past the width of the rest, and the checklist's sentence under the example assert ("what this block demonstrates least") reads awkwardly. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file, for all eighteen files. The five `examples/backups/*/00-helpers.sh` files change the diagnostics of `wait_hr_ready`, and the mariadb, mongodb, rabbitmq and clickhouse walkthrough scripts change wait budgets; the manifests the walkthroughs apply and the flows they document are unchanged, so no website page restates anything that moved. Four Chainsaw suites (mariadb, clickhouse, mongodb, rabbitmq) change timeouts and comments, and the harbor and opensearch suites change comments only. `docs/agents/e2e-testing.md` changes the reviewer checklist entry and adds the sizing section. `hack/mariadb-first-boot-waits.bats` is a new file picked up by the existing `BATS_UNIT_FILES` wildcard. `hack/package.mk`, `hack/common-envs.mk` and `hack/update-crd.sh` are untouched, and no `values.schema.json`, `values.yaml` default, version enum, `ApplicationDefinition`, CRD, namespace or release asset is touched. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(tests): the MariaDB, MongoDB, RabbitMQ and ClickHouse backup examples and the MariaDB Chainsaw suites now wait for a HelmRelease past the release's own install timeout, so a failed install reports its own reason instead of a bare timeout, and the examples' HelmRelease wait now tells a release that never appeared, one deleted mid-wait and a kubectl that could not answer apart. ``` |
||
|
Aleksei Sviridkin
|
9c97a7a7ac |
fix(cdi): validate kubevirt-cdi values against a schema
Without a schema, a cloneStrategyOverride outside the CDI enum, such as csi_clone, renders a CDI object the API server refuses on apply, so the mistake shows up as a failed HelmRelease rather than at the value. The field is constrained by a pattern, not by the CRD's enum. The empty string is the chart's way to omit the field and let CDI pick a strategy per StorageProfile, the CRD enum has no member for it, and cozyvalues-gen cannot emit an empty enum value. The key has a chart default, so Helm drops a null for it before validating, and a null keeps rendering as an omitted field. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
0c358addfb |
fix(cdi): drop the ExpandDisks gate from the CDI object
ExpandDisks is a KubeVirt feature gate. CDI declares no gate by that name and matches gate names exactly, so the entry was accepted and ignored. Disk expansion is a KubeVirt feature, GA there at the release the platform ships, so listing the gate on the CDI object points a reader tracing where expansion comes from at the wrong component. The feature-gate list is pinned whole in its own case, so a gate added beside the one upstream ships turns the suite red. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2b5770cb4a |
fix(vm-instance)!: refuse resources that cannot size the VM (#4405)
## What this PR does A vm-instance could ask for half a core and get one, with no error anywhere. `resources.cpu` and `resources.sockets` are typed as quantities, but they end up in `domain.cpu.cores` and `domain.cpu.sockets`, which KubeVirt declares as unsigned integers. The chart cast them with `int64`. That turns `500m` or `1.5` into `0` without an error, and KubeVirt boots a zero core count as one core. The chart now reads these two fields the way an integer field reads them. It decodes the raw value as YAML and stops the render unless the result is a positive whole number that fits in uint32. So `2`, `"2.0"`, `"+2"` and `"1e1"` render as the integer they decode to. `500m`, `2000m`, `1k`, `"1.5"`, `0`, `"0"`, `"-2"`, `"010"` and `"4294967296"` stop the render with an error that names the field. A leading zero is refused because YAML reads `010` as octal 8. The check looks at presence, not truthiness, so an unquoted `0` is refused instead of being read as unset. It is the same rule #4386 uses for kubernetes-nodes. Memory gets a presence check too, which covers the case from the issue thread. A quoted `"0"` was truthy and rendered `requests.memory: "0"`, while an unquoted `0` was falsy and got dropped. Both spellings of zero, and any negative amount, now stop the render. Any other value passes through unchanged, because KubeVirt takes memory as a quantity. That includes an amount below one byte, such as `500m` or `1e-3`, which renders as it does on `main` today; this change does not refuse it. The check lives in `virtual-machine.domainResources`. The VM spec, the instance type decision and the update hook's patch all take their values from that helper. The tests that pinned `cores: 0` as the old behaviour now expect the refusal, and the helper comments about the truncation are trimmed. This is a breaking change. A release that carries one of these values fails its next upgrade until the value is fixed: a cpu or sockets that is not a positive whole number within uint32 or has a leading zero, and a zero or negative memory. This includes a cpu or sockets set on its own next to an `instanceType`, which used to be ignored whatever it held. Some values change meaning instead of failing. A value like `"1e1"` or `"2."` was truncated to 0 by the `int64` cast, so the VM booted with one core. Now it gets the count the value spells, so that VM changes size on upgrade. I refuse these values instead of rounding them, because silent coercion is how `500m` became one core in the first place. The issue asks for a deprecation path, and I did not add a migration that rewrites stored values. It would have to guess what `500m` meant, and both options (one core, or dropping the field) hide the same mistake again. The HelmRelease upgrade retries without uninstalling, so the running VM stays up until someone fixes the value. #4386 makes the same trade-off. The check runs at render time, not at admission. The schema still accepts `500m`, so the API takes the object and the error shows up in the HelmRelease status. Narrowing the schema to integers, as the issue also proposes, would move the error to admission, but it needs a migration of stored values. I left it for a separate change. The field descriptions now say which values are accepted, and `make generate` carried them into the README, the schema, the Go types and the ApplicationDefinition. The new `tests/resources_values_test.yaml` covers the accepted and refused spellings for cpu, sockets and memory. `tests/instancetype_resources_test.yaml` gets a case for a lone cpu next to `cpuModel`. The whole vm-instance suite passes. Every refusal case in the new file, and the `"1e1"` case, fails against the current `main` template. Closes #4104 ### Screenshots No UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: I walked the trigger map against the diff. No field is added, removed or renamed, and no default or enum changes. Only three field descriptions change, and the website reference page is regenerated from the README at release time. ### Release note ```release-note fix(vm-instance)!: resources.cpu and resources.sockets must now be positive whole numbers, and resources.memory a positive quantity. A cpu or sockets that is not a plain whole number within uint32 (for example 500m, 1k or 1.5), or a zero or negative memory, now fails at render time with an error naming the field, instead of silently becoming a one-core VM or a zero memory request. A release carrying such a value fails its next upgrade until the value is fixed. A value such as 1e1 or 2., which used to be truncated to one core, now sizes the VM with the count it spells. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * VM sizing now accepts only positive whole-number CPU and socket values within the supported range. Fractional, millicore, zero, negative, and out-of-range values are rejected during rendering. * Memory values must be positive quantities. A CPU or socket value specified alone does not size the VM and is ignored; incomplete sizing is rejected where required. * **Documentation** * Updated VM instance resource guidance to clarify CPU and socket requirements, memory validation, and how socket values define vCPU topology. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
99de79ff52 |
fix(cdi): pin the CDI release kubevirt-cdi is reconciled against and stop update overwriting it (#4107)
## What this PR does `packages/system/kubevirt-cdi` resolved its CDI version at run time from GitHub's `releases/latest` redirect and wrote the result straight over `templates/cdi-cr.yaml`. That file is not vendored. Upstream ships a bare CDI object; ours adds Helm templating for `cloneStrategyOverride`, `uploadProxyURL` and the worker-pod `podResourceRequirements`, the `ExpandDisks` feature gate, a `CriticalAddonsOnly` toleration, and the two RBAC documents that let a tenant clone from the golden-image namespace. I diffed the two at v1.66.1 to write that list, so it is complete as of that release rather than only hedged: upstream ships 283 bytes with one feature gate, two nodeSelectors and an `imagePullPolicy`, and we are a strict superset of it. Every entry arrived by hand, so `wget -O templates/cdi-cr.yaml` was not refreshing a vendored file, it was overwriting a first-party one. Same loss as the `rm -rf templates` that #4097 removes, arriving through the fetch instead of the wipe. `update` now pins the release the object is reconciled against, and fetches it only to confirm the bytes still hash to a digest I measured. Nothing is written under `templates/`. At the next CDI bump whose asset differs, that check fails, which is the point: upstream moving its own defaults is when the local copy needs a human to reconcile it, not an overwrite. The download is kept on a mismatch, since reconciling one starts by reading it. `RELEASE` is `v1.66.1`, which is the release `kubevirt-cdi-operator` vendors, so the pin and the deployed version agree. What the platform deploys is decided by that package, and it can move to another release without this file changing, so this line is pointed at it by hand. `update` then checks the two agree: it compares `RELEASE` with the `cdi-operator` image tag that package vendors and fails naming both versions when they differ, because otherwise its closing line would report a release the platform may no longer run. That is a check and not a derivation, so `RELEASE` stays a pin a human moves. Upstream publishes no checksum for the asset, so the pin comes from a download rather than from a published file: `a497f90de608c1df26f9ee4095289373f17132d74a2042ca03e00c17c964f8a7`. It catches an asset that moved under a tag that should be immutable. It is not authenticity verification, and the comment beside it says so. The digest command is chosen when `update` expands it, `sha256sum` when it is on PATH and `shasum -a 256` otherwise. Which one ran here is worth stating, because the usual claim about macOS is out of date: this machine picked `sha256sum`, and that binary is `/sbin/sha256sum`, a native Darwin one rather than GNU coreutils. It reports `sha256sum (Darwin) 1.0` and prints the same `<digest> <path>` format. The package already had a `test:` target, from `0418577f5`, so what it gains here is the `.PHONY` line that commit did not add. `hack/package.mk` declares only the nine targets it defines itself, so without it a file or directory named `test` or `update` beside the Makefile makes make consider that target up to date and skip the recipe in silence. The new suite pins the `ExpandDisks` gate, the toleration, `podResourceRequirements`, the `uploadProxyURL` override and the clone RBAC, and renders the upload-proxy Ingress and TLSRoute, each under the `gateway-enabled` value that selects it. The clone-strategy override is pinned by `tests/clone_strategy_test.yaml`, which is already on `main`. One consequence of the pin move is a claim rather than a number, so it gets its own line. Upstream carried `WebhookPvcRendering` from v1.63.0 through v1.65.0 and dropped it by v1.66.0; the local object never carried it, so the divergence ran both ways across those three releases and one way since. The comment says to re-derive the delta at each bump instead of trusting it, because that was a fact about a span of releases and not about CDI. `packages/system/kubevirt-cdi/Makefile` carried two identical `test:` targets, one from `0418577f5` and one appended by a later merge, so make printed `overriding commands for target 'test'` on every invocation in the package. The second sat inside the span this recipe rewrites, so it goes with the rewrite rather than being re-authored into it. The comment kept above it is the reason a reader should not add another. Closes #4157. ### Scope An earlier revision also pinned and tested `kubevirt-cdi-operator`. #3920 has since landed the two real bugs in that recipe, the patch strip level and `patches/tolerations.diff`, so that half is dropped here rather than merged twice. Pinning and testing the operator package is #4190. Named improvements left alone, so they read as choices. - The two upload-proxy tests each render their route under the `gateway-enabled` value that selects it and neither asserts the other is absent; the templates already settle that with `ne "true"` and `eq "true"` on the same variable, and `tests/standalone_render_test.yaml` already asserts each route absent under the values that select the other. So the package does cover it; these two cases just do not, and a third copy would not add anything. - The CDI object carries no version string for a rendered-output assertion, so `RELEASE` is checked by `make update` rather than by `make test`. - Neither route case pins the backend service port, 443 in both templates; that is outside what the suite claims. - The template-loss guard still counts `cdi-cr.yaml` as named, because the download line in `update` mentions the stem, so re-adding a wipe here would have it report two files instead of three; the guard's header already says a mention is not proof of a write, and the case needs a wipe this recipe no longer has. - The suite pins the local additions by design, and most of the upstream-derived half is pinned by nothing: `imagePullPolicy` and the two `nodeSelector` blocks have no assertion anywhere under `tests/`. `HonorWaitForFirstConsumer` is the exception and only as a side effect: `featureGates` is pinned whole, so that gate rides along with `ExpandDisks`. The rest are maintained by the same hand reconciliation the pin forces whenever the upstream asset changes, so a carry-over that drops one of them is green. Three asserts would close what is left. - One more, remote enough to name rather than code around: if the digest tool is missing entirely, `$$actual` comes out empty, the comparison against `CR_SHA256` fails, and the error text diagnoses a moved asset when the real cause is a machine with neither `sha256sum` nor `shasum`. Busybox ships the first and macOS ships the second, so nobody has hit it. Closing it means an executable line, which would then owe mutation coverage of its own, and that is disproportionate to a case with no known instance. The mechanism is written here so the next reader does not have to re-derive it from an error message that points the wrong way. - Separately, the per-package `.PHONY` gap this Makefile's own line argues against is not specific to this package: most package Makefiles do not declare their own `test:` or `update:` phony, and #4214 tracks that. - A key that does not exist yet is not held either: every item in the delta is pinned whole, but a NEW key appearing at `spec` or `spec.config` level passes unnoticed, and such a key can change behaviour rather than sit inert. I confirmed it on a copy extracted from this commit, appending `spec.config.filesystemOverhead.global: "0.99"` and getting a green suite. That is the shape to recognise at the next bump: not a changed value, which is caught, but a new sibling key beside the ones we pin. - And `spec.config.podResourceRequirements` is now pinned whole here and by four leaves in `tests/importer-resources_test.yaml`: the whole-map form is the stronger one and the leaves are a subset of it, so changing the default takes two edits and the second failure reads as a surprise until you know why both exist. - If the operator's image reference ever gains a digest (`:v1.66.1@sha256:...`), `${deployed##*:}` takes the digest's hex instead of the tag, and `update` fails with a loud false mismatch rather than passing silently. - Deriving `RELEASE` with `$(shell ...)` from the sibling manifest was suggested and declined on merits: a derived value is not a pin, the point is that a human chooses which CDI release this object is reconciled against, and computing it from whatever the neighbour last vendored hides that choice from the diff. The operator comparison lives in `update` and not in `test:`, even though `kubevirt-operator` ties its own manifest to the tree from `test:`. A mismatch calls for a hand reconciliation, and `update` is the recipe that asks for one; it is also where the success line that would otherwise name the wrong release is printed. I checked it on a copy extracted from this commit. With the operator's image tag moved to v1.67.0, `make update` exits 2 and names both versions. With the tags equal it passes. With the check's `exit 1` removed, the moved tag passes again, so the red comes from the check. Worth knowing when reading the mismatch path: `kubevirt-operator` deletes its download on a hash mismatch and this package keeps it. That is deliberate here, since reconciling a moved asset starts by reading it, but the two packages do read oppositely and neither file says so. ### Base and ordering This stacks on #4097 and should land after it. Both edit `packages/system/kubevirt-cdi/Makefile`, and this change builds directly on that one's edit to the same recipe, so it is based on the head of `fix/cdi-update-keeps-local-templates` rather than on `main`. GitHub computes mergeability against the PR's base branch, which here is that stack branch and not `main`. Both branches are rebased onto current `main`, which has not touched these paths since the previous merge-base. ### Screenshots No UI changes. ### Downstream repositories Walked the trigger map against the diff. Two entries come close and neither fires. The website triggers on developer tooling and package Makefiles, but `content/en/docs/next/development.md` documents `make update` only as "Update Helm chart and versions from the upstream source", which still holds, and its worked example is cilium, which this does not touch. Its `make test` line refers to the e2e sandbox in `packages/core/testing`, not per-package unit tests. `ccp` triggers on `hack/` layout and on what a make target does, and nothing under `hack/` is touched. Its `package-bump` skill already assumes a package's `update` recipe hardcodes the upstream version instead of resolving it at run time, and names cilium, kafka-operator, opensearch-operator and postgres-operator as examples. `kubevirt-cdi` was one of those counterexamples and now is not. Pinning `kubevirt-cdi-operator` the same way is #4190 and out of scope here. The skill handles vendored Helm charts, in-repo image builds and version enums, and a raw manifest download is none of those. Everything else in the map needs an app, a schema, an installer value, a variant or a release asset. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(cdi): `make update` in kubevirt-cdi no longer overwrites the hand-maintained CDI object. It pins the CDI release that object is reconciled against instead of resolving GitHub's releases/latest at run time, and verifies the fetched asset against a pinned sha256 without writing under templates/. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Reliability** * Updates now use verified, pinned CDI releases to prevent unintended manifest changes. * Failed or checksum-mismatched downloads no longer overwrite existing configuration. * **Bug Fixes** * Preserved local CDI customizations during release updates. * Added validation for CDI resources, access controls, upload-proxy routing, and TLS configuration. * **Tests** * Added automated checks for incomplete template refreshes and generated Kubernetes resources. * Added chart tests covering CDI behavior, clone permissions, and upload-proxy networking. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
b1519acfe7 |
fix(e2e): cap the bucket S3 reads so the verify step honors its timeout (#4281)
## What this PR does Refs #4268 — a plain reference, not `Fixes`: the issue asks for a `script` op to be cut off at its declared ceiling, and that is Chainsaw behaviour this repo cannot change. Chainsaw runs the op's shell under Go's `exec.CommandContext`, whose default cancel SIGKILLs the shell PID alone and then blocks on the inherited stdout/stderr pipe until an orphaned `mc` closes it, so the op returns only when `mc` exits on its own rather than at the ceiling. This PR mitigates that where it bit — per-command caps so no descendant outlives its own budget — but the op-timeout gap itself lives in Chainsaw, so the link stays a reference rather than a `Fixes`. The sibling step that produced the 606s, `verify-every-bucket-takes-a-write` in the `bucket-1-capacity` Test, is on `main` and already carries its own per-call caps; this PR applies the same shape to `verify-s3-access`. On run 34948279363 the bucket suite's `verify-every-bucket-takes-a-write` step declared `timeout: 5m` but ran 606s and ended on `mc`'s own non-zero status rather than being cut at the ceiling. `verify-s3-access` in `hack/e2e-chainsaw/bucket/chainsaw-test.yaml` is the same-shaped step in this suite. This PR wraps every S3 (`mc`) and apiserver (`kubectl`) read in `verify-s3-access` in its own `timeout`, so a hung read dies at a known budget instead of holding the step open, and corrects the two places (`hack/e2e-chainsaw/.chainsaw.yaml` and `docs/agents/e2e-testing.md`) that explained the op ceiling with a mechanism that does not hold. It caps `verify-s3-access` for sharing the shape, not for producing the 606s number. ### Which of the issue's two candidate mechanisms holds The issue named two candidates and stated the log could not separate them. The Chainsaw v0.2.15 source separates them, and it is the second one, with the exact refinement the issue proposed. **Candidate 1 — "an operation `timeout` does not bound a `script` op" — is refuted.** The op timeout is applied and does reach the process: `pkg/runner/operations/script.go:24` copies the step's `timeout` into `Timeouts.Exec`, and `:38` wraps the op context with `ctx, cancel := context.WithTimeout(ctx, tc.Timeouts().Exec)` before calling `op.Exec(ctx, …)`. So a 5m step produces a real 300s deadline on the context the command runs under. **Candidate 2 — "it bounds it and kills the shell, but Chainsaw waits on a pipe a grandchild (`mc`) still holds open" — is confirmed.** In `pkg/engine/operations/script/operation.go` the command is built with `exec.CommandContext(ctx, shell, …)` (`:105`), and in `execute` its stdout/stderr are set to `*bytes.Buffer` (`:128`–`:129`, the buffers are `internal.CommandOutput` in `pkg/engine/operations/internal/command_output.go:12`–`13`) and run via `cmd.Run()` (`:130`). Two Go `os/exec` properties then produce the overrun: `exec.CommandContext`'s default cancel is `Process.Kill()` on the shell PID alone — not a process-group kill, and the operation sets no `SysProcAttr.Setpgid` — and because stdout/stderr are not `*os.File`, `os/exec` allocates OS pipes and copier goroutines, so `cmd.Wait()` blocks until those pipes reach EOF. EOF needs every inherited write-end closed. When the shell forks `mc`, `mc` inherits those fds; at 300s the deadline SIGKILLs the shell but `mc` is orphaned, keeps retrying the S3 PUT, and holds the pipe, so the op does not return until `mc` exits on its own at 606s. `cmd.Run()` then returns `signal: killed`, which is stored in the `$error` binding and never printed — only the `($error == null): true` check failure is logged — which is why grepping the run for `signal: killed` found nothing. **Empirical confirmation.** A standalone Go program reproduces it exactly. `exec.CommandContext` with a 1s timeout, `*bytes.Buffer` stdout/stderr, running `sh -c 'sleep 2 & sleep 4'` (a descendant that inherits the pipe): `cmd.Run()` returns after ~4s, not 1s, with `err=signal: killed` and `ctx.Err()=context deadline exceeded` — the ceiling fired at 1s, control came back only when the descendant closed the pipe. Redirecting the descendants' stdout/stderr away from the pipe (`… >/dev/null 2>&1`) returns at 1.00s. That is the whole mechanism: the ceiling cuts the shell but not a descendant that still holds the pipe. ### The fix Per-command caps, the remedy the issue itself points to ("every read carries its own cap") and the shape already used elsewhere in the suite. The bucket step now defines a `k()` wrapper (`timeout 30 kubectl`) and an `mc()` wrapper (`timeout 15 mc`) for the reads, and calls `timeout 45 mc cp` directly for the two PUTs; a hung read is killed at its own budget, releasing the inherited pipe promptly, so even when the op ceiling fires the residual is one inner cap rather than `mc`'s full retry exhaustion. The terminal precedents for these budgets are the redis suite's terminal `k()` (30s, `redis/chainsaw-test.yaml:298`) and the sibling `verify-every-bucket-takes-a-write` step's terminal `mc cp` (45s, `:390`, sized for a volume grow); the 15s cap on `mc` in `hack/e2e-chainsaw/_lib/backup-access-preflight.sh` is not a precedent, because it sits inside a `while :;` retry loop where a 124 costs one attempt, whereas here under `set -e` a 124 ends the step. The inner budgets — 2×30 for the two `k` reads + 2×45 for the two `mc cp` PUTs + at most 7×15 for the other `mc` calls (eight other call sites, but the two cleanup `mc rm` are mutually exclusive) + the 30s port-forward readiness wait = at most 285s — sit under the 5m op ceiling, which stays as a backstop, per the "op above the sum of inner budgets" rule in `docs/agents/e2e-testing.md`. The long-lived `kubectl port-forward` keeps no cap by design (it must persist for the whole step, and its stdout/stderr are already redirected to `/dev/null`, so it cannot hold the step's pipe). The `mc()` wrapper is non-recursive because `timeout` execs its argument through a PATH lookup that bypasses shell functions — verified in `dash`, `bash` and `sh`. The `catch` block's diagnostic read gets the same treatment — `timeout 30 kubectl` under a 1m op `timeout` — for the same pipe reason. ### The readonly deny probe fails on a timeout The readonly-upload probe used to branch on `mc` directly (`if mc cp …; then`), which suspends `set -e`, so a `timeout` there (exit 124) was indistinguishable from the access-denied non-zero the probe expects: a 45s hang on the deny path would exit 124, take the false branch, reach cleanup and exit 0 — a long hang turned green, the exact window this PR exists to bound. This is the first `ro-user` write in the step, so no earlier call proved that path healthy. The probe now captures the status: exit 0 is the readonly regression, 124 is a timed-out probe, both fail; any other non-zero is the expected denial. ### No unit surface; why the evidence is the proof This is an e2e-config and doc-correctness change. The embedded Chainsaw script is not extracted into a unit harness anywhere in the tree, so there is no red-to-green unit test to add without inventing a bespoke script-linter (which would be a new checker standing in for the fix). The regression is instead pinned by the mechanism proof above: the source citations show the ceiling cannot cut a descendant that holds the pipe, and the caps remove the only way a descendant outlives its budget. The deny-probe status handling and the pipe-vs-file secret reads were exercised against stub exit codes (0 → regression-fail, 124 → timeout-fail, other non-zero → expected denial) and the whole script passes `sh -n` and `bash -n`. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. The change touches only `hack/e2e-chainsaw/` suite content and `docs/agents/e2e-testing.md`; it moves or renames nothing under `hack/`, and touches no node-prerequisite or `package.mk` path. No downstream repository is reached. - [x] No downstream repository is affected by this change ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved end-to-end S3 access verification so stalled Kubernetes API and S3 operations fail promptly. - Correctly distinguishes timed-out access checks from expected permission denials. - Preserves partial command output and allows subsequent test steps to continue after individual timeouts. - Applies separate time limits to bucket reads, uploads, and access checks for more predictable test behavior. - **Documentation** - Clarified operation timeout behavior and recommended per-command limits within the overall operation limit. - Documented timeout handling for orphaned processes and updated worst-case timing guidance. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
ec706c49b1 |
docs(e2e): correct the Chainsaw Test-ordering claim in etcd and clickhouse suites (#4277)
Fixes #4269 ## What `hack/e2e-chainsaw/etcd/chainsaw-test.yaml` explained its layout with a comment claiming Chainsaw executes Tests in **name order, not file order**, and used that claim to justify why teardown lives inside each scenario. The clickhouse suite had picked up the same claim. This removes the ordering claim from both — it is false at the pinned version — and re-grounds the etcd teardown justification in a reason that does not depend on order. ## Why the claim was wrong (evidence, Chainsaw v0.2.15 — the pinned version) At v0.2.15 Chainsaw guarantees **no order between Tests at all**: not file, not document, not name. Restating "name order" as "document order" is wrong for the same reason. Verified against the pinned source: - `pkg/runner/runner.go` — the runner registers each Test through `testing.T.Run` and the per-Test body calls `t.Parallel()` whenever `spec.concurrent` is nil or true. Neither suite sets `concurrent`, so both take that path. Go pauses those subtests and releases them together once the parent returns; they then race for the `-test.parallel` semaphore. - `pkg/runner/flags/flags.go` — `execution.parallel` maps only to `-test.parallel`, a cap on how many Tests run at a time, not an order. - `hack/e2e-chainsaw/.chainsaw.yaml` sets `execution.parallel: 1`, so Tests are serialised — one at a time — but not ordered. The E2E run on this PR's own head demonstrates it: discovery loaded the five selected Tests in document order, then ran `clickhouse-2` before `clickhouse-1` with both etcd Tests interleaved between the clickhouse pair. Serial, non-overlapping, and in an order that is neither file, document, nor name. ## The fix The teardown decision the false claim justified is still correct, for a reason that does not touch order. Each etcd scenario shares the singleton `etcd` release and its cluster-scoped Kamaji `DataStore`, runs with `skipDelete: true` (opting out of Chainsaw auto-cleanup), and v0.2.15 has **no test-level `finally`** — so a scenario is reclaimed only if it reclaims itself, and that is true of whichever Test runs last regardless of order. The etcd comment now says exactly this, with the ordering clause dropped rather than restated. The clickhouse header's ordering sentence is removed outright: the three Tests use distinct releases and share no state, so nothing there needed an ordering claim. This is a comment-correctness change with no runtime surface, so there is no unit test to add — the load-bearing claim was verified empirically against the pinned Chainsaw source, as above. A repo-wide grep confirms no other ordering claim survives in `hack/e2e-chainsaw/` or `docs/agents/e2e-testing.md`. Per `docs/agents/e2e-testing.md`: comment-only, no new retries, no traps, no assertion or teardown-timing changes. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Corrected end-to-end test comments to drop an incorrect test-ordering claim; Chainsaw v0.2.15 guarantees no order between Tests (not file, not document, not name). * Clarified that teardown behavior is independent of test execution order. * Documented the relationship between ClickHouse backup tests and the corresponding backup test suite. * No test behavior or user-facing functionality changed. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
9e0e1239da |
fix(kubevirt-cdi-operator): pin the vendored CDI release (#4383)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does This pins the CDI release the platform deploys, so it moves only when someone edits a version line. Until now `make update` in `packages/system/kubevirt-cdi-operator` took the release from GitHub's `releases/latest` redirect. Any refresh could move CDI and its image tags, and the diff had no version line to review. The fix follows the shape `packages/system/kubevirt-operator` already uses. - `RELEASE = v1.66.1` names the release. It is the one already vendored, so `templates/` does not change, and `make update` reproduces it byte for byte. - `MANIFEST_SHA256` pins the downloaded asset and matches the digest GitHub publishes for it. On a mismatch the recipe writes nothing. - The download is transformed and patched outside `templates/` and moved in at the end. A refresh that fails halfway leaves the vendored file as it was. - A new `test` target runs a helm-unittest suite over the vendored manifest, then greps it for the operator image at `$(RELEASE)`. The grep is there because helm-unittest cannot see a Makefile variable. Moving the pin without re-running `update` fails here. One thing is left for later. The suite asserts three of the eight image references directly, and the digest keeps the rest in step on `update`, but a hand edit to one of the other image env entries would pass. Closes #4190 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: Nothing on the trigger map applies. The diff touches one package's own Makefile, `.gitignore` and tests, and no shared tooling, values, CRD, namespace or release asset. ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(kubevirt-cdi-operator): pin the vendored CDI release to v1.66.1 and verify the downloaded manifest against a digest, so refreshing the package no longer moves the deployed CDI. The deployed version stays the same. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Updates** * The KubeVirt CDI Operator is now pinned to version 1.66.1, providing a consistent operator version across deployments. * **Reliability** * Operator manifest updates are checked before they’re applied, helping prevent incomplete or mismatched manifests from replacing the current version. * Deployment configuration is validated to ensure it matches the pinned release. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
b96c822bde |
docs(agents): give the HR-Ready budget derivation its own section
The checklist entry had grown into a derivation and buried the rule it states. The rule stays in the checklist and the derivation moves to a section of its own. The derivation under-budgeted in two ways. It stated the 10m install timeout as universal, while releases an app chart renders itself take their template's spec.install.timeout, then its spec.timeout, or helm-controller's 5m where neither is set. And it restated which kinds kstatus gates on, which is easy to get wrong, so it now points at the upstream table instead. The harbor comments now name the floor each of their asserts actually sits against, and the opensearch comment stops citing a 5-6m standard the checklist no longer states. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
4373c8c0f2 |
test(e2e): pin the mariadb first-boot wait budgets against the tree
The waits are right only relative to numbers that live elsewhere: the chart's failureThreshold, the install timeout cozystack-api stamps, and the Chainsaw op that runs the walkthrough. Deriving the floors from the tree rather than pinning literals means a change to any of them reds the contract instead of silently invalidating the waits. The extractors are tested against fixtures as well, because the tree cannot tell a correct extractor from a broken one: the chart carries a single failureThreshold and every wait clears every floor, so a reader of the wrong operand stays green. The wait_hr_ready exit branches are tested by behaviour against a stubbed kubectl, one stub per answer they have to tell apart, because a static check cannot see what errexit does to a failing read. That test and a byte-identity check cover every definition of the helper in the tree, found by grep rather than listed, so a new copy is picked up without editing the test. The redis and kafka-metadata definitions are named exceptions with their reasons in the header, and each is held to still differ, so an exception whose copy converges goes red instead of lingering. The suite runs under /bin/sh, which is dash on the runner, so the behavioural test runs its body under an explicit bash. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
957ddd7f25 |
fix(tests): raise the mariadb suite HR asserts past the install timeout
At 6m the HelmRelease-ready asserts expired before the release's own 600s install timeout, so a failed install was reported as a bare Chainsaw timeout with no reason. 11m lets the release record why it failed first. The round-trip op grows by exactly what the two walkthrough waits gained. Otherwise a failure at either wait could run past the op, which SIGKILLs the process group and loses the script's error. That bounds one failure, not the whole flow: a run slow at every stage that fails late still reaches the op, and bounding that needs a phase budget rather than a larger literal. The endpoint asserts stay. A freshly applied CR has no conditions and kstatus reads it as current, so the HelmRelease wait can end before any pod is up; an endpoint address closes that window. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
f629280944 |
fix(tests): raise the mongodb, rabbitmq and clickhouse HR waits
Those walkthroughs gave their database HelmReleases 300s, under the 600s install timeout the generated releases carry, so a failed install surfaced as a bare timeout rather than its own reason. None of these charts sets a startup probe, so 660s clears both floors, as it does for mariadb. Each Chainsaw op that runs one of the scripts grows by exactly what the waits it reaches gained, so it keeps the margin it had. The clickhouse op still sits above the sum of every inner ceiling. The rabbitmq one covers a failed install at the source wait, while the ceilings up to its target wait add up to more than the op, as they did before. The mongodb round-trip runs with SKIP_RESTORE, so only its source wait counts there. postgres keeps its 360s: whether a suite pays the extra minutes on every red run is a per-suite call, and this one is not made here. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
289acb2ed6 |
fix(tests): let the mariadb backup example cover a first boot
The walkthrough gave each MariaDB HelmRelease 300s, less than the release can legitimately take, so a slow first boot failed the harness and a failed install surfaced as a bare timeout with no reason. A wait here has two floors and must beat the larger. The chart grants a 310s startup budget, and the install can span it: helm-controller defaults to the poller strategy, and kstatus reads the operator CR's Ready=False as InProgress. The generated release also carries a 600s install timeout, and a wait equal to it expires first, because its clock starts at the apply and the install's only once the chart is fetched. The install timeout is the binding floor, hence 660s. On timeout and on Stalled, wait_hr_ready now dumps every condition and the release history: a scheduled retry can overwrite the Ready message before anyone reads it. It also stops folding different answers into one "(none recorded)" line. An absent release, a release deleted during the wait, a lookup kubectl could not answer, an empty history and a failed history read each say so, with kubectl's own stderr kept, because that message is usually the cause. A release seen earlier still gets its dump when only the last lookup failed. Every backup example whose wait_hr_ready carries this body takes the same change, so the copies stay byte-identical. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
eec069b94f |
docs(kubevirt): name every payload line the tail checks do not cover (#4340)
## What this PR does Follow-up to a review comment on #4254 that was right and landed after the merge. The sentence that PR left in `packages/system/kubevirt/Makefile` says the device and CPU payload lines stay outside the marker checks, which reads as if the rest are handled. The `- {{ . }}` entries inside the `extraFeatureGates` and `disabledFeatureGates` ranges are neither device nor CPU and are not covered either. Deleting either of them from a copy of the CR and running `make update CR_FILE=<copy>` exits 0 and leaves the range wrapped around nothing, with no check firing; dropping `- Template` from a clean copy fails the target as a control. The wording now names every uncovered payload line: the device tables, the `cpuAllocationRatio` value and both feature-gate entries, and keeps `migrations` as the one block validated whole. Covering those payloads in code is a wider change; happy to do it separately if that is wanted. ### Screenshots Not a UI change. ### Downstream repositories Comment-only change in one Makefile. - [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 update validation guidance for disabled-feature markers. * Documented partial-block recovery requirements and the workflow for restoring missing markers. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
c290dee945 |
fix(vm-instance)!: refuse resources that cannot size the VM
resources.cpu and resources.sockets are typed as quantities, but they land in domain.cpu.cores and domain.cpu.sockets, which KubeVirt declares as uint32. The chart cast them with int64, which turns 500m, 1.5 or 1e1 into 0 without an error, and KubeVirt boots a zero core count as one core. The user asked for half a core and got a one-core VM with nothing reporting it. The raw spelling is now decoded as YAML, the way an integer field reads it, and the render stops unless the result is a positive whole number that fits in uint32. A leading zero is refused, because YAML reads 010 as octal 8. The check keys on presence rather than truthiness, so an unquoted 0 is refused instead of being read as unset. Memory gets the same presence check. A quoted "0" was truthy and rendered requests.memory: "0", while an unquoted 0 was falsy and dropped, so one quantity took two paths. A zero or negative amount now stops the render in both spellings. BREAKING CHANGE: values that used to render now either fail or render differently. A cpu or sockets that does not decode to a plain whole number within uint32 (a millicore, suffixed such as 1k, fractional, zero, negative or leading-zero value), and a zero or negative memory, fail the next upgrade of the release until the value is fixed. This includes cpu or sockets set on its own beside an instanceType, which used to be ignored whatever it held. A spelling that the int64 cast truncated to 0 but that YAML decodes to a whole number, such as the exponent 1e1 or the trailing dot 2., used to boot one core and now sizes the VM with the count it spells, so such a VM changes size on upgrade. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
89da4a121c |
fix(cdi): stop make update from deleting first-party templates (#4097)
## What this PR does `make update` in `packages/system/kubevirt-cdi` began with `rm -rf templates` and then refetched one file, `cdi-cr.yaml`. Nothing in that directory is purely vendored. Two of the three manifests, the upload-proxy Ingress and TLSRoute from the Gateway API work, are ours and no fetch recreates them, so the next CDI bump would have dropped upload-proxy exposure from the chart and still exited 0. The third is `cdi-cr.yaml` itself, which despite its name carries Helm templating, the `ExpandDisks` gate, a toleration and the two RBAC documents that preauthorize cloning from `cozy-public`; the fetch overwrites it, and removing the wipe does not change that. That recipe now opens with `mkdir -p templates` and deletes nothing, which is where `packages/system/kubevirt-operator` landed at the last KubeVirt bump. The wipe bought nothing there anyway: `wget -O` truncates the file it writes. `kubevirt-cdi-operator` has the same recipe shape, and the same edit there would be a regression rather than a fix. It applies two patches, and since #3920 moved them to `-p4` they resolve, so a hunk upstream has moved out from under leaves `templates/cdi-operator.yaml.rej`; its `awk -i inplace` step leaves `templates/cdi-operator.yaml.gawk.XXXXXX` if gawk dies abnormally. Helm renders every file under `templates/`, and I reproduced both: the reject fails the package with a YAML parse error, and the gawk temp renders a duplicate object at exit 0. The wipe is what takes both, so removing it for symmetry would trade a latent loss for a live one, and the quieter of the two is the one that survives. Deleting the wipe there wants the recipe restructured to transform outside `templates/` and move the result in once, which is what `kubevirt-operator` does today and what its own comment gives this exact reason for. That is a larger change than this one and belongs on its own; #4383 makes it. Meanwhile the scan below covers the package: I added a first-party file to its `templates/` and the guard reported it, so the day that package grows one, this fails rather than the file being lost. `hack/package-update-templates.bats` guards this across the tree rather than in these two packages. It scans every package Makefile and fails when a recipe that clears its own `templates/` leaves behind a tracked file it never names. Put the removal back into `kubevirt-cdi` and it goes red naming both upload-proxy files. A file counts as named only outside the Makefile's own `NAME` and `NAMESPACE`, however the assignment is written. The match is on the variable name rather than on a list of spellings: the tree already carries three of them, `export NAME=`, the bare `NAME=`, and `NAME := ` in `packages/core/testing`, and the hole is identical in each. Without that, a package called `foo-bar` satisfies the check for its own `templates/bar.yaml` through its identity alone. Nothing the scan reaches is exempted that way as the tree stands, so the exclusion is there for the first package that would be: rename the asset in `kubevirt-cdi-operator`'s recipe and put the removal back, and the guard reports the orphaned template only with the exclusion in place. The two variables are matched by name rather than by a `[A-Z_]+=` shape, because a shape that broad is one unspaced assignment away from swallowing a variable that does name templates, which is how `etcd-operator-crds` names all five of its CRDs. A control test runs the same helpers over a fixture broken the same way, over one whose name swallows a template stem, then over that fixture rewritten to spell the removal with a trailing slash, to take one file instead of the directory, and to drop the removal entirely. Both cases pass under `hack/cozytest.sh hack/package-update-templates.bats`. Two limits, both written into the file's header. The check fires on the consequence rather than the shape, so `rm -rf templates` in a package that does refetch everything stays green, which is what keeps `kubevirt-cdi-operator` green for as long as its recipe wipes. And it only looks at a recipe clearing its own directory: `cert-manager` clears `../cert-manager-crds/templates/*` across the package boundary, which needs the removal's target resolved rather than the wiping package's own, so that directory is unguarded. The other packages that clear their own `templates/` refetch all of it and are untouched here. Two more limits the header names. The scan reads the text of each package's own `update:` recipe, so a removal made elsewhere is out of range: in a prerequisite target, behind an include, or behind a recursive make. `capi-operator`, `clustersecret-operator`, `openbao` and `reloader` write `update: clean <name>-update` and take both recipes from `hack/package.mk`, which removes nothing under `templates/` today. And the glob stops at `packages/*/*/Makefile`. The package Makefiles off that depth are the group aggregators such as `packages/system/Makefile` and the vendored kamaji subcharts, and none of them declares `update`. One behaviour of the matcher worth naming, and deliberately not changed here. `wipe_command` evaluates `/^update:/` ahead of its terminator, so a second `update:` in the same Makefile re-arms the scan instead of ending it, and what that means depends on what sits between the two: adjacent or separated by a blank line, both recipes are read; separated by another target, only the first is. The direction that matters is a false positive rather than a miss. A Makefile whose first `update:` clears `templates/` and whose second does not would be reported by this scan, while make runs only the second, so the package fails over a recipe that never executes. No package in the tree defines `update:` twice today. Three more notes, all non-blocking and none acted on. The header says what the scan leaves uncovered is a recipe clearing someone else's `templates/`; that is the uncovered set for *deletion*, and `kubevirt-cdi` is green under the scan while still losing `cdi-cr.yaml`'s content to the in-place `wget -O` on every run, which the Makefile comment records but the header does not. The identity strip has a false-positive direction the header does not argue: a wiping package writing `templates/$(NAME).yaml` would be reported even though the recipe writes it, which no package does today. And the fixture stages `.gitkeep` with `git add --all`, so a developer whose global excludes list that name would get a red for environment rather than defect; `--force` would remove the dependency. The stem match is generous, and the helper's comment now names the two live shapes. A sibling name: `templates/crds.yaml` next to the existing `templates/crds-experimental.yaml` on `gateway-api-crds`. A mention from an unrelated context: `templates/sidecar.yaml` on `objectstorage-controller`, whose Makefile says `sidecar` across its image targets while its recipe clears `templates/` and writes only `templates/controller.yaml`. I fed both to the helper and both come back exempted; a stem the Makefile never mentions is still reported. The duplicate `test:` target in this Makefile is #4157 and stays there. It sits outside the hunks this diff touches, so removing it here would be reaching past what this change is about. One more note, not a defect. Where the comment points at `kubevirt-operator` as the shape that closes the transient-file class, `packages/system/kubevirt/Makefile` is the closer analogue for the `cdi-cr.yaml` half specifically: its `update` downloads nothing and re-applies the parameterization onto a hand-maintained CR with a per-marker self-check. And the failure message offers a third option, staging into a tmpdir the way `etcd-operator-crds` does, in a parenthetical that reads as a gloss on the option before it. What this PR does not fix, and what does. `make update` here still overwrites `templates/cdi-cr.yaml`, which the Makefile comment says plainly. A wholesale overwrite is loud: upstream's object carries neither `podResourceRequirements` nor `cloneStrategyOverride`, and `tests/importer-resources_test.yaml` and `tests/clone_strategy_test.yaml` pin both, so `helm unittest` goes red. The silent case is a partial hand reconciliation that keeps those two and drops something else, because nothing here pins the `ExpandDisks` gate, the `CriticalAddonsOnly` toleration, the `uploadProxyURL` override or the two RBAC documents that preauthorize tenant cloning from `cozy-public`. #4107, stacked on this branch, stops `update` writing the file at all and adds `tests/local_templates_test.yaml`, which pins those. One limit the header does not name, left alone here. The comment strip is anchored to the start of a line, so a trailing comment on a recipe line survives into the searched text and counts as naming the file it mentions. I confirmed it by feeding `wget ... -O templates/cdi-cr.yaml # local-extra.yaml is written by hand` through the filter. Nothing in the tree is written that way, and the header only claims comment lines are dropped, which is what it does. `sed 's/#.*//'` in place of the `grep -v` closes it if it ever matters. Closes #4028. ### Screenshots No UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. Two entries come close: the website's "developer tooling (the package `Makefile`s)" line, and ccp's "move or rename anything under `hack/`". Neither fires. `make update` still behaves as the website's development page describes, nothing under `hack/` moved or was renamed, and the two files ccp's skills key on (`hack/package.mk`, `hack/common-envs.mk`) are untouched. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(cdi): `make update` in kubevirt-cdi no longer deletes its first-party templates ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Updated Kubernetes CDI package updates to preserve existing template files while refreshing downloaded manifests. - Prevented update operations from clearing first-party and vendored template content. - **Tests** - Added safeguards to detect package update recipes that could remove tracked templates without restoring them. - Added Helm unit test support for the Kubernetes CDI package. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
8d37e0b950 |
docs(hack): drop a false universal from the nats-rd secrets check (#4390)
## What this PR does The header of the nats-rd secrets check explained its assertions with a claim about every `-rd` package: none has a tests directory or the `test` target that the helm-unittest runner looks for. `postgres-rd` has both now, so the claim told the next reader that the helm-unittest route is closed for this class of package when it is open. The claim is removed. The assertions rest only on `nats-rd` itself having neither, which is still true, so they stay as they are. Moving them to helm-unittest is a separate question and is not part of this change. Comment-only change. Closes #4209 ### Screenshots No UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note docs(hack): remove a false claim about -rd package tests from the nats-rd secrets check comment. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated a test-file comment to clarify that the inlined ResourceDefinition chart lacks a `tests` directory and the test target used by the Helm unit test script. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
ce35da451f |
docs(api): separate the healthCheckExprs failure modes (#4387)
## What this PR does The `healthCheckExprs` field doc said any bad expression makes the HelmRelease hang until its timeout. That is true for some mistakes and wrong for others, and the text ships in the CRDs, so `kubectl explain` sends people to the wrong place. The doc now lists the cases separately, checked against the controller versions this repo installs (helm-controller v1.5.0, fluxcd/pkg runtime v0.100.2, Helm v4.1.1). An expression that does not parse, an empty `current`, or a duplicate group and kind stall the release right away with reason `InvalidCELExpression`, and no Helm action runs. An expression that errors on the object keeps the wait polling until the timeout with the CEL error in the message. A `current` expression that evaluates but never turns true also runs the timeout out, with no error to point at it. A wrong group or kind leaves the expressions unused. Two details go against the issue text. Reading `status` before the CR's controller writes one errors even inside `has()`, as the issue says, but the object is only Unknown on that poll, and the wait still succeeds once a status shows up. A CRD that is not installed does not hang the wait: Helm fails to build the manifest and the action fails before any wait starts. Doc comments and the regenerated CRD YAML only, no behaviour change. Closes #4154 ### Screenshots No UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note docs(api): describe each healthCheckExprs failure mode separately in the ApplicationDefinition and PackageSource field docs. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Expanded the health-check expression guidance to explain how invalid or duplicate expressions, evaluation errors, expressions that remain false, unmatched resource types, and missing CRDs affect Helm releases. * Clarified that health-check expressions are not evaluated when Helm action waiting is disabled. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
ddcc317603 |
docs(api): stop listing helm-install-timeout carriers in rest.go (#4381)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does A comment in the Application to HelmRelease conversion listed which kinds set the helm install timeout annotation, and that list was already wrong. It named two kinds while more set `release.cozystack.io/helm-install-timeout` today, and nothing checks it, so it goes stale every time another kind opts in. The comment now says where the annotations live, on the kind's ApplicationDefinition, and when a kind should set them. Each kind that sets the annotation already explains why next to it in its cozyrds manifest, so nothing is lost. Comment-only change. Closes #4147 ### Screenshots No UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note docs(api): drop a stale list of helm-install-timeout carriers from a code comment. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified where per-application Helm timeout annotations are defined. No user-facing behavior changed. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
d48d3ecc91 |
test(console): add coverage for enrichSchemaWithEnums (#4186)
## What this PR does backup-utils.ts (in the in-tree console dashboard) had no test file at all, despite sibling files in the same directory (dynamic-options.test.ts, crd-option-sources.test.ts, immutable-paths.test.ts) establishing that convention. Adds backup-utils.test.ts covering enrichSchemaWithEnums: the non-object passthrough, a path with no match in the enum map, a top-level enum attach, a nested one, and that the caller's path array is not mutated (verified by reading the function, it builds a new array via spread on every recursive call rather than mutating the one it receives). Test-only change, no production code touched. Verified with vitest run src/lib/backup-utils.test.ts (5 tests pass) and tsc --noEmit (no new errors). ### Screenshots N/A, test-only change, no UI impact. ### Downstream repositories Walked the trigger map in docs/agents/contributing.md against the diff. This is a single new test file for a pure function, no production code, schema, API, CRD, annotation, or naming changed. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Tests** - Added coverage for schema enum enrichment, including top-level and nested paths. - Verified that unrelated schemas remain unchanged. - Confirmed unmatched paths and caller-provided path arrays are handled safely. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Yan Bondarenko
|
b6ed32c441 |
docs(kubevirt): name the disabledFeatureGates entry among unchecked lines
The disabledFeatureGates range renders each gate a user adds through a
`- {{ . }}` line with the same shape as the extraFeatureGates entry, and
no check looks at it either: deleting it from a copy of the CR leaves
`make update` exiting 0 and the configured gate silently stops taking
effect. The comment listed the extraFeatureGates entry alone, so it read
as complete while one uncovered line was missing from it.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
|
||
|
Yan Bondarenko
|
63c9271568 |
docs(kubevirt): name every payload line the tail checks do not cover
The sentence #4254 left behind says the device and CPU payload lines stay outside the marker checks, which reads as if the rest are handled. The extraFeatureGates entry is neither device nor CPU and is not covered either: deleting ` - {{ . }}` under its range on a copy of the CR and running `make update CR_FILE=<copy>` exits 0 and leaves the range wrapped around nothing, with no check firing. The sentence it replaced covered that case. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Aleksei Sviridkin
|
1071802e3f |
test(e2e): pin the release-history read against the upstream type (#3607)
## What this PR does
`hack/remediation-guard.bats` carried a test called "status.history
extraction pins HR v2 status.history shape". It wrote a HelmRelease
document containing the field names it expected, then read that same
document back with the same jsonpath through `yq`. What it established
is that `yq` can read a fixture the test itself composed.
Its own comment named the failure it was there to catch: a future Flux
release renames the field, the jsonpath matches nothing, the guard
reports no remediation cycle, and real cycles pass the e2e assertion. A
rename moves neither the fixture nor the expression, so the test stays
green through exactly that event. The comment also named where the truth
lives, `github.com/fluxcd/helm-controller/api/v2`, and then pinned a
copy of the author's expectations instead.
`internal/fluxcontract` supplies neither half. The expression is read
out of `hack/e2e-chainsaw/_lib/remediation-guard.sh` at test time, so it
is the one that ships rather than a copy. The object is a
`helmv2.HelmRelease` built from the upstream type at the version
`go.mod` holds, serialized and matched through
`k8s.io/client-go/util/jsonpath`, which is the engine kubectl's `-o
jsonpath` uses. A renamed or retyped field is then a compile error or a
failed match, and a guard that stops reading history fails the lookup
with a message saying so.
That is one test. Two more exist so that it reads the expression kubectl
actually receives, because the expression now lives in a named
assignment the test sources and the caller expands: between the value
checked here and the value that runs there is a shell variable, and each
step of that indirection can fail while the suite stays green. The first
follows the caller's own call: it reads the local handed to
`helmrelease_has_remediation_cycle`, requires that local to be assigned
exactly once, and requires that assignment to take the expansion of the
shared name rather than the bare name, which matters because kubectl
echoes a brace-less template back verbatim, so a read that lost its `$`
returns its own text, clears the script's empty-history check, and
matches no status. Starting from the call rather than from the text is
what makes the pin describe the read that runs: asking only whether a
well formed read exists somewhere in the file is satisfied by a second
assignment overwriting the same local, and by a read filling a local the
guard never consumes. The second holds that no shell library but the one
that owns the name assigns it, which does not follow from the first: an
assignment beside the read satisfies the first and still replaces the
value after this file has approved the one the library exported. It
covers every library in that directory rather than the caller alone,
because the caller sources three of them and the owning one first, so an
assignment in any of the others lands later and wins.
Those two match text with a pattern, and each carries both halves of its
contract. A refuse table lists the forms the pattern must reject, and an
accept table lists the forms it must keep accepting, one case per
freedom the code claims: for the call, the `if` prefix, the negation, an
expansion with or without braces; for the read, the spelling of `-o`,
the quoting of the substitution, an expansion with or without braces,
and the local's name, which stays free because the call and the read are
renamed together. Without the accept side those freedoms survive only
until someone tightens the pattern, and nothing reddens when they go.
Two boundaries, named rather than left to be found. The pin holds the
shape of the read, not the vocabulary of the answer: the guard greps for
`failed` and `uninstalled`, and the test supplies those two strings
itself, so a release status renamed upstream leaves the jsonpath
returning strings, the test green because it wrote them, and the shell
grep silently matching nothing. Those values come from Helm's release
status rather than the Flux type, so pinning them reaches past what this
change binds, and there is more than one way to do it. Second, a literal
history read that fills a local the guard never consumes is not caught,
and on its own that is harmless: the guard still runs the shared
expression, and the moment it stops doing so the read filling the
consumed local no longer matches and the caller test fails.
The caller-side half is a heuristic and is meant to be read as one. It
decides whether a piece of text is the read by matching it, and a
matcher that reasons about shell is wrong at some spelling by
construction, which is the same objection the file raises against
reading the library's assignment as text instead of sourcing it.
Sourcing is available for the library and not for the caller, so what is
left is to make the imprecision run one way: the count is of
occurrences, so an extra one reddens and a missed one cannot pass, and a
form outside the accepted set fails with the pattern printed rather than
quietly stopping asserting.
That direction is the whole argument for keeping it. A false red is a
broken build on a legitimate rewrite, and the failure names what it did
not recognise, so it costs a minute. A silent green is a guard that
asserts nothing for as long as nobody looks, which is the state this
branch exists to end. The ways the caller can break without anyone
noticing are worth that trade. A read that lost its `$` hands kubectl a
template with no braces, which comes back as the variable's own name; a
hand-written path that is valid but names the wrong field comes back as
that field's values. Both are non-empty, so the script's own
empty-history backstop stays quiet and the cycle grep matches nothing. A
deleted call is quieter still, since there is then no assertion to fail.
None of the three is visible to the library-side pin, which judges the
value the library hands out and never learns whether it reached kubectl.
What does fail loudly is an expression kubectl cannot use at all, and
that is the one case needing no pin.
The shape the fixed test now has already exists in this tree, in
`hack/seaweedfs-naming-audit.bats`, under "the audit's reconstruction
agrees with the chart helper it mirrors". That one also writes a
heredoc, which is what makes the pair worth comparing: it copies the
real `_naming.tpl` out of the chart, renders it through `helm`, and
checks the audit script's output against the render. The heredoc is a
harness for calling the real helper, not a record of what the author
expected it to say, and the two sides have different authors. A test
that writes both sides proves only that it is self-consistent.
Nothing is lost by the deletion. The removed test's tail fed extracted
statuses into `helmrelease_has_remediation_cycle`, and the seven tests
remaining in the bats file `hack/remediation-guard.bats` still cover
that, including `uninstalled then deployed still returns detected`. The
extraction half is now pinned more strictly than before: the old test
asserted only that the output was non-empty and contained one line, the
new one asserts the exact two bare tokens, so dropping the `{"\n"}`
separator also goes red.
CI runs it. `make test-controllers` is its own step in
`.github/workflows/pull-requests.yaml` and runs `go test ./internal/...
-count=1`, which picks the new package up without registration.
### Screenshots
Not a UI change.
### Downstream repositories
Walked the trigger map against the diff. It adds a Go test package under
`internal/`, deletes one test from `hack/remediation-guard.bats`, adds a
variable to `hack/e2e-chainsaw/_lib/remediation-guard.sh`, points the
kubectl invocation in `hack/e2e-chainsaw/_lib/run-kubernetes.sh` at that
variable, adds a sentence to `docs/agents/e2e-testing.md`, and records
`make test-controllers` in `docs/agents/overview.md`. No package is
added, renamed or removed, and nothing touches a `values.schema.json`, a
version enum, a `values.yaml` default, an `ApplicationDefinition`, a
CRD, a namespace, an annotation or a telemetry metric.
`hack/e2e-prepare-cluster.bats` is untouched, so the node contract that
`talm` and `ansible-cozystack` restate by hand does not move.
The closest call is `ccp`, whose skills anchor on files under `hack/`.
Its trigger is a move, a rename, or a make target that does something
different. This changes the contents of three files, not their names,
and no target behaves differently; `hack/package.mk` and
`hack/common-envs.mk` are untouched.
- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
### Release note
```release-note
NONE
```
|
||
|
Aleksei Sviridkin
|
90a1a947c5 |
feat(kube-ovn): bump kube-ovn to v1.15.26 (#4331)
## What this PR does Bumps kube-ovn from `v1.15.10` to `v1.15.26` (package `cozy-kubeovn` `0.38.0` -> `0.39.0`). The motivation is a cold-start failure mode in the OVN NB/SB raft leader checker. In `v1.15.10`, `checkDBClusterIntegrity` (`pkg/ovn_leader_checker/ovn.go:336`) called `os.Remove` on a healthy local raft DB (`ovn.go:367`) whenever a leader observed fewer members than expected. That count drops below the target transiently during cluster growth, a rolling restart, or recovery after a node address change, so the check could delete a valid DB and leave `ovn-central` unable to elect a leader. `v1.15.26` removes that function entirely, so the DB is no longer deleted. It is a removal of the faulty check, not a grace-period around it. The chart is re-vendored with `make update`, not hand-edited, so the templates match the image: - `charts/kube-ovn` is re-pulled from the `v1.15.26` tag and `patches/mtu.diff` is re-applied; the image build args are regenerated. - This brings the whole v1.15.26 chart: the reworked master-node affinity helpers, the widened kube-ovn CRD schema (`bandwidth.ingress`/`egress` from `integer` to int-or-string, `u2oFeatures` added), and the new `OVN_CENTRAL_MODE` (`cluster`/`single`) support with its `central-svc`, `central-pvc` and `central-hcp-rbac` templates. `OVN_CENTRAL_MODE` defaults to `cluster` (raft on hostPath), so an in-place upgrade keeps the existing topology. - Two vendored patches now included upstream in `v1.15.26` are dropped: `fix-keepvmip-portgroup.diff` (`hasAliveSiblingVMPod`, `pod.go:2437`) and `fix-vmim-scheduling-retry.diff` (scheduling requeue, `kubevirt.go:195`). `fix-vmim-target-pod-retry.diff` is kept: `v1.15.26` still returns nil at `kubevirt.go:215`. The image also picks up `kube-ovn-bfdd-supervisor`, which v1.15.26 adds (`makefiles/build.mk:33`) and upstream's `dist/images/Dockerfile:12` copies into `/kube-ovn`. `pkg/controller/vpc_egress_gateway.go:47` hardcodes that path for the `bfdd` container a `VpcEgressGateway` with `spec.bfd` gets, so a copy that stops at `kube-ovn-controller` ships a container that cannot exec. `hack/check-kubeovn-image-copies.sh` compares what our Dockerfile and upstream's copy into the image, `make update` runs it against the tag it pulls and `hack/check-kubeovn-image-copies.bats` runs it on every CI run, because `make update` otherwise rewrites only the ARG lines and the copy drifts at each bump. The `multus` commit moves the `kube-ovn-cni-plugins-checked-against` marker to `v1.15.26`. `CNI_PLUGINS_VERSION` is `v1.9.1` in kube-ovn's base image at both releases and in multus, so portmap and macvlan stay in lockstep; the marker is the only change. The image ref stays digest-pinned (`repository` + `tag@sha256`), which the promote-retag release contract requires for a cozystack-built image. The digest is carried over from `v1.15.10` as a placeholder and is refreshed to the real `v1.15.26` image by the release-candidate digest-vendoring step; no PR or main job rebuilds it. Verification: `helm unittest .` passes (22 tests), `helm template .` renders clean, the `cni-plugins-staging-contract` and `promote-retag` unit contracts are green, and the image-build `TestVMIMigrationSchedulingRetry` gate passes 6/6 on the patched v1.15.26 tree. > [!NOTE] > `start-db.sh` was substantially reworked upstream between `v1.15.10` and `v1.15.26`. The privileged fork E2E run installs into a sandbox and passes, but the in-tree E2E job is skipped on a fork PR, and a fresh install is not an upgrade. An in-place upgrade on a multi-node control plane (image rebuild plus OVN raft re-convergence) is still untested before merge. ### Screenshots N/A: no UI changes. ### Downstream repositories Walked the trigger map against the diff. The change is confined to `packages/system/kubeovn` (re-vendored chart, image build args, version pins) and a one-line marker in `packages/system/multus`. It does not add, rename or remove a package, does not touch platform values, variants, components, release assets, `ApplicationDefinition` semantics, the Talos version, or any hand-typed CRD schema in a downstream provider. No downstream trigger matches. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note feat(kube-ovn): bump kube-ovn to v1.15.26, which removes the OVN leader-checker integrity check that could delete a healthy raft DB and prevent ovn-central from electing a leader after a node address change or rolling restart ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added hosted control-plane support for OVN Central with StatefulSet deployment, persistent storage, service exposure, and dedicated RBAC. - Added split-cluster and data-plane-only configuration with external OVN endpoints. - Added configurable TLS, leader election, service, and storage settings. - Enhanced CRDs with overlay-only routing and flexible bandwidth quantities. - **Bug Fixes** - Improved VM migration retries during scheduling and preserved connectivity during live migration. - Improved upgrade handling and service configuration validation. - **Updates** - Upgraded Kube-OVN components and images to version 1.15.26. - Documented required OVN IPsec directory preparation for read-only filesystems. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
0f21d06520 |
fix(dashboard): raise the console nginx header buffers for large OIDC tokens (#4376)
## What this PR does A user whose OIDC access token carries many group claims cannot use the console: nginx answers `400 request header or cookie too large` before the request reaches the apiserver proxy. The config baked into the console image kept the nginx defaults, 1k for `client_header_buffer_size` and four buffers of 8k for `large_client_header_buffers`, while the token arrives as one Cookie header several kilobytes long. A header field has to fit into a single buffer, so the per-buffer size is what decides this. Both directives are now 16k, at the top level of that config rather than inside the server block: a header field read before the `Host` header has selected the virtual server is measured against the default server's buffers, so the same lines inside a server block cover that server only while it stays the default one. Measured in the pinned nginx base image: a 12 KB cookie gets 400 on the old config and 200 on the new one, a 20 KB one still gets 400. `nginx -t` accepts the result. This hop sees more than the browser sent. The gatekeeper in front of it runs with `--pass-access-token=true` and `--pass-authorization-header=true`, so the request arrives carrying the `kc-access` cookie, an `Authorization: Bearer` header and `X-Forwarded-Access-Token`, three separately bounded header fields where the edge saw one. This is the console image only. The platform ingress-nginx controller also runs on the defaults, which is #2853. Worth reading that trace again before fixing it there: the controller logged `upstream_status 400`, so it forwarded the request and the 400 came back from behind the gatekeeper, which is this layer. The check in `hack/console-nginx-header-buffers.bats` reads the config out of the Containerfile heredoc and fails when either directive is missing, below 16k, or no longer ahead of the first server block. That config lives inside an image build, so no chart template and no helm-unittest case can reach it. `make unit-tests` picks the file up on its own. No image reference changes here. CI stamps the console digest. Closes #3113 ### Screenshots No UI change. This is the nginx config in front of the SPA. ### Downstream repositories The diff is the console image's nginx config plus one new `hack/*.bats` file. `cozystack-ui` is archived and the console lives in this repo, so a console change belongs in this PR. Nothing under `hack/` moved or was renamed, and no make target changed behaviour. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(dashboard): the console no longer answers `400 request header or cookie too large` to users whose OIDC access token carries many group claims ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Increased the console’s NGINX request-header buffer capacity to support larger header fields and request headers. * Improved handling of requests with headers that previously exceeded the default buffer limits. * **Tests** * Added automated checks to verify the console’s NGINX header-buffer settings remain correctly configured. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
8c5126ccd9 |
fix(kubernetes): give System-mode tenant OIDC users a UID (#4379)
## What this PR does Users authenticated through `spec.oidc.mode: System` on a tenant Kubernetes cluster now get a UID. The AuthenticationConfiguration the chart renders mapped `username` and `groups` and nothing else, and with no `claimMappings.uid` entry the apiserver resolves such a user to an empty UID. The `X-Remote-Uid` header the tenant control plane publishes then reaches an aggregated API server carrying nothing. The mapping takes the `sub` claim. The username claim is `email`, and the platform's Keycloak realm turns on the UPDATE_EMAIL required action, so that value is one a user changes; `sub` is the account UUID and stays where it is. Users on a cluster that is already running gain a UID where they had none. Tenant RBAC does not notice, because a role binding matches a username or a group. Audit records on the tenant apiserver do carry `user.uid` now, `kubectl auth whoami` prints it, and an admission webhook or extension server inside the tenant that read the empty value reads the subject instead. `CustomConfig` mode stays as it was. An operator-supplied configuration is written verbatim, uid mapping or not, and a unit test pins that the chart adds none of its own. Closes #4166 ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map against the diff, which touches the tenant chart's OIDC template, its unit tests, `docs/oidc-tenant.md`, and the shared e2e runner library with its bats twin. `values.schema.json` does not change. No values key is added, renamed or removed, no default moves and the version enum is untouched, so the terraform-provider's hand-written schema and its expand/flatten pair stay correct. No package is added or renamed and no `ApplicationDefinition` changes, so the website's app lists and generated reference pages are unaffected, and its tenant OIDC page carries no claim about UIDs that this could make stale. The installer values, the platform values and the variant list are untouched, so the ansible role is unaffected. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(kubernetes): users authenticated through `spec.oidc.mode: System` on a tenant Kubernetes cluster now carry a UID, taken from the issuer's `sub` claim, where they previously had an empty one. Audit records, `kubectl auth whoami` and the `X-Remote-Uid` header that reaches an aggregated API server inside the tenant all show it. RBAC is unaffected: a role binding never matches on a UID. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * System-mode OIDC authentication now provides a stable user identifier based on the account subject. * Existing clusters now expose this identifier in audit records, `kubectl auth whoami`, and to extension servers without changing tenant RBAC behavior. * Custom OIDC configurations continue using operator-defined UID mappings. * **Documentation** * Updated OIDC documentation to explain stable identifiers and their visibility. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
cc602f0dca |
docs(hack): drop a false universal from the nats-rd secrets check
The justification for the two nats-rd assertions ended with a claim about every -rd package: none carries a tests directory or the test target the helm-unittest runner looks for. At least one does now, so a reader deciding where to put the next -rd assertion was told the helm-unittest route is closed when it is open. The assertions rest only on nats-rd lacking both, which still holds, so the claim goes and the assertions stay as they are. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
225b639c18 |
fix(cdi): stop make update from overwriting the first-party CDI object
templates/cdi-cr.yaml is ours. Against upstream's bare CDI object it adds Helm templating for cloneStrategyOverride, uploadProxyURL and the worker-pod podResourceRequirements, the ExpandDisks feature gate, a CriticalAddonsOnly toleration, and the two RBAC documents that let a tenant clone from the golden-image namespace. Five of those arrived by hand. The toleration did not: it came in with the first commit, which was byte-identical to upstream v1.58.0, and upstream dropped it at v1.64.0 while this object kept it. That is the same divergence `WebhookPvcRendering` records, running the other way. Either way `wget -O templates/cdi-cr.yaml` was not refreshing a vendored file, it was overwriting a first-party one -- the same loss as the `rm -rf templates` this recipe stopped doing, arriving through the fetch instead of the wipe. What replaces it: the release the object is derived from is pinned rather than resolved from GitHub's releases/latest redirect, and `update` fetches it only to confirm the bytes still hash to a measured sha256. Nothing is written under templates/. The next CDI bump that changes the asset fails that check, which is the point -- upstream moving its own defaults is when the local copy needs a human to reconcile it, not an overwrite. The download survives a mismatch, because reconciling one starts by reading it. RELEASE is v1.66.1, which is the release the operator manifest vendors. ../kubevirt-cdi-operator decides what the platform deploys, and it can move to another release without this file changing. RELEASE is still set by hand rather than derived, because a derived value is not a pin. What `update` adds is a comparison with the cdi-operator image tag that package vendors, failing when the two differ; without it the recipe's closing line would name a release the platform may no longer run. The difference from upstream is stated against v1.66.1 rather than as a standing fact: upstream carried WebhookPvcRendering from v1.63.0 through v1.65.0 and dropped it by v1.66.0, and this object never carried it, so the difference ran both ways for three releases and one way after. The comment says to re-derive it at each bump for that reason. The `test:` target this package needed arrived separately, so what it gains here is the `.PHONY` line that came without: hack/package.mk declares only the targets it defines itself, so a file or directory named test or update beside this Makefile would otherwise make that target up to date and skip the recipe. The package also carried a second, identical `test:` target, so make warned on every invocation in it; that one is gone, and the comment above the remaining target is what should stop another being added. The suite is what makes the local delta checkable rather than described, which is why the comment above `update` no longer lists what the object adds: two copies of that list would diverge, and only one of them can fail. hack/package-update-templates.bats attributed `@#` recipe comments to one named package. The tree writes them in more than one, and this adds another, so the attribution goes; the case below it exercises both spellings without naming who writes which. The header in that file also lost its two-item example of first-party manifests. The list named the two the recipe never wrote, and this change stops it writing `cdi-cr.yaml` as well, so the example would have been short by one the moment it landed. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
65a038caa4 |
docs(api): separate the healthCheckExprs failure modes
The field doc filed every mistake under one outcome, a HelmRelease that hangs until its timeout. helm-controller treats them differently. An expression that does not parse is rejected before any Helm action and stalls the release at once, with a named reason. One that parses but errors on the object keeps the wait polling with the CEL error attached, and a current expression that evaluates cleanly but never turns true runs the wait out with no error at all. A read of status errors until the CR's controller writes one, even inside has(), but the wait survives it. A wrong group or kind silently falls back to the built-in readiness rules, and a CR whose CRD is missing fails the Helm action before the wait starts. The text ships in the CRDs and is what kubectl explain shows, so a reader who cannot check it against the controller source was sent to the wrong place for the parse error and the missing CRD. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2f5625f0dc |
fix(cdi): stop make update from deleting first-party templates
The update recipe in kubevirt-cdi opened with `rm -rf templates` and then refetched one upstream manifest. The directory keeps two more files -- the upload-proxy Ingress and TLSRoute -- that are written by hand and never refetched, so the next CDI bump would drop upload-proxy exposure from the chart while `make update` still exited 0. The clean slate bought nothing either way: wget -O truncates the file it writes. The recipe now creates the directory with `mkdir -p` and leaves the rest of it alone, which is what kubevirt-operator moved to when it was last bumped. kubevirt-cdi-operator keeps its wipe. The same change there would be for symmetry rather than for a loss, and it is not free: both its patch calls resolve, so a hunk upstream has moved out from under leaves templates/cdi-operator.yaml.rej, and gawk's inplace edit leaves templates/cdi-operator.yaml.gawk.XXXXXX if it dies abnormally. Helm renders every file under templates/, so the first fails the package loudly and the second renders a duplicate object at exit 0. The wipe is what takes both. Removing it wants that recipe restructured to transform outside templates/ the way kubevirt-operator now does, which is its own change. Until then the scan below is what watches that package, with the limit the scan already documents: it reports a tracked template whose stem the Makefile never mentions, so a name that collides with something already written there, templates/ tolerations.yaml against the patches/tolerations.diff line, is exempted. It catches the loss it was written for, not every loss. The bats case scans every package Makefile rather than a pinned list, so a first-party template added under a recipe that clears templates/ fails the commit that adds it instead of vanishing at the next bump. A package's own NAME and NAMESPACE do not count as naming a file, because a package called foo-bar would otherwise vouch for its templates/bar.yaml with nothing writing it. A removal reaching into another package is outside the scan's range, and the header records the case in the tree that this leaves uncovered. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
a75919087e |
fix(opensearch-operator): bound every call to a cluster's HTTP API (#4341)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does A call to a cluster's HTTP API has no bound of its own. `NewOsClusterClient` builds its own `http.Transport`, and a fresh transport carries no dial, TLS-handshake or response-header timeout; the config sets no `http.Client`, so there is no timeout there either; the calls the client makes on its own behalf pass `context.Background()`; and the calls that take a context from the caller are handed one with no deadline on it, since nothing on the reconcile path sets one. The `Timeout` field the request types carry is the server-side query parameter: it bounds how long the server waits before answering, which helps only once something answers. Each controller runs a single worker, so one call that never returns stops every `OpenSearchCluster` from being reconciled, and the finalizer on the next deletion waits out its whole termination timeout with nothing in the log to say why. Reported in #4337, measured in #4332. A fourth patch on the image bounds every request the operator's client sends, at two levels. - Every request goes out through a `RoundTripper` that puts a 30s deadline on it. That is the only level that reaches the request opensearch-go builds for itself: before a client's first request the library runs a product check with a `context.Background()` of its own and decodes the body of the answer, and a client is built per reconcile, so that check runs on every one. The deadline cannot be released when `RoundTrip` returns, because the body goes back to the caller still open, so the cancel rides on the body and is released by `Close`, which is how `http.Client` applies its own `Timeout`. - That bounds one attempt. opensearch-go retries a request up to three more times on 502, 503 and 504, and the transport's deadline starts over on each of them, so the calls carry a 30s deadline a second time for the series: the 15 calls in `os_client.go` that request and decode before returning through `requestContext`, and the 28 that go through the `doHTTP*` helpers in `performRequest`. - The transport keeps a 10s dial and a 10s TLS handshake, so an unreachable node fails in a third of the budget with an error that names what went wrong. `ResponseHeaderTimeout` is gone from the transport. It stops counting the moment the headers arrive, so it is no bound on a body that starts and then stops, and at 60s it sat above a request deadline that now covers everything it covered. This bounds the wait, it does not make an unresponsive cluster answer: a call that passes its deadline fails the reconcile, which requeues. The change belongs upstream, and there is no upstream issue for it yet. The Dockerfile entry carries the drop condition the other entries carry. ### Tests Seven, in the package the image's own `go test -race` step already runs: - a call to a cluster that accepts the request and answers nothing returns instead of running forever; - a new client, against a cluster whose product check flushes its headers and then stops, returns instead of running forever; - a request through the operator's transport, to a server that accepts the connection and then says nothing, returns instead of running forever; - a body that arrives with its headers and then stops is abandoned; - a call retried on 503 is bounded as a series rather than per attempt; - a body is still readable after the call that fetched it has returned; - the transport carries the handshake bound by value, and the dial bound through the dialer it is built from. Every bound was checked against the mutation that removes it, and all seven turn a test red: the transport wrapper dropped from the constructor, the deadline dropped from `RoundTrip`, the cancel moved off the body there, the deadline dropped from `performRequest`, one call site reverted to `context.Background()`, and the dial and handshake timeouts dropped in turn. Waits are measured from the test goroutine, so an unbounded call fails the test rather than hanging the run until CI kills it. The silent-server test holds its accepted connections and forces a collection while the call is in flight, because an unreachable `net.Conn` is closed by the collector (`net/fd_posix.go` puts a finalizer on `netFD`) and that turns the silent server into one that resets the connection. The full four-patch sequence applies to a pristine v2.8.0 tree in the glob order the build relies on, with no offsets and no fuzz; `gofmt` and `go vet` are clean; and `docker buildx build images/opensearch-operator` builds the image on linux/arm64 with the `go test -race` step green. ### Screenshots Not a UI change. ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: Walked the trigger map against the diff: one comment block in `packages/system/opensearch-operator/images/opensearch-operator/Dockerfile` and one new file in that image's `patches/`. No package added, renamed or removed, no values key, no schema, no enum, no `hack/` file, no make target, no namespace and no `ApplicationDefinition`. ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(opensearch-operator): time out calls to a cluster's HTTP API, so one unanswered request no longer stops the operator reconciling ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Added timeout handling for OpenSearch cluster connections, including connection, TLS-handshake, response-header, and request deadlines. * Prevented requests from hanging indefinitely when a cluster is unavailable, unresponsive, or stops returning data. * Ensured response bodies remain readable after the initial request completes when applicable. * Bounded the full request lifecycle, including response-body reads and retried calls. * **Tests** * Added coverage for connection, response, retry, and body-read timeout scenarios. * Enabled race detection for relevant package tests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
36d5c31eaa |
docs(api): stop listing helm-install-timeout carriers in rest.go
The comment next to the timeout overrides named the kinds that set helm-install-timeout and gave each one's reason. The list had already fallen behind the ApplicationDefinitions that carry the annotation, and nothing checks it, so it drifts every time a kind opts in. Say where the annotations live and when a kind should set them instead. Which kinds set them is recorded in their ApplicationDefinitions, and a comment should not restate it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
8dfbaa1d66 |
fix(kubevirt-cdi-operator): pin the vendored CDI release
This package decides which CDI the platform runs, but its update recipe took the version from GitHub's releases/latest redirect. Any refresh, including one done for an unrelated reason, could move the deployed CDI and its seven image tags without anyone choosing a release, and the diff carried no version line to review. That version is becoming load-bearing for behaviour, not only for images: logic that enumerates CDI's DataVolume phases is correct against one release. The release is now named in the Makefile, the asset is checked against a digest before anything is written, and a refresh that fails partway leaves the vendored manifest untouched. The digest guards against an asset replaced under its tag; it is not an authenticity check. A test target ties the pin to the tree: helm-unittest reads only the vendored file, so a pin moved without re-running update would otherwise leave every assertion green against the old release. The suite also holds the two local patches and the namespace rewrite that every refresh has to reapply. The pinned release is the one already vendored, and update reproduces templates/ byte for byte. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
97cb2781db |
test(e2e): pin the release-history read against the upstream type
The test that claimed to pin the HelmRelease v2 history shape wrote a HelmRelease document containing the field names it expected, then read that same document back with the same jsonpath. It established that yq can read a fixture the test itself composed. Its own comment described the failure it was meant to catch: a future Flux release renames the field, the jsonpath matches nothing, the guard reports no remediation cycle, and real cycles pass the e2e assertion. A rename moves neither the fixture nor the expression, so the test stays green through exactly that event. The comment also named where the truth lives, github.com/fluxcd/helm-controller/api/v2, and pinned a copy of the author's expectations instead. Replace it with a Go test that supplies neither half. The expression is read out of the shell library that ships it, and the object is built from the upstream type at the version go.mod holds, then serialized and matched the way kubectl does. A renamed or retyped field is a compile error or a failed match; a guard that stops reading history fails the lookup with a message saying so. Both directions were confirmed by mutation. Locating the expression by searching the file is not enough on its own. Inline in a long script, locating it means deciding which occurrences are code and which are prose, and every way of getting that wrong fails green: an example quoted near a broken read satisfies the pin while the guard reads nothing. So the expression moves into a named assignment in remediation-guard.sh, which the guard already sources, and the test sources that library and takes the value the shell is left holding. Deciding which text is the assignment is then not the test's problem: an `export`, an indent, a second assignment later in the file all resolve the way the guard will resolve them, because the thing resolving them is the shell. Sourcing borrows the environment along with the shell, so the snippet unsets the name before it sources. `go test` hands the child its own environment, and a shell that had exported this name would answer in place of a library that stopped assigning it: the value comes back non-empty, the empty check stays quiet, and the pin passes over a guard that reads nothing. That is this file's own subject reaching it through the environment rather than through the source. Moving the expression out of the call site puts a step between what is pinned and what runs, so the call site is checked too, and the check starts from the call rather than from the text: the local the guard is handed must be assigned exactly once, by an anchored assignment whose jsonpath is the expansion of the shared name. That a well-formed read exists somewhere in the file is a weaker claim than it reads as, since a second assignment to the same local, or a read filling another one, satisfies it while the guard runs a literal nothing here pins. The expansion and not the name: kubectl echoes a braceless template back verbatim, so a read that dropped the `$` would return its own text, clear the script's empty-history backstop, and carry no status the cycle check matches. The library is checked by sourcing rather than by reading, so an assignment the shell would run is resolved by the shell instead of by a guess about which text the shell would pick. Each rule is pinned by its own test, so a regression names which one it opened rather than only that extraction broke. How kubectl's printer treats an absent path is its own configuration rather than part of this contract, so it is left unasserted: guessing at it would put the expectation and the fixture back in the same hands. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
IvanHunters
|
ff0b851bbc |
test(vpn): add default render helm unit tests (#4347)
## What this PR does vpn was one of the eight user-facing apps in `packages/apps/` called out in #3631 as shipping with zero helm-unittest coverage. This adds a `tests/` suite pinning the default render and wires the standard `test: helm unittest .` target the sibling charts already use, following the same pattern as #4081, #4074 and #4093. Ran `helm unittest .` locally against the chart, it passes. `make show` needs a live cluster connection through `cozyhr`, which was not available in this environment, so I relied on `helm unittest` for local verification, same as the referenced sibling PRs. ### Screenshots Not applicable, test-only change. ### Downstream repositories Walked the trigger map in docs/agents/contributing.md against the diff: this only adds a test file and a Makefile target inside an existing package, it does not add, rename or remove a package, and it does not touch `values.yaml`, `values.schema.json`, `Chart.yaml` or `README.md`, so none of the listed triggers apply. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note test(vpn): add Helm unit test coverage for the default render ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Tests** - Added automated validation for the VPN deployment’s default configuration, including replica count, container image, service port, and generated resource naming. - **Chores** - Added a convenient command for running the VPN chart’s automated tests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
4475fb4c17 |
test(e2e): assert the System-mode OIDC uid claim mapping
The System lane read the rendered AuthenticationConfiguration for the issuer URL and the audience but not for the mapping that gives the user a UID. A configuration missing that mapping is still valid, so the tenant control plane starts and the suite passes while every OIDC user authenticates with an empty UID. The unit twin mocks the Secret without the mapping, so dropping the grep from the helper reddens it rather than leaving it unpinned. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
cbf297fa65 |
docs(kubernetes): describe the UID System-mode OIDC users carry
The aggregation-layer section said System-mode OIDC users have no UID for the published header to carry, and that mapping a claim for them was somebody else's change. Both are now false: the chart maps the sub claim. An operator upgrading a running cluster needs to know the identity of those users gained a field. Tenant RBAC never matched on it, but audit records and kubectl auth whoami show it, and an in-tenant webhook or extension server that read the empty value now reads the subject. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
e2464ea788 |
fix(kubernetes): give System-mode tenant OIDC users a UID
The AuthenticationConfiguration the chart renders mapped username and groups only, so upstream resolved every System-mode user to an empty UID and the X-Remote-Uid header the tenant apiserver publishes reached an aggregated API server carrying nothing. Map uid to the sub claim. The platform realm enables Keycloak's UPDATE_EMAIL required action, so the address the username claim carries is mutable and cannot serve as a stable identifier, while sub is the account UUID and survives a change of either. CustomConfig mode is untouched: an operator-supplied configuration is still written verbatim, uid mapping included or not. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
4bf95815a8 |
fix(kubernetes): render the aggregation-layer UID header for tenant OIDC (#3539)
## What this PR does Tenant clusters with `spec.oidc.mode: System` or `CustomConfig` authenticate users fine, but a UID does not survive the hop to an aggregated API server. The tenant kube-apiserver publishes `requestheader-uid-headers` into the `extension-apiserver-authentication` ConfigMap only when the flag is set, and the chart never set it. The aggregation layer still sends `X-Remote-Uid`, but the extension server trusts only the header names published in that ConfigMap, so it ignores the one it gets and the call arrives without a UID. The field documentation even told operators to add the flag by hand via `controlPlane.apiServer.extraArgs`. It forwards a UID rather than creating one. ServiceAccount tokens carry one, and for those the aggregation hop starts working. Users authenticated by `spec.oidc.mode: System` still have none, because the generated AuthenticationConfiguration maps `username` and `groups` only; giving them one means mapping a claim, which changes the identity of users who have none today, and is tracked separately. The chart now renders `--requestheader-uid-headers=X-Remote-Uid` on v1.32 and newer, and adds `--feature-gates=RemoteRequestHeaderUID=true` on v1.32, where the gate is still alpha and the apiserver rejects the flag without it. From v1.33 the gate is beta and on by default, so only the header flag is rendered. v1.31 gets nothing: the flag does not exist there, and writing it by hand stops the apiserver from starting. `--emulated-version` in the same passthrough moves that decision, because the gate resolves through the versioned spec at the emulated version. Rendering the gate on every version from v1.32 rather than on v1.32 alone was the alternative, and it would remove the migration step entirely, because an opt-out would then drop two entries instead of one and move the entry count by itself. It loses on what it leaves behind: a redundant gate term on every cluster, which stops the apiserver from starting on the day the gate is retired from the known list. The design also prefers rendering over failing for the same reason the fail branches are narrow, since a cluster stranded on a HelmRelease that no longer converges is a worse outcome than one that never gets the UID fix. The chart reads an operator entry the way the apiserver reads it: header list elements matched whole and untrimmed and folded as `EqualFold` folds them, gate names compared exactly, gate values resolved as `strconv.ParseBool` resolves them, `AllAlpha` and `AllBeta` blankets honoured, an empty value treated as no entry. On v1.32 an entry that says nothing about `RemoteRequestHeaderUID` gets a second one carrying the operator's own terms plus `RemoteRequestHeaderUID=true`. Carrying those terms into the merged copy is what makes it land correctly whether the tenant control plane collapses duplicate flags into the last one or hands both to the apiserver, which merges them in the order it reads them. Tests cover the render matrix over mode, version and operator input: every fail path with its exact message, the ownership and merge paths, duplicate entries in both orders, blanket gates, empty values, and case variants on the header name and on the gate value. The latest-version tenant lane creates its tenant with `spec.oidc.mode: System` and asserts the flag on the rendered KamajiControlPlane, so the tenant apiserver starts with the flag in CI. Nothing asserts the UID arriving at an aggregated apiserver; that needs an extension server inside the tenant and is not in this change. The layer where the count-keyed reset actually bites is the tenant Deployment's arguments, and asserting there needs a live tenant control plane, so it is not in this change either. The rule this follows is that a configuration which renders and boots today keeps rendering. Both flags are new, so an `extraArgs` shape reaching this code was rendering before it existed, and stranding a running tenant on a HelmRelease that no longer converges is a worse outcome than one that never gets the fix. A few shapes do stop rendering, and reaching any of them takes writing `--requestheader-uid-headers` into the passthrough by hand, attempting exactly what the chart now does and getting it wrong; in each the kube-apiserver fails option validation and never starts, so nothing that was working is lost. `docs/oidc-tenant.md` covers these shapes. One known limitation is in there and worth naming here: on v1.33 and newer, adding the documented opt-out is count-neutral in `extraArgs`, and so is removing it again, and on v1.33 and v1.34 adding or removing `--emulated-version=1.31`. The pinned Kamaji build rewrites a running apiserver's arguments only when that count changes and otherwise keeps every flag that has left the list, so either the header flag stays beside a now-disabled gate or the disabled gate stays beside the header flag, and the apiserver refuses to start. Changing the entry count in the same edit is the mitigation, and the fix belongs to the pin rather than to this chart, tracked in #3541. The paths that need no operator edit are clear: turning the feature on for a default tenant moves the count, and a tenant already carrying the opt-out renders byte-identically before and after. Out of scope: `mode: None` still publishes no UID header even though ServiceAccount tokens carry UIDs. That is pre-existing, and widening the condition changes the render for every default tenant. Tracked separately, along with the claim mapping above. Closes #3377 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change Checked the trigger map against the diff: no package added or removed, no schema field, enum or default changed (description text only), no ApplicationDefinition surface touched, no hack/ layout moved. The managed-apps reference page picks up the new field description through release-time README regeneration. ### Release note ```release-note fix(kubernetes): tenant clusters with `spec.oidc.mode` set to `System` or `CustomConfig` now render `--requestheader-uid-headers=X-Remote-Uid` on the tenant kube-apiserver, plus the `RemoteRequestHeaderUID` feature gate on v1.32, so that an aggregated API server can read the caller's UID out of the request headers. Without that flag the tenant apiserver never publishes `requestheader-uid-headers`, and an aggregated API server ignores the UID header even for callers that have one, ServiceAccount tokens among them. It forwards a UID rather than creating one: users authenticated by `spec.oidc.mode: System` still have none, because the generated AuthenticationConfiguration maps `username` and `groups` only, and that is tracked separately. Adding the flag by hand through `controlPlane.apiServer.extraArgs` is no longer necessary, and an entry of your own is respected: the chart steps aside when yours already carries `X-Remote-Uid` or already enables the gate, merges `RemoteRequestHeaderUID=true` into a copy of a v1.32 `--feature-gates` entry that says nothing about the gate, and renders neither flag when you switch the gate off. The render fails where a `--requestheader-uid-headers` list of your own would stop the kube-apiserver from starting anyway: one that omits `X-Remote-Uid`, one with an empty element, and a list of your own while the gate is off. On v1.33 and newer there is a migration step, and it applies in both directions: the pinned Kamaji build rewrites a running apiserver's arguments only when the number of `extraArgs` entries changes, and otherwise keeps every flag that has left the list. Adding the documented opt-out keeps that number the same, so the header flag already on the Deployment stays beside the now-disabled gate. Removing the opt-out later keeps it the same too, so the disabled gate stays on the Deployment beside the header flag the chart renders again, although your spec no longer shows it. Adding or removing `--emulated-version=1.31` has the same shape both ways on v1.33 and v1.34. The apiserver refuses to start in each case, so change the entry count in the same edit. ``` |
||
|
IvanHunters
|
48058dcc74 |
build(opensearch-operator): run the patched tree's tests under -race
The timeout tests drive a listener from one goroutine and inspect what it accepted from another. The plain run passes an unsynchronised access there, so nothing in this repository would catch one. The builder image carries cgo and gcc, and the package images are built natively on x86-64 runners, so the detector is supported where this runs. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
1a2c87635a |
fix(opensearch-operator): bound every call to a cluster's HTTP API
Upstream gives the operator's calls to a cluster no deadline of any kind. The transport it builds carries no dial, handshake or response-header timeout, no http.Client is configured, the calls the client makes on its own behalf pass context.Background(), and the ones that take a context from the caller are handed one with no deadline on it. A cluster that accepts a connection and then says nothing holds such a call for as long as the process lives, and with one worker per controller that single call stops every OpenSearchCluster from being reconciled. The deletion that then waits out its whole termination timeout is the visible end of it. The bound goes on the RoundTripper, the one level every request passes through, the ones opensearch-go builds for itself included: before a client's first request the library runs a product check that carries a context.Background() of its own and decodes the body of the answer, and a client is built per reconcile, so that check runs on every one. The deadline cannot be released when RoundTrip returns, because the body goes back to the caller still open; it rides on the body and is released by Close, the way http.Client applies its own Timeout. That bounds one attempt. opensearch-go retries a request up to three more times on 502, 503 and 504 and the transport's deadline starts over on each, so the calls carry a deadline a second time for the series: through requestContext where the call decodes the answer before it returns, and through performRequest where the still-open body goes back to the caller. The dial and the TLS handshake keep a shorter bound of their own, so an unreachable node fails in a third of the budget with an error that names what went wrong. Each of those bounds has a test that fails when it is removed. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
08bbed3fb5 |
feat(talos-log-collector): add talos-log-collector package (#3260)
## What this PR does Adds `talos-log-collector`, an optional system package: a node-local Vector DaemonSet that receives Talos machine and kernel logs and forwards them to the tenant VictoriaLogs (vlinsert), alongside the container/audit/event logs already collected by monitoring-agents. Talos keeps system and kernel (kmsg) logs inside the OS and does not write them to a host file the Fluent Bit `tail` input could read, so these logs are currently never collected. Vector receives them over the node loopback (Talos pushes to 127.0.0.1:5170). Talos exposes the two streams through independent config paths, and both must point at the loopback socket: `machine.logging.destinations` forwards the JSON-lines service logs, and a `KmsgLogConfig` document forwards the kernel (kmsg) stream (including DRBD kmsg). The node-side setup for both is in the README. Design notes: - The pod stays on the pod network so cluster DNS resolves vlinsert with no extra config. Talos pushes to 127.0.0.1:5170; a `hostPort` bound to `hostIP: 127.0.0.1` publishes that loopback socket into the pod via portmap, keeping the receiver off the host network (no node-wide pod exposure). - The receiver binds inside the pod netns, so a NetworkPolicy is load-bearing: a CiliumNetworkPolicy restricts ingress to the host entity when Cilium is present, otherwise a default-deny-ingress `networking.k8s.io/v1` fallback keeps the socket protected on non-Cilium variants. - Runs non-root, read-only root filesystem, all capabilities dropped. - Optional package (opt-in via `bundles.enabledPackages`): inert until each Talos node is configured to push logs. Node-side setup is in the README. Verified end-to-end on a Talos v1.12 dev cluster: service and kernel logs (including DRBD kmsg) reach VictoriaLogs; pods run non-root with 0 restarts. ### Release note ```release-note feat(talos-log-collector): add talos-log-collector package to collect Talos machine and kernel logs into VictoriaLogs ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an optional node-local Talos log collector (via system bundle packages) that forwards Talos service and kernel logs over loopback to the platform log ingestion endpoint. * Introduced the collector Helm chart (Vector-based) with configurable log level, loopback TCP receive port, digest-pinned image, default resources, and routing target. * Added conditional Cilium network policy support with a safe deny-by-default fallback. * **Security** * Hardened the collector to run non-root with reduced privileges, read-only filesystem, and dropped Linux capabilities (and disabled service account token automount). * **Documentation** * Documented configuration requirements (including kernel log enablement), operational behavior, and teardown steps. * **Build & Release** * Extended the build to include building the collector’s image. * **Tests** * Added Helm rendering/contract tests covering daemonset, configmap, service account, and network policy. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
47a611cf9e |
docs(kube-ovn): point the update comment at the test that asks for the fixture
The comment said the bats suite compares against "a fixture this step tells you to refresh", but the update step never mentions the fixture: its failure asks you to fix our Dockerfile. The test named "the fixture tracks the vendored chart version" is the one that asks, and it fires after a bump lands. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
4fbef512b4 |
fix(opensearch-operator): name the component reconciler a stall sits in (#4333)
## What this PR does The cluster controller runs nine component reconcilers in sequence, and none of them logs unconditionally when it is entered. The TLS one has a line above its work, but it sits on the early return for a cluster with no security section, which `packages/apps/opensearch/templates/opensearch.yaml` never renders, and the rolling restart one opens at `V(1)`, off at the verbosity the operator ships with. So a call that stops returning leaves nothing behind at all: the operator log ends on whatever ran before it, the object stops being reconciled, and from the outside that is indistinguishable from an operator with nothing to do. That is not hypothetical. Two e2e runs on 2026-09-18 ended that way, and the evidence is the operator's own log, read out of the crust-gather host snapshot each failed run uploads, at `snapshots/opensearch/host/namespaces/cozy-opensearch-operator/v1/pod/opensearch-operator-controller-manager-*/operator-controller-manager/current.log`. In run 35343282947 the operator reconciled `OpenSearchCluster` 27 times on a steady cadence, gaps of 10.2s for almost all of them and 26.5s at the widest. Its last line is at 13:51:33.225Z. helm-controller applied the CR revision that moves the cluster onto the chart CA at 13:51:35Z. Nothing was written for the remaining four minutes, the pod stayed Running and Ready with 0 restarts, and the admin certificate the step was waiting on was never re-minted. Run 35333289958, a different branch and a different operator pod, has the same shape: last line 10:56:35.706Z, Apply at 10:56:38Z, widest gap before that 24.2s, then silence. The securityconfig Job had completed successfully in both, and the cluster itself was quiet. #4332 tracks it. A patch on the image now wraps each component so a call still in flight after a minute reports the component's own name and how long it has been running, and keeps reporting on the same interval while it is stuck. A pass through the components takes seconds on a healthy cluster and the controller requeues itself every 30s, so a minute is well past anything normal. Nothing at all is written while a component behaves, which is why this is a watchdog rather than an entry log: logging every entry would put nine lines per cluster into the log on every reconcile. The finalizer path is deliberately left alone, because every `DeleteResources` at this version returns immediately and a watchdog there could only spawn goroutines for calls that cannot block. The e2e lane already collects that log and nothing in it changes here. #4279 put a `podLogs` leg on the operator's own namespace into the opensearch catch, `namespace: cozy-opensearch-operator`, `selector: control-plane=controller-manager`, `tail: 2000`, and this branch sits on top of it. The other `podLogs` leg takes the cluster's own pods in the test namespace, and cozyreport's per-pod walk keeps only pods that are not Running or not Ready, which a stalled operator is neither, so before #4279 these lines reached the job log only through the crust-gather snapshot. This is a diagnostic, not the fix. Where the reconcile stops is still unknown, and the next occurrence should name it. The Dockerfile comment says to drop the patch once the stall itself is understood and fixed. The new code lives in `pkg/reconcilers`, already covered by the image's `go test` step. Five specs hold it: the wrapped result and error pass through untouched, a call that returns before the threshold is never reported, a call that returns before the threshold also takes the watchdog with it, the report names the component and a moving elapsed while the call is in flight, and the report repeats rather than firing once. Six mutations each fail the spec that owns them: dropping `close(done)`, `time.NewTimer(0)`, an empty component name, a frozen elapsed, a fire-once ticker, and a swallowed result. The full four-patch sequence applies to a fresh v2.8.0 tree in the alphabetical order the build relies on, and builds, vets and tests clean there, with five `-race` runs of the new specs. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map against the diff: one Dockerfile comment block and one new patch file under `packages/system/opensearch-operator/images/`. No values key, schema, make target or documented workflow changes. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added stall monitoring for OpenSearch component reconciliation. When reconciliation remains in progress beyond the configured threshold, the operator logs the component name and elapsed time at regular intervals. - Monitoring remains silent for promptly completed operations and preserves existing results and errors. - **Documentation** - Clarified reconciliation watchdog behavior and documented why certain resource deletion operations are not monitored. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
scooby87
|
95b8549238 |
feat(dashboard): brand the token-proxy login page from the console config (#4255)
## What this PR does Part of #4217. Dashboard branding (title, logo) is only reachable through the authenticated dashboard/kube-api path, so the pre-auth surfaces render unbranded. This PR fixes the first of them: the token-proxy login page — the first screen an unauthenticated user sees on a non-OIDC cluster — hardcoded `<title>Login</title>` and had no branding hookup at all. The branding config lives in the `cozy-dashboard-console-config` ConfigMap, which the SPA reads only through the authenticated kube-api. This mounts that same ConfigMap read-only into the token-proxy container and renders its `titleText`, `logoText` and `logoSvg` on the login page, falling back to the historical unbranded page when the file is absent or unreadable so a cluster without branding is unaffected. The SVG is encoded into an `<img>` data URI rather than inlined, which keeps any embedded script inert. The OIDC branch is untouched: its login is served by Keycloak, already branded through the realm login theme. The unauthorized/access-denied screens rendered by the SPA itself (for a logged-in user without kube-api RBAC) are handled in a separate PR. Verified with `go test`, `go vet`, `gofmt`, and `helm unittest` (branding mount asserted present in the non-OIDC branch and absent in the OIDC branch). ### Downstream repositories The diff touches only `packages/system/dashboard` (the in-repo token-proxy image and its chart). No app is added, renamed or removed, no `values.schema.json` / enum / default changes, no CRD or namespace changes, and the `config.json` branding keys are unchanged. Nothing in the trigger map applies. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note feat(dashboard): the token-proxy login page now renders the deployment's branding (title and logo from `branding.titleText` / `branding.logoText` / `branding.logoSvg`) instead of the hardcoded default, so unauthenticated users on non-OIDC clusters see a branded sign-in screen. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added deployment branding to the login page, including a custom title, logo, and heading. * Branding is automatically loaded from the dashboard configuration when available. * Login pages continue to display a default unbranded experience when branding configuration is unavailable or invalid. * **Tests** * Added coverage for branded and unbranded login-page rendering. * Added validation for branding volume mounts across authentication modes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
7d53f7af22 |
fix(dashboard): raise the console nginx header buffers for large OIDC tokens
The nginx config baked into the console image kept the nginx defaults for client_header_buffer_size (1k) and large_client_header_buffers (4 8k). An OIDC access token carrying many group claims arrives as a single Cookie header of several kilobytes, which no longer fits into one buffer, so nginx answered 400 "request header or cookie too large" before the request reached the /apis, /api or /k8s/ upstream. Raise both to 16k, at the top level of that config rather than inside the server block: a header field read before the Host header has selected the virtual server is measured against the default server's buffers, so the same lines inside a server block cover that server only while it stays the default one. Measured against the pinned nginx base image, a 12 KB cookie gets 400 on the old config and 200 on the new one. The ceiling moves rather than disappears: a 20 KB cookie is still rejected. This is the console hop alone. The platform ingress-nginx controller in front of it runs on the same defaults, so where the dashboard is published through an Ingress a token large enough to exceed them is still rejected at the edge; where it is published through the gateway, whose edge has no per-header ceiling of this kind, the console was the hop returning the 400. Raising the controller is a separate change in a separate package. The accompanying check extracts the config from the Containerfile heredoc and fails if either directive is missing, below 16k, or no longer ahead of the first server block: the config lives inside an image build, where no chart template and no helm-unittest case can reach it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |