From 16a9f6d671667e056cd04ebdd736d9b34b48c307 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Thu, 23 Mar 2017 13:34:30 -0700 Subject: [PATCH] Modifies server reconcile path to not use the server's token for internal operations. --- consul/leader.go | 26 ++++------ consul/leader_test.go | 51 ++++++++++++++++--- .../source/docs/agent/options.html.markdown | 5 +- .../source/docs/internals/acl.html.markdown | 10 ++-- 4 files changed, 61 insertions(+), 31 deletions(-) diff --git a/consul/leader.go b/consul/leader.go index dbc60d8a0..124a14a25 100644 --- a/consul/leader.go +++ b/consul/leader.go @@ -464,10 +464,9 @@ AFTER_CHECK: Status: structs.HealthPassing, Output: SerfCheckAliveOutput, }, - WriteRequest: structs.WriteRequest{Token: s.config.GetTokenForAgent()}, } - var out struct{} - return s.endpoints.Catalog.Register(&req, &out) + _, err = s.raftApply(structs.RegisterRequestType, &req) + return err } // handleFailedMember is used to mark the node's status @@ -505,10 +504,9 @@ func (s *Server) handleFailedMember(member serf.Member) error { Status: structs.HealthCritical, Output: SerfCheckFailedOutput, }, - WriteRequest: structs.WriteRequest{Token: s.config.GetTokenForAgent()}, } - var out struct{} - return s.endpoints.Catalog.Register(&req, &out) + _, err = s.raftApply(structs.RegisterRequestType, &req) + return err } // handleLeftMember is used to handle members that gracefully @@ -553,12 +551,11 @@ func (s *Server) handleDeregisterMember(reason string, member serf.Member) error // Deregister the node s.logger.Printf("[INFO] consul: member '%s' %s, deregistering", member.Name, reason) req := structs.DeregisterRequest{ - Datacenter: s.config.Datacenter, - Node: member.Name, - WriteRequest: structs.WriteRequest{Token: s.config.GetTokenForAgent()}, + Datacenter: s.config.Datacenter, + Node: member.Name, } - var out struct{} - return s.endpoints.Catalog.Deregister(&req, &out) + _, err = s.raftApply(structs.DeregisterRequestType, &req) + return err } // joinConsulServer is used to try to join another consul server @@ -712,10 +709,9 @@ func (s *Server) removeConsulServer(m serf.Member, port int) error { func (s *Server) reapTombstones(index uint64) { defer metrics.MeasureSince([]string{"consul", "leader", "reapTombstones"}, time.Now()) req := structs.TombstoneRequest{ - Datacenter: s.config.Datacenter, - Op: structs.TombstoneReap, - ReapIndex: index, - WriteRequest: structs.WriteRequest{Token: s.config.GetTokenForAgent()}, + Datacenter: s.config.Datacenter, + Op: structs.TombstoneReap, + ReapIndex: index, } _, err := s.raftApply(structs.TombstoneRequestType, &req) if err != nil { diff --git a/consul/leader_test.go b/consul/leader_test.go index 7dc9e0a24..243420e59 100644 --- a/consul/leader_test.go +++ b/consul/leader_test.go @@ -14,7 +14,12 @@ import ( ) func TestLeader_RegisterMember(t *testing.T) { - dir1, s1 := testServer(t) + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLMasterToken = "root" + c.ACLDefaultPolicy = "deny" + c.ACLEnforceVersion8 = true + }) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -81,7 +86,12 @@ func TestLeader_RegisterMember(t *testing.T) { } func TestLeader_FailedMember(t *testing.T) { - dir1, s1 := testServer(t) + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLMasterToken = "root" + c.ACLDefaultPolicy = "deny" + c.ACLEnforceVersion8 = true + }) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -140,7 +150,12 @@ func TestLeader_FailedMember(t *testing.T) { } func TestLeader_LeftMember(t *testing.T) { - dir1, s1 := testServer(t) + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLMasterToken = "root" + c.ACLDefaultPolicy = "deny" + c.ACLEnforceVersion8 = true + }) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -185,7 +200,12 @@ func TestLeader_LeftMember(t *testing.T) { } func TestLeader_ReapMember(t *testing.T) { - dir1, s1 := testServer(t) + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLMasterToken = "root" + c.ACLDefaultPolicy = "deny" + c.ACLEnforceVersion8 = true + }) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -244,7 +264,12 @@ func TestLeader_ReapMember(t *testing.T) { } func TestLeader_Reconcile_ReapMember(t *testing.T) { - dir1, s1 := testServer(t) + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLMasterToken = "root" + c.ACLDefaultPolicy = "deny" + c.ACLEnforceVersion8 = true + }) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -261,6 +286,9 @@ func TestLeader_Reconcile_ReapMember(t *testing.T) { Name: SerfCheckName, Status: structs.HealthCritical, }, + WriteRequest: structs.WriteRequest{ + Token: "root", + }, } var out struct{} if err := s1.RPC("Catalog.Register", &dead, &out); err != nil { @@ -284,7 +312,12 @@ func TestLeader_Reconcile_ReapMember(t *testing.T) { } func TestLeader_Reconcile(t *testing.T) { - dir1, s1 := testServer(t) + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLMasterToken = "root" + c.ACLDefaultPolicy = "deny" + c.ACLEnforceVersion8 = true + }) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -562,6 +595,9 @@ func TestLeader_TombstoneGC_Reset(t *testing.T) { func TestLeader_ReapTombstones(t *testing.T) { dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLMasterToken = "root" + c.ACLDefaultPolicy = "deny" c.TombstoneTTL = 50 * time.Millisecond c.TombstoneTTLGranularity = 10 * time.Millisecond }) @@ -579,6 +615,9 @@ func TestLeader_ReapTombstones(t *testing.T) { Key: "test", Value: []byte("test"), }, + WriteRequest: structs.WriteRequest{ + Token: "root", + }, } var out bool if err := msgpackrpc.CallWithCodec(codec, "KVS.Apply", &arg, &out); err != nil { diff --git a/website/source/docs/agent/options.html.markdown b/website/source/docs/agent/options.html.markdown index c523c0c0f..d20079320 100644 --- a/website/source/docs/agent/options.html.markdown +++ b/website/source/docs/agent/options.html.markdown @@ -454,9 +454,8 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass and servers to perform internal operations to the service catalog. If this isn't specified, then the `acl_token` will be used. This was added in Consul 0.7.2.

- For clients, this token must at least have write access to the node name it will register as. For - servers, this must have write access to all nodes that are expected to join the cluster, as well - as write access to the "consul" service, which will be registered automatically on its behalf. + This token must at least have write access to the node name it will register as in order to set any + of the node-level information in the catalog such as metadata, or the node's tagged addresses. * `acl_enforce_version_8` - Used for clients and servers to determine if enforcement should occur for new ACL policies being diff --git a/website/source/docs/internals/acl.html.markdown b/website/source/docs/internals/acl.html.markdown index 382566800..d0231ef84 100644 --- a/website/source/docs/internals/acl.html.markdown +++ b/website/source/docs/internals/acl.html.markdown @@ -571,13 +571,9 @@ Two new configuration options are used once complete ACLs are enabled: tokens during normal operation. * [`acl_agent_token`](/docs/agent/options.html#acl_agent_token) is used internally by Consul agents to perform operations to the service catalog when registering themselves - or sending network coordinates to the servers. -
-
- For clients, this token must at least have `node` ACL policy `write` access to the node - name it will register as. For servers, this must have `node` ACL policy `write` access to - all nodes that are expected to join the cluster, as well as `service` ACL policy `write` - access to the `consul` service, which will be registered automatically on its behalf. + or sending network coordinates to the servers. This token must at least have `node` ACL + policy `write` access to the node name it will register as in order to register any + node-level information like metadata or tagged addresses. Since clients now resolve ACLs locally, the [`acl_down_policy`](/docs/agent/options.html#acl_down_policy) now applies to Consul clients as well as Consul servers. This will determine what the