feat(monitoring): per-instance OIDC selector for the Grafana instance (Phase 1) - #3176

…roups

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 <[email protected]>

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 <[email protected]>

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 <[email protected]>

…te-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 <[email protected]>

…ry 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 <[email protected]>

…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 <[email protected]>

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 <[email protected]>

…chema

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 <[email protected]>

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 1f66456 but slipped out of
that commit's file list; this is the cleanup.

Signed-off-by: IvanHunters <[email protected]>

…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 <[email protected]>

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 <[email protected]>

…pat)

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 <[email protected]>

…st Job existence

CI E2E showed the users-Job assertion returning `DESIRED_USERS_JSON:
[]` — but the CR body had `users: [{email: [email protected],
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 `[email protected]`. 600s window matches the inner HR's
15m timeout with room for a slow reconcile.

Signed-off-by: IvanHunters <[email protected]>

… 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 <[email protected]>

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 <[email protected]>

…p 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 <[email protected]>

… 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 <[email protected]>

…ctually 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 <[email protected]>

… 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 <[email protected]>