Skip to content

Commit 467a8ed

Browse files
authored
docs(adr): record declarative Keycloak config decision (ADR-0009) + fix numbering (#483)
Record the accepted decision behind #154 (design) and #289 (Phase 1a implementation): replace the imperative realm-setup shell script with a declarative keycloak-config-cli pipeline, with realm input in an in-cluster Secret rather than the GitOps repo. Status is Accepted because the design was accepted before it ever reached an ADR. Also resolve the duplicate ADR-0005. The OpenTelemetry collector override ADR is renumbered 0005 -> 0008 (nic config CLI surface keeps 0005, since it is the ADR-0005 referenced by #434 and #479); its code reference in opentelemetry-collector.yaml is updated, and the README index is rebuilt to include the previously-missing ADR-0003 alongside the renumbered and new entries.
1 parent e406655 commit 467a8ed

4 files changed

Lines changed: 141 additions & 2 deletions

File tree

docs/adr/0005-otel-collector-software-pack-override-point.md renamed to docs/adr/0008-otel-collector-software-pack-override-point.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# ADR-0005: OpenTelemetry Collector Software Pack Override Point
1+
# ADR-0008: OpenTelemetry Collector Software Pack Override Point
22

33
## Status
44

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
# ADR-0009: Declarative Keycloak Configuration via keycloak-config-cli
2+
3+
## Status
4+
5+
Accepted
6+
7+
This ADR records a decision that was made and accepted before it reached an ADR. The declarative approach was proposed and accepted as a design in [PR #154](https://github.com/nebari-dev/nebari-infrastructure-core/pull/154) (closed without merge after the design was agreed, per the rewrite on the `design/declarative-keycloak-config` branch), and its Phase 1a implementation is [PR #289](https://github.com/nebari-dev/nebari-infrastructure-core/pull/289). Neither PR produced an ADR, so this document captures the decision so it is not lost.
8+
9+
## Date
10+
11+
2026-07-15
12+
13+
## Context
14+
15+
NIC bootstraps a `nebari` Keycloak realm during foundational-software install: realm settings, `admin`/`user` realm roles, an admin user, a `groups` client scope plus an `oidc-group-membership-mapper` (load-bearing, because operator-managed app authorization keys off group membership via `NebariApplication.allowedGroups`), the `argocd` OIDC client, and the `argocd-admins`/`argocd-viewers` groups.
16+
17+
Today this is done by an imperative shell script in `realm-setup-job.yaml`: chains of `kcadm` calls, each suffixed with `|| true` to fake idempotency. That shape has three problems:
18+
19+
- `|| true` masks real failures. A genuine error looks identical to a benign "already exists," so the Job reports success while leaving the realm half-configured.
20+
- There is no drift detection. The script pushes state forward but never reconciles against what the realm actually contains.
21+
- Extending the configuration means appending more brittle shell, and the original implementation rendered realm content (with `$(env:VAR)` placeholders) into a ConfigMap committed to the GitOps repo. That treats the GitOps repo as a place where realm content, and by extension secrets, can land.
22+
23+
[PR #154](https://github.com/nebari-dev/nebari-infrastructure-core/pull/154) proposed replacing the shell script with a declarative pipeline. The design was accepted; [PR #289](https://github.com/nebari-dev/nebari-infrastructure-core/pull/289) implements Phase 1a (defaults only). This ADR records the architectural decision behind that work.
24+
25+
## Decision Drivers
26+
27+
- Make realm configuration declarative and idempotent, with real failures surfaced instead of swallowed by `|| true`.
28+
- Keep the GitOps repo free of realm content and secrets. The repo is treated as untrusted.
29+
- Use one tool that covers realm settings, clients, groups, scopes, and identity providers, so there is no separate code path for "SSO setup" versus "realm setup."
30+
- Coexist safely with operator-managed OAuth2 clients that NebariApplication CRDs create at runtime. The bootstrap tool must never delete resources it did not create.
31+
- Keep the toolchain pinnable and reproducible: the reconciler and Keycloak versions move together.
32+
33+
## Considered Options
34+
35+
1. **Keep the imperative `kcadm` shell script.**
36+
2. **keycloak-config-cli (kcc) with realm input in a GitOps-repo ConfigMap**, using `$(env:VAR)` placeholders for secret values.
37+
3. **keycloak-config-cli (kcc) with realm input in an in-cluster Secret** created by NIC at bootstrap from embedded defaults.
38+
39+
## Decision Outcome
40+
41+
Chosen option: **Option 3 (kcc with an in-cluster Secret)**.
42+
43+
[keycloak-config-cli](https://github.com/adorsys/keycloak-config-cli) reads a declarative YAML realm definition and reconciles it against Keycloak through the Admin API. NIC renders embedded defaults (`{{ .Domain }}` substituted at deploy time, used in the argocd client redirect URI) into a Secret named `keycloak-config-import` in the `keycloak` namespace, created immediately after the other Keycloak secret creators during foundational bootstrap. The GitOps repo holds only the Job manifest; realm content never leaves the cluster.
44+
45+
The Phase 1a implementation in #289 introduces `pkg/argocd/keycloak_defaults.yaml` (realm input in kcc's native format), `pkg/argocd/keycloak_defaults.go` (an embed plus a render helper), the `createKeycloakImportSecret` step in `pkg/argocd/foundational.go`, and a rewritten `realm-setup-job.yaml` that runs the kcc image and mounts the Secret. The previous gitops `realm-config-cm.yaml` ConfigMap is deleted.
46+
47+
This ADR commits to:
48+
49+
- **kcc as the reconciler** for the bootstrap Keycloak realm, replacing the imperative shell script.
50+
- **An in-cluster Secret, not a GitOps-repo ConfigMap, as the carrier** for realm input. The GitOps repo is untrusted; a "the file only ever contains placeholders, never literals" rule is a footgun, because a single operator mistake leaks a secret into git history permanently. The Secret carrier also accepts a future user-supplied `keycloak.yaml` (Phase 1b) without restructuring.
51+
- **Three-layer protection for operator coexistence**, so operator-managed clients are never touched by kcc:
52+
1. The kcc input declares only the `argocd` client. Operator-managed clients are not declared.
53+
2. `IMPORT_REMOTESTATE_ENABLED=true` (kcc default on v6.x): kcc only manages resources it itself created, tracked in a realm attribute.
54+
3. `IMPORT_MANAGED_CLIENT=no-delete`: belt-and-braces, so even if the input shape changes, kcc still will not delete unlisted clients.
55+
- **Coupled version pinning**: the Keycloak chart image and the kcc image are pinned together (Keycloak `26.5.4` and kcc `6.5.0-26.5.4`), because kcc image tags are tighter than the upstream CI matrix advertises.
56+
57+
This ADR explicitly does NOT commit to:
58+
59+
- The user-supplied `keycloak.yaml` schema (Phase 1b) or the user-secrets pipeline (Phase 1c). Those follow in separate PRs.
60+
- A NIC-side merge engine. kcc owns reconciliation via remote-state; the #154 design rewrite rejected building a separate merge/prefix engine in NIC.
61+
- Backup and restore of realm state.
62+
63+
### Consequences
64+
65+
**Good:**
66+
67+
- Realm configuration is declarative and idempotent, and real failures surface instead of being hidden by `|| true`.
68+
- No realm content or secrets live in the GitOps repo. The repo holds only the Job manifest.
69+
- One tool spans realm settings, clients, groups, scopes, and identity providers. Declarative SSO (Google, GitHub, generic OIDC/SAML) lands as the same shape in Phase 1b/1c, with no separate "SSO setup" path.
70+
- Migration from the shell setup is effectively a no-op for already-correct values: kcc detects existing realm, users, groups, scopes, and clients by natural key and adopts them into remote state. The pre-generated argocd OIDC client secret is byte-identical across the keycloak-namespace Secret, the argocd Secret, and Keycloak's stored value, so OIDC trust survives the cutover.
71+
- A unit test (`keycloak_defaults_test.go`) asserts each load-bearing field renders and that no unresolved Go-template markers leak into the output, catching regressions before they reach a cluster.
72+
73+
**Bad:**
74+
75+
- **kcc applies list fields as full-replace, not merge.** `defaultDefaultClientScopes: [groups]` replaces the entire list, so every built-in default scope must be listed alongside `groups`, both at realm level and on the argocd client. The admin user's `groups` field is full-replace the same way: memberships added manually via the Keycloak UI are reverted on the next `nic deploy`.
76+
- **A `HookSucceeded`-only delete policy traps stuck Jobs.** A kcc Job that fails never succeeds, so its cleanup never fires, and ArgoCD's hook finalizer keeps the immutable Job around indefinitely. `BeforeHookCreation` is added to the policy so each fresh sync first deletes any existing Job.
77+
- **kcc runs string substitution over the whole file before parsing it as YAML,** comments included. A comment that literally contains the `$(env:VAR)` shape trips the substitutor. Comments must describe the syntax without writing the literal pattern.
78+
- kcc is a new foundational tool dependency, and the Keycloak-to-kcc version coupling has to be maintained on upgrades.
79+
- Force-deleting a stuck Job (by stripping its finalizer) can leave the ArgoCD application in a stuck operation state. This only surfaces when iterating on the same cluster, not on normal first deploys; the unstick pattern is `kubectl patch app <name> --type=merge -p '{"operation":null,"status":{"operationState":null}}'` followed by a fresh sync.
80+
81+
## Options Detail
82+
83+
### Option 1: Imperative kcadm shell script (status quo)
84+
85+
The existing `realm-setup-job.yaml` runs `kcadm` commands sequentially, each ending in `|| true` so a re-run does not fail on "already exists."
86+
87+
**Pros:**
88+
89+
- No new tool dependency; it works today.
90+
- Fully visible in the Job manifest.
91+
92+
**Cons:**
93+
94+
- `|| true` masks genuine failures as benign, so a half-configured realm reports success.
95+
- No drift detection or reconciliation; the script only pushes forward.
96+
- Brittle and increasingly hard to extend as realm configuration grows.
97+
- The variant that rendered realm content into a GitOps-repo ConfigMap puts realm content, and the risk of secrets, into the repo.
98+
99+
### Option 2: kcc with a GitOps-repo ConfigMap
100+
101+
kcc reconciles declaratively, but its realm input is a ConfigMap rendered into the GitOps repo with `$(env:VAR)` placeholders for secret values.
102+
103+
**Pros:**
104+
105+
- Declarative and idempotent, same reconciliation benefits as Option 3.
106+
- Realm input is visible in the GitOps repo.
107+
108+
**Cons:**
109+
110+
- Realm content lives in the GitOps repo, which is treated as untrusted.
111+
- Depends on a "placeholders only, never literals" rule that is a footgun: one mistake leaks a secret into git history permanently.
112+
- No clean carrier for a future user-supplied `keycloak.yaml` without repeating the same repo-exposure problem.
113+
114+
### Option 3: kcc with an in-cluster Secret (chosen)
115+
116+
kcc reconciles declaratively, and its realm input is a Secret (`keycloak-config-import`) that NIC creates in-cluster at bootstrap from embedded, domain-substituted defaults. The GitOps repo holds only the Job manifest.
117+
118+
**Pros:**
119+
120+
- Declarative and idempotent, with the GitOps repo kept free of realm content and secrets.
121+
- The Secret carrier accepts a user-supplied `keycloak.yaml` in Phase 1b without restructuring.
122+
- Operator coexistence handled by the three-layer protection above.
123+
124+
**Cons:**
125+
126+
- Realm input is not directly visible in the GitOps repo; it is a cluster Secret rendered by NIC.
127+
- Requires a NIC bootstrap step to render and create the Secret before the Job runs.
128+
129+
## Links
130+
131+
- [PR #154](https://github.com/nebari-dev/nebari-infrastructure-core/pull/154) - accepted design: declarative Keycloak configuration.
132+
- [PR #289](https://github.com/nebari-dev/nebari-infrastructure-core/pull/289) - Phase 1a implementation (shell-to-kcc swap, defaults only).
133+
- [PR #234](https://github.com/nebari-dev/nebari-infrastructure-core/pull/234) - ArgoCD OIDC SSO, whose realm-setup state is preserved through the migration.
134+
- [Issue #288](https://github.com/nebari-dev/nebari-infrastructure-core/issues/288) - user-facing docs follow-up (full-replace semantics for user groups and identity providers).
135+
- [ADR-0007](0007-cloudnativepg-managed-databases.md) - CloudNativePG, which references the Keycloak-to-kcc version-pinning pattern established here.
136+
- [keycloak-config-cli](https://github.com/adorsys/keycloak-config-cli) - the declarative reconciler.

docs/adr/README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,13 @@ An ADR is a document that captures an important architectural decision made alon
1212
|----|-------|--------|------|
1313
| [ADR-0001](0001-git-provider-for-gitops-bootstrap.md) | Git Provider for GitOps Bootstrap | Proposed | 2025-01-21 |
1414
| [ADR-0002](0002-longhorn-distributed-block-storage-for-aws.md) | Longhorn Distributed Block Storage for AWS | Proposed | 2026-02-13 |
15+
| [ADR-0003](0003-software-pack-codegen.md) | Software Pack Codegen via ArgoCD Application Generation | Proposed | 2026-03-12 |
1516
| [ADR-0004](0004-out-of-tree-provider-plugins.md) | Out-of-Tree Provider Plugin Architecture | Proposed | 2026-04-15 |
1617
| [ADR-0005](0005-nic-config-cli-surface.md) | nic config CLI surface | Proposed | 2026-06-03 |
1718
| [ADR-0006](0006-conditional-foundational-software-helm.md) | Conditional Foundational Software via Provider-Driven Helm Installs | Proposed | 2026-06-03 |
1819
| [ADR-0007](0007-cloudnativepg-managed-databases.md) | CloudNativePG as Foundational Database Infrastructure | Proposed | 2026-05-12 |
20+
| [ADR-0008](0008-otel-collector-software-pack-override-point.md) | OpenTelemetry Collector Software Pack Override Point | Accepted | 2026-06-02 |
21+
| [ADR-0009](0009-declarative-keycloak-configuration.md) | Declarative Keycloak Configuration via keycloak-config-cli | Accepted | 2026-07-15 |
1922

2023
## ADR Statuses
2124

pkg/argocd/templates/apps/opentelemetry-collector.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ spec:
190190
servicePort: 8888
191191
protocol: TCP
192192
193-
# Software-pack extension pointsee ADR-0005. A Nebari software
193+
# Software-pack extension point: see ADR-0008. A Nebari software
194194
# pack (e.g. nebari-lgtm-pack) creates a ConfigMap named
195195
# `opentelemetry-collector-overrides` in the `monitoring` namespace
196196
# containing a `relay.yaml` key — a partial OTel config that's

0 commit comments

Comments
 (0)