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

adapter/s3_admin.go

@@ -0,0 +1,112 @@
+package adapter
+
+import (
+	"context"
+	"sort"
+
+	"github.com/bootjp/elastickv/internal/s3keys"
+	"github.com/cockroachdb/errors"
+)
+
+// 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; both
+// share the same underlying ScanAt so the two views cannot drift.
+//
+// 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()
+	kvs, err := s.store.ScanAt(ctx,
+		[]byte(s3keys.BucketMetaPrefix),
+		prefixScanEnd([]byte(s3keys.BucketMetaPrefix)),
+		s3MaxKeys, readTS)
+	if err != nil {
+		return nil, errors.Wrap(err, "admin list buckets: scan metadata")
+	}
+	out := make([]AdminBucketSummary, 0, len(kvs))
+	for _, kvp := range kvs {
+		bucket, ok := s3keys.ParseBucketMetaKey(kvp.Key)
+		if !ok {
+			continue
+		}
+		meta, err := decodeS3BucketMeta(kvp.Value)
+		if err != nil {
+			return nil, errors.Wrapf(err, "admin list buckets: decode metadata for %q", bucket)
+		}
+		if meta == nil {
+			continue
+		}
+		out = append(out, summaryFromBucketMeta(bucket, meta))
+	}
+	// ScanAt returns metadata-prefix order which is already
+	// lexicographic by escaped name. The escape preserves byte
+	// ordering for the ASCII bucket-name alphabet, so a final
+	// sort is a defensive no-op rather than a correction — kept
+	// to lock the contract in case the encoding changes.
+	sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name })
+	return out, nil
+}
+
+// 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,99 @@
+package adapter
+
+import (
+	"context"
+	"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)
+}

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")

internal/admin/dynamo_handler.go

@@ -3,7 +3,6 @@ package admin
 import (
 	"bytes"
 	"context"
-	"encoding/base64"
 	"errors"
 	"io"
 	"log/slog"
@@ -265,14 +264,8 @@ type dynamoListResponse struct {
 }
 
 func (h *DynamoHandler) handleList(w http.ResponseWriter, r *http.Request) {
-	limit, err := parseDynamoListLimit(r.URL.Query().Get("limit"))
-	if err != nil {
-		writeJSONError(w, http.StatusBadRequest, "invalid_limit", err.Error())
-		return
-	}
-	startAfter, err := decodeDynamoNextToken(r.URL.Query().Get("next_token"))
-	if err != nil {
-		writeJSONError(w, http.StatusBadRequest, "invalid_next_token", err.Error())
+	limit, startAfter, ok := parseListPaginationParams(w, r, defaultDynamoListLimit, dynamoListLimitMax)
+	if !ok {
 		return
 	}
 
@@ -306,7 +299,7 @@ func (h *DynamoHandler) handleList(w http.ResponseWriter, r *http.Request) {
 	page, next := paginateDynamoTableNames(names, startAfter, limit)
 	resp := dynamoListResponse{Tables: page}
 	if next != "" {
-		resp.NextToken = encodeDynamoNextToken(next)
+		resp.NextToken = encodeListNextToken(next)
 	}
 	// paginateDynamoTableNames is total over its input — it always
 	// returns a non-nil slice (an empty []string{} on the
@@ -717,46 +710,11 @@ func (h *DynamoHandler) handleDescribe(w http.ResponseWriter, r *http.Request, n
 }
 
 // parseDynamoListLimit translates the ?limit= query parameter into a
-// concrete page size. Empty falls back to the design-doc default;
-// negatives or non-numerics are an outright client error; values past
-// the ceiling are silently clamped (not an error) so the SPA's
-// "request the maximum" pattern works without a probe round-trip.
-func parseDynamoListLimit(raw string) (int, error) {
-	if raw == "" {
-		return defaultDynamoListLimit, nil
-	}
-	n, err := strconv.Atoi(raw)
-	if err != nil {
-		return 0, errors.New("limit must be an integer")
-	}
-	if n <= 0 {
-		return 0, errors.New("limit must be positive")
-	}
-	if n > dynamoListLimitMax {
-		return dynamoListLimitMax, nil
-	}
-	return n, nil
-}
-
-// decodeDynamoNextToken reverses encodeDynamoNextToken. We base64-wrap
-// the raw last-table-name so the wire token is opaque from the
-// client's perspective and we can change the cursor representation
-// later without breaking the API contract.
-func decodeDynamoNextToken(raw string) (string, error) {
-	if raw == "" {
-		return "", nil
-	}
-	decoded, err := base64.RawURLEncoding.DecodeString(raw)
-	if err != nil {
-		return "", errors.New("next_token is not valid base64url")
-	}
-	return string(decoded), nil
-}
-
-func encodeDynamoNextToken(name string) string {
-	return base64.RawURLEncoding.EncodeToString([]byte(name))
-}
-
+// concrete page size. The shared parseListLimit lives in
+// list_pagination.go; this comment is preserved here only because
+// it documents the historical rationale for the default / clamp
+// policy that the shared helper inherited.
+//
 // paginateDynamoTableNames slices `names` (already lex-sorted by the
 // adapter) into a single page starting strictly after `startAfter`.
 // The second return is the opaque cursor the client should pass back