-
Notifications
You must be signed in to change notification settings - Fork 67
Enable wicket to configure the rack subnet #9653
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
Changes from all commits
4877ee0
8f5078d
f9c74b2
98b4e62
731b566
0af724c
19cf6de
8ed024f
4bf4f5e
e1db2d2
29a96d8
a0d858f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,10 +20,9 @@ use display_error_chain::DisplayErrorChain; | |
| use omicron_certificates::CertificateError; | ||
| use omicron_common::address; | ||
| use omicron_common::address::Ipv4Range; | ||
| use omicron_common::address::Ipv6Subnet; | ||
| use omicron_common::address::RACK_PREFIX; | ||
| use omicron_common::api::external::AllowedSourceIps; | ||
| use omicron_common::api::external::SwitchLocation; | ||
| use oxnet::Ipv6Net; | ||
| use sled_hardware_types::Baseboard; | ||
| use slog::debug; | ||
| use slog::warn; | ||
|
|
@@ -33,7 +32,6 @@ use std::collections::btree_map; | |
| use std::mem; | ||
| use std::net::IpAddr; | ||
| use std::net::Ipv6Addr; | ||
| use std::sync::LazyLock; | ||
| use thiserror::Error; | ||
| use wicket_common::inventory::MgsV1Inventory; | ||
| use wicket_common::inventory::SpType; | ||
|
|
@@ -52,14 +50,6 @@ use wicketd_api::CurrentRssUserConfig; | |
| use wicketd_api::CurrentRssUserConfigSensitive; | ||
| use wicketd_api::SetBgpAuthKeyStatus; | ||
|
|
||
| // TODO-correctness For now, we always use the same rack subnet when running | ||
| // RSS. When we get to multirack, this will be wrong, but there are many other | ||
| // RSS-related things that need to change then too. | ||
| static RACK_SUBNET: LazyLock<Ipv6Subnet<RACK_PREFIX>> = LazyLock::new(|| { | ||
| let ip = Ipv6Addr::new(0xfd00, 0x1122, 0x3344, 0x0100, 0, 0, 0, 0); | ||
| Ipv6Subnet::new(ip) | ||
| }); | ||
|
|
||
| const RECOVERY_SILO_NAME: &str = "recovery"; | ||
| const RECOVERY_SILO_USERNAME: &str = "recovery"; | ||
|
|
||
|
|
@@ -659,10 +649,15 @@ fn validate_rack_network_config( | |
| } | ||
| } | ||
|
|
||
| let rack_subnet = match validate_rack_subnet(config.rack_subnet_address) { | ||
| Ok(v) => v, | ||
| Err(e) => bail!(e), | ||
| }; | ||
|
|
||
| // TODO Add more client side checks on `rack_network_config` contents? | ||
|
|
||
| Ok(bootstrap_agent_client::types::RackNetworkConfigV2 { | ||
| rack_subnet: RACK_SUBNET.net(), | ||
| rack_subnet, | ||
| infra_ip_first: config.infra_ip_first, | ||
| infra_ip_last: config.infra_ip_last, | ||
| ports: config | ||
|
|
@@ -686,6 +681,49 @@ fn validate_rack_network_config( | |
| }) | ||
| } | ||
|
|
||
| pub fn validate_rack_subnet( | ||
| subnet_address: Option<Ipv6Addr>, | ||
| ) -> Result<Ipv6Net, String> { | ||
| use rand::prelude::*; | ||
|
|
||
| let rack_subnet_address = match subnet_address { | ||
| Some(addr) => addr, | ||
| None => { | ||
| let mut rng = rand::rng(); | ||
| let a: u16 = 0xfd00 + Into::<u16>::into(rng.random::<u8>()); | ||
| Ipv6Addr::new( | ||
| a, | ||
| rng.random::<u16>(), | ||
| rng.random::<u16>(), | ||
| 0x0100, | ||
| 0, | ||
| 0, | ||
| 0, | ||
| 0, | ||
| ) | ||
| } | ||
| }; | ||
|
|
||
| // first octet must be fd | ||
| if rack_subnet_address.octets()[0] != 0xfd { | ||
| return Err("rack subnet address must begin with 0xfd".into()); | ||
| }; | ||
|
|
||
| // Do not allow rack0 | ||
| if rack_subnet_address.octets()[6] == 0x00 { | ||
| return Err("rack number (seventh octet) cannot be 0".into()); | ||
| }; | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to check that the low 72 bits are all 0? That's guaranteed for our random ones, and converting the subnet address into an
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that's a reasonable check to add, as it does make things a bit clearer in terms of what we expect (and what the system will do if you set the lower bits of the address) |
||
| // Do not allow addresses more specific than /56 | ||
| if rack_subnet_address.octets()[7..].iter().any(|x| *x != 0x00) { | ||
| return Err("rack subnet address is /56, \ | ||
| but a more specific prefix was provided" | ||
| .into()); | ||
| }; | ||
|
|
||
| Ipv6Net::new(rack_subnet_address, 56).map_err(|e| e.to_string()) | ||
| } | ||
|
|
||
| /// Builds a `BaPortConfigV2` from a `UserSpecifiedPortConfig`. | ||
| /// | ||
| /// Assumes that all auth keys are present in `bgp_auth_keys`. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tiny nit - why is this string different from the one a few lines up?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I figured we wanted it to say "will be chosen randomly" if the user hasn't provided a ipv6 subnet address and we are still
uninitialzed, but it should say "chosen randomly` once we are initialized.This is how it looks when

Nonemaps to "will be chosen randomly"It seems like it could be a little confusing to users if we left it that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, got it. Yeah that works for me 👍
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's how it looks with the two different strings:
Before Rack Init
After Rack Init
With User Specified Address