mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-03 18:47:04 +09:00
ci/shellcheck-gate
741
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
879d0f6db6 |
fix(installer): make operator readiness honest in the chart and the install gate (#3466)
## What this PR does
The operator Deployment ran `replicas: 1` with `maxUnavailable: 1`, so
the Deployment controller took the minimum available replica count as `1
- 1 = 0` and the `Available` condition reported `True` with no running
pod at all. Measured against an operator whose image tag does not exist:
the condition was `True` with `reason: MinimumReplicasAvailable` while
the only pod sat in `ImagePullBackOff`, `unavailableReplicas: 1`, and no
`availableReplicas` key in the status.
That broke two separate things, so this fixes both.
The E2E install gate asked `kubectl wait --for=condition=Available` and
passed in 156 ms against that dead operator. The install then carried on
and died about two minutes later on the `packages.cozystack.io` CRD wait
further down the file, which sends you after CRD installation when the
operator never started. It now uses `kubectl rollout status`, which
reads updated and available replicas directly, and dumps the deployment,
pods, operator logs and namespace events when it fails. This half is
deliberately independent of the chart: `rollout status` is correct under
any rollout strategy, so a later change to that strategy cannot quietly
make the gate vacuous again.
The chart is the other half, and fixing the gate does not cover it.
`Available` is read by more than this test, and until the strategy
changes an alert on that condition, a health dashboard, or a person
reading `kubectl get deploy` all show green while the operator is dead.
The strategy moves to `maxSurge: 1` / `maxUnavailable: 0`, which
restores the ordinary meaning of the condition. Kubernetes refuses
`maxSurge: 0` and `maxUnavailable: 0` together, so the surge has to move
for the unavailability to reach zero.
That pair is worth reading twice, because it is not a configuration I
picked. At `replicas: 1` the Kubernetes default of `25%` / `25%`
resolves to exactly `maxSurge: 1` and `maxUnavailable: 0`, since the
surge rounds up and the unavailability rounds down. What this PR does is
stop overriding the default, not introduce a tuning of its own, and the
risk of the change should be judged on that basis.
Moving the surge is safe here, and I checked rather than assumed,
because the pair came from somewhere. It was carried over from the
earlier `cozystack` Deployment, where a sidecar published host port 8123
and `hostNetwork` left the surge pod unschedulable on a single-node
cluster. It got here in two steps, which is worth knowing if you go
looking for the reason: by the commit before this file existed the
sidecar was already gone and no port was declared, yet the pair stayed.
It had outlived its own reason before it was ever copied, which is why
there is no justification to find in this file's own history. On a
single-node cluster the surge pod is scheduled next to the outgoing one
and the rollout completes; adding a declared container port reproduces
the original `didn't have free ports for the requested pod ports`
failure exactly. Nothing else in the pod holds a port either: metrics
and health probes are bound to `0`, and the configured webhook server
never starts because the operator registers no webhook and never asks
the manager for the server.
The surge makes leader election load-bearing, though it covers less of
the overlap than it first appears. While `maxSurge: 0` capped total pods
at `replicas` a second manager process could not exist, so
`--leader-elect=true` was defence in depth. With a surge pod the lease
is what keeps the incoming manager's reconcile loops off the cluster
while the outgoing one is still leader, and because there is no
readiness probe the incoming pod counts as available as soon as its
container starts, so that overlap covers the whole image pull. It does
not cover everything: the operator installs CRDs, installs Flux and
creates the platform PackageSource before `mgr.Start()`, and the lease
is only taken inside `mgr.Start()`, so those three privileged installs
run outside leader election entirely. They are safe because both pods
write the same objects through server-side apply under the same field
manager, which is a different guarantee from the lease. The
`k8s-await-election` wrapper this operator replaced did hold the whole
process until it won leadership, so if that property sounds familiar it
belongs to the predecessor rather than to this binary.
The sibling `opensearch-operator` chart turns leader election off at one
replica for a real reason: a transient lease-renewal blip makes
controller-runtime exit the process, which at one replica reads as a
crashlooping Deployment. This operator installs first, when the
apiserver is at its shakiest, so that scenario applies here too. I am
accepting the risk knowingly, because after `maxSurge: 1` the
alternative is two managers reconciling the same cluster, which is
worse. The flag is pinned in the unit test so it cannot be turned off
without the test going red.
One consequence is permanent, so I am writing it down now instead of
leaving it to be found in a year. Because any bound port breaks the
surge pod under `hostNetwork`, an `httpGet` readiness probe is off the
table for this Deployment for good and only an `exec` probe could ever
work. So `Available` still answers "a container started", not "the
operator is healthy": an operator that starts and then dies partway
through installing CRDs is caught by the CRD waits below the gate, not
by the gate itself.
The surge also opens one new window. The operator installs CRDs and Flux
before the manager starts, which puts that work outside leader election,
so the incoming pod runs those applies while the outgoing pod is still
leader. Both apply the same objects server-side and the outgoing pod
exits moments later.
`--wait` is dropped from the helm step in the same change. Helm decides
a Deployment is ready with the same `replicas - maxUnavailable`
arithmetic, so on the old strategy it was satisfied at zero ready pods
and gated nothing, which is why the broken install got past it as well.
Once the strategy is honest it would gate for real, but on its own 2m
budget, and it would fail the install before the rollout gate ran and
hide the diagnostics behind a bare Helm timeout. Hooks are waited for
with or without the flag, and the release otherwise carries only the
Deployment and its RBAC objects, so the rollout gate is the better
single place to decide the operator is up.
A helm unittest suite pins the strategy and all four preconditions the
surge makes load-bearing: no container or initContainer declares a port,
both listener arguments stay disabled, leader election stays on, and no
probe uses `httpGet`. That last one follows from the two disabled
listeners leaving no port for a probe to reach, and it is the worst of
the four to get wrong, because the others wedge an upgrade while a
`livenessProbe` against a disabled listener crashloops the pod on every
cluster, fresh install included. Only the `httpGet` field is forbidden
rather than the probes themselves, so an `exec` probe stays available.
All four run against all three chart variants, because the template
branches on variant inside the container spec and a port added under one
branch would render for that variant alone.
I checked the suite fails for the right reasons instead of trusting it.
Reverting the strategy, declaring a `containerPort` on the operator
container, declaring one under a single variant branch, declaring one on
an initContainer, pointing a bind-address at a real address, turning
leader election off, and adding a readiness, liveness or startup probe
with `httpGet` each turns it red on its own assert. Inserting a
container ahead of the operator trips the container-name pin before the
argument asserts can start describing the wrong container. An `exec`
probe correctly stays green.
The rationale is split by audience. What an operator applying the
rendered manifest has to respect stays in plain comments and travels
into `_out/assets/cozystack-operator-*.yaml`: why the strategy is what
it is, that no port may be declared or bound, and that leader election
has to stay on. Everything that is only about this repository moves into
a `{{/* */}}` template comment that stops at the chart boundary. That
covers the pointer to the test and which assert pins what, where the
strategy was copied from, why the webhook server on 9443 is not a third
port binder, and why `opensearch-operator` reaches the opposite
conclusion on leader election. The other templates in this chart already
keep maintainer notes that way.
### Screenshots
Not applicable, no UI changes.
### Downstream repositories
I left the boxes empty on purpose rather than to skip the question,
because the honest answer is not one of them and someone else should
make the call.
Walking the trigger map in `docs/agents/contributing.md`, nothing fires.
The `hack/` triggers name `hack/e2e-prepare-cluster.bats`,
`hack/package.mk`, `hack/update-crd.sh` and moves or renames under
`hack/`, and this PR only edits the body of
`hack/e2e-install-cozystack.bats`. The `ansible-cozystack` trigger is
renaming an object the role waits on, and
`deployment/cozystack-operator` keeps its name.
The behaviour that role sees does change, though. It runs `kubectl wait
deployment/cozystack-operator --for=condition=Available`, the same
vacuous check this PR replaces here, so the chart change turns its wait
from one that returns instantly into one that means something. Its
`cozystack_operator_wait_timeout` defaults to 300 seconds, the same
budget I gave the gate here, so a cold image pull fits and I do not
think anything needs to change over there. No follow-up PR seemed
warranted for a change that only makes an existing check work, but that
is a judgement call rather than a trigger, which is why the boxes are
empty.
- [ ] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
### Release note
```release-note
fix(installer): the cozystack-operator Deployment no longer reports Available with zero running pods. It ran replicas: 1 with maxUnavailable: 1, which made the minimum available replica count 0, so the condition stayed True while the operator was dead and anything reading it, including the E2E install gate, saw green. The rollout strategy is now maxSurge: 1 / maxUnavailable: 0 and the install gate waits for the rollout instead of the condition.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved operator installation checks by monitoring deployment
rollouts and providing deployment, pod, log, and event details when a
rollout fails.
* Updated operator upgrades to maintain availability during rollout,
including in single-node environments.
* Documented rollout requirements and behavior across supported
installation configurations.
* **Tests**
* Added coverage for rollout settings and upgrade compatibility across
supported installation variants.
* Clarified Helm unit-test directory coverage documentation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
Aleksei Sviridkin
|
115d9bbaaf |
fix(installer): let the operator Deployment report Available honestly
At replicas: 1 with maxUnavailable: 1 the Deployment controller takes the minimum available replica count as 1 - 1 = 0, so the Available condition reports True with no running pod at all. That reading is what every consumer of the status sees: an alert on Available, a health dashboard, or a person reading kubectl get deploy all show green while the operator is dead. The replacement is not a tuned pair, it is the platform default written down. At replicas: 1 the default 25% / 25% resolves to maxSurge 1 and maxUnavailable 0, because the surge rounds up and the unavailability rounds down. So this change stops overriding the default rather than introducing a configuration of its own, and Kubernetes rejects maxUnavailable: 0 together with maxSurge: 0, so the surge could not have gone the other way regardless. The overridden pair was carried over from the earlier cozystack Deployment, where a darkhttpd sidecar published a host port and hostNetwork left a surge pod unschedulable on a single-node cluster. It arrived here in two steps: by the commit before this file was created the sidecar was already gone and no port was declared, yet the pair stayed, so it had outlived its own reason before it was ever copied. That is why looking for a justification in this file's history finds none. With no port declared anywhere in the pod there is no hostNetwork port conflict left, so the surge pod schedules beside the outgoing one; declaring one reintroduces exactly the scheduling failure the pair was added for. Not exercised on a live cluster in this rework. Stay on RollingUpdate rather than move to Recreate, which would fix the condition too, and by a shorter route: MaxUnavailable() reports 0 for any strategy that is not RollingUpdate, so the minimum available replica count becomes replicas - 0 = 1 and the condition is honest without any surge pod at all. The transition was not attempted, and the failure this project fixed in ouroboros 0.8.1 does not transfer to it. There the rollingUpdate block was the one the apiserver defaults onto a Deployment created without one, so it belonged to no applier and outlived the switch to Recreate. This Deployment has carried an explicit block since the commit that created it, and nothing applies it server-side, so whether Recreate would upgrade cleanly is an open question about the field-ownership history of the live object rather than a known rejection. What keeps RollingUpdate here is availability rather than apply semantics: maxUnavailable: 0 leaves the outgoing operator running when the replacement never starts, where Recreate deletes it first and leaves none. Editing the numbers inside the block is upgrade-safe from every state the field can already be in. The surge introduces new windows, and leader election covers less of them than it looks. The operator runs four privileged install steps before mgr.Start(), and the lease is only acquired inside mgr.Start(): the CRD install, the Flux install, the platform source resource and the platform PackageSource. All four therefore run in the incoming pod outside leader election entirely. What the lease does cover is the reconcile loops, the part that would otherwise conflict continuously instead of once at startup. That leaves three seams, and only the first two happen on a healthy upgrade. The routine one is the incoming pod's install steps against the outgoing pod's live reconcile loops, since the outgoing pod ran its own installs at its startup and has been inside mgr.Start() ever since. Those do not contend for the same fields: the installs write as field manager cozystack-operator, while the reconcilers write as cozystack-packagesource-controller and cozystack-package-controller and go mostly to status subresources. Different owners, different objects, so this seam is safe by separation rather than by any shared manager. The second is version skew rather than field contention, and maxSurge: 1 widens it rather than opening it. Under maxSurge: 0 the surge budget counts desired replicas rather than live pods, so the incoming pod is created while the outgoing one may still be Terminating - the overlap existed. What it did not have was a live counterparty: scale-down sends SIGTERM before the new pod is created, and that cancels the manager context, so the old loops were already unwinding. Here the outgoing pod is not signalled until the incoming one is available, so the overlap runs against a fully live reconciler for a termination grace period instead of a vestigial one. The incoming pod reports Ready the instant its container starts, because there is no readiness probe, so its CRD install runs while the outgoing binary's reconcile loops are still live and now writing against a schema the incoming pod has already moved. Old code against a new schema, which no field manager separates. It is bounded rather than benign: the outgoing pod exits on SIGTERM within seconds, a rejected write is transient and the next reconcile repairs it, and no object is left corrupt. Widening the skew, by adding a probe that delays Ready or by a migration the old code cannot round-trip, is what would make this seam matter. The third seam is install against install, and it needs the outgoing pod to be restarting, since a crashlooping pod re-runs its four steps on every restart, which is exactly the broken operator this gate exists to catch. Three of the four steps use server-side apply under the same field manager, so they never conflict and resolve last-writer-wins whenever the two pods carry different manifests. That ordering matters in one direction, and it is accepted here rather than guarded against: the CRD install and the Flux install both apply with Force: true, so an outgoing pod that writes last silently overwrites what the incoming pod has just applied, with no conflict and no error to exit on. The incoming pod is inside mgr.Start() by then and will not apply again until it restarts, so that revert of the upgrade's install phase persists rather than healing itself. Reaching it needs a restart landing in the seconds between the incoming pod's installs and the outgoing pod's SIGTERM. installPlatformSourceResource is the remaining exception, a read-modify-write of a Get followed by Create or by Update with the observed resourceVersion, so two pods running it at once can collide on AlreadyExists or on a conflict; that failure is loud and self-healing, since any error there exits the process and the pod restarts, and the object is not left corrupt. Pin the strategy and all four preconditions the surge makes load-bearing in a unit test: no container or initContainer declares a port, both listener arguments stay disabled, leader election stays on, and no probe uses httpGet. They fail differently and none is visible to the others, since a declared port stops the scheduler, a bound one only fails once the incoming process starts, and a second manager without a lease fails nothing at all until it writes. The httpGet precondition follows from the two disabled listeners leaving no port to target, and it is the worst of the four to get wrong: the other three wedge an upgrade, while a livenessProbe against a disabled listener crashloops the pod on every cluster, including a fresh install where no surge happens. Only the httpGet field is forbidden rather than the probes themselves, because an exec probe stays legitimate. All four are checked against all three variants, because the template branches on variant inside the container spec and a port added under one branch renders for that variant alone. The strategy type is pinned alongside its two values, since maxSurge and maxUnavailable say nothing on their own once the type is no longer RollingUpdate. The listener preconditions are pinned less tightly than the other two, and the test says so rather than implying coverage it does not have. A membership assert has no notion of position while Go's flag package keeps the last occurrence, so a duplicate after the disabled one wins and the assert still passes, while a duplicate before it loses and the assert passes just the same - the test cannot tell those two apart. Moving the disabled flag last fixes the process without making the assert catch anything. Rejecting the two addresses this binary defaults to closes the shape a values-gated block to enable metrics would take, and nothing available here closes an arbitrary address, since helm-unittest cannot express that a flag appears at most once. Note in hack/helm-unit-tests.sh that the loop covers packages/core, which its comment omitted. The suite added here lives there, so a later edit reconciling the loop with the stale comment would have silently stopped running it. Split the rationale by audience rather than keeping it all in shipped comments. What an operator applying the rendered manifest has to respect stays in plain comments and travels into the install artifacts: that the pair is the default, why maxUnavailable must stay 0, that no port may be declared or bound, that no probe may use httpGet, and that leader election has to stay on. Most of what stays in the template comment is what the chart's own suite pins, so it cannot drift out of step with the chart without a test going red: the four preconditions and which assert covers each, and why the variant loop is needed. Three things there are deliberate exceptions, kept because anyone editing the strategy has to know them and no assert can carry them - that the webhook server configured on 9443 is not a port binder, since controller-runtime adds it to the runnables only inside GetWebhookServer() and this binary never calls it; that two overlaps between the pods sit outside the lease; and why RollingUpdate is kept rather than Recreate. Those name what exists. The seam analysis above, which is how each one works, deliberately stays here instead. It describes another file's internals, nothing pins it, and a commit message is fixed in time, so it cannot quietly fall out of date the way a comment can. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
7739d1e294 |
feat(multus)!: stage reference CNI plugins from the multus image
Multus-backed NetworkAttachmentDefinitions delegate to the upstream containernetworking reference plugins (bridge, macvlan, ipvlan, ...), and nothing guarantees a node has them in the directory Multus searches. On k3s they live under /var/lib/rancher/k3s/data/cni while Multus and Cilium install into /opt/cni/bin, so a bridge-type NAD fails there with `failed to find plugin "bridge" in path [/opt/cni/bin]`. Bundle 14 of the release's plugins into the multus image and add an install-cni-plugins init container that installs them into the host /opt/cni/bin, mirroring the existing install-multus-binary container. loopback, dhcp, tap and dummy are left out; the Dockerfile records why. Gate it on a new chart value, stageCniPlugins, resolved per bundle and on for both. Talos bundles only bridge, firewall, flannel, host-local, loopback and portmap, so the other ten are missing there exactly as on generic Linux; the four that overlap are rewritten with the same upstream version, v1.9.1 on both sides, rather than an older one. A marker in the image Dockerfile records the Talos release that pin was compared against, and a test holds it to the release the platform ships. multus therefore moves out of system.common-packages into the two variant branches, the way linstor already does for talos.enabled. networking.stageCniPlugins overrides the variant default in either direction. Where staging is on the install is unconditional: a plugin is replaced on every pod recreation, not only when the destination is missing. A destination that is already a directory is reported and left alone, since replacing it is not something this can do safely. Turning the value off afterwards does not undo it; nothing here removes or restores a plugin. The container is not privileged and takes no mount propagation. It runs in the spc_t SELinux domain, without which an enforcing node refuses the write. Each plugin is installed under a temporary name and swapped in with a rename: /opt/cni/bin is shared with cilium and kube-ovn. The plugin tarball is checksummed before extraction. A plugin that cannot be installed is reported on stderr and to /dev/termination-log, and the container still exits 0: Multus stays the node's primary CNI and nothing removes its config, so an init container that never completes would leave the shim with no daemon behind it. The manifest change lives in customize-deployment.patch so it survives make update. BREAKING CHANGE: enabling stageCniPlugins replaces reference CNI plugins in /opt/cni/bin on existing nodes, Talos included -- four of the names it writes are ones the Talos node image also carries. Opt out before upgrading: declining afterwards does not restore what was replaced. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
myasnikovdaniil
|
3f967ec7e0 |
fix(platform): restore the isp-hosted dependency closure (#3391)
## Problem Fixes #3376. The `isp-hosted` preset shipped `bundles.system.enabled: false` while `paas` and `naas` were enabled, so it enabled an app catalog whose dependency closure it simultaneously disabled. Every emitted Package sat in `DependenciesNotReady`, the `*-rd` HelmReleases failed with `no matches for kind "ApplicationDefinition" in version "cozystack.io/v1alpha1"`, and the `cozystack-platform` HelmRelease never went Ready. Rendering the preset at `origin/main` shows the scale: **24 emitted Packages, 24 of them blocked**, by 6 missing dependencies — two more than the issue lists (`cert-manager` and `prometheus-operator-crds` are also absent). `isp-full` renders 75 Packages with a fully satisfied closure, which is a clean control. ## Root cause `templates/bundles/system.yaml` is gated end to end on `bundles.system.enabled`, but it carries two separable things: a variant-selected **data-plane head** (kubeovn/cilium/linstor on `isp-full`, a noop networking Package on `isp-hosted`) and a variant-agnostic **control-plane body** — `cozystack-engine`, the sole source of the `application-definition-crd` component, plus `cert-manager`, `prometheus-operator-crds`, `postgres-operator` and `objectstorage-controller`. "Hosted" means the host already provides CNI, storage and virtualisation, so only the data-plane head should be dropped. Disabling the whole bundle also deleted the control-plane body that every variant needs. Three things show the disable was the mistake rather than the intent: the preset still sets `variant: "isp-hosted"`; `system.yaml` contains an `isp-hosted` branch (noop networking plus default scheduler) that could never render; and `iaas.yaml` already refuses to render on this variant, encoding the same "system bundle minus iaas" contract. The preset has never worked — it was born this way in `2d022e38e` (January 2026), the refactor that replaced `bundles/paas-hosted.yaml`, whose contents confirm hosted was always meant to ship the engine, `cert-manager` and `prometheus-operator-crds`. ## Changes 1. **`values-isp-hosted.yaml`** — enable the system bundle. Hosted now emits 53 Packages with every `dependsOn` satisfied, installing 59 components against the 60 of the pre-refactor `paas-hosted` bundle, with no LINSTOR, KubeVirt, CAPI or Kamaji leakage. 2. **Move `cozystack.bucket-application`** from the iaas bundle into the system bundle. Its own `dependsOn` closure is `networking` + `objectstorage-controller` + `cozystack-engine`, none of them iaas, while `cozystack.backupstrategy-controller` — a *default* package of the system bundle — hard-depends on it. Any system-on/iaas-off configuration therefore wedged `backupstrategy-controller`; both `isp-full` presets masked this by enabling iaas. This is a second, independent bug not reported in the issue. 3. **Guards** — `paas`, `naas` and `iaas` now also require `bundles.system.enabled`. The pre-existing checks looked only at `bundles.system.variant`, which keeps its value when the bundle is disabled, so they passed on exactly this configuration. `iaas` had the identical hole (12 Packages, 6 unsatisfied deps). This turns a silent multi-hour `DependenciesNotReady` hang into a render-time failure that names the remedy. 4. **Two helm-unittest suites** — one pinning the dependency closure the catalog needs, one pinning the guards. ## Verification - All four operator-registered variants render dependency-closed: `default` 0 Packages, `isp-hosted` 53, `isp-full` 75, `isp-full-generic` 75. The two `isp-full` variants are byte-identical before and after, so the bucket move is a no-op for them. - Hosted is a strict subset of full with zero data-plane leakage, and its exclusions match the pre-refactor hosted bundle. - 23 suites / 97 tests pass in `packages/core/platform`; repo-wide `hack/helm-unit-tests.sh` is green. - Every change was **mutation-tested** — reverting each one in turn makes the suite fail, so the tests are not vacuous. - The workaround documented in #3376 (`paas`/`naas` disabled) still renders cleanly; the new guards do not fire on it. - Hosted runtime spot-checks: `cozystack-api` is a Deployment with only a *preferred* control-plane affinity plus blanket tolerations, and `cozystack-scheduler` uses its `default` variant (plain Deployment, no LINSTOR extender) — both schedule fine on a cluster with no control-plane nodes. ## What this does not fix This restores the dependency closure. It is **not** by itself sufficient for a fully converged hosted install. Review surfaced further hosted-unaware defaults that the dead bundle had been masking, filed separately: - #3387 — `apps/tenant` renders 14 `cilium.io/v2` policies with no `.Capabilities` guard, so `tenant-root` fails on a non-Cilium cluster. This is the blocker for actual hosted convergence, and it needs a deliberate decision rather than a reflexive capability guard, because those policies implement tenant network isolation. - #3388 — `extra/monitoring` defaults `logsStorages` to the LINSTOR-only `replicated` StorageClass. - #3389 — `bundles.disabledPackages` can still break the closure with no render-time error; the guards here cover the bundle flags only. - #3390 — nothing in CI exercises `isp-hosted`, which is why all of the above accumulated undetected. ## Upgrade note Existing `*-application` Package CRs carry `helm.sh/resource-policy: keep`. An operator who applied the #3376 workaround but did not also delete the retained Package CRs (as that issue instructs) will find them install once this change makes their dependencies Ready, since the reconciler re-enqueues dependents on readiness. Deleting the unwanted Packages before upgrading avoids that. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added fail-fast validation that requires the **System bundle** when enabling **IaaS**, **NaaS**, or **PaaS** to prevent incomplete catalogs. * Updated bundle composition so the **bucket application** is provided by the **System bundle**. * Enabled the **System bundle** by default for the `isp-hosted` variant. * **Bug Fixes** * Improved error messaging to clearly indicate the required bundle/variant configuration. * **Tests** * Added Helm unittest coverage for rendering-gate failures, dependency/package expectations, and hosted data-plane exclusions. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
myasnikovdaniil
|
5d420475f4 |
refactor(ci): drop the vestigial /tmp workspace from e2e (both workflows) (#3454)
## What this PR does
The E2E job's `if: always()` cleanup ran `rm -rf /tmp/$SANDBOX_NAME`
with no check that `SANDBOX_NAME` was ever set. `Set sandbox ID` carries
no condition, so it inherits the implicit `success()` every
unconditional step gets: any failure ahead of it — a conflicting
`pr.patch`, a `download-artifact` miss, `Checkout code`, the
release-path `curl` — or a cancellation that reaches the job after it
has started (the job condition is `!cancelled()`) skips it, and
`SANDBOX_NAME` is never written to `$GITHUB_ENV`. The `if: always()`
steps then run anyway and interpolate the empty value; `Remove
workspace` is the destructive one, expanding to `rm -rf /tmp/` and
failing the cleanup step against root-owned entries in the runner's
`/tmp`. This is not a "re-run skips the setup" bug: a re-run restarts
the job at step one and re-runs the unconditional `Set sandbox ID` — the
trigger is incomplete initialization, by any of the routes above.
The `/tmp/$SANDBOX_NAME` workspace is a vestige of the retired multi-job
self-hosted-runner design, where several `runs-on: [self-hosted]` jobs
shared no state and rendezvoused through a `sha256(REPO:WORKFLOW:REF)`
key, with a per-run `cp -r` of the checkout into `/tmp`. Since CI moved
onto the ephemeral pool (`runs-on: oracle-vm-32cpu-128gb-x86-64`) it is
a single job: there is no cross-job rendezvous and the checkout is never
reused across runs, so copying it into `/tmp` and removing it afterwards
is dead weight. (An earlier review justified the change by "wiping the
entire `/tmp` of the self-hosted runner" — that describes the topology
we retired; the hazard is real, but not for that reason.)
So this drops the workspace rather than guarding the `rm`. Both the PR
(`pull-requests.yaml`) and nightly (`nightly.yaml`) E2E jobs now run in
`$GITHUB_WORKSPACE`: the `Prepare workspace` and `Remove workspace`
steps are gone, the `cd /tmp/$SANDBOX_NAME` prefixes are dropped, and
the upload paths point at `_out/…` instead of
`/tmp/${SANDBOX_NAME}/_out/…`. That takes the empty-value expansion out
of the `cd`s, the stray `mkdir`, the upload paths and the destructive
`rm` in one move, across both workflows rather than one step of many.
`Set sandbox ID` stays: the job still reads `$SANDBOX_NAME` directly
(failure diagnostics, the chainsaw `docker cp`, teardown, and every
`make SANDBOX_NAME=…` argument) — the container needs a name, just not a
directory.
It also removes the dead code the single-runner retirement left in
`packages/core/testing/Makefile`: the world-writable `mkdir -p
/tmp/${SANDBOX_NAME}` + `chmod 777` in the `apply` target (they backed a
`-v /tmp/${SANDBOX_NAME}:/workspace/hosttmp` bind mount that was later
removed — nothing under `images/e2e-sandbox/` references
`/workspace/hosttmp`), and the `-e SANDBOX_NAME=${SANDBOX_NAME}` passed
into the container (nothing under `images/e2e-sandbox/` reads it). Every
other use of `SANDBOX_NAME` — the container name and the `make`
arguments — is kept.
Note this cannot make a job with an empty `SANDBOX_NAME` go green: such
a job already failed initialization or was cancelled, so its conclusion
is fixed before cleanup runs. It removes a second, destructive failure
the old cleanup piled on top, and it does not change whether re-runs
work in general — they do.
The diagnosis of the real trigger, the corrected blast radius, and the
scope across both workflows plus the Makefile are credited to the
maintainer review on this PR.
### Evidence
Run [30067779342 attempt
3](https://github.com/cozystack/cozystack/actions/runs/30067779342/attempts/3)
exhibits the mechanism: `Download Talos image (regular PR)` failed on
`GetSignedArtifactURL` with a 404, which skipped `Set sandbox ID`, and
the `always()` steps then ran on an empty value — `Collect chainsaw
report` created `/tmp/_out`, the uploads resolved to `/tmp//_out/…`, and
`Remove workspace` did:
```
rm: cannot remove '/tmp/systemd-private-…-systemd-timedated.service-1oADZP': Operation not permitted
rm: cannot remove '/tmp/.X11-unix': Operation not permitted
##[error]Process completed with exit code 1.
```
The step ran unprivileged in `/home/ubuntu/_work/…`, so all it could hit
was `Operation not permitted` on root-owned entries in the sticky `/tmp`
— it deleted nothing. That run is a later, independent occurrence (it
started well after this PR was opened), not this change's motivating
case. The 404 that triggered it is a separate, non-deterministic problem
— the artifact listing succeeded and only the signed-URL call failed
~190 ms in, `download-artifact` treats 404 as non-retryable, and attempt
2 of the same run fetched the artifact fine — and it deserves its own
issue rather than being folded in here.
### Screenshots
Not applicable — CI workflow change only.
### Downstream repositories
The change is confined to this repo's E2E CI
(`.github/workflows/pull-requests.yaml`,
`.github/workflows/nightly.yaml`) and its testing Makefile
(`packages/core/testing/Makefile`). No downstream repository consumes
it.
- [x] No downstream repository is affected by this change
### Release note
```release-note
refactor(ci): drop the vestigial /tmp/$SANDBOX_NAME workspace from the E2E PR and nightly jobs — they now run in $GITHUB_WORKSPACE, removing an unguarded `rm -rf /tmp/$SANDBOX_NAME` cleanup that failed the job on cleanup when SANDBOX_NAME was empty
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved end-to-end test environment setup and sandbox cleanup.
* Updated test reporting and artifact collection for more reliable
results.
* Ensured OpenAPI and full-suite tests run from the correct prepared
environment.
* **Chores**
* Simplified container and workflow configuration for sandbox-based
testing.
* Clarified checkout token handling in nightly workflows.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
Andrei Kvapil
|
ebeeab77aa |
docs(platform): correct the computeplane dependsOn and variant comments
The dependsOn comment claimed the kubernetes-application edge orders the addon artifacts the wrapped chart references; variant dependsOn only gates install: components — the operative effect is ordering the computeplane-rd install so the ComputePlane kind is not registered before the kubernetes machinery exists. Also document why the variant is named kubevirt. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Andrei Kvapil
|
aa5198b758 |
fix(platform): add cozystack-engine dependency to the computeplane package
The computeplane-rd component installs an ApplicationDefinition, whose CRD is served by cozystack-engine — hack/check-applicationdefinition-crd-ordering.bats enforces the edge for every package with a system/*-rd component. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Andrei Kvapil
|
4c3262a4ee |
feat(computeplane): deliver ComputePlane as a tenant module wrapping the kubernetes app
Add the operator-owned computeplane tenant module per the revised design (cozystack/community design-proposals/compute-plane): an extra/computeplane chart that provisions a single-tenant, Cozystack-managed cluster for untrusted-code apps by deploying the ordinary apps/kubernetes chart with operator-fixed values (fluxcd, cert-manager, ingress-nginx Proxied). The package re-declares apps/kubernetes as its own source-only component, so the module is self-contained; the cluster release is named computeplane so Kamaji writes the admin kubeconfig to computeplane-admin-kubeconfig — the contract consumed by placement: ComputePlane applications. The tenant holds no admin kubeconfig and can set cluster shape (nodeGroups) only. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Andrei Kvapil
|
78fc3f9178 |
refactor(ci): drop the vestigial /tmp workspace from e2e (both workflows)
The E2E job once ran as several state-sharing self-hosted-runner jobs that rendezvoused through a sha256(REPO:WORKFLOW:REF) key and a per-run /tmp/$SANDBOX_NAME copy of the checkout. Since the move to the ephemeral pool it is a single job: there is no cross-job rendezvous and the checkout is never reused, so copying it into /tmp and removing it afterwards is dead weight. Run every step in $GITHUB_WORKSPACE instead: drop the Prepare/Remove workspace steps and the `cd /tmp/$SANDBOX_NAME` prefixes, and point the upload paths at _out/. This deletes the unguarded `rm -rf /tmp/$SANDBOX_NAME` rather than guarding it: when an upstream step fails or the job is cancelled, the unconditional `Set sandbox ID` is skipped, SANDBOX_NAME stays empty, and the `if: always()` steps interpolate it — the removal expanding to `rm -rf /tmp/` and failing the cleanup step. `Set sandbox ID` stays, since the container still needs a name. Apply the same treatment to nightly.yaml, and drop the Makefile dead code the single-runner retirement left behind: the world-writable mkdir/chmod of /tmp/$SANDBOX_NAME (backed a bind mount removed long ago) and the `-e SANDBOX_NAME` container env (nothing in the e2e-sandbox image reads it). Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Aleksei Sviridkin
|
8bffa34b54 |
feat(platform): add opt-in in-cluster east-west encryption toggle (#2984)
## What this PR does Adds `networking.encryption.enabled` to the platform chart — a single operator-facing opt-in, off by default, that turns on in-cluster (east-west) transport encryption for the active datapath. When enabled, pod-to-pod traffic across nodes is encrypted at the network layer regardless of whether the workload implements TLS: defense-in-depth for the whole mesh without per-application changes. The default `isp-full` bundle selects the `kubeovn-cilium` datapath (and `isp-full-generic` selects `kubeovn-cilium-generic`), where KubeOVN owns the datapath and the overlay is a Geneve tunnel. The toggle therefore maps to KubeOVN's overlay IPsec (`kube-ovn` `ENABLE_OVN_IPSEC`). KubeOVN's own controller provisions the IPsec CA and per-node certificates through the Kubernetes CSR API, so no cert-manager wiring is needed. This complements, and does not replace, application-level / end-to-end TLS: in-cluster encryption never reaches an external client and gives a tenant no verifiable endpoint certificate. The values comments document the per-datapath scope — including that Kilo's WireGuard mesh (`meshGranularity: location`) covers only cross-location traffic and leaves the intra-location gap this toggle targets, and that the Cilium-native WireGuard/IPsec path on the `cilium` / `cilium-generic` variants is not reached because the system bundle does not select those variants — plus the MTU/throughput overhead. Scope: only the default KubeOVN datapath is wired through the platform toggle, which is what the system bundle actually selects. Wiring the Cilium-native path would require the bundle to expose cilium-only variant selection; that is out of scope here and called out in the values comments. Validation: `helm unittest` on the platform chart passes (66 tests, 7 new) covering on / off / absent / null and verifying the IPsec flag deep-merges without dropping the existing `kube-ovn` overrides, for both `isp-full` and `isp-full-generic`. Closes #2977 Part of #2811 ### Screenshots N/A — no UI changes. ### Release note ```release-note feat(platform): add opt-in `networking.encryption.enabled` (off by default) that enables in-cluster east-west transport encryption for the default KubeOVN datapath via overlay IPsec; complements, and does not replace, application-level TLS. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Introduced opt-in in-cluster network encryption using KubeOVN IPsec for east-west traffic protection. * Added `networking.encryption` configuration toggle (disabled by default) with full backward compatibility. * **Tests** * Added comprehensive test coverage for network encryption configurations across system variants and edge cases. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Timofei Larkin
|
2823b76802 |
feat(gateway): advertise HTTP/2 via ALPN on Gateway API listeners (#3398)
## What this PR does Browsers negotiate HTTP/2 exclusively through TLS ALPN: if the terminating listener does not advertise `h2`, every client silently falls back to HTTP/1.1, with no error surfaced anywhere. Cozystack's Gateway API path is served by Cilium's embedded Envoy, and Cilium ships with `enable-gateway-api-alpn=false` — so every Gateway listener advertised no ALPN protocols and all browser traffic through a Gateway ran over HTTP/1.1 (observable in browser dev tools as `http/1.1` on every request). The ingress-nginx path this replaces advertised `h2` out of the box, so the migration to Gateway API silently downgraded clients: head-of-line blocking and the ~6-connections-per-origin limit are back for anything served through a Gateway. This PR enables ALPN (`h2`, then `http/1.1`) on Gateway API listeners, default-on, with an opt-out at each management layer: - `packages/system/cilium/values.yaml` now defaults `gatewayAPI.enableAlpn: true`, which renders `enable-gateway-api-alpn: "true"` into the `cilium-config` ConfigMap. - The platform chart gains `gateway.http2` (default `true`) next to the existing `gateway.enabled`/`gateway.attachedNamespaces`. The `isp-full` and `isp-full-generic` bundles thread it as inline component values into the `cozystack.networking` Package CR (`components.cilium.values.cilium.gatewayAPI.enableAlpn`); inline HelmRelease values override the package `valuesFiles`, so an explicit `gateway.http2: false` actually reaches the dataplane rather than being shadowed by the new chart default. On `isp-hosted` the hosting-side CNI owns this setting and the flag is a no-op. - Tenant Kubernetes clusters (`packages/apps/kubernetes`) get the same fix: when the `gatewayAPI` addon is enabled, the tenant cilium HelmRelease now sets `enableAlpn: true`; individual clusters opt out via `addons.cilium.valuesOverride.cilium.gatewayAPI.enableAlpn=false`. ### Why default-on rather than opt-in HTTP/2 for browsers was the platform's behavior before the Gateway API migration, so keeping ALPN off would preserve a regression, not a status quo. Defaulting on is safe because ALPN is a negotiation: clients that cannot speak HTTP/2 simply keep negotiating `http/1.1`, exactly as before. Operators who need the old wire behavior can set `gateway.http2: false` platform-wide. ### Frontend vs backend hop This change affects the client↔gateway hop only. Gateway↔backend connections stay HTTP/1.1 unless a Service explicitly opts in per GEP-1911 by declaring `appProtocol: kubernetes.io/h2c` on its port. Cilium couples that backend-protocol support to the ALPN flag (`enable-gateway-api-app-protocol` flips to true together with it), but it only acts on Services that declare a recognized `appProtocol` value — and no cozystack-shipped Service does today (only plain `http`/`https` values exist in-tree, which Cilium ignores for protocol selection). So upstreams see no change from this PR alone; h2c/gRPC backends become possible to configure, not configured. The setting is cluster-wide Cilium agent config, so it covers the root Gateway and all tenant Gateways at once — Cilium offers no per-Gateway granularity here. Flipping it re-rolls the cilium DaemonSet on upgrade (`rollOutCiliumPods: true`), the same disruption profile as any other cilium config change. ### How it was verified - New helm-unittest suites: `packages/core/platform/tests/bundles_gateway_http2_wiring_test.yaml` asserts the Package CR carries `enableAlpn` true by default and false with `gateway.http2: false` for both cilium bundles; `packages/apps/kubernetes/tests/cilium_gateway_alpn_test.yaml` asserts the tenant HelmRelease default, the addon-off case, and the `valuesOverride` opt-out. Full suites for both charts pass (93 and 184 tests). - `helm template` of `packages/system/cilium` confirms the value is consumed, not just rendered: default produces `enable-gateway-api-alpn: "true"` (and `enable-gateway-api-app-protocol: "true"`) in `cilium-config`; the threaded `false` produces `"false"` for both. ### Screenshots Not a UI change. ### Downstream repositories The website's hand-written platform values table (`content/en/docs/next/operations/configuration/platform-package.md`) needs a row for `gateway.http2` — filed as cozystack/website#625. No other downstream repository restates anything in this diff (no app `values.schema.json`, ansible-consumed platform key, or hack/ tooling changed). - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: https://github.com/cozystack/website/pull/625 - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note feat(gateway): Gateway API listeners now advertise HTTP/2 via TLS ALPN (h2, then http/1.1) by default, restoring the browser HTTP/2 support lost in the migration from ingress-nginx. Opt out platform-wide with gateway.http2=false, or per tenant Kubernetes cluster via addons.cilium.valuesOverride.cilium.gatewayAPI.enableAlpn=false. Backend (gateway-to-upstream) connections are unchanged and stay HTTP/1.1 unless a Service declares appProtocol kubernetes.io/h2c. ``` |
||
|
myasnikovdaniil
|
d3815a4ce6 |
Merge branch 'main' into release-1.6.0
Signed-off-by: myasnikovdaniil <60174387+myasnikovdaniil@users.noreply.github.com> |
||
|
cozystack-ci[bot]
|
235d38dae1 |
Prepare release v1.6.0 (promoted from v1.6.0-rc.4)
Signed-off-by: cozystack-ci[bot] <274107086+cozystack-ci[bot]@users.noreply.github.com> |
||
|
Timofei Larkin
|
d5806eb1a0 |
feat(kubernetes-nodes): split worker node pool into its own app (Phase 2, additive) (#3314)
## What this PR does Phase 2 of the Kubernetes-app split ([cozystack/community#8](https://github.com/cozystack/community/pull/8)) — the **additive** half. Introduces `kubernetes-nodes`: a per-pool application that renders the worker node-pool objects (`KubevirtMachineTemplate`, `MachineDeployment`, `MachineHealthCheck`, worker `WorkloadMonitor`, and the `talos-reconcile` Job + RBAC) extracted from the monolithic `kubernetes` chart. A `KubernetesNodes` CR named `<cluster>-<pool>` attaches to a parent `Kubernetes` cluster in the same namespace by name convention (`kubernetes-<cluster>`), mirroring the `vm-instance`/`vm-disk` precedent. This PR is additive: the parent `kubernetes` chart gets exactly one scoping fix (its old-KMT preservation block is limited to KMTs owned by its own Helm release) so that attaching a `kubernetes-nodes` pool to a live cluster cannot break the parent's next upgrade. Existing clusters keep provisioning workers via `nodeGroups`. Removing `nodeGroups` from the parent and adopting existing pools into `kubernetes-nodes` releases is a follow-up **breaking** PR (Phase 2b). Key properties: - Pool objects render **byte-identical** to the parent for the same group (golden-parity check in `tests/render-parity.sh`), so the follow-up adoption never churns live worker VMs — the content-hashed `KubevirtMachineTemplate` name is preserved. - cluster-autoscaler (kept in the parent chart) discovers the pool's `MachineDeployment`s cross-release via the unchanged `cluster.x-k8s.io/cluster-name` label, namespace, and min/max annotations. - Registered as a dedicated `cozystack.kubernetes-nodes-application` PackageSource + `kubernetes-nodes-rd` ApplicationDefinition, wired into the iaas bundle (mirrors `vm-instance`/`vm-disk`). ### Downstream repositories Left unticked deliberately — this is a draft. A new user-facing `KubernetesNodes` API is added, so cozystack/website (docs) and cozystack/terraform-provider-cozystack likely need follow-ups; the trigger-map walk will be completed before this leaves draft. - [ ] 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: ```release-note Add the `kubernetes-nodes` application: manage the worker node pools of a managed Kubernetes cluster as independent resources, one HelmRelease per pool. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a new Kubernetes worker node pool offering with configurable sizing (CPU/memory/instance, disk), roles, storage class, GPUs, and Talos worker image settings. * Introduced a dedicated Helm package, dashboard/resource-map access, and Flux application wiring for installing/managing pools. * **Bug Fixes** * Improved safeguards against pool naming/ownership conflicts and tightened upgrade behavior for reusing previously created templates. * **Documentation** * Added full node pool configuration docs and operational notes (including supported Kubernetes versions and reconciliation expectations). * **Tests** * Added node pool rendering/validation tests plus golden parity checks between chart layouts. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
a302bd0ddc |
docs(backupstrategy-controller): reframe the migrations-image reuse as a workaround
Running the platform migrations image as the ClickHouse backup client is not a design decision worth defending, and the in-tree comments read as though it were. Reframe them. The strategy script uses exactly curl, jq, sleep and date. For that it pulls a 349 MB image carrying kubectl, helm, git, cozyhr, the etcd-migrate binary and every migration script into a tenant-adjacent Pod, and that image's ENTRYPOINT is run-migrations.sh -- so the `command:` override in the strategy template is load-bearing rather than incidental. A dedicated alpine + curl + jq + ca-certificates image measures 23 MB. The standing objection to a dedicated image -- "a second tag release CI would have to re-digest each cut" -- does not survive contact with the actual defect. Every other first-party image already pays that cost automatically. What broke was a pin with NO producer, which an image owned by its own package cannot reproduce; that fix would also delete the cross-package stamp introduced here. Comment-only. No rendered output changes. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Ivan Okhotnikov
|
e7947dd0e7 |
feat(platform): register kubernetes-nodes application
Add the kubernetes-nodes-rd package (ApplicationDefinition), the cozystack.kubernetes-nodes-application PackageSource (kubevirt variant, kubernetes-nodes + kubernetes-nodes-rd components), and wire it into the iaas bundle. Mirrors the vm-instance/vm-disk registration precedent. Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com> |
||
|
Myasnikov Daniil
|
427ed68761 |
fix(backupstrategy-controller): give chBackupClientImage a producer
platform-migrations was pinned at two different digests: v1.5.0 in packages/core/platform/values.yaml, stamped by its Makefile, and v1.4.0-rc.2 as .backupStrategyController.chBackupClientImage, stamped by nothing at all. The second is the Altinity strategy Pod's curl+jq runner, deliberately reusing the migrations image rather than shipping a second one-binary tag that release CI would have to re-digest every cut. That reuse only holds while the two move together, and the values.yaml comment said so -- but a comment was the only thing enforcing it, so the copy with no producer froze two minors back. This is a release blocker, not just staleness. Promotion retags by digest, copying every collected <repo>@<digest> to <repo>:<stable>. Two digests under one repository produce two copies competing for one destination tag: the first wins and the second hits the write-once guard in hack/promote-retag.sh, which fails the promotion. A dry run against the tree emitted exactly that pair. v1.6.0 is the first release cut through promotion rather than a full rebuild, so it would have been the first to hit it -- every earlier release ran `make build`, which restamped both copies as a side effect and hid the missing producer. packages/core/platform/Makefile now stamps both keys with the same ref, so they are in lockstep by construction rather than by instruction, and the committed value is aligned to the digest core/platform already carried. The image is used only as a curl + jq + ca-certificates runner, so moving it forward two minors changes no behaviour the strategy depends on; it renders and the chart's helm-unittest suite passes. hack/image-pin-consistency.bats adds the general guard -- no repository pinned at more than one digest, driven through the real promotion selector so it tracks whatever set promotion acts on -- plus a specific lockstep assertion so a recurrence names its cause. The general check could not be added before now: it fails on the very duplicate this commit removes. Relates to #3143 (the unowned-pin class) and unblocks the v1.6.0 promote. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Timofei Larkin
|
ebe111c439 |
feat(gateway): advertise HTTP/2 via ALPN on Gateway API listeners
Browsers negotiate HTTP/2 exclusively through TLS ALPN "h2". Cilium's Gateway API implementation ships with enable-gateway-api-alpn=false, so every Gateway listener advertised no ALPN protocols and every browser silently fell back to HTTP/1.1 — a regression against the ingress-nginx path, which advertised h2 out of the box. Enable ALPN (h2, then http/1.1) by default on the client-facing hop: - packages/system/cilium: default gatewayAPI.enableAlpn=true. - platform chart: new gateway.http2 value (default true), threaded as inline component values into the cozystack.networking Package for the isp-full and isp-full-generic bundles, so an explicit false overrides the system/cilium valuesFiles default either way. - apps/kubernetes: tenant-cluster cilium gets enableAlpn=true whenever the gatewayAPI addon is enabled; opt out per cluster via addons.cilium.valuesOverride. The backend hop is unchanged: gateway-to-upstream stays HTTP/1.1 unless a Service opts in per GEP-1911 via appProtocol kubernetes.io/h2c (that support is switched on together with ALPN; no cozystack-shipped Service declares such a value today). Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Andrey KolkovandClaude Fable 5
|
7d117679d4 |
docs(backups): document bucketName's two modes, cover isp-hosted in the wiring test
Branch-review on this PR: for provisionBucket: false the backupstrategy-controller.bucketName helper returns .Values.backupStorage.bucketName verbatim as the real S3 bucket name and every strategy CR plus the Velero BSL bakes it in, so the external-S3 section must require updating bucketName (the Secret's bucketName key alone is not read by the strategies), and the knob table must state the two-mode semantics instead of the COSI-only one. Also route the external-S3 instruction through the platform component override instead of naming raw chart values, and add the isp-hosted variant case to the forwarding test (92 tests green). Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey KolkovandClaude Fable 5
|
1422f8b78a |
fix(platform): forward backupStorage overrides to the backupstrategy-controller Package (#3245)
The documented admin override for the cozy-default backup S3 coordinates (spec.components.backupstrategy-controller.values on the cozystack.cozystack-platform Package) had no effect: that PackageSource exposes only the platform component, so the operator silently ignored the block, and nothing in the platform chart forwarded backupStorage to the child Package. Wire it the same way bundles/iaas.yaml forwards kubevirt / gpu-operator values: a new top-level backupStorage value on the platform chart is rendered into the emitted cozystack.backupstrategy-controller Package CR as components.backupstrategy-controller.values.backupStorage, from where the cozystack operator merges it into the backupstrategy-controller HelmRelease over the chart defaults. When backupStorage is unset the emitted Package is unchanged. Update docs/operations/backup-classes.md to the working override path (spec.components.platform.values.backupStorage) and call out the two plausible-looking paths that do not work. Add a helm-unittest suite covering the no-op default and the forwarding on isp-full and isp-full-generic. Fixes #3245 Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
cozystack-ci[bot]
|
d95685c427 |
Prepare release v1.6.0-rc.4
Signed-off-by: cozystack-ci[bot] <274107086+cozystack-ci[bot]@users.noreply.github.com> |
||
|
Myasnikov Daniil
|
3ca62266f8 |
fix(platform): qualify package names in the bundle guard messages
Address review feedback on the guards added alongside the isp-hosted dependency-closure fix. Spell every package the guards name in its fully-qualified form (cozystack.cozystack-engine rather than cozystack-engine, and likewise for cert-manager, prometheus-operator-crds, postgres-operator, objectstorage-controller and cozystack-basics). The messages exist to be acted on, and an operator who greps the emitted Package CRs for the name they were given should find it. Also correct the direction of the cross-reference in the bucket-application comment: the velero comment it points at sits above the block, not below. No behavioural change — the guards fire on the same conditions and the tests match on the unchanged leading clause. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
0bfbff26ff |
fix(platform): restore the isp-hosted dependency closure
The isp-hosted preset shipped bundles.system.enabled: false with paas and
naas enabled. templates/bundles/system.yaml is gated end to end on that
flag, but it carries two separable things: a variant-selected data-plane
head (kubeovn/cilium/linstor on isp-full, a noop networking Package on
isp-hosted) and a variant-agnostic control-plane body — cozystack-engine,
the sole source of the application-definition-crd component, plus
cert-manager, prometheus-operator-crds, postgres-operator and
objectstorage-controller.
"Hosted" means the host already provides CNI, storage and virtualisation,
so only the data-plane head should be dropped. Disabling the whole bundle
also deleted the control-plane body, leaving an app catalog whose entire
dependency closure was absent: all 24 emitted Packages sat in
DependenciesNotReady, the *-rd HelmReleases failed with `no matches for
kind "ApplicationDefinition" in version "cozystack.io/v1alpha1"`, and the
cozystack-platform HelmRelease never went Ready. That the variant field
was still set to isp-hosted — and that system.yaml has an isp-hosted
branch which could never render — shows the disable was the mistake, not
the intent; iaas.yaml already refuses to render on this variant, encoding
the same "system bundle minus iaas" contract. The preset has never worked
since it was introduced in
|
||
|
Myasnikov Daniil
|
47e24f01d1 |
fix(platform): hand seaweedfs-db over for every instance name, not just the default
Migration 43 performs the v1.5.0 db-split hand-over: it re-owns Cluster/seaweedfs-db from <name>-system to the new <name>-db release and stamps helm.sh/resource-policy: keep, so the <name>-system upgrade — whose post-split chart no longer renders the Cluster — does not prune it as a removed resource. It compared the owning release name against the literal "seaweedfs-system". SeaweedFS is a user-creatable kind, so an instance named `foo` is owned by `foo-system` and was silently skipped: no re-own, no keep. The <name>-system upgrade then deleted the Cluster, CNPG took its PVC with it, and the filer metadata — the index for every object in that tenant's S3 — was destroyed. Data loss, not an outage: the volume PVCs still hold the bytes, nothing can find them. Verified live on the upgrade stand, which is a natural control group. All five tenants sit in the SAME failed-upgrade loop (<name>-system last-deployed is rev 1, a pre-split revision whose manifest still contains the Cluster, so every retry recomputes the same deletion): tenant-root/dsplit/l instance `seaweedfs` owner=seaweedfs-db keep Cluster alive tenant-named instance `foo` no Cluster at all Cluster GONE The only difference is that migration 43 fired for the default-named instances and skipped `foo`. That also answers whether `keep` is sufficient: three tenants survive the identical prune loop with it. Correction to the reported mechanism: this is the 1.4->1.5 db split (PR #2601, v1.5.0), not a 1.5->1.6 change — v1.5.3 and main are identical on every db-split file. And the split does protect the object; it just only protected the instances someone happened to name `seaweedfs`. - lib/seaweedfs-db-adopt.sh: shared hand-over matching the `-system` SUFFIX, so `foo-system` -> `foo-db` exactly as `seaweedfs-system` -> `seaweedfs-db`. Idempotent, and refuses a release named literally `-system` rather than annotating an owner of `-db` that no release would claim. - Migration 43 sources it: covers clusters upgrading from before 43 (1.4.x). - Migration 53 (new) re-runs it: migrations never re-run, so any cluster already at >= 44 ran the hardcoded version and is still exposed. As a pre-upgrade hook it lands before <name>-system re-renders, closing the window on THIS upgrade. This is the one that matters for 1.5.x -> 1.6. - targetVersion 53 -> 54. Tests fail on unfixed code: the non-default-name case produces no ANNOTATE at all against the hardcoded 43, while the default-name case passes — which is precisely the bug's shape. They drive the real migration scripts against a fake kubectl, following hack/migration-50-etcd-adopt.bats. LIMIT, stated plainly: a Cluster that is already deleted cannot be recovered by either migration — there is nothing left to re-annotate and the PVC went with it. Such a tenant needs seaweedfs-db restored from a backup. Runbook Step 0 adds the ownership audit that distinguishes at-risk from already-lost, and records that <name>-db can report Ready while its Cluster is gone (it rendered fine; a later <name>-system prune removed it and Flux has not re-checked) — so the audit trusts `kubectl get cluster`, not the HelmRelease status. Review round 2 — two ways this still lost the database: - OWNERSHIP IS NOT SAFETY. The helper skipped any Cluster already owned by <name>-db, treating ownership as proof the hand-over was done. It is not: where the hand-over was skipped, <name>-system prunes the Cluster and <name>-db RECREATES it under its own ownership with NO keep, while <name>-system's prune baseline still lists it — so the next reconcile deletes it again. That is the observed delete/recreate loop, and the skip walked straight past it. Verified on the stand: tenant-l and tenant-root are seaweedfs-db-owned AND their <name>-system deployed revision (rev 1) still contains the Cluster, so only keep saves them; tenant-fresh is seaweedfs-db-owned without keep but was installed after the split, so its baseline never had one. Telling those apart needs the release's deployed manifest, which this script cannot read cheaply or reliably. The costs are asymmetric — a needless keep leaves an orphan on delete (now reclaimed by the cleanup hook), a missing one loses the database — so keep is stamped on every Cluster owned by either side of the split. - FAIL OPEN, THEN STAMP. `set -euo pipefail` does not abort on a failing command substitution in a `for` word-list: the loop ran zero times, the script continued, and stamp_cozystack_version ran anyway. Migrations never re-run, so one throttle, RBAC hiccup, or not-yet-established CNPG apiservice behind the pre-upgrade hook permanently left every at-risk tenant exposed with no later migration to catch them. Reproduced: against the old helper a failing fleet scan still logged STAMP 54. Now every kubectl failure is fatal EXCEPT the two that genuinely mean "nothing to do" — the CNPG resource type not being served (the fail-open that IS load-bearing: a cluster without CNPG must still upgrade) and a Cluster vanishing between scan and read. An unreadable annotation is no longer indistinguishable from an absent one, and the unowned case now warns instead of passing silently. - ORPHAN ON DELETE. `keep` is permanent (removal deferred to the 1.7 batch migrations) and also survives the <name>-db release's own uninstall, so `kubectl delete seaweedfs <name>` left the CNPG Cluster, its Postgres pods and its PVCs behind — the cleanup hook selects app.kubernetes.io/instance=<name>-system, which the db PVCs (cnpg.io/cluster=seaweedfs-db) do not carry. Migration 53 widened that from pre-1.5 default-named tenants to every instance name, so the hook now deletes the Cluster explicitly; its PVCs carry ownerReferences to it and follow. - Tests assert the stamped VERSION (44 / 54), not a bare "STAMP": a wrong number would loop run-migrations.sh forever and the old assertion could not tell. Review round 3 — the reclaim introduced a data-loss path of its own: - DELETING ONE APP DELETED ANOTHER APP'S DATABASE. The db chart hardcodes the Cluster name `seaweedfs-db` for every instance, and the reclaim was scoped by that name in the namespace rather than by what the release owns. templates/seaweedfs-db.yaml gates the <name>-db HelmRelease on `topology != "Client"`; the cleanup hook had no such gate. Verified by render: `helm template s3-remote . --set topology=Client` emits ZERO s3-remote-db HelmReleases yet still emitted the Role granting delete on clusters/seaweedfs-db AND `kubectl delete cluster.postgresql.cnpg.io -n tenant-root seaweedfs-db`. So in a namespace running a server instance plus a Client, deleting the CLIENT destroyed the SERVER's filer metadata — CNPG takes the PVC via ownerReferences — for an app the operator never touched. Reachable the same way via a second Simple instance, whose <name>-db loses Helm's ownership check but which still installs and still carries the hook. main's hook was instance-scoped; this was new, and mine. Now gated exactly as seaweedfs-db.yaml is, and the Job re-checks meta.helm.sh/release-name == <release>-db at run time before deleting. The gate stops the permission being granted where it can never be legitimate; the runtime check stops the delete where it can. - `|| echo` turned any API/authz/admission failure into exit zero, so backoffLimit never retried and hook-delete-policy removed the only cleanup Job while the keep-protected Cluster stayed orphaned. A failed reclaim now fails the Job. - TEST SHELL. The migrations run under /bin/sh = busybox ash (the image is FROM alpine, run-migrations.sh is #!/bin/sh), but the tests drove them with bash — so the fail-closed guarantees were asserted in a shell that never runs them, which matters more than usual given the bug they cover was a fail-open. The scripts are now invoked via `sh` and the fake kubectl is POSIX sh. Verified directly against the production base image (busybox 1.37.0 / alpine:3.24): `set -o pipefail` is supported, every script passes `ash -n`, and the failing-fleet-scan path exits non-zero without stamping under real ash. The recipe is in the bats header. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
efa85a4f61 |
feat(platform): make the etcd-adoption skip-backup hatch reachable
Migration 50 has honoured ETCD_ADOPT_SKIP_BACKUP since it was written, and its failure paths tell operators to "re-run with ETCD_ADOPT_SKIP_BACKUP=1". But migration-hook.yaml passes only NAMESPACE/CURRENT_VERSION/ TARGET_VERSION, and the Job is a Helm hook the chart re-renders, so there has never been a way to set it. The script advertised an escape the platform made impossible. The endpoint-classification work landing alongside this makes that worse: it adds refuse-rather-than-guess paths, so more clusters can now legitimately stop here. Plumb it through migrations.etcdAdoptSkipBackup, default false. Skipping the pre-adoption snapshot rewires ownership of LIVE etcd storage with no way back, so it stays opt-in and deliberate — the flag is the last resort for a cluster that has nothing to fix, not the first response to a refusal. Render "1"/"0" rather than the bare bool. The script matches the literal string "1", so a bool would render "true", sail through the Job spec looking correct, and be ignored — a safety valve that is set but silently disregarded is worse than one that was never offered, because the operator believes they opted in. The value is normalised through a string, so a bool, a quoted "true" out of the Package CR's Values JSON, or a numeric 1 all land on the same answer, while nil or garbage resolves to "0": taking this hatch must require an affirmative spelling, never a typo. Resolved with `dig` so a Platform Package predating the key renders "0" instead of failing the render of the entire platform chart and bricking every upgrade. Widen the script's own check to accept 1/true/yes as well. The template guarantees the GitOps path, but an operator running this image by hand will reasonably type "true", and that is exactly the moment they are already stuck. The two layers cover independent failure modes. Emit the var unconditionally, including the "0" default, so the rendered Job states the cluster's safety posture outright rather than leaving an operator to diff values to find out whether a snapshot was skipped. Point the failure messages at the values path that actually exists, and describe it honestly: editing the Platform Package and letting Flux re-render is a real operation, not a one-liner, and the flag has to be reverted afterwards or the next etcd migration skips its snapshot too. Fixing the condition the migration names remains the recommended path — the migration is idempotent, so re-running after a fix is safe. templates/migration-hook.yaml had no coverage at all, because the Job only renders when a lookup of the cozystack-version ConfigMap reports a version below targetVersion, and lookup is nil under helm template. The new suite mocks that ConfigMap and asserts the RENDERED VALUE — enabled, disabled, key absent, stringified true, garbage, and the pre-existing three env vars — rather than merely that the variable exists, which is what let the trap above go unnoticed in the first place. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
0ba5337526 |
fix(migrations): key the etcd-adoption endpoint scheme off the claimed bucket
Deriving the safety-snapshot target from the projected cozy-backups-creds Secret is right, but detecting the admin-managed external-S3 case (provisionBucket: false) by "the projected endpoint is absent" is not: that state does not exist. The projector substitutes BACKUP_STORAGE_ENDPOINT and fails ReasonSourceMalformed when both sources are empty, so a projected Secret always carries a non-empty, scheme-stripped endpoint. So for provisionBucket: false against a plaintext external S3 the projected endpoint is a bare host, the script saw it non-empty, forced https:// and never consulted the Strategy CR — a regression against main, which read the CR verbatim and passed http:// through. Being a pre-upgrade hook with no ETCD_ADOPT_SKIP_BACKUP plumbed through migration-hook.yaml, that is a total upgrade block with no operator escape. Classify on data instead: match the projected bucketName against the .status.bucketName of any BucketClaim. The COSI driver assigns that name and the projector republishes exactly it (packages/system/bucket user-credentials.yaml reads BucketInfo spec.bucketName), so a match means the creds describe a COSI bucket => the host is the always-TLS S3 ingress => force https, as the 1.6 chart helper does. No match means external S3. Keying on data rather than on the claim's name and namespace matters twice. backupStorage.namespace/.bucketName are supported Package-CR overrides that this hook cannot see, so a name lookup would miss a renamed bucket and fall back to the v1.5.x plaintext CR — the original P0. And a BucketClaim wedged Terminating (its cosi bucketclaim-protection finalizer outlives an uninstalled COSI controller) keeps answering a name lookup while its status names some other bucket. Split the two paths by which source is authoritative. On COSI the projected coordinates win: the bucket is the COSI-assigned name the CR can only reproduce through a live lookup that may never have run. On external S3 the Strategy CR wins WHOLESALE, not just for the endpoint — the CR renders backupStorage.* and points at this same cozy-backups-creds, so CR coordinates plus these credentials is exactly what the cluster's own BackupJobs already use, and the bucket an operator will look in. Keeping the Secret's bucket while taking the CR's endpoint assembled a pairing nothing else produces: the projector copies bucketName straight from the admin-managed source Secret and never from backupStorage.bucketName, so the two can disagree outright and the snapshot could land in a bucket the platform never writes. Relying on the CR is safe on this path alone, because bucketName short-circuits to values before the BucketClaim lookup that races on the COSI path, so the CR is guaranteed present. Match with a single jq `any`, never `jq -r | grep -q`: grep exits on the first match and SIGPIPEs jq, and pipefail reports jq's 141 rather than grep's 0, so a MATCH returns as a failure and the cluster is misclassified external. It is a race on the list outgrowing one stdio write (~8KB) with only a last-line match safe, so it strikes exactly the big multi-tenant clusters that can least afford it, and backoffLimit 3 can decide differently on each retry. Measured with the match first, 20 runs each: 300 claims went 0/20 correct (rc=141 every time) and 2000 likewise, while `any` is 20/20 at 3, 300 and 2000. Refuse rather than guess when the answer is not knowable. Three states qualify: the BucketClaim API is unreadable; the list is not interpretable (no producer emits that, but without a shape gate it reads as "no claim" = external = the plaintext P0, silently); or the bucket is claimed ONLY by a Terminating claim. That last one is ambiguous — an admin who moved COSI -> external S3 reusing the bucket name, versus a still-COSI cluster whose claim is mid-recreate — and both readings fail as the same confusing handshake error when wrong, so a guess buys no reliability while reading it as "external" specifically resurrects the plaintext-:8333 P0. A live claim still wins over an unrelated Terminating one, so healthy clusters are unaffected. Each refusal names the condition and the operator action; the migration is idempotent, so re-running after resolving it is the remedy. Behaviour change worth noting: a cluster on external S3 that still has COSI installed now refuses on a sustained BucketClaim-API failure where main would have proceeded off the Strategy CR. That is the intended trade — main proceeded only because it never had to tell the two apart — but it does mean an apiserver outage can now block this hook where it previously did not. ETCD_ADOPT_SKIP_BACKUP is still not plumbed through migration-hook.yaml, so the messages deliberately point at resolving the condition rather than at an escape hatch the operator cannot reach. Force the scheme only onto a non-empty host: an absent endpoint must degrade to the CR as main did, not to a literal "https://" that would pass the final non-empty guard and reach etcd-migrate as a destination. The old test modelled external S3 by dropping the projected endpoint entirely — a state the projector cannot produce — so it green-lit the regression. It now models what the projector really writes, and the suite pins the stale-claim, overridden-coordinates, empty-endpoint, 300-claim-multi-tenant, Terminating-match, hybrid-coordinate and unreadable/uninterpretable API cases. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
9ceccd2b07 |
fix(migrations): derive the etcd-adoption snapshot target from the projected bucket creds
Migration 50 is a pre-upgrade hook, so every chart-rendered resource it reads still belongs to the version being upgraded FROM. It read the snapshot target from the live cozy-default-etcd Strategy CR, which on a v1.5.x cluster is broken in one of two ways — and the platform upgrade is gated behind this snapshot, so neither can self-heal first: 1. Absent. strategy-etcd-default.yaml is guarded by `if $bucketName`, whose helper looks up the BucketClaim status. That BucketClaim is created by the same chart, so on first render the lookup is empty and the Strategy is silently skipped. Helm lookup only runs at install/upgrade time, so a cluster that reached 1.5.x in one hop never grows the CR on reconcile. 2. Present, carrying the static v1.5.x default http://seaweedfs-s3.tenant-root.svc.cozy.local:8333. Cozystack ships SeaweedFS with enableSecurity=true, so :8333 is a TLS listener and plaintext against it fails the handshake. Fixing the scheme alone is not enough: the Etcd Strategy's S3 schema has no caCert/insecureSkipVerify, so the self-signed in-cluster endpoint is unusable at any scheme and a trusted-cert endpoint is required. Either way migration 50 hard-fails and blocks the upgrade, and the documented ETCD_ADOPT_SKIP_BACKUP escape hatch cannot be reached — migration-hook.yaml passes only NAMESPACE/CURRENT_VERSION/TARGET_VERSION. Take the coordinates from cozy-backups-creds instead. The projector writes it from the COSI-provisioned bucket's system credentials — bucket provisioning, not chart values — so it describes the live bucket regardless of which version rendered the charts, and it carries endpoint, bucketName, region and forcePathStyle alongside the AWS keys. resolve_platform_backup_args already read this Secret to validate those keys, so preferring it adds no new dependency. The endpoint is a bare host fronted by the always-TLS S3 ingress, so force https://, exactly as the 1.6 chart helper backupstrategy-controller.endpoint does. The Strategy CR remains the fallback for the admin-managed external-S3 case (provisionBucket: false), where .Values.backupStorage.endpoint is authoritative and may legitimately be plaintext against a private store; that path is detected by the absence of a projected endpoint and its scheme is preserved verbatim. The bats fixture modelled cozy-backups-creds as carrying only the AWS keys, and test 1 asserted the broken plaintext endpoint as expected output — the test was written against the implementation rather than the requirement, which is why the defect shipped green. The fixture now carries the coordinates the real projected Secret has, test 1 asserts the derived https endpoint, and two cases are added: an absent Strategy CR still resolving, and external S3 keeping the CR endpoint verbatim. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
0238231303 |
fix(linstor-scheduler): re-vendor to upstream 0.3.1 with native admission webhook (#3201)
## What this PR does Re-vendors the `linstor-scheduler` chart from upstream **0.3.1** (appVersion **v0.3.6**). Upstream now ships the admission webhook natively (contributed via [piraeusdatastore/helm-charts#94](https://github.com/piraeusdatastore/helm-charts/pull/94)), so the loose downstream admission templates — which round-tripped every Pod through an old `k8s.io/api` and silently stripped fields they did not know about, the `image` volume source and the native-sidecar `restartPolicy` among them, breaking KubeVirt containerDisk VMs and native sidecars cluster-wide — are dropped in favour of the upstream copy. - The admission webhook comes from the upstream chart now; the previous downstream copy and its patch are removed. `make update` pulls 0.3.1 and applies only `cozystack-scheduler-deployment.patch` (the scheduler pod component label and the distro-suffix-stripped kube-scheduler image tag, neither of which is upstream yet), so vendoring stays deterministic and reproducible. - The upstream admission Deployment keeps the same name but changes its immutable pod selector, so an in-place upgrade of an existing install would fail with `field is immutable`. Migration 52 deletes the old admission Deployment before the upgrade so the new one is recreated cleanly (`targetVersion` 52 → 53). - appVersion v0.3.6 also carries the webhook's TLS certificate hot-reload fix ([piraeusdatastore/linstor-scheduler-extender#22](https://github.com/piraeusdatastore/linstor-scheduler-extender/pull/22)): the webhook used to load its certificate once at startup, so after the mounted Secret was rotated it kept serving a stale certificate and — with `failurePolicy: Ignore` — silently stopped mutating until it was restarted. It now reloads on rotation, so no external cert-reload workaround is needed here. - Wrapper values keep `replicaCount: 2`, now that the admission Pods have their own PodDisruptionBudget and no longer share the scheduler's. - Adds a helm-unittest suite covering the admission resources, PodDisruptionBudget selector isolation, least-privilege RBAC, the TLS provisioning modes and their rejection paths, the scheduler image-tag strip, and backward compatibility with the webhook disabled. Closes #3198. ### Screenshots N/A — no UI changes. ### Downstream repositories I walked the trigger map against the diff, file by file. It touches `packages/system/linstor-scheduler/` and, under `packages/core/platform/`, only migration 52 and the `migrations.targetVersion` bump — an internal, release-managed counter, not a user-facing `spec.components.platform.values.*` key documented on the website. No 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: ### Release note ```release-note fix(linstor-scheduler): the LINSTOR scheduler admission webhook no longer strips unknown Pod fields (the `image` volume source and native-sidecar `restartPolicy`), which had broken KubeVirt containerDisk VMs and native sidecars, and it now reloads its TLS certificate after rotation instead of silently failing. The chart is re-vendored from upstream 0.3.1; an automatic upgrade migration replaces the admission Deployment whose immutable selector changed. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Expanded the optional Linstor Scheduler admission webhook with richer configuration, including automated `schedulerName` mutation for LINSTOR CSI-backed PVC Pods. * **Improvements** * Updated the Linstor Scheduler Helm chart to **0.3.1** (scheduler app **v0.3.6**), including updated admission webhook behavior, safer defaults, and **2 replicas**. * Platform migrations now advance to **version 53**, with upgrade-safe cleanup of the existing admission deployment. * **Documentation** * Updated Helm README and post-install notes with detailed guidance for admission webhook and TLS modes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrei Kvapil
|
fbbe4af7df |
refactor(network): drop ExposureClass/ServiceExposure for native loadBalancerClass (#3218)
## What Removes the `network.cozystack.io` API group — `ExposureClass` + `ServiceExposure` (#3081) — and its reconciler, in favor of native `Service` `type: LoadBalancer` + `loadBalancerClass`. ## Why Full rationale and discussion in #3164. For **managed applications** the chart already owns the Service, so: - `type: LoadBalancer` + `loadBalancerClass` + an admin-provisioned pool covers vendor-neutral external exposure without a dedicated API group and controller; - the `externalIPs` (node-IP) default never went through `ServiceExposure` (its backend is a no-op) and is left unchanged; - the only current consumer, the host ingress, keeps rendering `type: LoadBalancer` and just drops the `ServiceExposure`; the admin provisions the address pool / announcer out of band, as the install docs already assume. `ServiceExposure`'s one unique feature — dynamic per-namespace scoping of a shared VIP pool — is a convenience the managed-app path does not need, and it does not justify a second tenant-networking API group next to `sdn.cozystack.io`. ## Changes - Delete `api/network/v1alpha1` and `internal/controller/serviceexposure` (+ CRD definitions, e2e bats, controller registration, RBAC). - Host ingress: drop the `ServiceExposure` template; add an optional `publishing.loadBalancerClass` that sets `spec.loadBalancerClass` on the host-ingress LoadBalancer Service. - The `externalIPs` path and default behavior are unchanged. ## Verification - `go build ./...` — green. - `helm unittest` — `core/platform` 72/72, `extra/ingress` 25/25 (incl. host-vs-tenant `loadBalancerClass` scoping and the immutable-field migration guard). ## Migration None required. `network.cozystack.io` (#3081) has never shipped in a release — it is on `main` only (not backported to `release-1.5`; absent from v1.4.x / v1.5.x), so no upgrading cluster has these CRDs and this removes it before it is ever released. Clusters that track `main` and explicitly set `publishing.exposureClass` (opt-in, only since 2026-06-30) should provision their MetalLB / Cilium address pool themselves and set `publishing.loadBalancerClass`; any orphaned `cozystack-<class>` pool can be deleted by hand. Default clusters (externalIPs path) are unaffected. ## Status **Draft / RFC for the #3164 discussion** — this removes an API group that landed only days ago, so nothing here should merge before the design call. cc @lexfrei (author of #3081) and @lllamnyp (#3164). Deferred: when Kubernetes actually removes `Service.spec.externalIPs`, the node-IP default migrates via a MetalLB `IPAddressPool` with no `L2Advertisement` (allocate-but-don't-announce) — a separate change. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for configuring the ingress host Service with a native `loadBalancerClass` value. * Expanded chart tests to verify class wiring and immutability behavior across install and upgrade scenarios. * **Bug Fixes** * Removed the legacy exposure-class/service-exposure flow and related controller behavior. * Updated templates and CRD packaging so the platform no longer renders or depends on those deprecated resources. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
f4aef73053 |
fix(linstor-scheduler): re-vendor to upstream 0.3.1 with native admission webhook
Re-vendor the linstor-scheduler chart from upstream 0.3.1 (appVersion v0.3.6), which carries the admission webhook binary that stopped stripping unknown Pod fields — the image VolumeSource and native-sidecar restartPolicy, which broke KubeVirt containerDisk VMs and native sidecars cluster-wide — and now reloads its TLS certificate on rotation instead of serving a stale one until restarted. The admission webhook ships in the upstream chart, so the previous downstream copy is dropped: `make update` pulls it directly and applies only cozystack-scheduler-deployment.patch (pod component label and a distro-suffix-stripped image tag), keeping the vendoring deterministic. Because the webhook now reloads its own certificate, the external cert-reload workaround is dropped. The upstream admission Deployment keeps the same name but changes its immutable pod selector, so an in-place upgrade of an existing install would fail. A migration deletes the old admission Deployment before the upgrade so the new one is recreated cleanly, and bumps the migration target version. Add a helm-unittest suite covering the admission resources, PDB selector isolation, least-privilege RBAC, the scheduler image tag, and backward compatibility. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
myasnikovdaniil
|
f6a03e93f8 |
fix(etcd): complete v1alpha2 transition for in-cluster 1.5→1.6 upgrades (#3270)
Consolidates the etcd `v1alpha2` transition fix for **in-cluster 1.5 → 1.6 upgrades** into a single PR, rebased on current `main`. Supersedes #3265 and #3261 — their commits are carried here (authorship preserved), so those PRs can be closed once this lands. Fresh installs use the new shapes directly and are unaffected, so fresh-install CI does not catch these; the 1.5 → 1.6 e2e upgrade path is the authoritative regression guard. ## What this PR does ### 1. Keep the legacy `etcd-headless` Service alive during adoption — `packages/extra/etcd` `etcd-migrate` adopts legacy clusters **in place**: the Pods keep their original `spec.subdomain: etcd-headless` and are dialed at `etcd-<i>.etcd-headless.<ns>.svc` until they roll onto the operator's native `<member>.etcd.<ns>.svc` domain. The v1alpha2 operator only creates the native `etcd` Service and the legacy `etcd-headless` Service is pruned, so those per-pod names stop resolving (`no such host`), `MemberList` fails, and `status.readyMembers` never populates — the `EtcdCluster` never goes `Ready` even though etcd is healthy and in quorum. We ship a chart-managed transitional headless `etcd-headless` Service (selector mirrors the operator's native `etcd` Service via `etcd-operator.cozystack.io/cluster`, `publishNotReadyAddresses: true`) — the DNS counterpart of the legacy `*.etcd-headless.<ns>.svc` SAN already kept for this window. Removable together with that SAN once members roll onto the native subdomain. ### 2. Survive the immutable controller-Deployment selector on upgrade — `packages/system/etcd-operator` (#3242) #2859 replaced the upstream etcd-operator chart with the cozystack-authored one, changing `Deployment.spec.selector.matchLabels`. `spec.selector` is immutable, so `helm upgrade` cannot patch the existing Deployment and the whole HelmRelease upgrade fails (`field is immutable`). A **pre-upgrade hook** (`templates/pre-upgrade-selector-fix.yaml`: ServiceAccount + Role + RoleBinding + Job) deletes the Deployment **only** when its live selector is the pre-1.6 one, so Helm recreates it cleanly. No-op when the selector already matches, never runs on fresh install. ### 3. Raise the operator's memory cold-start floor — `packages/system/etcd-operator` Steady-state working set is ~250Mi; the static `limits.memory: 128Mi` OOMKills a Pod that starts before the VPA admission webhook rewrites it. Raise the floor to `256Mi` (and VPA `minAllowed` to match) as defense in depth so the operator never depends on VPA timing to avoid crashing. ### 4. Make migration 50 (etcd adoption) robust in-cluster — `packages/core/platform/images/migrations/migrations/50` (was #3261) Exact server/peer cert-SAN match, Secret-gated wait on the adoption Secret, and in-cluster (IPv6-safe) kubeconfig handling, so the migration script drives the adoption reliably from inside the cluster. Covered by `hack/migration-50-etcd-adopt.bats`. ### 5. Hardening / review fixes (this PR's original scope) - **Hook `runAsUser: 65532`.** `pre-upgrade-selector-fix.yaml` set `runAsNonRoot: true` but no numeric `runAsUser`; `clastix/kubectl`'s image user is the non-numeric name `nonroot`, which the kubelet cannot verify against `runAsNonRoot` — so the hook Pod fails admission and silently blocks the very upgrade it exists to unblock. Adds `runAsUser: 65532`, matching the postgres-operator webhook-ready hook that runs the same image. - **Digest-pin the kubectl image.** The values comment claimed digest-pinning but shipped a floating `v1.32` tag. Reuses the digest postgres-operator vendors, adds the `renovate` annotation, and templates `repo:tag@digest`. - **Tests.** `etcd-operator/tests/selector-fix-hook_test.yaml` (hook wiring, weight ordering, namespaced least-privilege RBAC, numeric-non-root security context, digest-pinned image), `etcd-operator/tests/deployment_test.yaml` (256Mi cold-start floor), `extra/etcd/tests/etcd-cluster_test.yaml` (transitional `etcd-headless` Service). Each assertion was mutation-tested. ### 6. Derive the default S3 endpoint from the provisioned bucket — `packages/system/backupstrategy-controller` Derive the default S3 endpoint (and per-driver scheme / TLS / `secure_connection`) from the provisioned bucket Secret instead of requiring it to be hand-set, so etcd (and other) backup strategies get a working endpoint by default. Docs in `docs/operations/backup-classes.md`; covered by `tests/endpoint_form_test.yaml`. ### Verification - `helm unittest` green on current `main`: etcd-operator **18/18**, extra/etcd **18/18**, backupstrategy-controller **11/11**. - Behaviours 1 & 2 were reproduced and confirmed live on a 1.5.2 → 1.6.0-rc.1 adoption (3-node cluster) — recreating the `etcd-headless` Service took the adopted cluster to `readyMembers=3 / Available=True`. ```release-note fix(etcd): complete the v1alpha2 transition on in-cluster 1.5→1.6 upgrades — keep the legacy etcd-headless Service alive so adopted members stay resolvable, delete the pre-1.6 operator Deployment via a pre-upgrade hook to get past the immutable selector, raise the operator's memory floor so it does not OOM before the VPA scales it, and make the etcd adoption migration robust in-cluster. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved backup storage endpoint handling across supported backup drivers, including provisioned and external S3 storage. * Added compatibility support for legacy etcd pod discovery during migration. * Added an automated upgrade safeguard for etcd operator deployments. * **Bug Fixes** * Improved certificate SAN detection and etcd migration authentication. * Increased the etcd operator’s minimum startup memory to prevent early restarts. * **Documentation** * Clarified backup endpoint, TLS, and driver-specific behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey Kolkov
|
47d239c519 |
fix(platform): address review on migration 50 (exact SAN match, Secret-gated wait, IPv6 kubeconfig)
Review feedback on #3261 (gemini-code-assist, coderabbitai, myasnikovdaniil): - Exact SAN match. _san_present read the Certificate SANs as {.spec.dnsNames} (a bracketed JSON blob that `tr ','` cannot split) and matched with a substring `grep -qF`, so a wildcard that is a substring of a longer SAN (e.g. *.etcd.<ns>.svc inside *.etcd.<ns>.svc.cluster.local) could false-positive and skip the re-issue. Read {.spec.dnsNames[*]} (space separated) and match exactly with `grep -qxF`, matching the Secret-annotation branch. - Wait on the re-issued Secret, not the patched spec. The post-patch wait loop called _san_present, which returns as soon as the Certificate spec.dnsNames contains the wildcard -- but the patch just added it there, so the loop broke on the first iteration before cert-manager re-issued the Secret, and the "re-issued" log printed unconfirmed. Add _secret_has_san (Secret alt-names annotation only, same empty-read retry + exact match) and gate the wait on it. - IPv6-safe kubeconfig server. The synthesized kubeconfig used https://$KUBERNETES_SERVICE_HOST:$KUBERNETES_SERVICE_PORT, which is an invalid URL on an IPv6-only cluster (a bare IPv6 host must be bracketed). Use https://kubernetes.default.svc -- IP-family-agnostic, validated by the mounted SA CA, and the same server URL kubectl synthesizes in-cluster. Tests (the review blocker): extend the bats suite (and its fake kubectl) with two cases pinning these contracts -- a superstring dnsName must NOT skip the re-issue patch, and etcd-migrate must be invoked with a synthesized in-cluster kubeconfig whose server is kubernetes.default.svc (not the bare IPv6 host), authenticating via the SA token file + CA. Both the kubeconfig path and the SA directory are now env-overridable (ETCD_MIGRATE_KUBECONFIG, ETCD_ADOPT_SA_DIR) so the synthesis is exercisable off-cluster; production defaults are unchanged. Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
c2539ca935 |
fix(platform): make etcd v1alpha2 adoption (migration 50) robust in-cluster
Migration 50 (etcd.aenix.io -> etcd-operator.cozystack.io/v1alpha2 adoption) had two defects that blocked every in-cluster 1.5 -> 1.6 upgrade on a cluster with an existing etcd: 1. Cert-SAN wait treated transient kubectl failures as "SAN absent". ensure_wildcard_sans checked/awaited the wildcard SAN with `kubectl get ... 2>/dev/null | grep`, so any transient GET failure (API discovery refresh, apiserver blip, throttling) produced an empty string indistinguishable from a genuine absence -> false miss, and the 120s wait never recovered. Replace the two ad-hoc checks with a _san_present helper that retries on an empty read (a real Certificate/Secret never has empty dnsNames/alt-names) and accepts the native wildcard from EITHER the issued Secret's cert-manager.io/alt-names annotation OR the Certificate spec.dnsNames (the source of truth for what cert-manager will issue). 2. etcd-migrate had no kubeconfig in-cluster. etcd-migrate only reads a kubeconfig file (-k/--kubeconfig, default /root/.kube/config) and, unlike kubectl, does not fall back to the mounted in-cluster ServiceAccount. The hook Job set no KUBECONFIG and passed no --kubeconfig, so both the dry-run and --apply aborted with "error building kubeconfig: stat /root/.kube/config: no such file". Synthesize an in-cluster kubeconfig from the mounted ServiceAccount and pass --kubeconfig to both etcd-migrate invocations. Verified end-to-end on a 1.5.2 -> 1.6.0-rc.1 upgrade: the adoption now completes in-place (pods never restarted, data intact) and the cluster reaches readyMembers=3 / Available=True. Refs: #3243, #3255 Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
IvanHunters
|
d4f59af624 |
chore(monitoring): sync PackageSource comment with removed Keycloak groups
Update the oidc-variant comment on `packages/core/platform/sources/monitoring-application.yaml` so it no longer claims `spec.oidc.mode: System` renders three KeycloakRealmGroups. The dependsOn edge and the fail-close guard in `templates/grafana/oidc-keycloak.yaml` stay unchanged: the KeycloakClient + KeycloakClientScope path still needs the `v1.edp.epam.com` CRDs from `cozystack.keycloak-operator` to be registered before the monitoring chart reconciles. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Ivan Okhotnikov
|
b46543e5f2 |
fix(monitoring): defer platform GrafanaAdmin lever to a future phase
The MAJOR finding from the cozy-review agent: the platform-side `oidc.grafanaAdmin: true` documented in the PR body and in docs/oidc-grafana.md never actually reached the platform Grafana's HelmRelease. `Package.spec.components["monitoring-system"].values.oidc.grafanaAdmin` was set in packages/core/platform/templates/bundles/system.yaml, but `monitoring-system` (and `monitoring`) have no `install:` block in packages/core/platform/sources/monitoring-application.yaml — only `monitoring-rd` does. PackageReconciler skips components without an `install:` block before ever reaching the assignment that copies `pkg.Spec.Components[].Values` onto `hr.Spec.Values` (internal/operator/package_reconciler.go around lines 197-201 and 292-293). The values-override dictionary was silently dropped, no HR carried `oidc.grafanaAdmin: true`, and the platform Grafana rendered with the chart default (`false`). The `bundles_monitoring_grafana_admin_test.yaml` unit test only asserted against the emitted Package CR spec — exactly the layer where the value still existed but was about to be discarded — so the broken propagation slipped past. Trim the broken promise from Phase 1. The chart-side helper (`monitoring.oidc.allowAssignGrafanaAdmin`) stays as-is so a future phase that routes the override through the tenant-root chart's monitoring HR values (or adds an `install:` block on the `monitoring-system` component) lands on a green chart-side contract. The helm-unittest that verifies chart-side emission stays too, but with a docstring calling out that end-to-end wiring is deferred. Everything else that promised the lever works — bundle line, docs, platform bundle test — is either removed or rewritten to match reality. Also picks up two MINOR findings from the same review: * Drop the misleading `secret-hash` annotation on the OIDC client-secret Secret. The comment claimed it "rotates the KeycloakClient CR and therefore the Grafana Deployment env when the client-secret changes", but nothing downstream watches an annotation on the Secret itself — no rotation is triggered. The `lookup`-based fallback already keeps the value stable across reconciles, so no rotation is even needed. Removing the dead annotation avoids planting a broken assumption for future readers. * Add `release.cozystack.io/helm-install-timeout: "15m"` on the monitoring cozyrds ApplicationDefinition. Under `mode: System` the chart now emits, on top of the existing Grafana + Alerta + VMCluster + CNPG stack, four additional Keycloak CRs whose reconciliation is asynchronous against an operator in `cozy-keycloak` (not the tenant namespace). Flux's default 5m install window was already tight for the pre-OIDC stack; 15m matches the precedent set by tenant-rd/kubernetes-rd cozyrds. Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com> |
||
|
Ivan Okhotnikov
|
be52e80289 |
fix(monitoring): split monitoring-application PackageSource into default/oidc variants
Follow-up to the previous self-review-fix commit: the unconditional `- cozystack.keycloak-operator` in dependsOn was itself a CRITICAL cold-install regression on any cluster where `authentication.oidc.enabled: false` (the default). The keycloak-operator Package is only emitted by `packages/core/platform/templates/bundles/system.yaml:112` when that flag is on — a hard dep against it from the default variant would leave `cozystack.monitoring-application` stuck on DependenciesNotReady forever, cascading a `cozystack-platform` HR timeout that hits every OIDC-off customer. Fix by mirroring the cozystack-engine variant split: - `default` variant keeps the baseline reconciliation edges (no keycloak-operator dep). - `oidc` variant additionally waits for `cozystack.keycloak-operator` so the v1.edp.epam.com CRDs are registered before the monitoring chart reconciles and the `.Capabilities.APIVersions.Has` guard in templates/grafana/oidc-keycloak.yaml can safely emit KeycloakClient/Scope/Groups. The bundle template swings between variants on `.Values.authentication.oidc.enabled`, mirroring the cozystack-engine invocation two lines above. The `monitoring-system` component override (oidc.grafanaAdmin: true) is preserved across both branches — the values field is a no-op when mode: None (no auth.generic_oauth block is emitted at all), so keeping it identical avoids two divergent code paths. Also widened the platform helm-unittest coverage to lock: - default variant does NOT depend on keycloak-operator (permanent DependenciesNotReady guard). - oidc variant DOES. - Bundle emits the right variant based on authentication.oidc.enabled AND keeps the GrafanaAdmin uplift on the monitoring-system component in either branch. 83 platform tests + 23 monitoring tests still green. Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com> |
||
|
Ivan Okhotnikov
|
ffeb8e5e4f |
fix(monitoring): close self-review blockers for the OIDC PR
Address the two blocking-class findings from the cozy-review pass on
this PR:
(1) Missing PackageSource dependency on cozystack.keycloak-operator
(MAJOR cold-install race).
Without this edge, the monitoring chart can reconcile before the
v1.edp.epam.com CRDs are registered on a fresh cluster — the
`.Capabilities.APIVersions.Has` guard in
templates/grafana/oidc-keycloak.yaml then silently drops all
Keycloak-side objects for mode: System, and Grafana's
auth.generic_oauth block points at a client Keycloak never
provisions. Add the ordering edge, plus a
sources_monitoring_application_dependson_test.yaml regression
guard so a future edit cannot drop it silently.
(2) allow_assign_grafana_admin driven by `.Release.Name ==
monitoring-system` (MINOR fragile coupling).
Replace the release-name sniff with an explicit
`oidc.grafanaAdmin: false` values field, and flip it on for the
platform Grafana release via
Package.spec.components["monitoring-system"].values.oidc.grafanaAdmin
in packages/core/platform/templates/bundles/system.yaml —
mirroring the pattern monitoring-agents already uses for
per-component overrides. Tenant Grafana releases inherit the
chart default false and stay at org-level Admin.
Two new helm-unittest cases in packages/system/monitoring/tests/
lock the values-driven behaviour on both sides
(grafanaAdmin=true → Admin uplift, default → tenant stays at org
Admin), plus bundles_monitoring_grafana_admin_test.yaml in
packages/core/platform/tests/ regression-guards the Package's
component override so the platform release keeps its uplift on
future edits.
Also document the mode-toggle group-membership loss in
docs/oidc-grafana.md — mode: System → CustomConfig/None deletes the
three chart-owned KeycloakRealmGroups; membership is not restored on
toggle-back. Not a bug, but the operator UX cost of the toggle
deserves a bullet in Failure modes.
Regenerated artifacts (cozyvalues-gen + hack/update-crd.sh):
packages/extra/monitoring/values.schema.json
packages/extra/monitoring/README.md
packages/system/monitoring-rd/cozyrds/monitoring.yaml
Test suites still green: packages/system/monitoring 23/23,
packages/core/platform 81/81.
Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com>
|
||
|
Aleksei Sviridkin
|
249f7b5a5e |
feat(platform, ingress): propagate operator wildcard certificate to per-tenant termination points (#2990)
## What this PR does The operator-provided wildcard certificate (`publishing.certificates.wildcardSecretName`) currently serves the root tenant only: everything is same-namespace, so the root ingress controller and Gateway read the operator's TLS Secret directly. Child tenants run their own ingress controller / Gateway in their own namespace under a default-deny policy and cannot read that Secret across namespaces, so they fall back to per-host ACME. This propagates the operator wildcard to per-tenant termination points by replicating the Secret into each tenant namespace that owns one, keeping the root-MVP invariant intact: the wildcard is deliberately distributed to tenant namespaces, but no tenant RBAC is widened — every consumer reads only its own-namespace copy. A new controller in `cozystack-controller` does the replication with no extra operator input. It reads the same platform values channel the consumers read (`cozy-system/cozystack-values`): the wildcard Secret name from `_cluster.wildcard-secret-name` and the publishing namespace from `_cluster.expose-ingress`. It mirrors that Secret into every tenant namespace whose `namespace.cozystack.io/ingress` or `namespace.cozystack.io/gateway` label equals its own name — i.e. the namespace runs its own controller / Gateway. Because the source is derived from the same value that makes the consumers reference it, the replica is created whenever the consumers expect it — no manual labelling and no upgrade hazard where a child controller references a Secret that nothing creates. Replicas are refreshed on rotation and pruned when a namespace stops terminating TLS. Teardown of every replica happens on exactly one trigger: clearing `wildcardSecretName` (explicit disable). A source that is merely absent or mistyped, or a transiently-missing values channel, leaves existing replicas in place, so a brief gap, a delete+recreate rotation, or a misconfigured publishing namespace never drops tenant TLS. The platform controller's own ServiceAccount gains Secret writes; tenant RBAC is untouched. The controller does not cache every Secret in the cluster: the manager's Secret informer is scoped to managed replicas and the values channel only, and the operator source / foreign collisions are read through the uncached APIReader. The source is not watched (its name is dynamic), so an in-place rotation is picked up by a bounded periodic resync while the publishing tenant serves the source with no lag. On the consumer side, each tenant ingress controller now sets `--default-ssl-certificate` to the wildcard Secret in its own namespace (`.Release.Namespace/<name>`), so a child tenant serves the replica instead of minting per-host ACME. The Gateway path already renders `certMode: existingSecret` with a same-namespace `wildcardSecretRef` for every tenant, so it consumes the replica with no chart change. ### Why not clustersecret-operator The issue proposed the shipped `clustersecret-operator`. It materializes inline template data into selected namespaces; it cannot mirror an existing source Secret, and it is not installed in any default bundle. Driving it would mean embedding the certificate and private key as literal data in a cluster-scoped object rendered from a Helm `lookup` — that broadcasts key material the design keeps off any shared channel, and a `lookup` is invisible to the Flux helm-controller digest so it would not re-render on rotation. Its namespace selector also cannot express "the namespace whose owner label equals its own name", so it would over-replicate the key into every tenant namespace. A small dedicated reconciler targets exactly the termination namespaces and keeps the key off the values channel. ### Coverage caveat Replication delivers the certificate bytes, not SAN coverage: a single `*.<root-host>` does not match `*.<child-apex>`. Operators serving child-tenant subdomains supply a certificate whose SANs cover those apexes; the docs spell this out. The replica carries the wildcard private key into each terminating tenant namespace — the same exposure as the per-host ACME Secret it replaces, now shared across tenants. ### Tests - Go fake-client tests for the reconciler: termination-only replication (ingress-owning and gateway-owning namespaces only, never inheriting ones, never over the source), copy markers, rotation, prune on namespace demotion, the non-destructive source-deletion contract, replica self-heal after out-of-band deletion, the publishing-namespace self-replication guard, disable-via-config teardown, source rename, foreign-Secret collision safety (no overwrite, no requeue, does not block siblings), transient-error requeue, non-TLS source ignored, absent-channel keeps replicas, publishing-namespace mismatch keeps and requeues, the cache-scope pin, the enqueue-wiring map, an `ownsTerminationPoint` table, and the active-only requeue contract. - helm-unittest for the ingress `default-ssl-certificate` wiring (including a child-tenant controller pointing at its own-namespace replica) and for the cozystack-controller ClusterRole's new Secret-write rule. ### Screenshots N/A — no UI changes. ### Release note ```release-note feat(platform): the operator-provided wildcard TLS certificate (`publishing.certificates.wildcardSecretName`) is now served by per-tenant ingress controllers and Gateways automatically — the platform controller replicates it into each tenant namespace that terminates TLS, with no extra operator input and no cross-namespace Secret read ``` Closes #2820 Part of #2811 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Wildcard TLS Secret now replicates from the platform to tenant namespaces that terminate TLS via their own ingress controller or Gateway, including child-tenant scenarios. * **Documentation** * Clarified configuration, timing, and SAN matching requirements for wildcard replication. * Updated gateway and ingress-nginx default certificate documentation for replica mirroring behavior. * **Bug Fixes** * Improved handling for missing/invalid source secrets (no unnecessary pruning) and safe collision behavior. * **Tests** * Added comprehensive reconciliation/replication test coverage. * Added Helm unittest target for the controller chart. * **Chores** * Updated controller RBAC to support wildcard TLS Secret replication. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
f24b56327b |
fix(platform): route OIDC backend calls through the in-cluster Keycloak by default
authentication.oidc.keycloakInternalUrl defaulted to an empty string, which left oauth2-proxy performing OIDC discovery against the external Keycloak hostname. That request resolves through public DNS and lands on the root ingress controller, which does not serve yet on a fresh install: the root tenant's IngressClass only exists once tenant-root is ready, and tenant-root is still reconciling while the dashboard starts. Discovery has nothing to talk to, the gatekeeper fails, and the dashboard never comes up. The bundled Keycloak is the only supported provider — the external hostname is templated as keycloak.<host> and the realm is fixed — so backend-to-backend calls have no reason to leave the cluster. Default the value to http://keycloak-http.cozy-keycloak.svc:8080/realms/cozy, the same Service address keycloak-configure already uses. The dashboard and linstor-gui gatekeepers then run with --skip-oidc-discovery and send token, jwks, userinfo and logout requests to the Service, while browser redirects stay on the external host. Those calls travel as plaintext HTTP between pods, as keycloak-configure's admin calls to the same address already do. The value remains overridable for a Keycloak that does not live at the standard in-cluster address, and setting it back to an empty string restores external discovery. Existing OIDC clusters that never set it get a gatekeeper rollout onto the internal path on upgrade. Enabling OIDC swaps the dashboard's token-proxy container for oauth2-proxy, so the E2E install now drives that swap to completion: it waits for the auth-proxy container to appear, then gates on the gatekeeper rollout. Readiness alone proves nothing there — the HelmRelease is still Ready from the token-proxy install, and the Deployment keeps the old ReplicaSet Available while a new pod crashloops, so only the rollout catches a regression back to external discovery. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
myasnikovdaniil
|
99a28d82c2 |
test(e2e): migrate app suite from BATS to Chainsaw and wire into CI (#2826)
## What this PR does Migrates the E2E **app** test suite from BATS to [Kyverno Chainsaw](https://github.com/kyverno/chainsaw) — a declarative, Kubernetes-native E2E framework (CNCF, part of the Kyverno project; successor to KUTTL) — and wires it into CI as the replacement for the per-app BATS loop. This grows the original pilot (postgres + bucket) into the full suite. All 22 app suites are ported under `hack/e2e-chainsaw/` and the `hack/e2e-apps/*.bats` files are removed: `postgres`, `bucket`, `mariadb`, `mongodb`, `redis`, `qdrant`, `clickhouse`, `kafka`, `etcd`, `openbao`, `harbor`, `foundationdb`, `external-dns`, `kuberture`, `vminstance`, `gateway`, `kubernetes-latest`, `kubernetes-previous`, `kubernetes-oidc-system`, `kubernetes-oidc-customconfig`, `securitygroup`, `serviceexposure`. **Approach** - **Declarative** suites assert on `status.conditions` and concrete fields. The `timeout N sh -ec "until kubectl get ..."` + `kubectl wait` pair that appeared ~100 times collapses into a single `assert` that polls existence and state together, with a structured diff on failure and automatic `events`/`describe`/`podLogs` capture via `catch` (previously only `harbor.bats` did this, by hand). - **Imperative** suites keep their logic in `script` steps: `openbao` init/unseal, `kuberture` external-dns split-horizon probes, `vminstance`, the `gateway` admission/impersonation cases, and `kubernetes-latest`/`previous`, which wrap the relocated `hack/e2e-chainsaw/_lib/run-kubernetes.sh` verbatim (Kamaji bring-up, LB/NFS/ouroboros checks). - `gateway` tests derive the tenant apex from the namespace `namespace.cozystack.io/host` label at runtime, so they are host-independent. **CI** - The `e2e` job now runs `chainsaw test hack/e2e-chainsaw/` via a new `test-chainsaw` target and uploads the JUnit `chainsaw-report.xml`. - The `chainsaw` binary is added to the e2e-sandbox image. - `install-cozystack` and `test-openapi` stay BATS (cluster bootstrap + OpenAPI checks). - The per-app 3-retry loop is dropped — assertion polling replaces the fixed-timeout flakiness it papered over. **Validation** Ran against a development cluster: the DB/app, storage, and VM suites pass; the three suites that depend on platform features not present on that cluster (`gateway`, `kuberture`, `external-dns`) and the two heavyweight `kubernetes-*` suites are exercised by this PR's CI run on a freshly installed platform. Note for reviewers: service-port asserts use the `(ports[*].port)` projection form rather than a number-literal filter (`` ports[?port == `N`] ``), because Chainsaw v0.2.15 mis-evaluates JMESPath number-literal comparisons. **2026-07-09: reconciled with main (~520 commits of drift)** - etcd suite re-ported to the v1alpha2 operator contracts (#2859): `readyMembers=3` gate, pod-label/`/scale`/WorkloadMonitor/metrics/defrag contracts, plus the `ETCD_E2E_S3_ROUNDTRIP`-gated backup round-trip driving `examples/backups/etcd` - 12 new `ingress-hostname-policy` gateway cases ported (apex derived at runtime from the tenant-root namespace label) - post-branch bats drift folded in: `mariadb-single` webhook guard, kafka `c1.small` presets, bucket/harbor 2m BucketClaim fail-fast, bucket readonly-denial promoted to a hard fail (cosi-driver v0.3.1), qdrant PVC-reclaim guard (#3059), kuberture fail-loud negations, SC-fallback-default test (#2872 B1) - 4 suites that only exist on main since the branch was cut are ported: `kubernetes-oidc-system`, `kubernetes-oidc-customconfig`, `securitygroup`, `serviceexposure` - CI seams: `nightly.yaml` converted to the chainsaw invocation, `run-kubernetes.sh` reconciled (Talos/CABPT waits, tenant drain, LINSTOR pool wait, talos-image-cache under `_lib/`), `e2e-capture-dataplane.sh` replicated in the Chainsaw global catch ### Release note ```release-note fix(kubernetes): tenant Kubernetes teardown no longer hangs when the cluster has no working nodes — the pre-delete hook now bounds its wait for the in-tenant HelmReleases and force-clears their Flux finalizers on timeout (#3271) ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * End-to-end testing is now driven by a unified Chainsaw suite (platform apps, Kubernetes, networking, security groups, service exposure). * Test Impact Analysis selects affected suites, with an option to run the full suite. * CI produces a JUnit report artifact and collects suite-scoped diagnostics on failure. * **Bug Fixes** * CI E2E now fails fast and emits clearer Kubernetes/HelmRelease and sandbox bucket state diagnostics. * Improved teardown reliability with bounded wait + finalizer handling to avoid hangs. * **Documentation** * Updated E2E testing docs and local guidance to reflect the new Chainsaw-based workflow and suite paths. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov DaniilandClaude Opus 4.8
|
302a72f14a |
Merge origin/main into feat/e2e-chainsaw-example
Reconciles the talos-image-cache conflicts created when #3254 merged to main: keep the Chainsaw _lib/ port (hack/e2e-chainsaw/_lib/talos-image-cache.sh, a superset of #3254's logic + the node-join diagnose helper), drop the BATS-era hack/e2e-apps/talos-image-cache.sh, and keep the _lib/-path versions of the mirror manifest, install bats, and unit test. Also picks up main's zizmor action-pinning (#3223, unblocks pre-commit/zizmor) and Talos v1.13.6 (#3240). Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
renovate[bot]
|
5b9b420ffe | chore(deps): update container image & action digests | ||
|
Aleksei Sviridkin
|
3e9b5600e3 |
fix(talos): bump Talos to v1.13.6 (CVE-2026-53359)
Talos v1.13.5 ships Linux 6.18.36, which carries the fix for CVE-2026-46113 but not the one for CVE-2026-53359. The two are related KVM x86 shadow-paging bugs allowing a guest to escape to the host, and both patches are needed for the mitigation to hold. Talos v1.13.6 ships Linux 6.18.38, the first release on the 6.18 stable line that has both. Profiles regenerated with hack/gen-profiles.sh v1.13.6. Alongside the Talos version and the installer reference, this refreshes the pinned system-extension digests: the firmware extensions move from the 20260519 to the 20260622 build, and intel-ucode stays on its 20260512 tag but resolves to a new digest because upstream re-pushed that tag. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov DaniilandClaude Fable 5
|
1f84d2d49e |
Merge origin/main into feat/e2e-chainsaw-example (post-#3244)
Fold the in-sandbox Talos image cache work into the Chainsaw layout: the new shared helper relocates to hack/e2e-chainsaw/_lib/ talos-image-cache.sh and run-kubernetes.sh sources it from there (the ouroboros DNS-check no-retry rework merges clean). The OIDC bats are resolved as deleted again; their Chainsaw fixtures deliberately skip the talos.imageFactoryURL injection — the helper only emits it when the cache is up, and the zero-worker render-side tests instantiate no DataVolume imports for it to redirect. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
scooby87
|
f91841fce6 |
fix(monitoring): surface missing VictoriaLogs and gate dead-end vlinsert ExternalName (#3215)
## What this PR does Addresses #3181, where platform log ingestion could fail **silently**: fluent-bit shipped to `vlinsert-generic` while the monitoring `HelmRelease` reported `Ready`, and a `vlinsert-generic` `ExternalName` in `cozy-monitoring` dead-ended at a `tenant-root` VictoriaLogs that did not exist when the root tenant had monitoring disabled. Logs were dropped with no failing signal. Three complementary layers make the failure loud instead of silent: 1. **Readiness gate** — `packages/extra/monitoring/templates/helmrelease.yaml` now sets a `poller` `waitStrategy` and a `healthCheckExprs` entry on the `VLCluster` `status.updateStatus`. A missing/unhealthy log store makes the `monitoring-system` release **not-ready** instead of flipping `Ready` the moment the CR is applied. Mirrors the existing seaweedfs CNPG pattern. 2. **No empty-log-store hole** — `packages/system/monitoring/templates/vlogs/vlogs.yaml` fails the render on an empty `logsStorages`. With no entry no `VLCluster` exists, the readiness gate has nothing to match, and fluent-bit would silently drop logs — so this is now a loud misconfiguration. 3. **No silent fallback** — the `cozy-monitoring → tenant-root` `vlinsert`/`vminsert` `ExternalName` redirects in `cozystack-basics` are created only when the root tenant hosts monitoring. The template reads the live `Tenant/root` `spec.monitoring` via `lookup` (source of truth, so it cannot drift from the tenant's real state) and falls back to a new platform value `monitoring.rootEnabled` (via `_cluster.monitoring-enabled`) when `lookup` is empty (dry-run / first render). The default preserves the historical always-create behaviour. Covered by helm-unittest across all four affected charts (141 tests): readiness-gate fields, empty-vs-populated `logsStorages` contract, `ExternalName` present/absent fallback contract, and the platform wiring into the `cozystack-values` Secret. Also adds the missing `test:` target to `packages/extra/monitoring/Makefile` so its suite runs in CI. **Known limitation / follow-up:** the legacy platform monitoring release installed directly into `cozy-monitoring` via the `cozystack.monitoring` PackageSource cannot carry the readiness gate, because the engine's `ComponentInstall`/HelmRelease builder and the vendored `helm-controller` API v1.4.3 lack `waitStrategy`/`healthCheckExprs`. That source is not referenced by any active bundle; full coverage there needs a helm-controller bump plus engine support. ### Release note ```release-note fix(monitoring): surface missing platform VictoriaLogs as not-ready and stop the dangling vlinsert-generic ExternalName from silently dropping logs (#3181) ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a platform setting (`monitoring.rootEnabled`) to control whether root-level monitoring is enabled. * Monitoring redirect ExternalName services are now rendered only when root monitoring is enabled (defaulting to enabled). * **Bug Fixes** * Ensures an explicitly set `monitoring.rootEnabled=false` is preserved when generating platform configuration. * Prevents incomplete monitoring deployments by failing rendering when `logsStorages` is empty. * Improves VictoriaMetrics readiness by delaying “ready” until the operator reports an operational status. * **Tests** * Added/expanded Helm-unittest suites covering monitoring wiring, redirects, readiness gating, and `logsStorages` requirements. * Added a `make test` target to run chart unittests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Алексей Артамонов
|
f700c9043c |
fix(monitoring): backfill release label onto pre-existing VM/VL storage PVCs
Re-review on #3094 (IvanHunters, MAJOR): the volumeClaimTemplate label lands on PVCs only at creation — the StatefulSet controller never re-labels existing PVCs and vm-operator's PVC reconcile is resize-only. So on an already-deployed cluster (exactly the #3091 population) the pre-upgrade VM/VL storage PVCs stay unlabeled and the cleanup hook's selector misses them. Add migration 49 (mirrors the ClickHouse-keeper backfill in migration 48 for the sibling #3072). It relabels the pre-existing vmstorage/vmselect/vlstorage PVCs of every cozystack-managed VMCluster/VLCluster with apps.cozystack.io/application.name read from the CR's own spec.managedMetadata, so only monitoring's clusters are touched (a user's own VMCluster carries no such label) and the value always matches what the cleanup hook selects. Best-effort like #48: a skipped relabel is no worse than the pre-existing leak, so transient apiserver errors never abort the platform upgrade. Bump platform targetVersion 49 -> 50. Also correct the misleading comments (IvanHunters MINOR): spec.managedMetadata is never applied to PVCs by vm-operator at any current version, so the claim-template label is the only mechanism — do not imply a v0.71.0 crossover. Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io> Assisted-By: Claude <noreply@anthropic.com> |
||
|
Alexey Artamonov
|
f9afc73096 |
fix(monitoring): surface missing VictoriaLogs and gate dead-end vlinsert ExternalName (#3181)
Platform log ingestion could fail silently: fluent-bit shipped to vlinsert-generic while the monitoring HelmRelease reported Ready, and a vlinsert-generic ExternalName in cozy-monitoring dead-ended at a tenant-root VictoriaLogs that did not exist when the root tenant had monitoring disabled. Logs were dropped with no failing signal. Three complementary changes: - Readiness (do not stay silently Ready): the monitoring-system HelmRelease now uses a `poller` waitStrategy and a healthCheckExpr on its VLCluster (status.updateStatus), so a missing/unhealthy log store makes the release not-ready instead of flipping Ready as soon as the CR is applied. Mirrors the seaweedfs CNPG pattern. - No empty-log-store hole: system/monitoring rejects an empty logsStorages at render time (helm fail). With no entry no VLCluster is created, the readiness gate has nothing to match, and fluent-bit would silently drop logs — so this is made a loud misconfiguration rather than a documented footgun. - No silent fallback: the cozy-monitoring -> tenant-root vlinsert/vminsert ExternalName redirects in cozystack-basics are now created only when the root tenant hosts monitoring, gated on _cluster.monitoring-enabled — fed from a new platform value monitoring.rootEnabled (default true, preserving the historical always-create behaviour). monitoring.rootEnabled is the single render-time source of truth: changing it re-renders cozystack-basics (helm-controller watches the cozystack-values Secret it flows into). A live `lookup` of Tenant/root spec.monitoring is deliberately NOT used: cozystack-basics is an operator-generated HelmRelease with drift detection off, so it re-renders only on install/upgrade/values-change — never on a reconcile tick or when a lookup result changes — and a lookup would go stale in both directions. Operators must keep monitoring.rootEnabled in sync with Tenant/root spec.monitoring. Covered by helm-unittest across all four charts: readiness gate fields, empty-vs-populated logsStorages contract, ExternalName present/absent and null-guard contract, and the platform wiring into the cozystack-values Secret. The CEL health predicates are exercised live, not in helm-unittest (which cannot evaluate CEL). Adds the missing `test:` target to packages/extra/monitoring/Makefile so its suite runs in CI. Known limitation (follow-up): the legacy cozystack.monitoring PackageSource that installs system/monitoring directly into cozy-monitoring cannot carry the readiness gate, because the engine's ComponentInstall/HelmRelease builder and the vendored helm-controller API v1.4.3 lack waitStrategy/healthCheckExprs. That source is not referenced by any active bundle; full coverage needs a helm-controller bump plus engine support. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Myasnikov DaniilandClaude Fable 5
|
9297e51170 |
Merge origin/main into feat/e2e-chainsaw-example
Reconcile the Chainsaw e2e migration with ~500 commits of main drift: - keep main's run-kubernetes.sh helpers (kubectl_wait_retry, tenant drain, LINSTOR pool-free wait, SC-fallback probe) under the branch's _lib/ rename with the Chainsaw finally/snapshot deltas re-applied - resolve modify/delete on 10 drifted hack/e2e-apps/*.bats as deleted; their post-branch assertion drift is re-ported into the Chainsaw suites in follow-up commits - drop the 4 new main-side bats (kubernetes-oidc-*, securitygroup, serviceexposure) from _lib/ relocation; ported as Chainsaw suites in follow-up commits - docs/agents/e2e-testing.md: merge main's plan-job TIA wording with the Chainsaw suite phrasing Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
myasnikovdaniil
|
cece5c23b5 |
feat(release): immutable tags & rc→stable promotion (#2677) (#3017)
## What this PR does Implements the immutable-tag + rc-promotion flow from #2677. A **stable release becomes a renamed release-candidate**: the bytes shipped as `vX.Y.Z` are bit-for-bit the bytes built and e2e-tested as `vX.Y.Z-rc.N`. No tag is ever force-moved, and stable is never rebuilt — it is *promoted* by retagging the rc's existing images. Layered onto the build matrix from #2937/#2983 (this PR is **stacked on `refactor/build-matrix-2937`** and must merge after it). ## The five force-retag sites, removed | Site | Before | After | |---|---|---| | `tags.yaml` api/apps/v1alpha1 tag | `git tag -f` / `push -f` | write-once (create-if-absent, fail if it would move) | | `tags.yaml` release-X.Y.Z branch | `git branch -f` / `push -f` | compare-before-force (no-op if unchanged; staging branch only) | | `pull-requests-release.yaml` stable tag | `git tag -f` / `push -f` | write-once at the PR merge commit (force impossible by construction) | | `pull-requests-release.yaml` maintenance branch | `updateRef force:true` | fast-forward-only | | `auto-release.yaml` patch tags | delete-recreate (cron) | **workflow deleted** — stable only via explicit promote | ## Version decoupling (the enabler) The operator baked its version into the image at build time, so an rc image self-reported the rc string — blocking retag-promotion. It now reads `COZYSTACK_VERSION` from the environment (threaded via `cozystackOperator.platformVersion` → Deployment env, stamped by `make manifests`), falling back to the build-time value when unset. The same image bits can report any release name. The only runtime reader is the telemetry metric `cozy_cluster_info{cozystack_version=...}`. ## Promotion flow `promote-rc.yaml` (`workflow_dispatch`, `rc_tag=vX.Y.Z-rc.N`): 1. Validate the rc release exists and the stable tag does not. 2. `hack/promote-retag.sh` reads the rc's digest-pinned image refs from `packages/*/*/values.yaml` and `skopeo copy`s each — **by digest** — to `:vX.Y.Z` and `:latest`, verifying with `skopeo inspect`. 3. Rewrite the cosmetic `-rc.N` substring in vendored tags to the stable version (the `@sha256` wins regardless), restamp the version-stamped assets (operator manifests, cozypkg, openapi; the heavy Talos assets are copied verbatim from the rc draft), open a `release-X.Y.Z` PR. 4. Merging the PR reuses the existing release-PR e2e and `pull-requests-release.yaml` finalize to cut the **write-once** stable tag at the merge commit and publish the release. Squash is disallowed (the tag needs a real merge commit). `nightly.yaml` cuts write-once `*-nightly.<date>` tags (gated by `NIGHTLY_ENABLED`); `retention.yaml` keeps the newest 14 per line (dry-run by default). ## Validation **Local end-to-end — the core claim is proven, not asserted.** The whole design rests on "copy-by-digest to a new tag preserves the digest, so stable == the e2e-tested rc, bit-for-bit." This was exercised against a real registry (`ttl.sh`) with real `skopeo`: - Pushed a multi-arch image under a `:v1.4.0-rc.2` tag, built a temp tree exercising all three `values.yaml` digest shapes (single `image:` string, split `repository`/`tag`/`digest` map, and the `platformSourceRef` OCI artifact), then ran the actual `hack/promote-retag.sh v1.4.0`. - **Result: `:v1.4.0-rc.2`, `:v1.4.0`, and `:latest` all resolve to the identical digest** (`sha256:fd8d9aa6…`), across all 17 platform manifests. The script's own `skopeo inspect` post-check passed, and an independent re-inspection confirmed it. Local validation caught (and this PR fixes) two bugs that only surface against a real registry — never in static lint: 1. `skopeo copy --multi-arch all --all` — mutually exclusive flags, fatal error; every retag would have failed on CI. Now `--multi-arch all`. 2. The same digest was retagged twice when an image appeared in two value shapes; the ref set is now deduped on the canonical `repo@digest`. **Other local checks:** `go test ./pkg/version` + `go build`/`vet`; `helm template` renders `COZYSTACK_VERSION` in all 3 operator variants (omitted when unset); `make manifests` stamps the version into the install assets; `shellcheck` clean on `promote-retag.sh`; `actionlint` clean and `act -l` resolves the job graph on all workflows; the rc-tag parser and nightly version-math unit-tested (accept/reject + minor-vs-patch bumps). **Still CI-only (cannot be exercised offline):** the full workflow runtime — github-script API calls, OCIR auth, the rc-release / staging-branch preconditions, the nightly `base_ref` push mechanics, and self-hosted runners. The *logic* inside the steps is unit-tested; the *orchestration* is not. Treat it as unproven until the workflows run. ## Out of scope / required follow-up (needs repo admin — not doable from a PR) - Tag-protection rules: `v*` and `api/apps/v1alpha1/*` create-only by the CI app, no delete/update; "limit branches/tags updated in a single push" = 1. - Enable `NIGHTLY_ENABLED` (and `RETENTION_APPLY` when ready) repo variables. - Registry-side pruning of `*-nightly.*` image tags (no OCIR delete parity yet — tracked TODO in `retention.yaml`). ## Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added nightly publishing (mirror-by-digest, disk build, e2e validation) and nightly retention pruning. * Added RC-to-stable promotion via digest retagging (no rebuild). * Operator/installer and console now expose version/platform metadata, including runtime override via `COZYSTACK_VERSION`. * **Bug Fixes** * Enforced write-once tag behavior and fast-forward-only maintenance updates across release workflows. * Made nightly mirroring/selection and retention pruning more selective and safer. * **Tests** * Added Bats coverage for nightly mirroring and RC retagging, plus new Helm/unit checks. * **Documentation** * Updated the release model to center RC promotion and tag immutability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
43104c9075 |
fix(ingress): guard the immutable loadBalancerClass migration on existing Services
spec.loadBalancerClass is immutable once a Service is type LoadBalancer, so setting, changing, or clearing publishing.loadBalancerClass on a cluster whose host ingress Service already exists would make Flux wedge on an API-rejected patch instead of migrating. Fail the render early with the exact recreate command whenever an existing type: LoadBalancer Service's class differs from the requested value; a fresh install (lookup nil) or a ClusterIP Service transitioning to LoadBalancer is left untouched. Note the caveat in the values comment and cover the guard with helm-unittest. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |