From ae8688b83c7f4af36f1c15280dd82cd2a62eb9b2 Mon Sep 17 00:00:00 2001 From: Mat Kowalski Date: Fri, 10 Jul 2026 11:19:29 +0200 Subject: [PATCH 1/3] network: render FRRConfiguration for BGP-based VIP management Render a sessions-only FRRConfiguration CR named bgp-vip (namespace openshift-frr-k8s) when BGP-based VIP management is active: BareMetal platform, the BGPBasedVIPManagement feature gate enabled, and the Infrastructure CR reporting vipManagement "BGP". Peer/VIP data is read from the installer's bgp-vip-config ConfigMap (schema = baremetal-runtimecfg's FRRPeerMapping). The CR carries only the BGP sessions (neighbors, BFD profiles). VIP advertisement deliberately does not use CRD prefixes/toAdvertise: frr-k8s renders router-level prefixes as unconditional `network` statements that would defeat kube-vip's health gating, and toAdvertise cannot express "advertise redistributed routes" - without it frr-k8s renders deny-any egress prefix-lists. Advertisement therefore happens exclusively via rawConfig: `ip import-table 198` plus health-gated table-direct redistribution of routing table 198 (populated by kube-vip only for VIPs with healthy backends), filtered through route-maps/prefix-lists permitting exactly the VIP prefixes, with high-sequence per-neighbor -out permits opening egress only for the VIP prefix-lists. The CR carries no node selector: workers' frr-k8s DaemonSet consumes the same sessions and gated redistribution so router-bearing workers advertise the ingress VIP. The feature gate is defined locally and guarded via KnownFeatures until openshift/api#2923 ships the gate, and the Infrastructure vipManagement field is read unstructured until it lands in the vendored openshift/api - the feature is inert until then. Validated end to end (github.com/mkowalski/bgp-vip-demo). --- pkg/network/bgp_vip.go | 317 ++++++++++++++++++++++++++++++++++++ pkg/network/bgp_vip_test.go | 173 ++++++++++++++++++++ pkg/network/render.go | 7 + 3 files changed, 497 insertions(+) create mode 100644 pkg/network/bgp_vip.go create mode 100644 pkg/network/bgp_vip_test.go diff --git a/pkg/network/bgp_vip.go b/pkg/network/bgp_vip.go new file mode 100644 index 0000000000..69193187bb --- /dev/null +++ b/pkg/network/bgp_vip.go @@ -0,0 +1,317 @@ +package network + +import ( + "context" + "encoding/json" + "fmt" + "log" + "slices" + "strings" + + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/cluster-network-operator/pkg/bootstrap" + cnoclient "github.com/openshift/cluster-network-operator/pkg/client" + "github.com/openshift/cluster-network-operator/pkg/names" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + uns "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" +) + +// bgpBasedVIPManagementFeatureGate gates BGP-based VIP management +// (enhancement openshift/enhancements#1982). Defined locally until the +// gate lands in vendored openshift/api (openshift/api#2923); the +// KnownFeatures guard keeps this feature inert on clusters whose +// FeatureGate status does not know the gate. +const bgpBasedVIPManagementFeatureGate = configv1.FeatureGateName("BGPBasedVIPManagement") + +// bgpVIPPeer represents a BGP peer configuration from the bgp-vip-config ConfigMap. +// String-typed BFDEnabled and EBGPMultiHop ("true"/"false") match the +// installer's BGPPeerConfig serialization. +type bgpVIPPeer struct { + PeerAddress string `json:"peerAddress"` + PeerASN int64 `json:"peerASN"` + Password string `json:"password,omitempty"` + BFDEnabled string `json:"bfdEnabled,omitempty"` + EBGPMultiHop string `json:"ebgpMultiHop,omitempty"` + HoldTime string `json:"holdTime,omitempty"` + KeepaliveTime string `json:"keepaliveTime,omitempty"` +} + +// bgpVIPConfigData is the parsed content of the bgp-vip-config ConfigMap's +// config.json key. The schema matches baremetal-runtimecfg's FRRPeerMapping. +type bgpVIPConfigData struct { + LocalASN int64 `json:"localASN"` + DefaultPeers []bgpVIPPeer `json:"defaultPeers"` + Communities []string `json:"communities,omitempty"` + APIVIPs []string `json:"apiVIPs"` + IngressVIPs []string `json:"ingressVIPs"` + HostOverrides map[string][]bgpVIPPeer `json:"hostOverrides,omitempty"` +} + +// isBGPVIPManagement reports whether BGP-based VIP management is active: +// the BGPBasedVIPManagement feature gate is known and enabled, the platform +// is BareMetal, and the Infrastructure CR reports vipManagement "BGP". The +// Infrastructure status field is read unstructured until the field lands in +// the vendored openshift/api (openshift/api#2923). +func isBGPVIPManagement(client cnoclient.Client, bootstrapResult *bootstrap.BootstrapResult, featureGates featuregates.FeatureGate) (bool, error) { + if bootstrapResult == nil || bootstrapResult.Infra.PlatformStatus == nil || + bootstrapResult.Infra.PlatformType != configv1.BareMetalPlatformType || + bootstrapResult.Infra.PlatformStatus.BareMetal == nil { + return false, nil + } + // Enabled panics on gates unknown to the cluster's FeatureGate status, + // so guard the lookup (same idiom as the NoOverlayMode gate). + if !slices.Contains(featureGates.KnownFeatures(), bgpBasedVIPManagementFeatureGate) || + !featureGates.Enabled(bgpBasedVIPManagementFeatureGate) { + return false, nil + } + // Read the Infrastructure CR through the untyped dynamic client: the + // typed client (and its schemes) would silently drop the not-yet-vendored + // vipManagement field. + infra, err := client.Default().Dynamic().Resource(schema.GroupVersionResource{ + Group: "config.openshift.io", Version: "v1", Resource: "infrastructures", + }).Get(context.TODO(), "cluster", metav1.GetOptions{}) + if err != nil { + return false, fmt.Errorf("failed to get Infrastructure CR: %v", err) + } + val, found, err := uns.NestedString(infra.Object, "status", "platformStatus", "baremetal", "vipManagement") + if err != nil || !found { + return false, nil + } + return val == "BGP", nil +} + +// renderBGPVIPFRRConfiguration builds FRRConfiguration CRs for BGP-managed VIPs. +// It is a no-op when the BGPBasedVIPManagement feature gate is disabled, the +// platform is not BareMetal, or VIPManagement is not "BGP". +func renderBGPVIPFRRConfiguration(client cnoclient.Client, bootstrapResult *bootstrap.BootstrapResult, featureGates featuregates.FeatureGate) ([]*uns.Unstructured, error) { + isBGP, err := isBGPVIPManagement(client, bootstrapResult, featureGates) + if err != nil { + return nil, fmt.Errorf("failed to check VIPManagement mode: %v", err) + } + if !isBGP { + return nil, nil + } + + log.Printf("BGP VIP management is active, rendering FRRConfiguration CRs") + + // Read the bgp-vip-config ConfigMap. + cm := &corev1.ConfigMap{} + err = client.Default().CRClient().Get(context.TODO(), + types.NamespacedName{Name: "bgp-vip-config", Namespace: names.APPLIED_NAMESPACE}, cm) + if err != nil { + if apierrors.IsNotFound(err) { + // ConfigMap not yet available (may happen during early bootstrap). + log.Printf("bgp-vip-config ConfigMap not found yet, skipping FRRConfiguration rendering") + return nil, nil + } + return nil, fmt.Errorf("failed to read bgp-vip-config ConfigMap: %v", err) + } + + configJSON := cm.Data["config.json"] + if configJSON == "" { + return nil, fmt.Errorf("bgp-vip-config ConfigMap has no config.json data") + } + + var bgpConfig bgpVIPConfigData + if err := json.Unmarshal([]byte(configJSON), &bgpConfig); err != nil { + return nil, fmt.Errorf("failed to parse bgp-vip-config: %v", err) + } + + return buildFRRConfigurationObjects(bgpConfig) +} + +// buildFRRConfigurationObjects constructs FRRConfiguration unstructured objects +// from the parsed BGP VIP config data. +// +// The CR carries the BGP sessions (neighbors, BFD) only. VIP advertisement +// deliberately does NOT use CRD prefixes/toAdvertise: frr-k8s renders +// router-level prefixes as unconditional `network` statements (bypassing the +// kube-vip health gate), and toAdvertise cannot express "advertise +// redistributed routes" at all - allowed mode=all only permits prefixes +// statically declared in router.prefixes and renders deny-any prefix-lists +// otherwise. Instead, advertisement happens exclusively via the rawConfig +// redistribution of routing table 198 (see buildBGPVIPRawConfig), into which +// kube-vip only installs routes for VIPs whose backends are healthy, and +// egress is opened by rawConfig permits appended to the per-neighbor +// -out route-maps that frr-k8s always renders. +// +// The CR carries no node selector, so it applies cluster-wide: the masters' +// static-pod controller and the workers' frr-k8s DaemonSet consume the same +// sessions and gated redistribution. Advertisement stays correct per node +// because it is health-gated: the ingress VIP route only exists in table 198 +// on nodes whose router healthz passes, so only router-bearing nodes +// advertise it, and the API VIP route only ever exists on masters +// (kube-vip-api is master-only). +func buildFRRConfigurationObjects(cfg bgpVIPConfigData) ([]*uns.Unstructured, error) { + // Build neighbors list. + neighbors := []interface{}{} + for _, peer := range cfg.DefaultPeers { + neighbor := map[string]interface{}{ + "address": peer.PeerAddress, + "asn": peer.PeerASN, + } + if peer.Password != "" { + neighbor["password"] = peer.Password + } + if peer.BFDEnabled == "true" { + neighbor["bfdProfile"] = "vip-bfd" + } + if peer.EBGPMultiHop == "true" { + neighbor["ebgpMultiHop"] = true + } + neighbors = append(neighbors, neighbor) + } + + // Build BFD profiles if any peer uses BFD. + bfdProfiles := []interface{}{} + for _, peer := range cfg.DefaultPeers { + if peer.BFDEnabled == "true" { + bfdProfiles = append(bfdProfiles, map[string]interface{}{ + "name": "vip-bfd", + "receiveInterval": int64(300), + "transmitInterval": int64(300), + }) + break + } + } + + spec := map[string]interface{}{ + "bgp": map[string]interface{}{ + "bfdProfiles": bfdProfiles, + "routers": []interface{}{ + map[string]interface{}{ + "asn": cfg.LocalASN, + "neighbors": neighbors, + }, + }, + }, + "raw": map[string]interface{}{ + "rawConfig": buildBGPVIPRawConfig(cfg), + }, + } + + obj := &uns.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "frrk8s.metallb.io/v1beta1", + "kind": "FRRConfiguration", + "metadata": map[string]interface{}{ + "name": "bgp-vip", + "namespace": "openshift-frr-k8s", + "labels": map[string]interface{}{ + "app.kubernetes.io/managed-by": "cluster-network-operator", + }, + }, + "spec": spec, + }, + } + + return []*uns.Unstructured{obj}, nil +} + +// buildBGPVIPRawConfig renders the health-gated, leak-proof FRR raw config +// that advertises the VIPs, mirroring the semantics of MCO's bootstrap +// frr.conf.tmpl: redistribute routing table 198 (populated by kube-vip only +// for VIPs with healthy backends) filtered through route-maps that permit +// exactly the VIP prefixes and deny everything else, plus per-peer egress +// permits appended to the frr-k8s-generated -out route-maps. +// Address-family blocks, route-maps, prefix-lists, and egress permits are +// emitted only for families that have VIPs. +// A VIP containing ":" is IPv6 (/128), otherwise IPv4 (/32). +func buildBGPVIPRawConfig(cfg bgpVIPConfigData) string { + var v4Prefixes, v6Prefixes []string + for _, vip := range slices.Concat(cfg.APIVIPs, cfg.IngressVIPs) { + if strings.Contains(vip, ":") { + v6Prefixes = append(v6Prefixes, vip+"/128") + } else { + v4Prefixes = append(v4Prefixes, vip+"/32") + } + } + + var b strings.Builder + // Zebra only tracks kernel routing tables other than main when + // explicitly instructed. Without import-table zebra never sees the + // kube-vip routes in table 198 and the table-direct redistribution + // below exports nothing. The bootstrap config (MCO's + // manifests/on-prem/frr.conf.tmpl) carries the same directive. + b.WriteString("ip import-table 198\n") + fmt.Fprintf(&b, "router bgp %d\n", cfg.LocalASN) + if len(v4Prefixes) > 0 { + b.WriteString(" address-family ipv4 unicast\n") + b.WriteString(" redistribute table-direct 198 route-map BGP-VIP-ROUTES-V4\n") + b.WriteString(" exit-address-family\n") + } + if len(v6Prefixes) > 0 { + b.WriteString(" address-family ipv6 unicast\n") + b.WriteString(" redistribute table-direct 198 route-map BGP-VIP-ROUTES-V6\n") + b.WriteString(" exit-address-family\n") + } + if len(v4Prefixes) > 0 { + b.WriteString("route-map BGP-VIP-ROUTES-V4 permit 10\n") + b.WriteString(" match ip address prefix-list BGP-VIP-PREFIXES-V4\n") + b.WriteString("route-map BGP-VIP-ROUTES-V4 deny 20\n") + } + if len(v6Prefixes) > 0 { + b.WriteString("route-map BGP-VIP-ROUTES-V6 permit 10\n") + b.WriteString(" match ipv6 address prefix-list BGP-VIP-PREFIXES-V6\n") + b.WriteString("route-map BGP-VIP-ROUTES-V6 deny 20\n") + } + for i, prefix := range v4Prefixes { + fmt.Fprintf(&b, "ip prefix-list BGP-VIP-PREFIXES-V4 seq %d permit %s\n", 10*(i+1), prefix) + } + for i, prefix := range v6Prefixes { + fmt.Fprintf(&b, "ipv6 prefix-list BGP-VIP-PREFIXES-V6 seq %d permit %s\n", 10*(i+1), prefix) + } + // Open egress for exactly the VIP prefixes. frr-k8s always renders a + // per-neighbor `route-map -out` (for non-VRF, non-interface peers + // the neighbor ID naming the map is the peer address, see frr-k8s + // internal/frr/config.go NeighborConfig.ID); with no toAdvertise its own + // permit seqs match deny-any prefix-lists, because the CRD cannot express + // "advertise redistributed routes" - allowed mode=all only covers + // prefixes statically declared in router.prefixes (see frr-k8s + // internal/controller/api_to_config.go prefixesToAdvertiseForFamily). + // A route-map entry whose prefix-list denies is a no-match, which falls + // through to the NEXT sequence rather than rejecting, so these high-seq + // permits open egress ONLY for the VIP prefix-lists while everything + // else stays implicitly denied - health-gated by the table-direct + // redistribution above and leak-proof. + for _, peer := range cfg.DefaultPeers { + if len(v4Prefixes) > 0 { + fmt.Fprintf(&b, "route-map %s-out permit 4000\n", peer.PeerAddress) + b.WriteString(" match ip address prefix-list BGP-VIP-PREFIXES-V4\n") + } + if len(v6Prefixes) > 0 { + fmt.Fprintf(&b, "route-map %s-out permit 4001\n", peer.PeerAddress) + b.WriteString(" match ipv6 address prefix-list BGP-VIP-PREFIXES-V6\n") + } + } + return strings.TrimSuffix(b.String(), "\n") +} + +// checkBGPSessionsEstablished checks whether all BGPSessionState resources +// report an "Established" status. This can be called from the reconciler +// after rendering to set degraded conditions. +func checkBGPSessionsEstablished(client cnoclient.Client) (bool, error) { + sessionList := &uns.UnstructuredList{} + sessionList.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "frrk8s.metallb.io", + Version: "v1beta1", + Kind: "BGPSessionStateList", + }) + + if err := client.Default().CRClient().List(context.TODO(), sessionList); err != nil { + return false, err + } + + for _, session := range sessionList.Items { + status, _, _ := uns.NestedString(session.Object, "status", "bgpStatus") + if status != "Established" { + return false, nil + } + } + return len(sessionList.Items) > 0, nil +} diff --git a/pkg/network/bgp_vip_test.go b/pkg/network/bgp_vip_test.go new file mode 100644 index 0000000000..4e0c25c5de --- /dev/null +++ b/pkg/network/bgp_vip_test.go @@ -0,0 +1,173 @@ +package network + +import ( + "encoding/json" + "testing" + + . "github.com/onsi/gomega" + uns "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +func TestBuildFRRConfigurationObjects(t *testing.T) { + g := NewGomegaWithT(t) + raw := `{"localASN":64512,"defaultPeers":[{"peerAddress":"192.168.111.1","peerASN":64513}],"apiVIPs":["192.168.111.5"],"ingressVIPs":["192.168.111.4"]}` + var cfg bgpVIPConfigData + g.Expect(json.Unmarshal([]byte(raw), &cfg)).To(Succeed()) + g.Expect(cfg.DefaultPeers).To(HaveLen(1)) + + objs, err := buildFRRConfigurationObjects(cfg) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(objs).To(HaveLen(1)) + g.Expect(objs[0].GetName()).To(Equal("bgp-vip")) + // No node selector: the CR applies to all nodes so workers' frr-k8s + // DaemonSet consumes the same sessions and gated redistribution. + _, found, err := uns.NestedMap(objs[0].Object, "spec", "nodeSelector") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(found).To(BeFalse()) + routers, found, err := uns.NestedSlice(objs[0].Object, "spec", "bgp", "routers") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(found).To(BeTrue()) + g.Expect(routers).To(HaveLen(1)) + neighbors, found, err := uns.NestedSlice(routers[0].(map[string]interface{}), "neighbors") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(found).To(BeTrue()) + g.Expect(neighbors).To(HaveLen(1)) + + // VIP advertisement must NOT use CRD prefixes: frr-k8s renders those as + // unconditional `network` statements, which bypass the kube-vip health + // gate (routing table 198). No router-level prefixes, no prefixes list + // under toAdvertise. + _, found, err = uns.NestedStringSlice(routers[0].(map[string]interface{}), "prefixes") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(found).To(BeFalse()) + neighbor := neighbors[0].(map[string]interface{}) + // No toAdvertise at all: the CRD cannot express "advertise redistributed + // routes" (allowed mode=all only covers prefixes statically declared in + // router.prefixes and renders deny-any lists otherwise). Egress is opened + // instead by rawConfig permits appended to the generated -out + // route-maps, asserted below. + _, found, err = uns.NestedMap(neighbor, "toAdvertise") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(found).To(BeFalse()) + g.Expect(neighbor["address"]).To(Equal("192.168.111.1")) + g.Expect(neighbor["asn"]).To(Equal(int64(64513))) + + // Advertisement happens exclusively via health-gated redistribution of + // routing table 198, filtered to exactly the VIP prefixes. + rawConfig, found, err := uns.NestedString(objs[0].Object, "spec", "raw", "rawConfig") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(found).To(BeTrue()) + // Zebra only tracks non-main kernel tables when told to: without + // import-table the table-direct redistribution exports nothing. + g.Expect(rawConfig).To(HavePrefix("ip import-table 198\n")) + g.Expect(rawConfig).To(ContainSubstring("router bgp 64512")) + g.Expect(rawConfig).To(ContainSubstring("redistribute table-direct 198 route-map BGP-VIP-ROUTES-V4")) + g.Expect(rawConfig).To(ContainSubstring("route-map BGP-VIP-ROUTES-V4 permit 10")) + g.Expect(rawConfig).To(ContainSubstring("match ip address prefix-list BGP-VIP-PREFIXES-V4")) + g.Expect(rawConfig).To(ContainSubstring("route-map BGP-VIP-ROUTES-V4 deny 20")) + g.Expect(rawConfig).To(ContainSubstring("ip prefix-list BGP-VIP-PREFIXES-V4 seq 10 permit 192.168.111.5/32")) + g.Expect(rawConfig).To(ContainSubstring("ip prefix-list BGP-VIP-PREFIXES-V4 seq 20 permit 192.168.111.4/32")) + // Egress: frr-k8s renders per-neighbor -out route-maps whose own + // permit seqs match deny-any prefix-lists (no toAdvertise); a prefix-list + // deny is a no-match, so processing falls through to these appended + // high-seq permits, opening egress ONLY for the VIP prefix-lists. + g.Expect(rawConfig).To(ContainSubstring("route-map 192.168.111.1-out permit 4000\n match ip address prefix-list BGP-VIP-PREFIXES-V4")) + // No IPv6 VIPs: the v6 address-family, route-map, prefix-list, and + // egress-permit blocks must be omitted entirely. + g.Expect(rawConfig).NotTo(ContainSubstring("-out permit 4001")) + g.Expect(rawConfig).NotTo(ContainSubstring("address-family ipv6")) + g.Expect(rawConfig).NotTo(ContainSubstring("BGP-VIP-ROUTES-V6")) + g.Expect(rawConfig).NotTo(ContainSubstring("BGP-VIP-PREFIXES-V6")) +} + +// TestBuildFRRConfigurationObjectsAllOptionalFields locks the full +// installer-shaped config.json payload, with every optional field populated +// exactly as the installer emits them (string-typed bfdEnabled/ebgpMultiHop, +// see installer pkg/types/baremetal BGPPeerConfig). +func TestBuildFRRConfigurationObjectsAllOptionalFields(t *testing.T) { + g := NewGomegaWithT(t) + raw := `{"localASN":64512,"defaultPeers":[{"peerAddress":"192.168.111.1","peerASN":64513,"password":"s3cret","bfdEnabled":"true","ebgpMultiHop":"true","holdTime":"90s","keepaliveTime":"30s"}],"communities":["64512:100"],"apiVIPs":["192.168.111.5"],"ingressVIPs":["192.168.111.4"],"hostOverrides":{"master-0":[{"peerAddress":"192.168.1.1","peerASN":64513}]}}` + var cfg bgpVIPConfigData + g.Expect(json.Unmarshal([]byte(raw), &cfg)).To(Succeed()) + g.Expect(cfg.DefaultPeers).To(HaveLen(1)) + g.Expect(cfg.HostOverrides).To(HaveLen(1)) + g.Expect(cfg.HostOverrides["master-0"]).To(HaveLen(1)) + + objs, err := buildFRRConfigurationObjects(cfg) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(objs).To(HaveLen(1)) + + routers, found, err := uns.NestedSlice(objs[0].Object, "spec", "bgp", "routers") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(found).To(BeTrue()) + g.Expect(routers).To(HaveLen(1)) + neighbors, found, err := uns.NestedSlice(routers[0].(map[string]interface{}), "neighbors") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(found).To(BeTrue()) + g.Expect(neighbors).To(HaveLen(1)) + + neighbor := neighbors[0].(map[string]interface{}) + // ebgpMultiHop must be emitted as a real bool in the CR even though the + // installer serializes it as a string. + g.Expect(neighbor["ebgpMultiHop"]).To(Equal(true)) + g.Expect(neighbor["password"]).To(Equal("s3cret")) + g.Expect(neighbor["bfdProfile"]).To(Equal("vip-bfd")) + // No toAdvertise (CRD egress surface cannot cover redistributed routes); + // advertisement content is controlled exclusively by gated redistribution + // plus the appended -out egress permits in rawConfig. No router-level + // prefixes either. + _, found, err = uns.NestedMap(neighbor, "toAdvertise") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(found).To(BeFalse()) + _, found, err = uns.NestedStringSlice(routers[0].(map[string]interface{}), "prefixes") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(found).To(BeFalse()) + + rawConfig, found, err := uns.NestedString(objs[0].Object, "spec", "raw", "rawConfig") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(found).To(BeTrue()) + g.Expect(rawConfig).To(HavePrefix("ip import-table 198\n")) + g.Expect(rawConfig).To(ContainSubstring("redistribute table-direct 198 route-map BGP-VIP-ROUTES-V4")) + g.Expect(rawConfig).To(ContainSubstring("route-map BGP-VIP-ROUTES-V4 permit 10")) + g.Expect(rawConfig).To(ContainSubstring("route-map BGP-VIP-ROUTES-V4 deny 20")) + g.Expect(rawConfig).To(ContainSubstring("ip prefix-list BGP-VIP-PREFIXES-V4 seq 10 permit 192.168.111.5/32")) + g.Expect(rawConfig).To(ContainSubstring("ip prefix-list BGP-VIP-PREFIXES-V4 seq 20 permit 192.168.111.4/32")) + g.Expect(rawConfig).To(ContainSubstring("route-map 192.168.111.1-out permit 4000\n match ip address prefix-list BGP-VIP-PREFIXES-V4")) + + bfdProfiles, found, err := uns.NestedSlice(objs[0].Object, "spec", "bgp", "bfdProfiles") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(found).To(BeTrue()) + g.Expect(bfdProfiles).To(HaveLen(1)) +} + +// TestBuildFRRConfigurationObjectsDualStack locks the general address-family +// handling: VIPs containing ":" are IPv6 (/128) and go to the V6 route-map +// and prefix-list; both families are emitted when both have VIPs. +func TestBuildFRRConfigurationObjectsDualStack(t *testing.T) { + g := NewGomegaWithT(t) + raw := `{"localASN":64512,"defaultPeers":[{"peerAddress":"192.168.111.1","peerASN":64513}],"apiVIPs":["192.168.111.5","fd2e:6f44:5dd8::5"],"ingressVIPs":["192.168.111.4","fd2e:6f44:5dd8::4"]}` + var cfg bgpVIPConfigData + g.Expect(json.Unmarshal([]byte(raw), &cfg)).To(Succeed()) + + objs, err := buildFRRConfigurationObjects(cfg) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(objs).To(HaveLen(1)) + + rawConfig, found, err := uns.NestedString(objs[0].Object, "spec", "raw", "rawConfig") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(found).To(BeTrue()) + g.Expect(rawConfig).To(HavePrefix("ip import-table 198\n")) + g.Expect(rawConfig).To(ContainSubstring("redistribute table-direct 198 route-map BGP-VIP-ROUTES-V4")) + g.Expect(rawConfig).To(ContainSubstring("redistribute table-direct 198 route-map BGP-VIP-ROUTES-V6")) + g.Expect(rawConfig).To(ContainSubstring("route-map BGP-VIP-ROUTES-V6 permit 10")) + g.Expect(rawConfig).To(ContainSubstring("match ipv6 address prefix-list BGP-VIP-PREFIXES-V6")) + g.Expect(rawConfig).To(ContainSubstring("route-map BGP-VIP-ROUTES-V6 deny 20")) + g.Expect(rawConfig).To(ContainSubstring("ip prefix-list BGP-VIP-PREFIXES-V4 seq 10 permit 192.168.111.5/32")) + g.Expect(rawConfig).To(ContainSubstring("ip prefix-list BGP-VIP-PREFIXES-V4 seq 20 permit 192.168.111.4/32")) + g.Expect(rawConfig).To(ContainSubstring("ipv6 prefix-list BGP-VIP-PREFIXES-V6 seq 10 permit fd2e:6f44:5dd8::5/128")) + g.Expect(rawConfig).To(ContainSubstring("ipv6 prefix-list BGP-VIP-PREFIXES-V6 seq 20 permit fd2e:6f44:5dd8::4/128")) + // Both families have VIPs: the peer's egress route-map gets both the v4 + // and v6 high-seq permits. + g.Expect(rawConfig).To(ContainSubstring("route-map 192.168.111.1-out permit 4000\n match ip address prefix-list BGP-VIP-PREFIXES-V4")) + g.Expect(rawConfig).To(ContainSubstring("route-map 192.168.111.1-out permit 4001\n match ipv6 address prefix-list BGP-VIP-PREFIXES-V6")) +} diff --git a/pkg/network/render.go b/pkg/network/render.go index ffa2579e0f..b7fdb0fd98 100644 --- a/pkg/network/render.go +++ b/pkg/network/render.go @@ -135,6 +135,13 @@ func Render(operConf *operv1.NetworkSpec, clusterConf *configv1.NetworkSpec, man } objs = append(objs, o...) + // render BGP VIP FRRConfiguration CRs if BGP VIP management is active + o, err = renderBGPVIPFRRConfiguration(client, bootstrapResult, featureGates) + if err != nil { + return nil, progressing, err + } + objs = append(objs, o...) + // render networking console plugin o, err = renderNetworkingConsolePlugin(manifestDir, bootstrapResult) if err != nil { From cd0bd10c21bb56af5cf0562b35c098187a77ef4f Mon Sep 17 00:00:00 2001 From: Mat Kowalski Date: Fri, 10 Jul 2026 11:22:42 +0200 Subject: [PATCH 2/3] network: frr-k8s DaemonSet avoids control plane nodes under BGP VIP management Under BGP VIP management the masters run the frr-k8s static pod (deployed by MCO), which owns the FRR sessions for the VIPs; the frr-k8s DaemonSet must therefore not schedule there. Label-based approaches fail: NodeRestriction prevents nodes from setting the exclusion label on themselves, and any external labeler races with the DaemonSet controller at node bring-up. A role-based required node-affinity (node-role.kubernetes.io/master DoesNotExist) has neither problem, so render it into the DaemonSet whenever BGP VIP management is active. renderAdditionalRoutingCapabilities gains the client, bootstrap result, and feature gates needed to make that call, reusing the same isBGPVIPManagement helper as the FRRConfiguration rendering. --- bindata/network/frr-k8s/frr-k8s.yaml | 9 ++++ pkg/network/render.go | 11 +++- pkg/network/render_test.go | 77 +++++++++++++++++++++++++++- 3 files changed, 94 insertions(+), 3 deletions(-) diff --git a/bindata/network/frr-k8s/frr-k8s.yaml b/bindata/network/frr-k8s/frr-k8s.yaml index 102c5007d9..6a66108da3 100644 --- a/bindata/network/frr-k8s/frr-k8s.yaml +++ b/bindata/network/frr-k8s/frr-k8s.yaml @@ -294,6 +294,15 @@ spec: requests: cpu: 10m memory: 20Mi +{{ if .BGPVIPManagement }} + affinity: + nodeAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + nodeSelectorTerms: + - matchExpressions: + - key: node-role.kubernetes.io/master + operator: DoesNotExist +{{ end }} nodeSelector: kubernetes.io/os: linux tolerations: diff --git a/pkg/network/render.go b/pkg/network/render.go index b7fdb0fd98..d7ad672b99 100644 --- a/pkg/network/render.go +++ b/pkg/network/render.go @@ -129,7 +129,7 @@ func Render(operConf *operv1.NetworkSpec, clusterConf *configv1.NetworkSpec, man } objs = append(objs, o...) - o, err = renderAdditionalRoutingCapabilities(operConf, manifestDir) + o, err = renderAdditionalRoutingCapabilities(operConf, manifestDir, client, bootstrapResult, featureGates) if err != nil { return nil, progressing, err } @@ -856,10 +856,16 @@ func registerNetworkingConsolePlugin(bootstrapResult *bootstrap.BootstrapResult, }) } -func renderAdditionalRoutingCapabilities(conf *operv1.NetworkSpec, manifestDir string) ([]*uns.Unstructured, error) { +func renderAdditionalRoutingCapabilities(conf *operv1.NetworkSpec, manifestDir string, client cnoclient.Client, bootstrapResult *bootstrap.BootstrapResult, featureGates featuregates.FeatureGate) ([]*uns.Unstructured, error) { if conf == nil || conf.AdditionalRoutingCapabilities == nil { return nil, nil } + // Under BGP VIP management the static FRR pods own the control plane + // nodes, so the frr-k8s DaemonSet must avoid masters by role. + bgpVIP, err := isBGPVIPManagement(client, bootstrapResult, featureGates) + if err != nil { + return nil, fmt.Errorf("failed to check VIPManagement mode: %v", err) + } var out []*uns.Unstructured for _, provider := range conf.AdditionalRoutingCapabilities.Providers { switch provider { @@ -869,6 +875,7 @@ func renderAdditionalRoutingCapabilities(conf *operv1.NetworkSpec, manifestDir s data.Data["ReleaseVersion"] = os.Getenv("RELEASE_VERSION") data.Data["NoOverlayManagedEnabled"] = conf.DefaultNetwork.OVNKubernetesConfig != nil && conf.DefaultNetwork.OVNKubernetesConfig.BGPManagedConfig.BGPTopology != "" + data.Data["BGPVIPManagement"] = bgpVIP objs, err := render.RenderDir(filepath.Join(manifestDir, "network/frr-k8s"), &data) if err != nil { return nil, fmt.Errorf("failed to render frr-k8s manifests: %w", err) diff --git a/pkg/network/render_test.go b/pkg/network/render_test.go index c9ce1f099e..bbf4f8e03b 100644 --- a/pkg/network/render_test.go +++ b/pkg/network/render_test.go @@ -18,6 +18,8 @@ import ( operv1 "github.com/openshift/api/operator/v1" "github.com/openshift/cluster-network-operator/pkg/bootstrap" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + uns "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" ) var manifestDir = "../../bindata" @@ -599,7 +601,7 @@ func Test_renderAdditionalRoutingCapabilities(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := renderAdditionalRoutingCapabilities(tt.args.operConf, manifestDir) + got, err := renderAdditionalRoutingCapabilities(tt.args.operConf, manifestDir, nil, nil, getDefaultFeatureGatesWithDualStack()) if !reflect.DeepEqual(tt.expectedErr, err) { t.Errorf("renderAdditionalRoutingCapabilities() err = %v, want %v", err, tt.expectedErr) } @@ -607,3 +609,76 @@ func Test_renderAdditionalRoutingCapabilities(t *testing.T) { }) } } + +// Test_renderAdditionalRoutingCapabilitiesBGPVIPManagement verifies the +// frr-k8s DaemonSet affinity: with BGP VIP management active the DaemonSet +// must avoid control plane nodes by role (the static FRR pods own them); +// otherwise no affinity is rendered at all. +func Test_renderAdditionalRoutingCapabilitiesBGPVIPManagement(t *testing.T) { + g := NewGomegaWithT(t) + + operConf := &operv1.NetworkSpec{ + AdditionalRoutingCapabilities: &operv1.AdditionalRoutingCapabilities{ + Providers: []operv1.RoutingCapabilitiesProvider{ + operv1.RoutingCapabilitiesProviderFRR, + }, + }, + } + featureGates := featuregates.NewFeatureGate( + []configv1.FeatureGateName{bgpBasedVIPManagementFeatureGate}, + []configv1.FeatureGateName{}, + ) + bootstrapResult := &bootstrap.BootstrapResult{ + Infra: bootstrap.InfraStatus{ + PlatformType: configv1.BareMetalPlatformType, + PlatformStatus: &configv1.PlatformStatus{ + Type: configv1.BareMetalPlatformType, + BareMetal: &configv1.BareMetalPlatformStatus{}, + }, + }, + } + // The Infrastructure vipManagement status field is not in the vendored + // openshift/api yet (openshift/api#2923), so seed it unstructured - + // exactly the shape isBGPVIPManagement reads. + infra := &uns.Unstructured{} + infra.SetGroupVersionKind(schema.GroupVersionKind{Group: "config.openshift.io", Version: "v1", Kind: "Infrastructure"}) + infra.SetName("cluster") + g.Expect(uns.SetNestedField(infra.Object, "BGP", "status", "platformStatus", "baremetal", "vipManagement")).To(Succeed()) + client := fake.NewFakeClient(infra) + + daemonSetAffinity := func(objs []*uns.Unstructured) (map[string]interface{}, bool) { + for _, obj := range objs { + if obj.GetKind() == "DaemonSet" && obj.GetName() == "frr-k8s" { + affinity, found, err := uns.NestedMap(obj.Object, "spec", "template", "spec", "affinity") + g.Expect(err).NotTo(HaveOccurred()) + return affinity, found + } + } + t.Fatal("frr-k8s DaemonSet not found in rendered objects") + return nil, false + } + + // BGP VIP management active: the DaemonSet must avoid masters by role. + got, err := renderAdditionalRoutingCapabilities(operConf, manifestDir, client, bootstrapResult, featureGates) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(got).To(HaveLen(21)) + affinity, found := daemonSetAffinity(got) + g.Expect(found).To(BeTrue()) + terms, found, err := uns.NestedSlice(affinity, "nodeAffinity", "requiredDuringSchedulingIgnoredDuringExecution", "nodeSelectorTerms") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(found).To(BeTrue()) + g.Expect(terms).To(HaveLen(1)) + matchExpressions := terms[0].(map[string]interface{})["matchExpressions"].([]interface{}) + g.Expect(matchExpressions).To(HaveLen(1)) + g.Expect(matchExpressions[0]).To(Equal(map[string]interface{}{ + "key": "node-role.kubernetes.io/master", + "operator": "DoesNotExist", + })) + + // BGP VIP management inactive (nil bootstrap result): no affinity. + got, err = renderAdditionalRoutingCapabilities(operConf, manifestDir, client, nil, featureGates) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(got).To(HaveLen(21)) + _, found = daemonSetAffinity(got) + g.Expect(found).To(BeFalse()) +} From 70cd187e14d899018e78eec78c6e46cc72ca5351 Mon Sep 17 00:00:00 2001 From: Mat Kowalski Date: Fri, 10 Jul 2026 11:23:13 +0200 Subject: [PATCH 3/3] network: RBAC for the frr-k8s static pod node credentials The frr-k8s static pod on control plane nodes has no ServiceAccount; its controller and frr-status containers authenticate with the node kubeconfig (/etc/kubernetes/kubeconfig), whose identity is the MCO node-bootstrapper ServiceAccount (verified live via oc auth whoami). Grant that identity read access to FRRConfigurations and FRRK8sConfigurations, write access to the node state CRs (FRRNodeStates, BGPSessionStates and their statuses), and the namespace-scoped secrets/pods reads that 002-rbac.yaml gives the DaemonSet service account, so the static pod controller's informer caches can sync. Known limitation: these grants are not scoped per-node - any holder of the node-bootstrapper credential can write any node's state CRs. Admission-level enforcement is needed before GA. --- .../network/frr-k8s/003-static-pod-rbac.yaml | 106 ++++++++++++++++++ pkg/network/render_test.go | 6 +- 2 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 bindata/network/frr-k8s/003-static-pod-rbac.yaml diff --git a/bindata/network/frr-k8s/003-static-pod-rbac.yaml b/bindata/network/frr-k8s/003-static-pod-rbac.yaml new file mode 100644 index 0000000000..3dca54eb9f --- /dev/null +++ b/bindata/network/frr-k8s/003-static-pod-rbac.yaml @@ -0,0 +1,106 @@ +# RBAC for the frr-k8s static pod on control plane nodes (BGP VIP management). +# The static pod has no ServiceAccount; its controller and frr-status +# containers authenticate with the node kubeconfig +# (/etc/kubernetes/kubeconfig), whose identity is the MCO node-bootstrapper +# ServiceAccount (verified live via oc auth whoami). Grant it read access to +# FRRConfigurations/FRRK8sConfigurations and write access to node state CRs, +# plus the namespace-scoped reads (secrets for BGP session passwords, pods) +# that 002-rbac.yaml grants the DaemonSet service account, so the +# controller's informer caches can sync. +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: frr-k8s-static-pod +rules: +- apiGroups: + - frrk8s.metallb.io + resources: + - frrconfigurations + - frrk8sconfigurations + verbs: + - get + - list + - watch +- apiGroups: + - frrk8s.metallb.io + resources: + - frrnodestates + - bgpsessionstates + verbs: + - get + - list + - watch + - create + - update + - patch + - delete +- apiGroups: + - frrk8s.metallb.io + resources: + - frrnodestates/status + - bgpsessionstates/status + verbs: + - get + - update + - patch +- apiGroups: + - "" + resources: + - nodes + verbs: + - get + - list + - watch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: frr-k8s-static-pod +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: frr-k8s-static-pod +subjects: +- kind: ServiceAccount + name: node-bootstrapper + namespace: openshift-machine-config-operator +--- +# Namespace-scoped reads mirroring the DaemonSet SA's Role in 002-rbac.yaml +# (read-only: the static pod controller watches Secrets for BGP session +# passwords and pods; it does not need the DaemonSet's secrets update grant). +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: frr-k8s-static-pod + namespace: openshift-frr-k8s +rules: +- apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch +- apiGroups: + - "" + resources: + - pods + verbs: + - get + - list + - watch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: frr-k8s-static-pod + namespace: openshift-frr-k8s +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: frr-k8s-static-pod +subjects: +- kind: ServiceAccount + name: node-bootstrapper + namespace: openshift-machine-config-operator diff --git a/pkg/network/render_test.go b/pkg/network/render_test.go index bbf4f8e03b..63f8972bda 100644 --- a/pkg/network/render_test.go +++ b/pkg/network/render_test.go @@ -595,7 +595,7 @@ func Test_renderAdditionalRoutingCapabilities(t *testing.T) { }, }, }, - want: 21, + want: 25, expectedErr: nil, }, } @@ -661,7 +661,7 @@ func Test_renderAdditionalRoutingCapabilitiesBGPVIPManagement(t *testing.T) { // BGP VIP management active: the DaemonSet must avoid masters by role. got, err := renderAdditionalRoutingCapabilities(operConf, manifestDir, client, bootstrapResult, featureGates) g.Expect(err).NotTo(HaveOccurred()) - g.Expect(got).To(HaveLen(21)) + g.Expect(got).To(HaveLen(25)) affinity, found := daemonSetAffinity(got) g.Expect(found).To(BeTrue()) terms, found, err := uns.NestedSlice(affinity, "nodeAffinity", "requiredDuringSchedulingIgnoredDuringExecution", "nodeSelectorTerms") @@ -678,7 +678,7 @@ func Test_renderAdditionalRoutingCapabilitiesBGPVIPManagement(t *testing.T) { // BGP VIP management inactive (nil bootstrap result): no affinity. got, err = renderAdditionalRoutingCapabilities(operConf, manifestDir, client, nil, featureGates) g.Expect(err).NotTo(HaveOccurred()) - g.Expect(got).To(HaveLen(21)) + g.Expect(got).To(HaveLen(25)) _, found = daemonSetAffinity(got) g.Expect(found).To(BeFalse()) }