diff --git a/backend/src/utils/route-security.ts b/backend/src/utils/route-security.ts index 070edd3cb2..5dc8476c9f 100644 --- a/backend/src/utils/route-security.ts +++ b/backend/src/utils/route-security.ts @@ -11,7 +11,7 @@ import { FastifyReply, FastifyRequest } from 'fastify'; import { isUserAdmin } from './adminUtils'; import { getNamespaces } from './notebookUtils'; import { logRequestDetails } from './fileUtils'; -import { DEV_MODE } from './constants'; +import { DEV_MODE, MODEL_REGISTRY_NAMESPACE } from './constants'; const testAdmin = async ( fastify: KubeFastifyInstance, @@ -73,7 +73,7 @@ const requestSecurityGuard = async ( const isReadRequest = request.method.toLowerCase() === 'get'; // Check to see if a request was made against one of our namespaces - if (![notebookNamespace, dashboardNamespace].includes(namespace)) { + if (![notebookNamespace, dashboardNamespace, MODEL_REGISTRY_NAMESPACE].includes(namespace)) { // Not a valid namespace -- cannot make direct calls to just any namespace no matter who you are fastify.log.error( `User requested a resource that was not in our namespaces. Namespace: ${namespace}`, diff --git a/frontend/src/__tests__/cypress/cypress/e2e/modelRegistrySettings/ModelRegistrySettings.cy.ts b/frontend/src/__tests__/cypress/cypress/e2e/modelRegistrySettings/ModelRegistrySettings.cy.ts new file mode 100644 index 0000000000..2680fcfb03 --- /dev/null +++ b/frontend/src/__tests__/cypress/cypress/e2e/modelRegistrySettings/ModelRegistrySettings.cy.ts @@ -0,0 +1,101 @@ +import { mockDashboardConfig, mockDscStatus } from '~/__mocks__'; +import { mockDsciStatus } from '~/__mocks__/mockDsciStatus'; +import { StackCapability, StackComponent } from '~/concepts/areas/types'; +import { modelRegistrySettings } from '~/__tests__/cypress/cypress/pages/modelRegistrySettings'; +import { pageNotfound } from '~/__tests__/cypress/cypress/pages/pageNotFound'; +import { asProductAdminUser, asProjectAdminUser } from '~/__tests__/cypress/cypress/utils/users'; + +const setupMocksForMRSettingAccess = () => { + asProductAdminUser(); + cy.interceptOdh( + 'GET /api/config', + mockDashboardConfig({ + disableModelRegistry: false, + }), + ); + cy.interceptOdh( + 'GET /api/dsc/status', + mockDscStatus({ + installedComponents: { + [StackComponent.MODEL_REGISTRY]: true, + [StackComponent.MODEL_MESH]: true, + }, + }), + ); + cy.interceptOdh( + 'GET /api/dsci/status', + mockDsciStatus({ + requiredCapabilities: [StackCapability.SERVICE_MESH, StackCapability.SERVICE_MESH_AUTHZ], + }), + ); +}; + +it('Model registry settings should not be available for non product admins', () => { + asProjectAdminUser(); + modelRegistrySettings.visit(false); + pageNotfound.findPage().should('exist'); + modelRegistrySettings.findNavItem().should('not.exist'); +}); + +it('Model registry settings should be available for product admins with capabilities', () => { + setupMocksForMRSettingAccess(); + // check page is accessible + modelRegistrySettings.visit(true); + // check nav item exists + modelRegistrySettings.findNavItem().should('exist'); +}); + +describe('CreateModal', () => { + beforeEach(() => { + setupMocksForMRSettingAccess(); + }); + + it('should display error messages when form fields are invalid and not allow submit', () => { + modelRegistrySettings.visit(true); + cy.findByText('Create model registry').click(); + // Fill in the form with invalid data + cy.get('#mr-name').clear(); + cy.get('#mr-host').clear(); + cy.get('#mr-port').clear(); + cy.get('#mr-username').clear(); + cy.get('#mr-password').clear(); + cy.get('#mr-database').clear(); + cy.get('#mr-database').blur(); + // Assert the submit button is disabled + cy.findByTestId('modal-submit-button').should('be.disabled'); + // Assert that error messages are displayed + cy.findByTestId('mr-name-error') + .should('be.visible') + .contains( + "Must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character", + ); + cy.findByTestId('mr-host-error').should('be.visible').contains('Host cannot be empty'); + cy.findByTestId('mr-port-error').should('be.visible').contains('Port cannot be empty'); + cy.findByTestId('mr-username-error').should('be.visible').contains('Username cannot be empty'); + cy.findByTestId('mr-password-error').should('be.visible').contains('Password cannot be empty'); + cy.findByTestId('mr-database-error').scrollIntoView(); + cy.findByTestId('mr-database-error').should('be.visible').contains('Database cannot be empty'); + }); + + it('should enable submit button if fields are valid', () => { + modelRegistrySettings.visit(true); + cy.findByText('Create model registry').click(); + // Fill in the form with invalid data + cy.get('#mr-name').type('valid-mr-name'); + cy.get('#mr-host').type('host'); + cy.get('#mr-port').type('1234'); + cy.get('#mr-username').type('validUser'); + cy.get('#mr-password').type('strongPassword'); + cy.get('#mr-database').type('myDatabase'); + cy.get('#mr-database').blur(); + // Assert the submit button is disabled + cy.findByTestId('modal-submit-button').should('be.enabled'); + // Assert that error messages are not displayed + cy.findByTestId('mr-name-error').should('not.exist'); + cy.findByTestId('mr-host-error').should('not.exist'); + cy.findByTestId('mr-port-error').should('not.exist'); + cy.findByTestId('mr-username-error').should('not.exist'); + cy.findByTestId('mr-password-error').should('not.exist'); + cy.findByTestId('mr-database-error').should('not.exist'); + }); +}); diff --git a/frontend/src/__tests__/cypress/cypress/pages/modelRegistrySettings.ts b/frontend/src/__tests__/cypress/cypress/pages/modelRegistrySettings.ts new file mode 100644 index 0000000000..275148afa2 --- /dev/null +++ b/frontend/src/__tests__/cypress/cypress/pages/modelRegistrySettings.ts @@ -0,0 +1,31 @@ +import { appChrome } from './appChrome'; + +class ModelRegistrySettings { + visit(wait = true) { + cy.visit('/modelRegistrySettings'); + if (wait) { + this.wait(); + } + } + + navigate() { + this.findNavItem().click(); + this.wait(); + } + + private wait() { + this.findHeading(); + cy.testA11y(); + } + + private findHeading() { + cy.findByTestId('app-page-title').should('exist'); + cy.findByTestId('app-page-title').contains('Model Registry Settings'); + } + + findNavItem() { + return appChrome.findNavItem('Model registry settings', 'Settings'); + } +} + +export const modelRegistrySettings = new ModelRegistrySettings(); diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts index c135d12762..86ceef6d30 100644 --- a/frontend/src/api/index.ts +++ b/frontend/src/api/index.ts @@ -39,6 +39,9 @@ export * from './prometheus/distributedWorkloads'; // Network error handling export * from './errorUtils'; +// Models for use when constructing API objects +export * from './models'; + // User access review hook export * from './useAccessReview'; diff --git a/frontend/src/components/FieldList.tsx b/frontend/src/components/FieldList.tsx index b1e5d022c2..4085c08efe 100644 --- a/frontend/src/components/FieldList.tsx +++ b/frontend/src/components/FieldList.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; import { Stack, StackItem, FormGroup, TextInput } from '@patternfly/react-core'; import { EnvVariableDataEntry } from '~/pages/projects/types'; -import PasswordInput from '~/pages/projects/components/PasswordInput'; +import PasswordInput from '~/components/PasswordInput'; export type FieldOptions = { key: string; diff --git a/frontend/src/pages/projects/components/PasswordInput.tsx b/frontend/src/components/PasswordInput.tsx similarity index 60% rename from frontend/src/pages/projects/components/PasswordInput.tsx rename to frontend/src/components/PasswordInput.tsx index 12ca71378a..787cd66b72 100644 --- a/frontend/src/pages/projects/components/PasswordInput.tsx +++ b/frontend/src/components/PasswordInput.tsx @@ -3,20 +3,20 @@ import { Button, InputGroup, TextInput, InputGroupItem } from '@patternfly/react import { EyeIcon, EyeSlashIcon } from '@patternfly/react-icons'; const PasswordInput: React.FC> = (props) => { - const [isPassword, setPassword] = React.useState(true); + const [isPasswordHidden, setPasswordHidden] = React.useState(true); return ( - + diff --git a/frontend/src/k8sTypes.ts b/frontend/src/k8sTypes.ts index 5ae1c0a363..062b7d2d20 100644 --- a/frontend/src/k8sTypes.ts +++ b/frontend/src/k8sTypes.ts @@ -1302,10 +1302,11 @@ export type ModelRegistryKind = K8sResourceCommon & { }; mysql?: { database: string; + username: string; host: string; port?: number; }; - postgres: { + postgres?: { database: string; host?: string; passwordSecret?: { diff --git a/frontend/src/pages/modelRegistrySettings/CreateModal.scss b/frontend/src/pages/modelRegistrySettings/CreateModal.scss new file mode 100644 index 0000000000..56ae84e93e --- /dev/null +++ b/frontend/src/pages/modelRegistrySettings/CreateModal.scss @@ -0,0 +1,6 @@ +// targeting specific form section title elements to override --pf-v5-c-form__section-title--FontSize, --pf-v5-c-form__section-title--FontWeight +.form-subtitle-text { + color: var(--pf-v5-global--Color--200); + font-size: initial; + font-weight: initial; +} \ No newline at end of file diff --git a/frontend/src/pages/modelRegistrySettings/CreateModal.tsx b/frontend/src/pages/modelRegistrySettings/CreateModal.tsx new file mode 100644 index 0000000000..ccc27c9644 --- /dev/null +++ b/frontend/src/pages/modelRegistrySettings/CreateModal.tsx @@ -0,0 +1,270 @@ +import * as React from 'react'; +import { + Button, + Form, + FormGroup, + FormSection, + HelperText, + HelperTextItem, + Modal, + Text, + TextInput, + Title, +} from '@patternfly/react-core'; +import './CreateModal.scss'; +import PasswordInput from '~/components/PasswordInput'; +import DashboardModalFooter from '~/concepts/dashboard/DashboardModalFooter'; +import { ModelRegistryKind } from '~/k8sTypes'; +import { MODEL_REGISTRY_DEFAULT_NAMESPACE } from '~/concepts/modelRegistry/const'; +import { ModelRegistryModel } from '~/api'; +import { createModelRegistryBackend } from '~/services/modelRegistryService'; +import { isValidK8sName } from '~/concepts/k8s/utils'; + +type CreateModalProps = { + isOpen: boolean; + onClose: () => void; +}; + +const CreateModal: React.FC = ({ isOpen, onClose }) => { + const [isSubmitting, setIsSubmitting] = React.useState(false); + const [error, setError] = React.useState(); + const [name, setName] = React.useState(''); + const [host, setHost] = React.useState(''); + const [port, setPort] = React.useState(''); + const [username, setUsername] = React.useState(''); + const [password, setPassword] = React.useState(''); + const [database, setDatabase] = React.useState(''); + const [isNameTouched, setIsNameTouched] = React.useState(false); + const [isHostTouched, setIsHostTouched] = React.useState(false); + const [isPortTouched, setIsPortTouched] = React.useState(false); + const [isUsernameTouched, setIsUsernameTouched] = React.useState(false); + const [isPasswordTouched, setIsPasswordTouched] = React.useState(false); + const [isDatabaseTouched, setIsDatabaseTouched] = React.useState(false); + const [showPassword, setShowPassword] = React.useState(false); + + const onBeforeClose = () => { + setIsSubmitting(false); + setError(undefined); + setName(''); + setHost(''); + setPort(''); + setUsername(''); + setPassword(''); + setDatabase(''); + setIsNameTouched(false); + setIsHostTouched(false); + setIsPortTouched(false); + setIsUsernameTouched(false); + setIsPasswordTouched(false); + setIsDatabaseTouched(false); + setShowPassword(false); + onClose(); + }; + + const onSubmit = async () => { + setIsSubmitting(true); + setError(undefined); + const data: ModelRegistryKind = { + apiVersion: `${ModelRegistryModel.apiGroup}/${ModelRegistryModel.apiVersion}`, + kind: 'ModelRegistry', + metadata: { + name, + namespace: MODEL_REGISTRY_DEFAULT_NAMESPACE, + }, + spec: { + grpc: { + port: 9090, + }, + rest: { + port: 8080, + serviceRoute: 'disabled', + }, + mysql: { + host, + port: Number(port), + database, + username, + }, + }, + }; + try { + await createModelRegistryBackend(data); + onBeforeClose(); + } catch (e) { + if (e instanceof Error) { + setError(e); + } + setIsSubmitting(false); + } + }; + + const hasContent = (value: string): boolean => !!value.trim().length; + + const canSubmit = () => + !isSubmitting && + isValidK8sName(name) && + hasContent(host) && + hasContent(password) && + hasContent(port) && + hasContent(username) && + hasContent(database); + + return ( + + Create + , + , + ]} + variant="medium" + footer={ + + } + > +
+ + setIsNameTouched(true)} + onChange={(_e, value) => setName(value)} + validated={isNameTouched && !isValidK8sName(name) ? 'error' : 'default'} + /> + {isNameTouched && !isValidK8sName(name) && ( + + + {`Must consist of lower case alphanumeric characters or '-', and must start and end + with an alphanumeric character`} + + + )} + + + Database + + This is where model data is stored. You need to connect to an external database. + + + } + > + + setIsHostTouched(true)} + onChange={(_e, value) => setHost(value)} + validated={isHostTouched && !hasContent(host) ? 'error' : 'default'} + /> + {isHostTouched && !hasContent(host) && ( + + + Host cannot be empty + + + )} + + + setIsPortTouched(true)} + onChange={(_e, value) => setPort(value)} + validated={isPortTouched && !hasContent(port) ? 'error' : 'default'} + /> + {isPortTouched && !hasContent(port) && ( + + + Port cannot be empty + + + )} + + + setIsUsernameTouched(true)} + onChange={(_e, value) => setUsername(value)} + validated={isUsernameTouched && !hasContent(username) ? 'error' : 'default'} + /> + {isUsernameTouched && !hasContent(username) && ( + + + Username cannot be empty + + + )} + + + setIsPasswordTouched(true)} + onChange={(_e, value) => setPassword(value)} + validated={isPasswordTouched && !hasContent(password) ? 'error' : 'default'} + /> + {isPasswordTouched && !hasContent(password) && ( + + + Password cannot be empty + + + )} + + + setIsDatabaseTouched(true)} + onChange={(_e, value) => setDatabase(value)} + validated={isDatabaseTouched && !hasContent(database) ? 'error' : 'default'} + /> + {isDatabaseTouched && !hasContent(database) && ( + + + Database cannot be empty + + + )} + + +
+ + ); +}; + +export default CreateModal; diff --git a/frontend/src/pages/modelRegistrySettings/ModelRegistrySettings.tsx b/frontend/src/pages/modelRegistrySettings/ModelRegistrySettings.tsx index b9d0cdbc9c..876d8e1b25 100644 --- a/frontend/src/pages/modelRegistrySettings/ModelRegistrySettings.tsx +++ b/frontend/src/pages/modelRegistrySettings/ModelRegistrySettings.tsx @@ -1,5 +1,4 @@ import React from 'react'; -import { useNavigate } from 'react-router-dom'; import { Button, EmptyState, @@ -12,9 +11,10 @@ import { } from '@patternfly/react-core'; import { PlusCircleIcon } from '@patternfly/react-icons'; import ApplicationsPage from '~/pages/ApplicationsPage'; +import CreateModal from './CreateModal'; const ModelRegistrySettings: React.FC = () => { - const navigate = useNavigate(); + const [createModalOpen, setCreateModalOpen] = React.useState(false); return ( { - + setCreateModalOpen(false)} /> ); }; diff --git a/frontend/src/pages/projects/screens/spawner/environmentVariables/GenericKeyValuePairField.tsx b/frontend/src/pages/projects/screens/spawner/environmentVariables/GenericKeyValuePairField.tsx index 82d501390c..00406ad6f8 100644 --- a/frontend/src/pages/projects/screens/spawner/environmentVariables/GenericKeyValuePairField.tsx +++ b/frontend/src/pages/projects/screens/spawner/environmentVariables/GenericKeyValuePairField.tsx @@ -11,7 +11,7 @@ import { } from '@patternfly/react-core'; import { MinusCircleIcon, PlusCircleIcon } from '@patternfly/react-icons'; import { EnvVariableDataEntry } from '~/pages/projects/types'; -import PasswordInput from '~/pages/projects/components/PasswordInput'; +import PasswordInput from '~/components/PasswordInput'; import { EMPTY_KEY_VALUE_PAIR } from './const'; import { removeArrayItem, updateArrayValue } from './utils';