From eb2db8559361c55b7ece69897f220a5a91be5c59 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Wed, 29 Mar 2017 16:01:53 -0700 Subject: [PATCH] Bans check updates for nodes other than top-level reg. updates. --- consul/autopilot_test.go | 2 +- consul/state/catalog.go | 14 ++++++++------ consul/state/catalog_test.go | 34 +++++++++++++++++++++++++++++++++- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/consul/autopilot_test.go b/consul/autopilot_test.go index e2811a49bb20..5aed5719ba25 100644 --- a/consul/autopilot_test.go +++ b/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() diff --git a/consul/state/catalog.go b/consul/state/catalog.go index 217ad02ea56b..ef44d403834e 100644 --- a/consul/state/catalog.go +++ b/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) } diff --git a/consul/state/catalog_test.go b/consul/state/catalog_test.go index 21bf59ab6733..c10f5182a2ec 100644 --- a/consul/state/catalog_test.go +++ b/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) {