Skip to content

Commit 4e29fed

Browse files
IvanHuntersIvanHunters
committed
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 <[email protected]>
1 parent 12bfa77 commit 4e29fed

5 files changed

Lines changed: 141 additions & 33 deletions

File tree

‎docs/oidc-grafana.md‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,12 @@ work — useful when Keycloak is down or misconfigured.
185185
render hard-fails:
186186
`spec.oidc.mode: System requires the platform-level OIDC feature`.
187187
Flip `authentication.oidc.enabled: true` in the platform values, or
188-
switch to `CustomConfig`.
188+
switch to `CustomConfig`. On the platform side that flag also
189+
swings the `cozystack.monitoring-application` PackageSource from
190+
variant `default` (baseline dependsOn only) to variant `oidc`
191+
(additionally waits for `cozystack.keycloak-operator` so the
192+
`v1.edp.epam.com` CRDs consumed by the Keycloak-side of System
193+
mode are registered before the monitoring chart reconciles).
189194
- **`CustomConfig` with an unreachable issuer or wrong claim
190195
mappings** — Grafana rejects the callback and the login screen
191196
shows an error; the `admin_user`/`admin_password` Secret keeps

‎packages/core/platform/sources/monitoring-application.yaml‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,40 @@ spec:
1818
- cozystack.victoria-metrics-operator
1919
# cozystack-engine ships the ApplicationDefinition CRD rendered by the *-rd component below.
2020
- cozystack.cozystack-engine
21-
# keycloak-operator ships the v1.edp.epam.com CRDs consumed by
22-
# the OIDC selector (spec.oidc.mode: System renders KeycloakClient,
23-
# KeycloakClientScope and three KeycloakRealmGroups in the cozy
24-
# realm). Without this dep the monitoring chart can reconcile
25-
# before the CRDs are registered on cold install — the
26-
# .Capabilities.APIVersions.Has guard then silently drops all
27-
# Keycloak-side objects and Grafana's auth.generic_oauth block
28-
# points at a client that never gets provisioned.
21+
libraries:
22+
- name: cozy-lib
23+
path: library/cozy-lib
24+
components:
25+
- name: monitoring-system
26+
path: system/monitoring
27+
libraries: ["cozy-lib"]
28+
- name: monitoring
29+
path: extra/monitoring
30+
libraries: ["cozy-lib"]
31+
- name: monitoring-rd
32+
path: system/monitoring-rd
33+
install:
34+
namespace: cozy-system
35+
releaseName: monitoring-rd
36+
# `oidc` variant — additionally waits for cozystack.keycloak-operator so
37+
# the v1.edp.epam.com CRDs consumed by the Phase-1 OIDC selector
38+
# (spec.oidc.mode: System renders KeycloakClient, KeycloakClientScope and
39+
# three KeycloakRealmGroups in the cozy realm) are registered before the
40+
# monitoring chart reconciles. Without this edge, the
41+
# `.Capabilities.APIVersions.Has "v1.edp.epam.com/v1"` guard in
42+
# templates/grafana/oidc-keycloak.yaml would silently drop the whole
43+
# Keycloak side and Grafana's auth.generic_oauth block would point at
44+
# a client that never gets provisioned. Selected from the bundle when
45+
# `authentication.oidc.enabled` is true — see
46+
# packages/core/platform/templates/bundles/system.yaml. Mirrors the
47+
# cozystack-engine default/oidc variant split.
48+
- name: oidc
49+
dependsOn:
50+
- cozystack.networking
51+
- cozystack.postgres-operator
52+
- cozystack.grafana-operator
53+
- cozystack.victoria-metrics-operator
54+
- cozystack.cozystack-engine
2955
- cozystack.keycloak-operator
3056
libraries:
3157
- name: cozy-lib

‎packages/core/platform/templates/bundles/system.yaml‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,19 @@
196196
so `<clientId>-admin` group members become server-level Grafana
197197
admins. Tenant Grafana releases (component `monitoring`) inherit
198198
the chart default `oidc.grafanaAdmin: false` and stay at
199-
org-level Admin. See docs/oidc-grafana.md and cozystack#3176. */}}
199+
org-level Admin. See docs/oidc-grafana.md and cozystack#3176.
200+
201+
Variant selection mirrors cozystack-engine above: `oidc` when
202+
platform-level OIDC is on (extra dependsOn on
203+
cozystack.keycloak-operator so the v1.edp.epam.com CRDs are
204+
registered before the monitoring chart reconciles), `default`
205+
otherwise (no wait on a Package that would not be emitted). */}}
200206
{{- $monitoringApplicationComponents := dict "monitoring-system" (dict "values" (dict "oidc" (dict "grafanaAdmin" true))) -}}
207+
{{- if .Values.authentication.oidc.enabled }}
208+
{{include "cozystack.platform.package" (list "cozystack.monitoring-application" "oidc" $ $monitoringApplicationComponents) }}
209+
{{- else }}
201210
{{include "cozystack.platform.package" (list "cozystack.monitoring-application" "default" $ $monitoringApplicationComponents) }}
211+
{{- end }}
202212
{{include "cozystack.platform.package.default" (list "cozystack.etcd-application" $) }}
203213
{{include "cozystack.platform.package.default" (list "cozystack.cozystack-basics" $) }}
204214
{{include "cozystack.platform.package.default" (list "cozystack.backupstrategy-controller" $) }}

‎packages/core/platform/tests/bundles_monitoring_grafana_admin_test.yaml‎

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,32 @@
1-
suite: bundles.system monitoring-application component wiring (Grafana OIDC uplift)
1+
suite: bundles.system monitoring-application variant + component wiring
2+
# Two invariants exercised together, because they share the same Package
3+
# emission site:
4+
# 1. `authentication.oidc.enabled` gates the PackageSource variant —
5+
# `oidc` when on (extra dep on cozystack.keycloak-operator so the
6+
# v1.edp.epam.com CRDs are registered before the monitoring chart
7+
# reconciles), `default` when off (no wait on a Package that would
8+
# not be emitted).
9+
# 2. Both variants MUST carry the GrafanaAdmin uplift for the platform
10+
# Grafana release (`monitoring-system`) via
11+
# Package.spec.components["monitoring-system"].values.oidc.grafanaAdmin.
12+
# Tenant Grafana (component `monitoring`) inherits the chart default
13+
# `oidc.grafanaAdmin: false` and stays at org-level Admin — regression
14+
# guard the absence of a `monitoring` override here.
215
templates:
316
- templates/bundles/system.yaml
417
release:
518
name: cozystack
619
namespace: cozy-system
720
tests:
8-
- it: monitoring-application Package emits the GrafanaAdmin uplift for the platform Grafana release
21+
- it: authentication.oidc.enabled=false selects default variant and keeps the platform GrafanaAdmin uplift
922
set:
1023
bundles:
1124
system:
1225
enabled: true
1326
variant: isp-full
27+
authentication:
28+
oidc:
29+
enabled: false
1430
documentSelector:
1531
path: metadata.name
1632
value: cozystack.monitoring-application
@@ -21,11 +37,31 @@ tests:
2137
- equal:
2238
path: spec.variant
2339
value: default
24-
# The platform Grafana release (component `monitoring-system`) is
25-
# promoted to server-level GrafanaAdmin via the explicit values
26-
# override. Tenant Grafana (component `monitoring`) is NOT listed
27-
# in .spec.components — inherits the chart default
28-
# oidc.grafanaAdmin: false and stays at org-level Admin.
40+
- equal:
41+
path: spec.components.monitoring-system.values.oidc.grafanaAdmin
42+
value: true
43+
- notExists:
44+
path: spec.components.monitoring.values.oidc.grafanaAdmin
45+
46+
- it: authentication.oidc.enabled=true selects oidc variant and keeps the platform GrafanaAdmin uplift
47+
set:
48+
bundles:
49+
system:
50+
enabled: true
51+
variant: isp-full
52+
authentication:
53+
oidc:
54+
enabled: true
55+
documentSelector:
56+
path: metadata.name
57+
value: cozystack.monitoring-application
58+
asserts:
59+
- equal:
60+
path: kind
61+
value: Package
62+
- equal:
63+
path: spec.variant
64+
value: oidc
2965
- equal:
3066
path: spec.components.monitoring-system.values.oidc.grafanaAdmin
3167
value: true

‎packages/core/platform/tests/sources_monitoring_application_dependson_test.yaml‎

Lines changed: 47 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,70 @@
1-
suite: monitoring-application waits for the Keycloak operator that owns the OIDC CRDs
1+
suite: monitoring-application variant ordering edges (default + oidc)
22
# Regression guard for a fresh-install ordering race introduced by the Phase-1
3-
# OIDC selector (cozystack/cozystack#3176). Without the keycloak-operator edge,
4-
# the monitoring chart can reconcile before the v1.edp.epam.com CRDs are
5-
# registered on cold install — the `.Capabilities.APIVersions.Has` guard in
6-
# templates/grafana/oidc-keycloak.yaml silently drops the whole Keycloak side
7-
# of System mode (KeycloakClient / KeycloakClientScope / KeycloakRealmGroups),
8-
# Grafana boots with an auth.generic_oauth block pointing at a client that was
9-
# never provisioned, and OIDC login fails at request time with no build-time
10-
# signal.
3+
# OIDC selector (cozystack/cozystack#3176). The `oidc` variant additionally
4+
# depends on cozystack.keycloak-operator so the v1.edp.epam.com CRDs are
5+
# registered before the monitoring chart reconciles — without this edge the
6+
# `.Capabilities.APIVersions.Has` guard in
7+
# templates/grafana/oidc-keycloak.yaml would silently drop the whole
8+
# Keycloak side of mode: System and Grafana's auth.generic_oauth block would
9+
# point at a client that never gets provisioned. The `default` variant MUST
10+
# NOT depend on keycloak-operator — that Package is only emitted when
11+
# authentication.oidc.enabled is true, so a hard dep on the default variant
12+
# would leave every OIDC-off cluster stuck on DependenciesNotReady forever.
1113
templates:
1214
- templates/sources.yaml
1315
release:
1416
name: cozystack
1517
namespace: cozy-system
1618
tests:
17-
- it: monitoring-application dependsOn keycloak-operator (OIDC CRDs)
19+
- it: default variant carries the baseline reconciliation edges without keycloak-operator
1820
documentSelector:
1921
path: metadata.name
2022
value: cozystack.monitoring-application
2123
asserts:
2224
- contains:
2325
path: spec.variants[0].dependsOn
24-
content: cozystack.keycloak-operator
25-
# The other deps we rely on transitively for the Grafana instance to
26-
# even come up on a fresh cluster; regression-guard them together so
27-
# a future edit does not accidentally drop the ordering edges.
26+
content: cozystack.networking
2827
- contains:
2928
path: spec.variants[0].dependsOn
30-
content: cozystack.networking
29+
content: cozystack.postgres-operator
3130
- contains:
3231
path: spec.variants[0].dependsOn
3332
content: cozystack.grafana-operator
3433
- contains:
3534
path: spec.variants[0].dependsOn
36-
content: cozystack.postgres-operator
35+
content: cozystack.victoria-metrics-operator
3736
- contains:
3837
path: spec.variants[0].dependsOn
38+
content: cozystack.cozystack-engine
39+
- notContains:
40+
path: spec.variants[0].dependsOn
41+
content: cozystack.keycloak-operator
42+
43+
- it: oidc variant additionally waits for keycloak-operator (OIDC CRDs)
44+
documentSelector:
45+
path: metadata.name
46+
value: cozystack.monitoring-application
47+
asserts:
48+
- equal:
49+
path: spec.variants[1].name
50+
value: oidc
51+
- contains:
52+
path: spec.variants[1].dependsOn
53+
content: cozystack.keycloak-operator
54+
# oidc variant inherits the baseline edges (regression guard against
55+
# someone dropping one when refactoring the variant).
56+
- contains:
57+
path: spec.variants[1].dependsOn
58+
content: cozystack.networking
59+
- contains:
60+
path: spec.variants[1].dependsOn
61+
content: cozystack.grafana-operator
62+
- contains:
63+
path: spec.variants[1].dependsOn
64+
content: cozystack.postgres-operator
65+
- contains:
66+
path: spec.variants[1].dependsOn
3967
content: cozystack.victoria-metrics-operator
68+
- contains:
69+
path: spec.variants[1].dependsOn
70+
content: cozystack.cozystack-engine

0 commit comments

Comments
 (0)