From 6a30d4e5b0e55823c8f6b2ff73f985addcdb80b0 Mon Sep 17 00:00:00 2001 From: Amir Aslamov Date: Wed, 12 Feb 2025 15:16:34 -0500 Subject: [PATCH] VAULT-33603: normalize start time in export api (#29562) * copy subset of oss changes from ent pr * add changelog entree --- changelog/29562.txt | 3 + vault/activity_log.go | 5 ++ vault/activity_log_test.go | 85 +++++++++++++++++++ vault/core.go | 2 +- vault/core_util.go | 6 +- .../activity_testonly_test.go | 17 ++-- 6 files changed, 106 insertions(+), 12 deletions(-) create mode 100644 changelog/29562.txt diff --git a/changelog/29562.txt b/changelog/29562.txt new file mode 100644 index 0000000000..80345750ec --- /dev/null +++ b/changelog/29562.txt @@ -0,0 +1,3 @@ +```release-note:bug +export API: Normalize the start_date parameter to the start of the month as is done in the sys/counters API to keep the results returned from both of the API's consistent. +``` \ No newline at end of file diff --git a/vault/activity_log.go b/vault/activity_log.go index 98886e626c..fd68f23e41 100644 --- a/vault/activity_log.go +++ b/vault/activity_log.go @@ -2971,6 +2971,11 @@ func (a *ActivityLog) writeExport(ctx context.Context, rw http.ResponseWriter, f } defer a.inprocessExport.Store(false) + // Normalize the start time to the beginning of the month to keep consistency with the sys/counters API + // Without this, if the start time falls within the same month as the billing start date, the Export API + // could omit data that the sys/counters API includes, leading to discrepancies + startTime = timeutil.StartOfMonth(startTime) + // Find the months with activity log data that are between the start and end // months. We want to walk this in cronological order so the oldest instance of a // client usage is recorded, not the most recent. diff --git a/vault/activity_log_test.go b/vault/activity_log_test.go index 90df8d75e0..03a6575f03 100644 --- a/vault/activity_log_test.go +++ b/vault/activity_log_test.go @@ -5038,3 +5038,88 @@ func TestActivityLog_Export_CSV_Header(t *testing.T) { require.Empty(t, deep.Equal(expectedColumnIndex, encoder.columnIndex)) } + +// TestActivityLog_partialMonthClientCountUsingWriteExport verifies that the writeExport +// method returns the same number of clients when queried with a start_time that is at +// different times during the same month. +func TestActivityLog_partialMonthClientCountUsingWriteExport(t *testing.T) { + ctx := namespace.RootContext(nil) + now := time.Now().UTC() + a, expectedClients, _ := setupActivityRecordsInStorage(t, timeutil.StartOfMonth(now), true, true) + + // clients[0] belongs to previous month + // the rest belong to the current month + expectedCurrentMonthClients := expectedClients[1:] + + type record struct { + ClientID string `json:"client_id"` + NamespaceID string `json:"namespace_id"` + Timestamp string `json:"timestamp"` + NonEntity bool `json:"non_entity"` + MountAccessor string `json:"mount_accessor"` + ClientType string `json:"client_type"` + } + + startOfMonth := timeutil.StartOfMonth(now) + endOfMonth := timeutil.EndOfMonth(now) + middleOfMonth := startOfMonth.Add(endOfMonth.Sub(startOfMonth) / 2) + testCases := []struct { + name string + requestedStartTime time.Time + }{ + { + name: "start time is the start of the current month", + requestedStartTime: startOfMonth, + }, + { + name: "start time is the middle of the current month", + requestedStartTime: middleOfMonth, + }, + { + name: "start time is the end of the current month", + requestedStartTime: endOfMonth, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + rw := &fakeResponseWriter{ + buffer: &bytes.Buffer{}, + headers: http.Header{}, + } + + // Test different start times but keep the end time at the end of the current month + // Start time of any timestamp within the current month should result in the same output from the export API + if err := a.writeExport(ctx, rw, "json", tc.requestedStartTime, endOfMonth); err != nil { + t.Fatal(err) + } + + // Convert the json objects from the buffer and compare the results + var results []record + jsonObjects := strings.Split(strings.TrimSpace(rw.buffer.String()), "\n") + for _, jsonObject := range jsonObjects { + if jsonObject == "" { + continue + } + + var result record + if err := json.Unmarshal([]byte(jsonObject), &result); err != nil { + t.Fatalf("Error unmarshaling JSON object: %v\nJSON: %s", err, jsonObject) + } + results = append(results, result) + } + + // Compare expectedClients with actualClients + for i := range expectedCurrentMonthClients { + resultTimeStamp, err := time.Parse(time.RFC3339, results[i].Timestamp) + require.NoError(t, err) + require.Equal(t, expectedCurrentMonthClients[i].ClientID, results[i].ClientID) + require.Equal(t, expectedCurrentMonthClients[i].NamespaceID, results[i].NamespaceID) + require.Equal(t, expectedCurrentMonthClients[i].Timestamp, resultTimeStamp.Unix()) + require.Equal(t, expectedCurrentMonthClients[i].NonEntity, results[i].NonEntity) + require.Equal(t, expectedCurrentMonthClients[i].MountAccessor, results[i].MountAccessor) + require.Equal(t, expectedCurrentMonthClients[i].ClientType, results[i].ClientType) + } + }) + } +} diff --git a/vault/core.go b/vault/core.go index 668d0e4c4d..085a334943 100644 --- a/vault/core.go +++ b/vault/core.go @@ -778,7 +778,7 @@ func (c *Core) HALock() sync.Locker { // CoreConfig is used to parameterize a core type CoreConfig struct { - entCoreConfig + EntCoreConfig DevToken string diff --git a/vault/core_util.go b/vault/core_util.go index 549971b9e4..5d9bd43b16 100644 --- a/vault/core_util.go +++ b/vault/core_util.go @@ -28,11 +28,11 @@ const ( type ( entCore struct{} - entCoreConfig struct{} + EntCoreConfig struct{} ) -func (e entCoreConfig) Clone() entCoreConfig { - return entCoreConfig{} +func (e EntCoreConfig) Clone() EntCoreConfig { + return EntCoreConfig{} } type LicensingConfig struct { diff --git a/vault/external_tests/activity_testonly/activity_testonly_test.go b/vault/external_tests/activity_testonly/activity_testonly_test.go index 8141357efe..644ba70e8d 100644 --- a/vault/external_tests/activity_testonly/activity_testonly_test.go +++ b/vault/external_tests/activity_testonly/activity_testonly_test.go @@ -496,12 +496,12 @@ func Test_ActivityLog_MountDeduplication(t *testing.T) { } // getJSONExport is used to fetch activity export records using json format. -// The records will returned as a map keyed by client ID. -func getJSONExport(t *testing.T, client *api.Client, monthsPreviousTo int, now time.Time) (map[string]vault.ActivityLogExportRecord, error) { +// The records will be returned as a map keyed by client ID. +func getJSONExport(t *testing.T, client *api.Client, startTime time.Time, now time.Time) (map[string]vault.ActivityLogExportRecord, error) { t.Helper() resp, err := client.Logical().ReadRawWithData("sys/internal/counters/activity/export", map[string][]string{ - "start_time": {timeutil.StartOfMonth(timeutil.MonthsPreviousTo(monthsPreviousTo, now)).Format(time.RFC3339)}, + "start_time": {startTime.Format(time.RFC3339)}, "end_time": {timeutil.EndOfMonth(now).Format(time.RFC3339)}, "format": {"json"}, }) @@ -540,7 +540,7 @@ func getJSONExport(t *testing.T, client *api.Client, monthsPreviousTo int, now t // getCSVExport fetches activity export records using csv format. All flattened // map and slice fields will be unflattened so that the a proper ActivityLogExportRecord // can be formed. The records will returned as a map keyed by client ID. -func getCSVExport(t *testing.T, client *api.Client, monthsPreviousTo int, now time.Time) (map[string]vault.ActivityLogExportRecord, error) { +func getCSVExport(t *testing.T, client *api.Client, startTime time.Time, now time.Time) (map[string]vault.ActivityLogExportRecord, error) { t.Helper() boolFields := map[string]struct{}{ @@ -559,7 +559,7 @@ func getCSVExport(t *testing.T, client *api.Client, monthsPreviousTo int, now ti } resp, err := client.Logical().ReadRawWithData("sys/internal/counters/activity/export", map[string][]string{ - "start_time": {timeutil.StartOfMonth(timeutil.MonthsPreviousTo(monthsPreviousTo, now)).Format(time.RFC3339)}, + "start_time": {startTime.Format(time.RFC3339)}, "end_time": {timeutil.EndOfMonth(now).Format(time.RFC3339)}, "format": {"csv"}, }) @@ -677,7 +677,8 @@ func Test_ActivityLog_Export_Sudo(t *testing.T) { require.NoError(t, err) // Ensure access via root token - clients, err := getJSONExport(t, client, 1, now) + startTime := timeutil.StartOfMonth(timeutil.MonthsPreviousTo(1, now)) + clients, err := getJSONExport(t, client, startTime, now) require.NoError(t, err) require.Len(t, clients, 10) @@ -696,7 +697,7 @@ path "sys/internal/counters/activity/export" { client.SetToken(nonSudoToken) // Ensure no access via token without sudo access - clients, err = getJSONExport(t, client, 1, now) + clients, err = getJSONExport(t, client, startTime, now) require.ErrorContains(t, err, "permission denied") client.SetToken(rootToken) @@ -715,7 +716,7 @@ path "sys/internal/counters/activity/export" { client.SetToken(sudoToken) // Ensure access via token with sudo access - clients, err = getJSONExport(t, client, 1, now) + clients, err = getJSONExport(t, client, startTime, now) require.NoError(t, err) require.Len(t, clients, 10) }