🐛 Fix IPv6 preAllocation canonicalization in IPAM #1250
Conversation
metal3-io-bot
added
the
size/M
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where IPv6 addresses in non-canonical format (e.g., 2001:db8::0005) were not correctly matched with their canonical equivalents (e.g., 2001:db8::5) during IPAM pre-allocation processing.
Key Changes:
- Introduced an
ipEqual()helper function that parses and compares IP addresses semantically rather than as strings - Updated all IP address comparisons in pre-allocation logic to use
ipEqual()instead of string equality - Added regression tests for both Metal3 IPClaim and CAPI IPAddressClaim with non-canonical IPv6 pre-allocations
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ipam/ippool_manager.go | Added ipEqual() function and replaced 8 string comparisons with semantic IP equality checks in both allocateAddress() and capiAllocateAddress() functions |
| ipam/ippool_manager_test.go | Added two test cases verifying that non-canonical IPv6 pre-allocations (e.g., ::0005) correctly match and return canonical addresses (e.g., ::5) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Compare IP addresses by parsed value instead of raw strings to handle non-canonical IPv6 formats (::0005 vs ::5). - Also adding regression tests. Signed-off-by: Kashif Khan <[email protected]>
kashifest
force-pushed
the
fix/ip-checking
branch
from
59d4fa8 to
d6ffa11
Compare
|
/test metal3-centos-e2e-integration-test-main metal3-ubuntu-e2e-integration-test-main |
|
Whats the relation with #1249 ? Both claim to fix the same issue. |
@peppi-lotta is adding validation there, she asked me to push the fix and she will rebase |
|
/cherry-pick release-1.11 |
|
@adilGhaffarDev: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tuminoid The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
metal3-io-bot
added
the
approved
|
@adilGhaffarDev: new pull request created: #1251 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@adilGhaffarDev: new pull request created: #1252 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
4 participants
Fixes #1244
Checklist: