fix(engine): treat $patch:delete on absent paths as no-op

pkg/engine/engine.go

@@ -250,6 +250,14 @@ func MergeFileAsPatch(rendered []byte, patchFile string) ([]byte, error) {
 			patchFile,
 		)
 	}
+	cleanedPatch, err = stripPatchDeleteDirectivesAbsentInTarget(cleanedPatch, cleanedRendered)
+	if err != nil {
+		return nil, errors.WithHintf(
+			errors.Wrapf(err, "stripping no-op $patch:delete directives from %q", patchFile),
+			"the node body did not parse as YAML; verify %q is well-formed",
+			patchFile,
+		)
+	}
 	prunedBytes, allPruned, err := pruneBodyIdentitiesAgainstRendered(cleanedPatch, cleanedRendered)
 	if err != nil {
 		return nil, errors.WithHintf(
@@ -383,6 +391,159 @@ func stripPatchDeleteDirectivesAtPaths(data []byte, paths []string) ([]byte, err
 	return encodeAllYAMLDocuments(docs)
 }
 
+// stripPatchDeleteDirectivesAbsentInTarget walks every YAML document
+// in `data` and removes $patch:delete directives whose path does not
+// resolve to a key in the matching `target` document. configpatcher.Apply
+// otherwise errors with `failed to delete path '...': lookup failed`
+// — its Selector-based deleteForPath walks the parsed v1alpha1.Config
+// struct and rejects any path segment that does not resolve. Kubernetes
+// strategic merge patch treats delete-of-absent as a no-op, so this
+// helper restores that semantic before the patch reaches the apply RPC,
+// which keeps the chart's own pattern (a body that re-states a chart-
+// emitted directive after `talm template -I`) usable on a fresh apply
+// where the targeted key has not yet been populated on the node.
+//
+// `target` is the rendered template AFTER stripAllPatchDeleteDirectives
+// has removed every chart-side directive — i.e. the structural shape
+// configpatcher.Apply will see as the merge target. A directive whose
+// path isn't reachable in that shape is a no-op by definition.
+//
+// Pairs body and target documents by identity tuple (apiVersion+kind+name,
+// or the legacy-root sentinel) so a body re-ordering its typed documents
+// relative to rendered still resolves directive paths against the right
+// target document. A body document with no matching target document
+// (no rendered counterpart at all) gets every directive stripped,
+// matching the upstream contract: there is nothing to delete.
+func stripPatchDeleteDirectivesAbsentInTarget(data, target []byte) ([]byte, error) {
+	bodyDocs, err := decodeAllYAMLDocuments(data)
+	if err != nil {
+		return nil, err
+	}
+	if len(bodyDocs) == 0 {
+		return data, nil
+	}
+	targetDocs, err := decodeAllYAMLDocuments(target)
+	if err != nil {
+		return nil, err
+	}
+	targetByID := make(map[string]*yaml.Node, len(targetDocs))
+	for _, doc := range targetDocs {
+		targetByID[documentIdentityFromNode(doc)] = doc
+	}
+	pruneSet := make(map[string]struct{})
+	for _, bdoc := range bodyDocs {
+		id := documentIdentityFromNode(bdoc)
+		targetDoc := targetByID[id]
+		for _, rel := range collectDeleteDirectivePaths(bdoc, "") {
+			if !pathExistsInDoc(targetDoc, rel) {
+				pruneSet[joinYAMLPath("/"+id, rel)] = struct{}{}
+			}
+		}
+	}
+	if len(pruneSet) == 0 {
+		return data, nil
+	}
+	stripped := 0
+	for _, doc := range bodyDocs {
+		stripped += len(removePatchDeleteFromNode(doc, "/"+documentIdentityFromNode(doc), pruneSet))
+	}
+	if stripped == 0 {
+		return data, nil
+	}
+	return encodeAllYAMLDocuments(bodyDocs)
+}
+
+// collectDeleteDirectivePaths walks `node` and returns the
+// JSON-pointer-escaped paths (relative to the document root, no
+// identity prefix) of every $patch:delete directive it contains.
+// Used by stripPatchDeleteDirectivesAbsentInTarget to enumerate body's
+// directives so each can be checked against the target document.
+func collectDeleteDirectivePaths(node *yaml.Node, parentRel string) []string {
+	if node == nil {
+		return nil
+	}
+	var found []string
+	switch node.Kind {
+	case yaml.DocumentNode:
+		for _, child := range node.Content {
+			found = append(found, collectDeleteDirectivePaths(child, parentRel)...)
+		}
+	case yaml.MappingNode:
+		for i := 0; i+1 < len(node.Content); i += 2 {
+			keyNode := node.Content[i]
+			valueNode := node.Content[i+1]
+			if keyNode.Kind != yaml.ScalarNode {
+				continue
+			}
+			childRel := joinYAMLPath(parentRel, jsonPointerEscape(keyNode.Value))
+			if isPatchDeleteDirective(valueNode) {
+				found = append(found, childRel)
+				continue
+			}
+			if valueNode.Kind == yaml.MappingNode {
+				found = append(found, collectDeleteDirectivePaths(valueNode, childRel)...)
+			}
+		}
+	}
+	return found
+}
+
+// pathExistsInDoc resolves `path` (a slash-separated sequence of
+// JSON-pointer-escaped segments, no leading slash, no document
+// identity prefix) against the YAML document `doc` and returns true
+// when every segment names an existing key in the corresponding
+// mapping. An empty path resolves to the document root (true unless
+// doc is nil or non-mapping at the root).
+//
+// The walk is deliberately mapping-only: configpatcher.Apply's
+// Selector-based deleteForPath addresses scalar map fields by name
+// (machine.nodeLabels.<label>) and bails on the first non-matching
+// segment regardless of the target's kind below it. This helper
+// reproduces the same predicate so a path declared no-op here is
+// guaranteed to be the same path the apply RPC would have erred on.
+func pathExistsInDoc(doc *yaml.Node, path string) bool {
+	if doc == nil {
+		return false
+	}
+	cur := doc
+	if cur.Kind == yaml.DocumentNode && len(cur.Content) > 0 {
+		cur = cur.Content[0]
+	}
+	if cur == nil || cur.Kind != yaml.MappingNode {
+		return false
+	}
+	if path == "" {
+		return true
+	}
+	for _, escaped := range strings.Split(path, "/") {
+		seg := jsonPointerUnescape(escaped)
+		if cur.Kind != yaml.MappingNode {
+			return false
+		}
+		found := false
+		for i := 0; i+1 < len(cur.Content); i += 2 {
+			if cur.Content[i].Value == seg {
+				cur = cur.Content[i+1]
+				found = true
+				break
+			}
+		}
+		if !found {
+			return false
+		}
+	}
+	return true
+}
+
+// jsonPointerUnescape reverses jsonPointerEscape per RFC 6901
+// (~1 → /, ~0 → ~). Order matters: ~0 must be processed last so a
+// literal "~0" written into a YAML key survives the round-trip.
+func jsonPointerUnescape(s string) string {
+	s = strings.ReplaceAll(s, "~1", "/")
+	s = strings.ReplaceAll(s, "~0", "~")
+	return s
+}
+
 func decodeAllYAMLDocuments(data []byte) ([]*yaml.Node, error) {
 	dec := yaml.NewDecoder(bytes.NewReader(data))
 	var docs []*yaml.Node

pkg/engine/render_test.go

@@ -3182,6 +3182,141 @@ machine:
 		}
 	})
 
+	t.Run("body $patch:delete on absent path is a no-op", func(t *testing.T) {
+		// Kubernetes strategic merge patch treats a $patch:delete on an
+		// absent path as a no-op (the key is already absent, nothing to
+		// delete). Talos's configpatcher.Apply does not: its Selector-
+		// based deleteForPath walks the parsed v1alpha1.Config struct and
+		// returns ErrLookupFailed when any path segment doesn't resolve,
+		// surfacing as `failed to delete path '...': lookup failed` from
+		// the apply RPC.
+		//
+		// Stripping these no-op directives at the talm side before the
+		// patch reaches configpatcher.Apply matches the k8s SMP semantic
+		// and stops a real-world failure: a node body restating a chart-
+		// emitted directive (e.g. machine.nodeLabels.<label>: $patch:
+		// delete) errors out when rendered for the first time on a
+		// freshly generated config that hasn't yet acquired the label.
+		// Without this strip, the chart's own pattern fails on every
+		// fresh apply and bootstrap is broken.
+		const renderedTemplate = `version: v1alpha1
+machine:
+  type: controlplane
+  install:
+    disk: /dev/sda
+cluster:
+  controlPlane:
+    endpoint: https://10.0.0.10:6443
+`
+		const userBody = `# talm: nodes=["10.0.0.1"]
+machine:
+  nodeLabels:
+    node.kubernetes.io/exclude-from-external-load-balancers:
+      $patch: delete
+`
+		dir := t.TempDir()
+		nodeFile := filepath.Join(dir, "node0.yaml")
+		if err := os.WriteFile(nodeFile, []byte(userBody), 0o644); err != nil {
+			t.Fatalf("write node file: %v", err)
+		}
+
+		merged, err := MergeFileAsPatch([]byte(renderedTemplate), nodeFile)
+		if err != nil {
+			t.Fatalf("MergeFileAsPatch must accept a delete directive on an absent path as a no-op, got error: %v", err)
+		}
+		out := string(merged)
+		if strings.Contains(out, "$patch: delete") {
+			t.Errorf("merged output still carries the directive literal:\n%s", out)
+		}
+		if strings.Contains(out, "exclude-from-external-load-balancers") {
+			t.Errorf("merged output mentions the never-rendered key — the body's no-op directive should not have re-introduced it:\n%s", out)
+		}
+	})
+
+	t.Run("body $patch:delete on partially-present path is a no-op when leaf is absent", func(t *testing.T) {
+		// A subtler form of the absent-path case: the body's directive
+		// addresses a leaf under a parent that DOES exist in rendered,
+		// but the leaf itself doesn't. configpatcher.Apply walks the
+		// path segment-by-segment and fails on the missing leaf with
+		// the same ErrLookupFailed. The fix must treat any path whose
+		// final segment doesn't resolve as a no-op, not just paths
+		// missing at the top level.
+		const renderedTemplate = `version: v1alpha1
+machine:
+  type: controlplane
+  install:
+    disk: /dev/sda
+  nodeLabels:
+    other-label: present
+cluster:
+  controlPlane:
+    endpoint: https://10.0.0.10:6443
+`
+		const userBody = `# talm: nodes=["10.0.0.1"]
+machine:
+  nodeLabels:
+    node.kubernetes.io/exclude-from-external-load-balancers:
+      $patch: delete
+`
+		dir := t.TempDir()
+		nodeFile := filepath.Join(dir, "node0.yaml")
+		if err := os.WriteFile(nodeFile, []byte(userBody), 0o644); err != nil {
+			t.Fatalf("write node file: %v", err)
+		}
+
+		merged, err := MergeFileAsPatch([]byte(renderedTemplate), nodeFile)
+		if err != nil {
+			t.Fatalf("MergeFileAsPatch: %v", err)
+		}
+		out := string(merged)
+		if strings.Contains(out, "$patch: delete") {
+			t.Errorf("merged output still carries the directive literal:\n%s", out)
+		}
+		if !strings.Contains(out, "other-label: present") {
+			t.Errorf("rendered sibling key under nodeLabels was incorrectly stripped along with the absent target:\n%s", out)
+		}
+	})
+
+	t.Run("body $patch:delete on present path still removes the key", func(t *testing.T) {
+		// Regression-safety probe: the no-op-on-absent fix must not
+		// over-trigger and silently drop directives whose target IS
+		// present in rendered. The user-intent delete must still land
+		// as a Selector and remove the key from the merged config.
+		const renderedTemplate = `version: v1alpha1
+machine:
+  type: controlplane
+  install:
+    disk: /dev/sda
+  nodeLabels:
+    user-label: please-delete-me
+cluster:
+  controlPlane:
+    endpoint: https://10.0.0.10:6443
+`
+		const userBody = `# talm: nodes=["10.0.0.1"]
+machine:
+  nodeLabels:
+    user-label:
+      $patch: delete
+`
+		dir := t.TempDir()
+		nodeFile := filepath.Join(dir, "node0.yaml")
+		if err := os.WriteFile(nodeFile, []byte(userBody), 0o644); err != nil {
+			t.Fatalf("write node file: %v", err)
+		}
+
+		merged, err := MergeFileAsPatch([]byte(renderedTemplate), nodeFile)
+		if err != nil {
+			t.Fatalf("MergeFileAsPatch: %v", err)
+		}
+		out := string(merged)
+		if strings.Contains(out, "$patch: delete") {
+			t.Errorf("merged output still carries the directive literal:\n%s", out)
+		}
+		if strings.Contains(out, "user-label") {
+			t.Errorf("user-intent delete on a present path was suppressed; the key still appears in merged output:\n%s", out)
+		}
+	})
 }
 
 // TestMergeFileAsPatch_PreservesUserIntentPatchDelete pins the contract