From d4701903f6ad2a404e2bc47368ede639d7b5383f Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 30 Jul 2021 18:56:11 -0400 Subject: [PATCH] acl: replace ACLResolver.Config with its own struct This is step toward decoupling ACLResolver from the agent/consul package. --- agent/consul/acl.go | 25 ++++++++++++++++++------- agent/consul/acl_test.go | 12 +++++++++++- agent/consul/client.go | 19 +++++++++++++++---- agent/consul/server.go | 12 +++++++++++- 4 files changed, 55 insertions(+), 13 deletions(-) diff --git a/agent/consul/acl.go b/agent/consul/acl.go index dd90d267e9..49fdde84da 100644 --- a/agent/consul/acl.go +++ b/agent/consul/acl.go @@ -189,7 +189,8 @@ func (e policyOrRoleTokenError) Error() string { // ACLResolverConfig holds all the configuration necessary to create an ACLResolver type ACLResolverConfig struct { - Config *Config + // TODO: rename this field? + Config ACLResolverSettings Logger hclog.Logger // CacheConfig is a pass through configuration for ACL cache limits @@ -211,6 +212,20 @@ type ACLResolverConfig struct { Tokens *token.Store } +// TODO: remove these fields from consul.Config and config.RuntimeConfig +// TODO: rename the fields to remove the ACL prefix +type ACLResolverSettings struct { + ACLsEnabled bool + Datacenter string + NodeName string + ACLPolicyTTL time.Duration + ACLTokenTTL time.Duration + ACLRoleTTL time.Duration + ACLDisabledTTL time.Duration + ACLDownPolicy string + ACLDefaultPolicy string +} + // ACLResolver is the type to handle all your token and policy resolution needs. // // Supports: @@ -237,7 +252,8 @@ type ACLResolverConfig struct { // upon. // type ACLResolver struct { - config *Config + // TODO: store the ACLResolverConfig as a field instead of copying all the fields onto ACLResolver. + config ACLResolverSettings logger hclog.Logger delegate ACLResolverDelegate @@ -289,11 +305,6 @@ func NewACLResolver(config *ACLResolverConfig) (*ACLResolver, error) { if config == nil { return nil, fmt.Errorf("ACL Resolver must be initialized with a config") } - - if config.Config == nil { - return nil, fmt.Errorf("ACLResolverConfig.Config must not be nil") - } - if config.Delegate == nil { return nil, fmt.Errorf("ACL Resolver must be initialized with a valid delegate") } diff --git a/agent/consul/acl_test.go b/agent/consul/acl_test.go index 1c870b823f..804e775f62 100644 --- a/agent/consul/acl_test.go +++ b/agent/consul/acl_test.go @@ -722,7 +722,17 @@ func newTestACLResolver(t *testing.T, delegate *ACLResolverTestDelegate, cb func config.ACLDownPolicy = "extend-cache" config.ACLsEnabled = delegate.enabled rconf := &ACLResolverConfig{ - Config: config, + Config: ACLResolverSettings{ + ACLsEnabled: config.ACLsEnabled, + Datacenter: config.Datacenter, + NodeName: config.NodeName, + ACLPolicyTTL: config.ACLPolicyTTL, + ACLTokenTTL: config.ACLTokenTTL, + ACLRoleTTL: config.ACLRoleTTL, + ACLDisabledTTL: config.ACLDisabledTTL, + ACLDownPolicy: config.ACLDownPolicy, + ACLDefaultPolicy: config.ACLDefaultPolicy, + }, Logger: testutil.Logger(t), CacheConfig: &structs.ACLCachesConfig{ Identities: 4, diff --git a/agent/consul/client.go b/agent/consul/client.go index d09dd34c73..b966919181 100644 --- a/agent/consul/client.go +++ b/agent/consul/client.go @@ -10,6 +10,10 @@ import ( "github.com/armon/go-metrics" "github.com/armon/go-metrics/prometheus" + "github.com/hashicorp/go-hclog" + "github.com/hashicorp/serf/serf" + "golang.org/x/time/rate" + "github.com/hashicorp/consul/agent/pool" "github.com/hashicorp/consul/agent/router" "github.com/hashicorp/consul/agent/structs" @@ -17,9 +21,6 @@ import ( "github.com/hashicorp/consul/logging" "github.com/hashicorp/consul/tlsutil" "github.com/hashicorp/consul/types" - "github.com/hashicorp/go-hclog" - "github.com/hashicorp/serf/serf" - "golang.org/x/time/rate" ) var ClientCounters = []prometheus.CounterDefinition{ @@ -122,7 +123,17 @@ func NewClient(config *Config, deps Deps) (*Client, error) { c.useNewACLs = 0 aclConfig := ACLResolverConfig{ - Config: config, + Config: ACLResolverSettings{ + ACLsEnabled: config.ACLsEnabled, + Datacenter: config.Datacenter, + NodeName: config.NodeName, + ACLPolicyTTL: config.ACLPolicyTTL, + ACLTokenTTL: config.ACLTokenTTL, + ACLRoleTTL: config.ACLRoleTTL, + ACLDisabledTTL: config.ACLDisabledTTL, + ACLDownPolicy: config.ACLDownPolicy, + ACLDefaultPolicy: config.ACLDefaultPolicy, + }, Delegate: c, Logger: c.logger, AutoDisable: true, diff --git a/agent/consul/server.go b/agent/consul/server.go index 4cab854e09..f9885348d2 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -426,7 +426,17 @@ func NewServer(config *Config, flat Deps) (*Server, error) { s.aclConfig = newACLConfig(logger) s.useNewACLs = 0 aclConfig := ACLResolverConfig{ - Config: config, + Config: ACLResolverSettings{ + ACLsEnabled: config.ACLsEnabled, + Datacenter: config.Datacenter, + NodeName: config.NodeName, + ACLPolicyTTL: config.ACLPolicyTTL, + ACLTokenTTL: config.ACLTokenTTL, + ACLRoleTTL: config.ACLRoleTTL, + ACLDisabledTTL: config.ACLDisabledTTL, + ACLDownPolicy: config.ACLDownPolicy, + ACLDefaultPolicy: config.ACLDefaultPolicy, + }, Delegate: s, CacheConfig: serverACLCacheConfig, AutoDisable: false,