From 32d81c6692e6c27264caa518fd5571b56752a15f Mon Sep 17 00:00:00 2001 From: Elay Aharoni Date: Thu, 13 Aug 2026 08:56:08 +0300 Subject: [PATCH 1/6] OSAC-3601: send lock=true from useUpdateStorageBackend for optimistic concurrency Assisted-by: Claude Code Signed-off-by: Elay Aharoni --- .../src/api/v1/private/storage-backends.test.ts | 9 +++++++++ .../ui-components/src/api/v1/private/storage-backends.ts | 1 + 2 files changed, 10 insertions(+) diff --git a/libs/ui-components/src/api/v1/private/storage-backends.test.ts b/libs/ui-components/src/api/v1/private/storage-backends.test.ts index bb9b3e04..f7c92c85 100644 --- a/libs/ui-components/src/api/v1/private/storage-backends.test.ts +++ b/libs/ui-components/src/api/v1/private/storage-backends.test.ts @@ -214,6 +214,15 @@ describe('useUpdateStorageBackend', () => { password: 'test-updated-secret', }); }); + + it('sends lock: true for optimistic concurrency', async () => { + const captured = await mutateAndCaptureUpdate({ + id: 'b-1', + spec: { endpoint: 'new.example.com' }, + }); + + expect(captured?.lock).toBe(true); + }); }); describe('useDeleteStorageBackend', () => { diff --git a/libs/ui-components/src/api/v1/private/storage-backends.ts b/libs/ui-components/src/api/v1/private/storage-backends.ts index 0b95eb0d..083f056c 100644 --- a/libs/ui-components/src/api/v1/private/storage-backends.ts +++ b/libs/ui-components/src/api/v1/private/storage-backends.ts @@ -83,6 +83,7 @@ export const useUpdateStorageBackend = () => { const resp = await client.update({ object: { id: input.id, spec }, updateMask: { paths: buildUpdateMaskPaths({ spec } as Record) }, + lock: true, }); if (!resp.object) { throw new Error('Update response missing object'); From 4f84e88734d366d592d89ea1298f1ec5059241fb Mon Sep 17 00:00:00 2001 From: Elay Aharoni Date: Thu, 13 Aug 2026 09:07:15 +0300 Subject: [PATCH 2/6] OSAC-3601: add StorageBackendEditPage Assisted-by: Claude Code Signed-off-by: Elay Aharoni --- libs/i18n/locales/en/translation.json | 5 + .../admin/StorageBackendEditPage.test.tsx | 232 ++++++++++++++++++ .../pages/admin/StorageBackendEditPage.tsx | 230 +++++++++++++++++ 3 files changed, 467 insertions(+) create mode 100644 libs/ui-components/src/pages/admin/StorageBackendEditPage.test.tsx create mode 100644 libs/ui-components/src/pages/admin/StorageBackendEditPage.tsx diff --git a/libs/i18n/locales/en/translation.json b/libs/i18n/locales/en/translation.json index b80d82cd..efe09a19 100644 --- a/libs/i18n/locales/en/translation.json +++ b/libs/i18n/locales/en/translation.json @@ -187,6 +187,7 @@ "Encryption enabled": "Encryption enabled", "Endpoint": "Endpoint", "Endpoint is required": "Endpoint is required", + "Enter both username and password, or leave both blank": "Enter both username and password, or leave both blank", "Error": "Error", "Error loading external IP pools": "Error loading external IP pools", "Error loading virtual networks": "Error loading virtual networks", @@ -230,6 +231,7 @@ "Failed to fetch instance type": "Failed to fetch instance type", "Failed to fetch role binding": "Failed to fetch role binding", "Failed to fetch security groups": "Failed to fetch security groups", + "Failed to fetch storage backend": "Failed to fetch storage backend", "Failed to fetch subnet": "Failed to fetch subnet", "Failed to fetch virtual network": "Failed to fetch virtual network", "Failed to load cluster password": "Failed to load cluster password", @@ -243,6 +245,7 @@ "Failed to start virtual machine": "Failed to start virtual machine", "Failed to stop virtual machine": "Failed to stop virtual machine", "Failed to update role binding": "Failed to update role binding", + "Failed to update storage backend": "Failed to update storage backend", "Filter bare metal instances by name": "Filter bare metal instances by name", "Filter catalog by keyword": "Filter catalog by keyword", "Filter catalog by resource type": "Filter catalog by resource type", @@ -280,6 +283,7 @@ "Issuer is required": "Issuer is required", "JWKS URL": "JWKS URL", "Keep editing": "Keep editing", + "Leave blank to keep the current credentials.": "Leave blank to keep the current credentials.", "Lifecycle state": "Lifecycle state", "Loading cluster password": "Loading cluster password", "Loading security groups...": "Loading security groups...", @@ -430,6 +434,7 @@ "Stopped": "Stopped", "Stopping": "Stopping", "Storage": "Storage", + "Storage backend not found": "Storage backend not found", "Storage backends": "Storage backends", "Storage tabs": "Storage tabs", "Storage tiers": "Storage tiers", diff --git a/libs/ui-components/src/pages/admin/StorageBackendEditPage.test.tsx b/libs/ui-components/src/pages/admin/StorageBackendEditPage.test.tsx new file mode 100644 index 00000000..16aef380 --- /dev/null +++ b/libs/ui-components/src/pages/admin/StorageBackendEditPage.test.tsx @@ -0,0 +1,232 @@ +import { Route, Routes } from 'react-router-dom'; +import { create } from '@bufbuild/protobuf'; +import { Code, ConnectError } from '@connectrpc/connect'; +import { screen, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { + StorageBackendSchema, + type StorageBackendsUpdateRequest, + StorageBackendsUpdateResponseSchema, +} from '@osac/types/private'; + +import { StorageBackendEditPage } from './StorageBackendEditPage'; +import type { + MockApiFixtures, + MockTransportOverrides, +} from '../../test-utils/createMockConnectTransport'; +import { renderWithProviders } from '../../test-utils/TestProviders'; + +const mockNavigate = vi.fn(); +vi.mock('react-router-dom', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useNavigate: () => mockNavigate, + // useBlocker requires a data router; this test harness renders under a plain + // MemoryRouter, so LeaveFormConfirmation's blocking behavior is stubbed out + // rather than exercised here (mirrors StorageBackendCreatePage.test.tsx). + useBlocker: () => ({ state: 'unblocked' as const }), + }; +}); + +const EDIT_ROUTE_PATH = '/admin/infrastructure/storage/backends/:id/edit'; +const EDIT_PATH = '/admin/infrastructure/storage/backends/b-1/edit'; + +const existingBackend = create(StorageBackendSchema, { + id: 'b-1', + metadata: { name: 'vast-prod-1' }, + spec: { + provider: 'vast', + endpoint: 'vast.example.com:443', + description: 'primary array', + credentials: { username: 'existing-admin', password: 'existing-secret' }, + }, +}); + +const renderPage = (overrides?: MockTransportOverrides, apiFixtures?: MockApiFixtures) => + renderWithProviders( + + } /> + , + { + routerEntries: [EDIT_PATH], + apiFixtures: { storageBackends: [existingBackend], ...apiFixtures }, + transportOverrides: overrides, + }, + ); + +describe('StorageBackendEditPage', () => { + beforeEach(() => { + mockNavigate.mockReset(); + }); + + it('shows a loading spinner before the backend has been fetched', () => { + renderPage(); + + expect(screen.getByRole('progressbar')).toBeInTheDocument(); + }); + + it('renders the page prefilled with the backend endpoint and description', async () => { + renderPage(); + + await waitFor(() => { + expect(screen.getByRole('heading', { name: 'Edit storage backend' })).toBeInTheDocument(); + }); + expect(screen.getByRole('textbox', { name: 'Endpoint' })).toHaveValue('vast.example.com:443'); + expect(screen.getByRole('textbox', { name: 'Description' })).toHaveValue('primary array'); + expect(screen.getByRole('textbox', { name: 'Name' })).toHaveValue('vast-prod-1'); + expect(screen.getByLabelText(/^Provider/)).toHaveTextContent('VAST'); + }); + + it('renders name and provider as disabled', async () => { + renderPage(); + + await waitFor(() => { + expect(screen.getByRole('textbox', { name: 'Name' })).toBeInTheDocument(); + }); + expect(screen.getByRole('textbox', { name: 'Name' })).toBeDisabled(); + expect(screen.getByLabelText(/^Provider/)).toBeDisabled(); + }); + + it('renders credential fields blank regardless of the fetched record', async () => { + renderPage(); + + await waitFor(() => { + expect(screen.getByLabelText(/^Username/)).toBeInTheDocument(); + }); + expect(screen.getByLabelText(/^Username/)).toHaveValue(''); + expect(screen.getByLabelText(/^Password/)).toHaveValue(''); + }); + + it('shows a validation error and does not submit when only username is filled', async () => { + const onStorageBackendUpdate = vi.fn(); + const { user } = renderPage({ onStorageBackendUpdate }); + + await waitFor(() => { + expect(screen.getByLabelText(/^Username/)).toBeInTheDocument(); + }); + await user.type(screen.getByLabelText(/^Username/), 'new-admin'); + await user.click(screen.getByRole('button', { name: 'Save' })); + + await waitFor(() => { + expect( + screen.getAllByText('Enter both username and password, or leave both blank').length, + ).toBeGreaterThan(0); + }); + expect(onStorageBackendUpdate).not.toHaveBeenCalled(); + expect(mockNavigate).not.toHaveBeenCalled(); + }); + + it('shows a validation error and does not submit when only password is filled', async () => { + const onStorageBackendUpdate = vi.fn(); + const { user } = renderPage({ onStorageBackendUpdate }); + + await waitFor(() => { + expect(screen.getByLabelText(/^Password/)).toBeInTheDocument(); + }); + await user.type(screen.getByLabelText(/^Password/), 'new-secret'); + await user.click(screen.getByRole('button', { name: 'Save' })); + + await waitFor(() => { + expect( + screen.getAllByText('Enter both username and password, or leave both blank').length, + ).toBeGreaterThan(0); + }); + expect(onStorageBackendUpdate).not.toHaveBeenCalled(); + expect(mockNavigate).not.toHaveBeenCalled(); + }); + + it('omits credentials and never submits name/provider when both credential fields are left blank', async () => { + let capturedRequest: StorageBackendsUpdateRequest | undefined; + const { user } = renderPage({ + onStorageBackendUpdate: (req) => { + capturedRequest = req; + return create(StorageBackendsUpdateResponseSchema, { object: existingBackend }); + }, + }); + + await waitFor(() => { + expect(screen.getByRole('textbox', { name: 'Endpoint' })).toHaveValue('vast.example.com:443'); + }); + await user.clear(screen.getByRole('textbox', { name: 'Endpoint' })); + await user.type(screen.getByRole('textbox', { name: 'Endpoint' }), 'new.example.com:443'); + await user.click(screen.getByRole('button', { name: 'Save' })); + + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/backends'); + }); + expect(capturedRequest?.object?.spec?.endpoint).toBe('new.example.com:443'); + expect(capturedRequest?.object?.spec?.credentials).toBeUndefined(); + expect(capturedRequest?.object?.metadata).toBeUndefined(); + expect(capturedRequest?.updateMask?.paths).not.toContain('spec.provider'); + expect(capturedRequest?.updateMask?.paths).not.toContain('metadata.name'); + expect(capturedRequest?.updateMask?.paths).not.toContain('spec.credentials'); + expect(capturedRequest?.lock).toBe(true); + }); + + it('submits a complete credentials object when both fields are filled', async () => { + let capturedRequest: StorageBackendsUpdateRequest | undefined; + const { user } = renderPage({ + onStorageBackendUpdate: (req) => { + capturedRequest = req; + return create(StorageBackendsUpdateResponseSchema, { object: existingBackend }); + }, + }); + + await waitFor(() => { + expect(screen.getByLabelText(/^Username/)).toBeInTheDocument(); + }); + await user.type(screen.getByLabelText(/^Username/), 'new-admin'); + await user.type(screen.getByLabelText(/^Password/), 'new-secret'); + await user.click(screen.getByRole('button', { name: 'Save' })); + + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/backends'); + }); + expect(capturedRequest?.object?.spec?.credentials).toMatchObject({ + username: 'new-admin', + password: 'new-secret', + }); + }); + + it('shows a submission error and does not navigate on a stale-version conflict', async () => { + const { user } = renderPage({ + onStorageBackendUpdate: () => { + throw new ConnectError('Storage backend was modified by another request', Code.Aborted); + }, + }); + + await waitFor(() => { + expect(screen.getByRole('textbox', { name: 'Endpoint' })).toHaveValue('vast.example.com:443'); + }); + await user.click(screen.getByRole('button', { name: 'Save' })); + + await waitFor(() => { + expect(screen.getByText('Failed to update storage backend')).toBeInTheDocument(); + }); + expect(screen.getByText('Storage backend was modified by another request')).toBeInTheDocument(); + expect(mockNavigate).not.toHaveBeenCalled(); + }); + + it('shows a not-found error and no form when the backend does not exist', async () => { + renderPage(undefined, { storageBackends: [] }); + + await waitFor(() => { + expect(screen.getByText('Failed to fetch storage backend')).toBeInTheDocument(); + }); + expect(screen.getByText('Storage backend not found')).toBeInTheDocument(); + expect(screen.queryByRole('textbox', { name: 'Endpoint' })).not.toBeInTheDocument(); + }); + + it('navigates back to the backends list on cancel', async () => { + const { user } = renderPage(); + + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Cancel' })).toBeInTheDocument(); + }); + await user.click(screen.getByRole('button', { name: 'Cancel' })); + + expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/backends'); + }); +}); diff --git a/libs/ui-components/src/pages/admin/StorageBackendEditPage.tsx b/libs/ui-components/src/pages/admin/StorageBackendEditPage.tsx new file mode 100644 index 00000000..7bb2f61e --- /dev/null +++ b/libs/ui-components/src/pages/admin/StorageBackendEditPage.tsx @@ -0,0 +1,230 @@ +import { useNavigate, useParams } from 'react-router-dom'; +import { + ActionList, + ActionListGroup, + ActionListItem, + Alert, + Breadcrumb, + BreadcrumbItem, + Bullseye, + Button, + PageSection, + Spinner, + Stack, + StackItem, + Title, +} from '@patternfly/react-core'; +import { Formik } from 'formik'; +import type { TFunction } from 'i18next'; +import * as Yup from 'yup'; + +import type { StorageBackend } from '@osac/types/private'; +import { + usePrivateStorageBackend, + useUpdateStorageBackend, +} from '@osac/ui-components/api/v1/private/storage-backends'; +import NameField from '@osac/ui-components/components/catalogProvision/wizard/fields/NameField'; +import { InputField } from '@osac/ui-components/components/Form/InputField'; +import LeaveFormConfirmation from '@osac/ui-components/components/Form/LeaveFormConfirmation'; +import OsacForm from '@osac/ui-components/components/Form/OsacForm'; +import { + SelectField, + type SelectFieldOption, +} from '@osac/ui-components/components/Form/SelectField'; +import { useTranslation } from '@osac/ui-components/hooks/useTranslation'; +import { getErrorMessage } from '@osac/ui-components/utils/error'; + +const BACKENDS_LIST_PATH = '/admin/infrastructure/storage/backends'; + +interface StorageBackendEditFormValues { + metadata: { name: string }; + provider: string; + endpoint: string; + description: string; + credentials: { username: string; password: string }; +} + +const getStorageBackendEditSchema = (t: TFunction) => { + const pairError = t('Enter both username and password, or leave both blank'); + return Yup.object({ + endpoint: Yup.string().required(t('Endpoint is required')), + description: Yup.string(), + credentials: Yup.object({ + username: Yup.string().test('credentials-pair', pairError, function (value) { + const parent = this.parent as { username?: string; password?: string } | undefined; + return !!value === !!parent?.password; + }), + password: Yup.string().test('credentials-pair', pairError, function (value) { + const parent = this.parent as { username?: string; password?: string } | undefined; + return !!value === !!parent?.username; + }), + }), + }); +}; + +const StorageBackendEditForm = ({ backend }: { backend: StorageBackend }) => { + const { t } = useTranslation(); + const navigate = useNavigate(); + const { mutateAsync, error } = useUpdateStorageBackend(); + + const providerOptions: SelectFieldOption[] = [ + { value: 'vast', label: t('VAST') }, + { value: 'ceph', label: t('Ceph') }, + { value: 'pure', label: t('Pure') }, + ]; + + const initialValues: StorageBackendEditFormValues = { + metadata: { name: backend.metadata?.name ?? '' }, + provider: backend.spec?.provider ?? '', + endpoint: backend.spec?.endpoint ?? '', + description: backend.spec?.description ?? '', + credentials: { username: '', password: '' }, + }; + + return ( + <> + + + + + + + {t('Edit')} + + + {t('Edit storage backend')} + + + + + { + try { + await mutateAsync({ + id: backend.id, + spec: { + endpoint: values.endpoint, + description: values.description, + ...(values.credentials.username && values.credentials.password + ? { + credentials: { + username: values.credentials.username, + password: values.credentials.password, + }, + } + : {}), + }, + }); + navigate(BACKENDS_LIST_PATH); + } catch { + // Surfaced via the mutation's own `error` state below; nothing further to do here. + } + }} + > + {({ submitForm, isSubmitting }) => ( + + + + + + + + + + + + + + {!!error && ( + + + {getErrorMessage(error)} + + + )} + + + + + + + + + + + + + + )} + + + + ); +}; + +export const StorageBackendEditPage = () => { + const { t } = useTranslation(); + const { id } = useParams<{ id: string }>(); + const { data, isLoading, error } = usePrivateStorageBackend(id ?? ''); + + if (isLoading) { + return ( + + + + ); + } + + if (error || !data) { + return ( + + {error ? getErrorMessage(error) : t('Storage backend not found')} + + ); + } + + return ; +}; From 07b267c42ba6969c9b845c656fe135c2184b6d5c Mon Sep 17 00:00:00 2001 From: Elay Aharoni Date: Thu, 13 Aug 2026 09:11:42 +0300 Subject: [PATCH 3/6] OSAC-3601: wire StorageBackendEditPage into storage routes Assisted-by: Claude Code Signed-off-by: Elay Aharoni --- .../src/shell/StorageRoutes.test.tsx | 25 +++++++++++++++---- apps/app-frontend/src/shell/StorageRoutes.tsx | 6 ++--- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/apps/app-frontend/src/shell/StorageRoutes.test.tsx b/apps/app-frontend/src/shell/StorageRoutes.test.tsx index 8aa2d014..aafa8710 100644 --- a/apps/app-frontend/src/shell/StorageRoutes.test.tsx +++ b/apps/app-frontend/src/shell/StorageRoutes.test.tsx @@ -1,7 +1,10 @@ import { Route, Routes } from 'react-router-dom'; +import { create } from '@bufbuild/protobuf'; import { screen, waitFor } from '@testing-library/react'; import { describe, expect, it, vi } from 'vitest'; +import { StorageBackendSchema } from '@osac/types/private'; +import type { MockApiFixtures } from '@osac/ui-components/test-utils/createMockConnectTransport'; import { renderWithProviders } from '@osac/ui-components/test-utils/TestProviders'; import { StorageRoutes } from './StorageRoutes'; @@ -17,12 +20,12 @@ vi.mock('react-router-dom', async (importOriginal) => { }; }); -const renderAt = (path: string) => +const renderAt = (path: string, apiFixtures?: MockApiFixtures) => renderWithProviders( } /> , - { routerEntries: [path] }, + { routerEntries: [path], apiFixtures }, ); describe('StorageRoutes', () => { @@ -39,10 +42,22 @@ describe('StorageRoutes', () => { expect(screen.getByRole('textbox', { name: 'Name' })).toBeInTheDocument(); }); - it('renders a placeholder for backends/:id/edit', () => { - renderAt('/admin/infrastructure/storage/backends/abc-123/edit'); + it('renders the real edit form for backends/:id/edit', async () => { + renderAt('/admin/infrastructure/storage/backends/abc-123/edit', { + storageBackends: [ + create(StorageBackendSchema, { + id: 'abc-123', + metadata: { name: 'vast-prod-1' }, + spec: { provider: 'vast', endpoint: 'vast.example.com:443', description: '' }, + }), + ], + }); - expect(screen.getByText('Edit storage backend')).toBeInTheDocument(); + await waitFor(() => { + expect(screen.getByRole('heading', { name: 'Edit storage backend' })).toBeInTheDocument(); + }); + expect(screen.getByRole('textbox', { name: 'Endpoint' })).toHaveValue('vast.example.com:443'); + expect(screen.queryByText('This feature is coming soon.')).not.toBeInTheDocument(); }); it('renders the Tiers tab at /admin/infrastructure/storage/tiers', async () => { diff --git a/apps/app-frontend/src/shell/StorageRoutes.tsx b/apps/app-frontend/src/shell/StorageRoutes.tsx index 72825216..806fe1a5 100644 --- a/apps/app-frontend/src/shell/StorageRoutes.tsx +++ b/apps/app-frontend/src/shell/StorageRoutes.tsx @@ -3,6 +3,7 @@ import { Navigate, Route, Routes } from 'react-router-dom'; import StorageTierCreatePage from '@osac/ui-components/components/Storage/StorageTierCreatePage'; import { useTranslation } from '@osac/ui-components/hooks/useTranslation'; import { StorageBackendCreatePage } from '@osac/ui-components/pages/admin/StorageBackendCreatePage'; +import { StorageBackendEditPage } from '@osac/ui-components/pages/admin/StorageBackendEditPage'; import { StorageManagementPage } from '@osac/ui-components/pages/admin/StorageManagementPage'; import { StoragePlaceholder } from '@osac/ui-components/pages/admin/StoragePlaceholder'; @@ -14,10 +15,7 @@ export const StorageRoutes = () => { } /> } /> } /> - } - /> + } /> } /> } /> Date: Thu, 13 Aug 2026 09:33:02 +0300 Subject: [PATCH 4/6] OSAC-3601: address validation review findings useUpdateStorageBackend sent lock: true but never included object.metadata, so the server's optimistic-lock check (which requires metadata on both the request and current object to compare versions) silently no-opped. Forward the fetched record's metadata.version alongside lock: true so a concurrent edit is actually rejected. Assisted-by: Claude Code Signed-off-by: Elay Aharoni --- .../api/v1/private/storage-backends.test.ts | 19 +++++++++++++++++-- .../src/api/v1/private/storage-backends.ts | 4 +++- .../admin/StorageBackendEditPage.test.tsx | 5 +++-- .../pages/admin/StorageBackendEditPage.tsx | 1 + 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/libs/ui-components/src/api/v1/private/storage-backends.test.ts b/libs/ui-components/src/api/v1/private/storage-backends.test.ts index f7c92c85..aa76ee7a 100644 --- a/libs/ui-components/src/api/v1/private/storage-backends.test.ts +++ b/libs/ui-components/src/api/v1/private/storage-backends.test.ts @@ -176,6 +176,7 @@ describe('useUpdateStorageBackend', () => { it('sends a single spec.endpoint mask entry and never masks metadata.name or spec.provider', async () => { const captured = await mutateAndCaptureUpdate({ id: 'b-1', + version: 3, spec: { endpoint: 'new.example.com' }, }); @@ -183,13 +184,14 @@ describe('useUpdateStorageBackend', () => { expect(paths).toEqual(['spec.endpoint']); expect(paths).not.toContain('metadata.name'); expect(paths).not.toContain('spec.provider'); - const object = captured?.object as { metadata?: unknown }; - expect(object.metadata).toBeUndefined(); + const object = captured?.object as { metadata?: { name?: string } }; + expect(object.metadata?.name).toBe(''); }); it('sends spec.endpoint and spec.description as separate mask entries when both change', async () => { const captured = await mutateAndCaptureUpdate({ id: 'b-1', + version: 3, spec: { endpoint: 'new.example.com', description: 'updated description' }, }); @@ -202,6 +204,7 @@ describe('useUpdateStorageBackend', () => { it('sends a single spec.credentials mask entry, never split into username/password leaves', async () => { const captured = await mutateAndCaptureUpdate({ id: 'b-1', + version: 3, spec: { credentials: { username: 'test-updated-admin', password: 'test-updated-secret' } }, }); @@ -218,11 +221,23 @@ describe('useUpdateStorageBackend', () => { it('sends lock: true for optimistic concurrency', async () => { const captured = await mutateAndCaptureUpdate({ id: 'b-1', + version: 3, spec: { endpoint: 'new.example.com' }, }); expect(captured?.lock).toBe(true); }); + + it('sends the current version in object.metadata so the server can enforce the lock', async () => { + const captured = await mutateAndCaptureUpdate({ + id: 'b-1', + version: 7, + spec: { endpoint: 'new.example.com' }, + }); + + const object = captured?.object as { metadata?: { version?: number } }; + expect(object.metadata?.version).toBe(7); + }); }); describe('useDeleteStorageBackend', () => { diff --git a/libs/ui-components/src/api/v1/private/storage-backends.ts b/libs/ui-components/src/api/v1/private/storage-backends.ts index 083f056c..9dc5744e 100644 --- a/libs/ui-components/src/api/v1/private/storage-backends.ts +++ b/libs/ui-components/src/api/v1/private/storage-backends.ts @@ -67,6 +67,8 @@ export const useCreateStorageBackend = () => { export type UpdateStorageBackendInput = { id: string; + /** The `metadata.version` of the record the caller last fetched — required so the server can enforce the lock below; a request with no metadata is exempt from the optimistic-lock check regardless of `lock: true`. */ + version: number; spec: MessageInitShape; }; @@ -81,7 +83,7 @@ export const useUpdateStorageBackend = () => { } const resp = await client.update({ - object: { id: input.id, spec }, + object: { id: input.id, metadata: { version: input.version }, spec }, updateMask: { paths: buildUpdateMaskPaths({ spec } as Record) }, lock: true, }); diff --git a/libs/ui-components/src/pages/admin/StorageBackendEditPage.test.tsx b/libs/ui-components/src/pages/admin/StorageBackendEditPage.test.tsx index 16aef380..255ddee3 100644 --- a/libs/ui-components/src/pages/admin/StorageBackendEditPage.test.tsx +++ b/libs/ui-components/src/pages/admin/StorageBackendEditPage.test.tsx @@ -35,7 +35,7 @@ const EDIT_PATH = '/admin/infrastructure/storage/backends/b-1/edit'; const existingBackend = create(StorageBackendSchema, { id: 'b-1', - metadata: { name: 'vast-prod-1' }, + metadata: { name: 'vast-prod-1', version: 7 }, spec: { provider: 'vast', endpoint: 'vast.example.com:443', @@ -158,11 +158,12 @@ describe('StorageBackendEditPage', () => { }); expect(capturedRequest?.object?.spec?.endpoint).toBe('new.example.com:443'); expect(capturedRequest?.object?.spec?.credentials).toBeUndefined(); - expect(capturedRequest?.object?.metadata).toBeUndefined(); + expect(capturedRequest?.object?.metadata?.name).toBe(''); expect(capturedRequest?.updateMask?.paths).not.toContain('spec.provider'); expect(capturedRequest?.updateMask?.paths).not.toContain('metadata.name'); expect(capturedRequest?.updateMask?.paths).not.toContain('spec.credentials'); expect(capturedRequest?.lock).toBe(true); + expect(capturedRequest?.object?.metadata?.version).toBe(7); }); it('submits a complete credentials object when both fields are filled', async () => { diff --git a/libs/ui-components/src/pages/admin/StorageBackendEditPage.tsx b/libs/ui-components/src/pages/admin/StorageBackendEditPage.tsx index 7bb2f61e..abbfcf31 100644 --- a/libs/ui-components/src/pages/admin/StorageBackendEditPage.tsx +++ b/libs/ui-components/src/pages/admin/StorageBackendEditPage.tsx @@ -106,6 +106,7 @@ const StorageBackendEditForm = ({ backend }: { backend: StorageBackend }) => { try { await mutateAsync({ id: backend.id, + version: backend.metadata?.version ?? 0, spec: { endpoint: values.endpoint, description: values.description, From 8e06045f03025a90d0c594ab72bf50c45206ebec Mon Sep 17 00:00:00 2001 From: Elay Aharoni Date: Thu, 13 Aug 2026 14:36:24 +0300 Subject: [PATCH 5/6] OSAC-3601: merge StorageBackendEditPage into StorageBackendCreatePage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follows the same convention already used by RoleBindingCreatePage: one component takes an optional fetched backend, branching on its presence for disabled fields, credential validation shape, which mutation hook to call, and copy — instead of two near-duplicate page components. Both routes now point at the same component. Assisted-by: Claude Code Signed-off-by: Elay Aharoni --- apps/app-frontend/src/shell/StorageRoutes.tsx | 3 +- libs/i18n/locales/en/translation.json | 1 - .../admin/StorageBackendCreatePage.test.tsx | 490 +++++++++++++----- .../pages/admin/StorageBackendCreatePage.tsx | 173 +++++-- .../admin/StorageBackendEditPage.test.tsx | 233 --------- .../pages/admin/StorageBackendEditPage.tsx | 231 --------- 6 files changed, 483 insertions(+), 648 deletions(-) delete mode 100644 libs/ui-components/src/pages/admin/StorageBackendEditPage.test.tsx delete mode 100644 libs/ui-components/src/pages/admin/StorageBackendEditPage.tsx diff --git a/apps/app-frontend/src/shell/StorageRoutes.tsx b/apps/app-frontend/src/shell/StorageRoutes.tsx index 806fe1a5..d28d84f6 100644 --- a/apps/app-frontend/src/shell/StorageRoutes.tsx +++ b/apps/app-frontend/src/shell/StorageRoutes.tsx @@ -3,7 +3,6 @@ import { Navigate, Route, Routes } from 'react-router-dom'; import StorageTierCreatePage from '@osac/ui-components/components/Storage/StorageTierCreatePage'; import { useTranslation } from '@osac/ui-components/hooks/useTranslation'; import { StorageBackendCreatePage } from '@osac/ui-components/pages/admin/StorageBackendCreatePage'; -import { StorageBackendEditPage } from '@osac/ui-components/pages/admin/StorageBackendEditPage'; import { StorageManagementPage } from '@osac/ui-components/pages/admin/StorageManagementPage'; import { StoragePlaceholder } from '@osac/ui-components/pages/admin/StoragePlaceholder'; @@ -15,7 +14,7 @@ export const StorageRoutes = () => { } /> } /> } /> - } /> + } /> } /> } /> { const testBackendPassword = 'test-password'; -const renderPage = (overrides?: MockTransportOverrides) => +const renderCreatePage = (overrides?: MockTransportOverrides) => renderWithProviders(, { transportOverrides: overrides, }); +const EDIT_ROUTE_PATH = '/admin/infrastructure/storage/backends/:id/edit'; +const EDIT_PATH = '/admin/infrastructure/storage/backends/b-1/edit'; + +const existingBackend = create(StorageBackendSchema, { + id: 'b-1', + metadata: { name: 'vast-prod-1', version: 7 }, + spec: { + provider: 'vast', + endpoint: 'vast.example.com:443', + description: 'primary array', + credentials: { username: 'existing-admin', password: 'existing-secret' }, + }, +}); + +const renderEditPage = (overrides?: MockTransportOverrides, apiFixtures?: MockApiFixtures) => + renderWithProviders( + + } /> + , + { + routerEntries: [EDIT_PATH], + apiFixtures: { storageBackends: [existingBackend], ...apiFixtures }, + transportOverrides: overrides, + }, + ); + describe('StorageBackendCreatePage', () => { beforeEach(() => { mockNavigate.mockReset(); }); - const fillValidForm = async (user: ReturnType['user']) => { - await user.type(screen.getByRole('textbox', { name: 'Name' }), 'vast-prod-1'); - await user.click(screen.getByLabelText(/^Provider/)); - await user.click(screen.getByRole('option', { name: 'VAST' })); - await user.type(screen.getByRole('textbox', { name: 'Endpoint' }), 'vast.example.com:443'); - await user.type(screen.getByLabelText(/^Username/), 'admin'); - await user.type(screen.getByLabelText(/^Password/), testBackendPassword); - }; + describe('create mode', () => { + const fillValidForm = async (user: ReturnType['user']) => { + await user.type(screen.getByRole('textbox', { name: 'Name' }), 'vast-prod-1'); + await user.click(screen.getByLabelText(/^Provider/)); + await user.click(screen.getByRole('option', { name: 'VAST' })); + await user.type(screen.getByRole('textbox', { name: 'Endpoint' }), 'vast.example.com:443'); + await user.type(screen.getByLabelText(/^Username/), 'admin'); + await user.type(screen.getByLabelText(/^Password/), testBackendPassword); + }; + + it('renders the page title, breadcrumb, and all fields', () => { + renderCreatePage(); + + expect(screen.getByRole('heading', { name: 'Create storage backend' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Storage backends' })).toBeInTheDocument(); + expect(screen.getByRole('textbox', { name: 'Name' })).toBeInTheDocument(); + expect(screen.getByLabelText(/^Provider/)).toBeInTheDocument(); + expect(screen.getByRole('textbox', { name: 'Endpoint' })).toBeInTheDocument(); + expect(screen.getByRole('textbox', { name: 'Description' })).toBeInTheDocument(); + expect(screen.getByLabelText(/^Username/)).toBeInTheDocument(); + expect(screen.getByLabelText(/^Password/)).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Create' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Cancel' })).toBeInTheDocument(); + }); - it('renders the page title, breadcrumb, and all fields', () => { - renderPage(); - - expect(screen.getByRole('heading', { name: 'Create storage backend' })).toBeInTheDocument(); - expect(screen.getByRole('button', { name: 'Storage backends' })).toBeInTheDocument(); - expect(screen.getByRole('textbox', { name: 'Name' })).toBeInTheDocument(); - expect(screen.getByLabelText(/^Provider/)).toBeInTheDocument(); - expect(screen.getByRole('textbox', { name: 'Endpoint' })).toBeInTheDocument(); - expect(screen.getByRole('textbox', { name: 'Description' })).toBeInTheDocument(); - expect(screen.getByLabelText(/^Username/)).toBeInTheDocument(); - expect(screen.getByLabelText(/^Password/)).toBeInTheDocument(); - expect(screen.getByRole('button', { name: 'Create' })).toBeInTheDocument(); - expect(screen.getByRole('button', { name: 'Cancel' })).toBeInTheDocument(); - }); + it('renders name and provider as enabled', () => { + renderCreatePage(); - it('renders the provider select with exactly vast, ceph, and pure options', async () => { - const { user } = renderPage(); + expect(screen.getByRole('textbox', { name: 'Name' })).toBeEnabled(); + expect(screen.getByLabelText(/^Provider/)).toBeEnabled(); + }); - await user.click(screen.getByLabelText(/^Provider/)); + it('renders the provider select with exactly vast, ceph, and pure options', async () => { + const { user } = renderCreatePage(); - const options = screen.getAllByRole('option'); - expect(options).toHaveLength(3); - expect(options.map((option) => option.textContent)).toEqual(['VAST', 'Ceph', 'Pure']); - }); + await user.click(screen.getByLabelText(/^Provider/)); - it('renders the password field as a masked input', () => { - renderPage(); + const options = screen.getAllByRole('option'); + expect(options).toHaveLength(3); + expect(options.map((option) => option.textContent)).toEqual(['VAST', 'Ceph', 'Pure']); + }); - const passwordField = screen.getByLabelText(/^Password/); - expect(passwordField).toHaveAttribute('type', 'password'); - }); + it('renders the password field as a masked input', () => { + renderCreatePage(); - it('shows a DNS-label validation error for an invalid name and does not submit', async () => { - const onStorageBackendCreate = vi.fn(); - const { user } = renderPage({ onStorageBackendCreate }); + const passwordField = screen.getByLabelText(/^Password/); + expect(passwordField).toHaveAttribute('type', 'password'); + }); - await user.type(screen.getByRole('textbox', { name: 'Name' }), 'Invalid_Name'); - await user.click(screen.getByRole('button', { name: 'Create' })); + it('shows a DNS-label validation error for an invalid name and does not submit', async () => { + const onStorageBackendCreate = vi.fn(); + const { user } = renderCreatePage({ onStorageBackendCreate }); - await waitFor(() => { - expect( - screen.getByText( - 'Name must only contain lowercase letters (a-z), digits (0-9), and hyphens (-)', - ), - ).toBeInTheDocument(); + await user.type(screen.getByRole('textbox', { name: 'Name' }), 'Invalid_Name'); + await user.click(screen.getByRole('button', { name: 'Create' })); + + await waitFor(() => { + expect( + screen.getByText( + 'Name must only contain lowercase letters (a-z), digits (0-9), and hyphens (-)', + ), + ).toBeInTheDocument(); + }); + expect(onStorageBackendCreate).not.toHaveBeenCalled(); + expect(mockNavigate).not.toHaveBeenCalled(); }); - expect(onStorageBackendCreate).not.toHaveBeenCalled(); - expect(mockNavigate).not.toHaveBeenCalled(); - }); - it('shows required-field validation errors for endpoint, username, and password', async () => { - const onStorageBackendCreate = vi.fn(); - const { user } = renderPage({ onStorageBackendCreate }); + it('shows required-field validation errors for endpoint, username, and password', async () => { + const onStorageBackendCreate = vi.fn(); + const { user } = renderCreatePage({ onStorageBackendCreate }); - await user.type(screen.getByRole('textbox', { name: 'Name' }), 'vast-prod-1'); - await user.click(screen.getByLabelText(/^Provider/)); - await user.click(screen.getByRole('option', { name: 'VAST' })); - await user.click(screen.getByRole('button', { name: 'Create' })); + await user.type(screen.getByRole('textbox', { name: 'Name' }), 'vast-prod-1'); + await user.click(screen.getByLabelText(/^Provider/)); + await user.click(screen.getByRole('option', { name: 'VAST' })); + await user.click(screen.getByRole('button', { name: 'Create' })); - await waitFor(() => { - expect(screen.getByText('Endpoint is required')).toBeInTheDocument(); + await waitFor(() => { + expect(screen.getByText('Endpoint is required')).toBeInTheDocument(); + }); + expect(screen.getByText('Username is required')).toBeInTheDocument(); + expect(screen.getByText('Password is required')).toBeInTheDocument(); + expect(onStorageBackendCreate).not.toHaveBeenCalled(); }); - expect(screen.getByText('Username is required')).toBeInTheDocument(); - expect(screen.getByText('Password is required')).toBeInTheDocument(); - expect(onStorageBackendCreate).not.toHaveBeenCalled(); - }); - it('shows a required error for provider when nothing is selected, not the removed oneOf message', async () => { - const onStorageBackendCreate = vi.fn(); - const { user } = renderPage({ onStorageBackendCreate }); + it('shows a required error for provider when nothing is selected, not the removed oneOf message', async () => { + const onStorageBackendCreate = vi.fn(); + const { user } = renderCreatePage({ onStorageBackendCreate }); - await user.type(screen.getByRole('textbox', { name: 'Name' }), 'vast-prod-1'); - await user.type(screen.getByRole('textbox', { name: 'Endpoint' }), 'vast.example.com:443'); - await user.type(screen.getByLabelText(/^Username/), 'admin'); - await user.type(screen.getByLabelText(/^Password/), testBackendPassword); - await user.click(screen.getByRole('button', { name: 'Create' })); + await user.type(screen.getByRole('textbox', { name: 'Name' }), 'vast-prod-1'); + await user.type(screen.getByRole('textbox', { name: 'Endpoint' }), 'vast.example.com:443'); + await user.type(screen.getByLabelText(/^Username/), 'admin'); + await user.type(screen.getByLabelText(/^Password/), testBackendPassword); + await user.click(screen.getByRole('button', { name: 'Create' })); - await waitFor(() => { - expect(screen.getByText('Provider is required')).toBeInTheDocument(); + await waitFor(() => { + expect(screen.getByText('Provider is required')).toBeInTheDocument(); + }); + expect( + screen.queryByText('Provider must be one of vast, ceph, or pure'), + ).not.toBeInTheDocument(); + expect(onStorageBackendCreate).not.toHaveBeenCalled(); }); - expect( - screen.queryByText('Provider must be one of vast, ceph, or pure'), - ).not.toBeInTheDocument(); - expect(onStorageBackendCreate).not.toHaveBeenCalled(); - }); - it('disables Create while the submission is pending, to prevent duplicate submissions', async () => { - let resolveCreate: (() => void) | undefined; - const onStorageBackendCreate = () => - new Promise((resolve) => { - resolveCreate = () => - resolve(create(StorageBackendsCreateResponseSchema, { object: { id: 'new-backend-1' } })); + it('disables Create while the submission is pending, to prevent duplicate submissions', async () => { + let resolveCreate: (() => void) | undefined; + const onStorageBackendCreate = () => + new Promise((resolve) => { + resolveCreate = () => + resolve( + create(StorageBackendsCreateResponseSchema, { object: { id: 'new-backend-1' } }), + ); + }); + + const { user } = renderCreatePage({ onStorageBackendCreate }); + + await fillValidForm(user); + await user.click(screen.getByRole('button', { name: 'Create' })); + + // Once isLoading is true, PatternFly's Spinner contributes its own + // "Contents" accessible name to the button, so an exact "Create" match + // no longer resolves — match by substring instead (same pattern already + // used elsewhere in this file for accessible-name additions). + await waitFor(() => { + expect(screen.getByRole('button', { name: /Create/ })).toBeDisabled(); }); + expect(screen.getByRole('button', { name: 'Cancel' })).toBeDisabled(); - const { user } = renderPage({ onStorageBackendCreate }); + resolveCreate?.(); - await fillValidForm(user); - await user.click(screen.getByRole('button', { name: 'Create' })); + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/backends'); + }); + }, 15000); + + it('submits the expected payload and navigates to the backends list on success', async () => { + let capturedRequest: StorageBackendsCreateRequest | undefined; + const { user } = renderCreatePage({ + onStorageBackendCreate: (req) => { + capturedRequest = req; + return create(StorageBackendsCreateResponseSchema, { + object: { + id: 'new-backend-1', + metadata: req.object?.metadata, + spec: req.object?.spec, + }, + }); + }, + }); + + await fillValidForm(user); + await user.click(screen.getByRole('button', { name: 'Create' })); + + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/backends'); + }); + + expect(capturedRequest?.object?.metadata?.name).toBe('vast-prod-1'); + expect(capturedRequest?.object?.spec?.provider).toBe('vast'); + expect(capturedRequest?.object?.spec?.endpoint).toBe('vast.example.com:443'); + expect(capturedRequest?.object?.spec?.credentials?.username).toBe('admin'); + expect(capturedRequest?.object?.spec?.credentials?.password).toBe(testBackendPassword); + }, 15000); + + it('shows a form-level error and does not navigate when the name already exists', async () => { + const { user } = renderCreatePage({ + onStorageBackendCreate: () => { + throw new ConnectError('Storage backend name already exists', Code.AlreadyExists); + }, + }); - // Once isLoading is true, PatternFly's Spinner contributes its own - // "Contents" accessible name to the button, so an exact "Create" match - // no longer resolves — match by substring instead (same pattern already - // used elsewhere in this file for accessible-name additions). - await waitFor(() => { - expect(screen.getByRole('button', { name: /Create/ })).toBeDisabled(); + await fillValidForm(user); + await user.click(screen.getByRole('button', { name: 'Create' })); + + await waitFor(() => { + expect(screen.getByText('Failed to create storage backend')).toBeInTheDocument(); + }); + expect(screen.getByText('Storage backend name already exists')).toBeInTheDocument(); + expect(mockNavigate).not.toHaveBeenCalled(); + }, 15000); + + it('navigates back to the backends list on cancel', async () => { + const { user } = renderCreatePage(); + + await user.click(screen.getByRole('button', { name: 'Cancel' })); + + expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/backends'); }); - expect(screen.getByRole('button', { name: 'Cancel' })).toBeDisabled(); - resolveCreate?.(); + it('navigates back to the backends list via breadcrumb', async () => { + const { user } = renderCreatePage(); + + await user.click(screen.getByRole('button', { name: 'Storage backends' })); - await waitFor(() => { expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/backends'); }); - }, 15000); + }); - it('submits the expected payload and navigates to the backends list on success', async () => { - let capturedRequest: StorageBackendsCreateRequest | undefined; - const { user } = renderPage({ - onStorageBackendCreate: (req) => { - capturedRequest = req; - return create(StorageBackendsCreateResponseSchema, { - object: { id: 'new-backend-1', metadata: req.object?.metadata, spec: req.object?.spec }, - }); - }, + describe('edit mode', () => { + it('shows a loading spinner before the backend has been fetched', () => { + renderEditPage(); + + expect(screen.getByRole('progressbar')).toBeInTheDocument(); }); - await fillValidForm(user); - await user.click(screen.getByRole('button', { name: 'Create' })); + it('renders the page prefilled with the backend endpoint and description', async () => { + renderEditPage(); - await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/backends'); + await waitFor(() => { + expect(screen.getByRole('heading', { name: 'Edit storage backend' })).toBeInTheDocument(); + }); + expect(screen.getByRole('textbox', { name: 'Endpoint' })).toHaveValue('vast.example.com:443'); + expect(screen.getByRole('textbox', { name: 'Description' })).toHaveValue('primary array'); + expect(screen.getByRole('textbox', { name: 'Name' })).toHaveValue('vast-prod-1'); + expect(screen.getByLabelText(/^Provider/)).toHaveTextContent('VAST'); }); - expect(capturedRequest?.object?.metadata?.name).toBe('vast-prod-1'); - expect(capturedRequest?.object?.spec?.provider).toBe('vast'); - expect(capturedRequest?.object?.spec?.endpoint).toBe('vast.example.com:443'); - expect(capturedRequest?.object?.spec?.credentials?.username).toBe('admin'); - expect(capturedRequest?.object?.spec?.credentials?.password).toBe(testBackendPassword); - }, 15000); + it('renders name and provider as disabled', async () => { + renderEditPage(); - it('shows a form-level error and does not navigate when the name already exists', async () => { - const { user } = renderPage({ - onStorageBackendCreate: () => { - throw new ConnectError('Storage backend name already exists', Code.AlreadyExists); - }, + await waitFor(() => { + expect(screen.getByRole('textbox', { name: 'Name' })).toBeInTheDocument(); + }); + expect(screen.getByRole('textbox', { name: 'Name' })).toBeDisabled(); + expect(screen.getByLabelText(/^Provider/)).toBeDisabled(); }); - await fillValidForm(user); - await user.click(screen.getByRole('button', { name: 'Create' })); + it('renders credential fields blank regardless of the fetched record', async () => { + renderEditPage(); - await waitFor(() => { - expect(screen.getByText('Failed to create storage backend')).toBeInTheDocument(); + await waitFor(() => { + expect(screen.getByLabelText(/^Username/)).toBeInTheDocument(); + }); + expect(screen.getByLabelText(/^Username/)).toHaveValue(''); + expect(screen.getByLabelText(/^Password/)).toHaveValue(''); }); - expect(screen.getByText('Storage backend name already exists')).toBeInTheDocument(); - expect(mockNavigate).not.toHaveBeenCalled(); - }, 15000); - it('navigates back to the backends list on cancel', async () => { - const { user } = renderPage(); + it('shows a validation error and does not submit when only username is filled', async () => { + const onStorageBackendUpdate = vi.fn(); + const { user } = renderEditPage({ onStorageBackendUpdate }); - await user.click(screen.getByRole('button', { name: 'Cancel' })); + await waitFor(() => { + expect(screen.getByLabelText(/^Username/)).toBeInTheDocument(); + }); + await user.type(screen.getByLabelText(/^Username/), 'new-admin'); + await user.click(screen.getByRole('button', { name: 'Save' })); - expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/backends'); - }); + await waitFor(() => { + expect( + screen.getAllByText('Enter both username and password, or leave both blank').length, + ).toBeGreaterThan(0); + }); + expect(onStorageBackendUpdate).not.toHaveBeenCalled(); + expect(mockNavigate).not.toHaveBeenCalled(); + }); - it('navigates back to the backends list via breadcrumb', async () => { - const { user } = renderPage(); + it('shows a validation error and does not submit when only password is filled', async () => { + const onStorageBackendUpdate = vi.fn(); + const { user } = renderEditPage({ onStorageBackendUpdate }); - await user.click(screen.getByRole('button', { name: 'Storage backends' })); + await waitFor(() => { + expect(screen.getByLabelText(/^Password/)).toBeInTheDocument(); + }); + await user.type(screen.getByLabelText(/^Password/), 'new-secret'); + await user.click(screen.getByRole('button', { name: 'Save' })); + + await waitFor(() => { + expect( + screen.getAllByText('Enter both username and password, or leave both blank').length, + ).toBeGreaterThan(0); + }); + expect(onStorageBackendUpdate).not.toHaveBeenCalled(); + expect(mockNavigate).not.toHaveBeenCalled(); + }); + + it('omits credentials and never submits name/provider when both credential fields are left blank', async () => { + let capturedRequest: StorageBackendsUpdateRequest | undefined; + const { user } = renderEditPage({ + onStorageBackendUpdate: (req) => { + capturedRequest = req; + return create(StorageBackendsUpdateResponseSchema, { object: existingBackend }); + }, + }); + + await waitFor(() => { + expect(screen.getByRole('textbox', { name: 'Endpoint' })).toHaveValue( + 'vast.example.com:443', + ); + }); + await user.clear(screen.getByRole('textbox', { name: 'Endpoint' })); + await user.type(screen.getByRole('textbox', { name: 'Endpoint' }), 'new.example.com:443'); + await user.click(screen.getByRole('button', { name: 'Save' })); + + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/backends'); + }); + expect(capturedRequest?.object?.spec?.endpoint).toBe('new.example.com:443'); + expect(capturedRequest?.object?.spec?.credentials).toBeUndefined(); + expect(capturedRequest?.object?.metadata?.name).toBe(''); + expect(capturedRequest?.updateMask?.paths).not.toContain('spec.provider'); + expect(capturedRequest?.updateMask?.paths).not.toContain('metadata.name'); + expect(capturedRequest?.updateMask?.paths).not.toContain('spec.credentials'); + expect(capturedRequest?.lock).toBe(true); + expect(capturedRequest?.object?.metadata?.version).toBe(7); + }); + + it('submits a complete credentials object when both fields are filled', async () => { + let capturedRequest: StorageBackendsUpdateRequest | undefined; + const { user } = renderEditPage({ + onStorageBackendUpdate: (req) => { + capturedRequest = req; + return create(StorageBackendsUpdateResponseSchema, { object: existingBackend }); + }, + }); - expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/backends'); + await waitFor(() => { + expect(screen.getByLabelText(/^Username/)).toBeInTheDocument(); + }); + await user.type(screen.getByLabelText(/^Username/), 'new-admin'); + await user.type(screen.getByLabelText(/^Password/), 'new-secret'); + await user.click(screen.getByRole('button', { name: 'Save' })); + + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/backends'); + }); + expect(capturedRequest?.object?.spec?.credentials).toMatchObject({ + username: 'new-admin', + password: 'new-secret', + }); + }); + + it('shows a submission error and does not navigate on a stale-version conflict', async () => { + const { user } = renderEditPage({ + onStorageBackendUpdate: () => { + throw new ConnectError('Storage backend was modified by another request', Code.Aborted); + }, + }); + + await waitFor(() => { + expect(screen.getByRole('textbox', { name: 'Endpoint' })).toHaveValue( + 'vast.example.com:443', + ); + }); + await user.click(screen.getByRole('button', { name: 'Save' })); + + await waitFor(() => { + expect(screen.getByText('Failed to update storage backend')).toBeInTheDocument(); + }); + expect( + screen.getByText('Storage backend was modified by another request'), + ).toBeInTheDocument(); + expect(mockNavigate).not.toHaveBeenCalled(); + }); + + it('navigates back to the backends list on cancel', async () => { + const { user } = renderEditPage(); + + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Cancel' })).toBeInTheDocument(); + }); + await user.click(screen.getByRole('button', { name: 'Cancel' })); + + expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/backends'); + }); }); }); diff --git a/libs/ui-components/src/pages/admin/StorageBackendCreatePage.tsx b/libs/ui-components/src/pages/admin/StorageBackendCreatePage.tsx index 27f1aaf1..52a90dc6 100644 --- a/libs/ui-components/src/pages/admin/StorageBackendCreatePage.tsx +++ b/libs/ui-components/src/pages/admin/StorageBackendCreatePage.tsx @@ -1,4 +1,4 @@ -import { useNavigate } from 'react-router-dom'; +import { useNavigate, useParams } from 'react-router-dom'; import { ActionList, ActionListGroup, @@ -6,8 +6,10 @@ import { Alert, Breadcrumb, BreadcrumbItem, + Bullseye, Button, PageSection, + Spinner, Stack, StackItem, Title, @@ -16,7 +18,12 @@ import { Formik } from 'formik'; import type { TFunction } from 'i18next'; import * as Yup from 'yup'; -import { useCreateStorageBackend } from '@osac/ui-components/api/v1/private/storage-backends'; +import type { StorageBackend } from '@osac/types/private'; +import { + useCreateStorageBackend, + usePrivateStorageBackend, + useUpdateStorageBackend, +} from '@osac/ui-components/api/v1/private/storage-backends'; import NameField from '@osac/ui-components/components/catalogProvision/wizard/fields/NameField'; import { InputField } from '@osac/ui-components/components/Form/InputField'; import LeaveFormConfirmation from '@osac/ui-components/components/Form/LeaveFormConfirmation'; @@ -39,30 +46,49 @@ interface StorageBackendFormValues { credentials: { username: string; password: string }; } -const initialValues: StorageBackendFormValues = { - metadata: { name: '' }, - provider: '', - endpoint: '', - description: '', +const getInitialValues = (backend?: StorageBackend): StorageBackendFormValues => ({ + metadata: { name: backend?.metadata?.name ?? '' }, + provider: backend?.spec?.provider ?? '', + endpoint: backend?.spec?.endpoint ?? '', + description: backend?.spec?.description ?? '', credentials: { username: '', password: '' }, -}; +}); -const getStorageBackendSchema = (t: TFunction) => - Yup.object({ +const getStorageBackendSchema = (t: TFunction, isEdit: boolean) => { + const pairError = t('Enter both username and password, or leave both blank'); + return Yup.object({ metadata: Yup.object({ name: resourceNameSchema(t) }), provider: Yup.string().required(t('Provider is required')), endpoint: Yup.string().required(t('Endpoint is required')), description: Yup.string(), - credentials: Yup.object({ - username: Yup.string().required(t('Username is required')), - password: Yup.string().required(t('Password is required')), - }), + // On edit, credentials start blank and are all-or-nothing: both blank keeps them + // unchanged, both filled replaces them, exactly one filled is invalid (there's no + // server-side way to update just one). On create they're always required. + credentials: isEdit + ? Yup.object({ + username: Yup.string().test('credentials-pair', pairError, function (value) { + const parent = this.parent as { username?: string; password?: string } | undefined; + return !!value === !!parent?.password; + }), + password: Yup.string().test('credentials-pair', pairError, function (value) { + const parent = this.parent as { username?: string; password?: string } | undefined; + return !!value === !!parent?.username; + }), + }) + : Yup.object({ + username: Yup.string().required(t('Username is required')), + password: Yup.string().required(t('Password is required')), + }), }); +}; -export const StorageBackendCreatePage = () => { +const StorageBackendForm = ({ backend }: { backend?: StorageBackend }) => { const { t } = useTranslation(); const navigate = useNavigate(); - const { mutateAsync, error } = useCreateStorageBackend(); + const isEdit = !!backend; + const { mutateAsync: create, error: createError } = useCreateStorageBackend(); + const { mutateAsync: update, error: updateError } = useUpdateStorageBackend(); + const error = isEdit ? updateError : createError; const providerOptions: SelectFieldOption[] = [ { value: 'vast', label: t('VAST') }, @@ -70,6 +96,45 @@ export const StorageBackendCreatePage = () => { { value: 'pure', label: t('Pure') }, ]; + const onSubmit = async (values: StorageBackendFormValues) => { + try { + if (backend) { + await update({ + id: backend.id, + version: backend.metadata?.version ?? 0, + spec: { + endpoint: values.endpoint, + description: values.description, + ...(values.credentials.username && values.credentials.password + ? { + credentials: { + username: values.credentials.username, + password: values.credentials.password, + }, + } + : {}), + }, + }); + } else { + await create({ + metadata: values.metadata, + spec: { + provider: values.provider, + endpoint: values.endpoint, + description: values.description, + credentials: { + username: values.credentials.username, + password: values.credentials.password, + }, + }, + }); + } + navigate(BACKENDS_LIST_PATH); + } catch { + // Surfaced via the mutation's own `error` state below; nothing further to do here. + } + }; + return ( <> @@ -80,48 +145,31 @@ export const StorageBackendCreatePage = () => { {t('Storage backends')} - {t('Create')} + {isEdit ? t('Edit') : t('Create')} - {t('Create storage backend')} + {isEdit ? t('Edit storage backend') : t('Create storage backend')} { - try { - await mutateAsync({ - metadata: values.metadata, - spec: { - provider: values.provider, - endpoint: values.endpoint, - description: values.description, - credentials: { - username: values.credentials.username, - password: values.credentials.password, - }, - }, - }); - navigate(BACKENDS_LIST_PATH); - } catch { - // Surfaced via the mutation's own `error` state below; nothing further to do here. - } - }} + initialValues={getInitialValues(backend)} + validationSchema={getStorageBackendSchema(t, isEdit)} + onSubmit={onSubmit} > {({ submitForm, isSubmitting }) => ( - + { name="credentials.username" label={t('Username')} fieldId="storage-backend-username" - isRequired + isRequired={!isEdit} + helperText={ + isEdit ? t('Leave blank to keep the current credentials.') : undefined + } /> {!!error && ( - + {getErrorMessage(error)} @@ -168,7 +227,7 @@ export const StorageBackendCreatePage = () => { isDisabled={isSubmitting} isLoading={isSubmitting} > - {t('Create')} + {isEdit ? t('Save') : t('Create')} @@ -190,3 +249,29 @@ export const StorageBackendCreatePage = () => { ); }; + +export const StorageBackendCreatePage = () => { + const { t } = useTranslation(); + const { id } = useParams<{ id: string }>(); + const { data, isLoading, error } = usePrivateStorageBackend(id ?? ''); + + if (id) { + if (isLoading) { + return ( + + + + ); + } + + if (error) { + return ( + + {getErrorMessage(error)} + + ); + } + } + + return ; +}; diff --git a/libs/ui-components/src/pages/admin/StorageBackendEditPage.test.tsx b/libs/ui-components/src/pages/admin/StorageBackendEditPage.test.tsx deleted file mode 100644 index 255ddee3..00000000 --- a/libs/ui-components/src/pages/admin/StorageBackendEditPage.test.tsx +++ /dev/null @@ -1,233 +0,0 @@ -import { Route, Routes } from 'react-router-dom'; -import { create } from '@bufbuild/protobuf'; -import { Code, ConnectError } from '@connectrpc/connect'; -import { screen, waitFor } from '@testing-library/react'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; - -import { - StorageBackendSchema, - type StorageBackendsUpdateRequest, - StorageBackendsUpdateResponseSchema, -} from '@osac/types/private'; - -import { StorageBackendEditPage } from './StorageBackendEditPage'; -import type { - MockApiFixtures, - MockTransportOverrides, -} from '../../test-utils/createMockConnectTransport'; -import { renderWithProviders } from '../../test-utils/TestProviders'; - -const mockNavigate = vi.fn(); -vi.mock('react-router-dom', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - useNavigate: () => mockNavigate, - // useBlocker requires a data router; this test harness renders under a plain - // MemoryRouter, so LeaveFormConfirmation's blocking behavior is stubbed out - // rather than exercised here (mirrors StorageBackendCreatePage.test.tsx). - useBlocker: () => ({ state: 'unblocked' as const }), - }; -}); - -const EDIT_ROUTE_PATH = '/admin/infrastructure/storage/backends/:id/edit'; -const EDIT_PATH = '/admin/infrastructure/storage/backends/b-1/edit'; - -const existingBackend = create(StorageBackendSchema, { - id: 'b-1', - metadata: { name: 'vast-prod-1', version: 7 }, - spec: { - provider: 'vast', - endpoint: 'vast.example.com:443', - description: 'primary array', - credentials: { username: 'existing-admin', password: 'existing-secret' }, - }, -}); - -const renderPage = (overrides?: MockTransportOverrides, apiFixtures?: MockApiFixtures) => - renderWithProviders( - - } /> - , - { - routerEntries: [EDIT_PATH], - apiFixtures: { storageBackends: [existingBackend], ...apiFixtures }, - transportOverrides: overrides, - }, - ); - -describe('StorageBackendEditPage', () => { - beforeEach(() => { - mockNavigate.mockReset(); - }); - - it('shows a loading spinner before the backend has been fetched', () => { - renderPage(); - - expect(screen.getByRole('progressbar')).toBeInTheDocument(); - }); - - it('renders the page prefilled with the backend endpoint and description', async () => { - renderPage(); - - await waitFor(() => { - expect(screen.getByRole('heading', { name: 'Edit storage backend' })).toBeInTheDocument(); - }); - expect(screen.getByRole('textbox', { name: 'Endpoint' })).toHaveValue('vast.example.com:443'); - expect(screen.getByRole('textbox', { name: 'Description' })).toHaveValue('primary array'); - expect(screen.getByRole('textbox', { name: 'Name' })).toHaveValue('vast-prod-1'); - expect(screen.getByLabelText(/^Provider/)).toHaveTextContent('VAST'); - }); - - it('renders name and provider as disabled', async () => { - renderPage(); - - await waitFor(() => { - expect(screen.getByRole('textbox', { name: 'Name' })).toBeInTheDocument(); - }); - expect(screen.getByRole('textbox', { name: 'Name' })).toBeDisabled(); - expect(screen.getByLabelText(/^Provider/)).toBeDisabled(); - }); - - it('renders credential fields blank regardless of the fetched record', async () => { - renderPage(); - - await waitFor(() => { - expect(screen.getByLabelText(/^Username/)).toBeInTheDocument(); - }); - expect(screen.getByLabelText(/^Username/)).toHaveValue(''); - expect(screen.getByLabelText(/^Password/)).toHaveValue(''); - }); - - it('shows a validation error and does not submit when only username is filled', async () => { - const onStorageBackendUpdate = vi.fn(); - const { user } = renderPage({ onStorageBackendUpdate }); - - await waitFor(() => { - expect(screen.getByLabelText(/^Username/)).toBeInTheDocument(); - }); - await user.type(screen.getByLabelText(/^Username/), 'new-admin'); - await user.click(screen.getByRole('button', { name: 'Save' })); - - await waitFor(() => { - expect( - screen.getAllByText('Enter both username and password, or leave both blank').length, - ).toBeGreaterThan(0); - }); - expect(onStorageBackendUpdate).not.toHaveBeenCalled(); - expect(mockNavigate).not.toHaveBeenCalled(); - }); - - it('shows a validation error and does not submit when only password is filled', async () => { - const onStorageBackendUpdate = vi.fn(); - const { user } = renderPage({ onStorageBackendUpdate }); - - await waitFor(() => { - expect(screen.getByLabelText(/^Password/)).toBeInTheDocument(); - }); - await user.type(screen.getByLabelText(/^Password/), 'new-secret'); - await user.click(screen.getByRole('button', { name: 'Save' })); - - await waitFor(() => { - expect( - screen.getAllByText('Enter both username and password, or leave both blank').length, - ).toBeGreaterThan(0); - }); - expect(onStorageBackendUpdate).not.toHaveBeenCalled(); - expect(mockNavigate).not.toHaveBeenCalled(); - }); - - it('omits credentials and never submits name/provider when both credential fields are left blank', async () => { - let capturedRequest: StorageBackendsUpdateRequest | undefined; - const { user } = renderPage({ - onStorageBackendUpdate: (req) => { - capturedRequest = req; - return create(StorageBackendsUpdateResponseSchema, { object: existingBackend }); - }, - }); - - await waitFor(() => { - expect(screen.getByRole('textbox', { name: 'Endpoint' })).toHaveValue('vast.example.com:443'); - }); - await user.clear(screen.getByRole('textbox', { name: 'Endpoint' })); - await user.type(screen.getByRole('textbox', { name: 'Endpoint' }), 'new.example.com:443'); - await user.click(screen.getByRole('button', { name: 'Save' })); - - await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/backends'); - }); - expect(capturedRequest?.object?.spec?.endpoint).toBe('new.example.com:443'); - expect(capturedRequest?.object?.spec?.credentials).toBeUndefined(); - expect(capturedRequest?.object?.metadata?.name).toBe(''); - expect(capturedRequest?.updateMask?.paths).not.toContain('spec.provider'); - expect(capturedRequest?.updateMask?.paths).not.toContain('metadata.name'); - expect(capturedRequest?.updateMask?.paths).not.toContain('spec.credentials'); - expect(capturedRequest?.lock).toBe(true); - expect(capturedRequest?.object?.metadata?.version).toBe(7); - }); - - it('submits a complete credentials object when both fields are filled', async () => { - let capturedRequest: StorageBackendsUpdateRequest | undefined; - const { user } = renderPage({ - onStorageBackendUpdate: (req) => { - capturedRequest = req; - return create(StorageBackendsUpdateResponseSchema, { object: existingBackend }); - }, - }); - - await waitFor(() => { - expect(screen.getByLabelText(/^Username/)).toBeInTheDocument(); - }); - await user.type(screen.getByLabelText(/^Username/), 'new-admin'); - await user.type(screen.getByLabelText(/^Password/), 'new-secret'); - await user.click(screen.getByRole('button', { name: 'Save' })); - - await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/backends'); - }); - expect(capturedRequest?.object?.spec?.credentials).toMatchObject({ - username: 'new-admin', - password: 'new-secret', - }); - }); - - it('shows a submission error and does not navigate on a stale-version conflict', async () => { - const { user } = renderPage({ - onStorageBackendUpdate: () => { - throw new ConnectError('Storage backend was modified by another request', Code.Aborted); - }, - }); - - await waitFor(() => { - expect(screen.getByRole('textbox', { name: 'Endpoint' })).toHaveValue('vast.example.com:443'); - }); - await user.click(screen.getByRole('button', { name: 'Save' })); - - await waitFor(() => { - expect(screen.getByText('Failed to update storage backend')).toBeInTheDocument(); - }); - expect(screen.getByText('Storage backend was modified by another request')).toBeInTheDocument(); - expect(mockNavigate).not.toHaveBeenCalled(); - }); - - it('shows a not-found error and no form when the backend does not exist', async () => { - renderPage(undefined, { storageBackends: [] }); - - await waitFor(() => { - expect(screen.getByText('Failed to fetch storage backend')).toBeInTheDocument(); - }); - expect(screen.getByText('Storage backend not found')).toBeInTheDocument(); - expect(screen.queryByRole('textbox', { name: 'Endpoint' })).not.toBeInTheDocument(); - }); - - it('navigates back to the backends list on cancel', async () => { - const { user } = renderPage(); - - await waitFor(() => { - expect(screen.getByRole('button', { name: 'Cancel' })).toBeInTheDocument(); - }); - await user.click(screen.getByRole('button', { name: 'Cancel' })); - - expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/backends'); - }); -}); diff --git a/libs/ui-components/src/pages/admin/StorageBackendEditPage.tsx b/libs/ui-components/src/pages/admin/StorageBackendEditPage.tsx deleted file mode 100644 index abbfcf31..00000000 --- a/libs/ui-components/src/pages/admin/StorageBackendEditPage.tsx +++ /dev/null @@ -1,231 +0,0 @@ -import { useNavigate, useParams } from 'react-router-dom'; -import { - ActionList, - ActionListGroup, - ActionListItem, - Alert, - Breadcrumb, - BreadcrumbItem, - Bullseye, - Button, - PageSection, - Spinner, - Stack, - StackItem, - Title, -} from '@patternfly/react-core'; -import { Formik } from 'formik'; -import type { TFunction } from 'i18next'; -import * as Yup from 'yup'; - -import type { StorageBackend } from '@osac/types/private'; -import { - usePrivateStorageBackend, - useUpdateStorageBackend, -} from '@osac/ui-components/api/v1/private/storage-backends'; -import NameField from '@osac/ui-components/components/catalogProvision/wizard/fields/NameField'; -import { InputField } from '@osac/ui-components/components/Form/InputField'; -import LeaveFormConfirmation from '@osac/ui-components/components/Form/LeaveFormConfirmation'; -import OsacForm from '@osac/ui-components/components/Form/OsacForm'; -import { - SelectField, - type SelectFieldOption, -} from '@osac/ui-components/components/Form/SelectField'; -import { useTranslation } from '@osac/ui-components/hooks/useTranslation'; -import { getErrorMessage } from '@osac/ui-components/utils/error'; - -const BACKENDS_LIST_PATH = '/admin/infrastructure/storage/backends'; - -interface StorageBackendEditFormValues { - metadata: { name: string }; - provider: string; - endpoint: string; - description: string; - credentials: { username: string; password: string }; -} - -const getStorageBackendEditSchema = (t: TFunction) => { - const pairError = t('Enter both username and password, or leave both blank'); - return Yup.object({ - endpoint: Yup.string().required(t('Endpoint is required')), - description: Yup.string(), - credentials: Yup.object({ - username: Yup.string().test('credentials-pair', pairError, function (value) { - const parent = this.parent as { username?: string; password?: string } | undefined; - return !!value === !!parent?.password; - }), - password: Yup.string().test('credentials-pair', pairError, function (value) { - const parent = this.parent as { username?: string; password?: string } | undefined; - return !!value === !!parent?.username; - }), - }), - }); -}; - -const StorageBackendEditForm = ({ backend }: { backend: StorageBackend }) => { - const { t } = useTranslation(); - const navigate = useNavigate(); - const { mutateAsync, error } = useUpdateStorageBackend(); - - const providerOptions: SelectFieldOption[] = [ - { value: 'vast', label: t('VAST') }, - { value: 'ceph', label: t('Ceph') }, - { value: 'pure', label: t('Pure') }, - ]; - - const initialValues: StorageBackendEditFormValues = { - metadata: { name: backend.metadata?.name ?? '' }, - provider: backend.spec?.provider ?? '', - endpoint: backend.spec?.endpoint ?? '', - description: backend.spec?.description ?? '', - credentials: { username: '', password: '' }, - }; - - return ( - <> - - - - - - - {t('Edit')} - - - {t('Edit storage backend')} - - - - - { - try { - await mutateAsync({ - id: backend.id, - version: backend.metadata?.version ?? 0, - spec: { - endpoint: values.endpoint, - description: values.description, - ...(values.credentials.username && values.credentials.password - ? { - credentials: { - username: values.credentials.username, - password: values.credentials.password, - }, - } - : {}), - }, - }); - navigate(BACKENDS_LIST_PATH); - } catch { - // Surfaced via the mutation's own `error` state below; nothing further to do here. - } - }} - > - {({ submitForm, isSubmitting }) => ( - - - - - - - - - - - - - - {!!error && ( - - - {getErrorMessage(error)} - - - )} - - - - - - - - - - - - - - )} - - - - ); -}; - -export const StorageBackendEditPage = () => { - const { t } = useTranslation(); - const { id } = useParams<{ id: string }>(); - const { data, isLoading, error } = usePrivateStorageBackend(id ?? ''); - - if (isLoading) { - return ( - - - - ); - } - - if (error || !data) { - return ( - - {error ? getErrorMessage(error) : t('Storage backend not found')} - - ); - } - - return ; -}; From d4180d1ae74c43883098138e7457477ffb6ce405 Mon Sep 17 00:00:00 2001 From: Elay Aharoni Date: Thu, 13 Aug 2026 14:56:06 +0300 Subject: [PATCH 6/6] OSAC-3601: prefix test credential literals with test- for secret scanner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit's No-Hardcoded-Secrets check flagged plain 'existing-secret'/ 'new-admin'-style literals in StorageBackendCreatePage.test.tsx since they weren't obviously placeholder values. Same fix already applied to storage-backends.test.ts in OSAC-3597 (a506d1a) — prefix every test credential literal with test- so the scanner's placeholder heuristic recognizes them. Assisted-by: Claude Code Signed-off-by: Elay Aharoni --- .../admin/StorageBackendCreatePage.test.tsx | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/libs/ui-components/src/pages/admin/StorageBackendCreatePage.test.tsx b/libs/ui-components/src/pages/admin/StorageBackendCreatePage.test.tsx index 75e5e501..498b0abe 100644 --- a/libs/ui-components/src/pages/admin/StorageBackendCreatePage.test.tsx +++ b/libs/ui-components/src/pages/admin/StorageBackendCreatePage.test.tsx @@ -51,7 +51,7 @@ const existingBackend = create(StorageBackendSchema, { provider: 'vast', endpoint: 'vast.example.com:443', description: 'primary array', - credentials: { username: 'existing-admin', password: 'existing-secret' }, + credentials: { username: 'test-existing-admin', password: 'test-existing-secret' }, }, }); @@ -78,7 +78,7 @@ describe('StorageBackendCreatePage', () => { await user.click(screen.getByLabelText(/^Provider/)); await user.click(screen.getByRole('option', { name: 'VAST' })); await user.type(screen.getByRole('textbox', { name: 'Endpoint' }), 'vast.example.com:443'); - await user.type(screen.getByLabelText(/^Username/), 'admin'); + await user.type(screen.getByLabelText(/^Username/), 'test-admin'); await user.type(screen.getByLabelText(/^Password/), testBackendPassword); }; @@ -162,7 +162,7 @@ describe('StorageBackendCreatePage', () => { await user.type(screen.getByRole('textbox', { name: 'Name' }), 'vast-prod-1'); await user.type(screen.getByRole('textbox', { name: 'Endpoint' }), 'vast.example.com:443'); - await user.type(screen.getByLabelText(/^Username/), 'admin'); + await user.type(screen.getByLabelText(/^Username/), 'test-admin'); await user.type(screen.getByLabelText(/^Password/), testBackendPassword); await user.click(screen.getByRole('button', { name: 'Create' })); @@ -231,7 +231,7 @@ describe('StorageBackendCreatePage', () => { expect(capturedRequest?.object?.metadata?.name).toBe('vast-prod-1'); expect(capturedRequest?.object?.spec?.provider).toBe('vast'); expect(capturedRequest?.object?.spec?.endpoint).toBe('vast.example.com:443'); - expect(capturedRequest?.object?.spec?.credentials?.username).toBe('admin'); + expect(capturedRequest?.object?.spec?.credentials?.username).toBe('test-admin'); expect(capturedRequest?.object?.spec?.credentials?.password).toBe(testBackendPassword); }, 15000); @@ -315,7 +315,7 @@ describe('StorageBackendCreatePage', () => { await waitFor(() => { expect(screen.getByLabelText(/^Username/)).toBeInTheDocument(); }); - await user.type(screen.getByLabelText(/^Username/), 'new-admin'); + await user.type(screen.getByLabelText(/^Username/), 'test-new-admin'); await user.click(screen.getByRole('button', { name: 'Save' })); await waitFor(() => { @@ -334,7 +334,7 @@ describe('StorageBackendCreatePage', () => { await waitFor(() => { expect(screen.getByLabelText(/^Password/)).toBeInTheDocument(); }); - await user.type(screen.getByLabelText(/^Password/), 'new-secret'); + await user.type(screen.getByLabelText(/^Password/), 'test-new-secret'); await user.click(screen.getByRole('button', { name: 'Save' })); await waitFor(() => { @@ -389,16 +389,16 @@ describe('StorageBackendCreatePage', () => { await waitFor(() => { expect(screen.getByLabelText(/^Username/)).toBeInTheDocument(); }); - await user.type(screen.getByLabelText(/^Username/), 'new-admin'); - await user.type(screen.getByLabelText(/^Password/), 'new-secret'); + await user.type(screen.getByLabelText(/^Username/), 'test-new-admin'); + await user.type(screen.getByLabelText(/^Password/), 'test-new-secret'); await user.click(screen.getByRole('button', { name: 'Save' })); await waitFor(() => { expect(mockNavigate).toHaveBeenCalledWith('/admin/infrastructure/storage/backends'); }); expect(capturedRequest?.object?.spec?.credentials).toMatchObject({ - username: 'new-admin', - password: 'new-secret', + username: 'test-new-admin', + password: 'test-new-secret', }); });