diff --git a/client/acl.go b/client/acl.go index 049b85568..89c065214 100644 --- a/client/acl.go +++ b/client/acl.go @@ -4,10 +4,57 @@ import ( "time" metrics "github.com/armon/go-metrics" + lru "github.com/hashicorp/golang-lru" "github.com/hashicorp/nomad/acl" "github.com/hashicorp/nomad/nomad/structs" ) +const ( + // policyCacheSize is the number of ACL policies to keep cached. Policies have a fetching cost + // so we keep the hot policies cached to reduce the ACL token resolution time. + policyCacheSize = 64 + + // aclCacheSize is the number of ACL objects to keep cached. ACLs have a parsing and + // construction cost, so we keep the hot objects cached to reduce the ACL token resolution time. + aclCacheSize = 64 + + // tokenCacheSize is the number of ACL tokens to keep cached. Tokens have a fetching cost, + // so we keep the hot tokens cached to reduce the lookups. + tokenCacheSize = 64 +) + +// clientACLResolver holds the state required for client resolution +// of ACLs +type clientACLResolver struct { + // aclCache is used to maintain the parsed ACL objects + aclCache *lru.TwoQueueCache + + // policyCache is used to maintain the fetched policy objects + policyCache *lru.TwoQueueCache + + // tokenCache is used to maintain the fetched token objects + tokenCache *lru.TwoQueueCache +} + +// init is used to setup the client resolver state +func (c *clientACLResolver) init() error { + // Create the ACL object cache + var err error + c.aclCache, err = lru.New2Q(aclCacheSize) + if err != nil { + return err + } + c.policyCache, err = lru.New2Q(policyCacheSize) + if err != nil { + return err + } + c.tokenCache, err = lru.New2Q(tokenCacheSize) + if err != nil { + return err + } + return nil +} + // cachedACLValue is used to manage ACL Token or Policy TTLs type cachedACLValue struct { Token *structs.ACLToken @@ -17,17 +64,17 @@ type cachedACLValue struct { // Age is the time since the token was cached func (c *cachedACLValue) Age() time.Duration { - return time.Now().Sub(c.CacheTime) + return time.Since(c.CacheTime) } -// resolveToken is used to translate an ACL Token Secret ID into +// ResolveToken is used to translate an ACL Token Secret ID into // an ACL object, nil if ACLs are disabled, or an error. -func (c *Client) resolveToken(secretID string) (*acl.ACL, error) { +func (c *Client) ResolveToken(secretID string) (*acl.ACL, error) { // Fast-path if ACLs are disabled if !c.config.ACLEnabled { return nil, nil } - defer metrics.MeasureSince([]string{"client", "acl", "resolveToken"}, time.Now()) + defer metrics.MeasureSince([]string{"client", "acl", "resolve_token"}, time.Now()) // Resolve the token value token, err := c.resolveTokenValue(secretID) diff --git a/client/acl_test.go b/client/acl_test.go index af0f5a63d..a98c532a2 100644 --- a/client/acl_test.go +++ b/client/acl_test.go @@ -98,7 +98,7 @@ func TestClient_ACL_resolvePolicies(t *testing.T) { } } -func TestClient_ACL_resolveToken_Disabled(t *testing.T) { +func TestClient_ACL_ResolveToken_Disabled(t *testing.T) { s1, _ := testServer(t, nil) defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) @@ -109,12 +109,12 @@ func TestClient_ACL_resolveToken_Disabled(t *testing.T) { defer c1.Shutdown() // Should always get nil when disabled - aclObj, err := c1.resolveToken("blah") + aclObj, err := c1.ResolveToken("blah") assert.Nil(t, err) assert.Nil(t, aclObj) } -func TestClient_ACL_resolveToken(t *testing.T) { +func TestClient_ACL_ResolveToken(t *testing.T) { s1, _ := testServer(t, nil) defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) @@ -139,26 +139,26 @@ func TestClient_ACL_resolveToken(t *testing.T) { assert.Nil(t, err) // Test the client resolution - out, err := c1.resolveToken(token.SecretID) + out, err := c1.ResolveToken(token.SecretID) assert.Nil(t, err) assert.NotNil(t, out) // Test caching - out2, err := c1.resolveToken(token.SecretID) + out2, err := c1.ResolveToken(token.SecretID) assert.Nil(t, err) if out != out2 { t.Fatalf("should be cached") } // Test management token - out3, err := c1.resolveToken(token2.SecretID) + out3, err := c1.ResolveToken(token2.SecretID) assert.Nil(t, err) if acl.ManagementACL != out3 { t.Fatalf("should be management") } // Test bad token - out4, err := c1.resolveToken(structs.GenerateUUID()) + out4, err := c1.ResolveToken(structs.GenerateUUID()) assert.Equal(t, structs.TokenNotFound, err) assert.Nil(t, out4) } diff --git a/client/client.go b/client/client.go index b30fdcca8..c5ed19097 100644 --- a/client/client.go +++ b/client/client.go @@ -18,7 +18,6 @@ import ( consulapi "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/lib" "github.com/hashicorp/go-multierror" - lru "github.com/hashicorp/golang-lru" "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/driver" @@ -78,18 +77,6 @@ const ( // allocSyncRetryIntv is the interval on which we retry updating // the status of the allocation allocSyncRetryIntv = 5 * time.Second - - // policyCacheSize is the number of ACL policies to keep cached. Policies have a fetching cost - // so we keep the hot policies cached to reduce the ACL token resolution time. - policyCacheSize = 64 - - // aclCacheSize is the number of ACL objects to keep cached. ACLs have a parsing and - // construction cost, so we keep the hot objects cached to reduce the ACL token resolution time. - aclCacheSize = 64 - - // tokenCacheSize is the number of ACL tokens to keep cached. Tokens have a fetching cost, - // so we keep the hot tokens cached to reduce the lookups. - tokenCacheSize = 64 ) // ClientStatsReporter exposes all the APIs related to resource usage of a Nomad @@ -164,14 +151,8 @@ type Client struct { // in the node automatically garbageCollector *AllocGarbageCollector - // aclCache is used to maintain the parsed ACL objects - aclCache *lru.TwoQueueCache - - // policyCache is used to maintain the fetched policy objects - policyCache *lru.TwoQueueCache - - // tokenCache is used to maintain the fetched token objects - tokenCache *lru.TwoQueueCache + // clientACLResolver holds the ACL resolution state + clientACLResolver } var ( @@ -192,19 +173,6 @@ func NewClient(cfg *config.Config, consulCatalog consul.CatalogAPI, consulServic } tlsWrap = tw } - // Create the ACL object cache - aclCache, err := lru.New2Q(aclCacheSize) - if err != nil { - return nil, err - } - policyCache, err := lru.New2Q(policyCacheSize) - if err != nil { - return nil, err - } - tokenCache, err := lru.New2Q(tokenCacheSize) - if err != nil { - return nil, err - } // Create the client c := &Client{ @@ -220,9 +188,6 @@ func NewClient(cfg *config.Config, consulCatalog consul.CatalogAPI, consulServic servers: newServerList(), triggerDiscoveryCh: make(chan struct{}), serversDiscoveredCh: make(chan struct{}), - aclCache: aclCache, - policyCache: policyCache, - tokenCache: tokenCache, } // Initialize the client @@ -230,6 +195,11 @@ func NewClient(cfg *config.Config, consulCatalog consul.CatalogAPI, consulServic return nil, fmt.Errorf("failed to initialize client: %v", err) } + // Initialize the ACL state + if err := c.clientACLResolver.init(); err != nil { + return nil, fmt.Errorf("failed to initialize ACL state: %v", err) + } + // Add the stats collector statsCollector := stats.NewHostStatsCollector(logger, c.config.AllocDir) c.hostStatsCollector = statsCollector diff --git a/nomad/structs/funcs.go b/nomad/structs/funcs.go index 4db0092b7..b0fe90080 100644 --- a/nomad/structs/funcs.go +++ b/nomad/structs/funcs.go @@ -279,7 +279,9 @@ func ACLPolicyListHash(policies []*ACLPolicy) string { // CompileACLObject compiles a set of ACL policies into an ACL object with a cache func CompileACLObject(cache *lru.TwoQueueCache, policies []*ACLPolicy) (*acl.ACL, error) { // Sort the policies to ensure consistent ordering - sort.Sort(ACLPolicyList(policies)) + sort.Slice(policies, func(i, j int) bool { + return policies[i].Name < policies[j].Name + }) // Determine the cache key cacheKey := ACLPolicyListHash(policies) @@ -308,18 +310,3 @@ func CompileACLObject(cache *lru.TwoQueueCache, policies []*ACLPolicy) (*acl.ACL cache.Add(cacheKey, aclObj) return aclObj, nil } - -// ACLPolicyList is used to sort a set of policies by name -type ACLPolicyList []*ACLPolicy - -func (l ACLPolicyList) Len() int { - return len(l) -} - -func (l ACLPolicyList) Swap(i, j int) { - l[i], l[j] = l[j], l[i] -} - -func (l ACLPolicyList) Less(i, j int) bool { - return l[i].Name < l[j].Name -}