-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix misattribution of activity log entries to incorrect auth methods #18809
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
2115ae4
Fix misattribution of activity log entries to incorrect auth methods
maxb 1b65ae9
Changelog
maxb 24dc441
Merge branch 'main' into activity-log-namespace
maxb 7c92afd
Merge branch 'main' into activity-log-namespace
VioletHynes 640ebe0
Merge branch 'main' into activity-log-namespace
maxb 56478d3
Use a real namespace ID in tests where it now matters
maxb 26eb250
gofumpt
maxb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
```release-note:bug | ||
activity (enterprise): Fix misattribution of entities to no or child namespace auth methods | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,11 +96,6 @@ type segmentInfo struct { | |
tokenCount *activity.TokenCount | ||
} | ||
|
||
type clients struct { | ||
distinctEntities uint64 | ||
distinctNonEntities 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. | ||
|
@@ -1915,39 +1910,50 @@ func (a *ActivityLog) loadConfigOrDefault(ctx context.Context) (activityConfig, | |
|
||
// HandleTokenUsage adds the TokenEntry to the current fragment of the activity log | ||
// This currently occurs on token usage only. | ||
func (a *ActivityLog) HandleTokenUsage(ctx context.Context, entry *logical.TokenEntry, clientID string, isTWE bool) { | ||
func (a *ActivityLog) HandleTokenUsage(ctx context.Context, entry *logical.TokenEntry, clientID string, isTWE bool) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to reviewer: A great deal of this PR is just dealing with this method now being able to return an error. |
||
// First, check if a is enabled, so as to avoid the cost of creating an ID for | ||
// tokens without entities in the case where it not. | ||
a.fragmentLock.RLock() | ||
if !a.enabled { | ||
a.fragmentLock.RUnlock() | ||
return | ||
return nil | ||
} | ||
a.fragmentLock.RUnlock() | ||
|
||
// Do not count wrapping tokens in client count | ||
if IsWrappingToken(entry) { | ||
return | ||
return nil | ||
} | ||
|
||
// Do not count root tokens in client count. | ||
if entry.IsRoot() { | ||
return | ||
return nil | ||
} | ||
|
||
// Tokens created for the purpose of Link should bypass counting for billing purposes | ||
if entry.InternalMeta != nil && entry.InternalMeta[IgnoreForBilling] == "true" { | ||
return | ||
return nil | ||
} | ||
|
||
// Look up the mount accessor of the auth method that issued the token, taking care to resolve the token path | ||
// against the token namespace, which may not be the same as the request namespace! | ||
tokenNS, err := NamespaceByID(ctx, entry.NamespaceID, a.core) | ||
if err != nil { | ||
return err | ||
} | ||
if tokenNS == nil { | ||
return namespace.ErrNoNamespace | ||
} | ||
tokenCtx := namespace.ContextWithNamespace(ctx, tokenNS) | ||
mountAccessor := "" | ||
mountEntry := a.core.router.MatchingMountEntry(ctx, entry.Path) | ||
mountEntry := a.core.router.MatchingMountEntry(tokenCtx, entry.Path) | ||
if mountEntry != nil { | ||
mountAccessor = mountEntry.Accessor | ||
} | ||
|
||
// Parse an entry's client ID and add it to the activity log | ||
a.AddClientToFragment(clientID, entry.NamespaceID, entry.CreationTime, isTWE, mountAccessor) | ||
return nil | ||
} | ||
|
||
func (a *ActivityLog) namespaceToLabel(ctx context.Context, nsID string) string { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to reviewer: Removed unused struct noticed in passing