Skip to content

Conversation

@QxBytes
Copy link
Contributor

@QxBytes QxBytes commented Jun 12, 2025

Reason for Change:

This PR creates a new binary ip masq merger binary which merges two ip masq configs together and places them in an output directory for consumption. If no configmaps are found, it will also automatically delete the previously merged configmap. Adds makefile targets for building the binary and image with the appropriate dockerfile. Does not make pipeline changes. Tested on a cluster as a sidecar.

Issue Fixed:

Requirements:

Notes:

QxBytes added 4 commits June 12, 2025 15:05
merges ip masq agent configs into one configmap and outputs to a directory adds the non masquerade ips list together and ORs the other flags together if no configmap that starts with ip-masq exists, no merged configmap is output and any previously created merged configmap in the merged-config directory is deleted this binary is separate from the rest of the repo similar to azure-ipam
ip masq merger is only meant to work on linux since ip masq agent only runs on linux
rename ip masq merger to be consistent with other files add golangci yml for linting fix shadowing issues with err and use newer method to check file not found error fix linter issues use require for tests
@QxBytes QxBytes self-assigned this Jun 12, 2025
@QxBytes QxBytes added the cilium Related to Cilium. label Jun 12, 2025
@QxBytes QxBytes requested a review from Copilot June 12, 2025 22:41
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 binary called "azure ip masq merger" to merge multiple IP masquerade configurations for Azure Container Networking. Key changes include:

  • New implementation of the merger logic in ip_masq_merger.go.
  • Comprehensive unit tests in ip_masq_merger_test.go.
  • Updates to Dockerfile, Makefile, and go.mod to support building and packaging the new binary.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
azure-ip-masq-merger/ip_masq_merger.go New merger implementation and underlying config validation.
azure-ip-masq-merger/ip_masq_merger_test.go Added unit tests for merging and CIDR validation.
azure-ip-masq-merger/go.mod Module and dependency management updates.
azure-ip-masq-merger/Dockerfile Dockerfile updates to build multi-platform images.
azure-ip-masq-merger/.golangci.yml Linters configuration for code quality.
Makefile Build targets and packaging instructions for the new binary.
Comments suppressed due to low confidence (1)

azure-ip-masq-merger/ip_masq_merger.go:250

  • Consider sorting 'cidrsList' after merging to ensure deterministic ordering of CIDRs, which could aid in debugging and configuration comparisons.
for cidr := range cidrsSet { cidrsList = append(cidrsList, cidr) } 
@QxBytes QxBytes marked this pull request as ready for review June 12, 2025 22:48
@QxBytes QxBytes requested a review from a team as a code owner June 12, 2025 22:48
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.

Looks good, a few minor comments. Also, are you using ip-masq-agent-v2 basically?

camrynl
camrynl previously approved these changes Jun 12, 2025
Copy link
Contributor

@camrynl camrynl left a comment

Choose a reason for hiding this comment

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

Lgtm!

Summarizing our discussion offline for context: ip masq merger will merge configs when ebpf host routing and bpf ip masq agent is enabled. It will be deployed to clusters either with sidecare or initcontainer in the cilium daemonset. Dockerfile images will be updated in later stages along with pipeline image builds.

camrynl
camrynl previously approved these changes Jun 13, 2025
@QxBytes QxBytes enabled auto-merge June 16, 2025 18:00
@jpayne3506
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 Jun 16, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 16, 2025
@QxBytes QxBytes enabled auto-merge June 16, 2025 22:08
@QxBytes
Copy link
Contributor Author

QxBytes commented Jun 16, 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 Jun 16, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 17, 2025
@QxBytes QxBytes added this pull request to the merge queue Jun 17, 2025
Merged via the queue into master with commit d044fca Jun 17, 2025
96 of 98 checks passed
@QxBytes QxBytes deleted the alew/add-azure-ip-masq-merger branch June 17, 2025 18:55
sivakami-projects pushed a commit that referenced this pull request Oct 23, 2025
* add azure ip masq merger merges ip masq agent configs into one configmap and outputs to a directory adds the non masquerade ips list together and ORs the other flags together if no configmap that starts with ip-masq exists, no merged configmap is output and any previously created merged configmap in the merged-config directory is deleted this binary is separate from the rest of the repo similar to azure-ipam * add azure ip masq merger make targets ip masq merger is only meant to work on linux since ip masq agent only runs on linux * add ip masq merger tests rename ip masq merger to be consistent with other files add golangci yml for linting fix shadowing issues with err and use newer method to check file not found error fix linter issues use require for tests * add make test for azure ip masq merger tested * add input output configuration * update images to mariner 3.0 * update temp base to 3.0 and fix formatting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cilium Related to Cilium.

5 participants