Skip to content

Commit f43665a

Browse files
committed
docs(services/keycloak,modules/lib): simpler comment wording
Trim and simplify the inline comments throughout the keycloak pairing and modules/lib/default.nix. Plain words over jargon: "wait for" instead of "gate on", "reuse a client" instead of "tolerate ... from a partial earlier bootstrap", "shared" instead of "provider-agnostic", drop the multi-paragraph file-header preamble in modules/lib, shorten every option description that ran to two or more sentences. Includes one small correctness fix: ldap_user_federations.blockAttrs now lists `kerberos` and `cache` so the nested sub-blocks emit as `[{ ... }]` -- previously the option type accepted them but the rendered JSON would have used the plain-object form. No fixture exercises this today, so the existing tests are byte-identical.
1 parent 78e4eba commit f43665a

4 files changed

Lines changed: 190 additions & 220 deletions

File tree

modules/lib/default.nix

Lines changed: 43 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,42 @@
1-
# Shared, provider-agnostic helpers for the declarative-service pairings: the
2-
# .tf.json label/file helpers and the run-once OpenTofu reconciler unit.
3-
#
4-
# Provider-specific pieces — the provider-wrapped executor, the .tf.json
5-
# provider/resource generation, the token variable name — live in each pairing's
6-
# services/<svc>/lib.nix and are injected into the helpers below.
1+
# shared helpers for the pairings: tf-label/file helpers and the run-once
2+
# reconciler unit. provider-specific bits live in services/<svc>/lib.nix.
73
{ pkgs }:
84
let
95
inherit (pkgs) lib;
106
in
117
rec {
12-
# Sanitize an arbitrary string into a valid Terraform block label
13-
# ([A-Za-z_][A-Za-z0-9_-]*). Always prefixed, so the result starts with a
14-
# letter regardless of the input.
8+
# turn an arbitrary string into a valid Terraform block label. always
9+
# prefixed so the result starts with a letter.
1510
tfLabel =
1611
prefix: name:
1712
"${prefix}_"
1813
+ lib.stringAsChars (c: if builtins.match "[A-Za-z0-9_-]" c != null then c else "_") name;
1914

20-
# Render a config attrset to a .tf.json store file. Safe by construction: the
21-
# config must carry no secrets (anything in the store is world-readable).
15+
# write the config as a .tf.json file in the nix store. must contain no
16+
# secrets -- the store is world-readable.
2217
tfJsonFile = name: config: pkgs.writeText "${name}.tf.json" (builtins.toJSON config);
2318

24-
# Build the run-once reconciler systemd service definition.
19+
# build the run-once reconciler systemd service.
2520
#
26-
# name unit + generated-config name (e.g. "declarative-forgejo")
27-
# tfConfig the provider-specific config attrset to apply
28-
# afterUnits units to order/require after (the service's primary unit)
29-
# healthUrl URL polled until the service answers, before applying
30-
# tokenFile runtime path to the admin token, exposed via LoadCredential
31-
# executor OpenTofu wrapped with the pairing's provider (offline mirror)
32-
# tokenVar Terraform input variable carrying the token; also the
33-
# LoadCredential id, so `TF_VAR_<tokenVar>` is fed from it
34-
# credentials extra TF_VAR_<id> -> host file path pairs (per-resource
35-
# secrets); each is LoadCredential'd and exported like the token
36-
# user/group the base service's user/group the reconciler runs as
37-
# stateDir the base service's primary state dir; Terraform state lives in
38-
# a `declarative-terraform` subdir of it, co-located with the service
39-
# dynamicUser set when the base service runs as systemd DynamicUser (so
40-
# the `User=` name only exists per-unit). The reconciler then
41-
# runs with `DynamicUser=true` too, so systemd allocates the
42-
# same hashed UID as the primary unit, and the work dir is
43-
# created via `StateDirectory=` (derived from `stateDir`)
44-
# rather than `mkdir`. Requires `stateDir` to live under
45-
# `/var/lib`; enforced at eval time.
21+
# name unit + generated-config name (e.g. "declarative-forgejo")
22+
# tfConfig the .tf.json config to apply
23+
# afterUnits units to order/require after (the service's main unit)
24+
# healthUrl url polled until the service answers, before applying
25+
# tokenFile path to the admin token on the host, read via LoadCredential
26+
# executor OpenTofu wrapped with the pairing's provider (offline)
27+
# tokenVar name of the sensitive tf variable carrying the token; also
28+
# the LoadCredential id -- exported as TF_VAR_<tokenVar>
29+
# credentials extra TF_VAR_<id> -> host path pairs for per-resource
30+
# secrets, handled the same way as tokenFile
31+
# user/group the service's user/group; the reconciler runs as them
32+
# stateDir base dir for the reconciler; tfstate lives in a
33+
# `declarative-terraform` subdir of it
34+
# dynamicUser set when the base service uses systemd DynamicUser=true
35+
# (the `User=` name only exists per-unit). the reconciler
36+
# then runs with DynamicUser=true too, so it picks up the
37+
# same hashed UID as the main unit, and systemd creates
38+
# the work dir via StateDirectory= (derived from stateDir).
39+
# requires stateDir to live under /var/lib.
4640
mkReconcileService =
4741
{
4842
name,
@@ -60,19 +54,17 @@ rec {
6054
}:
6155
let
6256
confFile = tfJsonFile name tfConfig;
63-
# Admin token + any per-resource secret files, each kept out of the store
64-
# and exposed to tofu as TF_VAR_<id> via systemd LoadCredential=.
57+
# admin token + per-resource secrets, all read via LoadCredential
58+
# so they never land in the world-readable store.
6559
allCredentials = {
6660
${tokenVar} = tokenFile;
6761
}
6862
// credentials;
69-
# Terraform state is co-located with the base service: a subdir of its
70-
# primary state directory, created and owned by the service user.
63+
# tfstate lives in this subdir of stateDir.
7164
workDir = "${stateDir}/declarative-terraform";
72-
# Under DynamicUser= the absolute work dir is also expressed as a
73-
# relative `StateDirectory=` so systemd creates and owns it. Deriving
74-
# both from `stateDir` is the single source of truth: the path the
75-
# script `cd`s into and the path the unit declares can never drift.
65+
# under DynamicUser= systemd creates the dir via StateDirectory=
66+
# (relative to /var/lib). derive both from stateDir so the script
67+
# path and the unit declaration cannot drift.
7668
stateDirectoryRelative = lib.removePrefix "/var/lib/" workDir;
7769
in
7870
assert lib.assertMsg (!dynamicUser || lib.hasPrefix "/var/lib/" stateDir)
@@ -82,7 +74,7 @@ rec {
8274
after = afterUnits;
8375
requires = afterUnits;
8476
wantedBy = [ "multi-user.target" ];
85-
# Re-apply whenever the generated configuration changes.
77+
# re-apply when the generated config changes.
8678
restartTriggers = [ confFile ];
8779
path = [
8880
executor
@@ -96,17 +88,16 @@ rec {
9688
serviceConfig = {
9789
Type = "oneshot";
9890
RemainAfterExit = true;
99-
# Run as the base service's user so Terraform state can live in (and be
100-
# backed up alongside) that service's primary state directory.
91+
# run as the service's own user so tfstate sits next to its data.
10192
User = user;
10293
Group = group;
103-
# Secrets stay out of the store: read from the credentials dir at runtime.
94+
# secrets stay out of the store; loaded into $CREDENTIALS_DIRECTORY at runtime.
10495
LoadCredential = lib.mapAttrsToList (id: path: "${id}:${path}") allCredentials;
10596
}
10697
// lib.optionalAttrs dynamicUser {
107-
# Bases like Keycloak ship no persistent state dir and run as
108-
# systemd DynamicUser=true; the `User=` name is hashed to a stable UID
109-
# that's reused across units, and systemd creates/owns the state dir.
98+
# services like Keycloak use DynamicUser=true and have no
99+
# persistent state dir. systemd hashes the User= name to a
100+
# stable UID that's shared across units and owns the state dir.
110101
DynamicUser = true;
111102
StateDirectory = stateDirectoryRelative;
112103
StateDirectoryMode = "0700";
@@ -115,23 +106,23 @@ rec {
115106
set -euo pipefail
116107
umask 077
117108
118-
# Work in a Terraform state dir under the base service's primary state
119-
# directory, created 0700 on first run and owned by the service user.
109+
# work in a subdir of the service's state dir; created 0700 on
110+
# first run, owned by the service user.
120111
mkdir -p ${lib.escapeShellArg workDir}
121112
cd ${lib.escapeShellArg workDir}
122113
123-
# Refresh the generated config (state persists across runs).
114+
# refresh the generated config (tfstate persists across runs).
124115
install -m 0600 ${confFile} ./main.tf.json
125116
126-
# Gate on the service actually answering before applying.
117+
# wait for the service to actually answer before applying.
127118
for _ in $(seq 1 60); do
128119
if curl -fsS -o /dev/null "${healthUrl}"; then
129120
break
130121
fi
131122
sleep 2
132123
done
133124
134-
# Feed every credential to tofu as TF_VAR_<id>, read from the creds dir.
125+
# pass each credential to tofu as TF_VAR_<id>.
135126
for id in ${lib.escapeShellArgs (lib.attrNames allCredentials)}; do
136127
export "TF_VAR_$id=$(cat "$CREDENTIALS_DIRECTORY/$id")"
137128
done

services/keycloak/checks.nix

Lines changed: 30 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
1-
# Per-resource-family keycloak tests. The `keycloak` test is a full VM
2-
# (specialisations only work in QEMU nodes); the rest are nspawn containers
3-
# for faster boot and tighter focus.
1+
# keycloak tests, one per resource family. the `keycloak` test boots a
2+
# full VM (specialisations need QEMU). the rest run as nspawn containers.
43
{ pkgs, self }:
54
let
65
inherit (pkgs) lib;
76
keycloakAdminPassword = "hackme";
87

9-
# Python helpers; each takes the machine reference (`machine` for VMs,
10-
# `keycloak` for containers) so the body is identical across both shapes.
8+
# python helpers; each takes the machine reference (`machine` in the VM
9+
# test, `keycloak` in the container tests) so bodies match.
1110
pyHelpers = ''
1211
import json
1312
def admin_token(m):
@@ -28,8 +27,8 @@ let
2827
return admin_get(m, realm)
2928
'';
3029

31-
# Common keycloak service config every test reuses. `runtime` carries
32-
# the per-test resource fixture; `extraEtc` mocks operator secret files.
30+
# shared keycloak host config. `runtime` is the per-test fixture;
31+
# `extraEtc` mocks operator-supplied secret files.
3332
mkHost =
3433
{
3534
runtime,
@@ -58,7 +57,7 @@ let
5857
settings = {
5958
hostname = "keycloak";
6059
http-port = 8080;
61-
http-enabled = true; # HTTP-only test deployment
60+
http-enabled = true; # http-only test deployment
6261
hostname-strict = false;
6362
};
6463

@@ -72,8 +71,8 @@ let
7271
};
7372
in
7473
{
75-
# Core test: full VM proving the boot -> bootstrap -> reconcile chain
76-
# plus config-change reconciliation via a specialisation.
74+
# core test: full vm covering boot -> bootstrap -> reconcile, plus a
75+
# specialisation switch that adds a second realm.
7776
keycloak = pkgs.testers.runNixOSTest {
7877
name = "declarative-keycloak";
7978

@@ -87,7 +86,7 @@ in
8786
};
8887
} args)
8988
{
90-
# keycloak is thicc -- only VMs accept memorySize.
89+
# keycloak is thicc; only VMs accept memorySize.
9190
virtualisation.memorySize = 3072;
9291
specialisation.addRealm.configuration.services.keycloak.runtime.realms.delta = {
9392
display_name = "Delta Realm";
@@ -98,7 +97,7 @@ in
9897
${pyHelpers}
9998
machine.start()
10099
101-
# whole chain (keycloak -> bootstrap -> reconciler) must converge.
100+
# wait for the chain: keycloak -> bootstrap -> reconciler.
102101
machine.wait_for_unit("declarative-keycloak.service")
103102
104103
with subtest("declared realm exists with display_name applied"):
@@ -138,7 +137,7 @@ in
138137
'';
139138
};
140139

141-
# RBAC: roles, groups, users + bindings via managed-key list refs.
140+
# roles, groups, users + bindings via managed-key list refs.
142141
keycloak-rbac = pkgs.testers.runNixOSTest {
143142
name = "declarative-keycloak-rbac";
144143

@@ -243,7 +242,7 @@ in
243242
'';
244243
};
245244

246-
# OpenID clients + scopes + a protocol mapper + default-scope binding.
245+
# openid clients + scopes + protocol mapper + default-scope binding.
247246
keycloak-clients = pkgs.testers.runNixOSTest {
248247
name = "declarative-keycloak-clients";
249248

@@ -284,7 +283,7 @@ in
284283
];
285284
};
286285

287-
# Protocol mapper attached to the managed scope via its key.
286+
# protocol mapper attached to the managed scope by key.
288287
openid_user_attribute_protocol_mappers.team_claim = {
289288
realm = "acme";
290289
client_scope = "acme_profile";
@@ -322,7 +321,7 @@ in
322321
f"acme-profile not bound as default scope: {names}"
323322
324323
with subtest("protocol mapper attached to client scope"):
325-
# protocolMappers travel with the client-scope representation.
324+
# protocolMappers ride along on the client-scope representation.
326325
scopes = admin_get(keycloak, "acme/client-scopes")
327326
s = next((x for x in scopes if x["name"] == "acme-profile"), None)
328327
assert s, f"acme-profile scope missing: {[x['name'] for x in scopes]}"
@@ -339,9 +338,9 @@ in
339338
'';
340339
};
341340

342-
# Realm extras: extended realm attrs, nested-secret smtp, security
343-
# defenses (nested-in-nested), otp_policy, realm_user_profile (nested
344-
# in list elements), a keystore, required_action, localization.
341+
# realm extras: extended realm attrs, smtp with nested-secret,
342+
# security_defenses (nested-in-nested), otp_policy, realm_user_profile
343+
# (nested-in-list), a keystore, required_action, localization.
345344
keycloak-realm-extras = pkgs.testers.runNixOSTest {
346345
name = "declarative-keycloak-realm-extras";
347346

@@ -351,7 +350,7 @@ in
351350
realms.acme = {
352351
display_name = "ACME Corp.";
353352
display_name_html = "<b>ACME</b> Corp.";
354-
# extended attrs
353+
# cross-section of the extended realm attrs.
355354
registration_allowed = true;
356355
login_theme = "keycloak";
357356
ssl_required = "external";
@@ -365,7 +364,7 @@ in
365364
];
366365
default_locale = "en";
367366
};
368-
# smtp_server with nested-secret indirection.
367+
# smtp with a nested-secret indirection (auth.passwordFile).
369368
smtp_server = {
370369
host = "smtp.example.com";
371370
from = "noreply@example.com";
@@ -376,8 +375,8 @@ in
376375
passwordFile = "/etc/acme-smtp-password";
377376
};
378377
};
379-
# nested-in-nested block-list wrap (security_defenses.headers,
380-
# security_defenses.brute_force_detection).
378+
# nested-in-nested block wrap (headers + brute_force_detection
379+
# inside security_defenses).
381380
security_defenses = {
382381
headers = {
383382
x_frame_options = "DENY";
@@ -419,9 +418,9 @@ in
419418
texts.loginAccountTitle = "ACME";
420419
};
421420

422-
# realm_user_profile exercises a MaxItems:1 nested block inside a
423-
# list element (attribute[].permissions). Keycloak refuses to drop
424-
# the built-in attrs; declare them alongside the custom one.
421+
# realm_user_profile exercises a nested MaxItems:1 block inside a
422+
# list element (attribute[].permissions). keycloak refuses to drop
423+
# the built-in attrs, so declare them alongside the custom one.
425424
realm_user_profiles.acme = {
426425
realm = "acme";
427426
unmanaged_attribute_policy = "ENABLED";
@@ -582,7 +581,7 @@ in
582581
'';
583582
};
584583

585-
# Identity providers + IdP mappers + an authentication flow.
584+
# identity providers + IdP mappers + an authentication flow.
586585
keycloak-idp = pkgs.testers.runNixOSTest {
587586
name = "declarative-keycloak-idp";
588587

@@ -591,14 +590,14 @@ in
591590
runtime = {
592591
realms.acme.display_name = "ACME";
593592

594-
# google IdP exercises realmAliasRef and secret-file indirection.
593+
# google IdP exercises realm-alias resolution + secret-file indirection.
595594
oidc_google_identity_providers.acme_google = {
596595
realm = "acme";
597596
client_id = "fake-client-id";
598597
client_secretFile = "/etc/acme-google-secret";
599598
};
600599

601-
# IdP mapper exercises idpAliasRequiredRef across IdP collections.
600+
# IdP mapper exercises the multi-target idp-alias ref.
602601
attribute_importer_identity_provider_mappers.google_email = {
603602
realm = "acme";
604603
identity_provider = "acme_google";
@@ -621,8 +620,8 @@ in
621620
keycloak.wait_for_unit("declarative-keycloak.service")
622621
623622
with subtest("google IdP exists, client_secret kept out of .tf.json"):
624-
# alias defaults to the collection key (`acme_google`) via nameAttr;
625-
# the provider sets providerId="google".
623+
# alias defaults to the collection key (`acme_google`);
624+
# providerId is fixed to "google" by the resource type.
626625
idp = admin_get(keycloak, "acme/identity-provider/instances/acme_google")
627626
assert idp.get("providerId") == "google", f"idp: {idp}"
628627
assert idp.get("alias") == "acme_google"

0 commit comments

Comments
 (0)