diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml new file mode 100644 index 0000000..84807e1 --- /dev/null +++ b/.github/workflows/ui-tests.yml @@ -0,0 +1,44 @@ +name: UI Tests + +on: + push: + branches: [ "main" ] + paths: + - 'ui/**' + - '.github/workflows/ui-tests.yml' + pull_request: + branches: [ "main" ] + paths: + - 'ui/**' + - '.github/workflows/ui-tests.yml' + +jobs: + ui-test: + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + defaults: + run: + working-directory: ui + steps: + - name: Checkout code + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + + - name: Set up node.js + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + node-version: '18' + cache: 'yarn' + cache-dependency-path: ui/yarn.lock + + - name: Install dependencies + run: yarn install --frozen-lockfile --network-timeout 1000000 + + # Jest unit + component tests (all HTTP is mocked; no external services). + - name: Run unit + component tests + run: CI=true yarn test --watchAll=false + + # Production build type-checks the whole app (tsc is stricter than jest). + - name: Production build + run: yarn build diff --git a/ui/package.json b/ui/package.json index 265f8eb..d1c0c30 100644 --- a/ui/package.json +++ b/ui/package.json @@ -62,5 +62,11 @@ "resolutions": { "react-scripts/resolve-url-loader/postcss": "8.4.49", "react-scripts/@svgr/webpack/**/nth-check": "2.1.1" + }, + "jest": { + "moduleNameMapper": { + "^axios$": "axios/dist/node/axios.cjs", + "^antd/es/(.*)$": "antd/lib/$1" + } } } diff --git a/ui/src/App.tsx b/ui/src/App.tsx index 1a21173..f8ff195 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -10,8 +10,10 @@ import { } from 'antd'; import { createBrowserRouter, + Navigate, Outlet, RouterProvider, + useLocation, useNavigate, } from 'react-router-dom'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; @@ -30,6 +32,10 @@ import Login from './pages/Login'; import { AuthProvider, useAuth } from './context/auth-context'; import { UserOutlined } from '@ant-design/icons'; import ModelVersionDetails from './pages/ModelVersionDetails'; +import ExternalData from './pages/ExternalData'; +import CredentialDetails from './pages/CredentialDetails'; +import ExternalLocationDetails from './pages/ExternalLocationDetails'; +import UsersList from './pages/UsersList'; // TODO: // As of [19/02/2025], this implementation should be updated once the following PR are merged. @@ -73,6 +79,30 @@ const router = createBrowserRouter([ path: '/models/:catalog/:schema/:model/versions/:version', element: , }, + { + path: '/external-data', + element: , + }, + { + path: '/external-data/external-locations', + element: , + }, + { + path: '/external-data/credentials', + element: , + }, + { + path: '/external-data/external-locations/:name', + element: , + }, + { + path: '/external-data/credentials/:name', + element: , + }, + { + path: '/users', + element: , + }, ], }, ]); @@ -80,6 +110,13 @@ const router = createBrowserRouter([ function AppProvider() { const { logout, currentUser } = useAuth(); const navigate = useNavigate(); + const { pathname } = useLocation(); + + const selectedNavKey = pathname.startsWith('/external-data') + ? 'external-data' + : pathname.startsWith('/users') + ? 'users' + : 'catalogs'; const profileMenuItems = useMemo( (): MenuProps['items'] => [ @@ -146,13 +183,23 @@ function AppProvider() { navigate('/'), }, + { + key: 'external-data', + label: 'External Data', + onClick: () => navigate('/external-data'), + }, + { + key: 'users', + label: 'Users', + onClick: () => navigate('/users'), + }, ]} style={{ flex: 1, minWidth: 0 }} /> @@ -215,7 +262,12 @@ function AppProvider() { function App() { const queryClient = new QueryClient({ - defaultOptions: { queries: { staleTime: QUERY_STALE_TIME } }, + defaultOptions: { + // No retry: these are authenticated admin CRUD reads — a 401/403/404 is + // deterministic, and the default 3 retries just delayed the error state + // by several seconds of blank UI. + queries: { staleTime: QUERY_STALE_TIME, retry: false }, + }, }); return authEnabled ? ( diff --git a/ui/src/components/credentials/CredentialsList.tsx b/ui/src/components/credentials/CredentialsList.tsx new file mode 100644 index 0000000..8162761 --- /dev/null +++ b/ui/src/components/credentials/CredentialsList.tsx @@ -0,0 +1,90 @@ +import { useState } from 'react'; +import { Button, Tag, Typography } from 'antd'; +import { useNavigate } from 'react-router-dom'; +import ListLayout from '../layouts/ListLayout'; +import { formatTimestamp } from '../../utils/formatTimestamp'; +import { + CredentialInterface, + useListCredentials, +} from '../../hooks/credentials'; +import { useListExternalLocations } from '../../hooks/externalLocations'; +import { credentialIdentityOf, credentialTypeOf } from '../../utils/credential'; +import { CreateCredentialModal } from '../modals/CreateCredentialModal'; +import ValidateCredentialButton from './ValidateCredentialButton'; + +const TYPE_COLORS: Record = { + 'Aliyun RAM (STS)': 'orange', + 'Aliyun AK/SK (static)': 'gold', + 'AWS IAM role': 'geekblue', +}; + +export default function CredentialsList() { + const { data, isLoading, error } = useListCredentials(); + const { data: locationsData } = useListExternalLocations(); + const navigate = useNavigate(); + const [open, setOpen] = useState(false); + const externalLocations = locationsData?.external_locations ?? []; + + return ( + <> + + loading={isLoading} + error={error} + title={ +
+ +
+ } + data={data?.credentials} + onRowClick={(record) => + navigate(`/external-data/credentials/${record.name}`) + } + rowKey={(record) => `credential-${record.id}`} + columns={[ + { title: 'Name', dataIndex: 'name', key: 'name', width: '25%' }, + { + title: 'Type', + key: 'type', + width: '15%', + render: (_, record) => { + const type = credentialTypeOf(record); + return {type}; + }, + }, + { + title: 'Role ARN / Access key', + key: 'identity', + width: '30%', + render: (_, record) => ( + + {credentialIdentityOf(record)} + + ), + }, + { title: 'Owner', dataIndex: 'owner', key: 'owner', width: '10%' }, + { + title: 'Created At', + dataIndex: 'created_at', + key: 'created_at', + width: '12%', + render: (value) => (value ? formatTimestamp(value) : ''), + }, + { + title: 'Actions', + key: 'actions', + width: '8%', + render: (_, record) => ( + + ), + }, + ]} + /> + setOpen(false)} /> + + ); +} diff --git a/ui/src/components/credentials/ValidateCredentialButton.test.tsx b/ui/src/components/credentials/ValidateCredentialButton.test.tsx new file mode 100644 index 0000000..4f1069d --- /dev/null +++ b/ui/src/components/credentials/ValidateCredentialButton.test.tsx @@ -0,0 +1,76 @@ +import { fireEvent, screen, waitFor } from '@testing-library/react'; +import { renderWithProviders } from '../../test-utils/render'; +import { programClient, requestsTo } from '../../test-utils/mockClient'; +import ValidateCredentialButton from './ValidateCredentialButton'; + +jest.mock('../../context/client', () => ({ CLIENT: { request: jest.fn() } })); + +const CREDENTIAL = { + name: 'demo-sts', + aliyun_ram_role: { role_arn: 'acs:ram::1:role/r' }, +}; +const LOCATION = { + name: 'loc1', + url: 'oss://bkt/path', + credential_name: 'demo-sts', +}; + +describe('ValidateCredentialButton', () => { + it('is disabled when the credential has no bound external location', () => { + programClient([]); + renderWithProviders( + , + ); + expect(screen.getByRole('button', { name: /validate/i })).toBeDisabled(); + }); + + it('vends PATH_READ credentials for the bound location url', async () => { + programClient([ + { + method: 'post', + url: '/temporary-path-credentials', + response: { expiration_time: 123 }, + }, + ]); + renderWithProviders( + , + ); + fireEvent.click(screen.getByRole('button', { name: /validate/i })); + await waitFor(() => + expect(requestsTo('post', '/temporary-path-credentials')).toHaveLength(1), + ); + expect(requestsTo('post', '/temporary-path-credentials')[0].data).toEqual({ + url: 'oss://bkt/path', + operation: 'PATH_READ', + }); + // Success notification names the probed path and the credential. + await screen.findByText( + /Credential vending succeeded for oss:\/\/bkt\/path using demo-sts/, + ); + }); + + it('surfaces the server error message on failure', async () => { + programClient([ + { + method: 'post', + url: '/temporary-path-credentials', + response: { message: 'STS AssumeRole denied for role' }, + status: 403, + }, + ]); + renderWithProviders( + , + ); + fireEvent.click(screen.getByRole('button', { name: /validate/i })); + await screen.findByText(/STS AssumeRole denied for role/); + }); +}); diff --git a/ui/src/components/credentials/ValidateCredentialButton.tsx b/ui/src/components/credentials/ValidateCredentialButton.tsx new file mode 100644 index 0000000..fd8b6c1 --- /dev/null +++ b/ui/src/components/credentials/ValidateCredentialButton.tsx @@ -0,0 +1,81 @@ +import { Button, Tooltip } from 'antd'; +import { SafetyCertificateOutlined } from '@ant-design/icons'; +import { CredentialInterface } from '../../hooks/credentials'; +import { ExternalLocationInterface } from '../../hooks/externalLocations'; +import { useVendPathCredentials } from '../../hooks/pathCredentials'; +import { useNotification } from '../../utils/NotificationContext'; +import { PathOperation } from '../../types/api/catalog.gen'; + +interface ValidateCredentialButtonProps { + credential: CredentialInterface; + externalLocations: ExternalLocationInterface[]; + size?: 'small' | 'middle'; +} + +/** + * Validates that the Unity Catalog server can actually use a credential + * (i.e. assume the RAM/IAM role, or use the static AK/SK) by vending + * temporary PATH_READ credentials for an external location bound to it — + * the exact server-side flow real table access uses. Requires the + * credential to be bound to at least one external location. + */ +export default function ValidateCredentialButton({ + credential, + externalLocations, + size = 'small', +}: ValidateCredentialButtonProps) { + const mutation = useVendPathCredentials(); + const { setNotification } = useNotification(); + + const boundLocation = externalLocations.find( + (location) => location.credential_name === credential.name, + ); + + return ( + + + + ); +} diff --git a/ui/src/components/externalLocations/ExternalLocationsList.tsx b/ui/src/components/externalLocations/ExternalLocationsList.tsx new file mode 100644 index 0000000..27b8aee --- /dev/null +++ b/ui/src/components/externalLocations/ExternalLocationsList.tsx @@ -0,0 +1,76 @@ +import { useState } from 'react'; +import { Button, Typography } from 'antd'; +import { Link, useNavigate } from 'react-router-dom'; +import ListLayout from '../layouts/ListLayout'; +import { formatTimestamp } from '../../utils/formatTimestamp'; +import { + ExternalLocationInterface, + useListExternalLocations, +} from '../../hooks/externalLocations'; +import { CreateExternalLocationModal } from '../modals/CreateExternalLocationModal'; + +export default function ExternalLocationsList() { + const { data, isLoading, error } = useListExternalLocations(); + const navigate = useNavigate(); + const [open, setOpen] = useState(false); + + return ( + <> + + loading={isLoading} + error={error} + title={ +
+ +
+ } + data={data?.external_locations} + onRowClick={(record) => + navigate(`/external-data/external-locations/${record.name}`) + } + rowKey={(record) => `external-location-${record.id}`} + columns={[ + { title: 'Name', dataIndex: 'name', key: 'name', width: '20%' }, + { + title: 'URL', + dataIndex: 'url', + key: 'url', + width: '35%', + render: (value) => {value}, + }, + { + title: 'Credential', + dataIndex: 'credential_name', + key: 'credential_name', + width: '15%', + render: (value) => + value ? ( + e.stopPropagation()} + > + {value} + + ) : ( + '' + ), + }, + { title: 'Owner', dataIndex: 'owner', key: 'owner', width: '15%' }, + { + title: 'Created At', + dataIndex: 'created_at', + key: 'created_at', + width: '15%', + render: (value) => (value ? formatTimestamp(value) : ''), + }, + ]} + /> + setOpen(false)} + /> + + ); +} diff --git a/ui/src/components/layouts/ListLayout.module.css b/ui/src/components/layouts/ListLayout.module.css index 238ee18..1a4fe61 100644 --- a/ui/src/components/layouts/ListLayout.module.css +++ b/ui/src/components/layouts/ListLayout.module.css @@ -1,5 +1,5 @@ .clickableListLayout { - [class~=ant-table-row] { + [class~='ant-table-row'] { cursor: pointer; } -} \ No newline at end of file +} diff --git a/ui/src/components/layouts/ListLayout.tsx b/ui/src/components/layouts/ListLayout.tsx index 090b9a7..532edff 100644 --- a/ui/src/components/layouts/ListLayout.tsx +++ b/ui/src/components/layouts/ListLayout.tsx @@ -1,5 +1,6 @@ import { SearchOutlined } from '@ant-design/icons'; import { + Alert, Col, Flex, Input, @@ -12,6 +13,10 @@ import { AnyObject } from 'antd/es/_util/type'; import { ReactNode, useMemo, useState } from 'react'; import styles from './ListLayout.module.css'; +// Page-size options shown by the table's size changer, with 20 as the default. +export const PAGE_SIZE_OPTIONS = [20, 50, 100]; +const DEFAULT_PAGE_SIZE = 20; + interface ListLayoutProps { data: T[] | undefined; columns: TableColumnsType; @@ -21,6 +26,12 @@ interface ListLayoutProps { loading?: boolean; filters?: ReactNode; showSearch?: boolean; + // Error from the data query; when set, an alert replaces the table so a + // failed load is never shown as an empty ("no data") list. + error?: Error | null; + // Text a row is matched against by the search box. Defaults to the row's + // name; pass a custom accessor to also search other fields (e.g. email). + searchText?: (item: T) => string; } export default function ListLayout({ @@ -32,16 +43,21 @@ export default function ListLayout({ loading, filters, showSearch = true, + error, + searchText, }: ListLayoutProps) { const [filterValue, setFilterValue] = useState(''); - const [pageSize, setPageSize] = useState(10); + const [pageSize, setPageSize] = useState(DEFAULT_PAGE_SIZE); const filteredData = useMemo(() => { if (!filterValue) return data; + const needle = filterValue.toLowerCase(); return data?.filter((item) => - String(item.name).toLowerCase().includes(filterValue.toLowerCase()), + (searchText ? searchText(item) : String(item.name)) + .toLowerCase() + .includes(needle), ); - }, [data, filterValue]); + }, [data, filterValue, searchText]); const onShowSizeChange = (current: number, pageSize: number) => { setPageSize(pageSize); @@ -50,6 +66,14 @@ export default function ListLayout({ return ( {title} + {error && ( + + )} {showSearch && ( ({ rowKey={rowKey} loading={loading} className={onRowClick ? styles.clickableListLayout : undefined} - dataSource={filteredData} + dataSource={error ? [] : filteredData} columns={columns} pagination={{ - hideOnSinglePage: true, pageSize: pageSize, + showSizeChanger: true, + pageSizeOptions: PAGE_SIZE_OPTIONS, onShowSizeChange: onShowSizeChange, showTotal: (total, range) => `${range[0]}-${range[1]} of ${total}`, }} diff --git a/ui/src/components/modals/CreateCredentialModal.test.tsx b/ui/src/components/modals/CreateCredentialModal.test.tsx new file mode 100644 index 0000000..bb978b6 --- /dev/null +++ b/ui/src/components/modals/CreateCredentialModal.test.tsx @@ -0,0 +1,113 @@ +import { fireEvent, screen, waitFor } from '@testing-library/react'; +import { renderWithProviders } from '../../test-utils/render'; +import { clickModalOk } from '../../test-utils/antd'; +import { programClient, requestsTo } from '../../test-utils/mockClient'; +import { CreateCredentialModal } from './CreateCredentialModal'; + +jest.mock('../../context/client', () => ({ CLIENT: { request: jest.fn() } })); + +describe('CreateCredentialModal', () => { + it('creates an Aliyun STS credential (role_arn only)', async () => { + programClient([ + { method: 'post', url: '/credentials', response: { name: 'c1' } }, + ]); + renderWithProviders(); + + fireEvent.change(screen.getByLabelText('Name'), { + target: { value: 'c1' }, + }); + fireEvent.change(screen.getByLabelText('Role ARN'), { + target: { value: 'acs:ram::1:role/demo' }, + }); + clickModalOk(); + + await waitFor(() => + expect(requestsTo('post', '/credentials')).toHaveLength(1), + ); + expect(requestsTo('post', '/credentials')[0].data).toMatchObject({ + name: 'c1', + purpose: 'STORAGE', + aliyun_ram_role: { role_arn: 'acs:ram::1:role/demo' }, + }); + }); + + it('switches to static AK/SK fields and sends them (never a role_arn)', async () => { + programClient([ + { method: 'post', url: '/credentials', response: { name: 'c2' } }, + ]); + renderWithProviders(); + + fireEvent.click(screen.getByLabelText('Aliyun static AK/SK')); + // Role ARN input disappears, AK/SK inputs appear. + expect(screen.queryByLabelText('Role ARN')).toBeNull(); + fireEvent.change(screen.getByLabelText('Name'), { + target: { value: 'c2' }, + }); + fireEvent.change(screen.getByLabelText('Access key ID'), { + target: { value: 'LTAI5tXXXX' }, + }); + fireEvent.change(screen.getByLabelText('Access key secret'), { + target: { value: 's3cret' }, + }); + clickModalOk(); + + await waitFor(() => + expect(requestsTo('post', '/credentials')).toHaveLength(1), + ); + const body = requestsTo('post', '/credentials')[0].data; + expect(body).toMatchObject({ + name: 'c2', + purpose: 'STORAGE', + aliyun_ram_role: { + access_key_id: 'LTAI5tXXXX', + access_key_secret: 's3cret', + }, + }); + expect(body.aliyun_ram_role.role_arn).toBeUndefined(); + expect(body.aws_iam_role).toBeUndefined(); + }); + + it('creates an AWS IAM role credential', async () => { + programClient([ + { method: 'post', url: '/credentials', response: { name: 'c3' } }, + ]); + renderWithProviders(); + + fireEvent.click(screen.getByLabelText('AWS IAM role')); + fireEvent.change(screen.getByLabelText('Name'), { + target: { value: 'c3' }, + }); + fireEvent.change(screen.getByLabelText('Role ARN'), { + target: { value: 'arn:aws:iam::1:role/demo' }, + }); + clickModalOk(); + + await waitFor(() => + expect(requestsTo('post', '/credentials')).toHaveLength(1), + ); + expect(requestsTo('post', '/credentials')[0].data).toMatchObject({ + name: 'c3', + aws_iam_role: { role_arn: 'arn:aws:iam::1:role/demo' }, + }); + }); + + it('surfaces a server-side permission error', async () => { + programClient([ + { + method: 'post', + url: '/credentials', + response: { message: 'Access denied: CREATE_STORAGE_CREDENTIAL' }, + status: 403, + }, + ]); + renderWithProviders(); + fireEvent.change(screen.getByLabelText('Name'), { + target: { value: 'c4' }, + }); + fireEvent.change(screen.getByLabelText('Role ARN'), { + target: { value: 'acs:ram::1:role/demo' }, + }); + clickModalOk(); + await screen.findByText(/Access denied: CREATE_STORAGE_CREDENTIAL/); + }); +}); diff --git a/ui/src/components/modals/CreateCredentialModal.tsx b/ui/src/components/modals/CreateCredentialModal.tsx new file mode 100644 index 0000000..7fe27d5 --- /dev/null +++ b/ui/src/components/modals/CreateCredentialModal.tsx @@ -0,0 +1,171 @@ +import { Button, Form, Input, Modal, Radio, Typography } from 'antd'; +import { useCallback, useEffect, useRef, useState } from 'react'; +import TextArea from 'antd/es/input/TextArea'; +import { useNavigate } from 'react-router-dom'; +import { useNotification } from '../../utils/NotificationContext'; +import { useCreateCredential } from '../../hooks/credentials'; +import { CredentialPurpose } from '../../types/api/catalog.gen'; + +type CredentialType = 'aliyun_sts' | 'aliyun_static' | 'aws_iam'; + +interface CreateCredentialFormValues { + name: string; + comment?: string; + role_arn?: string; + access_key_id?: string; + access_key_secret?: string; +} + +interface CreateCredentialModalProps { + open: boolean; + closeModal: () => void; +} + +export function CreateCredentialModal({ + open, + closeModal, +}: CreateCredentialModalProps) { + const navigate = useNavigate(); + const mutation = useCreateCredential(); + const { setNotification } = useNotification(); + const submitRef = useRef(null); + const [credentialType, setCredentialType] = + useState('aliyun_sts'); + + // destroyOnClose resets the form fields but not this local state; re-default + // the type each time the modal opens so it doesn't reveal the prior choice. + useEffect(() => { + if (open) setCredentialType('aliyun_sts'); + }, [open]); + + const handleSubmit = useCallback(() => { + submitRef.current?.click(); + }, []); + + return ( + Create credential} + okText="Create" + cancelText="Cancel" + open={open} + destroyOnClose + onCancel={closeModal} + onOk={handleSubmit} + okButtonProps={{ loading: mutation.isPending }} + > + + Register a storage credential used to vend temporary credentials for + external locations. Creation requires metastore OWNER or the + CREATE_STORAGE_CREDENTIAL privilege; the server enforces this on submit. + + + layout="vertical" + onFinish={(values) => { + mutation.mutate( + { + name: values.name, + comment: values.comment, + purpose: CredentialPurpose.STORAGE, + ...(credentialType === 'aws_iam' + ? { aws_iam_role: { role_arn: values.role_arn ?? '' } } + : credentialType === 'aliyun_sts' + ? { aliyun_ram_role: { role_arn: values.role_arn } } + : { + aliyun_ram_role: { + access_key_id: values.access_key_id, + access_key_secret: values.access_key_secret, + }, + }), + }, + { + onError: (error: Error) => { + setNotification(error.message, 'error'); + }, + onSuccess: (credential) => { + setNotification( + `Credential ${credential.name} created`, + 'success', + ); + closeModal(); + navigate(`/external-data/credentials/${credential.name}`); + }, + }, + ); + }} + name="Create credential form" + > + Name} + name="name" + rules={[{ required: true, message: 'Name is required' }]} + > + + + Type}> + setCredentialType(e.target.value)} + options={[ + { value: 'aliyun_sts', label: 'Aliyun RAM role (STS)' }, + { value: 'aliyun_static', label: 'Aliyun static AK/SK' }, + { value: 'aws_iam', label: 'AWS IAM role' }, + ]} + /> + + {credentialType !== 'aliyun_static' && ( + Role ARN} + name="role_arn" + rules={[{ required: true, message: 'Role ARN is required' }]} + > + :role/' + : 'arn:aws:iam:::role/' + } + /> + + )} + {credentialType === 'aliyun_static' && ( + <> + Access key ID} + name="access_key_id" + rules={[{ required: true, message: 'Access key ID is required' }]} + > + + + Access key secret + } + name="access_key_secret" + rules={[ + { required: true, message: 'Access key secret is required' }, + ]} + extra="Stored server-side and never returned by the API; responses always redact it." + > + + + + )} + Comment} + name="comment" + > +