Skip to content

Use Ipv4Addr instead of String in RackNetworkConfig IP address fields#3399

Merged
jgallagher merged 2 commits into
mainfrom
rack-network-config-types
Jun 26, 2023
Merged

Use Ipv4Addr instead of String in RackNetworkConfig IP address fields#3399
jgallagher merged 2 commits into
mainfrom
rack-network-config-types

Conversation

@jgallagher

Copy link
Copy Markdown
Contributor

I believe based on current usage that all of these are required to be Ipv4Addrs specifically, and not IpAddr/Ipv6Addr:

  • infra_ip_first and infra_ip_last are stored in a variable named ipv4_block, and a later ipv6_block (that does not use those config values) is also created
  • uplink_ip's doc comment says it should be in the infra_ip_first..last range, which means it would also have to be ipv4
  • gateway_ip was being parsed into nexthop: Option<Ipv4Addr>.

but I don't have a lot of context here so would appreciate confirmation.

Fixes #3397.

@internet-diglett internet-diglett left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jgallagher jgallagher force-pushed the rack-network-config-types branch from 46b3efc to 2615da2 Compare June 26, 2023 14:25
@jgallagher jgallagher merged commit 51d2c58 into main Jun 26, 2023
@jgallagher jgallagher deleted the rack-network-config-types branch June 26, 2023 16:18
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.

RackNetworkConfig: Use stronger types than String?

2 participants