Commit Graph
18 Commits
Author SHA1 Message Date
Andrei Kvapil 705e39d3d7 feat(monitoring): install Grafana plugins from the in-cluster mirror
Point GF_INSTALL_PLUGINS at the grafana-dashboards static file server
instead of the public Grafana catalog, so plugin installation needs no
internet egress at pod startup and keeps working on air-gapped installs.
Route the upstream Grafana image reference through cozy-lib.image so
mirrored-registry installs keep working, and add regression tests for
the image reference and the plugin install path.

Assisted-By: Claude
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
2026-07-24 16:08:51 +02:00
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>
2026-07-15 15:31:45 +03:00
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>
2026-07-15 15:31:45 +03:00
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>
2026-07-15 15:31:45 +03:00
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>
2026-07-15 15:31:45 +03:00
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>
2026-07-15 15:31:45 +03:00
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>
2026-07-15 15:31:45 +03:00
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>
2026-07-15 15:31:45 +03:00
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>
2026-07-15 15:31:45 +03:00
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>
2026-07-15 15:31:45 +03:00
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>
2026-07-15 15:31:44 +03:00
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>
2026-07-15 15:31:44 +03:00
scooby87 f91841fce6 fix(monitoring): surface missing VictoriaLogs and gate dead-end vlinsert ExternalName (#3215)
## What this PR does

Addresses #3181, where platform log ingestion could fail **silently**:
fluent-bit shipped to `vlinsert-generic` while the monitoring
`HelmRelease` reported `Ready`, and a `vlinsert-generic` `ExternalName`
in `cozy-monitoring` dead-ended at a `tenant-root` VictoriaLogs that did
not exist when the root tenant had monitoring disabled. Logs were
dropped with no failing signal.

Three complementary layers make the failure loud instead of silent:

1. **Readiness gate** —
`packages/extra/monitoring/templates/helmrelease.yaml` now sets a
`poller` `waitStrategy` and a `healthCheckExprs` entry on the
`VLCluster` `status.updateStatus`. A missing/unhealthy log store makes
the `monitoring-system` release **not-ready** instead of flipping
`Ready` the moment the CR is applied. Mirrors the existing seaweedfs
CNPG pattern.
2. **No empty-log-store hole** —
`packages/system/monitoring/templates/vlogs/vlogs.yaml` fails the render
on an empty `logsStorages`. With no entry no `VLCluster` exists, the
readiness gate has nothing to match, and fluent-bit would silently drop
logs — so this is now a loud misconfiguration.
3. **No silent fallback** — the `cozy-monitoring → tenant-root`
`vlinsert`/`vminsert` `ExternalName` redirects in `cozystack-basics` are
created only when the root tenant hosts monitoring. The template reads
the live `Tenant/root` `spec.monitoring` via `lookup` (source of truth,
so it cannot drift from the tenant's real state) and falls back to a new
platform value `monitoring.rootEnabled` (via
`_cluster.monitoring-enabled`) when `lookup` is empty (dry-run / first
render). The default preserves the historical always-create behaviour.

Covered by helm-unittest across all four affected charts (141 tests):
readiness-gate fields, empty-vs-populated `logsStorages` contract,
`ExternalName` present/absent fallback contract, and the platform wiring
into the `cozystack-values` Secret. Also adds the missing `test:` target
to `packages/extra/monitoring/Makefile` so its suite runs in CI.

**Known limitation / follow-up:** the legacy platform monitoring release
installed directly into `cozy-monitoring` via the `cozystack.monitoring`
PackageSource cannot carry the readiness gate, because the engine's
`ComponentInstall`/HelmRelease builder and the vendored
`helm-controller` API v1.4.3 lack `waitStrategy`/`healthCheckExprs`.
That source is not referenced by any active bundle; full coverage there
needs a helm-controller bump plus engine support.

### Release note

```release-note
fix(monitoring): surface missing platform VictoriaLogs as not-ready and stop the dangling vlinsert-generic ExternalName from silently dropping logs (#3181)
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added a platform setting (`monitoring.rootEnabled`) to control whether
root-level monitoring is enabled.
* Monitoring redirect ExternalName services are now rendered only when
root monitoring is enabled (defaulting to enabled).
* **Bug Fixes**
* Ensures an explicitly set `monitoring.rootEnabled=false` is preserved
when generating platform configuration.
* Prevents incomplete monitoring deployments by failing rendering when
`logsStorages` is empty.
* Improves VictoriaMetrics readiness by delaying “ready” until the
operator reports an operational status.
* **Tests**
* Added/expanded Helm-unittest suites covering monitoring wiring,
redirects, readiness gating, and `logsStorages` requirements.
  * Added a `make test` target to run chart unittests.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-07-09 01:55:01 +03:00
Алексей Артамонов f700c9043c fix(monitoring): backfill release label onto pre-existing VM/VL storage PVCs
Re-review on #3094 (IvanHunters, MAJOR): the volumeClaimTemplate label lands on
PVCs only at creation — the StatefulSet controller never re-labels existing PVCs
and vm-operator's PVC reconcile is resize-only. So on an already-deployed cluster
(exactly the #3091 population) the pre-upgrade VM/VL storage PVCs stay unlabeled
and the cleanup hook's selector misses them.

Add migration 49 (mirrors the ClickHouse-keeper backfill in migration 48 for the
sibling #3072). It relabels the pre-existing vmstorage/vmselect/vlstorage PVCs of
every cozystack-managed VMCluster/VLCluster with apps.cozystack.io/application.name
read from the CR's own spec.managedMetadata, so only monitoring's clusters are
touched (a user's own VMCluster carries no such label) and the value always
matches what the cleanup hook selects. Best-effort like #48: a skipped relabel is
no worse than the pre-existing leak, so transient apiserver errors never abort the
platform upgrade. Bump platform targetVersion 49 -> 50.

Also correct the misleading comments (IvanHunters MINOR): spec.managedMetadata is
never applied to PVCs by vm-operator at any current version, so the claim-template
label is the only mechanism — do not imply a v0.71.0 crossover.

Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io>
Assisted-By: Claude <noreply@anthropic.com>
2026-07-08 22:27:42 +03:00
Алексей Артамонов e01b725a48 fix(monitoring): stamp release-scoping label on VM/VL storage PVCs so cleanup matches
Re-review on #3094 (IvanHunters, lexfrei): the release-scoped selector added
in ba5797f4 was a silent no-op on the shipped vm-operator v0.68.4. The
apps.cozystack.io/application.name label lived only under the VMCluster/VLCluster
spec.managedMetadata.labels, and v0.68.4 does not propagate managedMetadata onto
PVCs (that lands only in v0.71.0). So the delete matched nothing and the #3091
storage leak stayed open.

Stamp the label where it actually reaches the PVCs: on
spec.{vmselect,vmstorage,vlstorage}.storage.volumeClaimTemplate.metadata.labels
of the VMCluster/VLCluster CRs (packages/system/monitoring). vm-operator copies
claim-template labels onto the PVCs (IntoSTSVolume) even on v0.68.4, so the
existing cleanup selector (managed-by=vm-operator,apps.cozystack.io/application.name=
<release>-system) now matches and stays scoped to this release. Verified by
rendering the CRs (label resolves to monitoring-system, matching the hook).

Also add the explicit subjects[].namespace on the cleanup RoleBinding (lexfrei
non-blocking) and a helm-unittest suite asserting the label lands on all three
storage claim templates. Corrected the misleading test comment about how the
label reaches the PVCs.

Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io>
Assisted-By: Claude <noreply@anthropic.com>
2026-07-08 22:27:42 +03:00
Alexey Artamonov f9afc73096 fix(monitoring): surface missing VictoriaLogs and gate dead-end vlinsert ExternalName (#3181)
Platform log ingestion could fail silently: fluent-bit shipped to
vlinsert-generic while the monitoring HelmRelease reported Ready, and a
vlinsert-generic ExternalName in cozy-monitoring dead-ended at a
tenant-root VictoriaLogs that did not exist when the root tenant had
monitoring disabled. Logs were dropped with no failing signal.

Three complementary changes:

- Readiness (do not stay silently Ready): the monitoring-system
  HelmRelease now uses a `poller` waitStrategy and a healthCheckExpr on
  its VLCluster (status.updateStatus), so a missing/unhealthy log store
  makes the release not-ready instead of flipping Ready as soon as the
  CR is applied. Mirrors the seaweedfs CNPG pattern.

- No empty-log-store hole: system/monitoring rejects an empty
  logsStorages at render time (helm fail). With no entry no VLCluster is
  created, the readiness gate has nothing to match, and fluent-bit would
  silently drop logs — so this is made a loud misconfiguration rather
  than a documented footgun.

- No silent fallback: the cozy-monitoring -> tenant-root
  vlinsert/vminsert ExternalName redirects in cozystack-basics are now
  created only when the root tenant hosts monitoring, gated on
  _cluster.monitoring-enabled — fed from a new platform value
  monitoring.rootEnabled (default true, preserving the historical
  always-create behaviour). monitoring.rootEnabled is the single
  render-time source of truth: changing it re-renders cozystack-basics
  (helm-controller watches the cozystack-values Secret it flows into). A
  live `lookup` of Tenant/root spec.monitoring is deliberately NOT used:
  cozystack-basics is an operator-generated HelmRelease with drift
  detection off, so it re-renders only on install/upgrade/values-change
  — never on a reconcile tick or when a lookup result changes — and a
  lookup would go stale in both directions. Operators must keep
  monitoring.rootEnabled in sync with Tenant/root spec.monitoring.

Covered by helm-unittest across all four charts: readiness gate fields,
empty-vs-populated logsStorages contract, ExternalName present/absent
and null-guard contract, and the platform wiring into the
cozystack-values Secret. The CEL health predicates are exercised live,
not in helm-unittest (which cannot evaluate CEL). Adds the missing
`test:` target to packages/extra/monitoring/Makefile so its suite runs
in CI.

Known limitation (follow-up): the legacy cozystack.monitoring
PackageSource that installs system/monitoring directly into
cozy-monitoring cannot carry the readiness gate, because the engine's
ComponentInstall/HelmRelease builder and the vendored helm-controller
API v1.4.3 lack waitStrategy/healthCheckExprs. That source is not
referenced by any active bundle; full coverage needs a helm-controller
bump plus engine support.

Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
2026-07-08 22:27:22 +03:00
Aleksei Sviridkin 9e3ae7447b fix(ingress): keep tls section in operator wildcard mode
Dropping the whole tls block in wildcard mode disabled the forced
HTTP-to-HTTPS redirect: ingress-nginx only issues the 308 redirect when
TLS is enabled for the Ingress, while a host without a tls section is
still served on 443 with the default certificate but stays reachable
over plain HTTP. Keep the tls section with the host list and omit only
the per-host secretName, so ingress-nginx falls back to the operator
wildcard configured as the default SSL certificate and the redirect
stays enforced, matching the non-wildcard behaviour.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-06-10 03:18:42 +03:00
Aleksei Sviridkin b1f5d9579c feat(ingress): serve operator wildcard via default SSL certificate
When publishing.certificates.wildcardSecretName is set, the root
tenant's ingress-nginx controller now serves the operator-supplied
wildcard Secret as its --default-ssl-certificate, and the platform
service Ingresses stop requesting their own per-host ACME certificates.

The flag is set only on the publishing controller (Release.Namespace ==
expose-ingress) because ingress-nginx loads the Secret from its own
namespace; child-tenant controllers cannot reach a Secret in another
namespace, so they keep the existing ACME path.

Each service Ingress (dashboard, keycloak, grafana, alerta, bucket,
linstor-gui, harbor, seaweedfs S3, bootbox) drops its cert-manager
cluster-issuer annotation, the http01 challenge-class annotation, and
its tls block in wildcard mode; nginx then falls back to the default
certificate for those hosts. Other annotations and routing are
unchanged. Each file keeps its existing render guard intact, and the
annotations block collapses cleanly where the cert annotations were its
only content.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-06-10 03:08:41 +03:00