(kvrocks-controller) branch unstable updated: Validate the preferred node id format in failover API (#323)

[hulk]

This is an automated email from the ASF dual-hosted git repository.

hulk pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks-controller.git


The following commit(s) were added to refs/heads/unstable by this push:
     new 4899f99  Validate the preferred node id format in failover API (#323)
4899f99 is described below

commit 4899f99d613d043c5a8b010956ea78463dce7beb
Author: hulk <hulk.webs...@gmail.com>
AuthorDate: Mon Jul 7 23:55:20 2025 +0800

    Validate the preferred node id format in failover API (#323)
    
    Currently, it will still promote a new master even through the preferred
    node is in invalid format, and it might cause confusing since the new
    master is not what users expected.
---
 server/api/shard.go      | 16 ++++++----------
 server/api/shard_test.go | 14 ++++++++++++++
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/server/api/shard.go b/server/api/shard.go
index 6af2fdf..2706146 100644
--- a/server/api/shard.go
+++ b/server/api/shard.go
@@ -20,9 +20,8 @@
 package api
 
 import (
-       "encoding/json"
        "errors"
-       "io"
+       "fmt"
        "strconv"
 
        "github.com/gin-gonic/gin"
@@ -123,17 +122,14 @@ func (handler *ShardHandler) Failover(c *gin.Context) {
                PreferredNodeID string `json:"preferred_node_id"`
        }
        if c.Request.Body != nil {
-               body, err := io.ReadAll(c.Request.Body)
-               if err != nil {
+               if err := c.ShouldBindJSON(&req); err != nil {
                        helper.ResponseBadRequest(c, err)
                        return
                }
-               if len(body) > 0 {
-                       if err := json.Unmarshal(body, &req); err != nil {
-                               helper.ResponseBadRequest(c, err)
-                               return
-                       }
-               }
+       }
+       if len(req.PreferredNodeID) > 0 && len(req.PreferredNodeID) != 
store.NodeIDLen {
+               helper.ResponseBadRequest(c, fmt.Errorf("invalid node id: %s", 
req.PreferredNodeID))
+               return
        }
        // We have checked this if statement in middleware.RequiredClusterShard
        shardIndex, _ := strconv.Atoi(c.Param("shard"))
diff --git a/server/api/shard_test.go b/server/api/shard_test.go
index 967400f..39d9f91 100644
--- a/server/api/shard_test.go
+++ b/server/api/shard_test.go
@@ -220,4 +220,18 @@ func TestClusterFailover(t *testing.T) {
                require.NoError(t, err)
                require.EqualValues(t, "master", clusterNodeInfo1.Role)
        })
+
+       t.Run("failover with invalid node id", func(t *testing.T) {
+               recorder := httptest.NewRecorder()
+               ctx := GetTestContext(recorder)
+               ctx.Set(consts.ContextKeyStore, handler.s)
+               ctx.Params = []gin.Param{
+                       {Key: "namespace", Value: ns},
+                       {Key: "cluster", Value: clusterName},
+                       {Key: "shard", Value: "0"},
+               }
+               ctx.Request.Body = 
io.NopCloser(bytes.NewBufferString(`{"preferred_node_id": "1234567890"}`))
+               middleware.RequiredClusterShard(ctx)
+               require.Equal(t, http.StatusOK, recorder.Code)
+       })
 }