From 4e9867e63ff8fbea8626a10ba59cdc53f6d83afb Mon Sep 17 00:00:00 2001 From: Kyle Felter Date: Thu, 23 Jul 2026 13:07:21 -0500 Subject: [PATCH 1/4] fix: Decode DPU network policy overrides (#3866) Fixes an HTTP 500 when retrieving attached DPU details for machines with network security policy overrides by preserving protobuf fields through the Temporal workflow. The REST response stays the same, and the API documentation now makes clear that only selected DPU network configuration fields are exposed. None. - [ ] **Add** - New feature or capability - [ ] **Change** - Changes in existing functionality - [x] **Fix** - Bug fixes - [ ] **Remove** - Removed features or deprecated functionality - [ ] **Internal** - Internal changes (refactoring, tests, docs, etc.) - [ ] **This PR contains breaking changes** - [x] Unit tests added/updated - [ ] Integration tests added/updated - [x] Manual testing performed - [ ] No testing required (docs, internal refactor, etc.) Signed-off-by: Kyle Felter --- rest-api/api/pkg/api/handler/machine.go | 12 ++- rest-api/api/pkg/api/handler/machine_test.go | 60 +++++++++++++-- rest-api/api/pkg/api/model/dpumachine.go | 6 +- rest-api/docs/index.html | 10 +-- rest-api/openapi/spec.yaml | 8 +- rest-api/sdk/standard/api_machine.go | 2 +- rest-api/sdk/standard/model_dpu_machine.go | 4 +- .../sdk/standard/model_dpu_network_config.go | 2 +- .../site-workflow/pkg/activity/machine.go | 4 +- .../site-workflow/pkg/workflow/machine.go | 6 +- .../pkg/workflow/machine_test.go | 26 +++++-- .../site-agent/workflows/v1/dpu_machine.pb.go | 73 ++++++++++++++++--- .../site-agent/workflows/v1/dpu_machine.proto | 5 ++ 13 files changed, 170 insertions(+), 48 deletions(-) diff --git a/rest-api/api/pkg/api/handler/machine.go b/rest-api/api/pkg/api/handler/machine.go index c94f1a4895..b100f5d0cc 100644 --- a/rest-api/api/pkg/api/handler/machine.go +++ b/rest-api/api/pkg/api/handler/machine.go @@ -19,6 +19,7 @@ import ( "go.opentelemetry.io/otel/attribute" temporalClient "go.temporal.io/sdk/client" + tsdkConverter "go.temporal.io/sdk/converter" tp "go.temporal.io/sdk/temporal" "github.com/google/uuid" @@ -2123,8 +2124,15 @@ func (gadmh GetAllDpuMachineHandler) Handle(c echo.Context) error { logger.Info().Str("Workflow ID", wid).Msg("executed synchronous GetDpuMachines workflow") // Block until the workflow has completed and returned success/error. - var controllerDpuMachines []*cwssaws.DpuMachine + var controllerDpuMachines cwssaws.DpuMachineList wferr := we.Get(wfCtx, &controllerDpuMachines) + if errors.Is(wferr, tsdkConverter.ErrUnableToDecode) { + var legacyDpuMachines []*cwssaws.DpuMachine + wferr = we.Get(wfCtx, &legacyDpuMachines) + if wferr == nil { + controllerDpuMachines.Machines = legacyDpuMachines + } + } if wferr != nil { var timeoutErr *tp.TimeoutError if errors.As(wferr, &timeoutErr) || wferr == context.DeadlineExceeded || wfCtx.Err() != nil { @@ -2136,7 +2144,7 @@ func (gadmh GetAllDpuMachineHandler) Handle(c echo.Context) error { return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to retrieve Machine DPU information from Site: %s", uwerr), nil) } - apiDpuMachines = model.NewAPIDpuMachines(controllerDpuMachines, model.APIDpuMachineProtoContext{ + apiDpuMachines = model.NewAPIDpuMachines(controllerDpuMachines.GetMachines(), model.APIDpuMachineProtoContext{ HostMachineID: mID, SiteID: site.ID, InfrastructureProviderID: site.InfrastructureProviderID, diff --git a/rest-api/api/pkg/api/handler/machine_test.go b/rest-api/api/pkg/api/handler/machine_test.go index 227904fcc0..5ddac20f8c 100644 --- a/rest-api/api/pkg/api/handler/machine_test.go +++ b/rest-api/api/pkg/api/handler/machine_test.go @@ -36,6 +36,7 @@ import ( "go.temporal.io/api/enums/v1" temporalClient "go.temporal.io/sdk/client" + tsdkConverter "go.temporal.io/sdk/converter" tmocks "go.temporal.io/sdk/mocks" tp "go.temporal.io/sdk/temporal" @@ -3227,6 +3228,21 @@ func TestMachineHandler_Delete(t *testing.T) { } } +func TestGetDpuMachinesLegacyTemporalPayload(t *testing.T) { + legacyDpuMachines := []*cwssaws.DpuMachine{{Machine: &cwssaws.Machine{Id: &cwssaws.MachineId{Id: uuid.NewString()}}}} + dataConverter := tsdkConverter.GetDefaultDataConverter() + payloads, err := dataConverter.ToPayloads(legacyDpuMachines) + require.NoError(t, err) + + var currentResult cwssaws.DpuMachineList + err = dataConverter.FromPayloads(payloads, ¤tResult) + require.ErrorIs(t, err, tsdkConverter.ErrUnableToDecode) + + var decodedLegacyResult []*cwssaws.DpuMachine + require.NoError(t, dataConverter.FromPayloads(payloads, &decodedLegacyResult)) + require.Equal(t, legacyDpuMachines, decodedLegacyResult) +} + func TestMachineHandler_GetDpuMachines(t *testing.T) { ctx := context.Background() dbSession := testMachineInitDB(t) @@ -3317,7 +3333,7 @@ func TestMachineHandler_GetDpuMachines(t *testing.T) { cfg := common.GetTestConfig() // Mock Temporal: success path returns two DPU machines for the workflow. - dpuMachineList := []*cwssaws.DpuMachine{ + dpuMachineList := &cwssaws.DpuMachineList{Machines: []*cwssaws.DpuMachine{ { Machine: &cwssaws.Machine{ Id: &cwssaws.MachineId{Id: dpu1.ID}, @@ -3356,27 +3372,44 @@ func TestMachineHandler_GetDpuMachines(t *testing.T) { DatacenterAsn: 65000, }, }, - } + }} wrun := &tmocks.WorkflowRun{} wrun.On("GetID").Return("test-workflow-id-dpu") wrun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - result := args.Get(1).(*[]*cwssaws.DpuMachine) - *result = dpuMachineList + result := args.Get(1).(*cwssaws.DpuMachineList) + result.Machines = dpuMachineList.Machines }).Return(nil) tsc := &tmocks.Client{} tsc.Mock.On("ExecuteWorkflow", mock.Anything, mock.AnythingOfType("internal.StartWorkflowOptions"), "GetDpuMachines", mock.Anything).Return(wrun, nil) + wrunLegacy := &tmocks.WorkflowRun{} + wrunLegacy.On("GetID").Return("test-workflow-id-dpu-legacy") + wrunLegacy.Mock.On("Get", mock.Anything, mock.MatchedBy(func(result any) bool { + _, ok := result.(*cwssaws.DpuMachineList) + return ok + })).Return(fmt.Errorf("payload item 0: %w: cannot unmarshal array into DpuMachineList", tsdkConverter.ErrUnableToDecode)).Once() + wrunLegacy.Mock.On("Get", mock.Anything, mock.MatchedBy(func(result any) bool { + _, ok := result.(*[]*cwssaws.DpuMachine) + return ok + })).Run(func(args mock.Arguments) { + result := args.Get(1).(*[]*cwssaws.DpuMachine) + *result = dpuMachineList.Machines + }).Return(nil).Once() + + tscLegacy := &tmocks.Client{} + tscLegacy.Mock.On("ExecuteWorkflow", mock.Anything, mock.AnythingOfType("internal.StartWorkflowOptions"), "GetDpuMachines", mock.Anything).Return(wrunLegacy, nil) + wrunErr := &tmocks.WorkflowRun{} wrunErr.On("GetID").Return("test-workflow-error-id") - wrunErr.Mock.On("Get", mock.Anything, mock.Anything).Return(fmt.Errorf("workflow failed")) + wrunErr.Mock.On("Get", mock.Anything, mock.Anything).Return(fmt.Errorf("workflow failed")).Once() tscErr := &tmocks.Client{} tscErr.Mock.On("ExecuteWorkflow", mock.Anything, mock.AnythingOfType("internal.StartWorkflowOptions"), "GetDpuMachines", mock.Anything).Return(wrunErr, nil) wrunTimeout := &tmocks.WorkflowRun{} wrunTimeout.On("GetID").Return("test-workflow-timeout-id") - wrunTimeout.Mock.On("Get", mock.Anything, mock.Anything).Return(tp.NewTimeoutError(enums.TIMEOUT_TYPE_UNSPECIFIED, nil, nil)) + wrunTimeout.Mock.On("Get", mock.Anything, mock.Anything).Return(tp.NewTimeoutError(enums.TIMEOUT_TYPE_UNSPECIFIED, nil, nil)).Once() tscTimeout := &tmocks.Client{} tscTimeout.Mock.On("ExecuteWorkflow", mock.Anything, mock.AnythingOfType("internal.StartWorkflowOptions"), "GetDpuMachines", mock.Anything).Return(wrunTimeout, nil) tscTimeout.Mock.On("TerminateWorkflow", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) @@ -3390,6 +3423,9 @@ func TestMachineHandler_GetDpuMachines(t *testing.T) { scpErr := sc.NewClientPool(tcfg) scpErr.IDClientMap[site.ID.String()] = tscErr + scpLegacy := sc.NewClientPool(tcfg) + scpLegacy.IDClientMap[site.ID.String()] = tscLegacy + scpTimeout := sc.NewClientPool(tcfg) scpTimeout.IDClientMap[site.ID.String()] = tscTimeout @@ -3481,6 +3517,16 @@ func TestMachineHandler_GetDpuMachines(t *testing.T) { expectedDpuCount: 2, verifyChildSpanner: true, }, + { + name: "provider admin: legacy workflow result remains supported during rollout", + reqOrgName: ipOrg1, + user: ipu, + mID: mWithDpu.ID, + scp: scpLegacy, + expectedStatus: http.StatusOK, + expectedDpuCount: 2, + verifyChildSpanner: true, + }, { name: "privileged tenant with account on provider can access", reqOrgName: tnOrgPriv, @@ -3555,6 +3601,8 @@ func TestMachineHandler_GetDpuMachines(t *testing.T) { // regressions in timeout cleanup are caught instead of passing silently. tsc.AssertExpectations(t) wrun.AssertExpectations(t) + tscLegacy.AssertExpectations(t) + wrunLegacy.AssertExpectations(t) tscErr.AssertExpectations(t) wrunErr.AssertExpectations(t) tscTimeout.AssertExpectations(t) diff --git a/rest-api/api/pkg/api/model/dpumachine.go b/rest-api/api/pkg/api/model/dpumachine.go index 662ea3e6a1..f968ed490c 100644 --- a/rest-api/api/pkg/api/model/dpumachine.go +++ b/rest-api/api/pkg/api/model/dpumachine.go @@ -12,7 +12,8 @@ import ( cwssaws "github.com/NVIDIA/infra-controller/rest-api/workflow-schema/schema/site-agent/workflows/v1" ) -// APIDpuNetworkConfig represents the complete network configuration for a DPU +// APIDpuNetworkConfig represents the network configuration fields exposed by the REST API for a DPU. +// Internal-only and sensitive Core fields are omitted; this is not the complete Core configuration. type APIDpuNetworkConfig struct { // Asn is the Autonomous System Number for BGP routing Asn int `json:"asn"` @@ -440,7 +441,8 @@ type APIDpuMachine struct { Labels map[string]string `json:"labels"` // State is the lifecycle state of the DPU as reported by NICo Core State string `json:"state"` - // DpuNetworkConfig is the complete network configuration sent to the DPU agent + // DpuNetworkConfig contains the network configuration fields exposed by the REST API for the DPU. + // Internal-only and sensitive Core fields are omitted; the REST response retains its public JSON shape. DpuNetworkConfig APIDpuNetworkConfig `json:"dpuNetworkConfig"` // LastRebooted is the last reboot timestamp reported by NICo Core LastRebooted *time.Time `json:"lastRebooted"` diff --git a/rest-api/docs/index.html b/rest-api/docs/index.html index e268a998f9..c0c594caf9 100644 --- a/rest-api/docs/index.html +++ b/rest-api/docs/index.html @@ -10016,7 +10016,7 @@

Typical API Call Flow for Tenant

" class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when user is not authorized to call an endpoint or retrieve/modify objects

Response samples

Content type
application/json
[
  • {
    }
]

Retrieve DPU Machines attached to a host Machine

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/machine/{machineId}/status-history

Response samples

Content type
application/json
[
  • {
    }
]

Retrieve DPU Machines attached to a host Machine

Typical API Call Flow for Tenant <li>The caller&#39;s org has an Infrastructure Provider entity that owns the Machine&#39;s Site, and the user has authorization role with <code>PROVIDER_ADMIN</code> suffix on that org.</li> <li>The caller&#39;s org has a Tenant entity with <code>TargetedInstanceCreation</code> enabled and an active Tenant Account on the Machine&#39;s Site&#39;s Infrastructure Provider, and the user has authorization role with <code>TENANT_ADMIN</code> suffix on that org.</li> </ul> -" class="sc-iJSMbW sc-cBEgGa fiNpIH ewCFMV">

Retrieve DPU Machines attached to the host Machine, including the full network configuration sent to the DPU agent.

+" class="sc-iJSMbW sc-cBEgGa fiNpIH ewCFMV">

Retrieve DPU Machines attached to the host Machine, including network configuration fields exposed by the REST API. Internal-only and sensitive fields from the Core configuration are omitted.

The response is built by scheduling a synchronous Temporal GetDpuMachines workflow against the Machine's Site for the DPU Machine IDs referenced by the host Machine's interfaces.

Authorization:

Access is restricted to users associated with the Machine's Site. Either of the following grants access:

@@ -10124,8 +10124,8 @@

Typical API Call Flow for Tenant

" class="sc-iJSMbW sc-cBEgGa fiNpIH bAoMjv">

Labels associated with the DPU

property name*
additional property
string
state
required
string

Lifecycle state of the DPU

-
required
object (DpuNetworkConfig)

Complete network configuration sent to the DPU agent

+
required
object (DpuNetworkConfig)

Network configuration fields exposed by the REST API

asn
required
integer <uint32>

Autonomous System Number for BGP routing

dhcpServers
Array of strings
Typical API Call Flow for Tenant