Return 404 from force-leave for nodes that are not cluster members

.changelog/24082.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+http: return 404 instead of 500 when force-leaving a node that is not a cluster member
+```

agent/agent_endpoint.go

@@ -795,10 +795,14 @@ func (s *HTTPHandlers) AgentForceLeave(resp http.ResponseWriter, req *http.Reque
 
 	addr := strings.TrimPrefix(req.URL.Path, "/v1/agent/force-leave/")
 	if wan {
-		return nil, s.agent.ForceLeaveWAN(addr, prune, entMeta)
+		err = s.agent.ForceLeaveWAN(addr, prune, entMeta)
 	} else {
-		return nil, s.agent.ForceLeave(addr, prune, entMeta)
+		err = s.agent.ForceLeave(addr, prune, entMeta)
 	}
+	if errors.Is(err, consul.ErrNodeNotFound) {
+		return nil, HTTPError{StatusCode: http.StatusNotFound, Reason: err.Error()}
+	}
+	return nil, err
 }
 
 // syncChanges is a helper function which wraps a blocking call to sync

agent/agent_endpoint_test.go

@@ -2630,6 +2630,25 @@ func TestAgent_ForceLeave_ACLDeny(t *testing.T) {
 	})
 }
 
+func TestAgent_ForceLeave_UnknownNode(t *testing.T) {
+	if testing.Short() {
+		t.Skip("too slow for testing.Short")
+	}
+
+	t.Parallel()
+	a := NewTestAgent(t, "")
+	defer a.Shutdown()
+	testrpc.WaitForLeader(t, a.RPC, "dc1")
+
+	// Force-leaving a node that is not a cluster member is a client
+	// error, not a server error: the caller named something that does
+	// not exist, so the endpoint must answer 404 (see #21878).
+	req, _ := http.NewRequest("PUT", "/v1/agent/force-leave/nonexistent-node", nil)
+	resp := httptest.NewRecorder()
+	a.srv.h.ServeHTTP(resp, req)
+	require.Equal(t, http.StatusNotFound, resp.Code)
+}
+
 func TestAgent_ForceLeavePrune(t *testing.T) {
 	if testing.Short() {
 		t.Skip("too slow for testing.Short")

agent/consul/client.go

@@ -5,6 +5,7 @@ package consul
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"strconv"
@@ -258,6 +259,11 @@ func (c *Client) LANMembers(filter LANMemberFilter) ([]serf.Member, error) {
 	return c.serf.Members(), nil
 }
 
+// ErrNodeNotFound is returned when a node targeted by an operation, such
+// as a force-leave, is not a member of the cluster. Callers that surface
+// this to clients can map it to a 404 instead of a generic 500.
+var ErrNodeNotFound = errors.New("agent: No node found")
+
 // RemoveFailedNode is used to remove a failed node from the cluster.
 func (c *Client) RemoveFailedNode(node string, prune bool, entMeta *acl.EnterpriseMeta) error {
 	// Partitions definitely have to match.
@@ -266,7 +272,7 @@ func (c *Client) RemoveFailedNode(node string, prune bool, entMeta *acl.Enterpri
 			c.config.AgentEnterpriseMeta().PartitionOrDefault(), entMeta.PartitionOrDefault())
 	}
 	if !isSerfMember(c.serf, node) {
-		return fmt.Errorf("agent: No node found with name '%s'", node)
+		return fmt.Errorf("%w with name '%s'", ErrNodeNotFound, node)
 	}
 	if prune {
 		return c.serf.RemoveFailedNodePrune(node)

agent/consul/server_ce.go

@@ -84,7 +84,7 @@ func (s *Server) removeFailedNode(
 	}
 
 	if !foundAny {
-		return fmt.Errorf("agent: No node found with name '%s'", node)
+		return fmt.Errorf("%w with name '%s'", ErrNodeNotFound, node)
 	}
 
 	return nil