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>
## 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 -->
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>
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
Backing resource names are not part of the aggregated API contract.
Clients need the requested resource identity and the original status
code and reason to handle rejected writes consistently.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
Assisted-by: LLM
Aggregated endpoints must retain backend HTTP codes and reasons so
clients can distinguish conflicts and admission failures. Dry-run
deletion also depends on current finalizers, which a cached read can miss.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
Assisted-by: LLM
Dry-run must expose the same deletion state as a real request without
persisting it. Shared option objects can hide dropped fields in tests,
and an unreachable patch method offers no API coverage.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
Assisted-by: LLM
A Kafka cluster's KRaft controller pods are named
"<release>-c-<hash>-<id>" (8-hex hash, id at least one digit), and that
derived pod hostname must fit the 63-char DNS-1123 label limit. That is
tighter than the 53-char Helm release-name budget validateNameLength
enforced, so a release name of 51-53 characters was admitted but then
failed the chart render (kafka.assertNameLength) forever, an
un-recoverable upgrade dead-end for an existing cluster whose name is
immutable. Cap the Kafka app name at admission, mirroring the existing
Kubernetes worker-pool cap, so the overflow surfaces on the Kafka CR the
operator is editing instead of at render time on a pod Strimzi can never
create. The chart's render guard remains the second line of defense and
also catches larger clusters whose higher node ids need more digits.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
## 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 -->
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>
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>
## 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 -->
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>
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>
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>
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>
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>
- 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>
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>
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>
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>
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>
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>
## 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 -->
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>
TestWarnRemovedKubernetesFields pinned the helper in isolation, so deleting
either call site (rest.go Create/Update) left it green while an operator editing
a Phase 2 removed field would silently get no warning. These drive Create and
Update end to end and assert the warning surfaces; removing a call site now
fails loudly (verified by mutation).
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
The Kubernetes CR still accepts and stores spec.nodeGroups, spec.nodeHealthCheck
and spec.maxNodeProvisionTime after the Phase 2 split (the adoption migration
leaves them on an upgraded CR rather than rewriting parent values mid-upgrade),
but they no longer render anything, so an operator scaling via
spec.nodeGroups.<x>.minReplicas silently got nothing.
Emit a non-blocking client-facing admission warning on Create/Update when any
of these fields is present, pointing the operator at the KubernetesNodes
resources. A hard rejection is intentionally avoided: it would break edits to
already-upgraded clusters that still carry the fields. Also enumerate all the
Phase 2 breaking changes (three removed fields, two KubernetesNodes
immutability rules, the 32-char cluster-name cap) in the chart README.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
A Kubernetes cluster's worker pools are separate KubernetesNodes releases
named "kubernetes-nodes-<cluster>-<pool>". validateNameLength only checked
the parent's own "kubernetes-" prefix budget (42 chars), so a cluster name
of 33-42 chars was admitted while every child pool release overflowed the
53-char Helm limit and could not render -- the migration then pinned and
skipped the pool, leaving the cluster with no way to add workers. Pre-split
nodeGroups had no such constraint, so this was a regression.
Add a kind-specific 32-char cap for the Kubernetes kind so the overflow
fails at admission on the parent CR instead of at render time on an
un-creatable child.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Add first-class fields so a package/app can declare the Flux helm-controller
v1.5.0 kstatus health gate for the custom resource it wraps, so
HelmRelease.Ready means "the CR is actually healthy" instead of "helm applied
the CR" (#2642). API-owner approved in #3263.
- api: WaitStrategy (scalar, maps to HelmReleaseSpec.WaitStrategy.Name — a
deliberate simplification) + HealthCheckExprs ([]kustomize.CustomHealthCheck,
the upstream type verbatim) on both ApplicationDefinition.spec.release (the
apiserver path) and ComponentInstall (the operator path).
- config.ResolveWaitStrategy: shared by both HelmRelease builders. Couples the
default — healthCheckExprs imply the poller strategy when none is set, since
exprs are only evaluated under poller. This removes the "must also set
waitStrategy: poller" footgun and does not depend on the controller's global
default.
- thread through buildHelmReleaseSpec (operator) and
convertApplicationToHelmRelease (apiserver); the apiserver loads the typed
spec.release fields in start.go.
- regenerate deepcopy, the ApplicationDefinition and PackageSource CRDs, and
openapi. The backupstrategy CRDs also regenerate — benign k8s-0.35 PodSpec
description drift, not a functional change.
disableWait is intentionally not added (YAGNI): the only known need (the
kubernetes tenant-addon deadlock) was resolved child-side, and waitStrategy:
legacy is not equivalent (legacy still waits Helm-v3-style; disableWait skips
waiting entirely). Add it later when a concrete leaf needs a true no-gate
escape hatch.
Refs #2642
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
parentPoolUsage listed ResourceQuotas through the aggregated apiserver's cached
client, which lazily starts a cluster-wide ResourceQuota informer. cozystack-api
has no RBAC to list resourcequotas, so the informer never synced and tenant
create/update of a bounded child hung until the request timed out (observed on a
cluster). Read quota usage through the direct (uncached) watch client instead,
scoped to the pool's member namespaces, so no cluster-wide informer is started,
and grant cozystack-api get/watch/list on resourcequotas for those reads.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com>
The declaration-time gate only subtracted sibling carve-outs from a parent's
budget, not the parent pool's current usage. A tenant already consuming most of
its budget could therefore carve a full-budget child, letting the sub-tree
collectively exceed the root's allocation.
Sum the current usage of the parent pool (its own namespace plus its unbounded
descendants, read from each member's rendered ResourceQuota.status.used) and
subtract it from the remaining budget, mapping each shorthand quota key to its
rendered limit key (renderedLimitKey). Boundedness is decided by declared
resourceQuotas so the result is independent of whether Flux has rendered a
freshly-created tenant's quota yet. This is best-effort at declaration time (a
snapshot; concurrent writes can briefly overshoot and running pods are never
evicted) — the tenant-quota controller remains the runtime backstop.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com>
Add a declaration-time gate in the aggregated apiserver: a child tenant's
declared resourceQuotas are carved out of its parent tenant's remaining
quota and may not exceed it. A child without a quota shares the parent
pool and is allowed; a parent without a quota does not constrain children.
This blocks quota escalation via sub-tenant creation (declaring a bigger
or unbounded child) without re-implementing pod-level admission.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
## What this PR does
Introduces a tenant-facing `SecurityGroup` resource in a new
`sdn.cozystack.io/v1alpha1` API group, served by the Cozystack
aggregated API server as a 1:1 projection of a `CiliumNetworkPolicy`. A
SecurityGroup attaches to one of the tenant's managed applications by
reference (`spec.targetRef: {apiGroup, kind, name}`) and declares
allow-list ingress/egress for that application. Tenants manage network
policy for their own apps entirely through
`securitygroups.sdn.cozystack.io`, without being granted any access to
the `cilium.io` API group.
The translation is synchronous and stateless — the API server converts
each SecurityGroup into a CiliumNetworkPolicy in the same namespace on
write and back on read; there is no controller. The backing policy's
`endpointSelector` is **derived** from `targetRef` (matched on the
application's lineage labels
`apps.cozystack.io/application.{group,kind,name}`), never copied from
tenant input, so the selector is machine-generated and can only ever
match the referenced application's own pods in the tenant's namespace.
Policies owned by this API carry an `sdn.cozystack.io/securitygroup`
marker label that the storage always re-asserts and hides from the view,
so it only ever surfaces or mutates its own policies and never touches
platform-managed network policies.
The value is self-service network policy on a Cozystack-native,
RBAC-scoped surface, without exposing Cilium's version-coupled CRDs to
tenants. Because the selector is derived from an authorized application
reference rather than free-form tenant input, the RBAC boundary is
structural, not selector-validated: a tenant cannot express network
policy against arbitrary or platform-owned pods. The full design —
selector derivation from lineage labels, the structural boundary, the
additive-policy reality (a SecurityGroup widens a managed application's
allowed traffic but cannot sever its platform-managed management plane,
and equally cannot tighten below the platform baseline), the in-tree
CiliumNetworkPolicy mirror that avoids a Cilium Go-module dependency,
RBAC, and deferred extensions (raw-pod targeting, cluster scope,
reusable groups) — is in `pkg/apis/sdn/DESIGN.md`.
Implements the attachment model from #1614.
### Release note
```release-note
feat(api): add the SecurityGroup resource (sdn.cozystack.io/v1alpha1) — a tenant-facing, namespace-scoped firewall that attaches to a managed application by reference and projects 1:1 onto a CiliumNetworkPolicy with a machine-derived endpoint selector, letting tenants manage their application's network policy without cilium.io access.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added the `SecurityGroup` API resource (`sdn.cozystack.io` v1alpha1)
with a namespaced projection backed by Cilium network policies.
* Enabled the SDN `APIService` and wired SDN routing, scheme, and REST
support.
* Expanded RBAC to include SecurityGroup access and required Cilium
policy permissions.
* **Documentation**
* Added a design document describing projection behavior and rule
semantics.
* **Bug Fixes**
* Improved API rule validation coverage for missing list-type fields.
* **Tests**
* Added SDN round-trip, fuzzing, RBAC, and comprehensive REST
create/get/list/update/delete/watch/resource-version tests.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
The talos-reconcile Job is the only thing that creates the
TalosConfigTemplate the worker MachineDeployment's bootstrap.configRef
points to; CAPI blocks MachineSet creation until it exists, so tenant
workers cannot scale until the Job has run.
The Job was a post-install/post-upgrade Helm hook, which runs only after
Helm has applied the release's main resources and, when Helm waits, after
they become Ready. But the same release emits in-tenant addon HelmReleases
— notably cilium — whose readiness needs worker nodes, and those nodes
need the TalosConfigTemplate this Job produces. So when the main-resource
wait is in effect the install deadlocks: it waits for cilium to be Ready,
cilium waits for workers, the workers wait for the template, and the
template waits for this hook — which never runs because the install never
finishes the wait that precedes the hook phase. The install times out and
the MachineDeployment never scales. The release sets
helm-install-disable-wait to skip that wait, but its effect is unreliable,
which makes the failure bimodal: the template lands in ~46s when the wait
is skipped, and the install is stranded when it is not.
Render the Job and its RBAC/ServiceAccount/egress policy as ordinary
main-phase resources. The Job is then created during the apply itself, not
after the wait, and runs on the management cluster where nodes exist
(unlike the tenant cilium-operator), so it produces the template, the
MachineDeployment scales, workers join, cilium goes Ready, and the wait
(if any) closes within the install budget — independent of whether
helm-install-disable-wait takes effect.
The Job is immutable, so it carries a content-hash name suffix over its
rendered spec (mirrors the KubevirtMachineTemplate idiom in cluster.yaml):
an identical render keeps the name and Helm no-ops it, while any spec
change yields a new name and a fresh Job, so no spec change ever lands on
the same name and triggers an immutable-field patch error. The applied
TalosConfigTemplate keeps its stable <release>-<group> name, so an
unchanged render re-applies a byte-identical template — a CAPI no-op that
never rolls existing workers.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The ingress fromApp/fromSG projection is exercised end to end, but the
symmetric egress toApp/toSG path — toApp to a lineage-label
endpointSelector and toSG to the referenced group's membership-label
endpointSelector — had no test. A wrong label key there would silently
point an egress allow at the wrong endpoints. Pin the projection and the
reverse reconstruction with a Create+Get round-trip, mirroring the
existing ingress coverage.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The backing CiliumNetworkPolicy carries the membership finalizer, so
deleting a SecurityGroup is virtually always asynchronous: the policy
gets a deletionTimestamp and survives until the securitygroup-controller
strips the membership labels off member pods and removes the finalizer.
REST.Delete nonetheless returned (nil, true), reporting the object as
instantly deleted, so the endpoint emitted a Success status claiming
completion instead of surfacing the terminating object — a
rest.GracefulDeleter contract violation for any client that distinguishes
the two to detect pending teardown.
After the delete, read the object's state back through the direct
(uncached) client and report deleted=true only when it is actually gone;
while a finalizer holds it, return deleted=false with the terminating
object. Using the direct client avoids a stale cache-backed read
misreporting an async delete as instant, and a dry-run (which leaves the
object in place) is resolved from the object's authoritative finalizers.
Implement rest.MayReturnFullObjectDeleter so the DELETE endpoint is
advertised and serialized as returning the SecurityGroup object, matching
the generic registry.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
A SecurityGroup with attachments but no ingress or egress rules exists
only to stamp its membership label on the attached applications' pods, so
other SecurityGroups can reference it as a peer. Its backing
CiliumNetworkPolicy carried only an endpointSelector, which the
cilium.io/v2 CRD rejects synchronously: the spec anyOf requires at least
one of ingress/ingressDeny/egress/egressDeny, so a selector-only policy
fails with "spec.ingress: Required value". A membership-only group
could therefore never be created.
Always serialize the backing policy's ingress section, emitting an empty
list when the group has no ingress rules. An empty list is the CRD's
documented no-op ("if omitted or empty, this rule does not apply at
ingress") and enableDefaultDeny defaults to false for a direction with
no rules, so the policy is schema-valid yet datapath-inert: it neither
allows nor denies, matching the additive, non-isolating semantics of an
empty rule list. The projection guarantees the slice is non-nil so it
never marshals to null, which the CRD (no nullable fields) would also
reject. The empty section collapses back to nil on read, so a
membership-only group round-trips with no ingress or egress.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Two watch-proxy defects in the SecurityGroup REST storage:
- watch.Error events from the backing CiliumNetworkPolicy watch (e.g. a 410 Gone
for an expired resourceVersion) carry a *metav1.Status, not a
CiliumNetworkPolicy. The CNP type assertion dropped them, so the client saw a
cleanly closed stream and never performed the required relist. Forward error
events verbatim before the type assertion.
- The DELETED-event bypass the label selector needs (a policy whose labels mutate
out of the selector is deleted carrying non-matching labels, so dropping it
would strand cached clients) was wrongly applied to the field selector too.
Name and namespace are immutable, so a field-selected watch (metadata.name=sg-a)
received other objects' deletions and could corrupt filtered client caches.
Apply the field filter to every event type.
Both paths are covered by new tests driving a fake backing watch.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
## What this PR does
> **Supersedes #2610.** Same work, rebased cleanly onto current `main`
so it now sits on top of #2867 (the `kube-controller-manager` VAP
type-checker fix). That fix is what was killing the base install in
#2610's E2E *before* the `kubernetes` app test ever ran, leaving the
Talos worker-bootstrap rewrite verified only by code reading. #2610's
67-commit branch is squashed here into 8 logical commits with original
authorship preserved (@kvaps and @IvanHunters as authors); the tree is
**byte-identical** to #2610's head — no code changes, just the rebase
onto `main` (to pick up #2867) plus history cleanup. The detailed review
history lives on #2610.
Phase 1 of the Kubernetes-app split design (see cozystack/community#8):
replace the Ubuntu+kubeadm worker bootstrap path with Talos, driven by
`cluster-api-bootstrap-provider-talos` (CABPT) and a
`clastix/talos-csr-signer` sidecar embedded in the Kamaji control-plane
pod. Existing tenants keep working — old machines roll out and get
replaced by Talos workers without manual intervention.
Highlights:
- Add CABPT (v0.6.12) as a second `BootstrapProvider` alongside the
existing kubeadm one in `capi-providers-bootstrap`.
- Bump the Kamaji control-plane provider with an upstream-bound patch
that exposes `KamajiControlPlane.spec.network.additionalServicePorts`,
used to publish trustd (50001/TCP) on the apiserver service.
- Generate Talos PKI (Ed25519 CA + trustd TLS) via cert-manager and
stable random Talos secrets (`token`, `clusterId`, `clusterSecret`,
`bootstrapToken`) using the helm lookup-and-reuse pattern.
- Materialise a kubeadm-format `bootstrap-token-<id>` Secret inside the
tenant `kube-system` via a Helm post-install/upgrade Job.
- Render a `TalosConfigTemplate` (worker machineconfig, `generateType:
none`) and switch the `MachineDeployment.bootstrap` reference from
`KubeadmConfigTemplate` to `TalosConfigTemplate`, gated on Talos secrets
and the Kamaji apiserver service being ready.
- Boot workers from the Talos openstack image via a CDI `DataVolume`
(`source.http.url` → `factory.talos.dev`), expose the system disk as
virtio-blk with `blockSize.custom: logical=512, physical=4096` so
4Ki-native block storage backends (LINSTOR/DRBD) play nicely with QEMU's
O_DIRECT writes and SeaBIOS still boots, drop the separate kubelet disk
(Talos lays out EPHEMERAL itself).
- Pin pod/service CIDRs in the worker machineconfig to the tenant ranges
(`10.243.0.0/16` / `10.95.0.0/16`) to avoid Talos's address-overlap
diagnostic against the host pod CIDR.
- Set `cluster.controlPlane.endpoint` and `cilium.k8sServiceHost` to the
Kamaji apiserver Service ClusterIP (looked up at render time), so
bootstrap survives the chicken-and-egg moment where tenant DNS does not
exist yet and the host CoreDNS does not serve the tenant zone.
End-to-end on a dev cluster: a fresh tenant brings up a Talos worker,
kubelet CSRs get approved (apiserver-client by Kamaji, kubelet-serving
by the talos-csr-signer sidecar), Cilium initialises with hostNetwork
against the apiserver ClusterIP, CoreDNS comes up, and the node
transitions to `Ready`.
### Release note
```release-note
feat(kubernetes): bootstrap tenant workers with Talos Linux instead of Ubuntu+kubeadm. Existing clusters roll over to Talos workers automatically; new tenants come up on Talos from the start. Powered by cluster-api-bootstrap-provider-talos and the talos-csr-signer sidecar in the Kamaji control-plane pod. Note: the tenant `kubernetes` HelmRelease now reports Ready as soon as Helm completes (Install/Upgrade DisableWait) and no longer blocks on worker/addon readiness — worker-rollout health is tracked by WorkloadMonitor, not the HelmRelease.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
## Release Notes
**New Features**
* Added Talos worker configuration for `talos.version` and
`talos.schematicID`
* Introduced `nodeHealthCheck` (`maxUnhealthy`, `nodeStartupTimeout`)
for worker remediation tuning
* Added image overrides for `kubectl` and `talosCsrSigner`
* Exposed additional tenant control-plane Service ports support
**Breaking Changes**
* Removed Kubernetes v1.30; supported versions start at v1.31
* Renamed `nodeGroups.ephemeralStorage` → `nodeGroups.diskSize` with
consolidated Talos system-disk semantics
**Removals**
* Removed Ubuntu container-disk images and related containerd patch
mechanisms
* Air-gapped tenant workers temporarily unsupported during the Talos
transition
**Documentation**
* Updated chart docs, parameters, and GitOps upgrade guidance for
version bumps
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Give each SecurityGroup its own membership label
(securitygroup.sdn.cozystack.io/<name>) and select it from the backing
CiliumNetworkPolicy, instead of deriving the endpointSelector from a single
app's lineage labels. spec.targetRef becomes spec.attachments, so one
SecurityGroup can apply to several applications at once.
Peers move from free-form fromEndpoints/toEndpoints selectors to
fromApp/toApp (resolved to lineage labels) and fromSG/toSG (resolved to
another group's membership label, a reference the Cilium dataplane resolves
live). Attachments are persisted in a storage-owned CiliumNetworkPolicy
annotation, round-tripped and hidden from the SecurityGroup view like the
marker label. Validation now covers attachments and app/SG peers and rejects
names that collide with reserved Cilium entities.
The membership labels are maintained by a separate controller added in a
follow-up commit; this change covers the API types and the
aggregated-apiserver projection only.
BREAKING CHANGE: SecurityGroup spec.targetRef is replaced by
spec.attachments, and ingress/egress peers use fromApp/fromSG/toApp/toSG
instead of fromEndpoints/toEndpoints. The feature is unreleased, so no
migration is required.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The access-check helpers returned plain fmt.Errorf values, which the
apiserver coerces to 500 InternalError regardless of the actual failure.
A request without user identity now surfaces as 401 Unauthorized, and a
RoleBinding list failure as 500 InternalError with a typed Status body.
Get also repackages a NotFound from the backing Namespace lookup so the
Status body reports the tenantnamespaces resource instead of namespaces.
The Watch goroutine already logs access-check failures and skips the
event, so no untyped error escapes to the client from that path.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Parse a new release.cozystack.io/helm-install-disable-wait annotation in start.go and apply it as Install/Upgrade.DisableWait in rest.go, so helm-controller no longer blocks post-install hooks on in-tenant addon readiness. This breaks the reconcile-hook chicken-and-egg where the hook that creates the worker TalosConfigTemplate was gated behind addons that need workers to schedule. Covered by a config table test.
Co-authored-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Update the REST storage fixtures and e2e bats to the targetRef shape, and
add cases for the new behaviour: the derived endpointSelector matchLabels,
the apiGroup default to apps.cozystack.io, an empty-targetRef rejection,
and invalid-label-value rejection (over-length or malformed kind/name/
apiGroup). The e2e asserts the dotted-key lineage labels serialise and are
accepted by the real cilium.io/v2 CRD, and that the view reconstructs the
targetRef.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Replace the tenant-authored endpointSelector on SecurityGroup with a
targetRef that references a managed application by {apiGroup, kind, name}.
The backing CiliumNetworkPolicy's endpointSelector is now derived by the
API server from the application's lineage labels
(apps.cozystack.io/application.{group,kind,name}) instead of copied from
tenant input, so the selector is machine-generated and can only ever
match the referenced application's own pods in the tenant's namespace.
The in-tree CiliumNetworkPolicy mirror gets its own spec type carrying a
concrete endpointSelector; the ingress/egress rule lists still project
1:1. Reads reconstruct the targetRef from the derived matchLabels, which
is lossless because validation caps each component at a valid label value
below the lineage webhook's truncation threshold.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
validateCIDRs rejected a bare IP (10.0.0.1) because net.ParseCIDR
requires a prefix, but Cilium's CIDR.sanitize accepts a bare IP as a
single-host CIDR. That made the validator stricter than Cilium and
blocked a legitimate single-host fromCIDR/toCIDR rule, contrary to the
design goal of rejecting only specs Cilium would silently discard.
Mirror Cilium by falling back to a plain-address parse, and add a
regression test for bare-IP CIDR acceptance.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The resourceVersion de-dup filter dropped ADDED events whose RV was <=
the requested startingRV. During a sendInitialEvents replay the backing
WatchList intentionally re-emits existing objects as initial ADDED
events with RV <= startingRV, so the filter broke the initial-state
replay. Skip the de-dup while sendInitialEvents is set; it still applies
when resuming a plain watch from a resourceVersion. Add a regression
test covering sendInitialEvents with a non-zero starting RV.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Enforce in the REST storage that Create persists into the request
namespace and that Update/force-create targets the request name,
rejecting a conflicting metadata.namespace or metadata.name with a
BadRequest. The aggregated apiserver already rejects these before the
storage is reached, but binding them here keeps the storage
self-defending against a cross-namespace or off-target write regardless
of the calling path. Add regression tests that drive the storage
directly to lock the contract.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The aggregated API server translates each SecurityGroup into a single
CiliumNetworkPolicy in the same namespace and back. Policies owned by the
SecurityGroup API are marked with the sdn.cozystack.io/securitygroup label,
which the storage always re-asserts on every write and hides from the
SecurityGroup view, so it only ever surfaces or mutates its own policies,
leaves platform-managed policies untouched, and cannot be orphaned by a
tenant overwriting the marker through spec labels. Mutating verbs run the
genericapiserver admission hooks, and updates replace labels/annotations
rather than merging them, matching Kubernetes PUT semantics.
To avoid importing the full Cilium module — which pins a Kubernetes
version incompatible with this project's apimachinery fork — the backing
object is a minimal in-tree CiliumNetworkPolicy mirror that reuses
SecurityGroupSpec as its spec body, making the projection a near-identity
translation in both directions.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Co-authored-by: Timofei Larkin <lllamnyp@gmail.com>
Aleksei Sviridkin