From 7a7701a4eb10634b941e8ffab4630c9bfba67c58 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Fri, 27 Mar 2020 14:07:55 -0600 Subject: [PATCH 1/3] consul: annotate Consul interfaces with ACLs --- client/consul/consul.go | 8 +++++++- command/agent/consul/client.go | 15 +++++++++++++++ nomad/consul.go | 9 ++++++--- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/client/consul/consul.go b/client/consul/consul.go index f8348c220..ee66a8c05 100644 --- a/client/consul/consul.go +++ b/client/consul/consul.go @@ -6,7 +6,10 @@ import ( ) // ConsulServiceAPI is the interface the Nomad Client uses to register and -// remove services and checks from Consul. +// remove services and checks from Consul.css +// +// ACL requirements +// - service:write type ConsulServiceAPI interface { // RegisterWorkload with Consul. Adds all service entries and checks to Consul. RegisterWorkload(*consul.WorkloadServices) error @@ -31,6 +34,9 @@ type TokenDeriverFunc func(*structs.Allocation, []string) (map[string]string, er // ServiceIdentityAPI is the interface the Nomad Client uses to request Consul // Service Identity tokens through Nomad Server. +// +// ACL requirements +// - acl:write (used by Server only) type ServiceIdentityAPI interface { // DeriveSITokens contacts the nomad server and requests consul service // identity tokens be generated for tasks in the allocation. diff --git a/command/agent/consul/client.go b/command/agent/consul/client.go index 578c1bfc9..e98152a3f 100644 --- a/command/agent/consul/client.go +++ b/command/agent/consul/client.go @@ -75,13 +75,25 @@ const ( deregisterProbationPeriod = time.Minute ) +// Additional Consul ACLs required +// - Consul Template: key:read +// Used in tasks with template stanza that use Consul keys. + // CatalogAPI is the consul/api.Catalog API used by Nomad. +// +// ACL requirements +// - node:read (listing datacenters) +// - service:read type CatalogAPI interface { Datacenters() ([]string, error) Service(service, tag string, q *api.QueryOptions) ([]*api.CatalogService, *api.QueryMeta, error) } // AgentAPI is the consul/api.Agent API used by Nomad. +// +// ACL requirements +// - agent:read +// - service:write type AgentAPI interface { Services() (map[string]*api.AgentService, error) Checks() (map[string]*api.AgentCheck, error) @@ -94,6 +106,9 @@ type AgentAPI interface { } // ACLsAPI is the consul/api.ACL API subset used by Nomad Server. +// +// ACL requirements +// - acl:write (server only) type ACLsAPI interface { // We are looking up by [operator token] SecretID, which implies we need // to use this method instead of the normal TokenRead, which can only be diff --git a/nomad/consul.go b/nomad/consul.go index 9d1f86289..5dbce3e50 100644 --- a/nomad/consul.go +++ b/nomad/consul.go @@ -30,9 +30,9 @@ const ( // revocation requests Nomad will make against Consul. siTokenMaxParallelRevokes = 64 - // siTokenRevocationIterval is the interval at which SI tokens that failed + // siTokenRevocationInterval is the interval at which SI tokens that failed // initial revocation are retried. - siTokenRevocationIterval = 5 * time.Minute + siTokenRevocationInterval = 5 * time.Minute ) const ( @@ -77,6 +77,9 @@ func (sii ServiceIdentityIndex) Description() string { // ConsulACLsAPI is an abstraction over the consul/api.ACL API used by Nomad // Server. +// +// ACL requirements +// - acl:write (transitive through ACLsAPI) type ConsulACLsAPI interface { // CheckSIPolicy checks that the given operator token has the equivalent ACL @@ -350,7 +353,7 @@ func (c *consulACLsAPI) singleRevoke(ctx context.Context, accessor *structs.SITo } func (c *consulACLsAPI) bgRetryRevokeDaemon() { - ticker := time.NewTicker(siTokenRevocationIterval) + ticker := time.NewTicker(siTokenRevocationInterval) defer ticker.Stop() for { From 92e75ad6220afa05b4074ac033204d42af723c15 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Mon, 30 Mar 2020 11:37:03 -0600 Subject: [PATCH 2/3] e2e: minimize Consul ACL policies used in e2e tests Issue #7523 documents the Consul ACLs used in each Consul interface used by Nomad. Minimize the policies used in e2e tests so that we are setting a good example. --- e2e/consulacls/nomad-client-policy.hcl | 8 ++++---- e2e/consulacls/nomad-server-policy.hcl | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/e2e/consulacls/nomad-client-policy.hcl b/e2e/consulacls/nomad-client-policy.hcl index 464d22a5e..3cd908769 100644 --- a/e2e/consulacls/nomad-client-policy.hcl +++ b/e2e/consulacls/nomad-client-policy.hcl @@ -4,10 +4,10 @@ service_prefix "" { policy = "write" } -node_prefix "" { - policy = "write" -} - agent_prefix "" { policy = "read" } + +node_prefix "" { + policy = "read" +} diff --git a/e2e/consulacls/nomad-server-policy.hcl b/e2e/consulacls/nomad-server-policy.hcl index fd47ae56b..160d1a966 100644 --- a/e2e/consulacls/nomad-server-policy.hcl +++ b/e2e/consulacls/nomad-server-policy.hcl @@ -7,10 +7,10 @@ service_prefix "" { policy = "write" } -node_prefix "" { - policy = "write" -} - agent_prefix "" { policy = "read" } + +node_prefix "" { + policy = "read" +} From 0957c24646244d5c8bf68c2036b17a332881aca5 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Mon, 30 Mar 2020 13:26:48 -0600 Subject: [PATCH 3/3] docs: remove erroneous characters from comment --- client/consul/consul.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/consul/consul.go b/client/consul/consul.go index ee66a8c05..5cb2ef165 100644 --- a/client/consul/consul.go +++ b/client/consul/consul.go @@ -6,7 +6,7 @@ import ( ) // ConsulServiceAPI is the interface the Nomad Client uses to register and -// remove services and checks from Consul.css +// remove services and checks from Consul. // // ACL requirements // - service:write