Skip to content

chore: Remove deprecated flat fields from Vpc object - #4382

Draft
nvlitagaki wants to merge 3 commits into
NVIDIA:mainfrom
nvlitagaki:chore/remove-deprecated-vpc-fields
Draft

chore: Remove deprecated flat fields from Vpc object#4382
nvlitagaki wants to merge 3 commits into
NVIDIA:mainfrom
nvlitagaki:chore/remove-deprecated-vpc-fields

Conversation

@nvlitagaki

@nvlitagaki nvlitagaki commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Since the REST API no longer relies on the deprecated Vpc fields that were relocated to the Metadata, VpcConfig and VpcStatus, we can now go ahead and remove those fields. This work removes fallback logic to those deprecated fields that was present in the admin cli and admin ui logic and updates tests as necessary.

Related issues

#2794

Type of Change

  • Change - Changes in existing functionality
  • Internal - Internal changes (refactoring, tests, docs, etc.)

Breaking Changes

Testing

  • Unit tests added/updated

Additional Notes

@nvlitagaki nvlitagaki added this to the v2.2 milestone Jul 30, 2026
@nvlitagaki
nvlitagaki requested a review from a team as a code owner July 30, 2026 17:18
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Summary by CodeRabbit

  • Improvements

    • VPC details are now rendered consistently from structured configuration and status across admin pages and API responses, including VNI and network virtualization type.
    • VPC “nice format” output and VPC list/detail mappings were updated to improve reliability and metadata handling.
  • Compatibility

    • Deprecated legacy VPC fields have been retired and reserved to prevent reuse.
    • VPC-related API mappings and database/proto handling were adjusted accordingly.
  • Other

    • BMC credential rotation now accepts targeting by machine or switch (mutually exclusive) and renumbers the optional MAC field.

Walkthrough

VPC protobuf contracts now reserve deprecated flat fields. RPC mappings, fixtures, CLI output, and web rendering use structured config and status fields. BMC credential rotation requests now support machine or switch targeting through a mutually exclusive device identifier.

Changes

Structured VPC Migration

Layer / File(s) Summary
Reserve legacy VPC fields
crates/rpc/proto/forge.proto, rest-api/proto/core/src/v1/nico_nico.proto
Deprecated VPC fields are removed and their numeric tags and names are reserved.
Map structured VPC configuration and status
crates/rpc/src/model/vpc.rs, rest-api/site-workflow/..., rest-api/workflow/..., rest-api/db/..., crates/api-core/src/tests/vpc.rs
RPC mappings and fixtures populate structured configuration/status fields, while tests remove flat-field compatibility assertions.
Consume structured VPC data
crates/admin-cli/..., crates/api-web/src/vpc.rs, crates/api-web/src/ipam.rs, crates/api-web/src/tests/vpc.rs
CLI and web display paths derive VNI, virtualization type, tenant, metadata, and flat-VPC validation from structured fields.

BMC Rotation Device Targeting

Layer / File(s) Summary
Update BMC rotation request schema
crates/rpc/proto/forge.proto
BmcCredentialRotationRequest now selects a machine or switch through oneof device_id, with bmc_mac moved to field 4.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the removal of deprecated flat fields from the Vpc object.
Description check ✅ Passed The description directly explains the Vpc field removal, fallback logic changes, breaking impact, and test updates.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@nvlitagaki
nvlitagaki marked this pull request as draft July 30, 2026 17:19
@copy-pr-bot

copy-pr-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown

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.

@github-actions

Copy link
Copy Markdown

🔐 TruffleHog Secret Scan

No secrets or credentials found!

Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉

🔗 View scan details

🕐 Last updated: 2026-07-30 17:22:05 UTC | Commit: 3328713

@thossain-nv thossain-nv added the rest-api Add this label when an issue or PR concerns NICo REST API label Jul 30, 2026 — with ChatGPT Codex Connector
@nvlitagaki

Copy link
Copy Markdown
Contributor Author

/ok to test b23ac64

@nvlitagaki

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
crates/rpc/proto/forge.proto (2)

1692-1693: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Reserve dpa_vni by name in both schemas.

Both definitions reserve field number 12 but omit the legacy field name. Add reserved "dpa_vni"; in each message.

  • crates/rpc/proto/forge.proto#L1692-L1693: add the reserved name alongside reserved 12.
  • rest-api/proto/core/src/v1/nico_nico.proto#L1671-L1672: add the reserved name alongside reserved 12.

As per path instructions, protobuf field numbers and names must preserve compatibility.

🤖 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` around lines 1692 - 1693, Reserve the legacy
dpa_vni field name alongside field number 12 in both schemas: update
crates/rpc/proto/forge.proto lines 1692-1693 and
rest-api/proto/core/src/v1/nico_nico.proto lines 1671-1672 with the named
reservation, preserving both number and name compatibility.

Source: Path instructions


5892-5905: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Preserve the original bmc_mac field number in both schemas.

Reusing tag 3 for switch_id while moving bmc_mac to tag 4 breaks protobuf wire compatibility for rolling upgrades and previously serialized requests.

  • crates/rpc/proto/forge.proto#L5892-L5905: retain bmc_mac = 3 and assign switch_id an unused tag.
  • rest-api/proto/core/src/v1/nico_nico.proto#L5893-L5905: retain bmc_mac = 3 and assign switch_id an unused tag.

As per path instructions, protobuf field numbers must preserve wire compatibility. The supplied change details identify the previous bmc_mac tag as 3.

🤖 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` around lines 5892 - 5905, Preserve the original
bmc_mac field number in both schemas: in crates/rpc/proto/forge.proto lines
5892-5905 and rest-api/proto/core/src/v1/nico_nico.proto lines 5893-5905, keep
bmc_mac as field 3 and assign switch_id an unused field number instead; update
the device_id oneof consistently in both files.

Source: Path instructions

🤖 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.

Outside diff comments:
In `@crates/rpc/proto/forge.proto`:
- Around line 1692-1693: Reserve the legacy dpa_vni field name alongside field
number 12 in both schemas: update crates/rpc/proto/forge.proto lines 1692-1693
and rest-api/proto/core/src/v1/nico_nico.proto lines 1671-1672 with the named
reservation, preserving both number and name compatibility.
- Around line 5892-5905: Preserve the original bmc_mac field number in both
schemas: in crates/rpc/proto/forge.proto lines 5892-5905 and
rest-api/proto/core/src/v1/nico_nico.proto lines 5893-5905, keep bmc_mac as
field 3 and assign switch_id an unused field number instead; update the
device_id oneof consistently in both files.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d49c4a46-24b0-4753-b987-72f003addbb8

📥 Commits

Reviewing files that changed from the base of the PR and between 3328713 and b23ac64.

⛔ Files ignored due to path filters (1)
  • rest-api/proto/core/gen/v1/nico_nico.pb.go is excluded by !**/*.pb.go, !**/gen/**, !rest-api/**/*.pb.go
📒 Files selected for processing (2)
  • crates/rpc/proto/forge.proto
  • rest-api/proto/core/src/v1/nico_nico.proto

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rest-api Add this label when an issue or PR concerns NICo REST API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants