mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-08 21:14:56 +09:00
97fc4359ad804b2fa343dc54037f0bbce84be829
7010
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
97fc4359ad |
feat(scheduler): set scheduling-class label on mutated pods (#2621)
## What this PR does
Extends the lineage-controller-webhook so that, on every Pod it mutates
with a `SchedulingClass`, it also sets a label of the same name (in
addition to the existing annotation).
Today, when a SchedulingClass is in effect on a tenant, the webhook
injects:
- `spec.schedulerName: cozystack-scheduler`
- annotation `scheduler.cozystack.io/scheduling-class: <name>`
This is enough for the cozystack-scheduler to look the class up and
apply its `nodeAffinity`, `nodeSelector`, and topology rules. However,
it is **not** enough to write a `SchedulingClass` whose
`spec.podAffinity` rules `labelSelector` targets pods across multiple
applications, because `labelSelector` matches labels and not
annotations.
This PR adds the label, leaving the annotation untouched for backward
compatibility:
```go
podLabels := obj.GetLabels()
if podLabels == nil {
podLabels = make(map[string]string)
}
podLabels[schedulerapi.SchedulingClassLabel] = schedulingClass
obj.SetLabels(podLabels)
```
The constants `SchedulingClassLabel` and `SchedulingClassAnnotation`
exported by `github.com/cozystack/cozystack-scheduler/pkg/apis/v1alpha1`
already share the same value
(`scheduler.cozystack.io/scheduling-class`), so no new key is
introduced.
**Why this is useful**: a single SchedulingClass can now express
"co-locate every pod that uses me at the same topology" with a rule
like:
```yaml
apiVersion: cozystack.io/v1alpha1
kind: SchedulingClass
metadata:
name: co-region-prod
spec:
podAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
- labelSelector:
matchLabels:
scheduler.cozystack.io/scheduling-class: co-region-prod
topologyKey: topology.kubernetes.io/region
```
When several applications share this class, the second-and-later pods
will be constrained to the region the first one landed on — without
hard-coding any region in the class itself.
### Tests
Adds a `webhook_test.go` (none existed in the package). Five unit tests,
using `clientfake.NewClientBuilder` +
`dynamicfake.NewSimpleDynamicClientWithCustomListKinds`, cover:
- Label, annotation, and `schedulerName` are all set when a class
exists.
- Existing pod labels are preserved (no clobbering).
- Non-Pod objects are a no-op.
- Pods in a namespace without a scheduling-class label are a no-op.
- Pods whose referenced `SchedulingClass` CR does not exist are a no-op
(the existing defensive behavior, preserved).
### Release note
```release-note
feat(scheduler): the lineage-controller-webhook now sets a scheduler.cozystack.io/scheduling-class label on mutated pods (in addition to the existing annotation), enabling SchedulingClass authors to write podAffinity rules that match pods across applications.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **New Features**
- Pods with a valid scheduling class now receive its label alongside the
scheduler configuration and annotation.
- Existing Pod labels are preserved when the scheduling-class label is
added.
- **Bug Fixes**
- Scheduling-class settings are applied only when the namespace
references a scheduling class that exists. Non-Pods and namespaces
without a scheduling-class label are left unchanged.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
mattia-eleuteri
|
250f89dc72 |
feat(scheduler): set scheduling-class label on mutated pods
The lineage-controller-webhook now sets a scheduler.cozystack.io/scheduling-class label on mutated pods (in addition to the existing annotation of the same name). This unlocks SchedulingClass authors writing podAffinity rules whose labelSelector targets pods across different applications that share the same class. A natural pattern becomes: define a class whose name is the affinity-group identifier, attach it to several applications, and have the scheduler co-locate them at the chosen topology key (region, zone, hostname). Behavior preserved when the SchedulingClass CR is missing: pods are left untouched (no scheduler injection, no label) so they do not stay Pending on a non-existent scheduler. Tests cover label/annotation/schedulerName injection, existing label preservation, non-Pod no-op, no-class no-op, and missing-CR no-op. Signed-off-by: mattia-eleuteri <mattia@hidora.io> |
||
|
Aleksei Sviridkin
|
0592d0e324 |
chore(ci): add area/openbao to the label set and the scope mapping (#4182)
## What this PR does Every openbao PR lands on `area/uncategorized`. The labeler has no scope entry for it, and there would be no label to apply if it did, so both #4168 and #4177 are sitting on the fallback right now. `labels.yml` puts the bar for a new area at three open issues on the topic, and openbao is at three: #2787, #2483 and #2793. The shape follows what is already there for a single shipped component, next to `area/kamaji`, `area/keycloak` and `area/cilium`. The mapping takes the plain `openbao` scope and `openbao-central` alongside it, because the platform instance lives in its own package and carries its own scope. Checked that every `area/*` and `kind/*` the labeler references is declared in `labels.yml`: 27 referenced, none missing, before and after this change. zizmor and the pre-commit hooks pass, and the embedded script still parses under github-script semantics. ### Screenshots No UI change. ### Downstream repositories Walked the trigger map against the diff. Nothing matches: the change is two entries under `.github/`, and no trigger in the map is keyed on the label set or the labeler. No package added, renamed or removed, no platform component or variant, no `values.schema.json`, no `ApplicationDefinition`, no `hack/` layout change, no namespace rename. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added an `area/openbao` label for tracking issues and pull requests related to OpenBAO. - Updated automatic pull request labeling to apply this label to changes scoped to OpenBAO and OpenBAO Central. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
9ac67d76a2 |
test(tests): preserve Bats failure reports during fixture cleanup (#3848)
## What this PR does Test-level `EXIT` cleanup overrides the Bats TAP handler, so a failed assertion can disappear from test results. This moves cleanup in eight unit files after their assertions. Failed tests keep scratch files for inspection. Prerequisite for [the Bats runner change](https://github.com/cozystack/cozystack/pull/3849). Existing subshell traps in the OpenAPI and CAPK tests remain supported. Closes #3623 ### Validation `make unit-tests test-controllers -j4 --output-sync=target` passed. The converted files passed under Bats 1.14.0 and `dash`. A deliberately failed assertion produces `not ok` after this change, and the trap audit rejects an added test-level handler. ### Downstream repositories Test cleanup and its documentation only. No downstream consumer changes. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note NONE ``` |
||
|
Aleksei Sviridkin
|
15f57be6f9 |
feat(controller): expose bucket class on BucketClaim Workloads (#3501)
## What this PR does `WorkloadMonitorReconciler.reconcileBucketClaimForMonitor` builds a `Workload` for each `BucketClaim` but never read `spec.bucketClassName`, so nothing on the resulting `Workload` told a consumer which bucket class was in use (e.g. an encrypted class vs. a plain one). This PR reads `BucketClaim.spec.bucketClassName` and sets it as a `workloads.cozystack.io/bucket-class` label on the `Workload`, following the same `workloads.cozystack.io/*` labeling convention already used for KubeVirt instance-type/profile metadata elsewhere in this controller. A regression test (`TestReconcileBucketClaim_BucketClassLabelPropagated`) asserts the label is present with the correct value. Closes #3419 ### 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 feat(controller): expose bucket class on BucketClaim Workloads via the workloads.cozystack.io/bucket-class label ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Workloads created from ready BucketClaims include the associated bucket class label when a valid value is specified. * **Bug Fixes** * Invalid or empty bucket class values are omitted from Workloads. * Stale bucket class labels are removed when the associated claim does not provide a valid value. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
3111fa7802 |
chore(ci): add area/openbao to the label set and the scope mapping
Every openbao PR lands on area/uncategorized, because the labeler has no scope entry for it and there would be no label to apply if it did. #4168 and #4177 are both sitting on the fallback right now. labels.yml sets the bar for a new area at three open issues on the topic, and openbao is at three: #2787, #2483 and #2793. It also follows the shape already used for a single shipped component, next to area/kamaji, area/keycloak and area/cilium. Maps the plain `openbao` scope and `openbao-central` alongside it, since the platform instance lives in its own package and gets its own scope. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
9d3661416b |
ci: drop unused packages:write from the pull-request workflow (#3422)
## What this PR does The `build`, `build-talos` and `finalize` jobs in `pull-requests.yaml` each requested `packages: write`, but none of them touches GHCR: the workflow's `REGISTRY` is OCIR and they authenticate with `OCIR_USER` / `OCIR_TOKEN`. The only GHCR login in the file is in `verify-release-candidate`, which has `packages: read` and is not touched here. The scope was dead. It is worth removing rather than leaving alone. GHCR pushes elsewhere in CI authenticate with `GITHUB_TOKEN` plus `packages: write` rather than with a stored secret, so on those paths the scope *is* the whole credential, so granting it on the `pull_request` lane hands every same-repo PR run a token carrying GHCR write authority that lane has no use for. Whether a given package would actually honour that write depends on its Actions access setting, which lives outside this repo, so this is least-privilege hygiene rather than a fix for a demonstrated exploit. It also brings the three jobs in line with the `permissions: contents: read` default the workflow already sets at the top level. The e2e job keeps its `packages: read`; only the write scope is removed. Verified: `build`, `build-talos` and `finalize` log in to OCIR only (`docker/login-action` with `OCIR_USER` / `OCIR_TOKEN`), the sole `ghcr.io` login is in `verify-release-candidate` with `packages: read`, unchanged. The file still parses as YAML after the change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: Walked the trigger map in `contributing.md` against the diff. It removes three permission lines from one workflow and changes nothing any downstream repository restates. The nearest entry is the `ccp` one on release-prep behaviour, which is scoped to `tags.yaml` and untouched here. ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated the build, Talos build, and finalization workflows to request read-only repository access. * Removed package write permissions from these workflows. * Registry login and image publishing remain configured separately and are unchanged. No other workflow behavior or processing steps changed. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
c983b5c404 |
fix(linstor): use severity warning instead of warn in piraeus-datastore alerts (#3799)
## What this PR does
Seven of the piraeus-datastore alert rules label themselves `severity:
warn`. Alerta, which the monitoring package ships as the alert sink,
validates that value and `warn` is not on its list. Every webhook
delivery for these alerts dies with a 500.
```
Severity (warn) is not one of security, critical, major, minor, warning,
indeterminate, informational, normal, ok, cleared, debug, trace, unknown
```
Alertmanager retries seven times and drops the notification, and the
alert never shows up anywhere. The affected rules are the LINSTOR/DRBD
ones, such as storage pool at capacity, lost quorum, unintentional
diskless and stuck resync. These are exactly the alerts you least want
silently dropped.
Found this on a v1.4.6 cluster where `linstorStoragePoolAtCapacity` was
genuinely firing. The cluster had 153 dropped notifications in 40
minutes and nothing in Alerta. Renaming the label to `warning`, which
the other three rules in the same file already use, stopped the failures
immediately and the alerts came through.
The same labels sit on `main` and in `v1.6.1`, so a backport to
`release-1.6` would be appreciated.
The test commit adds a bats guard so the next one gets caught in CI. It
walks every `PrometheusRule` and `VMRule` under `packages/` outside
vendored `charts/`, helm templates included, and fails on a severity
outside Alerta's default alarm model, which is exactly where `warn`,
`info`, `error` and `fatal` fall. Helm actions are stripped before the
file is read, so a rule behind a `{{- if }}` or an unquoted `{{
.Release.Namespace }}` is checked rather than skipped, and a file that
still does not parse fails the contract instead of thinning it out. On
the current tree that is 194 alerting rules in 41 scanned files.
The guard found one more on `main`. `DatabaseAutoscalerAtMax` in
`packages/system/keda/templates/alerts.yaml` carries `severity: info`,
which Alerta refuses the same way. It is renamed to `informational`, the
value the model has for that level and the one seven other rules in the
tree already use.
Moving that alert off `info` also stops `InfoInhibitor` in
`packages/system/monitoring-agents/alerts/general.rules.yaml` from
firing. That rule matches any `severity="info"` alert and carries
`severity: major`, so on `main` it usually reached Alerta whenever
`DatabaseAutoscalerAtMax` fired, even though the keda alert itself was
refused. After this change no rule carries `info`, so operators no
longer see that `InfoInhibitor` notification.
Fixes #2411.
### Screenshots
Not a UI change.
### Downstream repositories
Walked the trigger map against the diff. It touches severity labels
inside `packages/system/piraeus-operator/alerts/` and
`packages/system/keda/templates/`, plus a bats file under `hack/`. No
package is added or renamed, and there is no values schema, CRD, tooling
the satellites gate on or node contract change. Nothing downstream
restates these labels.
- [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(linstor): LINSTOR/DRBD alerts carried severity `warn`, which Alerta rejects, so they were never delivered. They now use the standard `warning` severity, and the keda at-max autoscaler alert moves from `info` to `informational` for the same reason.
```
|
||
|
harshitrwt
|
ffb6564f8b |
fix(controller): drop stale bucket-class label from BucketClaim labels before applying validated value
maps.Copy(workload.Labels, bc.Labels) can bring in a workloads.cozystack.io/bucket-class label directly set on the BucketClaim, which would survive unmodified when spec.bucketClassName is empty or invalid, leaving the Workload reporting a class that does not match the source BucketClaim. Signed-off-by: harshitrwt <harshit.rawat1804@gmail.com> |
||
|
Aleksei Sviridkin
|
2c2f2c194e |
fix(dashboard, foundationdb): show the connection ConfigMap to tenants (#4612)
## What this PR does Fixes #4611. Since #4148 a tenant may read the `<release>-config` ConfigMap that holds the FoundationDB cluster file, but the console has no view for ConfigMaps, so the connection string was still reachable only with kubectl. Chart: - `<release>-resourcemap` is rendered regardless of `monitoring.enabled`, because the console now reads it; - the tenant Role grants `get`, `list` and `watch` on `<release>-resourcemap` next to `<release>-config`, still by `resourceNames`, with no wider ConfigMap access. Console: - the application page gets a ConfigMaps tab when the application's resource map lists at least one ConfigMap in the same namespace; - each listed ConfigMap is read and watched with a `metadata.name` field selector, which RBAC checks against `resourceNames`, so a tenant never needs `list` on all ConfigMaps; - values are shown as plain text with a copy button, the cluster file needs no decoding; - a resource map that is missing or forbidden hides the tab, a listed ConfigMap that cannot be read shows the API error instead of looking empty, and a malformed map or an invalid name is reported. The tab is generic, but today only FoundationDB ships a resource map, so it is the only application that gets it. #3490 keeps `monitoring.enabled` as the switch for the resource map and asks whether the map should render unconditionally. This PR answers that for FoundationDB by rendering it always, so the wording in #3490 would need to follow if both land. Tests: `helm unittest` for the chart (6 tests), `tsc --noEmit`, `vitest run` (438 tests in 60 files) and `eslint` on the changed files pass. The new tests cover reading only the listed names in the active namespace, the watch, copying, a clipboard failure, a read denial, and malformed maps. The chart renders with `monitoring.enabled` true and false. ### Screenshots The ConfigMaps tab of a FoundationDB application, rendered by the console from this branch against a local stub API with synthetic data:  ### Downstream repositories Walked the trigger map against the diff: it changes a FoundationDB chart template and its test, and the console. No `values.yaml`, `values.schema.json`, README, CRD or `hack/` file is touched. The website FoundationDB page is generated from the chart README and says nothing about connecting, and the Terraform provider does not read these ConfigMaps. The console is vendored in this repository. - [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(dashboard, foundationdb): the application page shows the FoundationDB connection ConfigMap with the cluster file to tenants who may read it ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Application details now include a ConfigMaps tab when configuration maps are available. View their values and copy them to the clipboard. * The tab displays loading, empty, unavailable, and error states. * **Bug Fixes** * The resource map remains available when monitoring is disabled, and its access permissions are included. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
scooby87
|
51155f1003 |
feat(postgres-operator): alert when the WAL archive backlog does not drain (#4572)
## What this PR does `LastFailedArchiveTime` compares a cluster's last failed WAL archival with its last successful one, so it resolves as soon as any segment gets through. Archiving that fails intermittently, or runs slower than the cluster writes WAL, keeps clearing that alert while the queue of segments waiting on the Postgres volume grows, until the volume is full and the database stops. The alert is also only a warning. This PR adds `CNPGWALArchiveBacklog`, a critical alert that fires when a primary's `archive_status` queue has not been empty once in 30 minutes. A healthy archiver empties the queue within seconds of each WAL switch, so a queue that never drains means point-in-time recovery is falling behind, whatever the archiver reports. Idle clusters and replicas have an empty queue and stay quiet. Validated live, since the repository has no rule-evaluation tooling: on a test cluster, two CNPG clusters archived into an object store that refused connections and into one that accepted them and never answered, while writing WAL continuously. The rule, evaluated by the root vmalert, fired for both. Over the previous seven days of the same cluster's metrics, none of its healthy clusters would have matched; those clusters carry light load, so a cluster whose archiver runs near its limit all the time has not been observed and would page, which is the intended reading of a queue that never drains. The unit test pins only that the rule ships in the rendered PrometheusRule as critical, since the rules reach the cluster through a file glob that drops a moved file silently. ### 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: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff touches an operator chart's alert rules and its test; no path from the trigger map. ### Release note ```release-note feat(postgres-operator): page when a Postgres cluster's WAL archive queue has not drained for 30 minutes, which the existing archiving alert misses while some archives still succeed. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added a critical alert for sustained PostgreSQL WAL archive backlogs. It triggers when the minimum observed ready archive status remains above zero over a 30-minute window, then stays pending for 10 minutes before firing. The alert describes potential impacts on point-in-time recovery and storage usage. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
c5865061a3 |
feat(dashboard): show a connection hostname for LoadBalancer services (#4035)
## What this PR does The Services tab on the application page shows a connection hostname under the External IP of every LoadBalancer Service, as `<service-name>.<suffix>`, also while the IP is still Pending. ClusterIP Services get nothing. The suffix is a new platform value, `publishing.serviceDomain`. It travels the same path as `cluster-domain` and `branding`: `_cluster.service-domain` in the cozystack-values Secret (leading and trailing dots stripped there), then `serviceDomain` in the dashboard's `config.json`, written only when non-empty. Empty by default, and with it empty the tab shows no hostname. There is deliberately no fallback to a tenant-derived name: the app charts certify `<release>.<tenant host>` while the LoadBalancer Services are named `<release>-external-write` and similar, so a name built from the tenant apex would match neither DNS nor the certificate. Showing nothing beats showing a name nobody publishes. The value is display-only. Nothing in the platform creates DNS records for it. Set it when your DNS already resolves `<service-name>.<serviceDomain>` to that Service's LoadBalancer IP, external-dns with a matching fqdn template for example. A CoreDNS `k8s_external` zone does not fit, it answers `<service>.<namespace>.<zone>`. The shown name is not the one in the app certificates, so a TLS client may see a SAN mismatch unless DNS and certificates are aligned by the operator, and two tenants with a release of the same name get the same hostname. The values comment says all of this. Left for follow-ups: reading `external-dns.alpha.kubernetes.io/hostname` from the Service as the first source (only the opensearch chart stamps it today), and a per-Service override annotation. Parked from review, not blocking: the Admin External IPs page does not show the hostname yet, the column header still says "External IP", and surrounding whitespace in the value is not trimmed. Closes #3112 ### Screenshots <img width="1200" height="360" alt="services-tab" src="https://github.com/user-attachments/assets/01e1f3e0-836e-41cc-b916-f675b62580cd" /> ### Downstream repositories `packages/core/platform/values.yaml` gains a key, so the hand-written table in `content/en/docs/next/operations/configuration/platform-package.md` on cozystack/website needs a row for `publishing.serviceDomain`. I will open that website PR once this one merges. No other repository restates this key: ansible-cozystack passes a fixed list of `publishing.*` keys that does not include it, and the terraform provider models app schemas, not platform values. - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: a `publishing.serviceDomain` row in the platform-package table, opened after this PR merges - [ ] [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(dashboard): the application Services tab shows `<service>.<suffix>` under the external IP of LoadBalancer services when the new platform value `publishing.serviceDomain` is set ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added an optional service domain setting for displaying LoadBalancer service hostnames in the dashboard. - When configured, services display hostnames in the format `<service-name>.<service-domain>` alongside external IP information. - Hostnames are shown only for LoadBalancer services and remain hidden when no service domain is configured. - Dashboard configuration now receives the service domain setting automatically, including support for custom domain formatting. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
harshitrwt
|
f41a500b07 |
test(controller): cover invalid bucket class label in reconcileBucketClaimForMonitor
A BucketClassName longer than 63 chars (valid as a Kubernetes object name, up to 253 chars) is not a valid label value, so the bucket-class label is skipped with an Info log rather than failing the Workload write. This can happen with SeaweedFS pool class names (<namespace>-<pool>-lock) when the tenant and pool names are both long -- a missing label in that case does not mean no class was set. Signed-off-by: harshitrwt <harshit.rawat1804@gmail.com> |
||
|
Aleksei Sviridkin
|
52c74beedd |
fix(api): let the garbage collector finalize Applications (#4610)
## What this PR does An application deleted with foreground or orphan propagation (`kubectl delete --cascade=foreground`, or any client that sends that `propagationPolicy`) could stay in deletion forever. Helm uninstalled the release and helm-controller removed `finalizers.fluxcd.io`, but the HelmRelease kept `foregroundDeletion` in `metadata.finalizers`, so the HelmRelease and its Application stayed terminating. We hit this on v1.6.4 with a Bucket and an OpenBAO application. An Application and its HelmRelease share one UID, so the garbage collector keeps a single graph node for both and may finalize it through the Application endpoint. The API never returned finalizers on an Application, and every update restored the HelmRelease finalizers from the live object. [`removeFinalizer`](https://github.com/kubernetes/kubernetes/blob/v1.33.1/pkg/controller/garbagecollector/operations.go#L85-L123) reads the owner, does not find the finalizer there and returns as if it had already been removed, so nothing ever took it off the HelmRelease. This PR: - returns `foregroundDeletion` and `orphan` in `metadata.finalizers` of an Application, and only those two; - carries an add or a remove of either of them from an Application update to the HelmRelease, as a change against the finalizers the update started from, so the result is the same when helm-controller or the garbage collector changes the list during the conflict retry; - keeps controller finalizers such as `finalizers.fluxcd.io` owned by the live object, so `helm uninstall` still runs before the release goes away; - copies the deletion timestamp and grace period from the live object into the rebuilt HelmRelease, so an update of a terminating release stays valid. `rest_garbage_collection_test.go` covers both finalizers with Helm finishing first and with the garbage collector finishing first, concurrent finalizer changes during the conflict retry, controller finalizers surviving an update, the cached HelmRelease not being mutated by the conversion, and an `UpdatedObject` that edits the object in place. All five tests fail on the current `main` without this change. `go test -race ./pkg/registry/...` passes. The tests use the controller-runtime fake client, so this has not been run against a live garbage collector with a patched cozystack-api yet. The same bug is in release-1.6. There the Update path has no finalizer carry-over at all, because #2914 is not backported yet, and its automatic backport #4577 currently carries conflict markers. A 1.6 backport of this fix should go on top of a working backport of #2914. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map against the diff: it only changes how cozystack-api handles `metadata.finalizers` of Applications in `pkg/registry/apps/application`. No package, values, schema, CRD, `hack/` file or convention is touched. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(api): an application deleted with foreground or orphan propagation no longer leaves its HelmRelease stuck with the foregroundDeletion or orphan finalizer ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Application updates now preserve Kubernetes deletion behavior, including foreground deletion and orphaning, even when concurrent changes occur. * Helm-managed protection metadata is retained during updates, and deletion timing details remain intact. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
harshitrwt
|
882929ff86 |
fix(controller): skip bucket-class label when BucketClassName is not a valid label value
Signed-off-by: harshitrwt <harshit.rawat1804@gmail.com> |
||
|
harshitrwt
|
0c37fff5f4 |
feat(controller): expose bucket class on BucketClaim Workloads
Signed-off-by: harshitrwt <harshit.rawat1804@gmail.com> |
||
|
Aleksei Sviridkin
|
97fa2a619f |
feat(dashboard): show a connection hostname for LoadBalancer services
Users who provision a service through the dashboard had no hostname to connect to: the Services tab showed only the raw LoadBalancer IP, while the pre-1.4.0 UI composed <service>.<root host> for every Service. Render <service>.<suffix> under the External IP of every LoadBalancer Service (also while the IP is still Pending), and nothing for ClusterIP Services. The suffix is config.json serviceDomain, set by the platform value publishing.serviceDomain. There is deliberately no fallback: the app charts fix their external name as <release>.<tenant host> while the LoadBalancer Services are named <release>-external-write and similar, so a name derived from the tenant apex would match neither the DNS nor the certificate. Without a configured suffix nothing is rendered. Runtime config previously reached only the Shell as a prop; it is now published through AppConfigContext and read from there by the Shell and the Services tab alike, so a single test on the rendered version pins the wiring. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
67adfbce3c |
feat(dashboard): surface service-domain in the console config.json
The console loads config.json once at startup and treats a missing serviceDomain key as "show no hostname", so the chart copies _cluster.service-domain into config.json only when it is non-empty rather than shipping an empty string the SPA would have to special-case. The helm-unittest suite pins the unset, empty and set cases. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
d463e00da1 |
feat(platform): add publishing.serviceDomain for the dashboard Services tab
The dashboard has no way to show a connection hostname for a LoadBalancer Service: before 1.4.0 the old UI composed one from the Service name and the cluster's root host, and operators whose DNS publishes LoadBalancer IPs under a dedicated zone still need that. Add the suffix as a platform value and propagate it into the cozystack-values map as service-domain, next to cluster-domain and branding, so the dashboard chart can read it. The value is display-only and opt-in: empty (the default) means the console shows no hostname, because nothing in the platform publishes DNS for a guessed name. Leading and trailing dots are stripped here, the one place every consumer reads from, so an operator's ".svc.example.org." cannot render as "demo..svc.example.org."; the value is coerced to a string first because an unquoted numeric scalar would otherwise abort the whole platform render. The helm-unittest suite pins the empty default, a configured value landing in the Secret, the dot trimming and the numeric case. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
0bc3f9f25c |
fix(kubernetes): name TalosConfigTemplate by content hash (#3523)
## What this PR does Fixes #3515. `TalosConfigTemplate.spec` is immutable — the `vtalosconfigtemplate.cluster.x-k8s.io` webhook denies any mutation with `TalosConfigTemplate.Spec is immutable` — but the `talos-reconcile` Job applied it under a fixed `<release>-<group>` name on every reconcile. Once the template existed, **no change to the rendered worker machine config could ever reach the cluster**: the apply was rejected server-side while the HelmRelease stayed `Ready`. Reported with `machine.network.searchDomains` after a `clusterDomain` correction; the same held for `certSANs`, `machine.registries` and nameservers. - Name the `TalosConfigTemplate` `<release>-<group>-<6-char hash of its spec>`, mirroring the `KubevirtMachineTemplate` content-hash idiom the chart already uses, and point `MachineDeployment.spec.template.spec.bootstrap.configRef` at that name. A content change now creates a **new** template instead of attempting a rejected mutation, and CAPI rolls the workers onto it. - Extract the machine config into a single named template (`_talosconfigtemplate.tpl`) shared by the two consumers that have to agree on the name byte-for-byte: the Job, which embeds it in the heredoc it `kubectl apply`s, and the `MachineDeployment`, which hashes it into `configRef`. - Assemble the node group once, in `kubernetes-nodes.group`, and read it back at both call sites. Sharing the rendering was not enough: each file also built its own `$group` dict, so the two names agreed only by coincidence of the keys the spec helper happens to read. - The runtime values the Job substitutes (`${TALOS_CA_B64}`, `${SVC_IP}`, `${COREDNS_IP}`, tokens) stay shell placeholders and are deliberately **not** part of the hash: they are facts of an already-provisioned cluster, not chart inputs, so a certificate rotation or a Service re-address does not roll workers. ### What the shared spec carries Worker node pools live in the `kubernetes-nodes` chart, which owns `_talosconfigtemplate.tpl` outright: `make update` copies nothing from the parent chart. The helper renders everything `main` puts into the worker machine config, so the hashed spec is the one the cluster gets: - The heredoc escaping from #3513: backslash, dollar and backtick escaping on `talosVersion`, `installerRepository` and `schematicID`, the escaped `machine.registries.mirrors` block, and the `INVARIANT` comment above `data:`, which belongs in that file since it is where the next field will be added. The spec is still emitted from an unquoted heredoc in a host-cluster Pod, so an unescaped copy would be command injection. - The per-pool install image. `_talosconfigtemplate.tpl` calls `kubernetes-nodes.resolveOsImage` itself instead of reading `talos.schematicID` and `talos.version`, so a pool on `osImage.builtin` or `osImage.factory` reinstalls the schematic and version it actually booted. The install image is part of the hashed spec, so the hash has to be taken over the resolved image, or an `osImage` change would render a different config under an unchanged name. - The Proxmox branches: `nameservers` from `proxmox.dnsServers` where a KubeVirt pool points at the cluster's CoreDNS, and the kubelet's `cloud-provider: external`, without which the Proxmox cloud-controller-manager never initialises the Node. - `machine.kernel.modules`, emitted only when the pool resolves to a non-empty list. `kubernetes-nodes.group` carries every key those read, `podCpuLimit`, `podCpuRequest`, `osImage`, `proxmox` and `kernelModules` included. A key missing there is the failure this PR exists to prevent: with `kernelModules` read at one call site only, an explicit module list hashes to two different names and the `MachineDeployment` points at a template nothing creates. The `kubectl apply` stays `--server-side --force-conflicts` (#4163), so the Job is not OOMKilled on the merge path; with a content-hash name the apply is normally a create or a no-op, and server-side apply keeps the no-op cheap too. ### The replace-on-immutable path and the Role The runtime values stay outside the hash, so one failure is left that the hash cannot absorb: a new apiserver address renders a different spec under an unchanged name, and the webhook rejects it. The replace path from #4089 handles exactly that case and is kept, with its `kubectl delete` pointed at `${TCT_NAME}`. The talos-reconcile Role pins its TalosConfigTemplate `get`, `patch`, `update` and `delete` rights to the pool's own template, so it names the hashed template too. A server-side apply of a template that does not exist yet is still a PATCH on its name: a rule left on `<release>-<group>` would leave the Job Forbidden and the pool with no template to scale onto, with nothing turning red. The name-agreement guard below therefore covers the Role as well. ## Upgrade impact — please read **Existing clusters roll their Talos workers once.** The `configRef` moves from `<release>-<group>` to the hashed name; CAPI creates a new `MachineSet` for it and rolls machines onto it under the existing `RollingUpdate` strategy (`maxSurge: maxReplicas`, `maxUnavailable: 1`) plus the group's `MachineHealthCheck`. The rendered machine config itself is byte-identical to `main` for the same values, so this particular roll delivers no config change — it is the one-time cost of adopting the content-hash name, the same cost `KubevirtMachineTemplate` already paid. Proxmox pools roll the same way: the `ProxmoxMachineTemplate` name does not move, but the `configRef` does. **Until the new template exists, the pool cannot scale.** The Helm upgrade points the `MachineDeployment`'s `configRef` at the hashed name before the `talos-reconcile` Job has created that template, and there is no render-time ordering fix, since the template needs values Helm does not have. Existing workers are untouched in that window, but the pool is frozen: in CAPI v1.10.1, which this repository ships, `getTemplatesAndSetOwner` records the missing bootstrap template, and `rolloutRolling` then fails in `getAllMachineSetsAndSyncRevision` with `cannot create a new MachineSet when templates do not exist` and returns before it scales either the new or the old `MachineSet`. A replica change from the cluster-autoscaler therefore reaches no `MachineSet`: scale-up and scale-down both wait. On an upgraded cluster the Job's inputs already exist, so the window is normally a few seconds. **A failed `talos-reconcile` Job now blocks autoscaling, and nothing turns red.** After `backoffLimit: 30` the Job is not retried until the next Helm upgrade of the pool release, and the HelmRelease stays `Ready` meanwhile, as it did in #3515. Before this PR the same failure left the pool on its old template and still scaling; with it, the pool cannot scale at all until the template exists. Where to look: - the `MachineDeployment` `kubernetes-<cluster>-<group>`: its `ScalingUp` condition under `status.v1beta2.conditions` reads `Scaling up from N to M replicas is blocked because TalosConfigTemplate does not exist` (or `Scaling up would be blocked because …` while no scale-up is pending), and the capi-controller-manager logs carry `cannot create a new MachineSet when templates do not exist` for it; - whether a `TalosConfigTemplate` named after its `spec.template.spec.bootstrap.configRef.name` exists in the tenant namespace; - the `talos-reconcile` Job's status and Pod logs — but the Job has `ttlSecondsAfterFinished: 600`, so a failed Job and its logs are deleted ten minutes after it gives up. Past that, the `MachineDeployment` condition and the missing template are the only evidence left. The pre-existing fixed-name template is **not** deleted — the outgoing `MachineSet` still references it — and is garbage-collected with the `KamajiControlPlane` when the cluster is deleted. Hashed templates therefore accumulate, one per distinct machine config. Pruning them safely is filed as #4355 rather than widened into this PR. **The one roll is not the only one.** Hashing the whole rendered spec makes anything inside it a roll trigger, including two things that are not edits to this cluster's resource. For a pool sized by `instanceType`, the kubelet reservations come from a live `lookup` of the `VirtualMachineClusterInstancetype`, so a later platform-side edit to that instancetype rotates the name and rolls the pool again — and the `KubevirtMachineTemplate` hash this mirrors does not have that property, since it embeds the instancetype by name and never its resolved size. And `talos.registryMirrors` is inside the hashed spec, so setting or changing a mirror now rolls the pool rather than applying only to nodes created later; the field's documentation said nothing about this and now says it. The same holds on Proxmox for `proxmox.dnsServers`, and on every substrate for `kernelModules`: both are inside the hashed spec, so changing either rolls the pool, and the `kernelModules` documentation now says so instead of describing the fixed name as the current state. **The roll is not free on a node group holding a scarce resource.** From exercising this same rollout path on a production fleet of 13 tenant clusters, by rotating the `KubevirtMachineTemplate` hash on a GPU node group. `maxSurge: maxReplicas` is dangerous when the resource is constrained: with 6 GPUs and 1 free, CAPI created all 5 new machines at once and 3 sat `Pending` on `Insufficient nvidia.com/...`, each having already imported a 200 GiB disk. It converges, but it wastes a lot of import. The natural reflex, `maxSurge: 0` with `maxUnavailable: 1`, deadlocks instead: CAPI computes `maxScaledDown = available - (desired - maxUnavailable)`, which was `4 - 4 = 0`, so it removed no old machine while the new ones waited for exactly those machines to release their GPUs; nothing moved until `maxUnavailable` was loosened to 2. And the `MachineHealthCheck` amplifies both — at `maxUnhealthy: 100%` and `nodeStartupTimeout: 10m`, machines queuing for a GPU were declared unhealthy and remediated in a loop, three deleted and recreated while perfectly healthy, each iteration re-importing the disk. The per-node-group `nodeStartupTimeout` override fixes that, but the default is shorter than a plausible queue wait. Anyone upgrading a fleet with a GPU pool hits all three, and this PR is what starts the roll. ## Validation - `helm unittest packages/apps/kubernetes-nodes` passes, snapshots included. Pointing `configRef` back at the fixed name fails `nodegroup_test.yaml`, the snapshot and the name-agreement bats; dropping `osImage` from `kubernetes-nodes.group` fails the `worker_image_*` suites; removing the Proxmox nameserver branch fails `substrate_branches_test.yaml` and the Proxmox heredoc case. - `hack/check-talosconfigtemplate-name-agreement.bats` is new. The unittest suites pin each name against its own literal in its own file, so a divergence shows up as one literal having moved, which reads as a stale fixture and invites re-pasting the value. The guard renders the chart once and compares the Job's `TCT_NAME`, the Role's TalosConfigTemplate `resourceNames` and the `MachineDeployment`'s `configRef.name` against each other, so the failure states the invariant rather than a number. Asserted on a GPU-shaped pool as well as the default one, because the default renders `gpus []`, `kubelet {}` and `instanceType ""` as falsy and a consumer reading a subset of the group is indistinguishable there from one reading all of it; on an explicit `kernelModules` list, which neither of those shapes sets; and a last case pins that the shapes hash differently, so the others cannot hold vacuously. Verified by injecting each divergence: dropping `gpus` at one call site, dropping `kernelModules` at one call site, and pointing the Role back at `<release>-<group>` each fail it, the last one also failing the Role cases in `talos_tct_immutable_test.yaml`, as does pointing the `delete` back at the fixed name. - `hack/cozytest.sh hack/talos-reconcile-heredoc_test.bats` passes, the Proxmox cases included. This is the execution-level evidence that the escaping survived the move into `_talosconfigtemplate.tpl`: it renders with a hostile `registryMirrors` endpoint and a hostile `installerRepository`, runs the extracted heredoc through a real shell, and asserts the values come out literal rather than command-substituted; hostile `schematicID` and `version` values, and `proxmox.dnsServers` entries that are not addresses, are asserted refused at render; and the Proxmox heredoc is asserted to emit the nameservers it was given. A rendered-string regex cannot catch a heredoc the shell refuses to emit; this does. - `make generate` in `packages/apps/kubernetes-nodes` — no diff. - The "byte-identical to `main`" claim was measured against `main` on six pool shapes: default, GPU, an explicit `kernelModules` list, `kernelModules: []` on a GPU pool, `osImage.factory` and Proxmox. For each, the `TalosConfigTemplate` manifest the Job applies differs only in its name (`${RELEASE}-${GROUP_NAME}` → `${TCT_NAME}`); the spec is unchanged, the Job's `TCT_NAME` equals the `MachineDeployment`'s `configRef.name`, and the `KubevirtMachineTemplate` and `ProxmoxMachineTemplate` names do not move. - **Not verified:** no live-cluster or e2e run (needs bare-metal nodes). The upgrade sequence above, the scaling freeze included, comes from reading the CAPI v1.10.1 `MachineDeployment` controller source and the chart's existing `KubevirtMachineTemplate` precedent, not from watching it end to end. ### Downstream repositories - [ ] 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: Every box is left empty on purpose, including the first, because I am not confident about one line of the map and would rather a maintainer decided than have me tick it. The change adds no field and changes no default: the only schema movement is the description text of `talos.registryMirrors` and `kernelModules`, regenerated from their `values.yaml` doc comments. Nothing else downstream restates this chart's templates, the hash naming is internal to the chart, and no image reference, CI behaviour or CRD shape moves. The open question is whether `terraform-provider-cozystack` restates field descriptions as well as field shapes; if it does, those description changes reach it and want a follow-up. ## Release note ```release-note fix(kubernetes): name the worker TalosConfigTemplate after a hash of its content so machine-config changes (searchDomains, certSANs, registries, nameservers) actually propagate instead of being silently rejected by the immutability webhook. Upgrading rolls existing Talos workers once, on KubeVirt and Proxmox pools alike. Until the talos-reconcile Job has created the new template, the pool cannot scale, so a failed talos-reconcile Job now blocks cluster-autoscaler scale-up and scale-down while the HelmRelease stays Ready; check the MachineDeployment's ScalingUp condition ("blocked because TalosConfigTemplate does not exist") and the Job status, which is deleted ten minutes after it fails. On a node group holding a scarce resource such as GPUs, review the MachineDeployment surge settings and the MachineHealthCheck nodeStartupTimeout first: `maxSurge: 0` can deadlock the roll and the default startup timeout can trigger spurious remediation while machines queue for the resource. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Worker Talos configuration templates now use content-hashed names for consistent reconciliation and bootstrap references. * Changes to worker machine configuration, including GPU and registry mirror settings, automatically roll existing workers. * **Documentation** * Updated registry mirror documentation to explain that configuration changes roll all workers in the pool. * **Tests** * Added coverage for hashed template names, configuration-triggered worker rotations, and matching references across worker resources. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Yan Bondarenko
|
2fa7fea29b |
fix(dashboard): show ConfigMaps listed in the application resource map
FoundationDB keeps its connection string in a ConfigMap that its operator writes, and tenants may read it, but the console had no view for ConfigMaps. Tenant roles grant ConfigMaps by name only, so the tab takes the names from the application's resource map and reads each one with a metadata.name field selector, which RBAC checks against resourceNames. An application without a readable resource map shows no tab, while a listed ConfigMap that cannot be read is reported instead of looking empty. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Yan Bondarenko
|
e3ae8033e6 |
fix(foundationdb): let tenants read the dashboard resource map
The resource map is the only object that names the connection ConfigMap for a client such as the dashboard. It was rendered only with monitoring enabled, and the tenant role could not read it, so a tenant session had no way to find the ConfigMap it is allowed to read. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Yan Bondarenko
|
46b5a542fe |
fix(api): let the garbage collector finalize Applications
An Application and its HelmRelease share one UID, so the garbage collector keeps a single graph node for both and may finalize it through the Application endpoint. The API never returned finalizers on an Application, and every update restored the HelmRelease finalizers from the live object. When the collector came to drop foregroundDeletion or orphan through the Application, removeFinalizer found no such finalizer and returned as if it were already gone. The HelmRelease then kept foregroundDeletion after Helm had uninstalled the release, and the deletion never completed. The Application now carries the two garbage collection finalizers, and an update that adds or removes one of them applies the same change to the HelmRelease. Controller finalizers such as finalizers.fluxcd.io stay owned by the live object, so helm uninstall still runs before the release goes away. The deletion timestamp and grace period are copied from the live object, which keeps an update of a terminating release valid. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Aleksei Sviridkin
|
4f043b572e |
fix(api): pass the stored object to UpdatedObject in tenantsecret Update (#4603)
## What this PR does A PATCH against an existing TenantSecret returned 404 because `REST.Update` passed `nil` as the old object to `objInfo.UpdatedObject`, and the apiserver patch handler builds the patched object from that old object alone. `Update` now passes the stored Secret (converted with `secretToTenant`) and keeps `nil` only for the `forceCreate` path. The nil also made the patch reach admission as CREATE instead of UPDATE. Validation: the new `TestUpdate_PassesStoredObjectToUpdatedObjectInfo` fails before the change and passes after; `go test ./pkg/registry/...` and `go build ./pkg/... ./cmd/...` pass. I did not run a live `kubectl patch` or the real apiserver patch handler. Known limitation, unchanged by this PR: `tenantToSecret` prefers `Data` over `StringData` when `Data` is non-empty, so a patch that only sets `stringData` on a secret that already has data is a no-op, as a GET-then-PUT edit of `stringData` already is. Patching `data` or metadata works. Fixes #3601 ### 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: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(api): PATCH, `kubectl label` and `kubectl edit` on an existing TenantSecret no longer fail with 404 NotFound. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Tenant secret updates now correctly apply changes using the existing stored value, including label updates. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
3d610b7c31 |
feat(monitoring): add optional email receiver to alertmanager (#3800)
## What this PR does Adds an optional email receiver configured through `alerta.alerts.email`. It is disabled by default. Alerts carrying `notify_email="true"` are emailed when their severity is in `severities`, which defaults to critical and major. `alertnames` also selects platform alerts without changing their rules. Earlier heartbeat and blackhole routes keep precedence. Email routes preserve the Alerta copy through a preceding route with `continue: true`, and they inherit the root `group_by`, so Alerta and email agree on how many notifications one alert produces. The SMTP password is read from a Secret with a fixed name, `alertmanager-email-password`, mounted through `VMAlertmanager.spec.secrets` and referenced by `auth_password_file` when `authUsername` is set. The platform operator creates that Secret. The name is not configurable, because a tenant-chosen name would let a `Monitoring` writer mount any Secret of the namespace into Alertmanager and send its `password` key to an SMTP host of their choosing, and no tenant role can read those Secrets by any other route. `to` takes an address or a comma-separated list, and `from` takes one address, each with an optional display name. The accepted form is narrower than RFC 5322 on purpose, so that a bad address fails the render instead of being dropped when the first alert fires. The local part must be a dot-atom. A display name is a run of atoms or quoted strings without the specials `net/mail` rejects in a phrase. A domain literal must hold a dotted-quad IPv4 address. IPv6 literals are rejected because their syntax differs across Go mail parser versions. Template actions are rejected, because Alertmanager expands both fields as templates before it parses them. Quoted local parts, comments and a pasted `mailto:` prefix fail the render as well. `smarthost` must be `host:port`, where the host is a host name or a bracketed IP address. A bracketed value must be a valid IPv4 or IPv6 address, so `[deadbeef]:25` and `[1:2:3:4::5:6:7:8]:25` fail the render. The port must be between 1 and 65535 and must not have leading zeros, so `smtp.example.org:0465` fails too. `repeatInterval` is checked with Alertmanager's own duration grammar and limits. Units go in descending order, each at most once, and the value must be non-zero and below the int64 overflow, so what renders is what Alertmanager loads. Partial configuration fails the render and names the fields still set, so clearing the addresses to turn email off tells the operator which leftovers to clear. Subject and body templates are not exposed. Alertmanager parses them per notification, and a broken one surfaces only in a log the tenant cannot read. Checks run for this change. - The monitoring chart suites pass with 228 tests in `packages/system/monitoring` and 16 in `packages/extra/monitoring`. 117 of them are in the two email test files. - 234 address inputs were rendered through the chart. 91 were accepted and 143 rejected. Every accepted input executes as a template and passes `net/mail.ParseAddressList`, including the one-address check for `from`, on Go 1.25.8, 1.26.4 and 1.27.1. - Widening the IPv4 octet from `25[0-5]` to `25[0-9]` fails 4 tests. Putting back the loose `\[[0-9A-Fa-f:.]+\]` domain literal fails 19 tests. Replacing the group-count guard for compressed IPv6 smarthosts with `true` fails the `[1:2:3:4::5:6:7:8]:25` test. - `make generate` with cozyvalues-gen v1.6.0 leaves the tree unchanged. SMTP delivery against a real relay has not been tested. ### Screenshots No dashboard/UI change. ### Downstream repositories Walked the trigger map against the diff. `packages/extra/monitoring` gains values fields, so its README and schema changed. `monitoring` is in the website Makefile `SERVICES` list, and that reference page regenerates from `README.md` automatically on the next stable tag, so there is no manual follow-up to link. The terraform provider does not model the monitoring service, as there is no such resource under `internal/provider`, so there is nothing to update there either. No package is added or renamed, and no CRD, tooling or node contract is touched. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note feat(monitoring): add optional email alerting through alerta.alerts.email, with configurable severity and alert-name selection and an SMTP password read from the fixed Secret alertmanager-email-password ``` |
||
|
Yan Bondarenko
|
17aaf3e038 |
feat(monitoring): add optional email alert delivery
Platform alerts need an email delivery path alongside Alerta without exposing arbitrary namespace Secrets through tenant-controlled values. SMTP credentials therefore use a fixed Secret name. Alertmanager parses the addresses and connects to the SMTP host only when it sends a notification, so a malformed value there drops alerts instead of failing the release. The chart validates these fields when it renders. Addresses must be literal dot-atom mailboxes, and a domain literal must hold a dotted-quad IPv4 address, because IPv6 literal syntax differs across the supported Go mail parsers. A bracketed smarthost must be a valid IPv4 or IPv6 address, and its port must be between 1 and 65535 without leading zeros. The repeat interval follows the Alertmanager duration grammar and range, so a value that renders is one Alertmanager loads. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> Assisted-by: LLM |
||
|
Yan Bondarenko
|
bd2cc0a749 |
test(monitoring): guard alert severities against the Alerta alarm model
Alerta refuses an alert whose severity is not in its alarm model, so a rule with such a label is dropped every time it fires and the alert never reaches anyone. Seven piraeus-datastore rules used warn and the keda at-max rule used info, and nothing in the tree caught either. The contract collects every PrometheusRule and VMRule document under packages, in alerts files and in chart templates, and fails on any alerting rule whose labels.severity is missing or not in the accepted list. Helm actions are stripped before parsing, so a templated rule is still checked, and a file that cannot be parsed fails the run instead of being skipped. Values are compared as whole words. none stays out of the list because the Alertmanager config routes it to a blackhole receiver. Vendored charts under packages/**/charts/** are out of scope because they are re-vendored rather than edited. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> Assisted-by: LLM |
||
|
Yan Bondarenko
|
0cf6e8c98b |
fix(keda): use severity informational instead of info in the at-max alert
Alerta validates the severity label against its alarm model, which has no info in it, so the DatabaseAutoscalerAtMax notification is refused the same way the piraeus warn alerts were. informational is the value the model carries for this level, and the one the other rules in the tree already use. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> Assisted-by: LLM |
||
|
Yan Bondarenko
|
3ade16a380 |
fix(linstor): use severity warning instead of warn in piraeus-datastore alerts
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Pujitha Paladugu
|
e11e239044 |
fix(api): pass the stored object to UpdatedObject in tenantsecret Update
Motivation: a PATCH (kubectl patch, label, edit) against an existing
TenantSecret answered 404 NotFound while GET on the same name succeeded.
REST.Update called objInfo.UpdatedObject(ctx, nil). The apiserver patch
handler builds the patched object from that old object alone, so with nil it
takes its create-new-object branch and returns NotFound from inside the
handler. The same nil also made the patch reach admission as CREATE rather
than UPDATE.
Approach: hand UpdatedObject the stored Secret converted with
secretToTenant when it exists, and nil only when it does not (the
forceCreate path, unchanged), matching what the generic store and the
sibling application and securitygroup registries do.
Validation: added TestUpdate_PassesStoredObjectToUpdatedObjectInfo, which
drives Update with a patch-style UpdatedObjectInfo that fails on a nil old
object. It fails before the change ("UpdatedObject received a nil old
object") and passes after. Ran `go test ./pkg/registry/...` and
`go build ./pkg/... ./cmd/...`, all passing. Not run: a live cluster
kubectl patch, or the real apiserver patch handler.
Known limitation, unchanged by this fix: tenantToSecret prefers Data over
StringData when Data is non-empty, so a patch that only sets stringData on a
secret that already has data is a no-op, exactly as a GET-then-PUT edit of
stringData already is. Patching data or metadata works. That precedence is a
separate design question and is left alone here.
Report: https://github.com/cozystack/cozystack/issues/3601
Assisted-by: LLM
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
|
||
|
Alexey Artamonov
|
2c5c230435 |
fix(postgres-operator): count every scraped empty queue in the WAL backlog rule
The per-pod subquery stepped once a minute and took the latest sample at each step, so with a 30-second scrape it looked at half the samples. A queue that emptied only in a skipped sample did not count, and the rule could page on a cluster that was keeping up. While a recreated pod overlapped its old series, taking the maximum also hid the new series' zero. The minimum of each raw series, then the minimum per pod name, sees every sample and still keeps the window across a new instance label. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Andrey Kolkov
|
f6a615f693 |
fix(backups): scrub legacy plaintext passwords from CNPG Backup snapshots (#4558)
## What this PR does A CNPG `Backup` taken while the Postgres app still accepted `users[].password` copied that password into `status.underlyingResources`, where anyone with `get backups` in the namespace can read it. #4078 stopped new snapshots from carrying the field but left every existing `Backup` as it was (#4179). This adds `LegacyPasswordScrubber`, a leader-elected runnable in the backup strategy controller that, on start, lists `Backup` objects and removes `users.*.password` from every snapshot whose `kind`/`apiVersion` is the CNPG snapshot's. - **Why on every leader start and not a platform migration.** The migration hook runs pre-upgrade, while the old controller is still leader, so a `Backup` the old replica writes during the rollout would keep its password and the migration never re-runs. With `--leader-elect` on, the old replicas stop writing before the new leader's scrub runs, and a `Backup` brought back from an object-level export is cleaned on the next start too. On a clean cluster the whole pass is one List from the manager cache, which `BackupReconciler` already populates. - **Write shape.** The patch is a merge patch computed from the diff, so the request body only nulls the `password` keys and cannot overwrite status another writer changed since the List. The snapshot is handled as generic JSON (numbers kept as `json.Number`), so fields the current snapshot type does not know survive. - **Failure handling.** A `Backup` that cannot be decoded or patched is logged and skipped, and a failed List is logged too; `Start` always returns nil, because a returned error would stop the manager and take every backup down over one object. - **Restore compatibility.** Restore already ignored the key; a test decodes a scrubbed snapshot through `unmarshalCNPGBackupSnapshot` to pin that. - RBAC: `list` and `patch` on `backups.cozystack.io/backups` are already granted to the controller, so the chart is unchanged. - `docs/operations/backup-classes.md` no longer tells operators to delete pre-upgrade `Backup` objects, and says what the scrub does not reach (audit logs, etcd backups, external exports of the objects, Helm history of the init-script Secret), so a password once set through `users[].password` is still treated as exposed until rotated. The runnable carries a `TODO(#4179)` to remove it once upgrading from a release that accepted `users[].password` is no longer supported. Verification: `go test ./internal/backupcontroller/...` and `go build ./cmd/backupstrategy-controller/` are green. The new tests cover scrubbing with everything else kept (including an unknown field with a large integer), leaving non-CNPG, other-apiVersion, clean and snapshot-less Backups unpatched, and continuing past a Backup that fails to decode or patch. Disabling the delete, or returning on the first patch error, makes the tests fail. Not exercised on a live cluster yet. Fixes #4179 ### Screenshots N/A — no UI changes. ### Downstream repositories Walked the trigger map against the diff: it touches only `internal/backupcontroller`, `cmd/backupstrategy-controller` and the in-repo `docs/operations/backup-classes.md`, with no package, values, schema, CRD, variant or asset change. The website carries no "delete pre-upgrade Backups" workaround this would make obsolete; its stale `users[name].password` rows are the #4078 follow-up already noted there. - [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(backups): the backup strategy controller now removes plaintext `users[].password` values left in `status.underlyingResources` of CNPG `Backup` objects taken before managed Postgres stopped accepting user passwords. It runs on every controller start and needs no action. It does not reach copies outside the cluster (audit logs, etcd backups, exported `Backup` objects), so a password once set through `users[].password` should still be treated as exposed until it is rotated. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Security** * At controller startup, plaintext user passwords are removed from matching CNPG backup snapshots. Other snapshot data is preserved. * This cleanup does not affect external copies or remove older init-script manifests from Helm history. Treat passwords previously set through `users[].password` as exposed until rotated. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
941101b65e |
ci: drop unused packages:write from the pull-request workflow
The build, build-talos and finalize jobs each requested `packages: write`, but none of them writes to GHCR. They authenticate to OCIR with OCIR_USER/OCIR_TOKEN and push to REGISTRY, which the workflow sets to OCIR. The only GHCR login in the file is in verify-release-candidate, and that job holds `packages: read`. GHCR pushes elsewhere in CI authenticate with GITHUB_TOKEN plus packages:write rather than a stored secret, so on those paths the scope is the whole credential. Granting it on the pull_request lane hands every same-repo PR run a token with GHCR write authority that the lane has no use for. Whether a package honours that write depends on its Actions access setting, which lives outside this repo, so this is least-privilege hygiene rather than a fix for a demonstrated exploit. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Mattia Eleuteri
|
9afe7f1293 |
fix(kubernetes): assemble the node group once in kubernetes-nodes
The TalosConfigTemplate name is a hash of the machine-config spec, and two
templates have to agree on it: talos-reconcile-job.yaml creates the object,
nodegroup.yaml points MachineDeployment.bootstrap.configRef at it. Sharing the
rendering was not enough — each file also assembled its own $group dict, one
carrying every pool value and one only the few the spec reads, so the two
names agreed only by coincidence of the keys the spec helper happens to read.
Dropping "gpus" from the Job's dict diverged them on a GPU pool with the whole
suite still green, which is the "templates do not exist" deadlock this fix
exists to make impossible.
Move the assembly into a single kubernetes-nodes.group helper, read back with
fromYaml at both call sites, so there is one input as well as one rendering.
Cover it too, since the seam can be reintroduced. The chart's helm-unittest
suites pin each name against its own literal in its own file, which catches a
divergence but reports it as one literal having moved — indistinguishable from
a stale fixture, so re-pasting the new value is the obvious and wrong repair.
hack/check-talosconfigtemplate-name-agreement.bats renders the chart once and
compares the two values against each other, so the failure states the invariant
instead of a number. It asserts on a GPU-shaped pool as well as the default
one: the default renders gpus [], kubelet {} and instanceType "" as falsy, so a
consumer reading a subset of the group is indistinguishable there from one
reading all of it. kernelModules gets a case of its own, because it has no
default and stays unset on both of those shapes. A last case pins that the
shapes hash differently, so the others cannot hold vacuously.
The same guard compares the talos-reconcile Role's resourceNames on
TalosConfigTemplates against that name. The Job applies server-side, and
that is a PATCH on the name even when it creates the object, so a Role
naming anything else leaves the Job Forbidden and the pool without a
template, with the same silent outcome.
Document that talos.registryMirrors and kernelModules sit inside the hashed
spec, so changing either rolls the pool's existing workers instead of only
affecting nodes created later. The kernelModules text described the
fixed-name template as the present state and the content-hash name as a
future one.
No rendered output changes: the golden render snapshots are unchanged.
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
Assisted-by: LLM
|
||
|
Mattia Eleuteri
|
200226686e |
fix(kubernetes): name TalosConfigTemplate by content hash
TalosConfigTemplate.spec is immutable — vtalosconfigtemplate.cluster.x-k8s.io denies any mutation with "TalosConfigTemplate.Spec is immutable" — but the talos-reconcile Job applied it under a fixed <release>-<group> name on every reconcile. Once the template existed, no change to the rendered worker machine config could reach the cluster: the apply was rejected server-side while the HelmRelease stayed Ready. Observed with machine.network.searchDomains after a clusterDomain correction; the same held for certSANs, machine.registries and nameservers. Name the template after a 6-char hash of its rendered spec, the idiom KubevirtMachineTemplate already uses in the same chart, and point MachineDeployment.spec.template.spec.bootstrap.configRef at that name. A content change now creates a new template instead of attempting a rejected mutation, and CAPI rolls the workers onto it. The spec is rendered from one named template shared by the Job (which embeds it in its heredoc) and the MachineDeployment (which hashes it into configRef), so the two cannot compute different names. The runtime values the Job substitutes (CA certificates, Service ClusterIP, tokens) stay shell placeholders and are not hashed, so a certificate rotation does not roll workers. The talos-reconcile Role pins its TalosConfigTemplate get, patch, update and delete rights to the pool's own template, so it names the hashed template too. A server-side apply of a template that does not exist yet is still a PATCH on that name: a rule left on <release>-<group> would leave the Job Forbidden and the pool with no template to scale onto. The replace-on-immutable path is kept and pointed at the same name, because the runtime values sit outside the hash: a new apiserver address still renders a different spec under an unchanged name. Upgrade note: existing clusters roll their Talos workers once, because the configRef moves from <release>-<group> to the hashed name and CAPI creates a new MachineSet for it. The rendered machine config itself is unchanged by this commit. The pre-existing fixed-name template is left in place — the outgoing MachineSet still references it — and is garbage-collected with the KamajiControlPlane when the cluster is deleted. A pool sized by instanceType rather than by explicit resources has a second roll trigger. Its kubelet reservations are resolved through a live lookup of the VirtualMachineClusterInstancetype, so that object's size is part of the hashed spec even though the chart does not own it: a platform-side change to the instancetype rotates the name and rolls the pool again. That is the mechanism working as intended - the reservations really did change - but it is a roll the operator did not initiate, and the KubevirtMachineTemplate hash this mirrors does not have the property, since it embeds the instancetype by name and never its resolved size. Fixes #3515 Signed-off-by: Mattia Eleuteri <mattia@hidora.io> Assisted-by: LLM |
||
|
Aleksei Sviridkin
|
b86fd7aa5b |
fix(platform): make mongodb-application depend on its operator (#3845)
The mongodb chart renders `kind: PerconaServerMongoDB` from `psmdb.percona.com/v1`, but `cozystack.mongodb-application` depended only on `cozystack.networking` and `cozystack.cozystack-engine`. So `mongodb-rd` registers the ApplicationDefinition as soon as the engine is up, a tenant can create a MongoDB before the operator exists, and its HelmRelease then fails with `no matches for kind "PerconaServerMongoDB"`. Every other operator-backed application here already carries the edge. This is the same defect #3817 fixes for etcd. It also closes a test-selection gap. Without the edge `cozystack.mongodb-operator` reaches no runnable suite, so every change to the operator escalates to a run of all suites instead of selecting `mongodb`. See `src_to_suites()` in `hack/select-e2e.sh`. ## Checks `packages/core/platform/tests/sources_mongodb_application_dependson_test.yaml` pins the edge alongside the baseline ones, mirroring the etcd guard. Mutation checked: removing the edge turns it red, restoring it turns it green. `hack/select-install.sh --validate` reports the graph still has no cycle, and all platform helm unit tests pass. ## The exposure this adds An administrator who lists `cozystack.mongodb-operator` in `bundles.disabledPackages` while keeping the application now leaves it `DependenciesNotReady`. That is the same exposure postgres, mariadb, kafka, redis and etcd already carry, with `Package.spec.ignoreDependencies` as the escape hatch. ### Release note ```release-note fix(platform): the mongodb application now waits for the mongodb operator, so a tenant can no longer create a MongoDB before the CRD that serves it exists ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Ensured the MongoDB operator is available before MongoDB application components are registered or deployed. * Preserved required dependencies on networking and the platform engine. * **Tests** * Added coverage verifying the MongoDB application dependency configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Closes #3850 |
||
|
Myasnikov Daniil
|
b8bc0dfd5f |
fix(platform): make mongodb-application depend on its operator
The mongodb chart renders kind: PerconaServerMongoDB from psmdb.percona.com/v1, but cozystack.mongodb-application depended only on networking and cozystack-engine. So mongodb-rd registers the ApplicationDefinition as soon as the engine is up, and a tenant can create a MongoDB before the operator exists; its HelmRelease then fails with no matches for kind "PerconaServerMongoDB". This is the same defect #3817 fixed for etcd. Every other operator-backed app here already carries the edge. It also closes a test-selection gap: without it cozystack.mongodb-operator reaches no runnable suite, so every change to the operator escalates to a run of all suites instead of selecting mongodb. select-install.sh --validate reports the graph still has no cycle. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
d6b38f0a79 |
fix(kubernetes): fail the OIDC bootstrap Job when the tenant CA is missing (#4232)
<!-- 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 In `spec.oidc.mode: System`, the post-install/post-upgrade bootstrap Job patches the chart-owned `<release>-oidc-kubeconfig` Secret with a real kubeconfig built from the tenant admin kubeconfig's CA data. When that CA data was empty, the script logged a WARNING and continued, then exited 0. The hook was satisfied, the HelmRelease reported `Ready`, and the Secret was left holding the install-time placeholder kubeconfig, which cannot authenticate. This PR makes the Job fail (`exit 1`) instead, so the failure surfaces as a red install and can be retried (the Job already has `backoffLimit: 6` and `restartPolicy: OnFailure`) rather than reported as a falsely healthy release. This is the smallest of the three directions raised on the linked issue, and matches how the neighbouring failure mode in the same script already behaves (a failed `kubectl apply` of the ClusterRoleBindings already fails the Job under `set -euo pipefail`). Behaviour for `mode: None` and `mode: CustomConfig` is unchanged, and the common case — CA data present — runs the identical patch commands as before. A regression test was added to `packages/apps/kubernetes/tests/oidc_test.yaml` asserting the rendered Job script contains the `exit 1` on empty `CA_DATA` and no longer contains the old warn-and-continue text. Confirmed it fails against the pre-fix template and passes against the fix. Validation: - `helm unittest .` in `packages/apps/kubernetes` — 150/150 tests pass (up from 149 before the new test). - `helm template ... --show-only templates/oidc-rbac-job.yaml` with representative `System`-mode values renders valid YAML; the extracted bootstrap script passes `bash -n` and `shellcheck -s sh` introduces no new warnings versus the pre-change script. - `make generate` was not run/needed — this change touches neither `values.yaml`, `values.schema.json`, `Chart.yaml`, nor `README.md`. - Not run: a live cluster reproduction (Kamaji-issued admin kubeconfig actually missing CA data at Job run time). The defect is provable from the script logic itself and the added unit test demonstrates the fail-then-pass behaviour; there is no live cluster available in this environment. ### Screenshots Not applicable — no UI change. ### 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. --> - [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: This change is confined to the shell logic inside `packages/apps/kubernetes/templates/oidc-rbac-job.yaml`'s post-install hook Job (a `Values.oidc.mode: System`-only failure path) and its accompanying helm-unittest test. It does not add, rename or remove a package, a `values.yaml` key, a CRD, a namespace, a release-owned object name, or any other surface the trigger map lists, so no downstream repository is affected. ### 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(kubernetes): a System-mode OIDC install no longer reports Ready while the tenant OIDC kubeconfig Secret still holds the install-time placeholder — the bootstrap Job now fails (and retries) instead of only logging a warning when the tenant cluster's CA data isn't available yet. ``` Fixes #3765 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * OIDC setup now fails with an error if the tenant admin kubeconfig is missing certificate authority data, rather than leaving the OIDC kubeconfig Secret at its install-time placeholder. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
6580d2eeb0 |
feat(kubernetes): storage and cloud-controller integration for Proxmox-backed tenants (#4089)
## What this PR does **Series behind #3481** — three separable pull requests: 1. #4087 — the 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 #4088 has merged, so this diff is the eight commits on top of it: `proxmox.pool`, `csi-proxmox`, `ccm-proxmox`, `delete.yaml`, the tests, the e2e suite, one README sentence on token privileges, and the reconcile Job's replace-on-immutable step for existing clusters. The `substrate` value this gates on is on `main`. ### One chain, not two components Storage and the cloud-controller-manager are one chain, not two components: ``` kubelet --cloud-provider=external → taint node.cloudprovider.kubernetes.io/uninitialized → CCM applies topology.kubernetes.io/region|zone + providerID → CSI node plugin starts → PVC binds ``` The CSI node plugin reads region and zone off its own Node and exits fatally without them: ``` F main.go:126] Failed to get region or zone for node: …, region: , zone: , see documentation about topology labels ``` Those labels come from the cloud-controller-manager, and the CCM ignores a Node until the kubelet's `--cloud-provider=external` has tainted it. Shipping the CSI driver alone gives a tenant a StorageClass, a healthy controller, and a node plugin in CrashLoopBackOff. The CSI driver has no equivalent knob for this. Its `features.provider` accepts `default` and `capmox` and validates neither, so an unrecognised value falls through to the default path — the key is therefore not set at all. The cloud-controller-manager is the opposite case: `capmox` is a real mode there and is set, because the default one writes a Node providerID of `proxmox://<node>/<vmid>` while capmox has already written `proxmox://<SMBIOS UUID>` on the Machine. So the CCM has no `enabled` switch and its credentials reference is required: on this substrate, a cluster without a CCM is a cluster where every worker keeps the `uninitialized` taint and nothing schedules. A chart that cannot name the Secret should stop rather than render that. The CSI release depends on cilium only; the CCM is a Deployment of this chart, not a HelmRelease, so nothing in the tenant can depend on it — the node plugin waits on the labels the CCM applies, which is the ordering that matters. ### Packaging | Package | Chart | App | |---|---|---| | `packages/system/proxmox-csi` | proxmox-csi-plugin 0.5.12 | v0.20.0 | The vendored chart is the tenant-side half only. It **drops the upstream `values.talos.yaml` preset**, which pins the workloads onto `node-role.kubernetes.io/control-plane` and `node.cloudprovider.kubernetes.io/platform: nocloud`: a Kamaji tenant has no control-plane nodes at all — its control plane is a Deployment in the management cluster — so that preset leaves the components unschedulable. It is published as a component of the existing `kubevirt` variant of `kubernetes-application` rather than in a new variant: a second variant would rename the artifacts of all eighteen tenant addons, and exactly one component differs between the substrates. The cloud-controller-manager is not a vendored chart. It is a Deployment the `kubernetes` chart renders itself (`templates/ccm-proxmox/controller.yaml`), because it runs here, in the management cluster, and there is nothing of it to install in the tenant. Credentials never reach the tenant. Each controller names a Secret in the tenant's namespace of this cluster (`proxmox.csi.credentialsSecretName`, `proxmox.ccm.credentialsSecretName`), an init container composes the config file from it into a memory-backed emptyDir at start-up, and the `kubernetes` release stores only the Secret's name. They are separate knobs from the capmox credentials, and from each other, so an operator can scope them: attaching disks is a different blast radius from creating VMs, and the CCM only reads inventory. `proxmox.insecure` defaults to `false`; a stand whose Proxmox serves a self-signed certificate sets it to `true`. `proxmox.csi.enabled: false` switches off the controller as well as the tenant release, and needs no CSI token. ### Where the hypervisor token lives, and why it decides the shape Both controllers run in the tenant's namespace of **this** cluster and drive the tenant over its admin kubeconfig. Only the CSI node plugin goes into the tenant, and it mounts no credentials. Installing them into the tenant would hand the token to the tenant. The resource definition lists the admin kubeconfig in `secrets.include`, so a tenant is cluster-admin of its own cluster, and the vendored chart writes the driver's `config.yaml` — token included — wherever it is installed. The privileges the driver documents are granted on `/`, so such a token reaches every VM and every volume on the Proxmox cluster. The KubeVirt path already has the shape that works, in `templates/csi/deploy.yaml`, and the upstream components support it: the CSI controller's `--kubeconfig` is documented for running out of cluster. Three consequences of the split are handled rather than left to be discovered: the vendored chart has no switch for its controller, so the tenant release sets `replicaCount: 0`; its Secret template is gated on a non-empty `config.clusters`, so an empty list keeps the credential out; and `CSIStorageCapacity` is published by the provisioner sidecar, which now owns those objects through a Pod that does not exist in the tenant, so `CSIDriver.storageCapacity` is patched to false by a kustomize postRenderer — the chart hardcodes it, and left true the scheduler refuses every Pod with "node(s) did not have enough free storage". Volumes are owned per cluster. The driver names them `vm-<controllerVmID>-pvc-…` and owns every volume as VMID 9999 unless `controllerVmID` is set, which leaves different clusters' volumes indistinguishable on shared storage. Each cluster sets its own, derived from its tenant namespace and release name, so two tenants' clusters with the same name still get different owners. **This does not make one tenant unable to reach another's volumes.** A tenant can still create a static PV naming a foreign `volumeHandle`, and the controller will attach it, detach it or delete it: the driver checks neither the VMID a node ID names nor the volume a handle names against the cluster it serves. Closing that needs a per-tenant Proxmox scope, and the charts can now produce one: `proxmox.pool` on the `kubernetes-nodes` chart puts every worker VM into a Proxmox resource pool as capmox clones it, replacements included, so one ACL on `/pool/<name>` follows every VM without being granted again. The README states the scope where an operator configuring the chart reads: a storage of the tenant's own (`Datastore.Allocate` on a storage reaches every volume on it), `VM.Audit`/`VM.Config.Disk` on that pool for the CSI controller, `VM.Audit` and `VM.GuestAgent.Audit` on it for the CCM — a NotReady Node whose VM the token cannot see is reported as gone and deleted — and what that scope still leaves open: the tenant's own disks, which a cluster-admin of the tenant could reach anyway. ### Deleting a cluster frees its disks The pre-delete hook deletes the CSI driver in Step 2, and a volume is freed only by the driver answering DeleteVolume, so a Step 1b runs first: it deletes the tenant's claims (`--wait=false`; a claim in use carries `kubernetes.io/pvc-protection`, so it clears once nothing mounts it), then the Pods holding each claim, then waits for the PVs rather than the PVCs. The Pods are found by listing every Pod with the claims it mounts and matching in the shell: a JSONPath filter on the claim name fails for a whole namespace as soon as one Pod there mounts two claims, and would find no Pod at all. The claims go first because a claim being deleted is one the scheduler places no new Pod on, so a Pod that a StatefulSet brings back cannot mount it again. Without the step the disks stayed on the hypervisor with the tenant's data in them and no PV object left to name their owner. A tenant apiserver that does not answer is told apart from a tenant with no claims: the first goes through the hook's backstop and shows in its summary, the second is the clean case. The probe is bounded with `timeout`, not `--request-timeout`, which the repo's hook check refuses because a non-zero value diverts an in-cluster kubectl call to localhost:8080. On the KubeVirt path two things change. The pre-delete hook's Step 5 checks that the DataVolume CRD exists before deleting DataVolumes, so a hook that runs where CDI is not installed no longer fails after every earlier step has run; the hook runs only at deletion. The worker pool's talos-reconcile Job changes at upgrade: its Role's TalosConfigTemplate rights are narrowed to the pool's own template and gain a named `delete`, and its script gains the replace step, which moves the Job's content-hash name. Every existing worker pool therefore gets a new reconcile Job when it upgrades. Where a pool's live TalosConfigTemplate still matches the render, that Job changes nothing; where it has drifted, the Job now deletes and recreates it instead of failing. Machines already running keep the bootstrap data they joined with; Machines created afterwards boot from the current template. ### Existing clusters: the TalosConfigTemplate CABPT will not let us patch CABPT's validating webhook rejects any change to `TalosConfigTemplate.spec` (`TalosConfigTemplate.Spec is immutable`). The kubelet flag this PR adds changes that spec, so on a cluster created before it the reconcile Job's `kubectl apply` fails with `is immutable` on every run, and a pool whose `ProxmoxMachineTemplate` also changed (say, `proxmox.pool` set) rolls its workers off the old template: they join without `--cloud-provider=external`, the CCM leaves them unmanaged, and the CSI node plugin exits on the missing zone label. The last commit makes the Job replace the object when apply reports the spec immutable: delete, apply again, log both. Existing Machines keep their bootstrap data; new ones are built from the replaced template. Pinned by `talos_tct_immutable_test.yaml`, and the RBAC the Job runs with covers exactly that delete (`resourceNames` of its own template). ### Evidence ``` $ KUBECONFIG=<tenant> kubectl get node -o custom-columns=… kubernetes-px3-md0-rp4s5-rgr48 prox-home prox-home proxmox://b903f905-7b4f-4048-b1b6-3e8f58530d8f $ kubectl -n tenant-proxmox3 get deploy kubernetes-px3-pcsi-controller 1/1 kubernetes-px3-pccm 1/1 $ KUBECONFIG=<tenant> kubectl get pods -n csi-proxmox csi-plugin-node 3/3 Running (the controller Deployment has 0 replicas) $ KUBECONFIG=<tenant> kubectl get secret -A | grep -c token_secret 0 $ KUBECONFIG=<tenant> kubectl get pvc csi-probe csi-probe Bound pvc-… 1Gi RWO proxmox ``` That the volume is real, not merely `Bound` in the API, and that it is this tenant's: ``` $ qm config <vmid> scsi1: main-pool:vm-798497-pvc-…,size=1G ``` Deleting the PVC frees the disk from the storage; the e2e probe checks that too. ### Known limitation The CCM logs every five minutes on PVE 9: ``` error get ha-groups 500 cannot index groups: ha groups have been migrated to rules ``` Proxmox 9 changed the HA-groups API. Node initialization is unaffected — labels and providerID are applied — but the CCM image v0.15.0's HA-groups lookup is not compatible with PVE 9. ### Testing `make unit-tests` — exit 0. `apps/kubernetes` 28 suites / 338 tests; `apps/kubernetes-nodes` 33 suites / 182 tests / 12 snapshots. Each of the eight commits passes both suites on its own. What the new asserts pin, each shown to go red on the mutation it exists for: the tenant release carries no credentials and no `valuesFrom`; the controllers run here with the Secret mounted and the tenant as their target; `csi.enabled: false` renders no CSI controller and needs no CSI token; the kubelet runs with `cloud-provider: external` on proxmox and without it on kubevirt; Step 1b and the cleanup Role's admin-kubeconfig entry render on proxmox and not on kubevirt, and Step 1b deletes the claims before the Pods; `proxmox.pool` reaches the `ProxmoxMachineTemplate` and an empty one renders no `pool`; each tenant gets its own volume owner id; the CCM runs in capmox mode; the CCM tolerates exactly two taints. `proxmox.ccm`, `proxmox.csi` and `proxmox.insecure` are optional in the schema — each has a default, and the render enforces what the substrate needs — so a `Kubernetes` object written against `main`'s `proxmox` block stays valid; the API review gate reports no sizeable change at any of the commits. The e2e suite ships `.disabled` and no CI lane runs it, so the substrate has no automated coverage until a runner with the `proxmox` label exists. What is in the tree is runnable: the fixture is rendered by `run-proxmox.sh` with explicit defaults (GNU `envsubst` does not expand `${VAR:-default}`, so the defaults live in the script and the render refuses a fixture with a variable left in it — pinned by `hack/proxmox-e2e-fixture_test.bats`), the runner reads the `.conf` kubeconfig a runner outside the management cluster can reach, the StorageClass and the PVC probe read the same storage variable, the probe checks the disk is gone after the claim is deleted, `keep_tenant` is honoured by the suite's own teardown, the workflow fails on an unset stand variable instead of falling back to a hardcoded address, and its schedule runs only where `vars.PROXMOX_NIGHTLY` is set, since the job needs a self-hosted runner with the `proxmox` label. The fixture takes the stand's resource pool through `COZY_PVE_POOL`. ### 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/terraform-provider-cozystack — matches, no follow-up yet.** - `packages/apps/kubernetes/values.schema.json` gains `proxmox.ccm.credentialsSecretName`, `proxmox.csi` (`enabled`, `credentialsSecretName`, `storageClasses[]`) and `proxmox.insecure`, with defaults in `values.yaml`. Same coupling as the substrate PR: hand-written schema, model and expand/flatten, and defaults the provider restates and sends itself. **cozystack/website — unclear, deliberately left unticked.** This PR adds one package under `packages/system/`, not under `packages/apps/` or `packages/extra/`, so the app-list trigger does not fire. Whether `proxmox-csi` counts as a "platform component" for `guides/platform-stack/_index.md` is a judgement about that page's scope: it is not part of the platform stack — it is an addon installed *into* a tenant cluster, like the KubeVirt CSI driver, which has no entry there either. I read that as no, but I am not confident enough to decide it for you, so the box stays empty. The README section on token privileges and per-tenant scope is the kind of text `operations/` pages carry; that follows the decision #3481 asks for. **Everything else: no match.** No app package added or renamed; no version enum, `kind`, `plural` or `release.prefix` change; `packages/core/platform/values.yaml` untouched; `packages/core/platform/sources/kubernetes-application.yaml` gains one component but is not a renamed `*-rd` file (ccp); `hack/` gains `e2e-chainsaw/_lib/run-proxmox.sh`, a suite directory shipped `.disabled`, and `proxmox-e2e-fixture_test.bats`, which runs with the other bats files; no `ApplicationDefinition` semantics or `release.chartRef.kind` enum change (external-apps-example); no node prerequisites; no telemetry metric; no commit-convention change. - [ ] 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(kubernetes): add the Proxmox CSI driver and cloud-controller-manager for tenant clusters on the `proxmox` substrate, giving them PersistentVolumes backed by Proxmox storage. The cloud-controller-manager is not optional there: worker kubelets run with `--cloud-provider=external`, so without it every node keeps the `uninitialized` taint and the CSI node plugin cannot start. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## New Features - Added Proxmox VE as an infrastructure option alongside KubeVirt for Kubernetes clusters and worker pools. - Added configuration for Proxmox networking, DNS, VM templates, storage, node placement, credentials, IPv4 allocation, and storage classes. - Added Proxmox cloud-controller-manager and CSI integrations. - Added validation for required Proxmox settings and unsupported combinations. ## Bug Fixes - Improved recovery when Talos configuration templates cannot be updated because they are immutable. ## Documentation - Documented Proxmox configuration, requirements, and supported parameters. ## Tests - Added coverage for Proxmox rendering, sizing, integrations, validation, and KubeVirt compatibility. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ### Verified on a live stand Run against three throwaway tenants on Proxmox VE 9.2, each built and destroyed: - every Secret in the tenant cluster scanned for the token — none found, while the PVC round-trip works; - a PVC bound, mounted, written and read back, its disk visible on the hypervisor as `vm-798497-pvc-…` rather than `vm-9999-…`; - `helm uninstall` with the volume still mounted by a running Pod, after which the disk is gone from the storage with nothing done by hand. What that run pinned: the CCM's capmox mode, capacity-aware scheduling off with `CSIDriver.storageCapacity` patched, and the teardown Role's admin-kubeconfig entry. A second run, on the head of this PR, used a pool per tenant and two privilege-separated tokens holding exactly the README's rows, on a throwaway tenant and then on the stand's long-lived cluster upgraded in place: - the worker VM lands in the pool as capmox clones it, and so does its replacement after the Machine is deleted — the CCM initialises the new node and the CSI attaches a disk to it with no ACL touched; - no `401` or `403` from the hypervisor in the CSI controller or the CCM over the whole run, once the roles were granted to the user as well as the token; - the PVC round-trip on both clusters, the disk gone from the storage after the claim; `helm uninstall` with a claim held by a running Pod deletes the claim before the Pod and waits for the volume to be released; - the in-place upgrade replaced the immutable template on the first apply conflict and the re-created worker came up with `--cloud-provider=external`. |
||
|
Pujitha Paladugu
|
4fe45eba32 |
fix(kubernetes): fail the OIDC bootstrap Job when the tenant CA is missing
In mode=System the post-install hook Job patches the chart-owned <release>-oidc-kubeconfig Secret with a real kubeconfig built from the tenant admin kubeconfig's CA data. When that CA data was empty the script only warned and exited 0, so the hook was satisfied, the HelmRelease reported Ready, and the Secret kept the install-time placeholder kubeconfig, which cannot authenticate. The failure only showed up when a user tried to use the downloaded kubeconfig. Exit 1 instead, so the install fails visibly and is retried. The Job already has backoffLimit: 6 and restartPolicy: OnFailure, and a failed ClusterRoleBinding apply in the same script already fails the Job the same way. Assisted-by: LLM Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> |
||
|
Marian Koreniuk
|
e5dc086338 |
fix(kubernetes-nodes): replace the TalosConfigTemplate CABPT's webhook will not let us patch
CABPT's validating webhook rejects any change to TalosConfigTemplate.spec
(TalosConfigTemplate.Spec is immutable), so the worker reconcile Job's
`kubectl apply` fails the moment any input the render depends on genuinely
changes. The apiserver address is the one that moves in practice: the Job
writes it into certSANs and into the worker's endpoint, and a cluster that
gets a new address leaves the Job dead with
TalosConfigTemplate.bootstrap.cluster.x-k8s.io "…" is invalid:
spec: Invalid value: …: Spec is immutable
from then on, until somebody deletes the object by hand. Nothing in the
cluster says that is what has to happen.
The Job now recognises that one failure and replaces the object: delete, then
apply the same manifest. Every other apply error still stops the Job, because a
delete followed by a create that fails for the same reason — an apply rejected
for its content, say — would leave the pool with no template, unable to scale
until the render is fixed. The replace itself does not roll the pool: Machines
reference the template by name, and the name does not change. The match is on
the message for that reason, and the Role's new `delete` is bound to this pool's
own template by name, so the Job cannot reach a sibling pool's.
While the Role is being touched anyway, its other TalosConfigTemplate rights are
pinned to the same name. `get`, `patch` and `update` were unnamed, and several
clusters can share a tenant namespace: a Job that can patch a sibling pool's
template can rewrite the machineconfig that pool's next workers boot. `create`
stays unnamed because RBAC matches resourceNames only on verbs that name an
object.
The manifest is captured into a variable rather than written to a file: the
container runs with readOnlyRootFilesystem, and one variable also means the
replace applies exactly what the first attempt did.
Both applies stay server-side, as the Job's single apply already is: a
client-side merge pulls the full OpenAPI schema and runs the Job out of its
256Mi. The webhook rejects a spec change the same way under server-side apply,
so the `is immutable` match still fires.
UPGRADE NOTE. This changes the KubeVirt path as well, and the render with it:
the talos-reconcile Role gains the `delete` rule, its TalosConfigTemplate rights
are narrowed to this pool's own object, and the Job's script changes,
which moves the Job's content-hash name. Every existing worker pool therefore
gets a new reconcile Job on upgrade. On a pool whose live TalosConfigTemplate
no longer matches the current render, that Job will now delete and recreate it
instead of failing. Machines already running are unaffected — their bootstrap
data was rendered when they joined — but Machines created afterwards boot the
new machineconfig. On a pool where template and render agree, nothing happens.
Both new asserts were checked against the mutation they exist for: deleting the
`exit 1` from the `*)` branch and pointing the delete at `${RELEASE}` each turn
the matching test red. A bare `exit 1` pattern does not — the input-timeout path
near the top of the script already has one.
Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua>
Assisted-by: LLM
|
||
|
Marian Koreniuk
|
5be222ad75 |
docs(kubernetes): say the token's user needs the pool privileges too
A privilege-separated Proxmox token never exceeds its user's permissions. Granting the README's roles to the token alone left the CSI and CCM tokens with an empty effective set on the pool and the storage; granting the same roles to the user made the token's own ACL the effective one. Seen on Proxmox VE 9.2.20 while running the storage pair against a pool-scoped user, and worth one sentence next to the table so nobody repeats it. Assisted-by: LLM Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> |
||
|
Marian Koreniuk
|
7500a9fbae |
test(kubernetes): add the Proxmox e2e suite, parked disabled
Upstream CI has no hypervisor, so this suite cannot run there; it ships
`.disabled` and is run by a lane that owns a Proxmox stand.
Named for what the chart actually creates. The resource definition prefixes the
release, so a Kubernetes CR called k8s-e2e produces
ProxmoxCluster/kubernetes-k8s-e2e in the tenant's own namespace, and the admin
kubeconfig Secret is kubernetes-k8s-e2e-admin-kubeconfig.
Stand-specific values are variables rather than hardcoded. Every one of them is
a bare ${COZY_PVE_*} that run-proxmox.sh expands with envsubst, supplying the
defaults itself — GNU envsubst leaves a ${VAR:-default} in the text exactly as
it is, set or not, and that reaches the apiserver as a string where the schema
wants an integer — and refusing a fixture with a variable still in it. The
StorageClass and the PVC probe read the same COZY_PROXMOX_STORAGE, so the volume
lands where the probe looks. The runner reads the tenant kubeconfig's .conf key,
the address a runner outside the management cluster can reach.
The stand's resource pool comes in through COZY_PVE_POOL, empty by default.
hack/proxmox-e2e-fixture_test.bats pins the shape of the fixture and the render.
The PVC probe reads the storage again after the claim is deleted: a deleted
claim whose disk is still there is the leak the pre-delete hook's Step 1b
exists to prevent. keep_tenant is honoured by the suite's own teardown, not
only by the workflow's. The workflow carries no fallback for the stand's
variables: an unset one fails the job before a key is sent anywhere. The
schedule runs only where vars.PROXMOX_NIGHTLY is true: the job targets a
self-hosted runner with the `proxmox` label, and a scheduled run on a repository
without one would sit in the queue until GitHub cancels it.
The suite also asserts the half of the deployment that lives in the management
cluster — the CSI controller and the cloud-controller-manager — because that is
the property the pair is designed around and a tenant-only assertion would pass
with the token in the wrong place.
Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua>
Assisted-by: LLM
|
||
|
Marian Koreniuk
|
36b04218d7 |
test(kubernetes): assert where the hypervisor token is and where it is not
Every assert here was written against the mutation it exists for, and each was checked to turn red on it and green without: putting the controller back in the tenant, giving the tenant a non-empty config.clusters, pointing a sidecar at the wrong kubeconfig, restoring the blanket toleration, hardcoding the driver's default owner id, dropping capmox mode, and letting the controller ignore proxmox.csi.enabled. The negations carry `any: true`. Without it a negated containsDocument passes whether or not the document is in the render. Two of the cases are about objects rather than values: the tenant release must carry no valuesFrom at all, and two clusters must derive different volume owner ids. Both are properties a reader can check against the rendered output, which is why they are asserted that way rather than by reading the templates. Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> Assisted-by: LLM |
||
|
Marian Koreniuk
|
5ab36023e0 |
fix(kubernetes): release Proxmox volumes before the driver that frees them goes
Deleting a tenant cluster left its PVC disks on the hypervisor. The pre-delete
hook deletes every HelmRelease labelled with the cluster in Step 2, the CSI
driver among them, and nothing before that deleted the tenant's PVCs — a volume
is freed only by the driver answering DeleteVolume. capmox then destroys the
worker VMs, and Proxmox frees only what the destroyed VMID owns, which these
volumes are not. They stayed on storage with the tenant's data in them, and the
PV objects that could have named their owner went with the control plane.
Step 1b deletes the claims while the driver is still up, and it deletes the
claims before the Pods that hold them: a claim being deleted is one the
scheduler places no new Pod on, so a Pod that a StatefulSet or Deployment brings
back cannot mount it again and hold it to the backstop. The Pods go second: a
claim in use carries the kubernetes.io/pvc-protection finalizer, so it clears
only once nothing mounts it, and the Pods are going with the cluster in any
case. Then it waits for the PVs rather than the PVCs, because the claim
disappears as soon as its finalizer clears while the disk is freed only when the
PV is gone.
The Pods holding a claim are found by listing every Pod with the claims it
mounts and matching in the shell. A JSONPath filter on the claim name cannot do
it: with a Pod that mounts two claims its left side yields two values, client-go
refuses the whole expression ("can only compare one element at a time"), and no
Pod in that namespace would be found for any claim.
A kubeconfig that exists is not an apiserver that answers, and listing claims
against one that does not comes back empty — the clean case. Step 1b asks
/readyz first and sends an apiserver that stays silent through the backstop, so
the summary carries it. The probe is bounded by `timeout` rather than by
--request-timeout: a non-zero --request-timeout on an in-cluster kubectl call
diverts it to localhost:8080, which is why hack/check-hook-request-timeout.bats
refuses the flag in hook templates, and the check cannot tell this call, which
carries its own kubeconfig, from one that does not.
Best effort, and loud about it: a tenant that never came up has no apiserver to
ask, and a teardown must not wedge on that, but a timeout is reported through
the same backstop the other steps use.
The hook's Role gains one Secret by name — the tenant's admin kubeconfig, which
this step needs. Without it the step takes its no-kubeconfig branch and reports
that there is nothing to release while the volumes stay behind.
Step 5 no longer fails the whole hook where CDI is not installed. It deletes
DataVolumes, which are a KubeVirt-path component, and kubectl exits non-zero on
an unknown resource type; under set -e that failed the teardown after every
earlier step had already run, leaving the cluster half removed and the hook
reporting failure.
Verified on a throwaway tenant: a PVC bound and mounted by a running Pod, then
`helm uninstall`, and the volume freed by the driver rather than by hand.
delete_hook_test.yaml pins Step 1b and the Role's admin-kubeconfig entry on
proxmox and their absence on kubevirt, the /readyz probe, and the order of the
claim and Pod deletes.
Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua>
Assisted-by: LLM
|
||
|
Marian Koreniuk
|
d902421f47 |
feat(kubernetes): deploy the Proxmox cloud-controller-manager with the cluster
On this substrate the worker kubelet runs with --cloud-provider=external, which taints every node node.cloudprovider.kubernetes.io/uninitialized until a cloud provider clears it. Without a cloud-controller-manager the taint is permanent: nothing but tolerating DaemonSets schedules, and the CSI node plugin — which reads topology.kubernetes.io/region and zone off its own Node and exits fatally without them — never starts. It runs in the management cluster for the same reason as the CSI controller beside it: it authenticates to the hypervisor, and the tenant is cluster-admin of its own cluster. Its whole job is to write on the tenant's Nodes, and it does that over the tenant's admin kubeconfig from out of cluster, which also means there is no tenant-side RBAC left to install. capmox mode, not the default one. The default resolves a node to its VM by name and writes providerID as proxmox://<node>/<vmid>, while capmox reads the CAPI Machine and writes proxmox://<SMBIOS UUID> — the form capmox itself puts on the Machine. With the default the Node and its Machine carry two different identifiers for the same VM. The tolerations are spelled out rather than a blanket `operator: Exists`. That one also matches node.kubernetes.io/unreachable:NoExecute with no tolerationSeconds, which stops admission adding the bounded 300s one and lets the Pod outlive the Node it runs on. `proxmox.insecure` defaults to false: the controllers verify the hypervisor's certificate unless an operator turns that off, and both run here, so the value describes what the management cluster trusts, not the tenant. The README gains a section on what the two tokens can reach — the controller acts on whatever VMID and volume handle the tenant's objects name — and on the per-tenant Proxmox scope that contains it: a storage of the tenant's own and one ACL on the resource pool proxmox.pool puts its workers in, which follows every VM capmox creates — VM.Audit there for the CCM too, which deletes a NotReady Node whose VM it cannot see. ccm, csi and insecure are optional in the schema. Each has a default, and what the substrate needs is enforced at render time — a proxmox cluster without ccm.credentialsSecretName does not render — so a Kubernetes object written before these fields existed stays valid, and adding them is not a breaking change to the API. The kubelet flag is asserted on both substrates: present on proxmox, absent on kubevirt, where nothing would ever clear the taint it causes. Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> Assisted-by: LLM |
||
|
Marian Koreniuk
|
93c89c8bf6 |
feat(kubernetes): add the Proxmox CSI driver, with its controller in this cluster
Tenant clusters on the proxmox substrate get persistent volumes backed by the
hypervisor's storage. The driver is sergelogvinov/proxmox-csi-plugin, vendored
as a chart that reproduces byte-for-byte from `helm pull`.
It is deployed in halves, and which half goes where is the whole design. The
controller authenticates to Proxmox, so it runs HERE, in the tenant's namespace
of the management cluster, and drives the tenant over its admin kubeconfig —
the same shape templates/csi/deploy.yaml already uses for kubevirt-csi. The
tenant gets the node plugin, the CSIDriver object and the storage classes; its
DaemonSet mounts no credentials.
Putting the controller in the tenant would hand the token to the tenant. The
resource definition lists the admin kubeconfig in secrets.include, so the
tenant is cluster-admin of its own cluster and can read any Secret there, and
the chart writes the driver's config.yaml — token included — wherever it is
installed. The privileges the driver documents are granted on `/`, so that
token reaches every VM and every volume on the Proxmox cluster, not just this
tenant's.
Three consequences of the split are handled here rather than discovered later:
- The vendored chart has no switch for its controller, so the tenant release
sets replicaCount: 0. The Deployment object still renders; no Pod does.
- Its Secret template is gated on a non-empty config.clusters, so an empty
list is what keeps the credential out.
- CSIStorageCapacity is published by the provisioner sidecar, which now runs
here and owns those objects through a Pod that does not exist in the
tenant. Left on, CSIDriver.storageCapacity=true and the scheduler refuses
every Pod with "node(s) did not have enough free storage". The chart
hardcodes that field, so a kustomize postRenderer patches it rather than
forking the chart.
The controller follows proxmox.csi.enabled, like the tenant release: with the
driver switched off there is nothing for it to serve and no token to hold.
Volumes are owned per tenant. The driver names them vm-<controllerVmID>-pvc-…
and defaults the id to 9999; on shared storage that made every tenant's volumes
indistinguishable to an operator and to a VM-level ACL. Each cluster derives its
own from its release name.
What this does not do is make one tenant unable to reach another's volumes: a
tenant can still create a static PV naming a foreign volumeHandle. That needs a
per-tenant Proxmox scope — a dedicated storage and a VM ACL per tenant — which
is a deployment requirement rather than a chart change.
Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua>
Assisted-by: LLM
|
||
|
Marian Koreniuk
|
fc729299f4 |
feat(kubernetes-nodes): put Proxmox workers into a resource pool
The privileges a Proxmox token needs — VM.Audit and VM.Config.Disk for the CSI controller, VM.Audit and VM.GuestAgent.Audit for the cloud-controller-manager — are granted on an ACL path, and the only path that follows every VM capmox creates, replacements included, is a resource pool. Without one the choices are `/`, which reaches every VM on the hypervisor, or each VMID, which has to be granted again after every scale-up and every Machine replacement, and until it is the new worker gets no disks and no providerID. proxmox.pool names the pool; capmox adds each worker to it as it clones the VM (ProxmoxMachineTemplate.spec.template.spec.pool). The pool has to exist on the hypervisor — capmox does not create it — and every node pool of a tenant cluster sets the same one, or a worker outside it is a worker the tenant's tokens cannot reach. Empty adds the workers to no pool, which is what the chart did before. Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> Assisted-by: LLM |
||
|
Aleksei Sviridkin
|
7d44372a16 |
fix(postgres): read the live Cluster when deciding the WAL-archive prefix (#4299)
<!-- 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 Fixes #4298. `applyClusterPluginBackup` preserves the `serverName` already on the Cluster so that a backup does not flip the WAL-archive prefix mid-stream, which would split the archive across two prefixes. That rule is right; the read it rests on is not reliable in one window. The value was read through the manager's cached client, and `BackupJobReconciler.SetupWithManager` registers `For(&BackupJob{})` and nothing else, so a Cluster only reaches the cache through a lazily started informer with no resync tied to the Cluster's own changes. An in-place `RestoreJob` deletes the Cluster and the chart re-renders it with `bootstrap.newServerName` seconds later, so a read served during that window describes the object that was purged — and the source's `serverName` is written back over the freshly rendered one. `barman-cloud-check-wal-archive` then refuses the destination with `Expected empty archive` and the `RestoreJob` ends on its 30-minute deadline, which is the failure `newServerName` was added in #3988 to prevent. The same stale read carries a stale `UID`, and that `UID` owns the `ObjectStore` this function applies, so the store is garbage-collected as soon as it is written. Reading through the dynamic client this reconciler already carries keeps the preservation rule and makes the value it rests on truthful. There is precedent in the same file: the HelmRelease suspend/resume path reads that way for the same reason. Observed on a v1.6.3 cluster, twice on separate runs with the same four-second gap between the chart's apply and the driver's. The regression test makes the two clients disagree the way they disagree in that window and asserts the driver believes the live one; reverting the fix fails it on the serverName assertion. ### Screenshots <!-- 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. --> - [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 <!-- 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(postgres): an in-place restore no longer loses its fresh WAL-archive prefix to a stale Cluster read, which left the restored cluster archiving onto the source's prefix and the RestoreJob failing on its deadline ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Backup processing now uses the latest CNPG Cluster state rather than potentially outdated cached information. This ensures the cluster’s current server name and identity are used when associating backup ObjectStore resources, including when the cached and live cluster details differ. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
08c344ab5f |
feat(vpc): expose allowSubnets per subnet (#4478)
<!-- 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
Adds an optional `allowSubnets` list to each entry of `subnets` in the
VirtualPrivateCloud chart, rendered into `spec.allowSubnets` of the
kube-ovn `Subnet`.
Every VPC subnet is rendered `private: true`, so kube-ovn drops any
packet whose source is outside the subnet's own CIDR. A tenant who runs
a VPN or router VM on a subnet already gets the forward path through
`routes` (a static route with the VM as `nextHopIP`), but traffic
arriving from the remote network is dropped by the private ACL. Today
the only fix is an operator patching the cluster-scoped `Subnet` by
hand: the tenant cannot do it, no application value records it, and it
is lost whenever the subnet is recreated.
```yaml
spec:
subnets:
- name: gateway
cidr: 172.16.1.0/24
allowSubnets:
- 192.168.1.0/24
routes:
- cidr: 192.168.1.0/24
nextHopIP: 172.16.1.10
```
The subnet stays private; the list only widens the ingress ACL of the
subnet that declares it, inside the tenant's own VPC. An absent or empty
list renders nothing, so existing releases produce the same manifests.
The chart had no helm-unittest suite, so this adds a `test` target and
`tests/subnet_test.yaml`, covering a subnet without the field, one with
it, the field staying scoped to its own subnet, and an empty list. Two
of the four cases fail against the template on `main`.
`allowSubnets` is a kube-ovn field. #4034 teaches this chart to render
Cozyplane objects when kube-ovn is absent, so whichever of the two lands
second has to decide how the other backend treats the list.
### Screenshots
<!-- 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.
-->
- [ ] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [x]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up: cozystack/terraform-provider-cozystack#61 (the provider
models `subnets` by hand in `vpc_schema.go` and `vpc_model.go`, so the
new nested field needs a schema and expand/flatten change there; an
issue rather than a PR, since it should follow this one merging)
- [ ]
[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
feat(vpc): subnets accept an optional `allowSubnets` list of CIDRs admitted into the private subnet, so a VM on the subnet can act as a VPN or router gateway without a manual patch of the kube-ovn Subnet.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Private subnets can now allow additional source CIDR ranges, such as
remote networks routed through a gateway. This setting is optional and
can be configured for individual subnets.
* **Tests**
* Added checks for rendering subnet CIDRs, including cases where the
setting is omitted or applied to only one subnet.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
