feat: Add log filters, event handling, and time validation#767
feat: Add log filters, event handling, and time validation#767NishchayRajput wants to merge 3 commits intogoharbor:mainfrom
Conversation
|
Please drop me suggestions and something I missed. |
Signed-off-by: Nishchay Rajput <nishchayr@iitbhilai.ac.in>
Signed-off-by: Nishchay Rajput <nishchayr@iitbhilai.ac.in>
NishchayRajput
force-pushed
the
feat/logs-filters
branch
from
679146e to
98cccf8
Compare
Signed-off-by: Nishchay Rajput <nishchayr@iitbhilai.ac.in>
There was a problem hiding this comment.
Pull request overview
This PR enhances the Harbor CLI audit-log experience by adding a logs events subcommand, introducing convenience filter flags for harbor logs, and implementing helper logic for query building plus time normalization/validation.
Changes:
- Added
harbor logs eventsto list supported Harbor audit log event types (with optional client-side pagination and JSON output support). - Added convenience filter flags to
harbor logsand composed them into Harborqquery syntax. - Added helper functions for audit-log query building, time normalization, and pagination summary output, plus unit tests for these helpers.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| cmd/harbor/root/logs.go | Adds logs events, convenience filter flags, query/time helpers, and pagination summary printing. |
| cmd/harbor/root/logs_test.go | Adds unit tests for query building, time normalization, and event-type pagination/name helpers. |
Comments suppressed due to low confidence (1)
cmd/harbor/root/logs.go:103
- In
--followmode,buildAuditLogQueryis executed once before entering the polling loop. When--from-timeis provided without--to-time, the code bakes in anop_timeupper bound of “now” at startup, which prevents any newer audit logs from ever matching in subsequent polls. Consider requiring both times, or rebuilding the query (updating thetobound) on each refresh when following.
query, err := buildAuditLogQuery(
opts.Q,
operationFilter,
resourceTypeFilter,
resourceFilter,
usernameFilter,
fromTimeFilter,
toTimeFilter,
)
if err != nil {
return err
}
opts.Q = query
if follow {
var interval time.Duration = 5 * time.Second
if refreshInterval != "" {
interval, err = time.ParseDuration(refreshInterval)
if err != nil {
return fmt.Errorf("invalid refresh interval: %w", err)
}
if interval < 500*time.Millisecond {
return fmt.Errorf("refresh-interval must be at least 500ms (got: %v)", interval)
}
}
followLogs(opts, interval)
return nil
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func printAuditLogEventTypesTable(eventTypes []*models.AuditLogEventType, page, pageSize int64, total int, showPaginationSummary bool) { | ||
| if len(eventTypes) == 0 { | ||
| if showPaginationSummary { | ||
| fmt.Println("No audit log event types found for the requested page.") | ||
| return | ||
| } | ||
| fmt.Println("No audit log event types found.") | ||
| return | ||
| } | ||
|
|
||
| fmt.Printf("%-6s %-40s\n", "INDEX", "EVENT_TYPE") | ||
| fmt.Printf("%-6s %-40s\n", "-----", "----------------------------------------") | ||
|
|
||
| startIndex := int64(1) | ||
| if showPaginationSummary { | ||
| startIndex = (page-1)*pageSize + 1 | ||
| } | ||
| for i, eventType := range eventTypes { | ||
| fmt.Printf("%-6d %-40s\n", startIndex+int64(i), auditLogEventTypeName(eventType)) | ||
| } | ||
|
|
||
| if showPaginationSummary { | ||
| endIndex := startIndex + int64(len(eventTypes)) - 1 | ||
| fmt.Printf("\nShowing %d-%d of %d\n", startIndex, endIndex, total) | ||
| } | ||
| } |
There was a problem hiding this comment.
This command prints a custom table via fmt.Printf directly from the root command. In this repo, list-style outputs are typically implemented as views under pkg/views/* using tablelist.NewModel (e.g. pkg/views/logs/view.go) for consistent formatting and behavior. Consider moving the event-types table rendering into a view (and using the shared table component) to keep output consistent and reduce hand-formatted column maintenance.
| // --to-time alone is not allowed; if provided, --from-time must also be present | ||
| if from == "" && to != "" { | ||
| return "", fmt.Errorf("--to-time cannot be used without --from-time") | ||
| } | ||
|
|
||
| // If --from-time is present, use it with either provided --to-time or default to current time | ||
| if from != "" { | ||
| normalizedFrom, err := normalizeAuditTime(from) | ||
| if err != nil { | ||
| return "", fmt.Errorf("invalid --from-time: %w", err) | ||
| } | ||
|
|
||
| normalizedTo := to | ||
| if to == "" { | ||
| normalizedTo = time.Now().Format("2006-01-02 15:04:05") | ||
| } else { |
There was a problem hiding this comment.
PR description says --from-time and --to-time must be provided together, but the implementation allows --from-time without --to-time and defaults the end of the range to time.Now(). Please either enforce the “both together” validation here, or update the PR description/help text/tests to match the intended behavior.
| func normalizeAuditTime(input string) (string, error) { | ||
| layouts := []string{ | ||
| time.RFC3339, | ||
| "2006-01-02 15:04:05", | ||
| } | ||
|
|
||
| for _, layout := range layouts { | ||
| if parsed, err := time.Parse(layout, input); err == nil { | ||
| return parsed.Format("2006-01-02 15:04:05"), nil | ||
| } |
There was a problem hiding this comment.
normalizeAuditTime claims to accept RFC3339, but using only time.RFC3339 rejects valid RFC3339 timestamps that include fractional seconds (e.g. 2025-01-01T01:02:03.123Z). Consider adding time.RFC3339Nano (and a unit test) so valid RFC3339 inputs don’t fail unexpectedly.
Description
This PR improves Harbor CLI audit-log usability by adding:
Audit event-types command
harbor logs eventsConvenience filters for
harbor logs--operation--resource-type--resource--username--from-time--to-timeqquery syntax internally.Query/time validation helpers
--query/-qwith convenience flags.2025-01-01T01:02:03Z)YYYY-MM-DD HH:MM:SS--from-timeand--to-timemust be provided together.Unit tests for new behavior
Parent Issue: #737
Type of Change
Please select the relevant type.
Changes
cmd/harbor/root/logs.gologs eventscommandcmd/harbor/root/logs_test.gobuildAuditLogQueryandnormalizeAuditTime