## What this PR does
Seven guides under `docs/` were operator facing and written alongside
the code, and left in the source tree where no user looks. They are on
the website now, so this deletes them here and repoints every reference.
```
docs/operations/backup-classes.md -> operations/services/backup-classes
docs/operations/resource-presets.md -> guides/resource-management
docs/storage-immutability.md -> operations/storage-class-immutability
docs/operations/system-memory-limits.md -> operations/configuration/system-memory-limits
docs/gateway-classes.md -> networking/gateway-classes
docs/observability/distributed-tracing.md -> operations/services/monitoring/distributed-tracing
docs/operations/seaweedfs-capacity-for-backups.md -> operations/services/object-storage/capacity-for-backups
```
`docs/release.md`, `docs/deprecating-applications.md` and the two
SeaweedFS runbooks in `docs/operations/` stay. A workflow, `AGENTS.md`,
a migration and a bats suite read them by path, which is the test from
#3777.
References point at `https://cozystack.io/docs/v1/...`. The site
rewrites that short prefix to the latest version in `layouts/404.html`,
so the links follow releases and pin none. Backup Classes, resource
management and `storageClass` immutability exist in the latest version
already. The other four pages are only in `next/` until 1.7.0 is
promoted, and their links start to resolve then. The generated mongodb
files come from `values.yaml` through `make generate` with
cozyvalues-gen v1.7.0, the version pinned in
`.github/workflows/pre-commit.yml`.
Tests: helm unit tests for mongodb (151), postgres (209), vm-disk (114)
and kubernetes-nodes (223), go tests for backupcontroller, presets and
the operator, rd-presets-check, the migration-50 bats suite.
### Downstream repositories
- [x] [cozystack/website](https://github.com/cozystack/website) -
follow-up: cozystack/website#763
Walked the rest of the trigger map against the diff. No provider
follow-up: the mongodb `values.schema.json` diff is one `description`
string, with no field added, removed or renamed, no enum, no default,
and no `kind`, `plural` or `release.prefix` change.
`packages/core/platform/values.yaml` changed in comments only and no
key, so the hand written platform-package table on the site is
unaffected.
Please merge after cozystack/website#763, the links in the code only
resolve once those pages deploy.
### Release note
```release-note
docs: seven guides move from the docs/ directory of this repository to cozystack.io (Backup Classes, resource presets, storageClass immutability, system component memory limits, GatewayClass selection, distributed tracing, SeaweedFS capacity for backups), and in-repo references point there.
```
Seven guides in docs/ were written next to the code and left in the
source tree, where no operator looks. The website carries them now, so
a second copy here only lets the two drift again: the Backup Classes
page had already fallen behind its original.
docs/release.md, docs/deprecating-applications.md and the two SeaweedFS
runbooks in docs/operations stay. A migration, a bats suite, a workflow
or AGENTS.md reads each of them by path, which is the test for keeping
a document in this repository.
Assisted-by: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The case that resizes the other components looked only at nginx and
the portal, so it would keep passing if those overrides stopped
reaching the nested release, and then it could not catch nginx or the
portal following another component's values. It now checks the core,
registry and jobservice limits it sets and leaves trivy alone, as
nothing in the case reads it. The portal case also checks its memory
request, as the nginx case does.
Assisted-by: LLM
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
Both containers run nginx with worker_processes auto, so their idle
memory follows the CPU count of the node, whatever their own CPU limit
is. In a tenant namespace a container without resources gets the
LimitRange default of 128Mi, which the proxy outgrows on a node with
many CPUs before it serves a request. The presets that keep both out
of that default have no test, so dropping them or tying them to
another component's values would go unnoticed.
Assisted-by: LLM
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
## What this PR does
Adds `managementWhitelist` to the Harbor app, a list of IP addresses or
CIDR ranges allowed to reach the web portal and the Harbor API, and I
made it a typed list on purpose. The obvious alternative, a free-form
`ingress.annotations` map, gives tenants a way into the shared ingress
controller. The platform ingress-nginx runs with
`allowSnippetAnnotations: true` (the kubernetes app needs a
`configuration-snippet`), and v1.11.5 defaults `annotations-risk-level`
to `Critical`, so it accepts any annotation, `server-snippet` included.
Tenants can't create Ingress objects themselves, but they can write any
field of an app spec. An annotation map on the Harbor spec would let any
tenant put arbitrary nginx config into a controller that serves every
tenant. A typed list is also what the repo already does for this:
`filer.whitelist` in `extra/seaweedfs`, `whitelist` in `extra/ingress`
and `tcp-balancer`.
The use case is a registry that serves customers: pulls and pushes come
from anywhere, the admin UI and API should answer only to an office or
VPN range. Right now the portal, `/api/v2.0` and `/v2/` sit on one
Ingress and one public host, and a tenant can't restrict any of them.
With a non-empty list the existing Ingress gets
`nginx.ingress.kubernetes.io/whitelist-source-range`, and `/v2` and
`/service/token` move to a second Ingress, `<release>-registry-ingress`,
on the same host. ingress-nginx applies that annotation only to the
locations of the Ingress that carries it, so those two stay open. They
are what a registry client needs: Harbor answers an anonymous `/v2/`
request with a Bearer challenge whose realm is `/service/token` on the
same host. Everything else under `/service/` is
`/service/notifications/*`, internal job callbacks that the bundled
nginx already answers with 404, so it stays on the restricted side. The
second Ingress uses the same TLS secret and has no cert-manager
annotations, otherwise ingress-shim gets two owners for one Certificate.
The HTTP-01 solver is its own Ingress and is not restricted. With the
default empty list the chart renders exactly what it renders today.
An HTTPRoute can't express a source restriction, so in Gateway API mode
a non-empty list fails the render. Dropping it silently would leave the
portal public while the spec says it is restricted.
Each entry has to be an IP address or CIDR range, and a bad one fails
the render. ingress-nginx reads an empty `whitelist-source-range` as no
restriction and turns an unparseable one into a 503 for the whole
location, and the values schema generator can't validate list items.
Three things users should know. Harbor-to-Harbor replication that pulls
from this registry goes through its API, so the other registry's address
has to be on the list. The check uses the client address that
ingress-nginx sees: with `externalTrafficPolicy: Local` or PROXY
protocol that is the real client, behind another proxy it is the proxy.
And for the portal and API the list replaces the tenant ingress
`whitelist` rather than narrowing it.
Disabling the portal is not here. The vendored Harbor chart (1.19.1)
always renders the portal Deployment and its internal nginx always
proxies `/` to it, so there is no small switch to wire.
The new Ingress is added to the dashboard Role and to the ingress
selector in the ApplicationDefinition.
### Screenshots
No UI code change. The field reaches the dashboard form through the
regenerated schema and `keysOrder`.
### Downstream repositories
The terraform provider models Harbor by hand, so the new field needs a
follow-up there: cozystack/terraform-provider-cozystack#46. The website
reference page is generated from `README.md` on release. Nothing else in
the trigger map is touched: no package added or renamed, no platform,
installer or `hack/` change.
- [ ] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [x]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
https://github.com/cozystack/terraform-provider-cozystack/issues/46
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:
### Release note
```release-note
feat(harbor): add `managementWhitelist`, a list of IP addresses or CIDR ranges allowed to reach the Harbor web portal and API. Only `/v2/` and `/service/token` stay reachable from any address. The default is an empty list, which keeps the current behaviour. Not supported with Gateway API publishing: a non-empty list fails the render there. Each entry must be an IP address or CIDR range; for the portal and API the list replaces the tenant ingress `whitelist`.
```
Add per-template guard suites so each guarded hostname site is exercised
on its own: harbor harbor.yaml and httproute.yaml, bootbox machines.yaml,
seaweedfs cosi-deployment.yaml, bucket deployment.yaml and httproute.yaml,
monitoring alerta.yaml, grafana httproute.yaml and the grafanaHost helper
via oidc-keycloak.yaml. Each suite renders only its target template, so
removing that file's guard reddens it. Also add a postgres positive case
that renders the Certificate with an apex set (external=true).
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Charts that publish a service compose their hostname as
<sub>.<_namespace.host>. When the injected tenant apex is empty and no
per-app host override is set, they used to emit a malformed trailing-dot
hostname (e.g. "harbor.") into Ingresses, HTTPRoutes and Certificate
dnsNames silently. Source the apex through cozy-lib.ns-host-required at
each hostname-emitting site so the render fails with a clear message
instead, matching the gateway path.
Covered: harbor, postgres (external), qdrant (external), kubernetes
(control-plane endpoint), vpn, seaweedfs, bootbox, monitoring
(grafana, alerta, oidc client), and system/bucket (adds the cozy-lib
subchart symlink it lacked). The guard is placed at the point each
resource is actually emitted, so standalone renders, no-exposure
tenants and the kubernetes no-etcd beacon path are unaffected. The
post-delete cleanup hooks are left unguarded so an empty apex cannot
block uninstall.
Behavior change: a tenant with an empty injected apex and no host
override that enables one of these components now fails its render
(HelmRelease NotReady) instead of deploying a broken hostname. In
practice the apex is always injected (root has root-host, children
inherit it), so this only trips a genuinely misconfigured tenant.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
A per-Ingress whitelist-source-range replaces the controller-wide
default that the tenant ingress whitelist sets, so for the portal and
API the Harbor list is not an intersection: an address the tenant
policy excluded gets in when it is on this list.
The description also promised that every other path is restricted,
which holds only while the controller sees the real client address.
Behind a proxy or with a header-derived client address the check runs
against that instead.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
ingress-nginx reads an empty whitelist-source-range as no annotation
and falls back to the open default, so managementWhitelist: [""]
rendered a split that looked restricted while the portal and API
stayed public. One unparseable entry went the other way and turned
the whole portal location into a 503.
The values schema generator has no item-level constraints, so each
entry is checked at render time and a bad one fails the release with
the offending value in the message. IPv4 is matched strictly; IPv6 by
character set only, which still leaves a malformed IPv6 entry failing
closed in the controller.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Harbor serves its web portal and management API on the same host as
the registry endpoints, and a tenant had no way to limit who reaches
the portal and API without also cutting off image pulls.
managementWhitelist restricts every path except /v2 and
/service/token, which move to a second Ingress on the same host, so
docker login, pull and push keep working from any address. The field
is a list of ranges rather than a free-form annotation map: the
shared ingress-nginx accepts snippet annotations, so arbitrary
annotations from a tenant-writable spec would reach its nginx
configuration.
Gateway API publishing has no equivalent, so a non-empty list fails
the render there instead of being silently dropped.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
With watch granted, kubectl delete waits for the claims to be gone, and
its default for that wait is a week. A claim held by a pod that will not
stop, or a stuck detach, would hold the post-delete hook until
helm-controller's 300s uninstall budget ran out and leave the
HelmRelease stuck uninstalling.
The PVC deletes in the clickhouse, harbor, mariadb and qdrant hooks now
wait 120s at most and report the claims still terminating by name, and
each Job carries activeDeadlineSeconds 270 with a 10s termination grace
period, the numbers packages/apps/tenant/templates/cleanup-job.yaml
documents against that budget, so a kubectl that never returns cannot
hold the hook either.
The mariadb Role no longer grants watch on the named Secrets: they carry
no finalizer, so the delete never reaches the watch, and a resourceNames
rule does not authorize a collection watch anyway.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
Assisted-by: LLM
kubectl delete waits for the object to disappear unless told otherwise,
and that wait is a watch. The post-delete hook Roles of harbor,
clickhouse, mariadb and qdrant granted get, list and delete only, so
every uninstall logged a forbidden watch, the wait was skipped and the
hook reported success before the PVCs were actually gone.
The rules now carry watch for every resource the hooks delete, including
the named Secrets the mariadb hook removes.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
With expose.type=clusterIP the goharbor chart deploys an nginx proxy pod, but
the chart set no resources for it. In a tenant namespace the LimitRange defaults
any container without resources to a 128Mi memory limit, so the proxy is
OOMKilled (exit 137) and Harbor never comes up.
Set the nginx proxy to t1.small (512Mi) and bump the portal (also nginx-based)
from t1.nano (128Mi) to t1.micro (256Mi) for the same reason.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
The qdrant and harbor post-delete hooks guarded every delete with
`|| echo "WARNING: ..." >&2`, set no `-e`, and ended on an
unconditional completion message, so the container exited 0 on
every path. Helm read that as a successful hook and the
hook-succeeded delete policy removed the Job together with the pod
that carried the warning, leaving an uninstall that reclaimed
nothing indistinguishable from one that had nothing to reclaim.
Count the failures instead and set the exit code at the end, so a
failed delete does not skip the deletes that follow it. An object
that is already gone stays a success: every delete keeps
--ignore-not-found, which exits 0 in that case. This is the shape
packages/apps/mariadb already uses for the same problem.
Both hooks also consumed the output of `kubectl get` directly --
harbor by iterating it, qdrant by piping it into grep, whose status
the pipeline then reports. A listing that failed therefore arrived
as an empty list and the sweep was skipped rather than reported.
Each listing is now captured before it is used, so a listing that
failed and one that returned nothing are different events.
A hook that exits non-zero keeps its Job and pods, which is the
point: they hold the log. Give the Job one retry rather than the
Kubernetes default of six or none at all. Every delete is
idempotent, so a second attempt re-issues an API call rather than
re-running any logic, and one short backoff is what separates a
transient apiserver error from a real one -- these failures become
visible for the first time here, and a signal that fires on noise
stops being read, including on the day it is right. Six is the far
end of the same mistake: ten minutes of backoff across seven pods
outlasts the hook timeout, and Helm reports that timeout instead of
the hook's own error.
The qdrant Chainsaw suite explained its post-delete pod collector
by quoting the guard string removed here and concluding the pod is
always collected away. That is now backwards: a failed cleanup is
exactly when the pod survives and the collector is the evidence.
Rewrite the note, and the sentence in the harbor suite pointing at
it.
hack/app-cleanup-hooks_test.bats renders both scripts and runs them
against a fake kubectl. There are eight failure paths across the two
hooks -- six in harbor, two in qdrant -- and each has a case that
goes red when that one path alone is reverted to a swallowed
warning. Message assertions are anchored on the hook's own `ERROR: `
prefix, because the fake kubectl names the object in its own
diagnostic too and an unanchored match is satisfied by the fixture.
The helm-unittest suites pin the retry limit and the hook delete
policy: adding hook-failed to that annotation hands the Job back to
Helm to delete on failure and destroys the evidence this change
exists to keep, and until these asserts that reversal left both
suites green.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
<!-- Thank you for making a contribution! Here are some tips for you:
- Use Conventional Commits for the PR title: `type(scope): description`
- Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore
- Scopes are not an exhaustive list — pick the most specific scope for
the change and extend the list when a genuinely new area appears.
Examples:
- System components: dashboard, platform, operator, cilium, kube-ovn,
linstor, fluxcd, cluster-api
- Managed apps: postgres, mariadb, redis, kafka, clickhouse,
virtual-machine, kubernetes
- Development and maintenance: api, hack, tests, ci, docs, maintenance
- Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or
add a `BREAKING CHANGE:` footer
- If it's a work in progress, consider creating this PR as a draft.
- Don't hesistate to ask for opinion and review in the community chats,
even if it's still a draft.
- Add the label `backport` if it's a bugfix that needs to be backported
to a previous version.
-->
## What this PR does
Harbor's app chart renders a nested HelmRelease that owns the whole
workload: the Deployments, the inline RedisFailover and its volume. It
had no install strategy, so it got Flux's default, and install
remediation is an uninstall in every case Flux offers. A first install
that misses its budget gets torn down, and the next one provisions the
redis volume and runs the master election again from scratch.
That happens on a loaded runner. harbor-core waits on redis, redis waits
on its volume and the master election, and the nginx Deployment hits the
default `progressDeadlineSeconds` of 600s. Flux reads that as a stalled
resource and fails the install early. The e2e wait budget is 10m, the
same as the deadline, so after a teardown the second attempt has less
time than a first install needs.
`RetryOnFailure` keeps the manifests applied and retries as an upgrade.
Nothing is deleted, so the volume, the elected master and harbor-core's
startup all survive, and the retry costs 30s instead of a full install.
A Deployment clears `ProgressDeadlineExceeded` by itself once its pods
go ready, so the retry lands as soon as the stack converges.
This drops the teardown, not the deadline. The suite applies the release
in one step and waits for it in later ones, so part of the budget is
still left when the fail-early hits. That leftover is useless to a fresh
install and usable to a retry that resumes. Whether the deadline or the
budget also needs moving is a separate question, and an early deadline
only becomes safe once a failed install stops being torn down. In
production the win holds without any of that: a slow first install no
longer deletes the Redis volume and the database.
Both actions carry the strategy. The controller runs the retry of a
failed install as an upgrade, so with install alone the retry's own
failure falls through to upgrade remediation. The remediation blocks
stay: an absent release consults install, an out-of-sync one consults
upgrade, both by retry count rather than by strategy.
It goes on the nested release, not the parent. The parent is built from
the Application by the apiserver, which already stamps this strategy on
both actions. The nested one owns the workload and the volume a
remediation deletes.
Same shape as the tenant CNI and CSI releases in #3552 and #3581, and
the unit test follows theirs. Relates to #3445.
### Screenshots
<!-- REQUIRED for UI changes: attach screenshots or screen recordings
demonstrating
the visual impact of your changes. PRs with UI changes without
screenshots will not be merged. -->
### Downstream repositories
<!--
Cozystack is upstream for repositories that are NOT kept in sync with it
automatically, and nothing in CI compares the two sides, so a change
here can break them silently.
Which change forces what, and which file to touch, is the trigger map in
docs/agents/contributing.md under "Downstream Repositories". Walk it
before you tick anything.
IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM
TO FILL IN:
1. Walk the trigger map against the actual diff of this PR, file by
file. Do not decide from the PR title or from your memory of what you
changed.
2. For every repository you tick, open the follow-up PR there and link
it on the same line. A ticked box with no link claims work that does not
exist, which is worse than an empty box. Search that repository for an
open PR or issue covering it first, and link that instead of filing a
duplicate.
3. When the follow-up is out of scope, or needs a decision that is not
yours to make, open an issue in that repository instead, link that, and
say so in the PR body. Do not push a speculative PR into another
repository just to fill a line here.
4. Do not tick "No downstream repository is affected" to make the
checklist go away. If you are unsure, leave every box empty and say so
in the PR body, so a human decides.
-->
- [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:
### Release note
<!-- Write a release note:
- Explain what has changed internally and for users.
- Start with the same `type(scope):` prefix as in the PR title
- Follow the guidelines at
https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md.
-->
```release-note
fix(harbor): retry a failed Harbor install instead of uninstalling the release, so a slow first install no longer deletes the Redis volume and starts over
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Harbor installations and upgrades now automatically retry every 30
seconds when failures occur.
* Existing unlimited remediation retries remain available for both
operations.
* **Tests**
* Added automated coverage to verify Harbor retry behavior and
remediation settings.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Both suites addressed their subject by document index. An index is a
position, so a template that gains a document ahead of the one under test
moves every later assertion onto a different object, and what happens then
depends on the assertion: measured by inserting one document at the head of
each template, the cleanup suite reddens outright, while two of the four
jobservice tests keep passing because a `notExists` against an object that
never had the path is satisfied for the wrong reason.
Indices are also easy to get wrong by hand. helm-unittest numbers documents
in template source order, while `helm template` prints them sorted by kind,
so the rendered output is not the order being counted.
Every document these suites assert on is unique by kind within its template,
so each test now names its subject. A selector that matches nothing fails,
including on the `notExists` tests, so the anchor holds in the direction that
was silent before.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The harbor app chart renders a nested HelmRelease that owns the whole
workload: the harbor Deployments, the inline RedisFailover and its
volume. It carried Flux's default install strategy, and install
remediation is an uninstall in every case Flux offers, so a first
install that went over budget was torn down between attempts and the
next one re-provisioned the redis volume and re-ran the master election
from zero.
That is reachable on a loaded runner. harbor-core blocks on redis while
redis waits for its volume and its master election, and the nginx
Deployment reaches the default progressDeadlineSeconds of 600s, which
Flux reads as a stalled resource and fails the install early. The e2e
wait budget for the release is the same 600s, so one teardown cycle
consumes all of it and the second attempt cannot finish however fast it
converges.
RetryOnFailure keeps the manifests applied and retries the failed
release, so the convergence already under way continues instead of
restarting. Both actions carry the strategy because the controller
performs the retry of a failed install as an upgrade, so on install
alone the retry's own failure falls through to upgrade remediation. The
cost is that a real upgrade failure retries rather than rolls back,
which gives up no stable safety net: with retries: -1 and the default
strategy, upgrade remediation already rolled back and retried without
end. The remediation blocks stay because an absent release consults
install and an out-of-sync one consults upgrade, both by retry count
rather than by strategy.
The change lands on the nested release rather than on the parent: the
parent is built from the Application by the apiserver, which already
stamps this strategy on both actions, while the nested release is the
one that owns the workload and the volume a remediation deletes.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The prerequisites section described the resolved SeaweedFS class as being
surfaced on the tenant namespace as the `namespace.cozystack.io/seaweedfs`
annotation. The tenant chart renders that key under `metadata.labels`,
alongside the sibling `/etcd`, `/ingress`, `/gateway`, `/monitoring` and
`/host` keys; no annotation of that name exists.
The wrong noun sends an operator to the wrong field. When a Harbor release
stays unreconciled waiting on `BucketInfo`, the documented check is to
confirm the tenant chain resolved a SeaweedFS class. Looking for it in
`.metadata.annotations` returns nothing even on a correctly configured
tenant, which invites the wrong conclusion that the tenant chain is broken
and sends the investigation away from the actual cause.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The post-delete cleanup Jobs in the harbor and mariadb charts hardcoded
`docker.io/clastix/kubectl:v1.32` — a moving tag pulled directly from
Docker Hub. This broke build reproducibility (the tag can be re-pushed),
was a supply-chain risk, and could not be satisfied on air-gapped /
mirrored clusters, where it failed at the worst moment: during uninstall,
when the post-delete hook runs and stalls release teardown.
Add a canonical `cozy-lib.image` helper that prefixes a registry-relative
image path with `cozy-lib.images-registry` when set and returns it
unchanged (no leading "/") when empty, so standard installs resolve from
the default registry and mirrored installs resolve internally. Consume it
from both cleanup hooks with the image digest-pinned to the immutable
multi-arch index sha256:b9ef7d8dbe65bcc81a46c09b8dc7543103055021c4f43287bf59e92a8f4fe05c
(the same digest already pinned for postgres-operator's webhook hook).
Keep the pin fresh with a custom Renovate manager: the repo disables the
helm-values manager (enabledManagers), and the ref is built at render time
by the helper rather than being a Dockerfile/go.mod dependency, so the
built-in managers cannot see it. A regex manager over the hook templates
tracks clastix/kubectl by tag+digest — mirroring the existing extra/etcd
manager — so the supply-chain pin does not silently age.
Also harden `cozy-lib.images-registry` against a nil .Values._cluster so
the helper never crashes the render — relevant precisely because the hook
renders during teardown. Helm unit tests assert the rendered image (routed,
digest-pinned, no leading slash) by pattern rather than an exact digest, so
a Renovate bump keeps them green.
Closes#3088
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com>
## What this PR does
After deleting a `Harbor` app CR, two things were left orphaned in the
tenant
namespace:
- the **jobservice / trivy PVCs** (`<release>-jobservice`, and
`<release>-trivy`
when Trivy is enabled), because
`packages/apps/harbor/templates/harbor.yaml`
sets `persistence.resourcePolicy: "keep"`, which annotates the upstream
Harbor
subchart's PVCs with `helm.sh/resource-policy: keep` so Helm/Flux
deliberately
never deletes them;
- cert-manager's **ACME http01-solver** resources
(`cm-acme-http-solver-*`), via
the same stuck-challenge mechanism as the Bucket issue (#3060) when
Harbor
issues a non-wildcard cert.
This adds a single `post-delete` cleanup hook that:
- **deletes the upstream Harbor subchart's kept PVCs** — selecting by
`app.kubernetes.io/instance=<release>-system` (the subchart runs as the
`<release>-system` HelmRelease, so its PVCs carry that label, **not**
`<release>`). A hook is required precisely because `resourcePolicy:
keep`
blocks normal reclaim. This intentionally covers both the jobservice and
trivy
PVCs: both hold only regenerable data (jobservice file logs; trivy's
vulnerability-DB cache), and registry images live in S3.
- **deletes this release's `Certificate`** (deterministic name
`<release>-ingress-tls`); cert-manager then cascade-deletes the `Order`,
`Challenge` and solver `Pod`/`Service`/`Ingress` objects it owns via
owner
references.
- **host-scoped belt-and-braces sweep** for any ACME http01 solver
objects
cert-manager may have orphaned (e.g. the Certificate was already gone,
so
deleting it did not cascade). The sweep is scoped **strictly to this
release's
Harbor host** — Challenges filtered by `.spec.dnsName`, solver Ingresses
by
`.spec.rules[0].host` — **not** a namespace-wide
`acme.cert-manager.io/http01-solver=true` label sweep, because a
Cozystack
tenant namespace can host many apps and a blanket label sweep would
delete
sibling apps' in-flight challenge solvers.
The Job is hardened: non-root (uid 65534), read-only root fs, all caps
dropped,
`seccompProfile: RuntimeDefault`, with CPU/memory requests and limits
set. RBAC
is scoped to exactly the resources it deletes (`persistentvolumeclaims`,
`certificates`, `orders`, `challenges`, solver `ingresses`) and verified
against
the vendored cert-manager CRDs. Failures are logged but never block
teardown.
## Cleanup scope — what is and isn't removed
**Removed:** the upstream Harbor subchart's kept PVCs (jobservice +
trivy) and
this release's cert-manager ACME artifacts (Certificate, and host-scoped
Order/Challenge/solver-Ingress).
**Not touched (by design):**
- **CNPG database PVCs** (`<release>-db-N`) — owned by the CNPG
`Cluster` and
governed by CNPG's own reclaim policy; they carry CNPG labels, not
`app.kubernetes.io/instance=<release>-system`. Re-installing Harbor with
a
different DB size into the same namespace can therefore leak the old DB
PVCs;
this is left to CNPG's lifecycle on purpose (the data is the user's).
- **Redis PVCs** — managed by the redis-operator with their own labels,
likewise
untouched.
- **Solver `Pod`/`Service`** — not deleted directly; they are removed by
cert-manager's owner-ref cascade when the Certificate/Challenge is
deleted, so
the hook grants no RBAC for them.
> [!NOTE]
> The PVC sweep is eventually-consistent: while the inner
`<release>-system`
> HelmRelease is still being uninstalled by Flux, a PVC may briefly stay
in
> `Terminating` until its consumer Pods (jobservice/trivy) exit. The
Job's
> deletion request always succeeds; reclaim completes once the pods are
gone.
> [!WARNING]
> **This overrides the intentional `persistence.resourcePolicy: "keep"`
setting
> and force-deletes the jobservice/trivy PVCs on app deletion.** Those
PVCs hold
> only jobservice file logs and the trivy vuln-DB cache (registry images
live in
> S3), so data-loss risk is low — but if retaining them across
re-installs is
> desired, this is a conscious product decision to revisit. The ACME
cleanup
> affects only cert-manager's transient artifacts.
Fixes#3061.
> Follow-up tracked in #3088: digest-pin the cleanup `kubectl` image and
route it
> through `cozy-lib.images-registry` (pre-existing debt shared with the
MariaDB
> cleanup hook), out of scope for this PR.
### Release note
```release-note
fix(harbor): clean up the orphaned jobservice/trivy PVCs and cert-manager ACME http01-solver resources left behind after a Harbor app is deleted. NOTE: this force-deletes the jobservice/trivy PVCs, overriding persistence.resourcePolicy=keep (jobservice file logs and trivy vuln-DB cache only; registry images are in S3). CNPG database PVCs are not touched.
```
The post-delete cleanup hook selected PVCs by
app.kubernetes.io/instance=<release>-system, a label only the jobservice PVC
carries (via the chart's "harbor.labels" helper). The trivy PVC is a
StatefulSet volumeClaimTemplate labeled via "harbor.legacy.labels" with a
selector using "harbor.matchLabels" — neither sets
app.kubernetes.io/instance — so the sweep silently missed it. Trivy is enabled
by default, so a default Harbor delete left the trivy PVC orphaned,
contradicting the PR's own claim.
Select on release=<release>-system instead: both kept PVCs carry it, while
CNPG (db) and Redis PVCs use different label schemes and the registry is on S3,
so only the two intended PVCs match. The unit test now asserts the corrected
selector and guards against regressing to the instance label.
Also drop the by-name delete of Orders/Challenges (their hash-suffixed names
make a by-name delete a no-op; the Certificate owner-ref cascade plus the
host-scoped sweep handle them), narrow the hook RBAC to challenges only, and
note the Gateway-mode no-op.
Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com>
## What this PR does
The `apps/harbor` chart never passed `storageClass` to the jobservice
`jobLog` PVC, so the upstream chart rendered it without a
`storageClassName`. On first install the default-StorageClass admission
then stamps the cluster default onto the live PVC. Because a PVC's
`storageClassName` is immutable, the rendered manifest (no class) and
the live object (admission-defaulted class) drift apart, which surfaced
as upgrade failures on the `harbor-*-system` HelmRelease.
This propagates `storageClass` to the jobservice PVC, with one
safeguard: a `lookup` reads the live jobservice PVC and reuses its
current `storageClassName` when the PVC already exists, only falling
back to the configured `storageClass` for a fresh install. Keeping the
rendered value equal to the live one means an existing PVC whose
admission-defaulted class differs from the configured `storageClass` is
preserved rather than triggering an immutable-field rejection on
upgrade. A live PVC pinned to an explicit `""` (no dynamic provisioning)
round-trips through the upstream `"-"` sentinel instead of collapsing to
an omitted field, and the spec access is guarded so a PVC with no
`storageClassName` key stays omitted.
The trivy `StatefulSet`'s `volumeClaimTemplate` carried the same latent
drift — `.Values.storageClass` was forwarded blindly into a VCT, which
cannot be patched at all — so it now gets the identical lookup-preserve
treatment, keyed off `data-<release>-trivy-0`. This closes both halves
of #2368.
Also adds a helm-unittest regression suite and wires a `test:` target so
`make unit-tests` stops skipping `packages/apps/harbor`.
Validated with `helm template` across storageClass/trivy permutations
and the new unit tests. The jobservice preserve path was additionally
confirmed with a server-side dry-run against a running harbor release:
`lookup` resolved the live PVC's class, and with `storageClass`
overridden to a different class the jobservice render stayed pinned to
the live PVC's class — confirming the immutable-field rejection is
avoided. The trivy change applies the same proven pattern. Note: under
`helm template`/helm-unittest `lookup` returns empty, so the unit suite
exercises only the fresh-install fallback; the preserve branches are
covered by the live `lookup`.
Fixes#2368
### Release note
```release-note
fix(harbor): preserve the jobservice PVC and trivy volumeClaimTemplate storageClass on upgrade, fixing upgrade failures caused by storageClassName drift
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Harbor now preserves the existing jobservice storage class during
upgrades and ensures it’s applied on initial installation when
available, with consistent handling for Trivy persistence as well.
* **Tests**
* Added a Helm unittest regression suite covering storage class
propagation and the correct conditional rendering of persistence blocks
when storage class and Trivy settings vary.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
The cozy-harbor wrapper never passed storageClass to the jobservice
jobLog PVC, so it was created without a storageClassName and the
default-StorageClass admission stamped the cluster default. Because PVC
storageClassName is immutable, this caused rendered-vs-live drift on
upgrade. Preserve an existing PVC's class via lookup and fall back to the
configured storageClass for fresh installs.
The trivy StatefulSet's volumeClaimTemplate had the same latent drift --
.Values.storageClass was forwarded blindly into a VCT that cannot be
patched at all -- so it gets the same lookup-preserve treatment, keyed
off data-<release>-trivy-0.
A live object pinned to an explicit "" (no dynamic provisioning) now
round-trips through upstream's "-" sentinel instead of collapsing to an
omitted storageClassName, and the spec lookup is guarded with
`| default dict` so a PVC carrying no storageClassName key stays omitted.
Adds a helm-unittest regression suite and wires a test target so
make unit-tests stops skipping packages/apps/harbor.
Fixes#2368
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The jobservice PVC (resourcePolicy: keep) and cert-manager ACME
http01-solver objects were left orphaned after a Harbor app CR was deleted.
Add a post-delete hook that deletes the release PVCs (selecting by
app.kubernetes.io/instance=<release>-system, the label the upstream subchart
stamps) and the release Certificate/Order/Challenge plus the http01-solver
sweep. A hook is required because resourcePolicy: keep blocks normal reclaim.
The cleanup Job runs non-root with a read-only root fs and dropped
capabilities; failures are logged but never block teardown.
Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io>
Bump the vendored Harbor Helm chart from 1.18.2 to 1.19.1, which
upgrades Harbor itself from appVersion 2.14.2 to 2.15.1.
The bump adds PodDisruptionBudget templates for the core, exporter,
jobservice, nginx, portal, registry and trivy components, improving
availability during voluntary disruptions (node drains/upgrades).
<!-- Thank you for making a contribution! Here are some tips for you:
- Use Conventional Commits for the PR title: `type(scope): description`
- Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore
- Scopes are not an exhaustive list — pick the most specific scope for
the change and extend the list when a genuinely new area appears.
Examples:
- System components: dashboard, platform, operator, cilium, kube-ovn,
linstor, fluxcd, cluster-api
- Managed apps: postgres, mariadb, redis, kafka, clickhouse,
virtual-machine, kubernetes
- Development and maintenance: api, hack, tests, ci, docs, maintenance
- Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or
add a `BREAKING CHANGE:` footer
- If it's a work in progress, consider creating this PR as a draft.
- Don't hesistate to ask for opinion and review in the community chats,
even if it's still a draft.
- Add the label `backport` if it's a bugfix that needs to be backported
to a previous version.
-->
## What this PR does
### Screenshots
<!-- REQUIRED for UI changes: attach screenshots or screen recordings
demonstrating
the visual impact of your changes. PRs with UI changes without
screenshots will not be merged. -->
### Release note
<!-- Write a release note:
- Explain what has changed internally and for users.
- Start with the same `type(scope):` prefix as in the PR title
- Follow the guidelines at
https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md.
-->
```release-note
feat(harbor): bump Harbor to 2.15.1 and add PodDisruptionBudgets for core, portal, jobservice, nginx, registry, trivy and exporter
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
## Release Notes
* **New Features**
* Added Pod Disruption Budget support for core, portal, jobservice,
nginx, registry, trivy, and exporter.
* Made container health checks (liveness/readiness, plus core startup)
fully configurable per component.
* Added configurable registry HTTP client timeout.
* Enhanced Gateway API route configuration (hostnames formatting and
backend reference details).
* **Improvements**
* Improved external Redis credential handling for registry usage (safer
secret guarding and URL-encoding).
* Updated Harbor chart metadata and component images to **v2.15.1**.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Closes#2880
The system chart was bumped to Harbor 2.15.1, but the user-facing app
chart still declared appVersion 2.14.2. Since the Application type serves
appVersion as a top-level field, the platform API and dashboard misreported
the deployed Harbor version. Align appVersion with the shipped 2.15.1.
Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com>
Dropping the whole tls block in wildcard mode disabled the forced
HTTP-to-HTTPS redirect: ingress-nginx only issues the 308 redirect when
TLS is enabled for the Ingress, while a host without a tls section is
still served on 443 with the default certificate but stays reachable
over plain HTTP. Keep the tls section with the host list and omit only
the per-host secretName, so ingress-nginx falls back to the operator
wildcard configured as the default SSL certificate and the redirect
stays enforced, matching the non-wildcard behaviour.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
When publishing.certificates.wildcardSecretName is set, the root
tenant's ingress-nginx controller now serves the operator-supplied
wildcard Secret as its --default-ssl-certificate, and the platform
service Ingresses stop requesting their own per-host ACME certificates.
The flag is set only on the publishing controller (Release.Namespace ==
expose-ingress) because ingress-nginx loads the Secret from its own
namespace; child-tenant controllers cannot reach a Secret in another
namespace, so they keep the existing ACME path.
Each service Ingress (dashboard, keycloak, grafana, alerta, bucket,
linstor-gui, harbor, seaweedfs S3, bootbox) drops its cert-manager
cluster-issuer annotation, the http01 challenge-class annotation, and
its tls block in wildcard mode; nginx then falls back to the default
certificate for those hosts. Other annotations and routing are
unchanged. Each file keeps its existing render guard intact, and the
annotations block collapses cleanly where the cert annotations were its
only content.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Harbor stores registry data only in S3-compatible object storage and
has no local-filesystem backend, but the app docs never stated this.
Without SeaweedFS available in the tenant chain the registry
BucketClaim never yields its credentials secret and the HelmRelease
silently stays unreconciled — a common and confusing first-deploy
failure.
Add a Prerequisites section to the chart README (propagated to the
published app docs) explaining the requirement, how the bucket is
provisioned via COSI from SeaweedFS, how the class is inherited down
the tenant tree, and how to enable it.
Co-authored-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Add the storageclass dynamic-dropdown x-cozystack-options annotation to the
storageClass field of the data apps (openbao, clickhouse, mariadb, redis,
harbor, opensearch, rabbitmq, mongodb, postgres, nats, kafka, qdrant,
foundationdb, http-cache) at the values.yaml source, so the extension is
emitted by cozyvalues-gen instead of hand-edited into values.schema.json.
kafka carries two (Kafka and ZooKeeper structs); foundationdb is nested
under the Storage struct.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
Mark fields that reference cluster resources with the x-cozystack-options
vendor keyword so the dashboard renders them as dynamic dropdowns:
GPU, instanceType, instanceProfile, network (vm-instance), GPU/
instanceType (kubernetes nodeGroups), image (vm-disk), storagePool
(bucket), storageClass (all apps) and disks[].name (vm-instance). Drop
the stale static instanceProfile enum.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
Flux v2.8 / helm-controller v1.5.0 embeds Helm v4, which renamed --force
to --force-replace and rejects combining it with SSA's --force-conflicts.
Existing HelmReleases with spec.upgrade.force: true fail every upgrade with
"cannot use force conflicts and force replace together", looping
upgrade -> rollback -> upgrade and blocking dependants. Observed on PR #2619
CI for tenant-root/{ingress-nginx-system,seaweedfs-db}, tenant-test/info,
and others.
Per Flux docs, upgrade.force has been a no-op since v2.8 anyway, so removing
it is safe on green clusters and unblocks broken ones.
Also fix packages/apps/kafka/templates/workloadmonitor.yaml: it referenced
.Values.replicas (no such field; values.yaml has kafka.replicas and
zookeeper.replicas). Flux v2.7 silently dropped the null; v2.8 SSA forwards
it, and the WorkloadMonitor CRD rejects "spec.replicas: null" as not
type:integer, which blocks every kafka-test install.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
(cherry picked from commit 7e286cf8e1)
## Summary
Annotate `storageClass` as immutable across every stateful chart that
exposes one: clickhouse, foundationdb, harbor, http-cache, kafka (Kafka
+ ZooKeeper), kubernetes (top-level + nodeGroups[]), mariadb, mongodb,
nats, openbao, opensearch, postgres, qdrant, rabbitmq, redis, vm-disk.
The PVC-pinning argument is identical for every chart — Kubernetes fixes
`storageClassName` on each PVC at creation, so editing this field on an
existing resource never migrated data; the schema annotation makes that
contract explicit and lets schema-aware consumers refuse the edit
up-front.
Schema-aware consumers today: the Cozystack UI
(cozystack/cozystack-ui#6) reads the rule from the chart's openAPISchema
and renders the field read-only on edit forms.
This PR is **schema-side only**. The cozystack aggregated apiserver does
not yet evaluate CEL rules in `openAPISchema` (`MariaDB` / `Postgres` /
etc. are application-backed, not CRDs); tracking issue
cozystack/cozystack#2657 covers wiring CEL into
`pkg/registry/apps/application/rest.go::Update`. Until that lands, an
apiserver-level `kubectl patch` that changes `storageClass` is still
accepted — but the underlying StatefulSet PVCs do not move, so there's
no data-corruption risk, only a confusing UX. The UI catches that case
today; the apiserver gate closes it permanently.
## What changes
For each app in the list above:
- `packages/apps/<app>/values.yaml` — gain a `## @immutable` directive
directly above (or below, for fields inside a `@typedef`) the
`storageClass` line.
- Regenerated via `make generate` in each package:
- `api/apps/v1alpha1/<app>/types.go` — kubebuilder `XValidation` marker
on the relevant field(s).
- `packages/apps/<app>/values.schema.json` — `x-kubernetes-validations:
[{rule: "self == oldSelf", message: …}]` on `storageClass`.
- `packages/apps/<app>/README.md` — regenerated parameter table.
- `packages/system/<app>-rd/cozyrds/<app>.yaml` — the embedded
`openAPISchema` on the ApplicationDefinition reflects the change.
Refactor: `pkg/registry/apps/application/rest.go` factors the inline
schema-build path into a new `buildSpecSchema` helper so it can be
unit-tested without standing up a fake apiserver. Three regression tests
in `pkg/registry/apps/application/rest_spec_schema_test.go` pin the
contract that `x-kubernetes-validations` survives the
v1→internal→structural conversion (test loads the actual MariaDB
ApplicationDefinition fixture on disk, asserts the rule reaches the
structural schema). Empty-input and malformed-JSON paths are also
pinned.
No e2e admission test is added because the apiserver does not yet
enforce the rule — see cozystack/cozystack#2657.
No changelog entry: schema-side annotation is invisible at the apiserver
layer today (UI-only consumer) and the release-cut PR will add the entry
when admission enforcement lands.
## Dependencies
Requires cozystack/cozyvalues-gen#24 to be merged and released, and the
`cozyvalues-gen` binary pin in `.github/workflows/pre-commit.yml` line
50 bumped to the release that includes that PR. Pre-commit CI will fail
until both happen — the stock `v1.3.0` binary silently ignores `##
@immutable` and regenerates artefacts without the rule, producing a
drift against what's committed.
Once that ordering is satisfied, rebase this branch and the pre-commit
job will be green.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* storageClass is now treated as immutable after creation for many
stateful apps (enforced by chart/CRD schema validation).
* **Documentation**
* Added a storage immutability guide explaining the contract and
enforcement.
* Updated chart READMEs and values docs to mark storageClass as
immutable and clarified nodeGroup storage remains intentionally mutable.
* **Tests**
* Added tests validating schema parsing and preservation of immutability
rules.
<!-- review_stack_entry_start -->
[](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2639?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)
<!-- review_stack_entry_end -->
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Annotate storageClass as immutable across every stateful app whose
chart exposes one — clickhouse, foundationdb, harbor, http-cache,
kafka (Kafka + ZooKeeper), kubernetes (top-level only), mariadb,
mongodb, nats, openbao, opensearch, postgres, qdrant, rabbitmq,
redis, vm-disk. Without coverage across the catalogue the UI would
render storageClass read-only on some apps and editable on others —
a worse mental model than uniform behaviour.
The annotation lands at the schema layer (CEL XValidation rule
'self == oldSelf' on values.schema.json / types.go / embedded
openAPISchema in cozyrds/<app>.yaml). The cozystack aggregated
apiserver does not yet evaluate CEL on application-backed resources;
that is tracked at cozystack/cozystack#2657. The UI
(cozystack/cozystack-ui#6) consumes the marker today.
The PVC-pinning argument is identical for every chart: Kubernetes
fixes a PVC's storageClassName at creation time, so editing this
field on an existing resource never migrated data. The schema
annotation makes that contract explicit.
kubernetes.nodeGroups[name].storageClass is intentionally NOT
annotated: the field is optional and undefaulted, so a strict
'self == oldSelf' rule would block any future attempt to set it on
an existing node group. A richer ratcheting form is needed there —
see docs/storage-immutability.md.
A refactor in pkg/registry/apps/application/rest.go extracts the
inline schema-build path into buildSpecSchema so it can be
unit-tested. Three regression tests in rest_spec_schema_test.go pin
the contract that x-kubernetes-validations survives the
v1->internal->structural conversion (loaded from the actual MariaDB
ApplicationDefinition fixture on disk, not a hand-edited mock).
Each per-app README gains a one-line blockquote pointing to a new
docs/storage-immutability.md that explains the contract and which
consumers enforce it today.
Requires cozystack/cozyvalues-gen with @immutable support — bump the
pre-commit pin in .github/workflows/pre-commit.yml once the release
exists.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
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>
Rewrite resourcesPreset values in every apps/* and extras/* chart from
the legacy flat names to the new instance-type form, using the 1:1
mapping:
nano -> t1.nano micro -> t1.micro
small -> t1.small medium -> c1.small
large -> c1.medium xlarge -> c1.large
2xlarge -> c1.xlarge
Each affected values.yaml carries the canonical 47-value enum (40
instance-type names plus 7 legacy aliases) so the schema accepts both
forms during the deprecation window. cozyvalues-gen regenerates the
matching api/apps/v1alpha1/*/types.go kubebuilder enum and defaults,
the chart values.schema.json, the README parameter table, and the
ApplicationDefinition openAPISchema in packages/system/*-rd/cozyrds.
Three hardcoded legacy literals in chart templates migrated:
- packages/apps/harbor/templates/harbor.yaml:121 (portal)
- packages/apps/opensearch/templates/opensearch.yaml:43 (dashboards)
- packages/system/keycloak/templates/sts.yaml:115 (heap fallback)
packages/apps/foundationdb/Makefile gains the missing hack/update-crd.sh
call. Without it, every regen left foundationdb-rd cozyrds stale and
new FoundationDB app CRs through cozystack-api were rejected at
validation for any non-legacy resourcesPreset value.
Every default-rendered chart produces identical CPU and memory to the
value applied before the rename, courtesy of the legacy alias table in
cozy-lib and the 1:1 default mapping.
Co-authored-by: Matthieu ROBIN <matthieu@hidora.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
## Summary
- `cozy-harbor/templates/bucket-secret.yaml` no longer relies on
`lookup` against the BucketAccess credentials Secret. It is now driven
by `.Values.bucket.bucketInfo` (a JSON string) with `dig`-based safe
accessors, so a missing/empty/partial value renders nothing rather than
crashing with `index of untyped nil` (the failure mode that broke harbor
in CI #25049445125).
- The downstream `<release>-system` HelmRelease now sources `BucketInfo`
via `valuesFrom` (`valuesKey: BucketInfo`, `targetPath:
bucket.bucketInfo`). With the default `optional: false`, helm-controller
refuses to compose values until the COSI BucketAccess controller has
populated the Secret — both gating initial reconciliation and forcing a
config-digest change (and thus a helm upgrade) when the credentials
arrive. Flux HR `dependsOn` cannot reference COSI resources directly, so
`valuesFrom` is the correct primitive here.
- `bucket.secretName` was the only consumer of the old `lookup` and is
removed from both the system chart's `values.yaml` default and the apps
chart's `values:` block.
## Why this shape
helm-controller's upgrade trigger is digest-based over composed values +
chart artifact; a `lookup` returning new data on a later reconcile is
not enough to force an upgrade on its own. Routing `BucketInfo` through
`valuesFrom` puts the bucket data on the digest path, so the chart
re-applies the moment credentials become available.
## Test plan
- [x] `helm template` of `packages/system/harbor` with
`bucket.bucketInfo` unset, empty string, empty JSON object `{}`, and
fully populated — only the populated case renders the `*-registry-s3`
Secret; the others render nothing without erroring.
- [x] `helm template` of `packages/apps/harbor` produces the expected
`<release>-system` HR with the new `valuesFrom` entry and the cleaned-up
`values:` block.
- [ ] Re-run the harbor E2E on a fresh stand and confirm 15/15.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Chores**
* Harbor bucket config now comes from a centralized Secret merged into
chart values; in-values bucket entry removed.
* **Bug Fixes**
* Deployment synchronization and Helm upgrade behavior improved by
depending on the Secret’s readiness/contents.
* **Tests**
* Added unit tests covering when the bucket secret is rendered (or not)
and a make target to run those tests.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
The previous fix (a21c18f9) sourced BucketInfo via Flux valuesFrom with
`targetPath: bucket.bucketInfo`. Flux runs values with `targetPath`
through Helm's `strvals.ParseInto`, which splits the value on commas as
list separators. The COSI BucketInfo JSON is comma-rich, so values
resolution bailed:
could not resolve Secret chart values reference
'tenant-X/harbor-X-registry-bucket' with key 'BucketInfo':
key "\"spec\":{\"bucketName\":\"bucket-1785...\""
has no value (cannot end with ,)
Drop `targetPath`. With only `valuesKey: BucketInfo`, helm-controller
unmarshals the value as YAML and merges at the chart's values root, so
JSON commas stay nested instead of being split. The system chart's
bucket-secret.yaml now reads `.Values.spec.bucketName` /
`.Values.spec.secretS3.{accessKeyID,accessSecretKey,endpoint}`,
guarded by nested `with` blocks so the registry-s3 Secret renders only
when secretS3 has been populated.
Gating semantics from the previous fix are preserved: with the default
`optional: false`, helm-controller still refuses to compose values
until the COSI BucketAccess controller writes `BucketInfo` into the
Secret, and the value's content still drives the HR config-digest.
Verified via `helm template` against the system chart with three
scenarios: missing `spec` and `spec` without `secretS3` render
nothing; full BucketInfo renders all five S3 env keys correctly.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: Claude <noreply@anthropic.com>
The Helm-managed harbor-{name}-credentials Secret was not visible
through the TenantSecrets API because the lineage webhook compared the
real Secret name against the rendered template "{{ .name }}-credentials"
(which becomes "{name}-credentials" without the "harbor-" HelmRelease
prefix), failed to match, and labelled the Secret as
internal.cozystack.io/tenantresource=false.
Adopt the same pattern Bucket and RabbitMQ already use: stamp the
user-facing Secret with apps.cozystack.io/user-secret=true and add a
matchLabels rule alongside the existing resourceNames entry in the
ApplicationDefinition. The webhook now matches via labels instead of
relying on Secret-name templating, so the Secret is properly marked
tenantresource=true and surfaces in tenant-aware clients.
Signed-off-by: IvanHunters <ivan.okhotnikov@aenix.io>
Two bugs caused the harbor E2E to fail (CI #25049445125):
1. cozy-harbor's bucket-secret.yaml called `index $existingSecret.data
"BucketInfo"` unconditionally. During first-time install the COSI
BucketAccess controller may create the credentials Secret as a
placeholder before populating it, so `.data` is nil and the chart
render crashes with `index of untyped nil`.
2. The downstream `<release>-system` HelmRelease starts reconciling
immediately, in parallel with bucket provisioning, hitting bug #1
on every retry within the test's 5-minute window.
The previous shape relied on `lookup` to read the Secret at render
time. helm-controller's upgrade trigger is digest-based over composed
values + chart artifact, so a `lookup` returning new data on a later
reconcile is not enough on its own to force an upgrade — the rendered
output may diverge but the digest does not.
Switch to a values-driven shape:
- `bucket-secret.yaml` now reads `.Values.bucket.bucketInfo` (a JSON
string) and uses `dig` for safe access; if the value is empty or the
expected nested fields are missing, no `*-registry-s3` Secret is
rendered.
- The `<release>-system` HR sources `BucketInfo` via `valuesFrom` with
`targetPath: bucket.bucketInfo`. With the default `optional: false`,
helm-controller will refuse to compose values until the key exists,
which both gates initial reconciliation on the BucketAccess Secret
being populated and forces a config-digest change (and thus a helm
upgrade) when its contents change.
- `bucket.secretName` is removed from the system chart's values and
from the apps chart's `values:` block; the only consumer (the
`lookup` call) is gone.
Verified four scenarios via `helm template` against the system chart:
unset bucketInfo, empty string, empty JSON object `{}`, and a
fully-populated BucketInfo — only the last renders the registry-s3
Secret; the others render nothing without erroring.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
ClusterIssuer solver referenced IngressClass "nginx" which does not
exist on cozystack clusters — real classes are named after tenant
namespaces (e.g. tenant-root). Cert issuance only worked because every
requesting Ingress overrode the ClusterIssuer via the legacy
acme.cert-manager.io/http01-ingress-class annotation.
Switch both sides to the modern cert-manager API (available since
cert-manager 1.12; cozystack ships 1.19.3):
- ClusterIssuer: http01.ingress.ingressClassName, value parameterized
from _cluster.expose-ingress (default "tenant-root")
- Ingress annotation: http01-ingress-ingressclassname
These must migrate together — mixing ingressClassName (ClusterIssuer)
with the old http01-ingress-class annotation triggers cert-manager's
"fields ingressClassName and class cannot be set at the same time"
validation and breaks issuance.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Follow the same hostname pattern as kubernetes and vpn apps: use
Release.Name directly (which already includes the harbor- prefix)
instead of adding an extra harbor. subdomain.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Change sslmode from disable to require for CNPG PostgreSQL connection,
as CNPG supports TLS out of the box. Fix token key/cert preservation to
verify both values are present before passing to Harbor core.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Use tenant base domain in default hostname construction (harbor.RELEASE.DOMAIN)
to match the pattern used by other apps (kubernetes, vpn). Remove unused $ingress
variable from harbor.yaml. Add cleanup of stale resources from previous failed
E2E runs.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
myasnikovdaniil
