Align frrk8s to upstream #2833
Align frrk8s to upstream #2833
Conversation
fedepaol
commented
Nov 10, 2025
fedepaol
commented
- Adds status cleaner job that acts as webhook backend plus takes care of removing frrstatus instances related to those nodes where an frrk8s instance is not running anymore
- Adds readOnlyFileSystem=true and allowPrivilegeEscalation=false to the frrk8s controller container
|
/testwith openshift/frr#107 |
WalkthroughAdded two emptyDir volumes and mounts for FRR and hardened both frr and controller containers. Introduced a new frr-k8s-statuscleaner Deployment (webhook-only mode, TLS from secret, probes, hostNetwork). Removed the previous webhook Deployment and repointed the webhook Service to the statuscleaner. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (1)**⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (2)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fedepaol The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
bindata/network/frr-k8s/frr-k8s.yaml(3 hunks)bindata/network/frr-k8s/node-status-cleaner.yaml(1 hunks)bindata/network/frr-k8s/webhook.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
bindata/network/frr-k8s/node-status-cleaner.yamlbindata/network/frr-k8s/webhook.yamlbindata/network/frr-k8s/frr-k8s.yaml
🪛 YAMLlint (1.37.1)
bindata/network/frr-k8s/frr-k8s.yaml
[error] 179-179: syntax error: expected , but found '-'
(syntax)
🔇 Additional comments (4)
bindata/network/frr-k8s/frr-k8s.yaml (2)
162-163: Verify readOnlyRootFilesystem=true compatibility with FRR runtime behavior.The
frrcontainer now hasreadOnlyRootFilesystem: trueadded alongside the new emptyDir volumes for/var/lib/frrand/var/tmp/frr. Confirm that FRR daemon can successfully initialize and write all necessary state/logs to these temporary volumes and doesn't require write access to other root filesystem paths.If the frr daemon requires write access to paths outside the mounted emptyDirs (e.g.,
/etc/frr), the readOnlyRootFilesystem setting may need adjustment or additional volume mounts.
48-51: Security hardening additions approved for frr container.The addition of
allowPrivilegeEscalation: falseandreadOnlyRootFilesystem: true, combined with emptyDir volumes for scratch space at/var/lib/frrand/var/tmp/frr, strengthens the security posture of the FRR daemon container. This aligns well with the PR's security hardening objectives.Also applies to: 162-163
bindata/network/frr-k8s/webhook.yaml (1)
14-14: Service selector correctly routes to status cleaner deployment.The selector change from
frr-k8s-webhook-servertofrr-k8s-statuscleaneraligns with the architectural refactoring to consolidate webhook serving into the status cleaner component. Ensure that the status cleaner deployment innode-status-cleaner.yamlincludes the labelcomponent: frr-k8s-statuscleanerin its pod template to match this selector.bindata/network/frr-k8s/node-status-cleaner.yaml (1)
1-82: Webhook configuration and security posture review.The status cleaner deployment is appropriately configured for webhook serving:
- HTTPS probes at
/healthzwith proper initial delays and timeouts (lines 48-62)- TLS certificate mounting from secret at
/tmp/k8s-webhook-server/serving-certs(lines 75-78)--disable-cert-rotation=trueflag (line 29) is appropriate since certificate lifecycle is managed externally via the secret- Tolerations for control-plane nodes (lines 68-73) enable webhook availability on cluster-critical nodes
privilegedSCC annotation (line 21) matches the frr-k8s DaemonSet requirementResource requests are minimal but appropriate for a webhook component (10m CPU, 50Mi memory).
fedepaol
force-pushed
the
alignfrrk8supstream
branch
2 times, most recently
from
57efca2 to
ae4e0e0
Compare
The status cleaner pod is in charge of removing stale frrnodestates resources from those nodes where frrk8s is not running anymore. At the same time, it acts as the backend for the validation webhook, so the configuration of the webhook now refers to it. Signed-off-by: Federico Paolinelli <[email protected]>
readOnlyRootFilesystem prevents containers from writing to the root filesystem, reducing attack surface and improving security posture by limiting potential malicious file modifications and ensuring immutable container runtime. allowPrivilegeEscalation=false prevents containers from gaining additional privileges beyond those initially granted, further hardening the security posture by blocking privilege escalation attacks. Signed-off-by: Federico Paolinelli <[email protected]>
fedepaol
force-pushed
the
alignfrrk8supstream
branch
from
ae4e0e0 to
5872e43
Compare
|
@fedepaol: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
1 participant
