From b88bb9269038fe7602bdee3ad90af55370a3bbc7 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Thu, 19 Aug 2021 10:22:15 -0700 Subject: [PATCH 01/11] vault-2823 adding changes --- vault/activity_log.go | 104 ++++++++++++++++++++++++++++++++---------- 1 file changed, 81 insertions(+), 23 deletions(-) diff --git a/vault/activity_log.go b/vault/activity_log.go index 6667d6bd24aba..548de73821a8c 100644 --- a/vault/activity_log.go +++ b/vault/activity_log.go @@ -143,6 +143,10 @@ type ActivityLog struct { // for testing: is config currently being invalidated. protected by l configInvalidationInProgress bool + + // All known active entity count by namespace ID this month; use fragmentLock read-locked + // to check whether it already exists. + entityCountByNamespaceID map[string]uint64 } // These non-persistent configuration options allow us to disable @@ -164,17 +168,18 @@ func NewActivityLog(core *Core, logger log.Logger, view *BarrierView, metrics me } a := &ActivityLog{ - core: core, - configOverrides: &core.activityLogConfig, - logger: logger, - view: view, - metrics: metrics, - nodeID: hostname, - newFragmentCh: make(chan struct{}, 1), - sendCh: make(chan struct{}, 1), // buffered so it can be triggered by fragment size - writeCh: make(chan struct{}, 1), // same for full segment - doneCh: make(chan struct{}, 1), - activeEntities: make(map[string]struct{}), + core: core, + configOverrides: &core.activityLogConfig, + logger: logger, + view: view, + metrics: metrics, + nodeID: hostname, + newFragmentCh: make(chan struct{}, 1), + sendCh: make(chan struct{}, 1), // buffered so it can be triggered by fragment size + writeCh: make(chan struct{}, 1), // same for full segment + doneCh: make(chan struct{}, 1), + activeEntities: make(map[string]struct{}), + entityCountByNamespaceID: make(map[string]uint64), currentSegment: segmentInfo{ startTimestamp: 0, currentEntities: &activity.EntityActivityLog{ @@ -531,7 +536,14 @@ func (a *ActivityLog) loadPriorEntitySegment(ctx context.Context, startTime time // Or the feature has been disabled. if a.enabled && startTime.Unix() == a.currentSegment.startTimestamp { for _, ent := range out.Entities { - a.activeEntities[ent.EntityID] = struct{}{} + //check if the entity already exists in the list of activeEntities this month + //if entity doesnot exist in active Entities, increment the namespaceID count + if _, ok := a.activeEntities[ent.EntityID]; !ok { + //do something here + a.activeEntities[ent.EntityID] = struct{}{} + a.entityCountByNamespaceID[ent.NamespaceID] = a.entityCountByNamespaceID[ent.NamespaceID] + 1 + } + } } a.fragmentLock.Unlock() @@ -571,7 +583,13 @@ func (a *ActivityLog) loadCurrentEntitySegment(ctx context.Context, startTime ti } for _, ent := range out.Entities { - a.activeEntities[ent.EntityID] = struct{}{} + //check if the entity already exists in the list of activeEntities this month + //if entity doesnot exist in active Entities, increment the namespaceID count + if _, ok := a.activeEntities[ent.EntityID]; !ok { + a.activeEntities[ent.EntityID] = struct{}{} + a.entityCountByNamespaceID[ent.NamespaceID] = a.entityCountByNamespaceID[ent.NamespaceID] + 1 + } + } return nil @@ -684,6 +702,7 @@ func (a *ActivityLog) resetCurrentLog() { a.fragment = nil a.activeEntities = make(map[string]struct{}) + a.entityCountByNamespaceID = make(map[string]uint64) a.standbyFragmentsReceived = make([]*activity.LogFragment, 0) } @@ -1095,6 +1114,7 @@ func (a *ActivityLog) perfStandbyFragmentWorker() { // clear active entity set a.fragmentLock.Lock() a.activeEntities = make(map[string]struct{}) + a.entityCountByNamespaceID = make(map[string]uint64) a.fragmentLock.Unlock() // Set timer for next month. @@ -1309,6 +1329,8 @@ func (a *ActivityLog) AddEntityToFragment(entityID string, namespaceID string, t NamespaceID: namespaceID, Timestamp: timestamp, }) + //incrementing the entity by namespace id count map + a.entityCountByNamespaceID[namespaceID] = a.entityCountByNamespaceID[namespaceID] + 1 a.activeEntities[entityID] = struct{}{} } @@ -1353,7 +1375,12 @@ func (a *ActivityLog) receivedFragment(fragment *activity.LogFragment) { } for _, e := range fragment.Entities { - a.activeEntities[e.EntityID] = struct{}{} + //check if the entity already exists in the list of activeEntities this month + //if entity doesnot exist in active Entities, increment the namespaceID count + if _, ok := a.activeEntities[e.EntityID]; !ok { + a.activeEntities[e.EntityID] = struct{}{} + a.entityCountByNamespaceID[e.NamespaceID] = a.entityCountByNamespaceID[e.NamespaceID] + 1 + } } a.standbyFragmentsReceived = append(a.standbyFragmentsReceived, fragment) @@ -1801,17 +1828,48 @@ func (a *ActivityLog) partialMonthClientCount(ctx context.Context) map[string]in return nil } - entityCount := len(a.activeEntities) - var tokenCount int - for _, countByNS := range a.currentSegment.tokenCount.CountByNamespaceID { - tokenCount += int(countByNS) + responseData := make(map[string]interface{}) + + byNamespace := make([]*ClientCountInNamespace, 0) + queryNS, err := namespace.FromContext(ctx) + if err != nil { + return responseData } - clientCount := entityCount + tokenCount - responseData := make(map[string]interface{}) - responseData["distinct_entities"] = entityCount - responseData["non_entity_tokens"] = tokenCount - responseData["clients"] = clientCount + totalEntities := 0 + totalTokens := 0 + for nsID, entityCount := range a.entityCountByNamespaceID { + ns, err := NamespaceByID(ctx, nsID, a.core) + if err != nil { + return responseData + } + if a.includeInResponse(queryNS, ns) { + var displayPath string + if ns == nil { + displayPath = fmt.Sprintf("deleted namespace %q", nsID) + } else { + displayPath = ns.Path + } + byNamespace = append(byNamespace, &ClientCountInNamespace{ + NamespaceID: nsID, + NamespacePath: displayPath, + Counts: ClientCountResponse{ + DistinctEntities: int(entityCount), + NonEntityTokens: int(a.currentSegment.tokenCount.CountByNamespaceID[nsID]), + Clients: int(entityCount + a.currentSegment.tokenCount.CountByNamespaceID[nsID]), + }, + }) + totalEntities += int(entityCount) + totalTokens += int(a.currentSegment.tokenCount.CountByNamespaceID[nsID]) + } + } + + responseData["by_namespace"] = byNamespace + responseData["total"] = &ClientCountResponse{ + DistinctEntities: totalEntities, + NonEntityTokens: totalTokens, + Clients: totalEntities + totalTokens, + } return responseData } From 1d61470cbf0cd6145b4d46d27b1060f0c2e15105 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Thu, 19 Aug 2021 14:58:26 -0700 Subject: [PATCH 02/11] VAULT-2823 adding alias --- vault/activity_log.go | 57 ++++++++++---------------------- vault/logical_system_activity.go | 2 +- 2 files changed, 18 insertions(+), 41 deletions(-) diff --git a/vault/activity_log.go b/vault/activity_log.go index 548de73821a8c..4bda4258212ef 100644 --- a/vault/activity_log.go +++ b/vault/activity_log.go @@ -12,6 +12,8 @@ import ( "sync" "time" + "q" + "github.com/golang/protobuf/proto" log "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/helper/metricsutil" @@ -21,6 +23,11 @@ import ( "github.com/hashicorp/vault/vault/activity" ) +//adding an alias for partialMonthClientCount +//open source vault calls partialMonthClientCount +//enterprise calls entPartialMonthClientCount +var PartialMonthClientCount = (*ActivityLog).partialMonthClientCount + const ( // activitySubPath is the directory under the system view where // the log will be stored. @@ -1820,6 +1827,7 @@ func (c *Core) activeEntityGaugeCollector(ctx context.Context) ([]metricsutil.Ga // partialMonthClientCount returns the number of clients used so far this month. // If activity log is not enabled, the response will be nil func (a *ActivityLog) partialMonthClientCount(ctx context.Context) map[string]interface{} { + q.Q("open source function called") a.fragmentLock.RLock() defer a.fragmentLock.RUnlock() @@ -1828,48 +1836,17 @@ func (a *ActivityLog) partialMonthClientCount(ctx context.Context) map[string]in return nil } - responseData := make(map[string]interface{}) - - byNamespace := make([]*ClientCountInNamespace, 0) - queryNS, err := namespace.FromContext(ctx) - if err != nil { - return responseData + entityCount := len(a.activeEntities) + var tokenCount int + for _, countByNS := range a.currentSegment.tokenCount.CountByNamespaceID { + tokenCount += int(countByNS) } + clientCount := entityCount + tokenCount - totalEntities := 0 - totalTokens := 0 - - for nsID, entityCount := range a.entityCountByNamespaceID { - ns, err := NamespaceByID(ctx, nsID, a.core) - if err != nil { - return responseData - } - if a.includeInResponse(queryNS, ns) { - var displayPath string - if ns == nil { - displayPath = fmt.Sprintf("deleted namespace %q", nsID) - } else { - displayPath = ns.Path - } - byNamespace = append(byNamespace, &ClientCountInNamespace{ - NamespaceID: nsID, - NamespacePath: displayPath, - Counts: ClientCountResponse{ - DistinctEntities: int(entityCount), - NonEntityTokens: int(a.currentSegment.tokenCount.CountByNamespaceID[nsID]), - Clients: int(entityCount + a.currentSegment.tokenCount.CountByNamespaceID[nsID]), - }, - }) - totalEntities += int(entityCount) - totalTokens += int(a.currentSegment.tokenCount.CountByNamespaceID[nsID]) - } - } + responseData := make(map[string]interface{}) + responseData["distinct_entities"] = entityCount + responseData["non_entity_tokens"] = tokenCount + responseData["clients"] = clientCount - responseData["by_namespace"] = byNamespace - responseData["total"] = &ClientCountResponse{ - DistinctEntities: totalEntities, - NonEntityTokens: totalTokens, - Clients: totalEntities + totalTokens, - } return responseData } diff --git a/vault/logical_system_activity.go b/vault/logical_system_activity.go index ca1a8fa5d9a85..aeb4216536325 100644 --- a/vault/logical_system_activity.go +++ b/vault/logical_system_activity.go @@ -142,7 +142,7 @@ func (b *SystemBackend) handleMonthlyActivityCount(ctx context.Context, req *log return logical.ErrorResponse("no activity log present"), nil } - results := a.partialMonthClientCount(ctx) + results := PartialMonthClientCount(a, ctx) if results == nil { return logical.RespondWithStatusCode(nil, req, http.StatusNoContent) } From b601a56b0f222c6b189b07cd861a390bf2f22bed Mon Sep 17 00:00:00 2001 From: akshya96 Date: Fri, 20 Aug 2021 15:57:26 -0700 Subject: [PATCH 03/11] Vault-2823 addressing comments --- vault/activity_log.go | 127 ++++++++++++++++++++++--------- vault/activity_log_test.go | 82 +++++++++++++++++++- vault/logical_system_activity.go | 7 +- 3 files changed, 176 insertions(+), 40 deletions(-) diff --git a/vault/activity_log.go b/vault/activity_log.go index 4bda4258212ef..c8da310079013 100644 --- a/vault/activity_log.go +++ b/vault/activity_log.go @@ -26,7 +26,6 @@ import ( //adding an alias for partialMonthClientCount //open source vault calls partialMonthClientCount //enterprise calls entPartialMonthClientCount -var PartialMonthClientCount = (*ActivityLog).partialMonthClientCount const ( // activitySubPath is the directory under the system view where @@ -74,6 +73,11 @@ type segmentInfo struct { entitySequenceNumber uint64 } +type clients struct { + distinctEntities uint64 + nonEntityTokens uint64 +} + // ActivityLog tracks unique entity counts and non-entity token counts. // It handles assembling log fragments (and sending them to the active // node), writing log segments, and precomputing queries. @@ -543,14 +547,7 @@ func (a *ActivityLog) loadPriorEntitySegment(ctx context.Context, startTime time // Or the feature has been disabled. if a.enabled && startTime.Unix() == a.currentSegment.startTimestamp { for _, ent := range out.Entities { - //check if the entity already exists in the list of activeEntities this month - //if entity doesnot exist in active Entities, increment the namespaceID count - if _, ok := a.activeEntities[ent.EntityID]; !ok { - //do something here - a.activeEntities[ent.EntityID] = struct{}{} - a.entityCountByNamespaceID[ent.NamespaceID] = a.entityCountByNamespaceID[ent.NamespaceID] + 1 - } - + a.AddEntity(ent) } } a.fragmentLock.Unlock() @@ -590,13 +587,7 @@ func (a *ActivityLog) loadCurrentEntitySegment(ctx context.Context, startTime ti } for _, ent := range out.Entities { - //check if the entity already exists in the list of activeEntities this month - //if entity doesnot exist in active Entities, increment the namespaceID count - if _, ok := a.activeEntities[ent.EntityID]; !ok { - a.activeEntities[ent.EntityID] = struct{}{} - a.entityCountByNamespaceID[ent.NamespaceID] = a.entityCountByNamespaceID[ent.NamespaceID] + 1 - } - + a.AddEntity(ent) } return nil @@ -1337,7 +1328,7 @@ func (a *ActivityLog) AddEntityToFragment(entityID string, namespaceID string, t Timestamp: timestamp, }) //incrementing the entity by namespace id count map - a.entityCountByNamespaceID[namespaceID] = a.entityCountByNamespaceID[namespaceID] + 1 + a.entityCountByNamespaceID[namespaceID] += 1 a.activeEntities[entityID] = struct{}{} } @@ -1382,12 +1373,7 @@ func (a *ActivityLog) receivedFragment(fragment *activity.LogFragment) { } for _, e := range fragment.Entities { - //check if the entity already exists in the list of activeEntities this month - //if entity doesnot exist in active Entities, increment the namespaceID count - if _, ok := a.activeEntities[e.EntityID]; !ok { - a.activeEntities[e.EntityID] = struct{}{} - a.entityCountByNamespaceID[e.NamespaceID] = a.entityCountByNamespaceID[e.NamespaceID] + 1 - } + a.AddEntity(e) } a.standbyFragmentsReceived = append(a.standbyFragmentsReceived, fragment) @@ -1826,27 +1812,96 @@ func (c *Core) activeEntityGaugeCollector(ctx context.Context) ([]metricsutil.Ga // partialMonthClientCount returns the number of clients used so far this month. // If activity log is not enabled, the response will be nil -func (a *ActivityLog) partialMonthClientCount(ctx context.Context) map[string]interface{} { - q.Q("open source function called") +func (a *ActivityLog) partialMonthClientCount(ctx context.Context) (map[string]interface{}, error) { a.fragmentLock.RLock() defer a.fragmentLock.RUnlock() if !a.enabled { // nothing to count - return nil + return nil, nil } + byNamespace := make([]*ClientCountInNamespace, 0) + responseData := make(map[string]interface{}) + totalEntities := 0 + totalTokens := 0 + + clientCountTable := createClientCountTable(a.entityCountByNamespaceID, a.currentSegment.tokenCount.CountByNamespaceID) - entityCount := len(a.activeEntities) - var tokenCount int - for _, countByNS := range a.currentSegment.tokenCount.CountByNamespaceID { - tokenCount += int(countByNS) + queryNS, err := namespace.FromContext(ctx) + if err != nil { + return responseData, err } - clientCount := entityCount + tokenCount - responseData := make(map[string]interface{}) - responseData["distinct_entities"] = entityCount - responseData["non_entity_tokens"] = tokenCount - responseData["clients"] = clientCount + for nsID, clients := range clientCountTable { + + ns, err := NamespaceByID(ctx, nsID, a.core) + if err != nil { + return responseData, err + } + + if a.includeInResponse(queryNS, ns) { + var displayPath string + if ns == nil { + displayPath = fmt.Sprintf("deleted namespace %q", nsID) + } else { + displayPath = ns.Path + } + + byNamespace = append(byNamespace, &ClientCountInNamespace{ + NamespaceID: nsID, + NamespacePath: displayPath, + Counts: ClientCountResponse{ + DistinctEntities: int(clients.distinctEntities), + NonEntityTokens: int(clients.nonEntityTokens), + Clients: int(clients.distinctEntities + clients.nonEntityTokens), + }, + }) + totalEntities += int(clients.distinctEntities) + totalTokens += int(clients.nonEntityTokens) + + } + } + + responseData["by_namespace"] = byNamespace + responseData["distinct_entities"] = totalEntities + responseData["non_entity_tokens"] = totalTokens + responseData["clients"] = totalEntities + totalTokens + + q.Q(responseData) + + return responseData, nil +} + +//createClientCountTable maps the entitycount and token count to the namespace id +func createClientCountTable(entityMap map[string]uint64, tokenMap map[string]uint64) map[string]*clients { + //add distinct entity count + clientCountTable := make(map[string]*clients) + for nsID, count := range entityMap { + if _, ok := clientCountTable[nsID]; !ok { + //client := &clients{distinctEntities: 0, nonEntityTokens: 0} + clientCountTable[nsID] = &clients{distinctEntities: 0, nonEntityTokens: 0} + } + clientCountTable[nsID].distinctEntities += count + + } + //add non-entity token count + for nsID, count := range tokenMap { + if _, ok := clientCountTable[nsID]; !ok { + //client := &clients{distinctEntities: 0, nonEntityTokens: 0} + clientCountTable[nsID] = &clients{distinctEntities: 0, nonEntityTokens: 0} + } + clientCountTable[nsID].nonEntityTokens += count + + } + return clientCountTable + +} + +//AddEntity updates the activeEntities list as well as the activityentities by namespace map +func (a *ActivityLog) AddEntity(e *activity.EntityRecord) { + if _, ok := a.activeEntities[e.EntityID]; !ok { + a.activeEntities[e.EntityID] = struct{}{} + a.entityCountByNamespaceID[e.NamespaceID] += 1 + } - return responseData } diff --git a/vault/activity_log_test.go b/vault/activity_log_test.go index 86f4577ad8fe1..f1ae29f140041 100644 --- a/vault/activity_log_test.go +++ b/vault/activity_log_test.go @@ -1390,6 +1390,79 @@ func setupActivityRecordsInStorage(t *testing.T, base time.Time, includeEntities return a, entityRecords, tokenRecords } +func setupActivityRecordsInStorageRootNS(t *testing.T, base time.Time, includeEntities, includeTokens bool) (*ActivityLog, []*activity.EntityRecord, map[string]uint64) { + t.Helper() + + core, _, _ := TestCoreUnsealed(t) + a := core.activityLog + monthsAgo := base.AddDate(0, -3, 0) + + var entityRecords []*activity.EntityRecord + if includeEntities { + entityRecords = []*activity.EntityRecord{ + { + EntityID: "11111111-1111-1111-1111-111111111111", + NamespaceID: "root", + Timestamp: time.Now().Unix(), + }, + { + EntityID: "22222222-2222-2222-2222-222222222222", + NamespaceID: "root", + Timestamp: time.Now().Unix(), + }, + { + EntityID: "33333333-2222-2222-2222-222222222222", + NamespaceID: "root", + Timestamp: time.Now().Unix(), + }, + } + + testEntities1 := &activity.EntityActivityLog{ + Entities: entityRecords[:1], + } + entityData1, err := proto.Marshal(testEntities1) + if err != nil { + t.Fatalf(err.Error()) + } + testEntities2 := &activity.EntityActivityLog{ + Entities: entityRecords[1:2], + } + entityData2, err := proto.Marshal(testEntities2) + if err != nil { + t.Fatalf(err.Error()) + } + testEntities3 := &activity.EntityActivityLog{ + Entities: entityRecords[2:], + } + entityData3, err := proto.Marshal(testEntities3) + if err != nil { + t.Fatalf(err.Error()) + } + + WriteToStorage(t, core, ActivityLogPrefix+"entity/"+fmt.Sprint(monthsAgo.Unix())+"/0", entityData1) + WriteToStorage(t, core, ActivityLogPrefix+"entity/"+fmt.Sprint(base.Unix())+"/0", entityData2) + WriteToStorage(t, core, ActivityLogPrefix+"entity/"+fmt.Sprint(base.Unix())+"/1", entityData3) + } + + var tokenRecords map[string]uint64 + if includeTokens { + tokenRecords = make(map[string]uint64) + tokenRecords["root"] = 4 + tokenCount := &activity.TokenCount{ + CountByNamespaceID: tokenRecords, + } + + tokenData, err := proto.Marshal(tokenCount) + if err != nil { + t.Fatalf(err.Error()) + } + + WriteToStorage(t, core, ActivityLogPrefix+"directtokens/"+fmt.Sprint(base.Unix())+"/0", tokenData) + } + + return a, entityRecords, tokenRecords +} + func TestActivityLog_refreshFromStoredLog(t *testing.T) { a, expectedEntityRecords, expectedTokenCounts := setupActivityRecordsInStorage(t, time.Now().UTC(), true, true) a.SetEnable(true) @@ -2499,9 +2572,9 @@ func TestActivityLog_Deletion(t *testing.T) { func TestActivityLog_partialMonthClientCount(t *testing.T) { timeutil.SkipAtEndOfMonth(t) - ctx := context.Background() + ctx := namespace.RootContext(nil) now := time.Now().UTC() - a, entities, tokenCounts := setupActivityRecordsInStorage(t, timeutil.StartOfMonth(now), true, true) + a, entities, tokenCounts := setupActivityRecordsInStorageRootNS(t, timeutil.StartOfMonth(now), true, true) a.SetEnable(true) var wg sync.WaitGroup @@ -2520,7 +2593,10 @@ func TestActivityLog_partialMonthClientCount(t *testing.T) { expectedClientCount := partialMonthEntityCount + partialMonthTokenCount - results := a.partialMonthClientCount(ctx) + results, err := a.partialMonthClientCount(ctx) + if err != nil { + t.Fatal(err.Error()) + } if results == nil { t.Fatal("no results to test") } diff --git a/vault/logical_system_activity.go b/vault/logical_system_activity.go index aeb4216536325..9164be74ff81e 100644 --- a/vault/logical_system_activity.go +++ b/vault/logical_system_activity.go @@ -142,7 +142,12 @@ func (b *SystemBackend) handleMonthlyActivityCount(ctx context.Context, req *log return logical.ErrorResponse("no activity log present"), nil } - results := PartialMonthClientCount(a, ctx) + results, err := a.partialMonthClientCount(ctx) + if err != nil { + return &logical.Response{ + Data: results, + }, err + } if results == nil { return logical.RespondWithStatusCode(nil, req, http.StatusNoContent) } From b2ccd9b6ae448471e259248b7f7469076f7fa766 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Fri, 20 Aug 2021 16:05:50 -0700 Subject: [PATCH 04/11] Vault-2823 removing comments --- vault/activity_log.go | 7 ------- vault/activity_log_test.go | 1 + 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/vault/activity_log.go b/vault/activity_log.go index c8da310079013..0823f6774dac3 100644 --- a/vault/activity_log.go +++ b/vault/activity_log.go @@ -23,10 +23,6 @@ import ( "github.com/hashicorp/vault/vault/activity" ) -//adding an alias for partialMonthClientCount -//open source vault calls partialMonthClientCount -//enterprise calls entPartialMonthClientCount - const ( // activitySubPath is the directory under the system view where // the log will be stored. @@ -1878,7 +1874,6 @@ func createClientCountTable(entityMap map[string]uint64, tokenMap map[string]uin clientCountTable := make(map[string]*clients) for nsID, count := range entityMap { if _, ok := clientCountTable[nsID]; !ok { - //client := &clients{distinctEntities: 0, nonEntityTokens: 0} clientCountTable[nsID] = &clients{distinctEntities: 0, nonEntityTokens: 0} } clientCountTable[nsID].distinctEntities += count @@ -1887,7 +1882,6 @@ func createClientCountTable(entityMap map[string]uint64, tokenMap map[string]uin //add non-entity token count for nsID, count := range tokenMap { if _, ok := clientCountTable[nsID]; !ok { - //client := &clients{distinctEntities: 0, nonEntityTokens: 0} clientCountTable[nsID] = &clients{distinctEntities: 0, nonEntityTokens: 0} } clientCountTable[nsID].nonEntityTokens += count @@ -1897,7 +1891,6 @@ func createClientCountTable(entityMap map[string]uint64, tokenMap map[string]uin } -//AddEntity updates the activeEntities list as well as the activityentities by namespace map func (a *ActivityLog) AddEntity(e *activity.EntityRecord) { if _, ok := a.activeEntities[e.EntityID]; !ok { a.activeEntities[e.EntityID] = struct{}{} diff --git a/vault/activity_log_test.go b/vault/activity_log_test.go index f1ae29f140041..7a4b48887e480 100644 --- a/vault/activity_log_test.go +++ b/vault/activity_log_test.go @@ -1390,6 +1390,7 @@ func setupActivityRecordsInStorage(t *testing.T, base time.Time, includeEntities return a, entityRecords, tokenRecords } +//setupActivityRecordsInStorageRootNS creates entity and non-entity tokens only with root namespace func setupActivityRecordsInStorageRootNS(t *testing.T, base time.Time, includeEntities, includeTokens bool) (*ActivityLog, []*activity.EntityRecord, map[string]uint64) { t.Helper() From 06736f5c2e4bfc8697cd66e96c4cc51f013574e1 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Fri, 20 Aug 2021 16:30:26 -0700 Subject: [PATCH 05/11] Vault-2823 removing comments --- vault/activity_log.go | 1 - 1 file changed, 1 deletion(-) diff --git a/vault/activity_log.go b/vault/activity_log.go index 0823f6774dac3..75d19e220ba63 100644 --- a/vault/activity_log.go +++ b/vault/activity_log.go @@ -1323,7 +1323,6 @@ func (a *ActivityLog) AddEntityToFragment(entityID string, namespaceID string, t NamespaceID: namespaceID, Timestamp: timestamp, }) - //incrementing the entity by namespace id count map a.entityCountByNamespaceID[namespaceID] += 1 a.activeEntities[entityID] = struct{}{} } From 2ed883ce2d8a6f09c086453d09e3a5867cc8e7b5 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Fri, 20 Aug 2021 16:32:16 -0700 Subject: [PATCH 06/11] vault-2823 removing q debug --- vault/activity_log.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/vault/activity_log.go b/vault/activity_log.go index 75d19e220ba63..fe2e266d74697 100644 --- a/vault/activity_log.go +++ b/vault/activity_log.go @@ -12,8 +12,6 @@ import ( "sync" "time" - "q" - "github.com/golang/protobuf/proto" log "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/helper/metricsutil" @@ -1862,8 +1860,6 @@ func (a *ActivityLog) partialMonthClientCount(ctx context.Context) (map[string]i responseData["non_entity_tokens"] = totalTokens responseData["clients"] = totalEntities + totalTokens - q.Q(responseData) - return responseData, nil } From 23b3e3f803b847db23c9f63828cfc9158c53f848 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Fri, 20 Aug 2021 16:45:37 -0700 Subject: [PATCH 07/11] adding changelog --- changelog/12393.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/12393.txt diff --git a/changelog/12393.txt b/changelog/12393.txt new file mode 100644 index 0000000000000..7997d51e1b196 --- /dev/null +++ b/changelog/12393.txt @@ -0,0 +1,3 @@ +```release-note: improvement +core: observe the client counts broken down by namespace for partial month client count +``` \ No newline at end of file From f226f53e7cab95cea90d849f9c3f636b4edbdcee Mon Sep 17 00:00:00 2001 From: akshya96 Date: Mon, 23 Aug 2021 15:14:46 -0700 Subject: [PATCH 08/11] Vault-2823 updating external test --- vault/activity_log_testing_util.go | 23 +++++++++++++++++++ .../external_tests/activity/activity_test.go | 2 +- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/vault/activity_log_testing_util.go b/vault/activity_log_testing_util.go index ba9fe6542bfea..f4bbdf9175ef9 100644 --- a/vault/activity_log_testing_util.go +++ b/vault/activity_log_testing_util.go @@ -36,6 +36,29 @@ func (c *Core) InjectActivityLogDataThisMonth(t *testing.T) (map[string]struct{} return activeEntities, tokens } +// InjectActivityLogDataThisMonth populates the in-memory client store +// with some entities and tokens for root namespace , overriding what was already there +// It is currently used for API integration tests +func (c *Core) InjectActivityLogDataThisMonthRootNS(t *testing.T) (map[string]uint64, map[string]uint64) { + t.Helper() + + entitiesByNS := map[string]uint64{ + "root": 1, + } + tokens := map[string]uint64{ + "root": 5, + } + + c.activityLog.l.Lock() + defer c.activityLog.l.Unlock() + c.activityLog.fragmentLock.Lock() + defer c.activityLog.fragmentLock.Unlock() + + c.activityLog.currentSegment.tokenCount.CountByNamespaceID = tokens + c.activityLog.entityCountByNamespaceID = entitiesByNS + return entitiesByNS, tokens +} + // Return the in-memory activeEntities from an activity log func (c *Core) GetActiveEntities() map[string]struct{} { out := make(map[string]struct{}) diff --git a/vault/external_tests/activity/activity_test.go b/vault/external_tests/activity/activity_test.go index 6a0233280f107..2d6e358c63ec6 100644 --- a/vault/external_tests/activity/activity_test.go +++ b/vault/external_tests/activity/activity_test.go @@ -87,7 +87,7 @@ func TestActivityLog_MonthlyActivityApi(t *testing.T) { validateClientCounts(t, resp, 0, 0) // inject some data and query the API - entities, tokens := core.InjectActivityLogDataThisMonth(t) + entities, tokens := core.InjectActivityLogDataThisMonthRootNS(t) expectedEntities := len(entities) var expectedTokens int for _, tokenCount := range tokens { From 9bc2ee538c4fb18955e0a68e7fb13b02f06c6577 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Thu, 2 Sep 2021 21:55:51 -0700 Subject: [PATCH 09/11] adding approved changes --- vault/activity_log.go | 91 +++++---- vault/activity_log_test.go | 186 +++++++----------- vault/activity_log_testing_util.go | 57 ++---- .../external_tests/activity/activity_test.go | 7 +- vault/logical_system_activity.go | 4 +- 5 files changed, 148 insertions(+), 197 deletions(-) diff --git a/vault/activity_log.go b/vault/activity_log.go index fe2e266d74697..340008c26b24a 100644 --- a/vault/activity_log.go +++ b/vault/activity_log.go @@ -124,10 +124,6 @@ type ActivityLog struct { // Channel to stop background processing doneCh chan struct{} - // All known active entities this month; use fragmentLock read-locked - // to check whether it already exists. - activeEntities map[string]struct{} - // track metadata and contents of the most recent log segment currentSegment segmentInfo @@ -149,8 +145,14 @@ type ActivityLog struct { // for testing: is config currently being invalidated. protected by l configInvalidationInProgress bool - // All known active entity count by namespace ID this month; use fragmentLock read-locked + // entityTracker tracks active entities this month. Protected by fragmentLock. + entityTracker *EntityTracker +} + +type EntityTracker struct { + // All known active entities this month; use fragmentLock read-locked // to check whether it already exists. + activeEntities map[string]struct{} entityCountByNamespaceID map[string]uint64 } @@ -173,18 +175,20 @@ func NewActivityLog(core *Core, logger log.Logger, view *BarrierView, metrics me } a := &ActivityLog{ - core: core, - configOverrides: &core.activityLogConfig, - logger: logger, - view: view, - metrics: metrics, - nodeID: hostname, - newFragmentCh: make(chan struct{}, 1), - sendCh: make(chan struct{}, 1), // buffered so it can be triggered by fragment size - writeCh: make(chan struct{}, 1), // same for full segment - doneCh: make(chan struct{}, 1), - activeEntities: make(map[string]struct{}), - entityCountByNamespaceID: make(map[string]uint64), + core: core, + configOverrides: &core.activityLogConfig, + logger: logger, + view: view, + metrics: metrics, + nodeID: hostname, + newFragmentCh: make(chan struct{}, 1), + sendCh: make(chan struct{}, 1), // buffered so it can be triggered by fragment size + writeCh: make(chan struct{}, 1), // same for full segment + doneCh: make(chan struct{}, 1), + entityTracker: &EntityTracker{ + activeEntities: make(map[string]struct{}), + entityCountByNamespaceID: make(map[string]uint64), + }, currentSegment: segmentInfo{ startTimestamp: 0, currentEntities: &activity.EntityActivityLog{ @@ -541,7 +545,7 @@ func (a *ActivityLog) loadPriorEntitySegment(ctx context.Context, startTime time // Or the feature has been disabled. if a.enabled && startTime.Unix() == a.currentSegment.startTimestamp { for _, ent := range out.Entities { - a.AddEntity(ent) + a.entityTracker.addEntity(ent) } } a.fragmentLock.Unlock() @@ -581,7 +585,7 @@ func (a *ActivityLog) loadCurrentEntitySegment(ctx context.Context, startTime ti } for _, ent := range out.Entities { - a.AddEntity(ent) + a.entityTracker.addEntity(ent) } return nil @@ -693,8 +697,8 @@ func (a *ActivityLog) resetCurrentLog() { a.currentSegment.entitySequenceNumber = 0 a.fragment = nil - a.activeEntities = make(map[string]struct{}) - a.entityCountByNamespaceID = make(map[string]uint64) + a.entityTracker.activeEntities = make(map[string]struct{}) + a.entityTracker.entityCountByNamespaceID = make(map[string]uint64) a.standbyFragmentsReceived = make([]*activity.LogFragment, 0) } @@ -1105,8 +1109,8 @@ func (a *ActivityLog) perfStandbyFragmentWorker() { // clear active entity set a.fragmentLock.Lock() - a.activeEntities = make(map[string]struct{}) - a.entityCountByNamespaceID = make(map[string]uint64) + a.entityTracker.activeEntities = make(map[string]struct{}) + a.entityTracker.entityCountByNamespaceID = make(map[string]uint64) a.fragmentLock.Unlock() // Set timer for next month. @@ -1294,7 +1298,7 @@ func (a *ActivityLog) AddEntityToFragment(entityID string, namespaceID string, t a.fragmentLock.RLock() if a.enabled { - _, present = a.activeEntities[entityID] + _, present = a.entityTracker.activeEntities[entityID] } else { present = true } @@ -1308,21 +1312,20 @@ func (a *ActivityLog) AddEntityToFragment(entityID string, namespaceID string, t defer a.fragmentLock.Unlock() // Re-check entity ID after re-acquiring lock - _, present = a.activeEntities[entityID] + _, present = a.entityTracker.activeEntities[entityID] if present { return } a.createCurrentFragment() - a.fragment.Entities = append(a.fragment.Entities, - &activity.EntityRecord{ - EntityID: entityID, - NamespaceID: namespaceID, - Timestamp: timestamp, - }) - a.entityCountByNamespaceID[namespaceID] += 1 - a.activeEntities[entityID] = struct{}{} + entityRecord := &activity.EntityRecord{ + EntityID: entityID, + NamespaceID: namespaceID, + Timestamp: timestamp, + } + a.fragment.Entities = append(a.fragment.Entities, entityRecord) + a.entityTracker.addEntity(entityRecord) } func (a *ActivityLog) AddTokenToFragment(namespaceID string) { @@ -1366,7 +1369,7 @@ func (a *ActivityLog) receivedFragment(fragment *activity.LogFragment) { } for _, e := range fragment.Entities { - a.AddEntity(e) + a.entityTracker.addEntity(e) } a.standbyFragmentsReceived = append(a.standbyFragmentsReceived, fragment) @@ -1783,7 +1786,7 @@ func (a *ActivityLog) PartialMonthMetrics(ctx context.Context) ([]metricsutil.Ga // Empty list return []metricsutil.GaugeLabelValues{}, nil } - count := len(a.activeEntities) + count := len(a.entityTracker.activeEntities) return []metricsutil.GaugeLabelValues{ { @@ -1818,7 +1821,7 @@ func (a *ActivityLog) partialMonthClientCount(ctx context.Context) (map[string]i totalEntities := 0 totalTokens := 0 - clientCountTable := createClientCountTable(a.entityCountByNamespaceID, a.currentSegment.tokenCount.CountByNamespaceID) + clientCountTable := createClientCountTable(a.entityTracker.entityCountByNamespaceID, a.currentSegment.tokenCount.CountByNamespaceID) queryNS, err := namespace.FromContext(ctx) if err != nil { @@ -1832,6 +1835,8 @@ func (a *ActivityLog) partialMonthClientCount(ctx context.Context) (map[string]i return responseData, err } + // Only include namespaces that are the queryNS or within it. If queryNS is the + // root namespace, include all namespaces, even those which have been deleted. if a.includeInResponse(queryNS, ns) { var displayPath string if ns == nil { @@ -1849,12 +1854,17 @@ func (a *ActivityLog) partialMonthClientCount(ctx context.Context) (map[string]i Clients: int(clients.distinctEntities + clients.nonEntityTokens), }, }) + totalEntities += int(clients.distinctEntities) totalTokens += int(clients.nonEntityTokens) } } + sort.Slice(byNamespace, func(i, j int) bool { + return byNamespace[i].NamespaceID < byNamespace[j].NamespaceID + }) + responseData["by_namespace"] = byNamespace responseData["distinct_entities"] = totalEntities responseData["non_entity_tokens"] = totalTokens @@ -1886,10 +1896,9 @@ func createClientCountTable(entityMap map[string]uint64, tokenMap map[string]uin } -func (a *ActivityLog) AddEntity(e *activity.EntityRecord) { - if _, ok := a.activeEntities[e.EntityID]; !ok { - a.activeEntities[e.EntityID] = struct{}{} - a.entityCountByNamespaceID[e.NamespaceID] += 1 +func (et *EntityTracker) addEntity(e *activity.EntityRecord) { + if _, ok := et.activeEntities[e.EntityID]; !ok { + et.activeEntities[e.EntityID] = struct{}{} + et.entityCountByNamespaceID[e.NamespaceID] += 1 } - } diff --git a/vault/activity_log_test.go b/vault/activity_log_test.go index 7a4b48887e480..a6c2387bb69cc 100644 --- a/vault/activity_log_test.go +++ b/vault/activity_log_test.go @@ -15,10 +15,12 @@ import ( "github.com/go-test/deep" "github.com/golang/protobuf/proto" + "github.com/hashicorp/vault/helper/constants" "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/helper/timeutil" "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/vault/activity" + "github.com/mitchellh/mapstructure" ) func TestActivityLog_Creation(t *testing.T) { @@ -1005,7 +1007,7 @@ func (a *ActivityLog) resetEntitiesInMemory(t *testing.T) { entitySequenceNumber: 0, } - a.activeEntities = make(map[string]struct{}) + a.entityTracker.activeEntities = make(map[string]struct{}) } func TestActivityLog_loadCurrentEntitySegment(t *testing.T) { @@ -1188,7 +1190,7 @@ func TestActivityLog_loadPriorEntitySegment(t *testing.T) { if tc.refresh { a.l.Lock() a.fragmentLock.Lock() - a.activeEntities = make(map[string]struct{}) + a.entityTracker.activeEntities = make(map[string]struct{}) a.currentSegment.startTimestamp = tc.time a.fragmentLock.Unlock() a.l.Unlock() @@ -1326,129 +1328,55 @@ func setupActivityRecordsInStorage(t *testing.T, base time.Time, includeEntities entityRecords = []*activity.EntityRecord{ { EntityID: "11111111-1111-1111-1111-111111111111", - NamespaceID: "root", + NamespaceID: namespace.RootNamespaceID, Timestamp: time.Now().Unix(), }, { EntityID: "22222222-2222-2222-2222-222222222222", - NamespaceID: "root", + NamespaceID: namespace.RootNamespaceID, Timestamp: time.Now().Unix(), }, { EntityID: "33333333-2222-2222-2222-222222222222", - NamespaceID: "root", + NamespaceID: namespace.RootNamespaceID, Timestamp: time.Now().Unix(), }, } + if constants.IsEnterprise { + entityRecords = append(entityRecords, []*activity.EntityRecord{ + { + EntityID: "44444444-1111-1111-1111-111111111111", + NamespaceID: "ns1", + Timestamp: time.Now().Unix(), + }, + }...) + } + for i, entityRecord := range entityRecords { + entityData, err := proto.Marshal(&activity.EntityActivityLog{ + Entities: []*activity.EntityRecord{entityRecord}, + }) + if err != nil { + t.Fatalf(err.Error()) + } + if i == 0 { + WriteToStorage(t, core, ActivityLogPrefix+"entity/"+fmt.Sprint(monthsAgo.Unix())+"/0", entityData) + } else { + WriteToStorage(t, core, ActivityLogPrefix+"entity/"+fmt.Sprint(base.Unix())+"/"+strconv.Itoa(i-1), entityData) + } - testEntities1 := &activity.EntityActivityLog{ - Entities: entityRecords[:1], - } - entityData1, err := proto.Marshal(testEntities1) - if err != nil { - t.Fatalf(err.Error()) - } - testEntities2 := &activity.EntityActivityLog{ - Entities: entityRecords[1:2], } - entityData2, err := proto.Marshal(testEntities2) - if err != nil { - t.Fatalf(err.Error()) - } - testEntities3 := &activity.EntityActivityLog{ - Entities: entityRecords[2:], - } - entityData3, err := proto.Marshal(testEntities3) - if err != nil { - t.Fatalf(err.Error()) - } - - WriteToStorage(t, core, ActivityLogPrefix+"entity/"+fmt.Sprint(monthsAgo.Unix())+"/0", entityData1) - WriteToStorage(t, core, ActivityLogPrefix+"entity/"+fmt.Sprint(base.Unix())+"/0", entityData2) - WriteToStorage(t, core, ActivityLogPrefix+"entity/"+fmt.Sprint(base.Unix())+"/1", entityData3) } var tokenRecords map[string]uint64 if includeTokens { tokenRecords = make(map[string]uint64) - for i := 1; i < 4; i++ { - nsID := "ns" + strconv.Itoa(i) - tokenRecords[nsID] = uint64(i) - } - tokenCount := &activity.TokenCount{ - CountByNamespaceID: tokenRecords, - } - - tokenData, err := proto.Marshal(tokenCount) - if err != nil { - t.Fatalf(err.Error()) - } - - WriteToStorage(t, core, ActivityLogPrefix+"directtokens/"+fmt.Sprint(base.Unix())+"/0", tokenData) - } - - return a, entityRecords, tokenRecords -} - -//setupActivityRecordsInStorageRootNS creates entity and non-entity tokens only with root namespace -func setupActivityRecordsInStorageRootNS(t *testing.T, base time.Time, includeEntities, includeTokens bool) (*ActivityLog, []*activity.EntityRecord, map[string]uint64) { - t.Helper() - - core, _, _ := TestCoreUnsealed(t) - a := core.activityLog - monthsAgo := base.AddDate(0, -3, 0) - - var entityRecords []*activity.EntityRecord - if includeEntities { - entityRecords = []*activity.EntityRecord{ - { - EntityID: "11111111-1111-1111-1111-111111111111", - NamespaceID: "root", - Timestamp: time.Now().Unix(), - }, - { - EntityID: "22222222-2222-2222-2222-222222222222", - NamespaceID: "root", - Timestamp: time.Now().Unix(), - }, - { - EntityID: "33333333-2222-2222-2222-222222222222", - NamespaceID: "root", - Timestamp: time.Now().Unix(), - }, - } - - testEntities1 := &activity.EntityActivityLog{ - Entities: entityRecords[:1], - } - entityData1, err := proto.Marshal(testEntities1) - if err != nil { - t.Fatalf(err.Error()) - } - testEntities2 := &activity.EntityActivityLog{ - Entities: entityRecords[1:2], - } - entityData2, err := proto.Marshal(testEntities2) - if err != nil { - t.Fatalf(err.Error()) - } - testEntities3 := &activity.EntityActivityLog{ - Entities: entityRecords[2:], - } - entityData3, err := proto.Marshal(testEntities3) - if err != nil { - t.Fatalf(err.Error()) + tokenRecords[namespace.RootNamespaceID] = uint64(1) + if constants.IsEnterprise { + for i := 1; i < 4; i++ { + nsID := "ns" + strconv.Itoa(i) + tokenRecords[nsID] = uint64(i) + } } - - WriteToStorage(t, core, ActivityLogPrefix+"entity/"+fmt.Sprint(monthsAgo.Unix())+"/0", entityData1) - WriteToStorage(t, core, ActivityLogPrefix+"entity/"+fmt.Sprint(base.Unix())+"/0", entityData2) - WriteToStorage(t, core, ActivityLogPrefix+"entity/"+fmt.Sprint(base.Unix())+"/1", entityData3) - } - - var tokenRecords map[string]uint64 - if includeTokens { - tokenRecords = make(map[string]uint64) - tokenRecords["root"] = 4 tokenCount := &activity.TokenCount{ CountByNamespaceID: tokenRecords, } @@ -1479,7 +1407,7 @@ func TestActivityLog_refreshFromStoredLog(t *testing.T) { Entities: expectedEntityRecords[1:], } expectedCurrent := &activity.EntityActivityLog{ - Entities: expectedEntityRecords[2:], + Entities: expectedEntityRecords[len(expectedEntityRecords)-1:], } currentEntities := a.GetCurrentEntities() @@ -1520,7 +1448,7 @@ func TestActivityLog_refreshFromStoredLogWithBackgroundLoadingCancelled(t *testi wg.Wait() expected := &activity.EntityActivityLog{ - Entities: expectedEntityRecords[2:], + Entities: expectedEntityRecords[len(expectedEntityRecords)-1:], } currentEntities := a.GetCurrentEntities() @@ -1570,7 +1498,7 @@ func TestActivityLog_refreshFromStoredLogNoTokens(t *testing.T) { Entities: expectedEntityRecords[1:], } expectedCurrent := &activity.EntityActivityLog{ - Entities: expectedEntityRecords[2:], + Entities: expectedEntityRecords[len(expectedEntityRecords)-1:], } currentEntities := a.GetCurrentEntities() @@ -1669,7 +1597,7 @@ func TestActivityLog_refreshFromStoredLogPreviousMonth(t *testing.T) { Entities: expectedEntityRecords[1:], } expectedCurrent := &activity.EntityActivityLog{ - Entities: expectedEntityRecords[2:], + Entities: expectedEntityRecords[len(expectedEntityRecords)-1:], } currentEntities := a.GetCurrentEntities() @@ -2575,7 +2503,13 @@ func TestActivityLog_partialMonthClientCount(t *testing.T) { ctx := namespace.RootContext(nil) now := time.Now().UTC() - a, entities, tokenCounts := setupActivityRecordsInStorageRootNS(t, timeutil.StartOfMonth(now), true, true) + a, entities, tokenCounts := setupActivityRecordsInStorage(t, timeutil.StartOfMonth(now), true, true) + entities = entities[1:] + entityCounts := make(map[string]uint64) + + for _, entity := range entities { + entityCounts[entity.NamespaceID] += 1 + } a.SetEnable(true) var wg sync.WaitGroup @@ -2586,7 +2520,7 @@ func TestActivityLog_partialMonthClientCount(t *testing.T) { wg.Wait() // entities[0] is from a previous month - partialMonthEntityCount := len(entities[1:]) + partialMonthEntityCount := len(entities) var partialMonthTokenCount int for _, countByNS := range tokenCounts { partialMonthTokenCount += int(countByNS) @@ -2602,6 +2536,34 @@ func TestActivityLog_partialMonthClientCount(t *testing.T) { t.Fatal("no results to test") } + byNamespace, ok := results["by_namespace"] + if !ok { + t.Fatalf("malformed results. got %v", results) + } + + clientCountResponse := make([]*ClientCountInNamespace, 0) + err = mapstructure.Decode(byNamespace, &clientCountResponse) + if err != nil { + t.Fatal(err.Error()) + } + + for _, clientCount := range clientCountResponse { + + if int(entityCounts[clientCount.NamespaceID]) != clientCount.Counts.DistinctEntities { + t.Errorf("bad entity count for namespace %s . expected %d, got %d", clientCount.NamespaceID, int(entityCounts[clientCount.NamespaceID]), clientCount.Counts.DistinctEntities) + + } + if int(tokenCounts[clientCount.NamespaceID]) != clientCount.Counts.NonEntityTokens { + t.Errorf("bad token count for namespace %s . expected %d, got %d", clientCount.NamespaceID, int(tokenCounts[clientCount.NamespaceID]), clientCount.Counts.NonEntityTokens) + } + + totalCount := int(entityCounts[clientCount.NamespaceID] + tokenCounts[clientCount.NamespaceID]) + if totalCount != clientCount.Counts.Clients { + t.Errorf("bad client count for namespace %s . expected %d, got %d", clientCount.NamespaceID, totalCount, clientCount.Counts.Clients) + } + + } + entityCount, ok := results["distinct_entities"] if !ok { t.Fatalf("malformed results. got %v", results) diff --git a/vault/activity_log_testing_util.go b/vault/activity_log_testing_util.go index f4bbdf9175ef9..6d30693190ed7 100644 --- a/vault/activity_log_testing_util.go +++ b/vault/activity_log_testing_util.go @@ -4,6 +4,7 @@ import ( "context" "testing" + "github.com/hashicorp/vault/helper/constants" "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/vault/activity" ) @@ -11,42 +12,20 @@ import ( // InjectActivityLogDataThisMonth populates the in-memory client store // with some entities and tokens, overriding what was already there // It is currently used for API integration tests -func (c *Core) InjectActivityLogDataThisMonth(t *testing.T) (map[string]struct{}, map[string]uint64) { +func (c *Core) InjectActivityLogDataThisMonth(t *testing.T) (map[string]uint64, map[string]uint64) { t.Helper() + tokens := make(map[string]uint64, 0) + entitiesByNS := make(map[string]uint64, 0) + tokens["root"] = 5 + entitiesByNS["root"] = 5 - activeEntities := map[string]struct{}{ - "entity0": {}, - "entity1": {}, - "entity2": {}, - } - tokens := map[string]uint64{ - "ns0": 5, - "ns1": 1, - "ns2": 10, - } - - c.activityLog.l.Lock() - defer c.activityLog.l.Unlock() - c.activityLog.fragmentLock.Lock() - defer c.activityLog.fragmentLock.Unlock() - - c.activityLog.activeEntities = activeEntities - c.activityLog.currentSegment.tokenCount.CountByNamespaceID = tokens - - return activeEntities, tokens -} - -// InjectActivityLogDataThisMonth populates the in-memory client store -// with some entities and tokens for root namespace , overriding what was already there -// It is currently used for API integration tests -func (c *Core) InjectActivityLogDataThisMonthRootNS(t *testing.T) (map[string]uint64, map[string]uint64) { - t.Helper() - - entitiesByNS := map[string]uint64{ - "root": 1, - } - tokens := map[string]uint64{ - "root": 5, + if constants.IsEnterprise { + tokens["ns0"] = 5 + tokens["ns1"] = 1 + tokens["ns2"] = 1 + entitiesByNS["ns0"] = 1 + entitiesByNS["ns1"] = 1 + entitiesByNS["ns2"] = 1 } c.activityLog.l.Lock() @@ -55,7 +34,7 @@ func (c *Core) InjectActivityLogDataThisMonthRootNS(t *testing.T) (map[string]ui defer c.activityLog.fragmentLock.Unlock() c.activityLog.currentSegment.tokenCount.CountByNamespaceID = tokens - c.activityLog.entityCountByNamespaceID = entitiesByNS + c.activityLog.entityTracker.entityCountByNamespaceID = entitiesByNS return entitiesByNS, tokens } @@ -65,7 +44,7 @@ func (c *Core) GetActiveEntities() map[string]struct{} { c.stateLock.RLock() c.activityLog.fragmentLock.RLock() - for k, v := range c.activityLog.activeEntities { + for k, v := range c.activityLog.entityTracker.activeEntities { out[k] = v } c.activityLog.fragmentLock.RUnlock() @@ -127,7 +106,7 @@ func (a *ActivityLog) ExpectCurrentSegmentRefreshed(t *testing.T, expectedStart if a.currentSegment.currentEntities.Entities == nil { t.Errorf("expected non-nil currentSegment.currentEntities.Entities") } - if a.activeEntities == nil { + if a.entityTracker.activeEntities == nil { t.Errorf("expected non-nil activeEntities") } if a.currentSegment.tokenCount == nil { @@ -140,8 +119,8 @@ func (a *ActivityLog) ExpectCurrentSegmentRefreshed(t *testing.T, expectedStart if len(a.currentSegment.currentEntities.Entities) > 0 { t.Errorf("expected no current entity segment to be loaded. got: %v", a.currentSegment.currentEntities) } - if len(a.activeEntities) > 0 { - t.Errorf("expected no active entity segment to be loaded. got: %v", a.activeEntities) + if len(a.entityTracker.activeEntities) > 0 { + t.Errorf("expected no active entity segment to be loaded. got: %v", a.entityTracker.activeEntities) } if len(a.currentSegment.tokenCount.CountByNamespaceID) > 0 { t.Errorf("expected no token counts to be loaded. got: %v", a.currentSegment.tokenCount.CountByNamespaceID) diff --git a/vault/external_tests/activity/activity_test.go b/vault/external_tests/activity/activity_test.go index 2d6e358c63ec6..f482faacc560b 100644 --- a/vault/external_tests/activity/activity_test.go +++ b/vault/external_tests/activity/activity_test.go @@ -87,8 +87,11 @@ func TestActivityLog_MonthlyActivityApi(t *testing.T) { validateClientCounts(t, resp, 0, 0) // inject some data and query the API - entities, tokens := core.InjectActivityLogDataThisMonthRootNS(t) - expectedEntities := len(entities) + entities, tokens := core.InjectActivityLogDataThisMonth(t) + var expectedEntities int + for _, entityCount := range entities { + expectedEntities += int(entityCount) + } var expectedTokens int for _, tokenCount := range tokens { expectedTokens += int(tokenCount) diff --git a/vault/logical_system_activity.go b/vault/logical_system_activity.go index 9164be74ff81e..550aa5400795d 100644 --- a/vault/logical_system_activity.go +++ b/vault/logical_system_activity.go @@ -144,9 +144,7 @@ func (b *SystemBackend) handleMonthlyActivityCount(ctx context.Context, req *log results, err := a.partialMonthClientCount(ctx) if err != nil { - return &logical.Response{ - Data: results, - }, err + return nil, err } if results == nil { return logical.RespondWithStatusCode(nil, req, http.StatusNoContent) From 22a1820deb31cddbac4b22b1cd8d25a97de55877 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Fri, 3 Sep 2021 10:09:38 -0700 Subject: [PATCH 10/11] fixing returns --- vault/activity_log.go | 7 ++----- vault/activity_log_test.go | 4 ++-- vault/logical_system_activity.go | 4 +++- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/vault/activity_log.go b/vault/activity_log.go index 340008c26b24a..a66194d9872e2 100644 --- a/vault/activity_log.go +++ b/vault/activity_log.go @@ -1825,11 +1825,10 @@ func (a *ActivityLog) partialMonthClientCount(ctx context.Context) (map[string]i queryNS, err := namespace.FromContext(ctx) if err != nil { - return responseData, err + return nil, err } for nsID, clients := range clientCountTable { - ns, err := NamespaceByID(ctx, nsID, a.core) if err != nil { return responseData, err @@ -1875,7 +1874,6 @@ func (a *ActivityLog) partialMonthClientCount(ctx context.Context) (map[string]i //createClientCountTable maps the entitycount and token count to the namespace id func createClientCountTable(entityMap map[string]uint64, tokenMap map[string]uint64) map[string]*clients { - //add distinct entity count clientCountTable := make(map[string]*clients) for nsID, count := range entityMap { if _, ok := clientCountTable[nsID]; !ok { @@ -1884,7 +1882,7 @@ func createClientCountTable(entityMap map[string]uint64, tokenMap map[string]uin clientCountTable[nsID].distinctEntities += count } - //add non-entity token count + for nsID, count := range tokenMap { if _, ok := clientCountTable[nsID]; !ok { clientCountTable[nsID] = &clients{distinctEntities: 0, nonEntityTokens: 0} @@ -1893,7 +1891,6 @@ func createClientCountTable(entityMap map[string]uint64, tokenMap map[string]uin } return clientCountTable - } func (et *EntityTracker) addEntity(e *activity.EntityRecord) { diff --git a/vault/activity_log_test.go b/vault/activity_log_test.go index a6c2387bb69cc..0ba97d29dba05 100644 --- a/vault/activity_log_test.go +++ b/vault/activity_log_test.go @@ -2530,7 +2530,7 @@ func TestActivityLog_partialMonthClientCount(t *testing.T) { results, err := a.partialMonthClientCount(ctx) if err != nil { - t.Fatal(err.Error()) + t.Fatal(err) } if results == nil { t.Fatal("no results to test") @@ -2544,7 +2544,7 @@ func TestActivityLog_partialMonthClientCount(t *testing.T) { clientCountResponse := make([]*ClientCountInNamespace, 0) err = mapstructure.Decode(byNamespace, &clientCountResponse) if err != nil { - t.Fatal(err.Error()) + t.Fatal(err) } for _, clientCount := range clientCountResponse { diff --git a/vault/logical_system_activity.go b/vault/logical_system_activity.go index 550aa5400795d..9164be74ff81e 100644 --- a/vault/logical_system_activity.go +++ b/vault/logical_system_activity.go @@ -144,7 +144,9 @@ func (b *SystemBackend) handleMonthlyActivityCount(ctx context.Context, req *log results, err := a.partialMonthClientCount(ctx) if err != nil { - return nil, err + return &logical.Response{ + Data: results, + }, err } if results == nil { return logical.RespondWithStatusCode(nil, req, http.StatusNoContent) From 589fb786aee713ba43fcb964afa33cb513a6634f Mon Sep 17 00:00:00 2001 From: akshya96 Date: Fri, 3 Sep 2021 12:54:19 -0700 Subject: [PATCH 11/11] fixing returns --- vault/activity_log.go | 2 +- vault/logical_system_activity.go | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/vault/activity_log.go b/vault/activity_log.go index a66194d9872e2..33f9c24becbd6 100644 --- a/vault/activity_log.go +++ b/vault/activity_log.go @@ -1831,7 +1831,7 @@ func (a *ActivityLog) partialMonthClientCount(ctx context.Context) (map[string]i for nsID, clients := range clientCountTable { ns, err := NamespaceByID(ctx, nsID, a.core) if err != nil { - return responseData, err + return nil, err } // Only include namespaces that are the queryNS or within it. If queryNS is the diff --git a/vault/logical_system_activity.go b/vault/logical_system_activity.go index 9164be74ff81e..550aa5400795d 100644 --- a/vault/logical_system_activity.go +++ b/vault/logical_system_activity.go @@ -144,9 +144,7 @@ func (b *SystemBackend) handleMonthlyActivityCount(ctx context.Context, req *log results, err := a.partialMonthClientCount(ctx) if err != nil { - return &logical.Response{ - Data: results, - }, err + return nil, err } if results == nil { return logical.RespondWithStatusCode(nil, req, http.StatusNoContent)