Skip to content

Commit 69c4d5d

Browse files
committed
fix(docs+commands): pass 12 doc-vs-code drift + case-handling rationale
apply-safety-gates-test-plan.md described 'init --update non-tty UX gap (#174)' as a known limitation in two places (Setup section and Known limitations section), but #174 is closed by this PR — the overwrite policy resolves to NonInteractive in non-tty contexts with a hint-bearing error, and --force accepts all preset-template diffs non-interactively. Both stale references replaced with the correct guidance ('pass --force for non-interactive refresh'). manual-test-plan.md's J0-1 footgun example cited 192.168.1.1 (real RFC 1918 private). Replaced with 198.51.100.1 (RFC 5737 TEST-NET-2) to match the documentation-IP convention used elsewhere in the file. predicateType (selector.go) lowercases sel.Type before the type switch, accepting 'SSD' / 'NVMe' that Talos rejects with 'unsupported disk type "SSD"'. The previous godoc claim that predicateType 'mirrors Talos's v1alpha1 type-selector resolution' was accurate for the lookup logic but elided the case-handling divergence. Comment expanded to document the intentional permissiveness — Phase 1 catches declared-resource mismatches, not Talos's input-format strictness; tightening would duplicate Talos's check without operator benefit. No code change. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
1 parent d297d00 commit 69c4d5d

3 files changed

Lines changed: 15 additions & 3 deletions

File tree

docs/apply-safety-gates-test-plan.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ Before requesting human review, exercise the gates against a live Talos node.
137137
cd /path/to/your/talm/values/tree
138138
```
139139

140-
Use a 3-node Talos cluster (replace placeholders below with your own node IPs — examples use RFC 5737 documentation ranges). The vendored talm library in `charts/talm/` may need `talm init --update --preset cozystack` to pick up new helpers; preset templates (`templates/_helpers.tpl`) require interactive confirm and won't auto-update non-tty (known gap, see #174).
140+
Use a 3-node Talos cluster (replace placeholders below with your own node IPs — examples use RFC 5737 documentation ranges). The vendored talm library in `charts/talm/` may need `talm init --update --preset cozystack` to pick up new helpers; pass `--force` for non-interactive refresh (CI, scripted), or run under a tty for the interactive prompt.
141141

142142
### Sanity check
143143

@@ -203,7 +203,6 @@ go vet ./...
203203

204204
## Known limitations / follow-ups
205205

206-
- **`talm init --update` non-tty UX gap** (#174): preset-template overwrites require interactive confirmation, so a CI-driven refresh leaves the operator on a stale preset that doesn't surface new validation logic. Work around by copying preset templates from the repo manually or running update under a tty.
207206
- **Talos-mutated-field allowlist** (open in #172): Phase 2B reports cert hashes / timestamps as divergence today; the verify is off by default until an allowlist lands.
208207
- **`talm upgrade` has no pre-upgrade gates** (Phase 2C runs *after*, not before): the upgrade flow wraps `talosctl upgrade` and doesn't route through `buildApplyClosure` / `applyOneFileDirectPatchMode`, so Phase 1 / Phase 2A do not run. Phase 2C (post-upgrade version verify) was added precisely to catch the silent-rollback class without that refactor. Full pre-upgrade gates would require reproducing the gate calls in `upgrade_handler.go` or refactoring the apply flow.
209208
- **Phase 1/2 on `--insecure`**: the safety gates can't run before the chart renders, and the chart's `lookup` calls need an authenticated COSI connection. Insecure path = effectively no gates today.

docs/manual-test-plan.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ Expected: every shell prints a script that parses (for bash/zsh syntax-check con
407407

408408
Expected: with the post-#163 chart, fails fast with `talm: floatingIP "0700" is not a valid IPv4 / IPv6 literal`. Pre-#163 chart silently renders an invalid VIP.
409409

410-
**Operator footgun**: `--set floatingIP=192.168.1.1` *may* be parsed as the float `192.168 × 1.1` by Helm's loose type-coercion. For IPs use `--set-string floatingIP="192.168.1.1"` or put it in `values.yaml`.
410+
**Operator footgun**: `--set floatingIP=198.51.100.1` *may* be parsed as the float `198.51 × 100.1` by Helm's loose type-coercion. For IPs use `--set-string floatingIP="198.51.100.1"` or put it in `values.yaml`.
411411

412412
### J0-2. `--set-literal` keeps dotted keys intact
413413

pkg/applycheck/selector.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,19 @@ var diskPredicates = []diskPredicate{
120120
// type: ssd -> Rotational == false
121121
//
122122
// An empty selector type is "don't care".
123+
//
124+
// Case handling intentionally diverges from Talos: predicateType
125+
// lowercases sel.Type before the switch, whereas Talos returns
126+
// `unsupported disk type "SSD"` for mixed-case input. Phase 1's job
127+
// is to surface declared-resource mismatches at render time, NOT to
128+
// re-implement Talos's case-strict input validation — an operator
129+
// who declared `type: SSD` in values.yaml will get either (a) a
130+
// match here and a downstream Talos error at apply, or (b) the
131+
// "matches zero disks" finding from the rest of the selector logic.
132+
// Case (a) is acceptable: Phase 1 didn't introduce the typo, and
133+
// the Talos error itself is clear. Tightening this branch would
134+
// require its own enum hint and offer no protection beyond
135+
// duplicating Talos's check.
123136
func predicateType(sel *DiskSelector, disk *DiskInfo) bool {
124137
if sel.Type == "" {
125138
return true

0 commit comments

Comments
 (0)