diff --git a/rest-api/api/pkg/api/handler/expectedmachine.go b/rest-api/api/pkg/api/handler/expectedmachine.go index b5678ae3d7..5fc345319a 100644 --- a/rest-api/api/pkg/api/handler/expectedmachine.go +++ b/rest-api/api/pkg/api/handler/expectedmachine.go @@ -1259,6 +1259,52 @@ func NewUpdateExpectedMachinesHandler(dbSession *cdb.Session, scp *sc.ClientPool } } +type expectedMachineUpdateFieldSet struct { + bmcMacAddress bool + defaultBmcUsername bool + defaultBmcPassword bool + chassisSerialNumber bool + fallbackDPUSerialNumbers bool + skuID bool + rackID bool + name bool + manufacturer bool + model bool + description bool + slotID bool + trayIdx bool + hostID bool + isDpfEnabled bool + labels bool + hostLifecycleProfile bool +} + +// expectedMachineUpdateFields records field presence across the complete update +// request without comparing values. Keep it in sync with +// `model.APIExpectedMachineUpdateRequest`: `ID` selects the row, and +// `bmcIpAddress` is the sole documented per-machine exception. +func expectedMachineUpdateFields(req model.APIExpectedMachineUpdateRequest) expectedMachineUpdateFieldSet { + return expectedMachineUpdateFieldSet{ + bmcMacAddress: req.BmcMacAddress != nil, + defaultBmcUsername: req.DefaultBmcUsername != nil, + defaultBmcPassword: req.DefaultBmcPassword != nil, + chassisSerialNumber: req.ChassisSerialNumber != nil, + fallbackDPUSerialNumbers: req.FallbackDPUSerialNumbers != nil, + skuID: req.SkuID != nil, + rackID: req.RackID != nil, + name: req.Name != nil, + manufacturer: req.Manufacturer != nil, + model: req.Model != nil, + description: req.Description != nil, + slotID: req.SlotID != nil, + trayIdx: req.TrayIdx != nil, + hostID: req.HostID != nil, + isDpfEnabled: req.IsDpfEnabled != nil, + labels: req.Labels != nil, + hostLifecycleProfile: req.HostLifecycleProfile.ToDBModelPtr() != nil, + } +} + // Handle godoc // @Summary Batch update ExpectedMachines // @Description Update multiple ExpectedMachines in a single request. All machines must belong to the same site. @@ -1307,6 +1353,9 @@ func (uemh UpdateExpectedMachinesHandler) Handle(c echo.Context) error { idMap := make(map[uuid.UUID]int) // Map Expected Machine ID to its index in the request array serialMap := make(map[string]int) requestedSkuIDs := make(map[string]bool) + // The public batch contract requires one update field set. Enforce it before + // `UpdateMultiple` can apply its shared column list to rows that omitted a field. + batchFields := expectedMachineUpdateFields(apiRequests[0]) for i, req := range apiRequests { strIndex := strconv.Itoa(i) // index/key as string for validation errors map itemErrors := validation.Errors{} @@ -1347,6 +1396,10 @@ func (uemh UpdateExpectedMachinesHandler) Handle(c echo.Context) error { requestedSkuIDs[*req.SkuID] = true } + if expectedMachineUpdateFields(req) != batchFields { + common.AddToValidationErrors(itemErrors, "fields", errors.New("must provide the same set of fields as batch item 0, except bmcIpAddress")) + } + if len(itemErrors) > 0 { validationErrors[strIndex] = itemErrors } diff --git a/rest-api/api/pkg/api/handler/expectedmachine_test.go b/rest-api/api/pkg/api/handler/expectedmachine_test.go index 554fce50b8..c3043647d8 100644 --- a/rest-api/api/pkg/api/handler/expectedmachine_test.go +++ b/rest-api/api/pkg/api/handler/expectedmachine_test.go @@ -10,6 +10,8 @@ import ( "fmt" "net/http" "net/http/httptest" + "reflect" + "strings" "testing" "github.com/NVIDIA/infra-controller/rest-api/api/internal/config" @@ -2714,17 +2716,181 @@ func TestUpdateExpectedMachineHandler_BmcCredentialsForwardedToWorkflow(t *testi } } -// TestUpdateExpectedMachinesHandler_Handle tests the batch update handler +func TestExpectedMachineUpdateFields(t *testing.T) { + t.Run("tracks every request field except ID and BMC IP address", func(t *testing.T) { + requestFieldCount := reflect.TypeOf(model.APIExpectedMachineUpdateRequest{}).NumField() + updateFieldCount := reflect.TypeOf(expectedMachineUpdateFieldSet{}).NumField() + assert.Equal(t, requestFieldCount-2, updateFieldCount) + }) + + fieldPresenceTests := []struct { + name string + setField func(*model.APIExpectedMachineUpdateRequest) + wantEqual bool + }{ + { + name: "ID identifies the target instead of an update field", + setField: func(req *model.APIExpectedMachineUpdateRequest) { req.ID = cutil.GetPtr(uuid.NewString()) }, + wantEqual: true, + }, + { + name: "BMC IP address may vary per machine", + setField: func(req *model.APIExpectedMachineUpdateRequest) { req.BmcIpAddress = cutil.GetPtr("192.0.2.10") }, + wantEqual: true, + }, + { + name: "empty host lifecycle profile is an omitted update", + setField: func(req *model.APIExpectedMachineUpdateRequest) { + req.HostLifecycleProfile = &model.APIHostLifecycleProfile{} + }, + wantEqual: true, + }, + { + name: "BMC MAC address", + setField: func(req *model.APIExpectedMachineUpdateRequest) { + req.BmcMacAddress = cutil.GetPtr("00:11:22:33:44:55") + }, + }, + { + name: "default BMC username", + setField: func(req *model.APIExpectedMachineUpdateRequest) { req.DefaultBmcUsername = cutil.GetPtr("admin") }, + }, + { + name: "default BMC password", + setField: func(req *model.APIExpectedMachineUpdateRequest) { req.DefaultBmcPassword = cutil.GetPtr("password") }, + }, + { + name: "chassis serial number", + setField: func(req *model.APIExpectedMachineUpdateRequest) { req.ChassisSerialNumber = cutil.GetPtr("SERIAL") }, + }, + { + name: "empty fallback DPU serial numbers", + setField: func(req *model.APIExpectedMachineUpdateRequest) { req.FallbackDPUSerialNumbers = []string{} }, + }, + { + name: "SKU ID", + setField: func(req *model.APIExpectedMachineUpdateRequest) { req.SkuID = cutil.GetPtr(uuid.NewString()) }, + }, + { + name: "rack ID", + setField: func(req *model.APIExpectedMachineUpdateRequest) { req.RackID = cutil.GetPtr("rack-1") }, + }, + { + name: "name", + setField: func(req *model.APIExpectedMachineUpdateRequest) { req.Name = cutil.GetPtr("machine") }, + }, + { + name: "manufacturer", + setField: func(req *model.APIExpectedMachineUpdateRequest) { req.Manufacturer = cutil.GetPtr("NVIDIA") }, + }, + { + name: "model", + setField: func(req *model.APIExpectedMachineUpdateRequest) { req.Model = cutil.GetPtr("test") }, + }, + { + name: "description", + setField: func(req *model.APIExpectedMachineUpdateRequest) { req.Description = cutil.GetPtr("test") }, + }, + { + name: "slot ID", + setField: func(req *model.APIExpectedMachineUpdateRequest) { req.SlotID = cutil.GetPtr(int32(1)) }, + }, + { + name: "tray index", + setField: func(req *model.APIExpectedMachineUpdateRequest) { req.TrayIdx = cutil.GetPtr(int32(1)) }, + }, + { + name: "host ID", + setField: func(req *model.APIExpectedMachineUpdateRequest) { req.HostID = cutil.GetPtr(int32(1)) }, + }, + { + name: "DPF enablement", + setField: func(req *model.APIExpectedMachineUpdateRequest) { req.IsDpfEnabled = cutil.GetPtr(true) }, + }, + { + name: "empty labels", + setField: func(req *model.APIExpectedMachineUpdateRequest) { req.Labels = map[string]string{} }, + }, + { + name: "host lifecycle profile setting", + setField: func(req *model.APIExpectedMachineUpdateRequest) { + req.HostLifecycleProfile = &model.APIHostLifecycleProfile{DisableLockdown: cutil.GetPtr(true)} + }, + }, + } + + emptyFields := expectedMachineUpdateFields(model.APIExpectedMachineUpdateRequest{}) + for _, tt := range fieldPresenceTests { + t.Run(tt.name, func(t *testing.T) { + request := model.APIExpectedMachineUpdateRequest{} + tt.setField(&request) + assert.Equal(t, tt.wantEqual, expectedMachineUpdateFields(request) == emptyFields) + }) + } + + t.Run("values may differ when field presence matches", func(t *testing.T) { + first := model.APIExpectedMachineUpdateRequest{ + Name: cutil.GetPtr("first"), + FallbackDPUSerialNumbers: []string{"DPU-1"}, + Labels: map[string]string{"machine": "first"}, + HostLifecycleProfile: &model.APIHostLifecycleProfile{DisableLockdown: cutil.GetPtr(true)}, + } + second := model.APIExpectedMachineUpdateRequest{ + Name: cutil.GetPtr("second"), + FallbackDPUSerialNumbers: []string{}, + Labels: map[string]string{}, + HostLifecycleProfile: &model.APIHostLifecycleProfile{DisableLockdown: cutil.GetPtr(false)}, + } + assert.Equal(t, expectedMachineUpdateFields(first), expectedMachineUpdateFields(second)) + }) +} + func TestUpdateExpectedMachinesHandler_Handle(t *testing.T) { // Setup e := echo.New() + cfg := common.GetTestConfig() + + fieldSetTests := []struct { + name string + requestBody string + }{ + { + name: "rejects differing ordinary field sets before database access", + requestBody: `[ + {"id":"00000000-0000-0000-0000-000000000001","fallbackDPUSerialNumbers":[]}, + {"id":"00000000-0000-0000-0000-000000000002","labels":{}} + ]`, + }, + { + name: "rejects differing host lifecycle profile field sets before database access", + requestBody: `[ + {"id":"00000000-0000-0000-0000-000000000001","name":"first"}, + {"id":"00000000-0000-0000-0000-000000000002","name":"second","hostLifecycleProfile":{"disableLockdown":true}} + ]`, + }, + } + for _, tt := range fieldSetTests { + t.Run(tt.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodPatch, "/v2/org/test-org/nico/expected-machine/batch", strings.NewReader(tt.requestBody)) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + c.Set("user", &cdbm.User{}) + c.SetParamNames("orgName") + c.SetParamValues("test-org") + + handler := NewUpdateExpectedMachinesHandler(nil, nil, cfg) + err := handler.Handle(c) + require.NoError(t, err) + assert.Equal(t, http.StatusBadRequest, rec.Code) + assert.Contains(t, rec.Body.String(), "same set of fields") + }) + } // Initialize test database dbSession := testExpectedMachineInitDB(t) defer dbSession.Close() - cfg := common.GetTestConfig() - // Prepare client pool for workflow calls tcfg, _ := cfg.GetTemporalConfig() scp := sc.NewClientPool(tcfg) @@ -2966,13 +3132,17 @@ func TestUpdateExpectedMachinesHandler_Handle(t *testing.T) { }, { name: "BMC MAC address change rejects the whole batch", + // Keep both requests on the same field set so this case reaches the + // BMC MAC immutability check. requestBody: []model.APIExpectedMachineUpdateRequest{ { - ID: cutil.GetPtr(testEM1.ID.String()), - BmcMacAddress: cutil.GetPtr("AA:BB:CC:DD:EE:FF"), + ID: cutil.GetPtr(testEM1.ID.String()), + BmcMacAddress: cutil.GetPtr("AA:BB:CC:DD:EE:FF"), + ChassisSerialNumber: cutil.GetPtr("REJECTED-BATCH-COMPANION"), }, { ID: cutil.GetPtr(testEM2.ID.String()), + BmcMacAddress: cutil.GetPtr(testEM2.BmcMacAddress), ChassisSerialNumber: cutil.GetPtr("REJECTED-BATCH-CHANGE"), }, },