config: replace calls to config.NewBuilder with config.Load

This is another incremental change to reduce config loading to a single
small interface. All calls to NewBuilder can be replaced with Load.
This commit is contained in:
Daniel Nephin 2020-12-21 13:46:41 -05:00
parent bd545963cc
commit 08b2039f5a
10 changed files with 86 additions and 110 deletions

View File

@ -1,42 +0,0 @@
package config
import (
"testing"
"github.com/stretchr/testify/assert"
)
func TestBuildAndValidate_HTTPMaxConnsPerClientExceedsRLimit(t *testing.T) {
hcl := `
limits{
# We put a very high value to be sure to fail
# This value is more than max on Windows as well
http_max_conns_per_client = 16777217
}`
b, err := NewBuilder(LoadOpts{})
assert.NoError(t, err)
testsrc := FileSource{
Name: "test",
Format: "hcl",
Data: `
ae_interval = "1m"
data_dir="/tmp/00000000001979"
bind_addr = "127.0.0.1"
advertise_addr = "127.0.0.1"
datacenter = "dc1"
bootstrap = true
server = true
node_id = "00000000001979"
node_name = "Node-00000000001979"
`,
}
b.Head = append(b.Head, testsrc)
b.Tail = append(b.Tail, DefaultConsulSource(), DevConsulSource())
b.Tail = append(b.Head, FileSource{Name: "hcl", Format: "hcl", Data: hcl})
_, validationError := b.BuildAndValidate()
if validationError == nil {
assert.Fail(t, "Config should not be valid")
}
assert.Contains(t, validationError.Error(), "but limits.http_max_conns_per_client: 16777217 needs at least 16777237")
}

View File

@ -10,6 +10,7 @@ import (
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@ -185,3 +186,35 @@ func patchBuilderShims(b *Builder) {
return []*net.IPAddr{ipAddr("dead:beef::1")}, nil
}
}
func TestLoad_HTTPMaxConnsPerClientExceedsRLimit(t *testing.T) {
hcl := `
limits{
# We put a very high value to be sure to fail
# This value is more than max on Windows as well
http_max_conns_per_client = 16777217
}`
opts := LoadOpts{
DefaultConfig: FileSource{
Name: "test",
Format: "hcl",
Data: `
ae_interval = "1m"
data_dir="/tmp/00000000001979"
bind_addr = "127.0.0.1"
advertise_addr = "127.0.0.1"
datacenter = "dc1"
bootstrap = true
server = true
node_id = "00000000001979"
node_name = "Node-00000000001979"
`,
},
HCL: []string{hcl},
}
_, err := Load(opts)
require.Error(t, err)
assert.Contains(t, err.Error(), "but limits.http_max_conns_per_client: 16777217 needs at least 16777237")
}

View File

@ -269,15 +269,3 @@ func DevConsulSource() Source {
func strPtr(v string) *string {
return &v
}
func DefaultRuntimeConfig(hcl string) *RuntimeConfig {
b, err := NewBuilder(LoadOpts{HCL: []string{hcl}})
if err != nil {
panic(err)
}
rt, err := b.BuildAndValidate()
if err != nil {
panic(err)
}
return &rt
}

View File

@ -4996,8 +4996,8 @@ func assertDeepEqual(t *testing.T, x, y interface{}, opts ...cmp.Option) {
}
}
func TestNewBuilder_InvalidConfigFormat(t *testing.T) {
_, err := NewBuilder(LoadOpts{ConfigFormat: "yaml"})
func TestLoad_InvalidConfigFormat(t *testing.T) {
_, err := Load(LoadOpts{ConfigFormat: "yaml"})
require.Error(t, err)
require.Contains(t, err.Error(), "-config-format must be either 'hcl' or 'json'")
}

View File

@ -10,6 +10,10 @@ import (
"testing"
"time"
"github.com/hashicorp/serf/coordinate"
"github.com/miekg/dns"
"github.com/stretchr/testify/require"
"github.com/hashicorp/consul/agent/config"
agentdns "github.com/hashicorp/consul/agent/dns"
"github.com/hashicorp/consul/agent/structs"
@ -17,9 +21,6 @@ import (
"github.com/hashicorp/consul/lib"
"github.com/hashicorp/consul/sdk/testutil/retry"
"github.com/hashicorp/consul/testrpc"
"github.com/hashicorp/serf/coordinate"
"github.com/miekg/dns"
"github.com/stretchr/testify/require"
)
const (
@ -6664,7 +6665,7 @@ func TestDNS_trimUDPResponse_NoTrim(t *testing.T) {
},
}
cfg := config.DefaultRuntimeConfig(`data_dir = "a" bind_addr = "127.0.0.1"`)
cfg := loadRuntimeConfig(t, `data_dir = "a" bind_addr = "127.0.0.1"`)
if trimmed := trimUDPResponse(req, resp, cfg.DNSUDPAnswerLimit); trimmed {
t.Fatalf("Bad %#v", *resp)
}
@ -6698,7 +6699,7 @@ func TestDNS_trimUDPResponse_NoTrim(t *testing.T) {
func TestDNS_trimUDPResponse_TrimLimit(t *testing.T) {
t.Parallel()
cfg := config.DefaultRuntimeConfig(`data_dir = "a" bind_addr = "127.0.0.1"`)
cfg := loadRuntimeConfig(t, `data_dir = "a" bind_addr = "127.0.0.1"`)
req, resp, expected := &dns.Msg{}, &dns.Msg{}, &dns.Msg{}
for i := 0; i < cfg.DNSUDPAnswerLimit+1; i++ {
@ -6736,9 +6737,17 @@ func TestDNS_trimUDPResponse_TrimLimit(t *testing.T) {
}
}
func loadRuntimeConfig(t *testing.T, hcl string) *config.RuntimeConfig {
t.Helper()
result, err := config.Load(config.LoadOpts{HCL: []string{hcl}})
require.NoError(t, err)
require.Len(t, result.Warnings, 0)
return result.RuntimeConfig
}
func TestDNS_trimUDPResponse_TrimSize(t *testing.T) {
t.Parallel()
cfg := config.DefaultRuntimeConfig(`data_dir = "a" bind_addr = "127.0.0.1"`)
cfg := loadRuntimeConfig(t, `data_dir = "a" bind_addr = "127.0.0.1"`)
req, resp := &dns.Msg{}, &dns.Msg{}
for i := 0; i < 100; i++ {
@ -6791,7 +6800,7 @@ func TestDNS_trimUDPResponse_TrimSize(t *testing.T) {
func TestDNS_trimUDPResponse_TrimSizeEDNS(t *testing.T) {
t.Parallel()
cfg := config.DefaultRuntimeConfig(`data_dir = "a" bind_addr = "127.0.0.1"`)
cfg := loadRuntimeConfig(t, `data_dir = "a" bind_addr = "127.0.0.1"`)
req, resp := &dns.Msg{}, &dns.Msg{}
@ -7094,7 +7103,7 @@ func TestDNS_syncExtra(t *testing.T) {
func TestDNS_Compression_trimUDPResponse(t *testing.T) {
t.Parallel()
cfg := config.DefaultRuntimeConfig(`data_dir = "a" bind_addr = "127.0.0.1"`)
cfg := loadRuntimeConfig(t, `data_dir = "a" bind_addr = "127.0.0.1"`)
req, m := dns.Msg{}, dns.Msg{}
trimUDPResponse(&req, &m, cfg.DNSUDPAnswerLimit)

View File

@ -1773,7 +1773,7 @@ func TestAgent_ServiceTokens(t *testing.T) {
tokens := new(token.Store)
tokens.UpdateUserToken("default", token.TokenSourceConfig)
cfg := config.DefaultRuntimeConfig(`bind_addr = "127.0.0.1" data_dir = "dummy"`)
cfg := loadRuntimeConfig(t, `bind_addr = "127.0.0.1" data_dir = "dummy"`)
l := local.NewState(agent.LocalConfig(cfg), nil, tokens)
l.TriggerSyncChanges = func() {}
@ -1797,12 +1797,20 @@ func TestAgent_ServiceTokens(t *testing.T) {
}
}
func loadRuntimeConfig(t *testing.T, hcl string) *config.RuntimeConfig {
t.Helper()
result, err := config.Load(config.LoadOpts{HCL: []string{hcl}})
require.NoError(t, err)
require.Len(t, result.Warnings, 0)
return result.RuntimeConfig
}
func TestAgent_CheckTokens(t *testing.T) {
t.Parallel()
tokens := new(token.Store)
tokens.UpdateUserToken("default", token.TokenSourceConfig)
cfg := config.DefaultRuntimeConfig(`bind_addr = "127.0.0.1" data_dir = "dummy"`)
cfg := loadRuntimeConfig(t, `bind_addr = "127.0.0.1" data_dir = "dummy"`)
l := local.NewState(agent.LocalConfig(cfg), nil, tokens)
l.TriggerSyncChanges = func() {}
@ -1827,7 +1835,7 @@ func TestAgent_CheckTokens(t *testing.T) {
func TestAgent_CheckCriticalTime(t *testing.T) {
t.Parallel()
cfg := config.DefaultRuntimeConfig(`bind_addr = "127.0.0.1" data_dir = "dummy"`)
cfg := loadRuntimeConfig(t, `bind_addr = "127.0.0.1" data_dir = "dummy"`)
l := local.NewState(agent.LocalConfig(cfg), nil, new(token.Store))
l.TriggerSyncChanges = func() {}
@ -1891,7 +1899,7 @@ func TestAgent_CheckCriticalTime(t *testing.T) {
func TestAgent_AddCheckFailure(t *testing.T) {
t.Parallel()
cfg := config.DefaultRuntimeConfig(`bind_addr = "127.0.0.1" data_dir = "dummy"`)
cfg := loadRuntimeConfig(t, `bind_addr = "127.0.0.1" data_dir = "dummy"`)
l := local.NewState(agent.LocalConfig(cfg), nil, new(token.Store))
l.TriggerSyncChanges = func() {}
@ -1914,7 +1922,7 @@ func TestAgent_AliasCheck(t *testing.T) {
t.Parallel()
require := require.New(t)
cfg := config.DefaultRuntimeConfig(`bind_addr = "127.0.0.1" data_dir = "dummy"`)
cfg := loadRuntimeConfig(t, `bind_addr = "127.0.0.1" data_dir = "dummy"`)
l := local.NewState(agent.LocalConfig(cfg), nil, new(token.Store))
l.TriggerSyncChanges = func() {}
@ -1965,7 +1973,7 @@ func TestAgent_AliasCheck_ServiceNotification(t *testing.T) {
t.Parallel()
require := require.New(t)
cfg := config.DefaultRuntimeConfig(`bind_addr = "127.0.0.1" data_dir = "dummy"`)
cfg := loadRuntimeConfig(t, `bind_addr = "127.0.0.1" data_dir = "dummy"`)
l := local.NewState(agent.LocalConfig(cfg), nil, new(token.Store))
l.TriggerSyncChanges = func() {}

View File

@ -410,8 +410,7 @@ func NodeID() string {
return id
}
// TestConfig returns a unique default configuration for testing an
// agent.
// TestConfig returns a unique default configuration for testing an agent.
func TestConfig(logger hclog.Logger, sources ...config.Source) *config.RuntimeConfig {
nodeID := NodeID()
testsrc := config.FileSource{
@ -437,29 +436,25 @@ func TestConfig(logger hclog.Logger, sources ...config.Source) *config.RuntimeCo
`,
}
b, err := config.NewBuilder(config.LoadOpts{})
if err != nil {
panic("NewBuilder failed: " + err.Error())
opts := config.LoadOpts{
DefaultConfig: testsrc,
Overrides: sources,
}
b.Head = append(b.Head, testsrc)
b.Tail = append(b.Tail, config.DefaultConsulSource(), config.DevConsulSource())
b.Tail = append(b.Tail, sources...)
cfg, err := b.BuildAndValidate()
r, err := config.Load(opts)
if err != nil {
panic("Error building config: " + err.Error())
panic("config.Load failed: " + err.Error())
}
for _, w := range b.Warnings {
for _, w := range r.Warnings {
logger.Warn(w)
}
cfg := r.RuntimeConfig
// Effectively disables the delay after root rotation before requesting CSRs
// to make test deterministic. 0 results in default jitter being applied but a
// tiny delay is effectively thre same.
cfg.ConnectTestCALeafRootChangeSpread = 1 * time.Nanosecond
return &cfg
return cfg
}
// TestACLConfig returns a default configuration for testing an agent

View File

@ -19,26 +19,22 @@ func ServicesFromFiles(ui cli.Ui, files []string) ([]*api.AgentServiceRegistrati
// configuration. devMode doesn't set any services by default so this
// is okay since we only look at services.
devMode := true
b, err := config.NewBuilder(config.LoadOpts{
r, err := config.Load(config.LoadOpts{
ConfigFiles: files,
DevMode: &devMode,
})
if err != nil {
return nil, err
}
cfg, err := b.BuildAndValidate()
if err != nil {
return nil, err
}
for _, w := range b.Warnings {
for _, w := range r.Warnings {
ui.Warn(w)
}
// The services are now in "structs.ServiceDefinition" form and we need
// them in "api.AgentServiceRegistration" form so do the conversion.
result := make([]*api.AgentServiceRegistration, 0, len(cfg.Services))
for _, svc := range cfg.Services {
services := r.RuntimeConfig.Services
result := make([]*api.AgentServiceRegistration, 0, len(services))
for _, svc := range services {
apiSvc, err := serviceToAgentService(svc)
if err != nil {
return nil, err

View File

@ -15,18 +15,11 @@ import (
// We depend on this behavior for ServiesFromFiles so we want to fail
// tests if that ever changes.
func TestDevModeHasNoServices(t *testing.T) {
t.Parallel()
require := require.New(t)
devMode := true
b, err := config.NewBuilder(config.LoadOpts{
DevMode: &devMode,
})
require.NoError(err)
cfg, err := b.BuildAndValidate()
require.NoError(err)
require.Empty(cfg.Services)
result, err := config.Load(config.LoadOpts{DevMode: &devMode})
require.NoError(t, err)
require.Len(t, result.Warnings, 0)
require.Len(t, result.RuntimeConfig.Services, 0)
}
func TestStructsToAgentService(t *testing.T) {

View File

@ -52,17 +52,13 @@ func (c *cmd) Run(args []string) int {
return 1
}
b, err := config.NewBuilder(config.LoadOpts{ConfigFiles: configFiles, ConfigFormat: c.configFormat})
result, err := config.Load(config.LoadOpts{ConfigFiles: configFiles, ConfigFormat: c.configFormat})
if err != nil {
c.UI.Error(fmt.Sprintf("Config validation failed: %v", err.Error()))
return 1
}
if _, err := b.BuildAndValidate(); err != nil {
c.UI.Error(fmt.Sprintf("Config validation failed: %v", err.Error()))
return 1
}
if !c.quiet {
for _, w := range b.Warnings {
for _, w := range result.Warnings {
c.UI.Warn(w)
}
c.UI.Output("Configuration is valid!")