mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-13 08:05:19 +09:00
cc51e040ae1677c3e643ad06d50bc6c5e5cce1b8
1595
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Andrey Kolkov
|
cc51e040ae |
fix(mongodb): keep the cleanup escape hatch off legacy operator CRs, and surface a stuck finalizer
The skip-artifact-cleanup annotation reaches releaseMongoDBCleanup before the ownership check, and the operator-CR delete sat outside the finalizer gate, so a legacy backup (no delete-backup finalizer) lost its PerconaServerMongoDBBackup record when a tenant set the annotation to release a stuck Backup or applied it across a namespace during teardown. That record is what a psmdb restore-by-name resolves against, and both the RBAC comment and the docs promise it is never touched. Gate every write in the release on the finalizer being present: an unowned (legacy) CR or one that cannot be read at all is left intact, and the Cozystack Backup still releases. Surface a failed finalizer strip. The release must proceed even against an unreachable CR, but the swallowed patch error left the operator CR holding percona.com/delete-backup — pinning its namespace in Terminating, the outcome the branch exists to prevent — while the Event reported the success case. Emit a FinalizerNotStripped Warning naming the object so an operator can find it. Mark the sibling backup fields (schedule, retentionPolicy, destinationPath, endpointURL, insecureSkipTLSVerify, s3AccessKey, s3SecretKey) as ignored under useSystemBucket, so a tenant editing one instead of the flag itself is not misled by a description that describes only the legacy flow; regenerate the schema, README and mongodb-rd. Pair the insecureSkipTLSVerify rendering assertion with a case that flips the platform knob on, so the two together discriminate the rendered value from a hardcoded default. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
f27a02f1b6 |
docs(mongodb): correct the inlined-storage story and surface the TLS knob
The strategy godoc still said the storage config is NOT inlined and the strategy only names the storage — which controller-gen copies verbatim into the CRD, so kubectl explain served the new s3 field and its own denial from one object. Rewrite it (and the StorageName field doc) to describe both flows, and regenerate the CRD. Render insecureSkipTLSVerify on the default MongoDB strategy: the field is plumbed end to end into the driver-injected storage entry and backed by a platform knob, but the template set only forcePathStyle, pinning it to false so a self-signed external S3 that Redis backups honour would fail MongoDB x509 verification. Pin it in the rendering test alongside the other s3 coordinates. Fix the useSystemBucket field description: angle brackets rendered as empty HTML tags in the README table and the dashboard schema, so wrap the path in backticks (as the postgres field already does) and note that schedule, retentionPolicy and PITR are ignored on this flow. Regenerate the chart schema, README and mongodb-rd. Assisted-By: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
9545bee233 |
fix(mongodb): correct system-bucket prose, pin strategy rendering, make restore credential swap flow-aware
Several comments and the useSystemBucket field description had drifted from
what the code does.
The strategy header still said the $bucketName gate was "not because it
needs the value" and that the sibling strategies needed the bucket name
while this one only asked for ordering, but the strategy now renders
bucket: {{ $bucketName }} for the injected coordinates, so it needs the
value; the precondition comment claimed the s3-storage entry exists on
backup.enabled=true, which is only the legacy flow. The rendering-test
comment carried the same "needs no bucket coordinates" claim. The
useSystemBucket field description said injection happens "at first BackupJob
time" (it now runs on every BackupJob) and that backups scope to
<namespace>/<release> (the prefix templates the application name, not the
release), and it ran ~700 chars of implementation detail into the CRD that
backup-classes.md already carries; trimmed and corrected, then regenerated
schema/README/types/cozyrds. Same <release> slip fixed in the driver
comment and the docs.
Nothing asserted the new strategy rendering. Added the MongoDB row to the
endpoint-form suite (s3.endpointUrl, scheme preserved both ways) and field
assertions to the rendering suite — bucket, endpointUrl, region,
credentialsSecret, forcePathStyle, and prefix, which must reach the CR as a
literal go-template string so the driver can template it per-application;
a stray brace would empty it with every doc-count test still green.
The restore credential swap broke across flows. It replaced the source
credentialsSecret with whatever the target declared, comparing no buckets,
so a system-bucket backup restored into a legacy target kept the platform
bucket against the tenant's own -s3-creds, and a legacy backup restored
into a system-bucket target got cozy-backups-creds against the tenant
bucket - both AccessDenied. Skip the swap when the source already names the
platform-projected cozy-backups-creds (it outlives the source app, nothing
to repair), and only adopt a target credential whose storage is on the
same bucket as the archive. psmdbStorageS3 now reads .s3.bucket alongside
.s3.credentialsSecret; TestPsmdbTargetCredentialsSecret covers the match,
the cross-bucket refusal, and the empty-bucket no-op.
Also drop the post-apply re-Get: the typed SSA Patch decodes the server's
merged object back into the passed struct, so the precondition reads the
injected storage from it instead of a follow-up Get that hit the informer
cache and briefly reported Ready=False on the happy path.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-By: LLM
|
||
|
Andrey Kolkov
|
4ea4888685 |
fix(mongodb): key system-bucket injection off the app flag, not live storage
The MongoDB backup driver decided what to inject and what to record from the live cluster's storage set rather than from the app's opt-in flag. That had three consequences. The snapshot fallback fired for legacy apps too. Its gate was `snap.S3 == nil && rendered.S3 != nil`, but the coordinates moved onto the shared cozy-default strategy, so rendered.S3 is now non-nil for every MongoDB app. A legacy backup written to the tenant's own bucket got its snapshot rewritten to the platform bucket, sending restore to the wrong place. Gate the fallback on the app's backup.useSystemBucket flag instead, threaded through createMongoDBBackupArtifact, and pin the legacy case. The injected storage never caught up with the strategy. Injection was gated on the named storage being absent, so it ran once per cluster and froze the coordinates at the first backup; a later endpoint/region change or bucket re-provision was a silent no-op, and Helm could not repair it because the useSystemBucket chart renders no storages. Key injection off the app flag and re-apply on every BackupJob (the prefix is deterministic, so this never splits the archive the way CNPG's serverName would). Add UseSystemBucket to mongodbapp.MongoDBSpec so a legacy app is protected by the flag, not by whatever happens to be on the cluster. The docs said the opposite of what the driver does. backup-classes.md claimed "the MongoDB driver cannot inject its S3 target per-backup"; correct it and add an opt-in-to-the-system-bucket section that names which values stop being read and that scheduled backups + PITR must move to a Plan. Fix the endpoint table row, the tuning reference, the RBAC comment that said the driver never patches, the strategy header/inline comments, and the example comment. Also assert notExists on spec.backup.pitr in the helm test, and lift the duplicated cozy-backups-creds default into a const. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-By: LLM |
||
|
Andrey Kolkov
|
c036efaa5a |
test(mongodb): cover system-bucket storage injection gate and builder
Extract the two untested contracts from applyMongoDBSystemStorage into
pure functions and unit-test both, mirroring the CNPG driver's
buildBarmanObjectStore split:
- shouldInjectMongoDBSystemStorage decides whether the driver injects the
system-bucket storage; the tests pin both branches, including the one
that leaves a legacy app's manual bucket untouched.
- buildMongoDBSystemStorageEntry builds the {type:s3,s3:{...}} entry,
defaulting credentialsSecret and omitting forcePathStyle when nil.
Also add a helm-unittest for the restore.yaml guard rejecting
bootstrap.enabled together with backup.useSystemBucket.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-By: LLM
|
||
|
Andrey Kolkov
|
730592f323 |
feat(mongodb): support the system-bucket backup flow (useSystemBucket)
MongoDB was the only cozy-default app with no useSystemBucket flow: its backup storage was always static on the app CR (bucket/endpoint/creds from backup.* values + a chart-emitted <release>-s3-creds Secret), so a tenant could not back up to the platform system bucket out of the box the way postgres/clickhouse/etc. do. PBM takes bucket/endpointUrl/region/prefix as static CR fields (not secret refs) and the platform bucket name/endpoint are only known at BackupJob time, so - unlike ClickHouse's runtime env secretKeyRef - the chart cannot render them. The fix mirrors the CNPG driver: the chart adds a useSystemBucket flag that keeps spec.backup.enabled (so the pbm agents run) but omits the static storages/tasks/pitr and the creds Secret; the cozy-default MongoDB strategy carries the coordinates (endpointUrl already scheme'd by the endpoint helper, credentialsSecret cozy-backups-creds); and the psmdb driver SSA-injects spec.backup.storages[s3-storage] onto the live PerconaServerMongoDB at BackupJob time under its own field manager with ForceOwnership, so a Flux re-render never reverts it. Injection is gated on the storage being absent, so a legacy app that ships its own static storage is left untouched. The backup snapshot falls back to the strategy coordinates when the operator status echo is empty, so the restore path rebuilds backupSource (credentialsSecret cozy-backups-creds is projected into the restore namespace before dispatch). RBAC grows patch/update on perconaservermongodbs for the injection. A bootstrap.enabled + useSystemBucket render guard mirrors postgres. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-By: LLM |
||
|
Alexey Artamonov
|
ff637ed9a2 |
fix(platform): validate the realm-name charset and regenerate kubernetes artifacts
The realm-name guard checked only the type and echoed the parsed value back, so its hint told an operator who wrote `on` to set realmName: "true" — the token YAML had already discarded. Drop the echo and add a slug-charset check: the name is spliced verbatim into URL path segments and a shell token, so a "/" or a space must be rejected at the source rather than split a URL or the hook script downstream. The description edit in the previous commit was hand-applied to the generated kubernetes README and never reached the Go types, leaving pre-commit red; regenerate both with cozyvalues-gen. Finish the oidc-tenant System-mode prose that still read cozy as a fixed name. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
949096694b |
fix(platform): reject non-string realm names and shell-quote the hook realm
A realmName that YAML resolves to an int or a bool (a bare 123, or a reserved word like on/off) was coerced with toString, silently renaming the realm to something the operator never typed (on -> "true"). Fail the render with a quoting hint at the source instead of surfacing it later as a broken issuer URL. The update-email hook Job interpolated the realm into its shell script unquoted, so a name carrying a space or a shell metacharacter could break or inject into the script; single-quote it. The Grafana and managed-cluster issuer helpers build the URL with printf %s straight off the channel value, so guard them with toString for the same non-string reason. Name the configured realm in the tenant kube-apiserver oidc.mode description and the Grafana identity-mode comment, which still read cozy as if it were fixed. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
a3df02df23 |
fix(kubernetes): name the configured realm in the OIDC denial message
The claimValidationRules message in the System-mode AuthenticationConfiguration hardcoded "in the cozy realm", so on a renamed realm it told a rejected user to look in a realm that no longer exists. Build it from the same realm the issuer uses (toString-guarded, as the derived issuer URL already is). Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
44a9a8e735 |
feat(platform): make the Keycloak realm name configurable
The realm name "cozy" was hardcoded in keycloak-configure (which owns the realm, its clients and the cozystack-cluster-admin roles), in the dashboard and linstor-gui gatekeepers, in the info kubeconfig, and in the Grafana and managed-Kubernetes apiserver OIDC issuers. Operators could not rename the realm without patching vendored templates, which `make update` reverts. The apiserver OIDC issuer is already configurable via talm oidcIssuerUrl, so pointing it at a custom realm left every SSO component on "cozy" and broke login with an issuer mismatch. authentication.oidc.realmName (default "cozy") now rides the _cluster channel so every consumer resolves the same realm, and the default keycloakInternalUrl is derived from it so the realm is named in one place. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
3219f7ee9a |
fix(backups): give the barman-cloud sidecar its own resources (#4220)
## What this PR does The barman-cloud plugin injects its sidecar with no resource requests or limits. What that means depends on the namespace. A tenant with `resourceQuotas` set ships a `LimitRange` that defaults containers to 128Mi (`packages/apps/tenant/templates/quota.yaml`), so there the sidecar inherits that default and is OOMKilled mid-backup. A namespace without a `LimitRange`, which is `cozy-keycloak` and any tenant that leaves `resourceQuotas` empty, gave the sidecar no requests and no limit at all, so nothing reserved memory for it and nothing bounded it. The failure on the tenant path is quiet from the control plane: the `ObjectStore` stays healthy, the `Cluster` stays `Ready`, and only the backup fails. Observed on a 1.6 cluster while backing up a 38 MB database: `lastState.terminated.reason: OOMKilled`, `exit 137`, four restarts on the sidecar, and a cgroup high-water mark of 254 MiB. `container_memory_working_set_bytes` peaked at only 64 MiB over the same window — it is sampled, and it misses the spike that actually triggers the kill, which is why the working-set number does not explain the failure on its own. Both paths that build an ObjectStore now carry resources: - `cozy-lib.barman.sidecarConfiguration`, used by `packages/apps/postgres` for the backup and recovery ObjectStores and by `packages/system/keycloak`; - `barmanSidecarConfiguration()` in `internal/backupcontroller/cnpgstrategy_controller.go`, which builds the platform's own ObjectStore on the `useSystemBucket=true` path. Requests are 100m/256Mi with a 1Gi memory limit on every caller: 256Mi holds the measured working set and 1Gi is four times the measurement, which is also the ceiling a caller without a LimitRange gets where it had none. No CPU limit is set, following the `entityOperator` precedent in `packages/apps/kafka/templates/kafka.yaml`: a throttled sidecar stalls WAL archiving instead of failing it, which is harder to notice than an outright failure. Measured CPU peak was 50 mCPU. Inside a tenant the limit is charged against the `ResourceQuota` `limits.memory` budget, 1Gi per instance pod; a 4Gi tenant spends a quarter of it per Postgres instance. The helper is renamed from `checksumSidecarConfiguration` to `sidecarConfiguration`, because it no longer carries only the checksum pin and its name and doc comment would otherwise be wrong. Tests cover both paths and both render sites: a new helm suite in `packages/apps/postgres`, extra assertions in the existing keycloak suite, an assertion in the controller test, and a deepcopy test for the new field. Each of them fails against the unfixed sources. One thing this PR deliberately does not do. This is the fifth time the 128Mi tenant default has been worked around per component — kafka and zookeeper presets (#2537), the kafka entity-operator (#2934), the cert-manager cainjector (#3199), and the note carried in the etcd-operator values. Whether the default itself should move looks like your call rather than something to fold into a fix for one sidecar, so it is left alone here. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, entry by entry. The change touches one cozy-lib helper, the two chart templates that include it, one Go function, and tests. It adds, renames or removes no package; changes no `values.yaml`, `values.schema.json`, `Chart.yaml` or `README.md`, so no version enum, default or generated artifact moves; changes no `ApplicationDefinition` semantics, no `release.prefix`, no output Secret or Service name; touches no CRD, no namespace, no `hack/` file, no telemetry metric or label, no `cozy-proxy` annotation, no node or network requirement, and no commit or PR convention. No entry matches. - [x] No downstream repository is affected by this change ### Release note ```release-note fix(backups): give the barman-cloud sidecar its own resources The barman-cloud sidecar was injected without resource requests or limits, so in tenant namespaces it inherited the 128Mi `LimitRange` default and was OOMKilled during backups while the `ObjectStore` and the `Cluster` both stayed healthy. It now requests 100m/256Mi and caps memory at 1Gi on the chart path and on the platform's Go path alike; in namespaces without a LimitRange the sidecar had no limit before and gets the same 1Gi ceiling. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added explicit CPU and memory resource settings for barman-cloud backup and recovery sidecars. - Backup sidecars request 100m CPU and 256Mi memory, with a 1Gi memory limit. - Recovery sidecars receive a 1Gi memory limit. - Preserved the S3 checksum configuration. - **Bug Fixes** - Prevented sidecars from inheriting unsuitable tenant resource limits. - Ensured resource settings are preserved when backup configuration is copied. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
ee09e73343 |
fix(kafka): make topics[].config optional (#3935)
## What this PR does Makes `topics[].config` optional across the Kafka typed API, generated schema, documentation, and chart contract. The KafkaTopic template already omits an empty config, and Strimzi accepts the field as optional; the Cozystack schema was the layer incorrectly rejecting such topics. The Helm test renders a topic without config and asserts `spec.config` is absent. The Chainsaw fixture also omits config and waits for the resulting KafkaTopic to reach Ready. Go API compatibility: `Topic.Config` changes from `runtime.RawExtension` to `*runtime.RawExtension`, so Go callers constructing that field must pass a pointer. The Terraform provider intentionally does not model Kafka topic tuning (`config`), so this does not require a provider follow-up. Validation: - `helm unittest packages/apps/kafka`: 36/36 passed. - `go vet ./kafka` in `api/apps/v1alpha1`: clean; the package has no test files, so this shows it compiles. - root `make generate`, which owns `zz_generated.deepcopy.go`, and `make -C packages/apps/kafka generate`: no diff. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map against all nine changed files. The provider's Kafka model deliberately excludes `topics[].config`; no downstream repository mirrors this nested field or its requiredness. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note fix(kafka): allow topics to omit the optional Strimzi config map ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Kafka topic configuration is now optional. Topics can be created without explicitly providing configuration settings. - Topic configuration is omitted from serialized resources when not specified. - **Bug Fixes** - Improved handling of topic resources without configuration, including correct readiness validation and rendering. - **Documentation** - Updated Kafka configuration documentation and schemas to reflect that topic configuration is optional. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Yan Bondarenko
|
9fac0add84 |
docs(mongodb): describe the upgrade as a carry-over, not a rotation
The upgrade section still promised a password rotation and a regenerated credentials Secret. With the operator-copy fallback the chart reads the current passwords from internal-<release>-users and writes the same values, so nothing rotates and no session breaks. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Yan Bondarenko
|
0e17c6f65e |
test(mongodb): pin the reuse, fallback and ownership branches with lookup fixtures
Every test rendered with an empty lookup, so the branch that keeps a live password could be deleted with the suite green. Three fixtures through kubernetesProvider cover what a fresh render never reaches: a helm-owned users Secret is reused, a Secret the release does not own stays out of the manifest while its password still reaches the credentials, and the operator copy rebuilds a missing users Secret. Values with URI-reserved characters cover the userinfo encoding, and both Secrets are asserted Opaque again. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Yan Bondarenko
|
f0da352b30 |
fix(mongodb): refuse to invent a password for a users Secret the chart does not own
On the not-owned path a missing password key produced a generated password that reached <release>-credentials but never the users Secret the operator reads, so the dashboard advertised a password MongoDB never had. The render now fails and names the Secret and the key. The operator fills every key itself, so only a hand-made partial Secret reaches this branch. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Yan Bondarenko
|
348a65d18b |
docs(mongodb): keep the credentials paragraphs on one line
Two paragraphs under Credentials were hard-wrapped at 80 columns, the one place in this README that was; the rest follows the one-line-per-paragraph rule the repository asks for in markdown. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> Assisted-by: LLM |
||
|
Yan Bondarenko
|
66a00f552e |
fix(mongodb): url-encode the username in the connection uri
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Yan Bondarenko
|
b4be956d7e |
fix(mongodb): fill the dashboard credentials on a first install
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Yan Bondarenko
|
4dc6065ec9 |
fix(backups): give the sidecar sizing its reason per caller
The Keycloak comment and the helper doc claimed the tenant LimitRange as the reason for the resources, but cozy-keycloak has no LimitRange and neither does a tenant that leaves resourceQuotas empty: there the sidecar had no requests and no limit at all, and the change gives it a 1Gi ceiling where there was none. The comments and both doc blocks now say what applies to each caller. No rendered object changes. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Yan Bondarenko
|
9214430bd8 |
fix(backups): give the barman-cloud sidecar its own resources
The barman-cloud plugin injects its sidecar without resource requests or limits. Tenant namespaces ship a LimitRange that defaults containers to 128Mi, so the sidecar inherits that default and is OOMKilled mid-backup: the ObjectStore stays healthy, the Cluster stays Ready, and only the backup fails. A cgroup high-water mark of 254 MiB on a 38 MB database shows the default cannot hold it. The shared cozy-lib helper and the Go path that builds the platform's own ObjectStore now request 100m/256Mi and cap memory at 1Gi. No CPU limit is set, matching the entityOperator precedent in the kafka chart: a throttled sidecar stalls WAL archiving instead of failing it. The helper is renamed from checksumSidecarConfiguration to sidecarConfiguration, since it no longer carries only the checksum pin. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Yan Bondarenko
|
8ec76a3614 |
fix(kafka): make topics[].config optional
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Aleksei Sviridkin
|
08a2e76707 |
fix(opensearch): remove the data PVCs when the application is deleted (#4136)
## What this PR does Fixes #4086. The OpenSearch operator provisions its data volumes through a StatefulSet, so the PVCs carry no ownerReference, and the `opensearch` app chart has no cleanup hook. Removing the application therefore leaves every data volume `Bound`, counting against the tenant's storage quota with nothing left in the namespace that could reclaim it. `clickhouse`, `kafka` and `harbor` remove their operator-owned PVCs in a post-delete hook. This adds a post-delete Job to `opensearch` that requests the deletion of the claims labelled `opster.io/opensearch-cluster=<release>` with `--ignore-not-found --wait=false` and exits non-zero when the request fails, with its own ServiceAccount, a Role limited to `persistentvolumeclaims` (`get`, `list`, `delete`), the `clastix/kubectl` image pinned by digest through `cozy-lib.image`, a non-root restricted securityContext, `activeDeadlineSeconds: 120` so a kubectl that never returns cannot hold the uninstall, and `before-hook-creation` alone on the Job so a failed run and its log survive; the ServiceAccount, Role and RoleBinding keep `hook-succeeded`. The delete does not wait for the claims to disappear, on purpose: OpenSearch data pods stop slowly, and a claim they still hold would keep a waiting hook running into helm-controller's 300s uninstall budget. The claims go once their pods have stopped, which is what the log says. This is the shape the monitoring, etcd and seaweedfs hooks use. Retention is not offered as a knob, consistent with the other charts; an opt-out can be added if the maintainers prefer one. `helm unittest`: 64/64 in the chart, including `tests/cleanup_pvc_test.yaml` (hook annotations and deadline, selector and exit path, digest pin and registry prefix, securityContext, Role rules, RoleBinding). `helm template` renders the Job, ServiceAccount, Role and RoleBinding next to the existing objects. ### Downstream repositories A new hook template and its tests only: no values, schema, README or defaults 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(opensearch): reclaim the data PVCs of a removed application with a post-delete hook ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Automatically cleans up orphaned OpenSearch persistent volume claims after a release is deleted. * Cleanup waits for associated pods to stop and uses restricted permissions and hardened security settings. * Supports optional container registries with pinned cleanup images. * **Tests** * Added coverage for cleanup behavior, hook configuration, security settings, registry overrides, and required access permissions. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Timofei Larkin
|
8f19910988 |
chore(platform)!: remove the HTTPCache application (#4200)
## What this PR does Removes the HTTPCache application. Telemetry says it is the least installed app we ship: 3 instances in June, July and August 2026, 4 so far in September, out of 1162-1418 clusters reporting per month. That is not zero, so I am not going to claim nobody uses it. The maintenance side is what does not hold up. `make update` in this package is broken. All eight sed lines in the `update` target patch `images/nginx/Dockerfile`, but the image lives in `images/nginx-cache/Dockerfile`, so the target fails on the first line. It is the only package under `packages/apps/` pointing at a Dockerfile that does not exist. The pins show what that cost us: ``` nginx 1.25.3 -> 1.31.5 ngx_cache_purge 2.5.3 -> 3.0.2 nginx-module-vts 0.2.2 -> 0.2.7 ip2proxy-c 4.1.2 -> 4.3.0 IP2Location-C 8.6.1 -> 8.7.0 ip2location-nginx 8.6.0 -> 8.7.0 ip2proxy-nginx 8.1.1 -> 8.2.0 51Degrees 3.2.21.1 -> 3.2.21.1 (frozen upstream) ``` So we ship an old nginx with third party C modules compiled into it, and nothing tests it: no chainsaw suite, no `helm unittest` target. I don't think we should keep carrying that for 4 instances. Removed: the chart in `packages/apps/http-cache`, its ResourceDefinition in `packages/system/http-cache-rd`, the values type in `api/apps/v1alpha1/httpcache`, the `cozystack.http-cache-application` PackageSource, its line in the `naas` bundle, the `httpcaches` entry in `cozy:tenant:admin:base`, the dashboard icon, and the entry in the root `Makefile` build list. `cozy:tenant:admin:base` enumerates the `apps.cozystack.io` resources a tenant owner may write, so `httpcaches` comes out of it with the rest of the application. A cluster that freezes gets the grant back from the migration instead, which keeps the chart's list tracking the catalog and leaves no entry behind for a later sweep to read as dead config. ### Existing installations A cluster that still runs an HTTPCache keeps it, served from a frozen source. A cluster that never created one keeps nothing. Without a migration an upgrade would break both. `templates/sources.yaml` is a bare glob with no keep annotation, so Helm prunes the http-cache PackageSource; the ArtifactGenerator is owned by it and goes too, and both ExternalArtifacts go with the generator. Those are the `chartRef` targets of `hr/http-cache-rd` and of every tenant `http-cache-<name>` release, so all of them would sit `Ready=False` for good and `HelmReleaseNotReady` would fire on every cluster with the naas bundle. Migration 58 runs as a pre-upgrade hook, before that prune, and branches on whether any instance exists. Its header comment carries the mechanism in full; this is the summary. If one does, it copies the packages `OCIRepository` and pins the copy to the manifest digest that object resolves at hook time, which is the release the instances are rendering from right then. `spec.ref.digest` is used when the installer pinned one; a tag or semver ref goes through `status.artifact.revision`, whose digest half is the manifest digest. `status.artifact.digest` is the checksum of the downloaded tarball and cannot stand in, and a source with neither aborts the migration rather than publishing a source that points nowhere. Reading the digest live rather than baking one into the script makes the freeze a no-op by construction and asks no question about which release was the last to ship the chart. The copy takes the url, the pull secret, the interval and any verification block from the live object, so an air-gapped or mirrored registry keeps serving exactly what it already served. It is written with no owner references and no Helm metadata, and with the `platform.cozystack.io/no-delete` label the platform chart puts on the original, because it becomes the only chart source those instances have. The PackageSource is then repointed at the copy, annotated `helm.sh/resource-policy: keep` and stripped of its release metadata, so neither Helm nor a later release adopts or prunes it. Helm re-reads that annotation off the live object when it prunes, which is what makes disowning from inside a pre-upgrade hook work. Names stay as they are, deliberately. The ApplicationDefinition inside that artifact hardcodes its `chartRef`, so a renamed PackageSource would leave every release pointing at an artifact nobody produces. Because the names hold, `hr/http-cache-rd` and every tenant release keep the `chartRef` they already have and nothing goes not ready. What changes is where the chart comes from: a pinned digest that no later release moves. The migration also applies `cozy:tenant:admin:http-cache-frozen`, a ClusterRole labelled `rbac.cozystack.io/aggregate-to-tenant-admin`, granting the verbs `cozy:tenant:admin:base` used to carry on `httpcaches`. The hook applies it before the upgrade drops the rule from the chart, so an owner's access never lapses and nothing changes for them. Without it a frozen instance would be visible and undeletable by the person who owns it: the wildcard over `apps.cozystack.io` lives in `cozy:tenant:base`, which aggregates into `cozy:tenant` and not into `cozy:tenant:admin`, `cozy:tenant:view:base` is read only, and no role in the tenant chain carries a verb on `helm.toolkit.fluxcd.io`. If no instance exists, the migration removes the platform side instead: the Package first and waited on, because the operator reconciles off an informer cache and re-creates the release it owns while that Package is still there, then the release, its ApplicationDefinition and its Helm storage. Nothing is pinned or granted on a cluster that never used the application. Every path is fail-closed. A failed fleet scan, a failed read of the source OCIRepository, a source with no digest to freeze on, a failed patch or a failed delete stops the migration before it stamps the version. The migration is numbered 58, because main already carries a 57 from the SeaweedFS volume-size-limit change and its `targetVersion` is already 58. #4287 wants 58 as well, so whichever of the two lands second renumbers and bumps `targetVersion` with it. main is merged in rather than rebased, because the branch carries commits that are not mine to rewrite; the one conflict was `migrations.targetVersion`, resolved as 59, one past this branch's migration number. ### The procedure, written down `docs/deprecating-applications.md` is new, and is the point of doing this one carefully. It covers how to choose between migrating instances onto a successor and freezing them, the chain that makes a bare chart deletion strand every surviving instance, how the freeze reads its digest off the cluster's own source, what the migration owes a cluster on each branch, and where the tenant RBAC grant goes. `AGENTS.md` points at it so the next retirement starts there rather than rediscovering it. ### If you run HTTPCache Nothing changes on upgrade. Your instance keeps running and stays manageable through the Cozystack API and the dashboard, and `kubectl get httpcaches` keeps working. What changes is that it is frozen. The chart now comes from the packages artifact your cluster was already running, pinned by digest, and gets no further updates from us, including security updates to the nginx build. There is no replacement in the catalog. Delete the HTTPCache object when you no longer need it, the way you always did; the migration keeps the tenant permission that lets you. ### Verification `helm template` on `packages/core/platform` before and after, for `isp-full`, `isp-full-generic` and `isp-hosted`. In all three the differences are the `http-cache` Package and PackageSource going away and `migrations.targetVersion` moving from 57 to 58, which on a cluster below 58 also renders the migration hook Job and its RBAC. The migration suite `hack/migration-58-http-cache-freeze.bats` covers both branches, the order of every step and each fail-closed path. Freezing on the tarball checksum instead of the manifest digest, dropping the `no-delete` label, dropping the `managed-by` null, dropping the tenant grant, returning from the Package delete without waiting, freezing unconditionally and granting on a cluster with no instance each turn the matching test red. `packages/system/cozystack-basics/tests/clusterroles-tenant-admin-aggregation_test.yaml` pins the aggregation label the migration's ClusterRole reaches tenant admins through, so renaming it fails in the chart rather than silently on the clusters that need the grant. Green: `helm unittest` for `packages/core/platform` and `packages/system/cozystack-basics`, `make migrations-target-check`, `hack/cozystack-version-stamp.bats`, `go test ./internal/controller/cacert/...`, `go build ./...` in the `api/apps/v1alpha1` module, the console `ServicesTab` and `sidebar-icons` suites with `tsc --noEmit`, `make rd-presets-check`, and the bats suites that enumerate packages and image refs. ### Screenshots The catalog entry and its nginx sidebar icon are gone. Nothing new to show. ### Downstream repositories Two repos are affected. No follow-up is open yet, so no box is ticked. `cozystack/website` carries `content/en/docs/*/networking/http-cache.md` in eight doc versions and lists `http-cache` in the hardcoded `NETWORKING` list in its Makefile. The `next` page and the Makefile entry are what need to go, released versions stay as history. `cozystack/terraform-provider-cozystack` has a `cozystack_httpcache` resource and data source. The resource keeps working after this: it is hand written against its own terraform-plugin-framework model and imports nothing from here. What breaks, once that repo bumps its `api/apps/v1alpha1` pin past this release, is `internal/provider/httpcache_model_test.go`, the per kind schema guard, which is the only file over there importing this module. The resource and the guard should go together. The other repos have no reference to the package. - [ ] 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(http-cache)!: remove the HTTPCache application from the catalog. New instances can no longer be created from the catalog and the `HTTPCache` values type is gone from the `api/apps/v1alpha1` Go module. A platform migration keeps existing instances working: on a cluster that runs one, the application is frozen on the packages artifact that cluster was already rendering from, pinned by digest, and stays manageable through the Cozystack API and the dashboard, but receives no further updates, including security updates to the nginx build. On a cluster with no instance the application is removed outright. There is no replacement in the catalog; delete the HTTPCache object when you no longer need it. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Removed** * Removed the HTTP Cache application and its deployment, configuration, dashboard metadata, and packaging from the platform. * The HTTP Cache application is no longer included in the NaaS bundle. * HTTP Cache resources and related API definitions are no longer available. * **Migration** * Existing HTTP Cache installations are frozen during migration; environments without instances are cleaned up. * Platform migration target advanced to version 58. * **Documentation** * Updated storage immutability documentation to reflect Valkey instead of HTTP Cache. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Yan Bondarenko
|
54ac8ed135 |
fix(opensearch): keep the PVC cleanup hook from holding the uninstall
kubectl delete waits for the claims to be gone unless told otherwise, and OpenSearch data pods stop slowly, so the post-delete hook could outlive helm-controller's 300s uninstall budget on a claim that would not release and leave the HelmRelease stuck uninstalling. The delete now runs with --wait=false, the shape the monitoring, etcd and seaweedfs hooks use: the deletion is requested and the claims go once their pods have stopped, which the log now says instead of "complete". The Job carries activeDeadlineSeconds 120 as the backstop for a kubectl that never returns and keeps only before-hook-creation, so a failed run and its log survive. The Role drops watch, which nothing needs once the wait is gone. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> Assisted-by: LLM |
||
|
Yan Bondarenko
|
13be303fbb |
fix(opensearch): remove the data PVCs when the application is deleted
The OpenSearch operator creates its data volumes through a StatefulSet, so the PVCs carry no ownerReference, and the chart had no cleanup hook. Removing the application left every data volume Bound and counting against the tenant quota, unlike clickhouse, kafka and harbor, whose post-delete hooks reclaim the operator-owned storage. A post-delete hook in the same shape as the clickhouse one now deletes the PVCs labelled opster.io/opensearch-cluster=<release>, with a Role limited to persistentvolumeclaims and the kubectl image pinned by digest. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Aleksei Sviridkin
|
bf26ce9744 |
fix(apps): let the cleanup hooks watch the objects they delete (#4135)
## What this PR does Fixes #4085. `kubectl delete` waits for the object to disappear unless `--wait=false` is given, and that wait is a watch on the object. The post-delete hook Roles of `harbor`, `clickhouse`, `mariadb` and `qdrant` grant `get`, `list` and `delete` only, so every uninstall logs ``` Failed to watch *unstructured.Unstructured: persistentvolumeclaims "..." is forbidden: User "system:serviceaccount:<ns>:<release>-cleanup" cannot watch resource "persistentvolumeclaims" ``` the wait is skipped, and the hook prints `PVC cleanup complete` while the claims are still terminating. Every collection-scoped rule behind a delete in these four hooks now also carries `watch`: `persistentvolumeclaims` in all four; `certificates`, `challenges` and the solver `ingresses` in harbor. The four named Secrets the mariadb hook removes keep `get` and `delete` only: they carry no finalizer, so the delete never reaches the watch, and a `resourceNames` rule does not authorize a collection watch anyway. A real wait needs a bound. kubectl's default is a week, and a claim held by a pod that will not stop, or a stuck detach, would hold the hook until helm-controller's 300s uninstall budget ran out and leave the HelmRelease stuck uninstalling. The PVC deletes now run with `--wait=true --timeout=120s`, so the expiry is reported by the script with the claims still terminating named, and each Job carries `activeDeadlineSeconds: 270` with `terminationGracePeriodSeconds: 10`, the numbers `packages/apps/tenant/templates/cleanup-job.yaml` documents against that budget, as the backstop for a kubectl that never returns. `--wait=false` was not an option: it is exactly the report-complete-while-terminating behaviour this PR removes. The chart unit tests are updated and pass: harbor 21/21, clickhouse 24/24, mariadb 111/111, qdrant 104/104. The behavioural suite `hack/app-cleanup-hooks_test.bats` passes 11/11 against the changed scripts. The second observation in #4085, the harbor hook pod occasionally starting after its ServiceAccount is gone, is a hook-weight question and is not touched here. ### Downstream repositories Hook templates and their tests only: no values, schema, README or defaults 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(apps): let the harbor, clickhouse, mariadb and qdrant cleanup hooks wait for the deleted objects, with the wait bounded so a stuck claim cannot hang the uninstall ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Enhancements** - Cleanup operations for ClickHouse, Harbor, MariaDB, and Qdrant now wait for persistent volume claim deletion to complete, with a 120-second timeout. - Cleanup jobs are limited to 270 seconds and allow 10 seconds for graceful termination. - Cleanup permissions now support monitoring relevant resources, including persistent volume claims and, where applicable, certificates, challenges, and ingresses. - Named Secret access is limited to retrieval and deletion. - **Tests** - Updated chart validation to verify cleanup timing, deletion behavior, permissions, and restrictions on unrelated resources. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Timofei Larkin
|
c398e0e116 |
Merge branch 'main' into chore/drop-http-cache
main took migration 57 and moved migrations.targetVersion to 58 while this branch was open, so that line is the one conflict: 57 at the merge base, 58 on main, 59 here. Resolved as 59, which is what this branch's migration 58 requires, since run-migrations.sh loops up to targetVersion - 1. Merged rather than rebased because the branch carries commits that are not mine to rewrite. Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Yan Bondarenko
|
334f6adac6 |
fix(nats): merge config.merge.accounts with the generated accounts map
With users set the chart emitted accounts.A.users from the generated passwords and then appended .Values.config.merge verbatim. A tenant that also put an accounts block into config.merge, which is the only place to enable JetStream for the generated account, got the key twice in the rendered values and the release failed at post-render. The generated users are now merged into config.merge before it is rendered: the generated list wins for account A, everything else the tenant put under config.merge is kept, and a config.merge without users passes through unchanged. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Yan Bondarenko
|
9ee2a2375c |
fix(foundationdb): let tenant roles read the connection ConfigMap
The operator stores the cluster file in the <release>-config ConfigMap, but the dashboard resource map did not list it and no RBAC granted tenant subjects access to it, so a tenant could not retrieve the connection string for its own FoundationDB at all. Add the ConfigMap to the dashboard resource map and create a Role/RoleBinding scoped to exactly that ConfigMap (get/list/watch) for the tenant use-level subjects. Add helm unittest coverage. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Aleksei Sviridkin
|
bd3892253b |
fix(clickhouse): keep the Keeper name inside the StatefulSet volume-name limit (#4133)
## What this PR does Fixes #4083. clickhouse-operator names the Keeper config volume `chk-<chk>-deploy-confd-<cluster>-<shard>-<replica>`. With the CHK called `<release>-keeper` and `Release.Name` already prefixed with `clickhouse-`, an application name longer than 15 characters pushes that volume name past 63 characters. The StatefulSet is rejected on every reconcile, no Keeper pod appears, and the CHK still reports `Completed`; the only visible signal is the Keeper `WorkloadMonitor` staying at `operational: false`. `templates/_keeper.tpl` now computes the CHK name: `<release>-keeper` as before when it fits, otherwise the release name is truncated and a six-character hash of the release name is appended so two long names cannot collide. The budget accounts for the width of the replica index. `chkeeper.yaml` and the zookeeper `nodes` list in `clickhouse.yaml` both use the helper, so the hosts ClickHouse connects to always match the CHK the operator creates. The pod label, the `VMPodScrape` selector and the `WorkloadMonitor` stay on `<release>-keeper`. Names up to 15 characters render byte-for-byte as before. A release with a longer name never had a Keeper pod; on upgrade its pod-less CHK is replaced by the shortened one and the Keeper comes up. `helm unittest`: 30/30 in the chart, including the new `tests/keeper_name_test.yaml` (short name unchanged, the exact 63-character boundary, one character past it, a very long name, a two-digit replica index, no trailing dash after truncation). ### Downstream repositories Templates and tests only: no values, schema, README, defaults, `ApplicationDefinition` or tenant-facing Secret/Service names 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(clickhouse): shorten the Keeper name for application names longer than 15 characters so the Keeper StatefulSet can be created ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Fixed ClickHouse Keeper resource naming for long release names so generated names remain within Kubernetes’ 63-character limit. - Updated ClickHouse Keeper hostnames to consistently match the generated resource name, including when names are shortened. - Preserved identifying labels and monitoring selectors while applying the corrected Keeper naming behavior. - **Tests** - Added coverage for standard, boundary-length, and truncated Keeper names, including replica-specific naming scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
355630a5e7 |
fix(kubernetes): pin KubeVirt CSI sidecars to SIG Storage releases (#3934)
## What this PR does Replaces five mutable OpenShift `latest` image references (four unique sidecar images) in the KubeVirt CSI controller and tenant-node DaemonSet with versioned Kubernetes SIG Storage releases. This addresses the affected references from #3578 without claiming the other references in that issue. The selected controller versions match the sidecars already used by Cozystack's LINSTOR CSI deployment. Render tests pin every changed container name and image so a mutable tag cannot return silently. Validation: - `helm unittest packages/apps/kubernetes`: 247/247 passed. - `helm unittest packages/system/kubevirt-csi-node`: 6/6 passed. - Live MSK-prod guest-cluster canaries on 2026-08-20 and 2026-08-21 reproduced the failure with the stock chart: the `latest` sidecars exited on the old CPU with the x86-64-v3 glibc error and the controller stayed in CrashLoopBackOff. A disposable guest cluster with the pinned images reached a 7/7 Ready controller on 2026-08-19 and was removed after the check. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map against the four changed template/test files. No package, schema, API, default, CRD, tooling contract, or downstream hardcoded value changes. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note fix(kubernetes): pin KubeVirt CSI sidecars to versioned SIG Storage images that run on pre-x86-64-v3 CPUs ``` |
||
|
Yan Bondarenko
|
f886f26ad4 |
fix(apps): bound the wait the cleanup hooks now perform
With watch granted, kubectl delete waits for the claims to be gone, and its default for that wait is a week. A claim held by a pod that will not stop, or a stuck detach, would hold the post-delete hook until helm-controller's 300s uninstall budget ran out and leave the HelmRelease stuck uninstalling. The PVC deletes in the clickhouse, harbor, mariadb and qdrant hooks now wait 120s at most and report the claims still terminating by name, and each Job carries activeDeadlineSeconds 270 with a 10s termination grace period, the numbers packages/apps/tenant/templates/cleanup-job.yaml documents against that budget, so a kubectl that never returns cannot hold the hook either. The mariadb Role no longer grants watch on the named Secrets: they carry no finalizer, so the delete never reaches the watch, and a resourceNames rule does not authorize a collection watch anyway. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> Assisted-by: LLM |
||
|
IvanHunters
|
8a6fe02ee4 |
fix(bucket): stop the ACME cleanup hook from talking to localhost, and gate the class (#4289)
## What this PR does Fixes #4288, and closes the class behind it and #4276. `packages/apps/bucket/templates/hooks/cleanup-acme.yaml` passed `--request-timeout=30s` on all four of its kubectl calls. A non-zero value there makes kubectl skip its in-cluster credentials: client-go consults them only while the assembled config still equals the default, and the flag lands in that config, so every call went to kubectl's built-in default cluster and was refused. This package failed more quietly than kafka. Each call is guarded by `|| echo "WARNING: ..."` and the script sets no `-e`, so the hook exited 0 and printed `ACME cleanup complete.`, and `hook-succeeded` then deleted the Job holding those warnings. The ACME solver objects it exists to reap were left behind, with the only record removed moments after it was written. Its own suite matched the rendered command including the flag, so the broken form was pinned in place. The gate is the point of the change rather than a bonus. Two packages carried this, neither was visible in a rendered manifest, and both passed their unit suites. Nothing stops the next hook from reaching for the same flag, because it reads as ordinary defensive practice. `hack/check-hook-request-timeout.bats` fails any non-vendored template whose kubectl call carries a non-zero `--request-timeout`. The coarse part is stated in the file rather than left to be discovered: a template that names `KUBECONFIG` is skipped, because an explicit kubeconfig is not served by the in-cluster branch and the flag is harmless there. That is a per-file judgement, not per-command. It is what lets `apps/kubernetes` keep the flag on two calls that pass their own kubeconfig and stay green, and it is the first thing to tighten if a file ever mixes both styles. Checked in both directions rather than only on the clean tree: the gate passes as committed, fails if the flag is put back on any bucket call, fails if it is put back on the kafka call, and leaves the two `apps/kubernetes` files alone. It also carries a vacuity guard, because a collapsed glob would otherwise report success for having examined nothing. Bounding the work is still available where it belongs: `--timeout` for a watch, `activeDeadlineSeconds` for the Job, which this hook already had at 120s. **Depends on #4280** and is branched off it, so the gate is green here. #4280 removes the same flag from the kafka hook; merged before it, this check would be red on `main`. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map against the diff. A hook's kubectl flags and a new `hack/*.bats` check; no values key, schema, API, default or naming contract moves, and nothing under `hack/` is moved or renamed, nor does any make target change what it does. - [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(bucket): the ACME cleanup hook now reaches the apiserver instead of reporting success while doing nothing ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved ACME cleanup reliability by preventing cleanup commands from targeting an unavailable local Kubernetes endpoint. * Improved Kafka uninstall cleanup reliability by ensuring the pre-delete hook retains access to required credentials throughout execution. * Preserved existing execution time limits and warning-based failure handling for cleanup operations. * **Tests** * Added coverage to prevent regressions in Kubernetes cleanup command configuration and hook resource lifecycle behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
3a43bcf391 |
docs(kafka): say why the hook's Job and its credentials are reaped differently
The policies are right and nothing here changes them. What is missing is the reason, and this file has now been changed in both directions inside a week by people reasoning from the annotations alone, so the reason is the part worth keeping. The Job carries no hook-succeeded because the script exits 0 on the degraded path as well as the clean one, and its log is the only record that a finalizer outlived the wait; ttlSecondsAfterFinished reaps it on a timescale a human can act within. The ServiceAccount, Role and RoleBinding have no such clock, and before-hook-creation fires on a next activation that after the last uninstall never comes, so without a terminal policy they sit in the tenant namespace for the life of the cluster still granting delete over every KafkaTopic in it. Reaping them cannot strand a running pod, which is the objection the symmetrical shape was written against: helm applies hook-succeeded only once the hook is terminal, and activeDeadlineSeconds ends the Job inside the uninstall hook timeout. hook-failed would decide nothing, because a failing hook already reaps the earlier successful ones under hook-succeeded. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
coderabbitai[bot]
|
a295e29b8a | fix(kafka): clean up pre-delete hook ServiceAccount and RBAC on success | ||
|
IvanHunters
|
3615958240 |
fix(bucket): stop the ACME cleanup hook from talking to localhost, and gate the class
The hook's four kubectl calls carried --request-timeout=30s. A non-zero value there makes kubectl skip its in-cluster credentials: client-go consults them only while the assembled config still equals the default, and the flag lands in that config, so every call went to kubectl's built-in default cluster and was refused. This package failed more quietly than kafka did. Each call is guarded by `|| echo WARNING` and the script sets no -e, so the hook exited 0 and printed "ACME cleanup complete."; hook-succeeded then deleted the Job that held those warnings. The ACME solver objects it exists to reap were left behind, and the only record went with the Job. Its own suite matched the rendered command including the flag, so the broken form was pinned. The gate is the point of this change. Two packages had it, neither was visible in a rendered manifest, and nothing stops the next hook from reaching for the same flag, since it reads as ordinary defensive practice. hack/check-hook-request-timeout.bats fails any non-vendored template whose kubectl call carries a non-zero --request-timeout, skipping files that name KUBECONFIG because an explicit kubeconfig is not served by the in-cluster branch. That per-file judgement is the coarse part and is written down in the file rather than left to be inferred: apps/kubernetes keeps the flag on two calls that pass their own kubeconfig, and stays green. Bounding the work is still available where it belongs: --timeout for a watch, activeDeadlineSeconds for the Job, which is what this hook already had at 120s. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
e63b8595d3 |
fix(kafka): drop the request timeout that hid the apiserver from the hook
A non-zero --request-timeout makes kubectl skip its in-cluster credentials altogether. client-go consults them only while the assembled config still equals the default, and the flag lands in that config, so the branch that would load the ServiceAccount token never runs. kubectl falls back to its built-in default cluster, localhost:8080, is refused, and the script reads the refusal as a failed delete and exits 1. A non-zero pre-delete hook returns from Uninstall.Run before any resource is removed, so no Kafka release could finish deleting and helm-controller retried forever. Measured on the pinned image rather than argued from the docs: with the token and ca.crt mounted and KUBERNETES_SERVICE_HOST set, the same delete reaches the cluster without the flag, goes to localhost:8080 with --request-timeout=20s, goes to localhost:8080 with 1m, and reaches the cluster again with 0. The three in-tree hooks that work do not pass it. apps/kubernetes names it in a comment only, apps/harbor and apps/mariadb never mention it, and apps/tenant records that its DELETE calls "carry kubectl's default --request-timeout of 0". The watch stays bounded by --timeout, and the Job by activeDeadlineSeconds, which is that same shape. The suite now rejects the flag. It needs the "=" because the explanation in the script names the flag too, and matching the bare name would fail on the comment rather than on a use. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
64d5689434 |
fix(kafka): keep the pre-delete hook's credentials as long as its Job
The Job carries no hook-succeeded, so it outlives the hook run and ttlSecondsAfterFinished reclaims it, which is what leaves a readable record when the finalizer outlives the wait. Its ServiceAccount, Role and RoleBinding still carried hook-succeeded and hook-failed, so Helm reaped them at the end of the run either way and left the Job behind them. A Job whose ServiceAccount is gone still starts a pod. kubelet mounts the projected volume without a token, client-go finds no in-cluster config, kubectl falls back to localhost:8080, and the script takes its catch-all branch and exits 1. A non-zero pre-delete hook returns from Uninstall.Run before any resource goes, so the release never finishes deleting and helm-controller retries it: the e2e run on the merge commit shows six such pods in five minutes, in two namespaces, every one of them dead the same way. The hook meant to prevent one stuck Kafka release was instead stopping all of them. Give all four objects the same before-hook-creation policy, so the credentials live exactly as long as the consumer and the next run replaces them. Both other in-tree hooks that reach the apiserver, apps/kubernetes and apps/harbor, are symmetric this way already. The suite now pins the policy on each of the three, because nothing compared them: the asymmetry rendered cleanly, passed 37 helm-unittest cases and 5 bats cases, and only a cluster showed it. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
a2db2aeb8c |
fix(seaweedfs): raise the number of buckets a volume server can serve
A volume server started with -max 0 takes a slot count derived from disk rather than a fixed one, replication 001 spends a slot on each of the two servers per logical volume, and a collection claims COPY_2 volumes whenever it grows. So the bucket ceiling is set by the product of the size limit and COPY_2, and moving either number alone moves it. The mechanism is upstream in MaybeAdjustVolumeMax (weed/storage/store.go) and findVolumeCount (weed/topology/volume_growth.go). Do not divide these out of the nominal PVC size, because the count follows the space the filesystem makes available and not the volume's advertised size: the shipped 10Gi volume PVC was seen to give nine slots per server at the upstream 1000 and 19 at 500, which puts 400 at 23 to 24 by the same division, or eight collections against two. The span is the observation's own, since the free space behind 19 at 500 fits more than one filesystem geometry and those divide differently at 400; eight either way, because a collection that cannot get a whole batch of COPY_2 keeps the volumes it did get and writes into them. The division itself is measured: on a 1 GB image the master reports max as the filesystem's available space over the limit in every run, 25 slots and nine collections at the 40 and 3 that scale to this pair. Two is what the platform's own cozy-backups bucket and a single tenant bucket already use. Past that every S3 PUT to a new bucket times out while the disk still reads as nearly empty, and no COSI object reports anything wrong, because the claim and the access are both granted. 400 and 3 make the budget 1200 MB per collection. Splitting the reduction across both numbers rather than taking it out of the size limit alone keeps the cost off the curve that grows: open descriptors track stored bytes divided by the limit, so a 10 TiB server keeps roughly 52k of them at this limit against 21k upstream, where moving the size limit alone would have put it at 105k. A .dat and an .idx stay open per volume; the .vif beside them is written and closed. The ceiling reached is the same either way. Both figures describe the shipped replicationFactor of 2. The master picks its growth count from the replication the filer asks for, so at 1 and 3 copies it reads COPY_1 and COPY_3 instead and this COPY_2 does not apply; lowering the size limit still raises the ceiling there. The size limit was out of an operator's reach: it lived in the data-plane chart, which is pulled by ExternalArtifact from a component that declares no install section, so a Package-level values override for it is dropped before any merge. The app chart now declares master.volumeSizeLimitMB and sends it on every release, and 400 is set there. The data-plane chart keeps upstream's 1000 as its fallback on purpose. A platform upgrade can re-render that release from its artifact before the parent has sent anything new, and a lower fallback would then reach the master first and drop the limit ahead of an operator's own setting. COPY_2 is sent by nothing and lives in the data-plane chart, so it does land in that window, where a grow costs 3000 MB against the 6000 MB it cost before. The app chart also writes maxVolumes into dataDirs on every release, and because dataDirs is a list Helm replaces the data-plane chart's copy wholesale, so the zero that makes the slot count auto-derived at all comes from there. Losing it drops the server to the subchart's fixed seven without failing any render. The field is bounded at both ends, because either end takes an instance down. Zero is not a smaller limit but an off switch: the volume server leaves its slot calculation untouched at zero, keeps reporting no capacity, and nothing is ever grown. Past 30000 the master does not start at all. The bound sits in the values schema rather than in a template guard, so Helm refuses the value before any template runs. The apps API does not: it reads that schema only for defaults, so it accepts a value out of range, and the release then fails to render. An explicit null gets past both bounds, because Helm drops the key before it validates, and would render an empty -volumeSizeLimitMB= that the master exits on at start, measured on the shipped image, so the template requires the value. No bound against the disk goes with it, though the field invites one. A limit at or above the volume's own size does not break an instance: MaybeAdjustVolumeMax floors the slot count at one, measured on 4.31 with a 255 MiB disk against a 1000 MB limit, which reported Max 1 and served a write it then read back. Such a server is capped at a single slot rather than dead, so refusing it at render would be a capacity policy that stops a working release on every reconcile, not only on the upgrade. The field's description carries the relation instead, where the value is chosen. The growth count is deliberately not exposed beside the limit. The master picks which of its four growth counts to apply from the replication the filer requests, and the filer takes that from replicationFactor, so a field wired to COPY_2 would do nothing at 1 or 3 copies while still appearing to be a setting. A knob that is silently inert for two of the three supported values is worse than no knob. Two further prices. A collection absorbs the budget before it needs another grow, so growth requests arrive five times as often as at the upstream pair. A collection also starts with half as many writable volumes, and a volume server takes the writes into each volume one at a time (syncWrite holds that volume's dataFileAccessLock), so a heavily parallel writer spreads over fewer volumes until the collection grows again; that holds for instances kept at 1000 too. And lowering the limit on an instance that already holds data carries the other one: its volumes between the new limit and the old go oversized on the next heartbeat and leave the master's writable set, and each affected collection needs one successful grow before its next write lands. Nothing is lost and the volumes stay readable; the room left inside them for further writes is given up and does not return as capacity. Those grows compete for one pool, and the pool is smaller than it looks. An oversized volume is charged no headroom, while one below the limit is charged all of its unfilled room; a grow then claims a whole COPY_2 batch and takes as much of it as is there, and the volumes it creates are empty and charge their headroom straight back. Measured on a local SeaweedFS 4.31 setup: two oversized collections with two free slots between them, the first to write took both, and the second stayed refused with the disk still reporting a quarter of itself free. Collections that need no grow are not free either, since their volumes below the new limit were created six at a time and each can hold back a whole limit. Also measured there, with the limit and batch scaled down: a server with free space for one batch per collection refused the grow its one affected collection needed, because a second, nearly empty collection held the room, and the same free space without it let the grow through. The minFreeSpacePercent reserve only comes into it on a server whose writable volumes have all passed the limit, where nothing is charged headroom: from 8000 MiB of filesystem upward, a 10Gi PVC measuring 9979, five percent is at least 400 MB, and such a server then refuses a grow only once its disk is down to the reserve. Everywhere else the headroom charge decides, and a refusal can arrive with the disk far from full whatever the reserve. Deleting a collection gives the room back a slot per volume, however little it stored, since the headroom charged to those volumes leaves with them. So, on a variant that runs platform migrations, the upgrade does not change the limit on an existing instance unless its operator already set one, which the previous chart accepted and ignored. Platform migration 57 writes master.volumeSizeLimitMB: 1000 into every existing SeaweedFS instance that has not chosen a limit, and 400 applies only to instances created from here on. It leaves alone only an instance whose master is not an object, which no v1 chart renders, and names that one in its log. The default variant runs no platform migrations, so a cluster there that installed SeaweedFS by hand gets no pin, and the document below gives the same patch to apply before upgrading. A chosen limit is a whole number the new chart accepts; anything else counts as not chosen and is replaced with 1000, which is what the previous chart ran in its place, since it ignored the key and the new chart would refuse to render it. A valid value already set is kept, and since the previous chart ignored it, the upgrade is when it starts to apply; the document below lists the ones that change the limit ahead of it. Raised values are listed as well: existing buckets keep writing, but every volume below the new limit is charged up to it, and on a local SeaweedFS 4.31 setup seven nearly empty volumes left no free slot once the limit went up tenfold, so a new bucket was refused. The value goes into the application's HelmRelease, whose spec.values is the application spec, for an instance created through the API and for one the tenant module renders alike. The tenant module never renders spec.values on that release, and a Helm upgrade leaves a key alone that neither manifest carries: measured with Helm 4.3.0 on client-side, server-side and forced server-side apply, and read in createPatch at the v4.1.1 that helm-controller v1.5.0 runs. Where the migration runs, it is a pre-upgrade hook of the platform release, and the cozystack-packages source every application chart comes from is a manifest of that same release, applied only after its hooks; so no instance meets the new chart unstamped, and a failed migration stops the platform upgrade before that source moves. That holds while the platform source is pinned by digest, as it is out of the box; one that follows a tag, a semver range or a branch lets source-controller move cozystack-packages on its own schedule. Until cozystack-packages moves, each patched release keeps its old chart, which ignores the key, since no v1 release reads it or closes the master object in its schema. An instance kept at 1000 still gains, because at a replication factor of 2 a grow now claims three volumes rather than six, so it has room for roughly twice as many new buckets. Lowering an existing instance is left to its operator, and docs/operations/seaweedfs-lowering-the-volume-size-limit.md is the check for it. It lists each server's volume files and adds one limit for every volume below the limit and a batch for every collection with no volume at or below 90 percent of it, since the master grows such a collection on its own schedule without a write; measured the same way, a collection with every volume in that band and no writes got two new volumes per server within one pass. Where each disk type is served by as many volume servers as the replication factor, as the shipped two at a factor of 2 are, each collection's volumes sit on every server of that type, and that bounds the claim from above without assuming how the collections grew. The master places a new volume on any server of the requested disk type with room rather than where its collection already lives, whichever group or pool that server belongs to, and it reads an empty disk type as hdd; under MultiZone it also grows each zone on its own. So the document keeps zoned instances, and any disk type served by more servers than copies, at 1000. BREAKING CHANGE: on the default variant, which runs no platform migrations, set master.volumeSizeLimitMB to 1000 before upgrading on every SeaweedFS instance that has no valid value and whose master is an object, with the step in docs/operations/seaweedfs-lowering-the-volume-size-limit.md. On the variants that run platform migrations, migration 57 does this. If the step is skipped, every such instance drops to 400 on the upgrade: each of its volumes at or above 400 MB leaves the master's writable set, and a bucket whose volumes have all crossed that line takes no writes until it can grow. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
IvanHunters
|
637098f222 |
test(kafka): anchor the release selector the hook suites assert on
Both suites matched "strimzi.io/cluster=kafka-test" as a bare substring, and that is a prefix of the sibling release kafka-test2 and of kafka-test-topics. Rendering the hook with either selector left both suites green, so neither pinned the scoping they exist to pin. Anchor on the leading "-l " and on the trailing delimiter; both variants now red in both suites, and the six earlier hook mutations still red exactly where they did. Drop the notMatchRegex on a leading slash. It cannot fail on its own: the anchored image pattern above rejects a leading slash first, and an empty _cluster.images-registry renders no slash to begin with. Its rationale moves into that pattern's comment. Record why activeDeadlineSeconds and the 180s wait have to move together. A deadline that fires mid-wait kills the Job, and with backoffLimit: 0 the hook then returns non-zero, which is the wedged uninstall the deliberate exit 0 on the timeout branch exists to avoid. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
415aa82409 |
test(kafka): drive the hook suite through the runner CI actually uses
The suite used bats' run helper. make bats-unit-tests does not invoke the bats binary: it runs hack/cozytest.sh over every hack/*.bats, and that runner implements @test and little else, so the file died at 'run: command not found' and took make unit-tests with it. Under bats it was 5/5, which is where the 5/5 in the previous commit came from. Save $? by hand instead, the shape app-cleanup-hooks_test.bats already uses; verified under both runners. The fake kubectl no longer defaults DELETE_RC to 0. An unset value made every failure-path assertion vacuous; now it exits 64 and says so. Cover both RESTMapper wordings for an absent CRD rather than just the first. Widen the image pattern to v[0-9]+\.[0-9]+(\.[0-9]+)? like harbor and mariadb: pinning v1\.32 reintroduced the breakage the digest was made pattern-based to avoid, one Renovate bump later. Assert the _cluster.images-registry override and the no-leading-slash default, neither of which was covered: the hook is the one image in this chart whose pull failure surfaces at uninstall. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
3326e0fb9e |
test(kafka): run the pre-delete hook against a fake kubectl
The hook's exit code is the whole contract: a non-zero one returns from Uninstall.Run before any release resource goes, keeping the topic operator alive; a zero one lets Helm remove it. delete_test.yaml matches manifest text, so it stayed green when the timeout branch was put back to exit 1, when the CRD-absent branch was, and when the selector was widened past this release. Add a bats suite in the shape hack/app-cleanup-hooks_test.bats already uses for qdrant and harbor: render the script, run it against a fake kubectl, pin the exit code of each branch. Each of the three mutations above now turns it red. Drop hook-succeeded from the delete policy and reclaim the Job with ttlSecondsAfterFinished instead. The script exits 0 on the degraded path too, where topics still hold the finalizer, so deleting on success removed the only record of that seconds after it was written. Assert both in delete_test.yaml. Pin the hook image by shape rather than by literal digest: Renovate rewrites digests under packages/*/templates/hooks/ and nothing rewrites tests/, so the literal turns the next bump red for no defect. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Yan Bondarenko
|
210b7fe30c |
fix(kafka): stop the pre-delete hook from wedging the uninstall
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Yan Bondarenko
|
44ba13b527 |
fix(kafka): delete release topics before the topic operator is removed
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Andrei Kvapil
|
b24856561c |
chore(http-cache): remove the application and its ResourceDefinition
HTTPCache is the least-installed application in the catalog and the only one whose dependency-refresh path does not work: every sed in its update target rewrites images/nginx/Dockerfile while the image lives at images/nginx-cache/Dockerfile, so the target fails on its first line. The pins show the result - nginx sits at 1.25.3 against 1.31.5 upstream and ngx_cache_purge a major version behind. Maintaining a bespoke nginx build with third-party C modules is not something we are prepared to carry for this level of use. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
IvanHunters
|
4198914dae |
fix(opensearch): give the securityconfig update job its own resources
The operator hands spec.security.config.updateJob.resources straight to the Job container and the chart left it unset, so a tenant namespace applied its LimitRange default of 128Mi instead. securityadmin.sh runs java with no -Xmx and is OOMKilled at that size, and the operator builds the Job with backoffLimit 0, so that one kill is final: the security index is never written and every node stays unready. Constants rather than a preset value, because the tool configures the cluster once and exits. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
e3ab40eb83 |
fix(virtual-machine): guard Kube-OVN IP lookup (#4013)
## What this PR does Guards the Kube-OVN `IP` lookup behind API discovery. The `vm-instance` chart now renders on clusters using another CNI instead of trying to query a CRD that is not installed. The Helm regression suite mocks the IP object through `kubernetesProvider` and pins both sides: with `kubeovn.io/v1/IP` advertised the annotations carry the mocked MAC and IP, without it they are absent even though the object exists, which is the branch the guard adds. helm-unittest's fake lookup never errors on an unserved kind, so only the second case distinguishes the guard; it fails on main and passes here. Open PRs #3991, #3978, #3931, and #3900 touch the same template. Their current hunks do not alter this lookup; #3900 adds secondary-interface annotations immediately after it, so it is the only nearby merge hunk to watch. ### Screenshots Not applicable; there is no UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note fix(virtual-machine): render VM instances on clusters where the Kube-OVN IP API is not installed. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * VM instances now render correctly in environments without the Kube-OVN IP capability. * OVN IP lookups and MAC/IP annotations are only generated when the required Kube-OVN API is available. * MAC/IP annotations are omitted when the capability is unavailable, even if an IP object exists. * **Tests** * Added coverage for both supported and unsupported Kube-OVN IP discovery scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
cecb4b11d3 |
feat(opensearch)!: issue HTTP and Dashboards certificates from cert-manager (#2682)
## What this PR does The OpenSearch HTTP API (9200) and Dashboards (5601) already served TLS, from the operator's own private CA and with a SAN built from `cluster.local` while the credentials Secret advertised the release on the platform domain, so nothing a client was handed could verify. Both certificates now come from a per-release cert-manager CA in the tenant namespace, and the CA reaches tenants as a key-free trust anchor. - `tls.issuer` selects who issues the HTTP server certificate, `cert-manager` or `operator`, and is named for the issuer because TLS is served under both. Unset resolves from `external` (cert-manager for externally published services, the operator for cluster-internal); an explicit value always wins. - Renders a self-contained cert-manager chain in the tenant namespace: a self-signed Issuer, a CA Certificate, a CA Issuer, and two leaf Certificates, one for the HTTP API and one for Dashboards, each with the SANs of its own Service and the external hostname when `external: true`. Leaf keys are written as PKCS8, since the JVM loader rejects the PKCS1/SEC1 encoding cert-manager emits by default, and the CA subject is pinned so the operator's signature check accepts the chain. - The CA Secret is `<release>.http-ca`. The dot separates the chart-chosen suffix from release names, so no other release can produce that name; the two leaf Secrets stay hyphenated because none of the chart's suffixes is a tail of another. The operator's own transport CA remains at `<release>-ca` and is not touched. - Transport mTLS (9300) stays operator-managed (`transport.generate: true`, `perNode: true`). The security plugin requires it and the chart cannot turn it off. - `hack/e2e-chainsaw/opensearch/` runs the move on a cluster: it installs the app on the operator's HTTP listener, removes `tls.issuer` so it resolves from `external`, and asserts the cert-manager chain, the CR dropping `http.generate`, the admin certificate reissued under the chart CA, the StatefulSet rolling onto the three subPath mounts, the tenant anchor, and the certificate the endpoint actually presents to a client holding that anchor. ### Sysctl carrier and single-node rolls The E2E suite for this PR was red because the chart set `spec.general.setVMMaxMapCount: true`, which makes the operator prepend a privileged `init-sysctl` container to every OpenSearch pod. Tenant namespaces run at the cluster's default PodSecurity level, baseline on Talos, so admission refused the container and the StatefulSet never produced a pod (#4131). The carrier for the sysctl is now on main: #4152 added a DaemonSet to the `opensearch-operator` package that raises `vm.max_map_count` to a floor of 262144 on every node, from a namespace the platform labels privileged. This branch is rebased onto that main and sets the flag to an explicit `false`, with a template comment saying why and a unit test pinning it, so a release no longer asks the operator for a container admission refuses. The value stays with the DaemonSet until the node contract carries it (#4144); the DaemonSet's own template comment, which said the chart still asks, now says it does not. Three more things came out of the same pass. The chart asked the operator to drain data nodes before every rolling restart, and with a single data node the drain never finishes: at v2.8.0 `PreparePodForDelete` excludes the node from allocation and then waits for it to hold no shards, which the only data node can never satisfy, so a one-replica release could not roll for a version bump or for the certificate move this PR relies on. The chart now drains only when a second data node exists, pinned both ways in the unit tests. The chart maps an empty `storageClass` to `replicated` at one replica, and the PR lane runs LINSTOR without DRBD and creates only `local`, so the E2E fixture takes its class from the `storageClass` value the harness passes with `--set-string` from `COZY_E2E_STORAGE_CLASS`, the way the vminstance fixtures do; on the nightly QEMU lane that still resolves to `replicated`. And the external Dashboards Service selected labels no pod carries. `pkg/builders/dashboards.go` at v2.8.0 stamps `opensearch.cluster.dashboards: <cluster name>` on the Deployment, its selector and the pod template, and nothing else; the chart asked for `opster.io/opensearch-cluster` together with `app.kubernetes.io/component: dashboards`, so the LoadBalancer came up with no endpoints and no error to show for it. It selects the label the operator sets now. ### securityadmin cross-trust, still blocked on the DNS base Chart-managed HTTP TLS previously broke the operator's securityadmin reconciliation: the admin certificate was signed by the transport CA while the HTTP listener trusted the cert-manager CA, so the two never verified against each other and users and roles silently never applied. This chart puts one CA under both sides, handing the HTTP CA to the operator through `spec.security.tls.http.caSecret` so that it signs the admin certificate with the CA the listener trusts. That closes the CA mismatch. The other half, the job resolving a name the operator built from `cluster.local` on a cluster that does not use it, was closed separately and is on main, so the security configuration applies once this lands. No operator bump is required; the API exists in the pinned version. That path is version-gated upstream at OpenSearch 2.0.0: below it the operator falls back to the transport CA and the mismatch returns with no error anywhere. The chart therefore refuses chart-managed HTTP TLS on `version: v1` with an explicit render-time failure that names the mechanism and offers the alternatives, rather than letting it fail silently at runtime. ### Trust anchor for tenants The HTTP server's private key is not exposed to tenants. The chart renders a `TenantProjection` sentinel that names `<release>.http-ca` as its source; the CA-extraction controller publishes a `ca.crt`-only copy as `<release>.tenant-ca`, and the ResourceDefinition selects it with the engine-agnostic `internal.cozystack.io/tenant-ca` label. The sentinel is gated on the same condition as the chain, so a plaintext release renders no sentinel instead of one stuck at `SourceNotFound`. Tenants get what they need to verify the server and nothing more. ## Known limitations These are stated rather than left to be discovered: 1. **securityadmin needed both trust and reachability, and both are closed now.** This branch put one CA under the admin certificate and the HTTP listener. The other half was the operator resolving `<svc>.<ns>.svc.<DNS_BASE>` with `DNS_BASE` left at the vendored `cluster.local`, so the job waited on a name that never resolved whichever CA had signed; the platform now renders that value from `networking.clusterDomain` and it is on main, which is what unblocks this branch. 2. **The e2e suite reaches the move, not the chart bump that triggers it.** The suite above arrives at the chart-managed chain by removing `tls.issuer` from a running release, which leaves exactly the values an upgraded release arrives with. The one step it cannot run is the one where the previous chart version wrote the CR, because there is no second chart version to install from in this tree. Everything downstream of that write is exercised on a cluster. 3. **A reissue does not reach a running pod.** Neither service reloads its certificate, and cert-manager reissues on any change to `external`, the tenant host or the cluster domain, so an ordinary edit leaves the pods serving material that no longer matches their own names with no error, event or condition to show for it. Completing such a change is manual. So is removing the CA private key cert-manager leaves behind when TLS is turned off again, since the platform runs with `enableCertificateOwnerRef: false` and deleting a `Certificate` does not delete its `Secret`. `packages/apps/opensearch/README.md` carries both, with the commands. ### Downstream repositories `packages/apps/opensearch/values.schema.json` gains the `tls` object, so `cozystack/terraform-provider-cozystack` needs the field modelled for `cozystack_opensearch`; tracked in cozystack/terraform-provider-cozystack#41. ### Upgrade impact An existing `external: true` release with `tls.issuer` unset serves operator-issued HTTP TLS today and moves to chart-managed TLS on its first reconcile after this lands: the cluster rolls once and is re-anchored on a new per-release CA, so any client pinned to the operator's CA has to be repointed at the key-free `<release>.tenant-ca`. Setting `tls.issuer: operator` before the upgrade keeps that half as it is. Releases that are not external are untouched. Every existing release rolls once regardless of `tls.issuer` and `external`, because the pod template loses the operator's `init-sysctl` container; on a single-replica release that roll is the first one the operator can complete, since draining the only data node used to block every rolling restart. The published external-dns name changes on the same upgrade whatever `tls.issuer` says, because the Services are gated on `external` alone: they move from `<release>.<namespace>.<cluster-domain>`, an in-cluster name that never resolved outside the cluster, to `<release>.<tenant-host>` and `<release>-dashboards.<tenant-host>`. external-dns is configured upsert-only here, so the old record is not withdrawn and keeps pointing at the LoadBalancer until it is removed from the zone by hand. That cleanup is per zone rather than per release and nothing enumerates the records, so the README shows how to derive the list from the cluster. ### Release note ```release-note feat(opensearch)!: the OpenSearch HTTP API and Dashboards already served TLS, from the operator's own private CA and with a SAN built from cluster.local while the credentials Secret advertised the release on the platform domain; both certificates now come from a per-release cert-manager CA whose anchor the platform publishes into the tenant as <release>.tenant-ca. Any client that worked against the old endpoint was skipping verification, because no hostname it was handed could have matched that SAN. Existing external releases move from the operator's CA to the chart's on their first reconcile, and their published external-dns name moves from the in-cluster domain to <release>.<tenant-host> with the old record left in place; set tls.issuer: operator beforehand to opt out of the CA move. With dashboards.enabled the application name is limited to 41 characters, and to 32 when external is on as well; past either bound the chart fails the render and withholds every object in the release. A release already above a bound is not broken by the guard, it is told why it has not been upgrading: the Dashboards Service the operator builds from that name exceeds the 63-character limit for a DNS label, the operator returns that failure on every pass, and the reconcilers queued behind Dashboards, which are upgrade and restart and snapshot repository, have not run since the release was created. Set dashboards.enabled to false to drop the bound. The external Dashboards Service also selected labels the operator does not put on Dashboards pods, so it stood up with no endpoints; it selects opensearch.cluster.dashboards now. The drain of data nodes is now requested only with the data role on and more than two replicas, since at two the operator gives up on a full drain itself. The chart stops asking the operator for its privileged init-sysctl container, since the opensearch-operator package now sets vm.max_map_count from a DaemonSet, so every existing release rolls once on upgrade; single-replica releases no longer ask the operator to drain their only data node, which used to block every rolling restart ``` |
||
|
Aleksei Sviridkin
|
76c9ece4ab |
fix(opensearch): stop asking for a drain the cluster cannot perform
The chart asked the operator to drain data nodes before restarting them, whatever the replica count. Draining moves a node's shards to another data node and waits until none are left, and below three data nodes there is nowhere for them to go: with one the operator requeues forever and never deletes the pod, and with two it gives up on a full drain by itself, warning that some shards may not drain and then waiting for the system index primaries alone, which the surviving node cannot take while it holds their replicas. Such a release could not complete any rolling restart the operator drives: a version bump, a configuration change, or the move of the HTTP listener onto a cert-manager certificate. Drain only where a spare data node exists. Without draining, the operator pauses replica allocation for the restart and re-enables it once every pod runs the new revision, which is the path a small cluster can take. The predicate also reads the data role, since the operator counts data nodes over pools that carry it and a release with the role off has none. Every boundary is pinned in the unit tests. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
884f719b75 |
fix(opensearch): stop asking the operator for a privileged sysctl container
spec.general.setVMMaxMapCount makes the operator append a privileged init container to every OpenSearch pod, so that vm.max_map_count reaches the 262144 the store needs. Tenant namespaces run at the cluster's default PodSecurity level, and on Talos that is baseline, which refuses the container before a pod exists: the StatefulSet never produces one and the release never comes up. The opensearch-operator package now raises the sysctl on every node from a DaemonSet in its own namespace, which the platform labels privileged, so the pod no longer has to ask. The flag is set to an explicit false rather than dropped, and a unit test pins it, so the next reader sees a decision and not an omission. Carrying the value in the node contract would let the DaemonSet go; until then the DaemonSet is the carrier. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |