Skip to content

Commit

Permalink
Bans check updates for nodes other than top-level reg. updates.
Browse files Browse the repository at this point in the history
  • Loading branch information
slackpad committed Mar 29, 2017
1 parent 73f0e6f commit eb2db85
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 8 deletions.
2 changes: 1 addition & 1 deletion consul/autopilot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
14 changes: 8 additions & 6 deletions consul/state/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
34 changes: 33 additions & 1 deletion consul/state/catalog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) {
Expand Down

0 comments on commit eb2db85

Please sign in to comment.