fix(applycheck): drop false-positive blockers on virtual-link names

BondConfig.name, VLANConfig.name, and BridgeConfig.name describe
*virtual links being created* by the apply, not references to
existing links. The walker emitted them as Phase 1 link refs anyway,
so every legitimate 'add a new bond / vlan / bridge to a node'
apply tripped its own pre-flight gate with a 'declared link
bond99 not found' blocker on the doc that was supposed to create
bond99 in the first place. Operators would have had to pass
--skip-resource-validation on every such apply, defeating the gate.

New contract per kind:

  - LinkConfig.name -> emitted (override case dominates; the operator
    is usually tweaking ens5 / eth0 settings on a NIC that already
    exists, and a typo here is the most common Phase-1 catch).
  - VLANConfig.link -> emitted (parent must exist), .name skipped.
  - BondConfig.links[] -> emitted (slaves must exist), .name skipped.
  - BridgeConfig.ports[] -> emitted (ports must exist), .name skipped.

Pinned with explicit assertion: VLANConfig.name (eth0.4000 in the
fixture) MUST NOT surface as a ref. Verified on dev17 OCI cluster:
adding 'BondConfig{name: bond99, links: [ghost0, ens5]}' now blocks
only on ghost0 (the bad slave), not on bond99 (the new bond being
created).

Refs: #172
Signed-off-by: Aleksei Sviridkin <f@lex.la>

Author: lexfrei

pkg/applycheck/refs.go

@@ -165,10 +165,17 @@ type multidocHandler func(refs []Ref, doc map[string]any, basePath string) []Ref
 
 //nolint:gochecknoglobals // dispatch table for multidoc kinds; static after init.
 var multidocHandlers = map[string]multidocHandler{
+	// LinkConfig.name is emitted: the typical case is an override of an
+	// existing physical NIC (ens5 settings, MTU on eth0), where the
+	// operator wants validation to catch a typoed interface name. The
+	// rarer "create-a-fresh-virtual-link" case is covered by BondConfig/
+	// BridgeConfig/VLANConfig below, which intentionally do NOT validate
+	// their own .name field — those names describe a virtual link being
+	// created by the apply, not an existing one to reference.
 	"LinkConfig":       handleNameOnly,
-	"BondConfig":       handleNamePlusList("links"),
-	"VLANConfig":       handleNamePlusParent,
-	"BridgeConfig":     handleNamePlusList("ports"),
+	"BondConfig":       handleListOnly("links"),
+	"VLANConfig":       handleParentOnly,
+	"BridgeConfig":     handleListOnly("ports"),
 	"Layer2VIPConfig":  handleLayer2VIP,
 	"UserVolumeConfig": handleUserVolume,
 }
@@ -189,22 +196,26 @@ func handleNameOnly(refs []Ref, doc map[string]any, basePath string) []Ref {
 	return appendNameRef(refs, doc, basePath)
 }
 
-func handleNamePlusList(listKey string) multidocHandler {
+// handleListOnly emits only the list-valued slaves/ports of the doc,
+// not the doc's own .name. Used for BondConfig (its .name describes a
+// virtual bond being created by the apply; the .links[] members are
+// pre-existing physical NICs that must be present).
+func handleListOnly(listKey string) multidocHandler {
 	return func(refs []Ref, doc map[string]any, basePath string) []Ref {
-		refs = appendNameRef(refs, doc, basePath)
-
 		return appendListRefs(refs, doc, listKey, basePath+"."+listKey)
 	}
 }
 
-func handleNamePlusParent(refs []Ref, doc map[string]any, basePath string) []Ref {
-	refs = appendNameRef(refs, doc, basePath)
-
-	if parent, ok := doc["link"].(string); ok && parent != "" {
-		refs = append(refs, Ref{Kind: RefKindLink, Name: parent, Source: basePath + ".link"})
+// handleParentOnly emits only the parent reference of a VLAN doc, not
+// its own .name. The .name is the VLAN tag's child link name (a new
+// virtual link being created); the .link is the parent that must exist.
+func handleParentOnly(refs []Ref, doc map[string]any, basePath string) []Ref {
+	parent, ok := doc["link"].(string)
+	if !ok || parent == "" {
+		return refs
 	}
 
-	return refs
+	return append(refs, Ref{Kind: RefKindLink, Name: parent, Source: basePath + ".link"})
 }
 
 func handleLayer2VIP(refs []Ref, doc map[string]any, basePath string) []Ref {

pkg/applycheck/refs_test.go

@@ -125,13 +125,31 @@ func TestWalkRefs_v1_12_ExtractsLinksFromMultidocAndDiskSelector(t *testing.T) {
 		t.Fatalf("WalkRefs error: %v", err)
 	}
 
-	wantLinks := []string{"eth0", "eth0.4000", "bond0", "eth1", "eth2", "br0", "eth3"}
+	// Walker emits only references to *existing* links, not names of
+	// virtual links being created by the apply. So:
+	//   - LinkConfig.name -> emitted (override of an existing physical NIC)
+	//   - VLANConfig.link -> emitted (the parent link must exist)
+	//   - VLANConfig.name -> NOT emitted (the VLAN child is being created)
+	//   - BondConfig.links[] -> emitted (slave NICs must exist)
+	//   - BondConfig.name -> NOT emitted (the bond is being created)
+	//   - BridgeConfig.ports[] -> emitted (port NICs must exist)
+	//   - BridgeConfig.name -> NOT emitted (the bridge is being created)
+	wantLinks := []string{"eth0" /* LinkConfig.name + VLANConfig.link parent */, "eth1", "eth2" /* BondConfig.links */, "eth3" /* BridgeConfig.ports */}
 	for _, name := range wantLinks {
 		if _, ok := findRef(refs, applycheck.RefKindLink, name); !ok {
 			t.Errorf("expected link ref for %q, got refs=%+v", name, refs)
 		}
 	}
 
+	// Names of virtual links being created must NOT surface (bond0 is
+	// a BondConfig.name + a BridgeConfig.ports[] entry; the .name path
+	// should not appear, only the .ports[] one. eth0.4000 is a
+	// VLANConfig.name with no other references — should not appear at
+	// all).
+	if _, ok := findRef(refs, applycheck.RefKindLink, "eth0.4000"); ok {
+		t.Errorf("VLANConfig.name (eth0.4000) leaked as a ref; only the .link parent should validate")
+	}
+
 	// Layer2VIPConfig.link is a distinct ref site. Use a name that no other
 	// document mentions (bond1) so the assertion proves handleLayer2VIP
 	// actually emitted the ref, rather than incidentally picking it up