catalog: compare node names case insensitively in more places

.changelog/12444.txt

@@ -0,0 +1,7 @@
+```release-note:bug
+compare node names case insensitively in more places
+```
+
+```release-note:bug
+state: fix bug blocking snapshot restore when a node check and node differed in casing of the node string
+```

agent/catalog_endpoint_test.go

@@ -2,19 +2,21 @@ package agent
 
 import (
 	"fmt"
-	"github.com/hashicorp/consul/api"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
 	"testing"
 	"time"
 
-	"github.com/hashicorp/consul/agent/structs"
-	"github.com/hashicorp/consul/sdk/testutil/retry"
-	"github.com/hashicorp/consul/testrpc"
+	"github.com/hashicorp/consul/api"
+
 	"github.com/hashicorp/serf/coordinate"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/hashicorp/consul/agent/structs"
+	"github.com/hashicorp/consul/sdk/testutil/retry"
+	"github.com/hashicorp/consul/testrpc"
 )
 
 func TestCatalogRegister_Service_InvalidAddress(t *testing.T) {
@@ -412,76 +414,50 @@ func TestCatalogNodes_DistanceSort(t *testing.T) {
 		Address:    "127.0.0.1",
 	}
 	var out struct{}
-	if err := a.RPC("Catalog.Register", args, &out); err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	require.NoError(t, a.RPC("Catalog.Register", args, &out))
 
 	args = &structs.RegisterRequest{
 		Datacenter: "dc1",
 		Node:       "bar",
 		Address:    "127.0.0.2",
 	}
-	if err := a.RPC("Catalog.Register", args, &out); err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	require.NoError(t, a.RPC("Catalog.Register", args, &out))
 
 	// Nobody has coordinates set so this will still return them in the
 	// order they are indexed.
 	req, _ := http.NewRequest("GET", "/v1/catalog/nodes?dc=dc1&near=foo", nil)
 	resp := httptest.NewRecorder()
 	obj, err := a.srv.CatalogNodes(resp, req)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	require.NoError(t, err)
 
 	assertIndex(t, resp)
 	nodes := obj.(structs.Nodes)
-	if len(nodes) != 3 {
-		t.Fatalf("bad: %v", obj)
-	}
-	if nodes[0].Node != "bar" {
-		t.Fatalf("bad: %v", nodes)
-	}
-	if nodes[1].Node != "foo" {
-		t.Fatalf("bad: %v", nodes)
-	}
-	if nodes[2].Node != a.Config.NodeName {
-		t.Fatalf("bad: %v", nodes)
-	}
+	require.Len(t, nodes, 3)
+	require.Equal(t, "bar", nodes[0].Node)
+	require.Equal(t, "foo", nodes[1].Node)
+	require.Equal(t, a.Config.NodeName, nodes[2].Node)
 
 	// Send an update for the node and wait for it to get applied.
 	arg := structs.CoordinateUpdateRequest{
 		Datacenter: "dc1",
 		Node:       "foo",
 		Coord:      coordinate.NewCoordinate(coordinate.DefaultConfig()),
 	}
-	if err := a.RPC("Coordinate.Update", &arg, &out); err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	require.NoError(t, a.RPC("Coordinate.Update", &arg, &out))
 	time.Sleep(300 * time.Millisecond)
 
 	// Query again and now foo should have moved to the front of the line.
 	req, _ = http.NewRequest("GET", "/v1/catalog/nodes?dc=dc1&near=foo", nil)
 	resp = httptest.NewRecorder()
 	obj, err = a.srv.CatalogNodes(resp, req)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	require.NoError(t, err)
 
 	assertIndex(t, resp)
 	nodes = obj.(structs.Nodes)
-	if len(nodes) != 3 {
-		t.Fatalf("bad: %v", obj)
-	}
-	if nodes[0].Node != "foo" {
-		t.Fatalf("bad: %v", nodes)
-	}
-	if nodes[1].Node != "bar" {
-		t.Fatalf("bad: %v", nodes)
-	}
-	if nodes[2].Node != a.Config.NodeName {
-		t.Fatalf("bad: %v", nodes)
-	}
+	require.Len(t, nodes, 3)
+	require.Equal(t, "foo", nodes[0].Node)
+	require.Equal(t, "bar", nodes[1].Node)
+	require.Equal(t, a.Config.NodeName, nodes[2].Node)
 }
 
 func TestCatalogServices(t *testing.T) {

agent/checks/alias.go

@@ -2,6 +2,7 @@ package checks
 
 import (
 	"fmt"
+	"strings"
 	"sync"
 	"time"
 
@@ -246,7 +247,7 @@ func (c *CheckAlias) processChecks(checks []*structs.HealthCheck, CheckIfService
 	msg := "No checks found."
 	serviceFound := false
 	for _, chk := range checks {
-		if c.Node != "" && c.Node != chk.Node {
+		if c.Node != "" && !strings.EqualFold(c.Node, chk.Node) {
 			continue
 		}
 		serviceMatch := c.ServiceID.Matches(chk.CompoundServiceID())

agent/checks/alias_test.go

@@ -454,46 +454,55 @@ func TestCheckAlias_remoteNodeOnlyPassing(t *testing.T) {
 func TestCheckAlias_remoteNodeOnlyCritical(t *testing.T) {
 	t.Parallel()
 
-	notify := newMockAliasNotify()
-	chkID := structs.NewCheckID(types.CheckID("foo"), nil)
-	rpc := &mockRPC{}
-	chk := &CheckAlias{
-		Node:    "remote",
-		CheckID: chkID,
-		Notify:  notify,
-		RPC:     rpc,
-	}
+	run := func(t *testing.T, responseNodeName string) {
+		notify := newMockAliasNotify()
+		chkID := structs.NewCheckID(types.CheckID("foo"), nil)
+		rpc := &mockRPC{}
+		chk := &CheckAlias{
+			Node:    "remote",
+			CheckID: chkID,
+			Notify:  notify,
+			RPC:     rpc,
+		}
 
-	rpc.AddReply("Health.NodeChecks", structs.IndexedHealthChecks{
-		HealthChecks: []*structs.HealthCheck{
-			// Should ignore non-matching node
-			{
-				Node:      "A",
-				ServiceID: "web",
-				Status:    api.HealthCritical,
-			},
-
-			// Should ignore any services
-			{
-				Node:      "remote",
-				ServiceID: "db",
-				Status:    api.HealthCritical,
+		rpc.AddReply("Health.NodeChecks", structs.IndexedHealthChecks{
+			HealthChecks: []*structs.HealthCheck{
+				// Should ignore non-matching node
+				{
+					Node:      "A",
+					ServiceID: "web",
+					Status:    api.HealthCritical,
+				},
+
+				// Should ignore any services
+				{
+					Node:      responseNodeName,
+					ServiceID: "db",
+					Status:    api.HealthCritical,
+				},
+
+				// Match
+				{
+					Node:   responseNodeName,
+					Status: api.HealthCritical,
+				},
 			},
+		})
+
+		chk.Start()
+		defer chk.Stop()
+		retry.Run(t, func(r *retry.R) {
+			if got, want := notify.State(chkID), api.HealthCritical; got != want {
+				r.Fatalf("got state %q want %q", got, want)
+			}
+		})
+	}
 
-			// Match
-			{
-				Node:   "remote",
-				Status: api.HealthCritical,
-			},
-		},
+	t.Run("same case node name", func(t *testing.T) {
+		run(t, "remote")
 	})
-
-	chk.Start()
-	defer chk.Stop()
-	retry.Run(t, func(r *retry.R) {
-		if got, want := notify.State(chkID), api.HealthCritical; got != want {
-			r.Fatalf("got state %q want %q", got, want)
-		}
+	t.Run("lowercase node name", func(t *testing.T) {
+		run(t, "ReMoTe")
 	})
 }
 

agent/consul/catalog_endpoint.go

@@ -3,6 +3,7 @@ package consul
 import (
 	"fmt"
 	"sort"
+	"strings"
 	"time"
 
 	"github.com/armon/go-metrics"
@@ -285,7 +286,7 @@ func vetRegisterWithACL(
 		// note in state_store.go to ban this down there in Consul 0.8,
 		// but it's good to leave this here because it's required for
 		// correctness wrt. ACLs.
-		if check.Node != subj.Node {
+		if !strings.EqualFold(check.Node, subj.Node) {
 			return fmt.Errorf("Node '%s' for check '%s' doesn't match register request node '%s'",
 				check.Node, check.CheckID, subj.Node)
 		}