admin: read-only S3 bucket endpoints (P2 slice 1)

adapter/s3_admin.go

@@ -0,0 +1,177 @@
+package adapter
+
+import (
+	"bytes"
+	"context"
+	"sort"
+
+	"github.com/bootjp/elastickv/internal/s3keys"
+	"github.com/bootjp/elastickv/store"
+	"github.com/cockroachdb/errors"
+)
+
+// adminBucketScanPage is the per-iteration ScanAt page size used by
+// AdminListBuckets. Smaller than s3MaxKeys is unnecessary —
+// ScanAt's per-call memory budget is already this size on the
+// SigV4 listBuckets path — but we use it as a named constant so the
+// loop's intent is explicit. AdminListBuckets accumulates pages
+// until the prefix is exhausted, so the total returned size is
+// bounded by the cluster's bucket count rather than this knob.
+const adminBucketScanPage = 1000
+
+// AdminBucketSummary is the bucket-level information the admin
+// dashboard surfaces. It deliberately projects only the fields the
+// dashboard needs so the package's wire-format types
+// (s3BucketMeta, s3ListBucketsResult) stay internal.
+//
+// CreatedAtHLC is the same physical-time-bearing HLC the bucket
+// metadata persists; the admin HTTP handler formats it for the SPA.
+// ACL is the canned-ACL string ("private" / "public-read") — the
+// admin layer does not expand it into the AWS ACL XML grant tree
+// because the dashboard renders the canned form directly.
+type AdminBucketSummary struct {
+	Name         string
+	ACL          string
+	CreatedAtHLC uint64
+	Generation   uint64
+	Region       string
+	Owner        string
+}
+
+// AdminListBuckets returns every S3-style bucket this server knows
+// about, in lexicographic order (the metadata-prefix scan natural
+// ordering). Intended for the in-process admin listener as the
+// SigV4-free counterpart to the listBuckets HTTP handler.
+//
+// Unlike the SigV4 path (which intentionally caps each call at
+// s3MaxKeys = 1000 because the AWS API is page-based), the admin
+// dashboard's pagination is implemented at the handler layer, which
+// expects this method to return the full set. We loop the per-page
+// ScanAt until the metadata prefix is exhausted — same pattern as
+// scanAllByPrefixAt on the Dynamo side (Codex P1 + Claude Issue 1
+// on PR #658).
+//
+// Returns an empty slice (not nil) when no buckets exist so JSON
+// callers see `[]` instead of `null`.
+func (s *S3Server) AdminListBuckets(ctx context.Context) ([]AdminBucketSummary, error) {
+	readTS := s.readTS()
+	readPin := s.pinReadTS(readTS)
+	defer readPin.Release()
+	prefix := []byte(s3keys.BucketMetaPrefix)
+	end := prefixScanEnd(prefix)
+	start := bytes.Clone(prefix)
+	out := make([]AdminBucketSummary, 0, adminBucketScanPage)
+	for {
+		kvs, err := s.store.ScanAt(ctx, start, end, adminBucketScanPage, readTS)
+		if err != nil {
+			return nil, errors.Wrap(err, "admin list buckets: scan metadata")
+		}
+		if len(kvs) == 0 {
+			break
+		}
+		appended, halt, err := appendAdminBucketSummaries(out, kvs, prefix)
+		if err != nil {
+			return nil, err
+		}
+		out = appended
+		if halt {
+			// A key outside the metadata prefix means the
+			// table-of-contents layout changed mid-scan; returning
+			// what we have is safer than fabricating a summary
+			// from an unrelated key.
+			return finaliseAdminBucketList(out), nil
+		}
+		if len(kvs) < adminBucketScanPage {
+			break
+		}
+		start = nextScanCursor(kvs[len(kvs)-1].Key)
+		if end != nil && bytes.Compare(start, end) > 0 {
+			break
+		}
+	}
+	return finaliseAdminBucketList(out), nil
+}
+
+// appendAdminBucketSummaries projects one ScanAt page into the
+// accumulating result slice. Returns the extended slice plus a halt
+// flag the caller uses to short-circuit when ScanAt yielded a key
+// outside the bucket-meta prefix (a defensive check that should not
+// trigger in practice but locks the contract). Splitting this out
+// keeps AdminListBuckets under the cyclomatic ceiling without
+// hiding the per-row decode + skip logic.
+func appendAdminBucketSummaries(out []AdminBucketSummary, kvs []*store.KVPair, prefix []byte) ([]AdminBucketSummary, bool, error) {
+	for _, kvp := range kvs {
+		if !bytes.HasPrefix(kvp.Key, prefix) {
+			return out, true, nil
+		}
+		bucket, ok := s3keys.ParseBucketMetaKey(kvp.Key)
+		if !ok {
+			continue
+		}
+		meta, err := decodeS3BucketMeta(kvp.Value)
+		if err != nil {
+			return nil, false, errors.Wrapf(err, "admin list buckets: decode metadata for %q", bucket)
+		}
+		if meta == nil {
+			continue
+		}
+		out = append(out, summaryFromBucketMeta(bucket, meta))
+	}
+	return out, false, nil
+}
+
+// finaliseAdminBucketList sorts the accumulated summaries and
+// returns the result. Pulled out because the scan loop has two
+// early-exit branches (prefix-mismatch defensive return + the
+// natural end-of-prefix exit) and both must guarantee
+// lexicographic ordering — one place to enforce it is safer than
+// two near-identical sort.Slice calls.
+func finaliseAdminBucketList(out []AdminBucketSummary) []AdminBucketSummary {
+	// ScanAt yields metadata-prefix order, which is already
+	// lexicographic by escaped name on the ASCII bucket-name
+	// alphabet. The final sort is defensive against a future
+	// key-encoding change rather than a correction today.
+	sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name })
+	return out
+}
+
+// AdminDescribeBucket returns the bucket-level snapshot for name.
+// The triple (result, present, error) lets admin callers
+// distinguish a genuine "not found" from a storage error without
+// sniffing sentinels — when the bucket is missing the function
+// returns (nil, false, nil), mirroring AdminDescribeTable's
+// contract on the Dynamo side.
+//
+// Like AdminListBuckets this is a read-only path that bypasses
+// SigV4. The HTTP admin handler enforces session + CSRF + role at
+// the boundary; the adapter trusts the caller for authentication
+// (Section 3.2's exception for read-only paths).
+func (s *S3Server) AdminDescribeBucket(ctx context.Context, name string) (*AdminBucketSummary, bool, error) {
+	readTS := s.readTS()
+	readPin := s.pinReadTS(readTS)
+	defer readPin.Release()
+	meta, exists, err := s.loadBucketMetaAt(ctx, name, readTS)
+	if err != nil {
+		return nil, false, errors.Wrapf(err, "admin describe bucket %q", name)
+	}
+	if !exists || meta == nil {
+		return nil, false, nil
+	}
+	summary := summaryFromBucketMeta(name, meta)
+	return &summary, true, nil
+}
+
+// summaryFromBucketMeta projects the on-disk metadata into the
+// admin DTO. Pulled out so list and describe both produce the same
+// shape, including the empty-string defaults for optional fields —
+// the SPA depends on these being present even when blank.
+func summaryFromBucketMeta(name string, meta *s3BucketMeta) AdminBucketSummary {
+	return AdminBucketSummary{
+		Name:         name,
+		ACL:          meta.Acl,
+		CreatedAtHLC: meta.CreatedAtHLC,
+		Generation:   meta.Generation,
+		Region:       meta.Region,
+		Owner:        meta.Owner,
+	}
+}

adapter/s3_admin_test.go

@@ -0,0 +1,135 @@
+package adapter
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/bootjp/elastickv/store"
+	"github.com/stretchr/testify/require"
+)
+
+// TestS3Server_AdminListBuckets_EmptyReturnsEmptySlice covers the
+// "no buckets at all" case so the admin handler can rely on getting
+// an empty slice — not nil — and produce a stable `[]` JSON shape.
+func TestS3Server_AdminListBuckets_EmptyReturnsEmptySlice(t *testing.T) {
+	t.Parallel()
+
+	st := store.NewMVCCStore()
+	server := NewS3Server(nil, "", st, newLocalAdapterCoordinator(st), nil)
+
+	got, err := server.AdminListBuckets(context.Background())
+	require.NoError(t, err)
+	require.NotNil(t, got, "must return non-nil slice for empty state so the admin JSON shape is `[]`")
+	require.Empty(t, got)
+}
+
+// TestS3Server_AdminListBuckets_ReflectsCreatedBuckets confirms the
+// SigV4-bypass admin path sees the same buckets a normal SigV4
+// CreateBucket flow produced. The two views share loadBucketMetaAt
+// + the metadata-prefix scan, so any drift here is an encoding bug
+// in summaryFromBucketMeta — exactly the regression the test pins.
+func TestS3Server_AdminListBuckets_ReflectsCreatedBuckets(t *testing.T) {
+	t.Parallel()
+
+	st := store.NewMVCCStore()
+	server := NewS3Server(nil, "", st, newLocalAdapterCoordinator(st), nil)
+
+	for _, name := range []string{"alpha", "bravo", "charlie"} {
+		rec := httptest.NewRecorder()
+		req := newS3TestRequest(http.MethodPut, "/"+name, nil)
+		server.handle(rec, req)
+		require.Equal(t, http.StatusOK, rec.Code, "create %s", name)
+	}
+
+	got, err := server.AdminListBuckets(context.Background())
+	require.NoError(t, err)
+	require.Len(t, got, 3)
+	// ScanAt produces metadata-prefix order (lexicographic by
+	// escaped name); summaryFromBucketMeta preserves that.
+	require.Equal(t, "alpha", got[0].Name)
+	require.Equal(t, "bravo", got[1].Name)
+	require.Equal(t, "charlie", got[2].Name)
+	for _, b := range got {
+		require.Equal(t, s3AclPrivate, b.ACL,
+			"unspecified ACL must default to private (matches createBucket)")
+		require.NotZero(t, b.CreatedAtHLC, "creation HLC must be populated")
+		require.NotZero(t, b.Generation, "generation must be populated")
+	}
+}
+
+// TestS3Server_AdminDescribeBucket_Existing returns the populated
+// summary with ACL / region preserved through the bridge, and
+// (nil, false, nil) for a missing name. The handler depends on the
+// (nil, false, nil) shape to differentiate "not found" from a
+// storage failure without sniffing sentinels.
+func TestS3Server_AdminDescribeBucket_Existing(t *testing.T) {
+	t.Parallel()
+
+	st := store.NewMVCCStore()
+	server := NewS3Server(nil, "", st, newLocalAdapterCoordinator(st), nil)
+
+	rec := httptest.NewRecorder()
+	req := newS3TestRequest(http.MethodPut, "/orders", nil)
+	req.Header.Set("x-amz-acl", s3AclPublicRead)
+	server.handle(rec, req)
+	require.Equal(t, http.StatusOK, rec.Code)
+
+	got, exists, err := server.AdminDescribeBucket(context.Background(), "orders")
+	require.NoError(t, err)
+	require.True(t, exists)
+	require.NotNil(t, got)
+	require.Equal(t, "orders", got.Name)
+	require.Equal(t, s3AclPublicRead, got.ACL,
+		"explicit x-amz-acl must round-trip through the admin describe path")
+	require.NotZero(t, got.CreatedAtHLC)
+}
+
+func TestS3Server_AdminDescribeBucket_Missing(t *testing.T) {
+	t.Parallel()
+
+	st := store.NewMVCCStore()
+	server := NewS3Server(nil, "", st, newLocalAdapterCoordinator(st), nil)
+
+	got, exists, err := server.AdminDescribeBucket(context.Background(), "no-such-bucket")
+	require.NoError(t, err)
+	require.False(t, exists)
+	require.Nil(t, got)
+}
+
+// TestS3Server_AdminListBuckets_PaginatesPastSinglePage pins the
+// fix for the truncation bug Codex P1 / Claude Issue 1 / Gemini
+// flagged on PR #658: AdminListBuckets must walk the metadata
+// prefix until exhausted, not stop at adminBucketScanPage. The
+// test exceeds the per-iteration page by 100 buckets (1100 total)
+// so a regression that re-introduces a single-call ScanAt would
+// silently drop the tail and the assertion fails.
+//
+// Total bucket count (1100) is small enough to keep the test
+// O(seconds) on the in-memory MVCC store. Names are zero-padded to
+// 4 digits so lexicographic order matches numeric order — the test
+// pins both the count AND the ordering contract.
+func TestS3Server_AdminListBuckets_PaginatesPastSinglePage(t *testing.T) {
+	t.Parallel()
+
+	st := store.NewMVCCStore()
+	server := NewS3Server(nil, "", st, newLocalAdapterCoordinator(st), nil)
+
+	const total = adminBucketScanPage + 100
+	for i := range total {
+		name := fmt.Sprintf("bucket-%04d", i)
+		rec := httptest.NewRecorder()
+		req := newS3TestRequest(http.MethodPut, "/"+name, nil)
+		server.handle(rec, req)
+		require.Equal(t, http.StatusOK, rec.Code, "create %s", name)
+	}
+
+	got, err := server.AdminListBuckets(context.Background())
+	require.NoError(t, err)
+	require.Len(t, got, total,
+		"AdminListBuckets must continue past adminBucketScanPage; truncating here is the regression")
+	require.Equal(t, "bucket-0000", got[0].Name)
+	require.Equal(t, fmt.Sprintf("bucket-%04d", total-1), got[total-1].Name)
+}

internal/admin/buckets_source.go

@@ -0,0 +1,75 @@
+package admin
+
+import (
+	"context"
+	"errors"
+)
+
+// BucketsSource is the in-process surface the admin S3 handler
+// dispatches into. It mirrors TablesSource on the Dynamo side
+// (Section 3.2 of the admin design): defining the contract here lets
+// the bridge in main_admin.go translate adapter errors into the
+// admin-package vocabulary without the adapter package importing
+// internal/admin.
+//
+// All methods are read-only in this slice. Write methods
+// (AdminCreateBucket, AdminPutBucketAcl, AdminDeleteBucket) ship in
+// the next slice with AdminForward integration so a follower can
+// hand them off to the leader transparently.
+type BucketsSource interface {
+	// AdminListBuckets returns every bucket this server knows about,
+	// in stable lexicographic order. The empty list is a valid
+	// response — the handler returns `{"buckets":[]}` rather than
+	// 404 so the SPA can distinguish "no buckets yet" from "S3
+	// admin not configured" (the latter shape is a 404 from the
+	// router fallthrough).
+	AdminListBuckets(ctx context.Context) ([]BucketSummary, error)
+	// AdminDescribeBucket returns the metadata snapshot for name.
+	// The triple (result, present, error) lets the handler emit a
+	// 404 for missing buckets without sniffing sentinels; storage
+	// failures still surface via the error return.
+	AdminDescribeBucket(ctx context.Context, name string) (*BucketSummary, bool, error)
+}
+
+// BucketSummary is the bucket-level DTO the SPA receives. The JSON
+// shape matches the design doc Section 4.1 / web/admin's
+// `S3Bucket` interface — bucket_name + acl + created_at — plus
+// generation/region/owner for operators inspecting via curl.
+//
+// CreatedAt is an ISO-8601 string (UTC, second precision). The
+// adapter persists it as an HLC; the handler formats. Producing
+// the formatted string here rather than in the SPA keeps timezone
+// rendering server-side and prevents drift between the two SPA
+// pages that surface buckets (S3List + S3Detail).
+type BucketSummary struct {
+	Name       string `json:"bucket_name"`
+	ACL        string `json:"acl,omitempty"`
+	CreatedAt  string `json:"created_at,omitempty"`
+	Generation uint64 `json:"generation,omitempty"`
+	Region     string `json:"region,omitempty"`
+	Owner      string `json:"owner,omitempty"`
+}
+
+// ErrBucketsForbidden is returned when the principal lacks the
+// role required for the operation. Maps to 403. Kept as its own
+// sentinel (rather than reusing ErrTablesForbidden) so a future
+// per-resource role model can diverge without breaking either
+// handler's match list.
+var ErrBucketsForbidden = errors.New("admin buckets: principal lacks required role")
+
+// ErrBucketsNotLeader is returned when the local node is not the
+// Raft leader for the S3 group. Read-only methods do NOT return
+// this — list / describe are leader-agnostic in this slice. Kept
+// here so the next slice's write methods can wire it without
+// adding a new sentinel.
+var ErrBucketsNotLeader = errors.New("admin buckets: local node is not the raft leader")
+
+// ErrBucketsNotFound is returned when DELETE / DESCRIBE / a
+// follow-up read targets a bucket that does not exist. The triple
+// (nil, false, nil) is the preferred signal for the read path;
+// this sentinel covers the future write paths only.
+var ErrBucketsNotFound = errors.New("admin buckets: bucket does not exist")
+
+// ErrBucketsAlreadyExists is returned when CREATE targets a name
+// that is already in use. Maps to 409. Reserved for the next slice.
+var ErrBucketsAlreadyExists = errors.New("admin buckets: bucket already exists")