devantler-tech / devantler-tech/ksail
Registry credential drift is invisible to `cluster diff` and to non-Updater provisioners
- Dominant language
- Go
- Stars
- 165
- Forks
- 12
- Avg merge
- 5h 51m
- Merged PRs (30d)
- 347
Description
> 🤖 Generated by the Agentic Engineer
## Evidence
`computeSpecOnlyDiff` (`pkg/cli/cmd/cluster/orchestrator.go`) runs three drift checks. The
Updater diff path (`computeUpdateDiff`, same file) runs **four**. The one only the Updater
path runs is `checkRegistryCredentialDrift`:
| Check | Updater path | spec-only path |
|---|---|---|
| `checkWorkloadTagDrift` | yes | yes |
| `checkFluxDistributionVersionDrift` | yes | yes |
| `checkFluxVerifyDrift` | yes | yes (fixed in #6491) |
| `checkRegistryCredentialDrift` | yes | **no** |
`computeSpecOnlyDiff` has exactly two callers: `diff.go:128` (the `ksail cluster diff`
command) and `orchestrator.go:128` (`runWithoutUpdater`, the path for provisioners with no
`Updater` — e.g. VCluster).
The machinery to act on the change already exists: `reconciler.go` registers
`specdiff.RegistryCredentialField` → `reconcileRegistryCredentials`. Only the **detection**
is absent from this path.
`checkRegistryCredentialDrift` carries all its own guards (`GitOpsEngine == Flux`,
`HasExternalRegistryCredentials`, kubeconfig resolvable) and takes the same four arguments
the spec-only path already passes to `checkFluxVerifyDrift`, so nothing about it is
Updater-specific.
## Who this affects, and how
This is the same defect class #6480 described, on the sibling check that was left behind.
1. **Anyone running `ksail cluster diff` with Flux + external registry credentials.** A
credential-only rotation is the case `checkRegistryCredentialDrift` exists for: registry
passwords are redacted from the structural diff, so rotating the value behind an otherwise
identical configuration produces **no field change** (#6107). `cluster diff` therefore
previews nothing, and the following `cluster update` applies a credential refresh the
preview never showed. A preview that silently omits a security-relevant repair is worse
than no preview, because it is trusted.
2. **Provisioners with no `Updater` (VCluster).** `runWithoutUpdater` exits early on
`TotalChanges() == 0` with "no applicable changes". A credential-only rotation yields
exactly zero changes there, so the cluster keeps authenticating with the **revoked**
credential and nothing reports it — #6107's original failure mode, still unfixed on those
provisioners.
## Expected behaviour
`computeSpecOnlyDiff` runs `checkRegistryCredentialDrift` alongside its three siblings, so
the preview agrees with the apply and non-Updater provisioners detect credential drift.
## Acceptance criteria
- [ ] `computeSpecOnlyDiff` invokes `checkRegistryCredentialDrift`.
- [ ] A test pins that the spec-only path reaches the check, provable without a live cluster
(mirroring `specOnlyDiffRanVerifyCheck` in `flux_verify_reconcile_test.go`).
- [ ] A test pins the guards, so wiring the check into a second path cannot make it fire
where it must stay silent (non-Flux engine, no external registry credentials).
- [ ] No behaviour change for configurations already covered by the Updater path.
**Size:** small — a one-line wiring change plus its tests, directly mirroring #6491.
Contributor guide
Research direction
Start in pkg/cli/cmd/cluster/orchestrator.go at computeSpecOnlyDiff and compare its checks with computeUpdateDiff. Use the specOnlyDiffRanVerifyCheck pattern in flux_verify_reconcile_test.go to test that registry credential drift is reached and that its non-Flux and no-external-credentials guards remain silent. Done means the spec-only path invokes the check without changing configurations already covered by the Updater path.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- cli, devops, testing
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100