test(storage): promote driver testsuites to narrowed interface (S06 Step 15)

What

S06 Step 15 — promote the generic storage-driver conformance suite out of the Go-toolchain-ignored internal/storage/_imported/ quarantine into its final home internal/storage/driver/testsuites/, adapted to AR's narrowed StorageDriver interface.

This is a port

The bulk of this MR is a faithful lift of the Container Registry storage-driver conformance suite (registry/storage/driver/testsuites) and its test helpers (testutil/rand.go, testutil/blobber.go, the random path/filename generators). The deliberate goal is to keep the suite structurally identical to CR so the changes stay easy to diff and review. Every deviation from CR is limited to what AR's narrowed interface and Go toolchain actually require — nothing was reworded, reordered, or "cleaned up" gratuitously.

How to review

Because this is a move-with-adaptations, a plain diff looks enormous (~1.7k added / ~2.7k removed) but most of it is the same code relocated from _imported/ to its final path. To see only the real changes, review with rename detection + word-diff so the move collapses and only the adaptations remain:

git show -M10% --word-diff

(-M10% lets git treat the _imported deletion + new file as a rename even though the bodies were adapted; --word-diff then surfaces the intra-line edits.)

What we changed/added on top of the verbatim port

The port itself, adapted to compile against AR's interface:

  • Re-pointed from CR's storagedriver.StorageDriver to internal/storage.StorageDriver.
  • Removed all docker/distribution and libtrust imports (neither is in go.mod).
  • Dropped the tests for the narrowed-away surface (Stat/Walk/List/recursive Delete).
  • Replaced the List-based TearDownTest with explicit per-test path tracking + non-recursive Delete (the narrowed interface cannot enumerate the backend).
  • Replaced the s3/azure driver-specific chunk-size constants with a suite-level MinChunkSize field, so the package has no driver-package imports and compiles in isolation.

New test-helper files (CR ports):

  • internal/testutil/blobber.go — pseudo-random blob generator (CR testutil/rand.go + testutil/blobber.go), reimplemented on stdlib (math/rand/v2).
  • internal/testutil/storagepath.go — random path/filename generators.

Interface & error-surface additions (small, only to support the promoted suite + align with the spec):

  • internal/storage/driver.go: PutContent gains a metadata map[string]string parameter — S06 StoreBlob requires metadata to be written with the object, but the interface lacked it. The S06 spec is updated to match.
  • internal/storage/driver.go: documented that a negative offset on Reader/Writer returns ErrInvalidOffset; documented the FileWriter terminal-state contract.
  • internal/storage/errors.go: added ErrAlreadyClosed / ErrAlreadyCommited / ErrAlreadyCanceled sentinels (needed by the writer-lifecycle tests) and the InvalidPathError typed error.
  • internal/storage/driver/s3/limits.go: MinChunkSize const (the S3 5 MiB multipart minimum).

Tests restored/added beyond the Stat-dropped baseline:

  • TestExists (AC #7 (closed)) and TestWriterCancel (AC #15) — generically-testable criteria the original promotion had dropped along with Stat.
  • Writer-lifecycle robustness (TestWriterDoubleClose, TestWriterDoubleCommit, TestWriterCommitAfterCancel/Close, TestWriterCancelAfter*, TestWriterWriteAfter*) — restored from CR, enabled by the new ErrAlready* sentinels.
  • TestWriterOffsetDivergence (AC #6 (closed)) — now exercised generically.
  • TestPutContentWithMetadata — the PutContent metadata variant; metadata is verified via URLFor (as is TestMoveWithMetadata).

Config & docs:

  • .golangci.yaml: lint/format exclusions are now scoped to _imported only — the promoted files lint clean with no carve-out.
  • docs/specs/S06-storage-layer.md, docs/plans/2026-05-15-storage-layer.md: PutContent metadata signature; removed the offset-divergence deferral notes now that AC #6 (closed) is covered.

Could this be split?

The interface/error additions (driver.go, errors.go, s3/limits.go) and the spec/plan doc edits could in principle land as a small prep MR ahead of the test promotion. We chose not to: those additions total roughly ~70 lines and exist solely to satisfy the promoted suite, so peeling them out would not meaningfully shrink this MR — the weight is the ~1.7k-line suite relocation — while adding a second review + pipeline cycle. Hence the all-in-one. If a reviewer would still prefer the split, say so and it's an easy carve-out.

Spec coverage

# S06 StorageDriver criterion Retained test(s) Notes
1 PutContent→GetContent round-trip TestPutContentMultipleTimes, TestPutContentZeroSizeFile, TestTruncate, TestPutContentWithMetadata metadata variant verified via URLFor
2 Writer(0)→Commit→Reader(0) TestWriteRead1-4, TestWriteReadStreams1-4, …NonUTF8, …SmallStream, …LargeStreams
3 Writer(offset>0) append TestContinueStreamAppend{Large,Small}, TestContinueAppendZeroSizeBlob, TestOverwriteAppendBlob, TestOverwriteBlockBlob
4 Writer(0) errors if staging exists TestNewWriterExistingStaging
5 Writer(offset>0) → *PathNotFoundError TestAppendInexistentBlob, TestReadNonexistent{,Stream}
6 Writer(offset>0) → *OffsetDivergenceError TestWriterOffsetDivergence
7 Exists true / false TestExists (false,err) transient channel needs fault injection → driver-specific (16-17)
8 Move places at dest, removes source TestMovePutContentBlob, TestMoveWritterBlob, TestMoveOverwrite, TestMoveNonexistent, TestMoveInvalid
9 Move >5GB multipart (S3) deferred S3-specific; Step 16
10 Move applies metadata atomically TestMoveWithMetadata metadata verified via URLFor
11 Move error preserves source on metadata failure deferred needs fault injection; driver-specific (16-17)
12 URLFor valid pre-signed URL / ErrUnsupportedMethod TestURLFor skips on ErrUnsupportedMethod
13 Delete removes object TestDelete, TestDeleteFileEqualFolderFileName, TestDeleteNonexistent
14 Reader(offset>0) range read TestReaderWithOffset, TestConcurrentStreamReads
15 Cancel releases staging; later Writer(0) succeeds, Reader(0)→not-found TestWriterCancel
Lifecycle & large-object robustness TestWriter{DoubleClose,DoubleCommit,CommitAfter*,CancelAfter*,WriteAfter*}, TestMaxUploadSize, TestZeroByteBlobUploadSession, TestConcurrentFileStreams

Criteria 9/11 are inherently backend-specific (multipart >5 GB, metadata-failure fault injection) and land with the per-driver cases in Steps 16-17 (Adjust S3 / Adjust GCS), per the plan & spec §Testing. The suite stays generic (parameterized by a StorageDriver factory) and has no consumers yet — Steps 16/17 wire it to the concrete driver factories.

Acceptance

  • internal/storage/driver/testsuites/testsuites.go compiles in isolation — no driver-package, docker/distribution, or libtrust imports.
  • The promoted suite and both helper files lint clean with no .golangci.yaml exclusion (_imported is the only exclusion left).
  • internal/storage/_imported/driver/testsuites/ no longer exists.
  • Full pre-commit hook chain passes (go-test, golangci-lint, go-fmt, go-imports).

Plan: S06 Storage Layer — Step 15 · Spec: S06

Related to #160 (closed)

Edited by Pawel Rozlach

Merge request reports

Loading
Loading