-
Couldn't load subscription status.
- Fork 259
feat: add azure iptables monitor binary and makefile changes #3779
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.
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
Conversation
1166917 to d4832b9 Compare | /azp run Azure Container Networking PR |
| Azure Pipelines successfully started running 1 pipeline(s). |
rbac now requires: - apiGroups: ["cilium.io"] resources: ["ciliumnodes"] verbs: ["patch"] we also must pass NODE_UID as an environment variable to send events
086f3ba to ea9952a Compare There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new Azure iptables monitoring service that detects user-defined iptables rules on Kubernetes nodes and labels ciliumnode resources accordingly. The service monitors all iptables tables against configurable regex patterns and patches node labels to indicate if unexpected rules are found.
- Adds a new
azure-iptables-monitorbinary that runs as a Kubernetes daemon to monitor iptables rules - Implements pattern-based rule validation using regex files for different iptables tables (nat, mangle, filter, raw, security, global)
- Integrates the new service into the build system with Makefile updates for building binaries, images, and archives
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| azure-iptables-monitor/iptables_monitor.go | Main application logic for monitoring iptables rules and patching Kubernetes node labels |
| azure-iptables-monitor/iptables_monitor_test.go | Comprehensive unit tests with mock implementations for file reading and iptables client |
| azure-iptables-monitor/go.mod | Go module definition with Kubernetes client dependencies |
| azure-iptables-monitor/README.md | Documentation explaining usage, configuration, and pattern file format |
| azure-iptables-monitor/Dockerfile | Multi-stage Docker build for creating the monitoring service container image |
| Makefile | Build system integration for compiling binaries, creating archives, and building container images |
Comments suppressed due to low confidence (4)
azure-iptables-monitor/iptables_monitor.go:3
- Go version 1.23.0 does not exist. The latest stable Go version as of my knowledge cutoff is 1.22.x. Consider using a valid Go version like 1.22 or 1.21.
import ( azure-iptables-monitor/iptables_monitor.go:71
- [nitpick] The function name 'patchLabel' is ambiguous since it doesn't specify what type of resource is being patched. Consider renaming to 'patchCiliumNodeLabel' for clarity.
func patchLabel(clientset dynamic.Interface, labelValue bool, nodeName string) error { azure-iptables-monitor/iptables_monitor.go:164
- [nitpick] The variable name 'foundUnexpectedRules' could be simplified to 'hasUnexpectedRules' to match the function name and be more concise.
foundUnexpectedRules := false azure-iptables-monitor/iptables_monitor.go:208
- [nitpick] The variable name 'userIPTablesRules' is ambiguous. Consider renaming to 'hasUserIPTablesRules' to clearly indicate it's a boolean flag.
userIPTablesRules := false | /azp run Azure Container Networking PR |
| Azure Pipelines successfully started running 1 pipeline(s). |
| Let's also consider monitoring for the following (as future follow ups):
|
| /azp run Azure Container Networking PR |
| Azure Pipelines successfully started running 1 pipeline(s). |
) * add iptables monitor binary and makefile changes * address feedback * add option to send node events if enabled * remove dependency on node patching rbac now requires: - apiGroups: ["cilium.io"] resources: ["ciliumnodes"] verbs: ["patch"] we also must pass NODE_UID as an environment variable to send events * remove passing node uid in since not possible with downward api * update naming and readme for ciliumnodes * address feedback (noop)
* add iptables monitor binary and makefile changes * address feedback * add option to send node events if enabled * remove dependency on node patching rbac now requires: - apiGroups: ["cilium.io"] resources: ["ciliumnodes"] verbs: ["patch"] we also must pass NODE_UID as an environment variable to send events * remove passing node uid in since not possible with downward api * update naming and readme for ciliumnodes * address feedback (noop)
Reason for Change:
Creates a new image similar to azure-ip-masq-merger which monitors the iptables rules on a machine and patches a node label if an iptables rule that does not match the provided regex patterns is found. See the README for details. The binary/image is meant to be run in a k8 environment, have the node name available as an environment variable, and the regex patterns in the specified folder. Pipeline modifications will be made in a future PR.
Issue Fixed:
Requirements:
Notes: