Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VAULT-13061: Fix mount path discrepancy in activity log #18916

Merged
merged 3 commits into from
Feb 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 2 additions & 19 deletions vault/activity_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to verify that this is the correct behavior - it seems wrong to me that these currently get thrown out, but I definitely might be missing something

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and to clarify, deleted mounts were being excluded from the per-mount breakdown of counts. They were still present in the top-level totals.

}
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),
Expand Down Expand Up @@ -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),
Expand Down
104 changes: 104 additions & 0 deletions vault/activity_log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -3984,3 +3986,105 @@ func TestActivityLog_partialMonthClientCountUsingHandleQuery(t *testing.T) {
}
}
}

miagilepner marked this conversation as resolved.
Show resolved Hide resolved
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"
err = core.router.Mount(&NoopBackend{}, "auth/foo/", &MountEntry{UUID: meUUID, Accessor: "authfooaccessor", NamespaceID: namespace.RootNamespaceID, namespace: namespace.RootNamespace, Path: path}, view)
if err != nil {
t.Fatalf("err: %v", err)
}

entityRecords := []*activity.EntityRecord{
{
ClientID: "11111111-1111-1111-1111-111111111111",
NamespaceID: namespace.RootNamespaceID,
Timestamp: time.Now().Unix(),
},
{
ClientID: "22222222-2222-2222-2222-222222222222",
NamespaceID: namespace.RootNamespaceID,
Timestamp: time.Now().Unix(),
MountAccessor: "deleted",
},
{
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())
}
WriteToStorage(t, core, ActivityLogPrefix+"entity/"+fmt.Sprint(timeutil.StartOfMonth(now).Unix())+"/"+strconv.Itoa(i), entityData)
miagilepner marked this conversation as resolved.
Show resolved Hide resolved
}

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.Fatal("incorrect client count responses")
miagilepner marked this conversation as resolved.
Show resolved Hide resolved
}
if len(clientCountResponse[0].Mounts) != 3 {
miagilepner marked this conversation as resolved.
Show resolved Hide resolved
t.Fatal("incorrect client 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
}
expectedPaths := []string{
mpalmi marked this conversation as resolved.
Show resolved Hide resolved
path,
noMountAccessor,
fmt.Sprintf(deletedMountFmt, "deleted"),
}
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)
}
}
}
26 changes: 24 additions & 2 deletions vault/activity_log_util_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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
}