CSS-5179 Audit log enhacement #1026
Conversation
- Fixed inability to filter by method - Added ability to sort by time
| @@ -73,6 +73,8 @@ func (c *listAuditEventsCommand) SetFlags(f *gnuflag.FlagSet) { | |||
| f.StringVar(&c.args.Model, "model", "", "display events for a specific model (model name is controller/model)") | |||
| f.IntVar(&c.args.Limit, "offset", 0, "offset the set of returned audit events") | |||
| f.IntVar(&c.args.Limit, "limit", 0, "limit the maximum number of returned audit events") | |||
| f.BoolVar(&c.args.SortTime, "reverse", false, "reverse the order of logs, showing the most recent first") | |||
There was a problem hiding this comment.
This is a bit confusing. Having the SortTime as bool indicates that if set to false, then the records are not be sorted by time (either desc or asc), but it seems that SortTime would reverse the already sorted logs.
Maybe We can change the name to SortDescending instead of SortTime.
any instead of reverse, we can just call it descending.
Just a thought, though
There was a problem hiding this comment.
Or
We can use an enum to allow for
Descending
No order (default)
Ascending
There was a problem hiding this comment.
I think if the reverse flag is false (default) the expectation from a user would be that the order is not reversed i.e oldest logs first, this is the behavior you'll see with a lot of tools like journalctl and kubectl. And that is what JIMM will do as well because our logs (without sorting) are returned in order of ID which is effectively sorted in ascending order of time.
There was a problem hiding this comment.
But I agree the name could be better
| @@ -55,6 +55,10 @@ type AuditLogFilter struct { | |||
| // Limit is the maximum number of audit events to return. | |||
| // A value of zero will ignore the limit. | |||
| Limit int `json:"limit,omitempty"` | |||
|
|
|||
| // SortTime will sort by most recent first (time descending) when true. | |||
| // When false no explicit ordering will be applied. | |||
There was a problem hiding this comment.
if no specific ordering, should we call the flag reverse? What is it reversing?
There was a problem hiding this comment.
We could call it reverse, it's just a little tricky because as I mentioned above, the default ordering is by ID which will essentially be ordering by time in ascending order.
|
I think the |
Description
Based on a discussion with the web team, they found that audit logs were not filtering based on method, this was a bug due to the translation from API params to a db.AuditLogFilter struct (this has been moved to a separate function with its own tests). They also requested the ability to sort logs with the most recent first, this has been added as an optional flag.
Engineering checklist
Check only items that apply