From 7cc93265ef9be9d0eb4ccf50561ee87df31bb45a Mon Sep 17 00:00:00 2001 From: Jarek Kowalski Date: Sat, 3 Dec 2022 10:47:01 -0800 Subject: [PATCH] refactor(repository): moved cache protection to separate package (#2621) --- internal/cache/content_cache.go | 3 ++- internal/cache/persistent_lru_cache.go | 9 +++++---- internal/cache/persistent_lru_cache_test.go | 17 +++++++++-------- .../{cache => cacheprot}/storage_protection.go | 3 ++- .../storage_protection_test.go | 12 ++++++------ repo/content/committed_read_manager.go | 3 ++- repo/open.go | 3 ++- 7 files changed, 28 insertions(+), 22 deletions(-) rename internal/{cache => cacheprot}/storage_protection.go (97%) rename internal/{cache => cacheprot}/storage_protection_test.go (76%) diff --git a/internal/cache/content_cache.go b/internal/cache/content_cache.go index 74b5a5f04..c3d936eb0 100644 --- a/internal/cache/content_cache.go +++ b/internal/cache/content_cache.go @@ -5,6 +5,7 @@ "github.com/pkg/errors" + "github.com/kopia/kopia/internal/cacheprot" "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/impossible" "github.com/kopia/kopia/internal/metrics" @@ -189,7 +190,7 @@ func NewContentCache(ctx context.Context, st blob.Storage, opt Options, mr *metr } } - pc, err := NewPersistentCache(ctx, opt.CacheSubDir, cacheStorage, ChecksumProtection(opt.HMACSecret), opt.Sweep, mr) + pc, err := NewPersistentCache(ctx, opt.CacheSubDir, cacheStorage, cacheprot.ChecksumProtection(opt.HMACSecret), opt.Sweep, mr) if err != nil { return nil, errors.Wrap(err, "unable to create base cache") } diff --git a/internal/cache/persistent_lru_cache.go b/internal/cache/persistent_lru_cache.go index 6b539fabd..09d0f8afa 100644 --- a/internal/cache/persistent_lru_cache.go +++ b/internal/cache/persistent_lru_cache.go @@ -11,6 +11,7 @@ lru "github.com/hashicorp/golang-lru" "github.com/pkg/errors" + "github.com/kopia/kopia/internal/cacheprot" "github.com/kopia/kopia/internal/clock" "github.com/kopia/kopia/internal/ctxutil" "github.com/kopia/kopia/internal/gather" @@ -42,7 +43,7 @@ type PersistentCache struct { anyChange atomic.Bool cacheStorage Storage - storageProtection StorageProtection + storageProtection cacheprot.StorageProtection sweep SweepSettings description string @@ -137,7 +138,7 @@ func (c *PersistentCache) GetPartial(ctx context.Context, key string, offset, le prot := c.storageProtection if length >= 0 { // only full items have protection. - prot = nullStorageProtection{} + prot = cacheprot.NoProtection() } if err := prot.Verify(key, tmp.Bytes(), output); err == nil { @@ -333,7 +334,7 @@ func (s SweepSettings) applyDefaults() SweepSettings { } // NewPersistentCache creates the persistent cache in the provided storage. -func NewPersistentCache(ctx context.Context, description string, cacheStorage Storage, storageProtection StorageProtection, sweep SweepSettings, mr *metrics.Registry) (*PersistentCache, error) { +func NewPersistentCache(ctx context.Context, description string, cacheStorage Storage, storageProtection cacheprot.StorageProtection, sweep SweepSettings, mr *metrics.Registry) (*PersistentCache, error) { if cacheStorage == nil { return nil, nil } @@ -341,7 +342,7 @@ func NewPersistentCache(ctx context.Context, description string, cacheStorage St sweep = sweep.applyDefaults() if storageProtection == nil { - storageProtection = NoProtection() + storageProtection = cacheprot.NoProtection() } c := &PersistentCache{ diff --git a/internal/cache/persistent_lru_cache_test.go b/internal/cache/persistent_lru_cache_test.go index 24dec1373..76ee7ee56 100644 --- a/internal/cache/persistent_lru_cache_test.go +++ b/internal/cache/persistent_lru_cache_test.go @@ -11,6 +11,7 @@ "github.com/kopia/kopia/internal/blobtesting" "github.com/kopia/kopia/internal/cache" + "github.com/kopia/kopia/internal/cacheprot" "github.com/kopia/kopia/internal/fault" "github.com/kopia/kopia/internal/gather" "github.com/kopia/kopia/internal/testlogging" @@ -25,7 +26,7 @@ func TestPersistentLRUCache(t *testing.T) { cs := blobtesting.NewMapStorage(blobtesting.DataMap{}, nil, nil).(cache.Storage) - pc, err := cache.NewPersistentCache(ctx, "testing", cs, cache.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{ + pc, err := cache.NewPersistentCache(ctx, "testing", cs, cacheprot.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{ MaxSizeBytes: maxSizeBytes, TouchThreshold: cache.DefaultTouchThreshold, SweepFrequency: cache.DefaultSweepFrequency, @@ -67,7 +68,7 @@ func TestPersistentLRUCache(t *testing.T) { verifyBlobExists(ctx, t, cs, "key3") verifyBlobExists(ctx, t, cs, "key4") - pc, err = cache.NewPersistentCache(ctx, "testing", cs, cache.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{ + pc, err = cache.NewPersistentCache(ctx, "testing", cs, cacheprot.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{ MaxSizeBytes: maxSizeBytes, TouchThreshold: cache.DefaultTouchThreshold, SweepFrequency: cache.DefaultSweepFrequency, @@ -81,7 +82,7 @@ func TestPersistentLRUCache(t *testing.T) { // create another persistent cache based on the same storage but wrong protection key. // all reads from cache will be invalid, which means GetOrLoad will fetch them from the source. - pc2, err := cache.NewPersistentCache(ctx, "testing", cs, cache.ChecksumProtection([]byte{3, 2, 1}), cache.SweepSettings{ + pc2, err := cache.NewPersistentCache(ctx, "testing", cs, cacheprot.ChecksumProtection([]byte{3, 2, 1}), cache.SweepSettings{ MaxSizeBytes: maxSizeBytes, TouchThreshold: cache.DefaultTouchThreshold, SweepFrequency: cache.DefaultSweepFrequency, @@ -153,7 +154,7 @@ func TestPersistentLRUCache_GetDeletesInvalidBlob(t *testing.T) { fs := blobtesting.NewFaultyStorage(st) fc := faultyCache{fs} - pc, err := cache.NewPersistentCache(ctx, "test", fc, cache.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{}, nil) + pc, err := cache.NewPersistentCache(ctx, "test", fc, cacheprot.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{}, nil) require.NoError(t, err) pc.Put(ctx, "key", gather.FromSlice([]byte{1, 2, 3})) @@ -183,7 +184,7 @@ func TestPersistentLRUCache_PutIgnoresStorageFailure(t *testing.T) { fs := blobtesting.NewFaultyStorage(st) fc := faultyCache{fs} - pc, err := cache.NewPersistentCache(ctx, "test", fc, cache.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{}, nil) + pc, err := cache.NewPersistentCache(ctx, "test", fc, cacheprot.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{}, nil) require.NoError(t, err) fs.AddFault(blobtesting.MethodPutBlob).ErrorInstead(someError) @@ -209,7 +210,7 @@ func TestPersistentLRUCache_SweepMinSweepAge(t *testing.T) { fs := blobtesting.NewFaultyStorage(st) fc := faultyCache{fs} - pc, err := cache.NewPersistentCache(ctx, "test", fc, cache.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{ + pc, err := cache.NewPersistentCache(ctx, "test", fc, cacheprot.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{ SweepFrequency: 100 * time.Millisecond, MaxSizeBytes: 1000, MinSweepAge: 10 * time.Second, @@ -238,7 +239,7 @@ func TestPersistentLRUCache_SweepIgnoresErrors(t *testing.T) { fs := blobtesting.NewFaultyStorage(st) fc := faultyCache{fs} - pc, err := cache.NewPersistentCache(ctx, "test", fc, cache.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{ + pc, err := cache.NewPersistentCache(ctx, "test", fc, cacheprot.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{ SweepFrequency: 100 * time.Millisecond, MaxSizeBytes: 1000, }, nil) @@ -271,7 +272,7 @@ func TestPersistentLRUCache_Sweep1(t *testing.T) { fs := blobtesting.NewFaultyStorage(st) fc := faultyCache{fs} - pc, err := cache.NewPersistentCache(ctx, "test", fc, cache.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{ + pc, err := cache.NewPersistentCache(ctx, "test", fc, cacheprot.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{ SweepFrequency: 10 * time.Millisecond, MaxSizeBytes: 1, MinSweepAge: 0 * time.Second, diff --git a/internal/cache/storage_protection.go b/internal/cacheprot/storage_protection.go similarity index 97% rename from internal/cache/storage_protection.go rename to internal/cacheprot/storage_protection.go index 947a45c71..776582155 100644 --- a/internal/cache/storage_protection.go +++ b/internal/cacheprot/storage_protection.go @@ -1,4 +1,5 @@ -package cache +// Package cacheprot provides utilities for protection of cache entries. +package cacheprot import ( "crypto/sha256" diff --git a/internal/cache/storage_protection_test.go b/internal/cacheprot/storage_protection_test.go similarity index 76% rename from internal/cache/storage_protection_test.go rename to internal/cacheprot/storage_protection_test.go index 4d57fce09..31d5ee118 100644 --- a/internal/cache/storage_protection_test.go +++ b/internal/cacheprot/storage_protection_test.go @@ -1,4 +1,4 @@ -package cache_test +package cacheprot_test import ( "bytes" @@ -6,27 +6,27 @@ "github.com/stretchr/testify/require" - "github.com/kopia/kopia/internal/cache" + "github.com/kopia/kopia/internal/cacheprot" "github.com/kopia/kopia/internal/gather" ) func TestNoStorageProection(t *testing.T) { - testStorageProtection(t, cache.NoProtection(), false) + testStorageProtection(t, cacheprot.NoProtection(), false) } func TestHMACStorageProtection(t *testing.T) { - testStorageProtection(t, cache.ChecksumProtection([]byte{1, 2, 3, 4}), true) + testStorageProtection(t, cacheprot.ChecksumProtection([]byte{1, 2, 3, 4}), true) } func TestEncryptionStorageProtection(t *testing.T) { - e, err := cache.AuthenticatedEncryptionProtection([]byte{1}) + e, err := cacheprot.AuthenticatedEncryptionProtection([]byte{1}) require.NoError(t, err) testStorageProtection(t, e, true) } //nolint:thelper -func testStorageProtection(t *testing.T, sp cache.StorageProtection, protectsFromBitFlips bool) { +func testStorageProtection(t *testing.T, sp cacheprot.StorageProtection, protectsFromBitFlips bool) { payload := []byte{0, 1, 2, 3, 4} var protected gather.WriteBuffer diff --git a/repo/content/committed_read_manager.go b/repo/content/committed_read_manager.go index 9800bf824..1aa99fa79 100644 --- a/repo/content/committed_read_manager.go +++ b/repo/content/committed_read_manager.go @@ -11,6 +11,7 @@ "go.uber.org/zap" "github.com/kopia/kopia/internal/cache" + "github.com/kopia/kopia/internal/cacheprot" "github.com/kopia/kopia/internal/clock" "github.com/kopia/kopia/internal/epoch" "github.com/kopia/kopia/internal/gather" @@ -427,7 +428,7 @@ func (sm *SharedManager) setupReadManagerCaches(ctx context.Context, caching *Ca return errors.Wrap(err, "unable to initialize index blob cache storage") } - indexBlobCache, err := cache.NewPersistentCache(ctx, "index-blobs", indexBlobStorage, cache.ChecksumProtection(caching.HMACSecret), cache.SweepSettings{ + indexBlobCache, err := cache.NewPersistentCache(ctx, "index-blobs", indexBlobStorage, cacheprot.ChecksumProtection(caching.HMACSecret), cache.SweepSettings{ MaxSizeBytes: metadataCacheSize, MinSweepAge: caching.MinMetadataSweepAge.DurationOrDefault(DefaultMetadataCacheSweepAge), }, mr) diff --git a/repo/open.go b/repo/open.go index d44e73cf0..206c5f57f 100644 --- a/repo/open.go +++ b/repo/open.go @@ -13,6 +13,7 @@ "golang.org/x/crypto/scrypt" "github.com/kopia/kopia/internal/cache" + "github.com/kopia/kopia/internal/cacheprot" "github.com/kopia/kopia/internal/epoch" "github.com/kopia/kopia/internal/feature" "github.com/kopia/kopia/internal/metrics" @@ -143,7 +144,7 @@ func getContentCacheOrNil(ctx context.Context, opt *content.CachingOptions, pass return nil, errors.Wrap(err, "unable to derive cache encryption key from password") } - prot, err := cache.AuthenticatedEncryptionProtection(cacheEncryptionKey) + prot, err := cacheprot.AuthenticatedEncryptionProtection(cacheEncryptionKey) if err != nil { return nil, errors.Wrap(err, "unable to initialize protection") }