From af3aaa0397996d6431facebfbc7497af23e34e80 Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Fri, 14 Jun 2024 14:45:48 -0400 Subject: [PATCH] fix: make caching options more explicit (#2966) Signed-off-by: Keith Zantow --- cmd/syft/internal/options/cache.go | 49 +++++---- cmd/syft/internal/options/cache_test.go | 129 ++++++++++++++++++++++++ internal/cache/cache.go | 3 +- internal/cache/cache_test.go | 11 ++ internal/cache/memory.go | 3 + 5 files changed, 174 insertions(+), 21 deletions(-) diff --git a/cmd/syft/internal/options/cache.go b/cmd/syft/internal/options/cache.go index d8e4130a2..a8ee1c55f 100644 --- a/cmd/syft/internal/options/cache.go +++ b/cmd/syft/internal/options/cache.go @@ -24,29 +24,37 @@ type Cache struct { } func (c *Cache) DescribeFields(descriptions clio.FieldDescriptionSet) { - descriptions.Add(&c.Dir, "root directory to cache any downloaded content") - descriptions.Add(&c.TTL, "time to live for cached data") + descriptions.Add(&c.Dir, "root directory to cache any downloaded content; empty string will use an in-memory cache") + descriptions.Add(&c.TTL, "time to live for cached data; setting this to 0 will disable caching entirely") } func (c *Cache) PostLoad() error { - if c.Dir != "" { - ttl, err := parseDuration(c.TTL) + ttl, err := parseDuration(c.TTL) + if err != nil { + log.Warnf("unable to parse duration '%v', using default (%s) due to: %v", c.TTL, durationToString(defaultTTL()), err) + ttl = defaultTTL() + } + // if TTL is <= 0, disable caching entirely + if ttl <= 0 { + cache.SetManager(nil) + return nil + } + // if dir == "" but we have a TTL, use an in-memory cache + if c.Dir == "" { + cache.SetManager(cache.NewInMemory(ttl)) + return nil + } + dir, err := homedir.Expand(c.Dir) + if err != nil { + log.Warnf("unable to expand cache directory %s: %v", c.Dir, err) + cache.SetManager(cache.NewInMemory(ttl)) + } else { + m, err := cache.NewFromDir(dir, ttl) if err != nil { - log.Warnf("unable to parse duration '%v', using default (%s) due to: %v", c.TTL, durationToString(defaultTTL()), err) - ttl = defaultTTL() - } - dir, err := homedir.Expand(c.Dir) - if err != nil { - log.Warnf("unable to expand cache directory %s: %v", c.Dir, err) + log.Warnf("unable to get filesystem cache at %s: %v", c.Dir, err) cache.SetManager(cache.NewInMemory(ttl)) } else { - m, err := cache.NewFromDir(dir, ttl) - if err != nil { - log.Warnf("unable to get filesystem cache at %s: %v", c.Dir, err) - cache.SetManager(cache.NewInMemory(ttl)) - } else { - cache.SetManager(m) - } + cache.SetManager(m) } } return nil @@ -100,9 +108,8 @@ func durationToString(duration time.Duration) string { return out } -var whitespace = regexp.MustCompile(`\s+`) - func parseDuration(duration string) (time.Duration, error) { + whitespace := regexp.MustCompile(`\s+`) duration = strings.ToLower(whitespace.ReplaceAllString(duration, "")) parts := strings.SplitN(duration, "d", 2) var days time.Duration @@ -114,7 +121,9 @@ func parseDuration(duration string) (time.Duration, error) { return 0, daysErr } days = time.Duration(numDays) * 24 * time.Hour - remain, err = time.ParseDuration(parts[1]) + if len(parts[1]) > 0 { + remain, err = time.ParseDuration(parts[1]) + } } else { remain, err = time.ParseDuration(duration) } diff --git a/cmd/syft/internal/options/cache_test.go b/cmd/syft/internal/options/cache_test.go index e9bf92087..e4d527d3a 100644 --- a/cmd/syft/internal/options/cache_test.go +++ b/cmd/syft/internal/options/cache_test.go @@ -1,6 +1,7 @@ package options import ( + "io" "os" "path/filepath" "strings" @@ -10,6 +11,9 @@ import ( "github.com/adrg/xdg" "github.com/mitchellh/go-homedir" "github.com/stretchr/testify/require" + + "github.com/anchore/syft/internal" + "github.com/anchore/syft/internal/cache" ) func Test_defaultDir(t *testing.T) { @@ -91,12 +95,135 @@ func Test_defaultDir(t *testing.T) { } } +func Test_cacheOptions(t *testing.T) { + tmp := t.TempDir() + tests := []struct { + name string + opts Cache + test func(t *testing.T) + }{ + { + name: "disable-1", + opts: Cache{ + Dir: "some-dir", + TTL: "0", + }, + test: func(t *testing.T) { + c := cache.GetManager().GetCache("test-disable-1", "v-disable-1") + err := c.Write("key-disable-1", strings.NewReader("some-value-disable-1")) + require.NoError(t, err) + rdr, err := c.Read("key-disable-1") + require.Nil(t, rdr) + require.Error(t, err) + }, + }, + { + name: "disable-2", + opts: Cache{ + Dir: "some-dir", + TTL: "0s", + }, + test: func(t *testing.T) { + c := cache.GetManager().GetCache("test-disable-2", "v-disable-2") + err := c.Write("key-disable-2", strings.NewReader("some-value-disable-2")) + require.NoError(t, err) + rdr, err := c.Read("key-disable-2") + require.Nil(t, rdr) + require.Error(t, err) + }, + }, + + { + name: "disable-3", + opts: Cache{ + Dir: "some-dir", + TTL: "0d", + }, + test: func(t *testing.T) { + c := cache.GetManager().GetCache("test-disable-3", "v-disable-3") + err := c.Write("key-disable-3", strings.NewReader("some-value-disable-3")) + require.NoError(t, err) + rdr, err := c.Read("key-disable-3") + require.Nil(t, rdr) + require.Error(t, err) + }, + }, + { + name: "in-memory", + opts: Cache{ + Dir: "", + TTL: "10m", + }, + test: func(t *testing.T) { + c := cache.GetManager().GetCache("test-mem", "v-mem") + err := c.Write("key-mem", strings.NewReader("some-value-mem")) + require.NoError(t, err) + rdr, err := c.Read("key-mem") + require.NotNil(t, rdr) + defer internal.CloseAndLogError(rdr, "") + require.NoError(t, err) + data, err := io.ReadAll(rdr) + require.NoError(t, err) + require.Equal(t, "some-value-mem", string(data)) + require.NoDirExists(t, filepath.Join("test-mem", "v-mem")) + }, + }, + { + name: "on disk", + opts: Cache{ + Dir: tmp, + TTL: "10m", + }, + test: func(t *testing.T) { + c := cache.GetManager().GetCache("test-disk", "v-disk") + err := c.Write("key-disk", strings.NewReader("some-value-disk")) + require.NoError(t, err) + rdr, err := c.Read("key-disk") + require.NotNil(t, rdr) + defer internal.CloseAndLogError(rdr, "") + require.NoError(t, err) + data, err := io.ReadAll(rdr) + require.NoError(t, err) + require.Equal(t, "some-value-disk", string(data)) + require.DirExists(t, filepath.Join(tmp, "test-disk", "v-disk")) + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + original := cache.GetManager() + defer cache.SetManager(original) + + err := test.opts.PostLoad() + require.NoError(t, err) + + test.test(t) + }) + } +} + func Test_parseDuration(t *testing.T) { tests := []struct { duration string expect time.Duration err require.ErrorAssertionFunc }{ + { + duration: "0d", + expect: 0, + }, + { + duration: "0m", + expect: 0, + }, + { + duration: "0s", + expect: 0, + }, + { + duration: "0", + expect: 0, + }, { duration: "1d", expect: 24 * time.Hour, @@ -141,6 +268,8 @@ func Test_parseDuration(t *testing.T) { if test.err != nil { test.err(t, err) return + } else { + require.NoError(t, err) } require.Equal(t, test.expect, got) }) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index 02208504e..3ca2ece98 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -36,7 +36,8 @@ func GetManager() Manager { return manager } -// SetManager sets the global cache manager, which is used to instantiate all caches +// SetManager sets the global cache manager, which is used to instantiate all caches. +// Setting this to nil disables caching. func SetManager(m Manager) { if m == nil { manager = &bypassedCache{} diff --git a/internal/cache/cache_test.go b/internal/cache/cache_test.go index 548a4b03b..2fceda3af 100644 --- a/internal/cache/cache_test.go +++ b/internal/cache/cache_test.go @@ -2,6 +2,7 @@ package cache import ( "testing" + "time" "github.com/stretchr/testify/require" ) @@ -10,8 +11,18 @@ func Test_SetManager(t *testing.T) { original := GetManager() defer SetManager(original) + SetManager(nil) + + require.NotNil(t, GetManager()) + require.IsType(t, &bypassedCache{}, GetManager()) + SetManager(NewInMemory(0)) + require.NotNil(t, GetManager()) + require.IsType(t, &bypassedCache{}, GetManager()) + + SetManager(NewInMemory(1 * time.Hour)) + require.NotNil(t, GetManager()) require.IsType(t, &filesystemCache{}, GetManager()) diff --git a/internal/cache/memory.go b/internal/cache/memory.go index 247e67e0d..696f8c9be 100644 --- a/internal/cache/memory.go +++ b/internal/cache/memory.go @@ -8,6 +8,9 @@ import ( // NewInMemory returns an in-memory only cache manager func NewInMemory(ttl time.Duration) Manager { + if ttl <= 0 { + return &bypassedCache{} + } return &filesystemCache{ dir: "", fs: afero.NewMemMapFs(),