From a25b4d9d490437b4716cb6ef880818466606c4bc Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Tue, 11 Jun 2019 21:59:44 -0400 Subject: [PATCH 1/4] Revert "config explicitly merge defaults once when using a config directory" This reverts commit 006a9a1d454739eee21b7d8abb8b7aef1353b648. --- command/agent/config.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/command/agent/config.go b/command/agent/config.go index 974f8d29b36e..08783e0b4552 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -1479,14 +1479,8 @@ func LoadConfig(path string) (*Config, error) { return nil, err } - defaults := ParseConfigDefault() - if fi.IsDir() { - config, err := LoadConfigDir(path) - if err != nil { - return nil, err - } - return defaults.Merge(config), nil + return LoadConfigDir(path) } cleaned := filepath.Clean(path) @@ -1494,7 +1488,7 @@ func LoadConfig(path string) (*Config, error) { if err != nil { return nil, fmt.Errorf("Error loading %s: %s", cleaned, err) } - config = defaults.Merge(config) + config.Files = append(config.Files, cleaned) return config, nil } From cedd2ba17f5ec35e97dfe959725f7d6f238d31b0 Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Tue, 11 Jun 2019 21:58:19 -0400 Subject: [PATCH 2/4] config_parse get rid of ParseConfigDefault --- command/agent/config_parse.go | 9 ---- command/agent/config_parse_test.go | 82 ++++++++++++++++++++++++++---- 2 files changed, 72 insertions(+), 19 deletions(-) diff --git a/command/agent/config_parse.go b/command/agent/config_parse.go index 201508f6c2a3..225ca8942102 100644 --- a/command/agent/config_parse.go +++ b/command/agent/config_parse.go @@ -14,15 +14,6 @@ import ( "github.com/hashicorp/nomad/nomad/structs/config" ) -// ParseConfigDefault returns the configuration base -func ParseConfigDefault() *Config { - return &Config{ - Consul: config.DefaultConsulConfig(), - Autopilot: config.DefaultAutopilotConfig(), - Vault: config.DefaultVaultConfig(), - } -} - func ParseConfigFile(path string) (*Config, error) { // slurp var buf bytes.Buffer diff --git a/command/agent/config_parse_test.go b/command/agent/config_parse_test.go index aea94556a6c1..3266a958c752 100644 --- a/command/agent/config_parse_test.go +++ b/command/agent/config_parse_test.go @@ -411,8 +411,16 @@ func TestConfig_Parse(t *testing.T) { t.Fatalf("file: %s\n\n%s", tc.File, err) } - // Merge defaults like LoadConfig does - actual = ParseConfigDefault().Merge(actual) + // ParseConfig used to re-merge defaults for these three objects, + // despite them already being merged in LoadConfig. The test structs + // expect these defaults to be set, but not the DefaultConfig + // defaults, which include additional settings + oldDefault := &Config{ + Consul: config.DefaultConsulConfig(), + Vault: config.DefaultVaultConfig(), + Autopilot: config.DefaultAutopilotConfig(), + } + actual = oldDefault.Merge(actual) //panic(fmt.Sprintf("first: %+v \n second: %+v", actual.TLSConfig, tc.Result.TLSConfig)) require.EqualValues(tc.Result, removeHelperAttributes(actual)) @@ -576,15 +584,70 @@ func TestConfig_ParseSample0(t *testing.T) { require.EqualValues(t, sample0, c) } +var sample1 = &Config{ + Region: "global", + Datacenter: "dc1", + DataDir: "/opt/data/nomad/data", + LogLevel: "INFO", + BindAddr: "0.0.0.0", + AdvertiseAddrs: &AdvertiseAddrs{ + HTTP: "host.example.com", + RPC: "host.example.com", + Serf: "host.example.com", + }, + Client: &ClientConfig{ServerJoin: &ServerJoin{}}, + Server: &ServerConfig{ + Enabled: true, + BootstrapExpect: 3, + RetryJoin: []string{"10.0.0.101", "10.0.0.102", "10.0.0.103"}, + EncryptKey: "sHck3WL6cxuhuY7Mso9BHA==", + ServerJoin: &ServerJoin{}, + }, + ACL: &ACLConfig{ + Enabled: true, + }, + Telemetry: &Telemetry{ + PrometheusMetrics: true, + DisableHostname: true, + CollectionInterval: "60s", + collectionInterval: 60 * time.Second, + PublishAllocationMetrics: true, + PublishNodeMetrics: true, + }, + LeaveOnInt: true, + LeaveOnTerm: true, + EnableSyslog: true, + SyslogFacility: "LOCAL0", + Consul: &config.ConsulConfig{ + EnableSSL: helper.BoolToPtr(true), + Token: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", + ServerAutoJoin: helper.BoolToPtr(false), + ClientAutoJoin: helper.BoolToPtr(false), + }, + Vault: &config.VaultConfig{ + Enabled: helper.BoolToPtr(true), + Role: "nomad-cluster", + Addr: "http://host.example.com:8200", + }, + TLSConfig: &config.TLSConfig{ + EnableHTTP: true, + EnableRPC: true, + VerifyServerHostname: true, + CAFile: "/opt/data/nomad/certs/nomad-ca.pem", + CertFile: "/opt/data/nomad/certs/server.pem", + KeyFile: "/opt/data/nomad/certs/server-key.pem", + }, + Autopilot: &config.AutopilotConfig{ + CleanupDeadServers: helper.BoolToPtr(true), + }, +} + func TestConfig_ParseDir(t *testing.T) { c, err := LoadConfig("./testdata/sample1") require.NoError(t, err) - // Defaults - sample1 := ParseConfigDefault().Merge(sample0) - - // Merge makes empty maps & slices rather than nil, so set those for the expected - // value + // LoadConfig Merges with defaults, which makes empty maps & slices rather than nil, + // so set those for the expected value require.Nil(t, sample1.Client.Options) sample1.Client.Options = map[string]string{} require.Nil(t, sample1.Client.Meta) @@ -593,9 +656,8 @@ func TestConfig_ParseDir(t *testing.T) { sample1.Client.ChrootEnv = map[string]string{} require.Nil(t, sample1.Server.StartJoin) sample1.Server.StartJoin = []string{} - - // This value added to the config file - sample1.Consul.EnableSSL = helper.BoolToPtr(true) + require.Nil(t, sample1.HTTPAPIResponseHeaders) + sample1.HTTPAPIResponseHeaders = map[string]string{} // LoadDir listed the config files require.Nil(t, sample1.Files) From 62eb6081fa38cb072641fae45ae06b410b5ecf23 Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Tue, 11 Jun 2019 22:30:20 -0400 Subject: [PATCH 3/4] config_parse_test update comment for accuracy --- command/agent/config_parse_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/command/agent/config_parse_test.go b/command/agent/config_parse_test.go index 3266a958c752..a498b8561c88 100644 --- a/command/agent/config_parse_test.go +++ b/command/agent/config_parse_test.go @@ -646,8 +646,8 @@ func TestConfig_ParseDir(t *testing.T) { c, err := LoadConfig("./testdata/sample1") require.NoError(t, err) - // LoadConfig Merges with defaults, which makes empty maps & slices rather than nil, - // so set those for the expected value + // LoadConfig Merges all the config files in testdata/sample1, which makes empty + // maps & slices rather than nil, so set those require.Nil(t, sample1.Client.Options) sample1.Client.Options = map[string]string{} require.Nil(t, sample1.Client.Meta) @@ -659,7 +659,7 @@ func TestConfig_ParseDir(t *testing.T) { require.Nil(t, sample1.HTTPAPIResponseHeaders) sample1.HTTPAPIResponseHeaders = map[string]string{} - // LoadDir listed the config files + // LoadDir lists the config files require.Nil(t, sample1.Files) sample1.Files = []string{ "testdata/sample1/sample0.json", From 8b49c5e64a9ef8a9bfce01a4f7d1e94739120f1f Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Tue, 11 Jun 2019 22:35:43 -0400 Subject: [PATCH 4/4] command add comments re: defaults to LoadConfig --- command/agent/config.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/command/agent/config.go b/command/agent/config.go index 08783e0b4552..8153fe8aae97 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -1471,8 +1471,9 @@ func (r *Resources) Merge(b *Resources) *Resources { return &result } -// LoadConfig loads the configuration at the given path, regardless if -// its a file or directory. +// LoadConfig loads the configuration at the given path, regardless if its a file or +// directory. Called for each -config to build up the runtime config value. Do not apply any +// default values, defaults should be added once in DefaultConfig func LoadConfig(path string) (*Config, error) { fi, err := os.Stat(path) if err != nil {