Timeout annotations are read from an ApplicationDefinition and therefore
affect every Application of that kind. Describing them as per-Application
hides their actual scope and the upgrade override precedence.
Fixescozystack/cozystack#4382
Assisted-by: LLM
Signed-off-by: frolov.so <opsmon.sh@yandex.com>
A schema default cannot fill arbitrary map entries or replacement array
items. Keep those nulls, nullable fields and required fields without a
non-null default unchanged so Helm retains their validation semantics.
Validate the normalized copy on both write paths so admission sees the
values that reach storage without mutating the caller object.
Assisted-By: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
An Application spec reaches flux verbatim as HelmRelease .spec.values,
and Helm's value coalescing deletes a null-valued key together with the
chart default underneath it, before the result is validated against
values.schema.json. A required field written with nothing under it is
therefore rejected for being absent:
talos:
registryMirrors:
fails the whole release with "at '/talos': missing property
'registryMirrors'", naming a property the spec does contain, and nothing
reaching the user points at the empty key they typed.
Remove such a key on the write path, so the chart's own default applies
to it. Filling in the schema default instead would pin today's value
into the stored HelmRelease and freeze the application on it, so a later
bump of that default in the chart would never reach an application that
had once been created with an empty key.
Only required properties are repaired. A required property's null cannot
be load-bearing for anyone, because today it fails the render outright,
so resolving it is pure repair. Where the schema allows the field to be
absent, a null keeps the meaning it has now - the chart default is
suppressed and the key does not reach the template - and that meaning is
worth keeping: 167 optional fields across the catalogue carry a
non-trivial default, placeholder credentials and example S3 buckets
among them, so resolving a user's null to one of those would be worse
than leaving the key out.
Free-form subtrees, null map entries under a user-chosen key (a node
group, a registry host) and array elements are left as written, because
there a null may be the payload rather than an omission. Every removal
is reported back to the client as a warning, so the repair is visible
rather than silent.
Assisted-By: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
A Redis release with external: true and TLS off publishes a plaintext
endpoint outside the cluster, and nothing stopped a tenant from asking
for one. The chart never infers TLS from external, because enabling it
moves Redis to a TLS-only listener, so external: true without an
explicit tls.enabled: true is exactly that case. A
ValidatingAdmissionPolicy on the Redis kind now refuses it with a
message naming the release and a remedy that works.
TLS is fixed when a Redis instance is created: the operator's
RedisFailover schema refuses a change, but the Redis API accepts it.
A spec's tls.enabled is therefore a request, not the instance's state,
and cannot prove on UPDATE that an instance has TLS; trusting it would
let a tenant flip it, expose the instance, and remove it again. So
external access can be enabled only at creation, where tls.enabled
does decide the instance, and any UPDATE turning it on is refused.
The RedisFailover schema does not exist before the first reconcile, so
a release admitted as external with TLS could have TLS dropped before
the instance is created. tls.enabled is therefore frozen while
external access stays on. Withdrawing external access in the same edit
lifts the freeze, which leaves a tenant a way to unblock an instance
whose TLS was flipped earlier, and the callers trusted by the tenant
host policy are exempt so an administrator can repair one in place.
Other edits to an instance that is already external pass, so releases
exposed in plaintext before the policy keep working.
Postgres is left out on purpose. CNPG pins ssl=on, and its default
pg_hba accepts plaintext from any client that asks, whatever the
chart's tls.enabled says; that field only decides whether the external
hostname goes into the certificate. A policy keyed on it would close
nothing.
The decisions are tested by compiling the shipped template with the
apiserver's own CEL compiler, since a structural chart test cannot
show what the policy admits.
BREAKING CHANGE: creating a Redis with external: true and without
tls.enabled: true, which applied before, is now refused, including a
GitOps re-apply of such a manifest into a fresh cluster. Turning
external on for an existing Redis is refused as well, so exposing an
instance means creating a new one with TLS, and tls.enabled can no
longer be changed on an instance with external access.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
## What this PR does
This moves the root module's `k8s.io/*` from v0.35 to v0.37.1 and
controller-runtime from v0.23 to v0.25.1, and drops the
`k8s.io/apimachinery` fork on the way. Supersedes #3001, #4467 and
#4472; those were a stack of the same commits.
The four commits are meant to be read one by one, and each builds and
passes the tests on its own:
1. **Plain watches stop getting the initial-events-end bookmark.** The
apiserver defaults `sendInitialEvents=true` onto any watch with an empty
or `0` resourceVersion and leaves bookmarks off, and our aggregated
storages took that flag alone as a WatchList request. From
`k8s.io/apiserver` v0.36 the watch handler compiled into cozystack-api
panics on that bookmark, so `kubectl get -w` on any cozystack-api
resource would die after the bump. The marker now needs both flags, the
same condition the watch cache uses. There are tests per storage and one
through the real handler chain.
2. **The apimachinery fork goes.** Every apps kind shares the
`Application` Go type, and stock `Scheme.ConvertToVersion` picks the
first kind registered for it. The fork patched that; the apps group now
encodes through a serializer that keeps the kind the object carries. A
handler-chain test covers create, get, list, update, watch, apply and
table for two kinds and fails without it.
3. **MutatingAdmissionPolicy stays out of admission.** From v0.36 the
plugin is on by default and refuses every write until it has synced
`mutatingadmissionpolicies`, which cozystack-api has no RBAC to list and
which clusters before 1.36 do not serve. Without this, every create,
update and delete would fail after the bump. It is also re-applied in
`Complete`, since `--disable-admission-plugins` replaces the list.
4. **The bump itself**, with the regenerated clients, openapi and
strategy CRDs.
Commits 2 and 4 are where the review effort goes; 1 and 3 are small.
Known changes in behaviour:
- Without the fork, the row objects of a Table requested with
`includeObject=Object` carry the first registered app kind. kubectl asks
for that only with `--sort-by`, and it sorts on fields, not on the kind.
Nothing at this layer can fix it, because the handler converts through
`APIGroupInfo.Scheme`, a concrete `*runtime.Scheme`. Admission webhooks
and ValidatingAdmissionPolicy still see the right kind.
- The alpha PodSpec field `workloadRef` became `schedulingGroup`
upstream. Once the new strategy CRDs are applied, the apiserver prunes
`workloadRef` from stored strategies it serves, and their next write
erases it. It only ever worked behind an alpha feature gate.
The branch is rebased onto the Renovate rollup (#4447), with `go mod
tidy` and codegen rerun on top of it.
### Screenshots
No UI changes.
### Downstream repositories
- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:
### Release note
```release-note
chore(deps): cozystack-api and the controllers build against the Kubernetes 1.37 libraries without the apimachinery fork. Plain watches no longer receive the WatchList initial-events-end bookmark, and objects in Table rows requested with includeObject=Object may carry another app's kind.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Application API responses now preserve the requested resource kind
across operations, including lists and watches.
* Backup strategy resources support additional Kubernetes pod settings,
including probe protocols, volume ownership options, and scheduling
groups.
* Backup strategy schemas include updated Kubernetes pod fields and
descriptions.
* **Bug Fixes**
* Initial-event watch bookmarks are sent only when requested; plain
watches continue without an ending bookmark.
* Mutating Admission Policy is disabled by default, including when
admission plugin options are customized.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
WorkloadsReady=False covers different situations: a stopped VM has
no pod and its monitor reports it not operational on purpose, while a
disk whose DataVolume never populates is broken. A consumer that wants
to react to one and not the other had only the message text to go by.
The WorkloadMonitor status gains a Reason next to Message, set to
DataVolumeNotReady by the DataVolume check, and the Application API
carries it into the WorkloadsReady condition in place of the generic
reason whenever a monitor that is not operational names one.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Ready follows the Helm release, so for a kind whose install does not
wait on its workloads, a vm-disk among them, READY in kubectl get says
only that the manifests were applied. Whether the workloads came up
lives in WorkloadsReady, which the default table did not show, so a
disk CDI never populated looked healthy on the surface an operator
reads first.
The table now carries a WORKLOADS column beside READY. It reads
<none> for an Application with no WorkloadMonitor, which has no
WorkloadsReady condition at all.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
For a vm-disk, WorkloadsReady is the one condition that reports
whether CDI populated the disk, and its generic "One or more workloads
are not operational" gives an operator nothing to act on: a disk has
no workloads, and the stuck DataVolume and its phase are exactly what
they need to know.
The WorkloadMonitor status gains a message that the controller fills
with each selected DataVolume that is not ready and its phase, and the
Application API uses the messages of the monitors that are not
operational in WorkloadsReady instead of the generic text whenever one
of them names a cause.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
An Application and its HelmRelease share one UID, so the garbage
collector keeps a single graph node for both and may finalize it through
the Application endpoint. The API never returned finalizers on an
Application, and every update restored the HelmRelease finalizers from
the live object. When the collector came to drop foregroundDeletion or
orphan through the Application, removeFinalizer found no such finalizer
and returned as if it were already gone. The HelmRelease then kept
foregroundDeletion after Helm had uninstalled the release, and the
deletion never completed.
The Application now carries the two garbage collection finalizers, and
an update that adds or removes one of them applies the same change to
the HelmRelease. Controller finalizers such as finalizers.fluxcd.io stay
owned by the live object, so helm uninstall still runs before the
release goes away. The deletion timestamp and grace period are copied
from the live object, which keeps an update of a terminating release
valid.
Assisted-by: LLM
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
A 409 on the HelmRelease PUT means the live object changed after the
read the carry-over was built from, and the writer behind it is normally
helm-controller - the controller whose finalizer the carry-over exists
to protect, and which re-adds it on the reconcile following a
metadata-only update. Retrying with the metadata read before the
conflict wrote that stale copy back and dropped the finalizer the
reconcile had just restored, reopening the orphaning window the
carry-over closes.
Assisted-by: LLM
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
mergeMaps returned one of its inputs whenever the other was nil. Create
and Update seed the HelmRelease labels with
mergeMaps(r.releaseConfig.Labels, ...) and then write the application
metadata labels into the result in place, so for an Application with no
labels the writes landed on the long-lived config map shared by every
request for the kind: requests cross-contaminate (application.name
varies per request) and concurrent map writes crash the apiserver.
Signed-off-by: mattia-eleuteri <mattia@hidora.io>
Update rebuilds the HelmRelease from the Application and issues a
full-replace PUT, which stripped metadata owned by other controllers:
finalizers, ownerReferences, and any labels or annotations outside the
apps.cozystack.io- prefix. Losing helm-controller's
finalizers.fluxcd.io finalizer is the harmful case: a delete landing
before helm-controller re-adds it removes the HelmRelease without
running helm uninstall, orphaning every resource of the release (VMs,
databases, PVCs). A label patch immediately followed by a delete - a
common client pattern - hits that window almost every time.
Fetch the current HelmRelease before the PUT and carry over its
finalizers, ownerReferences, and non-prefixed labels and annotations.
Keys under the apps.cozystack.io- prefix remain owned by the API and
are recomputed from the Application, so label/annotation deletions
still propagate.
Signed-off-by: mattia-eleuteri <mattia@hidora.io>
The apiserver defaults SendInitialEvents on for every watch whose
resourceVersion is empty or "0", and leaves AllowWatchBookmarks off. The
aggregated storages read SendInitialEvents alone as a WatchList request,
so a plain `kubectl get -w` on applications, tenant modules, namespaces,
secrets or security groups got a bookmark annotated as the end of
initial events.
The k8s.io/apiserver v0.35 watch handler lets that through. From v0.36
it treats the annotated bookmark as the end of a WatchList and calls the
WatchList completion hook, which it only sets up when SendInitialEvents
and AllowWatchBookmarks are both on, so the handler dereferences nil and
the watch dies. The handler is the one compiled into cozystack-api, so
bumping the library is what breaks it, whatever the cluster version.
The marker now requires both flags, the same condition the watch cache
uses, and the backing watches ask for bookmarks only when the client
did, since every backing bookmark is forwarded. Security groups still
forward SendInitialEvents to their backing watch so it replays existing
objects as ADDED.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
## What this PR does
This replaces the open Renovate PRs with one rollup. Go modules move to
their newest releases that keep the Kubernetes libraries and
controller-runtime where they are, the toolchain and builder images move
to Go 1.27.1, and the first-party code gets the Go 1.27 modernizers.
Tests for the code the bumps touch come first, in their own commits, and
pass on both the old and the new dependencies.
Before the bumps:
- The e2e sandbox image builds again: dl.min.io answers 410 for mc since
MinIO archived it, so mc now comes from its last GitHub release with a
pinned checksum.
- Renovate stops offering tags of the apimachinery fork. They mirror
upstream, so #2999 is a downgrade that drops our patch.
- `make unit-tests` now runs the tests of the nested Go modules
(kubevirt-csi-driver, token-proxy, kubeovn-webhook, api/apps/v1alpha1).
They never ran in CI before.
- New tests: JWT verification in the dashboard token proxy, the kube-ovn
plunger metrics, cert-manager object idempotency in the tenant gateway,
values-file layering in the operator, and the kubevirt CSI driver
wrapper.
- One real fix. The lineage webhook decoded admitted objects with
encoding/json, so an int64 above 2^53 came back rounded. jsonpatch v2.5
compares numbers exactly and would emit a replace that rewrites such a
field in the pod. The webhook now decodes with apimachinery's json.
Held back on purpose, with the reasons in the commit messages:
- Kubernetes libraries stay on v0.35. #3001 needs the apimachinery fork
rebased onto release-1.37 first.
- cel-go v0.29.2 and etcd v3.6.15, the lines k8s.io/apiserver v0.35 is
built and tested with.
- grpc v1.83.2. v1.84.0 is affected by GO-2026-6443 and no v1.84 patch
release carries the fix yet.
- kubevirt.io/api and the CDI API stay on what upstream csi-driver
builds against. From CDI v1.60 it no longer compiles.
- api/apps/v1alpha1 keeps go 1.26.4, so consumers of the published types
are not forced onto Go 1.27.
- go fix runs without embedlit (Go 1.27-only syntax, breaks backports to
release branches on Go 1.26) and omitzero (would make metadata, spec and
status required in the published OpenAPI schemas), and one errorsastype
rewrite that did not compile is left out.
govulncheck reports nothing reachable in any of the five modules,
standard library included.
Supersedes #4311, #4310, #4309, #3452, #3993, #3927, #3902, #3805,
#3451, #3415, #3414, #3413, #3302, #4050 and #3363. #3001 stays open.
Not checked locally: the images that build upstream projects (kube-ovn,
kamaji, cilium, linstor-csi and others) with the Go 1.27 builder. The
image build jobs of this PR cover them.
Closes#4487.
### Screenshots
No UI changes.
### Downstream repositories
- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:
### Release note
```release-note
chore(deps): move the toolchain and builder images to Go 1.27.1 and raise Go dependencies to their latest compatible releases, closing the published advisories; the lineage webhook no longer rewrites int64 fields above 2^53 in admitted objects.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added a restore resource policy option for Velero strategies.
* Added a test target to run Go tests across tracked modules.
* **Bug Fixes**
* Updated the end-to-end test environment to download and verify the
MinIO client from its current release location.
* Added coverage for token validation, webhook patching, controller
behavior, and reconciliation.
* **Maintenance**
* Updated the Go toolchain, dependencies, and container image versions.
* Refreshed pinned workflow actions and base image digests.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
The apps API hands the stored spec to the HelmRelease unchanged, so a
users[].password left on a Postgres or MariaDB application is silently
ignored by the chart. Return a warning on create and update for each
such user, and for the superuser key each chart refuses to manage, so
the operator editing it learns it has no effect. Also warn when an
update introduces a "_"/"-" collision between usernames, database names
or Grant names, which the charts reject only on install.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
## What this PR does
Follow-up to #4367, taking the non-blocking findings from its approving
review. Each one is a case where a name declaration (`x-cozystack-name`)
was handled more harshly, or more quietly, than the declaration itself
warrants.
- **An unparseable schema is no longer blamed on a declaration.** A
schema that is not a JSON object has no declaration in it, and the spec
schema loader already reports it. The API server used to refuse every
create of the kind with "declares an invalid x-cozystack-name". It now
treats the schema as declaring nothing: creates keep working, and
`Kubernetes` and `Kafka` keep their legacy caps.
- **Annotation keywords no longer take a kind offline.** Any unknown key
inside a declaration used to refuse creates, so a generator adding a
`title` or an example would have done the same. JSON Schema annotation
keywords (`title`, `default`, `examples`, `deprecated`, `readOnly`,
`writeOnly`, `$comment`) and `x-` extensions are now ignored. A
constraint keyword the server does not implement, such as `format` or a
misspelled `maxLenght`, is still refused, because accepting it would
drop it silently.
- **The docs no longer overclaim.** They said a declaration no name can
satisfy is refused. That is true only for lengths: a pattern is checked
to compile, not to match anything, so `$^` rejects every name and names
the pattern as the reason. The docs now say that.
- **The API review gate sees the declarations the server refuses.** The
gate only read a declaration that was an object, and only diffed the
keywords it knew. A declaration turned into a string or null, or a
misspelled keyword, looked like no change. A changed declaration now
goes through the same parser the server uses, and the gate reports it if
it cannot be enforced. The parser moves to `pkg/apis/apps/validation` so
there is one copy of the rules.
- **The declaration no longer leaks into the published OpenAPI for
`spec`.** The whole `openAPISchema` is published as the schema of
`spec`, so `x-cozystack-name` appeared there as a vendor extension of
`spec`. It is now stripped from the published copy. `metadata.name`
still carries no limit in the published OpenAPI: it is the shared
`ObjectMeta` reference, and a per-kind copy is a bigger change than this
one.
Not changed: a pattern that can never match is still accepted. Deciding
whether a regular expression matches any string within the declared
lengths is a much larger piece of code than the mistake it would catch.
### How it is tested
- `TestUnparseableSchemaDeclaresNothing`: three non-object schemas;
creates still pass, and the Kubernetes legacy cap still holds.
- `TestNameDeclarationIgnoresAnnotations`: a declaration carrying
`title`, `examples`, `$comment` and an `x-` extension still enforces its
`maxLength`.
- `TestInvalidNameSchemaFailsClosed` gains `format` as an unsupported
constraint keyword.
- `TestClassifyNameDeclaration` gains a declaration turned into a
string, a misspelled keyword and a null declaration, all reported, and
an added annotation, which is not.
- `TestPatchSpecLeavesNameDeclarationOffSpec`: the published `spec`
drops `x-cozystack-name` and keeps an unrelated vendor extension.
### Downstream repositories
No repository is affected beyond the follow-ups already noted on #4367
(the `cozystack/website` pages on annotations and
ApplicationDefinitions). Those are still unfiled, so no box is ticked.
- [ ] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:
### Release note
```release-note
fix(api): An ApplicationDefinition whose openAPISchema cannot be parsed no longer refuses every create of its kind as an invalid name declaration, and annotation keywords (`title`, `examples`, `x-` extensions and the like) inside `x-cozystack-name` are now ignored instead of taking the kind offline. The API review gate now reports a changed name declaration that the API server would refuse, and `x-cozystack-name` no longer appears on the published OpenAPI schema of `spec`.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Application name declarations now support minimum and maximum lengths
and regular-expression patterns. Matching patterns are checked for
compilation; a pattern that matches no names rejects all names.
* **Bug Fixes**
* Invalid or unsupported name constraints now produce configuration
errors and block every create of the affected kind.
* Descriptive annotations and `x-` extensions are ignored when
validating name declarations.
* The custom name declaration is omitted from the published API
specification.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Run go fix over every first-party module: interface{} becomes any,
hand-written loops become slices, maps, min/max and range-over-int,
errors.As becomes errors.AsType, pointer helpers become new(expr) and
are inlined at their call sites, and so on. Every rewrite keeps
behaviour.
Three rewrites stay out. embedlit flattens embedded-field literals
such as ObjectMeta, syntax only Go 1.27 accepts, which would break
backports to release branches still on Go 1.26. omitzero drops
omitempty from struct fields, and openapi-gen reads omitempty as
optional, so metadata, spec and status would turn required in the
published schemas. And errorsastype rewrote an errors.As whose
target, apierrors.APIStatus, does not implement error, which
errors.AsType rejects at compile time.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
## What this PR does
This PR makes `REST.Update` keep live `spec.suspend` of the HelmRelease,
on first write and on conflict retry. Update rebuilds HelmRelease from
Application and writes it back whole, Application has no suspension
field, so every edit through aggregated API resumed a release that
operator or controller had suspended.
Caller that depends on it is cnpg restore driver. It suspends target
release, patches Postgres app through this API, purges Cluster and
resumes only after purge is done, so next chart render lands
`bootstrap.recovery` on empty namespace. When the patch resumes release,
helm-controller upgrades right away, chart can render recovery Cluster
while purge is still running, then driver deletes it as the Cluster
being replaced and nothing renders it again, because values do not
change anymore. RestoreJob stays `Running` until its deadline.
On main it works only because of timing. Same write also drops
`finalizers.fluxcd.io`, and helm-controller v1.5.0 spends its first pass
after that on putting finalizer back and requeue (750ms minimum retry
delay) before it upgrades, this delay usually lets purge finish first.
#3426 keeps controller finalizers across this write, so delay is gone,
and `postgres-2-backup-roundtrip` failed on both its runs there while it
passes on other PRs. In
[35714873620](https://github.com/cozystack/cozystack/actions/runs/35714873620)
stuck restore is `pg-src-to-pg-target-pitr`: `startedAt` 11:40:37, helm
upgrade v3 at 11:40:37, new Cluster created at 11:40:38 and gone by
11:40:47, no render after that.
[35652855170](https://github.com/cozystack/cozystack/actions/runs/35652855170)
has same picture on first restore, all inside 22:10:51.
Tests are in `rest_suspend_test.go`: edit leaves `spec.suspend` as it
was (both values) with values still taken from Application, and suspend
that lands between read and write survives conflict retry. Removing
either line fails its test.
### Downstream repositories
Change is in how aggregated API writes HelmRelease back, no downstream
repo restates that. Rollback guide on website (`guides/concepts.md`)
tells operators to suspend HelmRelease while they work, with this fix
that suspension holds across app edit, so docs need no change.
- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:
### Release note
```release-note
fix(api): app edit through cozystack API no longer resumes a suspended HelmRelease, so postgres restore waits for its purge before cluster is rendered again.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Application updates now preserve a Helm release’s suspended or active
state instead of unintentionally changing it.
* The release state is also preserved when an update encounters a
conflict and retries, preventing a concurrent suspension from being
lost.
* Application updates continue to use the application’s configured
values while retaining the release’s current suspension state.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Three cases refused creates of a kind, or were documented as refused,
without the declaration being at fault.
A schema that is not a JSON object was reported as an invalid
x-cozystack-name, although such a schema has no declaration to blame,
and the spec schema's own loader already reports it. It now declares
nothing, so creates keep working and a kind with a legacy cap keeps it.
Any unknown key inside a declaration took the kind offline, so a
generator adding a title or an example would do the same. Keywords that
constrain nothing, the JSON Schema annotations and x- extensions, are
now ignored. A constraint keyword the server does not implement is still
refused, because accepting it would drop it silently.
The docs said a declaration no name satisfies is refused. That holds
for lengths only: a pattern is checked to compile, not to match
anything. The docs now say so.
The parser moves to pkg/apis/apps/validation so the API review gate can
reuse it rather than keep a copy of its rules.
Assisted-by: LLM
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
## What this PR does
The aggregated API server capped two kinds' names by knowing what their
charts render. A `Kubernetes` cluster name was held to 32 characters
because every worker pool is a `KubernetesNodes` release named
`kubernetes-nodes-<name>-<pool>`, and the default `md0` pool has to fit
the 53-character Helm release name limit. A `Kafka` name was held to 44
because the KRaft controller pod `kafka-<name>-c-<hash>-<id>` has to fit
the 63-character DNS-1123 label limit. The arithmetic is right, but it
is the charts' knowledge living in the platform: the next application
with a sub-resource budget needs another `if r.kindName == ...` branch,
and an application served from outside this repository — the whole point
of `ApplicationDefinition` — cannot get one at all.
An application now declares the limits on its own name in its own
schema, and the server applies whatever it finds there for whatever kind
the definition describes. The chart's `values.yaml` carries a `## @name`
annotation, `cozyvalues-gen` v1.7.0 publishes it at the root of
`values.schema.json` as `x-cozystack-name`, and `hack/update-crd.sh`
carries it into the `ApplicationDefinition`:
```yaml
## @name {string} - Cluster name. Every worker pool is a separate KubernetesNodes release named `kubernetes-nodes-<name>-<pool>`, so the name must leave room for the default `md0` pool inside the 53-character Helm release name limit.
## @maxLength 32
```
```json
"x-cozystack-name": {
"type": "string",
"description": "Cluster name. Every worker pool is a separate KubernetesNodes release named `kubernetes-nodes-<name>-<pool>`, so the name must leave room for the default `md0` pool inside the 53-character Helm release name limit.",
"maxLength": 32
}
```
`maxLength`, `minLength` and `pattern` are honoured; `pattern` is
unanchored, as JSON Schema specifies. A declaration only tightens:
`prefix + name` must still fit the 53-character Helm limit. A rejected
name is answered with the chart's own description, or with "limit
declared by the <Kind> application" when the chart gives none.
The schema is untrusted input — definitions from other repositories are
written by hand — so a declaration that cannot be enforced as written
(not an object, a type error, an unknown keyword such as a misspelled
`maxLenght`, `maxLength` below 1, a negative `minLength`, `minLength`
above `maxLength` or above the Helm budget, a pattern that is not RE2)
refuses every create of that kind with `configuration error: the <Kind>
ApplicationDefinition declares an invalid x-cozystack-name: …`, and logs
the same at start-up. Dropping the declaration would admit names the
chart cannot render; applying it would reject every name and blame the
user. Other kinds are unaffected, and existing objects of the affected
kind can still be read, updated and deleted.
An upgrade brings `cozystack-api` up before `kubernetes-rd` and
`kafka-rd` re-render (`kubernetes-rd` depends on `cozystack-engine`), so
the new server can briefly read a definition that predates its
declaration. For exactly that case — a definition that declares nothing
— the two previous caps stay in `legacyNameCaps` as a floor, to be
removed once no supported upgrade starts from such a definition. That is
the only per-kind knowledge left in the name checks.
Also in this PR:
- The `pre-commit` job pins `cozyvalues-gen` v1.7.0, the first release
that understands `@name`
([cozystack/cozyvalues-gen#33](https://github.com/cozystack/cozyvalues-gen/pull/33)).
- The API review gate now diffs the root `x-cozystack-name` of every
definition as `metadata.name`, so a chart that lowers its cap or adds a
pattern is reported like any other tightened field. On this PR it
reports the Kubernetes and Kafka caps as added; their effective limits
are unchanged.
- The migration 56 comment that pointed at the removed
`maxKubernetesClusterName` now points at the chart's declaration.
Not in scope, because none of it is a static limit a chart can declare:
the `Tenant` name pattern in `pkg/apis/apps/validation`, the computed
tenant-namespace length check (it depends on the parent namespace at
request time), the tenant quota hierarchy, and the Phase 2 removed-field
warning for `Kubernetes`.
### How it is tested
`TestNameCapComesFromTheShippedSchema` reads the shipped `kubernetes`
and `kafka` ApplicationDefinitions off disk, builds storage through
`NewREST` as the server does, and checks the cap and the chart's
explanation; it then rewrites `maxLength` inside the shipped schema and
checks the limit moves with it, which a cap held in code would not.
`TestInvalidNameSchemaFailsClosed` covers each malformed declaration
through the same path, `TestLegacyNameCapsCoverUndeclaredDefinitions`
pins the upgrade floor and that a declaration replaces it, and
`TestClassifyNameDeclaration` covers the gate.
### Downstream repositories
Two repositories are affected and neither follow-up is filed yet, so no
box is ticked.
- `cozystack/website` — `content/en/docs/next/development.md` documents
the `values.yaml` annotations and needs `@name`;
`content/en/docs/next/cozystack-api/application-definitions.md`
describes `openAPISchema` and needs `x-cozystack-name`.
- `cozystack/external-apps-example` — it pins no `cozyvalues-gen`
version and regenerates against whatever is current; nothing in it
declares a name constraint, so it is unaffected until it chooses to.
- [ ] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:
### Release note
```release-note
feat(api): Application name limits now come from the application's own schema instead of being hardcoded per kind in the aggregated API server. A chart declares them with a `## @name` annotation in `values.yaml`, which cozyvalues-gen v1.7.0 publishes as `x-cozystack-name`; the API server enforces `maxLength`, `minLength` and `pattern` on `metadata.name` for any kind, answers a rejected name with the chart's own explanation, and refuses creates of a kind whose declaration it cannot enforce with a configuration error. The Kubernetes (32) and Kafka (44) name caps are unchanged in effect and are now declared by their charts.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Applications can declare minimum and maximum name lengths and
formatting patterns.
* Kubernetes cluster names are limited to 32 characters, and Kafka
cluster names to 44 characters, to meet generated resource naming
limits.
* Names that violate declared constraints are rejected with an
explanatory message. Tighter name constraints are also reported as
breaking changes.
* Declared maximum lengths cannot exceed the platform’s existing Helm
release-name budget.
* **Documentation**
* Added guidance on declaring and enforcing chart-specific resource-name
limits.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Update rebuilds the HelmRelease from the Application and writes it back
whole, and the Application carries no suspension, so every edit made
through the API resumed a release that an operator or a controller had
suspended.
The CNPG restore driver depends on that suspension. It suspends the
target release, patches the Postgres app through this API, purges the
Cluster, and resumes only once the purge is done, so the chart's next
render lands bootstrap.recovery on an empty namespace. Resumed by the
patch instead, helm-controller upgrades straight away: the chart can
render the recovery Cluster while the purge is still running, the
driver then deletes it as the Cluster being replaced, and nothing
renders it again because the values have not changed since. The
RestoreJob sits in Running until its deadline.
On main this has been masked by timing rather than prevented. The same
write also dropped the Flux finalizer, so helm-controller's first pass
after it only put the finalizer back and requeued, which held the
upgrade back by at least its 750ms minimum retry delay. A branch that
keeps the finalizer removes that delay, and the backup round-trip
failed on both of its runs there.
The suspension is re-read on a conflict retry as well, since the write
that caused the conflict may be the suspend itself.
Assisted-by: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The API server capped Kubernetes and Kafka names by knowing what those
two charts render: worker pool releases under a 17-character prefix,
controller pods with a 13-character suffix. That is the charts'
knowledge sitting in the platform, and it does not generalise. The next
application with a sub-resource budget needs another branch on Kind,
and an application served from outside this repository cannot get one
at all.
An application now declares the limits on its own name in its schema,
under x-cozystack-name, and the server applies whatever it finds there
for whatever kind the ApplicationDefinition describes. A declaration
only tightens: prefix + name must still fit the 53-character Helm
release name limit. A rejected name is answered with the chart's own
explanation of the limit.
The schema is untrusted input, and definitions from other repositories
are written by hand. A declaration that cannot be enforced as written,
whether an unknown keyword, a wrong type, a limit no name satisfies or
a pattern Go cannot compile, refuses creates of that kind with a
configuration error. Dropping it would admit names the chart cannot
render; applying it would reject every name and blame the user.
An upgrade brings cozystack-api up before the definitions it serves are
re-rendered, so for a while the new server can read a Kubernetes or
Kafka definition that predates its declaration. The two old caps stay
as a floor for exactly that case, a definition that declares nothing,
until no supported upgrade starts from one.
Assisted-by: LLM
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
The comment next to the timeout overrides named the kinds that set
helm-install-timeout and gave each one's reason. The list had already
fallen behind the ApplicationDefinitions that carry the annotation,
and nothing checks it, so it drifts every time a kind opts in.
Say where the annotations live and when a kind should set them
instead. Which kinds set them is recorded in their
ApplicationDefinitions, and a comment should not restate it.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Backing resource names are not part of the aggregated API contract.
Clients need the requested resource identity and the original status
code and reason to handle rejected writes consistently.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
Assisted-by: LLM
Aggregated endpoints must retain backend HTTP codes and reasons so
clients can distinguish conflicts and admission failures. Dry-run
deletion also depends on current finalizers, which a cached read can miss.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
Assisted-by: LLM
Dry-run must expose the same deletion state as a real request without
persisting it. Shared option objects can hide dropped fields in tests,
and an unreachable patch method offers no API coverage.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
Assisted-by: LLM
A Kafka cluster's KRaft controller pods are named
"<release>-c-<hash>-<id>" (8-hex hash, id at least one digit), and that
derived pod hostname must fit the 63-char DNS-1123 label limit. That is
tighter than the 53-char Helm release-name budget validateNameLength
enforced, so a release name of 51-53 characters was admitted but then
failed the chart render (kafka.assertNameLength) forever, an
un-recoverable upgrade dead-end for an existing cluster whose name is
immutable. Cap the Kafka app name at admission, mirroring the existing
Kubernetes worker-pool cap, so the overflow surfaces on the Kafka CR the
operator is editing instead of at render time on a pod Strimzi can never
create. The chart's render guard remains the second line of defense and
also catches larger clusters whose higher node ids need more digits.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Add release.cozystack.io/helm-server-side-apply on the ApplicationDefinition,
threaded through ReleaseConfig into every emitted HelmRelease's
Install/Upgrade.ServerSideApply. Empty leaves the helm-controller default
(server-side apply on v1.5.0); "false"/"disabled" forces client-side apply.
The postgres read-replica autoscaler needs this: KEDA's HPA writes the CNPG
Cluster's /scale subresource (spec.instances) while the chart renders a constant
seed. Under helm-controller's default server-side apply the rendered field is
force-owned and reverts KEDA's live value on every reconcile. Under client-side
apply helm-controller patches the CRD from the previous-vs-new rendered manifest
without consulting live state, so the unchanged constant yields no patch and the
/scale value survives (verified against Helm v4 createPatch: unstructured/CRD
objects take the two-way jsonpatch.CreateMergePatch(original, modified) path
unless threeWayMergeForUnstructured is set, which helm-controller v1.5.0 leaves
at its default).
Guarded by TestConvertApplicationToHelmRelease_ServerSideApplyOverride on the
tenant-release builder (convertApplicationToHelmRelease), and by
TestParseHelmServerSideApplyAnnotation. Removes a stale package_reconciler_test
guard that asserted a false invariant on the wrong (platform Package) builder.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
TestWarnRemovedKubernetesFields pinned the helper in isolation, so deleting
either call site (rest.go Create/Update) left it green while an operator editing
a Phase 2 removed field would silently get no warning. These drive Create and
Update end to end and assert the warning surfaces; removing a call site now
fails loudly (verified by mutation).
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
The Kubernetes CR still accepts and stores spec.nodeGroups, spec.nodeHealthCheck
and spec.maxNodeProvisionTime after the Phase 2 split (the adoption migration
leaves them on an upgraded CR rather than rewriting parent values mid-upgrade),
but they no longer render anything, so an operator scaling via
spec.nodeGroups.<x>.minReplicas silently got nothing.
Emit a non-blocking client-facing admission warning on Create/Update when any
of these fields is present, pointing the operator at the KubernetesNodes
resources. A hard rejection is intentionally avoided: it would break edits to
already-upgraded clusters that still carry the fields. Also enumerate all the
Phase 2 breaking changes (three removed fields, two KubernetesNodes
immutability rules, the 32-char cluster-name cap) in the chart README.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
A Kubernetes cluster's worker pools are separate KubernetesNodes releases
named "kubernetes-nodes-<cluster>-<pool>". validateNameLength only checked
the parent's own "kubernetes-" prefix budget (42 chars), so a cluster name
of 33-42 chars was admitted while every child pool release overflowed the
53-char Helm limit and could not render -- the migration then pinned and
skipped the pool, leaving the cluster with no way to add workers. Pre-split
nodeGroups had no such constraint, so this was a regression.
Add a kind-specific 32-char cap for the Kubernetes kind so the overflow
fails at admission on the parent CR instead of at render time on an
un-creatable child.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Add first-class fields so a package/app can declare the Flux helm-controller
v1.5.0 kstatus health gate for the custom resource it wraps, so
HelmRelease.Ready means "the CR is actually healthy" instead of "helm applied
the CR" (#2642). API-owner approved in #3263.
- api: WaitStrategy (scalar, maps to HelmReleaseSpec.WaitStrategy.Name — a
deliberate simplification) + HealthCheckExprs ([]kustomize.CustomHealthCheck,
the upstream type verbatim) on both ApplicationDefinition.spec.release (the
apiserver path) and ComponentInstall (the operator path).
- config.ResolveWaitStrategy: shared by both HelmRelease builders. Couples the
default — healthCheckExprs imply the poller strategy when none is set, since
exprs are only evaluated under poller. This removes the "must also set
waitStrategy: poller" footgun and does not depend on the controller's global
default.
- thread through buildHelmReleaseSpec (operator) and
convertApplicationToHelmRelease (apiserver); the apiserver loads the typed
spec.release fields in start.go.
- regenerate deepcopy, the ApplicationDefinition and PackageSource CRDs, and
openapi. The backupstrategy CRDs also regenerate — benign k8s-0.35 PodSpec
description drift, not a functional change.
disableWait is intentionally not added (YAGNI): the only known need (the
kubernetes tenant-addon deadlock) was resolved child-side, and waitStrategy:
legacy is not equivalent (legacy still waits Helm-v3-style; disableWait skips
waiting entirely). Add it later when a concrete leaf needs a true no-gate
escape hatch.
Refs #2642
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
parentPoolUsage listed ResourceQuotas through the aggregated apiserver's cached
client, which lazily starts a cluster-wide ResourceQuota informer. cozystack-api
has no RBAC to list resourcequotas, so the informer never synced and tenant
create/update of a bounded child hung until the request timed out (observed on a
cluster). Read quota usage through the direct (uncached) watch client instead,
scoped to the pool's member namespaces, so no cluster-wide informer is started,
and grant cozystack-api get/watch/list on resourcequotas for those reads.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com>
The declaration-time gate only subtracted sibling carve-outs from a parent's
budget, not the parent pool's current usage. A tenant already consuming most of
its budget could therefore carve a full-budget child, letting the sub-tree
collectively exceed the root's allocation.
Sum the current usage of the parent pool (its own namespace plus its unbounded
descendants, read from each member's rendered ResourceQuota.status.used) and
subtract it from the remaining budget, mapping each shorthand quota key to its
rendered limit key (renderedLimitKey). Boundedness is decided by declared
resourceQuotas so the result is independent of whether Flux has rendered a
freshly-created tenant's quota yet. This is best-effort at declaration time (a
snapshot; concurrent writes can briefly overshoot and running pods are never
evicted) — the tenant-quota controller remains the runtime backstop.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com>
Add a declaration-time gate in the aggregated apiserver: a child tenant's
declared resourceQuotas are carved out of its parent tenant's remaining
quota and may not exceed it. A child without a quota shares the parent
pool and is allowed; a parent without a quota does not constrain children.
This blocks quota escalation via sub-tenant creation (declaring a bigger
or unbounded child) without re-implementing pod-level admission.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
The talos-reconcile Job is the only thing that creates the
TalosConfigTemplate the worker MachineDeployment's bootstrap.configRef
points to; CAPI blocks MachineSet creation until it exists, so tenant
workers cannot scale until the Job has run.
The Job was a post-install/post-upgrade Helm hook, which runs only after
Helm has applied the release's main resources and, when Helm waits, after
they become Ready. But the same release emits in-tenant addon HelmReleases
— notably cilium — whose readiness needs worker nodes, and those nodes
need the TalosConfigTemplate this Job produces. So when the main-resource
wait is in effect the install deadlocks: it waits for cilium to be Ready,
cilium waits for workers, the workers wait for the template, and the
template waits for this hook — which never runs because the install never
finishes the wait that precedes the hook phase. The install times out and
the MachineDeployment never scales. The release sets
helm-install-disable-wait to skip that wait, but its effect is unreliable,
which makes the failure bimodal: the template lands in ~46s when the wait
is skipped, and the install is stranded when it is not.
Render the Job and its RBAC/ServiceAccount/egress policy as ordinary
main-phase resources. The Job is then created during the apply itself, not
after the wait, and runs on the management cluster where nodes exist
(unlike the tenant cilium-operator), so it produces the template, the
MachineDeployment scales, workers join, cilium goes Ready, and the wait
(if any) closes within the install budget — independent of whether
helm-install-disable-wait takes effect.
The Job is immutable, so it carries a content-hash name suffix over its
rendered spec (mirrors the KubevirtMachineTemplate idiom in cluster.yaml):
an identical render keeps the name and Helm no-ops it, while any spec
change yields a new name and a fresh Job, so no spec change ever lands on
the same name and triggers an immutable-field patch error. The applied
TalosConfigTemplate keeps its stable <release>-<group> name, so an
unchanged render re-applies a byte-identical template — a CAPI no-op that
never rolls existing workers.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Parse a new release.cozystack.io/helm-install-disable-wait annotation in start.go and apply it as Install/Upgrade.DisableWait in rest.go, so helm-controller no longer blocks post-install hooks on in-tenant addon readiness. This breaks the reconcile-hook chicken-and-egg where the hook that creates the worker TalosConfigTemplate was gated behind addons that need workers to schedule. Covered by a config table test.
Co-authored-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
## What this PR does
Implements the tenant Flux sharding design from #2817: a new
`flux-shard-operator` system package that spreads tenant HelmReleases
across multiple helm-controller shards, so a noisy tenant (e.g. a
HelmRelease stuck in infinite remediation) cannot degrade the others.
- **Placement controller** owns the tenant→shard assignment: the unit of
placement is the tenant (all of its HelmReleases share one shard label),
distribution is greedy least-loaded weighted by HelmRelease count, with
pinned tenants, threshold-gated rebalance, paced moves, and never moving
deleting tenants. Watches are metadata-only, so the controller does not
decode the helm-controller status-patch firehose. Assignments are
recorded on tenant namespaces; HelmRelease labels remain the source of
truth on restarts.
- **Shard runtime** is provisioned by the operator: `shardCount`
helm-controller Deployments cloned from the flux-aio `flux` Deployment
and sanitised, so the image and feature-gates stay version-synced
automatically. flux-aio itself is unchanged and keeps owning everything
without a shard key.
- **CREATE-only mutating webhook** (`failurePolicy: Ignore`) stamps the
tenant's shard onto every HelmRelease at admission, so each one is born
on the correct shard regardless of creation path; any miss degrades
gracefully to the placement controller.
- The hand-rolled `flux-tenants` Deployment is retired: the operator
drains it (backfill `tenants` → `shard<i>`) and deletes it once empty;
migration 44 removes a leftover only under the same guard.
Ships with `shardCount: auto` (the default): the operator sizes the
shard count from the tenant HelmRelease count — `K = clamp(ceil(H/100),
1, min(16, T))` with hysteresis (scale up past 120 HR/shard, down under
60) — so small clusters stay at one shard (≈ today's behaviour) and
large fleets shard out automatically. An integer value pins the count
explicitly.
One deviation from the design doc: the webhook/controller scope is the
`sharding.fluxcd.io/key` label rather than
`internal.cozystack.io/tenantmodule` — every tenant HelmRelease is born
with `key=tenants` (stamped by ApplicationDefinition `release.labels`
and chart templates), so the webhook rewrites it and the placement
controller continuously relabels stragglers; `tenantmodule` covers only
the 6 tenant chart modules.
### Screenshots
Not a UI change.
### Release note
```release-note
feat(fluxcd): tenant HelmReleases are now reconciled by operator-managed helm-controller shards; the flux-shard-operator balances tenants across `shardCount` instances and retires the legacy flux-tenants deployment
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Introduced **flux-shard-operator** to shard helm-controller workloads
with deterministic tenant placement, optional rebalance controls, and
autosized shard count.
* Added a **CREATE-only mutating webhook** to stamp the correct shard
label on tenant HelmReleases.
* **Bug Fixes**
* Application updates now preserve existing **flux-shard** label
assignments to prevent unwanted shard re-stamping.
* **Chores**
* Added platform packaging for the new component, including an image
build step.
* Removed the legacy tenants deployment and added a migration to retire
it safely.
* **Tests**
* Expanded unit and chart tests for placement, provisioning, webhook
behavior, and configuration parsing.
* **Documentation**
* Added operator documentation describing configuration and metrics.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Resolve go.mod conflict: keep main's go 1.26.4 and golang.org/x/* bumps, and keep gomodules.xyz/jsonpatch/v2 v2.4.0 as a direct dependency (imported by the flux-shard-operator admission webhook) instead of main's indirect entry. go mod tidy leaves go.sum consistent.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
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>
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>
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 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 Update handler rebuilds the HelmRelease from the Application, which
re-stamped sharding.fluxcd.io/key with the ApplicationDefinition default
and bounced the HelmRelease off its operator-assigned shard on every
update. Carry the live label over from the current HelmRelease instead.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
frolov.so