Skip to content

Commit 42f0f6f

Browse files
committed
docs(engine,apply): replace ticket numbers with self-explanatory descriptions
Source comments and tests had bug-number references (#77, PR #133) that violate the project's commit-message and code-comment standards: comments must be self-explanatory to a reader without access to the issue tracker. Replace each citation with a description of the bug class itself -- 'duplicate primitive-array entries per round-trip' instead of '#77', 'the version-mismatch warning that preflightCheckTalosVersion exists to surface' instead of 'PR #133'. The surrounding prose already says what the issue is, so the numbers came out without losing context. Pre-existing references in code this branch does not touch (preflight_test.go's #132 reproduction comments, engine_test.go's #66 fixture name) are left as-is -- out of scope here. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
1 parent 758e365 commit 42f0f6f

5 files changed

Lines changed: 21 additions & 16 deletions

File tree

pkg/commands/apply.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,8 @@ func apply(args []string) error {
138138
// length (its COSI guard is unconditional). cosiVersionReader
139139
// swallows errors and returns ok=false on rejection, so the
140140
// preflight would silently no-op on the auth path — defeating
141-
// the whole point of preflightCheckTalosVersion (PR #133).
141+
// the whole point of the version-mismatch warning that
142+
// preflightCheckTalosVersion exists to surface.
142143
// cosiPreflightContext rebuilds ctx with the singular "node"
143144
// key so the COSI router accepts the call; ApplyConfiguration
144145
// keeps the original ctx unchanged.

pkg/commands/apply_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -705,8 +705,9 @@ func TestApplyTemplatesPerNode_AuthModeUsesPluralNodesMetadataKey(t *testing.T)
705705
// outgoing context carries the plural key, regardless of slice
706706
// length. cosiPreflightContext rebuilds ctx with the singular "node"
707707
// key so the COSI router accepts the call. Without this, the version
708-
// preflight added in PR #133 silently no-ops on the auth path:
709-
// cosiVersionReader swallows errors and returns ok=false on rejection.
708+
// preflight silently no-ops on the auth path: cosiVersionReader
709+
// swallows errors and returns ok=false on rejection, so the user
710+
// never sees the mismatch warning the preflight exists to surface.
710711
func TestCosiPreflightContext_StripsPluralAndAttachesSingular(t *testing.T) {
711712
const node = "10.0.0.1"
712713
in := client.WithNodes(context.Background(), node)

pkg/engine/engine.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -470,8 +470,8 @@ func isPatchDeleteDirective(n *yaml.Node) bool {
470470
// to primitive arrays rather than treating them as a set, so a body that
471471
// re-states an unchanged primitive list (the dominant case after
472472
// `talm template -I` writes the rendered template back into the node file as
473-
// the body) would otherwise duplicate every entry on each apply round-trip.
474-
// See cozystack/talm#77.
473+
// the body) would otherwise duplicate every entry on each apply round-trip:
474+
// every certSAN, every nameserver, every podSubnet doubles per round-trip.
475475
//
476476
// Returns (prunedBytes, allPruned, err). When allPruned is true the body
477477
// carried no semantic change beyond the rendered template and the caller
@@ -557,7 +557,9 @@ func pruneBodyIdentitiesAgainstRendered(body, rendered []byte) ([]byte, bool, er
557557
// present in rendered's array, replacing body's slice with the user-add
558558
// difference (when the diff is empty the entry is deleted) — this
559559
// neutralises Talos's strategic-merge primitive-array append behaviour
560-
// (cozystack/talm#77) for both byte-identical and partial-edit cases.
560+
// for both byte-identical and partial-edit cases — without it, every
561+
// `talm template -I` round-trip would double every certSAN, nameserver,
562+
// and podSubnet entry on the next apply.
561563
//
562564
// Object arrays (arrays whose elements are maps) are intentionally left
563565
// alone: configpatcher's StrategicMerge handles them via patchMergeKey

pkg/engine/engine_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,8 +345,8 @@ func TestPruneIdenticalKeys_PreservesUserIntentEmptyMap(t *testing.T) {
345345
// child entry is deep-equal to rendered's counterpart), the dedup
346346
// pass MUST collapse it. Without this, byte-identical bodies would
347347
// leave behind a wrapping map that strategic-merge then re-stamps,
348-
// re-introducing the cozystack/talm#77 duplication this branch
349-
// fixes.
348+
// re-introducing the duplicate-primitive-array-entries-per-round-trip
349+
// regression this branch fixes.
350350
func TestPruneIdenticalKeys_RemovesIdenticalNestedMap(t *testing.T) {
351351
body := map[string]any{
352352
"a": map[string]any{

pkg/engine/render_test.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1762,9 +1762,9 @@ machine:
17621762
// rendered. Talos's strategic-merge appends to primitive arrays
17631763
// rather than treating them as a set, so without identity-pruning
17641764
// every certSANs/nameservers/validSubnets/endpoints entry doubles
1765-
// on every apply round-trip — this is the user-visible symptom of
1766-
// cozystack/talm#77 ("Duplicate array entries inserted into
1767-
// machine config when running talm apply").
1765+
// on every apply round-trip — every certSAN, every nameserver,
1766+
// every podSubnet appears twice on the second apply, four times
1767+
// on the third, and so on.
17681768
//
17691769
// Pin the post-fix contract: when the body's keys match the
17701770
// rendered's keys exactly (deep-equal), MergeFileAsPatch must
@@ -1819,8 +1819,9 @@ cluster:
18191819
// where the user adds one new entry to an array the chart already
18201820
// populated. Talos's strategic-merge appends primitive arrays
18211821
// rather than treating them as a set, so without per-element diff
1822-
// the rendered entries appear twice in the merged config — same
1823-
// duplication symptom as cozystack/talm#77, just one entry deeper.
1822+
// the rendered entries appear twice in the merged config — the
1823+
// same duplicate-primitive-array-entries-per-round-trip symptom
1824+
// as the byte-identical case, just one entry deeper.
18241825
const renderedTemplate = `version: v1alpha1
18251826
machine:
18261827
type: controlplane
@@ -1907,9 +1908,9 @@ machine:
19071908
// `talm template -I` writes each rendered document back as a body
19081909
// document, and Talos's strategic-merge appends to primitive arrays
19091910
// per-document. Without per-doc identity matching, the prune
1910-
// short-circuits the multi-doc input and the duplication symptom
1911-
// from cozystack/talm#77 reappears at the v1.12+ default — a
1912-
// `127.0.0.1` certSAN entry doubles on every apply round-trip.
1911+
// short-circuits the multi-doc input and the duplicate-primitive-
1912+
// array-entries-per-round-trip symptom reappears at the v1.12+
1913+
// default — a `127.0.0.1` certSAN entry doubles on every apply.
19131914
//
19141915
// Pin the post-fix contract: a body that re-states an unchanged
19151916
// multi-doc rendered template must merge to a config whose

0 commit comments

Comments
 (0)