Don't merge configurations.

In practice, this introduced more problems than it solved.

config/config.go

@@ -1,3 +1,5 @@
+// Package config implements configuration structures for Red
+// October.
 package config
 
 import (
@@ -5,12 +7,6 @@ import (
 	"io/ioutil"
 )
 
-func setIfNotEmpty(a *string, b string) {
-	if b != "" {
-		*a = b
-	}
-}
-
 // Server contains the configuration information required to start a
 // redoctober server.
 type Server struct {
@@ -33,25 +29,6 @@ type Server struct {
 	Systemd bool `json:"use_systemd,omitempty"`
 }
 
-// Merge copies over non-empty string values from other into the
-// current Server config.
-func (s *Server) Merge(other *Server) {
-	setIfNotEmpty(&s.Addr, other.Addr)
-	setIfNotEmpty(&s.CAPath, other.CAPath)
-
-	if len(other.KeyPaths) != 0 {
-		s.KeyPaths = other.KeyPaths
-	}
-
-	if len(other.CertPaths) != 0 {
-		s.CertPaths = other.CertPaths
-	}
-
-	if other.Systemd {
-		s.Systemd = true
-	}
-}
-
 // UI contains the configuration information for the WWW API.
 type UI struct {
 	// Root contains the base URL for the UI.
@@ -62,13 +39,6 @@ type UI struct {
 	Static string `json:"static"`
 }
 
-// Merge copies over non-empty string values from other into the
-// current UI config.
-func (ui *UI) Merge(other *UI) {
-	setIfNotEmpty(&ui.Root, other.Root)
-	setIfNotEmpty(&ui.Static, other.Static)
-}
-
 // HipChat contains the settings for Hipchat integration.
 type HipChat struct {
 	Host   string `json:"host"`
@@ -76,14 +46,6 @@ type HipChat struct {
 	APIKey string `json:"api_key"`
 }
 
-// Merge copies over non-empty settings from other into the current
-// HipChat config.
-func (hc *HipChat) Merge(other *HipChat) {
-	setIfNotEmpty(&hc.Host, other.Host)
-	setIfNotEmpty(&hc.Room, other.Room)
-	setIfNotEmpty(&hc.APIKey, other.APIKey)
-}
-
 // Valid returns true if the HipChat config is ready to be used for
 // HipChat notifications.
 func (hc *HipChat) Valid() bool {
@@ -109,13 +71,6 @@ type Metrics struct {
 	Port string `json:"port"`
 }
 
-// Merge copies over non-empty settings from other into the current
-// Metrics config.
-func (m *Metrics) Merge(other *Metrics) {
-	setIfNotEmpty(&m.Host, other.Host)
-	setIfNotEmpty(&m.Port, other.Port)
-}
-
 // Delegations contains configuration for persisting delegations.
 type Delegations struct {
 	// Persist controls whether delegations are persisted or not.
@@ -126,14 +81,6 @@ type Delegations struct {
 	Policy string `json:"policy"`
 }
 
-// Merge copies over non-empty settings from other into the current
-// Delegations config.
-func (d *Delegations) Merge(other *Delegations) {
-	setIfNotEmpty(&d.Policy, other.Policy)
-
-	d.Persist = d.Persist || other.Persist
-}
-
 // Config contains all the configuration options for a redoctober
 // instance.
 type Config struct {
@@ -144,16 +91,6 @@ type Config struct {
 	Delegations *Delegations `json:"delegations"`
 }
 
-// Merge copies over the non-empty settings from other into the
-// current Config.
-func (c *Config) Merge(other *Config) {
-	c.Server.Merge(other.Server)
-	c.UI.Merge(other.UI)
-	c.HipChat.Merge(other.HipChat)
-	c.Metrics.Merge(other.Metrics)
-	c.Delegations.Merge(other.Delegations)
-}
-
 // Valid ensures that the config has enough data to start a Red
 // October process.
 func (c *Config) Valid() bool {

config/config_test.go

@@ -100,84 +100,6 @@ func TestEmptyEqual(t *testing.T) {
 	}
 }
 
-// TestMergeEmpty verifies the behaviour where merging a config into
-// an empty config results in the empty config becoming the same
-// config as the config being merged.
-func TestMergeEmpty(t *testing.T) {
-	empty := New()
-	testConfig := &Config{
-		Server: &Server{
-			Addr:      "localhost:8080",
-			CAPath:    "",
-			KeyPaths:  "testdata/server.key",
-			CertPaths: "testdata/server.pem",
-			Systemd:   true,
-		},
-		UI: &UI{
-			Root: "https://ro.example.net",
-		},
-		Metrics: &Metrics{
-			Host: "127.0.0.1",
-			Port: "8081",
-		},
-		HipChat: &HipChat{
-			Host:   "hipchat.example.net",
-			Room:   "redoctober",
-			APIKey: "i don't this key will work",
-		},
-		Delegations: &Delegations{
-			Persist: true,
-			Policy:  "NONE",
-		},
-	}
-
-	if empty.equal(testConfig) {
-
-	}
-
-	empty.Merge(testConfig)
-	if !empty.equal(testConfig) {
-		t.Fatal("merging should result in equivalent configs")
-	}
-}
-
-// TestMergeOverride verifies that merges will combine two configs.
-func TestMergeOverride(t *testing.T) {
-	config := New()
-	config.Server = &Server{
-		Addr:      "localhost:443",
-		CAPath:    "",
-		KeyPaths:  "testdata/server.key",
-		CertPaths: "testdata/server.pem",
-	}
-
-	merge := New()
-	merge.Server = &Server{
-		Addr: "localhost:8000",
-	}
-
-	expected := New()
-	expected.Server = &Server{
-		Addr:      "localhost:8000",
-		CAPath:    "",
-		KeyPaths:  "testdata/server.key",
-		CertPaths: "testdata/server.pem",
-	}
-
-	if config.equal(merge) {
-		t.Fatal("configurations shouldn't match")
-	}
-
-	if config.equal(expected) {
-		t.Fatal("configurations shouldn't match")
-	}
-
-	config.Merge(merge)
-	if !config.equal(expected) {
-		t.Fatal("configurations don't match")
-	}
-}
-
 // TestLoadFile validates loading a configuration from disk.
 func TestLoadFile(t *testing.T) {
 	goodConfig := "testdata/config.json"

redoctober.go

@@ -237,12 +237,22 @@ var (
 	vaultPath string
 )
 
+const (
+	defaultAddr        = "localhost:8080"
+	defaultMetricsHost = "localhost"
+	defaultMetricsPort = "8081"
+)
+
 func init() {
 	// cli contains the configuration set by the command line
 	// options, and cfg is the actual Red October config.
 	cli = config.New()
 	cfg = config.New()
 
+	cli.Server.Addr = defaultAddr
+	cli.Metrics.Host = defaultMetricsHost
+	cli.Metrics.Port = defaultMetricsPort
+
 	flag.Usage = func() {
 		fmt.Fprint(os.Stderr, "main usage dump\n")
 		fmt.Fprint(os.Stderr, usage)
@@ -251,18 +261,29 @@ func init() {
 	}
 
 	flag.StringVar(&confFile, "f", "", "path to config file")
-	flag.StringVar(&cli.Server.Addr, "addr", "localhost:8080", "Server and port separated by :")
+	flag.StringVar(&cli.Server.Addr, "addr", cli.Server.Addr,
-	flag.StringVar(&cli.Server.CAPath, "ca", "", "Path of TLS CA for client authentication (optional)")
+		"Server and port separated by :")
-	flag.StringVar(&cli.Server.CertPaths, "certs", "", "Path(s) of TLS certificate in PEM format, comma-separated")
+	flag.StringVar(&cli.Server.CAPath, "ca", cli.Server.CAPath,
-	flag.StringVar(&cli.HipChat.Host, "hchost", "", "Hipchat Url Base (ex: hipchat.com)")
+		"Path of TLS CA for client authentication (optional)")
-	flag.StringVar(&cli.HipChat.APIKey, "hckey", "", "Hipchat API Key")
+	flag.StringVar(&cli.Server.CertPaths, "certs", cli.Server.CertPaths,
-	flag.StringVar(&cli.HipChat.Room, "hcroom", "", "Hipchat Room Id")
+		"Path(s) of TLS certificate in PEM format, comma-separated")
-	flag.StringVar(&cli.Server.KeyPaths, "keys", "", "Path(s) of TLS private key in PEM format, comma-separated, must me in the same order as the certs")
+	flag.StringVar(&cli.HipChat.Host, "hchost", cli.HipChat.Host,
-	flag.StringVar(&cli.Metrics.Host, "metrics-host", "localhost", "The `host` the metrics endpoint should listen on.")
+		"Hipchat Url Base (ex: hipchat.com)")
-	flag.StringVar(&cli.Metrics.Port, "metrics-port", "8081", "The `port` the metrics endpoint should listen on.")
+	flag.StringVar(&cli.HipChat.APIKey, "hckey", cli.HipChat.APIKey,
-	flag.StringVar(&cli.UI.Root, "rohost", "", "RedOctober Url Base (ex: localhost:8080)")
+		"Hipchat API Key")
-	flag.StringVar(&cli.UI.Static, "static", "", "Path to override built-in index.html")
+	flag.StringVar(&cli.HipChat.Room, "hcroom", cli.HipChat.Room,
-	flag.BoolVar(&cli.Server.Systemd, "systemdfds", false, "Use systemd socket activation to listen on a file. Useful for binding privileged sockets.")
+		"Hipchat Room ID")
+	flag.StringVar(&cli.Server.KeyPaths, "keys", cli.Server.KeyPaths,
+		"Comma-separated list of PEM-encoded TLS private keys in the same order as certs")
+	flag.StringVar(&cli.Metrics.Host, "metrics-host", cli.Metrics.Host,
+		"The `host` the metrics endpoint should listen on.")
+	flag.StringVar(&cli.Metrics.Port, "metrics-port", cli.Metrics.Port,
+		"The `port` the metrics endpoint should listen on.")
+	flag.StringVar(&cli.UI.Root, "rohost", cli.UI.Root, "RedOctober URL Base (ex: localhost:8080)")
+	flag.StringVar(&cli.UI.Static, "static", cli.UI.Static,
+		"Path to override built-in index.html")
+	flag.BoolVar(&cli.Server.Systemd, "systemdfds", cli.Server.Systemd,
+		"Use systemd socket activation to listen on a file. Useful for binding privileged sockets.")
 	flag.StringVar(&vaultPath, "vaultpath", "diskrecord.json", "Path to the the disk vault")
 
 	flag.Parse()
@@ -277,8 +298,9 @@ func main() {
 		if err != nil {
 			log.Fatal(err)
 		}
+	} else {
+		cfg = cli
 	}
-	cfg.Merge(cli)
 
 	if vaultPath == "" || !cfg.Valid() {
 		fmt.Fprint(os.Stderr, usage)