bugfix(serviveability): require ip-net for CYOA interfaces#3033
bugfix(serviveability): require ip-net for CYOA interfaces#3033bgm-malbeclabs wants to merge 6 commits intomainfrom
Conversation
bgm-malbeclabs
changed the title
bgm-malbeclabs
force-pushed
the
bgm/enforce_cyoa_ip-net
branch
from
285a719 to
8ffc9d1
Compare
bgm-malbeclabs
requested review from
ben-malbeclabs,
Copilot,
elitegreg and
nikw9944
and removed request for
Copilot and
elitegreg
There was a problem hiding this comment.
Pull request overview
This PR addresses a bug where CYOA (Choose Your Own Adventure) interfaces could be created without an ip_net, which would default to 0.0.0.0. The fix adds validation in the interface creation processor to require that CYOA interfaces must have an ip_net specified.
Changes:
- Added validation to require ip_net for CYOA interfaces during creation
- Added validation to restrict CYOA to physical interfaces only
- Updated E2E tests to include ip-net flag when creating CYOA interfaces
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| smartcontract/programs/doublezero-serviceability/src/processors/device/interface/create.rs | Added two new validation checks: CYOA can only be set on physical interfaces, and CYOA interfaces must have an ip_net |
| e2e/link_onchain_allocation_test.go | Updated CYOA interface creation commands to include --ip-net flag |
| e2e/interface_validation_test.go | Updated existing test to include --ip-net flag when creating CYOA interface |
| CHANGELOG.md | Added entry documenting the requirement for ip-net on CYOA interfaces |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // CYOA interfaces must have an ip_net | ||
| if value.interface_cyoa != InterfaceCYOA::None && value.ip_net.is_none() { | ||
| return Err(DoubleZeroError::InvalidInterfaceIp.into()); | ||
| } |
There was a problem hiding this comment.
The validation requiring CYOA interfaces to have an ip_net is only enforced during interface creation. However, this same validation should also be added to:
- The validate() function in interface.rs (around line 449) to ensure this constraint is checked consistently across all operations
- The update processor in update.rs to prevent setting interface_cyoa without an ip_net or clearing ip_net from a CYOA interface
Without these additional checks, it's possible to create inconsistent states through the update operation or have interfaces that pass validation but violate this business rule.
| // CYOA interfaces must have an ip_net | ||
| if value.interface_cyoa != InterfaceCYOA::None && value.ip_net.is_none() { | ||
| return Err(DoubleZeroError::InvalidInterfaceIp.into()); | ||
| } |
There was a problem hiding this comment.
This new validation enforces that CYOA interfaces must have an ip_net, but the existing unit test test_cyoa_on_physical_valid in interface.rs (lines 614-619) creates a CYOA interface without an ip_net and expects validation to pass. This test should be updated to include an ip_net to align with the new business rule, or a new test should be added to verify that CYOA without ip_net is rejected during creation.
| _, err := dn.Manager.Exec(t.Context(), []string{ | ||
| "doublezero", "device", "interface", "create", | ||
| testDeviceCode, testInterfaceName, | ||
| "--interface-cyoa", "gre-over-dia", | ||
| "--ip-net", "45.33.100.50/31", |
There was a problem hiding this comment.
While the E2E tests have been updated to include ip-net when creating CYOA interfaces, there's no negative test case to verify that creating a CYOA interface WITHOUT an ip-net is properly rejected. Consider adding a test case similar to "cyoa_on_loopback_rejected" that attempts to create a CYOA physical interface without ip-net and verifies that it fails with the appropriate error.
1 participant
Summary of Changes
This PR adds the requirement to have an ip_net when a CYOA interface is created. Previously one could create a CYOA interface without an ip_net which would default to
0.0.0.0which doesn't seem like desired behavior.Closes #3032
Testing Verification
ip-netflag for CYOA interfaces