Skip to content

Commit 6d8de1a

Browse files
KirylSAPKirylSAP
committed
refactor(storage): simplify Ceph service resolution in resolveS3Config
Refactored resolveS3Config to use ctx.openstack.service() API directly instead of manually searching through the catalog, addressing PR feedback from @andypf. Changes: - Removed redundant token.tokenData.catalog.find() logic - Use service.availableEndpoints() instead of accessing catalog directly - Simplified error handling and reduced nesting - Updated all test mocks to include availableEndpoints() method Fixes code duplication where we were already calling ctx.openstack.service("ceph") but then manually searching the catalog again.
1 parent 00ef56a commit 6d8de1a

4 files changed

Lines changed: 76 additions & 59 deletions

File tree

‎apps/aurora-portal/src/server/Storage/cephProcedure.test.ts‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ const createMockContext = (options: MockContextOptions = {}) => {
6666

6767
const mockCephService = {
6868
getEndpoint: () => endpoint,
69+
availableEndpoints: () =>
70+
hasRegion ? [{ region, url: endpoint, interface: "public", id: "test-id", region_id: region }] : [],
6971
}
7072

7173
const cephCatalogEntry = hasCephService
@@ -95,13 +97,19 @@ const createMockContext = (options: MockContextOptions = {}) => {
9597
}
9698
: null
9799

98-
const mockOpenstack = {
99-
service: (serviceName: string) => {
100-
if (serviceName === "ceph") return mockCephService
101-
return null
102-
},
103-
getToken: vi.fn().mockReturnValue(mockToken),
104-
}
100+
const mockOpenstack =
101+
hasToken && hasCatalog
102+
? {
103+
service: (serviceName: string) => {
104+
if (serviceName === "ceph" && hasCephService) return mockCephService
105+
return null
106+
},
107+
getToken: vi.fn().mockReturnValue(mockToken),
108+
}
109+
: {
110+
service: () => null,
111+
getToken: vi.fn().mockReturnValue(null),
112+
}
105113

106114
return {
107115
req: { headers: {} },

‎apps/aurora-portal/src/server/Storage/cephProcedure.ts‎

Lines changed: 43 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -20,65 +20,56 @@ export const NO_CEPH_CREDENTIALS = "NO_CEPH_CREDENTIALS" as const
2020
function resolveS3Config(ctx: AuroraPortalContext): { endpoint: string; region: string } {
2121
try {
2222
const service = ctx.openstack?.service("ceph")
23-
const endpoint = service?.getEndpoint?.()
24-
25-
if (endpoint) {
26-
// Extract base URL by removing Swift path suffix.
27-
// Ceph RGW serves both Swift and S3 APIs on the same host but different paths:
28-
// Swift: https://rgw.st1.qa-de-1.cloud.sap/swift/v1/AUTH_xxx
29-
// S3: https://rgw.st1.qa-de-1.cloud.sap
30-
const swiftIndex = endpoint.indexOf("/swift/")
31-
const baseEndpoint = swiftIndex !== -1 ? endpoint.substring(0, swiftIndex) : endpoint
32-
33-
const token = ctx.openstack?.getToken?.()
34-
35-
if (!token?.tokenData?.catalog) {
36-
throw new Error("OpenStack token or service catalog not available")
37-
}
38-
39-
// Find Ceph service in catalog by checking common type/name patterns
40-
const cephService = token.tokenData.catalog.find(
41-
(s) => s.type === "ceph" || s.name === "ceph" || s.type === "object-store" || s.type === "object-store-ceph"
42-
)
43-
44-
if (!cephService) {
45-
throw new Error("Ceph service not found in OpenStack service catalog")
46-
}
47-
48-
const openstackRegion = cephService.endpoints?.[0]?.region
49-
50-
if (!openstackRegion) {
51-
throw new Error("Region not found in Ceph service endpoints")
52-
}
53-
54-
// Construct Ceph-compatible region identifier using the pattern from Go SDK / Terraform.
55-
// Standard format: ceph-objectstore-st1-{region} (e.g., ceph-objectstore-st1-eu-de-2)
56-
// Exception: qa-de-1 uses "ec" prefix for historical reasons (ceph-objectstore-ec-st1-qa-de-1)
57-
//
58-
// This identifier is used for:
59-
// 1. AWS Signature V4 request signing (region field in Authorization header)
60-
// 2. LocationConstraint in CreateBucket API calls
61-
//
62-
// See: https://documentation.global.cloud.sap/docs/customer/storage/obj-v2-ceph/ceph-storage-options/
63-
const QA_DE_1_REGION = "qa-de-1"
64-
const CEPH_REGION_PREFIX_STANDARD = "ceph-objectstore-st1"
65-
const CEPH_REGION_PREFIX_EC = "ceph-objectstore-ec-st1"
66-
67-
const region =
68-
openstackRegion === QA_DE_1_REGION
69-
? `${CEPH_REGION_PREFIX_EC}-${openstackRegion}`
70-
: `${CEPH_REGION_PREFIX_STANDARD}-${openstackRegion}`
71-
72-
return { endpoint: baseEndpoint, region }
23+
24+
if (!service) {
25+
throw new Error("Ceph service not found in OpenStack service catalog")
26+
}
27+
28+
const endpoint = service.getEndpoint?.()
29+
30+
if (!endpoint) {
31+
throw new Error("Ceph service endpoint not found in catalog. Ensure the Ceph service is registered in OpenStack.")
32+
}
33+
34+
// Extract base URL by removing Swift path suffix.
35+
// Ceph RGW serves both Swift and S3 APIs on the same host but different paths:
36+
// Swift: https://rgw.st1.qa-de-1.cloud.sap/swift/v1/AUTH_xxx
37+
// S3: https://rgw.st1.qa-de-1.cloud.sap
38+
const swiftIndex = endpoint.indexOf("/swift/")
39+
const baseEndpoint = swiftIndex !== -1 ? endpoint.substring(0, swiftIndex) : endpoint
40+
41+
const endpoints = service.availableEndpoints?.()
42+
const openstackRegion = endpoints?.[0]?.region
43+
44+
if (!openstackRegion) {
45+
throw new Error("Region not found in Ceph service endpoints")
7346
}
47+
48+
// Construct Ceph-compatible region identifier using the pattern from Go SDK / Terraform.
49+
// Standard format: ceph-objectstore-st1-{region} (e.g., ceph-objectstore-st1-eu-de-2)
50+
// Exception: qa-de-1 uses "ec" prefix for historical reasons (ceph-objectstore-ec-st1-qa-de-1)
51+
//
52+
// This identifier is used for:
53+
// 1. AWS Signature V4 request signing (region field in Authorization header)
54+
// 2. LocationConstraint in CreateBucket API calls
55+
//
56+
// See: https://documentation.global.cloud.sap/docs/customer/storage/obj-v2-ceph/ceph-storage-options/
57+
const QA_DE_1_REGION = "qa-de-1"
58+
const CEPH_REGION_PREFIX_STANDARD = "ceph-objectstore-st1"
59+
const CEPH_REGION_PREFIX_EC = "ceph-objectstore-ec-st1"
60+
61+
const region =
62+
openstackRegion === QA_DE_1_REGION
63+
? `${CEPH_REGION_PREFIX_EC}-${openstackRegion}`
64+
: `${CEPH_REGION_PREFIX_STANDARD}-${openstackRegion}`
65+
66+
return { endpoint: baseEndpoint, region }
7467
} catch (error) {
7568
console.error("[ceph] Failed to resolve Ceph service from catalog:", error)
7669
throw new Error("Ceph service not found in catalog. Ensure the Ceph service is registered in OpenStack.", {
7770
cause: error,
7871
})
7972
}
80-
81-
throw new Error("Ceph service endpoint not found in catalog. Ensure the Ceph service is registered in OpenStack.")
8273
}
8374

8475
/**

‎apps/aurora-portal/src/server/Storage/routers/ceph/containerRouter.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,15 @@ const createMockContext = (shouldFailAuth = false, hasCredentials = true) => {
4545

4646
const mockCephService = {
4747
getEndpoint: () => "https://test-ceph.example.com",
48+
availableEndpoints: () => [
49+
{
50+
region: "test-region",
51+
url: "https://test-ceph.example.com",
52+
interface: "public",
53+
id: "test-id",
54+
region_id: "test-region",
55+
},
56+
],
4857
}
4958

5059
const mockToken = {

‎apps/aurora-portal/src/server/Storage/routers/ceph/objectRouter.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,15 @@ const createMockContext = (shouldFailAuth = false, hasCredentials = true) => {
4747

4848
const mockCephService = {
4949
getEndpoint: () => "https://test-ceph.example.com",
50+
availableEndpoints: () => [
51+
{
52+
region: "test-region",
53+
url: "https://test-ceph.example.com",
54+
interface: "public",
55+
id: "test-id",
56+
region_id: "test-region",
57+
},
58+
],
5059
}
5160

5261
const mockToken = {

0 commit comments

Comments
 (0)