From 62da123ef58c3a6320df0bac7f1c74762b74a9ba Mon Sep 17 00:00:00 2001 From: Mia Epner Date: Mon, 8 Jan 2024 15:10:03 +0100 Subject: [PATCH 1/6] refactor and include secret sync --- vault/activity_log.go | 157 ++++++++++++------------------ vault/activity_log_util_common.go | 35 +++++-- 2 files changed, 93 insertions(+), 99 deletions(-) diff --git a/vault/activity_log.go b/vault/activity_log.go index 2f6c8acb48ef..00f4d95a2c61 100644 --- a/vault/activity_log.go +++ b/vault/activity_log.go @@ -1578,6 +1578,16 @@ type ResponseCounts struct { NonEntityTokens int `json:"non_entity_tokens" mapstructure:"non_entity_tokens"` NonEntityClients int `json:"non_entity_clients" mapstructure:"non_entity_clients"` Clients int `json:"clients"` + SecretSyncs int `json:"secret_syncs" mapstructure:"secret_syncs"` +} + +func (existingRecord *ResponseCounts) Add(newRecord *ResponseCounts) { + existingRecord.EntityClients += newRecord.EntityClients + existingRecord.Clients += newRecord.Clients + existingRecord.DistinctEntities += newRecord.DistinctEntities + existingRecord.NonEntityClients += newRecord.NonEntityClients + existingRecord.NonEntityTokens += newRecord.NonEntityTokens + existingRecord.SecretSyncs += newRecord.SecretSyncs } type ResponseNamespace struct { @@ -1587,6 +1597,27 @@ type ResponseNamespace struct { Mounts []*ResponseMount `json:"mounts"` } +func (existingRecord *ResponseNamespace) Add(newRecord *ResponseNamespace) { + // Create a map of the existing mounts, so we don't duplicate them + mountMap := make(map[string]*ResponseCounts) + for _, erm := range existingRecord.Mounts { + mountMap[erm.MountPath] = erm.Counts + } + + existingRecord.Counts.Add(&newRecord.Counts) + + // Check the current month mounts against the existing mounts and if there are matches, update counts + // accordingly. If there is no match, append the new mount to the existing mounts, so it will be counted + // later. + for _, newRecordMount := range newRecord.Mounts { + if existingRecordMountCounts, ook := mountMap[newRecordMount.MountPath]; ook { + existingRecordMountCounts.Add(&newRecord.Counts) + } else { + existingRecord.Mounts = append(existingRecord.Mounts, newRecordMount) + } + } +} + type ResponseMonth struct { Timestamp string `json:"timestamp"` Counts *ResponseCounts `json:"counts"` @@ -1681,7 +1712,7 @@ func (a *ActivityLog) handleQuery(ctx context.Context, startTime, endTime time.T // Calculate the namespace response breakdowns and totals for entities and tokens from the initial // namespace data. - totalEntities, totalTokens, byNamespaceResponse, err := a.calculateByNamespaceResponseForQuery(ctx, pq.Namespaces) + totalCounts, byNamespaceResponse, err := a.calculateByNamespaceResponseForQuery(ctx, pq.Namespaces) if err != nil { return nil, err } @@ -1690,9 +1721,8 @@ func (a *ActivityLog) handleQuery(ctx context.Context, startTime, endTime time.T // breakdown for the current month as well. var partialByMonth map[int64]*processMonth var partialByNamespace map[string]*processByNamespace - var totalEntitiesCurrent int - var totalTokensCurrent int var byNamespaceResponseCurrent []*ResponseNamespace + var totalCurrentCounts *ResponseCounts if computePartial { // Traverse through current month's activitylog data and group clients // into months and namespaces @@ -1705,9 +1735,9 @@ func (a *ActivityLog) handleQuery(ctx context.Context, startTime, endTime time.T // endpoints. byNamespaceComputation := a.transformALNamespaceBreakdowns(partialByNamespace) - // Calculate the namespace response breakdowns and totals for entities and tokens from the initial - // namespace data. - totalEntitiesCurrent, totalTokensCurrent, byNamespaceResponseCurrent, err = a.calculateByNamespaceResponseForQuery(ctx, byNamespaceComputation) + // Calculate the namespace response breakdowns and totals for entities + // and tokens from current month namespace data. + totalCurrentCounts, byNamespaceResponseCurrent, err = a.calculateByNamespaceResponseForQuery(ctx, byNamespaceComputation) if err != nil { return nil, err } @@ -1723,34 +1753,7 @@ func (a *ActivityLog) handleQuery(ctx context.Context, startTime, endTime time.T // month counts, and append or update as necessary. We also want to account for mounts and their counts. for _, nrc := range byNamespaceResponseCurrent { if ndx, ok := nsrMap[nrc.NamespaceID]; ok { - existingRecord := byNamespaceResponse[ndx] - - // Create a map of the existing mounts, so we don't duplicate them - mountMap := make(map[string]*ResponseCounts) - for _, erm := range existingRecord.Mounts { - mountMap[erm.MountPath] = erm.Counts - } - - existingRecord.Counts.EntityClients += nrc.Counts.EntityClients - existingRecord.Counts.Clients += nrc.Counts.Clients - existingRecord.Counts.DistinctEntities += nrc.Counts.DistinctEntities - existingRecord.Counts.NonEntityClients += nrc.Counts.NonEntityClients - existingRecord.Counts.NonEntityTokens += nrc.Counts.NonEntityTokens - - // Check the current month mounts against the existing mounts and if there are matches, update counts - // accordingly. If there is no match, append the new mount to the existing mounts, so it will be counted - // later. - for _, nrcMount := range nrc.Mounts { - if existingRecordMountCounts, ook := mountMap[nrcMount.MountPath]; ook { - existingRecordMountCounts.EntityClients += nrcMount.Counts.EntityClients - existingRecordMountCounts.Clients += nrcMount.Counts.Clients - existingRecordMountCounts.DistinctEntities += nrcMount.Counts.DistinctEntities - existingRecordMountCounts.NonEntityClients += nrcMount.Counts.NonEntityClients - existingRecordMountCounts.NonEntityTokens += nrcMount.Counts.NonEntityTokens - } else { - existingRecord.Mounts = append(existingRecord.Mounts, nrcMount) - } - } + byNamespaceResponse[ndx].Add(nrc) } else { byNamespaceResponse = append(byNamespaceResponse, nrc) } @@ -1761,10 +1764,10 @@ func (a *ActivityLog) handleQuery(ctx context.Context, startTime, endTime time.T a.sortALResponseNamespaces(byNamespaceResponse) if limitNamespaces > 0 { - totalEntities, totalTokens, byNamespaceResponse = a.limitNamespacesInALResponse(byNamespaceResponse, limitNamespaces) + totalCounts, byNamespaceResponse = a.limitNamespacesInALResponse(byNamespaceResponse, limitNamespaces) } - distinctEntitiesResponse := totalEntities + distinctEntitiesResponse := totalCounts.EntityClients if computePartial { currentMonth, err := a.computeCurrentMonthForBillingPeriod(ctx, partialByMonth, startTime, endTime) if err != nil { @@ -1808,13 +1811,9 @@ func (a *ActivityLog) handleQuery(ctx context.Context, startTime, endTime time.T } responseData["by_namespace"] = byNamespaceResponse - responseData["total"] = &ResponseCounts{ - DistinctEntities: distinctEntitiesResponse, - EntityClients: totalEntities + totalEntitiesCurrent, - NonEntityTokens: totalTokens + totalTokensCurrent, - NonEntityClients: totalTokens + totalTokensCurrent, - Clients: totalEntities + totalEntitiesCurrent + totalTokens + totalTokensCurrent, - } + totalCounts.Add(totalCurrentCounts) + totalCounts.DistinctEntities = distinctEntitiesResponse + responseData["total"] = totalCounts // Create and populate the month response structs based on the monthly breakdown. months, err := a.prepareMonthsResponseForQuery(ctx, pq.Months) @@ -2610,32 +2609,25 @@ func (a *ActivityLog) transformMonthBreakdowns(byMonth map[int64]*processMonth) return monthly } -func (a *ActivityLog) calculateByNamespaceResponseForQuery(ctx context.Context, byNamespace []*activity.NamespaceRecord) (int, int, []*ResponseNamespace, error) { +func (a *ActivityLog) calculateByNamespaceResponseForQuery(ctx context.Context, byNamespace []*activity.NamespaceRecord) (*ResponseCounts, []*ResponseNamespace, error) { queryNS, err := namespace.FromContext(ctx) if err != nil { - return 0, 0, nil, err + return nil, nil, err } byNamespaceResponse := make([]*ResponseNamespace, 0) - totalEntities := 0 - totalTokens := 0 + totalCounts := &ResponseCounts{} for _, nsRecord := range byNamespace { ns, err := NamespaceByID(ctx, nsRecord.NamespaceID, a.core) if err != nil { - return 0, 0, nil, err + return nil, nil, err } if a.includeInResponse(queryNS, ns) { mountResponse := make([]*ResponseMount, 0, len(nsRecord.Mounts)) for _, mountRecord := range nsRecord.Mounts { mountResponse = append(mountResponse, &ResponseMount{ MountPath: mountRecord.MountPath, - Counts: &ResponseCounts{ - DistinctEntities: int(mountRecord.Counts.EntityClients), - EntityClients: int(mountRecord.Counts.EntityClients), - NonEntityClients: int(mountRecord.Counts.NonEntityClients), - NonEntityTokens: int(mountRecord.Counts.NonEntityClients), - Clients: int(mountRecord.Counts.EntityClients + mountRecord.Counts.NonEntityClients), - }, + Counts: a.countsRecordToCountsResponse(mountRecord.Counts, true), }) } // Sort the mounts in descending order of usage @@ -2649,23 +2641,17 @@ func (a *ActivityLog) calculateByNamespaceResponseForQuery(ctx context.Context, } else { displayPath = ns.Path } + nsCounts := a.namespaceRecordToCountsResponse(nsRecord) byNamespaceResponse = append(byNamespaceResponse, &ResponseNamespace{ NamespaceID: nsRecord.NamespaceID, NamespacePath: displayPath, - Counts: ResponseCounts{ - DistinctEntities: int(nsRecord.Entities), - EntityClients: int(nsRecord.Entities), - NonEntityTokens: int(nsRecord.NonEntityTokens), - NonEntityClients: int(nsRecord.NonEntityTokens), - Clients: int(nsRecord.Entities + nsRecord.NonEntityTokens), - }, - Mounts: mountResponse, + Counts: *nsCounts, + Mounts: mountResponse, }) - totalEntities += int(nsRecord.Entities) - totalTokens += int(nsRecord.NonEntityTokens) + totalCounts.Add(nsCounts) } } - return totalEntities, totalTokens, byNamespaceResponse, nil + return totalCounts, byNamespaceResponse, nil } func (a *ActivityLog) prepareMonthsResponseForQuery(ctx context.Context, byMonth []*activity.MonthRecord) ([]*ResponseMonth, error) { @@ -2677,11 +2663,7 @@ func (a *ActivityLog) prepareMonthsResponseForQuery(ctx context.Context, byMonth if err != nil { return nil, err } - newClientsResponse.Counts = &ResponseCounts{ - EntityClients: int(monthsRecord.NewClients.Counts.EntityClients), - NonEntityClients: int(monthsRecord.NewClients.Counts.NonEntityClients), - Clients: int(monthsRecord.NewClients.Counts.EntityClients + monthsRecord.NewClients.Counts.NonEntityClients), - } + newClientsResponse.Counts = a.countsRecordToCountsResponse(monthsRecord.NewClients.Counts, false) newClientsResponse.Namespaces = newClientsNSResponse } @@ -2693,11 +2675,7 @@ func (a *ActivityLog) prepareMonthsResponseForQuery(ctx context.Context, byMonth if err != nil { return nil, err } - monthResponse.Counts = &ResponseCounts{ - EntityClients: int(monthsRecord.Counts.EntityClients), - NonEntityClients: int(monthsRecord.Counts.NonEntityClients), - Clients: int(monthsRecord.Counts.EntityClients + monthsRecord.Counts.NonEntityClients), - } + monthResponse.Counts = a.countsRecordToCountsResponse(monthsRecord.Counts, false) monthResponse.Namespaces = nsResponse monthResponse.NewClients = newClientsResponse months = append(months, monthResponse) @@ -2732,11 +2710,7 @@ func (a *ActivityLog) prepareNamespaceResponse(ctx context.Context, nsRecords [] mountResponse = append(mountResponse, &ResponseMount{ MountPath: mountRecord.MountPath, - Counts: &ResponseCounts{ - EntityClients: int(mountRecord.Counts.EntityClients), - NonEntityClients: int(mountRecord.Counts.NonEntityClients), - Clients: int(mountRecord.Counts.EntityClients + mountRecord.Counts.NonEntityClients), - }, + Counts: a.countsRecordToCountsResponse(mountRecord.Counts, false), }) } @@ -2749,12 +2723,8 @@ func (a *ActivityLog) prepareNamespaceResponse(ctx context.Context, nsRecords [] nsResponse = append(nsResponse, &ResponseNamespace{ NamespaceID: nsRecord.NamespaceID, NamespacePath: displayPath, - Counts: ResponseCounts{ - EntityClients: int(nsRecord.Counts.EntityClients), - NonEntityClients: int(nsRecord.Counts.NonEntityClients), - Clients: int(nsRecord.Counts.EntityClients + nsRecord.Counts.NonEntityClients), - }, - Mounts: mountResponse, + Counts: *a.countsRecordToCountsResponse(nsRecord.Counts, false), + Mounts: mountResponse, }) } } @@ -2783,7 +2753,7 @@ func (a *ActivityLog) partialMonthClientCount(ctx context.Context) (map[string]i // Calculate the namespace response breakdowns and totals for entities and tokens from the initial // namespace data. - totalEntities, totalTokens, byNamespaceResponse, err := a.calculateByNamespaceResponseForQuery(ctx, byNamespaceComputation) + totalCounts, byNamespaceResponse, err := a.calculateByNamespaceResponseForQuery(ctx, byNamespaceComputation) if err != nil { return nil, err } @@ -2794,11 +2764,12 @@ func (a *ActivityLog) partialMonthClientCount(ctx context.Context) (map[string]i // Now populate the response based on breakdowns. responseData := make(map[string]interface{}) responseData["by_namespace"] = byNamespaceResponse - responseData["distinct_entities"] = totalEntities - responseData["entity_clients"] = totalEntities - responseData["non_entity_tokens"] = totalTokens - responseData["non_entity_clients"] = totalTokens - responseData["clients"] = totalEntities + totalTokens + responseData["distinct_entities"] = totalCounts.EntityClients + responseData["entity_clients"] = totalCounts.EntityClients + responseData["non_entity_tokens"] = totalCounts.NonEntityClients + responseData["non_entity_clients"] = totalCounts.NonEntityClients + responseData["clients"] = totalCounts.Clients + responseData["secret_syncs"] = totalCounts.SecretSyncs // The partialMonthClientCount should not have more than one month worth of data. // If it does, something has gone wrong and we should warn that the activity log data diff --git a/vault/activity_log_util_common.go b/vault/activity_log_util_common.go index eca59e3c0a55..5709093999c5 100644 --- a/vault/activity_log_util_common.go +++ b/vault/activity_log_util_common.go @@ -197,19 +197,17 @@ func (a *ActivityLog) transformALNamespaceBreakdowns(nsData map[string]*processB // limitNamespacesInALResponse will truncate the number of namespaces shown in the activity // endpoints to the number specified in limitNamespaces (the API filtering parameter) -func (a *ActivityLog) limitNamespacesInALResponse(byNamespaceResponse []*ResponseNamespace, limitNamespaces int) (int, int, []*ResponseNamespace) { +func (a *ActivityLog) limitNamespacesInALResponse(byNamespaceResponse []*ResponseNamespace, limitNamespaces int) (*ResponseCounts, []*ResponseNamespace) { if limitNamespaces > len(byNamespaceResponse) { limitNamespaces = len(byNamespaceResponse) } byNamespaceResponse = byNamespaceResponse[:limitNamespaces] // recalculate total entities and tokens - totalEntities := 0 - totalTokens := 0 + totalCounts := &ResponseCounts{} for _, namespaceData := range byNamespaceResponse { - totalEntities += namespaceData.Counts.DistinctEntities - totalTokens += namespaceData.Counts.NonEntityTokens + totalCounts.Add(&namespaceData.Counts) } - return totalEntities, totalTokens, byNamespaceResponse + return totalCounts, byNamespaceResponse } // transformActivityLogMounts is a helper used to reformat data for transformMonthlyNamespaceBreakdowns. @@ -383,3 +381,28 @@ func (e *segmentReader) ReadEntity(ctx context.Context) (*activity.EntityActivit } return out, nil } + +func (a *ActivityLog) countsRecordToCountsResponse(record *activity.CountsRecord, includeDeprecated bool) *ResponseCounts { + response := &ResponseCounts{ + EntityClients: record.EntityClients, + NonEntityClients: record.NonEntityClients, + Clients: record.EntityClients + record.NonEntityClients, + SecretSyncs: record.SecretSyncs, + } + if includeDeprecated { + response.NonEntityTokens = response.NonEntityClients + response.DistinctEntities = response.EntityClients + } + return response +} + +func (a *ActivityLog) namespaceRecordToCountsResponse(record *activity.NamespaceRecord) *ResponseCounts { + return &ResponseCounts{ + DistinctEntities: int(record.Entities), + EntityClients: int(record.Entities), + NonEntityTokens: int(record.NonEntityTokens), + NonEntityClients: int(record.NonEntityTokens), + Clients: int(record.Entities + record.NonEntityTokens), + SecretSyncs: int(record.SecretSyncs), + } +} From 48be56ca50038943a0142a36323ab85890d4ecb0 Mon Sep 17 00:00:00 2001 From: Mia Epner Date: Mon, 8 Jan 2024 18:51:43 +0100 Subject: [PATCH 2/6] add secret sync tests --- vault/activity/query.go | 4 + vault/activity_log.go | 11 +- .../activity_testonly_test.go | 128 ++++++++++++++++++ 3 files changed, 139 insertions(+), 4 deletions(-) diff --git a/vault/activity/query.go b/vault/activity/query.go index bfe9eb3a1ccc..24501fcd0d71 100644 --- a/vault/activity/query.go +++ b/vault/activity/query.go @@ -32,6 +32,10 @@ type CountsRecord struct { SecretSyncs int `json:"secret_syncs"` } +func (c *CountsRecord) HasCounts() bool { + return c.EntityClients+c.NonEntityClients+c.SecretSyncs != 0 +} + type NewClientRecord struct { Counts *CountsRecord `json:"counts"` Namespaces []*MonthlyNamespaceRecord `json:"namespaces"` diff --git a/vault/activity_log.go b/vault/activity_log.go index 00f4d95a2c61..59d66160ae04 100644 --- a/vault/activity_log.go +++ b/vault/activity_log.go @@ -1582,6 +1582,9 @@ type ResponseCounts struct { } func (existingRecord *ResponseCounts) Add(newRecord *ResponseCounts) { + if newRecord == nil { + return + } existingRecord.EntityClients += newRecord.EntityClients existingRecord.Clients += newRecord.Clients existingRecord.DistinctEntities += newRecord.DistinctEntities @@ -2658,7 +2661,7 @@ func (a *ActivityLog) prepareMonthsResponseForQuery(ctx context.Context, byMonth months := make([]*ResponseMonth, 0, len(byMonth)) for _, monthsRecord := range byMonth { newClientsResponse := &ResponseNewClients{} - if int(monthsRecord.NewClients.Counts.EntityClients+monthsRecord.NewClients.Counts.NonEntityClients) != 0 { + if monthsRecord.NewClients.Counts.HasCounts() { newClientsNSResponse, err := a.prepareNamespaceResponse(ctx, monthsRecord.NewClients.Namespaces) if err != nil { return nil, err @@ -2670,7 +2673,7 @@ func (a *ActivityLog) prepareMonthsResponseForQuery(ctx context.Context, byMonth monthResponse := &ResponseMonth{ Timestamp: time.Unix(monthsRecord.Timestamp, 0).UTC().Format(time.RFC3339), } - if int(monthsRecord.Counts.EntityClients+monthsRecord.Counts.NonEntityClients) != 0 { + if monthsRecord.Counts.HasCounts() { nsResponse, err := a.prepareNamespaceResponse(ctx, monthsRecord.Namespaces) if err != nil { return nil, err @@ -2693,7 +2696,7 @@ func (a *ActivityLog) prepareNamespaceResponse(ctx context.Context, nsRecords [] } nsResponse := make([]*ResponseNamespace, 0, len(nsRecords)) for _, nsRecord := range nsRecords { - if int(nsRecord.Counts.EntityClients) == 0 && int(nsRecord.Counts.NonEntityClients) == 0 { + if !nsRecord.Counts.HasCounts() { continue } @@ -2704,7 +2707,7 @@ func (a *ActivityLog) prepareNamespaceResponse(ctx context.Context, nsRecords [] if a.includeInResponse(queryNS, ns) { mountResponse := make([]*ResponseMount, 0, len(nsRecord.Mounts)) for _, mountRecord := range nsRecord.Mounts { - if int(mountRecord.Counts.EntityClients) == 0 && int(mountRecord.Counts.NonEntityClients) == 0 { + if !mountRecord.Counts.HasCounts() { continue } diff --git a/vault/external_tests/activity_testonly/activity_testonly_test.go b/vault/external_tests/activity_testonly/activity_testonly_test.go index 45f1fa3d3c88..6ce88403a108 100644 --- a/vault/external_tests/activity_testonly/activity_testonly_test.go +++ b/vault/external_tests/activity_testonly/activity_testonly_test.go @@ -7,6 +7,7 @@ package activity_testonly import ( "context" + "encoding/json" "testing" "time" @@ -237,3 +238,130 @@ func getMonthsData(t *testing.T, resp *api.Secret) []vault.ResponseMonth { require.NoError(t, err) return monthsResponse } + +func getNamespaceData(t *testing.T, resp *api.Secret) []vault.ResponseNamespace { + t.Helper() + nsRaw, ok := resp.Data["by_namespace"] + require.True(t, ok) + nsResponse := make([]vault.ResponseNamespace, 0) + err := mapstructure.Decode(nsRaw, &nsResponse) + require.NoError(t, err) + return nsResponse +} + +func getTotals(t *testing.T, resp *api.Secret) vault.ResponseCounts { + t.Helper() + totalRaw, ok := resp.Data["total"] + require.True(t, ok) + total := vault.ResponseCounts{} + err := mapstructure.Decode(totalRaw, &total) + require.NoError(t, err) + return total +} + +// Test_ActivityLog_SecretSyncResponse creates 10 secret sync clients and +// verifies that the activity log query response returns 10 secret sync clients +// at every level of the response hierarchy +func Test_ActivityLog_SecretSyncResponse(t *testing.T) { + t.Parallel() + cluster := minimal.NewTestSoloCluster(t, nil) + client := cluster.Cores[0].Client + _, err := client.Logical().Write("sys/internal/counters/config", map[string]interface{}{ + "enabled": "enable", + }) + _, err = clientcountutil.NewActivityLogData(client). + NewCurrentMonthData(). + NewClientsSeen(10, clientcountutil.WithClientType("secret-sync")). + Write(context.Background(), generation.WriteOptions_WRITE_ENTITIES) + require.NoError(t, err) + + now := time.Now().UTC() + resp, err := client.Logical().ReadWithData("sys/internal/counters/activity", map[string][]string{ + "end_time": {timeutil.EndOfMonth(now).Format(time.RFC3339)}, + "start_time": {timeutil.StartOfMonth(now).Format(time.RFC3339)}, + }) + require.NoError(t, err) + + total := getTotals(t, resp) + require.Equal(t, 10, total.SecretSyncs) + + byNamespace := getNamespaceData(t, resp) + require.Equal(t, 10, byNamespace[0].Counts.SecretSyncs) + require.Equal(t, 10, byNamespace[0].Mounts[0].Counts.SecretSyncs) + + byMonth := getMonthsData(t, resp) + require.Equal(t, 10, byMonth[0].NewClients.Counts.SecretSyncs) + require.Equal(t, 10, byMonth[0].Counts.SecretSyncs) + require.Equal(t, 10, byMonth[0].Namespaces[0].Counts.SecretSyncs) + require.Equal(t, 10, byMonth[0].Namespaces[0].Mounts[0].Counts.SecretSyncs) +} + +// Test_ActivityLogCurrentMonth_SecretSyncResponse creates 10 secret sync +// clients and verifies that the activity log partial month response returns +// 10 secret sync clients at every level of the response hierarchy +func Test_ActivityLogCurrentMonth_SecretSyncResponse(t *testing.T) { + t.Parallel() + cluster := minimal.NewTestSoloCluster(t, nil) + client := cluster.Cores[0].Client + _, err := client.Logical().Write("sys/internal/counters/config", map[string]interface{}{ + "enabled": "enable", + }) + _, err = clientcountutil.NewActivityLogData(client). + NewCurrentMonthData(). + NewClientsSeen(10, clientcountutil.WithClientType("secret-sync")). + Write(context.Background(), generation.WriteOptions_WRITE_ENTITIES) + require.NoError(t, err) + + resp, err := client.Logical().Read("sys/internal/counters/activity/monthly") + require.NoError(t, err) + + secretSyncs, ok := resp.Data["secret_syncs"] + require.True(t, ok) + require.Equal(t, json.Number("10"), secretSyncs) + + byNamespace := getNamespaceData(t, resp) + require.Equal(t, 10, byNamespace[0].Counts.SecretSyncs) + require.Equal(t, 10, byNamespace[0].Mounts[0].Counts.SecretSyncs) + + byMonth := getMonthsData(t, resp) + require.Equal(t, 10, byMonth[0].NewClients.Counts.SecretSyncs) + require.Equal(t, 10, byMonth[0].Counts.SecretSyncs) + require.Equal(t, 10, byMonth[0].Namespaces[0].Counts.SecretSyncs) + require.Equal(t, 10, byMonth[0].Namespaces[0].Mounts[0].Counts.SecretSyncs) +} + +// Test_SecretSync_Deduplication verifies that secret sync clients are +// deduplicated across months. The test creates 10 secret sync clients and +// repeats those clients in later months, then also registers 3 and then 2 new +// secret sync clients. The test verifies that the total number of secret sync +// clients is 15 (10 + 2 + 3), ensuring that the duplicates are not included +func Test_SecretSync_Deduplication(t *testing.T) { + t.Parallel() + cluster := minimal.NewTestSoloCluster(t, nil) + client := cluster.Cores[0].Client + _, err := client.Logical().Write("sys/internal/counters/config", map[string]interface{}{ + "enabled": "enable", + }) + _, err = clientcountutil.NewActivityLogData(client). + NewPreviousMonthData(3). + NewClientsSeen(10, clientcountutil.WithClientType("secret-sync")). + NewPreviousMonthData(2). + RepeatedClientsSeen(4, clientcountutil.WithClientType("secret-sync")). + NewClientsSeen(3, clientcountutil.WithClientType("secret-sync")). + NewPreviousMonthData(1). + RepeatedClientsSeen(5, clientcountutil.WithClientType("secret-sync")). + NewClientsSeen(2, clientcountutil.WithClientType("secret-sync")). + Write(context.Background(), generation.WriteOptions_WRITE_PRECOMPUTED_QUERIES) + require.NoError(t, err) + + now := time.Now().UTC() + resp, err := client.Logical().ReadWithData("sys/internal/counters/activity", map[string][]string{ + "end_time": {timeutil.StartOfMonth(now).Format(time.RFC3339)}, + "start_time": {timeutil.StartOfMonth(timeutil.MonthsPreviousTo(4, now)).Format(time.RFC3339)}, + }, + ) + require.NoError(t, err) + + total := getTotals(t, resp) + require.Equal(t, 15, total.SecretSyncs) +} From 39bf9e186e2d5d8663f9d73e0a926f6b1b18076d Mon Sep 17 00:00:00 2001 From: Mia Epner Date: Mon, 8 Jan 2024 18:55:17 +0100 Subject: [PATCH 3/6] changelog --- changelog/24710.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/24710.txt diff --git a/changelog/24710.txt b/changelog/24710.txt new file mode 100644 index 000000000000..4985cda86580 --- /dev/null +++ b/changelog/24710.txt @@ -0,0 +1,3 @@ +```release-note:improvement +core/activity: Include secret_syncs in activity log responses +``` \ No newline at end of file From 84c1b99e35dc9202e53119f8ccff79525a8d2a33 Mon Sep 17 00:00:00 2001 From: Mia Epner Date: Wed, 10 Jan 2024 12:15:46 +0100 Subject: [PATCH 4/6] include secret syncs in clients --- vault/activity_log_util_common.go | 4 ++-- .../activity_testonly/activity_testonly_test.go | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/vault/activity_log_util_common.go b/vault/activity_log_util_common.go index 5709093999c5..2681f513bb67 100644 --- a/vault/activity_log_util_common.go +++ b/vault/activity_log_util_common.go @@ -386,7 +386,7 @@ func (a *ActivityLog) countsRecordToCountsResponse(record *activity.CountsRecord response := &ResponseCounts{ EntityClients: record.EntityClients, NonEntityClients: record.NonEntityClients, - Clients: record.EntityClients + record.NonEntityClients, + Clients: record.EntityClients + record.NonEntityClients + record.SecretSyncs, SecretSyncs: record.SecretSyncs, } if includeDeprecated { @@ -402,7 +402,7 @@ func (a *ActivityLog) namespaceRecordToCountsResponse(record *activity.Namespace EntityClients: int(record.Entities), NonEntityTokens: int(record.NonEntityTokens), NonEntityClients: int(record.NonEntityTokens), - Clients: int(record.Entities + record.NonEntityTokens), + Clients: int(record.Entities + record.NonEntityTokens + record.SecretSyncs), SecretSyncs: int(record.SecretSyncs), } } diff --git a/vault/external_tests/activity_testonly/activity_testonly_test.go b/vault/external_tests/activity_testonly/activity_testonly_test.go index 6ce88403a108..580869d04eba 100644 --- a/vault/external_tests/activity_testonly/activity_testonly_test.go +++ b/vault/external_tests/activity_testonly/activity_testonly_test.go @@ -284,16 +284,23 @@ func Test_ActivityLog_SecretSyncResponse(t *testing.T) { total := getTotals(t, resp) require.Equal(t, 10, total.SecretSyncs) + require.Equal(t, 10, total.Clients) byNamespace := getNamespaceData(t, resp) require.Equal(t, 10, byNamespace[0].Counts.SecretSyncs) require.Equal(t, 10, byNamespace[0].Mounts[0].Counts.SecretSyncs) + require.Equal(t, 10, byNamespace[0].Counts.Clients) + require.Equal(t, 10, byNamespace[0].Mounts[0].Counts.Clients) byMonth := getMonthsData(t, resp) require.Equal(t, 10, byMonth[0].NewClients.Counts.SecretSyncs) require.Equal(t, 10, byMonth[0].Counts.SecretSyncs) require.Equal(t, 10, byMonth[0].Namespaces[0].Counts.SecretSyncs) require.Equal(t, 10, byMonth[0].Namespaces[0].Mounts[0].Counts.SecretSyncs) + require.Equal(t, 10, byMonth[0].NewClients.Counts.Clients) + require.Equal(t, 10, byMonth[0].Counts.Clients) + require.Equal(t, 10, byMonth[0].Namespaces[0].Counts.Clients) + require.Equal(t, 10, byMonth[0].Namespaces[0].Mounts[0].Counts.Clients) } // Test_ActivityLogCurrentMonth_SecretSyncResponse creates 10 secret sync @@ -318,16 +325,25 @@ func Test_ActivityLogCurrentMonth_SecretSyncResponse(t *testing.T) { secretSyncs, ok := resp.Data["secret_syncs"] require.True(t, ok) require.Equal(t, json.Number("10"), secretSyncs) + clients, ok := resp.Data["clients"] + require.True(t, ok) + require.Equal(t, json.Number("10"), clients) byNamespace := getNamespaceData(t, resp) require.Equal(t, 10, byNamespace[0].Counts.SecretSyncs) require.Equal(t, 10, byNamespace[0].Mounts[0].Counts.SecretSyncs) + require.Equal(t, 10, byNamespace[0].Counts.Clients) + require.Equal(t, 10, byNamespace[0].Mounts[0].Counts.Clients) byMonth := getMonthsData(t, resp) require.Equal(t, 10, byMonth[0].NewClients.Counts.SecretSyncs) require.Equal(t, 10, byMonth[0].Counts.SecretSyncs) require.Equal(t, 10, byMonth[0].Namespaces[0].Counts.SecretSyncs) require.Equal(t, 10, byMonth[0].Namespaces[0].Mounts[0].Counts.SecretSyncs) + require.Equal(t, 10, byMonth[0].NewClients.Counts.Clients) + require.Equal(t, 10, byMonth[0].Counts.Clients) + require.Equal(t, 10, byMonth[0].Namespaces[0].Counts.Clients) + require.Equal(t, 10, byMonth[0].Namespaces[0].Mounts[0].Counts.Clients) } // Test_SecretSync_Deduplication verifies that secret sync clients are @@ -364,4 +380,5 @@ func Test_SecretSync_Deduplication(t *testing.T) { total := getTotals(t, resp) require.Equal(t, 15, total.SecretSyncs) + require.Equal(t, 15, total.Clients) } From 2007d077d5e250bdf8b96c738c567f628335d7db Mon Sep 17 00:00:00 2001 From: Mia Epner Date: Wed, 10 Jan 2024 12:26:22 +0100 Subject: [PATCH 5/6] pr comments --- vault/activity_log.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/vault/activity_log.go b/vault/activity_log.go index 59d66160ae04..4a9f76264716 100644 --- a/vault/activity_log.go +++ b/vault/activity_log.go @@ -1581,16 +1581,16 @@ type ResponseCounts struct { SecretSyncs int `json:"secret_syncs" mapstructure:"secret_syncs"` } -func (existingRecord *ResponseCounts) Add(newRecord *ResponseCounts) { +func (r *ResponseCounts) Add(newRecord *ResponseCounts) { if newRecord == nil { return } - existingRecord.EntityClients += newRecord.EntityClients - existingRecord.Clients += newRecord.Clients - existingRecord.DistinctEntities += newRecord.DistinctEntities - existingRecord.NonEntityClients += newRecord.NonEntityClients - existingRecord.NonEntityTokens += newRecord.NonEntityTokens - existingRecord.SecretSyncs += newRecord.SecretSyncs + r.EntityClients += newRecord.EntityClients + r.Clients += newRecord.Clients + r.DistinctEntities += newRecord.DistinctEntities + r.NonEntityClients += newRecord.NonEntityClients + r.NonEntityTokens += newRecord.NonEntityTokens + r.SecretSyncs += newRecord.SecretSyncs } type ResponseNamespace struct { @@ -1600,23 +1600,23 @@ type ResponseNamespace struct { Mounts []*ResponseMount `json:"mounts"` } -func (existingRecord *ResponseNamespace) Add(newRecord *ResponseNamespace) { +func (r *ResponseNamespace) Add(newRecord *ResponseNamespace) { // Create a map of the existing mounts, so we don't duplicate them mountMap := make(map[string]*ResponseCounts) - for _, erm := range existingRecord.Mounts { + for _, erm := range r.Mounts { mountMap[erm.MountPath] = erm.Counts } - existingRecord.Counts.Add(&newRecord.Counts) + r.Counts.Add(&newRecord.Counts) // Check the current month mounts against the existing mounts and if there are matches, update counts // accordingly. If there is no match, append the new mount to the existing mounts, so it will be counted // later. for _, newRecordMount := range newRecord.Mounts { - if existingRecordMountCounts, ook := mountMap[newRecordMount.MountPath]; ook { + if existingRecordMountCounts, ok := mountMap[newRecordMount.MountPath]; ok { existingRecordMountCounts.Add(&newRecord.Counts) } else { - existingRecord.Mounts = append(existingRecord.Mounts, newRecordMount) + r.Mounts = append(r.Mounts, newRecordMount) } } } From 41c802f280ca6058686378529eed3f457ad09ca8 Mon Sep 17 00:00:00 2001 From: Mia Epner Date: Mon, 15 Jan 2024 10:54:38 +0100 Subject: [PATCH 6/6] add godocs --- vault/activity/query.go | 1 + vault/activity_log.go | 4 ++++ vault/activity_log_util_common.go | 7 +++++++ 3 files changed, 12 insertions(+) diff --git a/vault/activity/query.go b/vault/activity/query.go index 24501fcd0d71..7e035c3dd7fb 100644 --- a/vault/activity/query.go +++ b/vault/activity/query.go @@ -32,6 +32,7 @@ type CountsRecord struct { SecretSyncs int `json:"secret_syncs"` } +// HasCounts returns true when any of the record's fields have a non-zero value func (c *CountsRecord) HasCounts() bool { return c.EntityClients+c.NonEntityClients+c.SecretSyncs != 0 } diff --git a/vault/activity_log.go b/vault/activity_log.go index 4a9f76264716..e27ad14948ce 100644 --- a/vault/activity_log.go +++ b/vault/activity_log.go @@ -1581,6 +1581,7 @@ type ResponseCounts struct { SecretSyncs int `json:"secret_syncs" mapstructure:"secret_syncs"` } +// Add adds the new record's counts to the existing record func (r *ResponseCounts) Add(newRecord *ResponseCounts) { if newRecord == nil { return @@ -1600,6 +1601,9 @@ type ResponseNamespace struct { Mounts []*ResponseMount `json:"mounts"` } +// Add adds the namespace counts to the existing record, then either adds the +// mount counts to the existing mount (if it exists) or appends the mount to the +// list of mounts func (r *ResponseNamespace) Add(newRecord *ResponseNamespace) { // Create a map of the existing mounts, so we don't duplicate them mountMap := make(map[string]*ResponseCounts) diff --git a/vault/activity_log_util_common.go b/vault/activity_log_util_common.go index 2681f513bb67..152da93a9744 100644 --- a/vault/activity_log_util_common.go +++ b/vault/activity_log_util_common.go @@ -382,6 +382,10 @@ func (e *segmentReader) ReadEntity(ctx context.Context) (*activity.EntityActivit return out, nil } +// namespaceRecordToCountsResponse converts the record to the ResponseCounts +// type. The function sums entity, non-entity, and secret sync counts to get the +// total client count. If includeDeprecated is true, the deprecated fields +// NonEntityTokens and DistinctEntities are populated func (a *ActivityLog) countsRecordToCountsResponse(record *activity.CountsRecord, includeDeprecated bool) *ResponseCounts { response := &ResponseCounts{ EntityClients: record.EntityClients, @@ -396,6 +400,9 @@ func (a *ActivityLog) countsRecordToCountsResponse(record *activity.CountsRecord return response } +// namespaceRecordToCountsResponse converts the namespace counts to the +// ResponseCounts type. The function sums entity, non-entity, and secret sync +// counts to get the total client count. func (a *ActivityLog) namespaceRecordToCountsResponse(record *activity.NamespaceRecord) *ResponseCounts { return &ResponseCounts{ DistinctEntities: int(record.Entities),