fix(rest-api): Prevent user from updating ExpectedRack rackId after creation

rest-api/api/pkg/api/handler/expectedrack.go

@@ -582,27 +582,27 @@ func (uerh UpdateExpectedRackHandler) Handle(c echo.Context) error {
 		return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Current org is not associated with the Site of the Expected Rack", nil)
 	}
 
-	// If RackID is changing, ensure the new value is not already taken in this site
+	// RackID is immutable: Core and Flow identify expected racks by rackId, so
+	// PATCH may reassert the existing identity but cannot replace it. A rename
+	// would first mutate the Cloud record and only then fail in Core's lookup
+	// by the new rackId, so the mismatch is rejected here before any database
+	// write or workflow trigger. This mirrors the ExpectedMachine BMC MAC
+	// identity boundary.
 	if apiRequest.RackID != nil && *apiRequest.RackID != expectedRack.RackID {
-		_, count, err := erDAO.GetAll(ctx, nil, cdbm.ExpectedRackFilterInput{
-			SiteIDs: []uuid.UUID{expectedRack.SiteID},
-			RackIDs: []string{*apiRequest.RackID},
-		}, paginator.PageInput{Limit: cutil.GetPtr(1)}, nil)
-		if err != nil {
-			logger.Error().Err(err).Msg("error checking for duplicate Expected Rack")
-			return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to validate Expected Rack uniqueness due to DB error", nil)
-		}
-		if count > 0 {
-			return cutil.NewAPIErrorResponse(c, http.StatusConflict, "Expected Rack with specified RackID already exists for Site", validation.Errors{
-				"rackId": errors.New(*apiRequest.RackID),
-			})
-		}
+		logger.Warn().
+			Str("requestRackID", *apiRequest.RackID).
+			Str("expectedRackID", expectedRack.RackID).
+			Msg("RackID cannot be changed after creation")
+		return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Failed to validate ExpectedRack update request data", validation.Errors{
+			"rackId": errors.New("RackID cannot be changed after creation"),
+		})
 	}
 
-	// Build update input from request, mapping flat API fields to DAO fields
+	// Build update input from request, mapping flat API fields to DAO fields.
+	// RackID is intentionally not passed through: it is immutable, so the DAO
+	// update path is structurally incapable of renaming an Expected Rack.
 	updateInput := cdbm.ExpectedRackUpdateInput{
 		ExpectedRackID: expectedRack.ID,
-		RackID:         apiRequest.RackID,
 		RackProfileID:  apiRequest.RackProfileID,
 		Name:           apiRequest.Name,
 		Description:    apiRequest.Description,

rest-api/api/pkg/api/handler/expectedrack_test.go

@@ -844,7 +844,7 @@ func TestUpdateExpectedRackHandler_Handle(t *testing.T) {
 	assert.Nil(t, err)
 	assert.NotNil(t, testER2)
 
-	// A third ExpectedRack to anchor the duplicate-rack-id update test
+	// A third ExpectedRack to anchor the rack_id rename-rejection test
 	testER3, err := erDAO.Create(ctx, nil, cdbm.ExpectedRackCreateInput{
 		ExpectedRackID: uuid.New(),
 		RackID:         "update-rack-003",
@@ -894,11 +894,12 @@ func TestUpdateExpectedRackHandler_Handle(t *testing.T) {
 	}
 
 	tests := []struct {
-		name           string
-		id             string
-		requestBody    model.APIExpectedRackUpdateRequest
-		setupContext   func(c echo.Context)
-		expectedStatus int
+		name                 string
+		id                   string
+		requestBody          model.APIExpectedRackUpdateRequest
+		setupContext         func(c echo.Context)
+		expectedStatus       int
+		checkResponseContent func(t *testing.T, body []byte)
 	}{
 		{
 			name: "successful update of rack_profile_id",
@@ -932,7 +933,7 @@ func TestUpdateExpectedRackHandler_Handle(t *testing.T) {
 			expectedStatus: http.StatusOK,
 		},
 		{
-			name: "successful update of rack_id (operator-supplied identifier)",
+			name: "rack_id cannot be changed (immutable)",
 			id:   testER3.ID.String(),
 			requestBody: model.APIExpectedRackUpdateRequest{
 				RackID: cutil.GetPtr("update-rack-003-renamed"),
@@ -942,21 +943,49 @@ func TestUpdateExpectedRackHandler_Handle(t *testing.T) {
 				c.SetParamNames("orgName", "id")
 				c.SetParamValues(org, testER3.ID.String())
 			},
-			expectedStatus: http.StatusOK,
+			expectedStatus: http.StatusBadRequest,
+			checkResponseContent: func(t *testing.T, body []byte) {
+				assert.Contains(t, string(body), "RackID cannot be changed after creation")
+			},
 		},
 		{
-			name: "duplicate (siteId, rackId) on update should return 409",
+			name: "rack_id cannot be changed even to an existing value",
 			id:   testER3.ID.String(),
 			requestBody: model.APIExpectedRackUpdateRequest{
-				// testER's RackID is already taken in this site
+				// testER's RackID is already taken in this site, but the rename
+				// is rejected as immutable before any duplicate check.
 				RackID: cutil.GetPtr("update-rack-001"),
 			},
 			setupContext: func(c echo.Context) {
 				c.Set("user", createMockUser(org))
 				c.SetParamNames("orgName", "id")
 				c.SetParamValues(org, testER3.ID.String())
 			},
-			expectedStatus: http.StatusConflict,
+			expectedStatus: http.StatusBadRequest,
+			checkResponseContent: func(t *testing.T, body []byte) {
+				assert.Contains(t, string(body), "RackID cannot be changed after creation")
+			},
+		},
+		{
+			name: "identical rack_id remains compatible",
+			id:   testER2.ID.String(),
+			requestBody: model.APIExpectedRackUpdateRequest{
+				RackID:        cutil.GetPtr("update-rack-002"),
+				RackProfileID: cutil.GetPtr("profile-update-identical-rack-id"),
+			},
+			setupContext: func(c echo.Context) {
+				c.Set("user", createMockUser(org))
+				c.SetParamNames("orgName", "id")
+				c.SetParamValues(org, testER2.ID.String())
+			},
+			expectedStatus: http.StatusOK,
+			checkResponseContent: func(t *testing.T, body []byte) {
+				var response model.APIExpectedRack
+				err := json.Unmarshal(body, &response)
+				assert.Nil(t, err)
+				assert.Equal(t, "update-rack-002", response.RackID,
+					"reasserting the identity must preserve the stored rackId")
+			},
 		},
 		{
 			name: "body ID mismatch with URL should return 400",
@@ -1012,6 +1041,22 @@ func TestUpdateExpectedRackHandler_Handle(t *testing.T) {
 			},
 			expectedStatus: http.StatusForbidden,
 		},
+		{
+			name: "site access is checked before rack_id identity",
+			id:   unmanagedER.ID.String(),
+			requestBody: model.APIExpectedRackUpdateRequest{
+				RackID: cutil.GetPtr("update-rack-unmanaged-renamed"),
+			},
+			setupContext: func(c echo.Context) {
+				c.Set("user", createMockUser(org))
+				c.SetParamNames("orgName", "id")
+				c.SetParamValues(org, unmanagedER.ID.String())
+			},
+			expectedStatus: http.StatusForbidden,
+			checkResponseContent: func(t *testing.T, body []byte) {
+				assert.NotContains(t, string(body), "RackID cannot be changed after creation")
+			},
+		},
 		{
 			name: "rack not found",
 			id:   "12345678-1234-1234-1234-123456789099",
@@ -1050,10 +1095,129 @@ func TestUpdateExpectedRackHandler_Handle(t *testing.T) {
 			if tt.expectedStatus != rec.Code {
 				t.Errorf("Response: %v", rec.Body.String())
 			}
+			if tt.checkResponseContent != nil {
+				tt.checkResponseContent(t, rec.Body.Bytes())
+			}
 		})
 	}
 }
 
+// TestUpdateExpectedRackHandler_RackIDImmutable verifies that changing an
+// ExpectedRack's rackId is rejected before any database mutation or workflow
+// trigger, so Cloud, Core, and Flow can never hold different rack IDs for the
+// same ExpectedRack. Omitted or identical rackId values remain compatible.
+func TestUpdateExpectedRackHandler_RackIDImmutable(t *testing.T) {
+	e := echo.New()
+	dbSession := testExpectedRackInitDB(t)
+	defer dbSession.Close()
+
+	ctx := context.Background()
+	cfg := common.GetTestConfig()
+
+	tcfg, _ := cfg.GetTemporalConfig()
+	scp := sc.NewClientPool(tcfg)
+
+	org := "test-org"
+	_, site, _ := testExpectedRackSetupTestData(t, dbSession, org)
+
+	dbUser := &cdbm.User{
+		ID:          uuid.New(),
+		StarfleetID: cutil.GetPtr("test-user"),
+	}
+	_, err := dbSession.DB.NewInsert().Model(dbUser).Exec(ctx)
+	assert.Nil(t, err)
+
+	erDAO := cdbm.NewExpectedRackDAO(dbSession)
+	rack, err := erDAO.Create(ctx, nil, cdbm.ExpectedRackCreateInput{
+		ExpectedRackID: uuid.New(),
+		RackID:         "immutable-rack-001",
+		SiteID:         site.ID,
+		RackProfileID:  "profile-original",
+		CreatedBy:      dbUser.ID,
+	})
+	assert.Nil(t, err)
+	assert.NotNil(t, rack)
+
+	mockTemporalClient := &tmocks.Client{}
+	mockWorkflowRun := &tmocks.WorkflowRun{}
+	mockWorkflowRun.On("GetID").Return("test-workflow-id")
+	mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Return(nil)
+	mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "UpdateExpectedRack", mock.Anything).Return(mockWorkflowRun, nil)
+	scp.IDClientMap[site.ID.String()] = mockTemporalClient
+
+	handler := NewUpdateExpectedRackHandler(dbSession, scp, cfg)
+
+	createMockUser := func() *cdbm.User {
+		return &cdbm.User{
+			ID:          dbUser.ID,
+			StarfleetID: cutil.GetPtr("test-user"),
+			OrgData: cdbm.OrgData{
+				org: cdbm.Org{
+					ID:          123,
+					Name:        org,
+					DisplayName: org,
+					OrgType:     "ENTERPRISE",
+					Roles:       []string{"FORGE_PROVIDER_ADMIN"},
+				},
+			},
+		}
+	}
+
+	patch := func(body model.APIExpectedRackUpdateRequest) *httptest.ResponseRecorder {
+		reqBody, _ := json.Marshal(body)
+		req := httptest.NewRequest(http.MethodPatch, "/v2/org/"+org+"/expected-rack/"+rack.ID.String(), bytes.NewReader(reqBody))
+		req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON)
+		req = req.WithContext(context.Background())
+
+		rec := httptest.NewRecorder()
+		c := e.NewContext(req, rec)
+		c.Set("user", createMockUser())
+		c.SetParamNames("orgName", "id")
+		c.SetParamValues(org, rack.ID.String())
+
+		err := handler.Handle(c)
+		assert.Nil(t, err)
+		return rec
+	}
+
+	getRackID := func() string {
+		current, err := erDAO.Get(ctx, nil, rack.ID, nil, false)
+		assert.Nil(t, err)
+		return current.RackID
+	}
+
+	t.Run("changing rack_id is rejected without mutation or workflow", func(t *testing.T) {
+		rec := patch(model.APIExpectedRackUpdateRequest{
+			RackID:        cutil.GetPtr("immutable-rack-renamed"),
+			RackProfileID: cutil.GetPtr("profile-renamed"),
+		})
+
+		assert.Equal(t, http.StatusBadRequest, rec.Code)
+		assert.Contains(t, rec.Body.String(), "RackID cannot be changed after creation")
+		assert.Equal(t, "immutable-rack-001", getRackID())
+		mockTemporalClient.AssertNotCalled(t, "ExecuteWorkflow", mock.Anything, mock.Anything, "UpdateExpectedRack", mock.Anything)
+	})
+
+	t.Run("identical rack_id remains compatible", func(t *testing.T) {
+		rec := patch(model.APIExpectedRackUpdateRequest{
+			RackID:        cutil.GetPtr("immutable-rack-001"),
+			RackProfileID: cutil.GetPtr("profile-identical"),
+		})
+
+		assert.Equal(t, http.StatusOK, rec.Code)
+		assert.Equal(t, "immutable-rack-001", getRackID())
+	})
+
+	t.Run("omitted rack_id remains compatible", func(t *testing.T) {
+		rec := patch(model.APIExpectedRackUpdateRequest{
+			RackProfileID: cutil.GetPtr("profile-omitted"),
+		})
+
+		assert.Equal(t, http.StatusOK, rec.Code)
+		assert.Equal(t, "immutable-rack-001", getRackID())
+	})
+}
+
 func TestDeleteExpectedRackHandler_Handle(t *testing.T) {
 	e := echo.New()
 	dbSession := testExpectedRackInitDB(t)

rest-api/api/pkg/api/model/expectedrack.go

@@ -68,7 +68,10 @@ func (ercr *APIExpectedRackCreateRequest) Validate() error {
 type APIExpectedRackUpdateRequest struct {
 	// ID is required for batch updates (must be empty or match path value for single update).
 	ID *string `json:"id"`
-	// RackID is the optional new operator-supplied rack identifier
+	// RackID is the operator-supplied rack identifier. It is immutable on
+	// update: it may be omitted or set to the existing value, but a changed
+	// value is rejected by the handler before any database mutation because
+	// Core and Flow use rackId as the identity key.
 	RackID *string `json:"rackId"`
 	// RackProfileID is the optional new rack profile ID
 	RackProfileID *string `json:"rackProfileId"`

rest-api/api/pkg/api/model/expectedrack_test.go

@@ -189,7 +189,7 @@ func TestAPIExpectedRackUpdateRequest_Validate(t *testing.T) {
 			expectErr: false,
 		},
 		{
-			desc: "ok when RackID rename is provided",
+			desc: "ok when RackID is provided (structural validation; immutability is enforced by the handler)",
 			obj: APIExpectedRackUpdateRequest{
 				RackID: &validRackID,
 			},

rest-api/openapi/spec.yaml

@@ -4765,6 +4765,8 @@ paths:
         Infrastructure Provider must own the Expected Rack.
 
         Alternatively, Tenant Admins with `TargetedInstanceCreation` capability can also update Expected Racks if they have an account with the Site's Infrastructure Provider.
+
+        `rackId` is immutable: an update that changes it is rejected with `400` before any database mutation.
       requestBody:
         content:
           application/json:
@@ -24364,6 +24366,8 @@ components:
 
         For single updates (PATCH /expected-rack/{id}), the `id` field is optional in the body and will be ignored if provided (the `id` from the URL path is used).
 
+        The `rackId` field is immutable on update — omit it or provide the existing value. Renaming an Expected Rack is not supported.
+
         Chassis identity and physical location information are conveyed via well-known label keys in `labels`:
         - `chassis.manufacturer`, `chassis.serial-number`, `chassis.model`
         - `location.region`, `location.datacenter`, `location.room`, `location.position`
@@ -24390,7 +24394,9 @@ components:
             - string
             - 'null'
           minLength: 1
-          description: Optional new operator-supplied rack identifier. If provided, must be non-empty and unique within the Site.
+          deprecated: true
+          description: |-
+            Operator-supplied rack identifier. Immutable on update: omit this field, or provide the existing value as a compatibility no-op. A changed value is rejected because Core and Flow use rackId as the identity key for expected racks.
         rackProfileId:
           type:
             - string