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..1a2550712e --- /dev/null +++ b/bindata/network/frr-k8s/003-static-pod-rbac.yaml @@ -0,0 +1,100 @@ +{{ if .BGPVIPManagement }} +# RBAC for the frr-k8s static pod on control plane nodes (BGP VIP +# management). The static pod authenticates with the node kubeconfig, whose +# identity is the MCO node-bootstrapper ServiceAccount. +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 +# No delete: stale state objects are removed by frr-k8s's status cleaner. +- apiGroups: + - frrk8s.metallb.io + resources: + - frrnodestates + - bgpsessionstates + verbs: + - get + - list + - watch + - create + - update + - patch +- 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 +--- +# Read-only subset of the DaemonSet SA's Role in 002-rbac.yaml. +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 +{{ end }} diff --git a/bindata/network/frr-k8s/frr-k8s.yaml b/bindata/network/frr-k8s/frr-k8s.yaml index 102c5007d9..829b768283 100644 --- a/bindata/network/frr-k8s/frr-k8s.yaml +++ b/bindata/network/frr-k8s/frr-k8s.yaml @@ -294,6 +294,17 @@ spec: requests: cpu: 10m memory: 20Mi +{{ if .BGPVIPManagement }} + # Control plane nodes run the MCO-rendered frr-k8s static pod instead; + # on compact/SNO this DaemonSet correctly matches zero nodes. + affinity: + nodeAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + nodeSelectorTerms: + - matchExpressions: + - key: node-role.kubernetes.io/master + operator: DoesNotExist +{{ end }} nodeSelector: kubernetes.io/os: linux tolerations: diff --git a/bindata/network/frr-k8s/master-monitor.yaml b/bindata/network/frr-k8s/master-monitor.yaml new file mode 100644 index 0000000000..6e4827b0e8 --- /dev/null +++ b/bindata/network/frr-k8s/master-monitor.yaml @@ -0,0 +1,189 @@ +{{ if .BGPVIPManagement }} +# Metrics companion for the frr-k8s static pods on control plane nodes. +# Static pods cannot mount Secrets, so the FRR metrics exporter runs here +# instead, reading the static pod's FRR through hostPath sockets. Only the +# dataplane needs static pod survivability; metrics do not. +apiVersion: apps/v1 +kind: DaemonSet +metadata: + name: frr-k8s-master-monitor + namespace: openshift-frr-k8s + labels: + app: frr-k8s + component: frr-k8s-master-monitor + annotations: + release.openshift.io/version: "{{.ReleaseVersion}}" +spec: + selector: + matchLabels: + app: frr-k8s + component: frr-k8s-master-monitor + template: + metadata: + labels: + app: frr-k8s + component: frr-k8s-master-monitor + spec: + serviceAccountName: frr-k8s-daemon + nodeSelector: + kubernetes.io/os: linux + node-role.kubernetes.io/master: "" + tolerations: + - operator: "Exists" + initContainers: + - name: cp-metrics + image: {{.FRRK8sImage}} + command: ["/bin/sh", "-c", "cp -f /frr-metrics /etc/frr_metrics/"] + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + capabilities: + drop: ["ALL"] + seccompProfile: + type: RuntimeDefault + terminationMessagePolicy: FallbackToLogsOnError + volumeMounts: + - name: metrics + mountPath: /etc/frr_metrics + containers: + - name: frr-metrics + image: {{.FRRK8sImage}} + command: ["/etc/frr_metrics/frr-metrics"] + args: + - --metrics-port=9141 + - --metrics-bind-address=0.0.0.0 + - --tls-cert-file=/etc/metrics/tls.crt + - --tls-private-key-file=/etc/metrics/tls.key + ports: + - containerPort: 9141 + name: frrmetricshttps + # Root without extra capabilities except DAC_OVERRIDE: vtysh requires + # root, and the FRR vty sockets are owned by the frr user (100). + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + capabilities: + drop: ["ALL"] + add: ["DAC_OVERRIDE"] + seccompProfile: + type: RuntimeDefault + readinessProbe: + tcpSocket: + port: 9141 + initialDelaySeconds: 5 + periodSeconds: 10 + livenessProbe: + tcpSocket: + port: 9141 + initialDelaySeconds: 10 + periodSeconds: 30 + resources: + requests: + cpu: 10m + memory: 20Mi + terminationMessagePolicy: FallbackToLogsOnError + volumeMounts: + - name: frr-sockets + mountPath: /var/run/frr + - name: frr-conf + mountPath: /etc/frr + - name: metrics + mountPath: /etc/frr_metrics + - name: metrics-certs + mountPath: /etc/metrics + readOnly: true + volumes: + - name: frr-sockets + hostPath: + path: /run/frr-k8s/sockets + type: DirectoryOrCreate + - name: frr-conf + hostPath: + path: /run/frr-k8s/conf + type: DirectoryOrCreate + - name: metrics + emptyDir: {} + - name: metrics-certs + secret: + secretName: frr-k8s-master-monitor-certs +--- +apiVersion: v1 +kind: Service +metadata: + name: frr-k8s-master-monitor + namespace: openshift-frr-k8s + labels: + name: frr-k8s-master-monitor + annotations: + prometheus.io/scrape: "true" + service.beta.openshift.io/serving-cert-secret-name: frr-k8s-master-monitor-certs +spec: + selector: + app: frr-k8s + component: frr-k8s-master-monitor + clusterIP: None + ports: + - name: frrmetricshttps + port: 9141 + targetPort: 9141 + sessionAffinity: None + type: ClusterIP +--- +# The namespace carries a default-deny NetworkPolicy; the worker DaemonSet +# is exempt as hostNetwork, the pod-network companion is not. +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: frr-k8s-master-monitor + namespace: openshift-frr-k8s +spec: + podSelector: + matchLabels: + component: frr-k8s-master-monitor + policyTypes: + - Ingress + - Egress + ingress: + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: openshift-monitoring + ports: + - protocol: TCP + port: 9141 + # TokenReview/SubjectAccessReview for scrape authentication. The API + # server endpoints are node IPs, not selectable by pod selectors. + egress: + - ports: + - protocol: TCP + port: 443 + - protocol: TCP + port: 6443 +--- +apiVersion: monitoring.coreos.com/v1 +kind: ServiceMonitor +metadata: + name: frr-k8s-master-monitor + namespace: openshift-frr-k8s + labels: + app: frr-k8s + component: frr-k8s-master-monitor + annotations: + networkoperator.openshift.io/ignore-errors: "" +spec: + endpoints: + - bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token + honorLabels: true + port: frrmetricshttps + scheme: https + tlsConfig: + caFile: /etc/prometheus/configmaps/serving-certs-ca-bundle/service-ca.crt + serverName: frr-k8s-master-monitor.openshift-frr-k8s.svc + jobLabel: app + namespaceSelector: + matchNames: + - openshift-frr-k8s + selector: + matchLabels: + name: frr-k8s-master-monitor +{{ end }} diff --git a/docs/bgp_vip_management.md b/docs/bgp_vip_management.md new file mode 100644 index 0000000000..a901d7a5f0 --- /dev/null +++ b/docs/bgp_vip_management.md @@ -0,0 +1,47 @@ +# BGP-based VIP management + +Feature gate: `BGPBasedVIPManagement` (DevPreviewNoUpgrade). BareMetal +platform only, and only when the Infrastructure CR reports +`status.platformStatus.baremetal.vipManagement: BGP`. Without all three, +everything below is inert and frr-k8s behaves exactly as before. +When all three hold, the FRR additional routing capability provider +(`network.operator.openshift.io/cluster`: +`spec.additionalRoutingCapabilities.providers: [FRR]`) is additionally +required - it ships the FRRConfiguration CRD; without it, rendering fails +explicitly rather than staying inert. + +Enhancement: openshift/enhancements#1982. + +## What CNO does + +When active, CNO renders a single cluster-wide `FRRConfiguration` +(`openshift-frr-k8s/bgp-vip`) from the installer-generated `bgp-vip-config` +ConfigMap: + +- The CR spec carries the BGP **sessions** (neighbors, optional BFD). +- VIP **advertisement** is in `rawConfig`: `redistribute table-direct 198` + filtered to exactly the API/ingress VIP prefixes, plus per-neighbor egress + permits. kube-vip (rendered by MCO) installs a VIP route into kernel table + 198 only while that node's backend health check passes, so each node + advertises a VIP only while it can serve it; withdrawal is automatic. + Advertisement cannot use the CRD's `prefixes`/`toAdvertise` surface: it + renders unconditional `network` statements and cannot express redistributed + routes (native support proposed in metallb/frr-k8s#469). + +## Placement + +Control plane nodes run an MCO-rendered frr-k8s **static pod** (needed at +bootstrap, before any workload can schedule). The frr-k8s DaemonSet therefore +avoids masters by role under BGP VIP management; on compact/SNO topologies it +correctly matches zero nodes. Workers keep the DaemonSet and advertise the +ingress VIP when they host healthy routers. + +## RBAC + +The static pod authenticates with the node kubeconfig (the MCO +node-bootstrapper ServiceAccount). `003-static-pod-rbac.yaml` grants the +static pod the read permissions the frr-k8s controller's informers require, +and write access to `FRRNodeState`/`BGPSessionState`. Rendered only under +BGP VIP management. +Per-node write scoping is not expressible in RBAC; a ValidatingAdmissionPolicy +is planned follow-up. diff --git a/pkg/network/bgp_vip.go b/pkg/network/bgp_vip.go new file mode 100644 index 0000000000..0833c3e77e --- /dev/null +++ b/pkg/network/bgp_vip.go @@ -0,0 +1,308 @@ +package network + +import ( + "context" + "encoding/json" + "fmt" + "net" + "slices" + "strings" + "time" + + configv1 "github.com/openshift/api/config/v1" + operv1 "github.com/openshift/api/operator/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" + "k8s.io/klog/v2" +) + +// bgpBasedVIPManagementFeatureGate gates BGP-based VIP management. Defined +// locally until the gate lands in vendored openshift/api (openshift/api#2923). +const bgpBasedVIPManagementFeatureGate = configv1.FeatureGateName("BGPBasedVIPManagement") + +// bgpVIPPeer is one BGP peer from the bgp-vip-config ConfigMap. String-typed +// BFDEnabled and EBGPMultiHop 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 config.json of the bgp-vip-config +// ConfigMap; 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"` +} + +// apiCallTimeout bounds direct API calls made while rendering. +const apiCallTimeout = 30 * time.Second + +// validateBGPVIPConfig rejects non-IP values before they are spliced into +// the FRRConfiguration rawConfig. +func validateBGPVIPConfig(cfg bgpVIPConfigData) error { + if cfg.LocalASN < 1 || cfg.LocalASN > 4294967295 { + return fmt.Errorf("localASN %d out of range", cfg.LocalASN) + } + for _, vip := range slices.Concat(cfg.APIVIPs, cfg.IngressVIPs) { + if net.ParseIP(vip) == nil { + return fmt.Errorf("VIP %q is not a valid IP address", vip) + } + } + peers := slices.Clone(cfg.DefaultPeers) + for _, hostPeers := range cfg.HostOverrides { + peers = append(peers, hostPeers...) + } + for _, peer := range peers { + if net.ParseIP(peer.PeerAddress) == nil { + return fmt.Errorf("BGP peer address %q is not a valid IP address", peer.PeerAddress) + } + if peer.PeerASN < 1 || peer.PeerASN > 4294967295 { + return fmt.Errorf("BGP peer %s: peerASN %d out of range", peer.PeerAddress, peer.PeerASN) + } + for _, d := range []string{peer.HoldTime, peer.KeepaliveTime} { + if d == "" { + continue + } + if _, err := time.ParseDuration(d); err != nil { + return fmt.Errorf("BGP peer %s: invalid duration %q", peer.PeerAddress, d) + } + } + for _, v := range []string{peer.BFDEnabled, peer.EBGPMultiHop} { + if v != "" && v != "true" && v != "false" { + return fmt.Errorf("BGP peer %s: invalid boolean %q", peer.PeerAddress, v) + } + } + } + return nil +} + +// isBGPVIPManagement reports whether the BGPBasedVIPManagement gate is +// enabled and the BareMetal Infrastructure CR requests vipManagement "BGP". +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 unknown gates. + if !slices.Contains(featureGates.KnownFeatures(), bgpBasedVIPManagementFeatureGate) || + !featureGates.Enabled(bgpBasedVIPManagementFeatureGate) { + return false, nil + } + // The typed client would silently drop the not-yet-vendored + // vipManagement field (openshift/api#2923). + ctx, cancel := context.WithTimeout(context.Background(), apiCallTimeout) + defer cancel() + infra, err := client.Default().Dynamic().Resource(schema.GroupVersionResource{ + Group: "config.openshift.io", Version: "v1", Resource: "infrastructures", + }).Get(ctx, "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 { + return false, fmt.Errorf("malformed vipManagement in Infrastructure CR: %v", err) + } + if !found { + return false, nil + } + return val == "BGP", nil +} + +// renderBGPVIPFRRConfiguration builds FRRConfiguration CRs for BGP-managed +// VIPs; a no-op unless BGP VIP management is active. +func renderBGPVIPFRRConfiguration(conf *operv1.NetworkSpec, client cnoclient.Client, isBGP bool) ([]*uns.Unstructured, error) { + if !isBGP { + return nil, nil + } + // The FRRConfiguration CRD ships with the frr-k8s bundle. + if conf == nil || conf.AdditionalRoutingCapabilities == nil || + !slices.Contains(conf.AdditionalRoutingCapabilities.Providers, operv1.RoutingCapabilitiesProviderFRR) { + return nil, fmt.Errorf("BGP-based VIP management requires the FRR additional routing capability provider") + } + + klog.Infof("BGP VIP management is active, rendering FRRConfiguration CRs") + + cm := &corev1.ConfigMap{} + ctx, cancel := context.WithTimeout(context.Background(), apiCallTimeout) + defer cancel() + err := client.Default().CRClient().Get(ctx, + types.NamespacedName{Name: "bgp-vip-config", Namespace: names.APPLIED_NAMESPACE}, cm) + if err != nil { + if apierrors.IsNotFound(err) { + klog.Infof("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) + } + if err := validateBGPVIPConfig(bgpConfig); err != nil { + return nil, fmt.Errorf("invalid bgp-vip-config: %v", err) + } + + return buildFRRConfigurationObjects(bgpConfig) +} + +// buildFRRConfigurationObjects renders the cluster-wide (no node selector) +// FRRConfiguration. The CR spec carries only the BGP sessions; advertisement +// is in rawConfig because the CRD cannot express health-gated redistribution: +// declared prefixes render as unconditional `network` statements and +// toAdvertise only permits declared prefixes (metallb/frr-k8s#469). +// Per-node correctness comes from the health gate: kube-vip installs a +// table-198 route only where the VIP's backend is healthy. +func buildFRRConfigurationObjects(cfg bgpVIPConfigData) ([]*uns.Unstructured, error) { + 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 + } + if peer.HoldTime != "" { + neighbor["holdTime"] = peer.HoldTime + } + if peer.KeepaliveTime != "" { + neighbor["keepaliveTime"] = peer.KeepaliveTime + } + neighbors = append(neighbors, neighbor) + } + + 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 mirrors MCO's bootstrap frr.conf.tmpl: redistribute +// kernel table 198 filtered to exactly the VIP prefixes, plus egress permits +// for them. Per-family blocks render only when that family has VIPs. +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 + // table-direct reads the kernel table directly; no `ip import-table`. + 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) + } + // Egress: frr-k8s renders per-neighbor `
-out` route-maps whose + // generated entries deny everything without toAdvertise. A no-match + // falls through, so these high-sequence permits open egress for exactly + // the VIP prefixes. Couples to frr-k8s's route-map naming; native CRD + // support is proposed in metallb/frr-k8s#469. + 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") +} diff --git a/pkg/network/bgp_vip_test.go b/pkg/network/bgp_vip_test.go new file mode 100644 index 0000000000..1c40009bcb --- /dev/null +++ b/pkg/network/bgp_vip_test.go @@ -0,0 +1,193 @@ +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. + _, 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)) + + // CRD prefixes would render as unconditional `network` statements, + // bypassing the health gate. + _, 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{}) + _, 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))) + + 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("router bgp")) + g.Expect(rawConfig).NotTo(ContainSubstring("import-table")) + 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")) + 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: no v6 blocks at all. + 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 +// installer-shaped config.json payload with every optional field set. +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{}) + // The installer serializes ebgpMultiHop as a string; the CR wants a bool. + g.Expect(neighbor["ebgpMultiHop"]).To(Equal(true)) + g.Expect(neighbor["holdTime"]).To(Equal("90s")) + g.Expect(neighbor["keepaliveTime"]).To(Equal("30s")) + g.Expect(neighbor["password"]).To(Equal("s3cret")) + g.Expect(neighbor["bfdProfile"]).To(Equal("vip-bfd")) + _, 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("router bgp")) + g.Expect(rawConfig).NotTo(ContainSubstring("import-table")) + 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 per-family rendering. +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("router bgp")) + g.Expect(rawConfig).NotTo(ContainSubstring("import-table")) + 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")) + 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")) +} + +func TestValidateBGPVIPConfig(t *testing.T) { + g := NewGomegaWithT(t) + + valid := bgpVIPConfigData{ + LocalASN: 64512, + DefaultPeers: []bgpVIPPeer{{PeerAddress: "192.168.111.1", PeerASN: 64513}}, + APIVIPs: []string{"192.168.111.5"}, + IngressVIPs: []string{"fd2e:6f44:5dd8::4"}, + HostOverrides: map[string][]bgpVIPPeer{ + "master-0": {{PeerAddress: "192.168.111.2", PeerASN: 64513}}, + }, + } + g.Expect(validateBGPVIPConfig(valid)).To(Succeed()) + + badVIP := valid + badVIP.APIVIPs = []string{"192.168.111.5\n router bgp 1"} + g.Expect(validateBGPVIPConfig(badVIP)).NotTo(Succeed()) + + badPeer := valid + badPeer.DefaultPeers = []bgpVIPPeer{{PeerAddress: "not-an-ip", PeerASN: 64513}} + g.Expect(validateBGPVIPConfig(badPeer)).NotTo(Succeed()) + + badOverride := valid + badOverride.HostOverrides = map[string][]bgpVIPPeer{"master-0": {{PeerAddress: "", PeerASN: 1}}} + g.Expect(validateBGPVIPConfig(badOverride)).NotTo(Succeed()) + + badDuration := valid + badDuration.DefaultPeers = []bgpVIPPeer{{PeerAddress: "192.168.111.1", PeerASN: 64513, HoldTime: "ninety"}} + g.Expect(validateBGPVIPConfig(badDuration)).NotTo(Succeed()) + + badBool := valid + badBool.DefaultPeers = []bgpVIPPeer{{PeerAddress: "192.168.111.1", PeerASN: 64513, BFDEnabled: "True"}} + g.Expect(validateBGPVIPConfig(badBool)).NotTo(Succeed()) + + badLocalASN := valid + badLocalASN.LocalASN = 0 + g.Expect(validateBGPVIPConfig(badLocalASN)).NotTo(Succeed()) + + badPeerASN := valid + badPeerASN.DefaultPeers = []bgpVIPPeer{{PeerAddress: "192.168.111.1", PeerASN: 4294967296}} + g.Expect(validateBGPVIPConfig(badPeerASN)).NotTo(Succeed()) +} diff --git a/pkg/network/render.go b/pkg/network/render.go index ffa2579e0f..8649aaafbd 100644 --- a/pkg/network/render.go +++ b/pkg/network/render.go @@ -129,7 +129,18 @@ func Render(operConf *operv1.NetworkSpec, clusterConf *configv1.NetworkSpec, man } objs = append(objs, o...) - o, err = renderAdditionalRoutingCapabilities(operConf, manifestDir) + bgpVIP, err := isBGPVIPManagement(client, bootstrapResult, featureGates) + if err != nil { + return nil, progressing, fmt.Errorf("failed to check VIPManagement mode: %v", err) + } + + o, err = renderAdditionalRoutingCapabilities(operConf, manifestDir, bgpVIP) + if err != nil { + return nil, progressing, err + } + objs = append(objs, o...) + + o, err = renderBGPVIPFRRConfiguration(operConf, client, bgpVIP) if err != nil { return nil, progressing, err } @@ -849,7 +860,7 @@ func registerNetworkingConsolePlugin(bootstrapResult *bootstrap.BootstrapResult, }) } -func renderAdditionalRoutingCapabilities(conf *operv1.NetworkSpec, manifestDir string) ([]*uns.Unstructured, error) { +func renderAdditionalRoutingCapabilities(conf *operv1.NetworkSpec, manifestDir string, bgpVIP bool) ([]*uns.Unstructured, error) { if conf == nil || conf.AdditionalRoutingCapabilities == nil { return nil, nil } @@ -862,6 +873,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..9106366210 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, false) if !reflect.DeepEqual(tt.expectedErr, err) { t.Errorf("renderAdditionalRoutingCapabilities() err = %v, want %v", err, tt.expectedErr) } @@ -607,3 +609,97 @@ func Test_renderAdditionalRoutingCapabilities(t *testing.T) { }) } } + +// Test_renderAdditionalRoutingCapabilitiesBGPVIPManagement verifies that the +// master-avoiding DaemonSet affinity and the static pod RBAC render only +// under BGP VIP management. +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{}, + }, + }, + } + // vipManagement is not in the vendored openshift/api yet (#2923). + 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 + } + + // isBGPVIPManagement is evaluated once in Render and passed down; the + // fixture asserts it reads the unstructured vipManagement field. + bgpVIP, err := isBGPVIPManagement(client, bootstrapResult, featureGates) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(bgpVIP).To(BeTrue()) + + got, err := renderAdditionalRoutingCapabilities(operConf, manifestDir, bgpVIP) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(got).To(HaveLen(29)) + 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", + })) + + staticPodRBAC := func(objs []*uns.Unstructured) bool { + for _, obj := range objs { + if obj.GetName() == "frr-k8s-static-pod" { + return true + } + } + return false + } + g.Expect(staticPodRBAC(got)).To(BeTrue()) + + // Inactive (nil bootstrap result): no affinity, no static pod RBAC. + bgpVIP, err = isBGPVIPManagement(client, nil, featureGates) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(bgpVIP).To(BeFalse()) + got, err = renderAdditionalRoutingCapabilities(operConf, manifestDir, bgpVIP) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(got).To(HaveLen(21)) + _, found = daemonSetAffinity(got) + g.Expect(found).To(BeFalse()) + g.Expect(staticPodRBAC(got)).To(BeFalse()) + + // BGP VIP management without the FRR provider is a configuration error: + // the FRRConfiguration CRD ships with the frr-k8s bundle. + _, err = renderBGPVIPFRRConfiguration(&operv1.NetworkSpec{}, client, true) + g.Expect(err).To(MatchError(ContainSubstring("requires the FRR additional routing capability provider"))) +}