Skip to content

Conversation

@QxBytes
Copy link
Contributor

@QxBytes QxBytes commented Sep 4, 2025

Reason for Change:

Adds a new flag to iptables monitor so that the program can terminate if no user iptables rules are detected.
Refactors the code to make testing easier.
Changes the default location of the azure block iptables bpf map so it doesn't conflict with the location of the azure iptables block binary that was recently added to the image.
Adds tests.

Issue Fixed:

Requirements:

Notes:
Tested on dualstack cluster with iptables block running (ubuntu 24)

BPF Map

  • Adding a blocked iptable rule blocks the operation
  • The operation is logged with bpftool map dump pinned /sys/fs/bpf/azure-block-iptables/iptables_block_event_counter
  • The map is read by the program and sends an event if the number increases-- the number is also logged properly

Monitoring

  • A user iptable rule causes an event to be created
  • Removing the user iptable rule stops new events related to user iptables rules
  • Adding the user iptable rule creates a node label on ciliumnodes
  • Removing the user iptable rule sets the node label on ciliumnodes to false
  • Waiting 30 seconds (shortened interval) updates the node label and sends an event
  • Ip6 table rules are checked and causes an event and label to come out

Termination

  • when terminate on success is true, the iptables monitor that has user rules gets stuck in init
  • then, when there are no user iptables rules, the init container finishes

Copilot AI review requested due to automatic review settings September 4, 2025 00:33
@QxBytes QxBytes self-assigned this Sep 4, 2025
@QxBytes QxBytes added the cilium Related to Cilium. label Sep 4, 2025

This comment was marked as outdated.

Copy link
Contributor

@santhoshmprabhu santhoshmprabhu left a comment

Choose a reason for hiding this comment

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

Add a test? Also add in the description how you have validated this.

@QxBytes QxBytes force-pushed the alew/new-iptables-monitoring-mode branch from 429b87d to 4baf12b Compare September 5, 2025 00:12
@QxBytes QxBytes requested a review from Copilot September 5, 2025 00:20
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 adds a new terminateOnSuccess flag to the iptables monitor that allows the program to exit when no user iptables rules are detected. The changes also refactor the code for better testability by introducing dependency injection interfaces and comprehensive test coverage.

  • Added terminateOnSuccess configuration flag for conditional program termination
  • Refactored code into testable functions with dependency injection interfaces
  • Updated default BPF map path to avoid conflicts with azure iptables block binary

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
azure-iptables-monitor/interfaces.go New file defining interfaces for dependency injection and configuration structs
azure-iptables-monitor/iptables_monitor.go Refactored main logic into testable functions, added terminateOnSuccess support, updated default map path
azure-iptables-monitor/iptables_monitor_test.go Added comprehensive test coverage with mock implementations for all dependencies
azure-iptables-monitor/README.md Updated documentation for new flags and default map path

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@santhoshmprabhu
Copy link
Contributor

/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 Sep 5, 2025
Merged via the queue into master with commit fe86df4 Sep 6, 2025
15 of 16 checks passed
@QxBytes QxBytes deleted the alew/new-iptables-monitoring-mode branch September 6, 2025 01:58
sivakami-projects pushed a commit that referenced this pull request Oct 23, 2025
* initial refactor without tests

* refactor helper functions

* add test

* address linter

* change bpf map path default since iptables block already exists there

* fix cosmetic issue

* update readme (noop)

* add timeout to context
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cilium Related to Cilium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants