diff --git a/weed/s3api/s3tables/handler_bind_authorized_test.go b/weed/s3api/s3tables/handler_bind_authorized_test.go index ce48bdf3e1..0544ec5406 100644 --- a/weed/s3api/s3tables/handler_bind_authorized_test.go +++ b/weed/s3api/s3tables/handler_bind_authorized_test.go @@ -583,3 +583,28 @@ func TestUpdateNamespaceSparesARecreatedNamespace(t *testing.T) { assert.Equal(t, http.StatusConflict, code, "%+v", resp) assert.Equal(t, recreated, fs.Get(bucketPath, "ns").Extended, "the new namespace is untouched") } + +// DeleteNamespace removes only the namespace it was authorized against: one +// deleted and created again before the claim reads as the first one gone. +func TestDeleteNamespaceSparesARecreatedNamespace(t *testing.T) { + fs, m := startEmptyBucket(t) + requireOK(t, m, fs, "CreateNamespace", createNamespaceNsRequest(t)) + bucketPath := GetTableBucketPath(renameTestBucket) + recreated := recreatedNamespace(t, "ns") + fs.OnUpdate = func(dir, name string) { + if dir != bucketPath || name != "ns" { + return + } + fs.OnUpdate = nil + fs.Put(bucketPath, "ns", recreated) + } + + code, resp := runHandler(t, m, fs.Client, "DeleteNamespace", &DeleteNamespaceRequest{ + TableBucketARN: mustBucketARN(t), + Namespace: []string{"ns"}, + }) + + assert.Equal(t, http.StatusNotFound, code, "%+v", resp) + require.NotNil(t, fs.Get(bucketPath, "ns"), "the new namespace survives") + assert.Equal(t, recreated, fs.Get(bucketPath, "ns").Extended, "the new namespace is untouched") +} diff --git a/weed/s3api/s3tables/handler_namespace.go b/weed/s3api/s3tables/handler_namespace.go index 4327fc2f56..f693ce833e 100644 --- a/weed/s3api/s3tables/handler_namespace.go +++ b/weed/s3api/s3tables/handler_namespace.go @@ -757,6 +757,17 @@ func (h *S3TablesHandler) handleDeleteNamespace(w http.ResponseWriter, r *http.R if err := json.Unmarshal(data, &metadata); err != nil { return fmt.Errorf("failed to unmarshal metadata: %w", err) } + // Checked after the namespace read: a bucket created again holds only + // namespaces of its own, and the claim below is bound to this one. + if createdAt, bound := boundTableBucket(r.Context()); bound { + bucket, err := h.lookupEntry(r.Context(), client, bucketPath) + if err != nil { + return err + } + if !isTableBucketCreatedAt(createdAt)(bucket) { + return filer_pb.ErrNotFound + } + } // Fetch bucket policy if it exists policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy) @@ -802,11 +813,12 @@ func (h *S3TablesHandler) handleDeleteNamespace(w http.ResponseWriter, r *http.R // A namespace whose create has not confirmed it is absent, and claiming it // would replace the create's mark: a refused delete then lifts the claim and // leaves the namespace looking confirmed before its create checked the bucket. + // One created again since it was authorized is absent too. var claim string var claimed map[string][]byte err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { var claimErr error - claim, claimed, claimErr = h.claimForDelete(r.Context(), client, namespacePath, isFinishedNamespaceEntry) + claim, claimed, claimErr = h.claimForDelete(r.Context(), client, namespacePath, isNamespaceCreatedAt(metadata.CreatedAt)) return claimErr }) if err != nil { diff --git a/weed/s3api/s3tables/manager.go b/weed/s3api/s3tables/manager.go index 0334f80fec..e3be966476 100644 --- a/weed/s3api/s3tables/manager.go +++ b/weed/s3api/s3tables/manager.go @@ -80,18 +80,24 @@ func (m *Manager) PolicyContext(r *http.Request, bucket string) *PolicyContext { type tableBucketCreatedAtKey struct{} // WithTableBucketCreatedAt binds a DeleteTableBucket, CreateNamespace, -// CreateTable or DeleteTable run through Execute to the bucket created at -// createdAt, so a -// caller that authorized the bucket itself never writes into one created -// again under its name. In-process only: no wire request can carry it. +// DeleteNamespace, CreateTable or DeleteTable run through Execute to the +// bucket created at createdAt, so a caller that authorized the bucket itself +// never writes into one created again under its name. In-process only: no +// wire request can carry it. func WithTableBucketCreatedAt(ctx context.Context, createdAt time.Time) context.Context { return context.WithValue(ctx, tableBucketCreatedAtKey{}, createdAt) } +// boundTableBucket returns the bucket creation time ctx binds the request to. +func boundTableBucket(ctx context.Context) (time.Time, bool) { + createdAt, ok := ctx.Value(tableBucketCreatedAtKey{}).(time.Time) + return createdAt, ok +} + // otherTableBucket reports whether ctx binds the request to a bucket other // than the one created at createdAt. func otherTableBucket(ctx context.Context, createdAt time.Time) bool { - want, ok := ctx.Value(tableBucketCreatedAtKey{}).(time.Time) + want, ok := boundTableBucket(ctx) return ok && !createdAt.Equal(want) } diff --git a/weed/s3api/s3vectors/bind_bucket_test.go b/weed/s3api/s3vectors/bind_bucket_test.go index 3a73326874..b70a0cba36 100644 --- a/weed/s3api/s3vectors/bind_bucket_test.go +++ b/weed/s3api/s3vectors/bind_bucket_test.go @@ -207,3 +207,37 @@ func TestIndexWritesKeepToAuthorizedBucket(t *testing.T) { } }) } + +// DeleteVectorBucket drops the empty index namespace only from the bucket it +// authorized: a bucket created again before that cleanup keeps its own. +func TestDeleteVectorBucketCleanupKeepsReplacingBucket(t *testing.T) { + fs := s3tablestest.Start(t) + s := NewServer(s3tables.NewManagerClient(fs.Client), allowAllAuth{}, newFakeVectors(0)) + if w := postJSONAs(t, "", s.handleCreateVectorBucket, &CreateVectorBucketRequest{VectorBucketName: "vec-bucket"}); w.Code != http.StatusOK { + t.Fatalf("create bucket: %d %s", w.Code, w.Body.String()) + } + if w := postJSONAs(t, "", s.handleCreateIndex, &CreateIndexRequest{ + VectorBucketName: "vec-bucket", IndexName: "idx", DataType: "float32", Dimension: 3, DistanceMetric: "euclidean", + }); w.Code != http.StatusOK { + t.Fatalf("create index: %d %s", w.Code, w.Body.String()) + } + if w := postJSONAs(t, "", s.handleDeleteIndex, &DeleteIndexRequest{VectorBucketName: "vec-bucket", IndexName: "idx"}); w.Code != http.StatusOK { + t.Fatalf("delete index: %d %s", w.Code, w.Body.String()) + } + + bucketPath := s3tables.GetTableBucketPath("vec-bucket") + fs.OnLookup = func(d, n string) { + if d == bucketPath && n == indexNamespace { + fs.OnLookup = nil + replaceBucket(t, fs, "vec-bucket") + } + } + w := postJSONAs(t, "", s.handleDeleteVectorBucket, &DeleteVectorBucketRequest{VectorBucketName: "vec-bucket"}) + fs.OnLookup = nil + if w.Code == http.StatusOK { + t.Fatalf("delete over a replaced bucket succeeded: %s", w.Body.String()) + } + if fs.Get(bucketPath, indexNamespace) == nil { + t.Fatal("DeleteVectorBucket removed the index namespace of the bucket that replaced the authorized one") + } +} diff --git a/weed/s3api/s3vectors/handlers.go b/weed/s3api/s3vectors/handlers.go index af762d7097..9f9be68b51 100644 --- a/weed/s3api/s3vectors/handlers.go +++ b/weed/s3api/s3vectors/handlers.go @@ -421,7 +421,7 @@ func (s *Server) handleDeleteVectorBucket(w http.ResponseWriter, r *http.Request // holds no indexes; drop it under the delete's own authorization and // retry once. A namespace still holding indexes keeps the conflict. var nsResp map[string]interface{} - nsErr := s.internalCatalog(context.WithoutCancel(r.Context()), "DeleteNamespace", + nsErr := s.internalCatalog(context.WithoutCancel(ctx), "DeleteNamespace", &s3tables.DeleteNamespaceRequest{ TableBucketARN: s.s3tablesArn(arns.bucket), Namespace: []string{indexNamespace},