Skip to content

[wip] new version of external http client that actually works#10835

Draft
hawkw wants to merge 9 commits into
mainfrom
eliza/external-client
Draft

[wip] new version of external http client that actually works#10835
hawkw wants to merge 9 commits into
mainfrom
eliza/external-client

Conversation

@hawkw

@hawkw hawkw commented Jul 16, 2026

Copy link
Copy Markdown
Member

No description provided.

unfortunately, the "URL-is-an-IP" case is gonna be 1000x more annoying
because of...reqwest. yay.
hawkw added a commit that referenced this pull request Jul 21, 2026
The `omicron_common::address` module contains a whole bunch of constants
whose names end with or otherwise contain the word "prefix" (or, in this
case, the word "PREFIX"). Unfortunately, this word is used somewhat
inconsistently. In some cases, "prefix" means "the _length_ of a prefix
of a subnet", such as [`BOOTSTRAP_SUBNET_PREFIX`], which describes a /40
network, and therefore unsurprisingly is the number 40, or
[`RACK_PREFIX`] and [`AZ_PREFIX`], which are 56 and 48, respectively.
These constants are typically used as const generic parameters to the
`Ipv6Subnet` type, which is generic over the length of a prefix. In
other cases, we use the word "prefix" to refer to the "first octets in a
subnet", such as [`IPV6_MULTICAST_PREFIX`] and
`IPV6_ADMIN_SCOPED_MULTICAST_PREFIX`, which are `ff00` and `ff04`, and
respectivefly describe the first octet or frist two octets in subnets
which are /8 and /16, respectively.

This was a bit confusing for me to discover that sometimes ending with
"_PREFIX" means a constant contains one thing, and sometimes it means
another thing. If one were to use one kind of `_PREFIX` constant in a
place where the other kind is expected, I can imagine things going quite
wrong, although this is helped a bit by the ones that represent prefix
lengths typically being `u8`s and the ones that represent IPv6 octet
pairs being, naturally, octet _pairs_, i.e. `u16`s. Luckily, one cannot
accidentally insert a `u16` into a `u8`-shaped hole, or vice versa, so
we haven't messed this up _yet_, at least as far as I can tell.

However, there is another annoying issue here, which is that I would
quite like to move the constant [`BOOTSTRAP_PREFIX`], which represents
the first two octets of the bootstrap network, from
`sled_hardware_types::underlay` to `omicron_common::address`.[^1] I'd
like to be able to reference it in Nexus in #10835. Sadly, as I
mentioned above, there is already a `BOOTSTRAP_SUBNET_PREFIX` here,
which represents the _length_ of a prefix, rather than the actual
octets. Having both `BOOTSTRAP_PREFIX` and `BOOTSTRAP_SUBNET_PREFIX`,
with different meanings, seems bad and unfortunate.

Therefore, this branch adopts a more consistent naming scheme, where
constants that represent the length of a prefix are always named
`FOO_PREFIX_LENGTH`, and constants which represent the first octet(s) of
a subnet, often beginning with the letter `f`, are named `FOO_PREFIX`.
Doing this touches a lot of files, but reviewers should really only need
to look closely at `common/src/address.rs`, as the rest are just places
where these various constants are referenced. I would love to have a
network guy or two fact-check whether I am using these terms correctly.
Thanks!

**AI Usage Disclosure**: This was a large, mechanical change, so I used
an AI tool called "the rust-analyzer `Rename Symbol` button" to assist
in making this change.

[^1]: Which it's only currently referenced in `sled-hardware`, which
depends on `omicron-common` as well as on `sled-hardware-types`.

[`BOOTSTRAP_SUBNET_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L22
[`AZ_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L23
[`RACK_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L24
[`IPV6_MULTICAST_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L120
[`IPV6_ADMIN_SCOPED_MULTICAST_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L130
[`BOOTSTRAP_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/sled-hardware/types/src/underlay.rs#L6
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

Successfully merging this pull request may close these issues.

1 participant