Merge pull request #2846 from hashicorp/check-match

Bans check updates for nodes other than top-level reg. updates.

consul/autopilot_test.go

@@ -194,7 +194,7 @@ func TestAutopilot_CleanupStaleRaftServer(t *testing.T) {
 	// Add s4 to peers directly
 	s4addr := fmt.Sprintf("127.0.0.1:%d",
 		s4.config.SerfLANConfig.MemberlistConfig.BindPort)
-	s1.raft.AddVoter(raft.ServerID(s4.config.NodeID), raft.ServerAddress(s4addr),0, 0)
+	s1.raft.AddVoter(raft.ServerID(s4.config.NodeID), raft.ServerAddress(s4addr), 0, 0)
 
 	// Verify we have 4 peers
 	peers, err := s1.numPeers()

consul/state/catalog.go

@@ -126,19 +126,21 @@ func (s *StateStore) ensureRegistrationTxn(tx *memdb.Txn, idx uint64, req *struc
 		}
 	}
 
-	// TODO (slackpad) In Consul 0.8 ban checks that don't have the same
-	// node as the top-level registration. This is just weird to be able to
-	// update unrelated nodes' checks from in here. In 0.7.2 we banned this
-	// up in the ACL check since that's guarded behind an opt-in flag until
-	// Consul 0.8.
-
 	// Add the checks, if any.
 	if req.Check != nil {
+		if req.Check.Node != req.Node {
+			return fmt.Errorf("check node %q does not match node %q",
+				req.Check.Node, req.Node)
+		}
 		if err := s.ensureCheckTxn(tx, idx, req.Check); err != nil {
 			return fmt.Errorf("failed inserting check: %s", err)
 		}
 	}
 	for _, check := range req.Checks {
+		if check.Node != req.Node {
+			return fmt.Errorf("check node %q does not match node %q",
+				check.Node, req.Node)
+		}
 		if err := s.ensureCheckTxn(tx, idx, check); err != nil {
 			return fmt.Errorf("failed inserting check: %s", err)
 		}

consul/state/catalog_test.go

@@ -171,7 +171,7 @@ func TestStateStore_EnsureRegistration(t *testing.T) {
 	// Verify that the additional check got registered.
 	verifyNode()
 	verifyService()
-	{
+	verifyChecks := func() {
 		idx, out, err := s.NodeChecks(nil, "node1")
 		if err != nil {
 			t.Fatalf("err: %s", err)
@@ -194,6 +194,38 @@ func TestStateStore_EnsureRegistration(t *testing.T) {
 			t.Fatalf("bad check returned: %#v", c2)
 		}
 	}
+	verifyChecks()
+
+	// Try to register a check for some other node (top-level check).
+	req.Check = &structs.HealthCheck{
+		Node:    "nope",
+		CheckID: "check1",
+		Name:    "check",
+	}
+	err := s.EnsureRegistration(5, req)
+	if err == nil || !strings.Contains(err.Error(), "does not match node") {
+		t.Fatalf("err: %s", err)
+	}
+	verifyNode()
+	verifyService()
+	verifyChecks()
+
+	// Try to register a check for some other node (checks array).
+	req.Check = nil
+	req.Checks = structs.HealthChecks{
+		&structs.HealthCheck{
+			Node:    "nope",
+			CheckID: "check2",
+			Name:    "check",
+		},
+	}
+	err = s.EnsureRegistration(6, req)
+	if err == nil || !strings.Contains(err.Error(), "does not match node") {
+		t.Fatalf("err: %s", err)
+	}
+	verifyNode()
+	verifyService()
+	verifyChecks()
 }
 
 func TestStateStore_EnsureRegistration_Restore(t *testing.T) {