feat(backups): add page for cluster administration#21
feat(backups): add page for cluster administration#21Andrey Kolkov (androndo) wants to merge 2 commits into
Conversation
|
Warning Review limit reached
More reviews will be available in 58 minutes and 28 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
78bbb1d to
a00cefb
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces administration capabilities for cluster-scoped Backup Classes, including pages for listing, creating, viewing, and editing backup classes, protected by a permission-based route guard. It also updates the SchemaForm component to recursively bind custom key/value editors to additionalProperties maps nested inside array items. The reviewer feedback highlights two issues in BackupClassListPage.tsx where API errors are not handled in SystemBackupBucketPanel and BackupArtifactsCountPanel, which could lead to misleading UI states (e.g., showing "Not configured" or a count of "0" instead of the actual error).
| const { data: bucket, isLoading } = useK8sGet<ApplicationInstance>({ | ||
| apiGroup: "apps.cozystack.io", | ||
| apiVersion: "v1alpha1", | ||
| plural: "buckets", | ||
| name: "cozy-backups", | ||
| namespace: "tenant-root", | ||
| }) | ||
| // Reuse the Bucket ApplicationDefinition's own icon (the same red S3 | ||
| // glyph rendered on deployed-app cards) so the bucket here is visually | ||
| // recognisable as a Bucket app. | ||
| const { data: appDefs } = useApplicationDefinitions() | ||
| const bucketAd = appDefs?.items.find((d) => d.spec?.application.kind === "Bucket") | ||
| const bucketIcon = bucketAd ? iconDataUrl(bucketAd) : undefined | ||
|
|
||
| if (isLoading) { | ||
| return ( | ||
| <Section className="h-full" bodyClassName="p-0"> | ||
| <div className="flex h-full items-center gap-2 p-4 text-sm text-slate-500"> | ||
| <Spinner /> Loading system backup bucket… | ||
| </div> | ||
| </Section> | ||
| ) | ||
| } |
There was a problem hiding this comment.
The SystemBackupBucketPanel component does not handle the error state returned by useK8sGet. If the API request fails (e.g., due to RBAC issues or network errors), bucket will be undefined, causing the component to incorrectly display that the bucket is "Not configured" or "Not deployed on this cluster". Handling the error state explicitly ensures the user is presented with the actual error message instead of a misleading status.
const { data: bucket, isLoading, error } = useK8sGet<ApplicationInstance>({
apiGroup: "apps.cozystack.io",
apiVersion: "v1alpha1",
plural: "buckets",
name: "cozy-backups",
namespace: "tenant-root",
})
// Reuse the Bucket ApplicationDefinition's own icon (the same red S3
// glyph rendered on deployed-app cards) so the bucket here is visually
// recognisable as a Bucket app.
const { data: appDefs } = useApplicationDefinitions()
const bucketAd = appDefs?.items.find((d) => d.spec?.application.kind === "Bucket")
const bucketIcon = bucketAd ? iconDataUrl(bucketAd) : undefined
if (isLoading) {
return (
<Section className="h-full" bodyClassName="p-0">
<div className="flex h-full items-center gap-2 p-4 text-sm text-slate-500">
<Spinner /> Loading system backup bucket…
</div>
</Section>
)
}
if (error) {
return (
<Section className="h-full" bodyClassName="p-0">
<div className="flex h-full items-center gap-2 p-4 text-sm text-red-600">
Failed to load system backup bucket: {(error as Error).message}
</div>
</Section>
)
}
| function BackupArtifactsCountPanel() { | ||
| const { data, isLoading } = useK8sList<K8sResource>({ | ||
| apiGroup: "backups.cozystack.io", | ||
| apiVersion: "v1alpha1", | ||
| plural: "backups", | ||
| }) | ||
|
|
||
| if (isLoading) { | ||
| return ( | ||
| <Section className="h-full" bodyClassName="p-0"> | ||
| <div className="flex h-full items-center gap-2 p-4 text-sm text-slate-500"> | ||
| <Spinner /> Loading backup artifacts… | ||
| </div> | ||
| </Section> | ||
| ) | ||
| } |
There was a problem hiding this comment.
The BackupArtifactsCountPanel component does not handle the error state returned by useK8sList. If the API request fails, the component will silently fall back to displaying 0 backup artifacts, which is misleading to the administrator. Handling the error state explicitly provides a clear indication of the failure.
function BackupArtifactsCountPanel() {
const { data, isLoading, error } = useK8sList<K8sResource>({
apiGroup: "backups.cozystack.io",
apiVersion: "v1alpha1",
plural: "backups",
})
if (isLoading) {
return (
<Section className="h-full" bodyClassName="p-0">
<div className="flex h-full items-center gap-2 p-4 text-sm text-slate-500">
<Spinner /> Loading backup artifacts…
</div>
</Section>
)
}
if (error) {
return (
<Section className="h-full" bodyClassName="p-0">
<div className="flex h-full items-center gap-2 p-4 text-sm text-red-600">
Failed to load backup artifacts: {(error as Error).message}
</div>
</Section>
)
}
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
cf8e92e to
19c4a0f
Compare
Represent backups configuration for cluster admins: