From 0738eb8596703ec4cf84e381b2afcc49c49e8876 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 7 Aug 2020 18:30:32 -0400 Subject: [PATCH] Extract nodeID functions to a different file In preparation for turning them into functions. To reduce the scope of Agent, and refactor how Agent is created and started. --- agent/agent.go | 119 -------------------------------------- agent/agent_test.go | 115 ------------------------------------- agent/nodeid.go | 133 +++++++++++++++++++++++++++++++++++++++++++ agent/nodeid_test.go | 125 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 258 insertions(+), 234 deletions(-) create mode 100644 agent/nodeid.go create mode 100644 agent/nodeid_test.go diff --git a/agent/agent.go b/agent/agent.go index 385b5d8f44..81f68b9f0f 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -2,7 +2,6 @@ package agent import ( "context" - "crypto/sha512" "crypto/tls" "encoding/json" "fmt" @@ -52,11 +51,9 @@ import ( "github.com/hashicorp/consul/tlsutil" "github.com/hashicorp/consul/types" "github.com/hashicorp/go-multierror" - "github.com/hashicorp/go-uuid" "github.com/hashicorp/memberlist" "github.com/hashicorp/raft" "github.com/hashicorp/serf/serf" - "github.com/shirou/gopsutil/host" "golang.org/x/net/http2" ) @@ -1560,122 +1557,6 @@ func (a *Agent) segmentConfig() ([]consul.NetworkSegment, error) { return segments, nil } -// makeRandomID will generate a random UUID for a node. -func (a *Agent) makeRandomID() (string, error) { - id, err := uuid.GenerateUUID() - if err != nil { - return "", err - } - - a.logger.Debug("Using random ID as node ID", "id", id) - return id, nil -} - -// makeNodeID will try to find a host-specific ID, or else will generate a -// random ID. The returned ID will always be formatted as a GUID. We don't tell -// the caller whether this ID is random or stable since the consequences are -// high for us if this changes, so we will persist it either way. This will let -// gopsutil change implementations without affecting in-place upgrades of nodes. -func (a *Agent) makeNodeID() (string, error) { - // If they've disabled host-based IDs then just make a random one. - if a.config.DisableHostNodeID { - return a.makeRandomID() - } - - // Try to get a stable ID associated with the host itself. - info, err := host.Info() - if err != nil { - a.logger.Debug("Couldn't get a unique ID from the host", "error", err) - return a.makeRandomID() - } - - // Make sure the host ID parses as a UUID, since we don't have complete - // control over this process. - id := strings.ToLower(info.HostID) - if _, err := uuid.ParseUUID(id); err != nil { - a.logger.Debug("Unique ID from host isn't formatted as a UUID", - "id", id, - "error", err, - ) - return a.makeRandomID() - } - - // Hash the input to make it well distributed. The reported Host UUID may be - // similar across nodes if they are on a cloud provider or on motherboards - // created from the same batch. - buf := sha512.Sum512([]byte(id)) - id = fmt.Sprintf("%08x-%04x-%04x-%04x-%12x", - buf[0:4], - buf[4:6], - buf[6:8], - buf[8:10], - buf[10:16]) - - a.logger.Debug("Using unique ID from host as node ID", "id", id) - return id, nil -} - -// setupNodeID will pull the persisted node ID, if any, or create a random one -// and persist it. -func (a *Agent) setupNodeID(config *config.RuntimeConfig) error { - // If they've configured a node ID manually then just use that, as - // long as it's valid. - if config.NodeID != "" { - config.NodeID = types.NodeID(strings.ToLower(string(config.NodeID))) - if _, err := uuid.ParseUUID(string(config.NodeID)); err != nil { - return err - } - - return nil - } - - // For dev mode we have no filesystem access so just make one. - if a.config.DataDir == "" { - id, err := a.makeNodeID() - if err != nil { - return err - } - - config.NodeID = types.NodeID(id) - return nil - } - - // Load saved state, if any. Since a user could edit this, we also - // validate it. - fileID := filepath.Join(config.DataDir, "node-id") - if _, err := os.Stat(fileID); err == nil { - rawID, err := ioutil.ReadFile(fileID) - if err != nil { - return err - } - - nodeID := strings.TrimSpace(string(rawID)) - nodeID = strings.ToLower(nodeID) - if _, err := uuid.ParseUUID(nodeID); err != nil { - return err - } - - config.NodeID = types.NodeID(nodeID) - } - - // If we still don't have a valid node ID, make one. - if config.NodeID == "" { - id, err := a.makeNodeID() - if err != nil { - return err - } - if err := lib.EnsurePath(fileID, false); err != nil { - return err - } - if err := ioutil.WriteFile(fileID, []byte(id), 0600); err != nil { - return err - } - - config.NodeID = types.NodeID(id) - } - return nil -} - // setupBaseKeyrings configures the LAN and WAN keyrings. func (a *Agent) setupBaseKeyrings(config *consul.Config) error { // If the keyring file is disabled then just poke the provided key diff --git a/agent/agent_test.go b/agent/agent_test.go index 2865d5e0c6..4b109b9675 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -39,7 +39,6 @@ import ( "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/consul/testrpc" "github.com/hashicorp/consul/types" - "github.com/hashicorp/go-uuid" "github.com/hashicorp/serf/coordinate" "github.com/hashicorp/serf/serf" "github.com/stretchr/testify/assert" @@ -252,120 +251,6 @@ func TestAgent_ReconnectConfigWanDisabled(t *testing.T) { require.Nil(t, a.consulConfig().SerfWANConfig) } -func TestAgent_setupNodeID(t *testing.T) { - t.Parallel() - a := NewTestAgent(t, ` - node_id = "" - `) - defer a.Shutdown() - - cfg := a.config - - // The auto-assigned ID should be valid. - id := a.consulConfig().NodeID - if _, err := uuid.ParseUUID(string(id)); err != nil { - t.Fatalf("err: %v", err) - } - - // Running again should get the same ID (persisted in the file). - cfg.NodeID = "" - if err := a.setupNodeID(cfg); err != nil { - t.Fatalf("err: %v", err) - } - if newID := a.consulConfig().NodeID; id != newID { - t.Fatalf("bad: %q vs %q", id, newID) - } - - // Set an invalid ID via.Config. - cfg.NodeID = types.NodeID("nope") - err := a.setupNodeID(cfg) - if err == nil || !strings.Contains(err.Error(), "uuid string is wrong length") { - t.Fatalf("err: %v", err) - } - - // Set a valid ID via.Config. - newID, err := uuid.GenerateUUID() - if err != nil { - t.Fatalf("err: %v", err) - } - cfg.NodeID = types.NodeID(strings.ToUpper(newID)) - if err := a.setupNodeID(cfg); err != nil { - t.Fatalf("err: %v", err) - } - if id := a.consulConfig().NodeID; string(id) != newID { - t.Fatalf("bad: %q vs. %q", id, newID) - } - - // Set an invalid ID via the file. - fileID := filepath.Join(cfg.DataDir, "node-id") - if err := ioutil.WriteFile(fileID, []byte("adf4238a!882b!9ddc!4a9d!5b6758e4159e"), 0600); err != nil { - t.Fatalf("err: %v", err) - } - cfg.NodeID = "" - err = a.setupNodeID(cfg) - if err == nil || !strings.Contains(err.Error(), "uuid is improperly formatted") { - t.Fatalf("err: %v", err) - } - - // Set a valid ID via the file. - if err := ioutil.WriteFile(fileID, []byte("ADF4238a-882b-9ddc-4a9d-5b6758e4159e"), 0600); err != nil { - t.Fatalf("err: %v", err) - } - cfg.NodeID = "" - if err := a.setupNodeID(cfg); err != nil { - t.Fatalf("err: %v", err) - } - if id := a.consulConfig().NodeID; string(id) != "adf4238a-882b-9ddc-4a9d-5b6758e4159e" { - t.Fatalf("bad: %q vs. %q", id, newID) - } -} - -func TestAgent_makeNodeID(t *testing.T) { - t.Parallel() - a := NewTestAgent(t, ` - node_id = "" - `) - defer a.Shutdown() - - // We should get a valid host-based ID initially. - id, err := a.makeNodeID() - if err != nil { - t.Fatalf("err: %v", err) - } - if _, err := uuid.ParseUUID(id); err != nil { - t.Fatalf("err: %v", err) - } - - // Calling again should yield a random ID by default. - another, err := a.makeNodeID() - if err != nil { - t.Fatalf("err: %v", err) - } - if id == another { - t.Fatalf("bad: %s vs %s", id, another) - } - - // Turn on host-based IDs and try again. We should get the same ID - // each time (and a different one from the random one above). - a.GetConfig().DisableHostNodeID = false - id, err = a.makeNodeID() - if err != nil { - t.Fatalf("err: %v", err) - } - if id == another { - t.Fatalf("bad: %s vs %s", id, another) - } - - // Calling again should yield the host-based ID. - another, err = a.makeNodeID() - if err != nil { - t.Fatalf("err: %v", err) - } - if id != another { - t.Fatalf("bad: %s vs %s", id, another) - } -} - func TestAgent_AddService(t *testing.T) { t.Run("normal", func(t *testing.T) { t.Parallel() diff --git a/agent/nodeid.go b/agent/nodeid.go new file mode 100644 index 0000000000..06a9edaeda --- /dev/null +++ b/agent/nodeid.go @@ -0,0 +1,133 @@ +package agent + +import ( + "crypto/sha512" + "fmt" + "io/ioutil" + "os" + "path/filepath" + "strings" + + "github.com/hashicorp/consul/agent/config" + "github.com/hashicorp/consul/lib" + "github.com/hashicorp/consul/types" + "github.com/hashicorp/go-uuid" + "github.com/shirou/gopsutil/host" +) + +// setupNodeID will pull the persisted node ID, if any, or create a random one +// and persist it. +// FIXME: move to a different file. +func (a *Agent) setupNodeID(config *config.RuntimeConfig) error { + // If they've configured a node ID manually then just use that, as + // long as it's valid. + if config.NodeID != "" { + config.NodeID = types.NodeID(strings.ToLower(string(config.NodeID))) + if _, err := uuid.ParseUUID(string(config.NodeID)); err != nil { + return err + } + + return nil + } + + // For dev mode we have no filesystem access so just make one. + if a.config.DataDir == "" { + id, err := a.makeNodeID() + if err != nil { + return err + } + + config.NodeID = types.NodeID(id) + return nil + } + + // Load saved state, if any. Since a user could edit this, we also + // validate it. + fileID := filepath.Join(config.DataDir, "node-id") + if _, err := os.Stat(fileID); err == nil { + rawID, err := ioutil.ReadFile(fileID) + if err != nil { + return err + } + + nodeID := strings.TrimSpace(string(rawID)) + nodeID = strings.ToLower(nodeID) + if _, err := uuid.ParseUUID(nodeID); err != nil { + return err + } + + config.NodeID = types.NodeID(nodeID) + } + + // If we still don't have a valid node ID, make one. + if config.NodeID == "" { + id, err := a.makeNodeID() + if err != nil { + return err + } + if err := lib.EnsurePath(fileID, false); err != nil { + return err + } + if err := ioutil.WriteFile(fileID, []byte(id), 0600); err != nil { + return err + } + + config.NodeID = types.NodeID(id) + } + return nil +} + +// makeNodeID will try to find a host-specific ID, or else will generate a +// random ID. The returned ID will always be formatted as a GUID. We don't tell +// the caller whether this ID is random or stable since the consequences are +// high for us if this changes, so we will persist it either way. This will let +// gopsutil change implementations without affecting in-place upgrades of nodes. +func (a *Agent) makeNodeID() (string, error) { + // If they've disabled host-based IDs then just make a random one. + if a.config.DisableHostNodeID { + return a.makeRandomID() + } + + // Try to get a stable ID associated with the host itself. + info, err := host.Info() + if err != nil { + a.logger.Debug("Couldn't get a unique ID from the host", "error", err) + return a.makeRandomID() + } + + // Make sure the host ID parses as a UUID, since we don't have complete + // control over this process. + id := strings.ToLower(info.HostID) + if _, err := uuid.ParseUUID(id); err != nil { + a.logger.Debug("Unique ID from host isn't formatted as a UUID", + "id", id, + "error", err, + ) + return a.makeRandomID() + } + + // Hash the input to make it well distributed. The reported Host UUID may be + // similar across nodes if they are on a cloud provider or on motherboards + // created from the same batch. + buf := sha512.Sum512([]byte(id)) + id = fmt.Sprintf("%08x-%04x-%04x-%04x-%12x", + buf[0:4], + buf[4:6], + buf[6:8], + buf[8:10], + buf[10:16]) + + a.logger.Debug("Using unique ID from host as node ID", "id", id) + return id, nil +} + +// makeRandomID will generate a random UUID for a node. +func (a *Agent) makeRandomID() (string, error) { + id, err := uuid.GenerateUUID() + if err != nil { + return "", err + } + + a.logger.Debug("Using random ID as node ID", "id", id) + return id, nil +} diff --git a/agent/nodeid_test.go b/agent/nodeid_test.go new file mode 100644 index 0000000000..aeb45da4f3 --- /dev/null +++ b/agent/nodeid_test.go @@ -0,0 +1,125 @@ +package agent + +import ( + "io/ioutil" + "path/filepath" + "strings" + "testing" + + "github.com/hashicorp/consul/types" + "github.com/hashicorp/go-uuid" +) + +func TestSetupNodeID(t *testing.T) { + t.Parallel() + a := NewTestAgent(t, ` + node_id = "" + `) + defer a.Shutdown() + + cfg := a.config + + // The auto-assigned ID should be valid. + id := a.consulConfig().NodeID + if _, err := uuid.ParseUUID(string(id)); err != nil { + t.Fatalf("err: %v", err) + } + + // Running again should get the same ID (persisted in the file). + cfg.NodeID = "" + if err := a.setupNodeID(cfg); err != nil { + t.Fatalf("err: %v", err) + } + if newID := a.consulConfig().NodeID; id != newID { + t.Fatalf("bad: %q vs %q", id, newID) + } + + // Set an invalid ID via.Config. + cfg.NodeID = types.NodeID("nope") + err := a.setupNodeID(cfg) + if err == nil || !strings.Contains(err.Error(), "uuid string is wrong length") { + t.Fatalf("err: %v", err) + } + + // Set a valid ID via.Config. + newID, err := uuid.GenerateUUID() + if err != nil { + t.Fatalf("err: %v", err) + } + cfg.NodeID = types.NodeID(strings.ToUpper(newID)) + if err := a.setupNodeID(cfg); err != nil { + t.Fatalf("err: %v", err) + } + if id := a.consulConfig().NodeID; string(id) != newID { + t.Fatalf("bad: %q vs. %q", id, newID) + } + + // Set an invalid ID via the file. + fileID := filepath.Join(cfg.DataDir, "node-id") + if err := ioutil.WriteFile(fileID, []byte("adf4238a!882b!9ddc!4a9d!5b6758e4159e"), 0600); err != nil { + t.Fatalf("err: %v", err) + } + cfg.NodeID = "" + err = a.setupNodeID(cfg) + if err == nil || !strings.Contains(err.Error(), "uuid is improperly formatted") { + t.Fatalf("err: %v", err) + } + + // Set a valid ID via the file. + if err := ioutil.WriteFile(fileID, []byte("ADF4238a-882b-9ddc-4a9d-5b6758e4159e"), 0600); err != nil { + t.Fatalf("err: %v", err) + } + cfg.NodeID = "" + if err := a.setupNodeID(cfg); err != nil { + t.Fatalf("err: %v", err) + } + if id := a.consulConfig().NodeID; string(id) != "adf4238a-882b-9ddc-4a9d-5b6758e4159e" { + t.Fatalf("bad: %q vs. %q", id, newID) + } +} + +func TestMakeNodeID(t *testing.T) { + t.Parallel() + a := NewTestAgent(t, ` + node_id = "" + `) + defer a.Shutdown() + + // We should get a valid host-based ID initially. + id, err := a.makeNodeID() + if err != nil { + t.Fatalf("err: %v", err) + } + if _, err := uuid.ParseUUID(id); err != nil { + t.Fatalf("err: %v", err) + } + + // Calling again should yield a random ID by default. + another, err := a.makeNodeID() + if err != nil { + t.Fatalf("err: %v", err) + } + if id == another { + t.Fatalf("bad: %s vs %s", id, another) + } + + // Turn on host-based IDs and try again. We should get the same ID + // each time (and a different one from the random one above). + a.GetConfig().DisableHostNodeID = false + id, err = a.makeNodeID() + if err != nil { + t.Fatalf("err: %v", err) + } + if id == another { + t.Fatalf("bad: %s vs %s", id, another) + } + + // Calling again should yield the host-based ID. + another, err = a.makeNodeID() + if err != nil { + t.Fatalf("err: %v", err) + } + if id != another { + t.Fatalf("bad: %s vs %s", id, another) + } +}