From db4d64012dfb1df34639c5ee11f575a1f97b4fb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl-Einar=20Bergstr=C3=B8m?= Date: Wed, 14 Oct 2026 21:17:52 +0200 Subject: [PATCH] fix: review findings for lazysecrets list: recent means the last 90 days, cache saves lock and keep deletions and the TUI's cursors, failed lists aren't retried, one expiry rule --- cmd/lazysecrets/list.go | 42 ++++++++++---------- docs/integration.md | 2 +- internal/cachefile/cachefile.go | 58 ++++++++++++++++++++-------- internal/cachefile/cachefile_test.go | 31 +++++++++++++++ internal/cachefile/lock_other.go | 7 ++++ internal/cachefile/lock_unix.go | 25 ++++++++++++ internal/expiry/expiry.go | 41 ++++++++++++++++++++ internal/expiry/expiry_test.go | 27 +++++++++++++ internal/listing/listing.go | 43 +++++++++++++++------ internal/listing/listing_test.go | 36 +++++++++++++++-- internal/store/store.go | 7 ++++ internal/store/store_test.go | 12 ++++++ internal/ui/app/app.go | 13 ++++--- internal/ui/app/cache.go | 15 ++++++- internal/ui/app/expiring.go | 3 +- internal/ui/app/expiry.go | 30 ++++---------- internal/ui/app/favourites.go | 12 +++--- internal/ui/app/fields.go | 3 +- internal/ui/app/jump.go | 2 +- internal/ui/app/persist.go | 12 ++---- internal/ui/app/search.go | 13 ++++--- internal/ui/app/searchview.go | 3 +- internal/ui/app/sources.go | 3 +- 23 files changed, 333 insertions(+), 107 deletions(-) create mode 100644 internal/cachefile/lock_other.go create mode 100644 internal/cachefile/lock_unix.go create mode 100644 internal/expiry/expiry.go create mode 100644 internal/expiry/expiry_test.go diff --git a/cmd/lazysecrets/list.go b/cmd/lazysecrets/list.go index 1d43850..8bc6dc8 100644 --- a/cmd/lazysecrets/list.go +++ b/cmd/lazysecrets/list.go @@ -18,6 +18,7 @@ import ( "charm.land/lipgloss/v2" "git.kebcloud.no/keb/lazysecrets/internal/cachefile" + "git.kebcloud.no/keb/lazysecrets/internal/expiry" "git.kebcloud.no/keb/lazysecrets/internal/listing" "git.kebcloud.no/keb/lazysecrets/internal/provider" "git.kebcloud.no/keb/lazysecrets/internal/store" @@ -127,20 +128,18 @@ func listCmd(providers []provider.Provider, st *store.Store, cachePath string, t if cachePath != "" { f = cachefile.Read(cachePath) } - f = listing.Run(context.Background(), listing.Options{ + f, loaded := listing.Run(context.Background(), listing.Options{ Providers: providers, Cache: f, MaxAge: *maxAge, Cached: *cached, Kinds: only, Timeout: timeout, - Mark: func(link, account string) (bool, time.Time, int, string) { + Mark: func(link, account string) listing.Marks { n := st.Node(link) - return n.StarredFor(account), n.LastUsed, n.Uses, n.Note + return listing.Marks{Starred: n.StarredFor(account), Recent: n.UsedLately(now), + LastUsed: n.LastUsed, Uses: n.Uses, Note: n.Note} }, }, emit) - if cachePath != "" && !*cached { - byID := map[string]provider.Provider{} - for _, p := range providers { - byID[p.ID()] = p - } - keep := func(id string) bool { p, ok := byID[id]; return ok && p.Kind() != provider.KindRepo } - if err := cachefile.Save(cachePath, f, keep, time.Now()); err != nil { + // Only what was loaded: list never moves a cursor, so the TUI's stay. + if cachePath != "" && loaded { + f.Cursor = nil + if err := cachefile.Save(cachePath, f, cachefile.Keep(providers), time.Now()); err != nil { return err } } @@ -170,16 +169,13 @@ func noteOf(e listing.Entry, theme styles.Styles, iconSet icons.Set, icon, color // is "". Its mark (✗, ⚠) is before the name. func expiryOf(e listing.Entry, theme styles.Styles, warn time.Duration, now time.Time, color bool) string { render := renderer(color) - left := e.Expires.Sub(now) - switch { - case e.Expires.IsZero() || left > warn: - return "" - case left <= 0: + switch state, left := expiry.Of(e.Expires, now, warn); state { + case expiry.Expired: return render(theme.Error, "expired") - case left < 24*time.Hour: - return render(theme.Warning, fmt.Sprintf("expires in %dh", max(1, int(left.Hours())))) + case expiry.Soon: + return render(theme.Warning, "expires in "+expiry.Left(left)) } - return render(theme.Warning, fmt.Sprintf("expires in %dd", int(left.Hours()/24))) + return "" } // errorOf is why a list couldn't be read, in red, or "". Its mark (✗) @@ -230,10 +226,14 @@ func (m marks) of(e listing.Entry) string { } } // What's wrong, then what to watch: room for one of them. - switch left := e.Expires.Sub(m.now); { - case e.Error != "" || (m.expiry && !e.Expires.IsZero() && left <= 0): + state := expiry.None + if m.expiry { + state, _ = expiry.Of(e.Expires, m.now, m.warn) + } + switch { + case e.Error != "" || state == expiry.Expired: mark(true, m.theme.Error, "✗") - case m.expiry && !e.Expires.IsZero() && left <= m.warn: + case state == expiry.Soon: mark(true, m.theme.Warning, "⚠") default: mark(false, m.theme.Error, "") diff --git a/docs/integration.md b/docs/integration.md index 8e3098f..79e6ee3 100644 --- a/docs/integration.md +++ b/docs/integration.md @@ -72,7 +72,7 @@ may be added: | `note` | your note on it, if any | | `reference` | how config refers to a secret: `@Microsoft.KeyVault(SecretUri=…)` or `{{secrets/scope/key}}` (`Y` in lazysecrets) | | `url` | its web page, in the Azure portal or the Databricks workspace (`o` in lazysecrets) | -| `recent` | used lately, as Recent (`R`) lists: in the last 90 days, among the 100 used last | +| `recent` | used lately, as Recent (`R`) lists: in the last 90 days | | `lastUsed`, `uses` | when it was last used, and how often; only when `recent`, to sort by | | `expires` | when a secret, key or certificate expires, if it does (Azure) | | `enabled` | `false` for a disabled one; only where the provider says (Azure) | diff --git a/internal/cachefile/cachefile.go b/internal/cachefile/cachefile.go index 7d31792..6f56b82 100644 --- a/internal/cachefile/cachefile.go +++ b/internal/cachefile/cachefile.go @@ -32,6 +32,10 @@ type File struct { Objects map[string]Stamped[[]provider.Object] `json:"objects,omitempty"` Details map[string]Stamped[provider.Object] `json:"details,omitempty"` Cursor map[string]string `json:"cursor,omitempty"` + // Dropped are keys deleted since the file was read (a list that + // failed to load): Save deletes them from the file too, rather than + // merging the old entry back. Not saved. + Dropped map[string]bool `json:"-"` } // Stamped is a list or detail with when it was loaded. @@ -69,6 +73,16 @@ func ProviderOf(key string) string { return id } +// Keep is Save's keep for these providers: their lists are kept, but not +// repos', whose files are read from disk anyway, nor other accounts'. +func Keep(providers []provider.Provider) func(providerID string) bool { + keep := map[string]bool{} + for _, p := range providers { + keep[p.ID()] = p.Kind() != provider.KindRepo + } + return func(id string) bool { return keep[id] } +} + // Read reads path; a missing, unreadable or other version's file is an // empty cache. func Read(path string) File { @@ -90,17 +104,27 @@ func Read(path string) File { } // Save writes f to path, merged with the file there: of a list in both, -// the one loaded last wins, and f's cursors win. Entries older than -// MaxAge, and those of providers keep says no to (accounts signed out -// of), are dropped, from the file's side as well. +// the one loaded last wins, and f's cursors win (a nil Cursor keeps the +// file's). Entries older than MaxAge, those of providers keep says no to +// (accounts signed out of), and f.Dropped are dropped, from the file's +// side as well. Saves from several lazysecrets take turns (a lock file), +// so none undoes another's. func Save(path string, f File, keep func(providerID string) bool, now time.Time) error { + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + return err + } + unlock, err := lock(path + ".lock") + if err != nil { + return err + } + defer unlock() out := New() ok := func(key string, at time.Time) bool { return keep(ProviderOf(key)) && now.Sub(at) < MaxAge } disk := Read(path) - merge(out.Sources, disk.Sources, f.Sources, ok) - merge(out.Containers, disk.Containers, f.Containers, ok) - merge(out.Objects, disk.Objects, f.Objects, ok) - merge(out.Details, disk.Details, f.Details, ok) + merge(out.Sources, disk.Sources, f.Sources, ok, f.Dropped) + merge(out.Containers, disk.Containers, f.Containers, ok, f.Dropped) + merge(out.Objects, disk.Objects, f.Objects, ok, f.Dropped) + merge(out.Details, disk.Details, f.Details, ok, f.Dropped) for _, m := range []map[string]string{disk.Cursor, f.Cursor} { for k, v := range m { if keep(ProviderOf(k)) { @@ -111,12 +135,17 @@ func Save(path string, f File, keep func(providerID string) bool, now time.Time) return write(path, out) } -func merge[T any](out, disk, ours map[string]Stamped[T], ok func(string, time.Time) bool) { - for _, m := range []map[string]Stamped[T]{disk, ours} { - for k, e := range m { - if old, seen := out[k]; ok(k, e.At) && (!seen || !e.At.Before(old.At)) { - out[k] = e - } +// merge puts disk's entries in out, but not those dropped, then ours +// where they're newer. +func merge[T any](out, disk, ours map[string]Stamped[T], ok func(string, time.Time) bool, dropped map[string]bool) { + for k, e := range disk { + if ok(k, e.At) && !dropped[k] { + out[k] = e + } + } + for k, e := range ours { + if old, seen := out[k]; ok(k, e.At) && (!seen || !e.At.Before(old.At)) { + out[k] = e } } } @@ -127,9 +156,6 @@ func write(path string, f File) error { if err != nil { return err } - if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { - return err - } tmp, err := os.CreateTemp(filepath.Dir(path), ".cache-*.json") if err != nil { return err diff --git a/internal/cachefile/cachefile_test.go b/internal/cachefile/cachefile_test.go index 32c5b96..f1e9aad 100644 --- a/internal/cachefile/cachefile_test.go +++ b/internal/cachefile/cachefile_test.go @@ -89,3 +89,34 @@ func TestReadOtherVersion(t *testing.T) { t.Errorf("read another version's file: %+v", got) } } + +func TestSaveDroppedAndCursors(t *testing.T) { + path := filepath.Join(t.TempDir(), "cache.json") + now := time.Now() + all := func(string) bool { return true } + f := New() + f.Containers["az|bad"] = list(now, "kv-old") + f.Cursor["az|x"] = "kv-tui" + if err := Save(path, f, all, now); err != nil { + t.Fatal(err) + } + // The TUI fails to load it again and drops it: it stays gone. + f = New() + f.Dropped = map[string]bool{"az|bad": true} + if err := Save(path, f, all, now); err != nil { + t.Fatal(err) + } + if _, ok := Read(path).Containers["az|bad"]; ok { + t.Error("a dropped list came back") + } + // lazysecrets list saves without cursors: the TUI's stay. + listed := New() + listed.Cursor = nil + listed.Containers["az|y"] = list(now, "kv-y") + if err := Save(path, listed, all, now); err != nil { + t.Fatal(err) + } + if Read(path).Cursor["az|x"] != "kv-tui" { + t.Error("the TUI's cursor was lost") + } +} diff --git a/internal/cachefile/lock_other.go b/internal/cachefile/lock_other.go new file mode 100644 index 0000000..bb597f2 --- /dev/null +++ b/internal/cachefile/lock_other.go @@ -0,0 +1,7 @@ +//go:build !unix + +package cachefile + +// lock does nothing here: saves can then race, losing lists that load +// again. +func lock(string) (func(), error) { return func() {}, nil } diff --git a/internal/cachefile/lock_unix.go b/internal/cachefile/lock_unix.go new file mode 100644 index 0000000..f8281d3 --- /dev/null +++ b/internal/cachefile/lock_unix.go @@ -0,0 +1,25 @@ +//go:build unix + +package cachefile + +import ( + "os" + "syscall" +) + +// lock takes path as a lock file, waiting while another lazysecrets has +// it, and returns how to let go. +func lock(path string) (unlock func(), err error) { + f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + return nil, err + } + if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX); err != nil { + f.Close() + return nil, err + } + return func() { + _ = syscall.Flock(int(f.Fd()), syscall.LOCK_UN) + f.Close() + }, nil +} diff --git a/internal/expiry/expiry.go b/internal/expiry/expiry.go new file mode 100644 index 0000000..720a551 --- /dev/null +++ b/internal/expiry/expiry.go @@ -0,0 +1,41 @@ +// Package expiry says whether a secret, key or certificate has expired +// or will soon, and how long it has left, the same way everywhere: the +// TUI's lists and status pane, and lazysecrets list. +package expiry + +import ( + "fmt" + "time" +) + +type State int + +const ( + None State = iota // no expiry, or not within the warning period + Soon // within the warning period + Expired // already +) + +// Of tells whether something expiring at expires (zero: never) has +// expired by now or will within warn, and how long it has left. +func Of(expires, now time.Time, warn time.Duration) (State, time.Duration) { + if expires.IsZero() { + return None, 0 + } + left := expires.Sub(now) + switch { + case left <= 0: + return Expired, 0 + case left <= warn: + return Soon, left + } + return None, left +} + +// Left is a short "12d", or "5h" on the last day. +func Left(d time.Duration) string { + if d < 24*time.Hour { + return fmt.Sprintf("%dh", max(1, int(d.Hours()))) + } + return fmt.Sprintf("%dd", int(d.Hours()/24)) +} diff --git a/internal/expiry/expiry_test.go b/internal/expiry/expiry_test.go new file mode 100644 index 0000000..92b30a7 --- /dev/null +++ b/internal/expiry/expiry_test.go @@ -0,0 +1,27 @@ +package expiry + +import ( + "testing" + "time" +) + +func TestOf(t *testing.T) { + now := time.Date(2026, 10, 14, 12, 0, 0, 0, time.UTC) + warn := 30 * 24 * time.Hour + for _, c := range []struct { + expires time.Time + want State + left string + }{ + {time.Time{}, None, ""}, + {now.Add(-time.Hour), Expired, ""}, + {now.Add(12*24*time.Hour + time.Hour), Soon, "12d"}, + {now.Add(5 * time.Hour), Soon, "5h"}, + {now.Add(60 * 24 * time.Hour), None, ""}, + } { + state, left := Of(c.expires, now, warn) + if state != c.want || (c.left != "" && Left(left) != c.left) { + t.Errorf("%v: %v %s, want %v %s", c.expires, state, Left(left), c.want, c.left) + } + } +} diff --git a/internal/listing/listing.go b/internal/listing/listing.go index 73d4d2a..8c6f4ff 100644 --- a/internal/listing/listing.go +++ b/internal/listing/listing.go @@ -74,25 +74,37 @@ type Options struct { // Lists only needed for other kinds aren't loaded: sources and vaults // alone don't load any secrets. Lists that fail are always listed. Kinds []string - // Mark tells whether the entry with link is starred for account, - // when it was last used and how often (zero: not lately), and its - // note; nil marks nothing. - Mark func(link, account string) (starred bool, lastUsed time.Time, uses int, note string) - Timeout func(provider.Kind) time.Duration + // Mark tells what's remembered about the entry with link, seen + // through account; nil marks nothing. + Mark func(link, account string) Marks + Timeout func(provider.Kind) time.Duration // nil: 30s Now func() time.Time } +// Marks are what's remembered about an entry: starred for the account, +// used lately (Recent) and when and how often, and its note. +type Marks struct { + Starred, Recent bool + LastUsed time.Time + Uses int + Note string +} + // Run lists everything, calling emit for each entry once, from this // goroutine, and for each list that can't be loaded, as it fails. An // error from emit (the reader went away) stops it. It returns the cache -// with what was loaded. -func Run(ctx context.Context, o Options, emit func(Entry) error) cachefile.File { +// with what was loaded, and whether anything was. +func Run(ctx context.Context, o Options, emit func(Entry) error) (cachefile.File, bool) { ctx, cancel := context.WithCancel(ctx) defer cancel() - l := &lister{o: o, emit: emit, seen: map[string]bool{}, pending: map[string]bool{}, results: make(chan result)} + l := &lister{o: o, emit: emit, seen: map[string]bool{}, pending: map[string]bool{}, failed: map[string]bool{}, + results: make(chan result)} if l.o.Now == nil { l.o.Now = time.Now } + if l.o.Timeout == nil { + l.o.Timeout = func(provider.Kind) time.Duration { return 30 * time.Second } + } for _, p := range o.Providers { if l.stopped != nil { break @@ -111,7 +123,7 @@ func Run(ctx context.Context, o Options, emit func(Entry) error) cachefile.File cancel() } } - return l.o.Cache + return l.o.Cache, l.loaded } type job struct { @@ -135,6 +147,8 @@ type lister struct { emit func(Entry) error seen map[string]bool // provider ID + link of entries emitted pending map[string]bool // cache keys queued or loading + failed map[string]bool // cache keys that couldn't be loaded: not again + loaded bool // something was cached queue []job active int results chan result @@ -240,8 +254,11 @@ func (l *lister) put(p provider.Provider, e Entry) { } e.Account = p.ID() if l.o.Mark != nil && e.Link != "" { - e.Starred, e.LastUsed, e.Uses, e.Note = l.o.Mark(e.Link, e.Account) - e.Recent = !e.LastUsed.IsZero() + m := l.o.Mark(e.Link, e.Account) + e.Starred, e.Recent, e.Note = m.Starred, m.Recent, m.Note + if m.Recent { + e.LastUsed, e.Uses = m.LastUsed, m.Uses + } } id := p.ID() + "\x00" + e.Link + "\x00" + e.Error if l.stopped != nil || l.seen[id] { @@ -265,7 +282,7 @@ func (l *lister) wants(kinds ...string) bool { } func (l *lister) enqueue(j job) { - if l.pending[j.key] { + if l.pending[j.key] || l.failed[j.key] { return } l.pending[j.key] = true @@ -301,9 +318,11 @@ func (l *lister) handle(r result) { delete(l.pending, r.key) now := l.o.Now() if r.err != nil { + l.failed[r.key] = true l.put(r.p, r.failed()) return } + l.loaded = l.loaded || l.cacheable(r.p) cache := l.cacheable(r.p) switch { case r.c != nil: diff --git a/internal/listing/listing_test.go b/internal/listing/listing_test.go index 13167f4..75bdc03 100644 --- a/internal/listing/listing_test.go +++ b/internal/listing/listing_test.go @@ -42,7 +42,7 @@ func run(t *testing.T, o Options) ([]Entry, cachefile.File, []Entry) { } o.Timeout = func(provider.Kind) time.Duration { return 5 * time.Second } var got, failed []Entry - f := Run(context.Background(), o, func(e Entry) error { + f, _ := Run(context.Background(), o, func(e Entry) error { if e.Error != "" { failed = append(failed, e) } else { @@ -234,14 +234,17 @@ func TestListMarks(t *testing.T) { az := fake.Azure(0) var starredLink string got, _, _ := run(t, Options{Providers: []provider.Provider{az}, MaxAge: time.Minute, Kinds: []string{"secret"}, - Mark: func(link, account string) (bool, time.Time, int, string) { + Mark: func(link, account string) Marks { if starredLink == "" && strings.HasSuffix(link, "/db-password") { starredLink = link } if strings.HasSuffix(link, "/api-key") { - return false, time.Now(), 3, "rotate in May" + return Marks{Recent: true, LastUsed: time.Now(), Uses: 3, Note: "rotate in May"} } - return link == starredLink && account == az.ID(), time.Time{}, 0, "" + if strings.HasSuffix(link, "/legacy-token") { // used, but too long ago + return Marks{LastUsed: time.Now().AddDate(-2, 0, 0), Uses: 9} + } + return Marks{Starred: link == starredLink && account == az.ID()} }}) var starred, recent int for _, e := range got { @@ -267,3 +270,28 @@ func TestListMarks(t *testing.T) { t.Errorf("%d starred, %d recent", starred, recent) } } + +func TestFailedListNotRetried(t *testing.T) { + t.Parallel() + az := &counting{Provider: fake.Azure(0)} + _, f, _ := run(t, Options{Providers: []provider.Provider{az}, MaxAge: time.Minute}) + // Everything stale: sources reload, and their vaults are queued again + // as each list arrives. kv-restricted fails once, and isn't asked again. + before := az.calls.Load() + _, _, failed := run(t, Options{Providers: []provider.Provider{az}, Cache: f, MaxAge: time.Minute, + Now: func() time.Time { return time.Now().Add(time.Hour) }}) + if len(failed) != 1 { + t.Errorf("failed: %+v", failed) + } + fresh := &counting{Provider: fake.Azure(0)} + run(t, Options{Providers: []provider.Provider{fresh}, MaxAge: time.Minute}) + if n := az.calls.Load() - before; n > fresh.calls.Load() { + t.Errorf("%d requests with a stale cache, more than %d from scratch: something was asked twice", n, fresh.calls.Load()) + } +} + +func TestRunWithoutTimeout(t *testing.T) { + t.Parallel() + Run(context.Background(), Options{Providers: []provider.Provider{fake.Repos()}, Cache: cachefile.New()}, + func(Entry) error { return nil }) // no Timeout: a default, not a panic +} diff --git a/internal/store/store.go b/internal/store/store.go index 04f708a..77689dc 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -43,6 +43,13 @@ type Node struct { // alone isn't. func (n Node) keep() bool { return len(n.StarredBy) > 0 || n.Note != "" || n.Uses > 0 } +// UsedLately reports whether the item was used within the last 90 days +// (what Recent lists). Starred and noted records are kept longer, so a +// use alone doesn't say. +func (n Node) UsedLately(now time.Time) bool { + return !n.LastUsed.IsZero() && now.Sub(n.LastUsed) <= usageMaxAge +} + // StarredFor reports whether the item is starred for account. func (n Node) StarredFor(account string) bool { return slices.Contains(n.StarredBy, account) } diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 0be187c..065e822 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -119,3 +119,15 @@ func TestStarsPerAccount(t *testing.T) { t.Errorf("unstar alice: %v", n.StarredBy) } } + +func TestUsedLately(t *testing.T) { + now := time.Now() + for ago, want := range map[time.Duration]bool{time.Hour: true, 89 * 24 * time.Hour: true, 2 * 365 * 24 * time.Hour: false} { + if got := (Node{LastUsed: now.Add(-ago)}).UsedLately(now); got != want { + t.Errorf("used %v ago: %v", ago, got) + } + } + if (Node{}).UsedLately(now) { + t.Error("never used is lately") + } +} diff --git a/internal/ui/app/app.go b/internal/ui/app/app.go index 5447318..29c9f50 100644 --- a/internal/ui/app/app.go +++ b/internal/ui/app/app.go @@ -213,9 +213,12 @@ type Model struct { statusW int // the status pane's width: the left column's, zoomed or not changelog []panes.Field cacheDirty bool - sourcesAt map[string]time.Time - placeDir string - savedPlace string // the link last saved as the place + // cacheDropped are cache keys deleted since the last save, so saving + // deletes them from the file too. + cacheDropped map[string]bool + sourcesAt map[string]time.Time + placeDir string + savedPlace string // the link last saved as the place // A pending clear of a copied value, counted down in the footer. // clipToken identifies the latest copy so older ticks are ignored. @@ -688,7 +691,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if src, ok := m.selectedSource(); ok { m.logErrOf(src.Name, msg.err) } - delete(m.cache.containers, msg.key) + uncache(&m, m.cache.containers, msg.key) m.containers.SetError(msg.err) } else { m.cache.containers[msg.key] = entry[[]provider.Container]{msg.items, m.now()} @@ -709,7 +712,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if c, ok := m.containers.Selected(); ok { m.logErrOf(c.Name, msg.err) } - delete(m.cache.objects, msg.key) + uncache(&m, m.cache.objects, msg.key) m.secrets.SetError(msg.err) } else { if msg.bound != nil { diff --git a/internal/ui/app/cache.go b/internal/ui/app/cache.go index 2de8090..69d6192 100644 --- a/internal/ui/app/cache.go +++ b/internal/ui/app/cache.go @@ -67,10 +67,23 @@ func (m *Model) refreshShown(before []provider.Object) tea.Cmd { return nil } m.detail.Refresh(s) - delete(m.cache.details, detailKey(s)) + uncache(m, m.cache.details, detailKey(s)) return m.describe(s) } +// uncache deletes key from a cache, and, at the next save, from the file. +func uncache[T any](m *Model, c map[string]entry[T], key string) { + if _, ok := c[key]; !ok { + return + } + delete(c, key) + if m.cacheDropped == nil { + m.cacheDropped = map[string]bool{} + } + m.cacheDropped[key] = true + m.cacheDirty = true +} + // sameListing reports whether a and b list the same: what a list // says of an object, not what describing it adds. func sameListing(a, b provider.Object) bool { diff --git a/internal/ui/app/expiring.go b/internal/ui/app/expiring.go index dea864f..acdc123 100644 --- a/internal/ui/app/expiring.go +++ b/internal/ui/app/expiring.go @@ -5,6 +5,7 @@ import ( "fmt" "slices" + "git.kebcloud.no/keb/lazysecrets/internal/cachefile" "git.kebcloud.no/keb/lazysecrets/internal/provider" "git.kebcloud.no/keb/lazysecrets/internal/ref" "git.kebcloud.no/keb/lazysecrets/internal/ui/panes" @@ -28,7 +29,7 @@ func (m Model) expiringItems() []expiringItem { row := sourceRowKey(p.ID(), src.ID) for _, c := range m.cache.containers[row].value { for _, t := range c.ObjectTypes() { - for _, o := range m.cache.objects[containerKey(c)+"|"+string(t)].value { + for _, o := range m.cache.objects[cachefile.ObjectsKey(c, t)].value { if state, _ := m.expiry(o); state != notExpiring { out = append(out, expiringItem{o, row}) } diff --git a/internal/ui/app/expiry.go b/internal/ui/app/expiry.go index 2d25440..afc6d93 100644 --- a/internal/ui/app/expiry.go +++ b/internal/ui/app/expiry.go @@ -1,9 +1,10 @@ package app import ( - "fmt" "time" + "git.kebcloud.no/keb/lazysecrets/internal/expiry" + "git.kebcloud.no/keb/lazysecrets/internal/provider" ) @@ -11,28 +12,18 @@ import ( // list once they have expired (✗ expired) or will within the configured // warning period (⚠ 12d), and typing "expir" in the filter finds them. -type expiryState int +type expiryState = expiry.State const ( - notExpiring expiryState = iota - expiresSoon - expired + notExpiring = expiry.None + expiresSoon = expiry.Soon + expired = expiry.Expired ) // expiry tells whether o has expired or will within the warning period, // and how long it has left. func (m Model) expiry(o provider.Object) (expiryState, time.Duration) { - if o.Expires.IsZero() { - return notExpiring, 0 - } - left := o.Expires.Sub(m.now()) - switch { - case left <= 0: - return expired, 0 - case left <= m.expiryWarning: - return expiresSoon, left - } - return notExpiring, left + return expiry.Of(o.Expires, m.now(), m.expiryWarning) } // expiryWord is matched by the filter: "expired" or "expiring". @@ -47,9 +38,4 @@ func (m Model) expiryWord(o provider.Object) string { } // daysLeft is a short "12d", or "5h" on the last day. -func daysLeft(d time.Duration) string { - if d < 24*time.Hour { - return fmt.Sprintf("%dh", max(1, int(d.Hours()))) - } - return fmt.Sprintf("%dd", int(d.Hours()/24)) -} +func daysLeft(d time.Duration) string { return expiry.Left(d) } diff --git a/internal/ui/app/favourites.go b/internal/ui/app/favourites.go index e9f207b..4932634 100644 --- a/internal/ui/app/favourites.go +++ b/internal/ui/app/favourites.go @@ -3,9 +3,11 @@ package app import ( "slices" "strings" + "time" tea "charm.land/bubbletea/v2" + "git.kebcloud.no/keb/lazysecrets/internal/cachefile" "git.kebcloud.no/keb/lazysecrets/internal/provider" "git.kebcloud.no/keb/lazysecrets/internal/ref" "git.kebcloud.no/keb/lazysecrets/internal/store" @@ -24,12 +26,12 @@ const recentMark = "◷ " // marks is what goes before a list row's name: ★ when it's starred for // account and, with recent, ◷ when it was used lately. -func marks(st styles.Styles, n store.Node, account string, recent bool) string { +func marks(st styles.Styles, n store.Node, account string, recent bool, now time.Time) string { var s string if n.StarredFor(account) { s = st.Favourite.Render(star) } - if recent && !n.LastUsed.IsZero() { + if recent && n.UsedLately(now) { s += st.Muted.Render(recentMark) } return s @@ -110,7 +112,7 @@ func (m Model) containerRender(withSource bool) func(styles.Styles, provider.Con } line += sty.Muted.Render(" " + c.Detail) } - line = marks(sty, n, c.Source.Provider, m.recentIcon) + line + line = marks(sty, n, c.Source.Provider, m.recentIcon, m.now()) + line if withSource { src := c.Source.Name if account, ok := strings.CutPrefix(c.Source.ProviderLabel, string(c.Source.Kind)+" · "); ok { @@ -130,7 +132,7 @@ func (m Model) objectRender() func(styles.Styles, provider.Object, int) string { if o.Enabled != nil && !*o.Enabled { line = sty.Muted.Render(o.Name + " (disabled)") } - line = marks(sty, n, o.Container.Source.Provider, m.recentIcon) + line + line = marks(sty, n, o.Container.Source.Provider, m.recentIcon, m.now()) + line switch state, left := m.expiry(o); state { case expired: line += " " + sty.Error.Render("✗ expired") @@ -179,7 +181,7 @@ func (m *Model) rebind(c provider.Container) string { m.linked[i].Source = c.Source } } - key := containerKey(c) + "|" + string(m.objType) + key := cachefile.ObjectsKey(c, m.objType) if m.onLinks() { m.containers.SetItems(m.linked) m.containers.Select(containerKey(c)) diff --git a/internal/ui/app/fields.go b/internal/ui/app/fields.go index 4b6b2c8..71b3610 100644 --- a/internal/ui/app/fields.go +++ b/internal/ui/app/fields.go @@ -8,6 +8,7 @@ import ( tea "charm.land/bubbletea/v2" "github.com/charmbracelet/x/ansi" + "git.kebcloud.no/keb/lazysecrets/internal/cachefile" "git.kebcloud.no/keb/lazysecrets/internal/provider" "git.kebcloud.no/keb/lazysecrets/internal/ui/panes" ) @@ -160,7 +161,7 @@ func (m Model) containerFields(c provider.Container) []panes.Field { add("source", cmpOr(c.Source.ProviderLabel, string(c.Source.Kind))+" / "+c.Source.Name) add("web", c.URL) for _, t := range c.ObjectTypes() { - if e, ok := m.cache.objects[containerKey(c)+"|"+string(t)]; ok { + if e, ok := m.cache.objects[cachefile.ObjectsKey(c, t)]; ok { add(strings.ToLower(string(t))+"s", fmt.Sprint(len(e.value))) } } diff --git a/internal/ui/app/jump.go b/internal/ui/app/jump.go index 905cbec..066c841 100644 --- a/internal/ui/app/jump.go +++ b/internal/ui/app/jump.go @@ -107,7 +107,7 @@ func (m *Model) storeEntries() (favs, recent []option) { if m.history { var used []string for _, k := range keys { - if !nodes[k].LastUsed.IsZero() { + if nodes[k].UsedLately(m.now()) { used = append(used, k) } } diff --git a/internal/ui/app/persist.go b/internal/ui/app/persist.go index b2b1117..1fab15f 100644 --- a/internal/ui/app/persist.go +++ b/internal/ui/app/persist.go @@ -33,14 +33,7 @@ type persistedMsg struct{ err error } // isn't a repo: every key starts with the provider ID. func (m *Model) keepCached(key string) bool { return m.keepProvider(cachefile.ProviderOf(key)) } -func (m *Model) keepProvider(id string) bool { return keepIn(m.byID)(id) } - -func keepIn(byID map[string]provider.Provider) func(id string) bool { - return func(id string) bool { - p, ok := byID[id] - return ok && p.Kind() != provider.KindRepo - } -} +func (m *Model) keepProvider(id string) bool { return cachefile.Keep(m.providers)(id) } // loadCacheFile fills the cache from the file, leaving out what's too // old or belongs to accounts that aren't signed in any more. @@ -79,6 +72,7 @@ func (m *Model) loadCacheFile() { // cacheSnapshot is the cache as the file holds it. func (m *Model) cacheSnapshot() cachefile.File { f := cachefile.New() + f.Dropped, m.cacheDropped = m.cacheDropped, nil for id, r := range m.sourceResults { if r.err == nil && m.keepProvider(id) { f.Sources[id] = cachefile.Stamped[[]provider.Source]{At: m.sourcesAt[id], Value: r.sources} @@ -110,7 +104,7 @@ func (m *Model) cacheSnapshot() cachefile.File { // saveCache saves f, merged with the file (lazysecrets list may have // loaded more since), from any goroutine. func (m *Model) saveCache(f cachefile.File) func() error { - path, keep, now := m.cachePath, keepIn(m.byID), m.now() + path, keep, now := m.cachePath, cachefile.Keep(m.providers), m.now() return func() error { return cachefile.Save(path, f, keep, now) } } diff --git a/internal/ui/app/search.go b/internal/ui/app/search.go index 3a8ec03..7648853 100644 --- a/internal/ui/app/search.go +++ b/internal/ui/app/search.go @@ -10,6 +10,7 @@ import ( tea "charm.land/bubbletea/v2" + "git.kebcloud.no/keb/lazysecrets/internal/cachefile" "git.kebcloud.no/keb/lazysecrets/internal/provider" "git.kebcloud.no/keb/lazysecrets/internal/ref" ) @@ -89,7 +90,7 @@ func (m *Model) searchEntries() (sources, items []option) { return nil }, reload) for _, t := range []provider.ObjectType{provider.TypeSecret, provider.TypeKey, provider.TypeCertificate} { - for _, o := range m.cache.objects[containerKey(c)+"|"+string(t)].value { + for _, o := range m.cache.objects[cachefile.ObjectsKey(c, t)].value { r, err := ref.ForObject(o, "") if err != nil { continue @@ -166,7 +167,7 @@ func (m *Model) coverageOf(in providerFilter) coverage { } for _, v := range e.value { c.vaults++ - key := containerKey(v) + "|" + string(provider.TypeSecret) + key := cachefile.ObjectsKey(v, provider.TypeSecret) if objs, ok := m.cache.objects[key]; ok { c.vaultsLoaded++ c.secrets += len(objs.value) @@ -263,7 +264,7 @@ func (m *Model) queueSource(src provider.Source, force bool) { // queueContainer queues loading c's secrets, unless loaded lately and not // force. func (m *Model) queueContainer(c provider.Container, row string, force bool) { - key := containerKey(c) + "|" + string(provider.TypeSecret) + key := cachefile.ObjectsKey(c, provider.TypeSecret) if e, ok := m.cache.objects[key]; ok && e.fresh(m.now()) && !force { return } @@ -327,7 +328,7 @@ func (m *Model) handleCrawl(msg crawlMsg) tea.Cmd { m.crawl.active-- key := msg.job.row if msg.job.container != nil { - key = containerKey(*msg.job.container) + "|" + string(provider.TypeSecret) + key = cachefile.ObjectsKey(*msg.job.container, provider.TypeSecret) } delete(m.crawl.pending, key) switch { @@ -365,7 +366,7 @@ func (m *Model) sourceMark(row string) string { } loaded, loading := 0, false for _, c := range e.value { - key := containerKey(c) + "|" + string(provider.TypeSecret) + key := cachefile.ObjectsKey(c, provider.TypeSecret) loading = loading || m.crawl.pending[key] if _, ok := m.cache.objects[key]; ok || m.crawl.failed[key] { loaded++ @@ -386,7 +387,7 @@ func (m *Model) sourceMark(row string) string { // (✗) or are loading (◌): whether a subscription is loaded says whether // its vaults are searchable. func (m *Model) vaultMark(c provider.Container) string { - key := containerKey(c) + "|" + string(provider.TypeSecret) + key := cachefile.ObjectsKey(c, provider.TypeSecret) switch { case m.crawl.pending[key]: return m.st.Muted.Render("◌") diff --git a/internal/ui/app/searchview.go b/internal/ui/app/searchview.go index 4a467ad..8f7b469 100644 --- a/internal/ui/app/searchview.go +++ b/internal/ui/app/searchview.go @@ -12,6 +12,7 @@ import ( "charm.land/lipgloss/v2" "github.com/charmbracelet/x/ansi" + "git.kebcloud.no/keb/lazysecrets/internal/cachefile" "git.kebcloud.no/keb/lazysecrets/internal/match" "git.kebcloud.no/keb/lazysecrets/internal/provider" "git.kebcloud.no/keb/lazysecrets/internal/ref" @@ -617,7 +618,7 @@ func (m Model) loadHint() string { } loaded := 0 for _, c := range e.value { - key := containerKey(c) + "|" + string(provider.TypeSecret) + key := cachefile.ObjectsKey(c, provider.TypeSecret) if _, ok := m.cache.objects[key]; ok || m.crawl.failed[key] { loaded++ } diff --git a/internal/ui/app/sources.go b/internal/ui/app/sources.go index 4e45209..e1b3bd4 100644 --- a/internal/ui/app/sources.go +++ b/internal/ui/app/sources.go @@ -5,6 +5,7 @@ import ( "fmt" "slices" "strings" + "time" "git.kebcloud.no/keb/lazysecrets/internal/cachefile" "git.kebcloud.no/keb/lazysecrets/internal/provider" @@ -80,7 +81,7 @@ func newSourcesList(iconSet icons.Set, mem *store.Store, recent bool) panes.List kind := st.Kind(r.group) starred := "" if r.kind == rowSource { - starred = marks(st, node, r.source.Provider, recent) + starred = marks(st, node, r.source.Provider, recent, time.Now()) } switch { case r.kind == rowLinks: