Skip to content

Conversation

@QxBytes
Copy link
Contributor

@QxBytes QxBytes commented Jul 8, 2025

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:

@QxBytes QxBytes self-assigned this Jul 8, 2025
@QxBytes QxBytes added cilium Related to Cilium. linux labels Jul 8, 2025
@QxBytes QxBytes requested a review from Copilot July 8, 2025 22:50

This comment was marked as outdated.

@QxBytes QxBytes force-pushed the alew/add-iptables-monitor branch from 1166917 to d4832b9 Compare July 8, 2025 23:01
@QxBytes QxBytes requested a review from Copilot July 8, 2025 23:02

This comment was marked as outdated.

@QxBytes QxBytes marked this pull request as ready for review July 9, 2025 17:36
@QxBytes QxBytes requested a review from a team as a code owner July 9, 2025 17:36
@QxBytes QxBytes requested a review from ZetaoZhuang July 9, 2025 17:36
@QxBytes
Copy link
Contributor Author

QxBytes commented Jul 9, 2025

/azp run Azure Container Networking PR

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).
QxBytes added 3 commits July 18, 2025 16:55
rbac now requires: - apiGroups: ["cilium.io"] resources: ["ciliumnodes"] verbs: ["patch"] we also must pass NODE_UID as an environment variable to send events
@QxBytes QxBytes requested a review from Copilot July 24, 2025 18:10

This comment was marked as outdated.

@QxBytes QxBytes force-pushed the alew/add-iptables-monitor branch from 086f3ba to ea9952a Compare July 24, 2025 18:44
@QxBytes QxBytes requested a review from Copilot July 25, 2025 16:07
Copy link
Contributor

Copilot AI left a 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-monitor binary 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 
@QxBytes
Copy link
Contributor Author

QxBytes commented Jul 25, 2025

/azp run Azure Container Networking PR

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).
@santhoshmprabhu
Copy link
Contributor

Let's also consider monitoring for the following (as future follow ups):

  1. nftables rulesets that don't use the iptables tables
  2. iptables-legacy
@QxBytes QxBytes enabled auto-merge July 29, 2025 23:51
@QxBytes
Copy link
Contributor Author

QxBytes commented Jul 29, 2025

/azp run Azure Container Networking PR

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).
@QxBytes QxBytes added this pull request to the merge queue Jul 30, 2025
Merged via the queue into master with commit ef97f2a Jul 30, 2025
14 of 15 checks passed
@QxBytes QxBytes deleted the alew/add-iptables-monitor branch July 30, 2025 02:58
NihaNallappagari pushed a commit to NihaNallappagari/azure-container-networking that referenced this pull request Sep 4, 2025
) * 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)
sivakami-projects pushed a commit that referenced this pull request Oct 23, 2025
* 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cilium Related to Cilium. linux

4 participants