From a9155e8038faa75ac9bb27d98b25320136617743 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Wed, 10 Aug 2016 15:09:16 -0400 Subject: [PATCH 1/2] Fix Cluster object being returned as nil when unsealed --- http/sys_health.go | 5 --- http/sys_seal.go | 6 ---- vault/cluster.go | 88 ++++++++++++++++++++++++++++------------------ 3 files changed, 54 insertions(+), 45 deletions(-) diff --git a/http/sys_health.go b/http/sys_health.go index 338392d4a5..889cd3f6f1 100644 --- a/http/sys_health.go +++ b/http/sys_health.go @@ -118,14 +118,9 @@ func getSysHealth(core *vault.Core, r *http.Request) (int, *HealthResponse, erro var clusterName, clusterID string if !sealed { cluster, err := core.Cluster() - - // Don't set the cluster details in the health status when Vault is sealed if err != nil { return http.StatusInternalServerError, nil, err } - if cluster == nil { - return http.StatusInternalServerError, nil, nil - } clusterName = cluster.Name clusterID = cluster.ID } diff --git a/http/sys_seal.go b/http/sys_seal.go index 7b6dde3c2c..1551dc9da0 100644 --- a/http/sys_seal.go +++ b/http/sys_seal.go @@ -155,16 +155,10 @@ func handleSysSealStatusRaw(core *vault.Core, w http.ResponseWriter, r *http.Req var clusterName, clusterID string if !sealed { cluster, err := core.Cluster() - - // Don't set the cluster details in the status when Vault is sealed if err != nil { respondError(w, http.StatusInternalServerError, err) return } - if cluster == nil { - respondError(w, http.StatusInternalServerError, nil) - return - } clusterName = cluster.Name clusterID = cluster.ID } diff --git a/vault/cluster.go b/vault/cluster.go index ffdbb01b68..b10604b8cc 100644 --- a/vault/cluster.go +++ b/vault/cluster.go @@ -23,21 +23,23 @@ type Cluster struct { } // Cluster fetches the details of either local or global cluster based on the -// input. This method errors out when Vault is sealed. +// input. This method errors out when Vault is sealed. This function never +// returns a nil Cluster object. func (c *Core) Cluster() (*Cluster, error) { + var cluster Cluster + // Fetch the storage entry. This call fails when Vault is sealed. entry, err := c.barrier.Get(coreLocalClusterInfoPath) if err != nil { - return nil, err + return &cluster, err } if entry == nil { - return nil, nil + return &cluster, nil } // Decode the cluster information - var cluster Cluster if err = jsonutil.DecodeJSON(entry.Value, &cluster); err != nil { - return nil, fmt.Errorf("failed to decode cluster details: %v", err) + return &cluster, fmt.Errorf("failed to decode cluster details: %v", err) } // Set in config file @@ -58,42 +60,60 @@ func (c *Core) setupCluster() error { c.logger.Printf("[ERR] core: failed to get cluster details: %v", err) return err } - if cluster != nil { - // If index is already present, don't update it - return nil + + if cluster == nil { + cluster = &Cluster{} } - // If cluster name is not supplied, generate one - if c.clusterName == "" { - clusterNameBytes, err := uuid.GenerateRandomBytes(4) + var modified bool + + if cluster.Name == "" { + // If cluster name is not supplied, generate one + if c.clusterName == "" { + c.logger.Printf("[TRACE] core: cluster name not found/set, generating new") + clusterNameBytes, err := uuid.GenerateRandomBytes(4) + if err != nil { + c.logger.Printf("[ERR] core: failed to generate cluster name: %v", err) + return err + } + c.clusterName = fmt.Sprintf("vault-cluster-%08x", clusterNameBytes) + } + + cluster.Name = c.clusterName + c.logger.Printf("[DEBUG] core: cluster name set to %s", cluster.Name) + modified = true + } + + if cluster.ID == "" { + // Generate a clusterID + clusterID, err := uuid.GenerateUUID() if err != nil { - c.logger.Printf("[ERR] core: failed to generate cluster name: %v", err) + c.logger.Printf("[ERR] core: failed to generate cluster identifier: %v", err) return err } - c.clusterName = fmt.Sprintf("vault-cluster-%08x", clusterNameBytes) + cluster.ID = clusterID + c.logger.Printf("[DEBUG] core: cluster ID set to %s", cluster.ID) + modified = true } - // Generate a clusterID - clusterID, err := uuid.GenerateUUID() - if err != nil { - c.logger.Printf("[ERR] core: failed to generate cluster identifier: %v", err) - return err + if modified { + // Encode the cluster information into as a JSON string + rawCluster, err := json.Marshal(cluster) + if err != nil { + c.logger.Printf("[ERR] core: failed to encode cluster details: %v", err) + return err + } + + // Store it + err = c.barrier.Put(&Entry{ + Key: coreLocalClusterInfoPath, + Value: rawCluster, + }) + if err != nil { + c.logger.Printf("[ERR] core: failed to store cluster details: %v", err) + return err + } } - // Encode the cluster information into as a JSON string - rawCluster, err := json.Marshal(&Cluster{ - Name: c.clusterName, - ID: clusterID, - }) - if err != nil { - c.logger.Printf("[ERR] core: failed to encode cluster details: %v", err) - return err - } - - // Store it - return c.barrier.Put(&Entry{ - Key: coreLocalClusterInfoPath, - Value: rawCluster, - }) - + return nil } From baa1a1c9cff1ae989d205c02b694288bb9cc54e2 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Wed, 10 Aug 2016 15:22:12 -0400 Subject: [PATCH 2/2] Address review feedback from @jefferai --- http/sys_health.go | 4 ++++ http/sys_seal.go | 4 ++++ vault/cluster.go | 7 +++---- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/http/sys_health.go b/http/sys_health.go index 889cd3f6f1..da5922c5be 100644 --- a/http/sys_health.go +++ b/http/sys_health.go @@ -2,6 +2,7 @@ package http import ( "encoding/json" + "fmt" "net/http" "strconv" "time" @@ -121,6 +122,9 @@ func getSysHealth(core *vault.Core, r *http.Request) (int, *HealthResponse, erro if err != nil { return http.StatusInternalServerError, nil, err } + if cluster == nil { + return http.StatusInternalServerError, nil, fmt.Errorf("failed to fetch cluster details") + } clusterName = cluster.Name clusterID = cluster.ID } diff --git a/http/sys_seal.go b/http/sys_seal.go index 1551dc9da0..9a75f38443 100644 --- a/http/sys_seal.go +++ b/http/sys_seal.go @@ -159,6 +159,10 @@ func handleSysSealStatusRaw(core *vault.Core, w http.ResponseWriter, r *http.Req respondError(w, http.StatusInternalServerError, err) return } + if cluster == nil { + respondError(w, http.StatusInternalServerError, fmt.Errorf("failed to fetch cluster details")) + return + } clusterName = cluster.Name clusterID = cluster.ID } diff --git a/vault/cluster.go b/vault/cluster.go index b10604b8cc..b0a9b0c224 100644 --- a/vault/cluster.go +++ b/vault/cluster.go @@ -23,15 +23,14 @@ type Cluster struct { } // Cluster fetches the details of either local or global cluster based on the -// input. This method errors out when Vault is sealed. This function never -// returns a nil Cluster object. +// input. This method errors out when Vault is sealed. func (c *Core) Cluster() (*Cluster, error) { var cluster Cluster // Fetch the storage entry. This call fails when Vault is sealed. entry, err := c.barrier.Get(coreLocalClusterInfoPath) if err != nil { - return &cluster, err + return nil, err } if entry == nil { return &cluster, nil @@ -39,7 +38,7 @@ func (c *Core) Cluster() (*Cluster, error) { // Decode the cluster information if err = jsonutil.DecodeJSON(entry.Value, &cluster); err != nil { - return &cluster, fmt.Errorf("failed to decode cluster details: %v", err) + return nil, fmt.Errorf("failed to decode cluster details: %v", err) } // Set in config file