resourcemanager: tighten controller config API checks

Signed-off-by: okjiang <819421878@qq.com>

Author: okJiang

pkg/mcs/resourcemanager/metadataapi/config_service.go

@@ -253,7 +253,11 @@ func (s *ConfigService) SetControllerConfig(c *gin.Context) {
 			c.String(http.StatusForbidden, err.Error())
 			return
 		}
-		c.String(http.StatusBadRequest, err.Error())
+		if rmserver.IsControllerConfigValidationError(err) {
+			c.String(http.StatusBadRequest, err.Error())
+			return
+		}
+		c.String(http.StatusInternalServerError, err.Error())
 		return
 	}
 	c.String(http.StatusOK, "Success!")

pkg/mcs/resourcemanager/metadataapi/config_service_test.go

@@ -98,17 +98,36 @@ func TestConfigServiceControllerAllOrNothing(t *testing.T) {
 	handler := newTestHTTPHandler(store)
 
 	resp := doJSONRequest(re, handler, http.MethodPost, "/resource-manager/api/v1/config/controller", map[string]any{
-		"enable-controller-trace-log": true,
-		"unknown":                     1,
+		"write-base-cost": 2.0,
+		"unknown":         1,
 	})
 	re.Equal(http.StatusBadRequest, resp.Code)
 	re.Empty(store.updatedControllerConfigItems)
+	re.Zero(store.updateControllerConfigItemsCalls)
+	re.Zero(store.updateControllerConfigItemCalls)
 
 	resp = doJSONRequest(re, handler, http.MethodPost, "/resource-manager/api/v1/config/controller", map[string]any{
-		"enable-controller-trace-log": true,
+		"write-base-cost": 2.0,
 	})
 	re.Equal(http.StatusOK, resp.Code)
 	re.Len(store.updatedControllerConfigItems, 1)
+	re.Equal([]string{"request-unit.write-base-cost"}, store.updatedControllerConfigItems)
+	re.Equal(1, store.updateControllerConfigItemsCalls)
+	re.Zero(store.updateControllerConfigItemCalls)
+}
+
+func TestConfigServiceControllerStoreFailuresReturn500(t *testing.T) {
+	t.Parallel()
+
+	re := require.New(t)
+	store := newTestStore()
+	store.updateControllerConfigItemsErr = errors.New("save controller config failed")
+	handler := newTestHTTPHandler(store)
+
+	resp := doJSONRequest(re, handler, http.MethodPost, "/resource-manager/api/v1/config/controller", map[string]any{
+		"write-base-cost": 2.0,
+	})
+	re.Equal(http.StatusInternalServerError, resp.Code)
 }
 
 func TestConfigServiceKeyspaceServiceLimitAndErrors(t *testing.T) {
@@ -178,9 +197,12 @@ func TestConfigServiceMetadataWriteDisabledReturns403(t *testing.T) {
 	re.Equal(http.StatusForbidden, resp.Code)
 
 	resp = doJSONRequest(re, handler, http.MethodPost, "/resource-manager/api/v1/config/controller", map[string]any{
-		"enable-controller-trace-log": true,
+		"write-base-cost": 1.0,
 	})
 	re.Equal(http.StatusForbidden, resp.Code)
+
+	resp = doJSONRequest(re, handler, http.MethodPost, "/resource-manager/api/v1/config/controller", map[string]any{})
+	re.Equal(http.StatusOK, resp.Code)
 }
 
 func newTestHTTPHandler(configStore ConfigStore) http.Handler {
@@ -209,13 +231,16 @@ func (*tokenOnlyManagerProvider) AddStartCallback(...func()) {}
 func (*tokenOnlyManagerProvider) AddServiceReadyCallback(...func(context.Context) error) {}
 
 type testStore struct {
-	keyspaceIDs                  map[string]uint32
-	validKeyspaceIDs             map[uint32]struct{}
-	groups                       map[string]*rmserver.ResourceGroup
-	serviceLimits                map[uint32]float64
-	addErr                       error
-	setServiceLimitErr           error
-	updatedControllerConfigItems []string
+	keyspaceIDs                      map[string]uint32
+	validKeyspaceIDs                 map[uint32]struct{}
+	groups                           map[string]*rmserver.ResourceGroup
+	serviceLimits                    map[uint32]float64
+	addErr                           error
+	setServiceLimitErr               error
+	updateControllerConfigItemsErr   error
+	updatedControllerConfigItems     []string
+	updateControllerConfigItemsCalls int
+	updateControllerConfigItemCalls  int
 }
 
 func newTestStore() *testStore {
@@ -288,13 +313,18 @@ func (*testStore) GetControllerConfig() *rmserver.ControllerConfig {
 }
 
 func (s *testStore) UpdateControllerConfigItems(items map[string]any) error {
+	s.updateControllerConfigItemsCalls++
+	if s.updateControllerConfigItemsErr != nil {
+		return s.updateControllerConfigItemsErr
+	}
 	for key := range items {
 		s.updatedControllerConfigItems = append(s.updatedControllerConfigItems, key)
 	}
 	return nil
 }
 
 func (s *testStore) UpdateControllerConfigItem(key string, _ any) error {
+	s.updateControllerConfigItemCalls++
 	s.updatedControllerConfigItems = append(s.updatedControllerConfigItems, key)
 	return nil
 }

pkg/mcs/resourcemanager/server/apis/v1/api.go

@@ -249,6 +249,8 @@ func (s *Service) getControllerConfig(c *gin.Context) {
 //	@Param		config	body	object	true	"json params, rmserver.ControllerConfig"
 //	@Success	200		{string}	string	"Success!"
 //	@Failure	400 	{string}	error
+//	@Failure	403 	{string}	error
+//	@Failure	500 	{string}	error
 //	@Router		/config/controller [post]
 func (s *Service) setControllerConfig(c *gin.Context) {
 	s.configService.SetControllerConfig(c)

pkg/mcs/resourcemanager/server/controller_config_errors.go

@@ -0,0 +1,49 @@
+// Copyright 2026 TiKV Project Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package server
+
+import (
+	stderrors "errors"
+)
+
+var errControllerConfigValidation = stderrors.New("controller config validation failed")
+
+func wrapControllerConfigValidationError(err error) error {
+	if err == nil {
+		return nil
+	}
+	return &controllerConfigValidationError{err: err}
+}
+
+// IsControllerConfigValidationError reports whether err is a controller-config validation error.
+func IsControllerConfigValidationError(err error) bool {
+	return stderrors.Is(err, errControllerConfigValidation)
+}
+
+type controllerConfigValidationError struct {
+	err error
+}
+
+func (e *controllerConfigValidationError) Error() string {
+	return e.err.Error()
+}
+
+func (e *controllerConfigValidationError) Unwrap() error {
+	return e.err
+}
+
+func (*controllerConfigValidationError) Is(target error) bool {
+	return target == errControllerConfigValidation
+}

pkg/mcs/resourcemanager/server/manager.go

@@ -472,6 +472,9 @@ func (m *Manager) UpdateControllerConfigItem(key string, value any) error {
 
 // UpdateControllerConfigItems updates controller config items atomically.
 func (m *Manager) UpdateControllerConfigItems(items map[string]any) error {
+	if len(items) == 0 {
+		return nil
+	}
 	if !m.writeRole.AllowsMetadataWrite() {
 		return errMetadataWriteDisabled
 	}
@@ -485,7 +488,7 @@ func (m *Manager) UpdateControllerConfigItems(items map[string]any) error {
 		updated, err := applyControllerConfigItem(controllerConfig, key, value)
 		if err != nil {
 			m.Unlock()
-			return err
+			return wrapControllerConfigValidationError(err)
 		}
 		if updated {
 			updatedItems = append(updatedItems, struct {

pkg/mcs/resourcemanager/server/manager_test.go

@@ -620,6 +620,19 @@ func TestUpdateControllerConfigItemsAtomic(t *testing.T) {
 	re.Equal(before.EnableControllerTraceLog, after.EnableControllerTraceLog)
 }
 
+func TestUpdateControllerConfigValidationError(t *testing.T) {
+	re := require.New(t)
+	m := prepareManager()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	re.NoError(m.Init(ctx))
+
+	err := m.UpdateControllerConfigItem("ltb-max-wait-duration", "not-a-duration")
+	re.Error(err)
+	re.True(IsControllerConfigValidationError(err))
+}
+
 func TestKeyspaceNameLookup(t *testing.T) {
 	re := require.New(t)
 	m := prepareManager()

tests/integrations/mcs/resourcemanager/api_test.go

@@ -340,15 +340,24 @@ func (suite *resourceManagerAPITestSuite) TestControllerConfigAPIAllOrNothing()
 	re := suite.Require()
 
 	before := suite.mustGetControllerConfig(re)
-	resp, statusCode := tryToSetControllerConfig(re, suite.cluster.GetLeaderServer().GetAddr(), map[string]any{
-		"enable-controller-trace-log": "true",
-		"ltb-max-wait-duration":       "not-a-duration",
-	})
-	re.Equal(http.StatusBadRequest, statusCode)
-	re.Contains(resp, "time:")
+	payload := map[string]any{
+		"read-base-cost":           2.0,
+		"read-per-batch-base-cost": 3.0,
+		"read-cost-per-byte":       4.0,
+		"write-base-cost":          5.0,
+		"write-cost-per-byte":      6.0,
+		"ltb-max-wait-duration":    "not-a-duration",
+	}
+	for i := 0; i < 8; i++ {
+		// Old code updated fields one by one after decoding into a map, so repeating the
+		// same mixed payload makes a partial write overwhelmingly likely if batching is gone.
+		resp, statusCode := tryToSetControllerConfig(re, suite.cluster.GetLeaderServer().GetAddr(), payload)
+		re.Equal(http.StatusBadRequest, statusCode)
+		re.Contains(resp, "time:")
 
-	after := suite.mustGetControllerConfig(re)
-	re.Equal(before, after)
+		after := suite.mustGetControllerConfig(re)
+		re.Equal(before, after)
+	}
 }
 
 func (suite *resourceManagerAPITestSuite) mustGetControllerConfig(re *require.Assertions) *server.ControllerConfig {