Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions crates/admin-cli/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1661,11 +1661,17 @@ impl ApiClient {
CarbideCliError::GenericError(format!("VPC {vpc_id} was not found"))
})?;

let VpcVirtualizationType::Flat = vpc.network_virtualization_type() else {
let network_virtualization_type = vpc
.config
.as_ref()
.and_then(|config| config.network_virtualization_type)
.and_then(|value| VpcVirtualizationType::try_from(value).ok())
.unwrap_or_default();
let VpcVirtualizationType::Flat = network_virtualization_type else {
return Err(CarbideCliError::GenericError(format!(
"VPC {} is not a flat VPC, is of type {}",
vpc_id,
vpc.network_virtualization_type().as_str_name()
network_virtualization_type.as_str_name()
)));
};

Expand Down
59 changes: 16 additions & 43 deletions crates/admin-cli/src/vpc/show/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,40 +98,6 @@ async fn show_vpc_details(
Ok(())
}

#[allow(deprecated)]
fn vpc_config(vpc: &forgerpc::Vpc) -> forgerpc::VpcConfig {
if let Some(config) = vpc.config.clone() {
config
} else {
forgerpc::VpcConfig {
tenant_organization_id: vpc.tenant_organization_id.clone(),
tenant_keyset_id: vpc.tenant_keyset_id.clone(),
network_virtualization_type: vpc.network_virtualization_type,
network_security_group_id: vpc.network_security_group_id.clone(),
default_nvlink_logical_partition_id: vpc.default_nvlink_logical_partition_id,
vni: vpc.vni,
routing_profile_type: vpc.routing_profile_type.clone(),
}
}
}

#[allow(deprecated)]
fn vpc_allocated_vni(vpc: &forgerpc::Vpc) -> u32 {
vpc.status
.as_ref()
.and_then(|status| status.vni)
.or(vpc.deprecated_vni)
.unwrap_or_default()
}

#[allow(deprecated)]
fn vpc_virt_type(vpc: &forgerpc::Vpc) -> i32 {
vpc_config(vpc)
.network_virtualization_type
.or(vpc.network_virtualization_type)
.unwrap_or_default()
}

fn convert_vpcs_to_nice_table(vpcs: forgerpc::VpcList) -> Box<Table> {
let mut table = Table::new();

Expand All @@ -149,11 +115,13 @@ fn convert_vpcs_to_nice_table(vpcs: forgerpc::VpcList) -> Box<Table> {

for vpc in vpcs.vpcs {
let metadata = vpc.metadata.as_ref().unwrap_or(&default_metadata);
let config = vpc_config(&vpc);
let virt_type = forgerpc::VpcVirtualizationType::try_from(vpc_virt_type(&vpc))
.unwrap_or_default()
.as_str_name()
.to_string();
let config = vpc.config.clone().unwrap_or_default();
let virt_type = forgerpc::VpcVirtualizationType::try_from(
config.network_virtualization_type.unwrap_or_default(),
)
.unwrap_or_default()
.as_str_name()
.to_string();

table.add_row(row![
vpc.id.unwrap_or_default(),
Expand All @@ -179,11 +147,16 @@ fn convert_vpcs_to_nice_table(vpcs: forgerpc::VpcList) -> Box<Table> {
table.into()
}

#[allow(deprecated)]
pub fn convert_vpc_to_nice_format(vpc: &forgerpc::Vpc) -> CarbideCliResult<String> {
let width = 25;
let mut lines = String::new();
let config = vpc_config(vpc);
let config = vpc.config.clone().unwrap_or_default();
let allocated_vni = vpc
.status
.as_ref()
.and_then(|status| status.vni)
.unwrap_or_default();
let network_virtualization_type = config.network_virtualization_type.unwrap_or_default();

let vpc_name = vpc
.metadata
Expand Down Expand Up @@ -219,10 +192,10 @@ pub fn convert_vpc_to_nice_format(vpc: &forgerpc::Vpc) -> CarbideCliResult<Strin
"TENANT KEYSET",
config.tenant_keyset_id.unwrap_or_default().into(),
),
("VNI", format!("{}", vpc_allocated_vni(vpc)).into()),
("VNI", format!("{allocated_vni}").into()),
(
"NW VIRTUALIZATION",
forgerpc::VpcVirtualizationType::try_from(vpc_virt_type(vpc))
forgerpc::VpcVirtualizationType::try_from(network_virtualization_type)
.unwrap_or_default()
.as_str_name()
.into(),
Expand Down
53 changes: 9 additions & 44 deletions crates/api-core/src/tests/vpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,39 +33,12 @@ use crate::tests::common::api_fixtures::{TestEnvOverrides, create_test_env_with_
use crate::tests::common::rpc_builder::{VpcCreationRequest, VpcDeletionRequest, VpcUpdateRequest};
use crate::{DatabaseError, db_init};

#[allow(deprecated)]
fn forge_vpc_config(vpc: &rpc::forge::Vpc) -> &rpc::forge::VpcConfig {
vpc.config
.as_ref()
.expect("structured config must be populated")
}

/// Backware compatibility: deprecated fields mirror structured config/status.
/// TODO Remove after rest component migrates to config/status
#[allow(deprecated)]
fn assert_vpc_config_status_compat(vpc: &rpc::forge::Vpc) {
let config = forge_vpc_config(vpc);
assert_eq!(vpc.tenant_organization_id, config.tenant_organization_id);
assert_eq!(vpc.tenant_keyset_id, config.tenant_keyset_id);
assert_eq!(vpc.vni, config.vni);
assert_eq!(
vpc.network_virtualization_type,
config.network_virtualization_type
);
assert_eq!(
vpc.network_security_group_id,
config.network_security_group_id
);
assert_eq!(
vpc.default_nvlink_logical_partition_id,
config.default_nvlink_logical_partition_id
);
assert_eq!(vpc.routing_profile_type, config.routing_profile_type);

let status = vpc.status.as_ref().expect("status must be populated");
assert_eq!(vpc.deprecated_vni, status.vni);
}

#[crate::sqlx_test]
async fn create_vpc_for_tenant_without_profile(
pool: sqlx::PgPool,
Expand Down Expand Up @@ -298,8 +271,10 @@ async fn create_vpc(pool: sqlx::PgPool) -> Result<(), Box<dyn std::error::Error>
// A VNI is allocated
assert!(forge_vpc.status.as_ref().and_then(|s| s.vni).is_some());
// The 'config' VNI and the status VNI match
assert_eq!(forge_vpc.vni, forge_vpc.status.as_ref().and_then(|s| s.vni));
assert_vpc_config_status_compat(&forge_vpc);
assert_eq!(
forge_vpc_config(&forge_vpc).vni,
forge_vpc.status.as_ref().and_then(|s| s.vni)
);

// Create another VPC by explicitly selecting a VNI from
// the allowed pool, but use the same VNI, so it should fail.
Expand Down Expand Up @@ -348,10 +323,12 @@ async fn create_vpc(pool: sqlx::PgPool) -> Result<(), Box<dyn std::error::Error>
// A VNI is allocated
assert!(forge_vpc.status.as_ref().and_then(|s| s.vni).is_some());
// The 'config' VNI is still None because this was an auto-allocated VNI
assert!(forge_vpc.vni.is_none());
assert!(forge_vpc_config(&forge_vpc).vni.is_none());
// We default to EthernetVirtualizer (proto value 0).
assert_eq!(forge_vpc.network_virtualization_type, Some(0));
assert_vpc_config_status_compat(&forge_vpc);
assert_eq!(
forge_vpc_config(&forge_vpc).network_virtualization_type,
Some(0)
);

let no_org_vpc = env
.api
Expand Down Expand Up @@ -723,14 +700,6 @@ async fn create_vpc_with_labels(pool: sqlx::PgPool) -> Result<(), Box<dyn std::e
.tenant_organization_id,
"Forge_unit_tests"
);
assert_eq!(
fetched_vpc.tenant_organization_id,
fetched_vpc
.config
.as_ref()
.expect("config")
.tenant_organization_id
);
assert_eq!(
fetched_vpc.metadata.clone().unwrap().description,
"this VPC must have labels."
Expand Down Expand Up @@ -1081,7 +1050,6 @@ async fn create_update_network_security_group_for_vpc(
forge_vpc_config(&vpc).network_security_group_id.as_deref(),
Some(good_network_security_group_id)
);
assert_vpc_config_status_compat(&vpc);

let vpc_id = vpc.id;

Expand Down Expand Up @@ -1120,7 +1088,6 @@ async fn create_update_network_security_group_for_vpc(
forge_vpc_config(&vpc).network_security_group_id.as_deref(),
Some(good_network_security_group_id)
);
assert_vpc_config_status_compat(&vpc);

// Update again to clear the the NSG attachment.
let vpc = env
Expand All @@ -1139,7 +1106,6 @@ async fn create_update_network_security_group_for_vpc(

// Make sure the VPC has no NSG ID
assert!(forge_vpc_config(&vpc).network_security_group_id.is_none());
assert_vpc_config_status_compat(&vpc);

Ok(())
}
Expand Down Expand Up @@ -1285,7 +1251,6 @@ async fn create_flat_vpc_succeeds_without_routing_profile(
vpc.status.as_ref().and_then(|s| s.vni).is_some(),
"Flat VPCs still allocate a VNI for pluggable SDN hooks (e.g. switch-side VTEPs)",
);
assert_vpc_config_status_compat(&vpc);

Ok(())
}
Expand Down
3 changes: 1 addition & 2 deletions crates/api-web/src/ipam.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,12 +567,11 @@ pub async fn overlay_html(AxumState(state): AxumState<Arc<Api>>) -> Response {
.map(|vpc| {
let id = vpc.id.map(|id| id.to_string()).unwrap_or_default();
let prefixes = prefixes_by_vpc.remove(&id).unwrap_or_default();
#[allow(deprecated)]
let tenant = vpc
.config
.as_ref()
.map(|config| config.tenant_organization_id.clone())
.unwrap_or_else(|| vpc.tenant_organization_id.clone());
.unwrap_or_default();
OverlayVpcDisplay {
id,
name: vpc
Expand Down
4 changes: 0 additions & 4 deletions crates/api-web/src/tests/vpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ async fn response_body(response: Response) -> String {
}

#[crate::sqlx_test]
#[allow(deprecated)]
async fn vpc_pages_show_status_vni(pool: sqlx::PgPool) {
let env = TestEnv::new(pool).await;
let app = make_test_app(&env.test_harness);
Expand Down Expand Up @@ -68,9 +67,6 @@ async fn vpc_pages_show_status_vni(pool: sqlx::PgPool) {
.expect("expected status VNI")
.to_string();

// Ensure this test would fail if the UI still read the old VPC vni field.
assert!(vpc.vni.is_none());

// Add a VPC prefix so the IPAM prefix detail page can render parent VPC data.
let vpc_prefix = env
.api()
Expand Down
67 changes: 24 additions & 43 deletions crates/api-web/src/vpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,55 +44,38 @@ struct VpcRowDisplay {
vni: String,
}

#[allow(deprecated)]
fn vpc_config(vpc: &forgerpc::Vpc) -> forgerpc::VpcConfig {
if let Some(config) = vpc.config.clone() {
config
} else {
forgerpc::VpcConfig {
tenant_organization_id: vpc.tenant_organization_id.clone(),
tenant_keyset_id: vpc.tenant_keyset_id.clone(),
network_virtualization_type: vpc.network_virtualization_type,
network_security_group_id: vpc.network_security_group_id.clone(),
default_nvlink_logical_partition_id: vpc.default_nvlink_logical_partition_id,
vni: vpc.vni,
routing_profile_type: vpc.routing_profile_type.clone(),
}
}
}

#[allow(deprecated)]
fn vpc_allocated_vni(vpc: &forgerpc::Vpc) -> Option<u32> {
fn vpc_allocated_vni_display(vpc: &forgerpc::Vpc) -> String {
vpc.status
.as_ref()
.and_then(|status| status.vni)
.or(vpc.deprecated_vni)
.map(|vni| vni.to_string())
.unwrap_or_default()
}

#[allow(deprecated)]
fn vpc_virt_type(vpc: &forgerpc::Vpc) -> i32 {
vpc_config(vpc)
.network_virtualization_type
.or(vpc.network_virtualization_type)
fn vpc_virtualization_type_display(config: &forgerpc::VpcConfig) -> String {
format!(
"{:?}",
forgerpc::VpcVirtualizationType::try_from(
config.network_virtualization_type.unwrap_or_default(),
)
.unwrap_or_default()
)
}

impl From<forgerpc::Vpc> for VpcRowDisplay {
fn from(vpc: forgerpc::Vpc) -> Self {
let config = vpc_config(&vpc);
let vni = vpc_allocated_vni_display(&vpc);
let config = vpc.config.unwrap_or_default();
let network_virtualization_type = vpc_virtualization_type_display(&config);

Self {
network_virtualization_type: format!(
"{:?}",
forgerpc::VpcVirtualizationType::try_from(vpc_virt_type(&vpc)).unwrap_or_default()
),
network_virtualization_type,
id: vpc.id.unwrap_or_default().to_string(),
metadata: vpc.metadata.clone().unwrap_or_default(),
metadata: vpc.metadata.unwrap_or_default(),
tenant_organization_id: config.tenant_organization_id,
tenant_keyset_id: config.tenant_keyset_id.unwrap_or_default(),
routing_profile_type: config.routing_profile_type.unwrap_or("None".to_string()),
vni: vpc_allocated_vni(&vpc)
.map(|vni| vni.to_string())
.unwrap_or_default(),
vni,
}
}
}
Expand Down Expand Up @@ -194,21 +177,19 @@ struct VpcDetail {

impl From<forgerpc::Vpc> for VpcDetail {
fn from(vpc: forgerpc::Vpc) -> Self {
let config = vpc_config(&vpc);
let vni = vpc_allocated_vni_display(&vpc);
let config = vpc.config.unwrap_or_default();
let network_virtualization_type = vpc_virtualization_type_display(&config);

Self {
network_virtualization_type: format!(
"{:?}",
forgerpc::VpcVirtualizationType::try_from(vpc_virt_type(&vpc)).unwrap_or_default()
),
network_virtualization_type,
id: vpc.id.unwrap_or_default().to_string(),
tenant_organization_id: config.tenant_organization_id,
tenant_keyset_id: config.tenant_keyset_id.unwrap_or_default(),
routing_profile_type: config.routing_profile_type.unwrap_or("None".to_string()),
vni: vpc_allocated_vni(&vpc)
.map(|vni| vni.to_string())
.unwrap_or_default(),
vni,
metadata_detail: super::MetadataDetail {
metadata: vpc.metadata.clone().unwrap_or_default(),
metadata: vpc.metadata.unwrap_or_default(),
metadata_version: vpc.version,
},
peerings: Vec::new(),
Expand Down
Loading