Skip to content

OPNET-783: BGP-based VIP management: FRRConfiguration rendering, DaemonSet placement and static pod RBAC #3047

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
mkowalski wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

OPNET-783: BGP-based VIP management: FRRConfiguration rendering, DaemonSet placement and static pod RBAC #3047

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ae8688b
network: render FRRConfiguration for BGP-based VIP management
mkowalski Jul 10, 2026
cd0bd10
network: frr-k8s DaemonSet avoids control plane nodes under BGP VIP m…
mkowalski Jul 10, 2026
70cd187
network: RBAC for the frr-k8s static pod node credentials
mkowalski Jul 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 106 additions & 0 deletions bindata/network/frr-k8s/003-static-pod-rbac.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Comment on lines +10 to +53

@coderabbitai coderabbitai Bot Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Broad, cluster-wide RBAC grant to a shared bootstrap ServiceAccount, applied unconditionally.

Two concerns:

  1. This ClusterRole/ClusterRoleBinding is rendered whenever the FRR AdditionalRoutingCapabilities provider is enabled — not conditioned on BGP VIP management being active (confirmed by render_test.go, where the object count is identical with bootstrapResult == nil). Clusters using FRR for unrelated BGP use cases get this grant even though the static pod it's meant for never runs.
  2. node-bootstrapper is a single ServiceAccount used across every node's bootstrap process; granting it cluster-wide create/update/patch/delete on frrnodestates/bgpsessionstates (not scoped to a single node's own objects, since plain RBAC can't express that) means a compromised node's bootstrap credential could tamper with every other node's BGP/FRR state.

As per path instructions, RBAC for Kubernetes manifests should follow "least privilege; no cluster-admin for workloads". Consider scoping the verbs down (e.g. dropping delete) and/or gating this manifest on BGP VIP management being active.

Also applies to: 55-66

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bindata/network/frr-k8s/003-static-pod-rbac.yaml` around lines 10 - 53,
Restrict the FRR static-pod RBAC in the ClusterRole named frr-k8s-static-pod:
render this ClusterRole and its associated ClusterRoleBinding only when BGP VIP
management/bootstrap is active, not merely when AdditionalRoutingCapabilities is
enabled, and reduce node-state permissions to the minimum required by the static
pod, removing unnecessary destructive verbs such as delete. Update render tests,
including the bootstrapResult == nil case, to verify the resources are omitted
when BGP VIP management is inactive.

Source: Path instructions

---
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
9 changes: 9 additions & 0 deletions bindata/network/frr-k8s/frr-k8s.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading