diff --git a/docs/design/2026_04_29_implemented_snapshot_logical_decoder.md b/docs/design/2026_04_29_implemented_snapshot_logical_decoder.md index 1d3570a98..0beaf15db 100644 --- a/docs/design/2026_04_29_implemented_snapshot_logical_decoder.md +++ b/docs/design/2026_04_29_implemented_snapshot_logical_decoder.md @@ -772,6 +772,9 @@ decoder's answer: disjoint, and reads each SST with Pebble's sstable reader, yielding the same entries as the legacy stream. This is the one place the package links a storage library, because the files are in Pebble's format. + The package's import-boundary test allows exactly + `github.com/cockroachdb/pebble/v2/sstable`; importing the Pebble + engine or any other Pebble package fails it. - **Key budget.** The reader capped keys at the 1 MiB logical size, while the store streams stored keys up to `MaxSnapshotStoredKeySize`, 1 MiB plus a 32-byte internal envelope such as migration staging. The reader diff --git a/docs/design/2026_10_16_implemented_production_commonization.md b/docs/design/2026_10_16_implemented_production_commonization.md index 61986c9c0..799cc1761 100644 --- a/docs/design/2026_10_16_implemented_production_commonization.md +++ b/docs/design/2026_10_16_implemented_production_commonization.md @@ -184,7 +184,11 @@ All five milestones shipped in PR #1277. Each defect fix landed as a failing-tes - the FIFO dedup send timestamp sharing the record's millisecond; - no-TTL HLLs restoring as the legacy raw payload. - Mutation checks on both the backup copy and the live counterpart fail every family's test. -- `elastickv-backup`, `elastickv-snapshot-decode`, `elastickv-snapshot-encode` and `elastickv-snapshot-archive` no longer link `store`, Pebble or `internal/encryption`. `elastickv-snapshot-prepare-restore` still does, through `internal/raftengine/etcd`, which the restore path needs. +- `elastickv-backup`, `elastickv-snapshot-decode`, `elastickv-snapshot-encode` and `elastickv-snapshot-archive` no longer link `store`, the Pebble engine or `internal/encryption`. `elastickv-snapshot-prepare-restore` still does, through `internal/raftengine/etcd`, which the restore path needs. +- #1276 merged while PR #1277 was open. It made the decoder read SST ingest bundles with `github.com/cockroachdb/pebble/v2/sstable` (`internal/backup/snapshot_sst.go`), as the decoder design records. With both on `main`, the boundary test failed on that import. + - The test now exempts exactly that package (`backupAllowedDeps`), and fails if the exemption goes unused. + - The Pebble engine (`github.com/cockroachdb/pebble/v2`) imports `sstable`, so it cannot come in through it. Importing the engine or any other Pebble package still fails the test. + - The four CLIs above therefore link the sstable reader and the Pebble packages it depends on, about 5 MiB per linux/amd64 binary. Only `elastickv-snapshot-decode` and the self-test of `elastickv-snapshot-encode` reach it; `elastickv-backup` and `elastickv-snapshot-archive` carry it unused. **M5.** - The autosplit splitter converts SplitRange children with `distribution.RouteDescriptorFromProto`, which rejects a nil child, an unspecified state or invalid bounds. Such a response now fails the split call instead of becoming a write-fenced placeholder. @@ -196,5 +200,5 @@ All five milestones shipped in PR #1277. Each defect fix landed as a failing-tes - Coordinator wrappers share one forwarding base, and a capability-parity test is the guard against future drift (M1). - Admin writes and sensitive reads require both the JWT role and the live role (M2). - Admin S3 uploads go through the same PUT pipeline as SigV4 uploads, including admission control and blob offload; admin keeps its own size cap (M3). -- `internal/backup`'s import boundary is enforced by a test, and every backup mirror of an adapter encoder has an adapter-side byte-for-byte cross-check (M4). +- `internal/backup`'s import boundary is enforced by a test, and every backup mirror of an adapter encoder has an adapter-side byte-for-byte cross-check (M4). The boundary keeps the Pebble engine out but allows exactly Pebble's `sstable` reader, because SST ingest bundles are Pebble sstable files (#1276). - Shared server wiring lives in `internal/serverwiring`; the demo stays a separate binary (M5). diff --git a/internal/backup/import_boundary_test.go b/internal/backup/import_boundary_test.go index 06e3f41c6..b8e112074 100644 --- a/internal/backup/import_boundary_test.go +++ b/internal/backup/import_boundary_test.go @@ -4,6 +4,7 @@ import ( "go/build" "os" "path/filepath" + "slices" "strings" "testing" ) @@ -24,6 +25,16 @@ var backupForbiddenDeps = []string{ "github.com/cockroachdb/pebble", } +// backupAllowedDeps are exact package paths exempt from +// backupForbiddenDeps. Snapshot SST ingest bundles are Pebble sstable +// files, so the decoder reads them with Pebble's sstable reader +// (snapshot_sst.go, docs/design/2026_04_29_implemented_snapshot_logical_decoder.md). +// That package cannot reach the Pebble engine, which imports it, and +// importing the engine or any other Pebble package still fails the test. +var backupAllowedDeps = []string{ + "github.com/cockroachdb/pebble/v2/sstable", +} + // TestBackupLinksNoLiveClusterCode walks the non-test import closure of // internal/backup through first-party packages and fails on any // forbidden package, naming the import chain that reaches it. @@ -56,9 +67,43 @@ func TestBackupLinksNoLiveClusterCode(t *testing.T) { } } } + for _, allowed := range backupAllowedDeps { + if _, seen := via[allowed]; !seen { + t.Errorf("internal/backup no longer links %s; remove it from backupAllowedDeps", allowed) + } + } +} + +// TestForbiddenBackupDep pins the boundary's matching rules: a forbidden +// entry covers its subpackages, and an allowed entry exempts only its +// exact path. +func TestForbiddenBackupDep(t *testing.T) { + t.Parallel() + + const pebble = "github.com/cockroachdb/pebble" + cases := []struct { + pkgPath string + want string + }{ + {pkgPath: pebble + "/v2/sstable", want: ""}, + {pkgPath: pebble + "/v2", want: pebble}, + {pkgPath: pebble + "/v2/sstable/block", want: pebble}, + {pkgPath: pebble + "/sstable", want: pebble}, + {pkgPath: elastickvModule + "/store", want: elastickvModule + "/store"}, + {pkgPath: elastickvModule + "/internal/encryption/kek", want: elastickvModule + "/internal/encryption"}, + {pkgPath: elastickvModule + "/internal/s3keys", want: ""}, + } + for _, tc := range cases { + if got := forbiddenBackupDep(tc.pkgPath); got != tc.want { + t.Errorf("forbiddenBackupDep(%q) = %q, want %q", tc.pkgPath, got, tc.want) + } + } } func forbiddenBackupDep(pkgPath string) string { + if slices.Contains(backupAllowedDeps, pkgPath) { + return "" + } for _, forbidden := range backupForbiddenDeps { if pkgPath == forbidden || strings.HasPrefix(pkgPath, forbidden+"/") { return forbidden