Address @dadgar feedback

command/agent/config-test-fixtures/basic.hcl

@@ -85,8 +85,8 @@ server {
 }
 acl {
     enabled = true
-    token_ttl = "30s"
-    policy_ttl = "30s"
+    token_ttl = "60s"
+    policy_ttl = "60s"
 }
 telemetry {
 	statsite_address = "127.0.0.1:1234"

command/agent/config.go

@@ -240,15 +240,13 @@ type ACLConfig struct {
 	// how stale they can be when we are enforcing policies. Defaults
 	// to "30s". Reducing this impacts performance by forcing more
 	// frequent resolution.
-	TokenTTL string        `mapstructure:"token_ttl"`
-	tokenTTL time.Duration `mapstructure:"-"`
+	TokenTTL time.Duration `mapstructure:"token_ttl"`
 
 	// PolicyTTL controls how long we cache ACL policies. This controls
 	// how stale they can be when we are enforcing policies. Defaults
 	// to "30s". Reducing this impacts performance by forcing more
 	// frequent resolution.
-	PolicyTTL string        `mapstructure:"policy_ttl"`
-	policyTTL time.Duration `mapstructure:"-"`
+	PolicyTTL time.Duration `mapstructure:"policy_ttl"`
 }
 
 // ServerConfig is configuration specific to the server mode
@@ -594,8 +592,8 @@ func DefaultConfig() *Config {
 		},
 		ACL: &ACLConfig{
 			Enabled:   false,
-			TokenTTL:  "30s",
-			PolicyTTL: "30s",
+			TokenTTL:  30 * time.Second,
+			PolicyTTL: 30 * time.Second,
 		},
 		SyslogFacility: "LOCAL0",
 		Telemetry: &Telemetry{
@@ -949,10 +947,10 @@ func (a *ACLConfig) Merge(b *ACLConfig) *ACLConfig {
 	if b.Enabled {
 		result.Enabled = true
 	}
-	if b.TokenTTL != "" {
+	if b.TokenTTL != 0 {
 		result.TokenTTL = b.TokenTTL
 	}
-	if b.PolicyTTL != "" {
+	if b.PolicyTTL != 0 {
 		result.PolicyTTL = b.PolicyTTL
 	}
 	return &result

command/agent/config_parse_test.go

@@ -106,8 +106,8 @@ func TestConfig_Parse(t *testing.T) {
 				},
 				ACL: &ACLConfig{
 					Enabled:   true,
-					TokenTTL:  "30s",
-					PolicyTTL: "30s",
+					TokenTTL:  60 * time.Second,
+					PolicyTTL: 60 * time.Second,
 				},
 				Telemetry: &Telemetry{
 					StatsiteAddr:             "127.0.0.1:1234",

nomad/leader.go

@@ -672,10 +672,13 @@ REMOVE:
 // replicateACLPolicies is used to replicate ACL policies from
 // the authoritative region to this region.
 func (s *Server) replicateACLPolicies(stopCh chan struct{}) {
-	req := structs.ACLPolicyListRequest{}
-	req.Region = s.config.AuthoritativeRegion
+	req := structs.ACLPolicyListRequest{
+		QueryOptions: structs.QueryOptions{
+			Region: s.config.AuthoritativeRegion,
+		},
+	}
 	limiter := rate.NewLimiter(replicationRateLimit, int(replicationRateLimit))
-	s.logger.Printf("[DEBUG] nomad: starting ACL policy replication from authoritative region '%s'", req.Region)
+	s.logger.Printf("[DEBUG] nomad: starting ACL policy replication from authoritative region %q", req.Region)
 
 START:
 	for {
@@ -711,6 +714,7 @@ START:
 			}
 
 			// Fetch any outdated policies
+			// TODO: Optimize this fetching to batch all the requests.
 			var fetched []*structs.ACLPolicy
 			for _, policyName := range update {
 				req := structs.ACLPolicySpecificRequest{
@@ -721,7 +725,7 @@ START:
 				err := s.forwardRegion(s.config.AuthoritativeRegion,
 					"ACL.GetPolicy", &req, &reply)
 				if err != nil {
-					s.logger.Printf("[ERR] nomad: failed to fetch policy '%s' from authoritative region: %v", policyName, err)
+					s.logger.Printf("[ERR] nomad: failed to fetch policy %q from authoritative region: %v", policyName, err)
 					goto ERR_WAIT
 				}
 				if reply.Policy != nil {
@@ -809,10 +813,12 @@ func diffACLPolicies(state *state.StateStore, minIndex uint64, remoteList []*str
 func (s *Server) replicateACLTokens(stopCh chan struct{}) {
 	req := structs.ACLTokenListRequest{
 		GlobalOnly: true,
+		QueryOptions: structs.QueryOptions{
+			Region: s.config.AuthoritativeRegion,
+		},
 	}
-	req.Region = s.config.AuthoritativeRegion
 	limiter := rate.NewLimiter(replicationRateLimit, int(replicationRateLimit))
-	s.logger.Printf("[DEBUG] nomad: starting ACL token replication from authoritative region '%s'", req.Region)
+	s.logger.Printf("[DEBUG] nomad: starting ACL token replication from authoritative region %q", req.Region)
 
 START:
 	for {
@@ -847,7 +853,8 @@ START:
 				}
 			}
 
-			// Fetch any outdated policies
+			// Fetch any outdated policies.
+			// TODO: Optimize this fetching to batch all the requests.
 			var fetched []*structs.ACLToken
 			for _, tokenID := range update {
 				req := structs.ACLTokenSpecificRequest{
@@ -858,7 +865,7 @@ START:
 				err := s.forwardRegion(s.config.AuthoritativeRegion,
 					"ACL.GetToken", &req, &reply)
 				if err != nil {
-					s.logger.Printf("[ERR] nomad: failed to fetch token '%s' from authoritative region: %v", tokenID, err)
+					s.logger.Printf("[ERR] nomad: failed to fetch token %q from authoritative region: %v", tokenID, err)
 					goto ERR_WAIT
 				}
 				if reply.Token != nil {
@@ -866,7 +873,7 @@ START:
 				}
 			}
 
-			// Update local tokensj
+			// Update local tokens
 			if len(fetched) > 0 {
 				args := &structs.ACLTokenUpsertRequest{
 					Tokens: fetched,