Merge pull request #18499 from Budibase/fix/ceph-s3-api

Fix/ceph s3 api

Author: calexiou

packages/backend-core/src/middleware/errorHandling.ts

@@ -14,7 +14,7 @@ export async function errorHandling(ctx: ParameterizedContext, next: Next) {
     if (status >= 400 && status < 500) {
       console.warn(err)
     } else {
-      console.error("Got 400 response code", err)
+      console.error("Got 5xx response code", err)
     }
 
     let error: APIError = {

packages/backend-core/src/objectStore/objectStore.ts

@@ -167,35 +167,44 @@ export async function createBucketIfNotExists(
     })
     return { created: false, exists: true }
   } catch (err: any) {
-    const statusCode = err.statusCode || err.$response?.statusCode
+    const statusCode =
+      err.statusCode ||
+      err.$response?.statusCode ||
+      err.$metadata?.httpStatusCode
     const promises: Record<string, Promise<any> | undefined> =
       STATE.bucketCreationPromises
-    const doesntExist = statusCode === 404,
-      noAccess = statusCode === 403
+
+    if (statusCode === 403) {
+      throw new Error("Access denied to object store bucket." + err)
+    }
+
     if (promises[bucketName]) {
       await promises[bucketName]
       return { created: false, exists: true }
-    } else if (doesntExist || noAccess) {
-      if (doesntExist) {
-        promises[bucketName] = client
-          .createBucket({
-            Bucket: bucketName,
-          })
-          .catch((err: any) => {
-            // bucket was created in the meantime by another process
-            if (err.Code !== "BucketAlreadyOwnedByYou") {
-              throw err
-            }
-          })
-
-        await promises[bucketName]
-        delete promises[bucketName]
-        return { created: true, exists: false }
-      } else {
-        throw new Error("Access denied to object store bucket." + err)
-      }
-    } else {
-      throw new Error("Unable to write to object store bucket.")
+    }
+
+    // Attempt to create the bucket for any headBucket failure that is not an
+    // explicit access denial. This covers 404 (not found) and non-standard
+    // status codes returned by S3-compatible stores such as Ceph RadosGW.
+    promises[bucketName] = client
+      .createBucket({
+        Bucket: bucketName,
+      })
+      .catch((err: any) => {
+        // bucket was created in the meantime by another process
+        if (
+          err.Code !== "BucketAlreadyOwnedByYou" &&
+          err.name !== "BucketAlreadyOwnedByYou"
+        ) {
+          throw err
+        }
+      })
+
+    try {
+      await promises[bucketName]
+      return { created: true, exists: false }
+    } finally {
+      delete promises[bucketName]
     }
   }
 }
@@ -796,7 +805,10 @@ export async function objectExists(
     await client.headObject(params)
     return true
   } catch (err: any) {
-    const statusCode = err.statusCode || err.$response?.statusCode
+    const statusCode =
+      err.statusCode ||
+      err.$response?.statusCode ||
+      err.$metadata?.httpStatusCode
     if (statusCode === 404) {
       return false
     }

packages/backend-core/src/objectStore/tests/createBucketIfNotExists.spec.ts

@@ -0,0 +1,133 @@
+import { createBucketIfNotExists } from "../objectStore"
+
+const mockHeadBucket = jest.fn()
+const mockCreateBucket = jest.fn()
+
+const mockClient = {
+  headBucket: mockHeadBucket,
+  createBucket: mockCreateBucket,
+} as any
+
+describe("createBucketIfNotExists", () => {
+  beforeEach(() => {
+    jest.clearAllMocks()
+  })
+
+  it("returns exists=true without creating when bucket already exists", async () => {
+    mockHeadBucket.mockResolvedValue({})
+
+    const result = await createBucketIfNotExists(mockClient, "test-bucket")
+
+    expect(result).toEqual({ created: false, exists: true })
+    expect(mockCreateBucket).not.toHaveBeenCalled()
+  })
+
+  it("creates the bucket when headBucket returns 404 via statusCode", async () => {
+    const error: any = new Error("Not Found")
+    error.statusCode = 404
+    mockHeadBucket.mockRejectedValue(error)
+    mockCreateBucket.mockResolvedValue({})
+
+    const result = await createBucketIfNotExists(mockClient, "test-bucket")
+
+    expect(result).toEqual({ created: true, exists: false })
+    expect(mockCreateBucket).toHaveBeenCalledWith({ Bucket: "test-bucket" })
+  })
+
+  it("creates the bucket when headBucket returns 404 via $response", async () => {
+    const error: any = new Error("Not Found")
+    error.$response = { statusCode: 404 }
+    mockHeadBucket.mockRejectedValue(error)
+    mockCreateBucket.mockResolvedValue({})
+
+    const result = await createBucketIfNotExists(mockClient, "test-bucket")
+
+    expect(result).toEqual({ created: true, exists: false })
+    expect(mockCreateBucket).toHaveBeenCalledWith({ Bucket: "test-bucket" })
+  })
+
+  it("creates the bucket when headBucket returns 404 via $metadata (SDK v3 / Ceph)", async () => {
+    const error: any = new Error("Not Found")
+    error.$metadata = { httpStatusCode: 404 }
+    mockHeadBucket.mockRejectedValue(error)
+    mockCreateBucket.mockResolvedValue({})
+
+    const result = await createBucketIfNotExists(mockClient, "test-bucket")
+
+    expect(result).toEqual({ created: true, exists: false })
+    expect(mockCreateBucket).toHaveBeenCalledWith({ Bucket: "test-bucket" })
+  })
+
+  it("attempts creation when headBucket fails with an unrecognised status code", async () => {
+    const error: any = new Error("Bad Request")
+    error.$metadata = { httpStatusCode: 400 }
+    mockHeadBucket.mockRejectedValue(error)
+    mockCreateBucket.mockResolvedValue({})
+
+    const result = await createBucketIfNotExists(mockClient, "test-bucket")
+
+    expect(result).toEqual({ created: true, exists: false })
+    expect(mockCreateBucket).toHaveBeenCalledWith({ Bucket: "test-bucket" })
+  })
+
+  it("attempts creation when headBucket fails with no status code at all", async () => {
+    mockHeadBucket.mockRejectedValue(new Error("network error"))
+    mockCreateBucket.mockResolvedValue({})
+
+    const result = await createBucketIfNotExists(mockClient, "test-bucket")
+
+    expect(result).toEqual({ created: true, exists: false })
+  })
+
+  it("throws when headBucket returns 403", async () => {
+    const error: any = new Error("Forbidden")
+    error.statusCode = 403
+    mockHeadBucket.mockRejectedValue(error)
+
+    await expect(
+      createBucketIfNotExists(mockClient, "test-bucket")
+    ).rejects.toThrow("Access denied to object store bucket.")
+
+    expect(mockCreateBucket).not.toHaveBeenCalled()
+  })
+
+  it("swallows BucketAlreadyOwnedByYou via err.Code (race condition)", async () => {
+    const headError: any = new Error("Not Found")
+    headError.statusCode = 404
+    mockHeadBucket.mockRejectedValue(headError)
+
+    const createError: any = new Error("BucketAlreadyOwnedByYou")
+    createError.Code = "BucketAlreadyOwnedByYou"
+    mockCreateBucket.mockRejectedValue(createError)
+
+    const result = await createBucketIfNotExists(mockClient, "test-bucket")
+
+    expect(result).toEqual({ created: true, exists: false })
+  })
+
+  it("swallows BucketAlreadyOwnedByYou via err.name (Ceph SDK v3 format)", async () => {
+    const headError: any = new Error("Not Found")
+    headError.$metadata = { httpStatusCode: 404 }
+    mockHeadBucket.mockRejectedValue(headError)
+
+    const createError: any = new Error("BucketAlreadyOwnedByYou")
+    createError.name = "BucketAlreadyOwnedByYou"
+    mockCreateBucket.mockRejectedValue(createError)
+
+    const result = await createBucketIfNotExists(mockClient, "test-bucket")
+
+    expect(result).toEqual({ created: true, exists: false })
+  })
+
+  it("propagates unexpected createBucket errors", async () => {
+    const headError: any = new Error("Not Found")
+    headError.statusCode = 404
+    mockHeadBucket.mockRejectedValue(headError)
+
+    mockCreateBucket.mockRejectedValue(new Error("InternalError"))
+
+    await expect(
+      createBucketIfNotExists(mockClient, "test-bucket")
+    ).rejects.toThrow("InternalError")
+  })
+})

packages/backend-core/src/objectStore/tests/objectExists.spec.ts

@@ -48,6 +48,16 @@ describe("objectExists", () => {
     expect(result).toBe(false)
   })
 
+  it("should return false when object does not exist (404 error with $metadata)", async () => {
+    const error: any = new Error("Not Found")
+    error.$metadata = { httpStatusCode: 404 }
+    mockHeadObject.mockRejectedValue(error)
+
+    const result = await objectExists("test-bucket", "test-key")
+
+    expect(result).toBe(false)
+  })
+
   it("should throw error for other errors", async () => {
     const error: any = new Error("Access Denied")
     error.statusCode = 403