Skip to content

fix(capi): ship the core provider manifests the repository actually holds - #4483

Merged
Aleksei Sviridkin (lexfrei) merged 1 commit into
cozystack:mainfrom
themoriarti:proxmox/capi-core-components
Sep 25, 2026
Merged

Aleksei Sviridkin (lexfrei) merged 1 commit into
cozystack:mainfrom
themoriarti:proxmox/capi-core-components

Conversation

@themoriarti

@themoriarti Marian Koreniuk (themoriarti) commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

What this PR does

capi-providers-core ships the Cluster API core manifests twice: as files/core-components.yaml, which a reviewer reads, and as files/components.gz, which the chart mounts into the cluster. #2946 added a startupProbe to capi-controller-manager and edited only the YAML; the blob was never regenerated, so the probe never reached a cluster. This regenerates the blob from the YAML (gzip -9 -n -c files/core-components.yaml > files/components.gz), so the decompressed blob matches the YAML byte for byte, and the only change it carries is the six lines of #2946.

What this reaches, and what it does not

New installs only. capi-operator re-applies a provider when the hash of its spec and its config Secret changes (applied-spec-hash); the ConfigMap holding the components is in neither, so a cluster that already runs capi-providers-core keeps its manager without the probe until the provider's version moves. Whether existing clusters should get the probe, and how, is left open on purpose: moving spec.version and the ConfigMap name together would force the re-apply, but that is a version decision for the package, not a side effect of regenerating a blob.

Also in this diff

  • .helmignore: files/.*-components.yaml is a glob, so it matched only names beginning with a dot, and helm package shipped the readable YAML alongside the blob that duplicates it. Now files/*-components.yaml.
  • hack/capi-core-components_test.bats holds the two files to the same bytes from now on; it goes red on a YAML edited without regenerating the blob.

Found by the drift check written for the Proxmox provider packages in #4087, pointed at the package that was already here; split out because it changes what every new install runs and deserves its own release note.

Testing

bats hack/capi-core-components_test.bats: 1/1; red with a line appended to the YAML, green again after gzip -9 -n -c. helm package of the chart now contains files/components.gz and files/metadata.yaml only.

Screenshots

Not applicable, no UI change.

Downstream repositories

  • No downstream repository is affected by this change

Release note

fix(cluster-api): ship the `startupProbe` from #2946 in the packaged core provider components. New installs get it; a cluster that already runs the core provider keeps its manager without the probe until the provider's version moves, because capi-operator re-applies components only when the provider spec or its config Secret changes.

Summary by CodeRabbit

  • Tests
    • Added a check that the compressed core components match the YAML source, with a diff provided when they differ.
  • Chores
    • Updated chart packaging exclusions to cover component YAML files regardless of whether their names begin with a dot.

…olds

cozystack#2946 added a startupProbe to capi-controller-manager so it survives the
certificate provisioning delay. It edited
capi-providers-core/files/core-components.yaml, which is the copy a reviewer
reads. The chart mounts files/components.gz, and that blob was never
regenerated, so the probe never reached a cluster.

The blob is regenerated from the YAML it is supposed to carry:

  gzip -9 -n -c files/core-components.yaml > files/components.gz

The decompressed blob now matches the YAML byte for byte, and the only change
it carries is the six lines of cozystack#2946.

This reaches new installs only. capi-operator re-applies a provider when the
hash of its spec and its config Secret changes; the ConfigMap holding the
components is in neither, so a cluster that already runs capi-providers-core
keeps its manager without the probe until the provider's version moves.
Whether existing clusters should get the probe, and how, is left open here on
purpose: moving spec.version and the ConfigMap name together would force the
re-apply, but that is a version decision for the package, not a side effect of
regenerating a blob.

.helmignore is corrected in the same change: `files/.*-components.yaml` is a
glob, so it matched only names beginning with a dot, and `helm package`
shipped the readable YAML alongside the blob that duplicates it.

hack/capi-core-components_test.bats holds the two files to the same bytes from
now on, so the next edit of one without the other fails in CI.

Signed-off-by: Marian Koreniuk <[email protected]>
Assisted-by: LLM
@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: cozystack/cozystack/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 243c13d1-3611-45ee-b76c-19b39ef6b914

📥 Commits

Reviewing files that changed from the base of the PR and between b16edec and 9934014.

⛔ Files ignored due to path filters (1)
  • packages/system/capi-providers-core/files/components.gz is excluded by !**/*.gz
📒 Files selected for processing (2)
  • hack/capi-core-components_test.bats
  • packages/system/capi-providers-core/.helmignore

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The chart ignore pattern now matches component YAML files with any filename prefix. A Bats test checks that the decompressed component bundle matches its YAML source byte for byte.

Changes

Core component packaging

Layer / File(s) Summary
Component matching and consistency validation
packages/system/capi-providers-core/.helmignore, hack/capi-core-components_test.bats
The ignore pattern now matches filenames ending in -components.yaml without requiring a leading dot. The Bats test fails if either file is missing or the decompressed bundle differs from the YAML source. It prints regeneration guidance and a diff when contents differ.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 99340

No actionable merge-blocking risk is established for this change; it is ready to merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 99340

The shipped controller gains a startup health check, with no identified new external interface or privilege. The change may not reach existing installations until a later provider update; that rollout distinction should be kept explicit.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated runtime difference is confined to the core controller's startup health behavior when the updated bundle is applied; the evidence does not establish which existing clusters will reapply it.

Trust Boundaries and Controls

  • observed — The apparent public-entrypoint signal belongs to variables in a local Bats test. Its commands read package files and perform a local comparison; they do not invoke a production service or accept a new cluster-facing request.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: shipping the core provider manifests that exist in the repository. It is concise and specific enough for the changeset.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The shipped blob now carries what the YAML says, and I found nothing that blocks.

Marian Koreniuk (@themoriarti) The new components.gz decompresses to exactly core-components.yaml. Against the old blob the only hunk is the six startupProbe lines, and the healthz port the probe uses exists on the manager container. The gzip header has mtime 0 and no file name, and zlib at level 9 reproduces the deflate stream byte for byte. Apple's gzip -9 -n produces different compressed bytes from the same input, which does no harm because the test compares decompressed content.

"New installs only" is correct for the pinned operator version. In cluster-api-operator v0.19.0, calculateHash covers the provider spec and the config Secret, and reconcile returns early when the hash matches. An upgraded cluster gets the new ConfigMap and keeps the old Deployment. The body and the release note both say that.

With the old .helmignore, helm package ships files/core-components.yaml (853 KB) next to the blob. With the new one, files/ holds only components.gz and metadata.yaml, and the rendered ConfigMap still decodes to the YAML.

The test is picked up by make unit-tests through the hack/*.bats wildcard. I put the old blob back and it went red on the startupProbe diff, so it would have caught the original drift.

Two small things, neither blocks:

  1. The bats header comment names #2946. The comment explains the risk without it, and a PR number in a source comment goes stale.
  2. The PR body is wrapped at about 80 columns. AGENTS.md asks for one line per paragraph in PR bodies.

@lexfrei
Aleksei Sviridkin (lexfrei) merged commit e883f6f into cozystack:main Sep 25, 2026
20 checks passed
Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request Sep 25, 2026
…ster IPAM as optional packages (#4087)

## What this PR does

**Series behind #3481** — three separable pull requests:

1. **#4087 — Cluster API providers** (capmox + in-cluster IPAM)
2. #4088 — the `substrate` switch in `kubernetes` and `kubernetes-nodes`
3. #4089 — the Proxmox CSI driver and cloud-controller-manager

#4087 and #4088 share no files and can be reviewed in parallel. #4089
builds on #4088.

---

This is the provisioning layer #3481 asks to specify first: the part
that
creates the VMs. Neither of the other two is needed to review it.

### What it adds

Two Cluster API providers, packaged the way the existing KubeVirt
provider is —
vendored components, `gzip -9 -n` for a reproducible artifact, a `make
components` target that regenerates it from the upstream release and
checks
the downloads against the SHA-256 sums the Makefile pins:

| Package | Version | Contract |
|---|---|---|
| `capi-providers-infraprovider-proxmox` | capmox v0.7.7 | v1beta1 |
| `capi-providers-ipam-in-cluster` | CAIP v1.0.3 | v1beta1 |

capmox v0.7.7 rather than the current v0.9.x on purpose: from v0.8 it
declares
contract `v1beta2`, which the CAPI core shipped here (v1.10.1) does not
serve.

Both join the `iaas` bundle as **opt-in** packages, where the KubeVirt
infrastructure provider is unconditional. The asymmetry is deliberate:
capmox
drives a hypervisor outside the cluster and needs a Secret holding a
Proxmox API
token, which capi-operator reads before it installs anything — with no
Secret the
provider is never deployed at all. Enabling it by default would ask
every
Cozystack install that has no Proxmox to supply hypervisor credentials.

```yaml
bundles:
  enabledPackages:
    - cozystack.capi-provider-infra-proxmox     # brings IPAM with it
    - cozystack.capi-provider-ipam-in-cluster   # also enableable on its own
```

Enabling capmox emits the IPAM provider automatically — capmox allocates
worker
addresses from an `InClusterIPPool` and has no DHCP mode, so on its own
it never
gets a `ProxmoxCluster` to `Ready`. IPAM stays independently enableable,
since
assigning addresses is a generic Cluster API concern.

Naming capmox in `enabledPackages` while disabling IPAM stops the
render. The
cluster would catch that combination on its own — the capmox
`PackageSource`
depends on the IPAM `Package`, so with that `Package` absent capmox goes
`Ready=False` with `DependenciesNotReady` and no `HelmRelease` is
created — but
it would not say why. Failing the render names the two conflicting list
entries
at the moment someone writes them, instead of leaving a condition to
decode.

### Why the earlier attempt did not produce a VM

#1927's blocker — "ProxmoxMachine does not produce a VM" — was not a
capmox
incompatibility. It was four independent causes, each of which only
became
visible once the previous one was cleared:

1. **`PROXMOX_URL` included `/api2/json`.** capmox appends that itself,
so every
   call went to `/api2/json/api2/json/…` and returned 501.
2. **No IPAM provider.** `no matches for kind "InClusterIPPool"`; the
`ProxmoxCluster` never reached `Ready` and every `ProxmoxMachine` sat in
   `WaitingForClusterInfrastructure`.
3. **`templateSelector.matchTags` is set equality, not containment**
(`slices.Equal` after sort). A template carrying four tags is not
selected by
   naming one of them: `found 0 VM templates with tags`.
4. **capmox waits for the qemu-guest-agent, which Talos does not ship.**
   `error waiting for agent: timed out` — cleared with
   `checks.skipQemuGuestAgent` and `skipCloudInitStatus`.

None of these are in the provider. They are packaging and configuration,
which
is why the earlier work could look complete and still never boot a
machine.

### Evidence

Run on Proxmox VE 9.2.11 with Talos v1.13.6 and Cozystack's CAPI stack
(core v1.10.1, CABPT v0.6.12, Kamaji control-plane provider v0.19.0):

```
$ kubectl get proxmoxmachine,machine -n tenant-proxmox
proxmoxmachine/kubernetes-px-md0-…   Ready=True   VMProvisioned=True
machine/kubernetes-px-md0-…          Running      proxmox://0ef88cc1-6aea-4563-af64-344d8060d5c0

$ qm list
106 kubernetes-px-md0-jmzds-h9npx  running
```

The machine boots Talos from the nocloud ISO, reads the machineconfig
capmox
writes, and joins. That is the layer #69 called a hard blocker.

### What this PR does not claim

- One Proxmox node, one tenant cluster, manual runs — there is no e2e
coverage
  for Proxmox in CI, and this PR does not add any.
- It does not make Proxmox a supported platform. It makes the
provisioning layer
reviewable, which #3481 names as the precondition for that conversation.
- It does not touch `capi-providers-core`. The same drift check, pointed
at that
package, shows that the `startupProbe` from #2946 never reached the
shipped
blob; that fix changes what every new install runs and is #4483, with
its
  own release note.
- Nothing here exposes `ProxmoxCluster.spec.credentialsRef` to a tenant,
and
nothing should: `capmox-manager-role` grants get/list/watch/patch on
Secrets
  cluster-wide and that field takes a namespace. Whatever renders a
`ProxmoxCluster` for a tenant in the rest of the series has to keep the
field
  out of tenant hands.

### Testing

`make unit-tests` — exit 0. New suites:
`packages/core/platform/tests/sources_capi_provider_proxmox_test.yaml`
and
`bundles_proxmox_wiring_test.yaml` (both bundle paths, the
enable/disable
precedence, and the refusal above).

Every negated `containsDocument` in the wiring suite carries `any:
true`;
without it the negation passes whether or not the document is in the
render.
Three mutations, each reverted afterwards — emitting the capmox
`Package`
unconditionally, emitting the IPAM `Package` unconditionally, and
dropping the
`disabledPackages` check from `$proxmoxEnabled` — each turn red the
tests named
after the property they break, and the unmutated chart stays green (44
suites,
198 tests).

`hack/capi-provider-components_test.bats` covers both new provider
packages:
the blob and the YAML are the same bytes; the Makefile version, the
ConfigMap
name and `spec.version` are one value — `applied-spec-hash` covers the
provider
spec and the config Secret but not the ConfigMap, so a bump that
regenerates the
files while `spec.version` stays behind installs the old provider
silently; and
the vendored files match the SHA-256 sums the Makefile pins, so the pin
holds
even when nobody runs `make components`.

The render was compared against `main` for five configurations
(defaults,
`isp-full`, `isp-full-generic` with paas, an unrelated
`enabledPackages`, and
`disabledPackages` naming IPAM). No line is removed in any of them and
the
`Package` count is unchanged; the only additions are the two
`PackageSource`
objects, which the chart renders for every package, optional or not.

### Screenshots

Not applicable — no UI change.

### Downstream repositories

I walked the trigger map in `docs/agents/contributing.md` against this
diff, file by file. What it matches is below. **No box is ticked**,
because every follow-up it points at depends on a decision that is not
mine to make: #3481 asks whether Proxmox becomes a supported platform at
all, and writing its documentation or its Terraform schema before that
is answered would be work built on an assumption. I have not filed
speculative PRs or issues in those repositories for the same reason. If
the direction is accepted, I open them; a human should decide which.

**cozystack/website — matches, no follow-up yet.**

- `packages/core/platform/values.yaml` changed.
`content/en/docs/next/operations/configuration/platform-package.md` is a
hand-written table of the `spec.components.platform.values.*` keys; this
PR does not add a key, but it documents two new values that
`bundles.enabledPackages` now accepts, and the table is where a reader
would look for them.
- Two platform components are added
(`capi-providers-infraprovider-proxmox`,
`capi-providers-ipam-in-cluster`) → `guides/platform-stack/_index.md`,
`operations/configuration/licenses.md`.
- The `iaas` bundle gains two optional packages →
`operations/configuration/variants.md`.

**Everything else: no match.** Nothing under `packages/apps/` or
`packages/extra/` (terraform-provider, website app lists), no
`packages/core/installer/values.yaml` or `networking.*`/`publishing.*`
key the Ansible role sets, no `hack/` file or package-layout change
(ccp, external-apps-example), no `ApplicationDefinition` semantics, no
telemetry metric, no node prerequisites (talm, ansible), no
commit-convention change (community).

- [ ] 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
feat(cluster-api): add optional Cluster API packages for Proxmox — the capmox infrastructure provider and the in-cluster IPAM provider it requires. Neither is deployed unless named in `bundles.enabledPackages`, since capmox drives a hypervisor outside the cluster and does nothing without Proxmox API credentials. Enabling the Proxmox provider brings the IPAM provider with it.
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Added optional Proxmox infrastructure provider support for IaaS
clusters.
- Added in-cluster IP address management, usable independently or
alongside Proxmox.
- Proxmox deployments require a pre-created credentials Secret and
automatically include in-cluster IPAM.
  - Added validation for incompatible Proxmox and IPAM settings.

- **Tests**
- Added coverage for provider dependencies, package enablement,
disabling behavior, invalid configurations, and packaged provider
components.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/uncategorized PR auto-labeler could not map title scope to a known area/*; please review kind/bug Categorizes issue or PR as related to a bug size/M This PR changes 30-99 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants