mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-04 02:57:27 +09:00
feat/externalip-api
1507
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Andrei Kvapil
|
8b2223d1b9 |
feat(vm-instance): let the external endpoint wear a reserved address
The claim annotation lands on the VM's own Service beside the cozy-proxy datapath annotations already there. The two are independent: whole-IP NAT behaves the same on a held address as on an auto-assigned one, so both methods take a reservation. A VM has one external endpoint, so attaching several addresses to one VM stays out of reach of this field — that needs the exposure half of the model, not the allocation half. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
a1f58a5c08 |
feat(clickhouse): add external access and let it wear a reserved address
ClickHouse was the only managed database in the catalog a tenant could not reach from outside the cluster: the installation-wide Service had no way to become a LoadBalancer short of an operator editing the service template by hand. It now carries the same external boolean as the rest of the catalog, and the same externalIPs field. The Service is operator-owned, so this is the in-place type change mariadb and rabbitmq already make on theirs. It keeps its clusterIP, so the transition is one Kubernetes accepts. With external unset the rendered installation is unchanged. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
2e093c7a65 |
feat(mariadb): let the external endpoint wear a reserved address
Which Service actually receives the address depends on the replica count: the operator publishes the cluster Service for a single replica and primaryService once there are more, and the chart already branches on exactly that. The claim annotation follows the same branch, so a tenant writes one entry either way. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
ce00ac8652 |
feat(mongodb): let the sharded external endpoint wear a reserved address
Sharded mode publishes mongos through a chart-owned LoadBalancer Service, which takes the substrate's claim annotation directly. A replica set cannot: it is published as one LoadBalancer per pod, and the operator takes a single serviceAnnotations map for the whole replset, so one claim would be handed to every pod Service at once and the pins would collide. The chart refuses that combination with the reason rather than rendering something that cannot work. It is the discovery-engine case community#45 defers, and closing it needs an address per pod. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
f31f819366 |
feat(rabbitmq): let the external endpoint wear a reserved address
The chart owns no Service here — the operator does — but it already reaches into spec.service to flip the type, and the operator copies annotations from the same block onto the client Service that receives the load-balancer address. So the substrate's claim annotation gets to its consumer without the chart taking ownership of anything new. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
958dde8a22 |
feat(kafka): let the external endpoints wear reserved addresses
Strimzi publishes one bootstrap Service plus one Service per broker, and Kafka advertises every broker's own address to clients. Pinning the bootstrap address alone therefore does not make the cluster reachable at fixed addresses, so externalIPs accepts a broker-N target alongside bootstrap and each entry is rendered into the listener's own configuration block. Broker targets follow the replica count, so a claim for a broker that does not exist is refused as the typo it is rather than kept as a reservation for future growth. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
773d6b9aee |
feat(redis): let the external endpoint wear a reserved address
Threads externalIPs onto the LoadBalancer Service the chart renders for the Sentinel-elected master. Empty is the previous behaviour. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
7c2f73080f |
feat(postgres): let the external endpoint wear a reserved address
Adds externalIPs, a list of {claim, target} entries, and threads the
resulting annotation onto the LoadBalancer Service the chart already renders
for the primary. Empty is the previous behaviour, so nothing changes for an
existing release.
The list shape is deliberate even though the chart publishes a single endpoint
today: the same field lands on every catalog app in this series, and Kafka does
have several external endpoints. A target that the chart does not publish is
refused with the valid ones named, rather than being accepted and ignored.
Assisted-by: LLM
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
|
||
|
Aleksei Sviridkin
|
d000d02b99 |
feat(rabbitmq): add TLS support for AMQPS and management HTTPS (#2683)
## What this PR does Adds TLS for AMQP and the management API, and hands the tenant a key-free trust anchor instead of a private key. - A chart-authored cert-manager chain issues the broker certificate. The chart names its CA Secret in a `TenantProjection` sentinel; the CA-extraction controller reads `ca.crt` from that Secret and writes a key-free projection, stamping `internal.cozystack.io/tenant-ca` on the projection it owns. That label is what the ResourceDefinition selects. Nothing labels the CA Secret itself, which holds the CA private key and is excluded by name. - The leaf `<release>-tls` carries the broker's private key and reaches no tenant. Neither the dashboard Role nor the ResourceDefinition grants it, and the ResourceDefinition excludes it by name as well, so a label mis-stamped onto it later still does not get through. - SAN coverage now lists every resolvable form explicitly, including the bare `*.<release>-nodes` wildcard that the operator uses for `RABBITMQ_NODENAME`. ## TLS is opt-in here, unlike the sibling charts `tls.enabled` defaults to `false` and is not inferred from `external`. That diverges from nats, qdrant and kafka, which infer it, and the divergence is deliberate for a reason specific to this operator. Setting `spec.tls.secretName` makes the cluster operator rewrite the `port` key in the `<release>-default-user` Secret from 5672 to 5671, on the mere fact of TLS and regardless of whether the plaintext listener is still open. That Secret is the Service Binding the chart hands the tenant. A client with a hardcoded `:5672` keeps working. A client that reads the endpoint from the binding, which is the documented way, gets moved to the TLS port and must then speak TLS and be given a CA to verify against, neither of which it was configured for. Inferring TLS from `external` would fire all of that on an ordinary upgrade, with no configuration change, against exactly the clients that followed the documentation. Whether the other engines should follow is an epic-level question rather than this PR's to answer; for nats it is raised in #3348. ## The plaintext ports are still published, and this PR says so There is no operator-supported way to publish the TLS ports alone. `spec.override.service.spec.ports` is a strategic merge keyed on `port` against defaults the operator rebuilds every reconcile, so it adds and mutates entries but never subtracts one, and a TLS-only port list there would read as a guarantee while changing nothing. The chart renders no service override at all and says why at the callsite. `tls.disableNonTLSListeners` is offered as an explicit opt-in, and it refuses to render rather than failing silently in three situations. - Together with `users` or `vhosts`. The flag forces messaging-topology-operator onto TLS regardless of its own configuration, after which it verifies the broker against the system certificate pool, where a per-release CA cannot be. Every topology object bound to the release would stop reconciling, silently: already-created users keep working while password rotation and new users simply never apply. There is no working route left for it, so the chart refuses instead. Managing those objects as hand-written CRs is not a way out, because the operator takes TLS from the `RabbitmqCluster` and not from whoever created the CR, so manual objects break identically and without the render-time refusal. `rabbitmqctl` or a definitions import are. - While topology objects are still live in the namespace. See the next section. - With TLS off, where no listener of either kind would remain. Erlang distribution (25672) and epmd (4369) stay plaintext under every setting; the flag does not touch them, so `disableNonTLSListeners: true` is not "TLS everywhere". A third route, closing AMQP at the broker through `spec.rabbitmq.additionalConfig` while leaving the operator on plaintext management, has been suggested and is plausible, but the operator's sources are not vendored here and it was not verified. It is recorded as an open question rather than promised. ## Closing the listeners takes two upgrades, and the chart enforces it Clearing `users` and `vhosts` and setting `tls.disableNonTLSListeners: true` in one edit is the natural thing to try, and it wedges the objects being removed. The values-level refusal cannot catch it, because by then both maps are empty. Helm applies every object in the target release before pruning the ones the release dropped, so the broker closes plaintext and loses the `rabbitmq.com/operator-connection-uri` annotation while the topology objects still hold `deletion.finalizers.<plural>.rabbitmq.com`. Their operator then has to reach a management API it can only get to over a certificate it verifies against the system trust pool. The delete never completes, and the upgrade reports success over objects left in `Terminating`, with the users still live in the broker and `kubectl delete namespace` on that tenant hanging. The chart now reads the namespace rather than the values. When the flag is being introduced and any topology object is still there, the render refuses and names them, gives the two-upgrade order and the `kubectl get` that confirms nothing bound to this release is left, and says how to unwedge a release that is already stuck. The check keys on the live `RabbitmqCluster` as well, so it applies only to the transition: once the listeners are closed, an object a tenant creates afterwards is theirs to clean up rather than something that stops the release rendering. Eleven unit cases cover the firing and the silent paths. The set is wider than the three kinds this chart creates. All twelve kinds `messaging-topology-operator` drives through its shared `TopologyReconciler` carry that finalizer and share one deletion path, so a `Queue` or a `Shovel` written by hand against the release wedges exactly the same way, and a runbook step that checks only users, vhosts and permissions says the namespace is clear when it is not. The list is derived in one place and read from both the lookup and the `kubectl get` the refusal prints, and a unit test holds it against the operator's own CRDs so a bump cannot introduce a kind the guard has never heard of. `SuperStream` is the one CRD left out: it runs on its own reconciler with no finalizer, and the objects it owns are already on the list. An object that binds through `connectionSecret` instead of naming the cluster stays outside the guard, which the comment says. ## Breaking for metrics scrapers Enabling TLS moves the metrics endpoint from 15692 to 15691. A ServiceMonitor selecting the port by the name `prometheus` silently stops matching and the instance's metrics disappear with no error anywhere. Nothing in this repository scrapes RabbitMQ, so only user-defined monitoring is affected, silently, which is why it is stated here. ## Known limitations 1. Turning TLS back off leaves the raw CA material behind. The anchor itself is collected: the projection carries an owner reference to the `TenantProjection` sentinel, and Helm deletes the sentinel along with the rest of the chain. What survives is `<release>-ca` and `<release>-tls`, because cert-manager runs with `enableCertificateOwnerRef: false` and does not delete the Secrets it issued, so the README says to remove them by hand. #3349 asks the same question of the label-driven leg of the contract, which this chart does not use. 2. No e2e coverage for the TLS path. The one-upgrade wedge above is the case that most deserves it: install with TLS and users, upgrade to no users plus the flag, and assert the `User` is actually gone rather than merely marked for deletion. A template test cannot reach that. On certificate renewal, the leaf carries `duration: 8760h` and `renewBefore: 720h`, so cert-manager rewrites the Secret around day 335. The cluster operator mounts it as a projected volume with no `subPath`, so the kubelet refreshes the files in place, and the operator does not watch a user-supplied TLS Secret, so nothing rolls the pods. The last hop, the broker picking the new file up, rests on Erlang's `ssl_pem_cache` re-reading by mtime and has only been read from sources rather than exercised against a live broker. The merged qdrant chart ships the same duration and `rotationPolicy`, so this is a platform-wide property rather than something this PR introduces. ### Downstream repositories `packages/apps/rabbitmq/values.schema.json` gains the `tls` object, so `cozystack/terraform-provider-cozystack` needs the field modelled for `cozystack_rabbitmq`; tracked in cozystack/terraform-provider-cozystack#42. ### Release note ```release-note feat(rabbitmq): opt-in TLS for AMQP and the management API, with the CA published to tenants as a key-free trust anchor. Enabling TLS moves the metrics port from 15692 to 15691 (ServiceMonitors selecting the port named `prometheus` must be updated) and moves the `port` value in the `<release>-default-user` binding Secret from 5672 to 5671. Plaintext AMQP and management ports remain published; `tls.disableNonTLSListeners` closes them but also drops in-cluster plaintext clients, and must be enabled in a separate upgrade after the topology objects are gone. ``` |
||
|
myasnikovdaniil
|
94a202bc5a |
test(e2e): run the merge-gating lanes on Talos containers (#4020)
Replaces QEMU substrate with Talos containers on both lanes that gate a merge. srv1-srv3 run as containers, so tenant worker sits at L2 instead of L3. Measured in CI on commits where both substrates ran the same tree: | | qemu | container | |---|---|---| | `kubernetes-latest` / `-previous` | 1143-2040s, red on every vgif-absent runner | 466-809s, green in both strata | | whole job | | 24-49 min faster | Almost all of it is in Chainsaw. Install takes about half an hour either way. Node-join soft-red gate goes with it. It was a prosthetic for nested virt: a worker that registers in minutes elsewhere could miss any deadline this test can afford, and #3513 established the discriminator is host `kvm_amd` `vgif`, not anything about the product. Remove the nesting level and there is nothing left to tolerate, so `hack/e2e-node-join-soft-red.sh`, the marker and the `soft_red` outputs are gone from all four workflows. ### What moves off the per-PR path DRBD, and with it `replicated` StorageClass, its `Immediate` binding mode, tenant StorageClass propagation (applies only to remotely-accessible classes), and cozystack talos node image with its extensions. None of it is lost - `nightly.yaml` and `e2e-tag.yaml` still run the suite on QEMU, so all four keep nightly and release-candidate coverage. Live migration is not on that list because nothing tests it today anyway. One more belongs on the list and was missing from it: the nocloud disk build. `build-talos` no longer runs `make -C packages/core/talos talos-nocloud`, so `images/talos/profiles/nocloud.yaml` is compiled by `nightly.yaml` and by the tag build's `make assets` rather than by anything a PR runs, and `promote-rc.yaml` still hard-fails a promotion whose rc release has no `nocloud-amd64.raw.xz`. Nothing in that chain breaks, because the tag build supplies the asset as before. What a PR no longer catches is narrow: it still compiles `installer.yaml` through the same imager, and the two profiles are generated by the same script and differ only in `platform`, `kind`, `imageOptions` and `outFormat`. ### Product fixes Four, all found by measurement while building the lane, each on its own commit: The linstor `drbd.enabled` gate. Without it drbd-logger sidecar exits, satellite is never Ready, piraeus registers zero nodes and every PVC hangs Pending behind a cluster that looks healthy. linstor `--strict-topology`. Without it external-provisioner passes every topology segment as `requisite`, so a node-pinned volume can be provisioned away from its pod and the pod is then unschedulable forever with nothing erroring anywhere. kubevirt-cdi importer resources are configurable now. Default restates CDI's own values exactly so production behaviour does not move, and the lane raises the ceiling through its own Package. vm-disk binds standalone disks immediately. A VMDisk on `local` silently never populated, while the same disk on `replicated` did. ### Where the run stands The lane has gone green twice, most recently on `cc70cef7b` where `E2E (in-tree)` took 2h3m against its 215m cap with 45 Chainsaw tests passing. The measurements quoted elsewhere in this description come from the first of those runs, on a commit that has since been rebased away: the two tenant-Kubernetes suites came in at 569.72s (`kubernetes-latest`) and 470.19s (`kubernetes-previous`) against a 67m operation. Both runs answer the four earlier reds, all of which were the lua-protobuf segfault from #4012 once `vminstance`/`vmdisk` were fixed on the branch. ghcr.io mirror removal is carried here too and is the same change as #4007, whichever lands first makes the other a no-op. ### What review changed `drbd.enabled=false` now refuses to render together with `talos.enabled=false`. It removed the logger sidecar only, while the two DRBD initContainers piraeus contributes unconditionally are deleted by the Talos configuration alone, so that combination produced a satellite that can never reach Ready. `isp-full-generic` sets `talos.enabled=false`, so it was reachable and not theoretical. vm-disk keeps unconditional immediate binding, a standalone disk has no consumer to wait for. What it costs on a node-pinned class is in the chart README now, and the annotation is pinned per source type and on the existing-DataVolume path where `helm upgrade` retrofits it. The rest is the harness holding its own promises: HelmRelease gate reads are bounded and its minimum-count half is pinned by a test that fails without it, two Chainsaw ops sit above the deadlines they contain, the fallback-StorageClass render restores `replicated` when it fails, container lane compose project and pool files are per-checkout, and three comments that said the opposite of the code now say what is true (QEMU is still wired into nightly and e2e-tag, the lane replaces two Packages and not one, the argument-count assertion cannot see upstream drift). ### Release note ```release-note fix(linstor): CSI provisioning is constrained to the node the scheduler chose (`--strict-topology`), and the DRBD sidecar can be switched off with `drbd.enabled` on substrates whose kernel cannot run DRBD. On an existing cluster the new default re-templates `spec.csiController.podTemplate`, so piraeus rolls `linstor-csi-controller` once during the upgrade. `drbd.enabled=false` requires `talos.enabled=true` and the chart now refuses the other combination at render time, because only the Talos satellite configuration removes the DRBD initContainers. fix(kubevirt-cdi): CDI worker pod resources are configurable through `importerResources`, which reaches the importer, the uploader and the host-assisted cloner. The default restates CDI's own built-in values exactly, so an install that sets nothing behaves as before. fix(vm-disk): every disk requests immediate binding, not only `upload` sources, so a standalone disk on a WaitForFirstConsumer StorageClass is populated instead of waiting for a consumer that may never arrive. On a node-pinned class such as `local` the volume then binds where CDI's worker pod was scheduled rather than where the VM will run; see the chart README before choosing such a class for VM disks. The default `replicated` is unaffected. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added container-based end-to-end testing with local storage support. * Added optional backup-access preflight checks for database backup examples. * Added configurable CDI worker resources and LINSTOR storage settings. * Added configurable immediate storage binding for VM disks. * Added stronger readiness checks, diagnostics, and parallel test execution. * **Bug Fixes** * Improved cleanup failure reporting, capacity validation, and missing-image detection. * Improved FoundationDB health verification and storage placement behavior. * **Removed** * Removed GHCR mirror support and soft-red node-join tolerance. * Removed standalone OIDC render test suites. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
a115745c69 |
feat(vm-disk): let the operator defer binding to the consumer
Immediate binding for every source fixed a real bug: a standalone disk on a WaitForFirstConsumer class has no consumer to wait for, so it never populated. It also took away the path that is correct for the other case. For a disk a VMInstance consumes, KubeVirt renders a temporary launcher pod carrying the VM's own affinity and nodeSelector, so the volume binds where the VM can actually run; with the annotation always present the CDI worker becomes the first consumer instead and the volume lands where that pod happened to be scheduled. On a node-pinned class the VM is then unschedulable against a node-affinity conflict, and storageClass is immutable, so the only exit is deleting the disk. The two cases want opposite defaults and only the operator knows which one a given disk is. bindImmediately defaults to true, so nothing moves for existing disks or for anyone on `replicated`, which binds Immediate either way. Both branches are pinned in tests, including the existing-DataVolume path, which renders its own annotation block. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
Aleksei Sviridkin
|
3c5d9f49e5 |
docs(rabbitmq): scope the runbook check and close the narrower-variant question
The confirmation step told the operator to wait for an empty namespace, while the guard it confirms deliberately ignores objects naming another release. A tenant running two brokers in one namespace would wait for something that never happens. Say the check is release-scoped, in the README and in the refusal. Closing AMQP through additionalConfig while keeping the operator on plaintext management was recorded as an open question. It is answerable: cluster-operator pins the readiness probe to the amqp port unless disableNonTLSListeners is set, so that configuration leaves every pod failing readiness. Stated instead of left open. Also repoint one template reference at the heading that exists, and name the runbook test after what it compares. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2f6c1554ed |
docs(rabbitmq): correct two TLS mechanisms the prose states wrongly
The helper comment says a HelmRelease override carrying `tls: null` reaches the chart unpruned and is accepted by values.schema.json. Helm's coalescing drops the key instead, so the schema never sees a null and `.Values.tls` is nil rather than an empty dict. The `dig` on a defaulted dict is still what stops the panic; only the account of how the value arrives was wrong. The README says the flag forces the topology operator onto TLS regardless of how it is configured. The connection-uri annotation short-circuits that calculation before it is reached, so the reason the flag wins is that the port the annotation names is the port the flag closes, which is why the chart withdraws it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
69155919b4 |
fix(rabbitmq): refuse on every topology kind that wedges
The refusal that stops tls.disableNonTLSListeners from closing the plaintext listeners under live topology objects iterated User, Vhost and Permission, the three kinds the chart itself creates. All twelve kinds messaging-topology-operator drives through TopologyReconciler carry the same deletion.finalizers.<plural>.rabbitmq.com finalizer and share one deletion path, so a Queue or a Shovel bound to the release wedges identically while the render passes and the runbook step reports the namespace clear. Derive the list once in a template helper, read it from both the lookup loop and the confirmation command the refusal prints, and widen the README runbook to the same set. SuperStream stays out: its own reconciler sets no finalizer, and the Queue, Exchange and Binding objects it owns are already on the list. An object that binds through connectionSecret rather than by cluster name is still outside the guard, which the comment now says. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
cd675e113f |
fix(postgres): point the trust-anchor comment at the published projection (#4063)
## What this PR does The TLS comment in `db.yaml` told operators to take `ca.crt` from `<release>-credentials`. That key is not there. The chart renders that Secret from `.Values.users`, so it holds passwords and nothing else. The anchor a tenant can reach is `<release>.tenant-ca`, which the CA-extraction controller publishes from the sentinel this chart already renders in `templates/tenant-projection.yaml`, and which tenants read through `tenantsecrets`. The comment points there now. It also names the two Secrets it is not, because CNPG's own `<release>-ca` is one dash away and holds the CA private key next to `ca.crt`. Comment only. `README.md` already documented the projection correctly and needed no change, and `helm unittest packages/apps/postgres` is green. ### Screenshots Not a UI change. ### Downstream repositories A comment inside one chart template. `values.yaml`, `values.schema.json`, `Chart.yaml` and `README.md` are untouched, so the generated reference page does not move, and no schema field, default, version enum, release prefix or Secret name changes. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated TLS trust-anchor comments to clarify that the CA extraction controller publishes the key-free `<release>.tenant-ca` Secret. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
5a7e2d5546 |
fix(harbor): give the nginx proxy explicit resources so it survives the tenant LimitRange
With expose.type=clusterIP the goharbor chart deploys an nginx proxy pod, but the chart set no resources for it. In a tenant namespace the LimitRange defaults any container without resources to a 128Mi memory limit, so the proxy is OOMKilled (exit 137) and Harbor never comes up. Set the nginx proxy to t1.small (512Mi) and bump the portal (also nginx-based) from t1.nano (128Mi) to t1.micro (256Mi) for the same reason. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Myasnikov Daniil
|
f73729937c |
docs(vm-disk): record what immediate binding costs on a node-pinned class
Requesting immediate binding for every source, not just `upload`, is deliberate: a VMDisk is a standalone object populated before any VM consumes it, so on a WaitForFirstConsumer StorageClass a disk with no VM attached would never populate at all. What that costs was recorded nowhere an operator reads. On a node-pinned class the CDI worker pod chooses the node, so the volume binds where the importer landed rather than where the VM will run, and a VM constrained elsewhere -- a nodeSelector, a request only one node satisfies, or the Windows affinity from _cluster.scheduling.dedicatedNodesForWindowsVMs -- stays unschedulable against a node-affinity conflict. One disk is enough for that; it is not a multi-disk-only case. storageClass is immutable, so the exit is delete and recreate. The chart default `replicated` binds Immediate regardless and does not move. Also pinned in tests, in both directions the annotation is emitted: per source type (blank, image, disk) so a regression to the old upload-only condition reddens, and on the existing-DataVolume path, where `helm upgrade` retrofits the annotation onto disks created before it was unconditional -- inert for a volume that has already bound, and now a recorded decision rather than an unasserted side effect. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
Myasnikov Daniil
|
5eaf44b8ca |
fix(vm-disk): bind standalone disks immediately
A VMDisk is a standalone object that is populated before any VM consumes it, so on a WaitForFirstConsumer StorageClass CDI needs an explicit signal to schedule its worker and start provisioning. Without it the DataVolume sits in PendingPopulation indefinitely and the disk is never filled -- and nothing says so. This is a consistency fix rather than a new behaviour. The chart default `replicated` is volumeBindingMode: Immediate, so a disk on it already bound and populated at creation; `local`, the cluster's default class, is WaitForFirstConsumer, so the same VMDisk silently did nothing there. The annotation was previously emitted only for source: upload, which is the one source that could not work without it, leaving http, image, disk and blank to differ by class. Two consequences worth knowing. CDI checks the annotation by presence and ignores its value, so "" and "true" are equivalent and this is not a behaviour change on the upload path. And on a node-pinned class each disk now binds at its own creation rather than at VM scheduling, so a multi-disk VM on `local` can find its disks on different nodes -- that is the existing limitation of a node-pinned class, which is why the chart default is not one, but it is reached earlier now. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
3a369fed99 |
feat(redis): add opt-in TLS for Redis and Sentinel (#2729)
## What this PR does `tls.enabled` puts Redis and Sentinel on a TLS-only listener with a per-release cert-manager chain, and hands the tenant the CA certificate as `redis-<name>.tenant-ca`. Off unless set. redis-operator v3.3.5 has no TLS support. The work is ported upstream in freshworks-oss/redis-operator#85, which is open and not in a release, so this carries it as three patches over the v3.3.5 tarball. `images/redis-operator/patches/tls.diff` is the source half, cut from that PR's commit `2ad2ed8d1`, which the patch header pins; it is applied by the glob the image Dockerfile already runs, after the Dockerfile strips the tarball's `manifests`, `charts`, `example` and `.github`, so the image builds from source alone and the upstream tests that read the CRD YAML skip instead of failing against the stale v3.3.5 copy. `patches/crd-tls.diff` adds `spec.tls` to the vendored CRD, and with it the CEL rule that fixes `tls.enabled` at creation; without the schema block the apiserver prunes the field on admission and the patched binary waits for something it never sees, and the rule cannot exist without the block it references. `patches/rbac-tls.diff` grants Create, Get and Update on Secrets, so the operator can publish the CA-only Secret it addresses by name. That is the whole delta, and it is narrower than the ported operator's own chart asks for: those three are the only Secret calls in the patched source, and cert-manager Certificates get no grant at all, because the operator only touches one under `spec.tls.certManager` and this chart pins `certificateSecret` mode instead. One change in `tls.diff` is not TLS and takes effect with TLS off, named in the patch header: `MakeSlaveOfWithPort` connects to the replica on its own port rather than on the master's. It changes a dial, not a pod template, so shipping the rebuilt image rolls nothing; the plaintext pod templates stay byte-identical to v3.3.5. The image build registration this branch used to carry (`make -C packages/system/redis-operator image` in the root `build:` target) landed on main with the Valkey service (#3380), so the rebuilt operator image is produced by CI from main as well; the digest in `values.yaml` is still refreshed by release preparation, not here. The chart patches re-apply at the end of `update:`, the same way reloader, grafana-operator, ingress-nginx, seaweedfs and kamaji carry theirs. Otherwise `rm -rf charts` takes TLS back out on the next `make update` and nothing fails. `make update` reproduces the committed tree byte for byte. TLS is chosen when the instance is created. The config gets `port 0` and the listener moves to `tls-port`, so TLS replaces plaintext rather than running beside it, and a running instance cannot change mode: a replica restarted onto the TLS listener cannot replicate from a master still on plaintext, and Sentinels already on TLS would fail over to a stale replica. The RedisFailover schema in the carried CRD patch rejects a change of `tls.enabled`, which on an existing instance surfaces as a failed HelmRelease upgrade while the instance keeps serving what it did; the README points at creating a new instance and migrating. That is also why TLS is not inferred from `external`: an externally reachable instance is the one most likely to have clients an upgrade must not drop. Existing releases keep their plaintext listener and are not restarted by the rebuilt operator image. The tenant gets `ca.crt` and nothing else, through `tenantsecrets`. The chart's own Secrets take a dot between the release and the suffix (`<release>.ca-tls`, `<release>.tls`, `<release>.ca-cert`): the suffixes are dot-free, so the split is unique and release `foo`'s CA Secret can no longer be release `foo-ca`'s leaf, the collision the hyphenated form had. The chart renders a `TenantProjection` pointing at `<release>.ca-tls`, and the CA-extraction controller re-encodes the one named key from the parsed certificate, so nothing that is not a certificate gets through. Same path as postgres, mongodb, nats and qdrant. No Secret of this release is granted to the tenant by name beyond the generated password, and that includes `<release>.ca-cert`, the CA-only copy the operator publishes for itself. It holds no key today, but the operator has Create and Update on Secrets cluster-wide, so what sits at that name is the operator's call rather than the platform's, and RBAC cannot filter by key. `hack/check-redis-rd-secrets.bats` pins the whole rendered Secret surface instead of a list of known-bad names, because an include with no `resourceNames` and no `matchLabels` restricts nothing. The chainsaw suite for redis gains two cases. A TLS fixture asserts the cert-manager chain is issued, the TLS-probed Redis replicas and Sentinel come up, the operator publishes its CA-only Secret under the widened ClusterRole, the tenant receives exactly `ca.crt` through `tenantsecrets` while the key-bearing Secrets stay off that path, and a `redis-cli --tls` PING through the service name completes with the same CA. A second case applies `tls.enabled` to a running plaintext release and asserts the refusal: the HelmRelease upgrade fails with the schema's message, the StatefulSet keeps its revision with both replicas Ready, plaintext still answers and no TLS listener appears. Certificate renewal rolls the pods from inside the operator. Redis and Sentinel read the certificate once at startup, so the patched operator stamps a hash of the TLS Secret's `tls.crt` and `ca.crt` into the pod template of both the Redis StatefulSet and the Sentinel Deployment on every reconcile, as `redis-failover.freshworks.com/tls-secret-hash`. A renewal changes the hash; the operator's own roller replaces the Redis pods one at a time and the Deployment controller rolls Sentinel. README names the fallback: if the roll cannot run, the deadline is the old certificate's expiry, past which the operator's own TLS connections fail along with the clients. `tls.authClients` maps to `tls-auth-clients` and defaults to `no`. The platform mints no client certificate for the tenant and the CA key is out of reach, so `yes` is only usable when the client certificate comes from whoever also controls the CA. Changing the value on a running release restarts the Redis and Sentinel pods: Redis reads the directive from its config file only at startup, so the operator folds the effective value into the certificate hash that drives the rollout, and the change lands when it is made rather than at the next unrelated restart. Follow-ups in the carried operator patch go upstream first, to freshworks-oss/redis-operator#85 or a successor there, and are picked up here afterwards; the carried patch does not grow Go that upstream has not seen. `tlsConfigFor` fetches the Secret on every call, tens of GETs per reconcile. The renewal roll is static analysis so far: the live check, `cmctl renew` on a running release while watching the `rfr-*` and `rfs-*` pods roll, is still owed and belongs with the e2e follow-up. Two facts about the shipped image rather than the patch. `packages/system/redis-operator/values.yaml` keeps the digest of the image built before the patch until release preparation rebuilds it, so on `main` between this merge and the next release-prep, `tls.enabled` renders the chain and `spec.tls` while the running operator ignores them and serves plaintext. And the pinned v3.3.5 tarball carries GO-2026-5026 in `golang.org/x/net` v0.53.0, reached through `PodService.UpdatePodLabels`; the port neither introduces nor fixes it, upstream `master` is already on v0.55.0, so it lands with the next tag bump rather than here. Outside the patch: `cozyvalues-gen` renders `{}` in the README Value column for `tls.authClients`: a child property without a default inherits the parent object's `{}`, and `make generate` reproduces it byte for byte, so it is a generator bug rather than something to hand-edit here. ### Screenshots No UI changes. ### Downstream repositories - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: cozystack/terraform-provider-cozystack#37 - [ ] [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: I walked the trigger map against the diff file by file. terraform-provider-cozystack is reached, tracked as cozystack/terraform-provider-cozystack#37 rather than a speculative PR: `values.schema.json` gains a `tls` object, and the provider hand-writes a schema, model and expand/flatten pair per Kind. It already models `tls` for postgres, kafka, nats and qdrant, not for redis. Redis needs different wording from those four, whose descriptions say to omit the block to follow `external`, which is the one thing redis does not do. Nothing else is reached. The website regenerates this package's reference page from `README.md` on a stable tag, so the README changes need no PR there. No package added, renamed or removed. `packages/core/platform/values.yaml`, variant and bundle composition and release asset names are untouched, and so are `hack/package.mk`, `hack/common-envs.mk` and `hack/update-crd.sh`, which is what ccp and external-apps-example read. The two new files under `hack/` are bats unit tests. No node prerequisite, installer value, telemetry metric or cozy-proxy label changes. ### Release note ```release-note feat(redis): add opt-in TLS for Redis and Sentinel. Set `tls.enabled: true` to serve both on a TLS-only listener backed by a per-release cert-manager chain; the tenant reads the CA certificate as the `<release>.tenant-ca` tenant secret. TLS is off unless set and is not inferred from `external`, so existing releases keep serving plaintext and are not restarted by the rebuilt operator image. TLS is chosen at creation: a running instance cannot switch modes, the change is rejected and surfaces as a failed HelmRelease upgrade, so add TLS by creating a new instance and migrating to it. Requires cert-manager, which the platform's `system` bundle installs on every variant. A renewed certificate rolls the Redis and Sentinel pods through the operator. TLS support is carried as a downstream patch over redis-operator v3.3.5 until freshworks-oss/redis-operator#85 lands in a release. ``` |
||
|
Aleksei Sviridkin
|
d800526696 |
feat(mariadb): add TLS support via cert-manager (#2680)
## What this PR does Gives MariaDB a chart-managed certificate chain, makes TLS enforcement explicit, and hands the tenant a key-free trust anchor instead of a private key. - `tls.issuer` selects who issues the server certificate: `operator` (the CA mariadb-operator manages itself, the default and today's behaviour) or `cert-manager` (the instance gets its own CA, and this is what covers the external hostname). The field is named for the issuer rather than for on/off because TLS is served under both — unlike the similarly spelled field in some other charts, which really is a switch. - `tls.required` refuses plaintext connections. It is opt-in and defaults to false: enforcing TLS before the tenant can obtain the CA would be backwards, and a platform upgrade must not sever connections that work today. - Neither issuer is inferred from `external`. Existing instances keep the CA they have and change nothing on upgrade. - The tenant no longer receives `<release>-ca-tls` or `<release>-tls` — the CA private key and the server private key. It receives the operator's `ca.crt`-only bundle, which works today, and the ResourceDefinition additionally declares that bundle as the source for the CA-extraction controller. ### Fixes along the way - The TLS backup path could never have worked: it passed `--ssl-mode=VERIFY_IDENTITY`, a MySQL-8 flag the MariaDB client rejects during argument parsing. Replaced with `--ssl-ca` + `--ssl-verify-server-cert`, and the test now asserts the dialect rather than the presence of a substring. - The leaf certificate carried `clientAuth`. Because `REQUIRE X509` accepts any certificate signed by a trusted CA, the server's own key was usable as a client credential. Removed. - The CA and server private keys survived deletion of the instance: cert-manager runs with `enableCertificateOwnerRef: false`, so removing the `Certificate` left the Secrets behind. Both are now cleaned up by the existing post-delete hook. - The Secrets are labelled for the operator's watch predicate, without which a renewed leaf would not reach the pods until a full resync. ### Why the include carries an exclude list The trust-anchor selector matches by label with no name constraint, so the enumerable dangerous set is excluded explicitly — both issuers' private keys, the internal credentials, the backup keys, and the metrics configuration, which the operator writes unconditionally with credentials in plain text for a user holding `SELECT` on everything. The backstop is bounded and the comment says so: matching is by exact name with no globs, so a Secret introduced by a future operator version, or a transient object created mid-renewal, stays in scope of the label-only include. The primary defence remains that the controller stamps only its own projection. ## Known limitations 1. **Merge is blocked on #3299 — and the blocking is mechanical, not just procedural.** The CRD on `main` does not carry `spec.caCert`, and under Flux's Server-Side Apply the API server rejects a field the schema does not declare (`field not declared in schema`); under the older client-side apply the same field was silently dropped, so installing this branch without #3299 fails outright. An earlier revision of this description claimed the field would be silently pruned; that was wrong. The tenant still gets a usable trust anchor once merged in order, through the operator's bundle. 2. **The deprecated backup CronJob cannot start at all**, for reasons that predate this branch: its `secretKeyRef` names a Secret and key the chart never renders. That is filed separately as #3347 and deliberately not fixed here, since repairing it would start scheduled dumps on production instances from a TLS change. The tests that pin the backup TLS flags therefore pin a path that does not currently execute — the suite header says so. 3. **The operator's `clusterName` is hardcoded**, so on a cluster with a non-default domain the SANs and the name the operator dials diverge. Pre-existing, raised separately. 4. **No e2e coverage for the managed path.** Unit coverage pins the rendered chain, the SAN set, the exclusion set and the guards; the end-to-end handshake is not asserted in CI. ### On review coverage Two independent review passes were run repeatedly against this branch — seven rounds, converging from a security finding, through comment accuracy, to guard strength, and finally to nothing new. A third, model-based pass reviewed the branch through `6cb1bd236` and its findings were fixed; it could not be run against the final revision because of an external account quota lasting until 25 July. The delta since that revision is substantial and includes runtime code, not only tests and docs. Recording this so the coverage is not overstated: if the quota returns before this is ready to merge, that pass is worth running again. ### Downstream repositories `packages/apps/mariadb/values.schema.json` gains the `tls` object, so `cozystack/terraform-provider-cozystack` needs the field modelled for `cozystack_mariadb`; tracked in cozystack/terraform-provider-cozystack#40. ### Release note ```release-note feat(mariadb): chart-managed TLS via cert-manager, opt-in enforcement, and the CA published to tenants as a key-free trust anchor ``` |
||
|
Aleksei Sviridkin
|
487cafac3e |
feat(platform): let a tenant pick its Gateway class from an allowlist
Tenant Gateways were pinned to the bundled Cilium class. Give the platform a default in gateway.className, the set a tenant may choose from in gateway.tenantSelectableClasses, and the set whose provider terminates TLS upstream of the Gateway in gateway.edgeTerminatedClasses. A tenant picks its own with tenant.spec.gatewayClass, read only when it owns a Gateway; an inheriting tenant runs on its ancestor's. The selectable set is an allowlist rather than free choice because Tenant is a tenant-writable object while a GatewayClass is cluster-scoped: without it a tenant could attach its Gateway to a class an admin keeps for internal-only traffic. Naming anything outside the set fails that tenant's own gateway release and reaches no other tenant. Where TLS ends is a property of the class, so the gateway chart derives certMode from whichever class the tenant landed on instead of taking it as a separate input. The two therefore cannot be configured out of step. The publishing tenant cannot cross that line by itself. The cluster-wide ACME issuers point their HTTP-01 solver at its Gateway and are rendered from the platform default alone, so a mismatch would pin the solver to a listener that Gateway does not have. Those issuers now drop the sectionName pin when the default class is edge-terminated, letting the challenge attach by hostname instead. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
2d6a18888d |
docs(rabbitmq): name caSecretName for what the operator does with it
The README and the TLS suite called spec.tls.caSecretName the inter-node mTLS knob and said the chart leaves it unwired. The field has nothing to do with Erlang distribution: under it the operator writes ssl_options.cacertfile, ssl_options.verify = verify_peer, and a cacertfile for the management and metrics listeners, every one of them client-facing. The cluster-operator this platform ships carries no distribution TLS under any field, so there was no inter-node path to leave unwired either. Say what the field does at each place it is named, and give the client-verification section the operator field it was missing, along with the reason verify_peer on its own does not make a client certificate mandatory. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
a1f512664a |
fix(rabbitmq): shorten the refusals that land in a release condition
Both messages ran 700-800 characters. They surface in a HelmRelease condition read through kubectl describe, which truncates, so the part an operator needs was the part most likely to be cut. Keep the objects that block, the two-upgrade order, the command that confirms the namespace is clear and a pointer to the README section; the mechanism is already there in full. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2d2f2d1ecb |
docs(rabbitmq): argue the opt-in default from timing, not availability
The paragraph closed by saying TLS moves clients before there is a key-free way to hand them the CA. This chart ships that way, and the section two headings down documents it. The reason the default holds is that the move lands the instant the flag is set, before any client has had a chance to pick the trust anchor up. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
824fbefeef |
docs(rabbitmq): drop the Role header the rule below already carries
The file header and the block at the secrets rule both said no TLS Secret is granted in either TLS state. The one at the rule is where a reader editing the grant will be looking. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
9e6465d093 |
fix(rabbitmq): scope the topology-object refusal to this release
A tenant namespace holds every RabbitMQ application the tenant runs, and the listing enumerated all of them. A second instance with users kept the first from ever closing its listeners, and the refusal named the other application's objects and told the operator to delete them. The mechanism behind the refusal does not reach that far either: a sibling's topology objects are reconciled through their own broker, whose listeners this flag leaves alone. Read spec.rabbitmqClusterReference.name and keep only the objects that name this release. The hand-written fixture gained the reference it was missing, which is what hid the gap, and a sibling release now has a case of its own. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
e521c3b8c7 |
test(rabbitmq): isolate the external gate on the platform-host SAN
Every existing case with external: false also left _namespace.host unset, so the platform host stayed out of the SAN list because no host was supplied, not because the gate held. Replacing the gate with a constant left the suite green. Supply a host with external: false so the gate is the only thing deciding. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
fcab5dbb07 |
docs(rabbitmq): carry the TLS consequences to where the toggle is rendered
cozyvalues-gen propagates the @field line and nothing under it, so the description reaching values.schema.json, the README table and the dashboard form said only that TLS turns on, while the continuation lines holding the breaking part of the change stayed in values.yaml. Fold the listener, binding-port and metrics-port moves into the @field line, and leave the continuation with the part that is genuinely about the struct. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
0d8a5c7941 |
docs(rabbitmq): keep the Certificate rationale out of the applied object
Two comment blocks in certmanager.yaml used `#`, which the renderer does not strip, so fourteen lines of prose about the sentinel wiring and the CA rotation policy were copied verbatim into every Certificate the chart applies. The rest of the file already explains itself with Go-template comments; use those here too, and cut the parts the README carries. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
fd9b96b2c5 |
fix(rabbitmq): refuse to close the plaintext listeners in one upgrade
Clearing users and vhosts and setting tls.disableNonTLSListeners in the same edit rendered cleanly, because the existing refusal reads values and both were already empty by then. Helm applies every object in the target release before pruning the ones the release dropped, so the broker closed plaintext and lost the operator-connection-uri annotation while the User, Vhost and Permission objects still held their deletion finalizers. messaging-topology-operator then had to reach the management API over a certificate it verifies against the system trust pool, which cannot hold a per-release CA, so the delete never completed and the upgrade reported success over objects left in Terminating. Read the topology objects actually present in the namespace instead, and only while the flag is being introduced, so a release that already closed its listeners keeps rendering and an object created afterwards does not block it. Both refusals and the README now carry the two-upgrade order, the command that confirms the namespace is clear, and the way back out. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
e04c5c62a3 |
fix(rabbitmq): create the tenant-ca projection the Role relies on
The dashboard Role already refuses to grant the key-bearing TLS Secrets, but nothing in the chart told the CA-extraction controller to publish the key-free trust anchor those Secrets feed: no TenantProjection sentinel referenced the CA Certificate's Secret as a source. Without that sentinel the controller has nothing to act on, so rabbitmq-<name>.tenant-ca is never created and a tenant has no path to the CA at all, contrary to what the README and the Role's own comments describe. Add templates/tenant-projection.yaml, gated on the same TLS condition as the certificate chain and mirroring the nats and qdrant charts: it names the CA Certificate's Secret as the projection source. Drop the internal.cozystack.io/publish-ca-cert labels from certmanager.yaml, since the controller resolves its source by name through the sentinel rather than by a label on the Secret, and correct the comments and README passage that described that label as the actual publication mechanism. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ff56ebae72 |
docs(rabbitmq): cut the values.yaml TLS prose to the callsite facts
The annotation block restated the README's TLS section nearly verbatim. Only the @param and @field lines reach the generated schema and README table; the rest was a second copy that would drift against the first. Keeps the three facts that change what a caller sets — neither field is inferred from external, enabling TLS moves the binding port, and disableNonTLSListeners is not external-only — and points at the README. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
411a3d6c02 |
docs(rabbitmq): cut template comments duplicating the README
The TLS comments had grown to narrate rejected alternatives and operator internals at length, roughly matching the volume of configuration they annotate. The README already carries every long-form explanation, so the duplicates are prose that will drift out of sync with it. Each block keeps the mechanism a next editor needs to avoid breaking something and points at the README section holding the rest. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
a0efa75dd2 |
fix(rabbitmq): add the missing bare pod-wildcard SAN
The certificate covered the client and headless Services in all four resolvable forms but gave the pod wildcard only three: bare *.<release>-nodes was absent. A client in the broker's own namespace dialling <pod>.<release>-nodes resolves through the DNS search path, connects, and fails only at hostname verification — which reads as a certificate bug rather than a missing name, the exact failure the four-form rule exists to prevent. The suite asserted the headless forms with contains, so it stayed green for the form nobody listed; it now pins the whole list. Also drops key encipherment from the leaf usages: it authorises RSA key transport and the key is ECDSA, so it requested a capability the key type cannot supply. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
52e60935e9 |
docs(rabbitmq): correct the disableNonTLSListeners escape hatch
The refusal message and the README told the reader to manage users and vhosts outside the chart. That reads as an instruction to move them into hand-written User/Vhost/Permission objects, which fails the same way: the topology operator derives its connection from the RabbitmqCluster, not from who created the CR, so those objects break identically and without the chart's refusal to warn anyone. Say what the flag actually costs — the cluster leaves that operator's reach entirely — and point at rabbitmqctl or a definitions import. Also narrows the claim that no option closes plaintext: a variant through spec.rabbitmq.additionalConfig looks reachable but is not wired or validated here, so it is named as an open question rather than left implicitly denied. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
636228f706 |
test(rabbitmq): pin the operator connection URI to the cluster domain
Every existing assertion on rabbitmq.com/operator-connection-uri ran on the default domain, so a hardcoded cozy.local would have satisfied all of them. On a cluster with its own cluster-domain that URI does not resolve and messaging-topology-operator loses the broker outright, so the case that distinguishes the two is worth having. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
4e3d9cd784 |
docs(rabbitmq): drop the TLS claims that only held under inheritance
The TLS scope section described an upgrade that turns TLS on by itself and told the reader to set tls.enabled: false to avoid it. Neither is true now that the field defaults to false: the rolled pods, the cert-manager dependency, the metrics port swap and the rewritten binding port are all consequences of an explicit edit, so they are described that way. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
36ee8a2608 |
feat(rabbitmq): make TLS opt-in instead of following external
tls.enabled inherited the value of external when unset, so enabling external access also enabled TLS. That is not free on an existing instance: setting spec.tls.secretName makes the operator rewrite the port key of the <release>-default-user Secret from 5672 to 5671, and that Secret is the Service Binding handed to the tenant. Every client resolving its endpoint from the binding — the documented way — would move onto TLS on upgrade and have to verify a per-release CA it has no key-free way to obtain. Resolve unset, null and false to false instead, so TLS is only ever on because someone asked for it. enabled becomes a plain bool now that there is no third state to express. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
dd04377e71 |
docs(rabbitmq): trim template comments to what the callsite needs
The template had grown to roughly 60% comment prose, most of it restating the
README verbatim — the strategic-merge-patch reasoning, the topology-operator TLS
derivation and the disableNonTLSListeners scope each existed in two places that
can drift apart, and the README is the copy users read. Several also carried
review-thread voice ("both directions are deliberate", "refuse the contradiction
rather than drop it") rather than describing the artifact.
Keep at each callsite only the facts an editor needs before touching that line —
why the override exists in neither direction, why the connection URI is pinned to
plaintext, what the flag does not cover — and let the README own the rest.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
||
|
Aleksei Sviridkin
|
8e48cfc8fd |
fix(rabbitmq): refuse contradictory TLS settings and correct the upgrade note
Two things this change got wrong about its own behaviour. tls.disableNonTLSListeners was silently discarded whenever TLS resolved to off, because the value was folded into the TLS condition before anything read it. Asking to close the plaintext listeners with TLS off leaves nothing listening, and the operator rejects the combination itself, returning "TLS must be enabled if disableNonTLSListeners is set to true" and setting ReconcileSuccess=False. Swallowing it in the chart was worse than what the operator does: the operator at least records a TLSError event, while a dropped flag rendered a healthy-looking cluster with every plaintext listener open and no trace of the request. The likeliest way in is an operator hardening an internal broker who sets only the flag, since tls.enabled follows external and so resolves to false there. Fail with a message naming tls.enabled instead. The README also claimed messaging clients are unaffected by enabling TLS. That holds only for a client with a hardcoded port. <release>-default-user is the Service Binding object the chart grants to the tenant, and the operator rewrites its port key from 5672 to 5671 as soon as spec.tls.secretName is set — gated on TLS alone, not on whether the plaintext listener stayed open — along with the per-plugin port keys. A binding-driven client, which is the documented way to consume it, follows that change and must speak TLS from then on, without being able to verify the certificate until the trust anchor is obtainable. Say so. Also record that the connection-uri annotation fans out: the operator's annotation filter strips only kubernetes.io and k8s.io keys, so it lands on every child object including the Secret the tenant reads. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
d668dfbd6a |
feat(rabbitmq): add opt-in switch to close the plaintext listeners
TLS so far only added encrypted endpoints; nothing closed the unencrypted ones, and the chart had no way to express the intent. tls.disableNonTLSListeners gives the operator that switch, defaulting to false so an upgrade never closes a listener a client is already using — the choice to break plaintext has to be made deliberately. Its reach is worth stating plainly, because the natural reading is wrong in both directions. It writes `listeners.tcp = none` into rabbitmq.conf, which is broker level: the socket is not opened on any interface, so in-cluster clients lose plaintext at the same instant external ones do, and there is no configuration that closes it outside while keeping it inside. In the other direction it is not "TLS everywhere" either — Erlang distribution on 25672 and epmd on 4369 ride the headless Service, which the flag never touches, so inter-node traffic stays in the clear regardless. The flag also cannot coexist with chart-managed users. messaging-topology-operator derives its connection as TLSEnabled() && (!connectUsingHTTP || DisableNonTLSListeners()), so the flag forces TLS on that side whatever the operator's own configuration says, and it verifies the broker against x509.SystemCertPool(), which cannot hold a per-release CA. The connection-uri annotation is no escape, since it would point at the port the flag just closed — so that annotation is now omitted when the flag is set, rather than advertising a closed port. Every User, Vhost and Permission would silently stop reconciling. Rendering fails instead, naming the conflict, and the flag stays usable for an instance whose users and vhosts are managed outside the chart. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
6f1ada4159 |
fix(rabbitmq): add the two-segment wildcard SAN for pod addresses
The SAN list covered every Service name in all resolvable forms but applied the rule only to the non-wildcard entries, leaving the per-pod wildcard with .svc and .svc.<domain> and no two-segment form. That is the form the operator itself uses: RABBITMQ_NODENAME is rabbit@$(MY_POD_NAME).$(K8S_SERVICE_NAME).$(MY_POD_NAMESPACE) and K8S_HOSTNAME_SUFFIX matches, so <pod>.<name>-nodes.<namespace> is the canonical pod address rather than an incidental alias, and it is the form the upstream API documentation shows to users. A client dialling a pod by that name resolves it through the DNS search path, connects, and fails only at hostname verification — which presents as a certificate bug rather than a missing name. It would also break outright the moment inter-node mTLS is wired through caSecretName. Move the clusterDomain helper above the tri-state doc block while here, so each comment sits next to the definition it describes. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2f42cd6baf |
fix(rabbitmq): keep the topology operator on the plaintext management port
Setting spec.tls.secretName makes RabbitmqCluster.TLSEnabled() true, and
messaging-topology-operator derives its own connection from that:
useTLSForConnection = TLSEnabled() && (!connectUsingHTTP || DisableNonTLSListeners())
connectUsingHTTP comes from CONNECT_USING_PLAIN_HTTP, which the shipped
Deployment does not set, so the expression collapses to TLSEnabled(). The
operator would dial https://<release>:15671 and verify it against
x509.SystemCertPool() — a process-wide pool that cannot contain a per-release
self-signed CA. Every User, Vhost and Permission this chart renders from users
and vhosts would fail with "certificate signed by unknown authority".
The failure is quiet, which is what makes it worth guarding: users already
applied to the broker keep working, so nothing looks wrong, while password
rotation, new users, new vhosts and permission changes all stop taking effect.
Annotate the cluster with rabbitmq.com/operator-connection-uri on the plaintext
management port when TLS is on. extractURIandScheme reports tlsEnabled=false for
an http:// scheme, so the operator keeps using the listener the chart
deliberately leaves open — that listener is load-bearing, not merely tolerated,
and the README now says so. Only the operator's management connection is
affected; broker traffic is untouched.
Also add the two-segment <name>.<namespace> SANs for both the client and headless
Services. That form resolves through the DNS search path, so a client connecting
to amqps://<name>.<namespace>:5671 reached the broker and failed only at hostname
verification — which presents as a certificate bug rather than a missing name.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
||
|
Aleksei Sviridkin
|
b4c66f2b55 |
docs(rabbitmq): record the metrics port change when TLS is enabled
Enabling TLS swaps the published metrics port: the operator exposes either prometheus:15692 or prometheus-tls:15691 and never both, deliberately, so a scrape config selecting every port does not count each node twice. That choice keys off TLS alone rather than off disableNonTLSListeners, so a ServiceMonitor selecting the port by the name prometheus stops matching once TLS is on, even though the broker keeps listening on 15692. Since tls.enabled inherits external, this reaches existing instances on upgrade with no user action. Re-adding 15692 through spec.override.service looks like the fix and is not. updatePorts keeps only port NAMES the operator owns and carries their NodePort across, so every reconcile drops the re-added entry; the override then reinstates it with nodePort unset, which the apiserver reads as "allocate" on a LoadBalancer. The resulting write retriggers the operator's own Service watch and the NodePort is reallocated each round. ClusterIP escapes it only because Update zeroes every NodePort there, leaving the patched spec equal to the live one — and external:true, the configuration that turns TLS on by default, is precisely the one that would loop. Trading a renamed metrics port for continuous LoadBalancer and kube-proxy reprogramming is not a trade worth making. So the chart renders no service override at all. The README says what changed and how to retarget a scrape config, including that 15691 is HTTPS under the release CA and so needs the trust anchor to verify, and the template comment records why neither direction of override is used. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
a021bfa324 |
docs(rabbitmq): correct TLS comments that outran the code
Several comments described a shape the chart does not render. A test comment claimed the chart carries a management cacertfile while the assertion directly below it pins that no additionalConfig exists at all; the suite title and two section headers advertised a management HTTPS block that is never rendered; and a scope note named interNodeTLS, which is not a field on RabbitmqCluster — the CRD exposes exactly secretName, caSecretName and disableNonTLSListeners. Two tests were named for something they did not assert. The inter-node scope test checked disableNonTLSListeners, which by this chart's own reasoning has no bearing on port 25672; it now checks caSecretName, which is the field that would actually configure inter-node mTLS. State the ResourceDefinition backstop's real standing. The exclude list is defence in depth, not the first line: promotion runs on the tenantresource label that the lineage webhook stamps only when it resolves an ancestor, and a cert-manager Secret has no ownerReference or Helm metadata, so it fails closed before include or exclude is consulted. The list covers the case where that stops holding. Pin the dashboard Role's complete grant rather than only the absence of known-bad names — every existing assertion there was a notContains, so a Role that gained an unrelated Secret satisfied all of them. Record what upgrading an existing external instance does: tls.enabled inherits external, so the broker pods roll once when the operator mounts the new Secret, and the instance gains a cert-manager dependency. No client breaks, but a stateful broker restarting is not something to discover from a diff. Drop four tests that duplicated earlier ones verbatim. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
7406303701 |
fix(rabbitmq): skip the external certificate SAN instead of failing without a host
Requiring _namespace.host under external:true turned a working configuration into a render error. The value derives from _cluster.root-host, which the platform treats as optional — the ingress hostname policy skips rendering when it is unset and the platform chart passes it through default so it can be empty — and an external RabbitMQ is a bare LoadBalancer Service with no Ingress and no hostname involvement. Releases in that state predate the tls field, so they carry no tls.enabled: false to fall back on and would have failed on upgrade with no way out. Append the SAN when a host is supplied and skip it otherwise. Say plainly what the external SAN is worth: the chart creates no DNS record for that name, so verifying by hostname means pointing it at the LoadBalancer address yourself, and a client dialling the IP fails hostname verification. Guarding the render was protecting a guarantee the chart does not deliver. Document the one manual step when disabling TLS, which until now existed only in a template comment nobody performing the downgrade would read: cert-manager keeps the Secrets after Helm removes the Certificates, so the CA keeps its publication label and a stale anchor keeps being projected until it is deleted. Correct the tri-state helper's own comment while here. It claimed to guard a class of input it does not need to: values.schema.json types tls as an object, so non-map scalars are rejected by Helm before any template renders. Null is the only shape that reaches the template, and that is what the dig form covers. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
d8a711bd29 |
refactor(rabbitmq): resolve the TLS tri-state in one place
The tri-state resolution was copy-pasted into three templates. It had already
drifted: the copy in the dashboard Role outlived the grant it used to gate and
was removed with nothing to catch that the remaining two could diverge in the
same way. Extract rabbitmq.tls.enabled and have both callers use it.
Reading .Values.tls.enabled directly also panicked with "nil pointer evaluating
interface {}.enabled" under `tls: null`. The app CR cannot produce that shape,
since structural-schema pruning replaces null with the {} default, but a
HelmRelease values override reaches the chart without pruning. Resolve through
dig on a defaulted dict so the value falls back to external instead.
Cover both with a suite that renders the cluster and the certificate chain
together across every external x tls.enabled combination and asserts they agree,
so a future divergence fails a test rather than issuing certificates nobody uses.
Drop the management ssl cacertfile line while here. It was set on the management
listener but not on AMQPS, and on neither does a CA alone verify client
certificates without verify_peer and fail_if_no_peer_cert. It bought nothing and
left the two listeners inconsistent; the operator supplies everything else once
spec.tls.secretName is set, so no additionalConfig remains.
Pin the ResourceDefinition invariants that decide whether a private key can
reach a tenant. The chart's helm-unittest suite cannot see that file, so the
label-based include, the non-empty selector and the exclude backstop are
asserted in hack/check-rabbitmq-rd-secrets.bats instead.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
||
|
Aleksei Sviridkin
|
31101eeeaf |
fix(rabbitmq): drop schema guard that the aggregated API strips
values.schema.json carried additionalProperties:false on the tls object,
post-processed onto a generated file by a jq patch plus a drift guard, and a
test asserted that an unknown tls key is rejected. It is not rejected on the
path tenants use: the aggregated apiserver removes every boolean-form
additionalProperties before publishing, and its own comment states the schema
is consumed for documentation and type-checking rather than enforcement. The
constraint therefore held only for a direct helm install, while the test
documented it as a general guarantee. rabbitmq was also the only chart in the
catalog carrying it. Remove the patch, the guard and the test rather than keep
a check that does not check on the path that matters.
Also in the same area, three fixes for things that claimed more than they did:
The management ssl port, certfile and keyfile lines in additionalConfig
duplicated the operator's defaultTLSConf verbatim, which it writes as soon as
spec.tls.secretName is set. Only cacertfile adds anything outside mutual TLS.
Keep that one and record what it does not do — supplying a CA does not enable
client-certificate verification without verify_peer and fail_if_no_peer_cert,
so clients still authenticate by password inside the TLS session.
The dashboard Role computed the tri-state TLS flag and never read it, left over
from removing the Secret grant it used to gate.
A missing _namespace.host under external:true rendered as `nil pointer
evaluating interface {}.host`. Fail closed with a message naming the key: the
external DNS name is a certificate SAN, so omitting it silently produces a
certificate that fails verification at connect time instead of at render time.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
||
|
Aleksei Sviridkin
|
56043f962b |
fix(rabbitmq): drop the service port override that could not remove a port
The override listed only the TLS ports, which reads as "plaintext is not published on the LoadBalancer". It never did that. The operator rebuilds its default port set from generateServicePortsMap() on every reconcile and only then applies spec.override.service as a strategic merge patch keyed on `port` (corev1.ServiceSpec tags Ports with patchMergeKey:"port"), so a partial list can add or mutate entries but never remove them. With TLS on, the two ports it listed are byte-identical to what the operator already generates, making the whole block a no-op that credited itself with a guarantee it did not provide. Remove it and say plainly what holds: amqp:5672 and management:15672 stay published, on the LoadBalancer too when external is true. Closing them requires spec.tls.disableNonTLSListeners, which writes `listeners.tcp = none` into rabbitmq.conf — a broker-level switch that closes the socket on every interface at once and drops in-cluster plaintext clients along with external ones. It stays unset, so enabling TLS adds encrypted endpoints without breaking a working connection; treat an external RabbitMQ as plaintext-reachable and restrict it at the network layer if that is not acceptable. Erlang distribution on 25672 is unaffected either way: the operator never applies the flag to the headless Service that carries it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
d6c1539545 |
fix(rabbitmq): stop granting key-bearing TLS Secrets to tenants
Both objects holding ca.crt also hold a private key: the leaf <release>-tls carries the server key and the CA <release>-ca carries the CA key. RBAC has no key-level filter, so granting either one to deliver a trust anchor delivers a key with it — read access to the CA key is enough to issue a certificate for anything and MITM the endpoint undetected. Publish the CA instead: label the isCA Certificate so the CA-extraction controller projects ca.crt alone into the key-free <release>.tenant-ca, which tenants read through core.cozystack.io/tenantsecrets. The labels go on the CA Certificate rather than the leaf, since projecting from the leaf would read a Secret whose tls.key is the server key. The release label is required because cert-manager issues the Secret, leaving no OwnerReference or Helm metadata to attribute it by. Back the label-only include with a name-based exclude covering the cert-manager chain and the operator's erlang-cookie, so a mis-stamp of the trust-anchor label cannot promote a private key. The erlang cookie is worth the entry on its own: it grants peer-node code execution over port 25672, which disableNonTLSListeners does not close. Document the retrieval path and state the limits it comes with — the projection depends on a controller that ships separately, and the CA rotates, so a bundle copied once eventually stops verifying. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |