mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-03 10:37:21 +09:00
feat/externalip-api
6188
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Andrei Kvapil
|
99d16a21bc |
fix(hack): stop the RD preset check failing at random
grep -q exits on its first match, so piping the candidate list into it leaves the writer with unwritten output and kills it with SIGPIPE. Under pipefail the pipeline then exits 141, which the caller reads exactly like "not found" — so a preset that is present gets reported missing, naming a different file and a different preset on every run. Measured at roughly one spurious failure per run before the change, none after. A here-string removes the pipe and with it the race. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
d21feaed52 |
chore(api): regenerate deepcopy for the new ExternalIP value type
Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
26562a4f6d |
test(cozy-lib): cover the externalIP claim helpers
Eight catalog charts thread the same value through these two helpers, so the contract belongs in one suite rather than eight: which annotation key is written, that a target with no entry gets no annotation at all rather than an empty map, and which malformed values are refused. The malformed-value cases are only reachable here. In an app chart the generated values schema rejects them first, so the helper's own refusals would never be exercised. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
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>
|
||
|
Andrei Kvapil
|
75b119a760 |
feat(cozy-lib): add helpers for pinning endpoints to reserved external IPs
The address substrate from cozystack/community#35 lets a tenant hold an IP across a workload rebuild, and a workload consumes that reservation through a single annotation on the LoadBalancer Service that should wear the address. Every catalog chart needs to emit the same annotation, but each engine takes Service annotations in a different place, so without a shared helper the key would be spelled out once per chart. The naming of the kinds and the API group is still open on #35, so the key is expected to move. Writing it down once keeps that a one-line change instead of a sweep across the catalog. Rendering fails on a malformed externalIPs value rather than dropping it: a typo in a claim name is the difference between the address a customer allow-listed and some other address, and an install-time error is far more visible than a Service that quietly came up on the wrong IP. 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. ``` |
||
|
Aleksei Sviridkin
|
d685fb30ec |
fix(gateway)!: require tenant routes to declare their hostnames (#3475)
## What this PR does The tenant route hostname policy skipped the shape it most needed to check. Its CEL opened with `!has(object.spec.hostnames) ||`, so an HTTPRoute or TLSRoute declaring no hostnames satisfied the first operand and got admitted with nothing compared against the tenant apex. An empty list passed too: `has()` is true on `[]`, and `all()` over an empty list is vacuously true. Such a route is not inert. Under Gateway API it inherits the hostnames of every listener it attaches to, and `parentRefs` can name a Gateway in another namespace, so it covers whatever that listener covers. Reported in #3467. The predicate now requires the field to be present and non-empty before validating it. Both policies rendered from that template share one CEL string, so HTTPRoute and TLSRoute get the change together, and both match `CREATE` and `UPDATE`, so an existing route that drops its hostnames is denied exactly as a new one is. The issue is titled after TLSRoute, but the reachable shape is mostly HTTPRoute. TLSRoute allows a hostname-less route only on the deprecated `v1alpha2`; at `v1` and `v1alpha3` the bundle this repo vendors marks `spec.hostnames` required and rejects an empty list on `minItems: 1`, while `v1alpha2` carries neither constraint. HTTPRoute `v1` carries neither either, so it accepts both shapes. All four are checkable in `packages/system/gateway-api-crds/templates/crds-experimental.yaml`. Checking that turned up a stale claim in the template header, which said TLSRoute had no `v1` in Gateway API 1.4 to 1.5 and used it to justify the rule naming only `v1alpha2`. Gateway API 1.5.1, which this repo vendors, serves TLSRoute at `v1` as the storage version. The rule still covers every served version because `matchConstraints.matchPolicy` defaults to `Equivalent`, and that default is now written down, because setting `matchPolicy: Exact` reads like a tightening and would silently stop the policy seeing `v1` and `v1alpha3` TLSRoutes. I corrected the text and left the coverage alone: the wrong sentence predates this branch, but my change is what makes the file contradict itself, so the text is mine to fix while widening version coverage is a separate job. A `ValidatingAdmissionPolicy` has no cross-object lookup, so it cannot resolve the parent Gateway to see which hostnames a route would inherit. Requiring the route to name its own is what keeps every admitted hostname comparable against the apex. That has a cost: a route relying on listener inheritance now has to spell its hostnames out, and a tenant that hand-writes one gets a denial where it used to get an admission. ### The controller change belongs in the same PR `renderHTTPRedirect` built the http-to-https redirect HTTPRoute with no hostnames on purpose, to catch every host on port 80. It lands in a `tenant-*` namespace, so the stricter policy denies it, `reconcileHTTPToHTTPSRedirect` returns that error, and since it runs before the status update the TenantGateway never reaches Ready. That redirect is what keeps app routes attaching by hostname from serving plaintext on port 80. Neither half of this change is safe alone. The redirect now names the tenant apex and its wildcard. Both entries are needed: a wildcard hostname is a suffix match over any number of labels, so `*.<apex>` covers subdomains at every depth but not the bare `<apex>`. `renderWildcardCertificate` already builds the same pair. Coverage narrows in one direction. A request on port 80 with a Host outside the tenant apex now falls through to a 404 instead of a 301. That narrowing costs no covered host: the port-80 listener admits routes only from the Gateway's own namespace and cert-manager's, and the tightened policy requires every route in the tenant namespace to declare its hostnames inside the apex, so any host published from this namespace already falls under the apex or its wildcard. An inheriting child tenant does not widen that set either. Its hostnames would reach port 443 through a listener on this same Gateway, and `cozystack-gateway-hostname-policy` compares every listener against the host label of the Gateway's own namespace, so a child apex outside that label never gets its listener: the Gateway write carrying it is refused at admission whole, in `reconcileGateway` and before the redirect is rendered. A child apex under this one passes that check and the wildcard already covers it. That holds at merge base as well, so it is not something this branch changes. Deriving hostnames from `spec.apex` makes that field load-bearing for admission, so it gains `MinLength=1`. An empty apex used to reconcile to Ready, since the redirect declared no hostnames at all; it would now render `""` and `"*."` and be rejected on the HTTPRoute hostname schema, surfacing as a validation error naming the route rather than the field. The renderer checks it too, because the CRD and the controller binary roll out separately and an older CRD in the cluster will not carry the bound. That schema bound is a breaking change to an existing field and the API review gate flags it as one, so here is what it actually invalidates. The chart that renders these CRs already fails on an empty `_namespace.host`, and in DNS-01 or existingSecret mode an empty apex already broke earlier, in `renderGateway`, on a listener with `hostname: "*."`. Nothing that works today becomes invalid. What the bound rejects is a TenantGateway written by hand, past the chart, with an empty apex, and I would rather that be refused at admission naming the field than at reconcile time in controller logs, which is why the bound sits in the CRD as well as the renderer. Naming those hostnames also meant excluding the redirect from `collectHostnameClaims`. A route that attaches to the Gateway and declares hostnames otherwise reads as an app publishing them, and each claimed hostname becomes an HTTPS listener named after its first label. The first label of `*.<apex>` is `*`, which the Gateway CRD's listener-name pattern rejects, so the Gateway write fails and the reconcile stops before anything downstream. The exclusion matches on namespace, name and controller ownership rather than on the labels the renderer stamps: those labels are writable by anyone who can create a route, and the claim set also drives conflict resolution, so a label-keyed exception would let any route opt out of being marked `Accepted=False` when it loses a hostname race. Name and ownership are each required and each is pinned by its own test, since ownership alone would exclude any other controller-owned route and name alone would extend the exception to a route planted under that name by someone else. ### Radius Only namespaces starting with `tenant-` are matched. The three first-party TLSRoutes the platform ships, `kubernetes-api` in `default` plus `cdi-uploadproxy` and `vm-exportproxy`, set hostnames explicitly and none sits in a `tenant-*` namespace, so nothing changes for them. I went through every producer of routes in the tree, in three places rather than one. No Helm template emits an absent or empty hostname list in any configuration a tenant can reach: the vendored openbao, harbor, opencost and flux-operator route templates can render one, but each is behind a flag that defaults off with no values path to turn it on, and openbao would fail CRD validation first because its default route apiVersion requires hostnames. Exactly one Go site builds a route, the redirect fixed here. And one e2e fixture did rely on the old behaviour: the Gateway suite's parentRef probe declared no hostnames and is applied into `tenant-test`, which the policy matches, so it now reads the apex from the namespace label and names a hostname under it, the same way the neighbouring cases in that suite already did. cert-manager also writes routes into tenant namespaces, since it creates the Challenge in the Certificate's own namespace and the HTTP-01 solver route beside it. That route declares the hostname it is validating, so the policy admits it. ### Related issues Two open issues have a text half that this PR happens to settle, and neither of them closes here. #3788 reports that the port-80 allow-list contains the namespace its own comment says it excludes; the comment and the README paragraph now describe what that filter actually does, while the behavioural exit it names stays open. #3789 reports that the TLSRoute rule names only the deprecated API version; that premise is now written down in the template header, including what has to change before a bundle drops `v1alpha2`, but the rule still names `v1alpha2` alone. #3787 covers the wildcard-hostname listener name that the apiserver rejects, which is what makes the claim-set exclusion here load-bearing rather than tidy. ### Screenshots N/A (no UI changes). ### Downstream repositories I walked the trigger map in `docs/agents/contributing.md` against the diff, file by file. No package is added, renamed or removed; no `values.yaml` key, `values.schema.json` field, version enum or default moves; no `kind` or `plural` changes; no namespace or release-asset name changes; nothing under `hack/` moves, is renamed, or changes what a make target does. One row deserves a sentence rather than a blanket denial, because the diff does change a CRD schema: `spec.apex` on `gateway.cozystack.io/v1alpha1 TenantGateway` gains a `minLength` bound, which lands in the generated `gateway.cozystack.io_tenantgateways.yaml`. The provider hand-types `Package` under `cozystack.io` and `Plan`/`RestoreJob` under `backups.cozystack.io`; `gateway.cozystack.io` is not among them, and a tightening on an existing field adds no new field for it to offer typed access to. The `packages/extra/gateway/README.md` edit sits in the `## Security model` section, below the `## Parameters` block that `cozyvalues-gen` regenerates, so it survives `make generate`. - [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(gateway)!: the tenant route hostname policy now requires an HTTPRoute or TLSRoute in a `tenant-*` namespace to declare a non-empty `spec.hostnames`. A route that omitted the field, or set it to an empty list, was previously admitted without any hostname being checked against the tenant apex, even though such a route inherits the hostnames of the listener it attaches to rather than matching nothing. A route in a `tenant-*` namespace relying on that inheritance is now rejected and has to name its hostnames. The controller-owned http-to-https redirect route names the tenant apex and its wildcard accordingly, so a request reaching port 80 with a Host outside the tenant apex now gets a 404 instead of a 301. `TenantGateway.spec.apex` additionally gains a `minLength: 1` bound, because the redirect derives its hostnames from that field; a TenantGateway written past the chart with an empty apex is now rejected at admission instead of failing later in the controller. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added edge-terminated gateway support with plain-HTTP listeners and TLS passthrough handling. - HTTP-to-HTTPS redirects now explicitly cover the tenant apex and subdomains. - Gateway configurations require a non-empty apex hostname. - HTTPRoute, TLSRoute, and GRPCRoute resources must declare hostnames. - **Bug Fixes** - Invalid hostname-less routes are now rejected consistently. - Controller-managed redirects no longer trigger unnecessary hostname claims, listeners, or certificates. - Switching certificate modes now cleans up obsolete ACME resources and route status. - **Documentation** - Updated security and gateway guidance for hostname validation, edge termination, route compatibility, and certificate handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
myasnikovdaniil
|
7d7eb1c23e |
chore(codeowners): rebuild ownership by area, five owners minimum (#4129)
## What this PR does
CODEOWNERS applies only the last matching pattern and it replaces every
broader rule including `*`, it does not merge them. Half of what that
costs is already fixed: `bd7b76691` added @kvaps and @lllamnyp to the
rules that were missing them, and `hack/codeowners-invariant.bats` keeps
them there. That invariant reads "every rule with owners is a superset
of the `*` owners", and the other half of the problem satisfies it
without a complaint:
```
/dashboards/ @myasnikovdaniil @IvanHunters @kvaps @lllamnyp <- widened by
|
||
|
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 --> |
||
|
IvanHunters
|
67a86746a6 |
feat(marketplace): community marketplace for External-Apps repositories (#3940)
## What this PR does Implements the community marketplace for Cozystack External-Apps repositories, per the approved design proposals **community#18** and **community#23**, building on the **community#12** authoring workflow. Anyone can publish an External-Apps repository over OCI, discover it, connect it to a cluster, and install its apps the same way official packages are installed, with a publication gate that keeps the index safe. Additive by construction: the existing PackageSource / External-Apps pipeline is untouched, no CRD is added or migrated, and existing installs and upgrades are unaffected. Per-package version pinning stays out of scope, siding with #18. Components: - **cozypkg CLI** — new `validate`, `init`, `push`, `tap`/`untap`, `search`. Scaffold a repository, publish it as an OCI artifact, register it on a cluster in one command (optional pull secret for private repositories), and discover packages through a metadata-only community index. `validate` lints a repository offline (schema, chart paths, `helm lint`, dependsOn, ApplicationDefinition wiring, privileged components); with `--require-signature` it verifies a keyless cosign signature. - **Marketplace backend** — a read/write, cluster-scoped `Tap` resource in the existing `core.cozystack.io` group (a computed virtual resource: no new group, no CRD, no etcd). Lists connected repositories and the packages they expose, connects a repository (recording intent as a labeled Flux `OCIRepository`), and disconnects one. Catalog metadata only, never a pull credential. - **Tap materializer** — a controller in cozystack-controller that, once source-controller has pulled a connected repository's artifact, fetches it over the Flux artifact URL (digest-verified, size-bounded, path-traversal-guarded) and materializes the PackageSource(s) it carries under the `community.` prefix, so a dashboard-connected tap becomes installable without the apiserver pulling artifacts. - **Publication gate** — a template for a `cozystack/packages-index` repository: a metadata-only entry format and a GitHub Actions gate that validates every changed entry with `cozypkg validate` (mandatory cosign against the entry's recorded identity) under a two-lane merge policy (owner version bump auto-merges on signed validation; a new entry or a security-relevant edit goes to maintainer review). - **Dashboard** — a Repositories view in the console marketplace, backed by `Tap`, to browse and connect/disconnect repositories. Access control: reading the catalog is open to authenticated users (metadata only); connecting and disconnecting require cluster-admin via the delegated apiserver RBAC. Verification: `go build` / `go vet` / `go test`, dashboard `pnpm typecheck` + eslint, and the gate's `shellcheck` are all green. Server-side authz denial and the end-to-end connect→materialize→disconnect round-trip are not exercised here (hermetic change) and are the main follow-up. ### Screenshots The dashboard Repositories view is included, but screenshots are not attached: rendering it needs a live cluster running the new `Tap` apiserver, which this static change cannot stand up. Tracked as a follow-up (a cozystack-pr-test run on a disposable cluster); this is one reason the PR is a draft. ### Downstream repositories Walked the trigger map against the diff. This is additive tooling plus a new virtual resource; it does not force a change in any listed repository, and `external-apps-example` keeps working on the existing chart model. Left every box empty (rather than ticking "No downstream repository is affected") so a maintainer can confirm — in particular whether `external-apps-example` should later gain a PackageSource-based example that `cozypkg init` now scaffolds. - [ ] 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(marketplace): add a community marketplace for External-Apps repositories — cozypkg validate/init/push/tap/untap/search, a cluster-scoped Tap API in core.cozystack.io with an operator-side artifact materializer, keyless cosign verification, a two-lane publication gate, and a dashboard Repositories view ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added Marketplace repository connections (“Taps”) with dashboard management, readiness status, package catalogs, and optional credentials. - Added repository search by name, source, and package metadata. - Added a searchable community package index with version-pinned resolution. - Added `cozypkg init`, `push`, `tap`, and `untap` commands. - Added offline repository and artifact validation with optional chart linting and signature checks. - Added safeguards requiring confirmation before installing packages with privileged components. - Added automatic synchronization of tapped package catalogs. - **Documentation** - Added community package index guidance, validation workflow, and an example entry. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
62f2234aea |
fix(e2e): stop a failed teardown posting a failed test verdict
The teardown ends both merge-gating lanes under `if: always()`, and this branch made it `exit "$cleanup_rc"`. So a `docker compose down` hiccup, a busy zpool or a failed rmdir after a fully passing suite posts the required "E2E Tests" status as a failure, and nothing downstream can tell that apart from a test that actually broke. The runner is discarded when the job ends, so there is no next run for the strictness to protect; a reused host is a developer machine, and an annotation serves it. Loud, not fatal. Two comments described mechanisms that are gone. The breakpoint step still explained reading a soft-red output that this branch removed, so a maintainer would go looking for it; the node-join deadline now fails its suite like any other, which is what makes `failure()` sufficient. The build-talos header still described building the nocloud disk and e2e needing it, when the job builds neither and the container lane downloads no disk -- the profile is compiled by nightly and by the tag build's `make assets` instead, and what a PR still compiles is installer.yaml, which shares every input with it bar the platform and output format. And the platform-package override now states its consequence for both packages it replaces, not only for CDI: the merge-gating lane exercises neither the shipped CDI default nor `drbd.enabled: true`. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
Myasnikov Daniil
|
8456facc79 |
test(e2e): pin the stability-window reset and keep the fan-out test portable
The readiness gate resets its stability window when a list read fails, and nothing held that: the existing "transient list error" case fails on the first read, when there is no window to invalidate, so removing the reset left the suite green. The new case breaks a read mid-window, where a gate that kept counting through the gap would accept a release set it never observed stable across it. The parallel fan-out contract asked make for --output-sync, which arrived in GNU Make 4.0 while macOS still ships 3.81 -- so on a contributor's machine the suite died on an unknown option instead of reporting the contract. Guarded by version, with the reason in place. CI runs 4.x, so the contract is still enforced where it gates a merge. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
Myasnikov Daniil
|
b30c7efaf6 |
fix(e2e): make the OIDC teardown assertions fail closed
The three checks that System-mode artifacts are gone after the mode switch were `if kubectl get ... >/dev/null 2>&1`, which reads every non-zero exit as "the object is gone". An RBAC denial, a timeout or a missing CRD produces the same exit, and stderr went to /dev/null, so the suite reported a clean System teardown for never having observed one. --ignore-not-found is what separates absent from unaskable: absent is exit 0 with empty output, everything else stays non-zero. Preferred over matching NotFound in the message, which depends on wording and, under a runner with tracing on, captures the trace instead of the error. Each probe is pinned individually rather than as a group: the new case breaks one query per iteration while the other two answer absent, so reverting any single probe to the bare form reddens the suite. Verified by doing exactly that three times. A test that broke all three at once would have passed on any one surviving probe, which is how the first attempt at this test fooled me. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
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 |
||
|
Myasnikov Daniil
|
16a49740fe |
chore(codeowners): rebuild ownership by area, five owners minimum
CODEOWNERS applies only the last matching pattern and it replaces every broader rule including the catch-all, it does not merge them. Half of what that costs is already fixed: |
||
|
IvanHunters
|
109ee1dd9e |
refactor(cozypkg): share one reserved-prefix list between init and validate
init carried the same two prefixes inline that validate declares as reservedNamePrefixes, so a third prefix added for validate would have left init letting it through. Both files are package cmd, so this is one identifier. Drive the init test off the shared list for the same reason. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
8738063287 |
fix(operator): reject tar entries containing ".." before any path operation
safeJoin already rejected traversal and re-checked the joined path against destDir, but the guard sits behind a call boundary that CodeQL's go/zipslip barrier does not follow, so the extraction loop reported three high-severity arbitrary-write alerts. Reject the entry name up front instead, which is both the fail-closed spelling and the shape the analysis recognises. Extend the traversal test to cover a ".." buried mid-path, not just a leading one. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
0c90abcedd |
test(rbac): cover the marketplace tap grants in the pinned RBAC suites
The tap flow added a packagesources delete grant and an ocirepositories read grant to the cozystack-api ClusterRole, and taps to the tenant core.cozystack.io grants. Both suites pin those lists on purpose, so move the count bound and assert the new grants by name instead of leaving them uncovered. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
7ac9404057 |
fix(operator): do not force-overwrite a PackageSource the platform does not own
installPlatformPackageSource applied its PackageSources with Force: true and no ownership check, so a platform upgrade could silently overwrite a PackageSource a marketplace tap had materialized under a name the platform later ships. The collision check cannot cover this: it runs at tap time, the overwrite months later at upgrade. Drop Force so server-side apply surfaces a field-ownership conflict from any other manager as an error, and add an ownership pre-check that fails with a specific message when the existing object carries the marketplace-tap marker, leaving it untouched. Re-applying the platform's own object never conflicts. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
b8f8efa66d |
feat(marketplace): restore the reserved-name check as a usability hint
Per review: keep the authoring-time hint that steers away from the reserved cozystack. and community. prefixes, independent of the runtime naming model. cozypkg init rejects a reserved --name, validate reports ps-name-reserved at error severity, and --allow-reserved-names stays a caller-side escape hatch the index gate never passes. The on-cluster collision check remains the real boundary; this only lets an author learn at init rather than after publishing. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
2af19c4378 |
fix(marketplace): Get surfaces a pending tap instead of 404
List emits a pending Tap for a tap source (labeled OCIRepository) that has no materialized PackageSource yet, but Get looked up only the PackageSource and returned NotFound for the same object. A dashboard detail view (list then detail) of a connecting or collision-blocked tap would 404 instead of showing the block reason. Get now falls back to the pending tap source by name, the same lookup Delete already uses, so Get, List, and Delete agree. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
d1e0f41cbd |
refactor(marketplace): drop community-prefix naming in the dashboard
deriveTapName now yields the tap source id (tap-<org>-<repo>) that the server assigns, and the community-prefix constant is gone. The Repositories view labels a tapped source "external" rather than "community"; it already renders a pending or blocked tap's message, which the server now returns for taps with no materialized PackageSource yet. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
15be9329a1 |
refactor(marketplace): source-centric Tap resource, label-based identity
The aggregated Tap resource no longer derives a community.<org>.<repo> name from the connect URL. A connected repository is identified by its Flux source (tap-<org>-<repo>), and the PackageSource(s) it materializes keep their own declared names. Community is computed from the marketplace-tap label, and disconnect is guarded by that label instead of a name prefix. List also surfaces tap sources with no materialized PackageSource yet (a connect in progress, or one blocked by a collision), carrying the materializer error message, so a blocked tap is visible in the dashboard rather than silently never appearing. The community. prefix constant is removed. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
1653da967b |
refactor(marketplace): drop artifact-prefix injection and reserved names
Without the community. rename, a tapped repository's cozyrds ApplicationDefinition references its component's artifact by the concrete <packagesource>-<variant>-<component> name again, so the operator no longer injects a cozystackArtifactPrefix value and the -rd chart renders its cozyrds asset with a plain Files.Get. cozypkg init scaffolds the concrete chartRef. Reserved name prefixes are removed: repositories choose their own PackageSource names, and a clash with a core component is caught on-cluster at tap time (the collision check) rather than by refusing prefixes offline. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
95badea1c8 |
refactor(marketplace): identify taps by marker label, drop community rename
Tapped repositories keep their own declared PackageSource names instead of being renamed under the community. prefix. Tap-ness is now marked by the marketplace-tap label (and source annotation) on the OCIRepository and every materialized PackageSource, so untap, the Flux-source cleanup, and idempotent re-tap key on the marker rather than a name prefix. A declared name that clashes with an existing core component (or another tap) is rejected: the CLI tap fails fast before applying anything, and the operator materializer refuses to overwrite the foreign object, records the reason on the source (annotation and Event), and does not stamp the revision so a corrected artifact retries. The shared check lives in internal/marketplace/collision. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
0c0609ad63 |
fix(marketplace): resolve community chartRef via injected artifact prefix
Fixes the rename-vs-chartRef break found on dev3: a community-tapped
PackageSource is renamed to community.<org>.<repo>, so the assembled artifact
name changes, but a repo's cozyrds ApplicationDefinition baked chartRef.name
under the author's original name and the ExternalArtifact it referenced no
longer existed, leaving every community-installed app permanently ArtifactFailed.
Implements @kvaps's option 2 (derive at the -rd component, keep platform charts
literal):
- naming.ArtifactPrefix exposes the per-(source,variant) artifact-name prefix.
- The Package reconciler injects it as the cozystackArtifactPrefix Helm value,
but ONLY for community.-prefixed sources, so platform HelmRelease values are
byte-for-byte unchanged (no churn, no platform chart edits).
- cozypkg init now scaffolds a community-style -rd chart: cozyrd.yaml renders the
cozyrds asset through tpl, and chartRef.name templates
'{{ .Values.cozystackArtifactPrefix }}-<app>', resolving to the real artifact
whatever the tap-time rename produces.
- validate skips the dangling-chartRef check for a templated name (it cannot be
cross-checked at authoring time; the operator resolves it at install).
Verified end-to-end: init -> validate is clean, and helm template with the
prefix value renders chartRef.name to the exact artifact the reconciler emits.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
950268cccc |
feat(marketplace): reject reserved PackageSource name prefixes
From @kvaps's review: a repository author could name a PackageSource 'cozystack.<x>' or 'community.<x>' and claim the platform or marketplace namespace. Until now validate only rejected an empty metadata.name and the forced rename in the materializer was the sole guard. - validate now flags 'cozystack.'- and 'community.'-prefixed PackageSource names as an error. The escape hatch --allow-reserved-names lives on the caller side only: the index gate (validate-entry.sh) never passes it, so a submitter cannot hand it to themselves. - cozypkg init refuses the same prefixes early with a neutral-name hint. - Document why the ClusterIP fail-open is safe: it falls back only on address resolution, and integrity stays enforced by the fail-closed digest check, so a wrong host yields a mismatch error rather than a content substitution. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
b96a97e5c4 |
fix(marketplace): resolve tap artifact host to ClusterIP for the hostNetwork operator
The tap materializer fetches the Flux artifact from status.artifact.url, which is a cluster-DNS name (flux.<ns>.svc/...). cozystack-operator runs with hostNetwork: true and dnsPolicy: ClusterFirst, so it resolves against the host's DNS rather than CoreDNS and that name is NXDOMAIN — every dashboard-driven connect (API Create, which relies entirely on the materializer) then hangs in 'connecting' forever, while the CLI tap works only because it pulls via 'flux pull artifact' from the public registry. Resolve the artifact host to the backing Service's ClusterIP (which a hostNetwork pod reaches directly, the same way the in-cluster apiserver is reached via KUBERNETES_SERVICE_HOST) before fetching. This fails open: a non-cluster host, a missing Service, or a headless/empty ClusterIP falls back to the original URL, so clusters where DNS already works are unaffected. Services are readable through the operator's existing cluster-wide get/list/watch grant. Found on dev3 via cozystack-pr-test; the connect->materialize path never worked on a stock operator deployment before this. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
b6f14373b5 |
fix(marketplace): scope controller OCIRepository writes and split validate YAML safely
Close the two non-blocking notes from the latest review, mirroring fixes already applied on the other side: - The tap materializer's update/patch on ocirepositories moves from the cluster-wide cozystack-controller ClusterRole to a namespaced Role in cozy-system (get/list/watch stay cluster-wide for the informer), matching the API-side scoping. The materializer only ever writes tap sources in cozy-system. - cozypkg validate now splits multi-document YAML with the apimachinery reader instead of a naive ^---$ regex, so a PackageSource whose block scalar contains a separator-looking line is not fractured — the gate's validator now decodes the same content the materializer applies. Covered by a block-scalar test. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
c9accd6086 |
fix(marketplace): address maintainer review — url collisions, fail-closed digest, centralized prefix
From @kvaps's review of the untrusted-artifact path: - Refuse to silently retarget: two repositories that share an org/repo path on different registry hosts derive the same tap name. The API and CLI now reject a connect when an existing source points at a different spec.url, instead of blindly repointing the first at the second. - Fail closed on integrity: verifyAndExtract now errors on an absent digest (not just a mismatch), and the reconciler waits for status.artifact.digest, so a tap artifact is never materialized without a verified digest. - Centralize the load-bearing community. prefix in internal/marketplace/tapconst (removing two duplicate consts and several literals across the CLI, API, materializer, and dashboard) so the disconnect guard, the community badge, and the anti-shadowing guarantee cannot drift. - Split multi-document YAML with the apimachinery reader so a PackageSource whose values contain a separator-looking line is not fractured. (The short-name version-pin drop was already fixed in an earlier commit.) Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
3ce8279019 |
fix(marketplace): prune stale taps, preserve source state, scope RBAC, pull by digest
Address the remaining review findings: - The materializer now prunes PackageSources it materialized from an earlier revision that the current artifact no longer contains (including a single/multi rename), so a removed package leaves the catalog; installed Packages stay. - A repeat connect updates the fetched OCIRepository's spec, label and name annotation instead of replacing the object, so the operator's finalizer and materialized-revision annotation survive; covered by a test. - OCIRepository write verbs move from the cluster-wide cozystack-api ClusterRole to a namespaced Role in cozy-system (get/list/watch stay cluster-wide for the watch cache), since the API only manages tap sources there. - 'cozypkg validate --require-signature' now pulls the exact manifest digest cosign verified, closing the tag-mutation window between verify and pull. - safeJoin rejects any '..' path segment outright (defense in depth and to satisfy the Zip-Slip scanner) on top of the existing containment check. - Document why the index gate's merge/label steps need a workflow_run split for fork pull requests. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
d5891f7093 |
fix(marketplace): close gate rename bypass, label CLI tap sources, reject artifact-name collisions
Address review findings: - The index gate listed only added/modified entries (--diff-filter=AM), so a renamed entry file (R) skipped validation and signature checks and could auto-merge an unreviewed ociRef. Include renames and copies (ACMR). - 'cozypkg tap' created its OCIRepository without the marketplace-tap label or tap-name annotation, so the dashboard's orphan-disconnect could not find a CLI-created source. Set both, aligned with the tap's community name. - cozypkg validate now rejects two components whose artifact names collide after dot-to-dash normalisation (e.g. v1.0/c and v1/0-c), which would otherwise make the reconciler overwrite one OutputArtifact with the other. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
7b703f2859 |
fix(cozypkg): satisfy golangci-lint and derive a valid kind for hyphenated apps
Address the linters flagged in review: check the previously-ignored writer, Close, and RemoveAll return values (errcheck), lowercase an error string (ST1005), drop the embedded-field selector (QF1008), remove an unused const, and switch the materializer's post-finalizer requeue from the deprecated Result.Requeue to RequeueAfter. Also fix 'cozypkg init': capitalize now builds an upper CamelCase kind from a hyphenated --app (foo-bar -> FooBar) so the scaffolded ApplicationDefinition is not rejected by the apiserver at install time; covered by a test. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
f88d5c13c2 |
feat(cozypkg): confirm privileged components on add unless --allow-privileged
'cozypkg add' now lists the privileged components of the selected variant and requires an explicit confirmation (defaulting to No) before installing them, unless --allow-privileged is passed. This is the install-time privileged confirmation #23 specifies; the badge and validate warning already existed. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
35e54fb49e |
feat(dashboard): search and trust notice on the Repositories view
Add a search box that filters connected repositories and their packages by name, kind, description, category, or tag, and a trust notice stating that connecting a repository runs its charts in the management cluster and that signatures are verified by the index at publication time, not at connect time. Both close gaps the design-conformance review found against #18/#23. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
559c050de0 |
fix(cozypkg): pin tap short-name to the index version and harden the gate
Conformance review against the design proposals surfaced that a short-name tap ignored the index entry's recorded version: resolveTapTarget returned the bare ociRef, so 'cozypkg tap <name>' pulled :latest while the CI gate had validated and cosign-verified <ociRef>:<version>. The gate signed one tag and the cluster pulled another, breaking both the versioning model (#18) and the signature chain (#23). Resolve the short name to <ociRef>:<version> when a version is recorded. Also harden the publication gate: run 'cozypkg validate --helm-lint' (with helm installed in the workflow) so charts are linted as the design specifies, and add a CODEOWNERS so every index entry requires maintainer review. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
3643fceceb |
refactor(marketplace): fail-safe re-tap rollback and single-source artifact naming
Close the two non-blocking notes from the third review round: - objectExists now fails safe: it returns 'not existing' (eligible for rollback) only on a definite NotFound; any other Get error (RBAC, throttling, timeout) is treated as possibly-pre-existing, so an idempotent re-tap that hits a transient probe error cannot delete a pre-existing resource on rollback. Mirrors the fail-direction of sourceStillReferenced; covered by a new interceptor-based test. - The PackageSource reconciler now calls internal/marketplace/naming.ArtifactName instead of inlining the formula, so the artifact-name convention has a single definition shared by the producer, the marketplace backend, and the validator, and the parity test guards producer-vs-helper. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
1fbceee10e |
fix(marketplace): operator RBAC for tap materialization and dot-safe artifact naming
Second review round found the write-path RBAC fix was only half applied and the extracted naming helper diverged from its producer: - The tap materializer runs as the cozystack-controller ServiceAccount, whose ClusterRole granted source.toolkit.fluxcd.io/ocirepositories only get/list/watch. Its finalizer and materialized-revision writes (r.Update on the OCIRepository) were therefore Forbidden, so a dashboard-connected tap never materialized. Grant the controller update/patch on ocirepositories. - naming.ArtifactName replaced dots only in the PackageSource name, but the reconciler replaces them in the variant and component too, so a tapped repo with a dotted variant or component name dropped its packages from the catalog and tripped a false appdef-dangling warning. Replace dots in all three segments and add a direct parity test with dotted names. - Pin cosign-installer and the cosign release in the index gate so the signature verifier is reproducible. - Add a unit test for the tap rollback objectExists predicate. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
be03900262 |
refactor(marketplace): address review — shared naming, tap rollback, pinned gate, trust docs
- Extract the per-component artifact-name convention into internal/marketplace/naming and have the validator and the Tap backend delegate to it, so the component-to-ApplicationDefinition linkage cannot drift across copies. - cozypkg tap only rolls back resources it actually created, so an idempotent re-tap no longer deletes a pre-existing OCIRepository/PackageSource when a later apply fails. - Pin cozypkg and yq in the index publication gate so the auto-merge decision runs on a reproducible toolchain. - Document the trust model explicitly: tap validates structure but does not verify signatures; verification is enforced by the index CI gate (pinned to the recorded cosign identity) and optionally by Flux OCIRepository verification at pull time. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
52de2e460e |
fix(cozystack-api): grant Tap write-path RBAC and recover pending taps
The marketplace Tap create/delete run with the cozystack-api ServiceAccount, which was granted only get/watch/list on cozystack.io and nothing on source.toolkit.fluxcd.io, so connecting or disconnecting a tap returned Forbidden on every cluster. Grant the ServiceAccount delete on cozystack.io/packagesources and full management of source.toolkit.fluxcd.io/ocirepositories. Also make a pending or failed connect recoverable: when a tap has an OCIRepository but no materialized PackageSource yet, Delete now falls back to removing the labeled OCIRepository matched by its tap-name annotation, instead of returning NotFound and orphaning a finalized source. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
2cedd76fb9 |
feat(dashboard): add marketplace Repositories view backed by the Tap resource
Add a Repositories view to the console marketplace that lists connected External-Apps repositories from the taps.core.cozystack.io resource, showing each tap's source, community/official and ready status, and the packages it exposes with a privileged badge. Operators can connect a repository by oci:// reference (with an optional pull secret for private repositories) and disconnect community taps, backed by create and delete on the Tap resource; installed packages are left in place on disconnect. The view reuses the existing k8s-client list/watch and mutation hooks, so it updates live, and adds a Repositories entry to the marketplace sidebar. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
11e2f07fae |
feat(packages-index): add community index gate template with two-lane merge
Add a template for the standalone cozystack/packages-index repository: the metadata-only entry format (with a version and a recorded cosign signing identity), an example entry, and the publication gate. The gate script validates every changed entry with 'cozypkg validate --require-signature' against the entry's recorded identity and classifies the change into the auto-merge lane (a pure version bump of an already-listed entry) or the maintainer-review lane (a new entry or a change to ociRef, maintainer, or signing). It fails closed: a validation error, a parse error, a signature failure, or a security-relevant edit never auto-merges. A companion GitHub Actions workflow runs the gate and enables auto-merge or requests review accordingly. Add a version field to the index entry so a bump is detectable. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
66736fc2a0 |
feat(cozypkg): implement keyless cosign verification for validate
Replace the --require-signature stub with real keyless cosign verification: for an oci:// target it verifies the artifact against the expected certificate identity and OIDC issuer (--certificate-identity / --certificate-oidc-issuer) before pulling, shelling out to cosign with a bounded timeout. This is the trust anchor the community index CI gate relies on, so a version bump must remain signed by the entry's recorded identity. Verification fails closed when cosign is absent or the identity/issuer are not supplied. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |