From 3567ce6f18343cae0bdcb039e2d776a5bb5586a4 Mon Sep 17 00:00:00 2001 From: Mia Epner Date: Mon, 22 May 2023 16:55:14 +0200 Subject: [PATCH] simplify --- vault/activity_log.go | 33 +++++------ .../logical_system_activity_write_testonly.go | 55 +------------------ 2 files changed, 16 insertions(+), 72 deletions(-) diff --git a/vault/activity_log.go b/vault/activity_log.go index 7cfaae7e312e..32f0570ad038 100644 --- a/vault/activity_log.go +++ b/vault/activity_log.go @@ -360,7 +360,7 @@ func (a *ActivityLog) saveCurrentSegmentToStorageLocked(ctx context.Context, for } a.currentSegment.currentClients.Clients = segmentClients - _, err := a.saveSegmentInternal(ctx, a.currentSegment, force) + err := a.saveCurrentSegmentInternal(ctx, force) if err != nil { // The current fragment(s) have already been placed into the in-memory // segment, but we may lose any excess (in excessClients). @@ -384,7 +384,7 @@ func (a *ActivityLog) saveCurrentSegmentToStorageLocked(ctx context.Context, for excessClients = excessClients[:activitySegmentClientCapacity] } a.currentSegment.currentClients.Clients = excessClients - _, err := a.saveSegmentInternal(ctx, a.currentSegment, force) + err := a.saveCurrentSegmentInternal(ctx, force) if err != nil { return err } @@ -393,24 +393,23 @@ func (a *ActivityLog) saveCurrentSegmentToStorageLocked(ctx context.Context, for } // :force: forces a save of tokens/entities even if the in-memory log is empty -func (a *ActivityLog) saveSegmentInternal(ctx context.Context, currentSegment segmentInfo, force bool) ([]string, error) { - entityPath := fmt.Sprintf("%s%d/%d", activityEntityBasePath, currentSegment.startTimestamp, currentSegment.clientSequenceNumber) +func (a *ActivityLog) saveCurrentSegmentInternal(ctx context.Context, force bool) error { + entityPath := fmt.Sprintf("%s%d/%d", activityEntityBasePath, a.currentSegment.startTimestamp, a.currentSegment.clientSequenceNumber) // RFC (VLT-120) defines this as 1-indexed, but it should be 0-indexed - tokenPath := fmt.Sprintf("%s%d/0", activityTokenBasePath, currentSegment.startTimestamp) - paths := make([]string, 0, 2) + tokenPath := fmt.Sprintf("%s%d/0", activityTokenBasePath, a.currentSegment.startTimestamp) for _, client := range a.currentSegment.currentClients.Clients { // Explicitly catch and throw clear error message if client ID creation and storage // results in a []byte that doesn't assert into a valid string. if !utf8.ValidString(client.ClientID) { - return nil, fmt.Errorf("client ID %q is not a valid string:", client.ClientID) + return fmt.Errorf("client ID %q is not a valid string:", client.ClientID) } } - if len(currentSegment.currentClients.Clients) > 0 || force { - clients, err := proto.Marshal(currentSegment.currentClients) + if len(a.currentSegment.currentClients.Clients) > 0 || force { + clients, err := proto.Marshal(a.currentSegment.currentClients) if err != nil { - return nil, err + return err } a.logger.Trace("writing segment", "path", entityPath) @@ -419,15 +418,14 @@ func (a *ActivityLog) saveSegmentInternal(ctx context.Context, currentSegment se Value: clients, }) if err != nil { - return nil, err + return err } - paths = append(paths, entityPath) } // We must still allow for the tokenCount of the current segment to // be written to storage, since if we remove this code we will incur // data loss for one segment's worth of TWEs. - if len(currentSegment.tokenCount.CountByNamespaceID) > 0 || force { + if len(a.currentSegment.tokenCount.CountByNamespaceID) > 0 || force { // We can get away with simply using the oldest version stored because // the storing of versions was introduced at the same time as this code. oldestVersion, oldestUpgradeTime, err := a.core.FindOldestVersionTimestamp() @@ -444,7 +442,7 @@ func (a *ActivityLog) saveSegmentInternal(ctx context.Context, currentSegment se } tokenCount, err := proto.Marshal(a.currentSegment.tokenCount) if err != nil { - return nil, err + return err } a.logger.Trace("writing segment", "path", tokenPath) @@ -453,11 +451,10 @@ func (a *ActivityLog) saveSegmentInternal(ctx context.Context, currentSegment se Value: tokenCount, }) if err != nil { - return nil, err + return err } - paths = append(paths, tokenPath) } - return paths, nil + return nil } // parseSegmentNumberFromPath returns the segment number from a path @@ -1024,7 +1021,7 @@ func (a *ActivityLog) SetConfig(ctx context.Context, config activityConfig) { if forceSave { // l is still held here - a.saveSegmentInternal(ctx, a.currentSegment, true) + a.saveCurrentSegmentInternal(ctx, true) } a.defaultReportMonths = config.DefaultReportMonths diff --git a/vault/logical_system_activity_write_testonly.go b/vault/logical_system_activity_write_testonly.go index 5d74716a9f38..f02a8d51bd2b 100644 --- a/vault/logical_system_activity_write_testonly.go +++ b/vault/logical_system_activity_write_testonly.go @@ -54,34 +54,7 @@ func (b *SystemBackend) handleActivityWriteData(ctx context.Context, request *lo if len(input.Data) == 0 { return logical.ErrorResponse("Missing required \"data\" values"), logical.ErrInvalidRequest } - - numMonths := 0 - for _, month := range input.Data { - if int(month.GetMonthsAgo()) > numMonths { - numMonths = int(month.GetMonthsAgo()) - } - } - generated := newMultipleMonthsActivityClients(numMonths) - for _, month := range input.Data { - err := generated.processMonth(ctx, b.Core, month) - if err != nil { - return logical.ErrorResponse("failed to process data for month %d", month.GetMonthsAgo()), err - } - } - - opts := make(map[generation.WriteOptions]struct{}, len(input.Write)) - for _, opt := range input.Write { - opts[opt] = struct{}{} - } - paths, err := generated.write(ctx, opts, b.Core.activityLog) - if err != nil { - return logical.ErrorResponse("failed to write data"), err - } - return &logical.Response{ - Data: map[string]interface{}{ - "paths": paths, - }, - }, nil + return nil, nil } // singleMonthActivityClients holds a single month's client IDs, in the order they were seen @@ -317,32 +290,6 @@ func (m *multipleMonthsActivityClients) addRepeatedClients(monthsAgo int32, c *g return nil } -func (m *multipleMonthsActivityClients) write(ctx context.Context, opts map[generation.WriteOptions]struct{}, activityLog *ActivityLog) ([]string, error) { - paths := []string{} - for _, month := range m.months { - segments := month.populateSegments() - timestamp := 0 - for segmentIndex, segment := range segments { - if _, ok := opts[generation.WriteOptions_WRITE_ENTITIES]; ok { - if segment == nil { - // skip the index - continue - } - entityPaths, err := activityLog.saveSegmentInternal(ctx, segmentInfo{ - startTimestamp: int64(timestamp), - currentClients: &activity.EntityActivityLog{Clients: segment}, - clientSequenceNumber: uint64(segmentIndex), - }, true) - if err != nil { - return nil, err - } - paths = append(paths, entityPaths...) - } - } - } - return paths, nil -} - func newMultipleMonthsActivityClients(numberOfMonths int) *multipleMonthsActivityClients { m := &multipleMonthsActivityClients{ months: make([]*singleMonthActivityClients, numberOfMonths),