mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 05:25:14 +09:00
a6caad7c577778ceee50ff1d117c7a7658b23c19
6189
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
a6caad7c57 |
fix(platform): correct what disabledPackages does to ouroboros (#4098)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does `bundles.disabledPackages` does not uninstall anything. Two comments in the platform chart said it does, offered next to `kubectl delete package.cozystack.io cozystack.ouroboros` as if the two were interchangeable. A name in that list only stops `_helpers.tpl` emitting the Package document, which is the same suppression you already get from leaving `publishing.proxyProtocol` false. Every emitted Package carries `helm.sh/resource-policy: keep`, and that annotation is there precisely so disabling a package stops deleting it, so Helm leaves the Package CR alone. The CR stays, the HelmRelease it owns stays, ouroboros keeps running, and the pre-delete hook never fires because nothing uninstalls the release. Deleting the Package CR is what gets there: GC takes the HelmRelease, Flux uninstalls, the hook runs on the way out. There was a second cost. The guard in the system bundle looks up the live Package CR and never reads `disabledPackages`, so an operator who followed the old advice hit the guard anyway with no idea why. New test case pins that. Comments only in the templates, so rendered output is unchanged. Chart suite green at 156 tests. ### Screenshots Not a UI change. <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> Walked the trigger map against the diff. One repo is reached, and I left its box empty rather than tick it without a link. `cozystack/website` has the same wrong instruction verbatim in three files: `content/en/docs/next/networking/hairpin-proxy-protocol.md:122`, and the identical line under `v1.4/` and `v1.5/`. It tells the reader that removing the Package CR "(or add it to `bundles.disabledPackages`)" triggers helm uninstall and fires the pre-delete hook. It also contradicts `operations/configuration/components.md:72` on the same site, which says `disabledPackages` will not remove components that are already installed. The site fix is cozystack/website#683. It turned out to be four files rather than three: `origin/main` there also carries a `v1.6` tree, and all four copies were byte-identical. Nothing else is reached: no package added or renamed, no `values.schema.json`, no version enum, no default, no key the Ansible role sets, nothing under `hack/`. - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: https://github.com/cozystack/website/pull/683 - [ ] [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 <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(platform): the platform chart no longer says `bundles.disabledPackages` uninstalls a package. A name in that list only stops the Package document being rendered. `helm.sh/resource-policy: keep` leaves the existing Package CR on the cluster and the component keeps running, so deleting the Package CR is what uninstalls it. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Clarified proxy protocol safety guidance, including the effects of disabling the Ouroboros package and the limitations of using `bundles.disabledPackages`. - Documented that disabling package rendering does not remove an existing live package or stop its associated resources. - **Tests** - Added regression coverage confirming that disabling the package does not bypass the proxy protocol safety guard when the live package remains active. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
1fc990dd7f |
fix(cdi): update CDI to v1.66.1 (#3920)
## What this PR does CDI v1.64.0 -> v1.66.1, operator manifest refetched with `make update`. The bump was stuck since v1.65.0: its bundled qemu-img broke HTTP imports to block volumes on 4Kn devices (DRBD/LINSTOR). This PR first targeted v1.66.0 on the assumption that kubevirt/containerized-data-importer#4179 had closed it, and that was wrong: the qemu-img in the v1.66.0 importer is 10.1.0-21.el9, which does not carry the first-sector fix, and the E2E run on that head failed exactly there. The fix is in 10.1.0-24.el9, which reached the release branch through kubevirt/containerized-data-importer#4274 and shipped in v1.66.1. Verified by running the released image: `cdi-importer:v1.66.1` reports `qemu-img version 10.1.0 (qemu-kvm-10.1.0-24.el9)`. The other v1.64.0 symptom is a resourceVersion storm on the prime PVC during WFFC imports, which makes the scheduler retry FailedScheduling dozens of times per import; it was reported by a production user importing golden images on a local StorageClass. That one is fixed by commit e547586e9, a one-line guard in `updatePvcFromPod` that clears the bound-condition annotations only when they came from scratch-space creation. It is in v1.66.0 and later. The commit is squashed under a title about kubevirtci lanes, so it appears in no release note and searching for it by symptom finds nothing. Two local edits to the vendored manifest survive refetches now. The startupProbe patch gets the correct -p4 strip level, as written it never applied from the package directory. The control-plane tolerations were hand-edited into the manifest with no patch behind them, so any refetch silently dropped them; they live in patches/tolerations.diff now. `make update` round-trips to a byte-identical file. Behavior changes checked against this repo: /metrics on CDI components now requires a bearer token, nothing here scrapes it. Scratch PVCs inherit the target StorageClass instead of the cluster default (since v1.65.0). The PVC-rendering webhook is always registered now, but only matches PVCs labeled cdi.kubevirt.io/applyStorageProfile=true, which nothing here sets. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note fix(cdi): update CDI to v1.66.1. Fixes HTTP imports to block volumes on 4Kn devices (DRBD/LINSTOR) with the repaired qemu-img, and the resourceVersion storm on prime PVCs during WaitForFirstConsumer imports. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Kubernetes tolerations for critical add-on and control-plane nodes. * Added resource claim requests and webhook PVC rendering configuration to CDI custom resources. * Added permissions for token reviews, metrics, and profiling endpoints. * Updated affinity feature-gate documentation to beta status. * **Improvements** * Updated CDI operator and operand images to version 1.66.1. * Refreshed Kubernetes schema metadata and deployment configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
d0fce925cf |
fix(platform): declare the injected _cluster values in system charts (#4102)
## What this PR does
Ten charts under `packages/system` read `_cluster`, and `bucket` and
`monitoring` also read `_namespace`. The platform injects both through
the `cozystack-values` Secret, so installs never notice. Rendering a
chart on its own does: `helm template packages/system/kubevirt` dies on
`index of untyped nil` before it emits anything, and the `_namespace`
readers die on `nil pointer evaluating interface {}.host`.
Each of the ten now declares the keys it reads, with an empty default.
Injected values merge over chart defaults, so an installed release
renders exactly what it rendered before. Checked per chart against a
realistic injected `_cluster`/`_namespace`: six came out byte-identical,
and the other four differ only in `randAlphaNum` secrets, which also
differ between two renders of the same tree.
The declaration on its own is not worth much, so every chart also gets a
suite that renders with nothing injected. The existing suites all set
`_cluster` themselves and cannot see whether `values.yaml` still
declares it. Drop a declaration and exactly that chart's suite goes red,
with the error its comment quotes; I ran that for all twelve. None of
the new suites carries a suite-level `templates:` list, on purpose: that
list narrows what helm-unittest renders, and would hide the same
breakage in a template added later.
`kubevirt-cdi` had no `tests/` and no `test:` target, so it gets both.
`etcd-operator` reads `_cluster` too and stays as it is, since its one
call site already guards with `| default dict`. `cozystack-basics`
already declared the key.
Closes #4030.
### Not in this PR
No CI job runs `helm lint` or a standalone `helm template`, which is why
none of this was red. A gate is worth proposing separately. Its value is
narrow though: `_cluster` is always populated on a real install, and
eight of these ten already had a suite rendering the affected template
with `_cluster` supplied. The two with nothing were `kubevirt`, whose
`vm-exportproxy-*` templates no test touched, and `kubevirt-cdi`, which
had no tests at all.
`helm lint` still exits 1 on `keycloak` and `monitoring` with `chart
metadata is missing these dependencies: cozy-lib`. Both carry a
`charts/cozy-lib` symlink that `Chart.yaml` never declares. That one is
not a two-chart item: 82 charts across
`packages/{system,apps,extra,core}` fail lint the same way, which is
invisible because no job runs `helm lint`. Separate defect, untouched
here.
Fifteen charts outside `packages/system` break the same way:
`apps/{bucket,foundationdb,harbor,kubernetes,mongodb,nats,opensearch,postgres,vpn}`
and `extra/{bootbox,etcd,gateway,info,ingress,seaweedfs}`. `cozy-lib`'s
own helpers already guard every access with `| default dict`, so routing
those reads through the library would close the class instead of
patching fifteen more values files.
`kubevirt`'s suite only pins that the routes stay absent with nothing
injected. Nothing pins that they show up once `_cluster` selects the
service.
### Screenshots
No UI change: the diff is ten `values.yaml` files, ten helm-unittest
suites, and one package `Makefile`.
### Downstream repositories
Walked the trigger map in `docs/agents/contributing.md` file by file
against this diff. No package is added, renamed or removed under
`packages/apps` or `packages/extra`;
`packages/core/platform/values.yaml` is untouched; no variant, bundle,
component, release asset, namespace or `ApplicationDefinition` semantic
changes; no `values.schema.json` and no version enum moves, so the
provider's hand-written schemas and defaults are unaffected; nothing
under `hack/` moves and no existing make target changes behaviour. The
one `Makefile` edit adds a `test:` target to
`packages/system/kubevirt-cdi`, matching the target the other nine
already have, so `make generate` and the ccp skills that drive it see
nothing new.
- [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
fix(platform): declare the platform-injected `_cluster` and `_namespace` values in the ten system charts that read them, so each chart renders with `helm template` outside a cluster instead of failing on a nil map. No change to what an installed release renders.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Bug Fixes**
- Improved standalone Helm chart rendering when platform-provided values
are unavailable.
- Prevented rendering failures across system charts, including bucket,
dashboard, Keycloak, monitoring, KubeVirt, and related components.
- **Tests**
- Added comprehensive standalone rendering coverage for system charts.
- Added validation for expected ingress, gateway, TLS route, and
resource output under different configuration scenarios.
- Added a chart test command for KubeVirt CDI.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
Aleksei Sviridkin
|
675163399d |
fix(platform): accept the container gpu-operator variant in iaas (#4101)
## What this PR does `bundles.iaas.gpuOperatorVariant: container` failed the whole platform render. The `cozystack.gpu-operator` PackageSource ships that variant, `values-container.yaml` configures it deliberately, and `hack/check-gpu-operator-variants.bats` covers it, but the `iaas` bundle guard only accepted `default` and `vgpu`. The exclusion made sense when it was written and stopped making sense a few minutes later. The variant and the guard came from parallel branches, neither an ancestor of the other, variant merged first. Before the guard the bundle hardcoded `default`, so `container` was selected by hand-writing a Package CR, and leaving it out of a bundle-side list cost nothing. The guard made the bundle the variant owner and closed that route without noticing it was one. Its comment calls itself typo protection and names `passthrough` as the example typo. Allowing the value is not the whole fix. The same file puts `extraFeatureGates: [HostDevices]` on the KubeVirt CR whenever gpu-operator is enabled, and builds a `permittedHostDevices` table under it. Both are VM-facing. The container variant keeps the host driver bound and pins `vfioManager` off, so no GPU reaches a VM: the gate would advertise something the node cannot serve, and the table would name resources no sandbox device plugin registers there. Both now render for `default` and `vgpu` only, and `container` gets the gpu-operator Package with the kubevirt component untouched. The helm-unittest case that pinned the rejection is gone. Two replace it: the Package renders at the container variant with no component overrides, and the KubeVirt wiring stays absent even with `gpu.permittedHostDevices` and `gpu.mediatedDevicesConfiguration` set. Comments and docs that called the mirroring unconditional on variant, or the container variant unmerged, are fixed in the same pass. No issue for this one, so the history is in the body. ### Screenshots No UI change. ### Downstream repositories Walked the trigger map against the diff. The only file it touches that appears there is `packages/core/platform/values.yaml`, and the website hit is not the hand-written key table (that page has no GPU section): it is the documented workaround this change makes obsolete. `content/en/docs/next/operations/gpu-container-workloads.md` currently tells readers the bundle rejects `container` and routes them to a hand-crafted Package CR, and `virtualization/gpu.md` / `vgpu.md` describe the KubeVirt mirroring as unconditional on variant. Filed as an issue rather than a PR because the docs must not land ahead of this change, and the `v1.6` copies describe released behaviour and stay correct as they are. Nothing else matches: no package added, renamed or removed, no `values.schema.json` or version enum touched, no `ApplicationDefinition` semantics, no installer values key, no `hack/` layout change, no platform variant or bundle recomposed. `gpuOperatorVariant` appears in none of `ccp`, `ansible-cozystack` or `terraform-provider-cozystack`. - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: https://github.com/cozystack/website/issues/685 - [ ] [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 fix(platform): accept `container` as a value for `bundles.iaas.gpuOperatorVariant`, so the gpu-operator variant the platform already ships can be selected from platform values instead of a hand-crafted Package CR. The KubeVirt `HostDevices` feature gate and `permittedHostDevices` table are now emitted for the `default` and `vgpu` variants only; `container` serves GPUs to pods and gets neither. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added the `container` GPU operator variant for workloads that do not require VM passthrough. * Container workloads no longer receive KubeVirt host-device or mediated-device GPU wiring. * Clarified GPU configuration behavior for passthrough, vGPU, SR-IOV, and container workloads. * **Documentation** * Updated passthrough guidance, configuration references, examples, and KubeVirt wiring instructions for all supported variants. * Added details for manually configured package resources and platform-specific container setup. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
b7e60855e9 |
test(tests): wire the gpu-operator suite into the unit sweep (#4110)
<!-- Thank you for making a contribution! Here are some tips for you:
- Use Conventional Commits for the PR title: `type(scope): description`
- Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore
- Scopes are not an exhaustive list — pick the most specific scope for
the change and extend the list when a genuinely new area appears.
Examples:
- System components: dashboard, platform, operator, cilium, kube-ovn,
linstor, fluxcd, cluster-api
- Managed apps: postgres, mariadb, redis, kafka, clickhouse,
virtual-machine, kubernetes
- Development and maintenance: api, hack, tests, ci, docs, maintenance
- Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or
add a `BREAKING CHANGE:` footer
- If it's a work in progress, consider creating this PR as a draft.
- Don't hesistate to ask for opinion and review in the community chats,
even if it's still a draft.
- Add the label `kind/backport` if it's a bugfix that needs to be
backported to a previous version.
-->
## What this PR does
`hack/helm-unit-tests.sh` decides whether to enter a package by asking
`make -C <pkg> -n test`. `packages/system/gpu-operator` declared only
`update:`, so the sweep skipped it silently and the three test cases in
`tests/kernel_module_config_test.yaml` had never run. They pass, so
nothing was hiding behind them. Still worth fixing: the directory reads
as coverage to whoever opens it, and a suite nothing runs is where a
stale assertion sits.
The target is phony. `hack/package.mk`'s `.PHONY` line lists only the
targets that file defines, so a path named `test` next to the Makefile
would make the target up to date and skip the recipe.
Rather than fix the one package I enumerated the rest of the tree. 84
packages hold a `tests/` directory, 83 ship at least one
`tests/*_test.yaml`, and gpu-operator was the only one of those without
the target. The 84th is `packages/system/nfs-driver`, whose `tests/`
holds a PersistentVolumeClaim and a StorageClass to apply by hand. No
suite there, so a `test:` target would only make the sweep fail it for
reporting zero suites. Left alone.
`hack/package-test-target-coverage.bats` pins the class. Every other
guard in the sweep runs inside a directory it already reached, so a
package with no target escapes all of them at once and the run still
reports success. The new case probes with `make -n` instead of grepping
for `test:`, which is the question the sweep asks, and it mirrors the
sweep's other precondition too: the sweep requires `$dir/Makefile` and
returns before probing, where make would also accept `GNUmakefile` or no
makefile at all. The verdict comes from the sweep's command unchanged,
and a failing package is probed a second time with `LC_ALL=C
--no-print-directory` purely to read the diagnostic, since both of those
are visible to a Makefile through `LC_ALL` and `MAKEFLAGS`.
Mutations, because a guard that has never been red proves nothing:
```text
target removed from gpu-operator -> red, names packages/system/gpu-operator, quotes make's error
Makefile renamed to GNUmakefile -> red; make still answers 0 here, so this leg is load-bearing
glob pointed at a missing dir -> red via the blindness leg, instead of passing on an empty set
unmodified tree -> green, 83 packages scanned
```
The full `hack/helm-unit-tests.sh` sweep now enters gpu-operator and
finishes green.
One detail that is invisible on a Mac: the probe runs under
`--no-print-directory`. On modern GNU make `-C` turns on `-w`, and the
fatal path prints `Leaving directory` after the error, so `tail -n 1`
picks up the directory trace and drops the diagnostic. Measured on 4.4.1
where that happens, and on 3.81 (still `/usr/bin/make` on macOS) where
`-C` prints no trace at all.
### Screenshots
No UI changes.
### Downstream repositories
<!--
Cozystack is upstream for repositories that are NOT kept in sync with it
automatically, and nothing in CI compares the two sides, so a change
here can break them silently.
Which change forces what, and which file to touch, is the trigger map in
docs/agents/contributing.md under "Downstream Repositories". Walk it
before you tick anything.
IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM
TO FILL IN:
1. Walk the trigger map against the actual diff of this PR, file by
file. Do not decide from the PR title or from your memory of what you
changed.
2. For every repository you tick, open the follow-up PR there and link
it on the same line. A ticked box with no link claims work that does not
exist, which is worse than an empty box. Search that repository for an
open PR or issue covering it first, and link that instead of filing a
duplicate.
3. When the follow-up is out of scope, or needs a decision that is not
yours to make, open an issue in that repository instead, link that, and
say so in the PR body.
4. Do not tick "No downstream repository is affected" to make the
checklist go away. If you are unsure, leave every box empty and say so
in the PR body, so a human decides.
-->
Walked the trigger map against both changed files. Two entries come
close and both miss. `ccp` gates on `hack/package.mk`,
`hack/common-envs.mk` and the `packages/{apps,system,extra,core}`
layout; nothing here is moved or renamed and no existing target changes
what it does, so a new `test:` target sitting beside 80 identical ones
is not something its skills read. The `website` entry for developer
tooling points at `content/en/docs/next/development.md`, whose Testing
section covers the e2e sandbox under `packages/core/testing` and says
nothing about the per-package helm-unittest sweep.
- [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
test(tests): run the gpu-operator helm-unittest suite, which the sweep had been skipping because the package declared no `test:` target
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Tests**
- Added coverage checks to verify that all Helm test suites are included
in the package test sweep.
- Added diagnostics for packages missing a runnable test target.
- **Bug Fixes**
- Added a test target to the GPU operator package, enabling its Helm
tests to run through the standard package test workflow.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
Andrei Kvapil
|
d3559d48ed |
feat(migration): tenant self-service VM import from VMware vSphere (#3931)
## What this PR does Adds tenant self-service import of virtual machines from VMware vSphere, as a pair of CRDs in a new `migration.cozystack.io` group reconciled by a controller in core. Konveyor Forklift is the transfer engine underneath; tenants never see it. A `VMImportSource` registers a long-lived connection to a provider — type, endpoint, credentials entered on the spec and materialized into a Secret by the controller, because tenants cannot create Secrets in Cozystack. A `VMImportTask` runs the one-shot operation: reference a source, name the machines, get `VMDisk`s and `VMInstance`s. The outputs carry no owner reference back to the task, so deleting a finished import removes the migration machinery and nothing else. This supersedes the chart-based tenant API in #1982 while keeping its Forklift packages unchanged, carried over here with attribution. The design is written up in cozystack/community#62, which records why a Helm chart lost to a controller: `apps.cozystack.io` is a projection over HelmReleases, so "the outputs survive deletion" would need `helm.sh/resource-policy: keep`; a completed migration is terminal while Flux reconciles toward a steady state forever; and the number and size of the disks an import produces is only known after the source VM has been inspected, which a template cannot express. Two things are worth calling out for review. **The controller ships no vSphere client.** Forklift already authenticates, validates VM references and reports unmapped networks and datastores, and the KubeVirt VirtualMachine it builds carries the source VM's CPU, memory and disk order — so the inventory is read from Forklift's own objects rather than from a second client that could disagree with the one doing the work. A tenant sees Forklift's verdicts verbatim rather than a paraphrase. **The volume handoff copies nothing.** The transferred PersistentVolume is retained and re-bound into the claim a `VMDisk` expects, with `cdi.kubevirt.io/storage.populatedFor` telling CDI the claim is already populated — the same primitive the Velero VM-restore path already depends on in production. That closes both the raw-PVC gap raised in review on #1982 and the duplicate-copy waste tracked in #3924. Everything is opt-in: three packages added to `bundles.enabledPackages`, and one platform value (`vmImport.vddkImage`) naming the VDDK image, which Cozystack cannot ship because it is built from a proprietary VMware SDK. Empty is a supported state and makes a vSphere source report itself unavailable at registration rather than mid-transfer. ### Validation Run on a live Cozystack v1.6.2 cluster and rolled back afterwards. Confirmed working end to end: CRD install and printer columns, the VDDK policy (including that no credentials Secret is projected for a source that cannot work), credential projection with the ownership guard, the Provider pair with the operator's image stamped on it, the local-cluster destination Provider reaching Ready/Connected/Inventory, Forklift's connection verdict mirrored onto the source verbatim, the diagnostic chain from Forklift through the source to the task, that nothing is created while a source is unready, and that deleting a source garbage-collects both Providers and the Secret. That testing found four defects, none of which were visible from reading code, and all are fixed here: the task controller could not start at all when Forklift is absent (a watch on an absent CRD never syncs, so controller-runtime refused to start it and the pod sat at 0/1); a vSphere secret needs a CA certificate rather than a thumbprint, so as first modelled every source would have sat at `SecretNotValid` forever with a reason that reads like a credentials problem; Forklift's absence was returned as a reconcile error, so a source kept a stale condition indefinitely while the controller hot-looped; and the controller's own Deployment violated the restricted Pod Security Standard, which is indefensible for a feature whose design argument is that an import stays inside it. A further four assumptions were corrected against operational experience from the original implementation's work on a real vSphere environment, which this session had no access to. Each failed silently: the VirtualMachine Forklift creates is labelled with the Plan's **UID**, not its name, so the disk would never have been adopted; the transfer arrives as a CDI-populated claim with no DataVolume, carrying an owner reference to that VirtualMachine, and clearing it was conditional on a DataVolume existing — on the original implementation garbage collection removed such a claim 357ms after the VM went, destroying a successfully migrated disk; the terminal signal is the phase the engine settles on, not only a completion timestamp; and progress read from a name-matched pipeline step pegs at zero if a release renames it. **What could not be tested:** everything past authenticating to vCenter — inventory resolution, the loop that learns network and datastore mappings from Forklift's validation conditions, the transfer itself, and this controller's own code path through the handoff. No vCenter was available. The handoff mechanics were separately proven by hand on the same cluster in earlier work; what remains unproven is this implementation of them. The mapping loop is the least-supported part of the change and is the thing to look at hardest. ### Screenshots https://github.com/user-attachments/assets/f313f6ba-8e72-42d9-b65a-033866ad39d5 ### Downstream repositories Walked the trigger map against the diff. `cozystack/website` is reached on two counts and the follow-up is open as [cozystack/website#673](https://github.com/cozystack/website/pull/673), a migration guide covering the pre-flight checks, the administrator's VDDK build, and the driver caveat the lab run surfaced: - `packages/core/platform/values.yaml` gains `vmImport.vddkImage`, and `content/en/docs/next/operations/configuration/platform-package.md` is a hand-written table of those keys. - Three platform components are added, which reaches `guides/platform-stack/_index.md` and `operations/configuration/licenses.md` — the latter is where the proprietary VDDK image belongs, since Cozystack references it without shipping it. The others were checked and are not affected: no package is added under `packages/apps/` or `packages/extra/` (terraform-provider, website app lists), no key the Ansible role sets changes, nothing under `hack/` is moved or renamed and no make target changes behaviour (ccp, external-apps-example), and no node prerequisite changes (talm). - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: https://github.com/cozystack/website/pull/673 - [ ] [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(migration): import virtual machines from VMware vSphere into a tenant. A VMImportSource registers a connection to vCenter and a VMImportTask imports named machines, producing ordinary VMDisks and VMInstances that survive deletion of the import. Opt-in: add cozystack.forklift-operator, cozystack.forklift and cozystack.migration-controller to bundles.enabledPackages, and set vmImport.vddkImage to a VMware VDDK image you build yourself — Cozystack cannot redistribute it. Guest conversion and warm migration are not included in this version. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added vSphere VM migration support, including source connections, import tasks, inventory discovery, progress tracking, and conversion into Cozystack VMs and disks. * Added dashboard pages for managing migration sources and imports, with source-dependent VM selection and progress details. * Added optional VM firmware settings for BIOS/UEFI, Secure Boot, and persistent EFI storage. * Added opt-in platform packages for migration services and Forklift integration. * **Security** * Restricted access to the ingress admission webhook port with a network policy. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrei Kvapil
|
6384c8f4d1 |
feat(ingress-nginx): restrict the admission webhook to the API server
The admission webhook only ever needs to be reachable by the API server, and leaving it open to every pod turns any flaw in it into something any workload in any namespace can reach -- which is what made CVE-2025-1974 critical rather than merely serious. The policy is deny-only and explicitly disables default-deny: a Cilium rule carrying an ingress section would otherwise put the controller behind one, meaning every port it serves would have to be enumerated and the first one forgotten would break silently. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
54621bc046 |
feat(migration): tenant self-service VM import from VMware vSphere
Hidora's customers are leaving VMware now, and a tenant has to be able to import their own machines without a platform administrator. The chart approach could not express that: a tenant can neither create the Secret holding vCenter credentials nor name the VDDK image, so both had to come from somewhere they cannot reach. Two CRDs in a new `forklift.cozystack.io` group, split by lifecycle. A VMImportSource is a long-lived connection -- type, endpoint, credentials, readiness -- and a VMImportTask is the one-shot operation that names machines and produces VMDisks and VMInstances. The outputs carry no owner reference back to the task, so deleting a finished import leaves the machines alone. Konveyor Forklift does the transfer underneath; the controller renders its objects, mirrors its verdicts, and hands each finished volume into a VMDisk without copying it. What a live vCenter migration forced out, and no amount of source reading would have: - A guest copied verbatim keeps the drivers it had under VMware and no virtio, so on a virtio disk it does not fail the import -- it succeeds and then does not boot. Imported disks go on SATA. - VDDK connects straight to the ESXi host at the address vCenter advertises, which is routinely unreachable, and on the first cluster collided with the Service CIDR. `spec.hosts` redirects per host. - Firmware must be carried across or a UEFI guest imports as BIOS and never boots (stacked on #3002). - Deleting a task mid-transfer orphaned its DataVolume; a finalizer now removes the engine's volumes and leaves the outputs. The console gains a Migration section: connections are editable, and the VM field is a picker built from a machine list the controller publishes, since the aggregated API cannot reach the inventory and should not hold a source's credentials to try. Two general mechanisms fall out of that -- an option source may take an argument, and one named in a CRD annotation may reference a sibling field. Design: cozystack/community#62. Docs: cozystack/website#673. 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> |
||
|
IvanHunters
|
e61d2c7b53 |
feat(marketplace): connect a community tap from the API with async materialization
Add server-side connect to complete the marketplace write path. Creating a Tap records the intent only: the API validates the oci:// reference and creates a labeled Flux OCIRepository (with an optional pull-credential secretRef), never blocking the request on a registry pull, and returns a Tap whose status shows materialization pending. The connect inputs (url, tag, secretRef) are write-only and are never echoed back or returned on read. A new tap materializer reconciler in the operator watches community-tap OCIRepositories, and once source-controller has pulled the artifact, fetches the tarball over the Flux artifact URL (digest-verified, size-bounded, path-traversal guarded), parses the PackageSource(s) it carries, and materializes them under the community. prefix with their sourceRef repointed at the tap's source. A finalizer cleans up the materialized PackageSources when the source is removed, leaving installed Packages in place. The shared label/annotation keys live in internal/marketplace/tapconst so the API and the operator cannot drift on the connect-to-materialize handoff. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
c8fd40c419 |
feat(cozystack-api): support disconnecting a community tap via the Tap resource
Implement Delete on the Tap resource so the dashboard can disconnect a community tap, mirroring 'cozypkg untap'. It refuses official (non-community.*) sources, removes the PackageSource, and removes the backing Flux OCIRepository only when no other PackageSource still references it. Installed Packages are left untouched. The delete verb is not granted to the tenant catalog-read roles, so disconnecting stays cluster-admin only through the delegated RBAC. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
5848d85840 |
feat(cozystack-api): add read-only Tap resource for the marketplace view
Add a cluster-scoped, read-only Tap resource to the core.cozystack.io API group that powers the dashboard marketplace view. Each Tap is computed on read from a PackageSource and the ApplicationDefinitions attributable to it (matched by the component's assembled-artifact name), exposing the connected repository, whether it is a community tap, its readiness, and the packages it exposes with their catalog metadata. It is served through the existing aggregation layer, so the dashboard consumes it with the same list/watch client it already uses, and authorization is the delegated RBAC the apiserver already enforces. Taps carry catalog metadata only and never a pull-credential Secret, so browsing cannot leak a private tap's credentials across the tenant boundary. Read access is granted to the tenant catalog-read roles alongside options. Modeling the marketplace as a virtual resource in an existing API group avoids a new API group and needs no etcd or CRD: the object is computed from cluster state via the apiserver privileged dynamic client, mirroring the options resource. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
8035e5ae70 |
feat(cozypkg): add search command and community index resolution
Add 'cozypkg search' to query the community package index, a directory of metadata-only entries (name, ociRef, description, maintainer, homepage, and an expected cosign signing identity) read from a local path or pulled from an oci:// reference. The index is configured with --index or COZYPKG_INDEX. Index loading is strict: an entry that fails to decode or omits name/ociRef is an error, so a malformed index surfaces rather than dropping entries silently. Wire short-name resolution into 'cozypkg tap': a non-oci:// argument is looked up in the index and resolved to its artifact reference. Bound 'flux pull artifact' with a timeout so an unreachable registry fails with a legible error instead of hanging. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |