refactor(vpc): separate Vpc fields into config/status#2613
Open
ianderson-nvidia wants to merge 1 commit into
Open
refactor(vpc): separate Vpc fields into config/status#2613ianderson-nvidia wants to merge 1 commit into
ianderson-nvidia wants to merge 1 commit into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughIntroduces a consolidated ChangesStructured VPC Config Adoption
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
ianderson-nvidia
force-pushed
the
refactor/vpc-config-status-split
branch
from
ae7092a to
d3f8545
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/api-web/src/ipam.rs (1)
565-589:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the deprecated VNI fallback on the overlay page.
This path now handles the structured tenant field, but VNI still reads only
status.vni. If a compatibility VPC hasdeprecated_vnipopulated and nostatus, the overlay table renders a blank VNI while the other VPC web renderers preserve the fallback.🐛 Proposed fix
.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 vni = vpc + .status + .as_ref() + .and_then(|status| status.vni) + .or(vpc.deprecated_vni) + .map(|vni| vni.to_string()) + .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()); @@ - vni: vpc - .status - .as_ref() - .and_then(|status| status.vni) - .map(|vni| vni.to_string()) - .unwrap_or_default(), + vni, tenant, prefixes, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/api-web/src/ipam.rs` around lines 565 - 589, The VNI field construction in the OverlayVpcDisplay struct currently only reads from status.vni without a fallback mechanism, causing blank VNI values for compatibility VPCs that have deprecated_vni populated but no status. Add a fallback to the deprecated_vni field similar to how the tenant field is handled above it. Modify the VNI field assignment to check status.vni first, and if that is not available or the status is missing, fall back to reading from the deprecated_vni field, maintaining consistency with other VPC web renderers.
🧹 Nitpick comments (2)
crates/rpc/src/model/vpc.rs (2)
53-64: ⚡ Quick winRemove redundant clones in metadata conversion.
value.clone().is_empty()allocates just to inspect the value, andmetadata.clone()copies the whole metadata payload even though the local is not reused. As per coding guidelines, clippy/warnings are treated as errors and new Rust code should avoid needless clones.♻️ Proposed fix
- value: if value.clone().is_empty() { + value: if value.is_empty() { None } else { Some(value.clone()) }, }) .collect(), }); rpc::forge::Vpc { @@ - metadata: metadata.clone(), + metadata,Also applies to: 74-74
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/rpc/src/model/vpc.rs` around lines 53 - 64, In the label conversion mapping within the metadata processing block, remove the redundant clone operation on the value reference before calling is_empty() - simply use value.is_empty() instead since checking if a string is empty does not require cloning. Additionally, address the redundant metadata.clone() operation at line 74, which clones the entire metadata payload unnecessarily when the cloned result is not reused elsewhere in the code. Apply clippy's guidance to eliminate these needless allocations as they violate the project's coding standards.Source: Coding guidelines
262-312: ⚡ Quick winExercise every mirrored compatibility field in the conversion test.
The new conversion contract also mirrors
network_security_group_idanddefault_nvlink_logical_partition_id, but the fixture leaves both asNoneand the test never asserts them. Add non-Nonecases/assertions, preferably as table-driven scenarios covering populated and absent optional config fields. As per coding guidelines, conversions should use table-driven tests that enumerate input variants and assert observable outputs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/rpc/src/model/vpc.rs` around lines 262 - 312, The test `vpc_to_rpc_populates_structured_and_deprecated_flat_fields` currently only tests a single scenario with `network_security_group_id` and `default_nvlink_logical_partition_id` set to None, leaving the mirrored compatibility fields unvalidated. Refactor this test into a table-driven test that covers multiple scenarios, including cases where `network_security_group_id` and `default_nvlink_logical_partition_id` are populated with actual values (Some) as well as None. For each test scenario, create a helper function or parameterized approach (similar to `sample_vpc()`) to build VPC instances with different optional field combinations, and add assertions verifying that both the structured config fields and their deprecated flat field mirrors are correctly populated or absent in the RPC conversion output.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/admin-cli/src/vpc/show/cmd.rs`:
- Around line 118-125: The vpc_allocated_vni function returns u32 with
unwrap_or_default() which masks unallocated or missing VNI values as 0, making
them indistinguishable from actual VNI 0. Change the return type of
vpc_allocated_vni from u32 to Option<u32> and remove the unwrap_or_default()
call to preserve the None case. Then update the call site(s) that invoke
vpc_allocated_vni to handle the Option return type and apply appropriate
formatting logic (such as displaying "unallocated" or similar) when the value is
None.
In `@crates/api-core/src/ethernet_virtualization.rs`:
- Around line 720-723: The NotFoundError for NetworkSecurityGroup in the ok_or
block is incorrectly reporting the tenant_organization_id as the missing
identifier instead of the actual NSG ID. Replace the id field in the
CarbideError::NotFoundError from v.config.tenant_organization_id.clone() with
the vpc_nsg_id variable that represents the NetworkSecurityGroup ID that was not
found. This will make error messages during debugging and incident response more
accurate and helpful.
In `@crates/api-model/src/vpc/mod.rs`:
- Around line 131-139: The VpcConfig struct exposes tenant_keyset_id and
default_nvlink_logical_partition_id as public fields, but they are hard-coded to
None in the mapper which prevents synchronization with persisted data. Either
implement the TODO by reading tenant_keyset_id from the database row (similar to
how other fields like organization_id and network_security_group_id are read
using row.try_get()), and add the corresponding DB column read for
default_nvlink_logical_partition_id, or remove both fields from the VpcConfig
struct definition until the database persistence is ready to support them. Do
not expose public model fields that cannot be populated from their source
representation.
In `@crates/rpc/proto/forge.proto`:
- Around line 1491-1500: The forge.VpcConfig message needs to support
deserialization in addition to serialization for REST API handlers to properly
deserialize incoming client payloads. Update the type_attribute for
forge.VpcConfig in the build.rs file to derive both serde::Serialize and
serde::Deserialize, matching the pattern used for other user-settable config
fields. Change the attribute from deriving only Serialize to deriving both
Serialize and Deserialize.
---
Outside diff comments:
In `@crates/api-web/src/ipam.rs`:
- Around line 565-589: The VNI field construction in the OverlayVpcDisplay
struct currently only reads from status.vni without a fallback mechanism,
causing blank VNI values for compatibility VPCs that have deprecated_vni
populated but no status. Add a fallback to the deprecated_vni field similar to
how the tenant field is handled above it. Modify the VNI field assignment to
check status.vni first, and if that is not available or the status is missing,
fall back to reading from the deprecated_vni field, maintaining consistency with
other VPC web renderers.
---
Nitpick comments:
In `@crates/rpc/src/model/vpc.rs`:
- Around line 53-64: In the label conversion mapping within the metadata
processing block, remove the redundant clone operation on the value reference
before calling is_empty() - simply use value.is_empty() instead since checking
if a string is empty does not require cloning. Additionally, address the
redundant metadata.clone() operation at line 74, which clones the entire
metadata payload unnecessarily when the cloned result is not reused elsewhere in
the code. Apply clippy's guidance to eliminate these needless allocations as
they violate the project's coding standards.
- Around line 262-312: The test
`vpc_to_rpc_populates_structured_and_deprecated_flat_fields` currently only
tests a single scenario with `network_security_group_id` and
`default_nvlink_logical_partition_id` set to None, leaving the mirrored
compatibility fields unvalidated. Refactor this test into a table-driven test
that covers multiple scenarios, including cases where
`network_security_group_id` and `default_nvlink_logical_partition_id` are
populated with actual values (Some) as well as None. For each test scenario,
create a helper function or parameterized approach (similar to `sample_vpc()`)
to build VPC instances with different optional field combinations, and add
assertions verifying that both the structured config fields and their deprecated
flat field mirrors are correctly populated or absent in the RPC conversion
output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 388077e1-70d9-4f9b-b482-1c151cddf61f
📒 Files selected for processing (20)
crates/admin-cli/src/vpc/show/cmd.rscrates/api-core/src/db_init.rscrates/api-core/src/ethernet_virtualization.rscrates/api-core/src/handlers/dpu.rscrates/api-core/src/handlers/network_segment.rscrates/api-core/src/handlers/vpc.rscrates/api-core/src/handlers/vpc_peering.rscrates/api-core/src/handlers/vpc_prefix.rscrates/api-core/src/instance/mod.rscrates/api-core/src/tests/common/api_fixtures/mod.rscrates/api-core/src/tests/network_segment.rscrates/api-core/src/tests/vpc.rscrates/api-db/src/dpa_interface.rscrates/api-db/src/instance_address.rscrates/api-model/src/vpc/mod.rscrates/api-web/src/ipam.rscrates/api-web/src/vpc.rscrates/rpc/build.rscrates/rpc/proto/forge.protocrates/rpc/src/model/vpc.rs
ianderson-nvidia
force-pushed
the
refactor/vpc-config-status-split
branch
from
d3f8545 to
40bfd90
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/api-web/src/vpc.rs (1)
47-78: 💤 Low valueConsider adding doc comments to explain fallback behavior.
These helpers centralize the deprecation compatibility logic effectively. However, the double fallback in
vpc_virt_type(line 76) is subtle—it handles the case wherevpc.configexists butconfig.network_virtualization_typeisNone. A brief doc comment on each helper would clarify their role in the deprecation migration for future maintainers.📝 Suggested documentation
+/// Returns the VPC configuration, preferring the structured `config` field +/// and falling back to deprecated flat fields for backward compatibility. #[allow(deprecated)] fn vpc_config(vpc: &forgerpc::Vpc) -> forgerpc::VpcConfig { // ... } +/// Returns the allocated VNI, preferring `status.vni` over `deprecated_vni`. #[allow(deprecated)] fn vpc_allocated_vni(vpc: &forgerpc::Vpc) -> Option<u32> { // ... } +/// Returns the virtualization type as i32, with fallback chain: +/// config.network_virtualization_type → deprecated flat field → 0 (default). #[allow(deprecated)] fn vpc_virt_type(vpc: &forgerpc::Vpc) -> i32 { // ... }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/api-web/src/vpc.rs` around lines 47 - 78, Add doc comments to the three helper functions vpc_config, vpc_allocated_vni, and vpc_virt_type to explain their role in handling deprecation compatibility. For each function, document what it does and clarify the fallback behavior—particularly for vpc_virt_type which applies a double fallback (first from vpc.config, then from the deprecated field) that should be explained for future maintainers understanding the deprecation migration strategy.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/api-web/src/vpc.rs`:
- Around line 47-78: Add doc comments to the three helper functions vpc_config,
vpc_allocated_vni, and vpc_virt_type to explain their role in handling
deprecation compatibility. For each function, document what it does and clarify
the fallback behavior—particularly for vpc_virt_type which applies a double
fallback (first from vpc.config, then from the deprecated field) that should be
explained for future maintainers understanding the deprecation migration
strategy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 38a2f7a9-106e-4d0c-988f-201cb8ec0245
📒 Files selected for processing (20)
crates/admin-cli/src/vpc/show/cmd.rscrates/api-core/src/db_init.rscrates/api-core/src/ethernet_virtualization.rscrates/api-core/src/handlers/dpu.rscrates/api-core/src/handlers/network_segment.rscrates/api-core/src/handlers/vpc.rscrates/api-core/src/handlers/vpc_peering.rscrates/api-core/src/handlers/vpc_prefix.rscrates/api-core/src/instance/mod.rscrates/api-core/src/tests/common/api_fixtures/mod.rscrates/api-core/src/tests/network_segment.rscrates/api-core/src/tests/vpc.rscrates/api-db/src/dpa_interface.rscrates/api-db/src/instance_address.rscrates/api-model/src/vpc/mod.rscrates/api-web/src/ipam.rscrates/api-web/src/vpc.rscrates/rpc/build.rscrates/rpc/proto/forge.protocrates/rpc/src/model/vpc.rs
✅ Files skipped from review due to trivial changes (1)
- crates/rpc/build.rs
🚧 Files skipped from review as they are similar to previous changes (18)
- crates/api-web/src/ipam.rs
- crates/api-core/src/handlers/vpc_prefix.rs
- crates/api-core/src/handlers/vpc.rs
- crates/api-core/src/tests/common/api_fixtures/mod.rs
- crates/api-core/src/db_init.rs
- crates/api-core/src/handlers/dpu.rs
- crates/api-core/src/tests/network_segment.rs
- crates/api-core/src/instance/mod.rs
- crates/api-db/src/dpa_interface.rs
- crates/api-core/src/handlers/network_segment.rs
- crates/api-model/src/vpc/mod.rs
- crates/rpc/src/model/vpc.rs
- crates/api-core/src/handlers/vpc_peering.rs
- crates/api-db/src/instance_address.rs
- crates/admin-cli/src/vpc/show/cmd.rs
- crates/rpc/proto/forge.proto
- crates/api-core/src/tests/vpc.rs
- crates/api-core/src/ethernet_virtualization.rs
ianderson-nvidia
force-pushed
the
refactor/vpc-config-status-split
branch
from
40bfd90 to
decd88a
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/rpc/proto/forge.proto (1)
1557-1557: ⚡ Quick winDocument the compatibility fallback for
Vpc.config.Generated clients can see
configabsent when talking to older servers or reading legacy payloads. Add a short comment thatconfigis canonical when present and deprecated flat fields are fallback-only during the migration window.Proposed proto comment
- VpcConfig config = 17; + // Canonical desired configuration for this VPC. During the legacy + // compatibility window, older payloads may omit this field; readers should + // fall back to deprecated flat fields only when config is unset. + VpcConfig config = 17;As per coding guidelines, protobuf definitions should account for “generated-client impact,” and configurable resources should separate
configandstatusfields.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/rpc/proto/forge.proto` at line 1557, The VpcConfig config field in the Vpc message lacks documentation about compatibility fallback behavior for generated clients. Add a comment to the config field that explains: the config field is canonical when present, deprecated flat fields serve as fallback-only during the migration window, and generated clients may see config absent when communicating with older servers or reading legacy payloads. This accounts for generated-client impact and follows the pattern of separating config and status fields in configurable resources.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/rpc/proto/forge.proto`:
- Line 1557: The VpcConfig config field in the Vpc message lacks documentation
about compatibility fallback behavior for generated clients. Add a comment to
the config field that explains: the config field is canonical when present,
deprecated flat fields serve as fallback-only during the migration window, and
generated clients may see config absent when communicating with older servers or
reading legacy payloads. This accounts for generated-client impact and follows
the pattern of separating config and status fields in configurable resources.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 932bdcd1-7eb3-4850-8ed9-9db9674d745e
📒 Files selected for processing (20)
crates/admin-cli/src/vpc/show/cmd.rscrates/api-core/src/db_init.rscrates/api-core/src/ethernet_virtualization.rscrates/api-core/src/handlers/dpu.rscrates/api-core/src/handlers/network_segment.rscrates/api-core/src/handlers/vpc.rscrates/api-core/src/handlers/vpc_peering.rscrates/api-core/src/handlers/vpc_prefix.rscrates/api-core/src/instance/mod.rscrates/api-core/src/tests/common/api_fixtures/mod.rscrates/api-core/src/tests/network_segment.rscrates/api-core/src/tests/vpc.rscrates/api-db/src/dpa_interface.rscrates/api-db/src/instance_address.rscrates/api-model/src/vpc/mod.rscrates/api-web/src/ipam.rscrates/api-web/src/vpc.rscrates/rpc/build.rscrates/rpc/proto/forge.protocrates/rpc/src/model/vpc.rs
✅ Files skipped from review due to trivial changes (2)
- crates/api-core/src/handlers/network_segment.rs
- crates/rpc/build.rs
🚧 Files skipped from review as they are similar to previous changes (17)
- crates/api-db/src/dpa_interface.rs
- crates/api-core/src/instance/mod.rs
- crates/api-core/src/handlers/dpu.rs
- crates/api-core/src/tests/network_segment.rs
- crates/api-core/src/db_init.rs
- crates/api-core/src/tests/common/api_fixtures/mod.rs
- crates/api-db/src/instance_address.rs
- crates/rpc/src/model/vpc.rs
- crates/api-web/src/ipam.rs
- crates/api-core/src/handlers/vpc.rs
- crates/api-core/src/handlers/vpc_peering.rs
- crates/api-core/src/handlers/vpc_prefix.rs
- crates/admin-cli/src/vpc/show/cmd.rs
- crates/api-core/src/ethernet_virtualization.rs
- crates/api-core/src/tests/vpc.rs
- crates/api-model/src/vpc/mod.rs
- crates/api-web/src/vpc.rs
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
ianderson-nvidia
force-pushed
the
refactor/vpc-config-status-split
branch
from
decd88a to
cd77e87
Compare
ianderson-nvidia
force-pushed
the
refactor/vpc-config-status-split
branch
from
cd77e87 to
669b13a
Compare
|
🌿 Preview your docs: https://nvidia-preview-pull-request-2613.docs.buildwithfern.com/infra-controller |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/api-web/src/vpc.rs (1)
82-86: 💤 Low valueRedundant
vpc_config()clone on each conversion path.Both
VpcRowDisplay::from()andVpcDetail::from()callvpc_config(&vpc)explicitly (lines 82, 191), then callvpc_virt_type(&vpc)(lines 86, 195), which internally invokesvpc_config(&vpc)again. This results in two clones of the config per conversion.Consider extracting the virtualization type from the already-computed
configto eliminate the duplicate work.♻️ Proposed refactor
impl From<forgerpc::Vpc> for VpcRowDisplay { fn from(vpc: forgerpc::Vpc) -> Self { + #[allow(deprecated)] + let virt_type_raw = vpc_config(&vpc) + .network_virtualization_type + .or(vpc.network_virtualization_type) + .unwrap_or_default(); let config = vpc_config(&vpc); Self { network_virtualization_type: format!( "{:?}", - forgerpc::VpcVirtualizationType::try_from(vpc_virt_type(&vpc)).unwrap_or_default() + forgerpc::VpcVirtualizationType::try_from(virt_type_raw).unwrap_or_default() ), // ... } } }Alternatively, inline the virtualization type extraction after obtaining
config:let config = vpc_config(&vpc); #[allow(deprecated)] let virt_type_raw = config .network_virtualization_type .or(vpc.network_virtualization_type) .unwrap_or_default();Also applies to: 191-195
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/api-web/src/vpc.rs` around lines 82 - 86, The `VpcRowDisplay::from()` and `VpcDetail::from()` methods both call `vpc_config(&vpc)` explicitly and then call `vpc_virt_type(&vpc)`, which internally calls `vpc_config(&vpc)` again, resulting in duplicate clones. Remove the redundant call to `vpc_virt_type(&vpc)` and instead extract the virtualization type directly from the already-computed `config` variable by accessing `config.network_virtualization_type`, falling back to `vpc.network_virtualization_type`, and using `unwrap_or_default()` as the final fallback. Apply this same refactoring pattern to both `VpcRowDisplay::from()` and `VpcDetail::from()` methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/api-web/src/vpc.rs`:
- Around line 82-86: The `VpcRowDisplay::from()` and `VpcDetail::from()` methods
both call `vpc_config(&vpc)` explicitly and then call `vpc_virt_type(&vpc)`,
which internally calls `vpc_config(&vpc)` again, resulting in duplicate clones.
Remove the redundant call to `vpc_virt_type(&vpc)` and instead extract the
virtualization type directly from the already-computed `config` variable by
accessing `config.network_virtualization_type`, falling back to
`vpc.network_virtualization_type`, and using `unwrap_or_default()` as the final
fallback. Apply this same refactoring pattern to both `VpcRowDisplay::from()`
and `VpcDetail::from()` methods.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: de90395f-7f3e-4a29-bab2-8f05455728d4
📒 Files selected for processing (22)
crates/admin-cli/src/vpc/show/cmd.rscrates/api-core/src/db_init.rscrates/api-core/src/ethernet_virtualization.rscrates/api-core/src/handlers/dpu.rscrates/api-core/src/handlers/network_segment.rscrates/api-core/src/handlers/vpc.rscrates/api-core/src/handlers/vpc_peering.rscrates/api-core/src/handlers/vpc_prefix.rscrates/api-core/src/instance/mod.rscrates/api-core/src/tests/machine_network.rscrates/api-core/src/tests/network_segment.rscrates/api-core/src/tests/vpc.rscrates/api-core/src/tests/vpc_peering.rscrates/api-db/src/dpa_interface.rscrates/api-db/src/instance_address.rscrates/api-model/src/vpc/mod.rscrates/api-web/src/ipam.rscrates/api-web/src/tests/vpc.rscrates/api-web/src/vpc.rscrates/rpc/build.rscrates/rpc/proto/forge.protocrates/rpc/src/model/vpc.rs
✅ Files skipped from review due to trivial changes (1)
- crates/api-web/src/tests/vpc.rs
🚧 Files skipped from review as they are similar to previous changes (18)
- crates/api-core/src/handlers/vpc_peering.rs
- crates/api-db/src/instance_address.rs
- crates/api-core/src/handlers/dpu.rs
- crates/api-core/src/db_init.rs
- crates/api-core/src/handlers/network_segment.rs
- crates/api-core/src/handlers/vpc_prefix.rs
- crates/api-core/src/handlers/vpc.rs
- crates/api-core/src/instance/mod.rs
- crates/api-core/src/ethernet_virtualization.rs
- crates/api-web/src/ipam.rs
- crates/admin-cli/src/vpc/show/cmd.rs
- crates/api-core/src/tests/network_segment.rs
- crates/rpc/build.rs
- crates/api-db/src/dpa_interface.rs
- crates/api-model/src/vpc/mod.rs
- crates/rpc/src/model/vpc.rs
- crates/api-core/src/tests/vpc.rs
- crates/rpc/proto/forge.proto
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.
ianderson-nvidia
force-pushed
the
refactor/vpc-config-status-split
branch
from
669b13a to
433703b
Compare
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
rwthompsonii
approved these changes
Jun 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Breaking Changes
Testing
Additional Notes