oxidecomputer / oxidecomputer/omicron
We could be more consistent with `Ipv6Network` prefixes
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 572
- Forks
- 97
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 96
Description
While investigating #5665, I ran into a few places where we're using Ipv6Network for types that we know should have a specific prefix size. Two examples are https://github.com/oxidecomputer/omicron/blob/9c90e4b54694e8b4bec1884306d2626dcd062246/common/src/api/internal/shared.rs#L162 and https://github.com/oxidecomputer/omicron/blob/9c90e4b54694e8b4bec1884306d2626dcd062246/nexus/db-model/src/rack.rs#L19
Both of these should always be /56 networks, but the types don't enforce that, which allowed #5665 to sneak in in the first place. I think these should be Ipv6Subnet<RACK_PREFIX> (and the same for other places where we know we should have a specific prefix size). I don't think this is a trivial fix because at least the first of those examples above is serialized in the bootstore, so any change would either need to be backwards compatible or deal with migrating the format.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the Ipv6Network uses in common/src/api/internal/shared.rs around line 162 and nexus/db-model/src/rack.rs around line 19, then review #5665 to understand the prefix issue. Check how the first type is serialized in the bootstore; done means enforcing /56 prefixes where they are known while preserving or migrating the serialized format safely.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend-api-design
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100