mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-13 08:05:19 +09:00
2e9e6384e512f3ebd149c1c54821d0714bd07f7d
5911
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 cozystack-ci[bot]
|
2e9e6384e5 |
docs: add changelog for v1.6.3
Signed-off-by: cozystack-ci[bot] <274107086+cozystack-ci[bot]@users.noreply.github.com> |
||
|
Andrey Kolkov
|
7786f72b9f |
feat(etcd-operator): bump to v0.5.5 and vendor EtcdDefragPolicy CRDs (#3968)
## What this PR does Bumps the vendored `etcd-operator` from **v0.5.4** to **[v0.5.5](https://github.com/cozystack/etcd-operator/releases/tag/v0.5.5)** and adopts the new defrag API. v0.5.5 ships the `EtcdDefrag` and `EtcdDefragPolicy` types — a controller that runs a one-shot defrag against a target etcd cluster, plus a cron-style policy that stamps scheduled `EtcdDefrag` runs (`schedule`/`suspend`/`concurrencyPolicy`/`historyLimit`). - **`etcd-operator-crds`**: vendor the two new CRDs (`etcddefragpolicies`, `etcddefrags`) alongside the existing three; bump `ETCD_OPERATOR_REF`. Re-vendoring also refreshed `etcdmembers.yaml` (a doc-string change from the version-matched-`etcdutl` restore work in v0.5.5). All five carry `helm.sh/resource-policy: keep`; the existing resource-policy suite covers the new CRDs via its wildcard template scope. - **`etcd-operator`**: bump `appVersion`/image tag to v0.5.5 and re-vendor the manager `ClusterRole` from the operator's `charts/etcd-operator/files/manager-role-rules.yaml`. New grants match what the defrag controllers need: full CRUD on `etcddefrags`, read/patch/watch on `etcddefragpolicies`, the two new `/status` subresources, and cluster-scoped `events` create/patch. Fixed the stale `config/rbac/role.yaml` path in the Makefile/test comments (upstream moved the role rules). ### Screenshots n/a — no UI changes. ### Downstream repositories Walked the trigger map against the diff. The change is confined to `packages/system/etcd-operator{,-crds}`: etcd-operator is a system component (not a `packages/apps`/`packages/extra` package the website and TF provider track), the new CRDs are in group `etcd-operator.cozystack.io` (not `cozystack.io`/`backups.cozystack.io`, the only CRD groups the TF provider types), and nothing under `hack/`, the installer values, or the platform values changed. - [x] No downstream repository is affected by this change ### Release note ```release-note feat(etcd-operator): bump to v0.5.5, adding the EtcdDefrag and EtcdDefragPolicy APIs for on-demand and scheduled etcd defragmentation ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added APIs for configuring recurring and one-time etcd defragmentation tasks. * Added status reporting for defragmentation progress, results, and conditions. * Added validation and lifecycle controls for defragmentation policies and runs. * **Enhancements** * Updated the etcd operator to version 0.5.5. * Expanded permissions to manage defragmentation resources and report events. * Improved restore documentation for etcd members. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey Kolkov
|
e94fcf8219 |
feat(backups): RabbitMQ definitions backup via a dedicated strategy driver (#3990)
## What this PR does Adds backup/restore support for the managed `RabbitMQ` application. RabbitMQ's cluster operator ships no backup CRD and RabbitMQ has no consistent online export of message payloads, so this backs up broker **definitions** (vhosts, users, permissions, queues, exchanges, bindings, policies, parameters) over the management HTTP API (`GET`/`POST /api/definitions`) and stores them in the system bucket. Message payloads and point-in-time recovery are intentionally out of scope; a full-data backup would be a Velero volume snapshot, which is a separate strategy. It is delivered as a **first-class `strategy.backups.cozystack.io/Rabbitmq` driver**, following the Altinity pattern (a dedicated driver that renders a one-shot Job for an HTTP-API engine): - `reconcileRabbitmq` / `reconcileRabbitmqRestore` render the strategy's `PodTemplateSpec` — `curl` `GET`/`POST /api/definitions`, pushing/pulling the JSON to S3 with `curl --aws-sigv4` — as a `batch/v1.Job`, covering both restore flows: in-place (into the source broker) and to-copy (into a separate, differently-named RabbitMQ in the same namespace, authenticating with the target's own default-user credentials and reading the source's object key). - A **Go-read precondition** gates each run on the `RabbitmqCluster` reporting `AllReplicasReady` (read through a small `rabbitmqtypes` partial type), so a not-yet-ready broker surfaces a precise `Ready=False` wait on the BackupJob/RestoreJob rather than a Job that starts and fails against an unreachable management API. - The produced `Backup` records the stored object's location on `status.artifact.uri`, rendered from the strategy's `spec.artifactURITemplate`. The `Backup` CRD has no status subresource, so this persists on Create, mirroring the Etcd driver. Dispatch is wired in both controllers (plus `supportedBackupStrategyKinds`), `rabbitmqtypes` is registered in the scheme, and RBAC grants read on `rabbitmqclusters.rabbitmq.com` (the Job itself is already covered by the existing `batch/jobs` grant). It ships `cozy-default-rabbitmq` (a `$bucketName`-gated `Rabbitmq` strategy) and routes `apps.cozystack.io/RabbitMQ` to it in the `cozy-default` BackupClass, which also enrols it in the `DefaultObjectsGate`. The RabbitMQ application chart is left untouched — it had no prior backup wiring, and the strategy carries the S3 coordinates itself. Unit tests cover the driver's pure helpers (applicationRef validation, cluster-name derivation, the readiness-message builder, artifact-URI rendering, parameter round-trip) and the reconcile precondition gate (defers with `Ready=False` and no Job until the cluster is ready, then proceeds). A runnable example under `examples/backups/rabbitmq/` and an honest-roundtrip Chainsaw e2e under `hack/e2e-chainsaw/rabbitmq/` seed a sentinel vhost, back it up, drop it, and assert it round-trips through object storage on both restore flows. The Job pod reuses the existing `platform-migrations` image (curl + ca-certificates) under a new `backupStrategyController.rabbitmqBackupClientImage` key, stamped in lockstep with `chBackupClientImage` in `packages/core/platform/Makefile` so the repository stays pinned at a single digest (`hack/image-pin-consistency.bats`). A dedicated slim image built by this package is a follow-up. **Validation:** `go build`/`go vet`/`gofmt`/`go test ./internal/backupcontroller/... ./internal/template/...`, deepcopy + Rabbitmq CRD generation, `helm template` proving the CRD-ordering gate both on and off, `helm lint`, and `helm unittest` (18/18). The backup + in-place + to-copy roundtrip was verified end-to-end on a dev cluster (a real management-API export and a real S3 `--aws-sigv4` upload/download against SeaweedFS), so the driver's core contract is exercised on a real cluster; the in-CI Chainsaw run is the remaining automated evidence. The object-cleanup, terminating-namespace teardown and readiness-deadline paths added during review are unit-tested (including `Reconcile`-level finalizer retention and the terminating-namespace release) but not yet cluster-exercised. (Note found while testing: RabbitMQ 4.2 is OOMKilled on the `nano` preset, so the example/e2e use `c1.small`.) ### Screenshots Not applicable — no UI changes. ### Downstream repositories Walked the trigger map against the diff. This is an additive backup strategy plus a new strategy CRD; no downstream repository is mechanically forced by the change. A documentation page for RabbitMQ backup on [cozystack/website](https://github.com/cozystack/website) would be a reasonable follow-up, but it is not required by this diff — leaving the boxes unticked for a human to decide rather than ticking "no downstream" to clear the checklist. ### Release note ```release-note feat(backups): add RabbitMQ definitions backup/restore via a dedicated strategy.backups.cozystack.io/Rabbitmq driver; it gates each run on the RabbitmqCluster being ready and records the stored object on Backup.status.artifact.uri. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added RabbitMQ backup and restore support for definitions, including vhosts, users, queues, exchanges, bindings, policies, and related settings. * Added in-place and restore-to-copy workflows with readiness checks. * Added scheduled and on-demand backup examples. * Integrated RabbitMQ backups with the default backup configuration. * Backups store definitions only; message payloads and point-in-time recovery are not included. * **Documentation** * Added a complete RabbitMQ backup and restore demonstration guide. * **Tests** * Added end-to-end coverage for readiness and backup/restore round trips. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey KolkovandClaude Opus 4.8
|
6afe69b35b |
chore(etcd-operator): retrigger CI
The prior CI run stalled in the self-hosted runner queue for days and never scheduled; an empty commit forces a fresh run on the same tree. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Andrey KolkovandClaude Opus 4.8
|
5896484c4a |
docs(etcd-operator): fix stale role-rules vendoring path in rbac.yaml comment
The manager ClusterRole header still pointed at the upstream config/rbac/role.yaml path, which 404s at v0.5.5; the rules now live in charts/etcd-operator/files/manager-role-rules.yaml (already corrected in the Makefile and rbac_test.yaml). Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Andrey KolkovandClaude Opus 4.8
|
85cbb3d7cb |
feat(etcd-operator): bump to v0.5.5 and vendor EtcdDefragPolicy CRDs
v0.5.5 ships the EtcdDefrag and EtcdDefragPolicy API types (a controller plus a cron that stamps scheduled defrag runs). Vendor both CRDs into the etcd-operator-crds package and grant the manager ClusterRole the verbs the new controllers need: defragpolicies read/patch, defrags full CRUD, the two new /status subresources, and cluster-scoped event emission. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Aleksei Sviridkin
|
88bae1fd98 | docs(agents): require the neutral Assisted-by: LLM trailer (#4041) | ||
|
myasnikovdaniil
|
2b99853e6d |
fix(networking): prevent webhook admission outages during namespace churn (#3997)
## What this PR does Moves ingress-nginx and kubeovn-webhook pod deny rules to source-side cilium policies, and keeps a target-side `ingressDeny` on the webhook pods beside them, so enforcement does not depend on which component performs the service DNAT. Pod access to both webhooks stays denied, ingress-nginx keeps exception for konnectivity agent (now keyed on its ServiceAccount instead of `k8s-app` label that any pod can set on itself), and external `world` deny stays as static target rule. Source-side rules deny only backend port. That covers ClusterIP traffic too, because cilium translates service to its backends before it enforces egress policy, and that ordering exists because `packages/system/cilium/values.yaml` sets `kubeProxyReplacement: true` against upstream default of `"false"`. The value is operator-reachable, so the target-side rule is what keeps the deny working if it is ever flipped, and two render-time beacons make a flip visible instead of silent. Naming the frontend with `toServices` is not possible, and templates now say why, because the construct looks correct and fails silently: cilium resolves `toServices` only in allow rules (`pkg/policy/k8s/service.go` walks `spec.Egress`, never `spec.EgressDeny`), so such `egressDeny` gets non-nil empty L3 selector from `pkg/policy/utils/parserules.go`, and `pkg/policy/types/policyentry.go` treats that as wildcard when rule carries `toPorts`. This does not remove the namespace-churn map writes. The target-side rule carries a cluster-spanning peer, so those writes come back. What changes is that they stop being load-bearing, because whichever end is degraded the other still denies. Cost is policy-map cardinality on the two webhook endpoints, and residual risk is an endpoint near `MaxEntries` getting the inverted add-before-delete order (`pkg/endpoint/bpf.go:1168-1177`). Both policies also change kind and name: namespaced `CiliumNetworkPolicy/ingress-nginx-admission-webhook` and `CiliumNetworkPolicy/kube-ovn-webhook` become cluster-scoped `CiliumClusterwideNetworkPolicy/ingress-nginx-admission-webhook-c5d94713a379` and `kube-ovn-webhook-c2bdbc146112`. Anything keyed on the old namespaced objects goes quiet with no error. ## Tests Package suites pin the deny shape per package, one rule per spec with a destination selector cilium can actually resolve, and pin the peer shape of the target-side rule against the two substitutions that would deny the API server (`fromEntities: [all]`, and dropping the peer). Checked by mutation, all of those go red. `matchRegexRaw` and `notMatchRegexRaw` are not used, helm-unittest 1.0.3 ignores both. `packages/system/kubeovn-webhook` had no `test` target in Makefile, and `hack/helm-unit-tests.sh` finds suites by probing `make -n test` per package, so all three of its suites never ran in CI, including port agreement guard that exists to catch a drift failing open. Added the target, plus `.PHONY: test` to it and to cilium. E2e churn case proves webhooks stay available. Second case proves they are still unreachable from pods, and that deny is confined to webhook port. Both now gate every verdict on the admission endpoints having ready addresses before, during and after, on controller pod identity and restart count being equal across the window, and on a positive control where the same pods' own 443 must stay reachable. So a dead or flapping backend fails as setup failure instead of reporting a deny that was never enforced. First run of these cases reported a false verdict for exactly that reason. ## Not verified Original outage in #3983 was not reproduced in isolation, and its symptom argues against the mechanism this PR was written around. `connect: no route to host` is `EHOSTUNREACH`, and a policy denial cannot produce it, since a deny drops after `connect()` has already returned and the caller sees a timeout. On a ClusterIP that errno is what cilium's socket LB returns with no usable backend, and socket LB is active in host netns because `pkg/kpr/kpr.go:47-49` forces it whenever kube-proxy replacement is set. Namespace deletion churns EndpointSlices and service backend maps as well as identities, which is directly on that path. So this change is justified as removing a structural weakness and adding a topology-independent deny, not as a confirmed root cause fix. What would settle it: `cilium-dbg monitor --type drop` on the controller's node during the window, agent log checked for the map-overflow warning, and the admission Service's ready backend count across those seconds. `world` deny that issue named as its first suspect is kept, on `cilium monitor` evidence recorded when kubeovn-webhook policy was written - api server arrives with identity `host` on same node and `kube-apiserver` from another, never as `world`. That is one topology, a kube-ovn-chained management cluster, and it is not re-derivable on cilium-primary variants or on a Kamaji guest cluster without a run. Whether cilium without kube-proxy replacement is a supported configuration is a maintainer decision and is open. If it is not supported, a `fail()` in the cilium chart is the right shape and should follow this PR. Refs #3983. ### Screenshots Not applicable. ### 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: ### Release note ```release-note fix(networking): enforce the Ingress and Pod admission webhook deny on both the caller and the webhook endpoint, so it no longer depends on Cilium performing service load-balancing. Both policies become cluster-scoped: CiliumNetworkPolicy/ingress-nginx-admission-webhook and CiliumNetworkPolicy/kube-ovn-webhook are replaced by CiliumClusterwideNetworkPolicy/ingress-nginx-admission-webhook-c5d94713a379 and kube-ovn-webhook-c2bdbc146112. ``` ## Summary by CodeRabbit * **Bug Fixes** * Improved admission webhook reliability during namespace deletion and churn. * Preserved webhook protection while allowing unrelated secure traffic to continue. * Corrected webhook access controls for both ingress-nginx and kube-ovn. * **New Features** * Added cluster-wide policy support with safer backend-port enforcement and automatic capability detection. * Added safeguards for policy naming and configuration overrides. * **Tests** * Expanded automated coverage for webhook availability, access restrictions, policy rendering, and end-to-end suite selection. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
61026b732c |
docs(agents): require the neutral Assisted-by: LLM trailer
The attribution trailer exists for disclosure, not to advertise a model. A model name in the trailer tells a reviewer nothing they can act on, and it goes stale with every tool change, so the accepted form is a single `Assisted-by: LLM` line however many tools took part. A link to an assistant session is worse still: it rots, and it carries nothing about the change itself, so no session trailer and no transcript URL belong in a commit, a PR description, or a comment. The commit body explains why a change is made; the diff already shows what changed. The hygiene rules for messages, trailers and in-code comments are written down as review blockers for the same reason: a NOT LGTM on one of them is then a stated rule, not a reviewer's taste. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Andrey Kolkov
|
6eba4d9fa7 |
test(backups): use .svc FQDN for rabbitmq backup e2e S3 endpoint
The reworked rabbitmq backup roundtrip reached the upload step and failed TLS verification: `curl (60) no alternative certificate subject name matches target hostname 'seaweedfs-s3.tenant-root'`. The demo Bucket, in-cluster endpoint resolution and the copied SeaweedFS CA all worked; the remaining gap is the hostname. The seaweedfs filer serving cert's SANs are `*.tenant-root.svc` and `*.tenant-root.svc.cluster.local`, which the 2-label `seaweedfs-s3.tenant-root` matches neither; the 3-label `.svc` FQDN does (and still resolves via search-domain expansion). Point S3_ENDPOINT at `https://seaweedfs-s3.tenant-root.svc:8333`. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Myasnikov Daniil
|
686a81a491 |
docs(networking): stop claiming the churn coupling is removed
Restoring the target-side pod deny put a cluster-spanning peer selector back on the webhook endpoint, which is the exact thing the source-side rules were added to take off it. The comments still said the coupling was "structural and removed", and the release note still said deleting a namespace no longer rewrites that endpoint's policy map. Both were true of the previous revision and are not true of this one. What ships now is two denies that fail in different directions: the source-side one keeps enforcing while the webhook endpoint's policy is being recomputed or has overflowed its map, the target-side one keeps enforcing where Cilium is not the service load balancer, and neither subsumes the other. So the coupling is not eliminated, it is made non-load-bearing -- whichever end is degraded the other still denies, and the map writes are writes rather than an outage. Eliminating it outright would mean dropping the target-side rule and accepting a silent fail-open, which is the worse trade. The chainsaw case's header claimed the deletion "briefly made fail-closed admission webhooks unreachable". That is the unconfirmed hypothesis, and the errno in the report contradicts it: EHOSTUNREACH cannot come from a policy denial, which drops after connect() has returned. It now says what the case actually asserts -- both webhooks still admit across a namespace deletion -- rather than claiming to reproduce a cause. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
0335421769 |
test(e2e): check every webhook registration, and name the controller Service
Three narrowing fixes to the two webhook cases. The availability precondition resolved only the FIRST registration of each webhook name (awk ... exit) while every matching registration fires on the applies below, so an unrelated flaky ingress-nginx release would have aborted an apply and been reported as a churn-induced outage. It now waits on all of them and says which one it is waiting for. Split into require_service_up plus a loop, indexed with sed rather than piped into `while read`, because a pipeline's body runs in a subshell under dash and a `return` from it would not reach the caller. That split immediately produced its own bug, caught by the scenario harness: POSIX functions share one scope, so the callee's `label="$1"` overwrote the caller's copy and the second registration reported itself as "(1/2) (2/2)". The callee now uses distinct names. The positive control's Service was picked with `grep -v -- '-admission$' | head -1`, which relies on ingress-nginx-controller sorting before ingress-nginx-controller-metrics. Deterministic but undocumented, and the control has to be the Service that actually exposes the https port, so -metrics is now excluded explicitly. Also drops a stale sentence: the churn pod's comment claimed the old target-side policies coupled the webhook endpoint to identity churn, which overstates a map write as an outage and is moot now that the target-side rule is back. The pod is there because an empty namespace allocates no identity, so without it the case would exercise nothing it names. Eleven scenarios through both scripts under dash with a stubbed kubectl, all as specified, the new multi-registration case included. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
b78405cede |
fix(networking): keep a target-side pod deny beside the source-side rules
Restores the deny that does not care who performs the service DNAT, which is what the source-side rules on their own gave up. Both policies now carry it on their webhook endpoint for the backend port, alongside the retained world deny, so losing cilium.kubeProxyReplacement degrades enforcement rather than removing it. I argued against this and was wrong. The argument was that a target-side rule needs a peer covering pods, that such a peer either reattaches namespace churn to a fail-closed endpoint or, as a wildcard, denies the API server. The second half does not hold: a CRD-sourced fromEndpoints is never a true wildcard, because for a clusterwide policy Cilium rewrites every selector to also require k8s:io.kubernetes.pod.namespace to Exist plus the cluster label (Cilium v1.19.5, pkg/k8s/apis/cilium.io/utils/utils.go:120-154 getEndpointSelector, whose own comment at :141 says a wildcard must not become a truly empty allow-all selector), and no reserved identity carries either label. So this rule denies pods and leaves host, remote-node and kube-apiserver allowed. `fromEntities: [all]` and an omitted peer are the shapes that would reach them. The first half was a cost dressed up as a correctness problem. A cluster- spanning peer is enumerated per identity, so the rule does add policy-map entries on the webhook endpoints in proportion to pod-identity count and a map write per namespace event. It does not add a drop window: pkg/endpoint/bpf.go:1168-1177 adds entries before deleting them expressly to avoid transient drops, and inverts that order only when the addition would overflow the map, warning when it does. The residual risk is therefore cardinality, not churn -- an endpoint near MaxEntries gets the inverted order, and cozystack leaves endpointLockdownOnMapOverflow false (charts/cilium/values.yaml:1250) so an overflow degrades rather than locking the endpoint down. Both templates now say that where the rule is, rather than leaving the cost unstated. The peer is the load-bearing part and both ways of losing it render as a plausible ingressDeny, so each suite pins it: rule count, the exact peer selectors, no fromEntities on the pod rule, no fromEndpoints on the world rule. Mutation-proved by making exactly the two substitutions that would deny the API server -- fromEntities: [all], and dropping the peer -- in both packages; all four render cleanly and all four go red. The ingress-nginx rule carries the konnectivity-agent ServiceAccount exemption for the same reason the source-side rules do: a Kamaji-hosted tenant cluster reaches this webhook through that pod, so without it guest-cluster Ingress admission breaks. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
3daa83f4b5 |
feat(cilium): surface a kube-proxy-replacement override instead of nothing
The pin added earlier guards a cozystack developer changing the default. It cannot observe an operator override in the field, and neither can the e2e, which only ever runs the default configuration -- so the path Ivan's review actually names, addons.cilium.valuesOverride on a tenant cluster and the cozystack.networking Package's cilium component on the host, produced no signal at all. Both webhook deny policies name their webhook's backend port only, which covers a pod dialling the ClusterIP solely because Cilium performs the service DNAT ahead of its own egress hook, so an override degrades that half silently. A status-beacon ConfigMap rather than a fail(), following the -awaiting-etcd beacon in packages/apps/kubernetes/templates/cluster.yaml. Whether Cilium without kube-proxy replacement is a supported cozystack configuration is a platform decision, and refusing to render the CNI over a weakened defence-in-depth control is not a call these charts should make. The host beacon fires on a bool false, on the string "false", and on the umbrella key being deleted, because the subchart's own "false" default coalesces into the parent. The tenant beacon fires on an explicit non-true override only, since absence means the chart default applies, and it renders on the management cluster next to the HelmRelease carrying the override -- which is why the mismatch is detectable there at all rather than inside the guest cluster. Also withdraws the argument the suite header used to carry, that a target-side ingressDeny could not be re-added as a fallback. It was wrong, and the next commit adds that rule. The header now records what the two halves each buy. Makefile gets .PHONY: test for the reason this branch already argued for kubeovn-webhook, which applies with more force here now that the suite is load-bearing for two other packages: hack/package.mk's .PHONY list deliberately carries only the targets that file defines, so a path named `test` would otherwise make the target up to date and skip the recipe. Proven both ways. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
scooby87
|
3a70f91423 |
fix(vm-disk): fail on storageClass/source edit of existing disk (#3969)
## What this PR does Fixes #2985. Editing a `VMDisk`'s `storageClass` or `source` after its DataVolume already exists was silently ignored: `packages/apps/vm-disk/templates/dv.yaml` looks up the existing DataVolume and, when found, reused its `spec` verbatim (because `DataVolume.spec` is immutable in CDI). The edit therefore rendered as a no-op — the typed CR accepted it and the HelmRelease reported success, while the DataVolume/PVC kept the original spec and the source of truth silently diverged from the cluster. The template now builds the desired `source` into a shared `$desiredSource` variable (used both for rendering and comparison) and, when a DataVolume already exists, calls `fail()` with an explicit message if the requested `storageClass` or `source` differs from the immutable existing spec — mirroring the explicit size handling already done in `pvc-resize-hook.yaml`. Storage growth is unaffected and still flows through the resize hook. `source` is also marked `@immutable` in `values.yaml`, so the generated schema now carries the same `x-kubernetes-validations` transition rule (`self == oldSelf`) that `storageClass` already had; `types.go`, `values.schema.json` and the generated `vm-disk-rd` CRD are regenerated to match. ### Verification - `helm unittest` covers both the immutability guard (via a mocked existing DataVolume through `kubernetesProvider`, since `lookup` needs a cluster) and initial-creation source rendering for every source type — 12/12 pass. - Verified live on a dev cluster with `helm install --dry-run=server` against a real DataVolume: an unchanged edit renders, a `storageClass` change fails, a `source` change fails, and a storage grow still renders the resize hook without tripping the guard. ### Scope note This PR only makes the drift visible and marks `source` immutable. Two deeper platform findings sit outside its scope: (1) #2657 (pre-existing) — the `apps.cozystack.io` aggregated apiserver does not evaluate the `x-kubernetes-validations` CEL rules on update, so `@immutable` is a no-op at admission for every Application kind, which is exactly why the chart-level `fail()` — not the CEL rule — is the substantive guard here; (2) #3971 — that same apiserver does not honour `--dry-run=server` and persists the change (surfaced while testing this fix). ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff file by file. The diff touches only `packages/apps/vm-disk/` (template, `values.yaml`, regenerated `values.schema.json`, tests), the regenerated `packages/system/vm-disk-rd/cozyrds/vm-disk.yaml`, and `api/apps/v1alpha1/vmdisk/types.go`. No package was added, renamed or removed; no `values.schema.json` field was added, renamed or removed (only a validation rule on the existing `source` field); no default changed; `README.md` is unchanged; the `cozyrds` file was regenerated, not renamed. The terraform provider models fields, defaults and enums but not CEL immutability, so it is unaffected. No downstream repository is reached by this change. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note fix(vm-disk): editing storageClass or source on an existing VMDisk now fails with a clear message instead of being silently ignored; delete and recreate the disk to change these immutable fields. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for blank, HTTP, image, disk, and upload VM disk sources. * VM disk sources and storage classes are now protected from unintended changes, while storage expansion remains supported. * Existing disks preserve their configured source and storage settings. * **Bug Fixes** * Invalid or conflicting source configurations are rejected. * Prevented silent no-op updates and incorrect immutability errors for existing disks. * **Documentation** * Documented source and storage class immutability, including the need to recreate disks when changing either setting. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
cbd0e78cad |
test(e2e): stop the webhook cases passing for the wrong reason
Both new cases could report a verdict they had not observed, and the first CI run of them did exactly that against a crash-looping ingress-nginx controller. admission-webhooks-survive-namespace-churn died in setup: the admission Service had no ready backend, so the first apply -- which passes through that same fail-closed webhook -- failed with "no route to host" before any churn ran, and the failure read as the outage under test. It now asserts both webhooks are up BEFORE the churn, resolving their coordinates from the webhook registrations themselves rather than from a guessed release name, and says so in its own words when they are not. Proven against the pre-change script with a stubbed kubectl: with a dead admission Service the old script proceeded into the churn (delete namespace, two applies, delete namespace, two dry-runs); the new one stops and names the cause without touching the churn namespace. admission-webhook-deny-still-enforced measured a corpse. Its `kubectl wait` did observe both pods Ready, but the controller was flapping on a ~100s cycle and the probe ran against a dead backend 19 seconds later. "Denied by policy" and "no backend to route to" are the same observation from inside the probe, so the two deny assertions would have reported success against a cluster with no policy installed at all, and the port-scoping assertion reported a policy bug that did not exist. The deny verdicts are now gated on the admission Service having ready backends before the probe, on every poll while it runs, and after it finishes, and on the controller pods neither restarting nor being replaced across that window -- EndpointSlice readiness rather than pod readiness, because a `kubectl wait` is what a flapping pod satisfies transiently. The probe reports reachable/unreachable per destination and always exits 0; this script holds the endpoint evidence and is the only place that can attribute a refused connect, so anything undecidable now fails as a setup failure instead of as a verdict. Ten scenarios driven through both scripts under dash with a stubbed kubectl, all behaving as specified: a dead backend, a backend dying mid-probe, a controller restarting mid-probe, a probe that never completed, an unregistered webhook, the deny lost, the deny widened to every 443 destination, and the healthy path. The restart-mid-probe case is the one that used to produce a confident false pass. Two more things the same rewrite fixes. The step budget was 6m against inner budgets summing to 420s, so a slow-but-healthy cluster was killed with no verdict and no describe output; both steps now carry a timeout above their own sum, per the rule in hack/e2e-chainsaw/.chainsaw.yaml, and the `kubectl wait` whose 180s dominated that sum is gone. The churn namespace now carries a running pod, because a Cilium identity is a set of endpoint labels including k8s:io.kubernetes.pod.namespace: an empty namespace allocates no identity and its deletion drives none of the UpdateIdentities path the old target-side policies coupled the webhook endpoint to. And both server-dry-run Pods carry a restricted-compliant securityContext -- never scheduled, but a Pod that PodSecurity would reject aborts the check under set -eu and reads as the webhook being down. Refs #3983. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Andrey Kolkov
|
86fe8e5a7e |
feat(valkey): add managed Valkey service (#3380)
## What this PR does Adds a **managed Valkey** service to the catalog, alongside managed Redis. > **Stacked on #3406** (redis-operator migration to freshworks-oss). This PR targets that branch and depends on it; review/merge #3406 first. ### Why Redis relicensed at 8.x to a source-available family (AGPLv3 / SSPL / RSALv2), and the managed Redis app currently ships `v8: 8.4.0` with no BSD-licensed option ≤ 7.2. That is a licensing risk for CNCF Incubation. [Valkey](https://valkey.io) is the BSD-3-Clause fork of Redis 7.2.4, governed by the Linux Foundation, and a drop-in replacement for Redis. This is the same move Argo CD made — [`cncf/foundation#750`](https://github.com/cncf/foundation/issues/750) was closed with "no need for exception" after they migrated to Valkey. ### How it works Valkey runs on the same operator as managed Redis. #3406 migrates that operator from the archived `spotahome/redis-operator` to the actively-maintained **freshworks-oss/redis-operator** fork, which adds a native `spec.engine: Valkey` switch. With it, the operator runs `valkey-server` / `valkey-cli` natively (in the launch command and in its liveness / readiness / shutdown probes), so the chart uses the official `valkey/valkey` image directly — **no Redis-compatibility shim image**. The app mirrors managed Redis: same shape/values (`replicas`, `resources`/`resourcesPreset`, `size`, `storageClass`, `external`, `version`, `authEnabled`), Sentinel-based HA, metrics exporter, `WorkloadMonitor`s, dashboard resource map, and `ApplicationDefinition` (`kind: Valkey`). Versions offered: `v8 → 8.1.8` (default) and `v7 → 7.2.13` (the BSD-3-Clause Redis 7.2 line, most conservative drop-in). ### How it was verified - `helm template` renders cleanly for `v8`, `v7`, and `external: true`; the `RedisFailover` carries `spec.engine: Valkey` and `image: valkey/valkey:<version>`. `helm lint` shows only the pre-existing icon-URL / `cozy-lib` warnings that managed Redis emits too. - `helm template` of `valkey-rd` renders a valid `ApplicationDefinition` (`kind: Valkey`, `plural: valkeys`, `category: PaaS`). - Generated artifacts (`values.schema.json`, `README.md`, `types.go`, `zz_generated.deepcopy.go`, the ApplicationDefinition `openAPISchema`) reproduce with the pinned `cozyvalues-gen` v1.6.0 and `controller-gen` v0.16.4 — root `make generate` and per-app generate both produce no drift; `go build ./valkey/...` passes. - A live-cluster e2e was not run from this environment; the Chainsaw suite `hack/e2e-chainsaw/valkey/` (mirroring the redis suite) is included and `hack/select-e2e.sh` maps the `valkey-application` source to it, so it runs in CI once the freshworks operator image is built. ### Notes - The Valkey logo (`packages/apps/valkey/logos/valkey.svg`) is a placeholder in Valkey's brand colors, pending the official brand asset. - A Grafana `db/valkey` dashboard was intentionally left out of this initial PoC; it can follow. ### Screenshots N/A — no UI changes. ### Downstream repositories This adds a new managed-app kind, which likely warrants follow-ups in `cozystack/website` (user-facing docs for the new service) and possibly `cozystack/terraform-provider-cozystack`. No follow-up PRs are opened yet — leaving these for a maintainer to decide, per the template guidance. - [ ] 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 ```release-note feat(valkey): add managed Valkey service — a BSD-3-Clause, Linux Foundation-governed alternative to managed Redis, running on the freshworks-oss redis-operator via native spec.engine: Valkey (versions 8.1.8 and 7.2.13) ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added managed Valkey service support with high availability and configurable replicas. * Supports Valkey versions 7 and 8, persistent storage, resource presets, authentication, and optional external access. * Added monitoring metrics and dashboard integration. * Added platform package integration and tenant administrator access. * **Documentation** * Added Valkey configuration guidance and storage-class immutability documentation. * **Tests** * Added end-to-end coverage for deployment readiness, storage, and replicas. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
6a289cf6d0 |
test(cilium): pin the service-LB assumption both webhook denies rest on
Both admission-webhook deny policies name only their webhook's backend port. That covers a pod dialling the ClusterIP because Cilium translates a service to its backends before it enforces egress policy -- in v1.19.5, bpf/bpf_lxc.c runs __per_packet_lb_svc_xlate_4 and lb4_local() and only then tail-calls into handle_ipv4_from_lxc where policy_can_egress4 evaluates the rule. That ordering exists only while Cilium performs the DNAT, and the only reason it does is cilium's kubeProxyReplacement: true, set against an upstream chart default of "false". Set it false and the DNAT moves downstream of Cilium's egress hook: the packet arrives with dst = ClusterIP, matches no endpoint identity, and with default-deny off it is allowed. Both webhooks regain a pod -reachable ClusterIP path while the backend deny keeps working, so nothing fails and nothing is logged. Same shape as the port disagreement namespace-annotation-webhook.port already exists to prevent, and it had no pin. The suite asserts the rendered cilium-config key rather than .Values, so an upstream bump that renames it or stops honouring the override is caught too, and it spells out each of the five variant valuesFiles combinations from sources/networking.yaml so a future variant file that overrides the value reds here. Proven both ways: flipping values.yaml reds all six cases, and adding the override to values-kilo.yaml alone reds exactly the cilium-kilo case. The suite also records why a target-side ingressDeny is not available as a belt-and-braces fallback. A target-side rule needs a peer selector covering pods, and the peer selector is what carries namespace churn -- the failure being removed here. pkg/policy/selectorcache.go UpdateIdentities notifies a selector only when the churned identities were in its own cached selections, and pkg/policy/l4.go IdentitySelectionUpdated then writes the policy map of every endpoint the rule's endpointSelector selects, which for a target-side rule is the fail-closed webhook pods. The one churn-free peer is the wildcard, which Cilium collapses to a single ANY-identity entry and skips identity updates for -- but ANY covers reserved:host, remote-node and kube-apiserver, so it would deny the API server. There is no third shape. Refs #3983. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
042ac2715b |
test(kubeovn-webhook): declare the test target phony, fix the port comment
hack/package.mk's .PHONY line does not carry `test`, so a path named `test` next to the Makefile makes the target up to date and make skips the recipe. Proven both ways: with the declaration the three suites run with such a file present, without it make reports "'test' is up to date" and asserts nothing. That is the same fail-open shape the port agreement guard exists for, and hack/helm-unit-tests.sh only catches it after the fact. The port comment named a ValidatingWebhookConfiguration. This package ships a MutatingWebhookConfiguration, whose clientConfig.service omits `port` so the API server defaults it to 443. The assertion was right; the sentence explaining it was not, and it is the only place a reader learns why 443 is pinned. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
9f485a18ad |
docs(ingress-nginx): keep the policy rationale out of the render
The rationale above the CiliumClusterwideNetworkPolicy was a `#` block, which
Helm copies verbatim into the manifest: 53 comment lines shipped with every
render. The sibling policy added alongside it already uses `{{/* */}}`. Convert
this one to match; the text is unchanged.
Also state in the values description what `enabled: true` actually promises.
The template renders only while cilium.io/v2/CiliumClusterwideNetworkPolicy is
discoverable, so on a cluster without it the default emits no policy at all and
leaves no Event or render-time message behind. `enabled: false` is what
distinguishes an opt-out from an absent API.
Assisted-By: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
||
|
Myasnikov Daniil
|
f73b54fee9 |
test(e2e): assert the webhook deny is enforced and scoped to its port
The churn case added alongside it proves the webhooks stay AVAILABLE. Nothing proved they are still UNREACHABLE from pods, which is the property the policies exist for, and no test anywhere checked that the deny is confined to the webhook port -- which is why a rule that denied all TCP/443 egress cluster-wide passed every existing assertion. Probe from tenant-root, where the host cluster's ingress-nginx lives and where allow-internal-communication permits pods to reach it, so the deny is the only thing that can block the webhook port; from the isolated tenant-test the probe would pass for the wrong reason. Three assertions against the same pods and the same tenant egress allowances, differing only in port: the controller's own HTTPS port must stay reachable, the admission Service ClusterIP must not be, and the backend port must not be. The first is the one that separates a scoped deny from a wildcard. Uses busybox:musl, already staged on every node by hack/e2e-prepull-images.sh, with a securityContext that satisfies the restricted PodSecurity profile. Service names, ports and the backend port are discovered at runtime rather than hardcoded, since the release name differs between installs. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
bdc172a648 |
test(networking): pin the deny shape and run the kubeovn-webhook suites
The suites asserted the toServices rule that produced the cluster-wide 443 deny, so they pinned the defect: every rendered-YAML assertion passed on the broken policy, and removing the rule would have read as the regression. Replace those cases with the invariant that was actually missing -- one deny rule per spec, and its destination must be a selector Cilium can resolve (toEndpoints present, toServices absent, spec count fixed). Verified by mutation: reintroducing the toServices rule fails these assertions in both packages. Not used: matchRegexRaw / notMatchRegexRaw. helm-unittest 1.0.3 silently ignores both -- a notMatchRegexRaw against a pattern guaranteed to be present still reports pass -- so a document-wide regex guard would have asserted nothing. packages/system/kubeovn-webhook had no Makefile test target, and hack/helm-unit-tests.sh discovers suites by probing `make -n test` per package and skips any package without one. All three of its suites -- including the port agreement guard, whose whole purpose is to catch a drift that fails open -- had therefore never run in CI. Add the target; the sweep now reports "Running tests in packages/system/kubeovn-webhook". Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
6b2b68a685 |
fix(networking): drop unresolvable toServices deny from webhook policies
Cilium resolves toServices only inside ALLOW rules: pkg/policy/k8s/service.go
walks spec.Egress in processRule, hasMatchingToServices and specHasToServices,
and never spec.EgressDeny. An egressDeny whose only destination is a
toServices therefore never gains a ToEndpoints/ToCIDRSet, and the
EgressDeny -> PolicyEntry conversion in pkg/policy/utils/parserules.go does not
pass ToServices to mergeEndpointSelectors at all. Its nil-guard only fires for
a non-nil empty slice, so with every other destination field absent it returns
make(types.Selectors, 0, 0) -- non-nil and empty, which
pkg/policy/types/policyentry.go documents as an implicit WILDCARD whenever the
rule carries toPorts.
The rule therefore did not deny the webhook Service on 443; it denied every
destination on TCP/443 for every endpoint its spec selected -- all pods outside
kube-system for ingress-nginx, and with endpointSelector {} every pod in the
cluster for kubeovn-webhook. Cilium deny beats allow, so the tenant
allow-external-communication policy could not offset it. Validation accepts
the construct (l3DependentL4Support marks ToServices supported), which is why
it failed silently.
Deny the backend port only. That already covers ClusterIP-addressed traffic,
because Cilium translates a service to its backends before enforcing egress
policy -- the same reason its own toServices support resolves to backend
selectors rather than the frontend. The kubeovn-webhook servicePort helper
existed only to feed the removed rule, so the Service goes back to a literal
443.
Also correct the four comments that documented the frontend deny as working,
record why the world deny is kept (cilium monitor showed the API server
arriving as host / kube-apiserver, never world), and state plainly that the
namespace-churn coupling is structural rather than a reproduced root cause for
issue 3983.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
||
|
Myasnikov Daniil
|
f224baae46 |
docs(networking): document webhook policy scope
Explain which callers the source-side Cilium policies select and how the Service and backend ports are kept in agreement. Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
ad961f6de1 |
test(e2e): cover webhook admission after namespace churn
Exercise ingress and pod admission immediately after deleting a tenant namespace, and route ingress-nginx changes to the gateway suite. Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
fc5dc66c61 |
fix(networking): isolate webhook policies from namespace churn
Move pod denials to source-side Cilium policies so namespace churn cannot regenerate fail-closed webhook endpoints. Keep the external world deny as a static target rule. Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Andrey Kolkov
|
329ef9cf3f |
test(backups): make rabbitmq backup e2e self-contained via demo bucket
The rabbitmq backup roundtrip was the only backup e2e routing through the platform cozy-default system bucket, whose advertised endpoint in the CI sandbox is the unroutable s3.example.org placeholder, so the backup Job failed with "Could not resolve host: s3.example.org". Mirror the clickhouse/mariadb backup examples: provision a demo Bucket, project its access keys into rabbitmq-backup-creds and copy the seaweedfs CA into rabbitmq-backup-ca, and render a self-contained Rabbitmq strategy + rabbitmq-default BackupClass whose curl uploads to the in-cluster seaweedfs endpoint (S3_ENDPOINT override, CURL_CA_BUNDLE for the self-signed cert). The in-Pod script is kept identical to the shipped cozy-default-rabbitmq strategy, which is left unchanged; the demo only reuses its client image. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey KolkovandClaude Opus 4.8
|
99abee5da3 |
build(redis-operator): build the operator image in CI
The valkey readiness fix is a source patch on the operator image (patches/valkey-cli-auth.diff), but nothing rebuilds that image: the root build target has no redis-operator entry, and hack/build-matrix.sh derives the CI build set from those entries, so e2e runs the hand-pinned pre-patch digest and the patch ships zero bits. Register the image build so CI rebuilds the operator from source and the patch reaches the tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
myasnikovdaniil
|
67b4e23ca2 |
test(tests): cut the noise in unit and e2e job logs (#3973)
## What this PR does Unit test job log is 238,794 lines and 31MB, and 233,444 of those are `set -x` trace from `hack/cozytest.sh`. The one failing assertion sits at line 238,573. Nothing in that stream is needed there, `run_one` already tees raw output to `$log` and the fail handler dumps all of it for whichever test fails, so dropping the live stream loses output of passing tests only. `COZYTEST_TRACE` defaults to 1, so e2e call sites in `packages/core/testing/Makefile` keep the live stream. For a long e2e suite that stream is the only progress signal, and the only record at all when a step timeout or a dead runner kills the job before the fail handler runs. `bats-unit-tests` sets 0. Running the runner by hand stays verbose, that is the case where a human is actually watching. Second thing, previous-instance log capture now orders by restart recency. Container cap (`COZY_PREVLOG_MAX=12`) is spent in namespace order and "unrelated" meant only "in another namespace", time was never a key. So a container that restarted once during bring-up competed on equal footing with one that crash-looped during the test, and won whenever the pod list reached it first. Two measured e2e jobs spent 1034 and 1545 lines on `virt-api` and `cdi-apiserver`, both restarted once at install time by `Unable to create certwatcher`, hours before any app suite ran. On a live stand 178 containers carry a restart and their timestamps span 24 days, so the cap binds hard enough for order to decide everything. This orders, it does not filter. Install-time restart is the explanation when install is what broke, and the capture has no reference point for "when the test began" that would make a cutoff honest. Namespace stays primary key, because a stale restart in the namespace under test beats a fresh one three namespaces away. So it fixes cap displacement, not raw volume of a single run. `lastState.terminated.finishedAt` joins the row. The guard on it is three deep and the innermost one is load bearing, which I got wrong first time round and review caught: `finishedAt` is a metav1.Time behind `json:",omitempty"`, omitempty cannot omit a struct, and the zero value marshals to `null`. The kubelet does synthesize a terminated state with no time, ContainerStatusUnknown on a pod deleted mid-run, which also bumps restartCount and so clears the restart filter. kubectl renders that leaf as the literal `<no value>`, and `<` sorts above every digit, so under the reverse sort that row landed FIRST and displaced the most recent real restart. Exactly the inversion the ordering exists to prevent, plus a header reading `previous instance ended <no value>`. Fixed at both layers because they fail independently: the template guards the leaf so the string cannot be produced, and the sort normalises any field that is not shaped like a timestamp to empty so the ordering holds even if some other producer emits one. Worth naming how that slipped through. I probed the template against a live cluster, saw no `<no value>`, and read it as the guard being correct. It only meant no container on that cluster was in that state. Reading the field also fixes a latent bug it would otherwise have caused: `read -r ... restarts` folds a trailing field into the last variable, so the dump header would print `restarts=1|2026-08-22T09:06:00Z`. Now it names the time, which is the first thing telling a reader whether the dump predates the test. ### What quiet mode keeps and what it drops Two things are dropped, and the second one came out of review rather than out of me. First, xtrace and stdout of tests that pass. You cannot diff a passing test's trace against the failing one inside the same job log, and a warning printed by a test that passed is gone. Second, a test that is KILLED rather than failed loses its trace as well. The fail handler runs only after the pipeline returns, so a hang reaped by the job timeout never dumps anything, and quiet mode has by then dropped the live stream. Verbose mode never noticed, because the trace was already printed. Measured: verbose leaves the line the test blocked on, quiet leaves the `╭` and nothing else. So this one is a real regression on the unit lane, not a pre-existing gap. An INT/TERM trap is the obvious repair and it does not work. POSIX sh defers a trap until the current foreground command returns, so installing one stops the runner dying on the signal at all: a suite hung on `sleep 30` took 30s to exit with the trap against 2s under the default disposition. GitHub escalates INT, then TERM, then KILL, so the deferred handler gets SIGKILLed before it ever runs, and the grace period goes on waiting for the hang. Built it, measured it, reverted it, and the measurement is in the comment now so the next reader does not repeat it. Making it work needs the test pipeline moved into the background with `wait`, which is the interruptible one, and that is surgery on the path every suite in this repo takes for a partial trace in a rare case. What is left for that case is the escape hatch below, which for this lane costs 48s and no cluster. The same argument is why e2e keeps the stream on: re-running there is an hour and a live cluster, so the record has to be written as it goes. Everything else stays. The `╭`/`╰` pair per test with elapsed time, so which tests ran, in what order and how long each took is all still there. Full trace, stdout and stderr of the failing test, same bytes as before, because `run_one` tees to `$log` and the fail handler dumps it, and that path is untouched. Anything printed outside `run_one` as well, file level code, `.bats` parse errors, and the `--- running <file> ---` markers. A hang still shows up as `╭` with no matching `╰`. And `make unit-tests` stops at the first failing test anyway, so there was never more than one dump to read. Measured on a suite that is actually red (`hack/ghcr-mirror_test.bats` fails on my machine because it reads a live cluster): verbose 1472 lines, quiet 63 lines, and the post-failure dump is 23 lines and byte-identical in both modes. Getting the trace back, both paths checked: ``` COZYTEST_TRACE=1 make unit-tests # env prefix, 29200 trace lines make unit-tests COZYTEST_TRACE=1 # make cmdline, same hack/cozytest.sh hack/foo.bats # by hand, verbose by default ``` This lane needs no cluster and runs locally in 48s, so anyone who wants the trace just reruns it. That is exactly why the same argument does not extend to e2e, and why the default stays 1 there. Nothing else consumed that stream. Unit lane stdout goes only to the job log, `cozyreport.sh` folds in crust-gather snapshots and the trap writes those to disk independently, and the unit lane produces none of them. ### Verification Measured on this PR's own run: unit job went from 238,794 lines and 31MB with 233,444 trace lines, to 5,720 lines and 580KB with 0 trace lines and 1548 tests OK. Those two totals are not strictly like for like, the old run was red and this one is green, so the honest comparator is 233,444 trace lines down to 0. Quiet output is byte-for-byte the non-trace subset of verbose, and the fail handler dump is unaffected (checked with a deliberately failing suite). Verbose output unchanged against main, byte-identical sorted on `hack/bats-no-exit-trap.bats`. Other suites are not self-stable run to run (temp names, epochs, stdout/stderr interleaving) so that is the ceiling on that check, not a finding. Eight tests on the capture and seven on the switch. Every fix is mutation proven: reverting the template leaf guard, removing the sort key normalisation, dropping the read variable, dropping `-s`, silencing an e2e recipe, removing the fail handler dump, flipping the default, and making the switch a no-op are each caught. Three of those guards did not bite when first written, which review also found: both stability fixtures were pre-sorted ascending, which is what sort's last-resort whole-line compare reproduces with no `-s` at all, and no test fed a six-field row through the whole script, so deleting the read variable left the suite green at 39/39. Whole unit lane, 83 files, 1503 passed and 1 failed. The failure is `hack/ghcr-mirror_test.bats`, it reads a live cluster and fails identically under main's own runner. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map file by file against the diff. Nothing under `hack/` is moved or renamed, no make target changes what it does (`bats-unit-tests` runs the same files to the same verdict, only quieter), and `hack/package.mk`, `hack/common-envs.mk`, `hack/update-crd.sh` and `hack/e2e-prepare-cluster.bats` are untouched, which is what ccp, external-apps-example, talm and ansible-cozystack couple to. No package `values.yaml`, `values.schema.json`, CRD, namespace, label, annotation or metric name is involved. - [x] No downstream repository is affected by this change ### Release note ```release-note test(tests): bats unit test trace is now opt-in via COZYTEST_TRACE and quiet by default in `make unit-tests`, and the e2e previous-instance log capture now picks the most recently restarted containers first ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Improvements** - Test runs can suppress live trace output for passing tests while preserving full diagnostics for failures. - Trace visibility can be customized through the test configuration. - Previous-instance logs are prioritized by the most recent container termination within the preferred namespace. - Captured log headers now show when the previous instance ended. - Undated logs are retained and placed after dated entries. - **Tests** - Added coverage for trace controls, failure handling, and recent-log ordering. <!-- end of auto-generated comment: release notes by coderabbit.ai -->v1.7.0-alpha.1 |
||
|
myasnikovdaniil
|
5688b3c66a |
fix(ingress-nginx): re-pin to the segfault-free v1.11.5 rebuild (#4012)
Re-pins the ingress controller to the rebuilt `v1.11.5`. The digest pinned before this segfaults. `pb.c` in lua-protobuf violates strict aliasing and gcc 14 at `-O2` acts on it, so `pb.so` in the previously pinned build makes nginx `cache manager` and `cache loader` children segfault continuously, tens per second and it never stops. Workers are untouched, so the pod stays `Ready` with `restarts=0` and nothing surfaces until master itself dies, which is how it got here unnoticed. Fixed in cozystack/ingress-nginx-with-protobuf-exporter#7, tag `v1.11.5` re-pointed at that merge and both images rebuilt. Verified against the published image, not a local build: * `luajit -e 'require "protoc"'` exits 0, was 139. * 25 second nginx run on a real generated config: 0 `signal 11`, was 105. 0 emerg, all four lua modules load. * `luarocks` and `gcc` are gone from the runtime image now, so `apk` no longer upgrades musl underneath the nginx the base image already built. The exporter is re-pinned too, because one tag builds both images and the rebuild published a new exporter digest. Its source did not change. `release-1.5` and `release-1.6` pin v1.11.2 and were never affected, so nothing to backport. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated container image versions for the ingress controller and protobuf exporter to newer verified builds. * Existing image tags and configuration remain unchanged. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey KolkovandClaude Opus 4.8
|
ba32df69c0 |
feat(backups): add RabbitMQ definitions backup via a dedicated strategy driver
RabbitMQ's cluster operator ships no backup CRD and RabbitMQ has no
consistent online export of message payloads, so back up broker
DEFINITIONS (vhosts, users, permissions, queues, exchanges, bindings,
policies, parameters) over the management HTTP API and store them in the
system bucket. Message payloads and PITR are out of scope - a full-data
backup is a Velero volume snapshot, a separate strategy.
Add a first-class strategy.backups.cozystack.io/Rabbitmq driver, following
the Altinity pattern (a dedicated driver that renders a one-shot Job for an
HTTP-API engine):
- reconcileRabbitmq / reconcileRabbitmqRestore render the strategy's
PodTemplateSpec (curl GET/POST /api/definitions, push/pull to S3 with
--aws-sigv4) as a batch/v1.Job, for both in-place and to-copy restore.
- A Go-read precondition gates each run on the RabbitmqCluster reporting
AllReplicasReady (via the rabbitmqtypes partial type), surfacing a
precise Ready=False wait instead of a Job that starts and fails.
- The produced Backup records the object location on status.artifact.uri
(rendered from spec.artifactURITemplate; the Backup CRD has no status
subresource, so it persists on Create, as in the Etcd driver).
Wire dispatch in both controllers + supportedBackupStrategyKinds, register
rabbitmqtypes in the scheme, and grant read on rabbitmqclusters. Ship
cozy-default-rabbitmq (a $bucketName-gated Rabbitmq strategy) and route
apps.cozystack.io/RabbitMQ to it in the cozy-default BackupClass. The
application chart is untouched.
Add unit tests for the driver's pure helpers and the precondition gate, a
runnable example, and an honest-roundtrip Chainsaw e2e that seeds a
sentinel vhost, backs it up, drops it, and asserts it round-trips through
object storage on both restore flows.
The Job pod reuses the platform-migrations image (curl + ca-certificates)
under a new rabbitmqBackupClientImage key, stamped in lockstep with
chBackupClientImage so the repository stays pinned at one digest; a
dedicated slim image is a follow-up.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
|
||
|
Myasnikov Daniil
|
127cc2ce9a |
fix(ingress-nginx): re-pin to the segfault-free v1.11.5 rebuild
The v1.11.5 tag in cozystack/ingress-nginx-with-protobuf-exporter now points at the merge of its #7, which builds lua-protobuf with -fno-strict-aliasing. The digest pinned here was the build before that, whose pb.so makes nginx's cache manager and cache loader children segfault continuously: tens per second, indefinitely. The workers are unaffected, so the pod stays Ready with no restarts and nothing surfaces until the master itself segfaults, which is how this reached main unnoticed. Verified against the published image rather than a local build. The one-line repro, luajit -e 'require "protoc"', exits 0 where it exited 139, and a 25 second nginx run on a generated ingress-nginx config produces no signal 11 where the pinned build produced 105. The exporter is re-pinned alongside it because one tag builds both images, so the rebuild published a new exporter digest too. Its source did not change. release-1.5 and release-1.6 pin v1.11.2 and were never affected. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
scooby87
|
dc2e427262 |
docs(etcd): note that affinity is not applied to existing members (#3964)
## What this PR does Documentation follow-up to #3962. Both reviewers (CodeRabbit and @androndo/@IvanHunters) noted that the new `affinity` value is not applied retroactively: the etcd-operator sets `spec.affinity` only when it creates or replaces a member Pod and never reschedules running members. On a cluster that already has co-located members (the exact failure #3962 targets) the default soft rule — or a newly set hard rule — takes effect only as each member is replaced. This adds that caveat to the `affinity` parameter documentation in `values.yaml` and regenerates `values.schema.json`, `README.md`, and the embedded `openAPISchema` in `etcd-rd`. No template or behaviour change; `make test` passes (20/20). The other inline note (the soft podAntiAffinity default overlaps the soft `topologySpreadConstraints`) was explicitly flagged as "fine to keep as-is", so no code change is made for it. > Note: this PR is stacked on `fix/etcd-soft-antiaffinity` (#3962) and will retarget to `main` once that merges. ### Screenshots n/a — no UI changes. ### Downstream repositories Doc-only change to an app chart (regenerated artifacts). No package added/renamed/removed, `kind`/`plural` unchanged, no installer values/assets/variants touched. No downstream follow-up required. - [x] No downstream repository is affected by this change ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified that etcd affinity rules apply when member Pods are created or replaced. * Noted that running members are not rescheduled retroactively; the rule takes effect as each member is replaced. * Updated the guidance consistently across configuration documentation and schema descriptions. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
c8d073fff7 |
fix(api): drop caller-supplied internal.cozystack.io labels in tenantsecret writes (#3599)
## What this PR does Labels and annotations under `internal.cozystack.io/` belong to the platform. The lineage webhook stamps `internal.cozystack.io/tenantresource` to decide whether a Secret is visible through this API at all, and the cacert controller stamps `internal.cozystack.io/tenant-ca`, which an `ApplicationDefinition` selects on in `spec.secrets` to hand a trust anchor to the tenant. The registry copied every caller-supplied key onto the backing Secret as is, so anyone with write access to `tenantsecrets` could plant either marker on a Secret of their choice, or strip one to take an object out of the platform's view. `tenantToSecret` is the only place the write verbs go through, PATCH included, because `rest.Patcher` is `Getter`+`Updater` and the endpoint installer serves PATCH out of `Update`. It now skips keys with that prefix. The update target starts as a copy of the stored Secret, so a platform key cannot be removed either. The guard is on the whole prefix, so a platform key added later is covered without anyone touching this code again. Reads are unchanged. A tenant can see the platform's verdicts, it just cannot write them. Keys are dropped silently instead of rejected, otherwise a GET, edit and PUT round trip would break `kubectl edit` for no gain. Nothing writes through this API today. Every `tenantsecrets` grant in `cozystack-basics` is `get`/`list`/`watch`, and the console only lists and watches. That read-only RBAC is what keeps the hole inert, and it lives in a different package from the registry that implements the write verbs, which is why I would rather close it here than keep relying on it. Two things I found next to the diff and did not fix. The `Patch` method on this type is never reached: its signature is the `client-go` shape and matches no interface the endpoint installer asserts, so no request lands there. I gave it the same guard instead of leaving the weaker of two doors, but deleting it is probably the better answer and I would rather that call was yours. A repair after the write is weaker than the conversion anyway: the forged key is live between the patch and the repair, and forcing the object back to a pre-patch snapshot drops a key that a controller stamped inside that window. Second one, and the reason none of this is easy to spot: `Update` passes `nil` where the handler hands the storage the current object, so `hasUID` reports false and the patch mechanism takes its create-new-object branch. For JSON Patch, merge patch and strategic merge patch that branch returns `NotFound`, so all three get a 404 against an existing TenantSecret. Apply is the exception. `applyPatcher.createNewObject` builds a fresh object of the kind and applies onto that, so an apply does reach `Update`, carrying only the fields the caller sent, and the merge in `tenantToSecret` is what keeps the stored labels. Passing `secretToTenant(previous)` instead of `nil` is the fix and it belongs in its own change, since it makes the other three patch types live. Tell me if you want issues for either. Closes #3464 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: Walked the trigger map against the diff: two Go files under `pkg/registry/core/tenantsecret/`, no package added, no chart, values schema, CRD, `ApplicationDefinition` field, namespace, `hack/` target or release asset touched. Nothing in the map fires. ### Release note ```release-note fix(api): TenantSecret writes no longer accept caller-supplied `internal.cozystack.io/*` labels and annotations, so a principal with write access to the API can neither plant nor strip the platform markers that decide tenant visibility on the backing Secret. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Strengthened protection of platform-managed metadata during tenant-secret creation and updates. * Prevented tenant-provided platform-owned labels and annotations from overwriting protected values. * Preserved standard metadata while hiding sensitive resource markers. * Improved patch-based update handling and protected metadata consistency across changes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey KolkovandClaude Opus 4.8
|
ec0785fe2a |
fix(redis-operator): use REDISCLI_AUTH for valkey-cli auth in generated scripts
The freshworks-oss operator authenticates the CLI in its generated readiness and shutdown scripts by exporting the engine's auth env var before invoking it. For the Valkey engine it used VALKEYCLI_AUTH, but valkey-cli honours only REDISCLI_AUTH (its --help documents that name and no VALKEYCLI_AUTH). With auth enabled the readiness probe's valkey-cli call therefore returned NOAUTH, the role never resolved, and the data pods never became Ready. Patch valkeyEngine.CLIAuthEnvName to REDISCLI_AUTH so auth-enabled Valkey instances pass readiness. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Aleksei Sviridkin
|
df20f69e5b |
chore(ci): re-run with diagnostics enabled
Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ef4f5b45ab |
fix(api): drop caller-supplied internal.cozystack.io keys in tenantsecret writes
Labels and annotations under internal.cozystack.io/ are platform verdicts, not tenant metadata. The lineage admission webhook stamps internal.cozystack.io/tenantresource to decide whether a Secret is visible through this API at all, and the cacert controller stamps internal.cozystack.io/tenant-ca, which an ApplicationDefinition selects on in spec.secrets to promote a trust anchor to the tenant. The registry copied every caller-supplied key onto the backing Secret verbatim, so a principal holding write access to tenantsecrets could plant either one on a Secret of its choosing, or strip one to take an object out of the platform's view. Only read-only tenant RBAC, which lives in a different package, stood between a caller and that path. tenantToSecret is the choke point every write verb goes through, PATCH included: rest.Patcher is Getter+Updater, so the endpoint installer serves PATCH out of Update. It now skips the platform keys, and since the update target starts as a copy of the stored Secret, ignoring the caller there also means a platform key cannot be removed. Guard on the whole prefix rather than on a list of keys, so a platform key added later is protected without anyone remembering to come back here. Delete the Patch method on this type. Its signature matches no interface the endpoint installer asserts, so no request has ever reached it, and as a write path it was strictly weaker than the conversion: the raw patch hit the backing Secret before any guard ran, leaving a forged key live in etcd -- visible to every controller and to List -- until a second write put the platform keys back, or for good when that second write failed. One door is better than two, and the surviving one commits once. Keys stay visible on read. A tenant may see the platform's verdicts, and failing a write that echoes them back would break a read-modify-write round trip for no gain, so they are dropped silently instead of rejected. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
c94a1da5bb |
feat(mongodb): add TLS support via PSMDB operator-managed certificates (#2692)
## What this PR does Enables operator-managed TLS for MongoDB and stops handing tenants any Secret that carries private key material. - Percona Server for MongoDB serves TLS unconditionally (`mode: preferTLS`), so this PR drops the misleading `tls.enabled` toggle rather than pretending the chart controls it. - The tenant no longer receives the operator's CA Secret (`<release>-ca-cert`, which holds `ca.key`) or the leaf (`<release>-ssl`). Both were previously reachable, and either one is a private key in tenant hands, the CA one is a full trust-chain compromise. - The chart declares a `TenantProjection` sentinel naming its CA source Secret (`<release>-ssl`, the operator-managed leaf, which carries `ca.crt`), and the ResourceDefinition selects the resulting key-free `<release>.tenant-ca` projection by the engine-agnostic `internal.cozystack.io/tenant-ca` label. The CA-extraction controller reads the sentinel and publishes the projection holding only `ca.crt`. The tenant gets what it needs to verify the server and nothing else. ### Why the include also carries an exclude list The trust-anchor selector matches by label with no name constraint, so any Secret in the namespace that acquired that label would be promoted with its data copied verbatim. The enumerable dangerous set is therefore excluded explicitly: the CA, both leaves, the rotation snapshots (`-ssl-old`, `-ssl-internal-old`, same server keys as the leaves, and unlike them they carry no owner reference, so an interrupted rotation can leave them behind after the release is gone), the system-user passwords, the at-rest encryption key, the keyfile, the object-storage credentials, and both forms of the operator's internal Secret. That backstop is bounded, and the comment says so: matching is by exact name with no globs, so a Secret a future operator version introduces, or a transient object an issuer creates mid-renewal, stays in scope of the label-only include. The per-user Secrets cannot be enumerated at all, since their names carry the username. The primary defence remains that the controller stamps only its own projection; this closes the enumerable set, not the whole surface. ### Release note ```release-note feat(mongodb): stop exposing TLS private key material to tenants and publish the CA as a key-free trust anchor ``` |
||
|
Alexey Artamonov
|
6fdb5e0840 |
docs(etcd): note that affinity is not applied to existing members
The operator applies spec.affinity only when it creates or replaces a member Pod; it does not reschedule running members. Document that on the `affinity` value so operators do not expect the default (or a newly set rule) to redistribute already co-located members until each is replaced. Addresses the non-blocking review notes on #3962. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Andrey KolkovandClaude Opus 4.8
|
daa867c545 |
fix(valkey): label RedisFailover with operator-group so it is reconciled
The freshworks-oss redis-operator filters its RedisFailover List and Watch with a server-side selector on OPERATOR_GROUP_ID (cozystack). A RedisFailover without the matching operator-group label is never seen by the operator, so no data pods, sentinels, PVCs or services are created. Match the redis app chart, which already sets the label. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Aleksei Sviridkin
|
edd767ea0f |
feat(nats): publish ca.crt-only trust anchor to tenants (#3340)
## What this PR does Converges nats onto the key-free CA trust-anchor contract. A TLS-enabled nats release renders a `TenantProjection` sentinel naming the chart's cert-manager CA Secret as its source, and the nats ResourceDefinition selects the resulting `<release>.tenant-ca` object by label. The projection is built by the CA extraction controller in `internal/controller/cacert`, which parses the source certificate and re-encodes it into a single `ca.crt` key, so a tenant that enables TLS gets something to verify the server against without ever getting a private key. No key-bearing Secret is granted to a tenant, by name or by label. `packages/apps/nats/templates/certmanager.yaml` is not touched. An earlier draft of this change stamped publish labels onto the CA `Certificate`'s `secretTemplate`; the controller that shipped consumes a sentinel object instead, so the chart now declares its source rather than marking it. The functional change is one new template and one `matchLabels` entry in the ResourceDefinition. The sentinel is gated on the chart's effective TLS state, which is tri-state: left unset it follows `external`. That gate is the single divergence from the postgres implementation of the same contract, and it exists because the nats CA is only issued while TLS resolves true. An ungated sentinel would sit `Ready=False/SourceNotFound` forever on every plaintext release. The CA extraction controller is already on main, so this is live on merge rather than dormant until a follow-up lands. The `do-not-merge/hold` label predates that and no longer guards anything. ## Verified against a running cluster The premise this rests on is that cert-manager populates `ca.crt` on the Secret of a `selfSigned`, `isCA: true` Certificate. Nothing in this repository witnesses it: `hack/e2e-chainsaw/` has no nats suite, and nats appears in no e2e workflow, so every unit test here encodes the same assumption the chart encodes and the two cannot catch each other being wrong. I closed it against a live cluster instead, running cert-manager v1.20.2, the version this repo ships, on two independent nats releases: - `nats-<name>-ca`, the chart's CA Secret, holds `ca.crt`, `tls.crt` and `tls.key`. The declared source key exists. - `nats-<name>.tenant-ca`, the projection, holds exactly one key, `ca.crt`. - The projection's SHA-256 certificate fingerprint equals the source's, so it carries that CA and not some other certificate. - The projection carries `internal.cozystack.io/tenant-ca: "true"` and `internal.cozystack.io/tenantresource: "true"`. The second is stamped by the lineage webhook, computed from the ResourceDefinition selector this PR adds, so its presence means the whole chain held. - `kubectl get tenantsecret nats-<name>-ca` returns NotFound. The key-bearing Secret is not served through the tenant API. What is still missing is an automated regression witness for that premise. Adding one means a new chainsaw suite for nats, which has no e2e coverage of any kind today, and that is a larger change than this one. ## What the tests pin 61 helm-unittest cases and 3 bats cases. The sentinel's absence of `ownerReferences` (an owner would short-circuit the lineage walk before it reaches the Flux label the attribution depends on), the length of `spec.projections` as well as its single entry (the controller refuses a multi-entry sentinel, so a second entry is a silent loss of the anchor rather than a leak), both implied legs of the tri-state TLS gate, and `notExists` on either visibility label for both key-bearing Certificates. The bats guard covers the two grant surfaces nothing else asserts on. The ResourceDefinition is rendered, by the `nats-rd` chart inlining `cozyrds/*`, but that chart has neither a tests directory nor the `test` target `hack/helm-unit-tests.sh` looks for, and no `packages/system/*-rd` package does. The dashboard Role is rendered by a chart that has both, and no suite under `packages/apps/nats/tests/` asserts on it. Both are compared whole rather than filtered down to the entries that mention a Secret, because every such filter admits the spelling it did not anticipate. A name whitelist passes an added label selector, and `controller.cert-manager.io/fao` is stamped on both key-bearing Secrets, so a selector on it is a one-line grant of the CA private key. On the Role side, a rule with no `resourceNames` names no Secret, a rule with `resources: ["*"]` names no resource, and a ClusterRole is not a Role, while all three grant read access to the CA key. Comparing the rendered output whole needs no filter and leaves no spelling to find. ## Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file. No trigger fires, and no follow-up PR is owed. `packages/apps/nats/templates/tenant-projection.yaml` is a new template and `packages/apps/nats/tests/` holds chart tests. No package is added, renamed or removed; `values.yaml` and `values.schema.json` are untouched, so neither the hand-written Terraform schema, its version enums, nor its restated defaults have anything to track. `kind`, `plural` and `release.prefix` are unchanged, and no output Secret or Service is renamed, so the provider's concatenated name lookups still resolve. `packages/system/nats-rd/cozyrds/nats.yaml` is edited in place rather than renamed, so the ccp external-app skill's dependency-contract anchor still resolves. The provider offers no typed access to the new projection, which is the position it already holds for every Secret it does not model, and not a break. `packages/apps/nats/README.md` is covered by the one automated path that does exist: `nats` is present in the website `Makefile` `APPS` list, so the docs bot regenerates the managed-apps reference page from this `README.md` on the next stable tag. This is the silent-skip failure mode the trigger map warns about, so I checked the list rather than assuming; an app missing from it gets no page at all, which is what happened to `opensearch`. `hack/check-nats-rd-secrets.bats` is a new file, not a move or a rename, and it changes no make target's contract: `Makefile` globs `hack/*.bats` into `BATS_UNIT_FILES` automatically. The ccp skills anchor on `hack/package.mk` and `hack/common-envs.mk`, neither of which is touched. - [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: Part of #2814. ### Release note ```release-note feat(nats): expose the CA certificate to tenants as a key-free `<release>.tenant-ca` Secret ``` |
||
|
Aleksei Sviridkin
|
65e700486b |
feat(qdrant): publish ca.crt-only trust anchor to tenants (#3341)
## What this PR does Converges qdrant onto the key-free CA trust-anchor contract (#2814), the same way #3340 does it for nats. The chart declares a `TenantProjection` sentinel naming its cert-manager CA Secret. The CA-extraction controller reads that declaration and publishes a key-free copy, `ca.crt` and nothing else, as `<release>.tenant-ca`. The qdrant ResourceDefinition selects that projection by label so it shows up in the tenant's secret list. No key-bearing Secret gains a tenant-visible label. The sentinel is gated on the chart's TLS condition. Engines whose operator mints a CA unconditionally do not need that, but qdrant only has a CA while TLS is on, and an ungated sentinel would sit `Ready=False`/`SourceNotFound` on every plaintext release. This was held while the CA-extraction controller was still open. That controller landed in #3407, so the sentinel is honoured as soon as this merges and the hold is gone. Tests pin the invariants that otherwise fail silently. The sentinel carries no `ownerReferences`, so the lineage walk falls through to the Flux release label. Neither key-bearing Certificate stamps a tenant-visible label on its Secret. The two objects that decide which Secrets a tenant may actually read, the dashboard Role with its binding and the ResourceDefinition, are pinned whole rather than checked for known-bad names: a rule granting secrets with no `resourceNames` is unrestricted rather than narrow, and a label selector can reach a key-bearing Secret while every name in the file stays correct, so neither adds a name for a probe to notice. Part of #2814. ### Follow-ups What this PR does not do, and where each belongs instead. - nats converges the same contract in #3340 and has no `dashboard-resourcemap_test.yaml`, so the dashboard Role pin belongs there too. Tracked in #3701 along with postgres, which already carries the label selector with no guard behind it. - The qdrant e2e fixture runs plaintext (`hack/e2e-chainsaw/qdrant/qdrant.yaml` sets `external: false`), so nothing in CI walks the path from the sentinel to `tenantsecrets`. postgres has a `verify-tenant-ca-projection` step to copy. - `hack/check-qdrant-rd-secrets.bats` is engine-agnostic apart from two literals, the chart path and the expected Secret set. The third copy of it is the point to parameterise over `packages/system/*-rd/cozyrds/` instead of copying again. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file. Nothing needs a manual follow-up, but not for the reason a values-only walk would give: the website reference page for an app is regenerated from that package's `README.md`, and this PR does change it. `qdrant` is in the app list hardcoded in the website `Makefile`, so the docs bot picks the new section up on the next stable tag and there is nothing to open by hand. The Terraform provider's schema is hand-written against `values.schema.json`, which is untouched, and the provider offers no typed access to the new projection, the same position it holds for every other Secret it does not model. No package is added, renamed or removed, and the `cozyrds` file is edited in place rather than renamed, so the ccp dependency-contract anchor still resolves. User-facing documentation for consuming the trust anchor ships with the chart README, matching #3340. - [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 ```release-note feat(qdrant): expose the CA certificate to tenants as a key-free `<release>.tenant-ca` Secret ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Tenant environments can securely access the Qdrant TLS CA certificate without receiving the private key. * TLS CA access is automatically enabled when TLS or external access is configured. * **Documentation** * Expanded TLS guidance explains certificate validation and provides commands for retrieving the tenant CA certificate. * **Tests** * Added coverage for TLS projections, secret visibility, dashboard resources, and secure fail-closed secret exposure. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
8d386c2cbc |
docs(mongodb): name the trust anchor as the tenant sees it
Every object this chart creates is named after the Helm release, not after the MongoDB resource, so a MongoDB named catest yields mongodb-catest.tenant-ca. The retrieval commands used a bare <release> placeholder, which reads as the resource name and produces a command that returns nothing. Spell the prefix out in the two commands meant to be copied, and define the placeholder once for the prose. Split the sentinel's shape test in two. The absence of ownerReferences is the invariant — it breaks the walk from the projection back to the release, and the tenant stops receiving the anchor. An explicit namespace or a chart-added label breaks nothing by itself; grouping all three under one name presented them as equally load-bearing. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
04112dacdf |
test(mongodb): stop the tenant-Role guards passing on the wrong document
The assertions withholding the key-bearing TLS Secrets from the tenant Role addressed it as document 0 and, in two cases, asserted only with notContains. Neither survives a rearrangement. notContains passes when its path does not resolve, so a test built from it alone goes green against the wrong document. Reversing the order of the two documents in the template moved the RoleBinding into position 0, and "does not grant the key-bearing TLS secrets when external is enabled" and "does not grant the tenant-ca projection by name" both stayed green while the Role granted <release>-ca-cert by name — the Secret holding the CA private key. Select the Role by kind, and pair every notContains with a positive assertion on the same path so a rule reordering that moves rules[1] reddens too. Indexing survives neither rearrangement; selecting survives the first; the positive anchor catches the second. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
8b6867dc47 |
fix(mongodb): correct the trust-anchor prose and two e2e assertions
The name-clash note claimed the shared TLS helper documents <release>-ca-cert as the conventional key-free anchor. It documents the opposite: the canonical name is <release>.tenant-ca, and it names this engine as the reason, because MongoDB already claims <release>-ca-cert with a key-bearing Secret. There is no clash to warn about and the chart follows the convention rather than deviating from it. The paragraph above already says <release>-ca-cert holds the CA private key, so drop the note instead of inverting it. Guard the ownerReference assertions against an absent list. length() of null is a type error rather than a failed comparison, and the harness returns it as an error, which aborts the poll instead of retrying — so the timeout never applies while the operator is still patching the reference on. Restore the two-variable form of the fingerprint comparison. Inlining both command substitutions into the test swallows a non-zero exit: with no openssl on PATH both sides are empty, they compare equal, and the check passes having compared nothing. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
01cfb5fda0 |
fix(mongodb): correct what a missing extraction controller does
The README said the sentinel would sit inert with nothing else affected. The CRD is a template of the controller's own chart, so a cluster without the controller has no TenantProjection kind and the release fails to render at all. The caveat also guarded a state that cannot occur, since the controller is a mandatory platform component — drop it rather than restate it, matching the postgres chart, and keep the paragraph explaining the choice of source Secret. Enumerate all three key-bearing Secrets in the registry negative. Two of the three were listed, which leaves the third free to acquire the label and leak a key, the exact reasoning the step above it already spells out. Collect TenantProjection status on failure. The controller reports SourceNotFound and its siblings there, and without it a failed projection surfaces only as an absent Secret with no reason given. Rename the source-shape step, which no longer looks at the CA Secret, and cross-reference the two halves of the leaf Secret's name. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
fc57e928e1 |
test(mongodb): drop a document count the sentinel invalidates
The cert-manager exclusion suite justified its `any: true` requirement with a specific number of rendered documents. The argument only needs "more than one"; the number was already tied to one value set and the sentinel adds another document. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
c0b2ddcdf7 |
test(mongodb): assert the published trust anchor end-to-end
The existing steps only assert that key-bearing Secrets are NOT promoted to the tenant, and every one of those is satisfied by an absent label — they hold just as well if nothing was ever published. Add the positive: the projection exists, carries exactly ca.crt, matches the source certificate by fingerprint, and is reachable through the tenantsecrets registry rather than only as a raw Secret. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
29d2955241 |
fix(mongodb): declare the trust anchor with a TenantProjection
A tenant verifying a MongoDB endpoint needs the CA certificate, and every Secret that holds one also holds a private key: the operator's <release>-ca-cert carries the CA key, the leaf <release>-ssl the server key. Granting either would hand over the key along with the anchor. Render a TenantProjection naming the source, so the CA-extraction controller can publish the key-free <release>.tenant-ca, and select that projection by label from the managed-service definition. The sentinel carries no ownerReferences: it is resolved back to its release through the label Flux stamps, and an owner reference short-circuits that walk and silently costs the tenant its anchor. The sentinel is ungated. The operator issues the certificate chain whatever the chart asks for, so the source exists for every release and there is no plaintext leg to skip. The source is <release>-ssl rather than <release>-ca-cert. It is the name this chart itself sets in spec.secrets.ssl, so the reference cannot drift from the CR; the operator writes it on both its cert-manager path and the self-signed fallback it uses when cert-manager is absent, where no <release>-ca-cert exists at all; and it holds the merged old and new CA through a rotation, where <release>-ca-cert holds only the new one and would stop verifying pods still serving the previous leaf. Withhold the key-bearing Secrets from the tenant explicitly as well. The label selector matches on no name, so a single mis-stamp would promote a private key; the exclude list is evaluated first and closes the enumerable set. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
4092f59528 |
docs(mongodb): scope the upgrade-availability claim to quorum-capable sizes
The upgrade-restart note said the replica set stays available through the rollout. That holds at three members and fails below them, and the chart permits both smaller sizes: replicas 1 and 2 render unsafeFlags.replsetSize and are not excluded from the SAN change, so they gain horizons and roll on upgrade exactly as a three-member release does. At one replica the rollout takes the only mongod down; at two the write majority is still two, so restarting either member stalls writes until it returns. An operator running a single-replica external release would have read the old text and declined to book a window. Cover the two replicas case, which was untested, and assert horizons and the sub-quorum marker together at both sizes, so gating horizons to sizes that can absorb a restart turns the tests red and sends someone back to this paragraph. The default-size case carries the pair as a positive control: the marker proves nothing unless it is absent where a restart is survivable. Signed-off-by: Aleksei Sviridkin <f@lex.la> |