Skip to content

Commit f0168fc

Browse files
refactor(vpc): separate Vpc fields into config/status (#2613)
# Description Introduce VpcConfig on the forge Vpc proto and split the internal api-model into VpcConfig/VpcStatus, matching the NetworkSegment pattern. Deprecated flat fields remain populated for nico-rest compatibility; creation/update requests stay flat. Adds compatibility tests pinning structured fields against deprecated mirrors. ## Related issues #928 ## Type of Change <!-- Check one that best describes this PR --> - [ ] **Add** - New feature or capability - [X] **Change** - Changes in existing functionality - [ ] **Fix** - Bug fixes - [ ] **Remove** - Removed features or deprecated functionality - [X] **Internal** - Internal changes (refactoring, tests, docs, etc.) ## Breaking Changes <!-- If checked, describe the breaking changes and migration steps --> <!-- Breaking changes are not generally permitted, please discuss on a GitHub discussion or with the development team if you believe you need to break a backward compatibility guarantee --> - [ ] **This PR contains breaking changes** ## Testing <!-- How was this tested? Check all that apply --> - [X] Unit tests added/updated - [X] Integration tests added/updated - [ ] Manual testing performed - [ ] No testing required (docs, internal refactor, etc.) ## Additional Notes <!-- Any additional context, deployment notes, or reviewer guidance -->
1 parent e65e396 commit f0168fc

22 files changed

Lines changed: 449 additions & 213 deletions

File tree

crates/admin-cli/src/vpc/show/cmd.rs

Lines changed: 52 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,40 @@ async fn show_vpc_details(
9898
Ok(())
9999
}
100100

101+
#[allow(deprecated)]
102+
fn vpc_config(vpc: &forgerpc::Vpc) -> forgerpc::VpcConfig {
103+
if let Some(config) = vpc.config.clone() {
104+
config
105+
} else {
106+
forgerpc::VpcConfig {
107+
tenant_organization_id: vpc.tenant_organization_id.clone(),
108+
tenant_keyset_id: vpc.tenant_keyset_id.clone(),
109+
network_virtualization_type: vpc.network_virtualization_type,
110+
network_security_group_id: vpc.network_security_group_id.clone(),
111+
default_nvlink_logical_partition_id: vpc.default_nvlink_logical_partition_id,
112+
vni: vpc.vni,
113+
routing_profile_type: vpc.routing_profile_type.clone(),
114+
}
115+
}
116+
}
117+
118+
#[allow(deprecated)]
119+
fn vpc_allocated_vni(vpc: &forgerpc::Vpc) -> u32 {
120+
vpc.status
121+
.as_ref()
122+
.and_then(|status| status.vni)
123+
.or(vpc.deprecated_vni)
124+
.unwrap_or_default()
125+
}
126+
127+
#[allow(deprecated)]
128+
fn vpc_virt_type(vpc: &forgerpc::Vpc) -> i32 {
129+
vpc_config(vpc)
130+
.network_virtualization_type
131+
.or(vpc.network_virtualization_type)
132+
.unwrap_or_default()
133+
}
134+
101135
fn convert_vpcs_to_nice_table(vpcs: forgerpc::VpcList) -> Box<Table> {
102136
let mut table = Table::new();
103137

@@ -115,18 +149,17 @@ fn convert_vpcs_to_nice_table(vpcs: forgerpc::VpcList) -> Box<Table> {
115149

116150
for vpc in vpcs.vpcs {
117151
let metadata = vpc.metadata.as_ref().unwrap_or(&default_metadata);
118-
let virt_type = forgerpc::VpcVirtualizationType::try_from(
119-
vpc.network_virtualization_type.unwrap_or_default(),
120-
)
121-
.unwrap_or_default()
122-
.as_str_name()
123-
.to_string();
152+
let config = vpc_config(&vpc);
153+
let virt_type = forgerpc::VpcVirtualizationType::try_from(vpc_virt_type(&vpc))
154+
.unwrap_or_default()
155+
.as_str_name()
156+
.to_string();
124157

125158
table.add_row(row![
126159
vpc.id.unwrap_or_default(),
127160
metadata.name,
128-
vpc.tenant_organization_id,
129-
vpc.network_security_group_id.unwrap_or_default(),
161+
config.tenant_organization_id,
162+
config.network_security_group_id.unwrap_or_default(),
130163
vpc.version,
131164
vpc.created.unwrap_or_default(),
132165
virt_type,
@@ -146,9 +179,11 @@ fn convert_vpcs_to_nice_table(vpcs: forgerpc::VpcList) -> Box<Table> {
146179
table.into()
147180
}
148181

182+
#[allow(deprecated)]
149183
fn convert_vpc_to_nice_format(vpc: &forgerpc::Vpc) -> CarbideCliResult<String> {
150184
let width = 25;
151185
let mut lines = String::new();
186+
let config = vpc_config(vpc);
152187

153188
let vpc_name = vpc
154189
.metadata
@@ -159,10 +194,10 @@ fn convert_vpc_to_nice_format(vpc: &forgerpc::Vpc) -> CarbideCliResult<String> {
159194
let data: Vec<(&'static str, Cow<str>)> = vec![
160195
("ID", vpc.id.unwrap_or_default().to_string().into()),
161196
("NAME", vpc_name),
162-
("TENANT ORG", vpc.tenant_organization_id.as_str().into()),
197+
("TENANT ORG", config.tenant_organization_id.as_str().into()),
163198
(
164199
"NETWORK SECURITY GROUP",
165-
vpc.network_security_group_id().into(),
200+
config.network_security_group_id.unwrap_or_default().into(),
166201
),
167202
("VERSION", vpc.version.as_str().into()),
168203
(
@@ -180,19 +215,17 @@ fn convert_vpc_to_nice_format(vpc: &forgerpc::Vpc) -> CarbideCliResult<String> {
180215
None => "".into(),
181216
},
182217
),
183-
("TENANT KEYSET", vpc.tenant_keyset_id().into()),
184218
(
185-
"VNI",
186-
format!("{}", vpc.status.and_then(|s| s.vni).unwrap_or_default()).into(),
219+
"TENANT KEYSET",
220+
config.tenant_keyset_id.unwrap_or_default().into(),
187221
),
222+
("VNI", format!("{}", vpc_allocated_vni(vpc)).into()),
188223
(
189224
"NW VIRTUALIZATION",
190-
forgerpc::VpcVirtualizationType::try_from(
191-
vpc.network_virtualization_type.unwrap_or_default(),
192-
)
193-
.unwrap_or_default()
194-
.as_str_name()
195-
.into(),
225+
forgerpc::VpcVirtualizationType::try_from(vpc_virt_type(vpc))
226+
.unwrap_or_default()
227+
.as_str_name()
228+
.into(),
196229
),
197230
];
198231

crates/api-core/src/db_init.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ pub async fn create_initial_networks(
105105
ns.vpc_id = if let Some(vpc_name) = &def.vpc_name {
106106
match db::vpc::find_by_name(&mut txn, vpc_name).await?.as_slice() {
107107
[vpc] => {
108-
vpc.network_virtualization_type
108+
vpc.config
109+
.network_virtualization_type
109110
.ensure_supports_segment(&ns)?;
110111
Some(vpc.id)
111112
}
@@ -292,7 +293,7 @@ pub async fn update_network_segments_svi_ip(db_pool: &Pool<Postgres>) -> Result<
292293
};
293294

294295
// SVI IP is needed only for FNN.
295-
if vpc.network_virtualization_type != VpcVirtualizationType::Fnn {
296+
if vpc.config.network_virtualization_type != VpcVirtualizationType::Fnn {
296297
continue;
297298
}
298299

@@ -410,8 +411,8 @@ pub(crate) async fn create_admin_vpc(
410411
};
411412

412413
if let Some(mut existing_vpc) = existing_vpc {
413-
let existing_vni = existing_vpc.status.as_ref().and_then(|status| status.vni);
414-
if existing_vni != Some(configured_vni) || existing_vpc.vni != Some(configured_vni) {
414+
let existing_vni = existing_vpc.status.vni;
415+
if existing_vni != Some(configured_vni) || existing_vpc.config.vni != Some(configured_vni) {
415416
if let Some(conflicting_vpc) = db::vpc::find_by_vni(&mut txn, configured_vni)
416417
.await?
417418
.into_iter()

crates/api-core/src/ethernet_virtualization.rs

Lines changed: 47 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ pub(crate) async fn validate_instance_interface_routing_profiles(
152152
let vpc = db::vpc::find_by_segment(&mut *txn, segment_id).await?;
153153

154154
// Interface routing profiles are only valid on FNN VPC interfaces.
155-
if vpc.network_virtualization_type != VpcVirtualizationType::Fnn {
155+
if vpc.config.network_virtualization_type != VpcVirtualizationType::Fnn {
156156
return Err(CarbideError::InvalidArgument(
157157
"instance interface routing_profile is only supported for FNN VPC interfaces"
158158
.to_string(),
@@ -165,7 +165,8 @@ pub(crate) async fn validate_instance_interface_routing_profiles(
165165
)
166166
})?;
167167
let profile_type =
168-
vpc.routing_profile_type
168+
vpc.config
169+
.routing_profile_type
169170
.as_ref()
170171
.ok_or_else(|| CarbideError::Internal {
171172
message: "tenant routing profile type not found in VPC record".to_string(),
@@ -440,7 +441,7 @@ pub async fn admin_network(
440441
return Err(CarbideError::FindOneReturnedNoResultsError(vpc_id.into()).into());
441442
}
442443
let vpc = vpcs.remove(0);
443-
match vpc.status.and_then(|v| v.vni) {
444+
match vpc.status.vni {
444445
Some(vpc_vni) => {
445446
let tenant_loopback_ip = if use_vpc_vrf_loopback {
446447
Some(
@@ -643,50 +644,51 @@ pub async fn tenant_network(
643644
None => None,
644645
};
645646

646-
let vpc_vni = vpc
647-
.as_ref()
648-
.and_then(|v| v.status.as_ref().and_then(|vs| vs.vni))
649-
.unwrap_or_default() as u32;
647+
let vpc_vni = vpc.as_ref().and_then(|v| v.status.vni).unwrap_or_default() as u32;
650648

651649
// Resolve the routing profile from the VPC attached to this interface.
652-
let (vpc_routing_profile, interface_routing_profile) = match (vpc.as_ref(), fnn_config) {
653-
(Some(vpc), Some(fnn)) if vpc.network_virtualization_type == VpcVirtualizationType::Fnn => {
654-
let profile_type =
655-
vpc.routing_profile_type
656-
.as_ref()
657-
.ok_or_else(|| CarbideError::Internal {
650+
let (vpc_routing_profile, interface_routing_profile) =
651+
match (vpc.as_ref(), fnn_config) {
652+
(Some(vpc), Some(fnn))
653+
if vpc.config.network_virtualization_type == VpcVirtualizationType::Fnn =>
654+
{
655+
let profile_type = vpc.config.routing_profile_type.as_ref().ok_or_else(|| {
656+
CarbideError::Internal {
658657
message: "tenant routing profile type not found in VPC record".to_string(),
659-
})?;
660-
let profile = fnn.routing_profiles.get(profile_type).ok_or_else(|| {
661-
CarbideError::NotFoundError {
662-
kind: "routing_profile_type",
663-
id: profile_type.to_string(),
664-
}
665-
})?;
658+
}
659+
})?;
660+
let profile = fnn.routing_profiles.get(profile_type).ok_or_else(|| {
661+
CarbideError::NotFoundError {
662+
kind: "routing_profile_type",
663+
id: profile_type.to_string(),
664+
}
665+
})?;
666666

667-
(
668-
Some(rpc::RoutingProfile::from(profile)),
669-
iface
670-
.routing_profile
671-
.as_ref()
672-
.map(rpc::FlatInterfaceRoutingProfile::from),
673-
)
674-
}
675-
(Some(vpc), None) if vpc.network_virtualization_type == VpcVirtualizationType::Fnn => {
676-
return Err(CarbideError::Internal {
677-
message: "FNN VPC found but no FNN config found".to_string(),
667+
(
668+
Some(rpc::RoutingProfile::from(profile)),
669+
iface
670+
.routing_profile
671+
.as_ref()
672+
.map(rpc::FlatInterfaceRoutingProfile::from),
673+
)
678674
}
679-
.into());
680-
}
681-
_ if iface.routing_profile.is_some() => {
682-
return Err(CarbideError::InvalidArgument(
683-
"instance interface routing_profile is only supported for FNN VPC interfaces"
684-
.to_string(),
685-
)
686-
.into());
687-
}
688-
_ => (None, None),
689-
};
675+
(Some(vpc), None)
676+
if vpc.config.network_virtualization_type == VpcVirtualizationType::Fnn =>
677+
{
678+
return Err(CarbideError::Internal {
679+
message: "FNN VPC found but no FNN config found".to_string(),
680+
}
681+
.into());
682+
}
683+
_ if iface.routing_profile.is_some() => {
684+
return Err(CarbideError::InvalidArgument(
685+
"instance interface routing_profile is only supported for FNN VPC interfaces"
686+
.to_string(),
687+
)
688+
.into());
689+
}
690+
_ => (None, None),
691+
};
690692

691693
let rpc_ft: rpc::InterfaceFunctionType = iface.function_id.function_type().into();
692694
let (svi_ip, svi_ip_v6) = ds.svi_ips(network_virtualization_type, is_l2_segment)?;
@@ -701,14 +703,14 @@ pub async fn tenant_network(
701703
// see if there's an associated VPC (there should be),
702704
// and see if the VPC has an NSG attached.
703705
(false, None, Some(v)) => {
704-
match v.network_security_group_id.as_ref() {
706+
match v.config.network_security_group_id.as_ref() {
705707
None => None,
706708
Some(vpc_nsg_id) => {
707709
// Make our DB query for the IDs to get our NetworkSecurityGroup
708710
let network_security_group = network_security_group::find_by_ids(
709711
txn,
710712
&[vpc_nsg_id.to_owned()],
711-
Some(&v.tenant_organization_id.parse().map_err(|_| {
713+
Some(&v.config.tenant_organization_id.parse().map_err(|_| {
712714
CarbideError::Internal {
713715
message: "invalid tenant org in VPC data".to_string(),
714716
}
@@ -719,7 +721,7 @@ pub async fn tenant_network(
719721
.pop()
720722
.ok_or(CarbideError::NotFoundError {
721723
kind: "NetworkSecurityGroup",
722-
id: v.tenant_organization_id.clone(),
724+
id: vpc_nsg_id.to_string(),
723725
})?;
724726

725727
Some((

crates/api-core/src/handlers/dpu.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,9 +258,9 @@ pub(crate) async fn get_managed_host_network_config_inner(
258258
let vpc = db::vpc::find_by_segment(&mut txn, network_segment_id)
259259
.await?;
260260

261-
network_virtualization_type = vpc.network_virtualization_type;
261+
network_virtualization_type = vpc.config.network_virtualization_type;
262262

263-
vpc_vni = vpc.status.as_ref().and_then(|v| v.vni.map(|x|x as u32));
263+
vpc_vni = vpc.status.vni.map(|x| x as u32);
264264

265265
let suppress_tenant_security_groups = match &snapshot.managed_state {
266266
ManagedHostState::Assigned { instance_state } => {

crates/api-core/src/handlers/network_segment.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ pub(crate) async fn create(
143143
.first()
144144
.ok_or_else(|| CarbideError::internal(format!("VPC ID: {vpc_id} not found.")))?;
145145

146-
let virtualization_type = vpc.network_virtualization_type;
146+
let virtualization_type = vpc.config.network_virtualization_type;
147147

148148
// Segment compatibility (segment-type binding + IPv6 support)
149149
// and SVI allocation are both expressed as capability checks
@@ -214,7 +214,8 @@ pub(crate) async fn attach_to_vpc(
214214
.into());
215215
}
216216

217-
vpc.network_virtualization_type
217+
vpc.config
218+
.network_virtualization_type
218219
.ensure_supports_segment(&segment)
219220
.map_err(CarbideError::from)?;
220221

crates/api-core/src/handlers/vpc.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,8 @@ pub(crate) async fn update(
189189
&mut txn,
190190
std::slice::from_ref(&id),
191191
Some(
192-
&vpc.tenant_organization_id
192+
&vpc.config
193+
.tenant_organization_id
193194
.parse()
194195
.map_err(|e: InvalidTenantOrg| {
195196
CarbideError::from(RpcDataConversionError::InvalidTenantOrg(e.to_string()))
@@ -203,7 +204,7 @@ pub(crate) async fn update(
203204
{
204205
return Err(CarbideError::FailedPrecondition(format!(
205206
"NetworkSecurityGroup `{}` does not exist or is not owned by Tenant `{}`",
206-
id, vpc.tenant_organization_id
207+
id, vpc.config.tenant_organization_id
207208
))
208209
.into());
209210
}
@@ -299,13 +300,16 @@ pub(crate) async fn delete(
299300
}
300301
};
301302

302-
if let Some(vni) = vpc.status.as_ref().and_then(|s| s.vni) {
303+
if let Some(vni) = vpc.status.vni {
303304
// We can just keep deriving int/ext from the routing profile
304305
// because a VPC is not allowed to change its profile after
305306
// creation. VPC types that don't carry a routing profile
306307
// (ETV, Flat) land in the internal pool on create -- mirror
307308
// that here so the VNI is released back to the same pool.
308-
let internal = match (api.runtime_config.fnn.as_ref(), vpc.routing_profile_type) {
309+
let internal = match (
310+
api.runtime_config.fnn.as_ref(),
311+
vpc.config.routing_profile_type,
312+
) {
309313
(None, _) | (Some(_), None) => true,
310314
(Some(f), Some(profile_type)) => {
311315
let Some(profile) = f.routing_profiles.get(&profile_type) else {

crates/api-core/src/handlers/vpc_peering.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,9 @@ pub async fn create(
7575
// Make sure the VPCs are allowed to peer based on their
7676
// virtualization types. Their capabilities will determine
7777
// if they are allowed or not.
78-
vpc1.network_virtualization_type
79-
.ensure_can_peer_with(vpc2.network_virtualization_type)
78+
vpc1.config
79+
.network_virtualization_type
80+
.ensure_can_peer_with(vpc2.config.network_virtualization_type)
8081
.map_err(CarbideError::from)?;
8182
}
8283
Some(VpcPeeringPolicy::Mixed) => {

crates/api-core/src/handlers/vpc_prefix.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ pub async fn create(
7676
})?;
7777

7878
if new_prefix.config.prefix.is_ipv6() {
79-
vpc.network_virtualization_type
79+
vpc.config
80+
.network_virtualization_type
8081
.ensure_supports_ipv6_prefix()
8182
.map_err(CarbideError::from)?;
8283
}

0 commit comments

Comments
 (0)