mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-11-03 02:32:35 +01:00
fix(s3tables,s3vectors): bind DeleteNamespace and the vector bucket cleanup to the authorized records (#12331)
* s3tables: bind DeleteNamespace to the authorized namespace The delete claimed whatever finished namespace held the name at claim time, so one deleted and created again by another owner after the authorization read was removed under the first one's authorization. The claim now requires the creation time that was authorized. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3vectors: bind DeleteVectorBucket's namespace cleanup to the authorized bucket After the bound DeleteTableBucket reported the bucket not empty, the index namespace cleanup ran unbound, so a bucket deleted and created again in between lost its empty index namespace under the first bucket's authorization. DeleteNamespace now honors the bucket binding, checking the bucket after reading the namespace, and the cleanup carries it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
984eac6a79
commit
7298b4ed37
5 files changed
+84
-7
No files matched your search
@@ -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")
|
||||
}
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
@@ -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},
|
||||
|
||||
Reference in new issue
Block a user