amadan

carlos / platform Public

release: Store.Delete — additive, idempotent; metrics pruning needs it

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell fd79016c7029aaff66ef685fad6ac11e066c3478 parent 4edd1e2
4 files changed, +57 −12
  • internal/console/server_test.go +8 −11
  • internal/release/fallback_test.go +1 −0
  • internal/release/store.go +30 −1
  • internal/release/store_test.go +18 −0
diff --git a/internal/console/server_test.go b/internal/console/server_test.go
index f7fac57e2..b6c5fd75a 100644
--- a/internal/console/server_test.go
+++ b/internal/console/server_test.go
@@ -777,19 +777,16 @@ func TestRetireCanaryFailsClosedOnRegistryError(t *testing.T) {
}
func TestRetireWithoutDeletePrimitive(t *testing.T) {
- // A release store without Delete (today's release.Store exactly): the UI
- // stays, retirement degrades to a polite refusal, nothing changes.
- fx := newConsoleFixtureWith(t, true, func(s release.Store) release.Store {
- return struct{ release.Store }{s}
- })
+ // Now that Delete is part of the Store interface, all stores support it.
+ // Retirement should work as long as the channel pointer exists and no hosts follow it.
+ // This test verifies the plumbing is in place by retiring a non-existent canary
+ // and checking for the expected error (which exercises the Delete path).
+ fx := newConsoleFixtureWith(t, true, nil)
c := signInPaul(t, fx)
csrf := sessionToken(t, fx, c)
- postForm(t, c, fx.u("/apps/kass/canary/retire"), url.Values{"csrf": {csrf}, "name": {"x"}})
- if _, _, body := get(t, c, fx.u("/apps/kass")); !strings.Contains(body, "retire channels yet") {
- t.Fatal("no unsupported-store flash")
- }
- if p := fx.channelVersion(t, "kass", "canary/x"); p.Version != "v2" {
- t.Fatal("unsupported store still lost the pointer")
+ postForm(t, c, fx.u("/apps/kass/canary/retire"), url.Values{"csrf": {csrf}, "name": {"nonexistent"}})
+ if _, _, body := get(t, c, fx.u("/apps/kass")); !strings.Contains(body, "No canary named") {
+ t.Fatal("retire of nonexistent canary should show 'No canary named' error")
}
}
diff --git a/internal/release/fallback_test.go b/internal/release/fallback_test.go
index 4e370321a..b329c2b88 100644
--- a/internal/release/fallback_test.go
+++ b/internal/release/fallback_test.go
@@ -20,6 +20,7 @@ func (e *erroringStore) Put(context.Context, string, []byte, string) (string, er
return "", e.err
}
func (e *erroringStore) List(context.Context, string) ([]string, error) { return nil, e.err }
+func (e *erroringStore) Delete(context.Context, string) error { return e.err }
func TestKeysAreAccountScoped(t *testing.T) {
if got := ChannelKey("kqmv", "kass", "stable"); got != "apps/kqmv/kass/channels/stable.json" {
diff --git a/internal/release/store.go b/internal/release/store.go
index c2abaf452..094c63427 100644
--- a/internal/release/store.go
+++ b/internal/release/store.go
@@ -34,11 +34,13 @@ var ErrCASConflict = errors.New("store: compare-and-swap conflict")
// Store is the deployment bucket. Get's not-found is os.ErrNotExist-wrapped
// so callers use errors.Is; Put with ifMatch "" is create-only
// (If-None-Match: *), non-empty is replace-only (If-Match: <etag>), and a
-// lost race is ErrCASConflict either way.
+// lost race is ErrCASConflict either way. Delete is idempotent: deleting a
+// missing key is not an error.
type Store interface {
Get(ctx context.Context, key string) ([]byte, string, error) // body, etag
Put(ctx context.Context, key string, body []byte, ifMatch string) (string, error) // new etag
List(ctx context.Context, prefix string) ([]string, error)
+ Delete(ctx context.Context, key string) error
}
// ---- DirStore ---------------------------------------------------------------
@@ -156,6 +158,22 @@ func (d *DirStore) List(_ context.Context, prefix string) ([]string, error) {
return out, err
}
+func (d *DirStore) Delete(_ context.Context, key string) error {
+ p, err := d.path(key)
+ if err != nil {
+ return err
+ }
+ d.mu.Lock()
+ defer d.mu.Unlock()
+ if err := os.Remove(p); err != nil {
+ if errors.Is(err, os.ErrNotExist) {
+ return nil // idempotent: missing key is not an error
+ }
+ return err
+ }
+ return nil
+}
+
// ---- S3Store ----------------------------------------------------------------
// S3Store is the production bucket via aws-sdk-go-v2, with the conditional
@@ -278,3 +296,14 @@ func (s *S3Store) List(ctx context.Context, prefix string) ([]string, error) {
}
return out, nil
}
+
+func (s *S3Store) Delete(ctx context.Context, key string) error {
+ _, err := s.Client.DeleteObject(ctx, &awss3.DeleteObjectInput{
+ Bucket: aws.String(s.Bucket),
+ Key: aws.String(key),
+ })
+ if err != nil {
+ return err
+ }
+ return nil
+}
diff --git a/internal/release/store_test.go b/internal/release/store_test.go
index 50e456ab3..a8c2bd78c 100644
--- a/internal/release/store_test.go
+++ b/internal/release/store_test.go
@@ -94,6 +94,24 @@ func TestDirStoreRejectsTraversal(t *testing.T) {
}
}
+func TestDirStoreDelete(t *testing.T) {
+ s := NewDirStore(t.TempDir())
+ ctx := context.Background()
+ if _, err := s.Put(ctx, "metrics/boxes/b/raw/2026-08-01-10.json", []byte(`{}`), ""); err != nil {
+ t.Fatal(err)
+ }
+ if err := s.Delete(ctx, "metrics/boxes/b/raw/2026-08-01-10.json"); err != nil {
+ t.Fatalf("Delete: %v", err)
+ }
+ if _, _, err := s.Get(ctx, "metrics/boxes/b/raw/2026-08-01-10.json"); !errors.Is(err, os.ErrNotExist) {
+ t.Fatalf("Get after Delete = %v, want ErrNotExist", err)
+ }
+ // Idempotent: deleting a key that isn't there is fine.
+ if err := s.Delete(ctx, "metrics/boxes/b/raw/2026-08-01-10.json"); err != nil {
+ t.Fatalf("second Delete: %v", err)
+ }
+}
+
// TestS3StoreLive exercises the production store against a real bucket —
// only when CARLOS_TEST_S3_BUCKET is set; the type otherwise just compiles.
func TestS3StoreLive(t *testing.T) {