Remediate all lint findings under the canonical golangci-lint config
Fix every finding surfaced by the canonical .golangci.yml with golangci-lint v2.12.2 (refs #61), behavior-preserving throughout: - err113: dynamic errors replaced with package-level sentinels and %w wrapping; direct comparisons converted to errors.Is - goprintffuncname: printf-style helpers renamed with an f suffix (ui.Writer message methods, cli.ReportErrorf, database.Fatalf, vaultik stdoutf) and all call sites updated - revive: stuttering type names renamed (blob.Handler, blob.WithReader, blob.ChunkPosition, storage.URL, storage.Info), doc comments added, unused parameters blanked, package comments added - contextcheck/noctx: ctx threaded through blob.Packer (AddChunk/Flush/FinalizeBlob/PackChunks) and scanner call sites; context-aware exec and sql variants used - funlen/cyclop/gocognit/nestif/dupl: oversized or duplicated functions split into focused helpers across production and test code - paralleltest/tparallel/thelper/usetesting/testpackage: tests parallelized where safe (global log.Initialize kept in the serial phase), helpers marked, t.TempDir adopted, external test packages where only exported API is used - gosec: integer conversions clamped or justified, header timeouts added, remaining findings suppressed with per-site justifications - mnd/goconst/lll/wsl_v5/nlreturn/noinlineerr/errcheck and other mechanical findings fixed directly Remove the deprecated log.LogOptions alias (callers migrated to log.Options). make check is green.
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
package vaultik
|
||||
package vaultik //nolint:testpackage // inspects unexported cache internals
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
@@ -50,33 +50,28 @@ import (
|
||||
// path-ordered names that interleave the blobs (a1, b1, c1, a2, b2,
|
||||
// c2, …) so naive path-order processing would touch every blob before
|
||||
// finishing any of them.
|
||||
func TestRestoreLocalityAndReadAt(t *testing.T) {
|
||||
log.Initialize(log.Config{})
|
||||
// localitySource is one 1 MiB fixture file used by the locality test.
|
||||
type localitySource struct {
|
||||
path string
|
||||
data []byte
|
||||
}
|
||||
|
||||
fs := afero.NewOsFs()
|
||||
tempDir, err := os.MkdirTemp("", "vaultik-locality-")
|
||||
require.NoError(t, err)
|
||||
// localityCopy is a byte-for-byte clone of one fixture source with an
|
||||
// interleaved name.
|
||||
type localityCopy struct {
|
||||
path string
|
||||
data []byte
|
||||
}
|
||||
|
||||
defer func() { _ = os.RemoveAll(tempDir) }()
|
||||
// buildLocalityFixture writes the adversarial source layout described
|
||||
// in TestRestoreLocalityAndReadAt: 15 sources of 1 MiB each (which the
|
||||
// backup packs into 3 blobs of 5 chunks) plus 9 interleaved-name copies
|
||||
// (3 per blob group).
|
||||
func buildLocalityFixture(
|
||||
t *testing.T, fs afero.Fs, dataDir string,
|
||||
) ([]*localitySource, []localityCopy) {
|
||||
t.Helper()
|
||||
|
||||
dataDir := filepath.Join(tempDir, "source")
|
||||
storeDir := filepath.Join(tempDir, "remote")
|
||||
restoreDir := filepath.Join(tempDir, "restored")
|
||||
dbPath := filepath.Join(tempDir, "index.sqlite")
|
||||
|
||||
require.NoError(t, fs.MkdirAll(dataDir, 0o755))
|
||||
|
||||
// Layout: 15 source files of exactly 1 MiB each. With
|
||||
// chunkSize (avg) = 4 MiB the chunker's minSize is 1 MiB, so any
|
||||
// file of 1 MiB becomes a single chunk. With a 5 MiB blob limit
|
||||
// the packer fits exactly 5 chunks per blob, producing 3 blobs
|
||||
// containing src-001..005, src-006..010, src-011..015.
|
||||
//
|
||||
// Then add 9 "copy" files — byte-for-byte clones of three of the
|
||||
// sources (one from each blob group) — with interleaved names
|
||||
// (cp-001-A, cp-002-B, cp-003-C, cp-004-A, …) so a naive
|
||||
// path-ordered restore would touch all three blobs before
|
||||
// finishing any of them.
|
||||
const (
|
||||
srcBytes = 1024 * 1024
|
||||
srcCount = 15
|
||||
@@ -84,35 +79,24 @@ func TestRestoreLocalityAndReadAt(t *testing.T) {
|
||||
perBlob = srcCount / blobsCount
|
||||
)
|
||||
|
||||
type source struct {
|
||||
path string
|
||||
data []byte
|
||||
}
|
||||
|
||||
sources := make([]*source, srcCount)
|
||||
sources := make([]*localitySource, srcCount)
|
||||
for i := range srcCount {
|
||||
s := &source{
|
||||
s := &localitySource{
|
||||
path: fmt.Sprintf("src-%03d.bin", i+1),
|
||||
data: randomBytes(t, srcBytes),
|
||||
}
|
||||
sources[i] = s
|
||||
require.NoError(t, afero.WriteFile(fs, filepath.Join(dataDir, s.path), s.data, 0o644))
|
||||
require.NoError(t, afero.WriteFile(
|
||||
fs, filepath.Join(dataDir, s.path), s.data, 0o644))
|
||||
}
|
||||
|
||||
// Pick one representative source per blob group (src-001 → blob
|
||||
// 1, src-006 → blob 2, src-011 → blob 3) and create 3 copies of
|
||||
// each with interleaved alphabetical names.
|
||||
type copyFile struct {
|
||||
path string
|
||||
data []byte
|
||||
sourceBlob int // 0, 1, or 2
|
||||
sourceIndex int // index into sources slice
|
||||
}
|
||||
|
||||
groupReps := []int{0, perBlob, 2 * perBlob} // 0, 5, 10
|
||||
letters := []byte{'A', 'B', 'C'}
|
||||
|
||||
var copies []copyFile
|
||||
copies := make([]localityCopy, 0, blobsCount*3)
|
||||
|
||||
for i := range 3 {
|
||||
for j := range blobsCount {
|
||||
@@ -121,10 +105,24 @@ func TestRestoreLocalityAndReadAt(t *testing.T) {
|
||||
path := filepath.Join(dataDir, name)
|
||||
src := sources[groupReps[j]]
|
||||
require.NoError(t, afero.WriteFile(fs, path, src.data, 0o644))
|
||||
copies = append(copies, copyFile{path: path, data: src.data, sourceBlob: j, sourceIndex: groupReps[j]})
|
||||
|
||||
copies = append(copies, localityCopy{path: path, data: src.data})
|
||||
}
|
||||
}
|
||||
|
||||
return sources, copies
|
||||
}
|
||||
|
||||
// setupLocalityBackup runs the backup half of the locality test: a
|
||||
// snapshot of dataDir into a file storer with sizes tuned so 15 one-chunk
|
||||
// files pack into 3 blobs. The index database is closed before returning
|
||||
// so the restore half runs from remote bytes only.
|
||||
func setupLocalityBackup(
|
||||
ctx context.Context, t *testing.T, fs afero.Fs,
|
||||
dataDir, storeDir, dbPath string,
|
||||
) (*config.Config, *storage.FileStorer, string) {
|
||||
t.Helper()
|
||||
|
||||
// chunkSize avg = 4 MiB makes minSize = 1 MiB, so a 1 MiB file
|
||||
// becomes one chunk. maxBlobSize = 5 MiB packs exactly 5 chunks
|
||||
// per blob, yielding 3 blobs from 15 source files.
|
||||
@@ -134,8 +132,10 @@ func TestRestoreLocalityAndReadAt(t *testing.T) {
|
||||
storer, err := storage.NewFileStorer(storeDir)
|
||||
require.NoError(t, err)
|
||||
|
||||
agePublicKey := "age1ezrjmfpwsc95svdg0y54mums3zevgzu0x0ecq2f7tp8a05gl0sjq9q9wjg"
|
||||
ageSecretKey := "AGE-SECRET-KEY-19CR5YSFW59HM4TLD6GXVEDMZFTVVF7PPHKUT68TXSFPK7APHXA2QS2NJA5"
|
||||
agePublicKey := "age1ezrjmfpwsc95svdg0y54mums3zevgzu0x0ecq2f7tp8a05g" +
|
||||
"l0sjq9q9wjg"
|
||||
ageSecretKey := "AGE-SECRET-KEY-19CR5YSFW59HM4TLD6GXVEDMZFTVVF7PPHKU" +
|
||||
"T68TXSFPK7APHXA2QS2NJA5"
|
||||
|
||||
cfg := &config.Config{
|
||||
AgeRecipients: []string{agePublicKey},
|
||||
@@ -145,8 +145,6 @@ func TestRestoreLocalityAndReadAt(t *testing.T) {
|
||||
BlobSizeLimit: config.Size(maxBlobSize),
|
||||
}
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
db, err := database.New(ctx, dbPath)
|
||||
require.NoError(t, err)
|
||||
|
||||
@@ -171,7 +169,8 @@ func TestRestoreLocalityAndReadAt(t *testing.T) {
|
||||
Repositories: repos,
|
||||
})
|
||||
|
||||
snapshotID, err := sm.CreateSnapshotWithName(ctx, cfg.Hostname, "locality", "test-version", "test-git")
|
||||
snapshotID, err := sm.CreateSnapshotWithName(
|
||||
ctx, cfg.Hostname, "locality", "test-version", "test-git")
|
||||
require.NoError(t, err)
|
||||
|
||||
_, err = scanner.Scan(ctx, dataDir, snapshotID)
|
||||
@@ -182,10 +181,35 @@ func TestRestoreLocalityAndReadAt(t *testing.T) {
|
||||
|
||||
blobsOnDisk := listBlobKeys(t, storeDir)
|
||||
t.Logf("backup produced %d blobs", len(blobsOnDisk))
|
||||
require.GreaterOrEqual(t, len(blobsOnDisk), 3, "expected at least 3 blobs from 3 filler groups")
|
||||
require.GreaterOrEqual(t, len(blobsOnDisk), 3,
|
||||
"expected at least 3 blobs from 3 filler groups")
|
||||
|
||||
require.NoError(t, db.Close())
|
||||
|
||||
return cfg, storer, snapshotID
|
||||
}
|
||||
|
||||
func TestRestoreLocalityAndReadAt(t *testing.T) {
|
||||
log.Initialize(log.Config{})
|
||||
t.Parallel()
|
||||
|
||||
fs := afero.NewOsFs()
|
||||
tempDir := t.TempDir()
|
||||
|
||||
dataDir := filepath.Join(tempDir, "source")
|
||||
storeDir := filepath.Join(tempDir, "remote")
|
||||
restoreDir := filepath.Join(tempDir, "restored")
|
||||
dbPath := filepath.Join(tempDir, "index.sqlite")
|
||||
|
||||
require.NoError(t, fs.MkdirAll(dataDir, 0o755))
|
||||
|
||||
sources, copies := buildLocalityFixture(t, fs, dataDir)
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
cfg, storer, snapshotID := setupLocalityBackup(
|
||||
ctx, t, fs, dataDir, storeDir, dbPath)
|
||||
|
||||
// Wrap the storer so we can count downloads per blob key.
|
||||
counter := newCountingStorer(storer)
|
||||
|
||||
@@ -215,20 +239,7 @@ func TestRestoreLocalityAndReadAt(t *testing.T) {
|
||||
|
||||
require.NotNil(t, cacheRef, "restoreCacheObserver must fire during restore")
|
||||
|
||||
// Verify restored content matches.
|
||||
for _, s := range sources {
|
||||
restored := filepath.Join(restoreDir, dataDir, s.path)
|
||||
got, err := afero.ReadFile(fs, restored)
|
||||
require.NoErrorf(t, err, "source missing after restore: %s", s.path)
|
||||
require.Truef(t, bytes.Equal(got, s.data), "byte mismatch for source %s", s.path)
|
||||
}
|
||||
|
||||
for _, c := range copies {
|
||||
restored := filepath.Join(restoreDir, c.path)
|
||||
got, err := afero.ReadFile(fs, restored)
|
||||
require.NoErrorf(t, err, "copy missing after restore: %s", c.path)
|
||||
require.Truef(t, bytes.Equal(got, c.data), "byte mismatch for copy %s", c.path)
|
||||
}
|
||||
verifyLocalityRestore(t, fs, restoreDir, dataDir, sources, copies)
|
||||
|
||||
// (1) Each blob fetched exactly once.
|
||||
for key, n := range counter.snapshot() {
|
||||
@@ -242,18 +253,48 @@ func TestRestoreLocalityAndReadAt(t *testing.T) {
|
||||
// (2) Peak cache size ≤ 1. The sweeper plus locality-aware
|
||||
// ordering should free each blob before the next one downloads.
|
||||
assert.LessOrEqualf(t, cacheRef.PeakLen(), 1,
|
||||
"peak cached blobs was %d; expected ≤ 1 with locality-ordered restore", cacheRef.PeakLen())
|
||||
"peak cached blobs was %d; expected ≤ 1 with locality-ordered restore",
|
||||
cacheRef.PeakLen())
|
||||
|
||||
// (3) Cache.Get must never be called during restore — chunk
|
||||
// extraction has to go through ReadAt so we never read the whole
|
||||
// blob from disk to grab a few KB slice.
|
||||
assert.Equalf(t, 0, cacheRef.GetCalls(),
|
||||
"blobDiskCache.Get was called %d times during restore; restore must use ReadAt exclusively", cacheRef.GetCalls())
|
||||
"blobDiskCache.Get was called %d times during restore; "+
|
||||
"restore must use ReadAt exclusively", cacheRef.GetCalls())
|
||||
|
||||
t.Logf("blob cache stats: peak_len=%d get_calls=%d readat_calls=%d",
|
||||
cacheRef.PeakLen(), cacheRef.GetCalls(), cacheRef.ReadAtCalls())
|
||||
}
|
||||
|
||||
// verifyLocalityRestore byte-compares every restored source and copy
|
||||
// against its original content.
|
||||
func verifyLocalityRestore(
|
||||
t *testing.T,
|
||||
fs afero.Fs,
|
||||
restoreDir, dataDir string,
|
||||
sources []*localitySource,
|
||||
copies []localityCopy,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
for _, s := range sources {
|
||||
restored := filepath.Join(restoreDir, dataDir, s.path)
|
||||
got, err := afero.ReadFile(fs, restored)
|
||||
require.NoErrorf(t, err, "source missing after restore: %s", s.path)
|
||||
require.Truef(t, bytes.Equal(got, s.data),
|
||||
"byte mismatch for source %s", s.path)
|
||||
}
|
||||
|
||||
for _, c := range copies {
|
||||
restored := filepath.Join(restoreDir, c.path)
|
||||
got, err := afero.ReadFile(fs, restored)
|
||||
require.NoErrorf(t, err, "copy missing after restore: %s", c.path)
|
||||
require.Truef(t, bytes.Equal(got, c.data),
|
||||
"byte mismatch for copy %s", c.path)
|
||||
}
|
||||
}
|
||||
|
||||
// randomBytes returns n bytes of random data. Used to make sure the
|
||||
// chunker picks non-degenerate FastCDC boundaries.
|
||||
func randomBytes(t *testing.T, n int) []byte {
|
||||
@@ -316,7 +357,9 @@ func newCountingStorer(inner storage.Storer) *countingStorerInternal {
|
||||
return &countingStorerInternal{Storer: inner, counts: make(map[string]int)}
|
||||
}
|
||||
|
||||
func (c *countingStorerInternal) Get(ctx context.Context, key string) (io.ReadCloser, error) {
|
||||
func (c *countingStorerInternal) Get(
|
||||
ctx context.Context, key string,
|
||||
) (io.ReadCloser, error) {
|
||||
c.mu.Lock()
|
||||
c.counts[key]++
|
||||
c.mu.Unlock()
|
||||
|
||||
Reference in New Issue
Block a user