fix(kanidm): detect real changes, and make a failed build retry instead of hiding #266

Sammanfogat

supernaut sammanfogade 1 incheckning från fix/kanidm-provision-change-signal in i main

2026-07-30 20:58:47 +00:00

supernaut kommenterad

2026-07-30 20:40:59 +00:00

Ägare

Three fixes in the kanidm-provision path, all found by researching upstream after #265 landed.

1. `changed_when: false` was needlessly blind — a change signal does exist

#265 concluded the tool emits no change signal. That came from grepping the compiled binary for
Created / Updated / Modified — past tense. The tool uses gerunds, so the grep couldn't
match and a usable signal was missed. My error.

Upstream reality: update_entity_attrs() wraps every mutation in if current_values != values, so
when values match, no request is sent and nothing is printed. Only mutations emit a log_event, and
only four verbs exist — Creating, Updating, Deleting, Appending. None of the six unconditional
phase headers contains any of them, so a verb match is sound on v1.3.0 with no upstream change.

Appending is deliberately excluded. In append mode the comparison is against the declared
member list only, and every group here sets overwriteMembers: false — so any group whose real
membership is a superset of what we declare (idm_people_admins: we declare the service account,
Kanidm also holds idm_admin) compares unequal forever and logs Appending every run. Same for
Tracking provisioned entities, which appends to ext_idm_provisioned_entities. Matching it would pin
changed: true permanently — recreating the exact bug #265 fixed.

Unanchored search on purpose: log_event colourises unconditionally (no supports_color check), so
ANSI escapes are present even piped, and the {:>12} padding is computed over the escape-laden string.

2. A failed build was invisible on retry

The Containerfile is staged before the build and is changed was the rebuild trigger — true only
on the run that stages it. If that build then failed (a Rust compile on a 2 vCPU / 4 GB host; the
Containerfile warns a link step can be OOM-killed), the next apply found the file unchanged and the
old image still tagged, silently skipped the build, and provisioned against the stale image.

A failure didn't self-heal and became invisible on retry — the same shape as the
merged-but-unapplied Caddyfile in #250.

The trigger now compares the staged Containerfile against a .built-Containerfile marker written only
after the build task succeeds. A crashed build leaves the mismatch, so the next run retries.

Marker written on is not skipped, not is changed: podman_image reports ok when the tag exists
and needs no rebuild — the steady state — so gating on is changed would never write it and leave the
gate open forever.

3. The whole provisioning block was invisible to `--check`

_kanidm_provision_ready depends on a uri probe, and uri doesn't run under --check. So the build
context, Containerfile staging, image build, state render and provisioning run were all skipped in
every dry-run.

That is how #265's base-image change reached production without appearing in a single --check —
the dry-run showed changed=1 (an unrelated artefact) while the real apply rebuilt an image. Same trap
as #257, third occurrence.

Probe and image-exists check are read-only, so both now run under --check. podman_image declares
supports_check_mode, so the build reports what it would do rather than compiling, and the
provisioning command is still skipped — a dry-run never mutates Kanidm.

Verification

--check --tags kanidm on prod now shows the probe, image check, both stats and the build as ok
(previously all skipped), and the marker task as changed. --syntax-check passes; ansible-lint
clean at production.

Also corrected the CARGO_BUILD_JOBS hint: podman build -e does not exist, so the previous comment
was not copy-pasteable — it needs ARG+ENV or --build-arg.

Not in this PR

The upstream review (bitborg-internal/copilot/2026-07-30-kanidm-provision-upstream-review.md) found
two further items being handled separately: the service-accounts block in kanidm-state.json.j2 is
silently discarded by v1.3.0, and entry_managed_by is patchable in ~10 lines of Rust — which would
make the delegation that caused the 13-day sign-up outage declarative.

Three fixes in the kanidm-provision path, all found by researching upstream after #265 landed. ## 1. `changed_when: false` was needlessly blind — a change signal does exist #265 concluded the tool emits no change signal. That came from grepping the compiled binary for `Created` / `Updated` / `Modified` — **past tense**. The tool uses **gerunds**, so the grep couldn't match and a usable signal was missed. My error. Upstream reality: `update_entity_attrs()` wraps every mutation in `if current_values != values`, so when values match, no request is sent and nothing is printed. Only mutations emit a `log_event`, and only four verbs exist — `Creating`, `Updating`, `Deleting`, `Appending`. None of the six unconditional phase headers contains any of them, so a verb match is sound on v1.3.0 with no upstream change. **`Appending` is deliberately excluded.** In append mode the comparison is against the *declared* member list only, and every group here sets `overwriteMembers: false` — so any group whose real membership is a superset of what we declare (`idm_people_admins`: we declare the service account, Kanidm also holds `idm_admin`) compares unequal forever and logs `Appending` every run. Same for *Tracking provisioned entities*, which appends to `ext_idm_provisioned_entities`. Matching it would pin `changed: true` permanently — recreating the exact bug #265 fixed. Unanchored `search` on purpose: `log_event` colourises unconditionally (no `supports_color` check), so ANSI escapes are present even piped, and the `{:>12}` padding is computed over the escape-laden string. ## 2. A failed build was invisible on retry The Containerfile is staged **before** the build and `is changed` was the rebuild trigger — true only on the run that stages it. If that build then failed (a Rust compile on a 2 vCPU / 4 GB host; the Containerfile warns a link step can be OOM-killed), the next apply found the file unchanged **and** the old image still tagged, silently skipped the build, and provisioned against the stale image. A failure didn't self-heal and became *invisible on retry* — the same shape as the merged-but-unapplied Caddyfile in #250. The trigger now compares the staged Containerfile against a `.built-Containerfile` marker written only **after** the build task succeeds. A crashed build leaves the mismatch, so the next run retries. Marker written on `is not skipped`, not `is changed`: `podman_image` reports `ok` when the tag exists and needs no rebuild — the steady state — so gating on `is changed` would never write it and leave the gate open forever. ## 3. The whole provisioning block was invisible to `--check` `_kanidm_provision_ready` depends on a `uri` probe, and `uri` doesn't run under `--check`. So the build context, Containerfile staging, image build, state render **and** provisioning run were all skipped in every dry-run. **That is how #265's base-image change reached production without appearing in a single `--check`** — the dry-run showed `changed=1` (an unrelated artefact) while the real apply rebuilt an image. Same trap as #257, third occurrence. Probe and image-exists check are read-only, so both now run under `--check`. `podman_image` declares `supports_check_mode`, so the build reports what it *would* do rather than compiling, and the provisioning `command` is still skipped — a dry-run never mutates Kanidm. ## Verification `--check --tags kanidm` on prod now shows the probe, image check, both stats and the build as `ok` (previously **all skipped**), and the marker task as `changed`. `--syntax-check` passes; `ansible-lint` clean at `production`. Also corrected the `CARGO_BUILD_JOBS` hint: `podman build -e` does not exist, so the previous comment was not copy-pasteable — it needs `ARG`+`ENV` or `--build-arg`. ## Not in this PR The upstream review (`bitborg-internal/copilot/2026-07-30-kanidm-provision-upstream-review.md`) found two further items being handled separately: the `service-accounts` block in `kanidm-state.json.j2` is silently discarded by v1.3.0, and `entry_managed_by` is patchable in ~10 lines of Rust — which would make the delegation that caused the 13-day sign-up outage declarative.

supernaut lade till 1 incheckning

2026-07-30 20:40:59 +00:00

fix(kanidm): detect real changes, and make a failed build retry instead of hiding

ci / ci (pull_request) Successful in 1m28s

Detaljer

93d4c0a752

Three fixes in the kanidm-provision path, all found by researching upstream after
#265 landed.

1. changed_when: false was needlessly blind — a change signal DOES exist

#265 set `changed_when: false` on the reasoning that the tool emits no change
signal. That conclusion came from grepping the compiled binary for Created /
Updated / Modified — PAST TENSE. The tool uses GERUNDS. The grep could not match,
so a usable signal was missed.

Upstream reality: `update_entity_attrs()` wraps every mutation in
`if current_values != values`, so when values match, no request is sent and nothing
is printed. Only mutations emit a log_event, and only four verbs exist: Creating,
Updating, Deleting, Appending. None of the six unconditional phase headers contains
any of them, so a verb match is sound on v1.3.0 with no upstream change.

  changed_when: stdout is search('(Creating|Updating|Deleting)')

`Appending` is deliberately excluded. In append mode the comparison is against the
DECLARED members only, and every group here sets overwriteMembers:false — so any
group whose real membership is a superset of what we declare (idm_people_admins,
where we declare the service account but Kanidm also holds idm_admin) compares
unequal forever and logs Appending on every run. Same for the "Tracking provisioned
entities" step, which appends to ext_idm_provisioned_entities. Matching it would
pin changed:true permanently — recreating the bug #265 fixed.

Unanchored `search` on purpose: log_event colourises unconditionally (no
supports_color check), so ANSI escapes are present even when stdout is a pipe, and
the {:>12} padding is computed over the escape-laden string.

2. A failed build was invisible on retry

The Containerfile is staged BEFORE the build and `is changed` was the rebuild
trigger, so it is true only on the run that stages it. If that build then failed —
a Rust compile on a 2 vCPU / 4 GB host, and the Containerfile itself warns a link
step can be OOM-killed — the next apply found the staged file unchanged AND the old
image still tagged, silently skipped the build, and provisioned against the stale
image. A failure did not self-heal and became invisible on retry: the same shape as
the merged-but-unapplied Caddyfile in #250.

Now the trigger compares the staged Containerfile against a `.built-Containerfile`
marker written only AFTER the build task succeeds. A crashed build leaves the
mismatch, so the next run retries.

The marker is written on `is not skipped`, not `is changed`: podman_image reports ok
(not changed) when the tag exists and needs no rebuild, which is the steady state —
gating on `is changed` would never write the marker and leave the gate open forever.

3. The whole provisioning block was invisible to --check

`_kanidm_provision_ready` depends on a `uri` reachability probe, and `uri` does not
run under --check. So the build context, Containerfile staging, image build, state
render and provisioning run were ALL skipped in every dry-run. That is how #265's
base-image change reached production without appearing in a single --check: the
dry-run showed changed=1 (an unrelated artefact) while the real apply rebuilt an
image. Same trap as #257.

The probe and the image-exists check are read-only, so both now run under --check.
podman_image declares supports_check_mode, so the build reports what it would do
rather than compiling, and the provisioning `command` is still skipped — a dry-run
never mutates Kanidm.

Verified: `--check --tags kanidm` now shows the probe, image check, both stats and
the build as ok (previously all skipped), and the marker task as changed.

Also corrected the CARGO_BUILD_JOBS hint: `podman build -e` does not exist, so the
previous comment was not copy-pasteable. It needs ARG+ENV in the Containerfile or
--build-arg.