add validation around node-remove conditions

cluster.go

@@ -1690,6 +1690,16 @@ func (c *Cluster) NodeLeave(node *Node) error {
 		return fmt.Errorf("Cluster must be in state %s to remove a node. Current state: %s", ClusterStateNormal, c.State())
 	}
 
+	// Ensure that node is in the cluster.
+	if c.nodeByID(node.ID) == nil {
+		return fmt.Errorf("Node is not a member of the cluster: %s", node.ID)
+	}
+
+	// Prevent removing the coordinator node (this node).
+	if node.ID == c.Node.ID {
+		return fmt.Errorf("The coordinator node cannot be removed. First, make a different node the new coordinator.")
+	}
+
 	return c.nodeLeave(node)
 }
 

handler.go

@@ -2042,20 +2042,13 @@ func (h *Handler) handlePostClusterResizeRemoveNode(w http.ResponseWriter, r *ht
 
 	removeNode := h.Cluster.nodeByID(req.ID)
 	if removeNode == nil {
-		http.Error(w, err.Error(), http.StatusBadRequest)
+		http.Error(w, fmt.Sprintf("Node is not a member of the cluster: %s", req.ID), http.StatusBadRequest)
 		return
 	}
-	if err := func() error {
-		// TODO: prevent removing the coordinator node
 
-		// Start the resize process (similar to NodeJoin)
-		err := h.Cluster.NodeLeave(removeNode)
-		if err != nil {
-			return err
-		}
-
-		return nil
-	}(); err != nil {
+	// Start the resize process (similar to NodeJoin)
+	err = h.Cluster.NodeLeave(removeNode)
+	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
@@ -2208,7 +2201,6 @@ func GetTimeStamp(data map[string]interface{}, timeField string) (int64, error)
 func (h *Handler) handlePostClusterMessage(w http.ResponseWriter, r *http.Request) {
 	// Verify that request is only communicating over protobufs.
 	if r.Header.Get("Content-Type") != "application/x-protobuf" {
-		fmt.Println("**unsupported media type**")
 		http.Error(w, "Unsupported media type", http.StatusUnsupportedMediaType)
 		return
 	}

handler_test.go

@@ -1075,9 +1075,6 @@ func TestHandler_Frame_GetFields(t *testing.T) {
 			t.Fatal(err)
 		}
 		resp, err := http.Get(s.URL + "/index/i/frame/f/fields")
-		if err != nil {
-			t.Fatal(err)
-		}
 		if err != nil {
 			t.Fatal(err)
 		} else if resp.StatusCode != http.StatusOK {

server/cluster_test.go

@@ -16,7 +16,10 @@ package server_test
 
 import (
 	"context"
+	"fmt"
+	"net/http"
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 
@@ -435,3 +438,50 @@ func TestClusterResize_AddNode(t *testing.T) {
 		}
 	})
 }
+
+func TestClusterResize_RemoveNode(t *testing.T) {
+	cluster := test.MustRunMainWithCluster(t, 3)
+	m0 := cluster[0]
+	m1 := cluster[1]
+
+	t.Run("ErrorRemoveInvalidNode", func(t *testing.T) {
+		resp := test.MustDo("POST", m0.URL()+fmt.Sprintf("/cluster/resize/remove-node"), `{"id": "invalid-node-id"}`)
+		expBody := "Node is not a member of the cluster: invalid-node-id"
+		if resp.StatusCode != http.StatusBadRequest {
+			t.Fatalf("expected StatusCode %d but got %d", http.StatusBadRequest, resp.StatusCode)
+		} else if strings.TrimSpace(resp.Body) != expBody {
+			t.Fatalf("expected Body '%s' but got '%s'", expBody, strings.TrimSpace(resp.Body))
+		}
+	})
+
+	t.Run("ErrorRemoveCoordinator", func(t *testing.T) {
+		resp := test.MustDo("GET", m0.URL()+fmt.Sprintf("/id"), "")
+		nodeID := resp.Body
+
+		resp = test.MustDo("POST", m0.URL()+fmt.Sprintf("/cluster/resize/remove-node"), fmt.Sprintf(`{"id": "%s"}`, nodeID))
+
+		expBody := "The coordinator node cannot be removed. First, make a different node the new coordinator."
+		if resp.StatusCode != http.StatusInternalServerError {
+			t.Fatalf("expected StatusCode %d but got %d", http.StatusInternalServerError, resp.StatusCode)
+		} else if strings.TrimSpace(resp.Body) != expBody {
+			t.Fatalf("expected Body '%s' but got '%s'", expBody, strings.TrimSpace(resp.Body))
+		}
+	})
+
+	t.Run("ErrorRemoveOnNonCoordinator", func(t *testing.T) {
+		resp := test.MustDo("GET", m0.URL()+fmt.Sprintf("/id"), "")
+		coordinatorNodeID := resp.Body
+
+		resp = test.MustDo("GET", m1.URL()+fmt.Sprintf("/id"), "")
+		nodeID := resp.Body
+
+		resp = test.MustDo("POST", m1.URL()+fmt.Sprintf("/cluster/resize/remove-node"), fmt.Sprintf(`{"id": "%s"}`, nodeID))
+
+		expBody := fmt.Sprintf("Node removal requests are only valid on the Coordinator node: %s", coordinatorNodeID)
+		if resp.StatusCode != http.StatusInternalServerError {
+			t.Fatalf("expected StatusCode %d but got %d", http.StatusInternalServerError, resp.StatusCode)
+		} else if strings.TrimSpace(resp.Body) != expBody {
+			t.Fatalf("expected Body '%s' but got '%s'", expBody, strings.TrimSpace(resp.Body))
+		}
+	})
+}