devantler-tech / devantler-tech/ksail
EKS ownership record that is unreadable reads as absent, so a stale config binds unopposed
- Dominant language
- Go
- Stars
- 165
- Forks
- 12
- Avg merge
- 5h 41m
- Merged PRs (30d)
- 337
Description
> 🤖 Generated by the Agentic Engineer
## Evidence
`loadUsableEKSOwnershipRecord` (`pkg/svc/state/eks_ownership_state.go:167`) returns `nil` on **any**
`os.ReadFile` or parse failure. `ListEKSOwnershipStates` then counts usable records and, at zero,
returns `ErrEKSOwnershipStateNotFound` — so **a record that exists but is unreadable or corrupt is
indistinguishable from a record that was never written.**
That conflation is now load-bearing. `confirmConfigMatchesOwnership`
(`pkg/cli/clusterapi/distconfig.go`, added in #6385) treats the not-found error as "this cluster
predates ownership records, so the rendered config is the only binding evidence" and accepts the
config alone:
```go
ownerships, err := state.ListEKSOwnershipStates(name)
if err != nil {
return nil //nolint:nilerr // absence of a record is not an error; the config binds instead.
}
```
## Affected audience and impact
Operators running EKS clusters through KSail's local backend.
`eks.yaml` is a **rendered** file: before the #6385 binding existed, every delete/start/stop
re-rendered it from the ambient `AWS_REGION`, so a stale one can name a region the cluster was never
created in. The ownership record is the immutable check on that. If the record is present but
truncated (interrupted write, full disk) or unreadable (permissions), the check silently disappears
and a stale config binds unopposed — so `ksail cluster delete` can aim at a **same-named cluster in
the wrong region**. That is precisely the redirect the binding exists to prevent, reached through
record corruption rather than through the config.
Not a regression: without #6385 there is no binding at all, so this is an incomplete guard rather
than a new hazard. It is filed separately because the defect is in `pkg/svc/state`, which #6385 does
not touch.
## Expected behaviour
A record that cannot be read or parsed is **evidence that something is wrong**, not evidence of
absence. Only a genuinely absent record should license the legacy config-only path; an unreadable or
malformed one should surface and refuse, consistent with the fail-closed posture the surrounding
code already documents ("missing or unreadable evidence for a cluster that did complete creation is
an error rather than a silent fall back to the ambient region").
## Acceptance criteria
- `ListEKSOwnershipStates` distinguishes *no record present* from *a record present but unusable*
(separate sentinel, or a returned diagnostic alongside the usable set).
- `confirmConfigMatchesOwnership` accepts the config-only path **only** for genuine absence, and
refuses with an actionable message naming the unreadable path otherwise.
- Regression tests for both: a truncated `eks-ownership-.json` and a mode-000 one, each
asserted to refuse rather than silently bind, plus the existing legacy no-record control still
passing.
- Callers of `ListEKSOwnershipStates` outside this path reviewed for the same conflation
(`resolveCluster` uses it as an ownership signal).
Rough size: small — one behaviour change in `pkg/svc/state` plus its callers and tests.
Found during the local review round on #6385.
Contributor guide
Research direction
Start with loadUsableEKSOwnershipRecord and ListEKSOwnershipStates in pkg/svc/state/eks_ownership_state.go:167, then trace confirmConfigMatchesOwnership in pkg/cli/clusterapi/distconfig.go and resolveCluster callers. Verify that genuine absence retains the legacy path while truncated or mode-000 eks-ownership records produce an actionable refusal. Add regression coverage for both corrupt-record cases and preserve the no-record control described in the acceptance criteria.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- aws, go, kubernetes
- Domain
- cli, cloud
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100