mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 22:13:18 +09:00
4e5b76f2820cf805ced8ced4010fe1883ccd7d70
276
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
1f35c26ece |
fix(lineage, kubernetes): resolve CAPK node VMs to their cluster (#5124)
<!-- 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
The virt-launcher pods of Kubernetes worker nodes are never marked as
managed by the lineage webhook, so anything that relies on the webhook
having resolved them misses them. The visible symptom: a SecurityGroup
attached to a `Kubernetes` application labels the control-plane pods but
never the worker nodes, because the securitygroup-controller caches only
pods carrying `internal.cozystack.io/managed-by-cozystack=true`.
**Root cause.** The webhook is not skipped for these pods (they do not
carry the managed-by label, so the `objectSelector` lets them through);
it runs and fails to resolve their owner chain. The walk goes Pod →
VirtualMachineInstance → VirtualMachine and stops there: the Cluster API
KubeVirt provider creates the VirtualMachine with no ownerReference and
links it to its KubevirtMachine through the
`capk.cluster.x-k8s.io/kubevirt-machine-{name,namespace}` labels only.
With no ownerReference, no HelmRelease kind and no
`helm.toolkit.fluxcd.io/name` label on the VM, `WalkOwnershipGraph`
returns no ancestor, `computeLabels` returns nothing and the pod is
admitted unmanaged. The
`apps.cozystack.io/application.{group,kind,name}` labels the pods do
carry come from the `KubevirtMachineTemplate` in
`packages/apps/kubernetes-nodes/templates/nodegroup.yaml`, not from the
webhook.
**Measured on a v1.6.5 cluster.** Every Kubernetes cluster on it showed
the same shape (three clusters across two tenants). For one worker node:
the pod carries the three lineage labels but not `managed-by-cozystack`;
the pod is owned by its VMI and the VMI by its VM; the VM has no
ownerReferences, and its labels include
`capk.cluster.x-k8s.io/kubevirt-machine-name` and
`capk.cluster.x-k8s.io/kubevirt-machine-namespace` pointing at a
KubevirtMachine in the same namespace. From there ownerReferences run
KubevirtMachine → Machine → MachineSet → MachineDeployment → Cluster,
and the CAPI Cluster carries `helm.toolkit.fluxcd.io/name:
kubernetes-<name>`. The bridge is one hop. Setting
`managed-by-cozystack=true` by hand on a worker's virt-launcher pod made
the securitygroup-controller stamp the membership label within ten
seconds.
**Fix.** `WalkOwnershipGraph` now follows a VirtualMachine without
ownerReferences to the KubevirtMachine named by its CAPK labels, and
keeps walking from there. Node pods then resolve to the parent
`Kubernetes` application, which is the attribution the machine template
already hard-codes, and they get `managed-by-cozystack` like every other
managed pod. The bridge is only taken when the label names the VM's own
namespace: CAPK can target an external infrastructure cluster, and the
walk must never attribute an object across namespaces. The webhook's
ClusterRole already grants `get` on every resource, so no RBAC change is
needed.
**Why at the source rather than in the consumer.** Relaxing the
securitygroup-controller's pod cache to
`apps.cozystack.io/application.kind` Exists would also have fixed the
symptom, but it would leave every other consumer of the managed-by label
blind to node pods, and it would rest membership on labels the webhook
never vouched for. Stamping `managed-by-cozystack` from the machine
template was ruled out too: the `objectSelector` would then skip these
pods entirely, and they would lose the scheduling-class injection the
webhook performs on every pod.
**Blast radius.** Only objects below a CAPK VM change: the VMI, the
virt-launcher pods and the PVCs of the VM's `dataVolumeTemplates` now
get the lineage labels of their Kubernetes application. None of them
match a resource selector of the `Kubernetes` ApplicationDefinition, so
they get `tenantresource=false`. Nothing selects PVCs on
`application.kind=Kubernetes` (the Velero strategy selects `VMDisk`).
The `-userdata` cloud-init Secret CAPK creates is not affected either
way: at the pinned provider commit, `reconcileKubevirtBootstrapSecret`
creates it with the bootstrap Secret's labels and no ownerReferences, so
the walk reaches neither the VM nor the KubevirtMachine from it, before
or after this change. Existing node pods pick the labels up on their
next admission (recreation, live migration, or any update).
**Tests.** `pkg/lineage/capk_test.go` builds the observed chain in a
fake dynamic client and checks that a worker's virt-launcher pod
resolves to `Kubernetes/cluster-a`, that a VM pointing at a
KubevirtMachine in another namespace is not followed, that a VM without
CAPK labels still has no ancestor, and, in a table of negative cases,
that the CAPK labels are ignored on any object other than a
`kubevirt.io/v1` VirtualMachine (another kind of the `kubevirt.io`
group, a VirtualMachineInstance, a VirtualMachine of another group or
version). Weakening the kind and API version guard to match on either
field alone fails all four cases. With the bridge disabled, the first
test fails. No chainsaw case: the only suites that bring up worker VMs
are `kubernetes-latest` and `kubernetes-previous`, whose shared runner
script is already over its time budget, and the unit test covers the
walk exactly as it runs in the webhook.
### 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(lineage, kubernetes): the virt-launcher pods of Kubernetes worker nodes are now resolved to their Kubernetes application and marked as managed by Cozystack, so a SecurityGroup attached to a Kubernetes cluster now applies to its worker nodes, not only to its control plane.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
* **New Features**
* Lineage results now connect supported virtual machines to their
associated machine and Kubernetes application when the required
association labels are present.
* **Bug Fixes**
* Association tracing is limited to supported virtual machine resources
and machines in the same namespace. Unlabelled standalone virtual
machines and machines in another namespace do not gain ancestors through
this association.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
Aleksei Sviridkin
|
72ff22dc9e |
fix(securitygroups): project a membership-only group to a policy Cilium accepts (#5133)
<!-- 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 #5129. A SecurityGroup with `attachments` and no rules still projects to a backing CiliumNetworkPolicy, since that policy carries the attachments the securitygroup-controller reconciles. Its spec was `endpointSelector` plus `ingress: []`. The CRD `anyOf` accepts that, but the agent's `api.Rule.Sanitize` counts entries (`len(r.Ingress) == 0 && ...`, the same in v1.19.5 and v1.20.2), so Cilium marks the policy `Valid=False` with "rule must have at least one of Ingress, IngressDeny, Egress, EgressDeny". `pkg/apis/sdn/DESIGN.md` §5 claimed the opposite. The projection now writes one empty ingress rule, which allows nothing, plus `enableDefaultDeny: {ingress: false, egress: false}`, so that rule does not put the member pods into default deny. The override relies on the agent's `enable-non-default-deny-policies`, which defaults to `true` in the vendored chart; DESIGN.md says so. `ingress` is now `omitempty` like `egress`, because an empty list no longer serves any purpose. The reverse projection hides that exact spec, so the group still reads back with no rules. `enableDefaultDeny` is what tells it apart from a tenant-authored empty ingress rule, which keeps its default deny and stays visible. `TestRulesLessSecurityGroupProjectsValidPolicy` now checks the wire form against the Sanitize rule and fails on `main` (`ingress:[]`). `TestEmptyIngressRuleIsNotMembershipOnly` covers the tenant-authored empty rule. `go test ./pkg/registry/sdn/...` passes. Checked on a lab running Cozystack v1.6.5 with Cilium 1.19.5 (`enable-non-default-deny-policies: true`), in a throw-away namespace with no baseline policy. A membership-only group `front` attached to `web`, and a group `db` on `srv` allowing `fromSG: [front]` on 8080. The v1.6.5 projection gives `front` as `VALID=False`. With the spec this branch writes for `front` applied to its CNP, it turns `VALID=True` ("Policy validation succeeded"), and every probe is unchanged: `web -> srv:8080` OK, `web -> srv:9090` and `other -> srv:8080` refused, `other -> web:8080` and `web -> other:8080` OK, so the placeholder isolates nothing. Moving the attachment of `front` from `web` to `other` flips the `fromSG` peer: `other -> srv:8080` OK, `web -> srv:8080` refused. ### 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 The diff touches `pkg/registry/sdn/securitygroup/` and `pkg/apis/sdn/DESIGN.md` only: no package, chart, CRD, `hack/` or convention change. The website mentions SecurityGroups only in the v1.6 blog post and the PCI-DSS page, neither of which describes the backing policy. <!-- 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(securitygroups): a SecurityGroup with attachments and no rules no longer leaves an invalid CiliumNetworkPolicy behind; its backing policy is now accepted by Cilium and still allows or denies nothing. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * Membership-only SecurityGroups now round-trip without displaying ingress or egress rules, while remaining usable without triggering default-deny behavior. * SecurityGroups with an explicitly configured empty ingress rule continue to retain that rule when read back. * This behavior depends on Cilium’s `enable-non-default-deny-policies` option being enabled; when disabled, Cilium applies default deny to every policy. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Mattia Eleuteri
|
fd9245f008 |
test(lineage): reject CAPK labels on objects that are not KubeVirt VMs
The bridge from a VirtualMachine to its KubevirtMachine is guarded by the object's API version and kind, but no test exercised that guard: weakening it to match on either field alone still passed, and let a labelled kubevirt.io Cluster or a VirtualMachine of another group resolve to the Kubernetes application. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Aleksei Sviridkin
|
3d317c5f67 |
feat(api): expose HelmRelease failure state on Application status (#5171)
## What this PR does Tenants usually read Applications but not the HelmReleases behind them, and the Application status only carried the `Ready` and `Released` conditions. A client could see that a release was failing, but not how many times it had failed or whether helm-controller had stopped retrying it. This PR copies the `Stalled` condition onto the Application and adds three optional status fields taken from the HelmRelease: `installFailures`, `upgradeFailures` and `lastAttemptedReleaseAction`. All of them are additive and `omitempty`, so existing clients are unaffected. What the new fields do and do not tell a client: - With the `RetryOnFailure` policy the platform generates today, a failed release is retried rather than stalled, so the counters are the signal. That does not hold for every state: a release missing from Helm storage while an install failure count is retained still stalls (the case in #3554), a release on a remediation policy stalls once its retries run out, and terminal errors such as an invalid CEL expression or denied access stall too. - `Stalled` is copied only when it was observed for the HelmRelease's current generation. The Application carries no generation, so after a spec update a client would otherwise read the previous spec's failure as terminal for the new values; until helm-controller reconciles the new spec, the Application shows no `Stalled`. - The counters only count attempts that recorded a Helm release. A failure before Helm records one, such as a chart that fails to render (for example a `fail` guard rejecting the values), shows only as `Ready=False` and leaves them at zero; a failure to apply the rendered resources is counted. - helm-controller resets the counters when the desired state changes (generation, chart version, values), and on success only under `RetryOnFailure`, so a `Ready` release can still carry non-zero counters. The field descriptions say so. `TenantModule` is left unchanged on purpose: it still copies only `Ready` and `Released`. Related: #5109, #4155 Tests cover the copied condition and counters, a `Stalled` left over from an older generation after a spec update, the counters kept on a `Ready` release, and that `Reconciling` is still not copied. The OpenAPI schema and apply configurations are regenerated, and every per-kind status schema picks up the new fields. ### 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 Terraform provider reads only the `Ready` condition by type, so the extra condition and fields do not change its behaviour. The website only shows an example Application output, not a description of the status contract. ### Release note ```release-note feat(api): Applications now expose the HelmRelease `Stalled` condition and the `installFailures`, `upgradeFailures` and `lastAttemptedReleaseAction` status fields, so tenants without access to HelmReleases can see how a release is failing. Tools that compute health with kstatus, such as Flux Kustomizations waiting on Applications, now report an Application whose release has stalled as failed. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **New Features** * Application status now displays install and upgrade failure counts, along with the most recently attempted release action. Failure counts reflect attempts recorded as releases and reset when the desired state changes. * Current stalled release conditions are now reflected in application status, helping clarify when a release is blocked. * Failure details remain available when a release is ready, while reconciling conditions are not shown. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Alexey Artamonov
|
6e36385e00 |
feat(api): expose HelmRelease failure state on Application status
Tenants usually read Applications but not the HelmReleases behind them, and only Ready and Released were copied over. A client could therefore see that a release was failing but not how often it had failed, nor whether helm-controller had stopped retrying it. Copy the Stalled condition, which helm-controller sets once it stops reconciling a release (remediation retries exhausted, rollback target missing, invalid CEL, access denied), and surface the install and upgrade failure counters with the last attempted release action. Stalled is dropped when it was observed for an older generation than the HelmRelease's current one: the Application carries no generation, so after a spec update a client would otherwise read the previous spec's failure as terminal for the new values. With the RetryOnFailure policy the platform generates today, a failed release is retried rather than stalled, so the counters are the signal. That does not hold for every state: a release missing from storage while an install failure count is retained still stalls, and a release on a remediation policy stalls once its retries run out. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
myasnikovdaniil
|
4087218a02 |
docs: move the repo-only guides onto the website (#5034)
## What this PR does Seven guides under `docs/` were operator facing and written alongside the code, and left in the source tree where no user looks. They are on the website now, so this deletes them here and repoints every reference. ``` docs/operations/backup-classes.md -> operations/services/backup-classes docs/operations/resource-presets.md -> guides/resource-management docs/storage-immutability.md -> operations/storage-class-immutability docs/operations/system-memory-limits.md -> operations/configuration/system-memory-limits docs/gateway-classes.md -> networking/gateway-classes docs/observability/distributed-tracing.md -> operations/services/monitoring/distributed-tracing docs/operations/seaweedfs-capacity-for-backups.md -> operations/services/object-storage/capacity-for-backups ``` `docs/release.md`, `docs/deprecating-applications.md` and the two SeaweedFS runbooks in `docs/operations/` stay. A workflow, `AGENTS.md`, a migration and a bats suite read them by path, which is the test from #3777. References point at `https://cozystack.io/docs/v1/...`. The site rewrites that short prefix to the latest version in `layouts/404.html`, so the links follow releases and pin none. Backup Classes, resource management and `storageClass` immutability exist in the latest version already. The other four pages are only in `next/` until 1.7.0 is promoted, and their links start to resolve then. The generated mongodb files come from `values.yaml` through `make generate` with cozyvalues-gen v1.7.0, the version pinned in `.github/workflows/pre-commit.yml`. Tests: helm unit tests for mongodb (151), postgres (209), vm-disk (114) and kubernetes-nodes (223), go tests for backupcontroller, presets and the operator, rd-presets-check, the migration-50 bats suite. ### Downstream repositories - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: cozystack/website#763 Walked the rest of the trigger map against the diff. No provider follow-up: the mongodb `values.schema.json` diff is one `description` string, with no field added, removed or renamed, no enum, no default, and no `kind`, `plural` or `release.prefix` change. `packages/core/platform/values.yaml` changed in comments only and no key, so the hand written platform-package table on the site is unaffected. Please merge after cozystack/website#763, the links in the code only resolve once those pages deploy. ### Release note ```release-note docs: seven guides move from the docs/ directory of this repository to cozystack.io (Backup Classes, resource presets, storageClass immutability, system component memory limits, GatewayClass selection, distributed tracing, SeaweedFS capacity for backups), and in-repo references point there. ``` |
||
|
IvanHunters
|
a444ac4498 |
Merge branch 'main' into fix/tap-pin-digest
Reconcile the tap digest-pin change with #5071 (no-install-on-connect), now merged into main. Keep this branch's resetTapPinState, the verify-before-pull Trust text, and the index-README trust model, while adopting #5071's wording that installing (not tapping) runs charts and dropping the stale "re-registration" phrasing from the shared tap.go and rest.go comments. Conflicts resolved in cmd/cozypkg/cmd/tap.go, pkg/registry/core/tap/rest.go, and hack/packages-index/README.md. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Mattia Eleuteri
|
dde255b7af |
fix(securitygroups): project a membership-only group to a policy Cilium accepts
A SecurityGroup with attachments and no rules still needs its backing CiliumNetworkPolicy, which carries the attachments the controller reconciles. Its spec held only an empty ingress list: the CRD anyOf accepts that, but the agent's api.Rule.Sanitize counts entries, so Cilium 1.19 marks the policy Valid=False. Write one empty ingress rule, which allows nothing, and disable default deny in both directions so the rule does not isolate the member pods. The policy is then valid and still changes nothing in the datapath. The reverse projection recognises that exact spec, enableDefaultDeny included, so the group still reads back with no rules while a tenant-authored empty ingress rule keeps its default deny and stays visible. Fixes #5129 Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
93faa7a862 |
fix(lineage, kubernetes): resolve CAPK node VMs to their cluster
The Cluster API KubeVirt provider creates a node's VirtualMachine
without an ownerReference and links it to its KubevirtMachine through
the capk.cluster.x-k8s.io/kubevirt-machine-{name,namespace} labels
only. The ownership walk therefore stopped at the VM, found no
ancestor, and the lineage webhook left every object below it unmanaged:
the virt-launcher pods of worker nodes never got
internal.cozystack.io/managed-by-cozystack, while still carrying the
application lineage labels copied from the machine template.
Anything that relies on the webhook having resolved those pods misses
them. The securitygroup-controller caches only managed pods, so a
SecurityGroup attached to a Kubernetes application labelled the
control-plane pods and never the nodes.
Follow the CAPK labels from the VM to its KubevirtMachine, whose
ownerReferences reach the CAPI Cluster and from there the cluster's
HelmRelease. Only a KubevirtMachine in the VM's own namespace is
followed: CAPK can target an external infrastructure cluster, and the
walk must never attribute an object across namespaces.
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
Assisted-by: LLM
|
||
|
IvanHunters
|
0195e73d38 |
fix(marketplace): verify the signature before pulling, and anchor the identity match
With --certificate-identity and --certificate-oidc-issuer, `cozypkg tap` pulled by tag without verifying and applied the artifact's PackageSources with ForceOwnership; only the Flux source carried the verify policy. A tag moved by someone without the signing identity could land unsigned content in the cluster. Verify the signature at tap time and pin the verified digest, so the content pulled and applied is what a trusted identity signed. Flux evaluates a cosign matchOIDCIdentity issuer and subject as Go regexps, and they were written unanchored. An exact identity such as alice@example.com would also accept malice@example.com, and the dots would match any character. Anchor and quote both values at the API and the CLI. Reject --tag together with a digest reference instead of silently ignoring the tag. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
2f39e304f0 |
fix(marketplace): reconcile tap texts with no-install-on-connect
Removing on-connect auto-registration made several user-facing and doc texts wrong: they promised that tapping registers a repository's apps in the catalog and runs its charts. Tapping now installs nothing, so the texts told operators to wait for apps that never appear. Reword the `cozypkg tap`/`untap` help, the tap success output (now printing a `cozypkg add <name>` line per materialized PackageSource), the `cozypkg init` scaffold README, the packages-index trust model, the dashboard Repositories banner and per-repository empty state, and the stale re-registration code comments. No behaviour changes beyond the added `cozypkg add` hint in the tap output. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
f8e3b0d043 |
fix(marketplace): preserve the pull secret on API repeat connect
The repeat-connect merge preserved spec.verify but still dropped spec.secretRef when the connect omitted a secret, breaking a private tap on a pin move. It now preserves an existing secretRef the same way, so the API and CLI connect paths are symmetric: a repeat connect overrides verify and the secret only when it carries its own, and never silently drops either. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
10be3d2770 |
fix(marketplace): reject a non-cosign verify provider at connect
TapVerify advertised provider notation, but buildVerifySpec always emits a matchOIDCIdentity block, which is meaningful only for keyless cosign. Setting provider: notation produced a nonsensical verify block. The connect now rejects any provider other than cosign and the type doc no longer advertises notation, since only keyless cosign identity matching is settable here. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
5747f94781 |
docs(marketplace): document digest pinning and verify on tap
Update the tap command help and the community-index README for the new behaviour: digest references, the automatic tag-to-digest pin, the --certificate-identity / --certificate-oidc-issuer pull-time verify flags, and the optional index entry digest. Extend the generated openapi definitions and model-name helper for the new TapVerify type (code generation is otherwise run in CI). Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
c56b9876f4 |
feat(marketplace): accept a signature-verification policy at tap connect
A tap can now carry a keyless cosign verification policy (spec.verify), so a
reference is only pulled when signed by the recorded identity. The CLI takes
--certificate-identity and --certificate-oidc-issuer (both required together,
mirroring validate); the API takes spec.verify.{issuer,subject} on the Tap
connect object. Both map to a Flux OCIRepository verify block with provider
cosign and a matchOIDCIdentity entry. A half-configured policy (only one of
the identity pair) is refused rather than set, since a policy that matched any
signer is worse than none. A key-based policy an admin sets on the source by
hand is preserved across repeat connects.
Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
e93db88480 |
fix(marketplace): preserve verify and reset the pin on repeat connect (API)
The connect API replaced the whole OCIRepository spec on a repeat connect, so a spec.verify signature policy added earlier was silently dropped. It now updates url, interval, ref and secretRef from the request while preserving an existing spec.verify: a repeat connect never silently downgrades trust. An explicit digest reference is pinned on the source directly; a tag-only connect is left for the operator to pin. Replacing the whole ref on a repeat connect drops the operator's prior digest pin so the newly requested tag re- resolves, and the pinned-digest annotation is cleared alongside the materialized-revision annotation so the operator re-pins and re-materializes. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
b8a129e356 |
feat(marketplace): accept @sha256 digest references in tap connect
parseOCIRef (CLI) and parseConnectURL (API) now accept a pinned digest reference (oci://host/org/repo@sha256:...), with an optional informational tag before it. The digest must be a full sha256:<64 hex>, the same contract the platform source's digest= reference enforces. A reference carrying only a digest no longer defaults its tag to latest, since the digest is the authoritative reference Flux resolves before any tag. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Myasnikov Daniil
|
b83cc087cc |
docs: move the repo-only guides onto the website
Seven guides in docs/ were written next to the code and left in the source tree, where no operator looks. The website carries them now, so a second copy here only lets the two drift again: the Backup Classes page had already fallen behind its original. docs/release.md, docs/deprecating-applications.md and the two SeaweedFS runbooks in docs/operations stay. A migration, a bats suite, a workflow or AGENTS.md reads each of them by path, which is the test for keeping a document in this repository. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
frolov.so
|
9b35fa0b48 |
docs(api): clarify kind-wide Helm timeout overrides
Timeout annotations are read from an ApplicationDefinition and therefore affect every Application of that kind. Describing them as per-Application hides their actual scope and the upgrade override precedence. Fixes cozystack/cozystack#4382 Assisted-by: LLM Signed-off-by: frolov.so <opsmon.sh@yandex.com> |
||
|
Myasnikov Daniil
|
0474613482 |
fix(api): preserve nulls without applicable chart defaults
A schema default cannot fill arbitrary map entries or replacement array items. Keep those nulls, nullable fields and required fields without a non-null default unchanged so Helm retains their validation semantics. Validate the normalized copy on both write paths so admission sees the values that reach storage without mutating the caller object. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
2e88b39620 |
fix(api): repair an empty required field instead of failing the release
An Application spec reaches flux verbatim as HelmRelease .spec.values,
and Helm's value coalescing deletes a null-valued key together with the
chart default underneath it, before the result is validated against
values.schema.json. A required field written with nothing under it is
therefore rejected for being absent:
talos:
registryMirrors:
fails the whole release with "at '/talos': missing property
'registryMirrors'", naming a property the spec does contain, and nothing
reaching the user points at the empty key they typed.
Remove such a key on the write path, so the chart's own default applies
to it. Filling in the schema default instead would pin today's value
into the stored HelmRelease and freeze the application on it, so a later
bump of that default in the chart would never reach an application that
had once been created with an empty key.
Only required properties are repaired. A required property's null cannot
be load-bearing for anyone, because today it fails the render outright,
so resolving it is pure repair. Where the schema allows the field to be
absent, a null keeps the meaning it has now - the chart default is
suppressed and the key does not reach the template - and that meaning is
worth keeping: 167 optional fields across the catalogue carry a
non-trivial default, placeholder credentials and example S3 buckets
among them, so resolving a user's null to one of those would be worse
than leaving the key out.
Free-form subtrees, null map entries under a user-chosen key (a node
group, a registry host) and array elements are left as written, because
there a null may be the payload rather than an omission. Every removal
is reported back to the client as a warning, so the repair is visible
rather than silent.
Assisted-By: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
||
|
IvanHunters
|
f05718b2de |
feat(redis)!: refuse external access without TLS at admission (#4639)
<!-- 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 An admission policy now refuses a Redis release that would expose a plaintext endpoint outside the cluster. Redis never turns TLS on by itself, so `external: true` without `tls.enabled: true` is exactly that case. The error names the release and a remedy that works. The rule has three parts, because TLS is fixed when a Redis instance is created: the RedisFailover schema refuses a change, while the Redis API accepts one and the HelmRelease then fails. A spec's `tls.enabled` is therefore a request, not proof of what the instance runs. - On create, `external: true` needs `tls.enabled: true`. - On update, turning `external` on is refused whatever `tls.enabled` says. - While `external` stays on, `tls.enabled` cannot change. Before the first reconcile nothing else stops TLS being dropped from a release admitted as external with TLS. Withdrawing `external` in the same edit lifts the freeze, and the callers trusted by `cozystack-tenant-host-policy` are exempt, so an instance whose TLS was flipped earlier can still be unblocked. Edits to a release that was already external pass, so releases exposed in plaintext before this policy keep working. The guarantee covers the Redis app. A generic publisher such as `tcp-balancer` can still forward to a plaintext Redis, by design. This diverges from the Security section of the `external-database-exposure` proposal, which refuses only an explicit `tls.enabled: false` and covers every engine. That rule was written before redis had TLS and matches only postgres. Postgres is left out here on purpose: CloudNativePG keeps `ssl=on` and its default pg_hba accepts plaintext from any client that asks, whatever the chart's `tls.enabled` says, so an admission rule on that field closes nothing. That gap is #4619. The proposal update and a decision record are cozystack/community#91. The policy follows `cozystack-tenant-host-policy`: a ValidatingAdmissionPolicy in `cozystack-basics`, enforced inside `cozystack-api` for the aggregated kind. Its decisions are tested in Go by compiling the shipped template with the apiserver's own CEL compiler and running Redis objects through it, including the bypass sequences found in review. A structural chart test can't show what the policy admits. The redis README, the `external` field description and the threat model describe the new rule. Related: #4622 (valkey `external` is always plaintext), #4624 (the redis `tls.enabled` description implies a live switch). ### 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: - [ ] [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: I walked the trigger map against the diff and nothing in it fires, but I left the boxes empty because cozystack/community is reached anyway: the `external-database-exposure` proposal still states the old admission rule. Its update and a decision record are cozystack/community#91. ### 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(redis)!: a Redis release can be exposed outside the cluster only when it is created with `tls.enabled: true`. Creating one with `external: true` and no TLS, turning `external` on for an existing instance, or changing `tls.enabled` while `external` is on is refused at admission. Instances already external without TLS keep working and stay editable. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Redis admission checks now require TLS when external access is enabled at instance creation. They block enabling external access on an existing instance and changing TLS while external access remains enabled. Disabling external access in the same update allows TLS changes. * Existing instances that already had external access without TLS before the policy was introduced remain editable. * **Documentation** * Updated Redis configuration and security guidance to explain these restrictions and TLS migration behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
6a98acb61d |
fix(api): leave failed golden images out of the image options (#4568)
<!-- 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 The VM image picker no longer offers golden images whose import failed. A golden keeps its PVC in `cozy-public` after the import fails, so a picker built on the PVC list offered an image whose clone never completes. The `image` option source in cozystack-api now reads the `vm-default-images-*` DataVolumes next to the PVCs and drops names whose DataVolume is in phase `Failed`. I filter rather than mark them, because such an option can never be used and `OptionItem` has no disabled state. A golden that is still importing stays in the list, since CDI holds a clone until the source is populated, and so does a PVC with no DataVolume. When the DataVolumes cannot be read, the PVCs go out unfiltered. The Option list drops a source whose provider errors, and an empty picker would be worse than the old behaviour. cozystack-api gets a read grant on `cdi.kubevirt.io` DataVolumes for this. The console did not use that source at all. `SourceField` listed the PVCs itself, as the tenant, so the fix above would not reach the picker users see. It now reads the same Option list the other dynamic dropdowns use. With that, nothing in the console lists `cozy-public` PVCs, so I removed the read grant on them from the `cozy:tenant:dashboard` Role. That is a visible change: a tenant loses `kubectl get pvc --namespace cozy-public`, which was only ever granted for this picker in #2843. One limitation: in CDI v1.66.1, which cozystack ships, a golden import reaches `Failed` only on a checksum mismatch, and a broken URL keeps it in `ImportInProgress`. So this filter covers less than the issue assumed. I filed #4564 for that. Closes #4553 ### 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. --> I have no screenshot. The picker looks the same; only the list behind it changes. `apps/console/src/components/SourceField.test.tsx` covers it: the picker shows the images the option source serves and not the raw PVC names. ### 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(api): the VM image picker no longer offers golden images whose import failed. The console now reads images from the image option source, and tenants no longer get read access to PersistentVolumeClaims in cozy-public. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * The image picker now displays available images from the tenant’s image options. * **Bug Fixes** * Images whose imports have failed are excluded from the picker. Images remain available when import status is still in progress, has succeeded, or cannot be retrieved. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
800bb0369d |
feat(redis)!: refuse external access without TLS at admission
A Redis release with external: true and TLS off publishes a plaintext endpoint outside the cluster, and nothing stopped a tenant from asking for one. The chart never infers TLS from external, because enabling it moves Redis to a TLS-only listener, so external: true without an explicit tls.enabled: true is exactly that case. A ValidatingAdmissionPolicy on the Redis kind now refuses it with a message naming the release and a remedy that works. TLS is fixed when a Redis instance is created: the operator's RedisFailover schema refuses a change, but the Redis API accepts it. A spec's tls.enabled is therefore a request, not the instance's state, and cannot prove on UPDATE that an instance has TLS; trusting it would let a tenant flip it, expose the instance, and remove it again. So external access can be enabled only at creation, where tls.enabled does decide the instance, and any UPDATE turning it on is refused. The RedisFailover schema does not exist before the first reconcile, so a release admitted as external with TLS could have TLS dropped before the instance is created. tls.enabled is therefore frozen while external access stays on. Withdrawing external access in the same edit lifts the freeze, which leaves a tenant a way to unblock an instance whose TLS was flipped earlier, and the callers trusted by the tenant host policy are exempt so an administrator can repair one in place. Other edits to an instance that is already external pass, so releases exposed in plaintext before the policy keep working. Postgres is left out on purpose. CNPG pins ssl=on, and its default pg_hba accepts plaintext from any client that asks, whatever the chart's tls.enabled says; that field only decides whether the external hostname goes into the certificate. A policy keyed on it would close nothing. The decisions are tested by compiling the shipped template with the apiserver's own CEL compiler, since a structural chart test cannot show what the policy admits. BREAKING CHANGE: creating a Redis with external: true and without tls.enabled: true, which applied before, is now refused, including a GitOps re-apply of such a manifest into a fresh cluster. Turning external on for an existing Redis is refused as well, so exposing an instance means creating a new one with TLS, and tls.enabled can no longer be changed on an instance with external access. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
bf03c84711 |
chore(deps): move to the Kubernetes 1.37 libraries without the apimachinery fork (#4473)
## What this PR does This moves the root module's `k8s.io/*` from v0.35 to v0.37.1 and controller-runtime from v0.23 to v0.25.1, and drops the `k8s.io/apimachinery` fork on the way. Supersedes #3001, #4467 and #4472; those were a stack of the same commits. The four commits are meant to be read one by one, and each builds and passes the tests on its own: 1. **Plain watches stop getting the initial-events-end bookmark.** The apiserver defaults `sendInitialEvents=true` onto any watch with an empty or `0` resourceVersion and leaves bookmarks off, and our aggregated storages took that flag alone as a WatchList request. From `k8s.io/apiserver` v0.36 the watch handler compiled into cozystack-api panics on that bookmark, so `kubectl get -w` on any cozystack-api resource would die after the bump. The marker now needs both flags, the same condition the watch cache uses. There are tests per storage and one through the real handler chain. 2. **The apimachinery fork goes.** Every apps kind shares the `Application` Go type, and stock `Scheme.ConvertToVersion` picks the first kind registered for it. The fork patched that; the apps group now encodes through a serializer that keeps the kind the object carries. A handler-chain test covers create, get, list, update, watch, apply and table for two kinds and fails without it. 3. **MutatingAdmissionPolicy stays out of admission.** From v0.36 the plugin is on by default and refuses every write until it has synced `mutatingadmissionpolicies`, which cozystack-api has no RBAC to list and which clusters before 1.36 do not serve. Without this, every create, update and delete would fail after the bump. It is also re-applied in `Complete`, since `--disable-admission-plugins` replaces the list. 4. **The bump itself**, with the regenerated clients, openapi and strategy CRDs. Commits 2 and 4 are where the review effort goes; 1 and 3 are small. Known changes in behaviour: - Without the fork, the row objects of a Table requested with `includeObject=Object` carry the first registered app kind. kubectl asks for that only with `--sort-by`, and it sorts on fields, not on the kind. Nothing at this layer can fix it, because the handler converts through `APIGroupInfo.Scheme`, a concrete `*runtime.Scheme`. Admission webhooks and ValidatingAdmissionPolicy still see the right kind. - The alpha PodSpec field `workloadRef` became `schedulingGroup` upstream. Once the new strategy CRDs are applied, the apiserver prunes `workloadRef` from stored strategies it serves, and their next write erases it. It only ever worked behind an alpha feature gate. The branch is rebased onto the Renovate rollup (#4447), with `go mod tidy` and codegen rerun on top of it. ### Screenshots No UI changes. ### 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 chore(deps): cozystack-api and the controllers build against the Kubernetes 1.37 libraries without the apimachinery fork. Plain watches no longer receive the WatchList initial-events-end bookmark, and objects in Table rows requested with includeObject=Object may carry another app's kind. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Application API responses now preserve the requested resource kind across operations, including lists and watches. * Backup strategy resources support additional Kubernetes pod settings, including probe protocols, volume ownership options, and scheduling groups. * Backup strategy schemas include updated Kubernetes pod fields and descriptions. * **Bug Fixes** * Initial-event watch bookmarks are sent only when requested; plain watches continue without an ending bookmark. * Mutating Admission Policy is disabled by default, including when admission plugin options are customized. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
99f3d3b2b7 |
feat(controller): tag a DataVolume cause with a condition reason
WorkloadsReady=False covers different situations: a stopped VM has no pod and its monitor reports it not operational on purpose, while a disk whose DataVolume never populates is broken. A consumer that wants to react to one and not the other had only the message text to go by. The WorkloadMonitor status gains a Reason next to Message, set to DataVolumeNotReady by the DataVolume check, and the Application API carries it into the WorkloadsReady condition in place of the generic reason whenever a monitor that is not operational names one. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
b867ea1eda |
feat(api): show WorkloadsReady in the Application table
Ready follows the Helm release, so for a kind whose install does not wait on its workloads, a vm-disk among them, READY in kubectl get says only that the manifests were applied. Whether the workloads came up lives in WorkloadsReady, which the default table did not show, so a disk CDI never populated looked healthy on the surface an operator reads first. The table now carries a WORKLOADS column beside READY. It reads <none> for an Application with no WorkloadMonitor, which has no WorkloadsReady condition at all. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
48ece79ffc |
feat(controller): name the DataVolume that keeps a disk not ready
For a vm-disk, WorkloadsReady is the one condition that reports whether CDI populated the disk, and its generic "One or more workloads are not operational" gives an operator nothing to act on: a disk has no workloads, and the stuck DataVolume and its phase are exactly what they need to know. The WorkloadMonitor status gains a message that the controller fills with each selected DataVolume that is not ready and its phase, and the Application API uses the messages of the monitors that are not operational in WorkloadsReady instead of the generic text whenever one of them names a cause. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
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> |
||
|
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>
|
||
|
Aleksei Sviridkin
|
8ed34166cf |
perf(lineage): cache owner GETs and short-circuit re-admission (#2667)
## What this PR does The lineage mutating webhook walks the owner chain (Pod, StatefulSet, HelmRelease, application CR) on every admission, with one uncached dynamic GET per level, and sibling Pods of the same parent repeat the same lookups. On a multi-tenant cluster this put `lineage.cozystack.io` at 300-500ms p99 in `apiserver_admission_webhook_admission_duration_seconds`, the largest share of admission latency. Owners are now cached in-process for 60 seconds, and only successful lookups are stored. A failed lookup (RESTMapping, NotFound, timeout) leaves the walk without an owner, so the object is admitted with no lineage labels and stays unlabeled until its next UPDATE runs the walk again. Secrets, Services and PVCs can go a long time between updates, so a cached failure would leave every object admitted within the TTL unlabeled for that long. The aggregated cozystack-api makes such failures routine, since discovery of `apps.cozystack.io` fails while it rolls. The one staleness left is the scheduling class, which is read from the application's spec: a change to it reaches newly admitted Pods up to 60 seconds later. The cache is not swept below 4096 entries, so up to that many owner objects stay in memory for the life of the process. Above that, each insert drops the expired entries among a random sample of 64, which keeps the write lock short. Non-Pod objects that already carry every lineage label, `internal.cozystack.io/tenantresource` included, skip the walk, because the objectSelector still matches an UPDATE whose old object lacks the label. Pods go through regardless, as each Pod admission also sets the scheduling class. ### Screenshots Not a UI change. ### Downstream repositories The diff is Go under `pkg/lineage` and `internal/lineagecontrollerwebhook` only: no label, annotation, CRD, values key, `hack/` file or telemetry metric is added or renamed, so no entry of the trigger map applies. - [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 perf(lineage): the lineage mutating webhook caches owner lookups for 60 seconds and skips the owner walk for non-Pod objects that already carry lineage labels, cutting admission latency on clusters with many tenants. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Lineage and owner lookups now use a time-limited in-memory cache, reducing repeated API requests during admission processing. Scheduling-class updates may take up to 60 seconds to appear on new Pods. * Non-Pod objects with complete lineage labels are allowed without additional owner lookup or mutation. * **Tests** * Added coverage for cache hits, misses, expiration, and retrying lookups after errors. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Mattia Eleuteri
|
ca24d8f5f6 |
perf(lineage): cache owner GETs and short-circuit re-admission
Every admission walks the owner chain (Pod, StatefulSet, HelmRelease, application CR) with one uncached GET per level, and sibling Pods repeat the same lookups. On a cluster with many tenants that put the lineage webhook at 300-500ms p99, the largest share of apiserver admission latency. Owners are now reused for 60 seconds, successful lookups only. A failed lookup, whether RESTMapping, NotFound or a timeout, leaves the walk without an owner, so the object is admitted with no lineage labels and stays unlabeled until its next UPDATE runs the walk again. Secrets, Services and PVCs can go a long time between updates, so a cached failure would leave every object admitted within the TTL unlabeled for that long, and the aggregated cozystack-api makes such failures routine: discovery of apps.cozystack.io fails while it rolls. Non-Pod objects that already carry every lineage label, tenantresource included, skip the walk, since the objectSelector still matches an UPDATE whose old object lacks the label. Pods go through it regardless, as each Pod admission also sets the scheduling class. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
31948c8838 |
fix(api): redo the HelmRelease metadata carry-over on a conflict retry
A 409 on the HelmRelease PUT means the live object changed after the read the carry-over was built from, and the writer behind it is normally helm-controller - the controller whose finalizer the carry-over exists to protect, and which re-adds it on the reconcile following a metadata-only update. Retrying with the metadata read before the conflict wrote that stale copy back and dropped the finalizer the reconcile had just restored, reopening the orphaning window the carry-over closes. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
mattia-eleuteri
|
6d541c63eb |
fix(api): make mergeMaps always allocate, never return an input map
mergeMaps returned one of its inputs whenever the other was nil. Create and Update seed the HelmRelease labels with mergeMaps(r.releaseConfig.Labels, ...) and then write the application metadata labels into the result in place, so for an Application with no labels the writes landed on the long-lived config map shared by every request for the kind: requests cross-contaminate (application.name varies per request) and concurrent map writes crash the apiserver. Signed-off-by: mattia-eleuteri <mattia@hidora.io> |
||
|
mattia-eleuteri
|
a18cbc581c |
fix(api): preserve foreign HelmRelease metadata on Application update
Update rebuilds the HelmRelease from the Application and issues a full-replace PUT, which stripped metadata owned by other controllers: finalizers, ownerReferences, and any labels or annotations outside the apps.cozystack.io- prefix. Losing helm-controller's finalizers.fluxcd.io finalizer is the harmful case: a delete landing before helm-controller re-adds it removes the HelmRelease without running helm uninstall, orphaning every resource of the release (VMs, databases, PVCs). A label patch immediately followed by a delete - a common client pattern - hits that window almost every time. Fetch the current HelmRelease before the PUT and carry over its finalizers, ownerReferences, and non-prefixed labels and annotations. Keys under the apps.cozystack.io- prefix remain owned by the API and are recomputed from the Application, so label/annotation deletions still propagate. Signed-off-by: mattia-eleuteri <mattia@hidora.io> |
||
|
Aleksei Sviridkin
|
b201a0f878 |
fix(api): leave failed golden images out of the image options
The image option source lists every vm-default-images-* PVC in cozy-public. CDI creates that PVC as soon as an import starts, so a golden whose DataVolume ended in phase Failed is still offered, and a disk cloned from it never populates: the user picks an image the platform just suggested and ends up with a disk that never becomes usable. Read the DataVolumes next to the PVCs and drop the names whose import Failed. Filtering beats marking: the option can never be used, and OptionItem has no disabled state to show it in. A golden still importing stays, since CDI holds a clone until the source is populated, and so does a PVC with no DataVolume. When the DataVolumes cannot be read, without CDI or without the grant, the PVCs go out unfiltered: the Option list drops a source whose provider errors, and an empty picker would be a regression rather than a safeguard. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
e62f510faf |
chore(deps): move the Kubernetes libraries to v0.37.1
The root module's k8s.io/* move from v0.35 to v0.37.1 and controller-runtime from v0.23 to v0.25.1, its release for Kubernetes 1.37. Two things held them back: the apimachinery fork, which had to be rebased onto every minor, and the watch handler that from k8s.io/apiserver v0.36 panicked on the initial-events-end bookmark the aggregated storages sent to plain watches. Both are gone. The rest of the upgrades are what these releases require: cel-go v0.29.2, the etcd client v3.7.0, kube-openapi, structured-merge-diff and the x/ modules. code-generator moves with the libraries. Regenerating the strategy CRDs that embed core types adds the Kubernetes 1.37 fields and follows one removal: the alpha PodSpec field workloadRef became schedulingGroup upstream. Once the new CRD is applied, the apiserver prunes workloadRef from every stored strategy it serves, and the object's next write erases it from etcd. It only ever took effect behind an alpha feature gate. The gateway README and the tenantgateway CRD tests cite the apiextensions-apiserver list-type ratchet at the pinned version; it is byte-identical in v0.37.1, so only the version they name changes. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ef491f5ee8 |
fix(api): keep the MutatingAdmissionPolicy plugin out of admission
From k8s.io/apiserver v0.36 the MutatingAdmissionPolicy feature is GA and on by default, and the plugin sits in the recommended admission order, so cozystack-api would run it. The plugin refuses every write with "not yet ready to handle request" until its informer has synced mutatingadmissionpolicies. This server has no RBAC to list them, and clusters older than 1.36 do not serve the resource at all, so every create, update and delete on its resources would fail. Disable the plugin in the default options, and again in Complete, because --disable-admission-plugins replaces the default list instead of adding to it. Naming the plugin in --enable-admission-plugins still turns it on: that operator has granted it what it needs, and leaving it on both lists would fail validation. On the v0.35 libraries the feature gate already keeps the plugin off, so nothing changes today. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
3dde6503ec |
refactor(api): keep each app's kind without the apimachinery fork
Every apps kind is registered on the one Application Go type, and the stock Scheme.ConvertToVersion picks the first kind registered for a type, so without help a Redis comes back from the API as whichever app kind was registered first. So far the fix lived in a fork of k8s.io/apimachinery that patches ConvertToVersion. Upstream has not taken the change, and the fork has to be rebased onto every Kubernetes minor before the libraries can move. The apps group now encodes through a serializer that picks the kind the object carries, along the lines upstream suggested instead of changing Scheme. The replace directive goes, and with it the Renovate rule that kept the fork's stale release tags from being proposed as updates. The serializer relies on the storage stamping the kind on every object it returns, which it does. A test drives create, get, list, update, watch, apply and table through the real handler chain for two kinds and fails without the serializer on upstream apimachinery. One path converts through the Scheme rather than the serializer and goes back to the first registered kind: the row objects of a Table requested with includeObject=Object, which kubectl asks for only with --sort-by. Admission webhooks and ValidatingAdmissionPolicy get the request kind set explicitly. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
93ba1472ed |
fix(api): send the initial-events-end bookmark only to WatchList clients
The apiserver defaults SendInitialEvents on for every watch whose resourceVersion is empty or "0", and leaves AllowWatchBookmarks off. The aggregated storages read SendInitialEvents alone as a WatchList request, so a plain `kubectl get -w` on applications, tenant modules, namespaces, secrets or security groups got a bookmark annotated as the end of initial events. The k8s.io/apiserver v0.35 watch handler lets that through. From v0.36 it treats the annotated bookmark as the end of a WatchList and calls the WatchList completion hook, which it only sets up when SendInitialEvents and AllowWatchBookmarks are both on, so the handler dereferences nil and the watch dies. The handler is the one compiled into cozystack-api, so bumping the library is what breaks it, whatever the cluster version. The marker now requires both flags, the same condition the watch cache uses, and the backing watches ask for bookmarks only when the client did, since every backing bookmark is forwarded. Security groups still forward SendInitialEvents to their backing watch so it replays existing objects as ADDED. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2c2c0301f7 |
chore(deps): roll up pending Renovate updates and move to Go 1.27.1 (#4447)
## What this PR does This replaces the open Renovate PRs with one rollup. Go modules move to their newest releases that keep the Kubernetes libraries and controller-runtime where they are, the toolchain and builder images move to Go 1.27.1, and the first-party code gets the Go 1.27 modernizers. Tests for the code the bumps touch come first, in their own commits, and pass on both the old and the new dependencies. Before the bumps: - The e2e sandbox image builds again: dl.min.io answers 410 for mc since MinIO archived it, so mc now comes from its last GitHub release with a pinned checksum. - Renovate stops offering tags of the apimachinery fork. They mirror upstream, so #2999 is a downgrade that drops our patch. - `make unit-tests` now runs the tests of the nested Go modules (kubevirt-csi-driver, token-proxy, kubeovn-webhook, api/apps/v1alpha1). They never ran in CI before. - New tests: JWT verification in the dashboard token proxy, the kube-ovn plunger metrics, cert-manager object idempotency in the tenant gateway, values-file layering in the operator, and the kubevirt CSI driver wrapper. - One real fix. The lineage webhook decoded admitted objects with encoding/json, so an int64 above 2^53 came back rounded. jsonpatch v2.5 compares numbers exactly and would emit a replace that rewrites such a field in the pod. The webhook now decodes with apimachinery's json. Held back on purpose, with the reasons in the commit messages: - Kubernetes libraries stay on v0.35. #3001 needs the apimachinery fork rebased onto release-1.37 first. - cel-go v0.29.2 and etcd v3.6.15, the lines k8s.io/apiserver v0.35 is built and tested with. - grpc v1.83.2. v1.84.0 is affected by GO-2026-6443 and no v1.84 patch release carries the fix yet. - kubevirt.io/api and the CDI API stay on what upstream csi-driver builds against. From CDI v1.60 it no longer compiles. - api/apps/v1alpha1 keeps go 1.26.4, so consumers of the published types are not forced onto Go 1.27. - go fix runs without embedlit (Go 1.27-only syntax, breaks backports to release branches on Go 1.26) and omitzero (would make metadata, spec and status required in the published OpenAPI schemas), and one errorsastype rewrite that did not compile is left out. govulncheck reports nothing reachable in any of the five modules, standard library included. Supersedes #4311, #4310, #4309, #3452, #3993, #3927, #3902, #3805, #3451, #3415, #3414, #3413, #3302, #4050 and #3363. #3001 stays open. Not checked locally: the images that build upstream projects (kube-ovn, kamaji, cilium, linstor-csi and others) with the Go 1.27 builder. The image build jobs of this PR cover them. Closes #4487. ### Screenshots No UI changes. ### 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 chore(deps): move the toolchain and builder images to Go 1.27.1 and raise Go dependencies to their latest compatible releases, closing the published advisories; the lineage webhook no longer rewrites int64 fields above 2^53 in admitted objects. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a restore resource policy option for Velero strategies. * Added a test target to run Go tests across tracked modules. * **Bug Fixes** * Updated the end-to-end test environment to download and verify the MinIO client from its current release location. * Added coverage for token validation, webhook patching, controller behavior, and reconciliation. * **Maintenance** * Updated the Go toolchain, dependencies, and container image versions. * Refreshed pinned workflow actions and base image digests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
scooby87
|
03e2da5805 |
feat(postgres,mariadb)!: drop plaintext user passwords (#4078)
## What this PR does Removes the plaintext `users[].password` field from the `apps/postgres` and `apps/mariadb` charts (and the mariadb `root` account), so managed-database passwords can no longer be pinned in tenant values or committed to git. Passwords are always chart-generated into the `<release>-credentials` Secret and preserved across reconciles via `lookup`, matching the model `redis`/`valkey` already use. This closes the "User passwords" item of the portal upstream epic (aenix-org/cozyportal#1060). The generated password never lands in the init-script Secret or the SQL it runs: the postgres init-job reads each user's password from the mounted `<release>-credentials` Secret at run time and applies it through a psql variable (`ALTER ROLE … WITH PASSWORD :'pw'`), so it does not linger in the init-script manifest a tenant can read. It does live in cleartext `stringData` in the `<release>-credentials` Secret itself, which is a chart manifest and therefore kept in Helm release history for `MaxHistory` revisions like any chart-generated secret — that residue is exactly why rotation cannot be built on the chart (see below). A `password` or a `passwordRotation` key left over in a tenant's values is accepted-but-inert and draws an admission warning, so an operator editing it is told it has no effect rather than getting a silent 200. Backup/restore into a copy keeps the target's chart-managed credentials working: the CNPG driver clears `bootstrap.enabled` once recovery is healthy so the init-job re-applies the generated passwords onto the recovered roles, and the MariaDB driver excludes `mysql.global_priv` from the dump (`ignoreGlobalPriv`) for backups captured from this release onward, so restoring one of those into a copy does not clobber the target's chart-managed users or `root`. Excluding the grant table has a flip side, and it applies to every MariaDB restore, not only a to-copy one: only accounts declared through `User` CRs (the chart's `users` map) are captured, so an account created out of band directly in MySQL is in no backup and a restore brings back its data and `mysql.db` grants but not the account itself. This is also a backup-side fix only: a restore from a backup taken **before** this change still replays the source's `mysql.global_priv` and overwrites the target's users and `root`, and `root` then needs an out-of-band reset (the operator applies `rootPasswordSecretKeyRef` only at datadir bootstrap, so there is no in-band path). See `docs/operations/backup-classes.md` for the convergence window, the out-of-band-account limitation, and this old-backup caveat. Password **rotation** is deliberately **not** in this PR. A chart-rendered credentials Secret keeps its cleartext in Helm release history for `MaxHistory` revisions, so a rotation built on it cannot revoke a leaked password; rotation is being reworked as a controller that owns the Secret — design proposal in cozystack/community#72. Migration is seamless: existing releases keep their current passwords on upgrade (the value already lives in `<release>-credentials`, which `lookup` re-reads and preserves); a `password` still present in a tenant's values is ignored, so no existing HelmRelease fails validation. Verification: `helm unittest` passes for both charts (postgres 87, mariadb 126, including render-time username guards that reject any name k8s would later reject as an invalid Secret key or User resource name); `go build ./...`, `go vet`, the backup-controller tests and the apps-apiserver tests are green; `make generate` was run with the CI-pinned `cozyvalues-gen` v1.6.0 so `values.schema.json`, the READMEs, the API types, and the `cozyrds` ApplicationDefinitions are all in sync (the branch was rebased onto `main`, which reconciled cleanly with the read-replica autoscaling feature that landed there). ### Screenshots N/A — no UI change. ### Downstream repositories - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: affected — `content/en/docs/next/cozystack-api/go-types.md` is hand-written and still shows `Password:` in the Go examples, and the managed-app reference pages regenerate from these chart READMEs, so both document `users[name].password` until refreshed (not yet filed). - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: affected — the provider hand-maintains the postgres/mariadb schemas and per-field defaults; this PR removes `users[].password`. A follow-up issue/PR is needed there (not yet filed). `terraform-provider-cozystack` and `website` are reached by this diff; both boxes are left unticked because the follow-ups are not yet filed. Same-repo future work this change suggests: the controller-owned password rotation (design in cozystack/community#72), and extending the "passwords only in the Secret" model to the other engines that still accept plaintext (`clickhouse`, `mongodb`, `nats`, `opensearch`, `rabbitmq`, `vpn`). ### Release note ```release-note feat(postgres,mariadb)!: passwords for managed PostgreSQL and MariaDB users can no longer be set via `users[].password` — they are always auto-generated and stored only in the `<release>-credentials` Secret, never rendered into the init-script or the SQL it runs. Existing releases keep their current passwords on upgrade; a `password` (or a `passwordRotation`) left in values is ignored with an admission warning. Password rotation is deferred to a follow-up controller design (cozystack/community#72). MariaDB backups now exclude the `mysql.global_priv` grant table (`ignoreGlobalPriv`), so a restore recreates only accounts declared through the chart's `users` map — an account created out of band directly in MySQL is not captured and must be recreated after a restore. ``` |
||
|
IvanHunters
|
0b7dd23b99 |
fix(appdef): let one ApplicationDefinition own each application kind
Nothing enforced that spec.application.kind is unique, and each consumer that maps a kind back to a definition resolved a duplicate its own way: the chartRef reconciler let every definition rewrite every release of its kind, the lineage webhook and the CA projection reconciler took whichever came first in an unordered list, and cozystack-api registered one resource per definition. A second definition declaring a kind already in use could therefore repoint that kind's tenant releases at its own artifact. Add one ownership rule, the definition created first owns the kind with ties broken by name, and route every consumer through it. A definition that arrives later with a kind already in use no longer manages that kind's releases whatever it is named: the chartRef reconciler skips it with a Warning event and re-reconciles the next owner when the current one goes, cozystack-api registers the owner only and logs the rest, its restart hash moves when ownership moves, and the lineage and CA projection lookups resolve the owner. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Alexey Artamonov
|
9afdc49894 |
feat(apps): warn when a removed field is still set on a database app
The apps API hands the stored spec to the HelmRelease unchanged, so a users[].password left on a Postgres or MariaDB application is silently ignored by the chart. Return a warning on create and update for each such user, and for the superuser key each chart refuses to manage, so the operator editing it learns it has no effect. Also warn when an update introduces a "_"/"-" collision between usernames, database names or Grant names, which the charts reject only on install. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Timofei Larkin
|
ce3eb30e03 |
fix(api): refuse only what a name declaration actually breaks (#4429)
## What this PR does Follow-up to #4367, taking the non-blocking findings from its approving review. Each one is a case where a name declaration (`x-cozystack-name`) was handled more harshly, or more quietly, than the declaration itself warrants. - **An unparseable schema is no longer blamed on a declaration.** A schema that is not a JSON object has no declaration in it, and the spec schema loader already reports it. The API server used to refuse every create of the kind with "declares an invalid x-cozystack-name". It now treats the schema as declaring nothing: creates keep working, and `Kubernetes` and `Kafka` keep their legacy caps. - **Annotation keywords no longer take a kind offline.** Any unknown key inside a declaration used to refuse creates, so a generator adding a `title` or an example would have done the same. JSON Schema annotation keywords (`title`, `default`, `examples`, `deprecated`, `readOnly`, `writeOnly`, `$comment`) and `x-` extensions are now ignored. A constraint keyword the server does not implement, such as `format` or a misspelled `maxLenght`, is still refused, because accepting it would drop it silently. - **The docs no longer overclaim.** They said a declaration no name can satisfy is refused. That is true only for lengths: a pattern is checked to compile, not to match anything, so `$^` rejects every name and names the pattern as the reason. The docs now say that. - **The API review gate sees the declarations the server refuses.** The gate only read a declaration that was an object, and only diffed the keywords it knew. A declaration turned into a string or null, or a misspelled keyword, looked like no change. A changed declaration now goes through the same parser the server uses, and the gate reports it if it cannot be enforced. The parser moves to `pkg/apis/apps/validation` so there is one copy of the rules. - **The declaration no longer leaks into the published OpenAPI for `spec`.** The whole `openAPISchema` is published as the schema of `spec`, so `x-cozystack-name` appeared there as a vendor extension of `spec`. It is now stripped from the published copy. `metadata.name` still carries no limit in the published OpenAPI: it is the shared `ObjectMeta` reference, and a per-kind copy is a bigger change than this one. Not changed: a pattern that can never match is still accepted. Deciding whether a regular expression matches any string within the declared lengths is a much larger piece of code than the mistake it would catch. ### How it is tested - `TestUnparseableSchemaDeclaresNothing`: three non-object schemas; creates still pass, and the Kubernetes legacy cap still holds. - `TestNameDeclarationIgnoresAnnotations`: a declaration carrying `title`, `examples`, `$comment` and an `x-` extension still enforces its `maxLength`. - `TestInvalidNameSchemaFailsClosed` gains `format` as an unsupported constraint keyword. - `TestClassifyNameDeclaration` gains a declaration turned into a string, a misspelled keyword and a null declaration, all reported, and an added annotation, which is not. - `TestPatchSpecLeavesNameDeclarationOffSpec`: the published `spec` drops `x-cozystack-name` and keeps an unrelated vendor extension. ### Downstream repositories No repository is affected beyond the follow-ups already noted on #4367 (the `cozystack/website` pages on annotations and ApplicationDefinitions). Those are still unfiled, so no box is ticked. - [ ] 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 ApplicationDefinition whose openAPISchema cannot be parsed no longer refuses every create of its kind as an invalid name declaration, and annotation keywords (`title`, `examples`, `x-` extensions and the like) inside `x-cozystack-name` are now ignored instead of taking the kind offline. The API review gate now reports a changed name declaration that the API server would refuse, and `x-cozystack-name` no longer appears on the published OpenAPI schema of `spec`. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Application name declarations now support minimum and maximum lengths and regular-expression patterns. Matching patterns are checked for compilation; a pattern that matches no names rejects all names. * **Bug Fixes** * Invalid or unsupported name constraints now produce configuration errors and block every create of the affected kind. * Descriptive annotations and `x-` extensions are ignored when validating name declarations. * The custom name declaration is omitted from the published API specification. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
7a0aab2bdc |
refactor: apply the Go 1.27 modernizers
Run go fix over every first-party module: interface{} becomes any,
hand-written loops become slices, maps, min/max and range-over-int,
errors.As becomes errors.AsType, pointer helpers become new(expr) and
are inlined at their call sites, and so on. Every rewrite keeps
behaviour.
Three rewrites stay out. embedlit flattens embedded-field literals
such as ObjectMeta, syntax only Go 1.27 accepts, which would break
backports to release branches still on Go 1.26. omitzero drops
omitempty from struct fields, and openapi-gen reads omitempty as
optional, so metadata, spec and status would turn required in the
published schemas. And errorsastype rewrote an errors.As whose
target, apierrors.APIStatus, does not implement error, which
errors.AsType rejects at compile time.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
||
|
Aleksei Sviridkin
|
15035a915b |
fix(vm-instance): disable helm readiness wait on the VMInstance release (#3735)
## What this PR does A VMInstance is served as a Flux HelmRelease that renders a KubeVirt VirtualMachine. Since v1.5.0 (helm-controller v1.4.3 to v1.5.0) that release is health-assessed by the kstatus poller. vm-instance sets no `healthCheckExprs`, so the VirtualMachine goes through kstatus's own rules in `fluxcd/cli-utils` (`pkg/kstatus/status`): `checkGeneration` returns InProgress for any object whose `status.observedGeneration` differs from `metadata.generation`, and past that check a `Ready=False` condition is InProgress too, so a halted VM is never Current even with its generation in sync. KubeVirt keeps a VirtualMachine's `observedGeneration` behind its `generation` whenever a `spec.template` change is pending a restart, the VM is stopped, or a live-updatable change never converges (kubevirt/kubevirt#18700). So a healthy, Running VM is read as permanently InProgress, the upgrade waits its full timeout, and fails with `timeout waiting for: [VirtualMachine/... status: 'InProgress']`. On releases still carrying the pre-1.5.0 `remediation.retries: -1` + rollback spec, the timeout then reverts the previous manifest, dropping just-applied changes such as opened ports. This sets `release.cozystack.io/helm-install-disable-wait: "true"` on the vm-instance ApplicationDefinition, so cozystack-api generates the HelmRelease with Install/Upgrade DisableWait. The release settles once the VirtualMachine is applied, and VM health is read off the VirtualMachine itself (`status.printableStatus` / `Ready`) and off the WorkloadMonitor the chart already renders, which is what `WorkloadsReady` on the Application reports. This restores the pre-1.5.0 behaviour, under which the VM was never a helm readiness gate. The same mechanism is already used by `kubernetes-rd` and `kubernetes-nodes-rd`. On its own that annotation does not reach a running cluster, so two gaps in cozystack-controller are closed with it. The first is the rollout. `applicationdefinition-controller` hashed each definition's name and spec only, so a change confined to a `release.cozystack.io/*` annotation left the hash unmoved and cozystack-api kept serving the configuration it read at its last start-up, which is the only place those annotations are ever read. The hash now covers them too. Only that prefix, because kubectl and Helm rewrite annotations of their own on every apply and hashing those would roll the api Deployment on every reconcile. The second is the releases that already exist. `applicationdefinition-helm-reconciler` brings chartRef, valuesFrom and labels on existing HelmReleases in line with their definition; it now does the same for the readiness wait. Without it the annotation reaches new instances only, which on an affected cluster is the wrong half: #3734 reports 134 of 136 releases already looping upgrade and timeout, and re-applying every VMInstance by hand was the only way out. The setting is driven in both directions, since cozystack-api rebuilds the whole spec on every write and emits `DisableWait=false` once the annotation is gone. An absent install or upgrade block already behaves as `DisableWait=false` and is materialised only when the definition asks to disable the wait, so a release with nothing to change is not rewritten. Two alternatives were considered. A per-kind `healthCheckExprs` keyed on `status.printableStatus` cannot work: setting it moves the VirtualMachine from kstatus to the CEL status evaluator (`fluxcd/pkg/runtime/cel`), which runs the same `observedGeneration` check ahead of the expressions, and any `healthCheckExprs` added to this kind later is a no-op while the wait is disabled. The typed `spec.release.waitStrategy: legacy` is the narrower option, since the legacy checker never waits on a custom resource, but it still gates the release on the chart's Service, and with `external: true` that is a LoadBalancer the legacy checker holds not ready until it has ingress. So an external VM would still wait on IP allocation, which `WorkloadsReady` already watches. Disable-wait avoids that and is the mechanism this repository already uses for the same class of problem. Tests: a helm-unittest suite (`tests/readiness_gate_test.yaml`) guards that the annotation stays on the rendered ApplicationDefinition, and Go unit tests cover both cozystack-controller paths: the hash moves on a release annotation and stands still on an unrelated one, and the wait lands on an existing release, is withdrawn when the annotation goes, does not materialise empty install/upgrade blocks, does not rewrite a release already in sync, leaves a release alone when the annotation does not parse, and does not reach a release of another kind. Note on existing clusters: no manual step. The definition changes on upgrade, the config-hash rolls cozystack-api, and the helm reconciler brings existing VMInstance releases in line. Releases that predate v1.5.0 also carry a `remediation.retries: -1` + rollback spec, which is left alone: with the wait disabled the upgrade no longer times out, so that remediation no longer fires, and any write through the apps API drops it. Closes #3734. ### Downstream repositories - [x] No downstream repository is affected by this change <!-- Walked the trigger map against the diff. The change is a readiness annotation on an ApplicationDefinition plus two cozystack-controller paths. It does not touch openAPISchema, kind, plural, release.prefix, any app package, any values.schema.json or any installer/platform values key. The website's application-definitions.md describes the config-hash as a checksum "over the sorted definitions", which stays accurate now that the hash also covers the release.cozystack.io/* annotations, and no page documents those annotations today. --> ### Release note ```release-note fix(vm-instance): VMInstance HelmReleases no longer hang InProgress or roll back healthy VMs; the release no longer waits on VirtualMachine readiness, which helm-controller's kstatus poller cannot assess correctly for KubeVirt VMs. Existing VMInstance releases are brought in line automatically on upgrade. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * VM installations no longer wait indefinitely when Kubernetes reports the VM’s observed generation behind its current generation. * Changes to release settings are now detected and applied to generated Helm releases, including install and upgrade wait behavior. * **Tests** * Added coverage for VM readiness and release-setting reconciliation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
myasnikovdaniil
|
e9868b4f62 |
fix(api): keep a suspended HelmRelease suspended across an app update (#4411)
## What this PR does This PR makes `REST.Update` keep live `spec.suspend` of the HelmRelease, on first write and on conflict retry. Update rebuilds HelmRelease from Application and writes it back whole, Application has no suspension field, so every edit through aggregated API resumed a release that operator or controller had suspended. Caller that depends on it is cnpg restore driver. It suspends target release, patches Postgres app through this API, purges Cluster and resumes only after purge is done, so next chart render lands `bootstrap.recovery` on empty namespace. When the patch resumes release, helm-controller upgrades right away, chart can render recovery Cluster while purge is still running, then driver deletes it as the Cluster being replaced and nothing renders it again, because values do not change anymore. RestoreJob stays `Running` until its deadline. On main it works only because of timing. Same write also drops `finalizers.fluxcd.io`, and helm-controller v1.5.0 spends its first pass after that on putting finalizer back and requeue (750ms minimum retry delay) before it upgrades, this delay usually lets purge finish first. #3426 keeps controller finalizers across this write, so delay is gone, and `postgres-2-backup-roundtrip` failed on both its runs there while it passes on other PRs. In [35714873620](https://github.com/cozystack/cozystack/actions/runs/35714873620) stuck restore is `pg-src-to-pg-target-pitr`: `startedAt` 11:40:37, helm upgrade v3 at 11:40:37, new Cluster created at 11:40:38 and gone by 11:40:47, no render after that. [35652855170](https://github.com/cozystack/cozystack/actions/runs/35652855170) has same picture on first restore, all inside 22:10:51. Tests are in `rest_suspend_test.go`: edit leaves `spec.suspend` as it was (both values) with values still taken from Application, and suspend that lands between read and write survives conflict retry. Removing either line fails its test. ### Downstream repositories Change is in how aggregated API writes HelmRelease back, no downstream repo restates that. Rollback guide on website (`guides/concepts.md`) tells operators to suspend HelmRelease while they work, with this fix that suspension holds across app edit, so docs need no change. - [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): app edit through cozystack API no longer resumes a suspended HelmRelease, so postgres restore waits for its purge before cluster is rendered again. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Application updates now preserve a Helm release’s suspended or active state instead of unintentionally changing it. * The release state is also preserved when an update encounters a conflict and retries, preventing a concurrent suspension from being lost. * Application updates continue to use the application’s configured values while retaining the release’s current suspension state. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Timofei Larkin
|
a461dc4b38 |
fix(api): keep the name declaration off the published spec schema
The whole openAPISchema is published as the schema of spec, so x-cozystack-name showed up as a vendor extension of spec in the aggregated OpenAPI. That describes something spec does not contain. Assisted-by: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
180030be72 |
fix(api): refuse only what a name declaration actually breaks
Three cases refused creates of a kind, or were documented as refused, without the declaration being at fault. A schema that is not a JSON object was reported as an invalid x-cozystack-name, although such a schema has no declaration to blame, and the spec schema's own loader already reports it. It now declares nothing, so creates keep working and a kind with a legacy cap keeps it. Any unknown key inside a declaration took the kind offline, so a generator adding a title or an example would do the same. Keywords that constrain nothing, the JSON Schema annotations and x- extensions, are now ignored. A constraint keyword the server does not implement is still refused, because accepting it would drop it silently. The docs said a declaration no name satisfies is refused. That holds for lengths only: a pattern is checked to compile, not to match anything. The docs now say so. The parser moves to pkg/apis/apps/validation so the API review gate can reuse it rather than keep a copy of its rules. Assisted-by: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |