acl: make ACLDisabledTTL a constant

This field was never user-configurable. We always overwrote the value with 120s from
NonUserSource. However, we also never copied the value from RuntimeConfig to consul.Config,
So the value in NonUserSource was always ignored, and we used the default value of 30s
set by consul.DefaultConfig.

All of this code is an unnecessary distraction because a user can not actually configure
this value.

This commit removes the fields and uses a constant value instad. Someone attempting to set
acl.disabled_ttl in their config will now get an error about an unknown field, but previously
the value was completely ignored, so the new behaviour seems more correct.

We have to keep this field in the AutoConfig response for backwards compatibility, but the value
will be ignored by the client, so it doesn't really matter what value we set.
This commit is contained in:
Daniel Nephin 2021-08-09 16:04:23 -04:00
parent abd2e160f9
commit d5498770fa
13 changed files with 26 additions and 44 deletions

View File

@ -3,13 +3,14 @@ package autoconf
import (
"fmt"
"github.com/mitchellh/mapstructure"
"github.com/hashicorp/consul/agent/config"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/proto"
"github.com/hashicorp/consul/proto/pbautoconf"
"github.com/hashicorp/consul/proto/pbconfig"
"github.com/hashicorp/consul/proto/pbconnect"
"github.com/mitchellh/mapstructure"
)
// translateAgentConfig is meant to take in a proto/pbconfig.Config type
@ -48,7 +49,6 @@ func translateConfig(c *pbconfig.Config) config.Config {
DownPolicy: stringPtrOrNil(a.DownPolicy),
DefaultPolicy: stringPtrOrNil(a.DefaultPolicy),
EnableKeyListPolicy: &a.EnableKeyListPolicy,
DisabledTTL: stringPtrOrNil(a.DisabledTTL),
EnableTokenPersistence: &a.EnableTokenPersistence,
}

View File

@ -4,11 +4,12 @@ import (
"fmt"
"testing"
"github.com/stretchr/testify/require"
"github.com/hashicorp/consul/agent/config"
"github.com/hashicorp/consul/agent/structs"
pbconfig "github.com/hashicorp/consul/proto/pbconfig"
"github.com/hashicorp/consul/proto/pbconnect"
"github.com/stretchr/testify/require"
)
func stringPointer(s string) *string {
@ -65,7 +66,6 @@ func TestTranslateConfig(t *testing.T) {
DownPolicy: "deny",
DefaultPolicy: "deny",
EnableKeyListPolicy: true,
DisabledTTL: "4s",
EnableTokenPersistence: true,
MSPDisableBootstrap: false,
Tokens: &pbconfig.ACLTokens{
@ -127,7 +127,6 @@ func TestTranslateConfig(t *testing.T) {
DownPolicy: stringPointer("deny"),
DefaultPolicy: stringPointer("deny"),
EnableKeyListPolicy: boolPointer(true),
DisabledTTL: stringPointer("4s"),
EnableTokenPersistence: boolPointer(true),
Tokens: config.Tokens{
Master: stringPointer("99e7e490-6baf-43fc-9010-78b6aa9a6813"),

View File

@ -873,7 +873,6 @@ func (b *builder) build() (rt RuntimeConfig, err error) {
ACLPolicyTTL: b.durationVal("acl.policy_ttl", c.ACL.PolicyTTL),
ACLTokenTTL: b.durationValWithDefault("acl.token_ttl", c.ACL.TokenTTL, b.durationVal("acl_ttl", c.ACLTTL)),
ACLRoleTTL: b.durationVal("acl.role_ttl", c.ACL.RoleTTL),
ACLDisabledTTL: b.durationVal("acl.disabled_ttl", c.ACL.DisabledTTL),
ACLDownPolicy: stringValWithDefault(c.ACL.DownPolicy, stringVal(c.ACLDownPolicy)),
ACLDefaultPolicy: stringValWithDefault(c.ACL.DefaultPolicy, stringVal(c.ACLDefaultPolicy)),
},

View File

@ -268,8 +268,6 @@ type Config struct {
SnapshotAgent map[string]interface{} `mapstructure:"snapshot_agent"`
// non-user configurable values
// DEPRECATED (ACL-Legacy-Compat) - moved into the "acl" stanza
ACLDisabledTTL *string `mapstructure:"acl_disabled_ttl"`
AEInterval *string `mapstructure:"ae_interval"`
CheckDeregisterIntervalMin *string `mapstructure:"check_deregister_interval_min"`
CheckReapInterval *string `mapstructure:"check_reap_interval"`
@ -741,7 +739,6 @@ type ACL struct {
DefaultPolicy *string `mapstructure:"default_policy"`
EnableKeyListPolicy *bool `mapstructure:"enable_key_list_policy"`
Tokens Tokens `mapstructure:"tokens"`
DisabledTTL *string `mapstructure:"disabled_ttl"`
EnableTokenPersistence *bool `mapstructure:"enable_token_persistence"`
// Enterprise Only

View File

@ -184,9 +184,6 @@ func NonUserSource() Source {
Name: "non-user",
Format: "hcl",
Data: `
acl = {
disabled_ttl = "120s"
}
check_deregister_interval_min = "1m"
check_reap_interval = "30s"
ae_interval = "1m"

View File

@ -5241,7 +5241,6 @@ func TestLoad_FullConfig(t *testing.T) {
ACLsEnabled: true,
Datacenter: "rzo029wg",
NodeName: "otlLxGaI",
ACLDisabledTTL: 120 * time.Second,
ACLDefaultPolicy: "72c2e7a0",
ACLDownPolicy: "03eb2aee",
ACLTokenTTL: 3321 * time.Second,

View File

@ -3,7 +3,6 @@
"ACLMasterToken": "hidden",
"ACLResolverSettings": {
"ACLDefaultPolicy": "",
"ACLDisabledTTL": "0s",
"ACLDownPolicy": "",
"ACLPolicyTTL": "0s",
"ACLRoleTTL": "0s",

View File

@ -212,6 +212,8 @@ type ACLResolverConfig struct {
Tokens *token.Store
}
const aclDisabledTTL = 30 * time.Second
// TODO: rename the fields to remove the ACL prefix
type ACLResolverSettings struct {
ACLsEnabled bool
@ -228,11 +230,6 @@ type ACLResolverSettings struct {
// a major impact on performance. By default, it is set to 30 seconds.
ACLRoleTTL time.Duration
// ACLDisabledTTL is used by agents to determine how long they will
// wait to check again with the servers if they discover ACLs are not
// enabled. (not user configurable)
ACLDisabledTTL time.Duration
// ACLDownPolicy is used to control the ACL interaction when we cannot
// reach the PrimaryDatacenter and the token is not in the cache.
// There are the following modes:
@ -1200,9 +1197,9 @@ func (r *ACLResolver) disableACLsWhenUpstreamDisabled(err error) error {
return err
}
r.logger.Debug("ACLs disabled on upstream servers, will retry", "retry_interval", r.config.ACLDisabledTTL)
r.logger.Debug("ACLs disabled on upstream servers, will retry", "retry_interval", aclDisabledTTL)
r.disabledLock.Lock()
r.disabled = time.Now().Add(r.config.ACLDisabledTTL)
r.disabled = time.Now().Add(aclDisabledTTL)
r.disabledLock.Unlock()
return err

View File

@ -191,7 +191,6 @@ func (ac *AutoConfig) updateACLsInConfig(opts AutoConfigOptions, resp *pbautocon
PolicyTTL: ac.config.ACLResolverSettings.ACLPolicyTTL.String(),
RoleTTL: ac.config.ACLResolverSettings.ACLRoleTTL.String(),
TokenTTL: ac.config.ACLResolverSettings.ACLTokenTTL.String(),
DisabledTTL: ac.config.ACLResolverSettings.ACLDisabledTTL.String(),
DownPolicy: ac.config.ACLResolverSettings.ACLDownPolicy,
DefaultPolicy: ac.config.ACLResolverSettings.ACLDefaultPolicy,
EnableKeyListPolicy: ac.config.ACLEnableKeyListPolicy,

View File

@ -153,8 +153,6 @@ func TestAutoConfigInitialConfiguration(t *testing.T) {
}
c.AutoConfigAuthzAllowReuse = true
c.ACLResolverSettings.ACLDisabledTTL = 12 * time.Second
cafile := path.Join(c.DataDir, "cacert.pem")
err := ioutil.WriteFile(cafile, []byte(cacert), 0600)
require.NoError(t, err)
@ -265,7 +263,6 @@ func TestAutoConfigInitialConfiguration(t *testing.T) {
PolicyTTL: "30s",
TokenTTL: "30s",
RoleTTL: "30s",
DisabledTTL: "12s",
DownPolicy: "extend-cache",
DefaultPolicy: "deny",
Tokens: &pbconfig.ACLTokens{
@ -725,7 +722,6 @@ func TestAutoConfig_updateACLsInConfig(t *testing.T) {
ACLPolicyTTL: 7 * time.Second,
ACLRoleTTL: 10 * time.Second,
ACLTokenTTL: 12 * time.Second,
ACLDisabledTTL: 31 * time.Second,
ACLDefaultPolicy: "allow",
ACLDownPolicy: "deny",
},
@ -739,7 +735,6 @@ func TestAutoConfig_updateACLsInConfig(t *testing.T) {
PolicyTTL: "7s",
RoleTTL: "10s",
TokenTTL: "12s",
DisabledTTL: "31s",
DownPolicy: "deny",
DefaultPolicy: "allow",
EnableKeyListPolicy: true,
@ -759,7 +754,6 @@ func TestAutoConfig_updateACLsInConfig(t *testing.T) {
ACLPolicyTTL: 7 * time.Second,
ACLRoleTTL: 10 * time.Second,
ACLTokenTTL: 12 * time.Second,
ACLDisabledTTL: 31 * time.Second,
ACLDefaultPolicy: "allow",
ACLDownPolicy: "deny",
},
@ -773,7 +767,6 @@ func TestAutoConfig_updateACLsInConfig(t *testing.T) {
PolicyTTL: "7s",
RoleTTL: "10s",
TokenTTL: "12s",
DisabledTTL: "31s",
DownPolicy: "deny",
DefaultPolicy: "allow",
EnableKeyListPolicy: true,

View File

@ -450,7 +450,6 @@ func DefaultConfig() *Config {
ACLPolicyTTL: 30 * time.Second,
ACLTokenTTL: 30 * time.Second,
ACLRoleTTL: 30 * time.Second,
ACLDisabledTTL: 30 * time.Second,
ACLDownPolicy: "extend-cache",
ACLDefaultPolicy: "allow",
},

View File

@ -323,20 +323,22 @@ func (m *TLS) GetPreferServerCipherSuites() bool {
}
type ACL struct {
Enabled bool `protobuf:"varint,1,opt,name=Enabled,proto3" json:"Enabled,omitempty"`
PolicyTTL string `protobuf:"bytes,2,opt,name=PolicyTTL,proto3" json:"PolicyTTL,omitempty"`
RoleTTL string `protobuf:"bytes,3,opt,name=RoleTTL,proto3" json:"RoleTTL,omitempty"`
TokenTTL string `protobuf:"bytes,4,opt,name=TokenTTL,proto3" json:"TokenTTL,omitempty"`
DownPolicy string `protobuf:"bytes,5,opt,name=DownPolicy,proto3" json:"DownPolicy,omitempty"`
DefaultPolicy string `protobuf:"bytes,6,opt,name=DefaultPolicy,proto3" json:"DefaultPolicy,omitempty"`
EnableKeyListPolicy bool `protobuf:"varint,7,opt,name=EnableKeyListPolicy,proto3" json:"EnableKeyListPolicy,omitempty"`
Tokens *ACLTokens `protobuf:"bytes,8,opt,name=Tokens,proto3" json:"Tokens,omitempty"`
DisabledTTL string `protobuf:"bytes,9,opt,name=DisabledTTL,proto3" json:"DisabledTTL,omitempty"`
EnableTokenPersistence bool `protobuf:"varint,10,opt,name=EnableTokenPersistence,proto3" json:"EnableTokenPersistence,omitempty"`
MSPDisableBootstrap bool `protobuf:"varint,11,opt,name=MSPDisableBootstrap,proto3" json:"MSPDisableBootstrap,omitempty"`
XXX_NoUnkeyedLiteral struct{} `json:"-"`
XXX_unrecognized []byte `json:"-"`
XXX_sizecache int32 `json:"-"`
Enabled bool `protobuf:"varint,1,opt,name=Enabled,proto3" json:"Enabled,omitempty"`
PolicyTTL string `protobuf:"bytes,2,opt,name=PolicyTTL,proto3" json:"PolicyTTL,omitempty"`
RoleTTL string `protobuf:"bytes,3,opt,name=RoleTTL,proto3" json:"RoleTTL,omitempty"`
TokenTTL string `protobuf:"bytes,4,opt,name=TokenTTL,proto3" json:"TokenTTL,omitempty"`
DownPolicy string `protobuf:"bytes,5,opt,name=DownPolicy,proto3" json:"DownPolicy,omitempty"`
DefaultPolicy string `protobuf:"bytes,6,opt,name=DefaultPolicy,proto3" json:"DefaultPolicy,omitempty"`
EnableKeyListPolicy bool `protobuf:"varint,7,opt,name=EnableKeyListPolicy,proto3" json:"EnableKeyListPolicy,omitempty"`
Tokens *ACLTokens `protobuf:"bytes,8,opt,name=Tokens,proto3" json:"Tokens,omitempty"`
// DisabledTTL is deprecated. It is no longer populated and should be ignored
// by clients.
DisabledTTL string `protobuf:"bytes,9,opt,name=DisabledTTL,proto3" json:"DisabledTTL,omitempty"`
EnableTokenPersistence bool `protobuf:"varint,10,opt,name=EnableTokenPersistence,proto3" json:"EnableTokenPersistence,omitempty"`
MSPDisableBootstrap bool `protobuf:"varint,11,opt,name=MSPDisableBootstrap,proto3" json:"MSPDisableBootstrap,omitempty"`
XXX_NoUnkeyedLiteral struct{} `json:"-"`
XXX_unrecognized []byte `json:"-"`
XXX_sizecache int32 `json:"-"`
}
func (m *ACL) Reset() { *m = ACL{} }

View File

@ -43,6 +43,8 @@ message ACL {
string DefaultPolicy = 6;
bool EnableKeyListPolicy = 7;
ACLTokens Tokens = 8;
// DisabledTTL is deprecated. It is no longer populated and should be ignored
// by clients.
string DisabledTTL = 9;
bool EnableTokenPersistence = 10;
bool MSPDisableBootstrap = 11;