Modularize Run Command (#11573)

* initial refactoring of unseal step in run

* remove waitgroup

* remove waitgroup

* backup work

* backup

* backup

* completely modularize run and move into diagnose

* add diagnose errors for incorrect number of unseal keys

* comment tests back in

* backup

* first subspan

* finished subspanning but running into error with timeouts

* remove runtime checks

* meeting updates

* remove telemetry block

* roy comment

* subspans for seal finalization and wrapping diagnose latency checks

* fix storage latency test errors

* review comments

* use random uuid for latency checks instead of static id

command/operator_diagnose.go

@@ -4,18 +4,26 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io"
 	"os"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/hashicorp/consul/api"
 	log "github.com/hashicorp/go-hclog"
+	uuid "github.com/hashicorp/go-uuid"
+	"github.com/hashicorp/vault/helper/metricsutil"
+	"github.com/hashicorp/vault/internalshared/configutil"
 	"github.com/hashicorp/vault/internalshared/listenerutil"
 	"github.com/hashicorp/vault/internalshared/reloadutil"
 	physconsul "github.com/hashicorp/vault/physical/consul"
+	"github.com/hashicorp/vault/sdk/physical"
 	"github.com/hashicorp/vault/sdk/version"
+	sr "github.com/hashicorp/vault/serviceregistration"
 	srconsul "github.com/hashicorp/vault/serviceregistration/consul"
+	"github.com/hashicorp/vault/vault"
 	"github.com/hashicorp/vault/vault/diagnose"
 	"github.com/mitchellh/cli"
 	"github.com/posener/complete"
@@ -23,6 +31,10 @@ import (
 
 const OperatorDiagnoseEnableEnv = "VAULT_DIAGNOSE"
 
+const CoreUninitializedErr = "diagnose cannot attempt this step because core could not be initialized"
+const BackendUninitializedErr = "diagnose cannot attempt this step because backend could not be initialized"
+const CoreConfigUninitializedErr = "diagnose cannot attempt this step because core config could not be set"
+
 var (
 	_ cli.Command             = (*OperatorDiagnoseCommand)(nil)
 	_ cli.CommandAutocomplete = (*OperatorDiagnoseCommand)(nil)
@@ -37,11 +49,12 @@ type OperatorDiagnoseCommand struct {
 	flagConfigs  []string
 	cleanupGuard sync.Once
 
-	reloadFuncsLock *sync.RWMutex
+	reloadFuncsLock      *sync.RWMutex
-	reloadFuncs     *map[string][]reloadutil.ReloadFunc
+	reloadFuncs          *map[string][]reloadutil.ReloadFunc
-	startedCh       chan struct{} // for tests
+	ServiceRegistrations map[string]sr.Factory
-	reloadedCh      chan struct{} // for tests
+	startedCh            chan struct{} // for tests
-	skipEndEnd      bool          // for tests
+	reloadedCh           chan struct{} // for tests
+	skipEndEnd           bool          // for tests
 }
 
 func (c *OperatorDiagnoseCommand) Synopsis() string {
@@ -203,18 +216,230 @@ func (c *OperatorDiagnoseCommand) offlineDiagnostics(ctx context.Context) error
 	} else {
 		diagnose.SpotOk(ctx, "parse-config", "")
 	}
-	// Check Listener Information
-	// TODO: Run Diagnose checks on the actual net.Listeners
 
-	if err := diagnose.Test(ctx, "init-listeners", func(ctx context.Context) error {
+	var metricSink *metricsutil.ClusterMetricSink
+	var metricsHelper *metricsutil.MetricsHelper
+
+	var backend *physical.Backend
+	diagnose.Test(ctx, "storage", func(ctx context.Context) error {
+		diagnose.Test(ctx, "create-storage-backend", func(ctx context.Context) error {
+
+			b, err := server.setupStorage(config)
+			if err != nil {
+				return err
+			}
+			backend = &b
+			return nil
+		})
+
+		if config.Storage == nil {
+			return fmt.Errorf("no storage stanza found in config")
+		}
+
+		if config.Storage != nil && config.Storage.Type == storageTypeConsul {
+			diagnose.Test(ctx, "test-storage-tls-consul", func(ctx context.Context) error {
+				err = physconsul.SetupSecureTLS(api.DefaultConfig(), config.Storage.Config, server.logger, true)
+				if err != nil {
+					return err
+				}
+				return nil
+			})
+
+			diagnose.Test(ctx, "test-consul-direct-access-storage", func(ctx context.Context) error {
+				dirAccess := diagnose.ConsulDirectAccess(config.Storage.Config)
+				if dirAccess != "" {
+					diagnose.Warn(ctx, dirAccess)
+				}
+				return nil
+			})
+		}
+
+		// Attempt to use storage backend
+		if !c.skipEndEnd {
+			diagnose.Test(ctx, "test-access-storage", diagnose.WithTimeout(30*time.Second, func(ctx context.Context) error {
+				maxDurationCrudOperation := "write"
+				maxDuration := time.Duration(0)
+				uuidSuffix, err := uuid.GenerateUUID()
+				if err != nil {
+					return err
+				}
+				uuid := "diagnose/latency/" + uuidSuffix
+				dur, err := diagnose.EndToEndLatencyCheckWrite(ctx, uuid, *backend)
+				if err != nil {
+					return err
+				}
+				maxDuration = dur
+				dur, err = diagnose.EndToEndLatencyCheckRead(ctx, uuid, *backend)
+				if err != nil {
+					return err
+				}
+				if dur > maxDuration {
+					maxDuration = dur
+					maxDurationCrudOperation = "read"
+				}
+				dur, err = diagnose.EndToEndLatencyCheckDelete(ctx, uuid, *backend)
+				if err != nil {
+					return err
+				}
+				if dur > maxDuration {
+					maxDuration = dur
+					maxDurationCrudOperation = "delete"
+				}
+
+				if maxDuration > time.Duration(0) {
+					diagnose.Warn(ctx, diagnose.LatencyWarning+fmt.Sprintf("duration: %s, ", maxDuration)+fmt.Sprintf("operation: %s", maxDurationCrudOperation))
+				}
+				return nil
+			}))
+		}
+		return nil
+	})
+
+	var configSR sr.ServiceRegistration
+	diagnose.Test(ctx, "service-discovery", func(ctx context.Context) error {
+		if config.ServiceRegistration == nil || config.ServiceRegistration.Config == nil {
+			return fmt.Errorf("No service registration config")
+		}
+		srConfig := config.ServiceRegistration.Config
+
+		diagnose.Test(ctx, "test-serviceregistration-tls-consul", func(ctx context.Context) error {
+			// SetupSecureTLS for service discovery uses the same cert and key to set up physical
+			// storage. See the consul package in physical for details.
+			err = srconsul.SetupSecureTLS(api.DefaultConfig(), srConfig, server.logger, true)
+			if err != nil {
+				return err
+			}
+			return nil
+		})
+
+		if config.ServiceRegistration != nil && config.ServiceRegistration.Type == "consul" {
+			diagnose.Test(ctx, "test-consul-direct-access-service-discovery", func(ctx context.Context) error {
+				dirAccess := diagnose.ConsulDirectAccess(config.ServiceRegistration.Config)
+				if dirAccess != "" {
+					diagnose.Warn(ctx, dirAccess)
+				}
+				return nil
+			})
+		}
+		return nil
+	})
+
+	sealcontext, sealspan := diagnose.StartSpan(ctx, "create-seal")
+	var seals []vault.Seal
+	var sealConfigError error
+	barrierSeal, barrierWrapper, unwrapSeal, seals, sealConfigError, err := setSeal(server, config, make([]string, 0), make(map[string]string))
+	// Check error here
+	if err != nil {
+		diagnose.Fail(sealcontext, err.Error())
+		goto SEALFAIL
+	}
+	if sealConfigError != nil {
+		diagnose.Fail(sealcontext, "seal could not be configured: seals may already be initialized")
+		goto SEALFAIL
+	}
+
+	if seals != nil {
+		for _, seal := range seals {
+			// Ensure that the seal finalizer is called, even if using verify-only
+			defer func(seal *vault.Seal) {
+				sealType := (*seal).BarrierType()
+				finalizeSealContext, finalizeSealSpan := diagnose.StartSpan(ctx, "finalize-seal-"+sealType)
+				err = (*seal).Finalize(finalizeSealContext)
+				if err != nil {
+					diagnose.Fail(finalizeSealContext, "error finalizing seal")
+					finalizeSealSpan.End()
+				}
+				finalizeSealSpan.End()
+			}(&seal)
+		}
+	}
+
+	if barrierSeal == nil {
+		diagnose.Fail(sealcontext, "could not create barrier seal! Most likely proper Seal configuration information was not set, but no error was generated")
+	}
+
+SEALFAIL:
+	sealspan.End()
+	var coreConfig vault.CoreConfig
+	if err := diagnose.Test(ctx, "setup-core", func(ctx context.Context) error {
+		var secureRandomReader io.Reader
+		// prepare a secure random reader for core
+		secureRandomReader, err = configutil.CreateSecureRandomReaderFunc(config.SharedConfig, barrierWrapper)
+		if err != nil {
+			return diagnose.SpotError(ctx, "init-randreader", err)
+		}
+		diagnose.SpotOk(ctx, "init-randreader", "")
+
+		if backend == nil {
+			return fmt.Errorf(BackendUninitializedErr)
+		}
+		coreConfig = createCoreConfig(server, config, *backend, configSR, barrierSeal, unwrapSeal, metricsHelper, metricSink, secureRandomReader)
+		return nil
+	}); err != nil {
+		diagnose.Error(ctx, err)
+	}
+
+	var disableClustering bool
+	diagnose.Test(ctx, "setup-ha-storage", func(ctx context.Context) error {
+		if backend == nil {
+			return fmt.Errorf(BackendUninitializedErr)
+		}
+		diagnose.Test(ctx, "create-ha-storage-backend", func(ctx context.Context) error {
+			// Initialize the separate HA storage backend, if it exists
+			disableClustering, err = initHaBackend(server, config, &coreConfig, *backend)
+			if err != nil {
+				return err
+			}
+			return nil
+		})
+		diagnose.Test(ctx, "test-consul-direct-access-storage", func(ctx context.Context) error {
+			dirAccess := diagnose.ConsulDirectAccess(config.HAStorage.Config)
+			if dirAccess != "" {
+				diagnose.Warn(ctx, dirAccess)
+			}
+			return nil
+		})
+		if config.HAStorage != nil && config.HAStorage.Type == storageTypeConsul {
+			diagnose.Test(ctx, "test-storage-tls-consul", func(ctx context.Context) error {
+				err = physconsul.SetupSecureTLS(api.DefaultConfig(), config.HAStorage.Config, server.logger, true)
+				if err != nil {
+					return err
+				}
+				return nil
+			})
+		}
+		return nil
+	})
+
+	// Determine the redirect address from environment variables
+	err = determineRedirectAddr(server, &coreConfig, config)
+	if err != nil {
+		return diagnose.SpotError(ctx, "determine-redirect", err)
+	}
+	diagnose.SpotOk(ctx, "determine-redirect", "")
+
+	err = findClusterAddress(server, &coreConfig, config, disableClustering)
+	if err != nil {
+		return diagnose.SpotError(ctx, "find-cluster-addr", err)
+	}
+	diagnose.SpotOk(ctx, "find-cluster-addr", "")
+
+	var lns []listenerutil.Listener
+	diagnose.Test(ctx, "init-listeners", func(ctx context.Context) error {
 		disableClustering := config.HAStorage.DisableClustering
 		infoKeys := make([]string, 0, 10)
 		info := make(map[string]string)
-		status, lns, _, errMsg := server.InitListeners(config, disableClustering, &infoKeys, &info)
+		var listeners []listenerutil.Listener
+		var status int
+		diagnose.Test(ctx, "create-listeners", func(ctx context.Context) error {
+			status, listeners, _, err = server.InitListeners(config, disableClustering, &infoKeys, &info)
+			if status != 0 {
+				return err
+			}
+			return nil
+		})
 
-		if status != 0 {
+		lns = listeners
-			return errMsg
-		}
 
 		// Make sure we close all listeners from this point on
 		listenerCloseFunc := func() {
@@ -225,96 +450,39 @@ func (c *OperatorDiagnoseCommand) offlineDiagnostics(ctx context.Context) error
 
 		defer c.cleanupGuard.Do(listenerCloseFunc)
 
-		sanitizedListeners := make([]listenerutil.Listener, 0, len(config.Listeners))
+		diagnose.Test(ctx, "check-listener-tls", func(ctx context.Context) error {
-		for _, ln := range lns {
+			sanitizedListeners := make([]listenerutil.Listener, 0, len(config.Listeners))
-			if ln.Config.TLSDisable {
+			for _, ln := range lns {
-				diagnose.Warn(ctx, "TLS is disabled in a Listener config stanza.")
+				if ln.Config.TLSDisable {
-				continue
+					diagnose.Warn(ctx, "TLS is disabled in a Listener config stanza.")
-			}
+					continue
-			if ln.Config.TLSDisableClientCerts {
+				}
-				diagnose.Warn(ctx, "TLS for a listener is turned on without requiring client certs.")
+				if ln.Config.TLSDisableClientCerts {
-			}
+					diagnose.Warn(ctx, "TLS for a listener is turned on without requiring client certs.")
+				}
 
-			// Check ciphersuite and load ca/cert/key files
+				// Check ciphersuite and load ca/cert/key files
-			// TODO: TLSConfig returns a reloadFunc and a TLSConfig. We can use this to
+				// TODO: TLSConfig returns a reloadFunc and a TLSConfig. We can use this to
-			// perform an active probe.
+				// perform an active probe.
-			_, _, err := listenerutil.TLSConfig(ln.Config, make(map[string]string), c.UI)
+				_, _, err := listenerutil.TLSConfig(ln.Config, make(map[string]string), c.UI)
+				if err != nil {
+					return err
+				}
+
+				sanitizedListeners = append(sanitizedListeners, listenerutil.Listener{
+					Listener: ln.Listener,
+					Config:   ln.Config,
+				})
+			}
+			err = diagnose.ListenerChecks(sanitizedListeners)
 			if err != nil {
 				return err
 			}
-
+			return nil
-			sanitizedListeners = append(sanitizedListeners, listenerutil.Listener{
+		})
-				Listener: ln.Listener,
-				Config:   ln.Config,
-			})
-		}
-		return diagnose.ListenerChecks(sanitizedListeners)
-	}); err != nil {
-		return err
-	}
-
-	// Errors in these items could stop Vault from starting but are not yet covered:
-	// TODO: logging configuration
-	// TODO: SetupTelemetry
-	if err := diagnose.Test(ctx, "storage", func(ctx context.Context) error {
-		b, err := server.setupStorage(config)
-		if err != nil {
-			return err
-		}
-
-		dirAccess := diagnose.ConsulDirectAccess(config.HAStorage.Config)
-		if dirAccess != "" {
-			diagnose.Warn(ctx, dirAccess)
-		}
-
-		if config.Storage != nil && config.Storage.Type == storageTypeConsul {
-			err = physconsul.SetupSecureTLS(api.DefaultConfig(), config.Storage.Config, server.logger, true)
-			if err != nil {
-				return err
-			}
-
-			dirAccess := diagnose.ConsulDirectAccess(config.Storage.Config)
-			if dirAccess != "" {
-				diagnose.Warn(ctx, dirAccess)
-			}
-		}
-
-		if config.HAStorage != nil && config.HAStorage.Type == storageTypeConsul {
-			err = physconsul.SetupSecureTLS(api.DefaultConfig(), config.HAStorage.Config, server.logger, true)
-			if err != nil {
-				return err
-			}
-		}
-
-		// Attempt to use storage backend
-		if !c.skipEndEnd {
-			err = diagnose.StorageEndToEndLatencyCheck(ctx, b)
-			if err != nil {
-				return err
-			}
-		}
-
-		return nil
-	}); err != nil {
-		return err
-	}
-
-	diagnose.Test(ctx, "service-discovery", func(ctx context.Context) error {
-		srConfig := config.ServiceRegistration.Config
-		// Initialize the Service Discovery, if there is one
-		if config.ServiceRegistration != nil && config.ServiceRegistration.Type == "consul" {
-			// setupStorage populates the srConfig, so no nil checks are necessary.
-			dirAccess := diagnose.ConsulDirectAccess(config.ServiceRegistration.Config)
-			if dirAccess != "" {
-				diagnose.Warn(ctx, dirAccess)
-			}
-
-			// SetupSecureTLS for service discovery uses the same cert and key to set up physical
-			// storage. See the consul package in physical for details.
-			return srconsul.SetupSecureTLS(api.DefaultConfig(), srConfig, server.logger, true)
-		}
 		return nil
 	})
 
+	// TODO: Diagnose logging configuration
 	return nil
 }

command/operator_diagnose_test.go

@@ -44,18 +44,96 @@ func TestOperatorDiagnoseCommand_Run(t *testing.T) {
 					Status: diagnose.OkStatus,
 				},
 				{
-					Name:   "init-listeners",
+					Name:   "storage",
-					Status: diagnose.WarningStatus,
+					Status: diagnose.OkStatus,
-					Warnings: []string{
+					Children: []*diagnose.Result{
-						"TLS is disabled in a Listener config stanza.",
+						{
+							Name:   "create-storage-backend",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-storage-tls-consul",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-consul-direct-access-storage",
+							Status: diagnose.OkStatus,
+						},
 					},
 				},
 				{
-					Name:   "storage",
+					Name:   "service-discovery",
+					Status: diagnose.OkStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "test-serviceregistration-tls-consul",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-consul-direct-access-service-discovery",
+							Status: diagnose.OkStatus,
+						},
+					},
+				},
+				{
+					Name:   "create-seal",
 					Status: diagnose.OkStatus,
 				},
 				{
-					Name:   "service-discovery",
+					Name:   "setup-core",
+					Status: diagnose.OkStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "init-randreader",
+							Status: diagnose.OkStatus,
+						},
+					},
+				},
+				{
+					Name:   "setup-ha-storage",
+					Status: diagnose.OkStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "create-ha-storage-backend",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-consul-direct-access-storage",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-storage-tls-consul",
+							Status: diagnose.OkStatus,
+						},
+					},
+				},
+				{
+					Name:   "determine-redirect",
+					Status: diagnose.OkStatus,
+				},
+				{
+					Name:   "find-cluster-addr",
+					Status: diagnose.OkStatus,
+				},
+				{
+					Name:   "init-listeners",
+					Status: diagnose.WarningStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "create-listeners",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "check-listener-tls",
+							Status: diagnose.WarningStatus,
+							Warnings: []string{
+								"TLS is disabled in a Listener config stanza.",
+							},
+						},
+					},
+				},
+				{
+					Name:   "finalize-seal-shamir",
 					Status: diagnose.OkStatus,
 				},
 			},
@@ -71,16 +149,68 @@ func TestOperatorDiagnoseCommand_Run(t *testing.T) {
 					Status: diagnose.OkStatus,
 				},
 				{
-					Name:   "init-listeners",
+					Name:    "storage",
-					Status: diagnose.WarningStatus,
+					Status:  diagnose.ErrorStatus,
-					Warnings: []string{
+					Message: "no storage stanza found in config",
-						"TLS is disabled in a Listener config stanza.",
+					Children: []*diagnose.Result{
+						{
+							Name:   "create-storage-backend",
+							Status: diagnose.ErrorStatus,
+						},
 					},
 				},
 				{
-					Name:    "storage",
+					Name:   "service-discovery",
+					Status: diagnose.ErrorStatus,
+				},
+				{
+					Name:   "create-seal",
+					Status: diagnose.OkStatus,
+				},
+				{
+					Name:    "setup-core",
 					Status:  diagnose.ErrorStatus,
-					Message: "A storage backend must be specified",
+					Message: BackendUninitializedErr,
+					Children: []*diagnose.Result{
+						{
+							Name:   "init-randreader",
+							Status: diagnose.OkStatus,
+						},
+					},
+				},
+				{
+					Name:    "setup-ha-storage",
+					Status:  diagnose.ErrorStatus,
+					Message: BackendUninitializedErr,
+				},
+				{
+					Name:   "determine-redirect",
+					Status: diagnose.OkStatus,
+				},
+				{
+					Name:   "find-cluster-addr",
+					Status: diagnose.OkStatus,
+				},
+				{
+					Name:   "init-listeners",
+					Status: diagnose.WarningStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "create-listeners",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "check-listener-tls",
+							Status: diagnose.WarningStatus,
+							Warnings: []string{
+								"TLS is disabled in a Listener config stanza.",
+							},
+						},
+					},
+				},
+				{
+					Name:   "finalize-seal-shamir",
+					Status: diagnose.OkStatus,
 				},
 			},
 		},
@@ -94,17 +224,98 @@ func TestOperatorDiagnoseCommand_Run(t *testing.T) {
 					Name:   "parse-config",
 					Status: diagnose.OkStatus,
 				},
-				{
-					Name:   "init-listeners",
-					Status: diagnose.OkStatus,
-				},
 				{
 					Name:   "storage",
 					Status: diagnose.OkStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "create-storage-backend",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-storage-tls-consul",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-consul-direct-access-storage",
+							Status: diagnose.OkStatus,
+						},
+					},
 				},
 				{
 					Name:   "service-discovery",
 					Status: diagnose.OkStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "test-serviceregistration-tls-consul",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-consul-direct-access-service-discovery",
+							Status: diagnose.OkStatus,
+						},
+					},
+				},
+				{
+					Name:   "create-seal",
+					Status: diagnose.OkStatus,
+				},
+				{
+					Name:   "setup-core",
+					Status: diagnose.OkStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "init-randreader",
+							Status: diagnose.OkStatus,
+						},
+					},
+				},
+				{
+					Name:   "setup-ha-storage",
+					Status: diagnose.OkStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "create-ha-storage-backend",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-consul-direct-access-storage",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-storage-tls-consul",
+							Status: diagnose.OkStatus,
+						},
+					},
+				},
+				{
+					Name:   "determine-redirect",
+					Status: diagnose.OkStatus,
+				},
+				{
+					Name:   "find-cluster-addr",
+					Status: diagnose.OkStatus,
+				},
+				{
+					Name:   "init-listeners",
+					Status: diagnose.OkStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "create-listeners",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "check-listener-tls",
+							Status: diagnose.WarningStatus,
+							Warnings: []string{
+								"TLS is disabled in a Listener config stanza.",
+							},
+						},
+					},
+				},
+				{
+					Name:   "finalize-seal-shamir",
+					Status: diagnose.OkStatus,
 				},
 			},
 		},
@@ -119,15 +330,69 @@ func TestOperatorDiagnoseCommand_Run(t *testing.T) {
 					Status: diagnose.OkStatus,
 				},
 				{
-					Name:   "init-listeners",
+					Name:   "storage",
-					Status: diagnose.WarningStatus,
+					Status: diagnose.ErrorStatus,
-					Warnings: []string{
+					Children: []*diagnose.Result{
-						"TLS is disabled in a Listener config stanza.",
+						{
+							Name:   "create-storage-backend",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:    "test-storage-tls-consul",
+							Status:  diagnose.ErrorStatus,
+							Message: "expired",
+						},
+						{
+							Name:   "test-consul-direct-access-storage",
+							Status: diagnose.OkStatus,
+						},
 					},
 				},
 				{
-					Name:   "storage",
+					Name:   "service-discovery",
-					Status: diagnose.ErrorStatus,
+					Status: diagnose.WarningStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "test-serviceregistration-tls-consul",
+							Status: diagnose.WarningStatus,
+						},
+						{
+							Name:   "test-consul-direct-access-service-discovery",
+							Status: diagnose.OkStatus,
+						},
+					},
+				},
+				{
+					Name:   "create-seal",
+					Status: diagnose.OkStatus,
+				},
+				{
+					Name:   "setup-core",
+					Status: diagnose.OkStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "init-randreader",
+							Status: diagnose.OkStatus,
+						},
+					},
+				},
+				{
+					Name:   "setup-ha-storage",
+					Status: diagnose.OkStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "create-ha-storage-backend",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-consul-direct-access-storage",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-storage-tls-consul",
+							Status: diagnose.OkStatus,
+						},
+					},
 				},
 			},
 		},
@@ -142,17 +407,68 @@ func TestOperatorDiagnoseCommand_Run(t *testing.T) {
 					Status: diagnose.OkStatus,
 				},
 				{
-					Name:   "init-listeners",
+					Name:   "storage",
 					Status: diagnose.WarningStatus,
-					Warnings: []string{
+					Children: []*diagnose.Result{
-						"TLS is disabled in a Listener config stanza.",
+						{
+							Name:   "create-storage-backend",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-storage-tls-consul",
+							Status: diagnose.WarningStatus,
+						},
+						{
+							Name:   "test-consul-direct-access-storage",
+							Status: diagnose.OkStatus,
+						},
 					},
 				},
 				{
-					Name:   "storage",
+					Name:   "service-discovery",
+					Status: diagnose.OkStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "test-serviceregistration-tls-consul",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-consul-direct-access-service-discovery",
+							Status: diagnose.OkStatus,
+						},
+					},
+				},
+				{
+					Name:   "create-seal",
+					Status: diagnose.OkStatus,
+				},
+				{
+					Name:   "setup-core",
+					Status: diagnose.OkStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "init-randreader",
+							Status: diagnose.OkStatus,
+						},
+					},
+				},
+				{
+					Name:   "setup-ha-storage",
 					Status: diagnose.ErrorStatus,
-					Warnings: []string{
+					Children: []*diagnose.Result{
-						diagnose.AddrDNExistErr,
+						{
+							Name:   "create-ha-storage-backend",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-consul-direct-access-storage",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:    "test-storage-tls-consul",
+							Status:  diagnose.ErrorStatus,
+							Message: "x509: certificate has expired or is not yet valid",
+						},
 					},
 				},
 			},
@@ -168,22 +484,39 @@ func TestOperatorDiagnoseCommand_Run(t *testing.T) {
 					Status: diagnose.OkStatus,
 				},
 				{
-					Name:   "init-listeners",
+					Name:   "storage",
-					Status: diagnose.WarningStatus,
+					Status: diagnose.OkStatus,
-					Warnings: []string{
+					Children: []*diagnose.Result{
-						"TLS is disabled in a Listener config stanza.",
+						{
+							Name:   "create-storage-backend",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-storage-tls-consul",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-consul-direct-access-storage",
+							Status: diagnose.OkStatus,
+						},
 					},
 				},
 				{
-					Name:   "storage",
+					Name:   "service-discovery",
-					Status: diagnose.OkStatus,
+					Status: diagnose.ErrorStatus,
-				},
+					Children: []*diagnose.Result{
-				{
+						{
-					Name:    "service-discovery",
+							Name:    "test-serviceregistration-tls-consul",
-					Status:  diagnose.ErrorStatus,
+							Status:  diagnose.ErrorStatus,
-					Message: "failed to verify certificate: x509: certificate has expired or is not yet valid:",
+							Message: "failed to verify certificate: x509: certificate has expired or is not yet valid",
-					Warnings: []string{
+						},
-						diagnose.DirAccessErr,
+						{
+							Name:   "test-consul-direct-access-service-discovery",
+							Status: diagnose.WarningStatus,
+							Warnings: []string{
+								diagnose.DirAccessErr,
+							},
+						},
 					},
 				},
 			},
@@ -198,23 +531,101 @@ func TestOperatorDiagnoseCommand_Run(t *testing.T) {
 					Name:   "parse-config",
 					Status: diagnose.OkStatus,
 				},
-				{
-					Name:   "init-listeners",
-					Status: diagnose.WarningStatus,
-					Warnings: []string{
-						"TLS is disabled in a Listener config stanza.",
-					},
-				},
 				{
 					Name:   "storage",
 					Status: diagnose.WarningStatus,
-					Warnings: []string{
+					Children: []*diagnose.Result{
-						diagnose.DirAccessErr,
+						{
+							Name:   "create-storage-backend",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-storage-tls-consul",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-consul-direct-access-storage",
+							Status: diagnose.WarningStatus,
+							Warnings: []string{
+								diagnose.DirAccessErr,
+							},
+						},
 					},
 				},
 				{
 					Name:   "service-discovery",
 					Status: diagnose.OkStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "test-serviceregistration-tls-consul",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-consul-direct-access-service-discovery",
+							Status: diagnose.OkStatus,
+						},
+					},
+				},
+				{
+					Name:   "create-seal",
+					Status: diagnose.OkStatus,
+				},
+				{
+					Name:   "setup-core",
+					Status: diagnose.OkStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "init-randreader",
+							Status: diagnose.OkStatus,
+						},
+					},
+				},
+				{
+					Name:   "setup-ha-storage",
+					Status: diagnose.OkStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "create-ha-storage-backend",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-consul-direct-access-storage",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "test-storage-tls-consul",
+							Status: diagnose.OkStatus,
+						},
+					},
+				},
+				{
+					Name:   "determine-redirect",
+					Status: diagnose.OkStatus,
+				},
+				{
+					Name:   "find-cluster-addr",
+					Status: diagnose.OkStatus,
+				},
+				{
+					Name:   "init-listeners",
+					Status: diagnose.WarningStatus,
+					Children: []*diagnose.Result{
+						{
+							Name:   "create-listeners",
+							Status: diagnose.OkStatus,
+						},
+						{
+							Name:   "check-listener-tls",
+							Status: diagnose.WarningStatus,
+							Warnings: []string{
+								"TLS is disabled in a Listener config stanza.",
+							},
+						},
+					},
+				},
+				{
+					Name:   "finalize-seal-shamir",
+					Status: diagnose.OkStatus,
 				},
 			},
 		},
@@ -225,7 +636,6 @@ func TestOperatorDiagnoseCommand_Run(t *testing.T) {
 
 		for _, tc := range cases {
 			tc := tc
-
 			t.Run(tc.name, func(t *testing.T) {
 				t.Parallel()
 				client, closer := testVaultServer(t)
@@ -238,9 +648,12 @@ func TestOperatorDiagnoseCommand_Run(t *testing.T) {
 				result := cmd.diagnose.Finalize(context.Background())
 
 				for i, exp := range tc.expected {
+					if i >= len(result.Children) {
+						t.Fatalf("there are at least %d test cases, but fewer actual results", i)
+					}
 					act := result.Children[i]
 					if err := compareResult(t, exp, act); err != nil {
-						t.Fatalf("%v", err)
+						t.Fatalf("%v, %v, %v", err, act, exp)
 					}
 				}
 			})
@@ -271,7 +684,11 @@ func compareResult(t *testing.T, exp *diagnose.Result, act *diagnose.Result) err
 		}
 	}
 	if len(exp.Children) != len(act.Children) {
-		return fmt.Errorf("section %s, child count mismatch: %d vs %d", exp.Name, len(exp.Children), len(act.Children))
+		errStrings := []string{}
+		for _, c := range act.Children {
+			errStrings = append(errStrings, fmt.Sprintf("%+v", c))
+		}
+		return fmt.Errorf(strings.Join(errStrings, ","))
 	}
 	return nil
 }

command/server/test-fixtures/config_bad_https_storage.hcl

@@ -20,7 +20,7 @@ backend "consul" {
 ha_backend "consul" {
     address = "127.0.0.1:8500"
     bar = "baz"
-    advertise_addr = "snafu"
+    advertise_addr = "https://127.0.0.1:8500"
     disable_clustering = "true"
 }
 

command/server/test-fixtures/config_diagnose_hastorage_bad_https.hcl

@@ -11,12 +11,13 @@ listener "tcp" {
 backend "consul" {
     foo = "bar"
     advertise_addr = "foo"
-    address = "127.0.0.1:1028"
+    address = "http://remoteconsulserverIP:1028"
 
 }
 
 ha_backend "consul" {
     bar = "baz"
+    address = "https://remoteconsulserverIP:1028"
     advertise_addr = "snafu"
     disable_clustering = "true"
     scheme = "https"

command/server/test-fixtures/config_diagnose_ok.hcl

@@ -17,7 +17,7 @@ backend "consul" {
 ha_backend "consul" {
     address = "127.0.0.1:8500"
     bar = "baz"
-    advertise_addr = "snafu"
+    advertise_addr = "https://127.0.0.1:8500"
     disable_clustering = "true"
 }
 

command/server/test-fixtures/diagnose_ok_storage_direct_access.hcl

@@ -17,7 +17,7 @@ backend "consul" {
 ha_backend "consul" {
     address = "127.0.0.1:1024"
     bar = "baz"
-    advertise_addr = "snafu"
+    advertise_addr = "https://127.0.0.1:8500"
     disable_clustering = "true"
 }
 

command/server/test-fixtures/tls_config_ok.hcl

@@ -17,7 +17,7 @@ backend "consul" {
 
 ha_backend "consul" {
     bar = "baz"
-    advertise_addr = "snafu"
+    advertise_addr = "http://blah:8500"
     disable_clustering = "true"
     address = "127.0.0.1:8500"
 }

vault/diagnose/helpers.go

@@ -202,7 +202,7 @@ func WithTimeout(d time.Duration, f testFunction) testFunction {
 		rch := make(chan error)
 		t := time.NewTimer(d)
 		defer t.Stop()
-		go f(ctx)
+		go func() { rch <- f(ctx) }()
 		select {
 		case <-t.C:
 			return fmt.Errorf("timed out after %s", d.String())

vault/diagnose/mock_storage_backend.go

@@ -24,7 +24,7 @@ const (
 	deleteOp               string = "delete"
 )
 
-var goodEntry physical.Entry = physical.Entry{Key: secretKey, Value: []byte(secretVal)}
+var goodEntry physical.Entry = physical.Entry{Key: "diagnose", Value: []byte(secretVal)}
 var badEntry physical.Entry = physical.Entry{}
 
 type mockStorageBackend struct {
@@ -34,7 +34,7 @@ type mockStorageBackend struct {
 func (m mockStorageBackend) storageLogicGeneralInternal(op string) error {
 	if (m.callType == timeoutCallRead && op == readOp) || (m.callType == timeoutCallWrite && op == writeOp) ||
 		(m.callType == timeoutCallDelete && op == deleteOp) {
-		time.Sleep(25 * time.Second)
+		time.Sleep(2 * time.Second)
 	} else if m.callType == errCallWrite && op == writeOp {
 		return fmt.Errorf(storageErrStringWrite)
 	} else if m.callType == errCallDelete && op == deleteOp {
@@ -53,7 +53,10 @@ func (m mockStorageBackend) Put(ctx context.Context, entry *physical.Entry) erro
 
 // Get is used to fetch an entry
 func (m mockStorageBackend) Get(ctx context.Context, key string) (*physical.Entry, error) {
-	if m.callType == errCallRead || m.callType == timeoutCallRead {
+	if m.callType == timeoutCallRead {
+		return &goodEntry, m.storageLogicGeneralInternal(readOp)
+	}
+	if m.callType == errCallRead {
 		return nil, m.storageLogicGeneralInternal(readOp)
 	}
 	if m.callType == badReadCall {
@@ -73,3 +76,16 @@ func (m mockStorageBackend) Delete(ctx context.Context, key string) error {
 func (m mockStorageBackend) List(ctx context.Context, prefix string) ([]string, error) {
 	return nil, fmt.Errorf("method not implemented")
 }
+
+func callTypeToOp(ctype string) string {
+	if ctype == timeoutCallRead || ctype == errCallRead || ctype == badReadCall {
+		return readOp
+	}
+	if ctype == errCallWrite || ctype == storageErrStringWrite || ctype == timeoutCallWrite {
+		return writeOp
+	}
+	if ctype == errCallDelete || ctype == timeoutCallDelete || ctype == storageErrStringDelete {
+		return deleteOp
+	}
+	return ""
+}

vault/diagnose/storage_checks.go

@@ -11,71 +11,59 @@ import (
 
 const (
 	success   string = "success"
-	secretKey string = "diagnose"
 	secretVal string = "diagnoseSecret"
 
-	timeOutErr        string = "storage call timed out after 20 seconds: "
+	LatencyWarning    string        = "latency above 100 ms: "
-	DirAccessErr      string = "consul storage does not connect to local agent, but directly to server"
+	DirAccessErr      string        = "consul storage does not connect to local agent, but directly to server"
-	AddrDNExistErr    string = "config address does not exist: 127.0.0.1:8500 will be used"
+	AddrDNExistErr    string        = "config address does not exist: 127.0.0.1:8500 will be used"
-	wrongRWValsPrefix string = "Storage get and put gave wrong values: "
+	wrongRWValsPrefix string        = "Storage get and put gave wrong values: "
+	latencyThreshold  time.Duration = time.Millisecond * 100
 )
 
-// StorageEndToEndLatencyCheck calls Write, Read, and Delete on a secret in the root
+func EndToEndLatencyCheckWrite(ctx context.Context, uuid string, b physical.Backend) (time.Duration, error) {
-// directory of the backend.
+	start := time.Now()
-// Note: Just checking read, write, and delete for root. It's a very basic check,
+	err := b.Put(context.Background(), &physical.Entry{Key: uuid, Value: []byte(secretVal)})
-// but I don't think we can necessarily do any more than that. We could check list,
+	duration := time.Since(start)
-// but I don't think List is ever going to break in isolation.
+	if err != nil {
-func StorageEndToEndLatencyCheck(ctx context.Context, b physical.Backend) error {
+		return time.Duration(0), err
-
-	c2 := make(chan error)
-	go func() {
-		err := b.Put(context.Background(), &physical.Entry{Key: secretKey, Value: []byte(secretVal)})
-		c2 <- err
-	}()
-	select {
-	case errOut := <-c2:
-		if errOut != nil {
-			return errOut
-		}
-	case <-time.After(20 * time.Second):
-		return fmt.Errorf(timeOutErr + "operation: Put")
 	}
-
+	if duration > latencyThreshold {
-	c3 := make(chan *physical.Entry)
+		return duration, nil
-	c4 := make(chan error)
-	go func() {
-		val, err := b.Get(context.Background(), "diagnose")
-		if err != nil {
-			c4 <- err
-		} else {
-			c3 <- val
-		}
-	}()
-	select {
-	case err := <-c4:
-		return err
-	case val := <-c3:
-		if val.Key != "diagnose" && string(val.Value) != "diagnose" {
-			return fmt.Errorf(wrongRWValsPrefix+"expecting diagnose, but got %s, %s", val.Key, val.Value)
-		}
-	case <-time.After(20 * time.Second):
-		return fmt.Errorf(timeOutErr + "operation: Get")
 	}
+	return time.Duration(0), nil
+}
 
-	c5 := make(chan error)
+func EndToEndLatencyCheckRead(ctx context.Context, uuid string, b physical.Backend) (time.Duration, error) {
-	go func() {
+
-		err := b.Delete(context.Background(), "diagnose")
+	start := time.Now()
-		c5 <- err
+	val, err := b.Get(context.Background(), uuid)
-	}()
+	duration := time.Since(start)
-	select {
+	if err != nil {
-	case errOut := <-c5:
+		return time.Duration(0), err
-		if errOut != nil {
-			return errOut
-		}
-	case <-time.After(20 * time.Second):
-		return fmt.Errorf(timeOutErr + "operation: Delete")
 	}
-	return nil
+	if val == nil {
+		return time.Duration(0), fmt.Errorf("no value found when reading generated data")
+	}
+	if val.Key != uuid && string(val.Value) != secretVal {
+		return time.Duration(0), fmt.Errorf(wrongRWValsPrefix+"expecting diagnose, but got %s, %s", val.Key, val.Value)
+	}
+	if duration > latencyThreshold {
+		return duration, nil
+	}
+	return time.Duration(0), nil
+}
+func EndToEndLatencyCheckDelete(ctx context.Context, uuid string, b physical.Backend) (time.Duration, error) {
+
+	start := time.Now()
+	err := b.Delete(context.Background(), uuid)
+	duration := time.Since(start)
+	if err != nil {
+		return time.Duration(0), err
+	}
+	if duration > latencyThreshold {
+		return duration, nil
+	}
+	return time.Duration(0), nil
 }
 
 // ConsulDirectAccess verifies that consul is connecting to local agent,

vault/diagnose/storage_checks_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/hashicorp/vault/sdk/physical"
 )
@@ -15,15 +16,15 @@ func TestStorageTimeout(t *testing.T) {
 		mb           physical.Backend
 	}{
 		{
-			errSubString: timeOutErr + "operation: Put",
+			errSubString: LatencyWarning,
 			mb:           mockStorageBackend{callType: timeoutCallWrite},
 		},
 		{
-			errSubString: timeOutErr + "operation: Get",
+			errSubString: LatencyWarning,
 			mb:           mockStorageBackend{callType: timeoutCallRead},
 		},
 		{
-			errSubString: timeOutErr + "operation: Delete",
+			errSubString: LatencyWarning,
 			mb:           mockStorageBackend{callType: timeoutCallDelete},
 		},
 		{
@@ -42,18 +43,31 @@ func TestStorageTimeout(t *testing.T) {
 			errSubString: wrongRWValsPrefix,
 			mb:           mockStorageBackend{callType: badReadCall},
 		},
-		{
-			errSubString: "",
-			mb:           mockStorageBackend{callType: ""},
-		},
 	}
 
 	for _, tc := range testCases {
-		outErr := StorageEndToEndLatencyCheck(context.Background(), tc.mb)
+		var outErr error
+		var dur time.Duration
+		uuid := "foo"
+		backendCallType := tc.mb.(mockStorageBackend).callType
+		if callTypeToOp(backendCallType) == readOp {
+			dur, outErr = EndToEndLatencyCheckRead(context.Background(), uuid, tc.mb)
+		}
+		if callTypeToOp(backendCallType) == writeOp {
+			dur, outErr = EndToEndLatencyCheckWrite(context.Background(), uuid, tc.mb)
+		}
+		if callTypeToOp(backendCallType) == deleteOp {
+			dur, outErr = EndToEndLatencyCheckDelete(context.Background(), uuid, tc.mb)
+		}
+
 		if tc.errSubString == "" && outErr == nil {
 			// this is the success case where the Storage Latency check passes
 			continue
 		}
+		if tc.errSubString == LatencyWarning && dur > time.Duration(0) {
+			// this is the success case where the Storage Latency check successfully returns nonzero duration
+			continue
+		}
 		if !strings.Contains(outErr.Error(), tc.errSubString) {
 			t.Errorf("wrong error: expected %s to be contained in %s", tc.errSubString, outErr)
 		}

vault/diagnose/tls_verification.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/hashicorp/vault/internalshared/listenerutil"
 	"github.com/hashicorp/vault/sdk/helper/tlsutil"
-	"github.com/hashicorp/vault/vault"
 )
 
 const minVersionError = "'tls_min_version' value %q not supported, please specify one of [tls10,tls11,tls12,tls13]"
@@ -123,10 +122,3 @@ func TLSFileChecks(certFilePath, keyFilePath string) error {
 
 	return nil
 }
-
-// ServerListenerActiveProbe attempts to use TLS information to set up a TLS server with each listener
-// and generate a successful request through to the server.
-// TODO
-func ServerListenerActiveProbe(core *vault.Core) error {
-	return fmt.Errorf("Method not implemented")
-}

vault/init.go

@@ -11,6 +11,7 @@ import (
 
 	wrapping "github.com/hashicorp/go-kms-wrapping"
 	"github.com/hashicorp/vault/physical/raft"
+	"github.com/hashicorp/vault/vault/diagnose"
 	"github.com/hashicorp/vault/vault/seal"
 
 	aeadwrapper "github.com/hashicorp/go-kms-wrapping/wrappers/aead"
@@ -466,9 +467,11 @@ func (c *Core) UnsealWithStoredKeys(ctx context.Context) error {
 	// This usually happens when auto-unseal is configured, but the servers have
 	// not been initialized yet.
 	if len(keys) == 0 {
+		diagnose.Error(ctx, errors.New("stored unseal keys are supported, but none were found"))
 		return NewNonFatalError(errors.New("stored unseal keys are supported, but none were found"))
 	}
 	if len(keys) != 1 {
+		diagnose.Error(ctx, errors.New("expected exactly one stored key"))
 		return NewNonFatalError(errors.New("expected exactly one stored key"))
 	}
 
@@ -482,6 +485,7 @@ func (c *Core) UnsealWithStoredKeys(ctx context.Context) error {
 		// subset of the required threshold of keys. We still consider this a
 		// "success", since trying again would yield the same result.
 		c.Logger().Warn("vault still sealed after using stored unseal key")
+		diagnose.Warn(ctx, "vault still sealed after using stored unseal key")
 	} else {
 		c.Logger().Info("unsealed with stored key")
 	}