feat(aurora-portal): implement create and delete opetations bucket#806
Open
KirylSAP wants to merge 42 commits into
Open
feat(aurora-portal): implement create and delete opetations bucket#806KirylSAP wants to merge 42 commits into
KirylSAP wants to merge 42 commits into
Conversation
Align Ceph container data structure with Swift ContainerSummary: - Add count, bytes, last_modified fields to containerSchema - Update list procedure to fetch metadata via ListObjectsV2Command - Add deleteAll procedure for emptying buckets - Remove getDetails procedure (replaced by enhanced list) This enables UI parity between Swift and Ceph containers view.
…ulk operations Frontend refactor to achieve UI parity with Swift containers: - Add ListToolbar with sorting (name, count, bytes, last_modified) and search - Implement URL-persisted state for sort/search (survives navigation) - Create ContainerTableView with @tanstack/react-virtual for performance - Add checkbox selection for bulk operations - Implement EmptyBucketModal and EmptyBucketsModal - Update toast notifications to Swift standard (title/description structure) - Support "Empty All" bulk operation with progress tracking The UI is now fully consistent with Swift containers view.
Remove duplicate routes at /storage/ceph/* to consolidate all storage providers under the shared /storage/$provider/* route structure. This ensures Ceph uses the same routing as Swift with proper provider switching support.
- Auto-resolve Ceph region from OpenStack service catalog - Add special case for qa-de-1 region (uses ec prefix for historical reasons) - Remove CEPH_REGION environment variable (no longer needed) - Remove debug console.log statements - Add comprehensive JSDoc comments to resolveS3Config and bucket operations - Extract magic strings to constants for better maintainability
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDerives Ceph S3 endpoint and region from the OpenStack service catalog, adds Ceph TRPC routers for containers and objects, implements client-side bucket management (table, list, create/delete/empty modals, bulk-empty, toasts), migrates generated routes from /storage/ceph to /storage/$provider, and updates i18n and docs. ChangesCeph S3 Bucket Management and Provider-Based Storage
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Add unit tests for previously untested Ceph storage files: - cephProcedure.ts: S3 config resolution, middleware, and procedures (15 tests) - s3Client.ts: Client factory with validation (16 tests) - s3ErrorMapper.ts: Error mapping from AWS SDK to TRPC (27 tests) Fix existing router tests by adding catalog structure to mock context: - containerRouter.test.ts: Add catalog to token data, update expected results - objectRouter.test.ts: Add catalog to token data All tests follow existing patterns from router tests using Vitest. Total: 58 new tests, all passing (4149/4153 tests pass).
Replace all 'any' type assertions with proper TypeScript types: - s3Client.test.ts: Use 'as unknown as string' for null/undefined tests - cephProcedure.test.ts: Create MockS3Client type for S3Client mocks - s3ErrorMapper.test.ts: Use ReturnType<typeof vi.spyOn> for spy type All tests still passing (58/58). No eslint-disable comments needed.
…credential prompt Add comprehensive test coverage for Ceph frontend components: - ContainerToastNotifications.test.tsx (97 tests): - getBucketCreatedToast - getBucketCreateErrorToast - getBucketEmptiedToast (plural, singular, zero cases) - getBucketEmptyErrorToast - getBucketDeletedToast - getBucketDeleteErrorToast - getBucketsEmptyCompleteToast (success/warning variants) - Toast configuration validation - CredentialPrompt.test.tsx (16 tests): - Rendering setup prompt with EC2 credential explanation - Button interaction with TRPC mutation - Success handling with callback and query invalidation - Error handling with toast display - Layout and styling validation All tests follow existing patterns from Swift component tests. Total Ceph test coverage: 187 tests (58 backend + 129 frontend).
Add comprehensive test coverage for Ceph bucket modals: - CreateBucketModal.test.tsx (48 tests): - UI elements rendering - Valid bucket name validation (lowercase, numbers, periods, hyphens) - Length validation (3-63 characters) - Character validation (no uppercase, special chars, invalid start/end) - IP address format rejection - Reserved prefix/suffix validation (xn--, sthree-, amzn-s3-demo-, -s3alias, --ol-s3, etc) - Real-time validation feedback - Bucket creation with TRPC mutation - Success/error handling with callbacks - DeleteBucketModal.test.tsx (35 tests): - Visibility logic (isOpen, bucket null checks) - UI elements for empty/non-empty buckets - Loading state with bucket contents check - Non-empty bucket error message (plural/singular forms) - Objects query error handling - Bucket name display and copy functionality - Confirmation name validation - Bucket deletion with TRPC mutation - Success/error handling with query invalidation Total Ceph test coverage: 212 tests (58 backend + 154 frontend).
Add comprehensive test coverage for bucket emptying modals: - EmptyBucketModal.test.tsx (33 tests): - Visibility logic (isOpen, bucket null checks) - Empty bucket state (info message, Got it! button) - Non-empty bucket UI with warning and object count (plural/singular) - Bucket name display and copy functionality - Confirmation name validation - Bucket emptying mutation with TRPC - Success/error handling with callbacks - Modal close behavior - EmptyBucketsModal.test.tsx (23 tests): - Visibility with empty buckets array - UI elements with bucket count (plural/singular) - Bucket list display (max 20 visible, hidden count) - Object count for each bucket (plural/singular) - Batch emptying process with sequential mutations - Success handling with emptiedCount and totalDeleted - Error handling (partial failures, all failures) - Query invalidation logic (only when some succeed) - Modal close and state reset Total Ceph test coverage: 268 tests (58 backend + 210 frontend).
Add comprehensive test coverage for ContainerTableView component: - Empty state rendering - Table structure with headers (Bucket Name, Object Count, Last Modified, Total Size) - Footer with bucket count (singular/plural forms) - Selection functionality (select all checkbox, individual container checkboxes) - Row navigation (click and Enter key) - Action menu presence (PopupMenu for each container) - Modal visibility (CreateBucketModal, EmptyBucketModal, DeleteBucketModal) - Date formatting Total: 24 tests
Add comprehensive test coverage for ContainerListView component: - Loading state with spinner - Error handling (NO_CEPH_CREDENTIALS, other errors) - CredentialPrompt integration - Empty state rendering - Container list rendering (names, dates, delete buttons) - Create Bucket button and modal - Delete Bucket button and modal - Table structure Total: 21 tests
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds S3-compatible bucket management for Ceph RGW in Aurora Portal, including backend tRPC endpoints, updated Ceph S3 config resolution from the OpenStack service catalog, and a new frontend bucket management UI (create/empty/delete) with extensive test coverage and updated docs/localization.
Changes:
- Added backend bucket CRUD + “empty bucket” operations and expanded bucket listing to include metadata (count/bytes/last_modified).
- Implemented new frontend bucket management views/modals (create, empty single/multi, delete) with toasts, selection, and virtualization.
- Updated Ceph S3 config resolution (endpoint + region) and added comprehensive unit tests, docs, and i18n strings.
Reviewed changes
Copilot reviewed 32 out of 34 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/aurora-portal/src/server/Storage/types/ceph.ts | Updates container/bucket schema and introduces bucket create/delete input schemas. |
| apps/aurora-portal/src/server/Storage/routers/objectRouter.ts | Adds deleteAll mutation to empty a bucket via batched S3 deletes. |
| apps/aurora-portal/src/server/Storage/routers/objectRouter.test.ts | Updates mocked OpenStack token catalog setup for object router tests. |
| apps/aurora-portal/src/server/Storage/routers/containerRouter.ts | Adds bucket create/delete procedures and augments list output with metadata. |
| apps/aurora-portal/src/server/Storage/routers/containerRouter.test.ts | Updates tests for metadata-enriched bucket list and removes getDetails tests. |
| apps/aurora-portal/src/server/Storage/helpers/s3ErrorMapper.test.ts | Adds unit tests covering S3→tRPC error mapping behavior. |
| apps/aurora-portal/src/server/Storage/clients/s3Client.test.ts | Adds validation and configuration tests for the S3 client factory. |
| apps/aurora-portal/src/server/Storage/cephProcedure.ts | Resolves S3 endpoint/region from OpenStack service catalog; adds cephRegion to context. |
| apps/aurora-portal/src/server/Storage/cephProcedure.test.ts | Adds tests for endpoint extraction, region construction, and middleware behavior. |
| apps/aurora-portal/src/locales/en/messages.po | Adds/updates English strings for bucket operations and UI states. |
| apps/aurora-portal/src/locales/de/messages.po | Adds/updates German catalog entries for new bucket UI strings (many untranslated). |
| apps/aurora-portal/src/client/routeTree.gen.ts | Updates generated route tree to remove Ceph-specific routes and use provider-based routes. |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/ceph/index.tsx | Removes old Ceph storage layout route. |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/ceph/containers/index.tsx | Removes old Ceph containers page route. |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/ceph/containers/$containerName/objects/index.tsx | Removes old Ceph objects page route. |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/index.tsx | Implements the main Ceph bucket page behavior (sorting/searching/selection/toasts). |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/EmptyBucketsModal.tsx | Adds modal to empty multiple buckets with progress and partial failure handling. |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/EmptyBucketsModal.test.tsx | Adds tests for batch empty modal behavior and error handling. |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/EmptyBucketModal.tsx | Adds modal to empty a single bucket with name confirmation and copy helper. |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/EmptyBucketModal.test.tsx | Adds tests for single empty modal flows (success/error/empty-bucket state). |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/DeleteBucketModal.tsx | Adds delete modal with preflight “bucket is empty” check and confirmation. |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/DeleteBucketModal.test.tsx | Adds tests for delete modal flow, object-check gating, and error states. |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/CredentialPrompt.test.tsx | Adds tests for credential setup prompt behavior. |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/CreateBucketModal.tsx | Adds create-bucket modal with client-side S3 naming validation. |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/CreateBucketModal.test.tsx | Adds extensive tests for bucket name validation and create flows. |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/ContainerToastNotifications.tsx | Adds toast builders for create/empty/delete and batch-empty results. |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/ContainerToastNotifications.test.tsx | Adds tests for toast content/configuration and pluralization. |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/ContainerTableView.tsx | Adds virtualized bucket table with selection, navigation, and action menus. |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/ContainerTableView.test.tsx | Adds tests for table rendering, selection, navigation, and modal toggling. |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/ContainerListView.tsx | Extends list view with create/delete actions and toast wiring (legacy/non-virtualized view). |
| apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/ContainerListView.test.tsx | Adds tests for list view states and modal interactions. |
| apps/aurora-portal/docs/009_ceph_s3_bff.md | Updates Ceph S3 BFF documentation to reflect implemented bucket management and region resolution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fix comparator function to return 0 when both values are missing, preventing unstable/inconsistent sorting behavior. Previous logic: - Returned 1 when both last_modified were missing - Violated comparator contract (equal items should return 0) New logic: - Returns 0 when both are missing (equal) - Returns 1 when only first is missing (first goes last) - Returns -1 when only second is missing (first goes first) - Compares timestamps when both present This ensures stable sort order where items without last_modified consistently appear at the end of the sorted list.
Add validation to ensure all objects have valid Key field before attempting batch deletion. Previously, objects with undefined keys would be included in delete requests, causing S3 API errors. Changes: - Filter and validate object keys before deletion - Throw descriptive error if objects without keys are encountered - Fail fast with clear error message instead of sending invalid requests Tests added (5 new tests): - Delete all objects and return count - Handle paginated deletion with multiple batches - Return 0 when bucket is empty - Throw error when objects have undefined keys ✨ NEW - Throw NOT_FOUND when bucket does not exist This prevents invalid delete requests and provides better error messages when S3 API returns unexpected data.
Add includeMetadata parameter to containers.list endpoint to address performance issues when listing many buckets. Previously, every bucket triggered a ListObjectsV2 request in parallel, causing thundering herd problems and potential rate limit issues. Changes: - Add includeMetadata boolean parameter (default: false) to listContainersInputSchema - Fast path (includeMetadata=false): Returns basic bucket info only (name, creationDate) - Slow path (includeMetadata=true): Fetches full metadata with CONCURRENCY_LIMIT=5 - Batched concurrent requests prevent overwhelming S3 API Performance impact: - Without metadata: ~1 ListBuckets request (fast) - With metadata: 1 ListBuckets + N ListObjectsV2 requests (batched in groups of 5) This allows frontends to choose between fast listing (for initial load) and detailed metadata (when user explicitly requests it or for table views). Addresses Copilot feedback about thundering herd and rate limits.
Update frontend components to use new includeMetadata parameter: - ContainerTableView (index.tsx): Set includeMetadata=true for full table with sorting by object count, size, and last modified date - ContainerListView: Set includeMetadata=false for fast basic list view that only shows bucket names without metadata This optimizes performance: - Basic list view loads instantly (1 API call) - Table view with metadata loads progressively (batched requests)
Add explicit documentation that count, bytes, and last_modified are ESTIMATES when buckets contain >1000 objects, addressing Copilot feedback about potential data inaccuracy. Changes: - Updated containerSchema JSDoc to warn about estimation limitations - Added detailed comments in containerRouter about MaxKeys=1000 impact - Clarified that pagination would be needed for accurate data (expensive) Context: ListObjectsV2 with MaxKeys=1000 means: - count: Capped at 1000 (use KeyCount for actual count up to limit) - bytes: Only sums first 1000 objects - last_modified: May miss newer objects beyond first 1000 Trade-off: Performance vs accuracy. Current implementation prioritizes fast response times over perfect accuracy for large buckets. Alternative solutions considered: 1. Full pagination (accurate but very slow for large buckets) 2. S3 bucket metrics/inventory (requires additional setup) 3. Current approach: Fast estimates with clear documentation ✅ Addresses Copilot comment on lines +36 to +49.
Replace TODO with clear explanation that MaxKeys=1000 limitation is a deliberate design decision, not something that needs fixing. Rationale: - Fast response time for UI list view > perfect accuracy - Full pagination would be prohibitively expensive for large buckets - Current approach (estimates) is the correct trade-off for this use case - Misleading TODOs create false expectations of future "fixes" The limitation is already well-documented in comments and type definitions.
Move storage routers into separate subdirectories to improve organization and avoid confusion between Ceph and Swift implementations. Changes: - Create routers/ceph/ and routers/swift/ subdirectories - Move containerRouter, objectRouter, ec2CredentialRouter to ceph/ - Move swiftRouter to swift/ - Add index.ts exports for each subdirectory - Update all import paths in routers and tests - Update main routers/index.ts to import from subdirectories All 4397 tests passing after restructure.
Fix duplicate i18n message IDs identified by Copilot:
- Change 'EmptyBucketsModal' to use existing '... and {hiddenCount} more' (with spaces)
- Fix 'EmptyBucketModal' Trans block formatting to keep string on one line
This ensures consistent UI copy across features and reduces translation burden.
Regenerated translation files with lingui extract and compile.
Extracted hardcoded MaxKeys=1000 value to S3_MAX_KEYS_PER_REQUEST constant, addressing PR feedback from @mark-karnaukh-extern-sap. Changes: - Created src/server/Storage/constants.ts with S3_MAX_KEYS_PER_REQUEST constant - Updated objectRouter.ts to use the constant - Updated containerRouter.ts to use the constant - Updated comment in types/ceph.ts to reference the constant name This improves maintainability - the value is now defined in one place with proper documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
apps/aurora-portal/src/server/Storage/types/ceph.ts (1)
46-52: ⚡ Quick winConsider making
countandbytesoptional instead of defaulting to 0.The
.default(0)will always provide a value of 0 when these fields are absent, which could be misleading whenincludeMetadata: falseis used (the values aren't fetched, not actually zero). Making them optional (.optional()) would better represent the semantic difference between "no objects" and "metadata not fetched."If 0 is kept, consider clarifying in the comment that 0 may mean "metadata not included" rather than "bucket is empty."
♻️ Alternative schema with optional fields
export const containerSchema = z.object({ name: z.string(), - count: z.number().default(0), // Estimated number of objects (based on first 1000) - bytes: z.number().default(0), // Estimated total size in bytes (based on first 1000) + count: z.number().optional(), // Estimated number of objects (based on first 1000) - undefined when includeMetadata=false + bytes: z.number().optional(), // Estimated total size in bytes (based on first 1000) - undefined when includeMetadata=false last_modified: z.string().optional(), // ISO date string (may not be the absolute latest if >1000 objects) creationDate: z.string().optional(), // Bucket creation date (Ceph-specific, accurate) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/aurora-portal/src/server/Storage/types/ceph.ts` around lines 46 - 52, The containerSchema currently uses .default(0) for the count and bytes fields which masks "metadata not fetched" as zero; update the schema so count and bytes are optional (replace .default(0) with .optional() on the count and bytes z.number() entries in containerSchema) and adjust the inline comments for those fields to note that undefined means "metadata not fetched" (or, if you prefer to keep .default(0), update the comments to explicitly state that 0 may indicate "metadata not included" rather than an empty bucket).apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/index.tsx (1)
71-89: 🏗️ Heavy liftUse structured failure data instead of parsing error strings.
split(": ")in selection recovery couples logic to presentation-format text. A small format change can break failed-bucket retention.Consider changing
EmptyBucketsModalcompletion payload fromerrors: string[]toerrors: { bucketName: string; message: string }[], then derive failed names directly frombucketName.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/aurora-portal/src/client/routes/_auth/projects/`$projectId/storage/-components/Ceph/Containers/index.tsx around lines 71 - 89, handleEmptyAllComplete currently parses error strings with e.split(": ") which is fragile; change the EmptyBucketsModal completion payload to errors: { bucketName: string; message: string }[] and update handleEmptyAllComplete to accept that type, derive failedBucketNames from errors.map(e => e.bucketName) (e.g., new Set(errors.map(...))), and use that Set in the setSelectedContainers filter; also update any places that call handleEmptyAllComplete/EmptyBucketsModal to produce the structured errors and ensure getBucketsEmptyCompleteToast(handleToastDismiss) still receives the new errors shape or adapt the toast helper to accept the structured error objects.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/aurora-portal/docs/009_ceph_s3_bff.md`:
- Around line 824-826: The document currently contradicts itself by marking
"Create bucket (CreateBucketCommand)" and "Delete bucket (DeleteBucketCommand)"
as implemented while the "Next Steps" section still says to “Implement bucket
management”; update the "Next Steps" text to reflect that bucket creation and
deletion are implemented (or if they are not fully complete, remove the
checkmarks and keep the TODOs). Specifically edit the lines referencing
CreateBucketCommand and DeleteBucketCommand and the "Next Steps" paragraph so
both consistently state the current status of bucket management and any
remaining work (e.g., "Configure bucket policies, CORS, lifecycle rules" as the
remaining tasks).
In
`@apps/aurora-portal/src/client/routes/_auth/projects/`$projectId/storage/-components/Ceph/Containers/ContainerListView.tsx:
- Around line 173-179: The Button inside ContainerListView currently uses a
hardcoded title "Delete bucket"; replace that with the same localization helper
used elsewhere in this component (e.g., the t(...) or intl.formatMessage(...)
call present in this file) so the title reads the localized string instead
(e.g., title={t('deleteBucket')} or title={intl.formatMessage({ id:
'deleteBucket' })}), update/add the 'deleteBucket' key in the locale messages,
and keep the onClick handler setDeleteModalBucket(bucket) unchanged.
In
`@apps/aurora-portal/src/client/routes/_auth/projects/`$projectId/storage/-components/Ceph/Containers/ContainerTableView.tsx:
- Around line 66-72: The formatDate function builds a Date from dateString but
doesn't handle invalid dates (which yield "Invalid Date" instead of using the
fallback); update formatDate to create the Date, then check if the date is
invalid (e.g., Number.isNaN(date.getTime()) or Number.isNaN(date.valueOf())) and
return t`N/A` in that case, keeping the existing try/catch for unexpected
exceptions; refer to the formatDate function in ContainerTableView.tsx to apply
this change.
In
`@apps/aurora-portal/src/client/routes/_auth/projects/`$projectId/storage/-components/Ceph/Containers/CreateBucketModal.tsx:
- Around line 67-76: The explicit start/end validation in CreateBucketModal.tsx
is unreachable because S3_BUCKET_NAME_REGEX already rejects names with
leading/trailing '.' or '-', so setNameError for start/end never fires; move the
start/end checks (the /^[a-z0-9]/ and /[a-z0-9]$/ tests that setNameError with
the "start and end" message) before the S3_BUCKET_NAME_REGEX.test(trimmed)
branch (or adjust S3_BUCKET_NAME_REGEX to not reject starts/ends so the separate
check can run), ensuring that the start/end validation using trimmed and
setNameError is evaluated first and returns the more specific error when
applicable.
In
`@apps/aurora-portal/src/client/routes/_auth/projects/`$projectId/storage/-components/Ceph/Containers/DeleteBucketModal.tsx:
- Around line 25-28: navigator.clipboard.writeText(bucket.name) can reject
(permissions/security) and is currently unhandled; update the clipboard call in
the DeleteBucketModal component (the handler where navigator.clipboard.writeText
is invoked) to handle failures by adding a .catch or converting to async/await
with try/catch, only calling setCopied(true) on success, resetting copied state
as before, and logging or showing the error (e.g., processLogger/console.error
or UI toast) so the rejection doesn't become an unhandled promise.
- Around line 44-50: The callbacks use bucket! which can be null if props change
before the async mutation completes; capture the bucket name into a local
variable before starting the mutation and reference that captured name inside
onSuccess and onError instead of bucket! (e.g., const bucketName = bucket?.name
?? ''). Update the onSuccess callback to call onSuccess?.(bucketName) and the
onError callback to call onError?.(bucketName, error?.message) and still call
utils.storage.ceph.containers.list.invalidate() as before.
In
`@apps/aurora-portal/src/client/routes/_auth/projects/`$projectId/storage/-components/Ceph/Containers/EmptyBucketModal.tsx:
- Around line 33-40: The callbacks in emptyBucketMutation
(trpcReact.storage.ceph.objects.deleteAll.useMutation) dereference bucket!
inside async onSuccess/onError which can crash if the modal unmounts; capture a
stable const (e.g. const bucketName = bucket?.name ?? '') immediately before
calling the mutation, then use that bucketName inside onSuccess and onError (and
do the same for the other mutation block at lines ~60-69) so callbacks no longer
depend on the possibly-null bucket reference.
In `@apps/aurora-portal/src/locales/de/messages.po`:
- Around line 337-405: Populate the empty German translations (msgstr "") for
the bucket-related msgids shown (e.g., "Bucket \"{bucketName}\" was already
empty.", "Bucket \"{bucketName}\" was successfully created.", "Bucket Created",
"Bucket name must contain only lowercase letters, numbers, periods, and
hyphens", "Bucket to delete:", "Buckets to empty:" etc.) by providing accurate
German equivalents for each msgid, preserving interpolation tokens like
{bucketName}, {deletedCount}, {suffix}, and {prefix}; also apply the same fixes
for the other ranges called out (625-729, 913-930, 1015-1160, 1585-1587,
1825-1827, 1912-1914, 2287-2289, 2662-2664, 2704-2715, 2773-2775, 2842-2844) so
no msgstr remains empty and grammatical gender/case is correct where tokens are
used.
In `@apps/aurora-portal/src/server/Storage/clients/s3Client.test.ts`:
- Around line 35-53: The tests currently only run assertions inside if(typeof
... === "function") guards which lets a non-function regression slip through;
update the test to first assert that client.config.credentials,
client.config.endpoint, and client.config.region are functions (e.g.
expect(typeof client.config.credentials).toBe("function")) and then await each
resolver and assert their returned values (accessKeyId/secretAccessKey for
credentials, protocol/hostname for endpoint, and region value for region) so the
test fails if the resolvers are not functions as well as when their outputs are
incorrect.
In `@apps/aurora-portal/src/server/Storage/routers/ceph/containerRouter.ts`:
- Around line 129-131: The code claims the AWS SDK will auto-add
CreateBucketConfiguration.LocationConstraint but the CreateBucketCommand call in
containerRouter (around the create bucket logic) only sends { Bucket: bucketName
}; update the CreateBucketCommand invocation to include
CreateBucketConfiguration with LocationConstraint set to ctx.cephRegion so the
region is explicitly provided. Locate the create call that constructs new
CreateBucketCommand and add CreateBucketConfiguration: { LocationConstraint:
ctx.cephRegion } to the command input (reference: CreateBucketCommand,
ctx.cephRegion, containerRouter.ts).
---
Nitpick comments:
In
`@apps/aurora-portal/src/client/routes/_auth/projects/`$projectId/storage/-components/Ceph/Containers/index.tsx:
- Around line 71-89: handleEmptyAllComplete currently parses error strings with
e.split(": ") which is fragile; change the EmptyBucketsModal completion payload
to errors: { bucketName: string; message: string }[] and update
handleEmptyAllComplete to accept that type, derive failedBucketNames from
errors.map(e => e.bucketName) (e.g., new Set(errors.map(...))), and use that Set
in the setSelectedContainers filter; also update any places that call
handleEmptyAllComplete/EmptyBucketsModal to produce the structured errors and
ensure getBucketsEmptyCompleteToast(handleToastDismiss) still receives the new
errors shape or adapt the toast helper to accept the structured error objects.
In `@apps/aurora-portal/src/server/Storage/types/ceph.ts`:
- Around line 46-52: The containerSchema currently uses .default(0) for the
count and bytes fields which masks "metadata not fetched" as zero; update the
schema so count and bytes are optional (replace .default(0) with .optional() on
the count and bytes z.number() entries in containerSchema) and adjust the inline
comments for those fields to note that undefined means "metadata not fetched"
(or, if you prefer to keep .default(0), update the comments to explicitly state
that 0 may indicate "metadata not included" rather than an empty bucket).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d87cd74a-6050-416f-9659-795435d4afb1
📒 Files selected for processing (44)
apps/aurora-portal/docs/009_ceph_s3_bff.mdapps/aurora-portal/src/client/routeTree.gen.tsapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/ContainerListView.test.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/ContainerListView.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/ContainerTableView.test.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/ContainerTableView.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/ContainerToastNotifications.test.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/ContainerToastNotifications.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/CreateBucketModal.test.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/CreateBucketModal.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/CredentialPrompt.test.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/DeleteBucketModal.test.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/DeleteBucketModal.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/EmptyBucketModal.test.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/EmptyBucketModal.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/EmptyBucketsModal.test.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/EmptyBucketsModal.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/index.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/ceph/containers/$containerName/objects/index.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/ceph/containers/index.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/ceph/index.tsxapps/aurora-portal/src/locales/de/messages.poapps/aurora-portal/src/locales/de/messages.tsapps/aurora-portal/src/locales/en/messages.poapps/aurora-portal/src/locales/en/messages.tsapps/aurora-portal/src/server/Storage/cephProcedure.test.tsapps/aurora-portal/src/server/Storage/cephProcedure.tsapps/aurora-portal/src/server/Storage/clients/s3Client.test.tsapps/aurora-portal/src/server/Storage/constants.tsapps/aurora-portal/src/server/Storage/helpers/s3ErrorMapper.test.tsapps/aurora-portal/src/server/Storage/routers/ceph/containerRouter.test.tsapps/aurora-portal/src/server/Storage/routers/ceph/containerRouter.tsapps/aurora-portal/src/server/Storage/routers/ceph/ec2CredentialRouter.test.tsapps/aurora-portal/src/server/Storage/routers/ceph/ec2CredentialRouter.tsapps/aurora-portal/src/server/Storage/routers/ceph/index.tsapps/aurora-portal/src/server/Storage/routers/ceph/objectRouter.test.tsapps/aurora-portal/src/server/Storage/routers/ceph/objectRouter.tsapps/aurora-portal/src/server/Storage/routers/containerRouter.tsapps/aurora-portal/src/server/Storage/routers/index.tsapps/aurora-portal/src/server/Storage/routers/objectRouter.tsapps/aurora-portal/src/server/Storage/routers/swift/index.tsapps/aurora-portal/src/server/Storage/routers/swift/swiftRouter.test.tsapps/aurora-portal/src/server/Storage/routers/swift/swiftRouter.tsapps/aurora-portal/src/server/Storage/types/ceph.ts
💤 Files with no reviewable changes (6)
- apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/ceph/containers/index.tsx
- apps/aurora-portal/src/server/Storage/routers/containerRouter.ts
- apps/aurora-portal/src/server/Storage/routers/objectRouter.ts
- apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/ceph/containers/$containerName/objects/index.tsx
- apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/ceph/index.tsx
- apps/aurora-portal/src/client/routeTree.gen.ts
…containers Empty containers don't have last_modified field, so we now fall back to creationDate which is always present from S3 CreationDate. Changes: - Updated ContainerTableView to use creationDate as fallback - Updated test to reflect the new behavior - formatDate handles empty string gracefully (returns N/A)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/ContainerTableView.tsx (1)
236-236:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid rendering
"Invalid Date"when both date fields are absent/invalid.Line 236 passes
""intoformatDate, which can surface"Invalid Date"instead of a user-friendly fallback.Minimal fallback-safe rendering
- <DataGridCell>{formatDate(container.last_modified || container.creationDate || "")}</DataGridCell> + <DataGridCell> + {container.last_modified || container.creationDate + ? formatDate(container.last_modified || container.creationDate) + : t`N/A`} + </DataGridCell>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/aurora-portal/src/client/routes/_auth/projects/`$projectId/storage/-components/Ceph/Containers/ContainerTableView.tsx at line 236, The DataGridCell currently calls formatDate(container.last_modified || container.creationDate || "") which can render "Invalid Date"; update ContainerTableView to only call formatDate when a valid date value exists (check container.last_modified and container.creationDate explicitly) and otherwise render a safe fallback like an empty string or a placeholder (e.g., "—"); ensure you reference the formatDate call and the container.{last_modified,creationDate} checks so the rendered cell never receives an empty string that produces "Invalid Date".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/aurora-portal/src/client/routes/_auth/projects/`$projectId/storage/-components/Ceph/Containers/ContainerTableView.test.tsx:
- Around line 366-370: The current test "shows creationDate as fallback when
last_modified is missing" uses a broad screen.getAllByText(/2024/) that can
match any row; instead target the specific container row that lacks
last_modified (use the test fixture container name or id used by
renderTableView) and assert that within that row the creation date is rendered.
Locate the row via the known unique label (e.g., container name or data-testid),
then use a scoped query (within(row).getByText(expectedCreationDate) or
expect(row).toHaveTextContent(expectedCreationDate)) to verify the fallback,
keeping the test name and renderTableView usage intact.
---
Duplicate comments:
In
`@apps/aurora-portal/src/client/routes/_auth/projects/`$projectId/storage/-components/Ceph/Containers/ContainerTableView.tsx:
- Line 236: The DataGridCell currently calls formatDate(container.last_modified
|| container.creationDate || "") which can render "Invalid Date"; update
ContainerTableView to only call formatDate when a valid date value exists (check
container.last_modified and container.creationDate explicitly) and otherwise
render a safe fallback like an empty string or a placeholder (e.g., "—"); ensure
you reference the formatDate call and the container.{last_modified,creationDate}
checks so the rendered cell never receives an empty string that produces
"Invalid Date".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 741896db-245d-4573-b3ad-606997b30a7f
📒 Files selected for processing (4)
apps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/ContainerTableView.test.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/ContainerTableView.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/DeleteBucketModal.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/storage/-components/Ceph/Containers/EmptyBucketModal.tsx
Capture bucket name before async operations to prevent accessing null bucket reference if modal closes before callbacks execute. - EmptyBucketModal: capture bucketName in handleSubmit, pass to inline onSuccess/onError - DeleteBucketModal: capture bucketName in handleSubmit, pass to inline onSuccess/onError - Update tests to expect second parameter (options) in mutate() calls - Move invalidate() to onSettled to ensure it runs in both success and error cases
KirylSAP
force-pushed
the
kiryl-bucket-opetations-write
branch
from
27faf5e to
cf3ab7d
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/aurora-portal/src/server/Storage/cephProcedure.ts (1)
48-59:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve CEPH_REGION misconfiguration errors instead of rewriting them as catalog failures.
Line 48-50 throws a config error, but Line 57 rewrites it to a catalog-not-found message. That makes operator feedback incorrect and hides the true fix path.
Suggested fix
function resolveS3Config(ctx: AuroraPortalContext): { endpoint: string; region: string } { try { @@ - if (!process.env.CEPH_REGION) { + if (!process.env.CEPH_REGION) { throw new Error("CEPH_REGION environment variable is required. ") } @@ return { endpoint: baseEndpoint, region } } catch (error) { console.error("[ceph] Failed to resolve Ceph service from catalog:", error) - throw new Error("Ceph service not found in catalog. Ensure the Ceph service is registered in OpenStack.", { - cause: error, - }) + if (error instanceof Error && error.message.includes("CEPH_REGION environment variable is required")) { + throw error + } + throw new Error("Ceph service not found in catalog. Ensure the Ceph service is registered in OpenStack.", { + cause: error, + }) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/aurora-portal/src/server/Storage/cephProcedure.ts` around lines 48 - 59, The catch block in cephProcedure.ts is overwriting a configuration error thrown when CEPH_REGION is missing with a generic "Ceph service not found" message; update the catch logic so that if the original error is the CEPH_REGION misconfiguration (e.g., error.message includes "CEPH_REGION" or process.env.CEPH_REGION is falsy) you rethrow the original error unchanged, otherwise wrap/errors related to catalog lookup as currently done (preserving the original error as the cause). Target the throw that produces "CEPH_REGION environment variable is required." and the catch that currently throws new Error("Ceph service not found...") to implement this conditional rethrowing while still preserving original error details for non-config failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/aurora-portal/src/server/Storage/cephProcedure.ts`:
- Around line 48-53: The current truthy check for process.env.CEPH_REGION
permits whitespace-only values; update the validation so you trim the
environment value and reject empty strings—e.g., read raw =
process.env.CEPH_REGION, if raw is undefined or raw.trim().length === 0 throw
new Error("CEPH_REGION environment variable is required and must not be blank"),
then set region = raw.trim() so downstream S3 signing/config uses the trimmed
non-empty value (update any references to region accordingly).
---
Outside diff comments:
In `@apps/aurora-portal/src/server/Storage/cephProcedure.ts`:
- Around line 48-59: The catch block in cephProcedure.ts is overwriting a
configuration error thrown when CEPH_REGION is missing with a generic "Ceph
service not found" message; update the catch logic so that if the original error
is the CEPH_REGION misconfiguration (e.g., error.message includes "CEPH_REGION"
or process.env.CEPH_REGION is falsy) you rethrow the original error unchanged,
otherwise wrap/errors related to catalog lookup as currently done (preserving
the original error as the cause). Target the throw that produces "CEPH_REGION
environment variable is required." and the catch that currently throws new
Error("Ceph service not found...") to implement this conditional rethrowing
while still preserving original error details for non-config failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 89f6fd6e-b75f-4585-a33c-a72d9746c4aa
📒 Files selected for processing (5)
apps/aurora-portal/.env.exampleapps/aurora-portal/src/server/Storage/cephProcedure.test.tsapps/aurora-portal/src/server/Storage/cephProcedure.tsapps/aurora-portal/src/server/Storage/routers/ceph/containerRouter.test.tsapps/aurora-portal/src/server/Storage/routers/ceph/objectRouter.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/aurora-portal/src/server/Storage/routers/ceph/objectRouter.test.ts
- apps/aurora-portal/src/server/Storage/cephProcedure.test.ts
- apps/aurora-portal/src/server/Storage/routers/ceph/containerRouter.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/aurora-portal/.env.example`:
- Line 11: The example env currently sets INSECURE_COOKIES=true which is
insecure by default; change the INSECURE_COOKIES value in the .env.example to
false (INSECURE_COOKIES=false) and add a brief inline comment next to the
INSECURE_COOKIES setting explaining that developers can opt into insecure
cookies for local testing only (e.g., "set to true only for local/dev/testing"),
so production/staging are secure by default; ensure the variable name
INSECURE_COOKIES is the one updated so callers of that env var continue to work.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4d3dbe39-f06c-47b6-9d9c-f08bb26a960d
📒 Files selected for processing (5)
apps/aurora-portal/.env.exampleapps/aurora-portal/src/locales/de/messages.poapps/aurora-portal/src/locales/de/messages.tsapps/aurora-portal/src/locales/en/messages.poapps/aurora-portal/src/locales/en/messages.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/aurora-portal/src/locales/en/messages.po
- apps/aurora-portal/src/locales/de/messages.po
TilmanHaupt
previously approved these changes
May 28, 2026
TilmanHaupt
approved these changes
May 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implement S3-compatible bucket management for Ceph storage with full CRUD operations. This PR adds comprehensive frontend UI, backend API routes, and test coverage (268 tests total) for creating, deleting, and emptying S3 buckets through the Aurora Portal.
Changes Made
Frontend Components (New)
Container Management Views
Bucket Operation Modals
Backend Implementation (New)
TRPC Routers & Procedures
list- List all buckets in a projectcreate- Create new bucket with name validationdelete- Delete bucket (checks if empty first)list- List objects in a bucketdeleteAll- Delete all objects from a bucketUtilities & Helpers
Test Coverage (268 tests)
Frontend Tests (210 tests)
Backend Tests (58 tests)
Documentation
Localization
de) and English (en) translation files with new UI strings for bucket operationsRelated Issues
Testing Instructions
Install dependencies:
Run all tests:
pnpm run testChecklist
Additional Notes
S3 Bucket Naming Validation
Implements full S3-compliant bucket naming rules:
192.168.1.1)xn--,sthree-,sthree-configurator-s3alias,--ol-s3-mrap,--x-s3..)Architecture
Test Statistics
Summary by CodeRabbit
New Features
Improvements
Documentation