devantler-tech / devantler-tech/ksail

Registry credential drift is invisible to `cluster diff` and to non-Updater provisioners

Closed Beginner friendly
#6,495 1 comment 1 reaction 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.