Skip to content
Merged
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
1 change: 1 addition & 0 deletions crates/admin-cli/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2245,6 +2245,7 @@ impl ApiClient {
metadata,
network_security_group_id,
default_nvlink_logical_partition_id: None,
routing_profile_overrides: None,
};
self.0
.update_vpc(request)
Expand Down
114 changes: 113 additions & 1 deletion crates/agent/src/main_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -579,14 +579,35 @@ impl CurrentNetworkVersion {
// behavior from the versioning.
});

if let Some(routing_profile) = &conf.routing_profile {
let hash_routing_profile = |routing_profile: &rpc::RoutingProfile,
h: &mut DefaultHasher| {
routing_profile.accepted_leaks_from_underlay.hash(h);
routing_profile.allowed_anycast_prefixes.hash(h);
routing_profile.leak_default_route_from_underlay.hash(h);
routing_profile.leak_tenant_host_routes_to_underlay.hash(h);
routing_profile.route_target_imports.hash(h);
routing_profile.route_targets_on_exports.hash(h);
routing_profile.tenant_leak_communities_accepted.hash(h);
};

if let Some(routing_profile) = &conf.routing_profile {
hash_routing_profile(routing_profile, h);
}

// Parent VPC profile changes do not increment either version supplied
// to the agent, so hash every interface's resolved profile explicitly.
// TODO: Consider replacing this fallback hash with explicit invalidation
// by incrementing every affected instance's network-config or config
// version, or by introducing a dedicated dependency version. Fan-out
// updates could contend at scale, and changing an instance version is
// misleading when only its parent VPC policy changed.
conf.tenant_interfaces.len().hash(h);
for interface in &conf.tenant_interfaces {
interface.internal_uuid.hash(h);
interface.vpc_routing_profile.is_some().hash(h);
if let Some(routing_profile) = &interface.vpc_routing_profile {
hash_routing_profile(routing_profile, h);
}
}

if let Some(traffic_intercept_config) = &conf.traffic_intercept_config {
Expand Down Expand Up @@ -1641,6 +1662,97 @@ ATF: v2.2(release):4.9.3-")
mod test {
use super::*;

/// Verifies the supplemental cache hash tracks every interface's parent
/// VPC profile and stable identity so non-first changes trigger rendering.
#[test]
fn test_current_network_version_hashes_all_vpc_routing_profiles() {
use carbide_test_support::value_scenarios;

/// Selects the non-first interface mutation applied after caching.
#[derive(Clone, Copy, Debug)]
enum InterfaceChange {
Unchanged,
Profile,
ProfilePresence,
Identity,
}

value_scenarios!(run = |change| {
let first_profile = rpc::RoutingProfile {
leak_default_route_from_underlay: true,
..Default::default()
};
let second_profile = rpc::RoutingProfile::default();
let mut conf = Arc::new(ManagedHostNetworkConfigResponse {
managed_host_config_version: "managed-v1".to_string(),
instance_network_config_version: "instance-v1".to_string(),
// Preserve the compatibility field derived from the first
// interface so only per-interface hashing can catch changes.
routing_profile: Some(first_profile.clone()),
tenant_interfaces: vec![
rpc::FlatInterfaceConfig {
internal_uuid: Some(::rpc::common::Uuid {
value: "first-interface".to_string(),
}),
vpc_routing_profile: Some(first_profile),
..Default::default()
},
rpc::FlatInterfaceConfig {
internal_uuid: Some(::rpc::common::Uuid {
value: "second-interface".to_string(),
}),
vpc_routing_profile: Some(second_profile),
..Default::default()
},
],
..Default::default()
});
let mut current = CurrentNetworkVersion::default();
current.update_from(&conf);

// Change only the second interface while leaving both compared
// configuration versions and the compatibility profile untouched.
let second_interface = &mut Arc::get_mut(&mut conf)
.expect("network configuration must not be shared")
.tenant_interfaces[1];
match change {
InterfaceChange::Unchanged => {}
InterfaceChange::Profile => {
second_interface
.vpc_routing_profile
.as_mut()
.expect("second interface profile")
.leak_default_route_from_underlay = true;
}
InterfaceChange::ProfilePresence => {
second_interface.vpc_routing_profile = None;
}
InterfaceChange::Identity => {
second_interface.internal_uuid = Some(::rpc::common::Uuid {
value: "replacement-interface".to_string(),
});
}
}

current.matches_versions_from(&conf)
};
"unchanged interface state" {
// Identical interface profiles and identities remain cached.
InterfaceChange::Unchanged => true,
}
"non-first interface changes" {
// A changed parent VPC profile must invalidate the cached render.
InterfaceChange::Profile => false,
// Profile presence is meaningful even when all present fields
// would otherwise have default values.
InterfaceChange::ProfilePresence => false,
// Interface identity prevents profiles from being silently
// reassociated with a different port.
InterfaceChange::Identity => false,
}
);
}

#[test]
fn test_current_network_version_hashes_nested_managed_host_config() {
use carbide_test_support::value_scenarios;
Expand Down
2 changes: 1 addition & 1 deletion crates/api-core/src/cfg/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ override are combined, properties still unset use the effective defaults above.
| `organization_id` | `Option<String>` | — | Tenant organization that owns the seeded VPC. |
| `network_virtualization_type` | `VpcVirtualizationType` | **required** | Data plane used by the VPC. |
| `routing_profile_type` | `Option<String>` | — | Named FNN routing profile recorded on the seeded VPC. |
| `routing_profile_overrides` | `Option<VpcRoutingProfileOverrides>` | — | Unsupported for seeded VPCs. Any configured value causes startup to fail; inline overrides are accepted only by VPC creation requests. |
| `routing_profile_overrides` | `Option<VpcRoutingProfileOverrides>` | — | Unsupported for seeded VPCs. Any configured value causes startup to fail; inline overrides are accepted only by VPC creation or update requests. |
| `vni` | `Option<i32>` | — | Desired VNI; when absent, one is allocated. |

### `PrefixFilterPolicyEntry`
Expand Down
106 changes: 65 additions & 41 deletions crates/api-core/src/handlers/vpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,68 +158,92 @@ pub(crate) async fn update(
request: Request<rpc::VpcUpdateRequest>,
) -> Result<Response<rpc::VpcUpdateResult>, Status> {
log_request_data(&request);

let vpc_update_request = request.get_ref();
// Preserve operator errors previously returned by handler-level validation
// without changing how unrelated request-conversion errors are represented.
let vpc_update = UpdateVpc::try_from(request.into_inner()).map_err(|error| match error {
RpcDataConversionError::MissingArgument("id") => {
CarbideError::InvalidArgument("VPC ID is required".to_string()).into()
}
error @ RpcDataConversionError::InvalidNetworkSecurityGroupId(_) => {
CarbideError::from(error).into()
}
error => Status::from(error),
})?;

let mut txn = api.txn_begin().await?;

// If a security group is applied to the VPC, we need to do some validation.
if let Some(ref nsg_id) = vpc_update_request.network_security_group_id {
let id = nsg_id.parse::<NetworkSecurityGroupId>().map_err(|e| {
CarbideError::from(RpcDataConversionError::InvalidNetworkSecurityGroupId(
e.value(),
))
})?;

let vpc_id = vpc_update_request
.id
.ok_or_else(|| CarbideError::InvalidArgument("VPC ID is required".to_string()))?;

// Query for the VPC because we need to do
// some validation against the request.
let Some(vpc) = db::vpc::find_by(&mut txn, ObjectColumnFilter::One(vpc::IdColumn, &vpc_id))
.await?
.pop()
else {
// Security-group and routing-profile changes both require validation
// against the VPC's persisted tenant and virtualization type.
if vpc_update.network_security_group_id.is_some()
|| vpc_update.routing_profile_overrides.is_some()
{
let Some(vpc) = db::vpc::find_by(
&mut txn,
ObjectColumnFilter::One(vpc::IdColumn, &vpc_update.id),
)
.await?
.pop() else {
return Err(CarbideError::NotFoundError {
kind: "Vpc",
id: vpc_id.to_string(),
id: vpc_update.id.to_string(),
}
.into());
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Query to check the validity of the NSG ID but to also grab
// a row-level lock on it if it exists.
if network_security_group::find_by_ids(
&mut txn,
std::slice::from_ref(&id),
Some(
&vpc.config
.tenant_organization_id
.parse()
.map_err(|e: InvalidTenantOrg| {
// Validate ownership while taking a row lock on the security group.
if let Some(ref network_security_group_id) = vpc_update.network_security_group_id
&& network_security_group::find_by_ids(
&mut txn,
std::slice::from_ref(network_security_group_id),
Some(&vpc.config.tenant_organization_id.parse().map_err(
|e: InvalidTenantOrg| {
CarbideError::from(RpcDataConversionError::InvalidTenantOrg(e.to_string()))
})?,
),
true,
)
.await?
.pop()
.is_none()
},
)?),
true,
)
.await?
.pop()
.is_none()
{
return Err(CarbideError::FailedPrecondition(format!(
"NetworkSecurityGroup `{}` does not exist or is not owned by tenant `{}`",
id, vpc.config.tenant_organization_id
"NetworkSecurityGroup `{network_security_group_id}` does not exist or is not owned by tenant `{}`",
vpc.config.tenant_organization_id
))
.into());
}

// Only FNN data planes can consume the persisted routing policy.
if let Some(routing_profile_overrides) = vpc_update.routing_profile_overrides.as_ref() {
vpc.config
.network_virtualization_type
.ensure_supports_routing_profiles()
.map_err(CarbideError::from)?;

// Inline policy is meaningful only when the current runtime
// configuration can supply the VPC's named base profile.
let (Some(fnn), Some(_)) = (
api.runtime_config.fnn.as_ref(),
vpc.config.routing_profile_type.as_ref(),
) else {
return Err(CarbideError::FailedPrecondition(
"FNN configuration and a named VPC routing profile are required to update routing-profile overrides"
.to_string(),
)
.into());
};

let mut candidate_config = vpc.config.clone();
candidate_config.routing_profile_overrides = Some(routing_profile_overrides.clone());
fnn.resolve_vpc_routing_profile(&candidate_config)?;
}
}

// Note: Because VNI allocation happens on creation and depends on the routing profile type,
// we can't allow VPCs to change routing profiles unless we also release and re-allocate their VNIs.
// It's better to keep the property immutable.

let vpc = db::vpc::update(&UpdateVpc::try_from(request.into_inner())?, &mut txn).await?;
let vpc = db::vpc::update(&vpc_update, &mut txn).await?;

txn.commit().await?;

Expand Down
Loading
Loading