-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
We could be more consistent with Ipv6Network
prefixes
#5669
Comments
jgallagher
added a commit
that referenced
this issue
Apr 30, 2024
…8) (#5668) I think this should fix #5665. I checked a4x2 and it has a `/56`, so I think #5665 is specific to RSS when it's been run via wicket. I'll try this on madrid once a TUF repo is built. I opened #5669 for the fact that our types allow this mistake; e.g., I think both 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 are incorrect in that they allow any network size, and both should probably be `Ipv6Net<RACK_PREFIX>` instead. Fixing that is not trivial because at least the former is serialized in the bootstore.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 areomicron/common/src/api/internal/shared.rs
Line 162 in 9c90e4b
omicron/nexus/db-model/src/rack.rs
Line 19 in 9c90e4b
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 beIpv6Subnet<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.The text was updated successfully, but these errors were encountered: