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>
## 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>
## What this PR does
Implements the tenant Flux sharding design from #2817: a new
`flux-shard-operator` system package that spreads tenant HelmReleases
across multiple helm-controller shards, so a noisy tenant (e.g. a
HelmRelease stuck in infinite remediation) cannot degrade the others.
- **Placement controller** owns the tenant→shard assignment: the unit of
placement is the tenant (all of its HelmReleases share one shard label),
distribution is greedy least-loaded weighted by HelmRelease count, with
pinned tenants, threshold-gated rebalance, paced moves, and never moving
deleting tenants. Watches are metadata-only, so the controller does not
decode the helm-controller status-patch firehose. Assignments are
recorded on tenant namespaces; HelmRelease labels remain the source of
truth on restarts.
- **Shard runtime** is provisioned by the operator: `shardCount`
helm-controller Deployments cloned from the flux-aio `flux` Deployment
and sanitised, so the image and feature-gates stay version-synced
automatically. flux-aio itself is unchanged and keeps owning everything
without a shard key.
- **CREATE-only mutating webhook** (`failurePolicy: Ignore`) stamps the
tenant's shard onto every HelmRelease at admission, so each one is born
on the correct shard regardless of creation path; any miss degrades
gracefully to the placement controller.
- The hand-rolled `flux-tenants` Deployment is retired: the operator
drains it (backfill `tenants` → `shard<i>`) and deletes it once empty;
migration 44 removes a leftover only under the same guard.
Ships with `shardCount: auto` (the default): the operator sizes the
shard count from the tenant HelmRelease count — `K = clamp(ceil(H/100),
1, min(16, T))` with hysteresis (scale up past 120 HR/shard, down under
60) — so small clusters stay at one shard (≈ today's behaviour) and
large fleets shard out automatically. An integer value pins the count
explicitly.
One deviation from the design doc: the webhook/controller scope is the
`sharding.fluxcd.io/key` label rather than
`internal.cozystack.io/tenantmodule` — every tenant HelmRelease is born
with `key=tenants` (stamped by ApplicationDefinition `release.labels`
and chart templates), so the webhook rewrites it and the placement
controller continuously relabels stragglers; `tenantmodule` covers only
the 6 tenant chart modules.
### Screenshots
Not a UI change.
### Release note
```release-note
feat(fluxcd): tenant HelmReleases are now reconciled by operator-managed helm-controller shards; the flux-shard-operator balances tenants across `shardCount` instances and retires the legacy flux-tenants deployment
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Introduced **flux-shard-operator** to shard helm-controller workloads
with deterministic tenant placement, optional rebalance controls, and
autosized shard count.
* Added a **CREATE-only mutating webhook** to stamp the correct shard
label on tenant HelmReleases.
* **Bug Fixes**
* Application updates now preserve existing **flux-shard** label
assignments to prevent unwanted shard re-stamping.
* **Chores**
* Added platform packaging for the new component, including an image
build step.
* Removed the legacy tenants deployment and added a migration to retire
it safely.
* **Tests**
* Expanded unit and chart tests for placement, provisioning, webhook
behavior, and configuration parsing.
* **Documentation**
* Added operator documentation describing configuration and metrics.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Resolve go.mod conflict: keep main's go 1.26.4 and golang.org/x/* bumps, and keep gomodules.xyz/jsonpatch/v2 v2.4.0 as a direct dependency (imported by the flux-shard-operator admission webhook) instead of main's indirect entry. go mod tidy leaves go.sum consistent.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
release.cozystack.io/helm-install-timeout sets both Install.Timeout and
Upgrade.Timeout to one value. Add release.cozystack.io/helm-upgrade-timeout
so an ApplicationDefinition can override only Upgrade.Timeout — letting a
kind keep a short install budget but a longer upgrade budget (or vice
versa). When set it wins over the value the install annotation would
otherwise apply to the upgrade side.
- config: add HelmUpgradeTimeoutAnnotation + ReleaseConfig.HelmUpgradeTimeout;
generalize ParseHelmInstallTimeoutAnnotation to ParseHelmTimeoutAnnotation
since it now parses both annotations (identical Flux unit validation).
- server: parse the new annotation at startup with the same loud-fail
behavior; update --helmrelease-upgrade-timeout help.
- rest: apply HelmUpgradeTimeout after the install-annotation copy so the
upgrade-specific value wins; restore the accurate carrier comment.
- tests: cover the upgrade-only override, the asymmetric precedence, and
the install-sets-both regression guard.
Behavior is unchanged for every existing ApplicationDefinition (none set
the new annotation today).
Address review feedback from coderabbitai on rest.go.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
gofmt -w on the two touched test files: the case-table struct fields in
rest_helmrelease_spec_test.go and main_test.go were misaligned, and
main_test.go carried a trailing blank line. CI runs no gofmt gate
(pre-commit only runs make generate), so this would otherwise land
unformatted.
Address review feedback from lexfrei.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The convertApplicationToHelmRelease comment listed only kubernetes-rd
as carrying release.cozystack.io/helm-install-timeout, dropping
tenant-rd. tenant-rd/cozyrds/tenant.yaml sets the 15m annotation for
the seaweedfs-db CNPG bootstrap whose first reconcile exceeds the flux
default. Restore the accurate two-carrier list and rationale.
Address review feedback from lexfrei on rest.go.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The XValidation justification was factually wrong: the deployed
helm-controller CRD has no rule rejecting a Strategy alongside a
Remediation entry (only .retryInterval is gated on Strategy.Name), so
the claimed "silently break every HR" safety net does not exist.
Reword the comment in all three test files to the real reason
Remediation stays nil: retries are driven by a single
Strategy.RetryInterval path, matching cozystack-operator's
PackageReconciler.buildHelmReleaseSpec.
Address review feedback from lexfrei on rest_helmrelease_spec_test.go,
rest_timeout_test.go and internal/operator/package_reconciler_test.go.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Production change:
- convertApplicationToHelmRelease now sets Install/Upgrade.Timeout
unconditionally (matching cozystack-operator's buildHelmReleaseSpec).
The previous "stay nil if zero" branch was a leaky test shim — it kept
fixtures with empty ReleaseConfig{} green at the cost of api/operator
drift.
Test changes:
- newRESTForTimeout seeds the new HelmRelease* globals with
production-shaped defaults so the unset-annotation case exercises
"global default applies" rather than "field stays nil".
- TestConvertApplicationToHelmRelease_PerAppTimeoutOverridesGlobal is
table-driven and now covers asymmetric globals (install != upgrade)
with and without per-app override — guards against a regression that
only assigns one side under override.
- New start_test.go: table-driven validator coverage for each
malformed/non-positive duration flag, the negative MaxHistory branch,
the MaxHistory=0 unlimited branch, asymmetric timeout acceptance, and
a smoke test that NewCozyServerOptions defaults validate.
Refactor:
- Extracted parseAndValidateHelmReleaseFlags from Complete() so the
flag-validation portion is unit-testable without kubernetes I/O.
Documentation:
- Reworded --helmrelease-install/upgrade-timeout help so admins reading
--help see the dual-purpose annotation explicitly (one annotation
overrides both timeouts together).
- Dropped PR-#2509 references and "preserving the pre-#2509 behaviour
for tests" comments — kept the technical rationale (Strategy.Name
vs Remediation.Retries:-1 + 5m Interval coupling) on its own merits.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
convertApplicationToHelmRelease now builds the HelmRelease spec from
ReleaseConfig instead of hardcoding values, bringing it in lockstep
with cozystack-operator's PackageReconciler.buildHelmReleaseSpec
(PR #2509). The drift between the two HelmRelease-generating paths
was the rationale for this follow-up.
Changes to the generated spec:
- Spec.Interval reads ReleaseConfig.HelmReleaseInterval (was
hardcoded 5m).
- Spec.MaxHistory set as &maxHistory pointer from
ReleaseConfig.HelmReleaseMaxHistory; 0 survives as pointer-to-0
(unlimited per Helm semantics) - mirrors the operator's
TestBuildHelmReleaseSpecZeroMaxHistory.
- Install.Strategy{Name: RetryOnFailure, RetryInterval} replaces
Install.Remediation{Retries: -1}. Same for Upgrade.Strategy.
Functionally equivalent to "retry forever, never remediate" for
the api-side path (which also never relied on remediation
firing) but decouples failed-install retry timing from
Spec.Interval.
- Install.Remediation / Upgrade.Remediation stay nil:
helm-controller's XValidation rejects RetryOnFailure +
RetryInterval alongside a Remediation entry, so reintroducing
Retries:-1 "for safety" would silently break every HR.
Install/Upgrade.Timeout precedence:
- per-Application HelmInstallTimeout (annotation-driven) wins
when non-zero (preserves the kubernetes-rd opt-in path);
- otherwise HelmReleaseInstallTimeout / HelmReleaseUpgradeTimeout
globals apply;
- when both are zero, the field stays nil and flux defaults
apply (preserves the legacy code path for tests that
hand-construct ReleaseConfig without populating the new globals).
Tests:
- rest_timeout_test.go: Remediation.Retries == -1 assertions
replaced with Strategy.Name == RetryOnFailure +
Remediation == nil, matching the operator's spec test.
- rest_helmrelease_spec_test.go: new file mirroring
TestBuildHelmReleaseSpec / TestBuildHelmReleaseSpecZeroMaxHistory
from internal/operator/package_reconciler_test.go, plus a third
case asserting per-Application HelmInstallTimeout overrides the
global Install/Upgrade timeout defaults.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
REST.Update wraps the HelmRelease write in retry.RetryOnConflict so a stale
resourceVersion (the helm-controller racing the status subresource) is
refreshed and retried instead of surfacing a 409. Add a unit test using a fake
client whose interceptor returns one Conflict on the first Update and succeeds
on the second, asserting Update returns nil and writes exactly twice.
Addresses non-blocking follow-up 3 on #2948.
Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The aggregated API server translates app CR updates (Etcd, Postgres, ...)
into a read-convert-update of the backing HelmRelease. Flux's
helm-controller continuously writes the HelmRelease status, which shares
the object's resourceVersion, so a CR update that lands while a prior
reconcile is still in flight reads a resourceVersion that goes stale
before the Update and the request fails with a 409 Conflict
("the object has been modified; please apply your changes to the latest
version and try again").
The HelmRelease spec is fully derived from the Application the caller
just applied, so a stale-resourceVersion conflict is never a real spec
conflict here. Wrap the Update in retry.RetryOnConflict, refreshing the
live resourceVersion on each conflict before retrying. The no-conflict
path is unchanged, and this mirrors the existing RetryOnConflict usage in
workloadmonitor_controller.go.
Surfaced by the etcd.bats e2e test, which applies the same singleton
Etcd resource three times back-to-back and intermittently failed the
third apply with the 409.
Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
(cherry picked from commit 76410599de)
The Update handler rebuilds the HelmRelease from the Application, which
re-stamped sharding.fluxcd.io/key with the ApplicationDefinition default
and bounced the HelmRelease off its operator-assigned shard on every
update. Carry the live label over from the current HelmRelease instead.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
The Watch goroutine never closed the events channel it owns, and
ProxyWatcher.Stop does not close the channel it proxies. When the
backing watcher ended, consumers ranging over ResultChan blocked until
their own timeout instead of seeing end-of-stream. Closing the channel
on goroutine exit matches the watch.Interface contract and the pattern
already used by the tenantmodule and application registries.
Goroutine-termination tests now assert the result channel closes.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Four goroutine-termination tests duplicated the same IsStopped polling
loop; waitForFakeWatcherStop keeps the deadline logic in one place.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Manual runtime.NewScheme setups in the List/Get error-path tests silently
discarded AddToScheme errors; the shared helper fails the test instead.
Renamed from newWatchTestScheme since it now serves both test files.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Cover the per-event filtering paths of REST.Watch directly instead of
only transitively through the bookmark test: authorization filtering via
RoleBindings, privileged groups (system:masters, cozystack-cluster-admin),
non-tenant namespace and foreign object drops, backing bookmark
forwarding, field and label selector filtering, missing user rejection,
access-check error resilience, ADDED deduplication by resourceVersion,
terminating bookmark flush on backing watcher close, and goroutine
shutdown on Stop and context cancellation.
Also close the remaining gaps on the sibling read paths sharing the same
authorization logic: List/filterAccessible (per-user filtering, privileged
groups, missing user, RoleBindings outside the tenant set, backing list
errors) and the Get error branches.
Branches the controller-runtime fake client cannot drive (backing
bookmarks, non-Namespace objects, chosen resourceVersions, backing watch
errors) are exercised through a stub client.WithWatch backed by a
test-owned FakeWatcher.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
## What this PR does
While preparing for my CozySummit talk of last week (creating commits
from CozySummit edits with
[gitops-reverser](https://github.com/ConfigButler/gitops-reverser)) I
noticed warnings on my console:
```
Warning: event bookmark expired err="reflector.go:343: hasn't received required bookmark event marking the end of initial events stream, received last event 59.998946102s ago"
The warning fires on every reflector restart and continuously while the cache is unsynced from the WatchList perspective.
```
I've been searching a bit better and it turns out that `TenantSecret`,
`TenantModule`, and `TenantNamespace` (group `core.cozystack.io`) all
ignore `ListOptions.SendInitialEvents`. Interestingly enough
`apps.cozystack.io/Application` already does implement this in the right
way.
Under the [WatchList / streaming-list
protocol](https://kubernetes.io/docs/reference/using-api/api-concepts/#streaming-lists)
(client-go's `WatchListClient`, default-on since v1.35), clients expect
the initial `ADDED` events to be terminated by a
[`Bookmark`](https://kubernetes.io/docs/reference/using-api/api-concepts/#watch-bookmarks)
annotated with `k8s.io/initial-events-end: "true"`. These three
resources only forwarded the backing watcher's bookmarks (which lack the
annotation), so the marker was never sent and client-go reflectors never
reached `HasSynced` — looping on `reflector.go:343: hasn't received
required bookmark event marking the end of initial events stream`.
**Fix:** extracted the contract into a shared
`registry.InitialEventsBookmarker` helper (`pkg/registry/utils.go`) and
wired it into all four `Watch` implementations. It annotates the first
backing bookmark as the terminating marker, with fallbacks to emit it
before the first live event and on watcher close. The annotation reuses
the upstream `metav1.InitialEventsAnnotationKey` constant rather than a
string literal.
The full bookmark machinery in `k8s.io/apiserver/pkg/storage/cacher`
isn't reusable here: it's internal to the watch-cache layer and only
fires for cacher-backed storage, whereas these resources translate a
controller-runtime watch in custom REST storage that bypasses the
cacher.
`Application` is refactored onto the helper with its `WorkloadMonitor`
event-draining preserved. Adds unit tests for the helper and watch tests
for each core resource (`ADDED` → annotated `Bookmark` → live event).
### Screenshots
N/A — no UI changes.
### Release note
```release-note
fix(api): core.cozystack.io resources (TenantSecret, TenantModule, TenantNamespace) now emit the k8s.io/initial-events-end bookmark for SendInitialEvents=true (WatchList) watches, so client-go informers reach HasSynced instead of looping on the missing-bookmark warning.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Watch endpoints reliably emit the "initial-events-end" bookmark after
initial events, improving watch/list contract compliance and client
reflector synchronization.
* **Refactor**
* Watch implementations consolidated around a centralized initial-events
bookmarker and stop-aware event forwarding for consistent bookmark
behavior.
* **New Features**
* Introduced a reusable initial-events bookmarker to coordinate bookmark
emission and resourceVersion observation across translated watch
streams.
* **Tests**
* Added unit and integration tests validating bookmark emission,
ordering, resourceVersion seeding/observation, and close-time behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
IvanHunters