From 4f51b6e3c9870b95e2a7fe468b953d30d4b1aa9f Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Fri, 11 Dec 2015 15:58:10 -0500 Subject: [PATCH 1/3] Allow separate HA physical backend. With no separate backend specified, HA will be attempted on the normal physical backend. Fixes #395. --- command/server.go | 54 ++++++++++-- command/server/config.go | 7 ++ command/server/config_test.go | 15 ++++ command/server/test-fixtures/config.hcl | 5 ++ command/server/test-fixtures/config2.hcl.json | 6 ++ command/server_test.go | 87 +++++++++++++++++++ http/logical_test.go | 5 +- vault/core.go | 22 +++-- vault/core_test.go | 20 ++++- 9 files changed, 199 insertions(+), 22 deletions(-) create mode 100644 command/server_test.go diff --git a/command/server.go b/command/server.go index 99f3ba3513..8760e55ddd 100644 --- a/command/server.go +++ b/command/server.go @@ -39,12 +39,13 @@ type ServerCommand struct { } func (c *ServerCommand) Run(args []string) int { - var dev bool + var dev, verifyOnly bool var configPath []string var logLevel string flags := c.Meta.FlagSet("server", FlagSetDefault) flags.BoolVar(&dev, "dev", false, "") flags.StringVar(&logLevel, "log-level", "info", "") + flags.BoolVar(&verifyOnly, "verify-only", false, "") flags.Usage = func() { c.Ui.Error(c.Help()) } flags.Var((*sliceflag.StringFlag)(&configPath), "config", "config") if err := flags.Parse(args); err != nil { @@ -113,22 +114,47 @@ func (c *ServerCommand) Run(args []string) int { return 1 } + var advertiseAddr string = config.Backend.AdvertiseAddr + + // Note that "habackend" is a backend that *may* support HA; + // it defaults to the same backend as normal operations + var habackend physical.Backend = backend + + // Initialize the separate HA physical backend, if it exists + if config.HABackend != nil { + habackend, err = physical.NewBackend( + config.HABackend.Type, config.HABackend.Config) + if err != nil { + c.Ui.Error(fmt.Sprintf( + "Error initializing backend of type %s: %s", + config.HABackend.Type, err)) + return 1 + } + if _, ok := habackend.(physical.HABackend); !ok { + c.Ui.Error("Specified HA backend does not support HA") + return 1 + } + + advertiseAddr = config.HABackend.AdvertiseAddr + } + // Attempt to detect the advertise address possible - if detect, ok := backend.(physical.AdvertiseDetect); ok && config.Backend.AdvertiseAddr == "" { + if detect, ok := habackend.(physical.AdvertiseDetect); ok && advertiseAddr == "" { advertise, err := c.detectAdvertise(detect, config) if err != nil { c.Ui.Error(fmt.Sprintf("Error detecting advertise address: %s", err)) } else if advertise == "" { c.Ui.Error("Failed to detect advertise address.") } else { - config.Backend.AdvertiseAddr = advertise + advertiseAddr = advertise } } // Initialize the core core, err := vault.NewCore(&vault.CoreConfig{ - AdvertiseAddr: config.Backend.AdvertiseAddr, + AdvertiseAddr: advertiseAddr, Physical: backend, + HAPhysical: habackend, AuditBackends: c.AuditBackends, CredentialBackends: c.CredentialBackends, LogicalBackends: c.LogicalBackends, @@ -186,11 +212,17 @@ func (c *ServerCommand) Run(args []string) int { mlock.Supported(), !config.DisableMlock) infoKeys = append(infoKeys, "log level", "mlock", "backend") - // If the backend supports HA, then note it - if _, ok := backend.(physical.HABackend); ok { - info["backend"] += " (HA available)" - info["advertise address"] = config.Backend.AdvertiseAddr - infoKeys = append(infoKeys, "advertise address") + if config.HABackend != nil { + info["HA backend"] = config.HABackend.Type + info["advertise address"] = advertiseAddr + infoKeys = append(infoKeys, "HA backend", "advertise address") + } else { + // If the backend supports HA, then note it + if _, ok := habackend.(physical.HABackend); ok { + info["backend"] += " (HA available)" + info["advertise address"] = advertiseAddr + infoKeys = append(infoKeys, "advertise address") + } } // Initialize the telemetry @@ -225,6 +257,10 @@ func (c *ServerCommand) Run(args []string) int { lns = append(lns, ln) } + if verifyOnly { + return 0 + } + // Initialize the HTTP server server := &http.Server{} server.Handler = vaulthttp.Handler(core) diff --git a/command/server/config.go b/command/server/config.go index 073b42e024..253bfc55f2 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -17,6 +17,7 @@ import ( type Config struct { Listeners []*Listener `hcl:"-"` Backend *Backend `hcl:"-"` + HABackend *Backend `hcl:"-"` DisableCache bool `hcl:"disable_cache"` DisableMlock bool `hcl:"disable_mlock"` @@ -191,6 +192,12 @@ func LoadConfigFile(path string) (*Config, error) { return nil, err } } + if objs := obj.Get("ha_backend", false); objs != nil { + result.HABackend, err = loadBackend(objs) + if err != nil { + return nil, err + } + } // A little hacky but upgrades the old stats config directives to the new way if result.Telemetry == nil { diff --git a/command/server/config_test.go b/command/server/config_test.go index 020493defc..b29654700d 100644 --- a/command/server/config_test.go +++ b/command/server/config_test.go @@ -30,6 +30,14 @@ func TestLoadConfigFile(t *testing.T) { }, }, + HABackend: &Backend{ + Type: "consul", + AdvertiseAddr: "snafu", + Config: map[string]string{ + "bar": "baz", + }, + }, + Telemetry: &Telemetry{ StatsdAddr: "bar", StatsiteAddr: "foo", @@ -111,6 +119,13 @@ func TestLoadConfigFile_json2(t *testing.T) { }, }, + HABackend: &Backend{ + Type: "consul", + Config: map[string]string{ + "bar": "baz", + }, + }, + Telemetry: &Telemetry{ StatsiteAddr: "foo", StatsdAddr: "bar", diff --git a/command/server/test-fixtures/config.hcl b/command/server/test-fixtures/config.hcl index e945d612c3..a591b838e6 100644 --- a/command/server/test-fixtures/config.hcl +++ b/command/server/test-fixtures/config.hcl @@ -12,5 +12,10 @@ backend "consul" { advertise_addr = "foo" } +ha_backend "consul" { + bar = "baz" + advertise_addr = "snafu" +} + max_lease_ttl = "10h" default_lease_ttl = "10h" diff --git a/command/server/test-fixtures/config2.hcl.json b/command/server/test-fixtures/config2.hcl.json index 534317ff2e..7364480172 100644 --- a/command/server/test-fixtures/config2.hcl.json +++ b/command/server/test-fixtures/config2.hcl.json @@ -11,6 +11,12 @@ } }, + "ha_backend": { + "consul": { + "bar": "baz" + } + }, + "telemetry": { "statsd_address": "bar", "statsite_address": "foo", diff --git a/command/server_test.go b/command/server_test.go new file mode 100644 index 0000000000..203eb5276f --- /dev/null +++ b/command/server_test.go @@ -0,0 +1,87 @@ +package command + +import ( + "io/ioutil" + "os" + "testing" + + "github.com/mitchellh/cli" +) + +var ( + basehcl = ` +disable_mlock = true + +listener "tcp" { + address = "127.0.0.1:8200" + tls_disable = "true" +} +` + + consulhcl = ` +backend "consul" { + prefix = "foo/" + advertise_addr = "http://127.0.0.1:8200" +} +` + haconsulhcl = ` +ha_backend "consul" { + prefix = "bar/" + advertise_addr = "http://127.0.0.1:8200" +} +` + + badhaconsulhcl = ` +ha_backend "file" { + path = "/dev/null" +} +` +) + +func TestServer_GoodSeparateHA(t *testing.T) { + ui := new(cli.MockUi) + c := &ServerCommand{ + Meta: Meta{ + Ui: ui, + }, + } + + tmpfile, err := ioutil.TempFile("", "") + if err != nil { + t.Fatalf("error creating temp dir: %v", err) + } + + tmpfile.WriteString(basehcl + consulhcl + haconsulhcl) + tmpfile.Close() + defer os.Remove(tmpfile.Name()) + + args := []string{"-config", tmpfile.Name(), "-verify-only", "true"} + + if code := c.Run(args); code != 0 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } +} + +func TestServer_BadSeparateHA(t *testing.T) { + ui := new(cli.MockUi) + c := &ServerCommand{ + Meta: Meta{ + Ui: ui, + }, + } + + tmpfile, err := ioutil.TempFile("", "") + if err != nil { + t.Fatalf("error creating temp dir: %v", err) + } + + tmpfile.WriteString(basehcl + consulhcl + badhaconsulhcl) + tmpfile.Close() + defer os.Remove(tmpfile.Name()) + + args := []string{"-config", tmpfile.Name()} + + if code := c.Run(args); code == 0 { + t.Fatalf("bad: should have gotten an error on a bad HA config") + } +} diff --git a/http/logical_test.go b/http/logical_test.go index 0edf44f19a..369a768747 100644 --- a/http/logical_test.go +++ b/http/logical_test.go @@ -69,9 +69,11 @@ func TestLogical_StandbyRedirect(t *testing.T) { defer ln2.Close() // Create an HA Vault - inm := physical.NewInmemHA() + inm := physical.NewInmem() + inmha := physical.NewInmemHA() conf := &vault.CoreConfig{ Physical: inm, + HAPhysical: inmha, AdvertiseAddr: addr1, DisableMlock: true, } @@ -87,6 +89,7 @@ func TestLogical_StandbyRedirect(t *testing.T) { // Create a second HA Vault conf2 := &vault.CoreConfig{ Physical: inm, + HAPhysical: inmha, AdvertiseAddr: addr2, DisableMlock: true, } diff --git a/vault/core.go b/vault/core.go index eaa349d00c..f87fcd9cf3 100644 --- a/vault/core.go +++ b/vault/core.go @@ -273,20 +273,26 @@ type CoreConfig struct { CredentialBackends map[string]logical.Factory AuditBackends map[string]audit.Factory Physical physical.Backend - Logger *log.Logger - DisableCache bool // Disables the LRU cache on the physical backend - DisableMlock bool // Disables mlock syscall - CacheSize int // Custom cache size of zero for default - AdvertiseAddr string // Set as the leader address for HA - DefaultLeaseTTL time.Duration - MaxLeaseTTL time.Duration + + // Defaults to the same backend as Physical. This is not a backend that + // necessarily supports HA; it is merely the one that will be attempted + // for HA operations + HAPhysical physical.Backend + + Logger *log.Logger + DisableCache bool // Disables the LRU cache on the physical backend + DisableMlock bool // Disables mlock syscall + CacheSize int // Custom cache size of zero for default + AdvertiseAddr string // Set as the leader address for HA + DefaultLeaseTTL time.Duration + MaxLeaseTTL time.Duration } // NewCore is used to construct a new core func NewCore(conf *CoreConfig) (*Core, error) { // Check if this backend supports an HA configuraiton var haBackend physical.HABackend - if ha, ok := conf.Physical.(physical.HABackend); ok { + if ha, ok := conf.HAPhysical.(physical.HABackend); ok { haBackend = ha } if haBackend != nil && conf.AdvertiseAddr == "" { diff --git a/vault/core_test.go b/vault/core_test.go index 887b5c8ba4..6155986439 100644 --- a/vault/core_test.go +++ b/vault/core_test.go @@ -1110,10 +1110,12 @@ func TestCore_LimitedUseToken(t *testing.T) { func TestCore_CleanLeaderPrefix(t *testing.T) { // Create the first core and initialize it - inm := physical.NewInmemHA() + inm := physical.NewInmem() + inmha := physical.NewInmemHA() advertiseOriginal := "http://127.0.0.1:8200" core, err := NewCore(&CoreConfig{ Physical: inm, + HAPhysical: inmha, AdvertiseAddr: advertiseOriginal, DisableMlock: true, }) @@ -1172,6 +1174,7 @@ func TestCore_CleanLeaderPrefix(t *testing.T) { advertiseOriginal2 := "http://127.0.0.1:8500" core2, err := NewCore(&CoreConfig{ Physical: inm, + HAPhysical: inmha, AdvertiseAddr: advertiseOriginal2, DisableMlock: true, }) @@ -1256,10 +1259,12 @@ func TestCore_CleanLeaderPrefix(t *testing.T) { func TestCore_Standby(t *testing.T) { // Create the first core and initialize it - inm := physical.NewInmemHA() + inm := physical.NewInmem() + inmha := physical.NewInmemHA() advertiseOriginal := "http://127.0.0.1:8200" core, err := NewCore(&CoreConfig{ Physical: inm, + HAPhysical: inmha, AdvertiseAddr: advertiseOriginal, DisableMlock: true, }) @@ -1313,6 +1318,7 @@ func TestCore_Standby(t *testing.T) { advertiseOriginal2 := "http://127.0.0.1:8500" core2, err := NewCore(&CoreConfig{ Physical: inm, + HAPhysical: inmha, AdvertiseAddr: advertiseOriginal2, DisableMlock: true, }) @@ -2003,10 +2009,12 @@ func testWaitActive(t *testing.T, core *Core) { func TestCore_Standby_Rotate(t *testing.T) { // Create the first core and initialize it - inm := physical.NewInmemHA() + inm := physical.NewInmem() + inmha := physical.NewInmemHA() advertiseOriginal := "http://127.0.0.1:8200" core, err := NewCore(&CoreConfig{ Physical: inm, + HAPhysical: inmha, AdvertiseAddr: advertiseOriginal, DisableMlock: true, }) @@ -2025,6 +2033,7 @@ func TestCore_Standby_Rotate(t *testing.T) { advertiseOriginal2 := "http://127.0.0.1:8500" core2, err := NewCore(&CoreConfig{ Physical: inm, + HAPhysical: inmha, AdvertiseAddr: advertiseOriginal2, DisableMlock: true, }) @@ -2074,10 +2083,12 @@ func TestCore_Standby_Rotate(t *testing.T) { func TestCore_Standby_Rekey(t *testing.T) { // Create the first core and initialize it - inm := physical.NewInmemHA() + inm := physical.NewInmem() + inmha := physical.NewInmemHA() advertiseOriginal := "http://127.0.0.1:8200" core, err := NewCore(&CoreConfig{ Physical: inm, + HAPhysical: inmha, AdvertiseAddr: advertiseOriginal, DisableMlock: true, }) @@ -2096,6 +2107,7 @@ func TestCore_Standby_Rekey(t *testing.T) { advertiseOriginal2 := "http://127.0.0.1:8500" core2, err := NewCore(&CoreConfig{ Physical: inm, + HAPhysical: inmha, AdvertiseAddr: advertiseOriginal2, DisableMlock: true, }) From b1f815d7f84a43cac9ab2ec4c8c5bdaaeba79d03 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Mon, 14 Dec 2015 17:58:30 -0500 Subject: [PATCH 2/3] Address review feedback --- command/server.go | 84 +++++++++++++++++++++----------------------- http/logical_test.go | 3 -- vault/core.go | 12 ++----- vault/core_test.go | 11 ++++-- 4 files changed, 52 insertions(+), 58 deletions(-) diff --git a/command/server.go b/command/server.go index 8760e55ddd..5927b1b6db 100644 --- a/command/server.go +++ b/command/server.go @@ -114,47 +114,9 @@ func (c *ServerCommand) Run(args []string) int { return 1 } - var advertiseAddr string = config.Backend.AdvertiseAddr - - // Note that "habackend" is a backend that *may* support HA; - // it defaults to the same backend as normal operations - var habackend physical.Backend = backend - - // Initialize the separate HA physical backend, if it exists - if config.HABackend != nil { - habackend, err = physical.NewBackend( - config.HABackend.Type, config.HABackend.Config) - if err != nil { - c.Ui.Error(fmt.Sprintf( - "Error initializing backend of type %s: %s", - config.HABackend.Type, err)) - return 1 - } - if _, ok := habackend.(physical.HABackend); !ok { - c.Ui.Error("Specified HA backend does not support HA") - return 1 - } - - advertiseAddr = config.HABackend.AdvertiseAddr - } - - // Attempt to detect the advertise address possible - if detect, ok := habackend.(physical.AdvertiseDetect); ok && advertiseAddr == "" { - advertise, err := c.detectAdvertise(detect, config) - if err != nil { - c.Ui.Error(fmt.Sprintf("Error detecting advertise address: %s", err)) - } else if advertise == "" { - c.Ui.Error("Failed to detect advertise address.") - } else { - advertiseAddr = advertise - } - } - - // Initialize the core - core, err := vault.NewCore(&vault.CoreConfig{ - AdvertiseAddr: advertiseAddr, + coreConfig := &vault.CoreConfig{ Physical: backend, - HAPhysical: habackend, + HAPhysical: nil, AuditBackends: c.AuditBackends, CredentialBackends: c.CredentialBackends, LogicalBackends: c.LogicalBackends, @@ -163,7 +125,41 @@ func (c *ServerCommand) Run(args []string) int { DisableMlock: config.DisableMlock, MaxLeaseTTL: config.MaxLeaseTTL, DefaultLeaseTTL: config.DefaultLeaseTTL, - }) + } + + // Initialize the separate HA physical backend, if it exists + if config.HABackend != nil { + habackend, err := physical.NewBackend( + config.HABackend.Type, config.HABackend.Config) + if err != nil { + c.Ui.Error(fmt.Sprintf( + "Error initializing backend of type %s: %s", + config.HABackend.Type, err)) + return 1 + } + + var ok bool + if coreConfig.HAPhysical, ok = habackend.(physical.HABackend); !ok { + c.Ui.Error("Specified HA backend does not support HA") + return 1 + } + coreConfig.AdvertiseAddr = config.HABackend.AdvertiseAddr + } + + // Attempt to detect the advertise address possible + if detect, ok := coreConfig.HAPhysical.(physical.AdvertiseDetect); ok && coreConfig.AdvertiseAddr == "" { + advertise, err := c.detectAdvertise(detect, config) + if err != nil { + c.Ui.Error(fmt.Sprintf("Error detecting advertise address: %s", err)) + } else if advertise == "" { + c.Ui.Error("Failed to detect advertise address.") + } else { + coreConfig.AdvertiseAddr = advertise + } + } + + // Initialize the core + core, err := vault.NewCore(coreConfig) if err != nil { c.Ui.Error(fmt.Sprintf("Error initializing core: %s", err)) return 1 @@ -214,13 +210,13 @@ func (c *ServerCommand) Run(args []string) int { if config.HABackend != nil { info["HA backend"] = config.HABackend.Type - info["advertise address"] = advertiseAddr + info["advertise address"] = coreConfig.AdvertiseAddr infoKeys = append(infoKeys, "HA backend", "advertise address") } else { // If the backend supports HA, then note it - if _, ok := habackend.(physical.HABackend); ok { + if coreConfig.HAPhysical != nil { info["backend"] += " (HA available)" - info["advertise address"] = advertiseAddr + info["advertise address"] = coreConfig.AdvertiseAddr infoKeys = append(infoKeys, "advertise address") } } diff --git a/http/logical_test.go b/http/logical_test.go index 369a768747..8ae7f2a65c 100644 --- a/http/logical_test.go +++ b/http/logical_test.go @@ -70,10 +70,8 @@ func TestLogical_StandbyRedirect(t *testing.T) { // Create an HA Vault inm := physical.NewInmem() - inmha := physical.NewInmemHA() conf := &vault.CoreConfig{ Physical: inm, - HAPhysical: inmha, AdvertiseAddr: addr1, DisableMlock: true, } @@ -89,7 +87,6 @@ func TestLogical_StandbyRedirect(t *testing.T) { // Create a second HA Vault conf2 := &vault.CoreConfig{ Physical: inm, - HAPhysical: inmha, AdvertiseAddr: addr2, DisableMlock: true, } diff --git a/vault/core.go b/vault/core.go index f87fcd9cf3..b536bec0e6 100644 --- a/vault/core.go +++ b/vault/core.go @@ -277,7 +277,7 @@ type CoreConfig struct { // Defaults to the same backend as Physical. This is not a backend that // necessarily supports HA; it is merely the one that will be attempted // for HA operations - HAPhysical physical.Backend + HAPhysical physical.HABackend Logger *log.Logger DisableCache bool // Disables the LRU cache on the physical backend @@ -290,12 +290,7 @@ type CoreConfig struct { // NewCore is used to construct a new core func NewCore(conf *CoreConfig) (*Core, error) { - // Check if this backend supports an HA configuraiton - var haBackend physical.HABackend - if ha, ok := conf.HAPhysical.(physical.HABackend); ok { - haBackend = ha - } - if haBackend != nil && conf.AdvertiseAddr == "" { + if conf.HAPhysical != nil && conf.AdvertiseAddr == "" { return nil, fmt.Errorf("missing advertisement address") } @@ -305,7 +300,6 @@ func NewCore(conf *CoreConfig) (*Core, error) { if conf.MaxLeaseTTL == 0 { conf.MaxLeaseTTL = maxLeaseTTL } - if conf.DefaultLeaseTTL > conf.MaxLeaseTTL { return nil, fmt.Errorf("cannot have DefaultLeaseTTL larger than MaxLeaseTTL") } @@ -361,7 +355,7 @@ func NewCore(conf *CoreConfig) (*Core, error) { // Setup the core c := &Core{ - ha: haBackend, + ha: conf.HAPhysical, advertiseAddr: conf.AdvertiseAddr, physical: conf.Physical, barrier: barrier, diff --git a/vault/core_test.go b/vault/core_test.go index 6155986439..6a99b5d56c 100644 --- a/vault/core_test.go +++ b/vault/core_test.go @@ -1258,9 +1258,16 @@ func TestCore_CleanLeaderPrefix(t *testing.T) { } func TestCore_Standby(t *testing.T) { - // Create the first core and initialize it - inm := physical.NewInmem() inmha := physical.NewInmemHA() + testCore_Standby_Common(t, inmha, inmha) +} + +func TestCore_Standby_SeparateHA(t *testing.T) { + testCore_Standby_Common(t, physical.NewInmemHA(), physical.NewInmemHA()) +} + +func testCore_Standby_Common(t *testing.T, inm physical.Backend, inmha physical.HABackend) { + // Create the first core and initialize it advertiseOriginal := "http://127.0.0.1:8200" core, err := NewCore(&CoreConfig{ Physical: inm, From de0963cf16b805d6a089dd56f82f720ef636cf40 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Mon, 14 Dec 2015 20:48:22 -0500 Subject: [PATCH 3/3] Add test to ensure the right backend was used with separate HA --- physical/inmem_ha.go | 6 ++++++ vault/core_test.go | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/physical/inmem_ha.go b/physical/inmem_ha.go index 92ee314f77..36e09a087f 100644 --- a/physical/inmem_ha.go +++ b/physical/inmem_ha.go @@ -32,6 +32,12 @@ func (i *InmemHABackend) LockWith(key, value string) (Lock, error) { return l, nil } +// LockMapSize is used in some tests to determine whether this backend has ever +// been used for HA purposes rather than simply for storage +func (i *InmemHABackend) LockMapSize() int { + return len(i.locks) +} + // InmemLock is an in-memory Lock implementation for the HABackend type InmemLock struct { in *InmemHABackend diff --git a/vault/core_test.go b/vault/core_test.go index 6a99b5d56c..a3a0d0b8ca 100644 --- a/vault/core_test.go +++ b/vault/core_test.go @@ -1417,6 +1417,23 @@ func testCore_Standby_Common(t *testing.T, inm physical.Backend, inmha physical. if advertise != advertiseOriginal2 { t.Fatalf("Bad advertise: %v", advertise) } + + if inm.(*physical.InmemHABackend) == inmha.(*physical.InmemHABackend) { + lockSize := inm.(*physical.InmemHABackend).LockMapSize() + if lockSize == 0 { + t.Fatalf("locks not used with only one HA backend") + } + } else { + lockSize := inmha.(*physical.InmemHABackend).LockMapSize() + if lockSize == 0 { + t.Fatalf("locks not used with expected HA backend") + } + + lockSize = inm.(*physical.InmemHABackend).LockMapSize() + if lockSize != 0 { + t.Fatalf("locks used with unexpected HA backend") + } + } } // Ensure that InternalData is never returned