feat(charts): emit BridgeConfig + gate default-route fallback on configurable set

Three CodeRabbit findings on the latest review, all of the same
class: VIP-link selection paths can pick links the chart does not
emit a per-link document for, leaving Layer2VIPConfig dangling.

Resolved by implementing BridgeConfig emission properly rather
than excluding bridges as a workaround:

1. bridge_slaves discovery helper: walks lookup "links" by
   spec.slaveKind=="bridge" + spec.masterIndex, symmetric to
   bond_slaves.

2. cozystack and generic charts: replace the prior "bridge as
   gateway = fail, non-gateway bridge = silently skipped" branch
   with a typed BridgeConfig emission carrying links (ports),
   optional stp / vlan blocks from spec.bridgeMaster, plus the
   shared addresses / routes / mtu shape. Bridges now behave like
   bonds at render time.

3. Both charts: guard the default-route-link fallback in the
   discovery-derived Layer2VIPConfig block on the same
   configurable_link_names set the matched-link path uses.
   Previously the fallback would happily promote a Wireguard /
   slave / other unmanaged link if that was where the IPv4
   default route sat.

Tests:
  - TestContract_NetworkMultidoc_BridgeConfigEmitted: non-gateway
    bridge yields a BridgeConfig document.
  - TestMultiDocEmitsBridgeConfigWhenBridgeCarriesDefaultRoute:
    gateway bridge now emits BridgeConfig + routes.gateway (was
    a hard fail before).
  - TestContract_Errors_MultidocBridgeAsGateway_NoLongerFails:
    pins the negation of the prior fail-fast.
  - TestContract_NetworkMultidoc_VIPOnBridge: VIP inside a bridge
    subnet pins to that bridge (BridgeConfig + Layer2VIPConfig
    co-emit, STP + ports verified).
  - TestContract_NetworkMultidoc_VIPSkipsNonConfigurableDefaultRouteLink
    plus generic mirror: default-route fallback honours the
    configurable gate, no Layer2VIPConfig when the only resolvable
    link is Wireguard.

The unimplemented-BridgeConfig migration hint at engine.go:181 and
its generic mirror are now obsolete — the gateway-bridge fail
above them no longer fires, the typed branch handles both
gateway and non-gateway cases uniformly.

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

Author: lexfrei

charts/cozystack/templates/_helpers.tpl

@@ -249,17 +249,51 @@ link: {{ .Values.vipLink }}
 {{- $linkGateway = include "talm.discovered.gateway_by_link" $linkName }}
 {{- end }}
 {{- if eq $kind "bridge" }}
-{{- /* BridgeConfig is a separate v1alpha1 typed document the chart
-       does not yet emit. Skipping a non-gateway bridge leaves the
-       rendered config without a bridge document and the operator is
-       responsible for declaring it via a per-node body. A bridge
-       carrying the IPv4 default route, however, cannot be silently
-       skipped: that would drop every network document for the
-       gateway link and the rendered config would describe a node
-       with no working uplink. Surface a fail with the offending
-       link and the migration path. */ -}}
-{{- if $isGatewayLink }}
-{{- fail (printf "talm: discovered bridge %q is the IPv4-default link, but BridgeConfig emission is not yet implemented in the chart. Move the bridge declaration into a per-node body overlay (kind: BridgeConfig), or set Values.vipLink to a different link until bridge support lands." $linkName) }}
+{{- /* BridgeConfig emission. Discovers bridge ports (members) via
+       talm.discovered.bridge_slaves and emits a typed v1.12+
+       BridgeConfig document with the same address / route / mtu
+       shape as the other branches. STP and VLAN filtering are
+       opt-in: they are emitted only when the bridge controller
+       reported a non-nil spec.bridgeMaster.stp / spec.bridgeMaster
+       value, so a default-state bridge stays minimal. */ -}}
+{{- $bridgeMaster := $link.spec.bridgeMaster }}
+{{- $bridgePorts := fromJsonArray (include "talm.discovered.bridge_slaves" $link.spec.index) }}
+---
+apiVersion: v1alpha1
+kind: BridgeConfig
+name: {{ $linkName }}
+{{- if $bridgePorts }}
+links:
+{{- range $bridgePorts }}
+  - {{ . }}
+{{- end }}
+{{- end }}
+{{- if $bridgeMaster }}
+{{- if $bridgeMaster.stp }}
+{{- if hasKey $bridgeMaster.stp "enabled" }}
+stp:
+  enabled: {{ $bridgeMaster.stp.enabled }}
+{{- end }}
+{{- end }}
+{{- if $bridgeMaster.vlan }}
+{{- if hasKey $bridgeMaster.vlan "filtering" }}
+vlan:
+  filtering: {{ $bridgeMaster.vlan.filtering }}
+{{- end }}
+{{- end }}
+{{- end }}
+{{- if $addresses }}
+addresses:
+{{- range $addresses }}
+  - address: {{ . }}
+{{- end }}
+{{- end }}
+{{- if $linkGateway }}
+routes:
+  - gateway: {{ $linkGateway }}
+{{- end }}
+{{- if $link.spec.mtu }}
+mtu: {{ $link.spec.mtu }}
 {{- end }}
 {{- else if eq $kind "bond" }}
 {{- $bondMaster := $link.spec.bondMaster }}
@@ -377,9 +411,18 @@ mtu: {{ $link.spec.mtu }}
        emitted, matching the prior behaviour. */}}
 {{- if and .Values.floatingIP (not .Values.vipLink) (eq .MachineType "controlplane") }}
 {{- $vipLink := include "talm.discovered.link_name_for_address" .Values.floatingIP }}
+{{- /* Default-gateway fallback must also point at a configurable
+       link — otherwise an unmanaged default-route NIC (Wireguard,
+       a CNI bridge before BridgeConfig support, a slave link) would
+       silently win selection and the rendered Layer2VIPConfig would
+       dangle on a link the chart never emits a per-link document
+       for. Mirror the same configurable-link gate
+       link_name_for_address applies inside its own iteration. */ -}}
 {{- if not $vipLink }}
+{{- if has $defaultLinkName $configurableLinks }}
 {{- $vipLink = $defaultLinkName }}
 {{- end }}
+{{- end }}
 {{- if $vipLink }}
 ---
 apiVersion: v1alpha1

charts/generic/templates/_helpers.tpl

@@ -173,17 +173,51 @@ link: {{ .Values.vipLink }}
 {{- $linkGateway = include "talm.discovered.gateway_by_link" $linkName }}
 {{- end }}
 {{- if eq $kind "bridge" }}
-{{- /* BridgeConfig is a separate v1alpha1 typed document the chart
-       does not yet emit. Skipping a non-gateway bridge leaves the
-       rendered config without a bridge document and the operator is
-       responsible for declaring it via a per-node body. A bridge
-       carrying the IPv4 default route, however, cannot be silently
-       skipped: that would drop every network document for the
-       gateway link and the rendered config would describe a node
-       with no working uplink. Surface a fail with the offending
-       link and the migration path. */ -}}
-{{- if $isGatewayLink }}
-{{- fail (printf "talm: discovered bridge %q is the IPv4-default link, but BridgeConfig emission is not yet implemented in the chart. Move the bridge declaration into a per-node body overlay (kind: BridgeConfig), or set Values.vipLink to a different link until bridge support lands." $linkName) }}
+{{- /* BridgeConfig emission. Discovers bridge ports (members) via
+       talm.discovered.bridge_slaves and emits a typed v1.12+
+       BridgeConfig document with the same address / route / mtu
+       shape as the other branches. STP and VLAN filtering are
+       opt-in: they are emitted only when the bridge controller
+       reported a non-nil spec.bridgeMaster.stp / spec.bridgeMaster
+       value, so a default-state bridge stays minimal. */ -}}
+{{- $bridgeMaster := $link.spec.bridgeMaster }}
+{{- $bridgePorts := fromJsonArray (include "talm.discovered.bridge_slaves" $link.spec.index) }}
+---
+apiVersion: v1alpha1
+kind: BridgeConfig
+name: {{ $linkName }}
+{{- if $bridgePorts }}
+links:
+{{- range $bridgePorts }}
+  - {{ . }}
+{{- end }}
+{{- end }}
+{{- if $bridgeMaster }}
+{{- if $bridgeMaster.stp }}
+{{- if hasKey $bridgeMaster.stp "enabled" }}
+stp:
+  enabled: {{ $bridgeMaster.stp.enabled }}
+{{- end }}
+{{- end }}
+{{- if $bridgeMaster.vlan }}
+{{- if hasKey $bridgeMaster.vlan "filtering" }}
+vlan:
+  filtering: {{ $bridgeMaster.vlan.filtering }}
+{{- end }}
+{{- end }}
+{{- end }}
+{{- if $addresses }}
+addresses:
+{{- range $addresses }}
+  - address: {{ . }}
+{{- end }}
+{{- end }}
+{{- if $linkGateway }}
+routes:
+  - gateway: {{ $linkGateway }}
+{{- end }}
+{{- if $link.spec.mtu }}
+mtu: {{ $link.spec.mtu }}
 {{- end }}
 {{- else if eq $kind "bond" }}
 {{- $bondMaster := $link.spec.bondMaster }}
@@ -301,9 +335,17 @@ mtu: {{ $link.spec.mtu }}
        emitted, matching the prior behaviour. */}}
 {{- if and .Values.floatingIP (not .Values.vipLink) (eq .MachineType "controlplane") }}
 {{- $vipLink := include "talm.discovered.link_name_for_address" .Values.floatingIP }}
+{{- /* Default-gateway fallback must also point at a configurable
+       link — otherwise an unmanaged default-route NIC (Wireguard,
+       a slave link) would silently win selection and the rendered
+       Layer2VIPConfig would dangle on a link the chart never emits
+       a per-link document for. Mirror the same configurable-link
+       gate link_name_for_address applies inside its own iteration. */ -}}
 {{- if not $vipLink }}
+{{- if has $defaultLinkName $configurableLinks }}
 {{- $vipLink = $defaultLinkName }}
 {{- end }}
+{{- end }}
 {{- if $vipLink }}
 ---
 apiVersion: v1alpha1

charts/talm/templates/_helpers.tpl

@@ -248,6 +248,22 @@ busPath: {{ .spec.busPath }}
 {{- toJson $slaves -}}
 {{- end -}}
 
+{{- /* Get bridge member interfaces (ports) for a given bridge index.
+       Discovered via spec.slaveKind=="bridge" + spec.masterIndex
+       matching — symmetric to bond_slaves above. Returns the JSON list
+       so the multi-doc renderer can iterate without reaching back into
+       the links collection. */ -}}
+{{- define "talm.discovered.bridge_slaves" -}}
+{{- $bridgeIndex := . -}}
+{{- $slaves := list -}}
+{{- range (lookup "links" "" "").items -}}
+{{- if and (eq .spec.slaveKind "bridge") (eq (int .spec.masterIndex) (int $bridgeIndex)) -}}
+{{- $slaves = append $slaves .metadata.id -}}
+{{- end -}}
+{{- end -}}
+{{- toJson $slaves -}}
+{{- end -}}
+
 {{- /* Generate bond configuration from bondMaster spec */ -}}
 {{- define "talm.discovered.bond_config" -}}
 {{- $linkName := . -}}

pkg/engine/contract_errors_test.go

@@ -272,35 +272,24 @@ func bridgeAsGatewayLookup() func(string, string, string) (map[string]any, error
 	}
 }
 
-// Contract: multi-doc renderer aborts when a bridge carries the IPv4
-// default route. BridgeConfig emission is not yet implemented in the
-// chart; the renderer must not silently skip the gateway link (which
-// would produce a config describing a node with no working uplink).
-// The fail names the offending link, the missing feature
-// (BridgeConfig), and the recourse (per-node body overlay, or move
-// the VIP via vipLink).
-func TestContract_Errors_MultidocBridgeAsGateway(t *testing.T) {
+// Contract: multi-doc renderer no longer aborts when a bridge
+// carries the IPv4 default route — it now emits a typed
+// BridgeConfig document with the gateway. The previous shape
+// hard-failed here on the premise that BridgeConfig emission was
+// unimplemented, but the typed-document branch handles bridges
+// symmetrically to bonds today. The per-chart cross-product is
+// pinned in TestContract_NetworkMultidoc_BridgeConfigEmitted and
+// TestMultiDocEmitsBridgeConfigWhenBridgeCarriesDefaultRoute; this
+// test stays as the negative pin against the previous fail-fast.
+func TestContract_Errors_MultidocBridgeAsGateway_NoLongerFails(t *testing.T) {
 	for _, chartPath := range []string{cozystackChartPath, genericChartPath} {
 		t.Run(chartPath, func(t *testing.T) {
 			err := renderExpectingError(t, chartPath, multidocTalos, bridgeAsGatewayLookup(), map[string]any{
 				"endpoint":          testEndpoint,
 				"advertisedSubnets": []any{testAdvertisedSubnet},
 			})
-			if err == nil {
-				t.Fatalf("expected bridge-as-gateway fail, got nil")
-			}
-			msg := err.Error()
-			if !strings.Contains(msg, "talm:") {
-				t.Errorf("error must use 'talm:' prefix, got: %s", msg)
-			}
-			if !strings.Contains(msg, `"br0"`) {
-				t.Errorf("error must name the offending link 'br0', got: %s", msg)
-			}
-			if !strings.Contains(msg, "BridgeConfig") {
-				t.Errorf("error must mention BridgeConfig as the missing feature, got: %s", msg)
-			}
-			if !strings.Contains(msg, "vipLink") {
-				t.Errorf("error must suggest vipLink as a workaround, got: %s", msg)
+			if err != nil {
+				t.Errorf("bridge-as-gateway must no longer fail (BridgeConfig is emitted now), got: %v", err)
 			}
 		})
 	}

pkg/engine/contract_network_multidoc_test.go

@@ -237,19 +237,62 @@ func TestContract_NetworkMultidoc_VLANOnBondRendersVLANConfig(t *testing.T) {
 
 // === Bridge non-gateway: silent skip ===
 
-// Contract: a bridge link that is NOT the IPv4 default route is
-// skipped silently — no BridgeConfig is emitted (chart does not yet
-// support BridgeConfig output), and no LinkConfig is emitted (it is
-// not a physical NIC). The expectation is that operators who run
-// bridges declare them via per-node body overlays. The non-gateway
-// case is the silent path; the gateway case is a hard fail (pinned
-// in contract_errors_test.go).
-func TestContract_NetworkMultidoc_NonGatewayBridgeSkipped(t *testing.T) {
+// Contract: a bridge link discovered on a node is rendered as a
+// typed BridgeConfig document, symmetric to BondConfig for bonds.
+// The non-gateway case lands BridgeConfig with addresses and mtu
+// but no routes entry (no default gateway to emit). The gateway
+// case adds the routes.gateway entry — pinned by
+// TestMultiDocEmitsBridgeConfigWhenBridgeCarriesDefaultRoute.
+//
+// Prior to BridgeConfig support landing, a non-gateway bridge was
+// silently skipped (no document emitted) on the premise that the
+// feature was unimplemented; this contract pins the current
+// "always emit" shape.
+func TestContract_NetworkMultidoc_BridgeConfigEmitted(t *testing.T) {
 	out := renderCozystackWith(t, bridgeLookup(), map[string]any{
 		"advertisedSubnets": []any{testAdvertisedSubnet},
 	})
-	// No BridgeConfig — feature unimplemented.
-	assertNotContains(t, out, "kind: BridgeConfig")
+	assertContains(t, out, "kind: BridgeConfig")
+	assertContains(t, out, "name: br0")
+}
+
+// Contract: a controlplane floatingIP that lives inside the subnet
+// configured on a bridge link now legitimately lands on the bridge —
+// the bridge is fully rendered as a typed BridgeConfig document, so
+// pinning the VIP there no longer leaves it dangling without a
+// surrounding network document. Symmetric to the VLAN-child case
+// pinned in HetznerTopology_VIPOnPrivateVLAN.
+//
+// Fixture: bridgeWithClusterSubnetLookup has br0 carrying
+// 10.5.0.10/24 (global scope) and the IPv4 default route. floatingIP
+// 10.5.0.99 is inside that subnet, so link_name_for_address resolves
+// to br0; the discovery-derived Layer2VIPConfig pin'ит link=br0.
+// Without BridgeConfig emission (the prior shape), this would have
+// been a "VIP on undocumented link" symptom; now BridgeConfig
+// documents the link explicitly and the chart also emits STP
+// settings carried in spec.bridgeMaster.
+func TestContract_NetworkMultidoc_VIPOnBridge(t *testing.T) {
+	out := renderCozystackWith(t, bridgeWithClusterSubnetLookup(), map[string]any{
+		"floatingIP":        "10.5.0.99",
+		"advertisedSubnets": []any{"10.5.0.0/24"},
+	})
+
+	// BridgeConfig must be emitted alongside Layer2VIPConfig — that is
+	// the whole reason landing the VIP on a bridge is now safe.
+	assertContains(t, out, "kind: BridgeConfig")
+	assertContains(t, out, "name: br0")
+	assertContains(t, out, "- address: 10.5.0.10/24")
+	assertContains(t, out, "gateway: 10.5.0.1")
+	// STP setting from spec.bridgeMaster.stp.enabled must surface.
+	assertContains(t, out, "stp:")
+	assertContains(t, out, "enabled: true")
+	// Bridge port discovered via spec.slaveKind=="bridge" must be
+	// listed under the BridgeConfig.links.
+	assertContains(t, out, "links:")
+	assertContains(t, out, "  - eth0")
+	assertContains(t, out, "kind: Layer2VIPConfig")
+	assertContains(t, out, `name: "10.5.0.99"`)
+	assertContains(t, out, "link: br0")
 }
 
 // === Layer2VIPConfig: discovery-derived ===
@@ -602,6 +645,48 @@ func TestContract_NetworkMultidoc_VIPFailsOnInvalidFloatingIP(t *testing.T) {
 	}
 }
 
+// Contract: when the default-route-link fallback resolves to a
+// non-configurable link (Wireguard, slave NIC, anything outside the
+// {physical, bond, vlan, bridge} set), the chart MUST NOT emit
+// Layer2VIPConfig — the chart does not emit a per-link document
+// for such links, so the VIP would dangle on a link the chart
+// never configures. The fallback path mirrors the configurable-
+// link gate that link_name_for_address applies inside its own
+// iteration; matched-link selection and fallback-link selection
+// have to honour the same renderable-link set.
+//
+// Fixture: IPv4 default route on wg0 (Wireguard, not configurable).
+// floatingIP 10.99.99.99 falls outside every discovered subnet, so
+// link_name_for_address returns empty. The fallback would have
+// picked wg0 before the guard landed; with the guard it skips and
+// no Layer2VIPConfig is emitted at all.
+func TestContract_NetworkMultidoc_VIPSkipsNonConfigurableDefaultRouteLink(t *testing.T) {
+	out := renderCozystackWith(t, defaultRouteOnNonConfigurableLinkLookup(), map[string]any{
+		"floatingIP":        "10.99.99.99",
+		"advertisedSubnets": []any{testAdvertisedSubnet},
+	})
+	if strings.Contains(out, "kind: Layer2VIPConfig") {
+		t.Errorf("Layer2VIPConfig must not emit when the only resolvable link is non-configurable; got:\n%s", out)
+	}
+	if strings.Contains(out, "link: wg0") {
+		t.Errorf("VIP pinned to non-configurable wg0 — fallback must honour the configurable-link gate; got:\n%s", out)
+	}
+}
+
+// Generic-chart mirror of TestContract_NetworkMultidoc_VIPSkipsNonConfigurableDefaultRouteLink.
+func TestContract_NetworkMultidoc_Generic_VIPSkipsNonConfigurableDefaultRouteLink(t *testing.T) {
+	out := renderGenericWith(t, defaultRouteOnNonConfigurableLinkLookup(), map[string]any{
+		"floatingIP":        "10.99.99.99",
+		"advertisedSubnets": []any{testAdvertisedSubnet},
+	})
+	if strings.Contains(out, "kind: Layer2VIPConfig") {
+		t.Errorf("generic chart: Layer2VIPConfig must not emit when fallback target is non-configurable; got:\n%s", out)
+	}
+	if strings.Contains(out, "link: wg0") {
+		t.Errorf("generic chart: VIP pinned to non-configurable wg0:\n%s", out)
+	}
+}
+
 // Contract: discovery has populated the addresses table with a
 // configurable link whose subnet contains the floatingIP, but the
 // routes table has no IPv4 default route yet (fresh-boot before