Commit Graph
3 Commits
Author SHA1 Message Date
Aleksei Sviridkin 115d9bbaaf fix(installer): let the operator Deployment report Available honestly
At replicas: 1 with maxUnavailable: 1 the Deployment controller takes
the minimum available replica count as 1 - 1 = 0, so the Available
condition reports True with no running pod at all. That reading is what
every consumer of the status sees: an alert on Available, a health
dashboard, or a person reading kubectl get deploy all show green while
the operator is dead.

The replacement is not a tuned pair, it is the platform default written
down. At replicas: 1 the default 25% / 25% resolves to maxSurge 1 and
maxUnavailable 0, because the surge rounds up and the unavailability
rounds down. So this change stops overriding the default rather than
introducing a configuration of its own, and Kubernetes rejects
maxUnavailable: 0 together with maxSurge: 0, so the surge could not
have gone the other way regardless.

The overridden pair was carried over from the earlier cozystack
Deployment, where a darkhttpd sidecar published a host port and
hostNetwork left a surge pod unschedulable on a single-node cluster. It
arrived here in two steps: by the commit before this file was created
the sidecar was already gone and no port was declared, yet the pair
stayed, so it had outlived its own reason before it was ever copied.
That is why looking for a justification in this file's history finds
none. With no port declared anywhere in the pod there is no hostNetwork
port conflict left, so the surge pod schedules beside the outgoing one;
declaring one reintroduces exactly the scheduling failure the pair was
added for. Not exercised on a live cluster in this rework.

Stay on RollingUpdate rather than move to Recreate, which would fix the
condition too, and by a shorter route: MaxUnavailable() reports 0 for
any strategy that is not RollingUpdate, so the minimum available
replica count becomes replicas - 0 = 1 and the condition is honest
without any surge pod at all.
The transition was not attempted, and the failure this project fixed in
ouroboros 0.8.1 does not transfer to it. There the rollingUpdate block
was the one the apiserver defaults onto a Deployment created without
one, so it belonged to no applier and outlived the switch to Recreate.
This Deployment has carried an explicit block since the commit that
created it, and nothing applies it server-side, so whether Recreate
would upgrade cleanly is an open question about the field-ownership
history of the live object rather than a known rejection. What keeps
RollingUpdate here is availability rather than apply semantics:
maxUnavailable: 0 leaves the outgoing operator running when the
replacement never starts, where Recreate deletes it first and leaves
none. Editing the numbers inside the block is
upgrade-safe from every state the field can already be in.

The surge introduces new windows, and leader election covers less of
them than it looks. The operator runs four privileged install steps
before mgr.Start(), and the lease is only acquired inside mgr.Start():
the CRD install, the Flux install, the platform source resource and the
platform PackageSource. All four therefore run in the incoming pod
outside leader election entirely. What the lease does cover is the
reconcile loops, the part that would otherwise conflict continuously
instead of once at startup.

That leaves three seams, and only the first two happen on a healthy
upgrade. The routine one is the incoming pod's install steps against
the outgoing pod's live reconcile loops, since the outgoing pod ran its
own installs at its startup and has been inside mgr.Start() ever since.
Those do not contend for the same fields: the installs write as field
manager cozystack-operator, while the reconcilers write as
cozystack-packagesource-controller and cozystack-package-controller and
go mostly to status subresources. Different owners, different objects,
so this seam is safe by separation rather than by any shared manager.

The second is version skew rather than field contention, and maxSurge:
1 widens it rather than opening it. Under maxSurge: 0 the surge budget
counts desired replicas rather than live pods, so the incoming pod is
created while the outgoing one may still be Terminating - the overlap
existed. What it did not have was a live counterparty: scale-down
sends SIGTERM before the new pod is created, and that cancels the
manager context, so the old loops were already unwinding. Here the
outgoing pod is not signalled until the incoming one is available, so
the overlap runs against a fully live reconciler for a termination
grace period instead of a vestigial one. The incoming pod reports
Ready the instant its
container starts, because there is no readiness probe, so its CRD
install runs while the outgoing binary's reconcile loops are still live
and now writing against a schema the incoming pod has already moved.
Old code against a new schema, which no field manager separates. It is
bounded rather than benign: the outgoing pod exits on SIGTERM within
seconds, a rejected write is transient and the next reconcile repairs
it, and no object is left corrupt. Widening the skew, by adding a probe
that delays Ready or by a migration the old code cannot round-trip, is
what would make this seam matter.

The third seam is install against install, and it needs the outgoing
pod to be restarting, since a crashlooping pod re-runs its four steps
on every restart, which is exactly the broken operator this gate exists
to catch. Three of the four steps use server-side apply under the same
field manager, so they never conflict and resolve last-writer-wins
whenever the two pods carry different manifests. That ordering matters
in one direction, and it is accepted here rather than guarded against:
the CRD install and the Flux install both apply with Force: true, so an
outgoing pod that writes last silently overwrites what the incoming pod
has just applied, with no conflict and no error to exit on. The
incoming pod is inside mgr.Start() by then and will not apply again
until it restarts, so that revert of the upgrade's install phase
persists rather than healing itself. Reaching it needs a restart
landing in the seconds between the incoming pod's installs and the
outgoing pod's SIGTERM. installPlatformSourceResource is the remaining
exception, a read-modify-write of a Get followed by Create or by Update
with the observed resourceVersion, so two pods running it at once can
collide on AlreadyExists or on a conflict; that failure is loud and
self-healing, since any error there exits the process and the pod
restarts, and the object is not left corrupt.

Pin the strategy and all four preconditions the surge makes
load-bearing in a unit test: no container or initContainer declares a
port, both listener arguments stay disabled, leader election stays on,
and no probe uses httpGet. They fail differently and none is visible to
the others, since a declared port stops the scheduler, a bound one only
fails once the incoming process starts, and a second manager without a
lease fails nothing at all until it writes. The httpGet precondition
follows from the two disabled listeners leaving no port to target, and
it is the worst of the four to get wrong: the other three wedge an
upgrade, while a livenessProbe against a disabled listener crashloops
the pod on every cluster, including a fresh install where no surge
happens. Only the httpGet field is forbidden rather than the probes
themselves, because an exec probe stays legitimate. All four are
checked against all three variants, because the template branches on
variant inside the container spec and a port added under one branch
renders for that variant alone. The strategy type is pinned alongside
its two values, since maxSurge and maxUnavailable say nothing on their
own once the type is no longer RollingUpdate.

The listener preconditions are pinned less tightly than the other two,
and the test says so rather than implying coverage it does not have. A
membership assert has no notion of position while Go's flag package
keeps the last occurrence, so a duplicate after the disabled one wins
and the assert still passes, while a duplicate before it loses and the
assert passes just the same - the test cannot tell those two apart.
Moving the disabled flag last fixes the process without making the
assert catch anything. Rejecting the two addresses this
binary defaults to closes the shape a values-gated block to enable
metrics would take, and nothing available here closes an arbitrary
address, since helm-unittest cannot express that a flag appears at most
once.

Note in hack/helm-unit-tests.sh that the loop covers packages/core,
which its comment omitted. The suite added here lives there, so a later
edit reconciling the loop with the stale comment would have silently
stopped running it.

Split the rationale by audience rather than keeping it all in shipped
comments. What an operator applying the rendered manifest has to respect
stays in plain comments and travels into the install artifacts: that the
pair is the default, why maxUnavailable must stay 0, that no port may be
declared or bound, that no probe may use httpGet, and that leader
election has to stay on. Most of what stays in the template comment is
what the chart's own suite pins, so it cannot drift out of step with the
chart without a test going red: the four preconditions and which assert
covers each, and why the variant loop is needed. Three things there are
deliberate exceptions, kept because anyone editing the strategy has to
know them and no assert can carry them - that the webhook server
configured on 9443 is not a port binder, since controller-runtime adds
it to the runnables only inside GetWebhookServer() and this binary never
calls it; that two overlaps between the pods sit outside the lease; and
why RollingUpdate is kept rather than Recreate. Those name what exists.
The seam analysis above, which is how each one works, deliberately stays
here instead. It describes another file's internals, nothing pins it,
and a commit message is fixed in time, so it cannot quietly fall out of
date the way a comment can.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-08-05 18:15:41 +03:00
Aleksei Sviridkin d2e851f63b feat: Gateway API support via Cilium
Replace ingress-nginx as the default ingress layer for cozystack-native
services with Cilium Gateway API, materialised per-tenant via a new
TenantGateway CRD reconciled by cozystack-controller.

The extra/gateway chart renders one gateway.cozystack.io/v1alpha1
TenantGateway per tenant; the controller owns the resulting Gateway,
per-tenant Issuer, Certificate(s), and the http->https redirect
HTTPRoute. This avoids the helm-vs-controller race on
Gateway.spec.listeners when route-driven listeners are added.

publishing.certificates.solver selects HTTP-01 (default; per-app HTTPS
listener + per-app Certificate) or DNS-01 (opt-in; single wildcard
Certificate per apex; providers: cloudflare, route53, digitalocean,
rfc2136). HTTP-01 keeps zero-config per-app onboarding; DNS-01 saves
Let's Encrypt budget on apex-shared clusters.

Security model -- seven independent layers, all admission-time gates
fail-closed:

  1. Listener allowedRoutes namespace whitelist via the
     kube-apiserver-written kubernetes.io/metadata.name label;
     plain-HTTP listener is strictly narrower than HTTPS, and HTTPS /
     passthrough listeners restrict allowedRoutes.kinds to
     HTTPRoute / TLSRoute respectively.
  2. cozystack-gateway-hostname-policy VAP on Gateway.
  3. cozystack-gateway-attached-namespaces-policy VAP on Package --
     blocks tenant-* in gateway.attachedNamespaces.
  4. cozystack-tenant-host-policy VAP on Tenant -- blocks spec.host
     writes from non-trusted callers.
  5. cozystack-namespace-host-label-policy VAP on Namespace -- blocks
     writes to namespace.cozystack.io/host from non-trusted callers.
  6. Render-time fail in cozystack-basics for tenant-* in
     gateway.attachedNamespaces.
  7. cozystack-route-hostname-policy VAP on HTTPRoute / TLSRoute,
     scoped to tenant-* namespaces.

Within-apex cross-namespace hostname conflict is resolved at reconcile
time by a HostnameConflict condition (cozy-* namespace wins).

Five reconcile paths refuse to rewrite pre-existing objects with the
controller-derived name but no OwnerReference back to the
TenantGateway: Gateway, redirect HTTPRoute, per-tenant Issuer, wildcard
Certificate, per-listener Certificate.

Derived-apex tenants (tenants whose host is a subdomain of a parent
tenant's) auto-enable Gateway when the platform-level gateway.enabled
is on.

Bonus fix: pkg/registry/apps/application/rest.go was ignoring the
createValidation and deleteValidation callbacks that genericapiserver
hands to every storage method, silently bypassing admission for Create
and Delete on every apps.cozystack.io resource (Tenant, MariaDB,
Postgres, Kubernetes, ...). Aligned with the existing updateValidation
pattern.

Always-on changes: cilium.envoy.enabled + cilium.gatewayAPI.enabled
flip to true (extra cilium-envoy DaemonSet, ~100 MB RAM/node idle),
and the cozystack-api admission-chain fix. Everything else is gated
behind gateway.enabled=false and tenant.spec.gateway=false; existing
clusters see no behavioural change until an operator opts in.

Tests: 60+ Go cases in internal/controller/tenantgateway/, admission-
chain regression tests in pkg/registry/apps/application/, helm-unittest
across 7 charts (extra/gateway, core/platform, apps/tenant,
system/cozystack-basics, system/cert-manager-issuers,
system/cozystack-api, system/dashboard), and 14 e2e bats scenarios in
hack/e2e-apps/gateway.bats. New make test-controllers target wired
into pull-requests.yaml.

Deps: sigs.k8s.io/gateway-api v1.4.1, cert-manager v1.17.4.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-05-25 17:07:02 +03:00
Timofei Larkin 1c9ae2bec5 [ci,dx] Add unit tests for cozy-lib
## What this PR does

The cozy-lib library package got complicated enough to warrant its own
unit tests. Since unit tests are a "good thing" (tm), a somewhat generic
framework for running all kinds of unit tests was introduced into the CI
pipeline and Makefile targets. For now all it runs is `make test`
against the `packages/{library,apps,system,extra}/*` directories,
wherever a `test` target is present in the Makefile, and for now this is
only for the `cozy-lib` Helm library chart.

### Release note

```release-note
[ci,dx] Introduce a scaffold for running unit tests locally and in CI
and add the first unit tests for the cozy-lib helper Helm chart.
```

Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
2025-11-19 17:56:17 +03:00