fix: make caching options more explicit (#2966)

Signed-off-by: Keith Zantow <kzantow@gmail.com>
This commit is contained in:
Keith Zantow 2024-06-14 14:45:48 -04:00 committed by GitHub
parent 70098e20bb
commit af3aaa0397
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 174 additions and 21 deletions

View File

@ -24,17 +24,26 @@ type Cache struct {
} }
func (c *Cache) DescribeFields(descriptions clio.FieldDescriptionSet) { func (c *Cache) DescribeFields(descriptions clio.FieldDescriptionSet) {
descriptions.Add(&c.Dir, "root directory to cache any downloaded content") 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") descriptions.Add(&c.TTL, "time to live for cached data; setting this to 0 will disable caching entirely")
} }
func (c *Cache) PostLoad() error { func (c *Cache) PostLoad() error {
if c.Dir != "" {
ttl, err := parseDuration(c.TTL) ttl, err := parseDuration(c.TTL)
if err != nil { if err != nil {
log.Warnf("unable to parse duration '%v', using default (%s) due to: %v", c.TTL, durationToString(defaultTTL()), err) log.Warnf("unable to parse duration '%v', using default (%s) due to: %v", c.TTL, durationToString(defaultTTL()), err)
ttl = defaultTTL() 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) dir, err := homedir.Expand(c.Dir)
if err != nil { if err != nil {
log.Warnf("unable to expand cache directory %s: %v", c.Dir, err) log.Warnf("unable to expand cache directory %s: %v", c.Dir, err)
@ -48,7 +57,6 @@ func (c *Cache) PostLoad() error {
cache.SetManager(m) cache.SetManager(m)
} }
} }
}
return nil return nil
} }
@ -100,9 +108,8 @@ func durationToString(duration time.Duration) string {
return out return out
} }
var whitespace = regexp.MustCompile(`\s+`)
func parseDuration(duration string) (time.Duration, error) { func parseDuration(duration string) (time.Duration, error) {
whitespace := regexp.MustCompile(`\s+`)
duration = strings.ToLower(whitespace.ReplaceAllString(duration, "")) duration = strings.ToLower(whitespace.ReplaceAllString(duration, ""))
parts := strings.SplitN(duration, "d", 2) parts := strings.SplitN(duration, "d", 2)
var days time.Duration var days time.Duration
@ -114,7 +121,9 @@ func parseDuration(duration string) (time.Duration, error) {
return 0, daysErr return 0, daysErr
} }
days = time.Duration(numDays) * 24 * time.Hour days = time.Duration(numDays) * 24 * time.Hour
if len(parts[1]) > 0 {
remain, err = time.ParseDuration(parts[1]) remain, err = time.ParseDuration(parts[1])
}
} else { } else {
remain, err = time.ParseDuration(duration) remain, err = time.ParseDuration(duration)
} }

View File

@ -1,6 +1,7 @@
package options package options
import ( import (
"io"
"os" "os"
"path/filepath" "path/filepath"
"strings" "strings"
@ -10,6 +11,9 @@ import (
"github.com/adrg/xdg" "github.com/adrg/xdg"
"github.com/mitchellh/go-homedir" "github.com/mitchellh/go-homedir"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/anchore/syft/internal"
"github.com/anchore/syft/internal/cache"
) )
func Test_defaultDir(t *testing.T) { 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) { func Test_parseDuration(t *testing.T) {
tests := []struct { tests := []struct {
duration string duration string
expect time.Duration expect time.Duration
err require.ErrorAssertionFunc err require.ErrorAssertionFunc
}{ }{
{
duration: "0d",
expect: 0,
},
{
duration: "0m",
expect: 0,
},
{
duration: "0s",
expect: 0,
},
{
duration: "0",
expect: 0,
},
{ {
duration: "1d", duration: "1d",
expect: 24 * time.Hour, expect: 24 * time.Hour,
@ -141,6 +268,8 @@ func Test_parseDuration(t *testing.T) {
if test.err != nil { if test.err != nil {
test.err(t, err) test.err(t, err)
return return
} else {
require.NoError(t, err)
} }
require.Equal(t, test.expect, got) require.Equal(t, test.expect, got)
}) })

View File

@ -36,7 +36,8 @@ func GetManager() Manager {
return 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) { func SetManager(m Manager) {
if m == nil { if m == nil {
manager = &bypassedCache{} manager = &bypassedCache{}

View File

@ -2,6 +2,7 @@ package cache
import ( import (
"testing" "testing"
"time"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
@ -10,8 +11,18 @@ func Test_SetManager(t *testing.T) {
original := GetManager() original := GetManager()
defer SetManager(original) defer SetManager(original)
SetManager(nil)
require.NotNil(t, GetManager())
require.IsType(t, &bypassedCache{}, GetManager())
SetManager(NewInMemory(0)) SetManager(NewInMemory(0))
require.NotNil(t, GetManager())
require.IsType(t, &bypassedCache{}, GetManager())
SetManager(NewInMemory(1 * time.Hour))
require.NotNil(t, GetManager()) require.NotNil(t, GetManager())
require.IsType(t, &filesystemCache{}, GetManager()) require.IsType(t, &filesystemCache{}, GetManager())

View File

@ -8,6 +8,9 @@ import (
// NewInMemory returns an in-memory only cache manager // NewInMemory returns an in-memory only cache manager
func NewInMemory(ttl time.Duration) Manager { func NewInMemory(ttl time.Duration) Manager {
if ttl <= 0 {
return &bypassedCache{}
}
return &filesystemCache{ return &filesystemCache{
dir: "", dir: "",
fs: afero.NewMemMapFs(), fs: afero.NewMemMapFs(),