diff --git a/docs/internal/artifacts.md b/docs/internal/artifacts.md index eb54a7c..29aa8e4 100644 --- a/docs/internal/artifacts.md +++ b/docs/internal/artifacts.md @@ -121,6 +121,21 @@ complete restore authority. Callers receive its declared object identities and must not supplement them by listing mutable session prefixes. The legacy reader is intentionally separate and remains migration-only support. +The reader opens each small control object directly and enforces owner-specific +limits before decoding: 64 KiB for the mutable commit pointer, 4 MiB for the +immutable commit manifest, and 8 MiB for the selected session manifest. Legacy +compatibility applies a 4 KiB limit to `current/run_id.txt` and the same 8 MiB +manifest limit to `current/manifest.json`. These are exposed as +`MaxCurrentCommitPointerBytes`, `MaxRemoteCommitManifestBytes`, +`MaxRemoteSessionManifestBytes`, `MaxLegacyCurrentRunPointerBytes`, and +`MaxLegacyCurrentManifestBytes`. + +Each read uses the generation and size metadata returned with its opened body. +Actual bytes remain subject to a limit-plus-one read even if size metadata is +absent or inaccurate. Immutable selections then retain their declared-size, +checksum, generation, and identity checks. No current-state control object is +downloaded through a temporary file. + Core helpers: - `LoadCurrentState` @@ -185,7 +200,9 @@ physical layout. `internal/artifacts/extraction_catalog.go`, `internal/artifacts/extraction_evidence.go`, `internal/artifacts/extraction_input.go` -- Current state: `internal/artifacts/current_state.go` +- Current state: `internal/artifacts/current_state.go`, + `internal/artifacts/current_state_commit.go`, + `internal/artifacts/current_state_legacy.go` - Paths and keys: `internal/artifacts/paths.go`, `internal/artifacts/s3_keys.go` - Previous requirements: `internal/artifacts/previous_requirements.go` diff --git a/docs/internal/stage-publish.md b/docs/internal/stage-publish.md index 590a658..3e86d23 100644 --- a/docs/internal/stage-publish.md +++ b/docs/internal/stage-publish.md @@ -51,6 +51,10 @@ Exact remote placement and the operator workflow belong in - rechecks remote lock state immediately before the pointer update. A newly committed lock aborts selection, leaving any uploaded immutable attempt unselected. +- reads the mutable remote lock document through a direct limit-plus-one read + capped by `MaxRemoteLockStoreBytes` (1 MiB), retaining the generation returned + with the opened body for conditional updates. Oversized lock documents fail + before YAML decoding; published artifact payloads do not use this limit. ## Metadata Signals diff --git a/docs/internal/storage.md b/docs/internal/storage.md index 127ea59..bc6658f 100644 --- a/docs/internal/storage.md +++ b/docs/internal/storage.md @@ -22,6 +22,13 @@ Key invariant: - callers pass full bucket-relative keys; - storage implementations do not infer campaign/session/run prefixes. +`ReadObjectBounded` is the shared mechanism for small control objects. It opens +one object version, returns the metadata observed with that body, rejects an +oversized known size before transfer, and still performs a context-aware +limit-plus-one read. It closes the body on every exit. Callers own the policy +limit and add the control-object category to errors; this helper is not used for +large artifact payloads. + ## Composition `NewObjectStoreFromConfig` constructs the S3-backed implementation from @@ -45,6 +52,8 @@ not own discovery, defaults, or configuration validation. ## Invariants - storage layer is stateless regarding manifest/stage progression. +- bounded reads never retain more than the caller's limit plus one byte and do + not replace owner-specific size policy. - publish ordering semantics are owned by stage/app code, not storage adapters. ## Implementation And Tests diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index b381377..8c6a82e 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -296,7 +296,7 @@ unbounded allocation. Retain coherent legacy-reader tests and stateful S3/fake ordering and conditional-write tests. Run focused storage, artifacts, app, restore, and publish tests under `-race`, followed by the complete repository validation. -**Status:** Pending. +**Status:** Completed. ## Finding traceability inventory diff --git a/internal/adapters/storage/bounded_read.go b/internal/adapters/storage/bounded_read.go new file mode 100644 index 0000000..9ce1842 --- /dev/null +++ b/internal/adapters/storage/bounded_read.go @@ -0,0 +1,90 @@ +package storage + +import ( + "context" + "errors" + "fmt" + "io" + "math" + "strings" +) + +// ReadLimitError reports that a remote object exceeded its caller-owned read +// limit. The limit is enforced against both available object metadata and the +// bytes returned by the opened object body. +type ReadLimitError struct { + Key string + Limit int64 + Observed int64 +} + +func (e *ReadLimitError) Error() string { + return fmt.Sprintf("object %q exceeds %d-byte read limit (observed at least %d bytes)", e.Key, e.Limit, e.Observed) +} + +// ReadObjectBounded opens one object version and retains at most maxBytes of +// its content. Object metadata may reject an oversized body early, but a +// limit-plus-one read always enforces the boundary when transfer begins. +func ReadObjectBounded(ctx context.Context, store ObjectStore, key string, maxBytes int64) (info ObjectInfo, data []byte, err error) { + key = strings.TrimSpace(key) + if store == nil { + return ObjectInfo{}, nil, fmt.Errorf("read bounded object: store is required") + } + if key == "" { + return ObjectInfo{}, nil, fmt.Errorf("read bounded object: key is required") + } + if maxBytes <= 0 || maxBytes == math.MaxInt64 { + return ObjectInfo{}, nil, fmt.Errorf("read bounded object %q: limit must be between 1 and %d bytes", key, int64(math.MaxInt64-1)) + } + if err := ctx.Err(); err != nil { + return ObjectInfo{}, nil, err + } + + info, body, err := store.Read(ctx, key) + if err != nil { + return ObjectInfo{}, nil, err + } + if body == nil { + return ObjectInfo{}, nil, fmt.Errorf("read bounded object %q: store returned no body", key) + } + defer func() { + if closeErr := body.Close(); closeErr != nil { + data = nil + err = errors.Join(err, fmt.Errorf("close object %q: %w", key, closeErr)) + } + }() + + if info.Size > maxBytes { + return info, nil, &ReadLimitError{Key: key, Limit: maxBytes, Observed: info.Size} + } + + data, err = io.ReadAll(io.LimitReader(contextReader{ctx: ctx, reader: body}, maxBytes+1)) + if err != nil { + return info, nil, err + } + if err := ctx.Err(); err != nil { + return info, nil, err + } + if int64(len(data)) > maxBytes { + return info, nil, &ReadLimitError{Key: key, Limit: maxBytes, Observed: int64(len(data))} + } + return info, data, nil +} + +type contextReader struct { + ctx context.Context + reader io.Reader +} + +func (r contextReader) Read(p []byte) (int, error) { + if err := r.ctx.Err(); err != nil { + return 0, err + } + n, err := r.reader.Read(p) + if err == nil { + if contextErr := r.ctx.Err(); contextErr != nil { + return n, contextErr + } + } + return n, err +} diff --git a/internal/adapters/storage/bounded_read_test.go b/internal/adapters/storage/bounded_read_test.go new file mode 100644 index 0000000..4627fe8 --- /dev/null +++ b/internal/adapters/storage/bounded_read_test.go @@ -0,0 +1,135 @@ +package storage + +import ( + "bytes" + "context" + "errors" + "io" + "testing" +) + +func TestReadObjectBoundedAcceptsExactLimitWithAbsentSizeMetadata(t *testing.T) { + body := &trackingReadCloser{reader: bytes.NewReader([]byte("12345678")), chunkSize: 2} + store := &boundedReadStore{read: func(context.Context, string) (ObjectInfo, io.ReadCloser, error) { + return ObjectInfo{Key: "control.json", ETag: "generation"}, body, nil + }} + + info, data, err := ReadObjectBounded(context.Background(), store, "control.json", 8) + if err != nil { + t.Fatalf("ReadObjectBounded() error = %v", err) + } + if string(data) != "12345678" || info.ETag != "generation" { + t.Fatalf("ReadObjectBounded() = (%#v, %q), want opened object metadata and bytes", info, data) + } + if !body.closed { + t.Fatal("object body was not closed") + } +} + +func TestReadObjectBoundedRejectsLimitPlusOneDespiteMissingOrInaccurateMetadata(t *testing.T) { + tests := []struct { + name string + metadataSize int64 + }{ + {name: "missing", metadataSize: 0}, + {name: "inaccurate", metadataSize: 2}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + body := &trackingReadCloser{reader: bytes.NewReader([]byte("123456789")), chunkSize: 1} + store := &boundedReadStore{read: func(context.Context, string) (ObjectInfo, io.ReadCloser, error) { + return ObjectInfo{Key: "control.json", Size: test.metadataSize}, body, nil + }} + + _, data, err := ReadObjectBounded(context.Background(), store, "control.json", 8) + var limitErr *ReadLimitError + if !errors.As(err, &limitErr) { + t.Fatalf("ReadObjectBounded() error = %v, want ReadLimitError", err) + } + if data != nil || body.bytesRead != 9 || !body.closed { + t.Fatalf("data=%q bytes read=%d closed=%t, want nil, 9, true", data, body.bytesRead, body.closed) + } + }) + } +} + +func TestReadObjectBoundedRejectsOversizedMetadataBeforeTransfer(t *testing.T) { + body := &trackingReadCloser{reader: bytes.NewReader([]byte("small"))} + store := &boundedReadStore{read: func(context.Context, string) (ObjectInfo, io.ReadCloser, error) { + return ObjectInfo{Key: "control.json", Size: 9}, body, nil + }} + + _, _, err := ReadObjectBounded(context.Background(), store, "control.json", 8) + var limitErr *ReadLimitError + if !errors.As(err, &limitErr) { + t.Fatalf("ReadObjectBounded() error = %v, want ReadLimitError", err) + } + if body.bytesRead != 0 || !body.closed { + t.Fatalf("bytes read=%d closed=%t, want zero-byte transfer and closed body", body.bytesRead, body.closed) + } +} + +func TestReadObjectBoundedPropagatesCancellationAndClosesBody(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + body := &trackingReadCloser{reader: bytes.NewReader([]byte("12345678")), chunkSize: 1, afterRead: cancel} + store := &boundedReadStore{read: func(context.Context, string) (ObjectInfo, io.ReadCloser, error) { + return ObjectInfo{Key: "control.json"}, body, nil + }} + + _, data, err := ReadObjectBounded(ctx, store, "control.json", 8) + if !errors.Is(err, context.Canceled) { + t.Fatalf("ReadObjectBounded() error = %v, want context cancellation", err) + } + if data != nil || body.bytesRead != 1 || !body.closed { + t.Fatalf("data=%q bytes read=%d closed=%t, want nil, 1, true", data, body.bytesRead, body.closed) + } +} + +func TestReadObjectBoundedReturnsCloseFailure(t *testing.T) { + closeErr := errors.New("close failed") + body := &trackingReadCloser{reader: bytes.NewReader([]byte("ok")), closeErr: closeErr} + store := &boundedReadStore{read: func(context.Context, string) (ObjectInfo, io.ReadCloser, error) { + return ObjectInfo{Key: "control.json", Size: 2}, body, nil + }} + + _, data, err := ReadObjectBounded(context.Background(), store, "control.json", 8) + if !errors.Is(err, closeErr) || data != nil || !body.closed { + t.Fatalf("data=%q error=%v closed=%t, want close failure and no retained data", data, err, body.closed) + } +} + +type boundedReadStore struct { + ObjectStore + read func(context.Context, string) (ObjectInfo, io.ReadCloser, error) +} + +func (s *boundedReadStore) Read(ctx context.Context, key string) (ObjectInfo, io.ReadCloser, error) { + return s.read(ctx, key) +} + +type trackingReadCloser struct { + reader io.Reader + chunkSize int + afterRead func() + closeErr error + bytesRead int + closed bool +} + +func (r *trackingReadCloser) Read(p []byte) (int, error) { + if r.chunkSize > 0 && len(p) > r.chunkSize { + p = p[:r.chunkSize] + } + n, err := r.reader.Read(p) + r.bytesRead += n + if n > 0 && r.afterRead != nil { + r.afterRead() + r.afterRead = nil + } + return n, err +} + +func (r *trackingReadCloser) Close() error { + r.closed = true + return r.closeErr +} diff --git a/internal/adapters/storage/fake.go b/internal/adapters/storage/fake.go index 8e378d5..5f1cc25 100644 --- a/internal/adapters/storage/fake.go +++ b/internal/adapters/storage/fake.go @@ -22,6 +22,7 @@ type FakeBackend struct { Objects map[string]FakeObject Uploads []FakeUploadCall Downloads []FakeDownloadCall + Reads []FakeReadCall ListErr error DownloadErr error @@ -45,6 +46,11 @@ type FakeDownloadCall struct { Bytes int64 } +// FakeReadCall captures one opened object in call order. +type FakeReadCall struct { + Key string +} + // FakeObject is a deterministic fake object-store record. type FakeObject struct { Key string @@ -127,6 +133,9 @@ func (f *FakeBackend) Read(ctx context.Context, key string) (ObjectInfo, io.Read if !ok { return ObjectInfo{}, nil, fmt.Errorf("read object %q: %w", normalizedKey, os.ErrNotExist) } + f.mu.Lock() + f.Reads = append(f.Reads, FakeReadCall{Key: normalizedKey}) + f.mu.Unlock() return ObjectInfo{Key: obj.Key, Size: int64(len(obj.Data)), ETag: obj.ETag, LastModified: obj.LastModified}, io.NopCloser(bytes.NewReader(obj.Data)), nil } diff --git a/internal/adapters/storage/s3_backend_test.go b/internal/adapters/storage/s3_backend_test.go index 8944e46..0fe7397 100644 --- a/internal/adapters/storage/s3_backend_test.go +++ b/internal/adapters/storage/s3_backend_test.go @@ -23,8 +23,11 @@ type fakeS3API struct { listErr error listCalls int - getBody io.ReadCloser - getErr error + getBody io.ReadCloser + getErr error + getSize *int64 + getETag *string + getLastModified *time.Time putOut *s3.PutObjectOutput putErr error @@ -99,7 +102,7 @@ func (f *fakeS3API) GetObject(_ context.Context, params *s3.GetObjectInput, _ .. if body == nil { body = io.NopCloser(strings.NewReader("")) } - return &s3.GetObjectOutput{Body: body}, nil + return &s3.GetObjectOutput{Body: body, ContentLength: f.getSize, ETag: f.getETag, LastModified: f.getLastModified}, nil } func (f *fakeS3API) PutObject(_ context.Context, params *s3.PutObjectInput, _ ...func(*s3.Options)) (*s3.PutObjectOutput, error) { @@ -171,6 +174,33 @@ func TestS3BackendDownloadCreatesParentDirectory(t *testing.T) { } } +func TestS3BackendReadReturnsOpenedObjectMetadata(t *testing.T) { + lastModified := time.Date(2026, 8, 11, 1, 2, 3, 0, time.UTC) + client := &fakeS3API{ + getBody: io.NopCloser(strings.NewReader("locks")), + getSize: int64Ptr(5), + getETag: strPtr(`"generation"`), + getLastModified: &lastModified, + } + backend := &S3Backend{bucket: "bucket-1", client: client} + + info, body, err := backend.Read(context.Background(), `sessions\locks.yml`) + if err != nil { + t.Fatalf("Read() error = %v", err) + } + data, readErr := io.ReadAll(body) + closeErr := body.Close() + if readErr != nil || closeErr != nil { + t.Fatalf("read body error=%v close error=%v", readErr, closeErr) + } + if info.Key != "sessions/locks.yml" || info.Size != 5 || info.ETag != "generation" || info.LastModified == nil || !info.LastModified.Equal(lastModified) { + t.Fatalf("Read() info = %#v, want opened object metadata", info) + } + if string(data) != "locks" || client.lastGet == nil || *client.lastGet.Key != "sessions/locks.yml" { + t.Fatalf("Read() data=%q request=%#v", data, client.lastGet) + } +} + func TestS3BackendUploadAndExists(t *testing.T) { client := &fakeS3API{putOut: &s3.PutObjectOutput{ETag: strPtr(`"etag123"`)}} backend := &S3Backend{bucket: "bucket-1", client: client} diff --git a/internal/app/operator_helpers_test.go b/internal/app/operator_helpers_test.go index cee1bd4..0ad4413 100644 --- a/internal/app/operator_helpers_test.go +++ b/internal/app/operator_helpers_test.go @@ -576,6 +576,58 @@ func TestMutateRemoteLockStoreRetainsConcurrentUpdates(t *testing.T) { } } +func TestLoadRemoteLockStoreAcceptsExactLimitAndRejectsLimitPlusOne(t *testing.T) { + cfg := &config.Config{ + Pipeline: &config.PipelineConfig{Storage: config.StorageConfig{S3: &config.StorageS3Config{Bucket: "bucket", RootPrefix: "root"}}}, + Session: &config.SessionConfig{Campaign: "campaign", SessionID: "session"}, + } + key, err := remoteLocksKey(cfg) + if err != nil { + t.Fatalf("remoteLocksKey() error = %v", err) + } + encoded, err := config.MarshalPublishLockStore(&config.PublishLockStore{}) + if err != nil { + t.Fatalf("MarshalPublishLockStore() error = %v", err) + } + exact := append(append([]byte(nil), encoded...), bytes.Repeat([]byte(" "), int(MaxRemoteLockStoreBytes)-len(encoded))...) + store := &storage.FakeBackend{} + store.SeedObject(storage.FakeObject{Key: key, Data: exact, ETag: "lock-generation"}) + + locks, gotKey, generation, err := loadRemoteLockStore(context.Background(), cfg, store) + if err != nil { + t.Fatalf("loadRemoteLockStore() exact-limit error = %v", err) + } + if locks == nil || gotKey != key || generation != "lock-generation" { + t.Fatalf("loadRemoteLockStore() = (%#v, %q, %q), want decoded locks and opened generation", locks, gotKey, generation) + } + if len(store.Downloads) != 0 || len(store.Reads) != 1 || store.Reads[0].Key != key { + t.Fatalf("lock transfers reads=%#v downloads=%#v, want one direct read", store.Reads, store.Downloads) + } + + store.SeedObject(storage.FakeObject{Key: key, Data: append(exact, ' '), ETag: "new-generation"}) + _, _, _, err = loadRemoteLockStore(context.Background(), cfg, store) + if err == nil || !strings.Contains(err.Error(), "remote lock control object") || !strings.Contains(err.Error(), key) || !strings.Contains(err.Error(), fmt.Sprint(MaxRemoteLockStoreBytes)) { + t.Fatalf("loadRemoteLockStore() limit-plus-one error = %v, want category, key, and limit", err) + } +} + +func TestLoadRemoteLockStoreRejectsMalformedYAML(t *testing.T) { + cfg := &config.Config{ + Pipeline: &config.PipelineConfig{Storage: config.StorageConfig{S3: &config.StorageS3Config{Bucket: "bucket", RootPrefix: "root"}}}, + Session: &config.SessionConfig{Campaign: "campaign", SessionID: "session"}, + } + key, err := remoteLocksKey(cfg) + if err != nil { + t.Fatalf("remoteLocksKey() error = %v", err) + } + store := &storage.FakeBackend{} + store.SeedObject(storage.FakeObject{Key: key, Data: []byte("locks: [\n"), ETag: "lock-generation"}) + + if _, _, _, err := loadRemoteLockStore(context.Background(), cfg, store); err == nil { + t.Fatal("loadRemoteLockStore() error = nil, want malformed YAML failure") + } +} + func TestMutateRemoteLockStoreHonorsCancellation(t *testing.T) { cfg := &config.Config{ Pipeline: &config.PipelineConfig{Storage: config.StorageConfig{S3: &config.StorageS3Config{Bucket: "bucket", RootPrefix: "root"}}}, diff --git a/internal/app/remote_locks.go b/internal/app/remote_locks.go index 9f8dd78..e640100 100644 --- a/internal/app/remote_locks.go +++ b/internal/app/remote_locks.go @@ -5,7 +5,6 @@ import ( "context" "errors" "fmt" - "io" "os" "path/filepath" "strings" @@ -23,7 +22,11 @@ type effectiveLocks struct { Key string } -const remoteLockMutationAttempts = 4 +const ( + remoteLockMutationAttempts = 4 + // MaxRemoteLockStoreBytes bounds the mutable remote publish-lock document. + MaxRemoteLockStoreBytes int64 = 1 << 20 +) func remoteLocksKey(cfg *config.Config) (string, error) { if cfg == nil || cfg.Pipeline == nil || cfg.Session == nil { @@ -48,21 +51,20 @@ func loadRemoteLockStore(ctx context.Context, cfg *config.Config, store storage. if store == nil { return nil, key, "", fmt.Errorf("remote lock store is required") } - info, body, err := store.Read(ctx, key) + info, data, err := storage.ReadObjectBounded(ctx, store, key, MaxRemoteLockStoreBytes) if err != nil { if errors.Is(err, os.ErrNotExist) { return &config.PublishLockStore{}, key, "", nil } + var limitErr *storage.ReadLimitError + if errors.As(err, &limitErr) { + return nil, key, "", fmt.Errorf("read remote lock control object %q with %d-byte limit: %w", key, MaxRemoteLockStoreBytes, err) + } return nil, key, "", fmt.Errorf("read remote locks %q: %w", key, err) } - defer body.Close() if strings.TrimSpace(info.ETag) == "" { return nil, key, "", fmt.Errorf("read remote locks %q: object has no generation", key) } - data, err := io.ReadAll(body) - if err != nil { - return nil, key, "", fmt.Errorf("read remote locks %q: %w", key, err) - } lockStore, err := config.LoadPublishLockStoreBytes("s3://"+s3BucketName(cfg.Pipeline)+"/"+key, data, cfg.Pipeline.Scriptorium, cfg.Pipeline.Notarius) if err != nil { return nil, key, "", err diff --git a/internal/app/restore_commit_test.go b/internal/app/restore_commit_test.go index 22cb6b8..6535496 100644 --- a/internal/app/restore_commit_test.go +++ b/internal/app/restore_commit_test.go @@ -55,11 +55,11 @@ func TestCommittedRestoreReusesVerifiedManifestCandidate(t *testing.T) { if _, err := executeRestorePlan(context.Background(), cfg, current, plan, nil, fake); err != nil { t.Fatalf("executeRestorePlan() error = %v", err) } - if got := fakeDownloadCount(fake, current.CurrentManifestKey); got != 1 { - t.Fatalf("manifest downloads = %d, want one discovery transfer reused by restore", got) + if got := fakeReadCount(fake, current.CurrentManifestKey); got != 1 { + t.Fatalf("manifest reads = %d, want one discovery read reused by restore", got) } - if got := fakeDownloadBytes(fake, current.CurrentManifestKey); got != int64(len(current.ManifestData)) { - t.Fatalf("manifest bytes transferred = %d, want one verified candidate (%d)", got, len(current.ManifestData)) + if got := fakeDownloadCount(fake, current.CurrentManifestKey); got != 0 { + t.Fatalf("manifest downloads = %d, want no temporary download", got) } } @@ -77,8 +77,8 @@ func TestCommittedRestoreRejectsChangedGenerationForVerifiedManifestCandidate(t if err == nil || !strings.Contains(err.Error(), "generation mismatch") { t.Fatalf("executeRestorePlan() error = %v, want generation mismatch", err) } - if got := fakeDownloadCount(fake, current.CurrentManifestKey); got != 1 { - t.Fatalf("manifest downloads = %d, want no second transfer for rejected candidate", got) + if got := fakeReadCount(fake, current.CurrentManifestKey); got != 1 { + t.Fatalf("manifest reads = %d, want no second transfer for rejected candidate", got) } } @@ -292,11 +292,11 @@ func remoteRestoreArtifact(fake *storage.FakeBackend, artifactType artifacts.Rem } } -func fakeDownloadBytes(fake *storage.FakeBackend, key string) int64 { - var count int64 - for _, call := range fake.Downloads { +func fakeReadCount(fake *storage.FakeBackend, key string) int { + count := 0 + for _, call := range fake.Reads { if call.Key == key { - count += call.Bytes + count++ } } return count diff --git a/internal/app/restore_discovery_test.go b/internal/app/restore_discovery_test.go index 2ec5b5c..fb01a62 100644 --- a/internal/app/restore_discovery_test.go +++ b/internal/app/restore_discovery_test.go @@ -152,23 +152,18 @@ func TestDiscoverRemoteCurrentStateUsesCurrentKeysUnderSessionPrefix(t *testing. expectedRunKey := fmt.Sprintf("%scurrent/run_id.txt", sessionPrefix) expectedManifestKey := fmt.Sprintf("%scurrent/manifest.json", sessionPrefix) - if !containsString(store.existsKeys, expectedRunKey) { - t.Fatalf("exists keys = %#v, want run pointer key %q", store.existsKeys, expectedRunKey) + if !containsString(store.readKeys, expectedRunKey) { + t.Fatalf("read keys = %#v, want run pointer key %q", store.readKeys, expectedRunKey) } - if !containsString(store.existsKeys, expectedManifestKey) { - t.Fatalf("exists keys = %#v, want manifest key %q", store.existsKeys, expectedManifestKey) - } - if !containsString(store.downloadKeys, expectedRunKey) { - t.Fatalf("download keys = %#v, want run pointer key %q", store.downloadKeys, expectedRunKey) - } - if !containsString(store.downloadKeys, expectedManifestKey) { - t.Fatalf("download keys = %#v, want manifest key %q", store.downloadKeys, expectedManifestKey) + if !containsString(store.readKeys, expectedManifestKey) { + t.Fatalf("read keys = %#v, want manifest key %q", store.readKeys, expectedManifestKey) } } type captureObjectStore struct { delegate storage.ObjectStore existsKeys []string + readKeys []string downloadKeys []string } @@ -177,7 +172,7 @@ func (s *captureObjectStore) List(ctx context.Context, prefix string) ([]storage } func (s *captureObjectStore) Read(ctx context.Context, key string) (storage.ObjectInfo, io.ReadCloser, error) { - s.downloadKeys = append(s.downloadKeys, key) + s.readKeys = append(s.readKeys, key) return s.delegate.Read(ctx, key) } diff --git a/internal/app/restore_execution_test.go b/internal/app/restore_execution_test.go index c090506..c03d2d7 100644 --- a/internal/app/restore_execution_test.go +++ b/internal/app/restore_execution_test.go @@ -4,8 +4,6 @@ import ( "bytes" "context" "encoding/json" - "fmt" - "io" "os" "path/filepath" "strings" @@ -360,22 +358,12 @@ func TestExecuteRestoreLockConflictFailsAndWritesNothing(t *testing.T) { } func TestExecuteRestoreInvalidManifestDoesNotCorruptExistingManifest(t *testing.T) { - workspaceRoot := t.TempDir() - pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot) - - base := &storage.FakeBackend{} - cfg, sessionPrefix, manifestKey, _ := seedRestoreCommittedState(t, base, pipelinePath, campaignPath, sessionPath) - seedRestoreObject(base, sessionPrefix+"transcripts/full.json", []byte("remote-transcript")) - - toggled := &stagedManifestDownloadStore{ - delegate: base, - manifestKey: manifestKey, - firstManifest: restoreManifestJSON(t, cfg.Session.SessionID, cfg.Session.Campaign), - secondManifest: []byte("{invalid json"), - manifestReads: 0, - } - - sessionRoot := artifacts.SessionWorkDirForCampaign(workspaceRoot, cfg.Session.Campaign, cfg.Session.SessionID) + cfg := restorePlanConfig(t) + store := &storage.FakeBackend{} + current := seedCommittedRestoreSnapshot(t, cfg, store, "20260519T010203Z-a1b2c3d4", map[string][]byte{ + "transcripts/full.json": []byte("remote-transcript"), + }) + sessionRoot := artifacts.SessionWorkDirForCampaign(cfg.Pipeline.Workspace.Root, cfg.Session.Campaign, cfg.Session.SessionID) existing := manifest.New(cfg.Session.SessionID, nowUTC()) existing.Campaign = cfg.Session.Campaign existingPath := filepath.Join(sessionRoot, "manifest.json") @@ -388,25 +376,38 @@ func TestExecuteRestoreInvalidManifestDoesNotCorruptExistingManifest(t *testing. t.Fatalf("read existing local manifest: %v", err) } - restoreWithStoreAndRealPhases(t, toggled) - - var stdout bytes.Buffer - var stderr bytes.Buffer - code := Execute([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--force"}, &stdout, &stderr) - if code == 0 { - t.Fatal("exit code = 0, want non-zero") + plan, err := buildRestorePlan(context.Background(), cfg, current, store, RestorePlanOptions{Force: true}) + if err != nil { + t.Fatalf("buildRestorePlan() error = %v", err) } - if !strings.Contains(stderr.String(), "validate manifest decode") { - t.Fatalf("stderr = %q, want manifest validation failure", stderr.String()) + invalidManifest := []byte("{invalid json") + foundManifest := false + manifestKey := "" + manifestGeneration := "" + for index := range plan.Actions { + if plan.Actions[index].LocalRelativePath != config.PathManifestFile { + continue + } + foundManifest = true + plan.Actions[index].VerifiedContent = invalidManifest + plan.Actions[index].SHA256 = restoreCommitSHA256(invalidManifest) + plan.Actions[index].Size = int64(len(invalidManifest)) + manifestKey = plan.Actions[index].RemoteKey + manifestGeneration = plan.Actions[index].Generation + } + if !foundManifest { + t.Fatal("restore plan has no manifest action") + } + store.SeedObject(storage.FakeObject{Key: manifestKey, Data: invalidManifest, ETag: manifestGeneration}) + report, err := newRestoreReport(current, plan, RestorePlanOptions{Force: true}) + if err != nil { + t.Fatalf("newRestoreReport() error = %v", err) + } + _, err = executeRestorePlan(context.Background(), cfg, current, plan, report, store) + if err == nil || !strings.Contains(err.Error(), "validate manifest decode") { + t.Fatalf("executeRestorePlan() error = %v, want manifest validation failure", err) } mustReadEquals(t, filepath.Join(sessionRoot, "transcripts", "full.json"), "remote-transcript") - report := mustReadRestoreReport(t, filepath.Join(sessionRoot, "reports", "restore-latest.json")) - if report.Status != "failed" { - t.Fatalf("report status = %q, want failed", report.Status) - } - if strings.TrimSpace(report.Error) == "" { - t.Fatal("report error is empty, want failure context") - } afterData, err := os.ReadFile(existingPath) if err != nil { t.Fatalf("read local manifest after failure: %v", err) @@ -414,9 +415,6 @@ func TestExecuteRestoreInvalidManifestDoesNotCorruptExistingManifest(t *testing. if string(afterData) != string(existingData) { t.Fatalf("local manifest changed after failed restore; before=%q after=%q", string(existingData), string(afterData)) } - if _, err := os.Stat(artifacts.SessionRestoreMarkerPathForCampaign(workspaceRoot, cfg.Session.Campaign, cfg.Session.SessionID)); err != nil { - t.Fatalf("incomplete restore marker should remain after failed forced restore: %v", err) - } } func TestExecuteRestorePlanPathMismatchFails(t *testing.T) { @@ -562,62 +560,3 @@ func fakeDownloadCount(fake *storage.FakeBackend, key string) int { } return count } - -type stagedManifestDownloadStore struct { - delegate *storage.FakeBackend - manifestKey string - firstManifest []byte - secondManifest []byte - manifestReads int -} - -func (s *stagedManifestDownloadStore) List(ctx context.Context, prefix string) ([]storage.ObjectInfo, error) { - return s.delegate.List(ctx, prefix) -} - -func (s *stagedManifestDownloadStore) Read(ctx context.Context, key string) (storage.ObjectInfo, io.ReadCloser, error) { - return s.delegate.Read(ctx, key) -} - -func (s *stagedManifestDownloadStore) Download(ctx context.Context, key, localPath string) error { - if strings.TrimSpace(key) == strings.TrimSpace(s.manifestKey) { - s.manifestReads++ - payload := s.secondManifest - if s.manifestReads <= 1 { - payload = s.firstManifest - } - if err := os.MkdirAll(filepath.Dir(localPath), 0o755); err != nil { - return fmt.Errorf("download staged manifest: create parent: %w", err) - } - if err := os.WriteFile(localPath, payload, 0o644); err != nil { - return fmt.Errorf("download staged manifest: write local file: %w", err) - } - return nil - } - return s.delegate.Download(ctx, key, localPath) -} - -func (s *stagedManifestDownloadStore) DownloadTo(ctx context.Context, key string, destination io.Writer) error { - if strings.TrimSpace(key) == strings.TrimSpace(s.manifestKey) { - s.manifestReads++ - payload := s.secondManifest - if s.manifestReads <= 1 { - payload = s.firstManifest - } - _, err := destination.Write(payload) - return err - } - return storage.DownloadTo(ctx, s.delegate, key, destination) -} - -func (s *stagedManifestDownloadStore) Upload(ctx context.Context, localPath, key string, opts storage.UploadOptions) (storage.ObjectInfo, error) { - return s.delegate.Upload(ctx, localPath, key, opts) -} - -func (s *stagedManifestDownloadStore) UploadConditional(ctx context.Context, source io.Reader, key string, opts storage.UploadOptions, condition storage.WriteCondition) (storage.ObjectInfo, error) { - return s.delegate.UploadConditional(ctx, source, key, opts, condition) -} - -func (s *stagedManifestDownloadStore) Exists(ctx context.Context, key string) (bool, error) { - return s.delegate.Exists(ctx, key) -} diff --git a/internal/artifacts/current_state_commit.go b/internal/artifacts/current_state_commit.go index e0bab04..f07558b 100644 --- a/internal/artifacts/current_state_commit.go +++ b/internal/artifacts/current_state_commit.go @@ -1,15 +1,24 @@ package artifacts import ( + "bytes" "context" "fmt" - "os" "strings" "gitea.maximumdirect.net/eric/narratio/internal/adapters/storage" "gitea.maximumdirect.net/eric/narratio/internal/manifest" ) +const ( + // MaxCurrentCommitPointerBytes bounds the mutable commit-selection record. + MaxCurrentCommitPointerBytes int64 = 64 << 10 + // MaxRemoteCommitManifestBytes bounds an immutable commit manifest. + MaxRemoteCommitManifestBytes int64 = 4 << 20 + // MaxRemoteSessionManifestBytes bounds the selected session manifest. + MaxRemoteSessionManifestBytes int64 = 8 << 20 +) + func loadCommittedCurrentState( ctx context.Context, store storage.ObjectStore, @@ -17,9 +26,9 @@ func loadCommittedCurrentState( pointerKey string, validation CurrentStateValidation, ) (*CurrentState, error) { - pointerData, err := downloadRemoteObject(ctx, store, pointerKey, "narratio-current-commit-pointer-*.json") + _, pointerData, err := readCurrentStateControlObject(ctx, store, pointerKey, "current commit pointer", MaxCurrentCommitPointerBytes) if err != nil { - return nil, fmt.Errorf("download current commit pointer %q: %w", pointerKey, err) + return nil, err } pointer, err := DecodeCurrentCommitPointer(pointerData) if err != nil { @@ -29,7 +38,7 @@ func loadCommittedCurrentState( return nil, err } - commitData, err := readVerifiedRemoteObject(ctx, store, pointer.CommitKey, pointer.CommitSHA256, pointer.CommitSize, pointer.CommitGeneration, "narratio-remote-commit-*.json") + commitData, err := readVerifiedRemoteObject(ctx, store, pointer.CommitKey, pointer.CommitSHA256, pointer.CommitSize, pointer.CommitGeneration, "remote commit manifest", MaxRemoteCommitManifestBytes) if err != nil { return nil, fmt.Errorf("read selected remote commit %q: %w", pointer.CommitKey, err) } @@ -48,7 +57,7 @@ func loadCommittedCurrentState( if !ok { return nil, fmt.Errorf("remote commit must declare exactly one session manifest artifact") } - manifestData, err := readVerifiedRemoteObject(ctx, store, sessionManifest.DestinationKey, sessionManifest.SHA256, sessionManifest.Size, sessionManifest.Generation, "narratio-remote-session-manifest-*.json") + manifestData, err := readVerifiedRemoteObject(ctx, store, sessionManifest.DestinationKey, sessionManifest.SHA256, sessionManifest.Size, sessionManifest.Generation, "committed session manifest", MaxRemoteSessionManifestBytes) if err != nil { return nil, fmt.Errorf("read committed session manifest %q: %w", sessionManifest.DestinationKey, err) } @@ -106,9 +115,10 @@ func readVerifiedRemoteObject( wantSHA256 string, wantSize int64, wantGeneration string, - tempPattern string, + category string, + maxBytes int64, ) ([]byte, error) { - data, err := downloadRemoteObject(ctx, store, key, tempPattern) + info, data, err := readCurrentStateControlObject(ctx, store, key, category, maxBytes) if err != nil { return nil, err } @@ -118,11 +128,7 @@ func readVerifiedRemoteObject( if checksum := remoteObjectSHA256(data); checksum != wantSHA256 { return nil, fmt.Errorf("checksum mismatch: got %s, want %s", checksum, wantSHA256) } - info, err := remoteObjectInfo(ctx, store, key) - if err != nil { - return nil, err - } - if info.Size != wantSize { + if info.Size > 0 && info.Size != wantSize { return nil, fmt.Errorf("storage size mismatch: got %d, want %d", info.Size, wantSize) } if strings.TrimSpace(info.ETag) != wantGeneration { @@ -131,56 +137,16 @@ func readVerifiedRemoteObject( return data, nil } -func downloadRemoteObject(ctx context.Context, store storage.ObjectStore, key, tempPattern string) ([]byte, error) { - localPath, err := storage.DownloadObjectToTemp(ctx, store, key, tempPattern) +func readCurrentStateControlObject(ctx context.Context, store storage.ObjectStore, key, category string, maxBytes int64) (storage.ObjectInfo, []byte, error) { + info, data, err := storage.ReadObjectBounded(ctx, store, key, maxBytes) if err != nil { - return nil, err + return storage.ObjectInfo{}, nil, fmt.Errorf("read %s control object %q with %d-byte limit: %w", category, key, maxBytes, err) } - defer func() { _ = os.Remove(localPath) }() - data, err := os.ReadFile(localPath) - if err != nil { - return nil, fmt.Errorf("read downloaded object %q: %w", key, err) - } - return data, nil -} - -func remoteObjectInfo(ctx context.Context, store storage.ObjectStore, key string) (storage.ObjectInfo, error) { - objects, err := store.List(ctx, key) - if err != nil { - return storage.ObjectInfo{}, fmt.Errorf("list remote object %q: %w", key, err) - } - var found *storage.ObjectInfo - for _, object := range objects { - if object.Key != key { - continue - } - if found != nil { - return storage.ObjectInfo{}, fmt.Errorf("remote object %q is ambiguous", key) - } - copy := object - found = © - } - if found == nil { - return storage.ObjectInfo{}, fmt.Errorf("remote object %q is missing", key) - } - return *found, nil + return info, data, nil } func decodeCommittedManifest(ctx context.Context, data []byte) (*manifest.Manifest, error) { - file, err := os.CreateTemp("", "narratio-committed-session-manifest-*.json") - if err != nil { - return nil, fmt.Errorf("create committed manifest file: %w", err) - } - path := file.Name() - defer func() { _ = os.Remove(path) }() - if _, err := file.Write(data); err != nil { - _ = file.Close() - return nil, fmt.Errorf("write committed manifest file: %w", err) - } - if err := file.Close(); err != nil { - return nil, fmt.Errorf("close committed manifest file: %w", err) - } - m, err := (&manifest.LocalStore{}).Load(ctx, path) + m, err := (&manifest.LocalStore{}).LoadReader(ctx, bytes.NewReader(data)) if err != nil { return nil, fmt.Errorf("committed session manifest decode failed: %w", err) } diff --git a/internal/artifacts/current_state_legacy.go b/internal/artifacts/current_state_legacy.go index 862c402..3f01663 100644 --- a/internal/artifacts/current_state_legacy.go +++ b/internal/artifacts/current_state_legacy.go @@ -1,7 +1,9 @@ package artifacts import ( + "bytes" "context" + "errors" "fmt" "os" "strings" @@ -10,6 +12,13 @@ import ( "gitea.maximumdirect.net/eric/narratio/internal/manifest" ) +const ( + // MaxLegacyCurrentRunPointerBytes bounds the compatibility run selector. + MaxLegacyCurrentRunPointerBytes int64 = 4 << 10 + // MaxLegacyCurrentManifestBytes bounds the compatibility session manifest. + MaxLegacyCurrentManifestBytes int64 = MaxRemoteSessionManifestBytes +) + // This file is temporary compatibility support for sessions published before // immutable current commits. It can be removed after legacy remote state is migrated. @@ -48,17 +57,12 @@ func loadLegacyCurrentRunPointer(ctx context.Context, store storage.ObjectStore, return "", fmt.Errorf("current run pointer key is required") } - exists, err := store.Exists(ctx, key) + _, data, err := readCurrentStateControlObject(ctx, store, key, "legacy current run pointer", MaxLegacyCurrentRunPointerBytes) if err != nil { - return "", fmt.Errorf("check current run pointer %q: %w", key, err) - } - if !exists { - return "", &CurrentRunPointerMissingError{Key: key} - } - - data, err := downloadRemoteObject(ctx, store, key, "narratio-legacy-current-run-id-*.txt") - if err != nil { - return "", fmt.Errorf("download current run pointer %q: %w", key, err) + if errors.Is(err, os.ErrNotExist) { + return "", &CurrentRunPointerMissingError{Key: key} + } + return "", err } runID := strings.TrimSpace(string(data)) if runID == "" { @@ -76,21 +80,15 @@ func loadLegacyCurrentManifest(ctx context.Context, store storage.ObjectStore, c return nil, fmt.Errorf("current manifest key is required") } - exists, err := store.Exists(ctx, key) + _, data, err := readCurrentStateControlObject(ctx, store, key, "legacy current manifest", MaxLegacyCurrentManifestBytes) if err != nil { - return nil, fmt.Errorf("check current manifest %q: %w", key, err) - } - if !exists { - return nil, &CurrentManifestMissingError{Key: key} + if errors.Is(err, os.ErrNotExist) { + return nil, &CurrentManifestMissingError{Key: key} + } + return nil, err } - localPath, err := storage.DownloadObjectToTemp(ctx, store, key, "narratio-legacy-current-manifest-*.json") - if err != nil { - return nil, fmt.Errorf("download current manifest %q: %w", key, err) - } - defer func() { _ = os.Remove(localPath) }() - - m, err := (&manifest.LocalStore{}).Load(ctx, localPath) + m, err := (&manifest.LocalStore{}).LoadReader(ctx, bytes.NewReader(data)) if err != nil { return nil, fmt.Errorf("current manifest decode failed: %w", err) } diff --git a/internal/artifacts/current_state_test.go b/internal/artifacts/current_state_test.go index 920c231..173b828 100644 --- a/internal/artifacts/current_state_test.go +++ b/internal/artifacts/current_state_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "strings" "testing" "time" @@ -119,6 +120,50 @@ func TestLoadCurrentStateReadsCoherentLegacyPair(t *testing.T) { if state.Commit != nil || state.Pointer != nil { t.Fatalf("legacy current state unexpectedly includes immutable commit data: %#v", state) } + if len(store.Downloads) != 0 { + t.Fatalf("legacy current-state downloads = %#v, want direct bounded reads", store.Downloads) + } + wantReads := []string{state.CurrentRunIDKey, state.CurrentManifestKey} + if len(store.Reads) != len(wantReads) { + t.Fatalf("legacy current-state reads = %#v, want %q", store.Reads, wantReads) + } + for index, want := range wantReads { + if store.Reads[index].Key != want { + t.Fatalf("legacy current-state read %d = %q, want %q", index, store.Reads[index].Key, want) + } + } +} + +func TestCurrentStateControlObjectLimitsAcceptExactAndRejectLimitPlusOne(t *testing.T) { + tests := []struct { + name string + category string + key string + limit int64 + }{ + {name: "commit pointer", category: "current commit pointer", key: "current/commit-pointer.json", limit: MaxCurrentCommitPointerBytes}, + {name: "commit manifest", category: "remote commit manifest", key: "runs/run/commit.json", limit: MaxRemoteCommitManifestBytes}, + {name: "session manifest", category: "committed session manifest", key: "runs/run/session-manifest.json", limit: MaxRemoteSessionManifestBytes}, + {name: "legacy run pointer", category: "legacy current run pointer", key: "current/run_id.txt", limit: MaxLegacyCurrentRunPointerBytes}, + {name: "legacy manifest", category: "legacy current manifest", key: "current/manifest.json", limit: MaxLegacyCurrentManifestBytes}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + store := &storage.FakeBackend{} + exact := strings.Repeat("x", int(test.limit)) + store.SeedObject(storage.FakeObject{Key: test.key, Data: []byte(exact)}) + _, data, err := readCurrentStateControlObject(context.Background(), store, test.key, test.category, test.limit) + if err != nil || string(data) != exact { + t.Fatalf("exact-limit read data length=%d error=%v", len(data), err) + } + + store.SeedObject(storage.FakeObject{Key: test.key, Data: []byte(exact + "x")}) + _, _, err = readCurrentStateControlObject(context.Background(), store, test.key, test.category, test.limit) + if err == nil || !strings.Contains(err.Error(), test.category) || !strings.Contains(err.Error(), test.key) || !strings.Contains(err.Error(), fmt.Sprint(test.limit)) { + t.Fatalf("limit-plus-one error = %v, want category, key, and limit", err) + } + }) + } } func TestLoadCurrentStateRejectsTornLegacyPair(t *testing.T) { diff --git a/internal/artifacts/remote_commit_test.go b/internal/artifacts/remote_commit_test.go index 9f054a3..fbdf23f 100644 --- a/internal/artifacts/remote_commit_test.go +++ b/internal/artifacts/remote_commit_test.go @@ -3,6 +3,7 @@ package artifacts import ( "bytes" "context" + "errors" "strings" "testing" @@ -67,6 +68,28 @@ func TestLoadCurrentStateReadsVerifiedRemoteCommit(t *testing.T) { if state.Manifest.RunID != commit.RunID { t.Fatalf("manifest run ID = %q, want %q", state.Manifest.RunID, commit.RunID) } + if len(store.Downloads) != 0 { + t.Fatalf("committed current-state downloads = %#v, want direct bounded reads", store.Downloads) + } + wantReads := []string{state.CurrentPointerKey, state.Pointer.CommitKey, state.CurrentManifestKey} + if len(store.Reads) != len(wantReads) { + t.Fatalf("committed current-state reads = %#v, want %q", store.Reads, wantReads) + } + for index, want := range wantReads { + if store.Reads[index].Key != want { + t.Fatalf("committed current-state read %d = %q, want %q", index, store.Reads[index].Key, want) + } + } +} + +func TestLoadCurrentStateUsesMetadataFromOpenedObjectVersion(t *testing.T) { + store := &storage.FakeBackend{ListErr: errors.New("list must not be used for object verification")} + commit := testRemoteCommit(t, "20260519T010203Z-a1b2c3d4") + seedRemoteCommit(t, store, testCurrentSessionPrefix(), commit, "commit-generation", "manifest-generation") + + if _, err := LoadCurrentState(context.Background(), store, testCurrentSessionPrefix(), CurrentStateValidation{}); err != nil { + t.Fatalf("LoadCurrentState() error = %v, want verification from Read metadata", err) + } } func TestLoadCurrentStateRejectsPointerCommitIdentityMismatch(t *testing.T) { diff --git a/internal/stage/prepare_previous_test.go b/internal/stage/prepare_previous_test.go index f10b29e..4ae19f3 100644 --- a/internal/stage/prepare_previous_test.go +++ b/internal/stage/prepare_previous_test.go @@ -218,23 +218,17 @@ func TestHydratePreviousSessionArtifactsUsesExplicitStorageKeys(t *testing.T) { t.Fatalf("hydratePreviousSessionArtifacts() error = %v", err) } - if !containsString(capture.existsKeys, seed.RunPointerKey) { - t.Fatalf("exists keys = %#v, want %q", capture.existsKeys, seed.RunPointerKey) - } - if !containsString(capture.existsKeys, seed.ManifestKey) { - t.Fatalf("exists keys = %#v, want %q", capture.existsKeys, seed.ManifestKey) - } if !containsString(capture.existsKeys, seed.ArtifactKey) { t.Fatalf("exists keys = %#v, want %q", capture.existsKeys, seed.ArtifactKey) } - if !containsString(capture.downloadKeys, seed.RunPointerKey) { - t.Fatalf("download keys = %#v, want %q", capture.downloadKeys, seed.RunPointerKey) + if !containsString(capture.accessKeys, seed.RunPointerKey) { + t.Fatalf("access keys = %#v, want %q", capture.accessKeys, seed.RunPointerKey) } - if !containsString(capture.downloadKeys, seed.ManifestKey) { - t.Fatalf("download keys = %#v, want %q", capture.downloadKeys, seed.ManifestKey) + if !containsString(capture.accessKeys, seed.ManifestKey) { + t.Fatalf("access keys = %#v, want %q", capture.accessKeys, seed.ManifestKey) } - if !containsString(capture.downloadKeys, seed.ArtifactKey) { - t.Fatalf("download keys = %#v, want %q", capture.downloadKeys, seed.ArtifactKey) + if !containsString(capture.accessKeys, seed.ArtifactKey) { + t.Fatalf("access keys = %#v, want %q", capture.accessKeys, seed.ArtifactKey) } } @@ -378,9 +372,9 @@ func buildPreviousManifestForSeed( } type preparePreviousCaptureStore struct { - delegate storage.ObjectStore - existsKeys []string - downloadKeys []string + delegate storage.ObjectStore + existsKeys []string + accessKeys []string } func (s *preparePreviousCaptureStore) List(ctx context.Context, prefix string) ([]storage.ObjectInfo, error) { @@ -388,17 +382,17 @@ func (s *preparePreviousCaptureStore) List(ctx context.Context, prefix string) ( } func (s *preparePreviousCaptureStore) Read(ctx context.Context, key string) (storage.ObjectInfo, io.ReadCloser, error) { - s.downloadKeys = append(s.downloadKeys, key) + s.accessKeys = append(s.accessKeys, key) return s.delegate.Read(ctx, key) } func (s *preparePreviousCaptureStore) Download(ctx context.Context, key, localPath string) error { - s.downloadKeys = append(s.downloadKeys, key) + s.accessKeys = append(s.accessKeys, key) return s.delegate.Download(ctx, key, localPath) } func (s *preparePreviousCaptureStore) DownloadTo(ctx context.Context, key string, destination io.Writer) error { - s.downloadKeys = append(s.downloadKeys, key) + s.accessKeys = append(s.accessKeys, key) return storage.DownloadTo(ctx, s.delegate, key, destination) }