Skip to content
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

Open
jgallagher opened this issue Apr 30, 2024 · 0 comments
Open

We could be more consistent with Ipv6Network prefixes #5669

jgallagher opened this issue Apr 30, 2024 · 0 comments

Comments

@jgallagher
Copy link
Contributor

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

pub rack_subnet: Ipv6Network,
and
pub rack_subnet: Option<IpNetwork>,

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.

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
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant