devantler-tech / devantler-tech/ksail
EKS ownership binding discards the recorded AWS credential mapping
- Dominant language
- Go
- Stars
- 165
- Forks
- 12
- Avg merge
- 5h 41m
- Merged PRs (30d)
- 337
Description
> 🤖 Generated by the Agentic Engineer
## Evidence
`EKSOwnershipState` deliberately records the **complete AWS environment-variable-name mapping** used when a cluster was bound:
```
// AWSOptions stores the complete environment-variable-name mapping used to resolve AWS
// credentials. Credential values are never persisted. Records without this mapping predate the
// schema extension and require explicit rebind before state-only lifecycle commands may use them.
AWSOptions v1alpha1.OptionsAWS `json:"awsOptions,omitzero"`
```
`ListEKSOwnershipStates` enforces that: a record without a complete mapping is rejected as unusable. So the mapping is treated as a precondition for acting on a record at all.
But the binding path then discards it. `bindFromOwnershipRecord` keeps only the region:
```go
return &clusterprovisioner.EKSConfig{
Name: name,
Region: region,
ConfigPath: configPath,
}, nil
```
`clusterprovisioner.EKSConfig` has no field for it — its fields are `Name`, `NameFromConfig`, `Region`, `ConfigPath`, `KubeconfigPath` — so the mapping cannot be carried through even in principle without a type change.
## Problem / audience
Operators who resolve AWS credentials through **non-default environment-variable names** (a per-account or per-profile naming scheme). The lifecycle spec built from an ownership record carries no `Spec.Provider.AWS` options, so credential resolution falls back to the canonical `AWS_*` variables instead of the names captured at bind time.
Two outcomes, and the second is the dangerous one:
- the action fails despite valid credentials being present under the operator's own variable names; or
- the canonical variables select a **different AWS account** that happens to contain a cluster of the same name, and the action targets that one.
This is the same class of defect as the region redirect that motivated the ownership record in the first place — an action resolving its target from ambient environment rather than from what was recorded — one axis over. It affects delete, which makes it destructive.
Note the guard that still stands: `confirmEKSOwnership` runs regardless, so this is not an unauthenticated path. The risk is that it authenticates against the wrong account's cluster.
## Why it is not fixed in the PR that found it
Found by review on [#6385](https://github.com/devantler-tech/ksail/pull/6385). Fixing it means adding the mapping to `clusterprovisioner.EKSConfig` and threading it through `DefaultFactory` into credential resolution — a cross-package change to a shared provisioner type, on a destructive path. #6385 already carries two behaviour changes through several review rounds, and folding a third in would make it materially harder to review.
## Smallest useful change
1. Carry the recorded `AWSOptions` on the config the binding produces.
2. Resolve credentials from that frozen mapping for state-bound lifecycle actions, rather than from the ambient defaults.
3. Prove it with a test where the recorded variable names differ from the canonical ones, asserting which names are read — not merely that the action succeeds, since it succeeds either way when both point at the same account.
## Acceptance criteria
- [ ] A lifecycle action bound from an ownership record resolves AWS credentials using the variable names in that record.
- [ ] A test fails if resolution falls back to the canonical `AWS_*` names when the record names others.
- [ ] Records predating the mapping keep their current behaviour (explicit rebind required), with no new failure mode for them.
Rough size: S–M.
Contributor guide
Research direction
Start with bindFromOwnershipRecord and clusterprovisioner.EKSConfig, then trace DefaultFactory into AWS credential resolution. Follow the recorded AWSOptions mapping through state-bound lifecycle actions, especially delete. Add coverage using non-canonical variable names and verify those names are read, while preserving the existing behavior for records without a mapping.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- aws, go
- Domain
- backend, cloud
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 52/100