Fix segfault when removing both a service and associated check (#7108)

* Fix segfault when removing both a service and associated check

updateSyncState creates entries in the services and checks maps for
remote services/checks that are not found locally, so that we can then
make sure to delete them in our reconciliation process. However, the
values added to the map are missing key fields that the rest of the code
expects to not be nil.

* Add comment stating Check field can be nil

agent/local/state.go

@@ -70,7 +70,7 @@ type CheckState struct {
 	// Check is the local copy of the health check record.
 	//
 	// Must Clone() the overall CheckState before mutating this. After mutation
-	// reinstall into the checks map.
+	// reinstall into the checks map. If Deleted is true, this field can be nil.
 	Check *structs.HealthCheck
 
 	// Token is the ACL record to update or delete the health check
@@ -1093,7 +1093,7 @@ func (l *State) deleteService(key structs.ServiceID) error {
 		delete(l.services, key)
 		// service deregister also deletes associated checks
 		for _, c := range l.checks {
-			if c.Deleted && c.Check.ServiceID == key.ID {
+			if c.Deleted && c.Check != nil && c.Check.ServiceID == key.ID {
 				l.pruneCheck(c.Check.CompoundCheckID())
 			}
 		}

agent/local/state_test.go

@@ -1081,6 +1081,81 @@ func TestAgentAntiEntropy_Checks(t *testing.T) {
 	}
 }
 
+func TestAgentAntiEntropy_RemovingServiceAndCheck(t *testing.T) {
+	t.Parallel()
+	a := agent.NewTestAgent(t, t.Name(), "")
+	defer a.Shutdown()
+
+	testrpc.WaitForTestAgent(t, a.RPC, "dc1")
+	// Register info
+	args := &structs.RegisterRequest{
+		Datacenter: "dc1",
+		Node:       a.Config.NodeName,
+		Address:    "127.0.0.1",
+	}
+
+	var out struct{}
+
+	// Exists remote (delete)
+	svcID := "deleted-check-service"
+	srv := &structs.NodeService{
+		ID:      svcID,
+		Service: "echo",
+		Tags:    []string{},
+		Address: "127.0.0.1",
+		Port:    8080,
+	}
+	args.Service = srv
+	if err := a.RPC("Catalog.Register", args, &out); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Exists remote (delete)
+	chk := &structs.HealthCheck{
+		Node:           a.Config.NodeName,
+		CheckID:        "lb",
+		Name:           "lb",
+		ServiceID:      svcID,
+		Status:         api.HealthPassing,
+		EnterpriseMeta: *structs.DefaultEnterpriseMeta(),
+	}
+
+	args.Check = chk
+	if err := a.RPC("Catalog.Register", args, &out); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	if err := a.State.SyncFull(); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	var services structs.IndexedNodeServices
+	req := structs.NodeSpecificRequest{
+		Datacenter: "dc1",
+		Node:       a.Config.NodeName,
+	}
+
+	if err := a.RPC("Catalog.NodeServices", &req, &services); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// The consul service will still be registered
+	if len(services.NodeServices.Services) != 1 {
+		t.Fatalf("Expected all services to be deleted, got: %#v", services.NodeServices.Services)
+	}
+
+	var checks structs.IndexedHealthChecks
+	// Verify that we are in sync
+	if err := a.RPC("Health.NodeChecks", &req, &checks); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// The serfHealth check will still be here
+	if len(checks.HealthChecks) != 1 {
+		t.Fatalf("Expected the health check to be deleted, got: %#v", checks.HealthChecks)
+	}
+}
+
 func TestAgentAntiEntropy_Checks_ACLDeny(t *testing.T) {
 	t.Parallel()
 	dc := "dc1"