From 60c92df3f83df9410322935b71a939bee86df489 Mon Sep 17 00:00:00 2001 From: Hridoy Roy Date: Tue, 20 Oct 2020 15:20:25 -0700 Subject: [PATCH 1/5] first commit --- helper/metricsutil/metricsutil.go | 43 ++++- vault/auth.go | 38 +++-- vault/auth_test.go | 118 +++++++++++--- vault/core_metrics.go | 24 ++- .../metrics/core_metrics_int_test.go | 121 +++++++++++++- vault/mount.go | 147 ++++++++++++++---- vault/mount_test.go | 128 ++++++++++++--- vault/testing.go | 1 + website/pages/docs/internals/telemetry.mdx | 2 + 9 files changed, 537 insertions(+), 85 deletions(-) diff --git a/helper/metricsutil/metricsutil.go b/helper/metricsutil/metricsutil.go index 83ca85a2d8fe..31adf69551b2 100644 --- a/helper/metricsutil/metricsutil.go +++ b/helper/metricsutil/metricsutil.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "strings" + "sync" "github.com/armon/go-metrics" "github.com/hashicorp/vault/sdk/logical" @@ -26,13 +27,34 @@ const ( PrometheusMetricFormat = "prometheus" ) +// PhysicalTableSizeName is a set of gauge metric keys for physical mount table sizes +var PhysicalTableSizeName []string = []string{"core", "mount_table", "size"} + +// LogicalTableSizeName is a set of gauge metric keys for logical mount table sizes +var LogicalTableSizeName []string = []string{"core", "mount_table", "num_entries"} + type MetricsHelper struct { inMemSink *metrics.InmemSink PrometheusEnabled bool + LoopMetrics GaugeMetrics +} + +type GaugeMetrics struct { + // Metrics is a map from keys concatenated by "." to the metric. + // It is a map because although we do not care about distinguishing + // these loop metrics during emission, we must distinguish them + // when we update a metric. + Metrics sync.Map +} + +type GaugeMetric struct { + Value float32 + Labels []Label + Key []string } func NewMetricsHelper(inMem *metrics.InmemSink, enablePrometheus bool) *MetricsHelper { - return &MetricsHelper{inMem, enablePrometheus} + return &MetricsHelper{inMem, enablePrometheus, GaugeMetrics{Metrics: sync.Map{}}} } func FormatFromRequest(req *logical.Request) string { @@ -53,6 +75,25 @@ func FormatFromRequest(req *logical.Request) string { return "" } +func (m *MetricsHelper) AddGaugeLoopMetric(key []string, val float32, labels []Label) { + mapKey := m.CreateMetricsCacheKeyName(key, val, labels) + m.LoopMetrics.Metrics.Store(mapKey, + GaugeMetric{ + Key: key, + Value: val, + Labels: labels}) +} + +func (m *MetricsHelper) CreateMetricsCacheKeyName(key []string, val float32, labels []Label) string { + var keyJoin string = strings.Join(key, ".") + var labelJoinStr = "" + for _, label := range labels { + labelJoinStr = labelJoinStr + label.Name + "|" + label.Value + "||" + } + keyJoin = keyJoin + "." + labelJoinStr + return keyJoin +} + func (m *MetricsHelper) ResponseForFormat(format string) *logical.Response { switch format { case PrometheusMetricFormat: diff --git a/vault/auth.go b/vault/auth.go index dbf719198ff8..362415901cab 100644 --- a/vault/auth.go +++ b/vault/auth.go @@ -425,7 +425,7 @@ func (c *Core) taintCredEntry(ctx context.Context, path string, updateStorage bo // Taint the entry from the auth table // We do this on the original since setting the taint operates // on the entries which a shallow clone shares anyways - entry, err := c.auth.setTaint(ctx, strings.TrimPrefix(path, credentialRoutePrefix), true) + entry, err := c.auth.setTaint(ctx, strings.TrimPrefix(path, credentialRoutePrefix), true, mountStateUnmounting) if err != nil { return err } @@ -478,8 +478,10 @@ func (c *Core) loadCredentials(ctx context.Context) error { if c.auth == nil { c.auth = c.defaultAuthTable() needPersist = true + } else { + // only record tableMetrics if we have loaded something from storge + c.tableMetrics(len(c.auth.Entries), false, true, raw.Value) } - if rawLocal != nil { localAuthTable, err := c.decodeMountTable(ctx, rawLocal.Value) if err != nil { @@ -488,6 +490,7 @@ func (c *Core) loadCredentials(ctx context.Context) error { } if localAuthTable != nil && len(localAuthTable.Entries) > 0 { c.auth.Entries = append(c.auth.Entries, localAuthTable.Entries...) + c.tableMetrics(len(localAuthTable.Entries), true, true, rawLocal.Value) } } @@ -571,20 +574,24 @@ func (c *Core) persistAuth(ctx context.Context, table *MountTable, local *bool) Type: credentialTableType, } + var localAuthCount int + var nonLocalAuthCount int for _, entry := range table.Entries { if entry.Local { localAuth.Entries = append(localAuth.Entries, entry) + localAuthCount += 1 } else { nonLocalAuth.Entries = append(nonLocalAuth.Entries, entry) + nonLocalAuthCount += 1 } } - writeTable := func(mt *MountTable, path string) error { + writeTable := func(mt *MountTable, path string) ([]byte, error) { // Encode the mount table into JSON and compress it (lzw). compressedBytes, err := jsonutil.EncodeJSONAndCompress(mt, nil) if err != nil { c.logger.Error("failed to encode or compress auth mount table", "error", err) - return err + return nil, err } // Create an entry @@ -596,29 +603,40 @@ func (c *Core) persistAuth(ctx context.Context, table *MountTable, local *bool) // Write to the physical backend if err := c.barrier.Put(ctx, entry); err != nil { c.logger.Error("failed to persist auth mount table", "error", err) - return err + return nil, err } - return nil + return compressedBytes, nil } var err error + var compressedBytes []byte switch { case local == nil: // Write non-local mounts - err := writeTable(nonLocalAuth, coreAuthConfigPath) + compressedBytes, err := writeTable(nonLocalAuth, coreAuthConfigPath) if err != nil { return err } + c.tableMetrics(nonLocalAuthCount, false, true, compressedBytes) // Write local mounts - err = writeTable(localAuth, coreLocalAuthConfigPath) + compressedBytes, err = writeTable(localAuth, coreLocalAuthConfigPath) if err != nil { return err } + c.tableMetrics(localAuthCount, true, true, compressedBytes) case *local: - err = writeTable(localAuth, coreLocalAuthConfigPath) + compressedBytes, err = writeTable(localAuth, coreLocalAuthConfigPath) + if err != nil { + return err + } + c.tableMetrics(localAuthCount, true, true, compressedBytes) default: - err = writeTable(nonLocalAuth, coreAuthConfigPath) + compressedBytes, err = writeTable(nonLocalAuth, coreAuthConfigPath) + if err != nil { + return err + } + c.tableMetrics(nonLocalAuthCount, false, true, compressedBytes) } return err diff --git a/vault/auth_test.go b/vault/auth_test.go index 38a0a94f09d0..89fe95efdf2d 100644 --- a/vault/auth_test.go +++ b/vault/auth_test.go @@ -5,14 +5,17 @@ import ( "reflect" "strings" "testing" + "time" + metrics "github.com/armon/go-metrics" + "github.com/hashicorp/vault/helper/metricsutil" "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/logical" ) func TestAuth_ReadOnlyViewDuringMount(t *testing.T) { - c, _, _ := TestCoreUnsealed(t) + c, _, _, _ := TestCoreUnsealedWithMetrics(t) c.credentialBackends["noop"] = func(ctx context.Context, config *logical.BackendConfig) (logical.Backend, error) { err := config.StorageView.Put(ctx, &logical.StorageEntry{ Key: "bar", @@ -37,14 +40,85 @@ func TestAuth_ReadOnlyViewDuringMount(t *testing.T) { } } +func TestAuthMountMetrics(t *testing.T) { + c, _, _, _ := TestCoreUnsealedWithMetrics(t) + c.credentialBackends["noop"] = func(ctx context.Context, config *logical.BackendConfig) (logical.Backend, error) { + return &NoopBackend{ + BackendType: logical.TypeCredential, + }, nil + } + mountKeyName := "core.mount_table.num_entries.type|auth||local|false||" + mountMetrics := &c.metricsHelper.LoopMetrics.Metrics + loadMetric, ok := mountMetrics.Load(mountKeyName) + var numEntriesMetric metricsutil.GaugeMetric = loadMetric.(metricsutil.GaugeMetric) + + // 1 default nonlocal auth backend + if !ok || numEntriesMetric.Value != 1 { + t.Fatalf("Auth values should be: %+v", numEntriesMetric) + } + + me := &MountEntry{ + Table: credentialTableType, + Path: "foo", + Type: "noop", + } + err := c.enableCredential(namespace.RootContext(nil), me) + if err != nil { + t.Fatalf("err: %v", err) + } + mountMetrics = &c.metricsHelper.LoopMetrics.Metrics + loadMetric, ok = mountMetrics.Load(mountKeyName) + numEntriesMetric = loadMetric.(metricsutil.GaugeMetric) + if !ok || numEntriesMetric.Value != 2 { + t.Fatalf("mount metrics for num entries do not match true values") + } + if len(numEntriesMetric.Key) != 3 || + numEntriesMetric.Key[0] != "core" || + numEntriesMetric.Key[1] != "mount_table" || + numEntriesMetric.Key[2] != "num_entries" { + t.Fatalf("mount metrics for num entries have wrong key") + } + if len(numEntriesMetric.Labels) != 2 || + numEntriesMetric.Labels[0].Name != "type" || + numEntriesMetric.Labels[0].Value != "auth" || + numEntriesMetric.Labels[1].Name != "local" || + numEntriesMetric.Labels[1].Value != "false" { + t.Fatalf("mount metrics for num entries have wrong labels") + } + mountSizeKeyName := "core.mount_table.size.type|auth||local|false||" + loadMetric, ok = mountMetrics.Load(mountSizeKeyName) + sizeMetric := loadMetric.(metricsutil.GaugeMetric) + + if !ok { + t.Fatalf("mount metrics for size do not match exist") + } + if len(sizeMetric.Key) != 3 || + sizeMetric.Key[0] != "core" || + sizeMetric.Key[1] != "mount_table" || + sizeMetric.Key[2] != "size" { + t.Fatalf("mount metrics for size have wrong key") + } + if len(sizeMetric.Labels) != 2 || + sizeMetric.Labels[0].Name != "type" || + sizeMetric.Labels[0].Value != "auth" || + sizeMetric.Labels[1].Name != "local" || + sizeMetric.Labels[1].Value != "false" { + t.Fatalf("mount metrics for size have wrong labels") + } +} + func TestCore_DefaultAuthTable(t *testing.T) { - c, keys, _ := TestCoreUnsealed(t) + c, keys, _, _ := TestCoreUnsealedWithMetrics(t) verifyDefaultAuthTable(t, c.auth) // Start a second core with same physical + inmemSink := metrics.NewInmemSink(1000000*time.Hour, 2000000*time.Hour) conf := &CoreConfig{ - Physical: c.physical, - DisableMlock: true, + Physical: c.physical, + DisableMlock: true, + BuiltinRegistry: NewMockBuiltinRegistry(), + MetricSink: metricsutil.NewClusterMetricSink("test-cluster", inmemSink), + MetricsHelper: metricsutil.NewMetricsHelper(inmemSink, false), } c2, err := NewCore(conf) if err != nil { @@ -67,7 +141,7 @@ func TestCore_DefaultAuthTable(t *testing.T) { } func TestCore_EnableCredential(t *testing.T) { - c, keys, _ := TestCoreUnsealed(t) + c, keys, _, _ := TestCoreUnsealedWithMetrics(t) c.credentialBackends["noop"] = func(context.Context, *logical.BackendConfig) (logical.Backend, error) { return &NoopBackend{ BackendType: logical.TypeCredential, @@ -89,9 +163,13 @@ func TestCore_EnableCredential(t *testing.T) { t.Fatalf("missing mount, match: %q", match) } + inmemSink := metrics.NewInmemSink(1000000*time.Hour, 2000000*time.Hour) conf := &CoreConfig{ - Physical: c.physical, - DisableMlock: true, + Physical: c.physical, + DisableMlock: true, + BuiltinRegistry: NewMockBuiltinRegistry(), + MetricSink: metricsutil.NewClusterMetricSink("test-cluster", inmemSink), + MetricsHelper: metricsutil.NewMetricsHelper(inmemSink, false), } c2, err := NewCore(conf) if err != nil { @@ -122,7 +200,7 @@ func TestCore_EnableCredential(t *testing.T) { // entries, and that upon reading the entries from both are recombined // correctly func TestCore_EnableCredential_Local(t *testing.T) { - c, _, _ := TestCoreUnsealed(t) + c, _, _, _ := TestCoreUnsealedWithMetrics(t) c.credentialBackends["noop"] = func(context.Context, *logical.BackendConfig) (logical.Backend, error) { return &NoopBackend{ BackendType: logical.TypeCredential, @@ -211,7 +289,7 @@ func TestCore_EnableCredential_Local(t *testing.T) { } func TestCore_EnableCredential_twice_409(t *testing.T) { - c, _, _ := TestCoreUnsealed(t) + c, _, _, _ := TestCoreUnsealedWithMetrics(t) c.credentialBackends["noop"] = func(context.Context, *logical.BackendConfig) (logical.Backend, error) { return &NoopBackend{ BackendType: logical.TypeCredential, @@ -241,7 +319,7 @@ func TestCore_EnableCredential_twice_409(t *testing.T) { } func TestCore_EnableCredential_Token(t *testing.T) { - c, _, _ := TestCoreUnsealed(t) + c, _, _, _ := TestCoreUnsealedWithMetrics(t) me := &MountEntry{ Table: credentialTableType, Path: "foo", @@ -254,7 +332,7 @@ func TestCore_EnableCredential_Token(t *testing.T) { } func TestCore_DisableCredential(t *testing.T) { - c, keys, _ := TestCoreUnsealed(t) + c, keys, _, _ := TestCoreUnsealedWithMetrics(t) c.credentialBackends["noop"] = func(context.Context, *logical.BackendConfig) (logical.Backend, error) { return &NoopBackend{ BackendType: logical.TypeCredential, @@ -286,9 +364,13 @@ func TestCore_DisableCredential(t *testing.T) { t.Fatalf("backend present") } + inmemSink := metrics.NewInmemSink(1000000*time.Hour, 2000000*time.Hour) conf := &CoreConfig{ - Physical: c.physical, - DisableMlock: true, + Physical: c.physical, + DisableMlock: true, + BuiltinRegistry: NewMockBuiltinRegistry(), + MetricSink: metricsutil.NewClusterMetricSink("test-cluster", inmemSink), + MetricsHelper: metricsutil.NewMetricsHelper(inmemSink, false), } c2, err := NewCore(conf) if err != nil { @@ -311,7 +393,7 @@ func TestCore_DisableCredential(t *testing.T) { } func TestCore_DisableCredential_Protected(t *testing.T) { - c, _, _ := TestCoreUnsealed(t) + c, _, _, _ := TestCoreUnsealedWithMetrics(t) err := c.disableCredential(namespace.RootContext(nil), "token") if err.Error() != "token credential backend cannot be disabled" { t.Fatalf("err: %v", err) @@ -323,7 +405,7 @@ func TestCore_DisableCredential_Cleanup(t *testing.T) { Login: []string{"login"}, BackendType: logical.TypeCredential, } - c, _, _ := TestCoreUnsealed(t) + c, _, _, _ := TestCoreUnsealedWithMetrics(t) c.credentialBackends["noop"] = func(context.Context, *logical.BackendConfig) (logical.Backend, error) { return noop, nil } @@ -394,7 +476,7 @@ func TestCore_DisableCredential_Cleanup(t *testing.T) { } func TestDefaultAuthTable(t *testing.T) { - c, _, _ := TestCoreUnsealed(t) + c, _, _, _ := TestCoreUnsealedWithMetrics(t) table := c.defaultAuthTable() verifyDefaultAuthTable(t, table) } @@ -432,7 +514,7 @@ func TestCore_CredentialInitialize(t *testing.T) { BackendType: logical.TypeCredential, }, false} - c, _, _ := TestCoreUnsealed(t) + c, _, _, _ := TestCoreUnsealedWithMetrics(t) c.credentialBackends["initable"] = func(context.Context, *logical.BackendConfig) (logical.Backend, error) { return backend, nil } @@ -457,7 +539,7 @@ func TestCore_CredentialInitialize(t *testing.T) { BackendType: logical.TypeCredential, }, false} - c, _, _ := TestCoreUnsealed(t) + c, _, _, _ := TestCoreUnsealedWithMetrics(t) c.credentialBackends["initable"] = func(context.Context, *logical.BackendConfig) (logical.Backend, error) { return backend, nil } diff --git a/vault/core_metrics.go b/vault/core_metrics.go index 5dcd6ed176ac..cb6876ca8bb0 100644 --- a/vault/core_metrics.go +++ b/vault/core_metrics.go @@ -49,6 +49,9 @@ func (c *Core) metricsLoop(stopCh chan struct{}) { c.metricSink.SetGaugeWithLabels([]string{"core", "leader"}, 1, nil) } + // Refresh gauge metrics that are looped + c.cachedGaugeMetricsEmitter() + case <-writeTimer: if stopped := grabLockOrStop(c.stateLock.RLock, c.stateLock.RUnlock, stopCh); stopped { // Go through the loop again, this time the stop channel case @@ -65,8 +68,8 @@ func (c *Core) metricsLoop(stopCh chan struct{}) { } c.stateLock.RUnlock() case <-identityCountTimer: - // Only emit on active node - if c.PerfStandby() { + // Only emit on active node of cluster that is not a DR cecondary. + if standby, _ := c.Standby(); standby || c.IsDRSecondary() { break } @@ -193,10 +196,11 @@ func (c *Core) emitMetrics(stopCh chan struct{}) { }, } - // Disable collection if configured, or if we're a performance standby. + // Disable collection if configured, or if we're a performance standby + // node or DR secondary cluster. if c.MetricSink().GaugeInterval == time.Duration(0) { c.logger.Info("usage gauge collection is disabled") - } else if !c.PerfStandby() { + } else if standby, _ := c.Standby(); !standby && !c.IsDRSecondary() { for _, init := range metricsInit { if init.DisableEnvVar != "" { if os.Getenv(init.DisableEnvVar) != "" { @@ -425,3 +429,15 @@ func (c *Core) entityGaugeCollectorByMount(ctx context.Context) ([]metricsutil.G return values, nil } + +func (c *Core) cachedGaugeMetricsEmitter() { + loopMetrics := &c.metricsHelper.LoopMetrics.Metrics + + emit := func(key interface{}, value interface{}) bool { + metricValue := value.(metricsutil.GaugeMetric) + c.metricSink.SetGaugeWithLabels(metricValue.Key, metricValue.Value, metricValue.Labels) + return true + } + + loopMetrics.Range(emit) +} diff --git a/vault/external_tests/metrics/core_metrics_int_test.go b/vault/external_tests/metrics/core_metrics_int_test.go index 0696e7408774..1635c548cc3e 100644 --- a/vault/external_tests/metrics/core_metrics_int_test.go +++ b/vault/external_tests/metrics/core_metrics_int_test.go @@ -3,16 +3,132 @@ package metrics import ( "context" "encoding/json" + "errors" "io/ioutil" "testing" "time" "github.com/armon/go-metrics" + "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/helper/metricsutil" vaulthttp "github.com/hashicorp/vault/http" "github.com/hashicorp/vault/vault" ) +func TestMountTableMetrics(t *testing.T) { + inm := metrics.NewInmemSink(time.Minute, time.Minute*10) + clusterSink := metricsutil.NewClusterMetricSink("mycluster", inm) + clusterSink.GaugeInterval = time.Second + conf := &vault.CoreConfig{ + BuiltinRegistry: vault.NewMockBuiltinRegistry(), + MetricsHelper: metricsutil.NewMetricsHelper(inm, true), + MetricSink: clusterSink, + } + cluster := vault.NewTestCluster(t, conf, &vault.TestClusterOptions{ + KeepStandbysSealed: false, + HandlerFunc: vaulthttp.Handler, + NumCores: 2, + }) + + cluster.Start() + defer cluster.Cleanup() + + // Wait for core to become active + cores := cluster.Cores + vault.TestWaitActive(t, cores[0].Core) + + client := cores[0].Client + + // Verify that the nonlocal logical mount table has 3 entries -- cubbyhole, identity, and kv + + data, err := sysMetricsReq(client, cluster) + if err != nil { + t.Fatal(err) + } + + nonlocalLogicalMountsize, err := gaugeSearchHelper(data, 3) + if err != nil { + t.Errorf(err.Error()) + } + + // Mount new kv + if err = client.Sys().Mount("kv", &api.MountInput{ + Type: "kv", + Options: map[string]string{ + "version": "2", + }, + }); err != nil { + t.Fatal(err) + } + + data, err = sysMetricsReq(client, cluster) + if err != nil { + t.Fatal(err) + } + + nonlocalLogicalMountsizeAfterMount, err := gaugeSearchHelper(data, 4) + if err != nil { + t.Errorf(err.Error()) + } + + if nonlocalLogicalMountsizeAfterMount <= nonlocalLogicalMountsize { + t.Errorf("Mount size does not change after new mount is mounted") + } + +} + +func sysMetricsReq(client *api.Client, cluster *vault.TestCluster) (*SysMetricsJSON, error) { + r := client.NewRequest("GET", "/v1/sys/metrics") + r.Headers.Set("X-Vault-Token", cluster.RootToken) + var data SysMetricsJSON + mountAddResp, err := client.RawRequestWithContext(context.Background(), r) + if err != nil { + return nil, err + } + bodyBytes, err := ioutil.ReadAll(mountAddResp.Response.Body) + if err != nil { + return nil, err + } + if mountAddResp != nil { + defer mountAddResp.Body.Close() + } + if err := json.Unmarshal(bodyBytes, &data); err != nil { + return nil, errors.New("failed to unmarshal:" + err.Error()) + } + return &data, nil +} + +func gaugeSearchHelper(data *SysMetricsJSON, expectedValue int) (int, error) { + foundFlag := false + tablesize := int(^uint(0) >> 1) + for _, gauge := range data.Gauges { + labels := gauge.Labels + if loc, ok := labels["local"]; ok && loc.(string) == "false" { + if tp, ok := labels["type"]; ok && tp.(string) == "logical" { + if gauge.Name == "core.mount_table.num_entries" { + foundFlag = true + if err := gaugeConditionCheck("eq", expectedValue, gauge.Value); err != nil { + return int(^uint(0) >> 1), err + } + } else if gauge.Name == "core.mount_table.size" { + tablesize = gauge.Value + } + } + } + } + if !foundFlag { + return int(^uint(0) >> 1), errors.New("No metrics reported for mount sizes") + } + return tablesize, nil +} + +func gaugeConditionCheck(comparator string, compareVal int, compareToVal int) error { + if comparator == "eq" && compareVal != compareToVal { + return errors.New("equality gauge check for comparison failed") + } + return nil +} + func TestLeaderReElectionMetrics(t *testing.T) { inm := metrics.NewInmemSink(time.Minute, time.Minute*10) clusterSink := metricsutil.NewClusterMetricSink("mycluster", inm) @@ -127,6 +243,7 @@ type SysMetricsJSON struct { } type GaugeJSON struct { - Name string `json:"Name"` - Value int `json:"Value"` + Name string `json:"Name"` + Value int `json:"Value"` + Labels map[string]interface{} `json:"Labels"` } diff --git a/vault/mount.go b/vault/mount.go index 155ab5d4e54c..30d303a900cc 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -10,8 +10,10 @@ import ( "sync" "time" + "github.com/armon/go-metrics" uuid "github.com/hashicorp/go-uuid" "github.com/hashicorp/vault/builtin/plugin" + "github.com/hashicorp/vault/helper/metricsutil" "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/helper/jsonutil" @@ -123,6 +125,45 @@ type MountTable struct { Entries []*MountEntry `json:"entries"` } +// tableMetrics is responsible for setting gauge metrics for +// mount table storage sizes (in bytes) and mount table num +// entries. It does this via setGaugeWithLabels. It then +// saves these metrics in a cache for regular reporting in +// a loop, via AddGaugeLoopMetric. + +// Note that the reported storage sizes are pre-encryption +// sizes. Currently barrier uses aes-gcm for encryption, which +// preserves plaintext size, adding a constant of 30 bytes of +// padding, which is negligable and subject to change, and thus +// not accounted for. +func (c *Core) tableMetrics(entryCount int, isLocal bool, isAuth bool, compressedTable []byte) { + typeAuthLabelMap := map[bool]metrics.Label{ + true: metrics.Label{Name: "type", Value: "auth"}, + false: metrics.Label{Name: "type", Value: "logical"}, + } + + typeLocalLabelMap := map[bool]metrics.Label{ + true: metrics.Label{Name: "local", Value: "true"}, + false: metrics.Label{Name: "local", Value: "false"}, + } + + c.metricSink.SetGaugeWithLabels(metricsutil.LogicalTableSizeName, + float32(entryCount), []metrics.Label{typeAuthLabelMap[isAuth], + typeLocalLabelMap[isLocal]}) + + c.metricsHelper.AddGaugeLoopMetric(metricsutil.LogicalTableSizeName, + float32(entryCount), []metrics.Label{typeAuthLabelMap[isAuth], + typeLocalLabelMap[isLocal]}) + + c.metricSink.SetGaugeWithLabels(metricsutil.PhysicalTableSizeName, + float32(len(compressedTable)), []metrics.Label{typeAuthLabelMap[isAuth], + typeLocalLabelMap[isLocal]}) + + c.metricsHelper.AddGaugeLoopMetric(metricsutil.PhysicalTableSizeName, + float32(len(compressedTable)), []metrics.Label{typeAuthLabelMap[isAuth], + typeLocalLabelMap[isLocal]}) +} + // shallowClone returns a copy of the mount table that // keeps the MountEntry locations, so as not to invalidate // other locations holding pointers. Care needs to be taken @@ -132,6 +173,7 @@ func (t *MountTable) shallowClone() *MountTable { Type: t.Type, Entries: make([]*MountEntry, len(t.Entries)), } + for i, e := range t.Entries { mt.Entries[i] = e } @@ -140,7 +182,7 @@ func (t *MountTable) shallowClone() *MountTable { // setTaint is used to set the taint on given entry Accepts either the mount // entry's path or namespace + path, i.e. /secret/ or /token/ -func (t *MountTable) setTaint(ctx context.Context, path string, value bool) (*MountEntry, error) { +func (t *MountTable) setTaint(ctx context.Context, path string, tainted bool, mountState string) (*MountEntry, error) { n := len(t.Entries) ns, err := namespace.FromContext(ctx) if err != nil { @@ -148,7 +190,8 @@ func (t *MountTable) setTaint(ctx context.Context, path string, value bool) (*Mo } for i := 0; i < n; i++ { if entry := t.Entries[i]; entry.Path == path && entry.Namespace().ID == ns.ID { - t.Entries[i].Tainted = value + t.Entries[i].Tainted = tainted + t.Entries[i].MountState = mountState return t.Entries[i], nil } } @@ -207,6 +250,8 @@ func (t *MountTable) sortEntriesByPathDepth() *MountTable { return t } +const mountStateUnmounting = "unmounting" + // MountEntry is used to represent a mount table entry type MountEntry struct { Table string `json:"table"` // The table it belongs to @@ -222,6 +267,7 @@ type MountEntry struct { SealWrap bool `json:"seal_wrap"` // Whether to wrap CSPs ExternalEntropyAccess bool `json:"external_entropy_access"` // Whether to allow external entropy source access Tainted bool `json:"tainted,omitempty"` // Set as a Write-Ahead flag for unmount/remount + MountState string `json:"mount_state,omitempty"` // The current mount state. The only non-empty mount state right now is "unmounting" NamespaceID string `json:"namespace_id"` // namespace contains the populated namespace @@ -595,7 +641,7 @@ func (c *Core) unmountInternal(ctx context.Context, path string, updateStorage b entry := c.router.MatchingMountEntry(ctx, path) // Mark the entry as tainted - if err := c.taintMountEntry(ctx, path, updateStorage); err != nil { + if err := c.taintMountEntry(ctx, path, updateStorage, true); err != nil { c.logger.Error("failed to taint mount entry for path being unmounted", "error", err, "path", path) return err } @@ -713,13 +759,18 @@ func (c *Core) removeMountEntry(ctx context.Context, path string, updateStorage } // taintMountEntry is used to mark an entry in the mount table as tainted -func (c *Core) taintMountEntry(ctx context.Context, path string, updateStorage bool) error { +func (c *Core) taintMountEntry(ctx context.Context, path string, updateStorage, unmounting bool) error { c.mountsLock.Lock() defer c.mountsLock.Unlock() + mountState := "" + if unmounting { + mountState = mountStateUnmounting + } + // As modifying the taint of an entry affects shallow clones, // we simply use the original - entry, err := c.mounts.setTaint(ctx, path, true) + entry, err := c.mounts.setTaint(ctx, path, true, mountState) if err != nil { return err } @@ -814,7 +865,7 @@ func (c *Core) remount(ctx context.Context, src, dst string, updateStorage bool) } // Mark the entry as tainted - if err := c.taintMountEntry(ctx, src, updateStorage); err != nil { + if err := c.taintMountEntry(ctx, src, updateStorage, false); err != nil { return err } @@ -915,6 +966,7 @@ func (c *Core) loadMounts(ctx context.Context) error { c.logger.Error("failed to decompress and/or decode the mount table", "error", err) return err } + c.tableMetrics(len(mountTable.Entries), false, false, raw.Value) c.mounts = mountTable } @@ -932,13 +984,43 @@ func (c *Core) loadMounts(ctx context.Context) error { return err } if localMountTable != nil && len(localMountTable.Entries) > 0 { + c.tableMetrics(len(localMountTable.Entries), true, false, raw.Value) c.mounts.Entries = append(c.mounts.Entries, localMountTable.Entries...) } } - // Note that this is only designed to work with singletons, as it checks by - // type only. + // If this node is a performance standby we do not want to attempt to + // upgrade the mount table, this will be the active node's responsibility. + if !c.perfStandby { + err := c.runMountUpdates(ctx, needPersist) + if err != nil { + c.logger.Error("failed to run mount table upgrades", "error", err) + return err + } + } + + for _, entry := range c.mounts.Entries { + if entry.NamespaceID == "" { + entry.NamespaceID = namespace.RootNamespaceID + } + ns, err := NamespaceByID(ctx, entry.NamespaceID, c) + if err != nil { + return err + } + if ns == nil { + return namespace.ErrNoNamespace + } + entry.namespace = ns + + // Sync values to the cache + entry.SyncCache() + } + return nil +} +// Note that this is only designed to work with singletons, as it checks by +// type only. +func (c *Core) runMountUpdates(ctx context.Context, needPersist bool) error { // Upgrade to typed mount table if c.mounts.Type == "" { c.mounts.Type = mountTableType @@ -970,6 +1052,10 @@ func (c *Core) loadMounts(ctx context.Context) error { // Upgrade to table-scoped entries for _, entry := range c.mounts.Entries { + if !c.PR1103disabled && entry.Type == mountTypeNSCubbyhole && !entry.Local && !c.ReplicationState().HasState(consts.ReplicationPerformanceSecondary|consts.ReplicationDRSecondary) { + entry.Local = true + needPersist = true + } if entry.Type == cubbyholeMountType && !entry.Local { entry.Local = true needPersist = true @@ -999,17 +1085,6 @@ func (c *Core) loadMounts(ctx context.Context) error { entry.NamespaceID = namespace.RootNamespaceID needPersist = true } - ns, err := NamespaceByID(ctx, entry.NamespaceID, c) - if err != nil { - return err - } - if ns == nil { - return namespace.ErrNoNamespace - } - entry.namespace = ns - - // Sync values to the cache - entry.SyncCache() } // Done if we have restored the mount table and we don't need @@ -1048,20 +1123,24 @@ func (c *Core) persistMounts(ctx context.Context, table *MountTable, local *bool Type: mountTableType, } + var localMountsCount int + var nonLocalMountsCount int for _, entry := range table.Entries { if entry.Local { localMounts.Entries = append(localMounts.Entries, entry) + localMountsCount += 1 } else { nonLocalMounts.Entries = append(nonLocalMounts.Entries, entry) + nonLocalMountsCount += 1 } } - writeTable := func(mt *MountTable, path string) error { + writeTable := func(mt *MountTable, path string) ([]byte, error) { // Encode the mount table into JSON and compress it (lzw). compressedBytes, err := jsonutil.EncodeJSONAndCompress(mt, nil) if err != nil { c.logger.Error("failed to encode or compress mount table", "error", err) - return err + return nil, err } // Create an entry @@ -1073,34 +1152,46 @@ func (c *Core) persistMounts(ctx context.Context, table *MountTable, local *bool // Write to the physical backend if err := c.barrier.Put(ctx, entry); err != nil { c.logger.Error("failed to persist mount table", "error", err) - return err + return nil, err } - return nil + return compressedBytes, nil } var err error + var compressedBytes []byte switch { case local == nil: // Write non-local mounts - err := writeTable(nonLocalMounts, coreMountConfigPath) + compressedBytes, err := writeTable(nonLocalMounts, coreMountConfigPath) if err != nil { return err } + c.tableMetrics(nonLocalMountsCount, false, false, compressedBytes) // Write local mounts - err = writeTable(localMounts, coreLocalMountConfigPath) + compressedBytes, err = writeTable(localMounts, coreLocalMountConfigPath) if err != nil { return err } + c.tableMetrics(localMountsCount, true, false, compressedBytes) + case *local: // Write local mounts - err = writeTable(localMounts, coreLocalMountConfigPath) + compressedBytes, err = writeTable(localMounts, coreLocalMountConfigPath) + if err != nil { + return err + } + c.tableMetrics(localMountsCount, true, false, compressedBytes) default: // Write non-local mounts - err = writeTable(nonLocalMounts, coreMountConfigPath) + compressedBytes, err = writeTable(nonLocalMounts, coreMountConfigPath) + if err != nil { + return err + } + c.tableMetrics(nonLocalMountsCount, false, false, compressedBytes) } - return err + return nil } // setupMounts is invoked after we've loaded the mount table to diff --git a/vault/mount_test.go b/vault/mount_test.go index adf040ba6328..b0bf11e18dd9 100644 --- a/vault/mount_test.go +++ b/vault/mount_test.go @@ -8,8 +8,10 @@ import ( "testing" "time" + metrics "github.com/armon/go-metrics" "github.com/go-test/deep" "github.com/hashicorp/vault/audit" + "github.com/hashicorp/vault/helper/metricsutil" "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/sdk/helper/compressutil" "github.com/hashicorp/vault/sdk/helper/jsonutil" @@ -17,7 +19,7 @@ import ( ) func TestMount_ReadOnlyViewDuringMount(t *testing.T) { - c, _, _ := TestCoreUnsealed(t) + c, _, _, _ := TestCoreUnsealedWithMetrics(t) c.logicalBackends["noop"] = func(ctx context.Context, config *logical.BackendConfig) (logical.Backend, error) { err := config.StorageView.Put(ctx, &logical.StorageEntry{ Key: "bar", @@ -40,14 +42,84 @@ func TestMount_ReadOnlyViewDuringMount(t *testing.T) { } } +func TestLogicalMountMetrics(t *testing.T) { + c, _, _, _ := TestCoreUnsealedWithMetrics(t) + c.logicalBackends["noop"] = func(ctx context.Context, config *logical.BackendConfig) (logical.Backend, error) { + return &NoopBackend{ + BackendType: logical.TypeLogical, + }, nil + } + mountKeyName := "core.mount_table.num_entries.type|logical||local|false||" + mountMetrics := &c.metricsHelper.LoopMetrics.Metrics + loadMetric, ok := mountMetrics.Load(mountKeyName) + var numEntriesMetric metricsutil.GaugeMetric = loadMetric.(metricsutil.GaugeMetric) + + // 3 default nonlocal logical backends + if !ok || numEntriesMetric.Value != 3 { + t.Fatalf("Auth values should be: %+v", numEntriesMetric) + } + me := &MountEntry{ + Table: mountTableType, + Path: "foo", + Type: "noop", + } + err := c.mount(namespace.RootContext(nil), me) + if err != nil { + t.Fatalf("err: %v", err) + } + mountMetrics = &c.metricsHelper.LoopMetrics.Metrics + loadMetric, ok = mountMetrics.Load(mountKeyName) + numEntriesMetric = loadMetric.(metricsutil.GaugeMetric) + if !ok || numEntriesMetric.Value != 4 { + t.Fatalf("mount metrics for num entries do not match true values") + } + if len(numEntriesMetric.Key) != 3 || + numEntriesMetric.Key[0] != "core" || + numEntriesMetric.Key[1] != "mount_table" || + numEntriesMetric.Key[2] != "num_entries" { + t.Fatalf("mount metrics for num entries have wrong key") + } + if len(numEntriesMetric.Labels) != 2 || + numEntriesMetric.Labels[0].Name != "type" || + numEntriesMetric.Labels[0].Value != "logical" || + numEntriesMetric.Labels[1].Name != "local" || + numEntriesMetric.Labels[1].Value != "false" { + t.Fatalf("mount metrics for num entries have wrong labels") + } + mountSizeKeyName := "core.mount_table.size.type|logical||local|false||" + loadMetric, ok = mountMetrics.Load(mountSizeKeyName) + sizeMetric := loadMetric.(metricsutil.GaugeMetric) + + if !ok { + t.Fatalf("mount metrics for size do not match exist") + } + if len(sizeMetric.Key) != 3 || + sizeMetric.Key[0] != "core" || + sizeMetric.Key[1] != "mount_table" || + sizeMetric.Key[2] != "size" { + t.Fatalf("mount metrics for size have wrong key") + } + if len(sizeMetric.Labels) != 2 || + sizeMetric.Labels[0].Name != "type" || + sizeMetric.Labels[0].Value != "logical" || + sizeMetric.Labels[1].Name != "local" || + sizeMetric.Labels[1].Value != "false" { + t.Fatalf("mount metrics for size have wrong labels") + } +} + func TestCore_DefaultMountTable(t *testing.T) { - c, keys, _ := TestCoreUnsealed(t) + c, keys, _, _ := TestCoreUnsealedWithMetrics(t) verifyDefaultTable(t, c.mounts, 4) // Start a second core with same physical + inmemSink := metrics.NewInmemSink(1000000*time.Hour, 2000000*time.Hour) conf := &CoreConfig{ - Physical: c.physical, - DisableMlock: true, + Physical: c.physical, + DisableMlock: true, + BuiltinRegistry: NewMockBuiltinRegistry(), + MetricSink: metricsutil.NewClusterMetricSink("test-cluster", inmemSink), + MetricsHelper: metricsutil.NewMetricsHelper(inmemSink, false), } c2, err := NewCore(conf) if err != nil { @@ -69,7 +141,7 @@ func TestCore_DefaultMountTable(t *testing.T) { } func TestCore_Mount(t *testing.T) { - c, keys, _ := TestCoreUnsealed(t) + c, keys, _, _ := TestCoreUnsealedWithMetrics(t) me := &MountEntry{ Table: mountTableType, Path: "foo", @@ -85,9 +157,13 @@ func TestCore_Mount(t *testing.T) { t.Fatalf("missing mount") } + inmemSink := metrics.NewInmemSink(1000000*time.Hour, 2000000*time.Hour) conf := &CoreConfig{ - Physical: c.physical, - DisableMlock: true, + Physical: c.physical, + DisableMlock: true, + BuiltinRegistry: NewMockBuiltinRegistry(), + MetricSink: metricsutil.NewClusterMetricSink("test-cluster", inmemSink), + MetricsHelper: metricsutil.NewMetricsHelper(inmemSink, false), } c2, err := NewCore(conf) if err != nil { @@ -113,7 +189,7 @@ func TestCore_Mount(t *testing.T) { // entries, and that upon reading the entries from both are recombined // correctly func TestCore_Mount_Local(t *testing.T) { - c, _, _ := TestCoreUnsealed(t) + c, _, _, _ := TestCoreUnsealedWithMetrics(t) c.mounts = &MountTable{ Type: mountTableType, @@ -212,7 +288,7 @@ func TestCore_Mount_Local(t *testing.T) { } func TestCore_Unmount(t *testing.T) { - c, keys, _ := TestCoreUnsealed(t) + c, keys, _, _ := TestCoreUnsealedWithMetrics(t) err := c.unmount(namespace.RootContext(nil), "secret") if err != nil { t.Fatalf("err: %v", err) @@ -223,9 +299,13 @@ func TestCore_Unmount(t *testing.T) { t.Fatalf("backend present") } + inmemSink := metrics.NewInmemSink(1000000*time.Hour, 2000000*time.Hour) conf := &CoreConfig{ - Physical: c.physical, - DisableMlock: true, + Physical: c.physical, + DisableMlock: true, + BuiltinRegistry: NewMockBuiltinRegistry(), + MetricSink: metricsutil.NewClusterMetricSink("test-cluster", inmemSink), + MetricsHelper: metricsutil.NewMetricsHelper(inmemSink, false), } c2, err := NewCore(conf) if err != nil { @@ -254,7 +334,7 @@ func TestCore_Unmount_Cleanup(t *testing.T) { func testCore_Unmount_Cleanup(t *testing.T, causeFailure bool) { noop := &NoopBackend{} - c, _, root := TestCoreUnsealed(t) + c, _, root, _ := TestCoreUnsealedWithMetrics(t) c.logicalBackends["noop"] = func(context.Context, *logical.BackendConfig) (logical.Backend, error) { return noop, nil } @@ -362,7 +442,7 @@ func testCore_Unmount_Cleanup(t *testing.T, causeFailure bool) { } func TestCore_Remount(t *testing.T) { - c, keys, _ := TestCoreUnsealed(t) + c, keys, _, _ := TestCoreUnsealedWithMetrics(t) err := c.remount(namespace.RootContext(nil), "secret", "foo", true) if err != nil { t.Fatalf("err: %v", err) @@ -373,9 +453,13 @@ func TestCore_Remount(t *testing.T) { t.Fatalf("failed remount") } + inmemSink := metrics.NewInmemSink(1000000*time.Hour, 2000000*time.Hour) conf := &CoreConfig{ - Physical: c.physical, - DisableMlock: true, + Physical: c.physical, + DisableMlock: true, + BuiltinRegistry: NewMockBuiltinRegistry(), + MetricSink: metricsutil.NewClusterMetricSink("test-cluster", inmemSink), + MetricsHelper: metricsutil.NewMetricsHelper(inmemSink, false), } c2, err := NewCore(conf) if err != nil { @@ -410,7 +494,7 @@ func TestCore_Remount(t *testing.T) { func TestCore_Remount_Cleanup(t *testing.T) { noop := &NoopBackend{} - c, _, root := TestCoreUnsealed(t) + c, _, root, _ := TestCoreUnsealedWithMetrics(t) c.logicalBackends["noop"] = func(context.Context, *logical.BackendConfig) (logical.Backend, error) { return noop, nil } @@ -494,7 +578,7 @@ func TestCore_Remount_Cleanup(t *testing.T) { } func TestCore_Remount_Protected(t *testing.T) { - c, _, _ := TestCoreUnsealed(t) + c, _, _, _ := TestCoreUnsealedWithMetrics(t) err := c.remount(namespace.RootContext(nil), "sys", "foo", true) if err.Error() != `cannot remount "sys/"` { t.Fatalf("err: %v", err) @@ -502,13 +586,13 @@ func TestCore_Remount_Protected(t *testing.T) { } func TestDefaultMountTable(t *testing.T) { - c, _, _ := TestCoreUnsealed(t) + c, _, _, _ := TestCoreUnsealedWithMetrics(t) table := c.defaultMountTable() verifyDefaultTable(t, table, 3) } func TestCore_MountTable_UpgradeToTyped(t *testing.T) { - c, _, _ := TestCoreUnsealed(t) + c, _, _, _ := TestCoreUnsealedWithMetrics(t) c.auditBackends["noop"] = func(ctx context.Context, config *audit.BackendConfig) (audit.Backend, error) { return &NoopAudit{ @@ -710,7 +794,7 @@ func verifyDefaultTable(t *testing.T, table *MountTable, expected int) { } func TestSingletonMountTableFunc(t *testing.T) { - c, _, _ := TestCoreUnsealed(t) + c, _, _, _ := TestCoreUnsealedWithMetrics(t) mounts, auth := c.singletonMountTables() @@ -743,7 +827,7 @@ func TestCore_MountInitialize(t *testing.T) { BackendType: logical.TypeLogical, }, false} - c, _, _ := TestCoreUnsealed(t) + c, _, _, _ := TestCoreUnsealedWithMetrics(t) c.logicalBackends["initable"] = func(context.Context, *logical.BackendConfig) (logical.Backend, error) { return backend, nil } @@ -768,7 +852,7 @@ func TestCore_MountInitialize(t *testing.T) { BackendType: logical.TypeLogical, }, false} - c, _, _ := TestCoreUnsealed(t) + c, _, _, _ := TestCoreUnsealedWithMetrics(t) c.logicalBackends["initable"] = func(context.Context, *logical.BackendConfig) (logical.Backend, error) { return backend, nil } diff --git a/vault/testing.go b/vault/testing.go index 89ca6daeafcb..a38003dbb608 100644 --- a/vault/testing.go +++ b/vault/testing.go @@ -313,6 +313,7 @@ func TestCoreUnsealedWithMetrics(t testing.T) (*Core, [][]byte, string, *metrics conf := &CoreConfig{ BuiltinRegistry: NewMockBuiltinRegistry(), MetricSink: metricsutil.NewClusterMetricSink("test-cluster", inmemSink), + MetricsHelper: metricsutil.NewMetricsHelper(inmemSink, false), } core, keys, root := testCoreUnsealed(t, TestCoreWithSealAndUI(t, conf)) return core, keys, root, inmemSink diff --git a/website/pages/docs/internals/telemetry.mdx b/website/pages/docs/internals/telemetry.mdx index 124eafd283aa..60c42354590b 100644 --- a/website/pages/docs/internals/telemetry.mdx +++ b/website/pages/docs/internals/telemetry.mdx @@ -92,6 +92,8 @@ These metrics represent operational aspects of the running Vault instance. | `vault.core.leader` | Has value 1 when the vault node is leader, and 0 when node is in standby. | bool | gauge | | `vault.core.leadership_setup_failed` | Duration of time taken by cluster leadership setup failures which have occurred in a highly available Vault cluster. This should be monitored and alerted on for overall cluster leadership status. | ms | summary | | `vault.core.leadership_lost` | Duration of time taken by cluster leadership losses which have occurred in a highly available Vault cluster. This should be monitored and alerted on for overall cluster leadership status. | ms | summary | +| `vault.core.mount_table.num_entries` | Number of mounts in a particular mount table. This metric is labeled by table type (auth or logical) and whether or not the table is replicated (local or not) | objects | summary | +| `vault.core.mount_table.size` | Size of a particular mount table. This metric is labeled by table type (auth or logical) and whether or not the table is replicated (local or not) | objects | summary | | `vault.core.post_unseal` | Duration of time taken by post-unseal operations handled by Vault core | ms | summary | | `vault.core.pre_seal` | Duration of time taken by pre-seal operations | ms | summary | | `vault.core.seal-with-request` | Duration of time taken by requested seal operations | ms | summary | From ef938a30ec2b1525c09e834a036c4c2df7415fdd Mon Sep 17 00:00:00 2001 From: Hridoy Roy Date: Tue, 20 Oct 2020 15:43:05 -0700 Subject: [PATCH 2/5] update --- vault/auth.go | 2 +- vault/mount.go | 69 ++++++++++++++------------------------------------ 2 files changed, 20 insertions(+), 51 deletions(-) diff --git a/vault/auth.go b/vault/auth.go index 362415901cab..5e33c6d95c93 100644 --- a/vault/auth.go +++ b/vault/auth.go @@ -425,7 +425,7 @@ func (c *Core) taintCredEntry(ctx context.Context, path string, updateStorage bo // Taint the entry from the auth table // We do this on the original since setting the taint operates // on the entries which a shallow clone shares anyways - entry, err := c.auth.setTaint(ctx, strings.TrimPrefix(path, credentialRoutePrefix), true, mountStateUnmounting) + entry, err := c.auth.setTaint(ctx, strings.TrimPrefix(path, credentialRoutePrefix), true) if err != nil { return err } diff --git a/vault/mount.go b/vault/mount.go index 30d303a900cc..ea903f43bb40 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -182,7 +182,7 @@ func (t *MountTable) shallowClone() *MountTable { // setTaint is used to set the taint on given entry Accepts either the mount // entry's path or namespace + path, i.e. /secret/ or /token/ -func (t *MountTable) setTaint(ctx context.Context, path string, tainted bool, mountState string) (*MountEntry, error) { +func (t *MountTable) setTaint(ctx context.Context, path string, value bool) (*MountEntry, error) { n := len(t.Entries) ns, err := namespace.FromContext(ctx) if err != nil { @@ -190,8 +190,7 @@ func (t *MountTable) setTaint(ctx context.Context, path string, tainted bool, mo } for i := 0; i < n; i++ { if entry := t.Entries[i]; entry.Path == path && entry.Namespace().ID == ns.ID { - t.Entries[i].Tainted = tainted - t.Entries[i].MountState = mountState + t.Entries[i].Tainted = value return t.Entries[i], nil } } @@ -250,8 +249,6 @@ func (t *MountTable) sortEntriesByPathDepth() *MountTable { return t } -const mountStateUnmounting = "unmounting" - // MountEntry is used to represent a mount table entry type MountEntry struct { Table string `json:"table"` // The table it belongs to @@ -267,7 +264,6 @@ type MountEntry struct { SealWrap bool `json:"seal_wrap"` // Whether to wrap CSPs ExternalEntropyAccess bool `json:"external_entropy_access"` // Whether to allow external entropy source access Tainted bool `json:"tainted,omitempty"` // Set as a Write-Ahead flag for unmount/remount - MountState string `json:"mount_state,omitempty"` // The current mount state. The only non-empty mount state right now is "unmounting" NamespaceID string `json:"namespace_id"` // namespace contains the populated namespace @@ -641,7 +637,7 @@ func (c *Core) unmountInternal(ctx context.Context, path string, updateStorage b entry := c.router.MatchingMountEntry(ctx, path) // Mark the entry as tainted - if err := c.taintMountEntry(ctx, path, updateStorage, true); err != nil { + if err := c.taintMountEntry(ctx, path, updateStorage); err != nil { c.logger.Error("failed to taint mount entry for path being unmounted", "error", err, "path", path) return err } @@ -759,18 +755,13 @@ func (c *Core) removeMountEntry(ctx context.Context, path string, updateStorage } // taintMountEntry is used to mark an entry in the mount table as tainted -func (c *Core) taintMountEntry(ctx context.Context, path string, updateStorage, unmounting bool) error { +func (c *Core) taintMountEntry(ctx context.Context, path string, updateStorage bool) error { c.mountsLock.Lock() defer c.mountsLock.Unlock() - mountState := "" - if unmounting { - mountState = mountStateUnmounting - } - // As modifying the taint of an entry affects shallow clones, // we simply use the original - entry, err := c.mounts.setTaint(ctx, path, true, mountState) + entry, err := c.mounts.setTaint(ctx, path, true) if err != nil { return err } @@ -865,7 +856,7 @@ func (c *Core) remount(ctx context.Context, src, dst string, updateStorage bool) } // Mark the entry as tainted - if err := c.taintMountEntry(ctx, src, updateStorage, false); err != nil { + if err := c.taintMountEntry(ctx, src, updateStorage); err != nil { return err } @@ -989,38 +980,9 @@ func (c *Core) loadMounts(ctx context.Context) error { } } - // If this node is a performance standby we do not want to attempt to - // upgrade the mount table, this will be the active node's responsibility. - if !c.perfStandby { - err := c.runMountUpdates(ctx, needPersist) - if err != nil { - c.logger.Error("failed to run mount table upgrades", "error", err) - return err - } - } - - for _, entry := range c.mounts.Entries { - if entry.NamespaceID == "" { - entry.NamespaceID = namespace.RootNamespaceID - } - ns, err := NamespaceByID(ctx, entry.NamespaceID, c) - if err != nil { - return err - } - if ns == nil { - return namespace.ErrNoNamespace - } - entry.namespace = ns - - // Sync values to the cache - entry.SyncCache() - } - return nil -} + // Note that this is only designed to work with singletons, as it checks by + // type only. -// Note that this is only designed to work with singletons, as it checks by -// type only. -func (c *Core) runMountUpdates(ctx context.Context, needPersist bool) error { // Upgrade to typed mount table if c.mounts.Type == "" { c.mounts.Type = mountTableType @@ -1052,10 +1014,6 @@ func (c *Core) runMountUpdates(ctx context.Context, needPersist bool) error { // Upgrade to table-scoped entries for _, entry := range c.mounts.Entries { - if !c.PR1103disabled && entry.Type == mountTypeNSCubbyhole && !entry.Local && !c.ReplicationState().HasState(consts.ReplicationPerformanceSecondary|consts.ReplicationDRSecondary) { - entry.Local = true - needPersist = true - } if entry.Type == cubbyholeMountType && !entry.Local { entry.Local = true needPersist = true @@ -1085,6 +1043,17 @@ func (c *Core) runMountUpdates(ctx context.Context, needPersist bool) error { entry.NamespaceID = namespace.RootNamespaceID needPersist = true } + ns, err := NamespaceByID(ctx, entry.NamespaceID, c) + if err != nil { + return err + } + if ns == nil { + return namespace.ErrNoNamespace + } + entry.namespace = ns + + // Sync values to the cache + entry.SyncCache() } // Done if we have restored the mount table and we don't need From 2d2af0e86593af85794c32f6bf0f5781073df588 Mon Sep 17 00:00:00 2001 From: Hridoy Roy Date: Tue, 20 Oct 2020 15:48:11 -0700 Subject: [PATCH 3/5] removed some ent features from backport --- vault/core_metrics.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/vault/core_metrics.go b/vault/core_metrics.go index cb6876ca8bb0..a68a493e5cff 100644 --- a/vault/core_metrics.go +++ b/vault/core_metrics.go @@ -68,8 +68,8 @@ func (c *Core) metricsLoop(stopCh chan struct{}) { } c.stateLock.RUnlock() case <-identityCountTimer: - // Only emit on active node of cluster that is not a DR cecondary. - if standby, _ := c.Standby(); standby || c.IsDRSecondary() { + // Only emit on active node + if c.PerfStandby() { break } @@ -196,11 +196,10 @@ func (c *Core) emitMetrics(stopCh chan struct{}) { }, } - // Disable collection if configured, or if we're a performance standby - // node or DR secondary cluster. + // Disable collection if configured, or if we're a performance standby. if c.MetricSink().GaugeInterval == time.Duration(0) { c.logger.Info("usage gauge collection is disabled") - } else if standby, _ := c.Standby(); !standby && !c.IsDRSecondary() { + } else if !c.PerfStandby() { for _, init := range metricsInit { if init.DisableEnvVar != "" { if os.Getenv(init.DisableEnvVar) != "" { From 46b6e3e2c56dc76364790b60d094ff3d8342472e Mon Sep 17 00:00:00 2001 From: Hridoy Roy Date: Tue, 20 Oct 2020 16:08:22 -0700 Subject: [PATCH 4/5] final refactor --- vault/auth.go | 12 ++++-------- vault/mount.go | 12 ++++-------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/vault/auth.go b/vault/auth.go index 5e33c6d95c93..11efc118295b 100644 --- a/vault/auth.go +++ b/vault/auth.go @@ -574,15 +574,11 @@ func (c *Core) persistAuth(ctx context.Context, table *MountTable, local *bool) Type: credentialTableType, } - var localAuthCount int - var nonLocalAuthCount int for _, entry := range table.Entries { if entry.Local { localAuth.Entries = append(localAuth.Entries, entry) - localAuthCount += 1 } else { nonLocalAuth.Entries = append(nonLocalAuth.Entries, entry) - nonLocalAuthCount += 1 } } @@ -617,26 +613,26 @@ func (c *Core) persistAuth(ctx context.Context, table *MountTable, local *bool) if err != nil { return err } - c.tableMetrics(nonLocalAuthCount, false, true, compressedBytes) + c.tableMetrics(len(nonLocalAuth.Entries), false, true, compressedBytes) // Write local mounts compressedBytes, err = writeTable(localAuth, coreLocalAuthConfigPath) if err != nil { return err } - c.tableMetrics(localAuthCount, true, true, compressedBytes) + c.tableMetrics(len(localAuth.Entries), true, true, compressedBytes) case *local: compressedBytes, err = writeTable(localAuth, coreLocalAuthConfigPath) if err != nil { return err } - c.tableMetrics(localAuthCount, true, true, compressedBytes) + c.tableMetrics(len(localAuth.Entries), true, true, compressedBytes) default: compressedBytes, err = writeTable(nonLocalAuth, coreAuthConfigPath) if err != nil { return err } - c.tableMetrics(nonLocalAuthCount, false, true, compressedBytes) + c.tableMetrics(len(nonLocalAuth.Entries), false, true, compressedBytes) } return err diff --git a/vault/mount.go b/vault/mount.go index ea903f43bb40..cc028c345a36 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -1092,15 +1092,11 @@ func (c *Core) persistMounts(ctx context.Context, table *MountTable, local *bool Type: mountTableType, } - var localMountsCount int - var nonLocalMountsCount int for _, entry := range table.Entries { if entry.Local { localMounts.Entries = append(localMounts.Entries, entry) - localMountsCount += 1 } else { nonLocalMounts.Entries = append(nonLocalMounts.Entries, entry) - nonLocalMountsCount += 1 } } @@ -1135,14 +1131,14 @@ func (c *Core) persistMounts(ctx context.Context, table *MountTable, local *bool if err != nil { return err } - c.tableMetrics(nonLocalMountsCount, false, false, compressedBytes) + c.tableMetrics(len(nonLocalMounts.Entries), false, false, compressedBytes) // Write local mounts compressedBytes, err = writeTable(localMounts, coreLocalMountConfigPath) if err != nil { return err } - c.tableMetrics(localMountsCount, true, false, compressedBytes) + c.tableMetrics(len(localMounts.Entries), true, false, compressedBytes) case *local: // Write local mounts @@ -1150,14 +1146,14 @@ func (c *Core) persistMounts(ctx context.Context, table *MountTable, local *bool if err != nil { return err } - c.tableMetrics(localMountsCount, true, false, compressedBytes) + c.tableMetrics(len(localMounts.Entries), true, false, compressedBytes) default: // Write non-local mounts compressedBytes, err = writeTable(nonLocalMounts, coreMountConfigPath) if err != nil { return err } - c.tableMetrics(nonLocalMountsCount, false, false, compressedBytes) + c.tableMetrics(len(nonLocalMounts.Entries), false, false, compressedBytes) } return nil From 20fca6eefa44f7d9a3df085b5c1427a5f8219e9a Mon Sep 17 00:00:00 2001 From: Hridoy Roy Date: Wed, 21 Oct 2020 09:44:52 -0700 Subject: [PATCH 5/5] backport patch --- vault/core_metrics.go | 4 ++++ vault/mount.go | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/vault/core_metrics.go b/vault/core_metrics.go index a68a493e5cff..122bd615445b 100644 --- a/vault/core_metrics.go +++ b/vault/core_metrics.go @@ -430,6 +430,10 @@ func (c *Core) entityGaugeCollectorByMount(ctx context.Context) ([]metricsutil.G } func (c *Core) cachedGaugeMetricsEmitter() { + if c.metricsHelper == nil { + return + } + loopMetrics := &c.metricsHelper.LoopMetrics.Metrics emit := func(key interface{}, value interface{}) bool { diff --git a/vault/mount.go b/vault/mount.go index cc028c345a36..fb4aac99a910 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -137,6 +137,10 @@ type MountTable struct { // padding, which is negligable and subject to change, and thus // not accounted for. func (c *Core) tableMetrics(entryCount int, isLocal bool, isAuth bool, compressedTable []byte) { + if c.metricsHelper == nil { + // do nothing if metrics are not initialized + return + } typeAuthLabelMap := map[bool]metrics.Label{ true: metrics.Label{Name: "type", Value: "auth"}, false: metrics.Label{Name: "type", Value: "logical"},