feat: Added support for per-interface routing-profiles#591
Open
bcavnvidia wants to merge 1 commit into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
Summary by CodeRabbit
WalkthroughThis pull request adds per-interface routing profile support (allowed anycast prefixes) end-to-end: API contract and validation, DB model + migration and DAO wiring, SDK model generation, instance handlers (single and batch), workflow reconciliation, OpenAPI updates, and tests. ChangesPer-Interface Routing Profile Feature
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🔐 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-05-30 01:21:24 UTC | Commit: 750f0bb |
![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 (2)
openapi/spec.yaml (2)
16646-16651: 💤 Low valueAlign description terminology with the Interface schema.
The descriptions for
requestedIpAddressandroutingProfileinInterfaceCreateRequestuse "cannot be specified for Subnet-based interfaces", while the corresponding fields in theInterfacecomponent schema (hunk 1, lines 16582 and 16587) use "not valid for Subnet-based interfaces" and "Only valid for VPC Prefix-based interfaces". Consistent terminology across request and response schemas improves clarity.📝 Suggested alignment
- description: Explicitly requested IP address for the interface. It cannot be specified for Subnet-based interfaces. The least-significant host bit must be 1. + description: Explicitly requested IP address for the interface. This is only used for VPC Prefix-based interfaces and is not valid for Subnet-based interfaces. The least-significant host bit must be 1.- description: Interface-local routing profile options. It cannot be specified for Subnet-based interfaces. + description: Interface-local routing profile options. Only valid for VPC Prefix-based interfaces.🤖 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 `@openapi/spec.yaml` around lines 16646 - 16651, The descriptions in InterfaceCreateRequest for requestedIpAddress and routingProfile are inconsistent with the Interface component; update the description text for requestedIpAddress and routingProfile inside the InterfaceCreateRequest schema to match the terminology used in the Interface component (e.g., use "not valid for Subnet-based interfaces" for requestedIpAddress and "Only valid for VPC Prefix-based interfaces" or equivalent for routingProfile) so both request and response schemas use the same phrases and meaning.
16613-16619: ⚡ Quick winAdd pattern validation for CIDR prefixes.
The
allowedAnycastPrefixesarray items lack format or pattern constraints. Without a pattern, clients may submit invalid CIDR strings, leading to runtime validation failures. Consider adding a pattern constraint to guide client-side validation.📐 Proposed enhancement with CIDR pattern validation
allowedAnycastPrefixes: type: array description: CIDR prefixes this interface is allowed to announce as anycast routes. default: [] items: type: string + pattern: '^([0-9]{1,3}\.){3}[0-9]{1,3}/([0-9]|[1-2][0-9]|3[0-2])$' example: 192.0.2.0/24Alternatively, if the pattern becomes complex (e.g., to enforce valid IPv4 octets), add a description note:
allowedAnycastPrefixes: type: array - description: CIDR prefixes this interface is allowed to announce as anycast routes. + description: CIDR prefixes this interface is allowed to announce as anycast routes. Each prefix must be a valid IPv4 CIDR notation (e.g., 192.0.2.0/24). Server-side validation enforces CIDR format compliance. default: [] items: type: string example: 192.0.2.0/24🤖 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 `@openapi/spec.yaml` around lines 16613 - 16619, The items of the allowedAnycastPrefixes array lack validation; add a pattern constraint under allowedAnycastPrefixes -> items to validate CIDR strings (e.g., a regex that enforces IPv4 octets and a /prefix length like ^((25[0-5]|2[0-4]\d|1\d{2}|[1-9]?\d)\.){3}(25[0-5]|2[0-4]\d|1\d{2}|[1-9]?\d)\/([0-9]|[12][0-9]|3[0-2])$), and update the items description to mention the required CIDR format so clients can do client-side validation before sending values to allowedAnycastPrefixes.
🤖 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 `@api/pkg/api/model/interface.go`:
- Around line 67-75: The validation currently stops at the first bad prefix and
returns a capitalized error string; replace the manual loop over
rp.AllowedAnycastPrefixes with ozzo-validation's validation.Each combined with
validation.By so every element is validated (using netip.ParsePrefix) and
aggregated, and change the error text to start lowercase (e.g. "invalid anycast
prefix `%s`") while keeping the field key "allowedAnycastPrefixes" to match
tests; update the validation logic referenced by rp.AllowedAnycastPrefixes to
use validation.Each(validation.By(...)) that calls netip.ParsePrefix and returns
the lowercase error message for each invalid element.
In `@sdk/standard/model_interface_routing_profile.go`:
- Around line 1-7: The file sdk/standard/model_interface_routing_profile.go is
missing the SPDX-FileCopyrightText header present in other generated model files
(e.g., model_interface.go and model_interface_create_request.go); rebase onto
the latest main and re-run the SDK generator (make generate-sdk) so the SPDX
header injection script adds the header to model_interface_routing_profile.go,
then verify the header matches the other model files and commit the regenerated
SDK.
---
Nitpick comments:
In `@openapi/spec.yaml`:
- Around line 16646-16651: The descriptions in InterfaceCreateRequest for
requestedIpAddress and routingProfile are inconsistent with the Interface
component; update the description text for requestedIpAddress and routingProfile
inside the InterfaceCreateRequest schema to match the terminology used in the
Interface component (e.g., use "not valid for Subnet-based interfaces" for
requestedIpAddress and "Only valid for VPC Prefix-based interfaces" or
equivalent for routingProfile) so both request and response schemas use the same
phrases and meaning.
- Around line 16613-16619: The items of the allowedAnycastPrefixes array lack
validation; add a pattern constraint under allowedAnycastPrefixes -> items to
validate CIDR strings (e.g., a regex that enforces IPv4 octets and a /prefix
length like
^((25[0-5]|2[0-4]\d|1\d{2}|[1-9]?\d)\.){3}(25[0-5]|2[0-4]\d|1\d{2}|[1-9]?\d)\/([0-9]|[12][0-9]|3[0-2])$),
and update the items description to mention the required CIDR format so clients
can do client-side validation before sending values to allowedAnycastPrefixes.
🪄 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: f988646a-5096-4478-b39f-4fa7c4224d3a
⛔ Files ignored due to path filters (2)
workflow-schema/schema/site-agent/workflows/v1/nico_nico.pb.gois excluded by!**/*.pb.go,!**/*.pb.goworkflow-schema/site-agent/workflows/v1/nico_nico.protois excluded by!workflow-schema/site-agent/workflows/v1/*_nico.proto
📒 Files selected for processing (36)
api/pkg/api/handler/instance.goapi/pkg/api/handler/instance_test.goapi/pkg/api/handler/instancebatch.goapi/pkg/api/handler/instancebatch_test.goapi/pkg/api/model/interface.goapi/pkg/api/model/interface_test.godb/pkg/db/model/interface.godb/pkg/db/model/interface_test.godb/pkg/migrations/20260529120000_interface_routing_profile.godocs/index.htmlopenapi/spec.yamlsdk/standard/client.gosdk/standard/model_expected_machine.gosdk/standard/model_expected_machine_create_request.gosdk/standard/model_expected_machine_update_request.gosdk/standard/model_expected_power_shelf.gosdk/standard/model_expected_power_shelf_create_request.gosdk/standard/model_expected_power_shelf_update_request.gosdk/standard/model_expected_rack.gosdk/standard/model_expected_rack_create_request.gosdk/standard/model_expected_rack_update_request.gosdk/standard/model_expected_switch.gosdk/standard/model_expected_switch_create_request.gosdk/standard/model_expected_switch_update_request.gosdk/standard/model_infini_band_partition.gosdk/standard/model_infini_band_partition_create_request.gosdk/standard/model_infini_band_partition_update_request.gosdk/standard/model_interface.gosdk/standard/model_interface_create_request.gosdk/standard/model_interface_routing_profile.gosdk/standard/model_machine_update_request.gosdk/standard/model_vpc.gosdk/standard/model_vpc_create_request.gosdk/standard/model_vpc_update_request.goworkflow/pkg/activity/instance/instance.goworkflow/pkg/activity/instance/instance_test.go
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
bcavnvidia
force-pushed
the
interface-profile
branch
from
750f0bb to
941e092
Compare
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
1 participant
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
The Rust back-end now supports per-interface routing profiles to allow callers to further restrict anycast announcements between hosts and DPUs. This PR adds the related support to the REST API.
Type of Change
Services Affected
Related Issues (Optional)
Breaking Changes
Testing
Additional Notes