mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 22:13:18 +09:00
3fb8ec5b0ea4d505cfdfd72f0917cb6bb138ee65
4932
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 IvanHunters
|
3fb8ec5b0e |
feat(monitoring): per-instance OIDC selector for the Grafana instance (Phase 1) (#3176)
<!-- Follow-up to cozystack/cozystack#3044 — same design family, different subject. --> ## What this PR does Adds a Phase-1 OIDC selector to the Grafana instance shipped by `packages/system/monitoring/`, mirroring the shape of the tenant kube-apiserver PR ([cozystack/cozystack#3044](https://github.com/cozystack/cozystack/pull/3044)) so that operators can use one mental model for both. Rationale in [cozystack/community#24](https://github.com/cozystack/community/pull/24) — per-cluster (here: per-Monitoring-release) audience isolation on the flat `cozy` realm, no per-tenant realm. Authorization is **app-side** via a chart-owned `users:` map reconciled by a post-install/post-upgrade Job into Grafana org membership. The chart does NOT own Keycloak directory objects (no `KeycloakRealmGroup`s) and does NOT rely on Grafana's `role_attribute_path` — same design shape as #3044's ClusterRoleBinding reconcile, adapted for Grafana orgs. See `docs/oidc-grafana.md` for the full rationale. The `Monitoring` CR grows a new field: ```yaml spec: oidc: mode: None | System | CustomConfig # default None customConfig: # only for CustomConfig config: {} # inline grafana.ini keys, or... secretRef: name: "" # ...an operator-managed Secret users: # optional; ignored in mode=None - email: alice@example.com role: Admin | Editor | Viewer ``` ### Modes - **`None`** (default) — no OIDC; `admin_user` / `admin_password` Secret is the only auth path. Existing releases render byte-identical. - **`System`** — chart provisions, in the release namespace of the Monitoring CR: - `KeycloakClient` (`public: false`, confidential, `secret` sourced from a chart-owned Secret via the EDP `$<secret>:<key>` reference; `redirectUris` locked to `https://grafana.<host>/login/generic_oauth`). - `KeycloakClientScope` with an `oidc-audience-mapper` pinning `id_token.aud` to the per-release clientId — the isolation primitive. - Persistent Kubernetes Secret carrying the confidential `client-secret` (random on first install, preserved via `lookup`+fallback, same pattern as `packages/system/dashboard`). - Grafana CR's `spec.config.auth.generic_oauth` section wired to the `cozy` realm issuer + per-release audience scope, with two chart-forced settings the users-Job depends on: `skip_org_role_sync: "true"` (login never overwrites the Job's org-role assignments) and `oauth_allow_insecure_email_lookup: "true"` (OIDC identity binds to the pre-provisioned local account by email). - `GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET` env on the Grafana Deployment sourced from the Secret. - Post-install / post-upgrade **users-Job** reconciling `spec.oidc.users` into Grafana's Main Org.: pre-provisions a local account per listed email, `POST /api/orgs/1/users` + `PATCH /api/orgs/1/users/{userId}` to converge role changes, prunes every non-admin Main-Org member whose email is not in the list. - Server-level `GrafanaAdmin` promotion (`allow_assign_grafana_admin`) is out of scope for Phase 1. Every Grafana instance (platform and tenant) caps at org-level `Admin`. - **`CustomConfig`** — tenant supplies the whole `[auth.generic_oauth]` payload; `cozy` is not in the path. Two mutually exclusive paths: - `customConfig.config: {...}` — inline map of `grafana.ini` keys. The chart **merges** the two chart-forced settings (`skip_org_role_sync`, `oauth_allow_insecure_email_lookup`) on top so the users-Job contract holds. The operator can NOT override those two (merge wins). - `customConfig.secretRef.name: ...` — operator-owned Secret with an `auth.ini` fragment; the chart mounts it under `/etc/grafana/oidc` and wires `GF_PATHS_CUSTOM_INI`, and does NOT emit `auth.generic_oauth` under `spec.config` (the ini fragment is authoritative). Because the chart cannot inject the two settings into a mounted ini, `spec.oidc.users` is NOT supported in this branch — the chart hard-fails render on that combination. ### Fail-close guards - `mode: System` without `_cluster.oidc-enabled == "true"` (the platform-level `authentication.oidc.enabled` flag) → hard-fail with an actionable message pointing at the platform values. - `mode: System` on a cluster where the EDP Keycloak operator CRDs (`v1.edp.epam.com/v1`) are not registered → hard-fail (symmetric on `oidc-keycloak.yaml` and `grafana.yaml`). - `mode: CustomConfig` with both or neither of `config` / `secretRef.name` → hard-fail. - `mode: CustomConfig` with `secretRef.name` AND non-empty `users` → hard-fail (chart cannot inject the two forced settings into an operator-mounted ini; forcing an explicit choice avoids silent contract loss). - `spec.oidc.users[]` entry with missing / unknown `role` or malformed `email` → hard-fail at render time so the users-Job never fires with a body that would 400 on Grafana's admin API. ### Break-glass posture `admin_user` / `admin_password` and `disable_login_form: false` are unchanged in every mode. Locking the form off under `mode: System` is a documented follow-up hardening (see `docs/oidc-grafana.md`). ### What's NOT in this PR (documented as out of scope) - Per-tenant Keycloak realms (Phase 2 territory, tracked in community#24). - Full-logout through Keycloak's end-session endpoint (`backend-logout-url` and matching Keycloak client attribute). - CEL `claimValidationRules` gate on `email_verified` — layered guarantees are described in `docs/oidc-grafana.md`. - Multi-issuer composition on one Monitoring release (mutually exclusive modes). - Server-level `GrafanaAdmin` promotion. ### Docs - New `docs/oidc-grafana.md` — full operator guide (parallel to `docs/oidc-tenant.md`). - Website page follows in a paired PR against cozystack/website (`docs/monitoring-oidc-authentication` branch, under `content/en/docs/next/operations/services/monitoring/oidc-authentication.md`). ### Tests - 33 `helm-unittest` cases in `tests/oidc_test.yaml` + `tests/oidc_crd_missing_test.yaml` cover: all three modes; defensive nil paths (`oidc: ~`); System-mode Keycloak client + audience scope shape; CustomConfig chart-forced merge (positive assertion) AND `mode=CustomConfig inline — operator cannot override chart-forced OIDC settings` (regression guard); CustomConfig secretRef + `users` incompat fail-fast; `spec.oidc.users[].role` missing/unknown fail-fast; users-Job shape (hook annotations, RBAC-free, activeDeadlineSeconds, `ttlSecondsAfterFinished: 3600`, admin credential wiring); no chart-owned `KeycloakRealmGroup`s in any mode; no `role_attribute_path` in Grafana config. - Render-side bats in `hack/e2e-apps/monitoring-oidc-{system,customconfig}.bats` — mirror the tenant kube-apiserver's e2e shape (no live browser flow — deferred to a follow-up integration suite, same as #3044). Asserts on live cluster: users-Job appears with `DESIRED_USERS_JSON` populated, Grafana config carries `skip_org_role_sync=true` and no `role_attribute_path`, no chart-owned `KeycloakRealmGroup`s land in `cozy`. ### Screenshots N/A — no UI change on default values; the visual delta is Grafana's own "Sign in with Keycloak" button appearing under the login form when a release opts in. ### Release note ```release-note feat(monitoring): Each `Monitoring` CR can now opt its Grafana instance into OIDC via a flat selector — `spec.oidc.mode: System | CustomConfig | None` (default `None`). `System` trusts the platform `cozy` realm via a per-instance confidential Keycloak client + audience binding. Authorization is app-side: `spec.oidc.users: [{email, role: Admin|Editor|Viewer}]` is reconciled into Grafana's Main Org. by a chart-owned post-install/post-upgrade Job (pre-provision → org add → PATCH role → prune stale). `CustomConfig` accepts a tenant-supplied `[auth.generic_oauth]` payload — inline as a map (chart merges the two contract-critical settings `skip_org_role_sync` + `oauth_allow_insecure_email_lookup` on top) or as a Secret with an `auth.ini` key (in which case `spec.oidc.users` is not supported and the chart fails render on that combination). Server-level `GrafanaAdmin` promotion is out of scope for Phase 1. The `admin_user`/`admin_password` Secret stays a documented break-glass path in every mode. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added OIDC authentication for Monitoring Grafana with `None`, platform-managed `System`, and tenant-controlled `CustomConfig` modes. * System mode provisions platform-backed OIDC for each instance (including persistent client credentials) and can optionally promote matching users to server-level `GrafanaAdmin`. * CustomConfig supports inline OAuth settings or Secret-backed `auth.ini` (with mutual exclusivity validation). |
||
|
myasnikovdaniil
|
215bd430a9 |
test(e2e): diff-driven install selector select-install.sh (#3279)
## What this PR does Adds `hack/select-install.sh` — the install-side companion to `hack/select-e2e.sh`. Where `select-e2e.sh` picks *which* Chainsaw suites run for a diff (a reverse dependency walk), `select-install.sh` picks *what must be installed* for those suites: the forward `dependsOn` closure over the `PackageSource` graph. Given the suite names `select-e2e.sh` emits, it maps each back to its `*-application` `PackageSource` and walks `spec.variants[].dependsOn` forward to the transitive closure — the minimal set of packages to enable instead of provisioning the full `isp-full` platform. A `postgres` chart change, for example, resolves to 9 of 97 sources. Design notes: - Reuses the real `PackageSource` graph in `packages/core/platform/sources/` — no parallel dependency-declaration files to keep in sync. - Unlike `select-e2e.sh`'s reverse walk, the forward walk **keeps** the `cozystack.cozystack-engine` edge: for test *selection* it is noise (it would fan every app out to every other), but for *install* it is a genuine prerequisite (the app's `*-rd` HelmRelease needs the engine to register the ApplicationDefinition CRD). Because that edge lives only on `*-application` sources, a suite backed by a **system** package (e.g. `securitygroup`, whose `SecurityGroup` in `sdn.cozystack.io` is served by the aggregated `cozystack-api` — a `cozystack-engine` component) can't reach the engine via the walk, so the engine is **seeded explicitly for any non-empty closure** (and a graph missing it is a hard error). - `--validate` mode checks the whole graph — every `dependsOn` target resolves to a real `PackageSource` (reachability) and there are no dependency cycles — **and** the suite mapping: every `hack/e2e-chainsaw/*/` suite resolves to a real `PackageSource` or to `-` (a missing suites dir is itself a failure). The graph lives next to the packages and can't drift from the suite dirs; the hand-maintained mapping is the only part that can, so this is the cheap CI guard. - Suite→source mapping follows the `<suite>-application` convention with a bare-`<suite>` fallback, plus explicit cases for the irregular names (`kubernetes`/`vminstance`/`securitygroup`) and for core-platform features that have no separately-enabled package (`serviceexposure`, whose `ExposureClass`/`ServiceExposure` CRDs live in `packages/core/platform`). A suite may reference other app kinds by selector without standing them up (e.g. `securitygroup`'s `SecurityGroup` names `Postgres`/`Kubernetes` as selectors only) — only the primary package is mapped here; the forward closure and the seeded engine cover its prerequisites, and full multi-package modelling is left to the `test-minimal` follow-up. A suite with no known mapping is a **hard error** (fail closed), not a silent skip. Includes a 17-case unit suite (`hack/select-install_test.bats`) covering single-app closure, transitivity, multi-suite union, stdin input, the kubernetes/irregular/core-platform suite mappings, the `securitygroup` engine-seed regression, empty input, fail-closed unmapped suites (all reported, no partial output), the missing-engine hard error, and the `--validate` failure modes (dangling dep, cycle, unmapped suite dir, missing suites dir). It is auto-discovered by `make unit-tests` (`BATS_UNIT_FILES` globs `hack/*.bats`). Verified end-to-end against real git diffs across six path types (app chart, shared operator, docs-only, per-suite Chainsaw edit, library, shared e2e helper): a `postgres` chart change scopes the install to 9 of 97 sources; the full-suite fallback to 39 of 97. This is Track A of #2847 (diff-driven & scenario-based E2E); tracked in #3278. Consuming the output — a `test-minimal` platform variant/bundle plus wiring into the `pull-requests.yaml` install step — is a deliberate follow-up, since it depends on a platform-variant design decision. ### Screenshots Not applicable — CI/tooling-only change. ### Release note ```release-note test(e2e): add hack/select-install.sh to compute the minimal package install set for a diff-scoped E2E run (forward PackageSource dependency closure), with a --validate graph-consistency mode ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a tool to select the minimal package set required for one or more end-to-end test suites. * Supports combining multiple suites, resolving transitive dependencies, and handling unmapped or empty suite selections. * Added validation for missing dependencies and circular dependency definitions. * **Tests** * Added coverage for package selection, dependency resolution, suite mappings, empty selections, and validation failures. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Ivan Okhotnikov
|
5d13c15d65 |
fix(apps/kubernetes): compile OIDC groups claim rule via dyn() so the tenant apiserver boots
The System-mode AuthenticationConfiguration rendered a claimValidationRule ranging over claims.groups, which is statically typed `any` in the apiserver CEL environment. The .exists() comprehension rejects a range of type any, so the tenant kube-apiserver failed config compilation at startup and CrashLooped — the control plane never booted and cluster teardown then hung the e2e cleanup budget. Wrap the range in dyn() so CEL treats it as dynamic; the has() guard still short-circuits when groups is absent and the gate semantics are unchanged. Strengthen the oidc unit test to pin dyn() (mutation-verified). Verified on a live tenant apiserver (v1.32). Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com> |
||
|
Ivan Okhotnikov
|
0164a6afa4 |
fix(monitoring): set groups_attribute_path=groups so allowed_groups actually works on Grafana v11.x
Verified end-to-end on dev3 with Grafana v11.6.15: OAuth flow reaches Grafana, id_token and userinfo response both carry the correct `groups` claim (`[cozystack-cluster-admin, tenant-root-super-admin]` for kvaps), `allowed_groups` in grafana.ini lists `tenant-root-super-admin`, and login still fails with `user not a member of one of the required groups`. Grafana debug log shows `Attributes: map[]` — the extracted user's Groups slice is empty. On v11.x+ Grafana does not auto-map the `groups` claim onto user.Groups unless `groups_attribute_path` is set explicitly (the field is documented as optional but the gate is a silent no-op without it — a claim-name-string is not enough). Fix: hard-wire `groups_attribute_path=groups` in the chart-forced System-mode block alongside `allowed_groups`. "groups" points at the top-level `groups` array from userinfo, matching the shape Keycloak's oidc-group-membership-mapper emits (verified on the same dev3 run). Two helm-unittest cases updated to assert `groups_attribute_path` lives next to `allowed_groups` in both the users-map-active and empty-users branches (rejecting the silent no-op regression). Docs also updated to name the requirement. Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
874ca9427f |
fix(apps/kubernetes): close cross-tenant kube-apiserver OIDC hole via CEL claim gate
Symmetric with the Grafana-side allowed_groups gate — round-3 review from lllamnyp on PR#3176 covered both surfaces, and #3044's original System-mode wiring shipped without this guard. The rationale is identical: audience binding stops REPLAY of a minted token across tenant kube-apiservers, but nothing stopped Keycloak from ISSUING a usable token to a caller outside the tenant. Absent a claim-side gate, any authenticated cozy-realm user could `kubectl oidc-login` against another tenant's cluster and land at `system:authenticated`. RBAC default-denies named-resource access, but `system:authenticated` still leaks discovery (kubectl api-resources, OpenAPI schemata) — tenant-alice could enumerate tenant-bob's cluster's CRD shape and built-in resource surface. Fix: add a CEL `claimValidationRules` entry to the rendered AuthenticationConfiguration (System mode only) that requires at least one of the release's four tenant-scoped Keycloak groups (`<namespace>-{view,use,admin,super-admin}`) to be present in the token's `groups` claim. The `has(claims.groups) &&` guard is required — CEL evaluation on a missing claim surfaces as HTTP 500 from the authenticator instead of a clean 401. CustomConfig mode is not touched — the tenant's own AuthenticationConfiguration is authoritative and the tenant is responsible for claim-side guards on their own issuer. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
b002c967ac |
fix(monitoring): close cross-tenant Grafana OIDC hole via tenant-group gate
Round-3 review from lllamnyp: any user in the shared `cozy` Keycloak
realm could sign into any tenant's Grafana. The audience mapper stops
replay of a minted token across releases, but does not gate who may
mint one — tenant-alice could complete the OAuth flow against
tenant-bob's Grafana. With `spec.oidc.users` non-empty the earlier
`allow_sign_up=false` fix mostly closed the door (though tenant-alice
still hit `error 401` after landing in Grafana, revealing the release
exists); with empty users under mode: System — a config the schema
permits and the docs describe as hands-off — the login succeeded and
minted a Viewer account for every cozy-realm identity.
The fix uses the KeycloakRealmGroups the tenant chart already
provisions (`<namespace>-{view,use,admin,super-admin}` in
packages/apps/tenant/templates/keycloakgroups.yaml) — no new
directory objects owned by the Monitoring release. Two chart edits:
1. oidc-keycloak.yaml: add the platform-managed realm-default `groups`
scope to `defaultClientScopes` explicitly. EDP's KeycloakClient
does NOT merge realm-level defaults into `defaultClientScopes`, so
the scope must be listed here or the `groups` claim would be
missing from the token.
2. grafana.yaml: in System mode, unconditionally emit
`allowed_groups: <ns>-view <ns>-use <ns>-admin <ns>-super-admin`
in `auth.generic_oauth` and add `groups` to `scopes`. Grafana
rejects any login whose token does not carry at least one of the
groups.
The gate is chart-owned and does not depend on the users-map — with
`users: []` tenant members still get a self-service Viewer through
`auto_assign_org_role`, cross-tenant users get 401 in Keycloak's OAuth
callback. Docs updated to reflect the two-layered contract
(tenant-membership gate + optional users-map).
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
f64ba91872 |
fix(monitoring): do not render users-Job when spec.oidc.users is empty
Address PR#3176 review round-2 gap-2: the users-Job's prune pass deletes every non-admin Main-Org member whose email is not in the desired list. When the desired list is empty every helm-upgrade wiped the whole org. Three empty-users states used to fall into this trap: * customConfig.secretRef: users[] is forbidden by assertCustomSecretRefUsersEmpty, so the desired list was structurally always empty and the chart wiped an org whose auth.ini fragment the operator advertised as authoritative. * System with users[] unset: the operator opted into OIDC but not into the chart-managed users-map, yet the Job still pruned every Main-Org member on their behalf. * CustomConfig-inline with users[] unset: same shape as System. The mode=None guard (from an earlier round) already had the same shape; extend the render gate to cover the other empty-list doors — now the Job renders only when `mode != None AND users non-empty`. The trade-off: taking `users:` back to `[]` no longer prunes the last entry — matching the already-documented `System → None` posture — called out explicitly in the docs. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
ff4ef2c189 |
fix(monitoring): force allow_sign_up=false and gate isolation dict on users-map
Address PR#3176 review round-2 gap-1: the docs promise "users not in spec.oidc.users get nothing", but Grafana's default allow_sign_up=true combined with the chart-forced skip_org_role_sync=true was minting a Viewer account for every cozy-realm identity that completed the OAuth flow, then reaping it on the next helm-upgrade prune pass. The chart-forced isolation dict (now three keys: skip_org_role_sync, oauth_allow_insecure_email_lookup, allow_sign_up=false) is applied only when spec.oidc.users is non-empty. A BYO-IdP operator who does not use the users-map gets none of the three forced on top of their inline config, matching the review's minor: forcing insecure_email_lookup against an issuer that may not verify emails is a needless account- linking surface, and allow_sign_up=false with no pre-provisioned accounts would lock everyone out. Tests split the two branches: users-active asserts all three keys are present and the operator cannot override them via CustomConfig-inline; users-empty asserts the auth.generic_oauth block is emitted without any of the isolation keys. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
3776c447b9 |
test(e2e): poll users-Job DESIRED_USERS_JSON for target email, not just Job existence
CI E2E showed the users-Job assertion returning `DESIRED_USERS_JSON:
[]` — but the CR body had `users: [{email: e2e-admin@example.com,
role: Admin}]`. Root cause: `hook-delete-policy: before-hook-creation`
keeps the previous hook Job around across helm upgrades, so `until
kubectl get job` matches a stale Job from the previous customconfig-
mode test where users may have been unset (the secretRef swap CR had
no `users:` field, so the schema default `[]` propagated to the
prior Job's DESIRED_USERS_JSON). The System test then grabbed that
stale Job before the helm upgrade for the new CR completed and swapped
in the fresh Job.
Fix: poll on the target env value itself — the new Job with the
correct users list is the first Job whose DESIRED_USERS_JSON string
contains `e2e-admin@example.com`. 600s window matches the inner HR's
15m timeout with room for a slow reconcile.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
b2fd32385e |
fix(monitoring): rewrite users-Job shell in POSIX (alpine BusyBox compat)
CI E2E caught the Job container crash-looping within 20s of the first Start event. Root cause: the `alpine/k8s:1.33.4` image is Alpine-based (BusyBox coreutils + BusyBox grep), and the Job shell used four GNU-specific constructs that BusyBox does not implement: - `head --bytes=N` (BusyBox `head` only accepts `-c N`). - `tr --delete` (BusyBox `tr` only accepts `-d`). - `jq --raw-output0` (NUL-terminated output — jq 1.7+ feature; also paired with `read -r -d ''` which is a bash-ism, not POSIX `sh` and not honoured by BusyBox `ash`). - `grep --null-data --fixed-strings --line-regexp` (BusyBox `grep` supports `-F` and `-x` short forms but NOT `-z`/`--null-data`). Rewrite: - `head -c 24 /dev/urandom | base64 | tr -d '=+/' | head -c 32` — same random-32-char password, portable to BusyBox. - Emit desired emails and org membership one record per line (jq `--raw-output`) instead of NUL-terminated. Safe because the template-level `monitoring.oidc.assertUsersEmailShape` guard already rejects any email containing whitespace / control characters, so line-oriented iteration cannot get confused. - Read TSV membership records with `IFS="$(printf '\t')"` (POSIX- portable TAB) + plain `read -r uid ulogin uemail` — no need for `-d ''` once records are newline-terminated. - Match desired list with `grep -Fxq -e "$uemail" -e "$ulogin"` — short-form flags supported by both GNU and BusyBox `grep`. Syntax validated with both `sh -n` and `bash -n` on the extracted inline script. helm-unittest 34/34 still green (render shape is unchanged, only the shell literal is different). Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
1422495b88 |
fix(monitoring): do not render users-Job when mode=None (upgrade safety)
Caught by dev3 end-to-end test on the PR working tree
(cozystack-pr-test skill): the users-Job was rendered in every mode
including `None` on the rationale that a `mode: System → None`
switch should garbage-collect OIDC-provisioned local accounts. That
rationale ignored the upgrade path from the pre-PR chart: an
existing tenant that never had `spec.oidc` on their Monitoring CR
would receive `mode: None` on first render under the PR, the Job
would fire on every Flux reconcile of the outer HR, and its prune
pass would DELETE every non-admin Main-Org member — including users
the tenant added by hand through the Grafana UI on the pre-PR chart.
Silent breaking change with data loss.
Wrap the whole Job resource in `{{- if ne $mode "None" }} … {{- end }}`
so `mode: None` = "chart owns nothing in Grafana orgs, we don't
touch what's there". Operators who need to prune OIDC-provisioned
accounts after switching System | CustomConfig → None do it
themselves through the Grafana UI or admin API — same posture as
"we don't own the Keycloak directory objects" from the earlier
architectural change.
The `assertUsersPrologue` include stays outside the `if` so an
operator listing users in `mode: None` still gets the schema and
email-shape validation at render time — the Job body just doesn't
emit.
Test coverage:
- Flip the previous "mode=None still renders the users-Job so a
mode switch can prune stale members" case to
`mode=None does NOT render the users-Job (upgrade safety guard)`
as a regression lock.
- Add `mode=None with non-empty users still fails render` covering
the prologue-only-fires-on-schema-issue path.
Docs: rewrite the "Users and RBAC" section step 3 to name the
upgrade-safety motivation and the manual-cleanup handoff.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
7f9e1a8bf6 |
test(e2e): poll the reconciled Grafana CR field instead of racing on CR existence
CI run 28794778739 caught two race conditions in the render-side Monitoring OIDC bats: - `Grafana CR carries auth.generic_oauth pointing at the cozy realm` (monitoring-oidc-system.bats:86) failed with `enabled=` empty after a 0-second wait. The prior `until kubectl get grafana grafana` only proved the CR EXISTS — it did not prove grafana-operator had reconciled the mode=System values onto it. When the CR from the previous `Monitoring CR accepts spec.oidc.mode=System` test was already there but the operator hadn't yet flipped `.spec.config.auth .generic_oauth.enabled` to "true", the jsonpath returned empty and `[ "" = "true" ]` short-circuited the whole test. - `secretRef variant mounts operator Secret under /etc/grafana/oidc` (monitoring-oidc-customconfig.bats:119) timed out at 180s waiting for the volumeMount swap from inline → ini. On a warm sandbox the full inner-HR upgrade + grafana-operator reconcile cycle can legitimately take >3m — the timeout was too tight. Fix: replace `until kubectl get grafana` with `until [ "$(kubectl ... jsonpath=<target-field>)" = <expected> ]` on every subsequent assertion, and bump the secretRef-swap window to 600s (well inside the inner HR's 15m timeout). Same pattern used for the "no role_attribute_path" and "operator payload verbatim" assertions so the whole suite is race-free. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
02305eae59 |
docs(oidc-grafana): correct post-mortem window claim
The failed-hook post-mortem paragraph in the users-Job troubleshooting section still said the Job "stays around until the next helm-upgrade for post-mortem", but the shipped Job spec carries `ttlSecondsAfterFinished: 3600` — the Kubernetes TTL controller reaps the Job + Pod one hour after the terminal Failed condition regardless of when the next `helm upgrade` lands. Reword to name the 1h window explicitly and instruct the operator to re-trigger the hook if they missed the window. The edit was authored alongside commit 1f664566 but slipped out of that commit's file list; this is the cleanup. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
c31a9f8ec3 |
fix(monitoring): promote oidc.users[].role to required + regenerate schema
The `OIDCUser` typedef in `packages/extra/monitoring/values.yaml`
declared only `@field {string} email`; `cozyvalues-gen` regenerated
`values.schema.json`, `README.md`, and the mirrored `openAPISchema`
on the `Monitoring` ApplicationDefinition WITHOUT a `role` field.
Consequences:
- The `@enum OIDCRole` declaration was orphan — referenced from
nowhere, so the schema's items object had `required: ["email"]`
and one property.
- A dashboard-driven CR would offer a form without any role picker;
a schema-conformant `{email}` submission would render into the
users-Job with `role: null`, `jq --raw-output` yields the string
`"null"`, the `POST /api/orgs/1/users` body carries
`"role":"null"`, Grafana returns HTTP 400, and the post-install
hook exhausts `backoffLimit: 6` — HelmRelease never becomes Ready.
- Unit + e2e tests all hardcoded `role: Admin` / `role: Viewer`, so
the contract mismatch never surfaced in CI.
Fix:
- Add `@field {OIDCRole} role - ...` to the `OIDCUser` typedef in
`packages/extra/monitoring/values.yaml` so cozyvalues-gen materialises
the field with enum + required.
- Regenerate `values.schema.json`, `README.md`, and mirrored
`openAPISchema` via `make generate`.
- Add `monitoring.oidc.assertUsersRoleShape` template-level guard
(same shape as `assertUsersEmailShape`) so a stale-schema CR or a
direct-helm invocation fails at render time with a legible error
instead of at hook execution with a Grafana 400.
- Consolidate all `spec.oidc.users` guards into a single
`monitoring.oidc.assertUsersPrologue` include and call it once from
`oidc-users-job.yaml`, ordering the incompat check
(`assertCustomSecretRefUsersEmpty`) before the per-entry role /
email regexes so operators see the more actionable message first.
Drop the duplicate include from `grafana.yaml` (Job template renders
in every mode anyway).
- Two new helm-unittest cases lock the regression:
`oidc.users entry missing role fails the render` and
`oidc.users entry with unknown role fails the render`.
- Point the previous `secretRef + users` incompat unit test at the
users-Job template since that's where the prologue now fires.
- `docs/oidc-grafana.md`: correct the failed-hook post-mortem window
claim — with `ttlSecondsAfterFinished: 3600` the Kubernetes TTL
controller reaps the Job + Pod 1h after the terminal condition,
not "until the next helm-upgrade".
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
86e54f1296 |
docs(monitoring): align OIDC docs with the actual mode-agnostic contract
Previous docs claimed "the chart forces skip_org_role_sync = true regardless of mode" — a promise the code did not keep in CustomConfig mode. The chart is now fixed; sync the docs so the guarantee reads truthfully and the incompatibility surface is explicit: - `docs/oidc-grafana.md`: rewrite the CustomConfig section so both chart-forced settings are named explicitly, the `merge` semantics are documented (operator can't override), and the `secretRef` + `users` incompatibility is called out with an explicit failure path. - `system/monitoring/values.yaml`: expand the `config`/`secretRef` comments so operators reading the values doc see the merge behavior and the secretRef limitation before they hit the render-time fail. Add `enabled: "true"` to the "typical keys" list — Grafana defaults `enabled: false` and an inline map without it is inert. - `extra/monitoring`: regenerated values.schema.json + README.md via `make generate` (cozyvalues-gen + hack/update-crd.sh) to pick up the `oidc.users[].email` doc-string touch; mirrored openAPISchema on the ApplicationDefinition follows. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
ad67dee95b |
test(monitoring): lock the CustomConfig chart-force contract in helm-unittest
Two blockers on the previous review cycle were "code says X but no test enforces X" — so future edits to `grafana.yaml` could silently break the contract again. Add three assertions that fail loudly if the merge behavior regresses: - `mode=CustomConfig inline preserves the operator's map + chart-forced settings` — asserts `skip_org_role_sync=true` and `oauth_allow_insecure_email_lookup=true` are both present alongside the operator's `client_id` / `client_secret` / `auth_url`. - `mode=CustomConfig inline — operator cannot override chart-forced OIDC settings` — sets both settings to "false" in the operator's map; asserts they render as "true" (proves `merge` argument order is correct). - `mode=CustomConfig secretRef + non-empty users fails the render (contract incompat)` — asserts the new `monitoring.oidc.assertCustomSecretRefUsersEmpty` helper's error message fires so operators cannot silently strand their users-Job assignments by combining incompatible fields. - `mode=CustomConfig secretRef + empty users renders successfully` — the compatible path. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
26e5f06d27 |
fix(monitoring): chart-force skip_org_role_sync + email lookup in every OIDC mode
The chart previously injected the two settings the users-Job's
contract depends on — `skip_org_role_sync = true` and
`oauth_allow_insecure_email_lookup = true` — only inside the
`mode: System` branch of `grafana.yaml`. `values.yaml` and
`docs/oidc-grafana.md` both promised the settings would be forced
regardless of mode, but under `mode: CustomConfig` the operator's
inline `auth.generic_oauth` map was written verbatim, both
settings were absent, and the users-Job's role assignments were
silently stranded (no lookup by email → new Grafana account minted
at login) or transiently overwritten (no skip_org_role_sync → any
`role_attribute_path` or Grafana's `auto_assign_org_role: Viewer`
overwrote the Job's role on each login).
Fix: merge a `$chartForcedOidc` dict on top of both the generated
System block and the operator's inline map (`merge` wins for
overlapping keys, so the operator can NOT override the two
settings). CustomConfig secretRef mode cannot receive the merge
because the ini fragment is authoritative — the chart now hard-fails
render when both `customConfig.secretRef.name` and non-empty
`spec.oidc.users` are set, forcing the operator to either switch to
inline config or manage authorization themselves inside the ini.
Also:
- Align the inner HR `timeout` on `packages/extra/monitoring/templates/helmrelease.yaml`
from 10m to 15m so it matches the outer cozyrds
`release.cozystack.io/helm-install-timeout: "15m"` and does not
fail while the users-Job is still inside its
`activeDeadlineSeconds: 900`.
- Route the users-Job image through `cozy-lib.image` so
mirrored / air-gapped installations resolve it (matches the
pattern in `packages/apps/mariadb/templates/hooks/cleanup-pvc.yaml`).
- Add `spec.ttlSecondsAfterFinished: 3600` on the users-Job so a
succeeded Job + Pod are garbage-collected on the next reconcile
interval rather than lingering until the next hook activation
(which never fires on release uninstall).
- Harden the users-Job shell:
* emails read NUL-terminated (`jq --raw-output0`) so an address
containing whitespace cannot corrupt the `for email in ...`
loop's field splitting;
* `/api/users/lookup?loginOrEmail=<email>` URL-encoded via
`jq @uri` so `&`/`=`/`?` in the local part parse correctly on
the Grafana side;
* prune loop switched from pipe-delimited fields to TSV inside
NUL-terminated records, with `cut -f` for extraction.
- Add a template-level assertion (`monitoring.oidc.assertUsersEmailShape`)
rejecting emails that do not match a conservative regex —
cheaper failure mode than a 400 from Grafana.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
affe9beca5 |
docs(oidc-grafana): reflect users-map replacement of the role-attribute-path design
Rewrite `docs/oidc-grafana.md` for the new authorization model: - Selector example carries a `users:` map. - System-mode "what the chart provisions" list drops the three KeycloakRealmGroups + role_attribute_path, adds the audience scope-only KeycloakClient defaults, and adds the users-Job step with `skip_org_role_sync` + `oauth_allow_insecure_email_lookup` wiring notes. - Fresh "Users and RBAC" section describes the users-Job reconcile contract: pre-provision -> add-to-org -> PATCH role -> prune. Explains the "user not in list -> gets nothing" semantics (replaces the default-Viewer fallback). - CustomConfig section explicitly notes that `role_attribute_path` in the operator's payload is now dead config: the chart forces `skip_org_role_sync = true` regardless of mode. - "Failure modes" gains "user logs in but sees no dashboards" (missing from `users:`) and a users-Job troubleshooting entry. - Drop the `GrafanaAdmin` deferral notes and the "Mode toggle destroys chart-owned KeycloakRealmGroup membership" failure mode — both moot now that the groups are gone. - Removed the stale `KeycloakRealmUser` example that used `tenant-acme-monitoring-admin` (the chart never rendered a group by that name — the inner release is `monitoring-system` and the example was a live footgun). Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
123616a1d0 |
test(e2e): swap KeycloakRealmGroup assertions for the users-Job in bats
Aligns the render-side e2e bats with the users-map contract: - Feed a `spec.oidc.users:` entry to each mode's Monitoring CR so the reconcile Job has real input to act on. - Assert the users-Job appears in the release namespace with the desired `DESIRED_USERS_JSON` env and the expected `OIDC_MODE`. - Assert the Grafana CR renders with `skip_org_role_sync = "true"` and `oauth_allow_insecure_email_lookup = "true"` and NO `role_attribute_path`. - Flip the three KeycloakRealmGroup existence assertions to negative checks: chart-owned realm groups must NOT be present in either System or CustomConfig mode. Does not add a full OIDC login flow through Keycloak — that stays deferred to a follow-up integration suite, same posture as the tenant kube-apiserver's kubernetes-oidc-system.bats. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
c493f2cb96 |
test(monitoring): rewrite OIDC unit tests around the users-map contract
Rewrites the helm-unittest coverage for the Phase-1 OIDC selector
against the new chart contract:
- Drop assertions for the three KeycloakRealmGroups and for
`role_attribute_path` / `allow_assign_grafana_admin` / the
`oidc.grafanaAdmin` values lever — none of those render anymore.
- Assert the System-mode Grafana config carries
`skip_org_role_sync = "true"` and
`oauth_allow_insecure_email_lookup = "true"` and NO
`role_attribute_path`.
- Cover the new users-Job template: helm-hook annotations, absence
of a ServiceAccount, `activeDeadlineSeconds`, admin-credentials
wiring from the chart-managed `grafana-admin-password` Secret,
and correct serialisation of `spec.oidc.users` into the
`DESIRED_USERS_JSON` env.
- Assert the users-Job also renders under `mode: None` so a
System -> None flip still gets a prune pass.
- Cover the defensive `.Values.oidc | default dict` paths — omitted
block and explicit `oidc: ~` — across the three OIDC templates.
- Tighten the client-secret regex to the actual base64-encoded
length (`^[A-Za-z0-9+/]{43}=$`) so shortened random secrets are
caught.
32 tests, all green under helm-unittest v1.0.3.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
d4f59af624 |
chore(monitoring): sync PackageSource comment with removed Keycloak groups
Update the oidc-variant comment on `packages/core/platform/sources/monitoring-application.yaml` so it no longer claims `spec.oidc.mode: System` renders three KeycloakRealmGroups. The dependsOn edge and the fail-close guard in `templates/grafana/oidc-keycloak.yaml` stay unchanged: the KeycloakClient + KeycloakClientScope path still needs the `v1.edp.epam.com` CRDs from `cozystack.keycloak-operator` to be registered before the monitoring chart reconciles. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
0a65dd04a1 |
feat(monitoring): expose spec.oidc.users on the Monitoring CR
Extends the tenant-facing `Monitoring` CR with the new
`spec.oidc.users: [{email, role: Admin|Editor|Viewer}]` field
consumed by the users-Job introduced in the previous commit. Works
for both `System` and `CustomConfig` modes; ignored when
`mode: None`.
Regenerated `packages/extra/monitoring/values.schema.json`,
`README.md`, and the mirrored `openAPISchema` on the
`monitoring-rd` ApplicationDefinition through the standard
`make generate` (cozyvalues-gen + hack/update-crd.sh) pipeline.
Also refreshed the stale annotation comment on the CR that still
claimed the chart provisions three KeycloakRealmGroups.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
01d68904ea |
feat(monitoring): drop chart-owned Keycloak groups; app-side users-map + reconcile Job
Replace the group-based authorization model on the Phase-1 Grafana OIDC selector with an app-side users map, mirroring the tenant kube-apiserver sibling in cozystack/cozystack#3044. What is removed - The three chart-owned KeycloakRealmGroup objects in the cozy realm (`<clientId>-{admin,editor,viewer}`). A Helm release must not own directory objects — toggling `spec.oidc.mode: System -> None` or deleting the Monitoring release would Helm-delete the groups, and with them any membership curated out-of-band. Per-instance groups in the platform realm is exactly the design question left open at the end of cozystack/community#24; establishing it as precedent from a monitoring PR is out of scope. - Dash-separated group naming (`<namespace>-<release>-<role>`) was also collision-prone against tenant/subtenant paths, since both the namespace and release names can themselves contain dashes. - `role_attribute_path` in the Grafana `generic_oauth` block and the default-Viewer fallback, which handed org-Viewer to every cozy realm identity that logged in — cross-tenant metrics read across every opted-in Grafana. Also removed the `groups` client scope from the KeycloakClient defaults. - `monitoring.oidc.allowAssignGrafanaAdmin` helper and the internal `oidc.grafanaAdmin` values key: the feature was already deferred and the helper's release-name-only guard was tenant-spoofable. What replaces it - `spec.oidc.users: [{email, role: Admin|Editor|Viewer}]` on the Monitoring chart values. - A chart-owned post-install/post-upgrade Job (`templates/grafana/oidc-users-job.yaml`) that reconciles that list into Grafana's Main Org. via the admin API: pre-provisions a local Grafana account per listed email so the reconcile can set the org role before the operator's first OIDC login, adds the account to the org, PATCHes the role to converge across re-runs, and prunes every non-admin Main-Org member whose email is not in the list (handles both entry removal and `mode: None`). - `skip_org_role_sync = true` + `oauth_allow_insecure_email_lookup = true` in the generic_oauth block. The former stops a login from overwriting the Job's assignments; the latter binds the OIDC identity to the pre-provisioned local account. - Job talks to Grafana over the in-cluster Service and reads admin creds from the chart-managed `grafana-admin-password` Secret via `envFrom`: no ServiceAccount / Role / RoleBinding. `activeDeadlineSeconds: 900` caps a stuck Job so a broken Grafana cannot hold the release in `pending-upgrade`. Defensive rendering - `.Values.oidc | default dict` in the four templates that dereference the `oidc` block so `oidc: ~` or an omitted block no longer nil-derefs. - `oidc-client-secret.yaml` falls back to a fresh random when the existing Secret lookup succeeds but `data.client-secret` is empty. Rationale: cozystack/cozystack#3176 review from @lllamnyp (architectural CHANGES_REQUESTED points 1, 2, 3 + the default-Viewer leak). The three points stack: dropping the groups incidentally fixes the naming-collision and the default-Viewer leak, and the users-Job gives operators the same UX story as the tenant kube-apiserver PR across both sides of the platform. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Ivan Okhotnikov
|
aa63786acd |
docs(monitoring): sync oidc.grafanaAdmin values docstring with deferred-lever posture
Second-iteration cozy-review flagged this as a leftover from the platform-lever removal: `packages/system/monitoring/values.yaml` still described `grafanaAdmin` as "Set true for the platform Grafana release only ... the platform bundle passes this flag through Package.spec.components", which contradicts the deferred-lever reality already documented in the same commit's updates to `_helpers.tpl` and `docs/oidc-grafana.md`. Rewrite the docstring to match: chart-internal lever, defaults `false`, Phase 1 has no end-to-end wiring; kept as a values key so a future phase can add the wiring without touching the chart contract. Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com> |
||
|
Ivan Okhotnikov
|
b46543e5f2 |
fix(monitoring): defer platform GrafanaAdmin lever to a future phase
The MAJOR finding from the cozy-review agent: the platform-side `oidc.grafanaAdmin: true` documented in the PR body and in docs/oidc-grafana.md never actually reached the platform Grafana's HelmRelease. `Package.spec.components["monitoring-system"].values.oidc.grafanaAdmin` was set in packages/core/platform/templates/bundles/system.yaml, but `monitoring-system` (and `monitoring`) have no `install:` block in packages/core/platform/sources/monitoring-application.yaml — only `monitoring-rd` does. PackageReconciler skips components without an `install:` block before ever reaching the assignment that copies `pkg.Spec.Components[].Values` onto `hr.Spec.Values` (internal/operator/package_reconciler.go around lines 197-201 and 292-293). The values-override dictionary was silently dropped, no HR carried `oidc.grafanaAdmin: true`, and the platform Grafana rendered with the chart default (`false`). The `bundles_monitoring_grafana_admin_test.yaml` unit test only asserted against the emitted Package CR spec — exactly the layer where the value still existed but was about to be discarded — so the broken propagation slipped past. Trim the broken promise from Phase 1. The chart-side helper (`monitoring.oidc.allowAssignGrafanaAdmin`) stays as-is so a future phase that routes the override through the tenant-root chart's monitoring HR values (or adds an `install:` block on the `monitoring-system` component) lands on a green chart-side contract. The helm-unittest that verifies chart-side emission stays too, but with a docstring calling out that end-to-end wiring is deferred. Everything else that promised the lever works — bundle line, docs, platform bundle test — is either removed or rewritten to match reality. Also picks up two MINOR findings from the same review: * Drop the misleading `secret-hash` annotation on the OIDC client-secret Secret. The comment claimed it "rotates the KeycloakClient CR and therefore the Grafana Deployment env when the client-secret changes", but nothing downstream watches an annotation on the Secret itself — no rotation is triggered. The `lookup`-based fallback already keeps the value stable across reconciles, so no rotation is even needed. Removing the dead annotation avoids planting a broken assumption for future readers. * Add `release.cozystack.io/helm-install-timeout: "15m"` on the monitoring cozyrds ApplicationDefinition. Under `mode: System` the chart now emits, on top of the existing Grafana + Alerta + VMCluster + CNPG stack, four additional Keycloak CRs whose reconciliation is asynchronous against an operator in `cozy-keycloak` (not the tenant namespace). Flux's default 5m install window was already tight for the pre-OIDC stack; 15m matches the precedent set by tenant-rd/kubernetes-rd cozyrds. Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com> |
||
|
Ivan Okhotnikov
|
5f7ed89799 |
fix(e2e): work around EDP schema-default strip + kubectl delete/apply race
Two follow-up bats fixes uncovered by the first live e2e pass on the
singleton-aligned monitoring OIDC tests:
1. monitoring-oidc-system, test 4 (KeycloakClient assertion). The EDP
Keycloak operator's KeycloakClient CRD strips `spec.public: false`
on write because false is the schema default. The chart emits
`public: false` explicitly (confidentiality guard), but neither
managedFields nor the resulting spec ever carries the field, so
the assertion `[ "${public}" = "false" ]` grepped an empty string
and tripped. Assert `spec.clientAuthenticatorType == "client-secret"`
instead — the operator persists it, and it is the direct signal
for "confidential client" (which is what the `public: false`
assertion was testing for).
2. monitoring-oidc-customconfig, test 4 (secretRef variant). The
original flow was delete-then-recreate the Monitoring CR to swap
inline `customConfig.config` for `customConfig.secretRef.name`.
`kubectl apply` immediately after delete raced the API server's
grace-period cache: apply's GET-then-decide phase saw the CR
during grace, chose the patch path, and the patch hit NotFound
once the delete propagated. Drop the delete entirely — cozystack-
operator reconciles the CR mode change on the same object and
Flux re-renders the chart with the new values.
Add an explicit wait for the target state (Grafana CR's
`oidc-custom-ini` volumeMount present AND `spec.config.auth.generic_oauth`
absent) before the assertions, since the CR update triggers an
asynchronous chart re-render and the immediate get would still
see the inline auth.generic_oauth block from @test 1.
Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com>
|
||
|
Ivan Okhotnikov
|
cb04caecee |
fix(e2e): align monitoring OIDC bats with chart singleton naming
The extra/monitoring chart enforces `.Release.Name == "monitoring"`
in templates/check-release-name.yaml (the tenant Monitoring is a
per-namespace singleton). Naming the Monitoring CR anything else
makes the operator materialise an outer HelmRelease with that name
and the chart hard-fails at render with
execution error at (monitoring/templates/check-release-name.yaml:2:4):
The name of the release MUST BE monitoring
Rename the CR to `monitoring` in both files. Rework every derived
name to match the actual reality of the chart pipeline instead of
the invented `monitoring-<TEST_NAME>` prefix the tests polled for:
* outer HR = ${CR_NAME} = monitoring
* inner HR = ${CR_NAME}-system = monitoring-system
(rendered by extra/monitoring/templates/helmrelease.yaml as
`metadata.name: {{ .Release.Name }}-system`)
* clientId = ${namespace}-${inner Release.Name}
= tenant-test-monitoring-system
* scope = ${clientId}-audience
* groups = ${clientId}-{admin,editor,viewer}
* client-secret Secret = ${inner Release.Name}-oidc-client
= monitoring-system-oidc-client
The BYO Secret name in the customconfig test is chart-independent
(the tenant provides it via customConfig.secretRef.name), so it is
kept short and no longer tied to CR_NAME.
The wait sequence in the first @test of each file is also split
into two probes: outer HR appears first (from the operator), then
inner HR (from the extra/monitoring chart's helmrelease.yaml), then
the Grafana CR. Under the previous incorrect single-probe wait
(`hr monitoring-${TEST_NAME}`) the timeout tripped after 60s
because that HR never existed under any name.
Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com>
|
||
|
Ivan Okhotnikov
|
9ff05b2f32 |
test(e2e): port monitoring OIDC bats to cozytest lifecycle conventions
Mirrors the same port applied to kubernetes-oidc-*.bats in the tenant
kube-apiserver OIDC series: cozytest.sh (the e2e runner) is not real
bats, so setup()/teardown() are never invoked and any state they
established (TEST_NAME) is unset by the time an @test body runs,
tripping cozytest.sh's `set -eu` at the first "${TEST_NAME}"
expansion with `TEST_NAME: parameter not set`.
Lift TEST_NAME to file scope as a stable constant, register cleanup
via cozy_cleanup() so cozytest.sh runs it at suite exit and on the
first failing test, and call cleanup_mon() inline at the top of the
first @test for per-run isolation. Fixes the two E2E failures on
this PR: `Monitoring CR accepts spec.oidc.mode=CustomConfig with
inline config` and `Monitoring CR accepts spec.oidc.mode=System`.
Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com>
|
||
|
Ivan Okhotnikov
|
1679c84bb0 |
chore(monitoring): regenerate extra/monitoring README
Column widths in the OIDC table contract by one character once the tenant-facing `grafanaAdmin` row is gone. Regenerating the table via `make generate` (cozyvalues-gen) so the pre-commit `make generate` diff check in CI stays clean. Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com> |
||
|
Ivan Okhotnikov
|
586e77c361 |
fix(monitoring): hard-fail System OIDC render on missing Keycloak CRD
`mode: System` previously wrapped every Keycloak resource in the `Capabilities.APIVersions.Has "v1.edp.epam.com/v1"` guard, so a cluster where the Keycloak operator CRDs were not yet registered silently dropped the whole Keycloak side while Grafana still came up with an `auth.generic_oauth` block pointing at a client that never got provisioned. Login broke with an opaque OIDC error and no chart-side signal that the platform was misconfigured. Turn the missing CRD into an actionable render error. A new `monitoring.oidc.assertSystemKeycloakCRD` helper fails with a message that points at `cozystack.keycloak-operator`, called from both `templates/grafana/oidc-keycloak.yaml` (replacing the silent capability guard) and `templates/grafana/grafana.yaml` (symmetric defense-in-depth so a future refactor of oidc-keycloak.yaml cannot regress the invariant). For the platform `monitoring-system` release the oidc variant's `dependsOn: cozystack.keycloak-operator` already prevents the race; for tenant releases the assertion converts the misconfiguration into an operator-facing error instead of a broken login flow. Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com> |
||
|
Ivan Okhotnikov
|
e05ca91ff0 |
fix(monitoring): keep OIDC grafanaAdmin as a platform-only lever
The tenant-facing `Monitoring` CR exposed `spec.oidc.grafanaAdmin` via `packages/extra/monitoring/values.schema.json`, which the ApplicationDefinition mirrored into `openAPISchema`. A tenant with create/patch on `Monitoring` in their own namespace could set the flag on their release, promoting members of their `<clientId>-admin` Keycloak group to server-level `GrafanaAdmin` instead of the intended org-level Admin. Drop the field from the tenant surface. The platform bundle still flips it on for the `monitoring-system` component through `Package.spec.components["monitoring-system"].values.oidc.grafanaAdmin` (the platform Grafana chart continues to read the value verbatim in `packages/system/monitoring/values.yaml`); tenant Grafana releases inherit the chart default `false` and stay at org-level Admin. Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com> |
||
|
Ivan Okhotnikov
|
be52e80289 |
fix(monitoring): split monitoring-application PackageSource into default/oidc variants
Follow-up to the previous self-review-fix commit: the unconditional `- cozystack.keycloak-operator` in dependsOn was itself a CRITICAL cold-install regression on any cluster where `authentication.oidc.enabled: false` (the default). The keycloak-operator Package is only emitted by `packages/core/platform/templates/bundles/system.yaml:112` when that flag is on — a hard dep against it from the default variant would leave `cozystack.monitoring-application` stuck on DependenciesNotReady forever, cascading a `cozystack-platform` HR timeout that hits every OIDC-off customer. Fix by mirroring the cozystack-engine variant split: - `default` variant keeps the baseline reconciliation edges (no keycloak-operator dep). - `oidc` variant additionally waits for `cozystack.keycloak-operator` so the v1.edp.epam.com CRDs are registered before the monitoring chart reconciles and the `.Capabilities.APIVersions.Has` guard in templates/grafana/oidc-keycloak.yaml can safely emit KeycloakClient/Scope/Groups. The bundle template swings between variants on `.Values.authentication.oidc.enabled`, mirroring the cozystack-engine invocation two lines above. The `monitoring-system` component override (oidc.grafanaAdmin: true) is preserved across both branches — the values field is a no-op when mode: None (no auth.generic_oauth block is emitted at all), so keeping it identical avoids two divergent code paths. Also widened the platform helm-unittest coverage to lock: - default variant does NOT depend on keycloak-operator (permanent DependenciesNotReady guard). - oidc variant DOES. - Bundle emits the right variant based on authentication.oidc.enabled AND keeps the GrafanaAdmin uplift on the monitoring-system component in either branch. 83 platform tests + 23 monitoring tests still green. Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com> |
||
|
Ivan Okhotnikov
|
ffeb8e5e4f |
fix(monitoring): close self-review blockers for the OIDC PR
Address the two blocking-class findings from the cozy-review pass on
this PR:
(1) Missing PackageSource dependency on cozystack.keycloak-operator
(MAJOR cold-install race).
Without this edge, the monitoring chart can reconcile before the
v1.edp.epam.com CRDs are registered on a fresh cluster — the
`.Capabilities.APIVersions.Has` guard in
templates/grafana/oidc-keycloak.yaml then silently drops all
Keycloak-side objects for mode: System, and Grafana's
auth.generic_oauth block points at a client Keycloak never
provisions. Add the ordering edge, plus a
sources_monitoring_application_dependson_test.yaml regression
guard so a future edit cannot drop it silently.
(2) allow_assign_grafana_admin driven by `.Release.Name ==
monitoring-system` (MINOR fragile coupling).
Replace the release-name sniff with an explicit
`oidc.grafanaAdmin: false` values field, and flip it on for the
platform Grafana release via
Package.spec.components["monitoring-system"].values.oidc.grafanaAdmin
in packages/core/platform/templates/bundles/system.yaml —
mirroring the pattern monitoring-agents already uses for
per-component overrides. Tenant Grafana releases inherit the
chart default false and stay at org-level Admin.
Two new helm-unittest cases in packages/system/monitoring/tests/
lock the values-driven behaviour on both sides
(grafanaAdmin=true → Admin uplift, default → tenant stays at org
Admin), plus bundles_monitoring_grafana_admin_test.yaml in
packages/core/platform/tests/ regression-guards the Package's
component override so the platform release keeps its uplift on
future edits.
Also document the mode-toggle group-membership loss in
docs/oidc-grafana.md — mode: System → CustomConfig/None deletes the
three chart-owned KeycloakRealmGroups; membership is not restored on
toggle-back. Not a bug, but the operator UX cost of the toggle
deserves a bullet in Failure modes.
Regenerated artifacts (cozyvalues-gen + hack/update-crd.sh):
packages/extra/monitoring/values.schema.json
packages/extra/monitoring/README.md
packages/system/monitoring-rd/cozyrds/monitoring.yaml
Test suites still green: packages/system/monitoring 23/23,
packages/core/platform 81/81.
Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com>
|
||
|
Ivan Okhotnikov
|
4fe85d039c |
chore(monitoring): move OIDC schema source to packages/extra + regenerate
The Monitoring public API surface (openAPISchema in monitoring-rd
cozyrds) is generated from packages/extra/monitoring/values.yaml by
cozyvalues-gen + hack/update-crd.sh — the foundation commit
(186239044) instead wrote it by hand in
packages/system/monitoring-rd/cozyrds/monitoring.yaml, which the
pre-commit hook then reverts against every push.
Move the oidc annotations into packages/extra/monitoring/values.yaml
(matching the pattern used by the rest of the top-level fields) and
run `make generate` in that directory. That regenerates:
- packages/extra/monitoring/values.schema.json
- packages/extra/monitoring/README.md
- packages/system/monitoring-rd/cozyrds/monitoring.yaml
(openAPISchema + keysOrder, both including the new oidc keys)
No behaviour change — the values.yaml under packages/system/monitoring
still carries the same oidc defaults so `helm template` works
stand-alone, and helm-unittest keeps passing (22/22).
Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
|
||
|
Ivan Okhotnikov
|
ed4aa841c8 |
docs(oidc-grafana): operator guide for Phase-1 OIDC on the Grafana instance
New docs/oidc-grafana.md parallel to docs/oidc-tenant.md from cozystack/cozystack#3044. Covers: - The three modes on the Monitoring CR (None / System / CustomConfig) and how the selector maps to what the chart provisions. - What System mode creates on the cozy realm: a per-instance confidential KeycloakClient, a KeycloakClientScope with the audience mapper, three KeycloakRealmGroups, and the persistent client-secret Secret; plus the Grafana CR side (auth.generic_oauth section, GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET env). - Both CustomConfig payload paths (inline vs secretRef) and the XOR guard. - User-role mapping via role_attribute_path and how an operator adds users to the three groups through a KeycloakRealmUser. - Failure modes (assertSystemEnabled, XOR, wrong claims, emailVerified prescriptive requirement) and what is out of scope for Phase 1 (per-tenant realms, backend-logout-url, disabled login form, CEL claimValidationRules). Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io> |
||
|
Ivan Okhotnikov
|
fe2c606f79 |
test(e2e): add Phase-1 OIDC render-side bats for Monitoring
Two bats files mirroring the shape of the tenant kube-apiserver PR's
kubernetes-oidc-{system,customconfig}.bats:
- monitoring-oidc-system.bats — asserts the Monitoring CR admits the
new spec.oidc.mode: System field; the chart renders the Grafana CR
with an auth.generic_oauth section pointing at the cozy realm; a
32-char client-secret Secret is persisted; the KeycloakClient (with
public: false), KeycloakClientScope (audience mapper), and three
KeycloakRealmGroups (-admin, -editor, -viewer) land in cozy; and
the Grafana Deployment injects GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET
from the Secret.
- monitoring-oidc-customconfig.bats — asserts the CR admits inline
CustomConfig; the operator's payload lands on spec.config verbatim
with no cozy realm in the auth_url; NO KeycloakClient / Scope /
Groups are created in cozy; and the secretRef variant mounts the
operator Secret under /etc/grafana/oidc without emitting
auth.generic_oauth under spec.config.
Neither drives a real browser login through Keycloak — that is
deferred to the follow-up integration suite, same posture as the
tenant kube-apiserver's Phase-1 tests.
Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
|
||
|
Ivan Okhotnikov
|
1f44bc06cf |
test(monitoring): helm-unittest coverage for the OIDC selector
15 cases across the three modes plus the two fail-close guards: - mode: None emits nothing OIDC-related — no cozy-realm objects, no client-secret Secret, no auth.generic_oauth key on the Grafana CR, no GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET / GF_PATHS_CUSTOM_INI env. - mode: System renders KeycloakClient + KeycloakClientScope + three KeycloakRealmGroups + persistent client-secret Secret, and pins the Grafana spec.config to the cozy realm with a role_attribute_path that maps the -admin/-editor/-viewer groups to Grafana roles. - mode: System fails the render when the platform-level OIDC feature is off, via the assertSystemEnabled helper. - mode: CustomConfig inline writes the operator-supplied map verbatim into spec.config.auth.generic_oauth without touching cozy. - mode: CustomConfig secretRef mounts the operator Secret under /etc/grafana/oidc + wires GF_PATHS_CUSTOM_INI, and does NOT emit auth.generic_oauth under spec.config (the ini fragment is authoritative). - CustomConfig branch fails-close on both/neither of the two payloads via the assertCustomConfigXor helper. - Platform release (Release.Name == monitoring-system) upgrades the Admin role to server-level GrafanaAdmin through the allow_assign_grafana_admin flag; tenant Grafana stays at org-level Admin. Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io> |
||
|
Ivan Okhotnikov
|
8eff03b323 |
feat(monitoring): wire auth.generic_oauth into the Grafana CR
Patch the Grafana CR so that the oidc.mode selector drives the actual Grafana runtime config: - mode: System — emit a full auth.generic_oauth section under spec.config with client_id / client_secret / auth_url / token_url / api_url / scopes / role_attribute_path / allow_assign_grafana_admin built from the helpers, and inject GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET as an env from the persistent Secret introduced in the previous commit. role_attribute_path maps membership in the per-instance -admin / -editor / -viewer groups to Grafana's Admin / Editor / Viewer roles, defaulting to Viewer on authenticated identities with none of them. allow_assign_grafana_admin is set to true only for the platform release (Release.Name == 'monitoring-system'); tenant Grafana stays at org-level Admin. - mode: CustomConfig (inline) — pass the operator-supplied config map verbatim to spec.config.auth.generic_oauth. `cozy` is not in the path. - mode: CustomConfig (secretRef) — mount the operator-managed Secret under /etc/grafana/oidc and load it via GF_PATHS_CUSTOM_INI. The ini fragment is authoritative; the chart does not emit auth.generic_oauth under spec.config in this branch. - mode: None (default) — no change to the rendered manifests. CustomConfig branch fails the render at the XOR helper if both or neither of `config` / `secretRef.name` are set. `disable_login_form` stays false so the admin_user/admin_password Secret remains a documented break-glass path in every mode. Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io> |
||
|
Ivan Okhotnikov
|
473b112f66 |
feat(monitoring): persist confidential OIDC client secret for System mode
New oidc-client-secret.yaml renders a Kubernetes Secret with the confidential client_secret used by both the EDP-Keycloak operator (via \$<secret>:client-secret from oidc-keycloak.yaml) and the Grafana pod (via GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET, wired by the next commit). Value is generated at first install (randAlphaNum 32) and preserved across upgrades through the same \`lookup\` pattern used by packages/system/dashboard. secret-hash annotation surfaces the value change to whichever controller is watching, ensuring KeycloakClient + Grafana Deployment re-reconcile if it ever rotates. Rendered only for mode: System — CustomConfig operators bring their own credentials (inline plaintext or a Secret they manage out-of-band). Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io> |
||
|
Ivan Okhotnikov
|
9dd9f1d4dd |
feat(monitoring): render Keycloak client + scope + groups for System mode
New oidc-keycloak.yaml template mirrors the tenant kube-apiserver Phase-1 shape (cozystack/cozystack#3044) with three Grafana-specific adjustments: - Client is confidential (public: false, directAccess: false); Grafana is server-side so a client_secret is required. The value is wired via the EDP-Keycloak `$<secret>:<key>` reference; the backing Secret is created by the next commit. - redirectUri is https://grafana.<host>/login/generic_oauth — the Grafana generic_oauth callback path. - Three KeycloakRealmGroups (`<clientId>-{admin,editor,viewer}`) are chart-owned. Membership is set by an operator through the Keycloak UI or a KeycloakRealmUser CR; grafana.yaml will map group membership to Admin / Editor / Viewer via role_attribute_path. Guarded by mode == System AND _cluster.oidc-enabled == true (via the assertSystemEnabled helper). Renders nothing for None or CustomConfig. Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io> |
||
|
Ivan Okhotnikov
|
e12bb1a3ba |
feat(monitoring): scaffold OIDC selector on the Grafana instance
Add the plumbing needed for a Phase-1 OIDC integration on Grafana, matching the shape of cozystack/cozystack#3044 for the tenant kube-apiserver: - values.yaml grows a top-level oidc block with a three-value mode enum (None/System/CustomConfig) and a customConfig payload with the XOR-inline/secretRef surface. - monitoring-rd advertises the new selector via its openAPISchema and keysOrder so operators see it on the Monitoring CR. - templates/_helpers.tpl introduces the naming helpers reused by every downstream OIDC template: clientId, audienceScopeName, clientSecretName, grafanaHost, redirectUri, systemIssuerURL, roleAttributePath, allowAssignGrafanaAdmin (platform-only), assertSystemEnabled, assertCustomConfigXor. No behaviour change on default values — mode: None short-circuits and no OIDC objects are rendered. Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io> |
||
|
myasnikovdaniil
|
29e5a55473 |
fix(kubernetes): re-pin deleted centos:stream9 digest for kubevirt-csi-driver (#3303)
## What this PR does `packages/apps/kubernetes/images/kubevirt-csi-driver/Dockerfile` pins its runtime base image to `quay.io/centos/centos:stream9` by digest. `stream9` is a rolling tag, and upstream deleted the pinned digest (`sha256:0c9db821…`) when it published a fresh stream9 build. As a result `make image-kubevirt-csi-driver` now fails with `not found` on every build — open PRs and `main` alike (the `Build packages/apps/kubernetes` job goes red before E2E even starts). Re-pin to the current live `stream9` digest (`sha256:a93e3316…`, verified pullable and multi-arch: amd64/arm64/ppc64le/s390x). No Dockerfile logic changes. ## Known limitation and follow-up This is an emergency CI unblock, **not** a durable fix. `stream9` is a rolling, garbage-collected tag: upstream deletes old digests on every rebuild, so the same `not found` breakage will recur the next time CentOS rolls the tag faster than the pin is bumped. This is the same operation Renovate's weekly (non-automerged) digest PR performs. A durable fix is out of scope for this hotfix and left as follow-up: - mirror the base image into cozystack's own registry so old digests are not GC'd, or - move to an immutable-digest base such as UBI9 (`registry.access.redhat.com/ubi9`), or - enable automerge / tighten the schedule for the digest-only Renovate group so future rolls are absorbed automatically. ### Downstream repositories - [x] No downstream repository is affected by this change <!-- Runtime base-image digest of an internal component image; no API, values, CRD, or chart-interface surface changes. --> ### Release note ```release-note fix(kubernetes): restore the kubevirt-csi-driver image build after upstream deleted the pinned centos:stream9 base image digest ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated the Kubernetes CSI driver’s runtime base image to a newer verified version. * Runtime behavior and application functionality remain unchanged. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Ivan Okhotnikov
|
ce4b263ca6 |
fix(kubernetes): re-pin deleted centos:stream9 digest for kubevirt-csi-driver
quay.io/centos/centos:stream9 is a rolling tag; upstream deleted the pinned digest (sha256:0c9db821...) when it published a new stream9 build, so every build of image-kubevirt-csi-driver fails with "not found" — on open PRs and on main. Re-pin to the current live stream9 digest (sha256:a93e3316...). This is an emergency CI unblock, not a durable fix: stream9 is garbage-collected, so upstream will delete this digest too on its next rebuild and the same "not found" breakage recurs until the pin is bumped again — the same operation Renovate's weekly, non-automerged digest PR performs. A durable fix (mirror the base image into cozystack's own registry, or move to an immutable-digest base such as UBI9) is out of scope for this hotfix and left as follow-up. Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com> |
||
|
Myasnikov Daniil
|
1f084cecc8 |
test(e2e): harden select-install.sh per review
Address the review blockers on the diff-driven install selector: - B1: seed cozystack.cozystack-engine for any non-empty closure. Suites backed by a system package (e.g. securitygroup, which applies a sdn.cozystack.io resource served by the aggregated cozystack-api) carry no dependsOn edge to the engine, so the forward walk could never reach it and the emitted set could not run the suite. Hard-error if the engine source is absent from the graph. - B2: an unmapped suite is now a hard error in closure mode (every bad suite reported in one run, no partial output) instead of failing open. --validate additionally asserts every hack/e2e-chainsaw suite resolves to a real PackageSource or "-", and fails when the suites dir is missing. Also correct the inaccurate "securitygroup also stands up a Kubernetes app" comment (its fixtures reference Postgres/Kubernetes as selectors only), note the variant-union over-selection in build_forward_deps, drop the unnecessary source copies from the tests, and add coverage for stdin input, the engine seed, fail-closed mapping, the --validate suite guard, and the missing-suites-dir / missing-engine failure modes. Assisted-By: Claude <noreply@anthropic.com> Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
myasnikovdaniil
|
77389d6e99 |
chore(deps): update github.com/kubevirt/csi-driver digest to 27b52aa (#3066)
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [github.com/kubevirt/csi-driver](https://redirect.github.com/kubevirt/csi-driver) | replace | digest | `51c6bca` → `27b52aa` | --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/cozystack/cozystack). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yNDIuMiIsInVwZGF0ZWRJblZlciI6IjQzLjI1OS4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJhdXRvbWF0ZWQiLCJkby1ub3QtbWVyZ2UvaG9sZCJdfQ==--> |
||
|
myasnikovdaniil
|
a269eb3965 |
chore(deps): update actions/checkout action to v6.0.3 (#3133)
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [actions/checkout](https://redirect.github.com/actions/checkout) | action | patch | `v6.0.2` → `v6.0.3` | --- ### Release Notes <details> <summary>actions/checkout (actions/checkout)</summary> ### [`v6.0.3`](https://redirect.github.com/actions/checkout/blob/HEAD/CHANGELOG.md#v603) [Compare Source](https://redirect.github.com/actions/checkout/compare/v6.0.2...v6.0.3) - Fix checkout init for SHA-256 repositories by [@​yaananth](https://redirect.github.com/yaananth) in [#​2439](https://redirect.github.com/actions/checkout/pull/2439) - fix: expand merge commit SHA regex and add SHA-256 test cases by [@​yaananth](https://redirect.github.com/yaananth) in [#​2414](https://redirect.github.com/actions/checkout/pull/2414) </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/cozystack/cozystack). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yNDIuMiIsInVwZGF0ZWRJblZlciI6IjQzLjI0Mi4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJhdXRvbWF0ZWQiLCJkby1ub3QtbWVyZ2UvaG9sZCJdfQ==--> |
||
|
myasnikovdaniil
|
1d1c7c2c38 |
chore(deps): update altinity/clickhouse-backup docker tag to v2.7.4 (#3226)
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [altinity/clickhouse-backup](https://redirect.github.com/Altinity/clickhouse-backup) | final | patch | `2.7.2` → `2.7.4` | --- ### Release Notes <details> <summary>Altinity/clickhouse-backup (altinity/clickhouse-backup)</summary> ### [`v2.7.4`](https://redirect.github.com/Altinity/clickhouse-backup/blob/HEAD/ChangeLog.md#v274) [Compare Source](https://redirect.github.com/Altinity/clickhouse-backup/compare/v2.7.3...v2.7.4) NEW FEATURES - add `clickhouse.parts_columns_max_bytes_before_external_group_by` (env `CLICKHOUSE_PARTS_COLUMNS_MAX_BYTES_BEFORE_EXTERNAL_GROUP_BY`, default `100000000`) and `clickhouse.parts_columns_max_memory_usage` (env `CLICKHOUSE_PARTS_COLUMNS_MAX_MEMORY_USAGE`, default `200000000`) — make the memory caps injected into the `system.parts_columns` check query configurable instead of hardcoded, `0` means don't inject the corresponding setting (pre-2.7.2 behavior), fix [#​1420](https://redirect.github.com/Altinity/clickhouse-backup/issues/1420) BUG FIXES - fix `--restore-database-mapping`/`--restore-table-mapping` losing unrelated comma-separated `--tables` pattern items, or applying the mapping to the wrong item when the same source database/table name was repeated across items — each comma-separated pattern item is now matched and substituted independently, fix [#​1421](https://redirect.github.com/Altinity/clickhouse-backup/issues/1421) ### [`v2.7.3`](https://redirect.github.com/Altinity/clickhouse-backup/blob/HEAD/ChangeLog.md#v273) [Compare Source](https://redirect.github.com/Altinity/clickhouse-backup/compare/v2.7.2...v2.7.3) NEW FEATURES - add `general.max_broken_part_ratio` (env `MAX_BROKEN_PART_RATIO`, default `0`) — when >0, `create`/`create_remote` skips broken data parts instead of aborting the whole backup as long as the broken/total part ratio stays at or below the configured fraction; skipped parts are logged as warnings and the backup is marked successful but partial; `0` preserves the legacy any-broken-part-fails behavior, fix [#​1418](https://redirect.github.com/Altinity/clickhouse-backup/issues/1418) - add `clickhouse.rebind_replica_path_if_exists` (env `CLICKHOUSE_REBIND_REPLICA_PATH_IF_EXISTS`, CLI `--rebind-replica-path-if-exists`, also accepted by the REST API `POST /backup/restore` endpoint) — opt-in rebind of the replicated table ZK path to `default_replica_path` during `restore` when the resolved path already has leftover children but no local table uses it (async-stale state after DROP); MUST stay `false` during a concurrent multi-replica restore to avoid split-brain, fix [#​849](https://redirect.github.com/Altinity/clickhouse-backup/issues/849) - add `gcs.disable_http2` (env `GCS_DISABLE_HTTP2`) — force HTTP/1.1 for the GCS client to avoid single-TCP-connection HTTP/2 multiplexing becoming the bottleneck for high-throughput parallel downloads, fix [#​1437](https://redirect.github.com/Altinity/clickhouse-backup/pull/1437) IMPROVEMENTS - switch the S3 client retry mode from `aws.RetryModeStandard` to `aws.RetryModeAdaptive`, so upload/download retries back off with a client-side rate limiter under throttling (`SlowDown`/503) instead of a fixed token bucket - support tables with `ENGINE=Alias` during `restore` — strip the explicit column list (Alias inherits columns from the target table) and add `allow_experimental_alias_table_engine=1` for ClickHouse 25.11+, fix [#​1426](https://redirect.github.com/Altinity/clickhouse-backup/issues/1426) - skip tables from databases with engine `DataLakeCatalog` (read-only external catalogs) during backup, in addition to `MySQL`/`PostgreSQL`/`MaterializedPostgreSQL`, fix [#​1417](https://redirect.github.com/Altinity/clickhouse-backup/issues/1417) - speed up `create` on servers with many tables: batch the in-progress `system.mutations` lookup instead of issuing one query per table (O(N^2) -> O(N)), fix [#​1431](https://redirect.github.com/Altinity/clickhouse-backup/pull/1431) - speed up `restore`/`download` table list resolution: eliminate the O(N^2) linear duplicate scan in `getTableListByPatternLocal`/`getTableListByPatternRemote` via a shared O(1) dedup keyed on `metadata.TableTitle`, fix [#​1430](https://redirect.github.com/Altinity/clickhouse-backup/pull/1430) - extend TestFlows FIPS 140-3 test coverage: dedicated test steps, stronger `--fips-info`/version-output and TLS-handshake assertions, updated requirements documentation, fix [#​1443](https://redirect.github.com/Altinity/clickhouse-backup/pull/1443) BUG FIXES - restore the default `general.compression_use_multi_thread` to `true`: [#​1378](https://redirect.github.com/Altinity/clickhouse-backup/issues/1378) defaulted it to `false`, which silently dropped gzip/zstd compression from multi-threaded (pre-1378 gzip always used `pgzip`) to single-threaded and caused \~30% slower upload throughput for backups dominated by one large table (where `upload_concurrency` provides no per-stream parallelism); the option is now silently ignored instead of failing config validation for formats other than gzip/zstd - fix v2.7.1 regression where `restore --schema` on a second replica rebound the ZK replica path even while the first replica was active and healthy — a foreign/renamed local table occupying the resolved ZK path is still detected automatically via `system.replicas` and rebound to `default_replica_path`, but a non-empty ZK path without a local table now defaults to the HA-safe join and requires the new opt-in `rebind_replica_path_if_exists`, fix [#​1428](https://redirect.github.com/Altinity/clickhouse-backup/issues/1428) - fix inconsistent `--rbac` behavior: `restore --rbac` no longer returns an error when the backup contains no RBAC `access` directory, matching the `download --rbac` behavior, fix [#​1432](https://redirect.github.com/Altinity/clickhouse-backup/issues/1432) - inject `ON CLUSTER` for `LIVE VIEW` and `WINDOW VIEW` restored with explicit column lists when `restore_schema_on_cluster` is set — `enrichQueryWithOnCluster` missed these statements and the views were created only on the local replica, fix [#​1433](https://redirect.github.com/Altinity/clickhouse-backup/pull/1433) - mask sensitive values (passwords, storage credentials, encryption keys) when logging `--env` config overrides, and in the API server action logs and `/backup/status` output, fix [#​1441](https://redirect.github.com/Altinity/clickhouse-backup/pull/1441) </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/cozystack/cozystack). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yNDIuMiIsInVwZGF0ZWRJblZlciI6IjQzLjI0Mi4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJhdXRvbWF0ZWQiLCJkby1ub3QtbWVyZ2UvaG9sZCJdfQ==--> |
||
|
Aleksei Sviridkin
|
249f7b5a5e |
feat(platform, ingress): propagate operator wildcard certificate to per-tenant termination points (#2990)
## What this PR does The operator-provided wildcard certificate (`publishing.certificates.wildcardSecretName`) currently serves the root tenant only: everything is same-namespace, so the root ingress controller and Gateway read the operator's TLS Secret directly. Child tenants run their own ingress controller / Gateway in their own namespace under a default-deny policy and cannot read that Secret across namespaces, so they fall back to per-host ACME. This propagates the operator wildcard to per-tenant termination points by replicating the Secret into each tenant namespace that owns one, keeping the root-MVP invariant intact: the wildcard is deliberately distributed to tenant namespaces, but no tenant RBAC is widened — every consumer reads only its own-namespace copy. A new controller in `cozystack-controller` does the replication with no extra operator input. It reads the same platform values channel the consumers read (`cozy-system/cozystack-values`): the wildcard Secret name from `_cluster.wildcard-secret-name` and the publishing namespace from `_cluster.expose-ingress`. It mirrors that Secret into every tenant namespace whose `namespace.cozystack.io/ingress` or `namespace.cozystack.io/gateway` label equals its own name — i.e. the namespace runs its own controller / Gateway. Because the source is derived from the same value that makes the consumers reference it, the replica is created whenever the consumers expect it — no manual labelling and no upgrade hazard where a child controller references a Secret that nothing creates. Replicas are refreshed on rotation and pruned when a namespace stops terminating TLS. Teardown of every replica happens on exactly one trigger: clearing `wildcardSecretName` (explicit disable). A source that is merely absent or mistyped, or a transiently-missing values channel, leaves existing replicas in place, so a brief gap, a delete+recreate rotation, or a misconfigured publishing namespace never drops tenant TLS. The platform controller's own ServiceAccount gains Secret writes; tenant RBAC is untouched. The controller does not cache every Secret in the cluster: the manager's Secret informer is scoped to managed replicas and the values channel only, and the operator source / foreign collisions are read through the uncached APIReader. The source is not watched (its name is dynamic), so an in-place rotation is picked up by a bounded periodic resync while the publishing tenant serves the source with no lag. On the consumer side, each tenant ingress controller now sets `--default-ssl-certificate` to the wildcard Secret in its own namespace (`.Release.Namespace/<name>`), so a child tenant serves the replica instead of minting per-host ACME. The Gateway path already renders `certMode: existingSecret` with a same-namespace `wildcardSecretRef` for every tenant, so it consumes the replica with no chart change. ### Why not clustersecret-operator The issue proposed the shipped `clustersecret-operator`. It materializes inline template data into selected namespaces; it cannot mirror an existing source Secret, and it is not installed in any default bundle. Driving it would mean embedding the certificate and private key as literal data in a cluster-scoped object rendered from a Helm `lookup` — that broadcasts key material the design keeps off any shared channel, and a `lookup` is invisible to the Flux helm-controller digest so it would not re-render on rotation. Its namespace selector also cannot express "the namespace whose owner label equals its own name", so it would over-replicate the key into every tenant namespace. A small dedicated reconciler targets exactly the termination namespaces and keeps the key off the values channel. ### Coverage caveat Replication delivers the certificate bytes, not SAN coverage: a single `*.<root-host>` does not match `*.<child-apex>`. Operators serving child-tenant subdomains supply a certificate whose SANs cover those apexes; the docs spell this out. The replica carries the wildcard private key into each terminating tenant namespace — the same exposure as the per-host ACME Secret it replaces, now shared across tenants. ### Tests - Go fake-client tests for the reconciler: termination-only replication (ingress-owning and gateway-owning namespaces only, never inheriting ones, never over the source), copy markers, rotation, prune on namespace demotion, the non-destructive source-deletion contract, replica self-heal after out-of-band deletion, the publishing-namespace self-replication guard, disable-via-config teardown, source rename, foreign-Secret collision safety (no overwrite, no requeue, does not block siblings), transient-error requeue, non-TLS source ignored, absent-channel keeps replicas, publishing-namespace mismatch keeps and requeues, the cache-scope pin, the enqueue-wiring map, an `ownsTerminationPoint` table, and the active-only requeue contract. - helm-unittest for the ingress `default-ssl-certificate` wiring (including a child-tenant controller pointing at its own-namespace replica) and for the cozystack-controller ClusterRole's new Secret-write rule. ### Screenshots N/A — no UI changes. ### Release note ```release-note feat(platform): the operator-provided wildcard TLS certificate (`publishing.certificates.wildcardSecretName`) is now served by per-tenant ingress controllers and Gateways automatically — the platform controller replicates it into each tenant namespace that terminates TLS, with no extra operator input and no cross-namespace Secret read ``` Closes #2820 Part of #2811 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Wildcard TLS Secret now replicates from the platform to tenant namespaces that terminate TLS via their own ingress controller or Gateway, including child-tenant scenarios. * **Documentation** * Clarified configuration, timing, and SAN matching requirements for wildcard replication. * Updated gateway and ingress-nginx default certificate documentation for replica mirroring behavior. * **Bug Fixes** * Improved handling for missing/invalid source secrets (no unnecessary pruning) and safe collision behavior. * **Tests** * Added comprehensive reconciliation/replication test coverage. * Added Helm unittest target for the controller chart. * **Chores** * Updated controller RBAC to support wildcard TLS Secret replication. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
6b1ad96093 |
fix(e2e): gate LINSTOR node-list probe on a ready controller endpoint (#3250)
## What this PR does The E2E `Install Cozystack` step runs `hack/e2e-post-install-prep.sh` in the background, which waits for 3 LINSTOR satellites to come Online by dialing the `linstor-controller` Service (`linstor+ssl://linstor-controller:3371`) from the controller pod. That probe was gated only by a one-shot `kubectl wait deployment/linstor-controller --for=condition=available`, which goes stale. The controller carries `reloader.stakater.com/auto=true` and mounts the cert-manager-issued API-TLS Secret (`packages/system/linstor/templates/cluster.yaml`), so when cert-manager (re)writes that Secret during bring-up, reloader rolls the single-replica controller. The roll drops the Service to zero ready endpoints *after* `Available` already passed, and a dial into that window fails with `[Errno 113] No route to host` (EHOSTUNREACH — the state of a ClusterIP whose Service has no ready backend). The blind `until` loop then churned the CLI on that error, tolerating a real un-Ready-controller race instead of gating on it. This gates every probe on the `linstor-controller` Service actually having a ready endpoint (a non-empty `Endpoints .subsets[].addresses`), so the CLI only dials a routable controller and a mid-loop reload re-waits for the endpoint instead of erroring. The `Online == 3` satellite-convergence check stays the real readiness assertion, and the dedicated 300s budget is preserved from the previous `timeout 300`; on timeout the loop now dumps the controller endpoints and pods for diagnostics before failing, per the E2E diagnostics convention. Evidence for the mechanism: the same single `Error: Unable to connect to linstor+ssl://linstor-controller:3371: [Errno 113] No route to host` line appears exactly once per install across multiple runs and is immediately recovered by the retry — consistent with one reloader-driven controller roll during initial bring-up, not a persistent outage. Scoped to the LINSTOR readiness path in `hack/e2e-post-install-prep.sh` only; no change to install ordering, dependencies, or timeouts. ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved the LINSTOR startup readiness check to wait more reliably for the service to become reachable before verifying node status. * Made the node-online verification more resilient by handling transient connectivity errors and enforcing a clearer timeout. * Added extra diagnostic output when readiness does not complete in time, helping with troubleshooting failed installs. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
3763c528da |
fix(seaweedfs): soften master anti-affinity for small clusters (#3197)
## What this PR does Overrides the seaweedfs master anti-affinity in the package values from a hard `requiredDuringSchedulingIgnoredDuringExecution` hostname term to a strong preference (`preferredDuringSchedulingIgnoredDuringExecution`, weight 100). The selector and topologyKey are unchanged. The vendored subchart pins the master to a hard hostname anti-affinity — its own comment notes that emptying it is what allows single-node deployment. With the master StatefulSet at three replicas, the surplus master Pods stay Pending forever on clusters with fewer than three nodes, so `helm install --wait` never returns and the S3 stack never comes up. Softening to a strong preference keeps the masters spread across distinct nodes whenever the cluster has them (the scheduler only co-locates when there is no alternative — exactly the small-cluster case), while letting single- and dual-node clusters bring the master up. This mirrors the posture already applied to the s3 component in the same values file. A helm-unittest pins the softened term (asserts the hard term is gone and the preferred term carries the right weight, topologyKey, and component selector) so a future subchart bump cannot silently re-harden it. ### Screenshots N/A — no UI changes. ### Release note ```release-note fix(seaweedfs): soften the master anti-affinity to a preference so the S3 stack installs on clusters with fewer than three nodes ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * SeaweedFS master pods now use a softer scheduling preference for hostname separation, improving deployment success on smaller clusters. * **Tests** * Added a Helm chart validation to confirm the anti-affinity behavior is now expressed as a weighted preference (not a hard requirement), including the expected selector and topology key. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |