Skip to content

Conversation

@rejain789
Copy link
Contributor

@rejain789 rejain789 commented Oct 27, 2025

Reason for Change:
Previously, we called validateCIDRSuperset with only the IP address, which didn’t include the prefix length and resulted in incomplete CIDR notation, causing inaccurate superset validation for overlay subnet expansion.
For example, the function previously received just the IP address 10.240.0.0 instead of the full CIDR 10.240.0.0/24, which caused validateCIDRSuperset to read the logic inaccurately and led to incorrect subnet containment checks.
This happened because the IP and prefix length were stored separately in 2 different variables, and the code didn’t combine them before validation.
We considered explicitly building the CIDR string before validation. We chose to construct the full CIDR (IP/PrefixLength) for both the new and existing requests prior to calling validateCIDRSuperset, as this ensures correct subnet containment logic for subnet overlay expansion with minimal code change.

Issue Fixed:

Requirements:

Notes:

@rejain789 rejain789 requested a review from a team as a code owner October 27, 2025 20:22
@rejain789 rejain789 requested review from Copilot and rbtr October 27, 2025 20:22
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 fixes a bug in the overlay subnet expansion validation logic where IP addresses were being passed to validateCIDRSuperset without their prefix lengths, causing incorrect validation results. The fix ensures complete CIDR notation (IP/prefix) is provided to the validation function.

Key Changes:

  • Fixed validateCIDRSuperset call to include prefix length in CIDR format
  • Updated test cases to use separate IP address and prefix length fields instead of combined CIDR strings
  • Made test cases more comprehensive with varied prefix lengths

Reviewed Changes

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

File Description
cns/restserver/internalapi.go Fixed the validateCIDRSuperset call to properly format CIDR strings with prefix lengths
cns/restserver/internalapi_test.go Refactored test cases to separate IP addresses from prefix lengths and added more comprehensive test scenarios

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@rejain789 rejain789 changed the title [CNS] Overlay Expansion Subnet Update Job Hot Fix [CNS] Overlay Expansion Subnet Update Job Bug Fix Oct 27, 2025
@rejain789
Copy link
Contributor Author

rejain789 commented Oct 27, 2025

@microsoft-github-policy-service agree company="Microsoft"

@rejain789
Copy link
Contributor Author

/azp run Azure Container Networking PR

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rejain789
Copy link
Contributor Author

@microsoft-github-policy-service agree

@rejain789 rejain789 added this pull request to the merge queue Oct 27, 2025
Any commits made after this event will not be merged.
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 28, 2025
@rejain789 rejain789 added this pull request to the merge queue Oct 28, 2025
Any commits made after this event will not be merged.
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 28, 2025
@rejain789 rejain789 added this pull request to the merge queue Oct 28, 2025
Any commits made after this event will not be merged.
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 28, 2025
@rejain789 rejain789 added this pull request to the merge queue Oct 28, 2025
Any commits made after this event will not be merged.
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 28, 2025
@rejain789 rejain789 added this pull request to the merge queue Oct 28, 2025
Any commits made after this event will not be merged.
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 28, 2025
@rejain789 rejain789 added this pull request to the merge queue Oct 28, 2025
Any commits made after this event will not be merged.
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 28, 2025
@paulyufan2 paulyufan2 added this pull request to the merge queue Oct 28, 2025
Any commits made after this event will not be merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants