## Why
Follow-up to #2509. PR #2509 exposed five HelmRelease generation knobs
as `cozystack-operator` flags
(`--helmrelease-{interval,retry-interval,install-timeout,upgrade-timeout,max-history}`).
The other HelmRelease-generating path — `cozystack-api`'s
`convertApplicationToHelmRelease` in
`pkg/registry/apps/application/rest.go` — was deliberately left out of
#2509's scope and was flagged in review:
> `pkg/registry/apps/application/rest.go` (the other
HelmRelease-generating path) still hardcodes `Interval: 5m`,
`Remediation{Retries:-1}`, no `Strategy`. Out of this PR's scope; worth
a Followups bullet so the rationale doesn't drift across paths.
So today the api-side path hardcodes:
- `Spec.Interval = 5m`
- `Install.Remediation{Retries: -1}` + `Upgrade.Remediation{Retries:
-1}`
- no `Strategy`
- no `MaxHistory`
Only `HelmInstallTimeout` (per-Application annotation override) flows
through `ReleaseConfig`. This PR brings the api-side to feature parity
with the operator: same flag names, same defaults, same validation, same
generated HelmRelease shape.
## What
### New cozystack-api flags
Names, types, defaults, and validation match the operator's flags
introduced in #2509 verbatim, so both binaries can be tuned with the
same vocabulary.
| Flag | Default | HR field |
|---|---|---|
| `--helmrelease-interval` | `5m` | `Spec.Interval` |
| `--helmrelease-retry-interval` | `30s` |
`Spec.{Install,Upgrade}.Strategy.RetryInterval` |
| `--helmrelease-install-timeout` | `10m` | `Spec.Install.Timeout` |
| `--helmrelease-upgrade-timeout` | `10m` | `Spec.Upgrade.Timeout` |
| `--helmrelease-max-history` | `5` | `Spec.MaxHistory` |
Validated in `CozyServerOptions.Complete()` via
`config.ParsePositiveDuration` so a misconfigured server restarts loudly
instead of waiting until the first Application is created. `MaxHistory <
0` is rejected; `0` is honored (unlimited per Helm semantics, matching
the operator).
### Strategy switch
`Install.Strategy.Name` and `Upgrade.Strategy.Name` are now set to
`RetryOnFailure` (with `RetryInterval` from the new flag) instead of
`Install/Upgrade.Remediation{Retries: -1}`. Functionally equivalent for
this path (which never relied on remediation firing), but **decouples
failed-install retry timing from `Spec.Interval`** the same way the
operator change did.
`Install.Remediation` / `Upgrade.Remediation` stay nil: retries run
through the single `Strategy.RetryInterval` path, matching
cozystack-operator's `PackageReconciler`. Reintroducing
`Remediation{Retries:-1}` "for safety" would add a second, conflicting
retry mechanism and break parity with the operator path. A regression
test guards this. (The deployed helm-controller CRD has no rule
forbidding a `Strategy` alongside a `Remediation` entry — this is a
parity/clarity guard, not a CRD-validation one.)
### Per-Application timeout override is preserved (and extended)
The `release.cozystack.io/helm-install-timeout` annotation on
`ApplicationDefinition` (`HelmInstallTimeout` in `ReleaseConfig`) still
wins over the new global `HelmReleaseInstallTimeout` /
`HelmReleaseUpgradeTimeout` defaults when set, applying to **both**
`Install.Timeout` and `Upgrade.Timeout`. kubernetes-rd and tenant-rd
carry it today (Kamaji bootstrap; seaweedfs-db CNPG bootstrap).
A new `release.cozystack.io/helm-upgrade-timeout` annotation overrides
**only** `Upgrade.Timeout`, winning over the value the install
annotation would otherwise apply — so a kind can carry an asymmetric
install/upgrade budget. No `ApplicationDefinition` sets it today, so
behavior is unchanged for every existing kind.
`Install.Timeout` and `Upgrade.Timeout` are now always populated (from a
per-app override or the global default), matching the operator's
`buildHelmReleaseSpec`. The previous "stay nil when zero" branch was
removed — it was a test shim that kept empty-`ReleaseConfig{}` fixtures
green at the cost of api/operator drift.
### Shared validator
`parsePositiveDuration` is lifted from `cmd/cozystack-operator/main.go`
into `pkg/config` (exported as `ParsePositiveDuration`). Both
`cozystack-operator` and `cozystack-api` mains now share one validator
with identical reject rules (zero / negative / malformed / empty). The
table-driven test moves with it. The annotation parser is shared the
same way: `ParseHelmTimeoutAnnotation` validates both the install- and
upgrade-timeout annotation values against Flux's unit rules.
## Production safety
- Static flag defaults match #2509 exactly. **With stock defaults, every
api-generated HR changes shape**: it gains
`Strategy.Name=RetryOnFailure` + `RetryInterval`, an explicit
`MaxHistory`, and an explicit `Install/Upgrade.Timeout`. This is the
intended parity with the operator path, not a no-op.
- **Timeout-default change**: because `Install/Upgrade.Timeout` is now
always set, the effective install/upgrade timeout for api-generated HRs
grows from the implicit flux default (5m, field was previously nil) to
the explicit `--helmrelease-{install,upgrade}-timeout` default of `10m`.
This is desired — it matches the operator path.
- Same intentional retry-cadence change as #2509: failed-release retry
cadence on api-generated HRs drops from ~5m (previously coupled to
`Spec.Interval`) to 30s (`RetryInterval` default). Healthy-release
reconcile cadence is unchanged at 5m.
- Per-Application `HelmInstallTimeout` override semantics intact —
kubernetes-rd's long Kamaji bootstrap path continues to work;
tenant-rd's seaweedfs-db bootstrap budget is unchanged.
## Verification
- `go build ./...` clean
- `go vet ./...` clean
- `go test ./pkg/config/... ./pkg/cmd/server/...
./pkg/registry/apps/application/... ./cmd/cozystack-operator/...
./internal/operator/...` — all pass
- New / updated unit tests mirror the operator's spec tests:
- `TestConvertApplicationToHelmRelease_BuildsSpecFromConfig` — full HR
shape from config (mirrors `TestBuildHelmReleaseSpec`)
- `TestConvertApplicationToHelmRelease_ZeroMaxHistory` — `MaxHistory=0`
survives as pointer-to-0 (mirrors
`TestBuildHelmReleaseSpecZeroMaxHistory`)
- `TestConvertApplicationToHelmRelease_PerAppTimeoutOverridesGlobal` —
install-annotation override wins over globals (symmetric + asymmetric)
- `TestConvertApplicationToHelmRelease_UpgradeTimeoutAnnotation` —
upgrade-only override, asymmetric precedence over the install
annotation, and the install-sets-both regression guard
- `TestConvertApplicationToHelmRelease_AppliesReleaseConfigTimeout` —
`Remediation.Retries == -1` assertions replaced with `Strategy.Name ==
RetryOnFailure` + `Remediation == nil`
- `TestParsePositiveDuration` / `TestParseHelmTimeoutAnnotation` — flag
and annotation validation, including which error branch fires
- Pre-existing test failures in `pkg/apiserver` (randfill panic) and
`pkg/lineage` reproduce on `main` without these changes — not introduced
here.
## Test plan
- [ ] CI E2E install passes with default values (no flags set on
cozystack-api Deployment)
- [ ] HR objects generated from Application resources in a healthy
cluster render with `Strategy.Name=RetryOnFailure` and
`RetryInterval=30s`
- [ ] kubernetes-rd Application's HR still gets
`Install/Upgrade.Timeout=15m` (or whatever its annotation says) —
overrides the global default
## Followups (deliberately not bundled)
- **Helm chart exposure** — equivalent of #2509's
`packages/core/installer/values.yaml` knobs, but for the api server.
Touches a different chart (`packages/core/cozystack-api` or wherever the
api Deployment lives). Separate PR keeps the diff focused on the Go
side.
- **Per-Application override of the remaining globals** — interval /
retry-interval / max-history have no annotation hook yet; adding them
would require new annotations and is out of scope until a real need
surfaces.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added server-wide HelmRelease timing flags and defaults (interval,
retry interval, install/upgrade timeouts, max history), with
per-application timeout overrides.
* HelmRelease generation now consistently applies a retry-on-failure
strategy with explicit install/upgrade timeouts.
* **Bug Fixes**
* Stricter validation for duration-related flags to reject malformed,
zero, or negative values.
* **Tests**
* Expanded unit tests for flag parsing/validation, HelmRelease spec
generation (including max-history=0 handling), timeout override
precedence, and invalid Git ref handling.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
release.cozystack.io/helm-install-timeout sets both Install.Timeout and
Upgrade.Timeout to one value. Add release.cozystack.io/helm-upgrade-timeout
so an ApplicationDefinition can override only Upgrade.Timeout — letting a
kind keep a short install budget but a longer upgrade budget (or vice
versa). When set it wins over the value the install annotation would
otherwise apply to the upgrade side.
- config: add HelmUpgradeTimeoutAnnotation + ReleaseConfig.HelmUpgradeTimeout;
generalize ParseHelmInstallTimeoutAnnotation to ParseHelmTimeoutAnnotation
since it now parses both annotations (identical Flux unit validation).
- server: parse the new annotation at startup with the same loud-fail
behavior; update --helmrelease-upgrade-timeout help.
- rest: apply HelmUpgradeTimeout after the install-annotation copy so the
upgrade-specific value wins; restore the accurate carrier comment.
- tests: cover the upgrade-only override, the asymmetric precedence, and
the install-sets-both regression guard.
Behavior is unchanged for every existing ApplicationDefinition (none set
the new annotation today).
Address review feedback from coderabbitai on rest.go.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The ParsePositiveDuration table exercised all error inputs but did not
distinguish which branch fired. Add an errMatch field so the table
pins "must be > 0" for zero/negative and "invalid duration" for
malformed/empty inputs, mirroring TestParseHelmInstallTimeoutAnnotation
and keeping the two parsers' diagnostics from drifting.
Address review feedback from coderabbitai on config_test.go.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
HelmReleaseMaxHistory carried omitempty, so a value of 0 (which Helm
treats as "unlimited") would be dropped on marshal and become
indistinguishable from an unset field. The runtime path (start.go
always populates it from --helmrelease-max-history) is safe today, but
drop omitempty so the explicit "unlimited" intent survives if
ReleaseConfig is ever serialized.
Address review feedback from coderabbitai on config.go.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
gofmt -w on the two touched test files: the case-table struct fields in
rest_helmrelease_spec_test.go and main_test.go were misaligned, and
main_test.go carried a trailing blank line. CI runs no gofmt gate
(pre-commit only runs make generate), so this would otherwise land
unformatted.
Address review feedback from lexfrei.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The convertApplicationToHelmRelease comment listed only kubernetes-rd
as carrying release.cozystack.io/helm-install-timeout, dropping
tenant-rd. tenant-rd/cozyrds/tenant.yaml sets the 15m annotation for
the seaweedfs-db CNPG bootstrap whose first reconcile exceeds the flux
default. Restore the accurate two-carrier list and rationale.
Address review feedback from lexfrei on rest.go.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The XValidation justification was factually wrong: the deployed
helm-controller CRD has no rule rejecting a Strategy alongside a
Remediation entry (only .retryInterval is gated on Strategy.Name), so
the claimed "silently break every HR" safety net does not exist.
Reword the comment in all three test files to the real reason
Remediation stays nil: retries are driven by a single
Strategy.RetryInterval path, matching cozystack-operator's
PackageReconciler.buildHelmReleaseSpec.
Address review feedback from lexfrei on rest_helmrelease_spec_test.go,
rest_timeout_test.go and internal/operator/package_reconciler_test.go.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Production change:
- convertApplicationToHelmRelease now sets Install/Upgrade.Timeout
unconditionally (matching cozystack-operator's buildHelmReleaseSpec).
The previous "stay nil if zero" branch was a leaky test shim — it kept
fixtures with empty ReleaseConfig{} green at the cost of api/operator
drift.
Test changes:
- newRESTForTimeout seeds the new HelmRelease* globals with
production-shaped defaults so the unset-annotation case exercises
"global default applies" rather than "field stays nil".
- TestConvertApplicationToHelmRelease_PerAppTimeoutOverridesGlobal is
table-driven and now covers asymmetric globals (install != upgrade)
with and without per-app override — guards against a regression that
only assigns one side under override.
- New start_test.go: table-driven validator coverage for each
malformed/non-positive duration flag, the negative MaxHistory branch,
the MaxHistory=0 unlimited branch, asymmetric timeout acceptance, and
a smoke test that NewCozyServerOptions defaults validate.
Refactor:
- Extracted parseAndValidateHelmReleaseFlags from Complete() so the
flag-validation portion is unit-testable without kubernetes I/O.
Documentation:
- Reworded --helmrelease-install/upgrade-timeout help so admins reading
--help see the dual-purpose annotation explicitly (one annotation
overrides both timeouts together).
- Dropped PR-#2509 references and "preserving the pre-#2509 behaviour
for tests" comments — kept the technical rationale (Strategy.Name
vs Remediation.Retries:-1 + 5m Interval coupling) on its own merits.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
convertApplicationToHelmRelease now builds the HelmRelease spec from
ReleaseConfig instead of hardcoding values, bringing it in lockstep
with cozystack-operator's PackageReconciler.buildHelmReleaseSpec
(PR #2509). The drift between the two HelmRelease-generating paths
was the rationale for this follow-up.
Changes to the generated spec:
- Spec.Interval reads ReleaseConfig.HelmReleaseInterval (was
hardcoded 5m).
- Spec.MaxHistory set as &maxHistory pointer from
ReleaseConfig.HelmReleaseMaxHistory; 0 survives as pointer-to-0
(unlimited per Helm semantics) - mirrors the operator's
TestBuildHelmReleaseSpecZeroMaxHistory.
- Install.Strategy{Name: RetryOnFailure, RetryInterval} replaces
Install.Remediation{Retries: -1}. Same for Upgrade.Strategy.
Functionally equivalent to "retry forever, never remediate" for
the api-side path (which also never relied on remediation
firing) but decouples failed-install retry timing from
Spec.Interval.
- Install.Remediation / Upgrade.Remediation stay nil:
helm-controller's XValidation rejects RetryOnFailure +
RetryInterval alongside a Remediation entry, so reintroducing
Retries:-1 "for safety" would silently break every HR.
Install/Upgrade.Timeout precedence:
- per-Application HelmInstallTimeout (annotation-driven) wins
when non-zero (preserves the kubernetes-rd opt-in path);
- otherwise HelmReleaseInstallTimeout / HelmReleaseUpgradeTimeout
globals apply;
- when both are zero, the field stays nil and flux defaults
apply (preserves the legacy code path for tests that
hand-construct ReleaseConfig without populating the new globals).
Tests:
- rest_timeout_test.go: Remediation.Retries == -1 assertions
replaced with Strategy.Name == RetryOnFailure +
Remediation == nil, matching the operator's spec test.
- rest_helmrelease_spec_test.go: new file mirroring
TestBuildHelmReleaseSpec / TestBuildHelmReleaseSpecZeroMaxHistory
from internal/operator/package_reconciler_test.go, plus a third
case asserting per-Application HelmInstallTimeout overrides the
global Install/Upgrade timeout defaults.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Wire five flags into cozystack-api that mirror the cozystack-operator
flags introduced in PR #2509:
--helmrelease-interval (default 5m)
--helmrelease-retry-interval (default 30s)
--helmrelease-install-timeout (default 10m)
--helmrelease-upgrade-timeout (default 10m)
--helmrelease-max-history (default 5)
CozyServerOptions.Complete() validates each duration via
config.ParsePositiveDuration so a misconfigured operator restarts
loudly instead of waiting until the first Application is created,
and rejects MaxHistory < 0 (0 is valid: unlimited per Helm
semantics, matching the operator).
Validated values are written into every ReleaseConfig built from
the ApplicationDefinitionList. The per-Application
HelmInstallTimeout (annotation-driven) still wins over the new
global Install/Upgrade timeout defaults; this commit only plumbs
the values - the consumer in convertApplicationToHelmRelease lands
in the next commit.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Lift parsePositiveDuration from cmd/cozystack-operator/main.go into
pkg/config so cozystack-api can reuse the same startup validator
without duplication. Operator main.go now imports and delegates;
the table-driven test moves to pkg/config/config_test.go.
Extend ReleaseConfig with the five HelmRelease generation knobs
the cozystack-operator already exposes via flags (PR #2509):
HelmReleaseInterval, HelmReleaseRetryInterval,
HelmReleaseInstallTimeout, HelmReleaseUpgradeTimeout,
HelmReleaseMaxHistory. The existing per-Application
HelmInstallTimeout (set from the
release.cozystack.io/helm-install-timeout annotation) keeps its
override semantics - it still wins over the new global
Install/Upgrade timeout defaults when set.
No behaviour change in this commit; the new fields are wired up in
follow-up commits.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
## What
Consolidated landing branch for a set of interdependent platform/CI
stabilization fixes. It began as the source PRs in the table below (in
dependency order) and has since grown with review-driven fixes and a few
production-behavior changes — the full current inventory is under
**Scope** below. Landing them as one unit lets CI run against the real
combined state instead of a fragile bottom-up merge train.
## Source PRs (dependency order)
| # | Commit | Source PR | Addresses |
|---|--------|-----------|-----------|
| 1 | fix(capi): startupProbe on capi-controller-manager | #2946 | capi
crashloop during cert provisioning |
| 2 | test(e2e): bucket.bats port-forward + S3 client reliability |
#2944 | flaky bucket test |
| 3 | fix(seaweedfs): restore -lock BucketClass, s3 svc name, drop
volumeSizeLimitMB | #2943 | missing bucket/access classes |
| 4 | test(e2e): in-cluster Cilium endpoint-leak healer (install + apps)
| #2874 | cilium "IP already in use" leak |
| 5 | refactor(build): mode=max registry cache + main-only warmer |
#2938 | build cache |
| 6 | ci(build): isolate each PR build on its own ephemeral runner VM |
#2939 | shared-buildkit contention |
| 7 | fix(e2e): LINSTOR post-install waits on a single 15m deadline |
#2928 | LINSTOR provisioning timeouts |
| 8 | test(metallb): assert digest-pinned image form, not version
literal | #2873 | brittle metallb assertion |
## Why consolidated
These form a dependency DAG (verified from CI logs): e.g. 2943 needs
2946, 2938 needs 2874, 2928 needs 2938/2939, 2873 needs 2928. Tested
bottom-up, each lower PR runs with none of the fixes above it and can't
go green alone. This branch carries all of them, so CI runs against the
real combined state.
## Verification status
CI is green on the latest head (`5653c30`): full-suite E2E passes
end-to-end and is reproducible (2 of 3 runs on this SHA green). The one
red run was a LINSTOR tie-breaker / DRBD-metadata infra flake on the
sandbox, unrelated to the diff — a different environmental subsystem
failed each run (details in the comments below). The earlier 3-hour
crust-gather snapshot hang is fixed in `5653c30`.
## Scope beyond the original 8
The branch has grown past the 8 source PRs above with review-driven
fixes and a few production-behavior changes. For reviewer transparency,
the full set:
**Review fixes (@lexfrei review):**
- **B1 (blocker)** — `8f41910` converts the SeaweedFS
`-lock`/`-readonly` BucketClasses and the s3 service-name override into
`patches/`, wired into `make update`, so a re-vendor no longer drops
them.
- **FU1** — `cc99230` grants the cilium leak-healer `delete` on pods
(both delete remedies were RBAC-forbidden).
- **FU3** — `014de3f` adds a unit test for the HelmRelease update
conflict-retry path.
- **FU4** — `6deddc6` makes the leak-healer refuse a disconnect for any
non-terminal owner phase, not just `Running`.
- **FU7** — `f3914d2` sets `persist-credentials: false` on the
build-cache checkout (SHA-pinning is handled repo-wide in #2849).
**Production-behavior changes (not in the table above):**
- VPA `updateMode: Auto → Initial` for etcd (`6e9ff90`) and monitoring
(`0997105`), plus `vmselect`/`vmstorage` `minAllowed` floors — stops
install-time eviction churn.
- cozystack-api: `retry.RetryOnConflict` on the Application→HelmRelease
update path (`728f676`).
**E2E robustness follow-ups:** per-test crust-gather snapshots
(`50e5f94`), EtcdBackupSchedule wait (`763b85e`), tenant-node
single-deadline wait (`9e43d45`), harbor BucketClaim 10m budget
(`d7f244f`), tenant API via LoadBalancer (`9beda4e`), crust-gather pin
(`95f42aa`).
**Not addressed (by decision):** FU2 — the s3 Service keeps
`seaweedfs.name`-based naming via patch (rendered output unchanged;
cozystack renders `fullname == seaweedfs`, so it matches the
`componentName`/`fullname` siblings in practice). FU6 — fork PR build
push is handled by the existing mirror-to-same-repo-branch workflow.
## Relates to
#2946#2944#2943#2874#2938#2939#2928#2873 — if this lands, those
can be closed; otherwise they remain the granular per-PR review path.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added CI build cache warming for main branch builds.
* Added a best-effort in-cluster Cilium endpoint self-heal watchdog for
e2e installs.
* Extended SeaweedFS COSI with object-lock support plus separate
readonly access.
* **Improvements**
* Reduced e2e install flakiness with a shared readiness deadline, safer
waits, and improved cleanup/diagnostics capture.
* Prevented install-time churn by switching VPA update mode to
**Initial**.
* Added container startup probes and updated SeaweedFS volume sizing.
* **Tests**
* Improved e2e robustness (etcd backup schedule waiting, digest-pinned
image checks, and corrected S3 port-forwarding).
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## What this PR does
Adds an operator-facing way to serve platform services and the root
tenant's ingress under a pre-existing wildcard TLS certificate instead
of minting per-host ACME certificates. Implements the "native
references" design decided on #2812.
A single value, `publishing.certificates.wildcardSecretName`, names a
TLS Secret the operator has already created in the publishing namespace
(`tenant-root` by default). Only the name travels through the
`cozystack-values` channel — the certificate and private key are never
broadcast, unlike the public `kube-root-ca` that already rides it.
When set:
- **ingress-nginx path** (`gateway.enabled=false`): the publishing
tenant's ingress controller serves the Secret as its
`--default-ssl-certificate`, and the platform service Ingresses drop
their cert-manager cluster-issuer annotation and `tls` block. No
per-host ACME.
- **Gateway API path** (`gateway.enabled=true`): a new `TenantGateway`
cert mode, `existingSecret`, references the Secret in the wildcard/apex
listener `certificateRefs` and mints no Issuer or Certificate. Switching
into this mode garbage-collects any ACME machinery left from
`http01`/`dns01`.
Scope is the root tenant — everything is same-namespace, so no RBAC or
NetworkPolicy changes. Extending wildcard mode to child tenants needs
the Secret replicated into each tenant namespace and is left as a
follow-up.
Closes#2812. Part of #2811.
### Screenshots
N/A — no UI changes.
### Release note
```release-note
feat(platform): operators can serve platform services and the root tenant's ingress under a pre-existing wildcard TLS certificate via `publishing.certificates.wildcardSecretName`, instead of minting per-host ACME certificates
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added an "existingSecret" TLS certificate mode and a wildcardSecretRef
option for TenantGateway and platform values to use a pre-existing
wildcard TLS Secret.
* **Platform / Templates**
* Introduced operator "wildcard secret" mode across Gateway and many
Helm charts (Ingress, apps) to skip per-host ACME provisioning and
optionally wire a default SSL cert.
* **Tests**
* Added extensive chart and controller tests covering existingSecret
behavior and cert-mode transitions.
* **Documentation**
* Updated gateway docs describing the new wildcard secret mode and
requirements.
* **Chores**
* Added Helm unit test makefile targets to several packages.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
The per-test failure snapshots (host in cozytest.sh, tenant in run-kubernetes.sh)
ran `crust-gather collect` unbounded. On a contended/degraded cluster the collect
can hang indefinitely — observed wedging the E2E install step for ~2h35m after a
test failure until the job-level cancel, with no output after the capture line.
`|| true` only catches a bad exit code, never a hang.
Wrap both invocations in `timeout -k 30 300`: a 5-minute cap (hard SIGKILL 30s
later if the collect ignores SIGTERM) bounds the hang; a partial snapshot still
beats a multi-hour wedge, and timeout's exit 124 is already swallowed by `|| true`.
Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The build-main cache warmer is a privileged workflow (packages:write + an OCIR
push token) and never pushes git, so set persist-credentials: false to keep the
checkout's GITHUB_TOKEN out of .git/config where a later build step could read
it. (SHA-pinning the actions is handled repo-wide in #2849.)
Addresses non-blocking follow-up 7 on #2948.
Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
REST.Update wraps the HelmRelease write in retry.RetryOnConflict so a stale
resourceVersion (the helm-controller racing the status subresource) is
refreshed and retried instead of surfacing a 409. Add a unit test using a fake
client whose interceptor returns one Conflict on the first Update and succeeds
on the second, asserting Update returns nil and writes exactly twice.
Addresses non-blocking follow-up 3 on #2948.
Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The leak-healer only refused to disconnect an endpoint whose owning pod was
Running, so a Pending owner that already held a live IPAM allocation
(status.podIP is populated once the sandbox is created, before the pod goes
Running) could have its endpoint evicted mid-startup. Refuse for any
non-terminal phase (!= Succeeded && != Failed); only a terminal owner is
safely past needing its endpoint.
Addresses non-blocking follow-up 4 on #2948.
Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
No code change. The 'debug' PR label was pinning E2E to the contended
self-hosted runner; with it removed this synchronize event re-evaluates
runs-on to the ephemeral oracle-vm pool. Empty commit because the workflow
only triggers on opened/synchronize/reopened (not label changes) and a plain
re-run reuses the original event payload (stale labels).
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
kubernetes-{latest,previous} intermittently failed their node-Ready wait
with the tenant API "connection refused" on localhost, while the cluster was
in fact healthy (host snapshot: both CAPI Machines NodeHealthy=True, TCP
Ready 2/2). Root cause: `kubectl port-forward service/kubernetes-<name>`
pins to ONE of the two Kamaji control-plane pods; when that pod's
kube-apiserver container restarted once (observed restartCount=1), the
forward broke and did not recover, so the 12m wait ran against a dead socket
and the failure-path describe/snapshot also got nothing.
Expose the tenant API with a test-scoped LoadBalancer Service selecting the
Kamaji control-plane pods instead. It load-balances across ALL ready apiserver
endpoints, so a single pod's apiserver restart is routed around transparently;
the host runs MetalLB on the sandbox /24 (pool 192.168.123.200-250) so the IP
is directly routable from the test (the in-tenant LB test already curls such
an address). The kubeconfig points at the LB IP with insecure-skip-tls-verify
(the MetalLB IP isn't in the apiserver cert SANs; e2e only) and the CA dropped.
Bonus: the LB endpoint is stable until teardown, so the on-failure tenant
crust-gather snapshot can finally reach the tenant (the port-forward was dead
by snapshot time, which is why every prior tenant snapshot came back empty).
Removes the port-forward supervisor + pidfile machinery. Verified against the
real hack/cozytest.sh under dash (CI's /bin/sh) in ubuntu:22.04: no pipeline
deadlock, the in-subshell snapshot trap fires and captures the tenant, and the
LB Service is cleaned up on both the success path and cozy_cleanup.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The healer's reserved/infra-endpoint remedy and its disconnect-failure
fallback both run `kubectl delete pod` to reschedule the wedged pod onto a
free IP, but the ClusterRole granted pods only get/list — so the delete was
forbidden and the pod stayed wedged. Add delete to the pods rule.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
The restored COSI -lock/-readonly BucketClass/BucketAccessClass and the s3
Service name override lived only as raw edits in the vendored chart tree, so
the next `make update` (rm -rf charts && curl | tar && patch) would silently
drop them again — the same way the 4.31 bump did. Capture both as patch files
under patches/ and wire them into the Makefile update target, matching the
existing s3-tls-main-port / cosi-provisioner-sa-name customizations. No
rendered output changes; this only makes the customizations survive re-vendor.
Addresses the B1 review blocker on #2948.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
crust-gather can only reach a tenant Kamaji cluster through the test's
localhost port-forward (it connects directly to the kubeconfig server URL —
there is no apiserver-proxy mode — and the in-cluster URL is unreachable from
the runner). cozytest.sh's generic EXIT trap runs in the parent shell AFTER
the failing test subshell has exited and its port-forward is reaped, so a
collect there hits a dead socket and crust-gather bails before writing
anything (every tenant snapshot so far came back empty).
Capture the tenant from an EXIT trap registered INSIDE run_kubernetes_test
instead: it fires during THAT subshell's exit, while the supervised
port-forward is still alive. Verified end-to-end against the real
hack/cozytest.sh under dash (CI's /bin/sh) in an ubuntu:22.04 container —
the in-subshell trap fires on failure with the PF alive, the tenant snapshot
lands in the same snapshots/<test>/ dir as the host snapshot, the generic
cozy_cleanup still runs, and there is no pipeline deadlock.
Drop the now-dead tenant loop from cozytest.sh's generic trap (it could only
ever see a reaped PF); it keeps the host snapshot.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
A single `kubectl port-forward` to the tenant Kamaji API silently dies on
an SPDY/connection drop and never reconnects. The tenant apiserver itself
stays healthy (control-plane pods observed at 0 restarts while localhost
refused connections), so this is a client-side port-forward death, not a
control-plane outage. Once it dies every later tenant kubectl op wedges:
a long `kubectl wait` burns its full timeout against the dead socket
(observed: nfs-test-pod hit its 10m deadline) and the failure-path
`describe`/`get events` print 'connection refused', destroying the
diagnostics too.
Run the port-forward under a respawn loop so a drop self-heals within ~1s;
`kubectl wait` retries across the gap and the in-test failure dumps work
again. Track the supervisor PID in a pidfile so cozy_cleanup() can stop it
on a failed run (the success path kills it directly), and reap the child
port-forward with pkill in both paths.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The aggregated API server translates app CR updates (Etcd, Postgres, ...)
into a read-convert-update of the backing HelmRelease. Flux's
helm-controller continuously writes the HelmRelease status, which shares
the object's resourceVersion, so a CR update that lands while a prior
reconcile is still in flight reads a resourceVersion that goes stale
before the Update and the request fails with a 409 Conflict
("the object has been modified; please apply your changes to the latest
version and try again").
The HelmRelease spec is fully derived from the Application the caller
just applied, so a stale-resourceVersion conflict is never a real spec
conflict here. Wrap the Update in retry.RetryOnConflict, refreshing the
live resourceVersion on each conflict before retrying. The no-conflict
path is unchanged, and this mirrors the existing RetryOnConflict usage in
workloadmonitor_controller.go.
Surfaced by the etcd.bats e2e test, which applies the same singleton
Etcd resource three times back-to-back and intermittently failed the
third apply with the 409.
Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
(cherry picked from commit 76410599de)
The endpoint-leak healer evicts the stale endpoint holding a disputed IP
via 'cilium-dbg endpoint disconnect'. But when the IP is held by a
reserved/infra endpoint with no k8s pod identity — e.g. reserved:ingress,
the per-node Cilium Ingress endpoint — that call is rejected with
'endpoint may not be associated reserved labels', so the healer spins
forever and the wedged pod (observed: cozy-backup-controller) never
recovers, failing the run.
Such infra endpoints legitimately own the IP; the duplicate is an IPAM
race that also handed it to a pod. Evicting infra is both impossible and
wrong. Instead reschedule the wedged pod onto a free IP by deleting it
(verified live: the recreated pod lands on a clean address while the
ingress endpoint keeps the disputed one). Also fall back to deleting the
wedged pod if a disconnect of a presumed pod endpoint fails for any
reason, so a pod is never left wedged.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
(cherry picked from commit 19a9063ae0)
Harbor's registry BucketClaim wait timed out at 300s on an overloaded
runner even though the underlying Bucket reached bucketReady=true and the
BucketAccess was granted — only the central COSI controller's propagation
of readiness back onto the namespaced BucketClaim lagged (all HelmReleases,
harbor included, were Ready by end-of-run). Raise the wait to 600s to match
the dependent harbor-system HelmRelease budget and remove the flake.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
(cherry picked from commit 074501f999)
The crust-gather install.sh substitutes the version verbatim into both
the release-tag path and the asset filename:
releases/download/<ver>/kubectl-crust-gather_<ver>_<target>.tar.gz
The release tag is 'v0.15.2', so passing '0.15.2' 404s on the tag path
(curl exit 22) and fails the image build. The 'v0.15.2' asset variant
exists too, so the leading v satisfies both. Verified HTTP 200.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
cozytest.sh runs each .bats as one invocation, exits on the first failing @test, and never invokes bats setup/teardown. Add a single EXIT-trap hook: on failure, snapshot the host AND every nested tenant cluster (tenantkubeconfig-*) with crust-gather into per-app dirs BEFORE cleanup — so each failure is independently inspectable (crust-gather serve) instead of one end-of-suite host-only dump; then ALWAYS run the file's cozy_cleanup() if defined.
run-kubernetes.sh gains cozy_cleanup() (delete tenant clusters, wait for PVCs to free): a failed kubernetes test otherwise leaves tens of GiB of worker-VM PVCs in tenant-test, exhausting the shared tenant-quota and cascade-failing every storage-heavy app after it. The kubernetes .bats now source run-kubernetes.sh at file scope so the parent-shell trap can reach the hook.
Ship pinned crust-gather 0.15.2 in the e2e-sandbox image; fold per-test snapshots into cozyreport.tgz.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The e2e runner hack/cozytest.sh converts only @test blocks into functions and runs those; it never invokes bats setup_file/teardown_file. So the healer's setup_file deploy silently never executed (confirmed: the Job was absent from the cozyreport while the same manifest deploys and runs fine on a real Talos cozystack cluster). Move the deploy into a real, first @test so cozytest.sh runs it (with visible set -x trace) before the install churn, and assert the Job landed without ever failing the suite (band-aid).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
kubernetes-latest flaked because the worker-readiness wait was split into 8m-to-join + 3m-to-become-Ready. Under sandbox load a slow KubeVirt VM boot consumes most of the join budget, then the tenant cluster's cilium CNI needs several more minutes to make the freshly-joined nodes Ready, overflowing the fixed 3m Ready window even though the CNI converges fine (its HelmRelease reached True shortly after). Replace with one 12m deadline polling for >=2 Ready nodes, robust to wherever the time goes. Same single-deadline shape as the LINSTOR post-install fix (#2928).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Reconfiguring the Etcd CR triggers a HelmRelease upgrade that renders the EtcdBackupSchedule; hr/etcd and etcdcluster can report ready on the prior revision before the new resource materializes, so the immediate get raced to NotFound. Poll for it (timeout 120s), mirroring the CronJob wait already used later in the same test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Auto mode evicts running etcd members to apply VPA recommendations, churning the StatefulSet (disruptive to quorum and a Cilium IP-reuse-leak trigger under load). Initial sizes at pod creation without evicting; the existing 250m/256Mi minAllowed floor covers the no-runtime-resize tradeoff. Same eviction-churn class as the monitoring VPA fix.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
In Auto mode the VPA Updater evicts running pods to apply new recommendations. During install that eviction churn recreates VictoriaMetrics pods faster than Cilium releases their endpoint IPs, tripping the cluster-pool IPAM "IP already in use" (putEndpointIdInvalid) leak that wedges a pod and stalls the install. Initial sets requests at pod creation without ever evicting, removing the trigger (the cilium-leak healer remains as defense for non-VPA churn paths).
Also raise vmstorage/vmselect minAllowed floors to 200m/512Mi: under Initial a pod is sized once at creation and never re-sized at runtime, so the floor is the safety net against the heavy query/storage tiers being left under-provisioned.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
## What this PR does
Clears the OpenSSF Scorecard **Vulnerabilities** check (was **0/10**).
Scorecard queries OSV against the dependency graph, so reproducing it
with `osv-scanner` over both `go.mod` files surfaced **12 advisories**:
| Advisory | Package | Fixed in |
|---|---|---|
| GO-2026-4918, 4971, 4977, 4980, 4982, 4986 | stdlib (Go toolchain) |
1.26.3 |
| GO-2026-5037, 5038, 5039 | stdlib (Go toolchain) | 1.26.4 |
| GO-2026-4918, GO-2026-5026 | `golang.org/x/net` | 0.55.0 |
**10 of 12 are stdlib advisories** against the Go toolchain pinned at
`1.26.2` — so the dominant fix is raising the Go directive, not churning
dependencies.
### Changes
- **Go directive `1.26.2 → 1.26.4`** in the root module and the
`api/apps/v1alpha1` submodule (closes all 10 stdlib advisories; 1.26.4
is required for GO-2026-5037/5038/5039).
- **`golang.org/x/net 0.52.0 → 0.55.0`** (closes GO-2026-4918,
GO-2026-5026). `go mod tidy` carried the matching minor bumps to
`x/crypto`, `x/sys`, `x/term`, `x/text`, `x/tools` via MVS.
All version changes were made with `go mod edit` / `go get` / `go mod
tidy` — the generated `go.mod` was not hand-edited.
### Verification
- `osv-scanner` after the bump: **Total 0 packages affected by 0 known
vulnerabilities.**
- `go build ./...` passes on both modules (toolchain auto-upgraded to
go1.26.4).
- `make generate` produces **no drift** — `codegen-drift` CI stays
green.
> **Maintenance note:** the Renovate config (separate PR) deliberately
does **not** auto-bump the Go toolchain directive, so future stdlib
advisories (1.26.5+) will need a manual bump like this one. If you'd
prefer Renovate to keep the Go *patch* version current automatically,
that rule can be enabled — flagged for a decision.
### Release note
```release-note
fix(deps): bump Go to 1.26.4 and golang.org/x/net to v0.55.0, closing 12 known OSV advisories (10 stdlib, 2 in x/net)
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Chores**
* Updated the Go toolchain to the latest 1.26.x release for improved
build reliability and runtime compatibility.
* Bumped several indirect Go module dependencies (networking, crypto,
system, terminal, text, and tooling libraries) to newer stable versions
to enhance stability and security.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## What this PR does
Adds `spec.timeout: 10m` to the `kubevirt-csi-node` HelmRelease template
in `packages/apps/kubernetes/templates/helmreleases/csi.yaml`, aligning
it with every other tenant addon HR in the same directory (cilium,
coredns, ingress-nginx, etc. — all set `timeout: 10m`). `csi.yaml` was
the only one missing it; commit 9f9d8f85 ("Align timeouts for
HelmReleases") missed this template.
## Symptom
E2E NFS test times out waiting for `nfs-test-pod` to reach `Succeeded`.
Cozyreport events show the tenant `csi` HR running an Install →
Uninstall → Install remediation cycle. With
`install.remediation.retries: -1` and the default 5m Helm timeout,
kstatus marks the `kubevirt-csi-node` DaemonSet `InProgress` past 5m on
a slow tenant control-plane bringup (Kamaji APIs flapping, VM image
pulls), helm-controller times out, the remediation uninstalls the
DaemonSet, then reinstalls. Any volume mount in flight at that moment is
wedged.
## Origin
Single commit lifted unchanged from #2619.
### Release note
```release-note
NONE
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Chores**
* Updated the Container Storage Interface (CSI) Helm release settings to
include a `spec.timeout` of **10 minutes** when the etcd namespace value
is set (keeping the existing `spec.interval` at **5 minutes**).
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary
- Drop the `rm -f` line in
`packages/system/prometheus-operator-crds/Makefile` so the full upstream
CRD bundle (10 CRDs) ships instead of the 4-CRD subset.
- Pin `helm pull --version 25.0.0` to keep the operator CRD version at
`v0.87.0` (operator version bump is intentionally out of scope here).
- Re-run `make update` to repopulate `charts/crds/templates/` with the 6
previously-stripped CRDs: `Alertmanager`, `AlertmanagerConfig`,
`Prometheus`, `PrometheusAgent`, `ScrapeConfig`, `ThanosRuler`. The 4
already-vendored CRDs are byte-identical (version pin holds).
## Why
The bundle was originally trimmed to the 4 service-discovery CRDs that
VictoriaMetrics operator needs (`ServiceMonitor`, `PodMonitor`,
`PrometheusRule`, `Probe`). That was correct when the chart was vendored
inside `system/victoria-metrics-operator` (`ba5c785cc`, Dec 2023), but
the prune was carried over verbatim when the chart was extracted to its
own package, and again when the CRD release was propagated into tenant
Kubernetes clusters (`1e2b66131`, Dec 2025).
In tenant clusters the partial set blocks third-party apps that ship
their own `prometheus-operator` (e.g. `kube-prometheus-stack`): they
cannot deploy `Prometheus`/`Alertmanager` instances because the
platform-managed CRD set is incomplete, and `--server-side
--force-conflicts` workarounds get reverted by Flux as it re-reconciles
the narrower release manifest.
With the full bundle shipping, tenant apps can set `crds.enabled: false`
on their `kube-prometheus-stack` release and consume the platform CRDs
directly — the standard pattern for platform-level CRDs shared between
multiple stacks.
## Test plan
- [x] `helm template ./charts/prometheus-operator-crds` renders 10
`CustomResourceDefinition` resources, all in `monitoring.coreos.com` API
group.
- [x] `helm template packages/apps/kubernetes -f
tests/values/common.yaml` renders cleanly (75 kinds).
- [x] `helm unittest packages/apps/kubernetes` — 6 suites, 123 tests,
all pass.
- [x] No in-repo consumer hardcodes the 4-CRD list (verified by grep).
- [ ] Smoke-test on a tenant cluster: confirm `kube-prometheus-stack`
with `crds.enabled: false` installs cleanly and the operator can manage
`Prometheus`/`Alertmanager` instances against the platform CRDs.
## Out of scope
- Bumping the operator CRD version past `v0.87.0` — separate concern.
- Switching from `HelmRelease` to a Flux `Kustomization` for CRD
application — bigger refactor; current Helm ownership is fine now that
the bundle is complete and downstream apps can opt out of installing
CRDs.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Chores**
* Pinned the prometheus-operator-crds Helm chart pull to version
`25.0.0` for consistent regeneration.
* Streamlined the update workflow by removing the existing generated
charts directory before pulling the chart, improving build cleanliness.
* Removed the extra post-pull cleanup step that previously deleted
selected generated CRD templates.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Add `rm -rf charts/prometheus-operator-crds` before `helm pull` in the
update target so stale files from previous chart versions don't linger
across updates. This matches the convention used by cilium,
external-secrets-operator, external-dns, and cert-manager Makefiles.
Address review feedback from gemini-code-assist on
packages/system/prometheus-operator-crds/Makefile:9.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The chart previously stripped the bundle to 4 CRDs (ServiceMonitor,
PodMonitor, PrometheusRule, Probe) — the set VictoriaMetrics operator
needs for service discovery. This was correct when the chart was
vendored inside system/victoria-metrics-operator (commit ba5c785cc,
2023), but the prune was carried over when the chart was extracted
to its own package, and again when the CRDs were propagated into
tenant Kubernetes clusters (commit 1e2b66131, 2025).
In tenant clusters the partial set blocks third-party apps that ship
their own prometheus-operator (e.g. kube-prometheus-stack) — the
missing CRDs (Prometheus, Alertmanager, AlertmanagerConfig,
PrometheusAgent, ScrapeConfig, ThanosRuler) prevent the operator from
deploying its instances against the platform-managed CRD set.
Drop the rm -f line in the Makefile so the full upstream bundle ships,
and pin --version 25.0.0 to keep the operator CRD version at v0.87.0
(operator version bump is a separate concern, out of scope here).
Tenant apps can now set crds.enabled: false on their kube-prometheus-
stack release and consume the platform CRDs directly.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Aligns kubevirt-csi-node HR with every other tenant addon HR in
packages/apps/kubernetes/templates/helmreleases/ (cilium, coredns,
ingress-nginx, etc.) — all set timeout: 10m, csi.yaml was the only
exception. 9f9d8f85 ("Allign timeouts for HelmReleases") missed it.
Symptom: e2e NFS test (hack/e2e-apps/run-kubernetes.sh) times out
waiting for nfs-test-pod to reach Succeeded. Cozyreport events show
the tenant csi HR running Install -> Uninstall -> Install remediation
cycles. With install.remediation.retries: -1 and the default 5m Helm
timeout, kstatus marks the kubevirt-csi-node DaemonSet 'InProgress'
past 5m on a slow tenant control-plane bringup (Kamaji APIs flapping,
VM image pulls), helm-controller times out, the remediation uninstalls
the DaemonSet, then reinstalls. Any volume mount in flight at that
moment is wedged.
Observed on PR #2619 CI run 26142986969: csi HR remediation at
~07:07 happened ~10 min into the nfs-test-pod wait, killing the
mount and timing out the pod-Succeeded condition.
10m matches the surrounding HRs and gives the DaemonSet headroom
without changing remediation semantics on truly-broken installs.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
(cherry picked from commit 866588ff61)
## What this PR does
`cozystack-api` published the free-form `.spec` of aggregated
`apps.cozystack.io` resources (Tenant, Application, …) as the boolean
`additionalProperties: true`. The Kubernetes ValidatingAdmissionPolicy
**status type-checker** run by `kube-controller-manager`
nil-dereferences when it recurses into that node
(`k8s.io/apiserver/pkg/cel/openapi.isExtension`). As soon as a VAP
matches one of these resources — `cozystack-tenant-host-policy` matches
`tenants` — KCM `CrashLoopBackOff`s on every control-plane node, stalls
reconciliation, and times out the install (`kubectl wait hr --all` never
completes). With the 3× install retry removed, a single occurrence fails
E2E outright.
This emits `x-kubernetes-preserve-unknown-fields: true` instead, in
`pkg/cmd/server/openapi.go::patchSpec` (both the schemaless and
custom-schema branches, covering the OpenAPI v2 and v3 post-processors).
It is semantically equivalent — "arbitrary fields allowed" — and the
type-checker handles it safely. All managed-app custom schemas already
use the safe **object-form** `additionalProperties`, so `patchSpec` was
the only source of the boolean form.
### Regression test
`TestPatchedSpecDoesNotPanicVAPTypeChecker` reproduces the exact crash
through `k8s.io/apiserver/pkg/cel/openapi.SchemaDeclType` — the entry
point KCM's `validatingadmissionpolicystatus` controller uses. It
**panics on the pre-fix code and passes after the fix**. Companion
structural tests assert the published spec carries
`x-kubernetes-preserve-unknown-fields: true` and never the boolean
`additionalProperties: true`, and that genuine object-form maps are left
untouched.
Verified against the pristine vendored `k8s.io/apiserver@v0.34.1` (the
version cozystack-api builds against), which reproduces the panic, and
previously confirmed end-to-end on `kindest/node:v1.33.1`.
### Upstream
The underlying nil-dereference is a Kubernetes KCM bug — the VAP status
type-checker should treat `additionalProperties: true` as dynamic rather
than panic. A separate upstream report is being prepared; cozystack
fixes the symptom immediately with the schema change above.
Fixes#2863
### Release note
```release-note
fix(api): publish the free-form resource `.spec` as `x-kubernetes-preserve-unknown-fields` instead of `additionalProperties: true`, fixing a cluster-wide `kube-controller-manager` CrashLoopBackOff (nil-pointer panic in the ValidatingAdmissionPolicy status type-checker) that stalled installs.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Ensured published OpenAPI schemas no longer contain boolean-form
additionalProperties anywhere; top-level and nested schemas now use
preserve-unknown-fields for free-form specs, preventing
validation/runtime issues.
* **Tests**
* Added regression tests that feed varied untrusted schemas and verify
boolean additionalProperties are removed and published schemas remain
valid for tooling.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
