Skip to content

feat(rest-api): Allow users to specify UUID when creating Network Security Group#2993

Merged
bcavnvidia merged 1 commit into
NVIDIA:mainfrom
bcavnvidia:nsg-uuid-rest
Jun 29, 2026
Merged

feat(rest-api): Allow users to specify UUID when creating Network Security Group#2993
bcavnvidia merged 1 commit into
NVIDIA:mainfrom
bcavnvidia:nsg-uuid-rest

Conversation

@bcavnvidia

Copy link
Copy Markdown
Contributor

Allow callers to provide a Network Security Group UUID during create.

Related issues

#2214

Type of Change

  • Add - New feature or capability
  • Change - Changes in existing functionality
  • Fix - Bug fixes
  • Remove - Removed features or deprecated functionality
  • Internal - Internal changes (refactoring, tests, docs, etc.)

Breaking Changes

  • This PR contains breaking changes

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • No testing required (docs, internal refactor, etc.)

Additional Notes

Closes #2214.

@copy-pr-bot

copy-pr-bot Bot commented Jun 29, 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.

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

An optional ID *uuid.UUID field is added to APINetworkSecurityGroupCreateRequest. The create handler conditionally accepts this client-supplied UUID, queries the DB for conflicts, passes it into NetworkSecurityGroupCreateInput, and maps unique-constraint violations to HTTP 409. The OpenAPI schema and tests are updated accordingly.

Changes

NSG Optional Client-Specified UUID

Layer / File(s) Summary
Model field and OpenAPI contract
rest-api/api/pkg/api/model/networksecuritygroup.go, rest-api/openapi/spec.yaml
Adds optional ID *uuid.UUID (JSON: id) to APINetworkSecurityGroupCreateRequest; extends OpenAPI schema with a UUID-typed id property and example value.
Handler conflict detection and DB wiring
rest-api/api/pkg/api/handler/networksecuritygroup.go
When apiRequest.ID is present, queries the DB for an existing NSG with that ID and returns HTTP 409 on conflict. Passes the optional ID directly into NetworkSecurityGroupCreateInput (no longer unconditionally generates a UUID). Maps DB unique-constraint failures to HTTP 409 when a client-supplied ID was used.
Model and handler tests
rest-api/api/pkg/api/model/networksecuritygroup_test.go, rest-api/api/pkg/api/handler/networksecuritygroup_test.go
Updates validate and ToProto model tests for the new ID field. Adds handler fixtures for existing and soft-deleted NSGs; extends create table tests with success for a unique explicit ID, HTTP 409 for a conflicting ID, and HTTP 409 for a soft-deleted ID conflict.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description is directly related to the change and accurately states that callers can provide an NSG UUID on create.
Linked Issues check ✅ Passed The PR adds an optional client-supplied ID to NSG create request handling and tests the conflict paths, matching issue #2214.
Out of Scope Changes check ✅ Passed The visible changes stay focused on NSG create ID support, validation, tests, and API docs with no clear unrelated additions.
Title check ✅ Passed The title accurately summarizes the main change: optional user-specified UUIDs for Network Security Group creation.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

Actionable comments posted: 1

🤖 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/api/pkg/api/model/networksecuritygroup.go`:
- Around line 126-127: The NetworkSecurityGroup DTO currently declares ID as
*uuid.UUID while still applying the UUID validation rule, but that validator
only works on string-like inputs. Update the NetworkSecurityGroup model’s ID
field or its validation so the is.UUID check is not applied to *uuid.UUID;
either keep the field as a string and parse it after validation, or remove the
rule and rely on bind-time parsing. Use the ID field in NetworkSecurityGroup to
locate the change.
🪄 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: 3c96d33d-5524-43ca-8c31-9ed36f67623d

📥 Commits

Reviewing files that changed from the base of the PR and between f097c36 and 2618e89.

⛔ Files ignored due to path filters (38)
  • rest-api/flow/internal/nicoapi/gen/fmds.pb.go is excluded by !**/*.pb.go, !**/gen/**, !rest-api/**/*.pb.go
  • rest-api/flow/internal/nicoapi/gen/fmds_grpc.pb.go is excluded by !**/*.pb.go, !**/gen/**, !rest-api/**/*.pb.go, !rest-api/**/*_grpc.pb.go
  • rest-api/sdk/standard/client.go is excluded by !rest-api/sdk/standard/client.go
  • rest-api/sdk/standard/model_batch_instance_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_expected_machine.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_expected_machine_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_expected_machine_update_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_expected_power_shelf.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_expected_power_shelf_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_expected_power_shelf_update_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_expected_rack.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_expected_rack_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_expected_rack_update_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_expected_switch.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_expected_switch_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_expected_switch_update_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_infini_band_partition.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_infini_band_partition_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_infini_band_partition_update_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_instance.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_instance_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_instance_type.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_instance_type_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_instance_type_update_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_instance_update_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_interface.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_interface_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_machine.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_machine_update_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_network_security_group.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_network_security_group_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_network_security_group_update_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_tenant_summary.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_vpc.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_vpc_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_vpc_peering_tenant_summary.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_vpc_peering_vpc_summary.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_vpc_update_request.go is excluded by !rest-api/sdk/standard/model_*.go
📒 Files selected for processing (7)
  • rest-api/api/pkg/api/handler/networksecuritygroup.go
  • rest-api/api/pkg/api/handler/networksecuritygroup_test.go
  • rest-api/api/pkg/api/model/networksecuritygroup.go
  • rest-api/api/pkg/api/model/networksecuritygroup_test.go
  • rest-api/docs/index.html
  • rest-api/flow/internal/nicoapi/nicoproto/fmds.proto
  • rest-api/openapi/spec.yaml

Comment thread rest-api/api/pkg/api/model/networksecuritygroup.go
@bcavnvidia bcavnvidia marked this pull request as ready for review June 29, 2026 22:30
@bcavnvidia bcavnvidia requested a review from a team as a code owner June 29, 2026 22:30
@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown

🔍 Container Scan Summary

Service Total Critical High Medium Low Other
nico-flow 43 3 21 9 2 8
nico-nsm 49 0 10 31 8 0
nico-psm 43 3 21 9 2 8
nico-rest-api 43 3 21 9 2 8
nico-rest-cert-manager 42 3 21 9 1 8
nico-rest-db 43 3 21 9 2 8
nico-rest-site-agent 42 3 21 9 1 8
nico-rest-site-manager 42 3 21 9 1 8
nico-rest-workflow 43 3 21 9 2 8
TOTAL 390 24 178 103 21 64

Per-CVE detail lives in the per-service grype-* artifacts (JSON + SARIF). Severity counts only — no CVE IDs published 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-06-29 22:33:45 UTC | Commit: c2527bc

@thossain-nv thossain-nv changed the title fix(rest-api): Allow callers to send desired UUID for NSG creation fix(rest-api): Allow users to specify UUID when creating Network Security Group Jun 29, 2026
@thossain-nv thossain-nv changed the title fix(rest-api): Allow users to specify UUID when creating Network Security Group feat(rest-api): Allow users to specify UUID when creating Network Security Group Jun 29, 2026

@thossain-nv thossain-nv 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.

Looks good, thanks @bcavnvidia

@bcavnvidia bcavnvidia enabled auto-merge (squash) June 29, 2026 22:51
@bcavnvidia bcavnvidia merged commit 929e241 into NVIDIA:main Jun 29, 2026
119 checks passed
@bcavnvidia bcavnvidia deleted the nsg-uuid-rest branch June 29, 2026 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: add optional user-specified UUID to NetworkSecurityGroupCreateRequest

2 participants