luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

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 a8bc964241
commit 09ae0ab94a
13 changed files with 26 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
agent/auto-config/config_translate.go
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,
}

5
agent/auto-config/config_translate_test.go
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"),

1
agent/config/builder.go
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)),
},

3
agent/config/config.go
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

3
agent/config/default.go
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"

1
agent/config/runtime_test.go
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,

1
agent/config/testdata/TestRuntimeConfig_Sanitize.golden vendored
View File

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

11
agent/consul/acl.go
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

1
agent/consul/auto_config_endpoint.go
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,

7
agent/consul/auto_config_endpoint_test.go
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,

1
agent/consul/config.go
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",
},

30
proto/pbconfig/config.pb.go
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{} }

2
proto/pbconfig/config.proto
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;