Skip to content

feat(rest-api): Add DPU machine retrieval endpoint#2539

Merged
parmani-nv merged 3 commits into
NVIDIA:mainfrom
parmani-nv:feat/nested-machine-port
Jun 29, 2026
Merged

feat(rest-api): Add DPU machine retrieval endpoint#2539
parmani-nv merged 3 commits into
NVIDIA:mainfrom
parmani-nv:feat/nested-machine-port

Conversation

@parmani-nv

@parmani-nv parmani-nv commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

##Description
Adds a read endpoint to retrieve the DPU machines attached to a host Machine,
exposing DPU metadata REST API instead of only the internal gRPC/admin surface.

Related issues

#2176

Type of Change

  • Add - New feature or capability

What's included:

  • New endpoint GET /v2/org/{org}/nico/machine/{machineId}/dpu returning DPUs
    attached to the host, backed by a synchronous Temporal GetDpuMachines
    workflow scoped to the Machine's Site (api/pkg/api/handler/machine.go,
    routes.go).
  • DpuMachine API models + conversions (api/pkg/api/model/dpumachine.go,
    machine.go), OpenAPI spec + docs, and generated SDKs (standard + simple)
    and CLI TUI command.
  • site-agent registers the GetDpuMachines workflow and GetDpuMachinesByIDs
    activity (site-agent/.../machine/subscriber.go).

Breaking Changes

  • This PR contains breaking changes

Testing

  • Unit tests added/updated (machine_test.go, dpumachine_test.go,
    routes_test.go)

@copy-pr-bot

copy-pr-bot Bot commented Jun 12, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

A new DPU-machine retrieval endpoint returns DPU machines attached to a host machine through Temporal. The change also adds the corresponding API models, SDK clients, OpenAPI definitions, CLI command, and site-agent workflow/activity registration.

Changes

DPU Machines Endpoint

Layer / File(s) Summary
API Handler and Route Registration
rest-api/api/pkg/api/handler/machine.go, rest-api/api/pkg/api/routes.go, rest-api/api/pkg/api/routes_test.go
GetAllDpuMachineHandler validates org membership, loads the host machine and site, enforces authorization and site state, deduplicates attached DPU IDs from interfaces, short-circuits empty results, and executes the GetDpuMachines workflow with timeout/error handling. The route and route-count test are updated.
Handler Integration Tests
rest-api/api/pkg/api/handler/machine_test.go
DB fixtures, Temporal mocks, and table-driven cases cover authorization, site preconditions, workflow success, workflow error, timeout, and span validation.
DPU Machine API Models and Conversion
rest-api/api/pkg/api/model/dpumachine.go, rest-api/api/pkg/api/model/dpumachine_test.go
Adds APIDpuMachine and the nested DPU model graph with protobuf-to-API mapping helpers and tests for field mapping, nil handling, and context-derived IDs.
Machine Model Refactoring and New Placement Fields
rest-api/api/pkg/api/model/machine.go
Extracts model mapping helpers, adds deprecated health alert fields, and introduces AssociatedDpuMachineIds and PlacementInRack on APIMachine.
Standard SDK HTTP API Client
rest-api/sdk/standard/api_machine.go, rest-api/sdk/standard/api_expected_rack.go
Adds the generated GetDpuMachines request/execute path and updates Expected Racks authorization wording in the SDK docs.
Standard SDK DPU Core Models
rest-api/sdk/standard/model_dpu_machine.go, rest-api/sdk/standard/model_dpu_machine_interface.go, rest-api/sdk/standard/model_dpu_machine_software_component.go
Adds generated SDK models for DpuMachine, DpuMachineInterface, and DpuMachineSoftwareComponent with constructors, accessors, nullable wrappers, and strict JSON handling.
Standard SDK DPU Network and Security Group Models
rest-api/sdk/standard/model_dpu_network_config.go, rest-api/sdk/standard/model_managed_host_network_config.go, rest-api/sdk/standard/model_managed_host_quarantine_state.go, rest-api/sdk/standard/model_interface_network_security_group_config.go, rest-api/sdk/standard/model_resolved_network_security_group_rule.go
Adds the DPU network, managed-host network/quarantine, and security-group SDK models with accessors and strict decode behavior.
Standard SDK DpuInterfaceConfig Model
rest-api/sdk/standard/model_dpu_interface_config.go
Adds the generated DpuInterfaceConfig SDK model with required fields, optional nested config, JSON serialization, and required-field validation.
Simple SDK Client Wrapper
rest-api/sdk/simple/client.go, rest-api/sdk/simple/machine.go
Exposes GetDpuMachines on the simple client and machine manager, wiring the request through to the generated API client.
OpenAPI Spec, Mustache Template, and SDK Generation
rest-api/openapi/spec.yaml, rest-api/openapi/templates/go/model_simple.mustache, rest-api/Makefile
Adds the endpoint path, DPU schemas, PlacementInRack, and associatedDpuMachineIds, updates Expected Racks docs, adds the Go model template, and points SDK generation at that template directory.
CLI Command and Temporal Registration
rest-api/cli/tui/commands.go, rest-api/cli/tui/repl.go, rest-api/site-agent/pkg/components/managers/machine/subscriber.go
Adds the machine dpu get CLI command and autocomplete mapping, and registers the DPU workflow and activity in the site-agent subscriber.

Sequence Diagram

sequenceDiagram
  participant Client
  participant GetAllDpuMachineHandler
  participant Database
  participant TemporalClient

  Client->>GetAllDpuMachineHandler: GET /v2/org/{org}/nico/machine/{id}/dpu
  GetAllDpuMachineHandler->>Database: Load Machine + Site relation
  GetAllDpuMachineHandler->>Database: Query MachineInterfaces for AttachedDPUMachineIDs
  alt No DPU IDs
    GetAllDpuMachineHandler-->>Client: 200 empty array
  else DPU IDs present
    GetAllDpuMachineHandler->>TemporalClient: ExecuteWorkflow GetDpuMachines(dpuIDs)
    alt Workflow success
      TemporalClient-->>GetAllDpuMachineHandler: []DpuMachine protos
      GetAllDpuMachineHandler-->>Client: 200 []APIDpuMachine JSON
    else Workflow timeout
      TemporalClient-->>GetAllDpuMachineHandler: TimeoutError
      GetAllDpuMachineHandler->>TemporalClient: TerminateWorkflow
      GetAllDpuMachineHandler-->>Client: timeout error
    else Workflow error
      TemporalClient-->>GetAllDpuMachineHandler: WorkflowError
      GetAllDpuMachineHandler-->>Client: API error response
    end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 62.50% 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
Title check ✅ Passed The title clearly summarizes the main change: adding a DPU machine retrieval endpoint.
Description check ✅ Passed The description is directly aligned with the changeset and accurately summarizes the new endpoint, models, SDKs, CLI, and tests.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@parmani-nv

Copy link
Copy Markdown
Contributor Author

/ok to test 0085855

@parmani-nv

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 12, 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.

Actionable comments posted: 7

🧹 Nitpick comments (2)
rest-api/api/pkg/api/routes_test.go (1)

36-58: 📐 Maintainability & Code Quality | ⚡ Quick win

Assert the new DPU route explicitly, not just the bucket count.

Incrementing "machine" to 6 only proves that one more machine-scoped route exists. It will not catch a wrong method or path for /machine/:id/dpu, which is now part of the SDK/OpenAPI contract. Add a direct assertRouteExists for the new GET route.

♻️ Suggested assertion
+			expectedMachineDpuPath := "/org/:orgName/" + cfg.GetAPIName() + "/machine/:id/dpu"
+			assertRouteExists(t, got, http.MethodGet, expectedMachineDpuPath)
🤖 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/api/pkg/api/routes_test.go` around lines 36 - 58, The test currently
only increments the "machine" bucket in routeCount which won't verify the exact
new GET /machine/:id/dpu endpoint; update routes_test.go to add an explicit
assertion using the test helper (e.g., call assertRouteExists or the existing
route assertion function) to verify a GET route for the path "/machine/:id/dpu"
(or its test-path equivalent) is registered and uses the correct method, in
addition to leaving the "machine" count change; locate the assertion helpers and
the routeCount map to insert the new assertRouteExists("GET",
"/machine/:id/dpu") call so the SDK/OpenAPI contract is verified directly.
rest-api/api/pkg/api/handler/machine_test.go (1)

3441-3447: 📐 Maintainability & Code Quality | ⚡ Quick win

Split this negative-path case so each subtest targets one privilege predicate.

This fixture currently omits both targeted-creation capability and provider tenant-account linkage, so the 403 can come from multiple preconditions. Please split into two focused subtests (one missing capability, one missing account) to keep failure-branch intent deterministic.

Based on learnings, failure-path tests should isolate exactly one validation branch per subtest.

🤖 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/api/pkg/api/handler/machine_test.go` around lines 3441 - 3447, The
test case "non-privileged tenant is rejected (requirePrivilegedTenant=true)"
bundles two failure reasons; split it into two focused subtests: one named e.g.
"rejected when missing creation capability (requirePrivilegedTenant=true)" that
keeps reqOrgName: tnOrgRegular, user: tnuRegular, mID: mWithDpu.ID but sets scp
to the value representing "missing capability" (instead of scpOK) and
expectedStatus http.StatusForbidden; and a second named e.g. "rejected when not
linked to provider account (requirePrivilegedTenant=true)" that keeps scp: scpOK
but modifies the tenant/account linkage so the user’s org is not linked to the
provider account (use the existing fixture that represents an unlinked tenant or
adjust reqOrgName/user to the unlinked fixture) with expectedStatus
http.StatusForbidden; ensure both subtests only trigger one validation branch
each so failures are deterministic.

Source: Learnings

🤖 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/handler/machine_test.go`:
- Around line 3369-3375: The test sets up mock expectations on wrunTimeout and
tscTimeout (e.g., ExecuteWorkflow, TerminateWorkflow, wrunTimeout.Get/GetID) but
never verifies them; add assertions such as tscTimeout.AssertExpectations(t) and
wrunTimeout.AssertExpectations(t) (or specific AssertCalled/AssertNumberOfCalls
checks) after the test exercise so the ExecuteWorkflow and TerminateWorkflow
expectations and the wrunTimeout interactions are validated and regressions in
timeout cleanup are caught.

In `@rest-api/api/pkg/api/handler/machine.go`:
- Around line 2066-2071: The handler currently calls gmdh.scp.GetClientByID (the
site client lookup) before checking whether the machine has an
AttachedDPUMachineID, causing a 500 if the site client pool is unavailable even
though the DB can answer an empty result; update the handler in machine.go to
first check machine.AttachedDPUMachineID (or equivalent field) and immediately
return the empty response (e.g., []) when there's no attached DPU, only then
proceed to call gmdh.scp.GetClientByID; apply the same change to the other
occurrence that uses gmdh.scp.GetClientByID around the code referenced (lines
2092-2095) so the endpoint returns [] instead of a 500 when the site client is
unavailable but no DPU is attached.
- Around line 2098-2113: The current StartWorkflowOptions uses a fixed
WorkflowID ("dpu-machines-get-"+mID) which can cause Temporal to reuse an
existing run and return stale inputs; update the workflowOptions (where
StartWorkflowOptions is constructed for ExecuteWorkflow/GetDpuMachines with
dpuMachineIDs) to either omit WorkflowID so Temporal auto-generates a unique ID
or generate a request-scoped unique ID (e.g., append a UUID/timestamp) and/or
set WorkflowIDReusePolicy/WorkflowIDConflictPolicy to fail on conflict so each
request creates its own run; change the construction of workflowOptions before
calling stc.ExecuteWorkflow("GetDpuMachines", ...) accordingly.

In `@rest-api/api/pkg/api/model/dpumachine.go`:
- Around line 354-367: FromResponseProto currently appends an APIDpuMachine even
when APIDpuMachine.FromProto no-ops (producing a zero-value), which leaks empty
entries; update the loop in FromResponseProto to skip appending when
protoDpuMachine is nil or when the converted apiDpuMachine is empty/invalid —
for example, after calling apiDpuMachine.FromProto(protoDpuMachine, ctx) check a
definitive field (e.g., apiDpuMachine.Machine != nil or implement an
IsZero/IsValid method on APIDpuMachine) and only append to *apds when that check
passes, preserving existing symbols FromResponseProto, APIDpuMachines,
APIDpuMachine.FromProto and protoDpuMachine.

In `@rest-api/api/pkg/api/model/machine.go`:
- Around line 555-557: APIMachineHealth.FromProto and
APIMachineHealthProbeAlert.FromProto currently call ObservedAt.AsTime() and
InAlertSince.AsTime() unconditionally, which yields the 1970-01-01 epoch when
the protobuf Timestamp is nil; update both methods to check for nil (or the
Timestamp.IsValid/Has) before calling AsTime().Format(time.RFC3339) and only set
mh.ObservedAt, mh.ObservedAtDeprecated, and the probe's InAlertSince pointer
fields when the proto timestamp is present, leaving the *string fields nil when
absent to avoid emitting epoch strings.

In `@rest-api/openapi/spec.yaml`:
- Around line 9148-9152: The example response in the OpenAPI spec uses the same
value for the DPU Machine identifier and its host machine ("id" and
"hostMachineId"), which are distinct entities; update the example under the
relevant schema so "id" and "hostMachineId" use different unique values (e.g.,
different UUID-like strings) while keeping "infrastructureProviderId" and
"siteId" unchanged to clearly convey the DPU vs host relationship.

In `@rest-api/sdk/standard/model_dpu_machine.go`:
- Around line 533-572: The UnmarshalJSON currently only verifies keys exist in
allProperties but doesn't reject JSON nulls; update the requiredProperties check
in DpuMachine.UnmarshalJSON to also fail when the value is nil (e.g., after err
= json.Unmarshal into allProperties, loop: for _, requiredProperty := range
requiredProperties { val, exists := allProperties[requiredProperty]; if !exists
|| val == nil { return fmt.Errorf("no value given for required property %v",
requiredProperty) } } ), ensuring null values are rejected before decoding into
varDpuMachine and using decoder.DisallowUnknownFields().

---

Nitpick comments:
In `@rest-api/api/pkg/api/handler/machine_test.go`:
- Around line 3441-3447: The test case "non-privileged tenant is rejected
(requirePrivilegedTenant=true)" bundles two failure reasons; split it into two
focused subtests: one named e.g. "rejected when missing creation capability
(requirePrivilegedTenant=true)" that keeps reqOrgName: tnOrgRegular, user:
tnuRegular, mID: mWithDpu.ID but sets scp to the value representing "missing
capability" (instead of scpOK) and expectedStatus http.StatusForbidden; and a
second named e.g. "rejected when not linked to provider account
(requirePrivilegedTenant=true)" that keeps scp: scpOK but modifies the
tenant/account linkage so the user’s org is not linked to the provider account
(use the existing fixture that represents an unlinked tenant or adjust
reqOrgName/user to the unlinked fixture) with expectedStatus
http.StatusForbidden; ensure both subtests only trigger one validation branch
each so failures are deterministic.

In `@rest-api/api/pkg/api/routes_test.go`:
- Around line 36-58: The test currently only increments the "machine" bucket in
routeCount which won't verify the exact new GET /machine/:id/dpu endpoint;
update routes_test.go to add an explicit assertion using the test helper (e.g.,
call assertRouteExists or the existing route assertion function) to verify a GET
route for the path "/machine/:id/dpu" (or its test-path equivalent) is
registered and uses the correct method, in addition to leaving the "machine"
count change; locate the assertion helpers and the routeCount map to insert the
new assertRouteExists("GET", "/machine/:id/dpu") call so the SDK/OpenAPI
contract is verified directly.
🪄 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: cb93aa76-dd45-485b-a7ac-056f19a88ca1

📥 Commits

Reviewing files that changed from the base of the PR and between 64b8c1b and 0085855.

📒 Files selected for processing (25)
  • rest-api/api/pkg/api/handler/machine.go
  • rest-api/api/pkg/api/handler/machine_test.go
  • rest-api/api/pkg/api/model/dpumachine.go
  • rest-api/api/pkg/api/model/dpumachine_test.go
  • rest-api/api/pkg/api/model/machine.go
  • rest-api/api/pkg/api/routes.go
  • rest-api/api/pkg/api/routes_test.go
  • rest-api/cli/tui/commands.go
  • rest-api/cli/tui/repl.go
  • rest-api/docs/index.html
  • rest-api/openapi/spec.yaml
  • rest-api/sdk/simple/client.go
  • rest-api/sdk/simple/machine.go
  • rest-api/sdk/standard/api_expected_rack.go
  • rest-api/sdk/standard/api_machine.go
  • rest-api/sdk/standard/model_dpu_interface_config.go
  • rest-api/sdk/standard/model_dpu_machine.go
  • rest-api/sdk/standard/model_dpu_machine_interface.go
  • rest-api/sdk/standard/model_dpu_machine_software_component.go
  • rest-api/sdk/standard/model_dpu_network_config.go
  • rest-api/sdk/standard/model_interface_network_security_group_config.go
  • rest-api/sdk/standard/model_managed_host_network_config.go
  • rest-api/sdk/standard/model_managed_host_quarantine_state.go
  • rest-api/sdk/standard/model_resolved_network_security_group_rule.go
  • rest-api/site-agent/pkg/components/managers/machine/subscriber.go

Comment thread rest-api/api/pkg/api/handler/machine_test.go
Comment thread rest-api/api/pkg/api/handler/machine.go Outdated
Comment thread rest-api/api/pkg/api/handler/machine.go
Comment thread rest-api/api/pkg/api/model/dpumachine.go Outdated
Comment thread rest-api/api/pkg/api/model/machine.go Outdated
Comment thread rest-api/openapi/spec.yaml Outdated
Comment thread rest-api/sdk/standard/model_dpu_machine.go
@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-12 21:55:38 UTC | Commit: 0085855

@github-actions

github-actions Bot commented Jun 12, 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.

@parmani-nv parmani-nv force-pushed the feat/nested-machine-port branch from 0085855 to 933e461 Compare June 25, 2026 21:00
@parmani-nv

Copy link
Copy Markdown
Contributor Author

/ok to test 933e461

@parmani-nv parmani-nv force-pushed the feat/nested-machine-port branch 2 times, most recently from 22d60b5 to c8ee61c Compare June 25, 2026 22:27
@parmani-nv parmani-nv marked this pull request as ready for review June 25, 2026 22:27
@parmani-nv parmani-nv requested a review from a team as a code owner June 25, 2026 22:27
@parmani-nv

Copy link
Copy Markdown
Contributor Author

/ok to test c8ee61c

@parmani-nv parmani-nv force-pushed the feat/nested-machine-port branch from c8ee61c to 0894dcd Compare June 25, 2026 23:31

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

Overall looks great @parmani-nv, added a few notes.

Comment thread rest-api/api/pkg/api/model/dpumachine.go Outdated
Comment thread rest-api/api/pkg/api/model/dpumachine.go Outdated
Comment thread rest-api/api/pkg/api/model/dpumachine.go
Comment thread rest-api/api/pkg/api/handler/machine.go Outdated
Comment thread rest-api/api/pkg/api/model/dpumachine.go Outdated
Comment thread rest-api/api/pkg/api/model/machine.go Outdated
@@ -0,0 +1,573 @@
// checks if the {{classname}} type satisfies the MappedNullable interface at compile time

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.

@parmani-nv This seems very useful, can you please summarize the effect of using this template?

@parmani-nv parmani-nv force-pushed the feat/nested-machine-port branch from 0894dcd to 5caa900 Compare June 26, 2026 21:27

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

Thanks for porting this @parmani-nv Left some final suggestions.

Comment thread rest-api/openapi/spec.yaml Outdated
Comment thread rest-api/openapi/spec.yaml
Comment thread rest-api/api/pkg/api/model/dpumachine.go Outdated
@parmani-nv parmani-nv force-pushed the feat/nested-machine-port branch 3 times, most recently from 3d7d78f to bd5f067 Compare June 29, 2026 17:16

@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: 2

🤖 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/handler/machine.go`:
- Around line 2035-2043: Move the SiteStatusRegistered check in machine handler
flow so it runs only after authorization and association validation. In the
relevant machine endpoint logic around the IsProviderOrTenant and
site-association checks, return the 403 for unauthorized callers first, and only
then enforce the registered-site precondition for authorized requests. Apply the
same ordering consistently in the related duplicated path mentioned in the
review so the handler behavior remains aligned.
- Around line 2081-2085: The DPU ID collection in machine.go should de-duplicate
repeated AttachedDPUMachineID values before the workflow is invoked, since
multiple interfaces can point to the same DPU and create duplicate DpuMachine
entries. Update the dpuMachineIDs gathering logic to track seen IDs while
iterating over machineInterfaces, and only append each non-empty ID once; keep
the fix localized around the loop that builds dpuMachineIDs and the subsequent
workflow call that consumes it.
🪄 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: b3a8551e-5a52-406c-82f2-ceff0e13a9f0

📥 Commits

Reviewing files that changed from the base of the PR and between 3d7d78f and bd5f067.

⛔ Files ignored due to path filters (80)
  • 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/api_expected_rack.go is excluded by !rest-api/sdk/standard/api_*.go
  • rest-api/sdk/standard/api_machine.go is excluded by !rest-api/sdk/standard/api_*.go
  • rest-api/sdk/standard/model_action_config.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_allocation_constraint_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_allocation_constraint_update_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_allocation_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_batch_bring_up_rack_request.go is excluded by !rest-api/sdk/standard/model_*.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_batch_rack_firmware_update_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_batch_tray_firmware_update_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_batch_update_rack_power_state_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_batch_update_tray_power_state_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_bmc_credential.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_bmc_credential_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_bring_up_rack_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_cancel_task_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_create_rule_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_deletion_accepted_response.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_dpu_extension_service_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_dpu_extension_service_observability_logging.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_dpu_extension_service_observability_prometheus.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_dpu_interface_config.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_dpu_machine.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_dpu_machine_interface.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_dpu_machine_software_component.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_dpu_network_config.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_power_shelf_create_request.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_list.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_firmware_update_request.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_create_request.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_interface_network_security_group_config.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_ip_block_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_health_issue.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_machine_instance_type_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_machine_online_repair.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_machine_online_repair_acknowledgments.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_machine_online_repair_policy.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_managed_host_network_config.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_managed_host_quarantine_state.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_rule.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_nv_link_logical_partition_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_operating_system_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_operation_rule.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_placement_in_rack.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_resolved_network_security_group_rule.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_retry_policy.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_rule_definition.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_sequence_step.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_site_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_ssh_key_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_ssh_key_group_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_ssh_key_group_update_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_subnet_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_subnet_update_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_task_report_v1.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_task_report_v1_stage.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_task_report_v1_step.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_tenant_account_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_tenant_identity_basic_client_secret_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_tenant_identity_config_create_or_update_request_with_key_rotation.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_tenant_identity_config_create_or_update_request_without_key_rotation.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_tenant_identity_jwks.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_tenant_identity_signing_key.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_tenant_identity_token_delegation_create_or_update_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_update_power_state_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_update_rule_request.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_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_vpc_prefix_create_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_vpc_prefix_update_request.go is excluded by !rest-api/sdk/standard/model_*.go
📒 Files selected for processing (17)
  • rest-api/Makefile
  • rest-api/api/pkg/api/handler/machine.go
  • rest-api/api/pkg/api/handler/machine_test.go
  • rest-api/api/pkg/api/model/dpumachine.go
  • rest-api/api/pkg/api/model/dpumachine_test.go
  • rest-api/api/pkg/api/model/machine.go
  • rest-api/api/pkg/api/routes.go
  • rest-api/api/pkg/api/routes_test.go
  • rest-api/cli/tui/commands.go
  • rest-api/cli/tui/repl.go
  • rest-api/docs/index.html
  • rest-api/flow/internal/nicoapi/nicoproto/fmds.proto
  • rest-api/openapi/spec.yaml
  • rest-api/openapi/templates/go/model_simple.mustache
  • rest-api/sdk/simple/client.go
  • rest-api/sdk/simple/machine.go
  • rest-api/site-agent/pkg/components/managers/machine/subscriber.go
✅ Files skipped from review due to trivial changes (3)
  • rest-api/flow/internal/nicoapi/nicoproto/fmds.proto
  • rest-api/cli/tui/repl.go
  • rest-api/api/pkg/api/routes_test.go
🚧 Files skipped from review as they are similar to previous changes (7)
  • rest-api/api/pkg/api/routes.go
  • rest-api/site-agent/pkg/components/managers/machine/subscriber.go
  • rest-api/sdk/simple/machine.go
  • rest-api/cli/tui/commands.go
  • rest-api/sdk/simple/client.go
  • rest-api/api/pkg/api/handler/machine_test.go
  • rest-api/openapi/spec.yaml

Comment thread rest-api/api/pkg/api/handler/machine.go Outdated
Comment thread rest-api/api/pkg/api/handler/machine.go Outdated
The vendored Flow copy of fmds.proto and its generated code were stale: the
source crates/rpc/proto/fmds.proto now carries the full Apache license header,
while the committed Flow copy still had the short (duplicated) SPDX header.
Regenerated via `make -C flow gen-nicoapi-pb` so the check-protobuf-generated
CI gate passes.

Signed-off-by: Parham Armani <parmani@nvidia.com>

test(rest-api): assert Temporal mock expectations in GetDpuMachines test

Add AssertExpectations for the success, error, and timeout Temporal mocks so
the timeout path's TerminateWorkflow cleanup is verified instead of passing
silently. Addresses a CodeRabbit review comment.

Signed-off-by: Parham Armani <parmani@nvidia.com>
@parmani-nv parmani-nv force-pushed the feat/nested-machine-port branch from 5526a38 to 739f510 Compare June 29, 2026 19:52
@parmani-nv parmani-nv enabled auto-merge (squash) June 29, 2026 20:28
// LoopbackIP is the loopback IP address
LoopbackIP string `json:"loopbackIp"`
// QuarantineState is the quarantine state for the managed host
QuarantineState *APIManagedHostQuarantineState `json:"quarantineState,omitempty"`

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.

Let's remove omitempty

// Mode is the quarantine mode
Mode string `json:"mode"`
// Reason is the reason for quarantine
Reason *string `json:"reason,omitempty"`

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.

Let's remove omitempty

@parmani-nv parmani-nv merged commit 85159d3 into NVIDIA:main Jun 29, 2026
119 checks passed
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.

2 participants