mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-08 21:14:56 +09:00
2fa7fea29b8b9562a2c9e42d2e53ca9fa72fb01d
242
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Pujitha Paladugu
|
e11e239044 |
fix(api): pass the stored object to UpdatedObject in tenantsecret Update
Motivation: a PATCH (kubectl patch, label, edit) against an existing
TenantSecret answered 404 NotFound while GET on the same name succeeded.
REST.Update called objInfo.UpdatedObject(ctx, nil). The apiserver patch
handler builds the patched object from that old object alone, so with nil it
takes its create-new-object branch and returns NotFound from inside the
handler. The same nil also made the patch reach admission as CREATE rather
than UPDATE.
Approach: hand UpdatedObject the stored Secret converted with
secretToTenant when it exists, and nil only when it does not (the
forceCreate path, unchanged), matching what the generic store and the
sibling application and securitygroup registries do.
Validation: added TestUpdate_PassesStoredObjectToUpdatedObjectInfo, which
drives Update with a patch-style UpdatedObjectInfo that fails on a nil old
object. It fails before the change ("UpdatedObject received a nil old
object") and passes after. Ran `go test ./pkg/registry/...` and
`go build ./pkg/... ./cmd/...`, all passing. Not run: a live cluster
kubectl patch, or the real apiserver patch handler.
Known limitation, unchanged by this fix: tenantToSecret prefers Data over
StringData when Data is non-empty, so a patch that only sets stringData on a
secret that already has data is a no-op, exactly as a GET-then-PUT edit of
stringData already is. Patching data or metadata works. That precedence is a
separate design question and is left alone here.
Report: https://github.com/cozystack/cozystack/issues/3601
Assisted-by: LLM
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
|
||
|
Aleksei Sviridkin
|
8ed34166cf |
perf(lineage): cache owner GETs and short-circuit re-admission (#2667)
## What this PR does The lineage mutating webhook walks the owner chain (Pod, StatefulSet, HelmRelease, application CR) on every admission, with one uncached dynamic GET per level, and sibling Pods of the same parent repeat the same lookups. On a multi-tenant cluster this put `lineage.cozystack.io` at 300-500ms p99 in `apiserver_admission_webhook_admission_duration_seconds`, the largest share of admission latency. Owners are now cached in-process for 60 seconds, and only successful lookups are stored. A failed lookup (RESTMapping, NotFound, timeout) leaves the walk without an owner, so the object is admitted with no lineage labels and stays unlabeled until its next UPDATE runs the walk again. Secrets, Services and PVCs can go a long time between updates, so a cached failure would leave every object admitted within the TTL unlabeled for that long. The aggregated cozystack-api makes such failures routine, since discovery of `apps.cozystack.io` fails while it rolls. The one staleness left is the scheduling class, which is read from the application's spec: a change to it reaches newly admitted Pods up to 60 seconds later. The cache is not swept below 4096 entries, so up to that many owner objects stay in memory for the life of the process. Above that, each insert drops the expired entries among a random sample of 64, which keeps the write lock short. Non-Pod objects that already carry every lineage label, `internal.cozystack.io/tenantresource` included, skip the walk, because the objectSelector still matches an UPDATE whose old object lacks the label. Pods go through regardless, as each Pod admission also sets the scheduling class. ### Screenshots Not a UI change. ### Downstream repositories The diff is Go under `pkg/lineage` and `internal/lineagecontrollerwebhook` only: no label, annotation, CRD, values key, `hack/` file or telemetry metric is added or renamed, so no entry of the trigger map applies. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note perf(lineage): the lineage mutating webhook caches owner lookups for 60 seconds and skips the owner walk for non-Pod objects that already carry lineage labels, cutting admission latency on clusters with many tenants. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Lineage and owner lookups now use a time-limited in-memory cache, reducing repeated API requests during admission processing. Scheduling-class updates may take up to 60 seconds to appear on new Pods. * Non-Pod objects with complete lineage labels are allowed without additional owner lookup or mutation. * **Tests** * Added coverage for cache hits, misses, expiration, and retrying lookups after errors. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Mattia Eleuteri
|
ca24d8f5f6 |
perf(lineage): cache owner GETs and short-circuit re-admission
Every admission walks the owner chain (Pod, StatefulSet, HelmRelease, application CR) with one uncached GET per level, and sibling Pods repeat the same lookups. On a cluster with many tenants that put the lineage webhook at 300-500ms p99, the largest share of apiserver admission latency. Owners are now reused for 60 seconds, successful lookups only. A failed lookup, whether RESTMapping, NotFound or a timeout, leaves the walk without an owner, so the object is admitted with no lineage labels and stays unlabeled until its next UPDATE runs the walk again. Secrets, Services and PVCs can go a long time between updates, so a cached failure would leave every object admitted within the TTL unlabeled for that long, and the aggregated cozystack-api makes such failures routine: discovery of apps.cozystack.io fails while it rolls. Non-Pod objects that already carry every lineage label, tenantresource included, skip the walk, since the objectSelector still matches an UPDATE whose old object lacks the label. Pods go through it regardless, as each Pod admission also sets the scheduling class. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
31948c8838 |
fix(api): redo the HelmRelease metadata carry-over on a conflict retry
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> |
||
|
mattia-eleuteri
|
6d541c63eb |
fix(api): make mergeMaps always allocate, never return an input map
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> |
||
|
mattia-eleuteri
|
a18cbc581c |
fix(api): preserve foreign HelmRelease metadata on Application update
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> |
||
|
Aleksei Sviridkin
|
2c2c0301f7 |
chore(deps): roll up pending Renovate updates and move to Go 1.27.1 (#4447)
## 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 --> |
||
|
scooby87
|
03e2da5805 |
feat(postgres,mariadb)!: drop plaintext user passwords (#4078)
## What this PR does Removes the plaintext `users[].password` field from the `apps/postgres` and `apps/mariadb` charts (and the mariadb `root` account), so managed-database passwords can no longer be pinned in tenant values or committed to git. Passwords are always chart-generated into the `<release>-credentials` Secret and preserved across reconciles via `lookup`, matching the model `redis`/`valkey` already use. This closes the "User passwords" item of the portal upstream epic (aenix-org/cozyportal#1060). The generated password never lands in the init-script Secret or the SQL it runs: the postgres init-job reads each user's password from the mounted `<release>-credentials` Secret at run time and applies it through a psql variable (`ALTER ROLE … WITH PASSWORD :'pw'`), so it does not linger in the init-script manifest a tenant can read. It does live in cleartext `stringData` in the `<release>-credentials` Secret itself, which is a chart manifest and therefore kept in Helm release history for `MaxHistory` revisions like any chart-generated secret — that residue is exactly why rotation cannot be built on the chart (see below). A `password` or a `passwordRotation` key left over in a tenant's values is accepted-but-inert and draws an admission warning, so an operator editing it is told it has no effect rather than getting a silent 200. Backup/restore into a copy keeps the target's chart-managed credentials working: the CNPG driver clears `bootstrap.enabled` once recovery is healthy so the init-job re-applies the generated passwords onto the recovered roles, and the MariaDB driver excludes `mysql.global_priv` from the dump (`ignoreGlobalPriv`) for backups captured from this release onward, so restoring one of those into a copy does not clobber the target's chart-managed users or `root`. Excluding the grant table has a flip side, and it applies to every MariaDB restore, not only a to-copy one: only accounts declared through `User` CRs (the chart's `users` map) are captured, so an account created out of band directly in MySQL is in no backup and a restore brings back its data and `mysql.db` grants but not the account itself. This is also a backup-side fix only: a restore from a backup taken **before** this change still replays the source's `mysql.global_priv` and overwrites the target's users and `root`, and `root` then needs an out-of-band reset (the operator applies `rootPasswordSecretKeyRef` only at datadir bootstrap, so there is no in-band path). See `docs/operations/backup-classes.md` for the convergence window, the out-of-band-account limitation, and this old-backup caveat. Password **rotation** is deliberately **not** in this PR. A chart-rendered credentials Secret keeps its cleartext in Helm release history for `MaxHistory` revisions, so a rotation built on it cannot revoke a leaked password; rotation is being reworked as a controller that owns the Secret — design proposal in cozystack/community#72. Migration is seamless: existing releases keep their current passwords on upgrade (the value already lives in `<release>-credentials`, which `lookup` re-reads and preserves); a `password` still present in a tenant's values is ignored, so no existing HelmRelease fails validation. Verification: `helm unittest` passes for both charts (postgres 87, mariadb 126, including render-time username guards that reject any name k8s would later reject as an invalid Secret key or User resource name); `go build ./...`, `go vet`, the backup-controller tests and the apps-apiserver tests are green; `make generate` was run with the CI-pinned `cozyvalues-gen` v1.6.0 so `values.schema.json`, the READMEs, the API types, and the `cozyrds` ApplicationDefinitions are all in sync (the branch was rebased onto `main`, which reconciled cleanly with the read-replica autoscaling feature that landed there). ### Screenshots N/A — no UI change. ### Downstream repositories - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: affected — `content/en/docs/next/cozystack-api/go-types.md` is hand-written and still shows `Password:` in the Go examples, and the managed-app reference pages regenerate from these chart READMEs, so both document `users[name].password` until refreshed (not yet filed). - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: affected — the provider hand-maintains the postgres/mariadb schemas and per-field defaults; this PR removes `users[].password`. A follow-up issue/PR is needed there (not yet filed). `terraform-provider-cozystack` and `website` are reached by this diff; both boxes are left unticked because the follow-ups are not yet filed. Same-repo future work this change suggests: the controller-owned password rotation (design in cozystack/community#72), and extending the "passwords only in the Secret" model to the other engines that still accept plaintext (`clickhouse`, `mongodb`, `nats`, `opensearch`, `rabbitmq`, `vpn`). ### Release note ```release-note feat(postgres,mariadb)!: passwords for managed PostgreSQL and MariaDB users can no longer be set via `users[].password` — they are always auto-generated and stored only in the `<release>-credentials` Secret, never rendered into the init-script or the SQL it runs. Existing releases keep their current passwords on upgrade; a `password` (or a `passwordRotation`) left in values is ignored with an admission warning. Password rotation is deferred to a follow-up controller design (cozystack/community#72). MariaDB backups now exclude the `mysql.global_priv` grant table (`ignoreGlobalPriv`), so a restore recreates only accounts declared through the chart's `users` map — an account created out of band directly in MySQL is not captured and must be recreated after a restore. ``` |
||
|
IvanHunters
|
0b7dd23b99 |
fix(appdef): let one ApplicationDefinition own each application kind
Nothing enforced that spec.application.kind is unique, and each consumer that maps a kind back to a definition resolved a duplicate its own way: the chartRef reconciler let every definition rewrite every release of its kind, the lineage webhook and the CA projection reconciler took whichever came first in an unordered list, and cozystack-api registered one resource per definition. A second definition declaring a kind already in use could therefore repoint that kind's tenant releases at its own artifact. Add one ownership rule, the definition created first owns the kind with ties broken by name, and route every consumer through it. A definition that arrives later with a kind already in use no longer manages that kind's releases whatever it is named: the chartRef reconciler skips it with a Warning event and re-reconciles the next owner when the current one goes, cozystack-api registers the owner only and logs the rest, its restart hash moves when ownership moves, and the lineage and CA projection lookups resolve the owner. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Alexey Artamonov
|
9afdc49894 |
feat(apps): warn when a removed field is still set on a database app
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> |
||
|
Timofei Larkin
|
ce3eb30e03 |
fix(api): refuse only what a name declaration actually breaks (#4429)
## 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 --> |
||
|
Aleksei Sviridkin
|
7a0aab2bdc |
refactor: apply the Go 1.27 modernizers
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>
|
||
|
Aleksei Sviridkin
|
15035a915b |
fix(vm-instance): disable helm readiness wait on the VMInstance release (#3735)
## What this PR does A VMInstance is served as a Flux HelmRelease that renders a KubeVirt VirtualMachine. Since v1.5.0 (helm-controller v1.4.3 to v1.5.0) that release is health-assessed by the kstatus poller. vm-instance sets no `healthCheckExprs`, so the VirtualMachine goes through kstatus's own rules in `fluxcd/cli-utils` (`pkg/kstatus/status`): `checkGeneration` returns InProgress for any object whose `status.observedGeneration` differs from `metadata.generation`, and past that check a `Ready=False` condition is InProgress too, so a halted VM is never Current even with its generation in sync. KubeVirt keeps a VirtualMachine's `observedGeneration` behind its `generation` whenever a `spec.template` change is pending a restart, the VM is stopped, or a live-updatable change never converges (kubevirt/kubevirt#18700). So a healthy, Running VM is read as permanently InProgress, the upgrade waits its full timeout, and fails with `timeout waiting for: [VirtualMachine/... status: 'InProgress']`. On releases still carrying the pre-1.5.0 `remediation.retries: -1` + rollback spec, the timeout then reverts the previous manifest, dropping just-applied changes such as opened ports. This sets `release.cozystack.io/helm-install-disable-wait: "true"` on the vm-instance ApplicationDefinition, so cozystack-api generates the HelmRelease with Install/Upgrade DisableWait. The release settles once the VirtualMachine is applied, and VM health is read off the VirtualMachine itself (`status.printableStatus` / `Ready`) and off the WorkloadMonitor the chart already renders, which is what `WorkloadsReady` on the Application reports. This restores the pre-1.5.0 behaviour, under which the VM was never a helm readiness gate. The same mechanism is already used by `kubernetes-rd` and `kubernetes-nodes-rd`. On its own that annotation does not reach a running cluster, so two gaps in cozystack-controller are closed with it. The first is the rollout. `applicationdefinition-controller` hashed each definition's name and spec only, so a change confined to a `release.cozystack.io/*` annotation left the hash unmoved and cozystack-api kept serving the configuration it read at its last start-up, which is the only place those annotations are ever read. The hash now covers them too. Only that prefix, because kubectl and Helm rewrite annotations of their own on every apply and hashing those would roll the api Deployment on every reconcile. The second is the releases that already exist. `applicationdefinition-helm-reconciler` brings chartRef, valuesFrom and labels on existing HelmReleases in line with their definition; it now does the same for the readiness wait. Without it the annotation reaches new instances only, which on an affected cluster is the wrong half: #3734 reports 134 of 136 releases already looping upgrade and timeout, and re-applying every VMInstance by hand was the only way out. The setting is driven in both directions, since cozystack-api rebuilds the whole spec on every write and emits `DisableWait=false` once the annotation is gone. An absent install or upgrade block already behaves as `DisableWait=false` and is materialised only when the definition asks to disable the wait, so a release with nothing to change is not rewritten. Two alternatives were considered. A per-kind `healthCheckExprs` keyed on `status.printableStatus` cannot work: setting it moves the VirtualMachine from kstatus to the CEL status evaluator (`fluxcd/pkg/runtime/cel`), which runs the same `observedGeneration` check ahead of the expressions, and any `healthCheckExprs` added to this kind later is a no-op while the wait is disabled. The typed `spec.release.waitStrategy: legacy` is the narrower option, since the legacy checker never waits on a custom resource, but it still gates the release on the chart's Service, and with `external: true` that is a LoadBalancer the legacy checker holds not ready until it has ingress. So an external VM would still wait on IP allocation, which `WorkloadsReady` already watches. Disable-wait avoids that and is the mechanism this repository already uses for the same class of problem. Tests: a helm-unittest suite (`tests/readiness_gate_test.yaml`) guards that the annotation stays on the rendered ApplicationDefinition, and Go unit tests cover both cozystack-controller paths: the hash moves on a release annotation and stands still on an unrelated one, and the wait lands on an existing release, is withdrawn when the annotation goes, does not materialise empty install/upgrade blocks, does not rewrite a release already in sync, leaves a release alone when the annotation does not parse, and does not reach a release of another kind. Note on existing clusters: no manual step. The definition changes on upgrade, the config-hash rolls cozystack-api, and the helm reconciler brings existing VMInstance releases in line. Releases that predate v1.5.0 also carry a `remediation.retries: -1` + rollback spec, which is left alone: with the wait disabled the upgrade no longer times out, so that remediation no longer fires, and any write through the apps API drops it. Closes #3734. ### Downstream repositories - [x] No downstream repository is affected by this change <!-- Walked the trigger map against the diff. The change is a readiness annotation on an ApplicationDefinition plus two cozystack-controller paths. It does not touch openAPISchema, kind, plural, release.prefix, any app package, any values.schema.json or any installer/platform values key. The website's application-definitions.md describes the config-hash as a checksum "over the sorted definitions", which stays accurate now that the hash also covers the release.cozystack.io/* annotations, and no page documents those annotations today. --> ### Release note ```release-note fix(vm-instance): VMInstance HelmReleases no longer hang InProgress or roll back healthy VMs; the release no longer waits on VirtualMachine readiness, which helm-controller's kstatus poller cannot assess correctly for KubeVirt VMs. Existing VMInstance releases are brought in line automatically on upgrade. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * VM installations no longer wait indefinitely when Kubernetes reports the VM’s observed generation behind its current generation. * Changes to release settings are now detected and applied to generated Helm releases, including install and upgrade wait behavior. * **Tests** * Added coverage for VM readiness and release-setting reconciliation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
myasnikovdaniil
|
e9868b4f62 |
fix(api): keep a suspended HelmRelease suspended across an app update (#4411)
## 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 --> |
||
|
Timofei Larkin
|
a461dc4b38 |
fix(api): keep the name declaration off the published spec schema
The whole openAPISchema is published as the schema of spec, so x-cozystack-name showed up as a vendor extension of spec in the aggregated OpenAPI. That describes something spec does not contain. Assisted-by: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
180030be72 |
fix(api): refuse only what a name declaration actually breaks
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> |
||
|
Timofei Larkin
|
146e06e75e |
feat(api): take Application name limits from the application's own schema (#4367)
## 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 -->
|
||
|
IvanHunters
|
70ea242e1b |
docs(vm-instance-rd): trim the disable-wait comment and correct the mechanism
The annotation comment repeated the PR description and the test header; two lines on why are enough. The hang comes from the kstatus generation check in cli-utils, not the CEL status evaluator, since vm-instance sets no healthCheckExprs. The config-hash rollout runs in cozystack-controller, not cozystack-operator. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
1ed32aec58 |
fix(controller): roll cozystack-api when a release annotation changes
The ApplicationDefinition config hash was built from name and spec alone, so a definition whose whole change is a release.cozystack.io/* annotation left the hash untouched and cozystack-api kept running with the configuration it read at its last start-up. Those annotations are only ever read there, once, in buildResourceFromCRD, which makes the config-hash rollout the single mechanism that delivers a change in one: without it the annotation is inert until the api pod restarts for an unrelated reason, and an inert annotation is indistinguishable from one that was never delivered. Hash the release.cozystack.io/* annotations alongside the spec. Only that prefix, because kubectl and Helm rewrite annotations of their own on every apply and hashing those would roll the api Deployment on every reconcile. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Myasnikov Daniil
|
5df219b37a |
fix(api): keep a suspended HelmRelease suspended across an app update
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> |
||
|
Timofei Larkin
|
a3911811c7 |
feat(api): take Application name limits from the application's schema
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> |
||
|
Aleksei Sviridkin
|
36d5c31eaa |
docs(api): stop listing helm-install-timeout carriers in rest.go
The comment next to the timeout overrides named the kinds that set helm-install-timeout and gave each one's reason. The list had already fallen behind the ApplicationDefinitions that carry the annotation, and nothing checks it, so it drifts every time a kind opts in. Say where the annotations live and when a kind should set them instead. Which kinds set them is recorded in their ApplicationDefinitions, and a comment should not restate it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
scooby87
|
7d07bbe3ce |
[kafka] Migrate from ZooKeeper to KRaft mode (#2352)
## Summary - Replace ZooKeeper-based Kafka deployments with KRaft (Kafka Raft) metadata mode - Existing ZooKeeper instances migrate automatically and in place via a Helm `pre-install,pre-upgrade` hook Job that drives Strimzi's ZK-to-KRaft migration state machine - New instances deploy directly in KRaft mode with broker and controller `KafkaNodePool`s - The node pools are owned by the migration hook, not by regular chart manifests (see "Why the pools are hook-managed") - Kafka version is user-configurable (v3.9 default, v3.8 available) - Strimzi operator is unchanged in this PR (stays at 0.45.1-rc1); 0.45.x is the last line that supports ZooKeeper→KRaft migration - ZooKeeper removed from the dashboard UI (ApplicationDefinition) ## Migration strategy Both fresh installs and ZooKeeper→KRaft migrations are driven by a single Helm hook Job (`templates/migration-hook.yaml`, `pre-install,pre-upgrade`) that owns the `KafkaNodePool`s. On every install and upgrade the Job: 1. Ensures both node pools exist (idempotent): a broker pool named exactly `kafka` for a migrating cluster — so Strimzi adopts the existing `<cluster>-kafka-N` brokers and their PVCs in place — or `b-<hash>` for a fresh cluster, plus a controller pool `c-<hash>`. The controller pool pins `strimzi.io/next-node-ids` above the broker range so the broker pool keeps node IDs `[0..N-1]` and Strimzi cannot hand the existing broker IDs to the fresh controllers. The shared `kafka` name is claimed atomically (via `kubectl create`) so only one migration per namespace can win it. 2. Inspects the Kafka CR: no CR → fresh install (pools ensured; the main apply then creates the KRaft Kafka CR, nothing to migrate); already `strimzi.io/kraft=enabled` or `status.kafkaMetadataState: KRaft` → done; any other read error → fail closed. Otherwise (a ZooKeeper cluster) it migrates. 3. Annotates `strimzi.io/node-pools=enabled` then `strimzi.io/kraft=migration`, polls `status.kafkaMetadataState` through `ZooKeeper → KRaftMigration → KRaftDualWriting → KRaftPostMigration → PreKRaft → KRaft`, and only once a safe state is reached flips `strimzi.io/kraft=enabled`. If it times out in an intermediate state it aborts without finalising, so Helm fails the upgrade and the next reconcile retries from where it left off. Data is migrated in place: brokers keep their node IDs and reuse their existing PVCs, nothing is copied. Migration is one-way — Strimzi has no supported KRaft→ZooKeeper rollback once complete, so lowering the `version` field or rolling the app back to a pre-KRaft chart is unsupported (take a backup first). ### Why the pools are hook-managed The hook must create the pools and walk the migration state machine *before* the main release apply. A resource a hook creates cannot also be a regular chart manifest across the ZK→KRaft boundary: Helm's main apply finds the pool live in the cluster but absent from the previous (ZooKeeper) release revision and aborts with `original object KafkaNodePool ... not found`, leaving the release `failed` even though the cluster migrated fine (reproduced under server-side apply, the helm-controller default). Managing the pools only from the hook avoids that, so `templates/kafkanodepools.yaml` and the deployed-version gate ConfigMap (`templates/version.yaml`) are removed and the pools are applied idempotently by the Job on every install and upgrade. ## Bug fixes included - ZooKeeper `metricsConfig` referenced wrong ConfigMap key (`kafka-metrics-config.yml` instead of `zookeeper-metrics-config.yml`) - ZooKeeper `storageClass` used `kafka.storageClass` instead of `zookeeper.storageClass` - WorkloadMonitor used `.Values.replicas` instead of `.Values.kafka.replicas` - `validateNameLength` now caps a Kafka app name at the KRaft controller pod-hostname budget (`<release>-c-<hash>-<id>` must fit 63 chars), instead of admitting names that later fail the chart render forever ## What's next A follow-up PR will upgrade Strimzi to 0.51.0 and Kafka to 4.2 once all instances are confirmed migrated to KRaft. That PR is already prepared on branch `KRaft-upgrade-strimzi-051`. ## Test plan - [x] `helm template` renders correctly for new KRaft instances (no ZooKeeper refs) - [x] `helm template` with custom values (1 replica, external, topics, v3.8) - [x] Version validation rejects unsupported versions via JSON schema - [x] helm-unittest: 86/86 across 9 suites - [x] `hack/kafka-migration-hook.bats`: 12/12 — the real hook script executed against a stub kubectl (resolution branches, fail-closed reads, atomic collision guard, finalize guard) - [x] Operator template renders with the unchanged Strimzi 0.45.1-rc1 - [x] Live migration on a disposable cluster under server-side apply: fresh install → healthy KRaft, release `deployed`; in-place migration of a 20000-message ZooKeeper cluster → release `deployed`, broker PVCs adopted in place (same UIDs), and the KRaft data byte-for-byte identical to the pre-migration ZooKeeper data (matching sha256 over every `partition:offset:key:value`) ### Release note ```release-note [kafka] Enable KRaft metadata mode with automatic, in-place ZooKeeper-to-KRaft migration. Kafka version is now configurable (v3.9, v3.8). The Strimzi operator is unchanged (0.45.1-rc1). ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added chart-level Kafka version selector (default v3.9) and KRaft-aware node pool rendering with controller storage sizing. * **Updates** * Default Kafka appVersion updated to 3.9.0; operator and image tag defaults aligned to 0.45.0. * Templates now omit ZooKeeper resources and metrics when KRaft is in use. * **Tests** * New Helm unit tests for Kafka CR, node pools, metrics, topics, dashboard mappings, and workload monitors. * **Documentation** * Values, schema, and README updated to document new version and controller storage options. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
aa20495df3 |
feat(marketplace): register a tapped repository's apps on connect
Connecting a marketplace repository materialized its PackageSource but created no Package, so its ApplicationDefinitions never rendered and its apps never appeared in the dashboard catalog until a manual `cozypkg add` the dashboard cannot drive (the package list is computed from the ApplicationDefinitions, which do not exist yet). The tap materializer now creates a tap-managed registration Package for each materialized PackageSource, the way the platform ships a Package per built-in app, so the ApplicationDefinition registrations deploy on connect and the apps become browsable; the app components carry no install block and are still instantiated per user resource. Privileged install components of an unconfirmed tap auto-registration (a Package that still carries the marketplace-tap label) are refused on several best-effort layers: the Package reconciler skips their HelmRelease and prunes a lingering one at the install site, and the PackageSource reconciler withholds their ExternalArtifact so the helm-controller has no privileged content to deploy, re-running on confirmation via a Package watch. `cozypkg add --allow-privileged` sheds the label as an explicit ownership handover, which is the confirmation; a platform Package never carries the label. A benign component flipped to privileged in a new revision can still be materialized by source-watcher before these layers observe the flip; closing that race fully requires decoupling the catalog registration from install and is tracked in issue #4359. `cozypkg untap` and the dashboard disconnect remove the registration Package (de-registering the apps) under a UID + ResourceVersion precondition, so a concurrent `cozypkg add` handover is never clobbered; running instances are left but unmanaged. `cozypkg add` re-opens a tap auto-registration for variant selection and hands it over. The cozystack-api ClusterRole gains delete on packages for the disconnect path. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Alexey Artamonov
|
8ef230f0bf |
Merge remote-tracking branch 'origin/main' into kafka-multi-instance
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> # Conflicts: # docs/storage-immutability.md # packages/apps/kafka/tests/topics_test.yaml # packages/system/kafka-rd/cozyrds/kafka.yaml |
||
|
Yan Bondarenko
|
1b576bc638 |
fix(registry): drop the backing object's UID from remapped error details
WrapPreservingStatus rewrites Group, Kind and Name of a backend status to the aggregated resource but left Details.UID as the HelmRelease's, so a client that keys on details saw an identity the tenant cannot look up. The UID goes with the rest of the remap, and a test pins it. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> Assisted-by: LLM |
||
|
Yan Bondarenko
|
29351bd7fa |
fix(registry): report aggregated resource identity in errors
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 |
||
|
Yan Bondarenko
|
39da2300d2 |
fix(registry): keep backend status codes on aggregated write errors
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 |
||
|
Yan Bondarenko
|
a20c706940 |
fix(registry): align dry-run with storage semantics
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 |
||
|
Yan Bondarenko
|
a9812477e2 |
fix(registry): preserve write options in aggregated storage
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> Assisted-by: LLM |
||
|
Alexey Artamonov
|
5cbf16088f |
fix(api): cap Kafka app name to the KRaft controller pod-hostname budget
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> |
||
|
Aleksei Sviridkin
|
5b91c6b7c6 |
fix(registry): run validating admission on TenantSecret writes (#4226)
## What this PR does The apiserver hands validating admission to a storage as the `ValidateObjectFunc` it passes into `Create`, `Update` and `Delete`. Mutating admission runs in the handler, but validating webhooks and `ValidatingAdmissionPolicy` execute only when the storage calls that callback, as `rest.AdmissionToValidateObjectFunc` in `k8s.io/apiserver` builds it for exactly that purpose. TenantSecret bound all three callbacks to `_`, so every write answered as if no policy existed. The practical effect is that the cluster-wide `cozystack-no-delete-guardrail` policy, and any webhook an operator writes for `tenantsecrets`, are inert on this resource. `application` and `securitygroup` call all three callbacks; TenantSecret was the only write-capable storage in the tree that did not. The callbacks are now called on all four write paths, including the create-on-update branch, which is a create and so carries `createValidation` rather than `updateValidation`. What goes to admission is the aggregated TenantSecret the caller submitted, or the stored one it replaces, not the backing Secret, so a policy sees the resource it was written against. This is a behaviour change for anyone who has a policy matching `tenantsecrets` today: writes that silently passed will start being evaluated, and may now be denied. That is the point of the fix, but it is worth saying out loud before it ships. ### Tests Each write path is covered by a denying admission callback, asserting both that the callback runs and that the backing Secret is left alone when it denies. A nil callback keeps every path working, which is what the apiserver passes when no plugin handles the operation. Removing any one of the four calls, one at a time, turns the suite red. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. It touches one aggregated storage's write paths: no package added, renamed or removed, no `values.schema.json`, no version enum, no changed default, no `kind` or `plural`, no `release.prefix`, no renamed output Secret or Service, no hand-typed CRD, and nothing under `hack/`. - [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(registry): run validating admission on TenantSecret writes, so ValidatingAdmissionPolicies and webhooks matching tenantsecrets are no longer bypassed ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - TenantSecret create, update, force-create, and delete operations now apply validation checks before completing changes. - Denied create or update requests are stopped before data is saved. - Delete validation runs after the current secret is retrieved and before removal. - TenantSecret write operations remain available when no validation callback is provided. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
64eff1f3c1 |
fix(registry): run validating admission on TenantSecret writes
The apiserver hands validating admission to a storage as the ValidateObjectFunc it passes into Create, Update and Delete: mutating admission runs in the handler, but validating webhooks and ValidatingAdmissionPolicies only execute when the storage calls that callback. TenantSecret bound all three to _, so every write answered as if no policy existed. The cluster-wide no-delete guardrail and any webhook written for tenantsecrets were inert on this resource, while application and securitygroup honour them. Call the callbacks on all four write paths, including the create-on-update branch, which is a create and so carries createValidation rather than updateValidation. The object handed to admission is the aggregated TenantSecret the caller submitted or the stored one it replaces, not the backing Secret, so a policy sees the resource it was written against. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
6d42223208 |
fix(registry): preserve dry-run in tap storage
Tap Create and Delete bound the options parameter to _ and built their own metav1.CreateOptions and DeleteOptions for the dynamic client, so the caller's dry-run directive never reached the backing write. A --dry-run=server connect created the Flux OCIRepository and a dry-run disconnect deleted the PackageSource and its source for real, which is the opposite of what a server-side dry-run promises. Forward the directive to all five backing writes, including the orphan-source fallback that had no access to the options at all. Field manager stays as the storage sets it: the objects are synthesized here rather than supplied by the caller, so who owns them is a separate question from whether the write happens. The fake dynamic client's tracker ignores DryRun and persists regardless, so the tests assert the options recorded on the backing action instead of the absence of an object; reverting any one of the five turns the suite red. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
scooby87
|
afc003c464 |
feat(postgres): database read-replica autoscaling via KEDA (#3954)
## What this PR does Adds automatic horizontal autoscaling of a PostgreSQL database's **read replicas** in response to load, implementing the merged design proposal [Database Horizontal Autoscaler](https://github.com/cozystack/community/tree/main/design-proposals/database-horizontal-autoscaling) (community#44). The mechanism is entirely stock — no bespoke operator, no new CRD: - **KEDA as an optional platform component** (`packages/system/keda`, wired into `core/platform` as a source + system-bundle entry). It claims the `external.metrics.k8s.io` APIService that nothing served before. - **`cozy-lib.keda.scaledObject` helper** (`packages/library/cozy-lib/templates/_keda.tpl`) renders a KEDA `ScaledObject`, fail-closed on missing/invalid arguments. - **`packages/apps/postgres` autoscaling** — an `autoscaling` values block (`enabled`, `minReplicas`, `maxReplicas`, `metric`, `target`, `maxReplicationLagSeconds`, `dryRun`, `transition`) validated by `values.schema.json`; under active autoscaling the chart renders a `ScaledObject` and seeds `spec.instances` to a constant floor (`max(replicas, effectiveMin)`), while the postgres release is forced to client-side (two-way) Helm apply so the seed is a merge no-op on upgrade and the KEDA-managed HPA — the runtime writer via the CNPG `Cluster` scale subresource — is not reverted. - **Single-value metric (§1)** — one PromQL query returns `Σ(read load over standbys) + target`, so a stock HPA computes `desired = ceil((Σ+target)/target) = 1 + ceil(Σ/target) = primary + desiredRead`, with no per-pod emission. - **The four static brakes (§5)** — quorum floor `max(minReplicas, maxSyncReplicas+1, 2)` (quorum wins over `maxReplicas`), scale-down pacing (`Pods/1/600s`), a replication-lag clamp folded into the query (freeze both directions while lag exceeds the threshold and the primary is writing, with `max_over_time` hysteresis), and dry-run (render paused). - **Two-phase enable/disable migration** via a `transition` sub-flag: phase 1 stages `spec.instances` to the live count and brings KEDA up with its HPA paused in both directions; flipping to phase 2 hands the runtime count to KEDA, and because the seed re-renders the same count the flip is a merge no-op with no dip. - **Observability** — a database-autoscaling Grafana dashboard and PrometheusRule alerts. **Live validation (dev cluster, CNPG 1.30.0).** The full path works end-to-end: the CNPG `Cluster` scale subresource serves `status.selector`, KEDA renders the HPA, the metric reaches it (`ScalingActive=ValidMetricFound`, `TARGETS 151/150`, `desiredReplicas=2 = 1 primary + 1 replica` — the §1 arithmetic), and the lag-clamp query returns the base value on live VictoriaMetrics. Two packaging fixes were needed and are included: KEDA must run on the cozystack `cozy.local` cluster domain (the upstream `cluster.local` default makes the metrics gRPC target NXDOMAIN and every HPA hangs on `FailedGetExternalMetric`), and the operator's client-go throttle is raised so a single-replica operator does not lose its leader lease under scale-subresource polling. Implements #3953. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. Two repositories are affected; both are follow-ups to open once this lands (and after its dependency #3951 merges and the values surface is final), left unticked here for a maintainer to confirm rather than claimed with a speculative link: - **cozystack/terraform-provider-cozystack** — this adds the nested `autoscaling` block to `packages/apps/postgres/values.schema.json`. The provider hand-models each app's fields (schema + model + expand/flatten), so it needs the new block to expose it in Terraform. - **cozystack/website** — KEDA is a new platform component (`guides/platform-stack`), and the postgres reference page gains the `autoscaling` params. The managed-app reference is regenerated from `README.md` by the release bot; the platform-stack component list is hand-maintained. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: to open (new platform component + postgres autoscaling params) - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: to open (new `autoscaling` block in postgres values.schema.json) ### Depends on - #3951 — bump CloudNativePG to 1.30.0. The HPA can only drive the CNPG `Cluster` scale subresource on CNPG ≥ 1.28.4, where it serves `status.selector`; without it the HPA fails with `InvalidSelector`. This PR should not merge before #3951. ### Release note ```release-note feat(postgres): add optional horizontal autoscaling of read replicas via KEDA, configured through the new `autoscaling` values block; KEDA ships as a new optional platform component. Requires CloudNativePG >= 1.28.4 for the Cluster scale subresource selector. Note: every Postgres release now applies client-side (`release.cozystack.io/helm-server-side-apply: "false"`) so the autoscaler's live `/scale` count is not reverted by Helm; a kind-wide side effect is that chart-rendered fields on Postgres releases are no longer re-asserted by server-side-apply drift correction (driftDetection is unset), a documented trade-off with a per-instance apply strategy as follow-up. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configurable PostgreSQL read-replica autoscaling with KEDA. * Added CPU and read-connection metrics, replica bounds, replication-lag safeguards, dry-run mode, and transition handling. * Added optional KEDA platform support, Grafana autoscaling dashboard, and Prometheus alerts. * **Documentation** * Clarified integer thresholds, prerequisites, configuration, and transition guidance. * **Bug Fixes** * Improved validation and actionable errors for unavailable KEDA or invalid settings. * **Tests** * Expanded coverage for autoscaling, observability, package selection, and validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Alexey Artamonov
|
e89c39cb96 |
test(cozystack-api): cover the annotation-to-ReleaseConfig wiring
Complete() reads release.cozystack.io/* annotations into ReleaseConfig once per ApplicationDefinition, but nothing exercised the assignments that join them. A mutation dropping release.HelmServerSideApply = serverSideApply left the suite green while shipping a silent nil: every Postgres HelmRelease would keep helm-controller's server-side default, Helm would force-own the rendered spec.instances seed, and the next reconcile would revert KEDA's live /scale count - the highest-blast-radius line in the feature, untested. Extract the per-CRD assembly into buildResourceFromCRD so the wiring is unit-testable without a live discovery client, and add TestBuildResourceFromCRD covering the apply-strategy annotation (present -> false, absent -> nil), the disable-wait sibling on the same loop, and the healthCheckExprs pass-through. Reverting either the SSA or the healthCheckExprs assignment now turns the test red. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
3a4bade677 |
docs(postgres): document the kind-wide scope of the client-side apply flip
The release.cozystack.io/helm-server-side-apply annotation moves every Postgres HelmRelease to client-side apply, including releases that never enable autoscaling. IvanHunters flagged that this platform-wide side effect was unexplained and that non-autoscaled releases silently lose SSA drift re-assertion (client-side diffs previous-vs-new render only, never live state; driftDetection is configured nowhere in the repo). Record why the scope is deliberate rather than narrowing it. helm-controller v1.5.0 resolves an unset Upgrade.ServerSideApply through "auto" from the last release's apply method, so gating client-side on "autoscaling active" would force an SSA->client-side ownership handoff on the live spec.instances the moment a tenant enables autoscaling - the exact live-field handoff the seed design was chosen to avoid. Keeping the apply method stable for the kind's whole lifecycle keeps enable/dry-run/transition/disable clean two-way merges with no apply-method transition. The drift-re-assertion cost on never-autoscaled releases is called out as an accepted trade-off, with a per-instance apply strategy as the follow-up if it is ever needed. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Andrei Kvapil
|
54621bc046 |
feat(migration): tenant self-service VM import from VMware vSphere
Hidora's customers are leaving VMware now, and a tenant has to be able to import their own machines without a platform administrator. The chart approach could not express that: a tenant can neither create the Secret holding vCenter credentials nor name the VDDK image, so both had to come from somewhere they cannot reach. Two CRDs in a new `forklift.cozystack.io` group, split by lifecycle. A VMImportSource is a long-lived connection -- type, endpoint, credentials, readiness -- and a VMImportTask is the one-shot operation that names machines and produces VMDisks and VMInstances. The outputs carry no owner reference back to the task, so deleting a finished import leaves the machines alone. Konveyor Forklift does the transfer underneath; the controller renders its objects, mirrors its verdicts, and hands each finished volume into a VMDisk without copying it. What a live vCenter migration forced out, and no amount of source reading would have: - A guest copied verbatim keeps the drivers it had under VMware and no virtio, so on a virtio disk it does not fail the import -- it succeeds and then does not boot. Imported disks go on SATA. - VDDK connects straight to the ESXi host at the address vCenter advertises, which is routinely unreachable, and on the first cluster collided with the Service CIDR. `spec.hosts` redirects per host. - Firmware must be carried across or a UEFI guest imports as BIOS and never boots (stacked on #3002). - Deleting a task mid-transfer orphaned its DataVolume; a finalizer now removes the engine's volumes and leaves the outputs. The console gains a Migration section: connections are editable, and the VM field is a picker built from a machine list the controller publishes, since the aggregated API cannot reach the inventory and should not hold a source's credentials to try. Two general mechanisms fall out of that -- an option source may take an argument, and one named in a CRD annotation may reference a sibling field. Design: cozystack/community#62. Docs: cozystack/website#673. Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
IvanHunters
|
2af19c4378 |
fix(marketplace): Get surfaces a pending tap instead of 404
List emits a pending Tap for a tap source (labeled OCIRepository) that has no materialized PackageSource yet, but Get looked up only the PackageSource and returned NotFound for the same object. A dashboard detail view (list then detail) of a connecting or collision-blocked tap would 404 instead of showing the block reason. Get now falls back to the pending tap source by name, the same lookup Delete already uses, so Get, List, and Delete agree. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
15be9329a1 |
refactor(marketplace): source-centric Tap resource, label-based identity
The aggregated Tap resource no longer derives a community.<org>.<repo> name from the connect URL. A connected repository is identified by its Flux source (tap-<org>-<repo>), and the PackageSource(s) it materializes keep their own declared names. Community is computed from the marketplace-tap label, and disconnect is guarded by that label instead of a name prefix. List also surfaces tap sources with no materialized PackageSource yet (a connect in progress, or one blocked by a collision), carrying the materializer error message, so a blocked tap is visible in the dashboard rather than silently never appearing. The community. prefix constant is removed. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
c9accd6086 |
fix(marketplace): address maintainer review — url collisions, fail-closed digest, centralized prefix
From @kvaps's review of the untrusted-artifact path: - Refuse to silently retarget: two repositories that share an org/repo path on different registry hosts derive the same tap name. The API and CLI now reject a connect when an existing source points at a different spec.url, instead of blindly repointing the first at the second. - Fail closed on integrity: verifyAndExtract now errors on an absent digest (not just a mismatch), and the reconciler waits for status.artifact.digest, so a tap artifact is never materialized without a verified digest. - Centralize the load-bearing community. prefix in internal/marketplace/tapconst (removing two duplicate consts and several literals across the CLI, API, materializer, and dashboard) so the disconnect guard, the community badge, and the anti-shadowing guarantee cannot drift. - Split multi-document YAML with the apimachinery reader so a PackageSource whose values contain a separator-looking line is not fractured. (The short-name version-pin drop was already fixed in an earlier commit.) Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
3ce8279019 |
fix(marketplace): prune stale taps, preserve source state, scope RBAC, pull by digest
Address the remaining review findings: - The materializer now prunes PackageSources it materialized from an earlier revision that the current artifact no longer contains (including a single/multi rename), so a removed package leaves the catalog; installed Packages stay. - A repeat connect updates the fetched OCIRepository's spec, label and name annotation instead of replacing the object, so the operator's finalizer and materialized-revision annotation survive; covered by a test. - OCIRepository write verbs move from the cluster-wide cozystack-api ClusterRole to a namespaced Role in cozy-system (get/list/watch stay cluster-wide for the watch cache), since the API only manages tap sources there. - 'cozypkg validate --require-signature' now pulls the exact manifest digest cosign verified, closing the tag-mutation window between verify and pull. - safeJoin rejects any '..' path segment outright (defense in depth and to satisfy the Zip-Slip scanner) on top of the existing containment check. - Document why the index gate's merge/label steps need a workflow_run split for fork pull requests. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
be03900262 |
refactor(marketplace): address review — shared naming, tap rollback, pinned gate, trust docs
- Extract the per-component artifact-name convention into internal/marketplace/naming and have the validator and the Tap backend delegate to it, so the component-to-ApplicationDefinition linkage cannot drift across copies. - cozypkg tap only rolls back resources it actually created, so an idempotent re-tap no longer deletes a pre-existing OCIRepository/PackageSource when a later apply fails. - Pin cozypkg and yq in the index publication gate so the auto-merge decision runs on a reproducible toolchain. - Document the trust model explicitly: tap validates structure but does not verify signatures; verification is enforced by the index CI gate (pinned to the recorded cosign identity) and optionally by Flux OCIRepository verification at pull time. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
52de2e460e |
fix(cozystack-api): grant Tap write-path RBAC and recover pending taps
The marketplace Tap create/delete run with the cozystack-api ServiceAccount, which was granted only get/watch/list on cozystack.io and nothing on source.toolkit.fluxcd.io, so connecting or disconnecting a tap returned Forbidden on every cluster. Grant the ServiceAccount delete on cozystack.io/packagesources and full management of source.toolkit.fluxcd.io/ocirepositories. Also make a pending or failed connect recoverable: when a tap has an OCIRepository but no materialized PackageSource yet, Delete now falls back to removing the labeled OCIRepository matched by its tap-name annotation, instead of returning NotFound and orphaning a finalized source. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
e61d2c7b53 |
feat(marketplace): connect a community tap from the API with async materialization
Add server-side connect to complete the marketplace write path. Creating a Tap records the intent only: the API validates the oci:// reference and creates a labeled Flux OCIRepository (with an optional pull-credential secretRef), never blocking the request on a registry pull, and returns a Tap whose status shows materialization pending. The connect inputs (url, tag, secretRef) are write-only and are never echoed back or returned on read. A new tap materializer reconciler in the operator watches community-tap OCIRepositories, and once source-controller has pulled the artifact, fetches the tarball over the Flux artifact URL (digest-verified, size-bounded, path-traversal guarded), parses the PackageSource(s) it carries, and materializes them under the community. prefix with their sourceRef repointed at the tap's source. A finalizer cleans up the materialized PackageSources when the source is removed, leaving installed Packages in place. The shared label/annotation keys live in internal/marketplace/tapconst so the API and the operator cannot drift on the connect-to-materialize handoff. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
c8fd40c419 |
feat(cozystack-api): support disconnecting a community tap via the Tap resource
Implement Delete on the Tap resource so the dashboard can disconnect a community tap, mirroring 'cozypkg untap'. It refuses official (non-community.*) sources, removes the PackageSource, and removes the backing Flux OCIRepository only when no other PackageSource still references it. Installed Packages are left untouched. The delete verb is not granted to the tenant catalog-read roles, so disconnecting stays cluster-admin only through the delegated RBAC. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
5848d85840 |
feat(cozystack-api): add read-only Tap resource for the marketplace view
Add a cluster-scoped, read-only Tap resource to the core.cozystack.io API group that powers the dashboard marketplace view. Each Tap is computed on read from a PackageSource and the ApplicationDefinitions attributable to it (matched by the component's assembled-artifact name), exposing the connected repository, whether it is a community tap, its readiness, and the packages it exposes with their catalog metadata. It is served through the existing aggregation layer, so the dashboard consumes it with the same list/watch client it already uses, and authorization is the delegated RBAC the apiserver already enforces. Taps carry catalog metadata only and never a pull-credential Secret, so browsing cannot leak a private tap's credentials across the tenant boundary. Read access is granted to the tenant catalog-read roles alongside options. Modeling the marketplace as a virtual resource in an existing API group avoids a new API group and needs no etcd or CRD: the object is computed from cluster state via the apiserver privileged dynamic client, mirroring the options resource. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Alexey Artamonov
|
27d7dbb140 |
feat(api): add per-app helm apply-strategy override annotation
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> |
||
|
Aleksei Sviridkin
|
c8d073fff7 |
fix(api): drop caller-supplied internal.cozystack.io labels in tenantsecret writes (#3599)
## What this PR does Labels and annotations under `internal.cozystack.io/` belong to the platform. The lineage webhook stamps `internal.cozystack.io/tenantresource` to decide whether a Secret is visible through this API at all, and the cacert controller stamps `internal.cozystack.io/tenant-ca`, which an `ApplicationDefinition` selects on in `spec.secrets` to hand a trust anchor to the tenant. The registry copied every caller-supplied key onto the backing Secret as is, so anyone with write access to `tenantsecrets` could plant either marker on a Secret of their choice, or strip one to take an object out of the platform's view. `tenantToSecret` is the only place the write verbs go through, PATCH included, because `rest.Patcher` is `Getter`+`Updater` and the endpoint installer serves PATCH out of `Update`. It now skips keys with that prefix. The update target starts as a copy of the stored Secret, so a platform key cannot be removed either. The guard is on the whole prefix, so a platform key added later is covered without anyone touching this code again. Reads are unchanged. A tenant can see the platform's verdicts, it just cannot write them. Keys are dropped silently instead of rejected, otherwise a GET, edit and PUT round trip would break `kubectl edit` for no gain. Nothing writes through this API today. Every `tenantsecrets` grant in `cozystack-basics` is `get`/`list`/`watch`, and the console only lists and watches. That read-only RBAC is what keeps the hole inert, and it lives in a different package from the registry that implements the write verbs, which is why I would rather close it here than keep relying on it. Two things I found next to the diff and did not fix. The `Patch` method on this type is never reached: its signature is the `client-go` shape and matches no interface the endpoint installer asserts, so no request lands there. I gave it the same guard instead of leaving the weaker of two doors, but deleting it is probably the better answer and I would rather that call was yours. A repair after the write is weaker than the conversion anyway: the forged key is live between the patch and the repair, and forcing the object back to a pre-patch snapshot drops a key that a controller stamped inside that window. Second one, and the reason none of this is easy to spot: `Update` passes `nil` where the handler hands the storage the current object, so `hasUID` reports false and the patch mechanism takes its create-new-object branch. For JSON Patch, merge patch and strategic merge patch that branch returns `NotFound`, so all three get a 404 against an existing TenantSecret. Apply is the exception. `applyPatcher.createNewObject` builds a fresh object of the kind and applies onto that, so an apply does reach `Update`, carrying only the fields the caller sent, and the merge in `tenantToSecret` is what keeps the stored labels. Passing `secretToTenant(previous)` instead of `nil` is the fix and it belongs in its own change, since it makes the other three patch types live. Tell me if you want issues for either. Closes #3464 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: Walked the trigger map against the diff: two Go files under `pkg/registry/core/tenantsecret/`, no package added, no chart, values schema, CRD, `ApplicationDefinition` field, namespace, `hack/` target or release asset touched. Nothing in the map fires. ### Release note ```release-note fix(api): TenantSecret writes no longer accept caller-supplied `internal.cozystack.io/*` labels and annotations, so a principal with write access to the API can neither plant nor strip the platform markers that decide tenant visibility on the backing Secret. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Strengthened protection of platform-managed metadata during tenant-secret creation and updates. * Prevented tenant-provided platform-owned labels and annotations from overwriting protected values. * Preserved standard metadata while hiding sensitive resource markers. * Improved patch-based update handling and protected metadata consistency across changes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
ef4f5b45ab |
fix(api): drop caller-supplied internal.cozystack.io keys in tenantsecret writes
Labels and annotations under internal.cozystack.io/ are platform verdicts, not tenant metadata. The lineage admission webhook stamps internal.cozystack.io/tenantresource to decide whether a Secret is visible through this API at all, and the cacert controller stamps internal.cozystack.io/tenant-ca, which an ApplicationDefinition selects on in spec.secrets to promote a trust anchor to the tenant. The registry copied every caller-supplied key onto the backing Secret verbatim, so a principal holding write access to tenantsecrets could plant either one on a Secret of its choosing, or strip one to take an object out of the platform's view. Only read-only tenant RBAC, which lives in a different package, stood between a caller and that path. tenantToSecret is the choke point every write verb goes through, PATCH included: rest.Patcher is Getter+Updater, so the endpoint installer serves PATCH out of Update. It now skips the platform keys, and since the update target starts as a copy of the stored Secret, ignoring the caller there also means a platform key cannot be removed. Guard on the whole prefix rather than on a list of keys, so a platform key added later is protected without anyone remembering to come back here. Delete the Patch method on this type. Its signature matches no interface the endpoint installer asserts, so no request has ever reached it, and as a write path it was strictly weaker than the conversion: the raw patch hit the backing Secret before any guard ran, leaving a forged key live in etcd -- visible to every controller and to List -- until a second write put the platform keys back, or for good when that second write failed. One door is better than two, and the surviving one commits once. Keys stay visible on read. A tenant may see the platform's verdicts, and failing a write that echoes them back would break a read-modify-write round trip for no gain, so they are dropped silently instead of rejected. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |