diff --git a/changelog/18916.txt b/changelog/18916.txt new file mode 100644 index 000000000..eb2792b31 --- /dev/null +++ b/changelog/18916.txt @@ -0,0 +1,3 @@ +```release-note:bug +core/activity: report mount paths (rather than mount accessors) in current month activity log counts and include deleted mount paths in precomputed queries. +``` diff --git a/vault/activity_log.go b/vault/activity_log.go index de42223df..965d34a66 100644 --- a/vault/activity_log.go +++ b/vault/activity_log.go @@ -2166,13 +2166,8 @@ func (a *ActivityLog) precomputedQueryWorker(ctx context.Context) error { for nsID, entry := range byNamespace { mountRecord := make([]*activity.MountRecord, 0, len(entry.Mounts)) for mountAccessor, mountData := range entry.Mounts { - valResp := a.core.router.ValidateMountByAccessor(mountAccessor) - if valResp == nil { - // Only persist valid mounts - continue - } mountRecord = append(mountRecord, &activity.MountRecord{ - MountPath: valResp.MountPath, + MountPath: a.mountAccessorToMountPath(mountAccessor), Counts: &activity.CountsRecord{ EntityClients: len(mountData.Counts.Entities), NonEntityClients: int(mountData.Counts.Tokens) + len(mountData.Counts.NonEntities), @@ -2339,20 +2334,8 @@ func (a *ActivityLog) transformMonthBreakdowns(byMonth map[int64]*processMonth) // Process mount specific data within a namespace within a given month mountRecord := make([]*activity.MountRecord, 0, len(nsMap[nsID].Mounts)) for mountAccessor, mountData := range nsMap[nsID].Mounts { - var displayPath string - if mountAccessor == "" { - displayPath = "no mount accessor (pre-1.10 upgrade?)" - } else { - valResp := a.core.router.ValidateMountByAccessor(mountAccessor) - if valResp == nil { - displayPath = fmt.Sprintf("deleted mount; accessor %q", mountAccessor) - } else { - displayPath = valResp.MountPath - } - } - mountRecord = append(mountRecord, &activity.MountRecord{ - MountPath: displayPath, + MountPath: a.mountAccessorToMountPath(mountAccessor), Counts: &activity.CountsRecord{ EntityClients: len(mountData.Counts.Entities), NonEntityClients: int(mountData.Counts.Tokens) + len(mountData.Counts.NonEntities), diff --git a/vault/activity_log_test.go b/vault/activity_log_test.go index 40373a131..69887a39d 100644 --- a/vault/activity_log_test.go +++ b/vault/activity_log_test.go @@ -17,6 +17,8 @@ import ( "testing" "time" + "github.com/hashicorp/go-uuid" + "github.com/axiomhq/hyperloglog" "github.com/go-test/deep" "github.com/golang/protobuf/proto" @@ -3984,3 +3986,122 @@ func TestActivityLog_partialMonthClientCountUsingHandleQuery(t *testing.T) { } } } + +// TestActivityLog_partialMonthClientCountWithMultipleMountPaths verifies that logic in refreshFromStoredLog includes all mount paths +// in its mount data. In this test we create 3 entity records with different mount accessors: one is empty, one is +// valid, one can't be found (so it's assumed the mount is deleted). These records are written to storage, then this data is +// refreshed in refreshFromStoredLog, and finally we verify the results returned with partialMonthClientCount. +func TestActivityLog_partialMonthClientCountWithMultipleMountPaths(t *testing.T) { + timeutil.SkipAtEndOfMonth(t) + + core, _, _ := TestCoreUnsealed(t) + _, barrier, _ := mockBarrier(t) + view := NewBarrierView(barrier, "auth/") + + ctx := namespace.RootContext(nil) + now := time.Now().UTC() + meUUID, err := uuid.GenerateUUID() + if err != nil { + t.Fatal(err) + } + + a := core.activityLog + path := "auth/foo/bar" + accessor := "authfooaccessor" + + // we mount a path using the accessor 'authfooaccessor' which has mount path "auth/foo/bar" + // when an entity record references this accessor, activity log will be able to find it on its mounts and translate the mount accessor + // into a mount path + err = core.router.Mount(&NoopBackend{}, "auth/foo/", &MountEntry{UUID: meUUID, Accessor: accessor, NamespaceID: namespace.RootNamespaceID, namespace: namespace.RootNamespace, Path: path}, view) + if err != nil { + t.Fatalf("err: %v", err) + } + + entityRecords := []*activity.EntityRecord{ + { + // this record has no mount accessor, so it'll get recorded as a pre-1.10 upgrade + ClientID: "11111111-1111-1111-1111-111111111111", + NamespaceID: namespace.RootNamespaceID, + Timestamp: time.Now().Unix(), + }, + { + // this record's mount path won't be able to be found, because there's no mount with the accessor 'deleted' + // the code in mountAccessorToMountPath assumes that if the mount accessor isn't empty but the mount path + // can't be found, then the mount must have been deleted + ClientID: "22222222-2222-2222-2222-222222222222", + NamespaceID: namespace.RootNamespaceID, + Timestamp: time.Now().Unix(), + MountAccessor: "deleted", + }, + { + // this record will have mount path 'auth/foo/bar', because we set up the mount above + ClientID: "33333333-2222-2222-2222-222222222222", + NamespaceID: namespace.RootNamespaceID, + Timestamp: time.Now().Unix(), + MountAccessor: "authfooaccessor", + }, + } + for i, entityRecord := range entityRecords { + entityData, err := proto.Marshal(&activity.EntityActivityLog{ + Clients: []*activity.EntityRecord{entityRecord}, + }) + if err != nil { + t.Fatalf(err.Error()) + } + storagePath := fmt.Sprintf("%sentity/%d/%d", ActivityLogPrefix, timeutil.StartOfMonth(now).Unix(), i) + WriteToStorage(t, core, storagePath, entityData) + } + + a.SetEnable(true) + var wg sync.WaitGroup + err = a.refreshFromStoredLog(ctx, &wg, now) + if err != nil { + t.Fatalf("error loading clients: %v", err) + } + wg.Wait() + + results, err := a.partialMonthClientCount(ctx) + if err != nil { + t.Fatal(err) + } + if results == nil { + t.Fatal("no results to test") + } + + byNamespace, ok := results["by_namespace"] + if !ok { + t.Fatalf("malformed results. got %v", results) + } + + clientCountResponse := make([]*ResponseNamespace, 0) + err = mapstructure.Decode(byNamespace, &clientCountResponse) + if err != nil { + t.Fatal(err) + } + if len(clientCountResponse) != 1 { + t.Fatalf("incorrect client count responses, expected 1 but got %d", len(clientCountResponse)) + } + if len(clientCountResponse[0].Mounts) != len(entityRecords) { + t.Fatalf("incorrect client mounts, expected %d but got %d", len(entityRecords), len(clientCountResponse[0].Mounts)) + } + byPath := make(map[string]int, len(clientCountResponse[0].Mounts)) + for _, mount := range clientCountResponse[0].Mounts { + byPath[mount.MountPath] = byPath[mount.MountPath] + mount.Counts.Clients + } + + // these are the paths that are expected and correspond with the entity records created above + expectedPaths := []string{ + noMountAccessor, + fmt.Sprintf(deletedMountFmt, "deleted"), + path, + } + for _, expectedPath := range expectedPaths { + count, ok := byPath[expectedPath] + if !ok { + t.Fatalf("path %s not found", expectedPath) + } + if count != 1 { + t.Fatalf("incorrect count value %d for path %s", count, expectedPath) + } + } +} diff --git a/vault/activity_log_util_common.go b/vault/activity_log_util_common.go index 080f5e543..256b17aaf 100644 --- a/vault/activity_log_util_common.go +++ b/vault/activity_log_util_common.go @@ -207,9 +207,9 @@ func (a *ActivityLog) limitNamespacesInALResponse(byNamespaceResponse []*Respons // For more details, please see the function comment for transformMonthlyNamespaceBreakdowns func (a *ActivityLog) transformActivityLogMounts(mts map[string]*processMount) []*activity.MountRecord { mounts := make([]*activity.MountRecord, 0) - for mountpath, mountCounts := range mts { + for mountAccessor, mountCounts := range mts { mount := activity.MountRecord{ - MountPath: mountpath, + MountPath: a.mountAccessorToMountPath(mountAccessor), Counts: &activity.CountsRecord{ EntityClients: len(mountCounts.Counts.Entities), NonEntityClients: len(mountCounts.Counts.NonEntities) + int(mountCounts.Counts.Tokens), @@ -259,3 +259,25 @@ func (a *ActivityLog) sortActivityLogMonthsResponse(months []*ResponseMonth) { } } } + +const ( + noMountAccessor = "no mount accessor (pre-1.10 upgrade?)" + deletedMountFmt = "deleted mount; accessor %q" +) + +// mountAccessorToMountPath transforms the mount accessor to the mount path +// returns a placeholder string if the mount accessor is empty or deleted +func (a *ActivityLog) mountAccessorToMountPath(mountAccessor string) string { + var displayPath string + if mountAccessor == "" { + displayPath = noMountAccessor + } else { + valResp := a.core.router.ValidateMountByAccessor(mountAccessor) + if valResp == nil { + displayPath = fmt.Sprintf(deletedMountFmt, mountAccessor) + } else { + displayPath = valResp.MountPath + } + } + return displayPath +}