fix(unikernels): validate inputs in IsIPInSubnet#755
Conversation
Introduce a new test file for LinuxNet subnet validation, covering various scenarios including same subnet, different subnet, and invalid masks. Enhance the IsIPInSubnet function to handle edge cases by returning false for invalid inputs. Signed-off-by: DevMhrn <debashismaharana7854@gmail.com>
✅ Deploy Preview for urunc canceled.
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds defensive handling and unit tests for IsIPInSubnet to ensure invalid IP/mask inputs return false rather than producing incorrect behavior.
Changes:
- Added early-return validation in
IsIPInSubnetwhen address, gateway, or mask fail to parse. - Introduced table-driven tests covering valid and several invalid mask formats.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/unikontainers/unikernels/linux_network_test.go | Adds unit tests for IsIPInSubnet, primarily around mask parsing/validation. |
| pkg/unikontainers/unikernels/linux.go | Adds nil-check guard to return false on unparsable address/gateway/mask. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| assert.Equal(t, tt.want, IsIPInSubnet(tt.net)) | ||
| }) | ||
| } |
| func TestIsIPInSubnet(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| net LinuxNet | ||
| want bool | ||
| }{ |
|
As @cmainas pointed out on the issue, this isn't a user-visible failure in practice, by the time |
Description
This just adds a small nil-check to
IsIPInSubnetinpkg/unikontainers/unikernels/linux.go.IsIPInSubnetdecides if Linux unikernel needsURUNIT_DEFROUTE=1boot param, and currently it just returns true if any of its three string parameters (Address,Gateway,Mask) fails to parse-effectively silently turning off the flag added in #284.Default to false seems safer. If we're using this for Docker with a malformed mask, we just enable one extra optional
URUNIT_DEFROUTE=1flag which will harmlessly overwrite the eth0 default route, which would likely work fine anyways. The previous default risked silently losing egress in k8s, which is worse.I've also added a small table-driven test, in a new file
pkg/unikontainers/unikernels/linux_network_test.gocovering two known-good cases (same subnet, different subnet) as regression guards, plus the 5 malformed-mask cases mentioned in the issue (empty string, garbage string, partial mask, IP literal mask,/24style mask) plus the fully empty cases.Related issues
IsIPInSubnetsilently dropsURUNIT_DEFROUTEwhen network inputs are malformed #754How was this tested?
Locally, inside a Linux container (host is macOS, so e2e is left to CI).
LLM usage
N/A
Checklist
make lint).make test_ctr,make test_nerdctl,make test_docker,make test_crictl).