feat(rest-api): add VPC config/runtimeStatus response shape with backward compatibility#3350
Draft
hwadekar-nv wants to merge 2 commits into
Draft
feat(rest-api): add VPC config/runtimeStatus response shape with backward compatibility#3350hwadekar-nv wants to merge 2 commits into
hwadekar-nv wants to merge 2 commits into
Conversation
…ward compatibility
|
🌿 Preview your docs: https://nvidia-preview-pull-request-3350.docs.buildwithfern.com/infra-controller |
🔍 Container Scan SummaryNo Grype artifacts were found to aggregate. |
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-07-10 00:56:35 UTC | Commit: ee39bac |
Contributor
Summary by CodeRabbit
WalkthroughThe change adds structured VPC configuration and runtime status mappings across API, database, workflow, and OpenAPI layers. It also updates core protobuf imports, compatibility contracts, generated type names, legacy domain messages, and Forge-related documentation. ChangesVPC compatibility
Core protobuf contracts
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Controller as Controller VPC
participant Helpers as VpcProto helpers
participant Workflow as UpdateVpcsInDB
participant Database as VPC database
Controller->>Helpers: Extract structured config and runtime status
Helpers->>Workflow: Return normalized reported fields
Workflow->>Database: Compare and update VPC state
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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: 2
🧹 Nitpick comments (1)
rest-api/db/pkg/db/model/vpc.go (1)
209-223: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the shared pointer converters here
rest-api/db/pkg/db/model/vpc.go:209-223duplicatescutil.IntPtrToUint32Ptr/cutil.Uint32PtrToIntPtr, andrest-api/db/pkg/db/modelalready importsrest-api/common/pkg/utilelsewhere. Move these calls to the shared helper and delete the local copies so the trusted-cast behavior stays centralized.🤖 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 `@rest-api/db/pkg/db/model/vpc.go` around lines 209 - 223, Replace all uses of the local intPtrToUint32Ptr and uint32PtrToIntPtr functions in the VPC model with cutil.IntPtrToUint32Ptr and cutil.Uint32PtrToIntPtr, using the existing util import; then delete both local converter functions.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 `@rest-api/proto/core/src/v1/nico_nico.proto`:
- Around line 5161-5164: Rename all generated Go enum references from
InterfaceFunctionType_PHYSICAL_FUNCTION and
InterfaceFunctionType_VIRTUAL_FUNCTION to the constants produced by PHYSICAL and
VIRTUAL, and regenerate the Go bindings from the updated InterfaceFunctionType
definition. Update all in-tree consumers and surrounding field comments to use
the new names while preserving the existing wire values.
In `@rest-api/proto/core/src/v1/nmx_c_nico.proto`:
- Around line 1-14: Remove the contradictory duplicate license header in
nmx_c_nico.proto and retain one SPDX identifier consistent with neighboring
files and the repository’s licensing policy; update the header so it does not
simultaneously declare Apache-2.0 and LicenseRef-NvidiaProprietary.
---
Nitpick comments:
In `@rest-api/db/pkg/db/model/vpc.go`:
- Around line 209-223: Replace all uses of the local intPtrToUint32Ptr and
uint32PtrToIntPtr functions in the VPC model with cutil.IntPtrToUint32Ptr and
cutil.Uint32PtrToIntPtr, using the existing util import; then delete both local
converter functions.
🪄 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: bf84308e-3845-4ab5-91e2-9236f2db2844
⛔ Files ignored due to path filters (35)
rest-api/sdk/standard/client.gois excluded by!rest-api/sdk/standard/client.gorest-api/sdk/standard/model_batch_instance_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_machine.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_machine_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_machine_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_power_shelf.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_power_shelf_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_power_shelf_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_rack.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_rack_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_rack_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_switch.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_switch_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_expected_switch_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_partition.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_partition_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_infini_band_partition_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_type_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_instance_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_interface.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_interface_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_machine_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_network_security_group_update_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_config.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_runtime_status.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_vpc_update_request.gois excluded by!rest-api/sdk/standard/model_*.go
📒 Files selected for processing (19)
rest-api/api/pkg/api/handler/vpc.gorest-api/api/pkg/api/model/vpc.gorest-api/api/pkg/api/model/vpc_test.gorest-api/db/pkg/db/model/vpc.gorest-api/db/pkg/db/model/vpc_test.gorest-api/docs/index.htmlrest-api/openapi/spec.yamlrest-api/proto/core/src/v1/common_nico.protorest-api/proto/core/src/v1/dns_nico.protorest-api/proto/core/src/v1/fmds_nico.protorest-api/proto/core/src/v1/health_nico.protorest-api/proto/core/src/v1/machine_discovery_nico.protorest-api/proto/core/src/v1/measured_boot_nico.protorest-api/proto/core/src/v1/mlx_device_nico.protorest-api/proto/core/src/v1/nico_nico.protorest-api/proto/core/src/v1/nmx_c_nico.protorest-api/proto/core/src/v1/scout_firmware_upgrade_nico.protorest-api/proto/core/src/v1/site_explorer_nico.protorest-api/workflow/pkg/activity/vpc/vpc.go
💤 Files with no reviewable changes (2)
- rest-api/proto/core/src/v1/common_nico.proto
- rest-api/proto/core/src/v1/scout_firmware_upgrade_nico.proto
Comment on lines
5161
to
5164
| enum InterfaceFunctionType { | ||
| PHYSICAL_FUNCTION = 0; | ||
| VIRTUAL_FUNCTION = 1; | ||
| PHYSICAL = 0; | ||
| VIRTUAL = 1; | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "== Go references to the renamed enum constants =="
rg -nP 'InterfaceFunctionType_(PHYSICAL|VIRTUAL)(_FUNCTION)?\b' rest-api/ --type=go
echo "== lingering doc references to old names =="
rg -nP '(PHYSICAL_FUNCTION|VIRTUAL_FUNCTION)' rest-api/proto/core/src/v1/nico_nico.protoRepository: NVIDIA/infra-controller
Length of output: 5677
Regenerate the Go bindings with this rename The wire values are unchanged, but the generated Go constants change, so any consumer still referring to InterfaceFunctionType_PHYSICAL_FUNCTION / ..._VIRTUAL_FUNCTION will fail to compile. Several in-tree references and the surrounding field comments still use the old names; update them together.
🤖 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 `@rest-api/proto/core/src/v1/nico_nico.proto` around lines 5161 - 5164, Rename
all generated Go enum references from InterfaceFunctionType_PHYSICAL_FUNCTION
and InterfaceFunctionType_VIRTUAL_FUNCTION to the constants produced by PHYSICAL
and VIRTUAL, and regenerate the Go bindings from the updated
InterfaceFunctionType definition. Update all in-tree consumers and surrounding
field comments to use the new names while preserving the existing wire values.
Source: Path instructions
Comment on lines
+1
to
+14
| // SPDX-FileCopyrightText: Copyright (c) 2024 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| // | ||
| // SPDX-FileCopyrightText: Copyright (c) 2024 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: LicenseRef-NvidiaProprietary | ||
| // | ||
| // NVIDIA CORPORATION, its affiliates and licensors retain all intellectual | ||
| // property and proprietary rights in and to this material, related | ||
| // documentation and any modifications thereto. Any use, reproduction, | ||
| // disclosure or distribution of this material and related documentation | ||
| // without an express license agreement from NVIDIA CORPORATION or | ||
| // its affiliates is strictly prohibited. | ||
| // |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Contradictory license headers.
The header now declares two conflicting SPDX identifiers: Apache-2.0 (Line 2) immediately followed by a LicenseRef-NvidiaProprietary block (Lines 5–13) stating that use/redistribution is "strictly prohibited". In an Apache-2.0 open-source repository this dual header is contradictory and will confuse both humans and SPDX tooling. Please retain a single, correct identifier consistent with the rest of the tree.
Proposed correction
// SPDX-FileCopyrightText: Copyright (c) 2024 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0
-
-//
-// SPDX-FileCopyrightText: Copyright (c) 2024 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
-// SPDX-License-Identifier: LicenseRef-NvidiaProprietary
-//
-// NVIDIA CORPORATION, its affiliates and licensors retain all intellectual
-// property and proprietary rights in and to this material, related
-// documentation and any modifications thereto. Any use, reproduction,
-// disclosure or distribution of this material and related documentation
-// without an express license agreement from NVIDIA CORPORATION or
-// its affiliates is strictly prohibited.
-//📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // SPDX-FileCopyrightText: Copyright (c) 2024 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | |
| // SPDX-License-Identifier: Apache-2.0 | |
| // | |
| // SPDX-FileCopyrightText: Copyright (c) 2024 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | |
| // SPDX-License-Identifier: LicenseRef-NvidiaProprietary | |
| // | |
| // NVIDIA CORPORATION, its affiliates and licensors retain all intellectual | |
| // property and proprietary rights in and to this material, related | |
| // documentation and any modifications thereto. Any use, reproduction, | |
| // disclosure or distribution of this material and related documentation | |
| // without an express license agreement from NVIDIA CORPORATION or | |
| // its affiliates is strictly prohibited. | |
| // | |
| // SPDX-FileCopyrightText: Copyright (c) 2024 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | |
| // SPDX-License-Identifier: Apache-2.0 |
🤖 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 `@rest-api/proto/core/src/v1/nmx_c_nico.proto` around lines 1 - 14, Remove the
contradictory duplicate license header in nmx_c_nico.proto and retain one SPDX
identifier consistent with neighboring files and the repository’s licensing
policy; update the header so it does not simultaneously declare Apache-2.0 and
LicenseRef-NvidiaProprietary.
|
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. |
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.
Summary
Migrates the REST VPC response to the config/status pattern introduced in Core, while keeping the existing flat VPC fields populated for backward compatibility.
This is the REST-side follow-up to Core PR #2613 for issue #928.
Changes
VpcConfig/ structuredVpcStatusfrom CoreVpc.ToProto()/FromProto()to populate canonicalconfig/statusand mirror deprecated flat fieldsconfig— desired VPC configurationruntimeStatus— observed runtime state (active VNI)networkVirtualizationType,requestedVni,vni,networkSecurityGroupId, etc.) populated for existing clientsVpcConfigandVpcRuntimeStatus; marked superseded flat fields as deprecatedBackward compatibility
configandruntimeStatusstatus(Pending/Ready/etc.) is unchanged;runtimeStatusholds proto-aligned observed VNIReferences
Type of Change
Breaking Changes
Testing
Additional Notes