From a5ceb941c0a5ac0266ba0c36d647c27099b7889c Mon Sep 17 00:00:00 2001 From: Juntao Wang Date: Fri, 20 Sep 2024 15:09:34 -0400 Subject: [PATCH] Fix validation in the storage modal mount path --- .../src/__mocks__/mockNotebookK8sResource.ts | 5 ++- .../cypress/cypress/pages/clusterStorage.ts | 4 ++ .../mocked/projects/clusterStorage.cy.ts | 43 ++++++++++++++++++- .../src/pages/projects/pvc/MountPathField.tsx | 13 +++--- 4 files changed, 57 insertions(+), 8 deletions(-) diff --git a/frontend/src/__mocks__/mockNotebookK8sResource.ts b/frontend/src/__mocks__/mockNotebookK8sResource.ts index 6086799802..02a0499924 100644 --- a/frontend/src/__mocks__/mockNotebookK8sResource.ts +++ b/frontend/src/__mocks__/mockNotebookK8sResource.ts @@ -1,7 +1,7 @@ import _ from 'lodash-es'; import { KnownLabels, NotebookKind } from '~/k8sTypes'; import { DEFAULT_NOTEBOOK_SIZES } from '~/pages/projects/screens/spawner/const'; -import { ContainerResources, TolerationEffect, TolerationOperator } from '~/types'; +import { ContainerResources, TolerationEffect, TolerationOperator, VolumeMount } from '~/types'; import { genUID } from '~/__mocks__/mockUtils'; import { RecursivePartial } from '~/typeHelpers'; @@ -17,6 +17,7 @@ type MockResourceConfigType = { lastImageSelection?: string; opts?: RecursivePartial; uid?: string; + additionalVolumeMounts?: VolumeMount[]; }; export const mockNotebookK8sResource = ({ @@ -31,6 +32,7 @@ export const mockNotebookK8sResource = ({ lastImageSelection = 's2i-minimal-notebook:py3.8-v1', opts = {}, uid = genUID('notebook'), + additionalVolumeMounts = [], }: MockResourceConfigType): NotebookKind => _.merge( { @@ -144,6 +146,7 @@ export const mockNotebookK8sResource = ({ mountPath: '/opt/app-root/src', name, }, + ...additionalVolumeMounts, ], workingDir: '/opt/app-root/src', }, diff --git a/frontend/src/__tests__/cypress/cypress/pages/clusterStorage.ts b/frontend/src/__tests__/cypress/cypress/pages/clusterStorage.ts index 1574717ae2..967d4ad98f 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/clusterStorage.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/clusterStorage.ts @@ -49,6 +49,10 @@ class ClusterStorageModal extends Modal { return this.find().findByTestId('mount-path-folder-value'); } + findMountFieldHelperText() { + return this.find().findByTestId('mount-path-folder-helper-text'); + } + findWorkbenchRestartAlert() { return this.find().findByTestId('notebook-restart-alert'); } diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/clusterStorage.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/clusterStorage.cy.ts index be0981068e..1fe0f1bd9a 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/clusterStorage.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/clusterStorage.cy.ts @@ -53,7 +53,19 @@ const initInterceptors = ({ isEmpty = false, storageClassName }: HandlersProps) ], ), ); - cy.interceptK8sList(NotebookModel, mockK8sResourceList([mockNotebookK8sResource({})])); + cy.interceptK8sList( + NotebookModel, + mockK8sResourceList([ + mockNotebookK8sResource({ + additionalVolumeMounts: [ + { + mountPath: '/opt/app-root/src/test-dupe', + name: 'test-dupe-pvc-path', + }, + ], + }), + ]), + ); }; const [openshiftDefaultStorageClass, otherStorageClass] = mockStorageClasses; @@ -133,6 +145,35 @@ describe('ClusterStorage', () => { .findWorkbenchConnectionSelect() .findSelectOption('Test Notebook') .click(); + + // don't allow duplicate path + addClusterStorageModal.findMountField().clear(); + addClusterStorageModal.findMountField().fill('test-dupe'); + addClusterStorageModal + .findMountFieldHelperText() + .should('have.text', 'Mount folder is already in use for this workbench'); + + // don't allow number in the path + addClusterStorageModal.findMountField().clear(); + addClusterStorageModal.findMountField().fill('test2'); + addClusterStorageModal + .findMountFieldHelperText() + .should('have.text', 'Must only consist of lower case letters and dashes'); + + // Allow trailing slash + addClusterStorageModal.findMountField().clear(); + addClusterStorageModal.findMountField().fill('test/'); + addClusterStorageModal + .findMountFieldHelperText() + .should('have.text', 'Must consist of lower case letters and dashes.'); + + addClusterStorageModal.findMountField().clear(); + addClusterStorageModal + .findMountFieldHelperText() + .should( + 'have.text', + 'Enter a path to a model or folder. This path cannot point to a root folder.', + ); addClusterStorageModal.findMountField().fill('data'); addClusterStorageModal.findWorkbenchRestartAlert().should('exist'); diff --git a/frontend/src/pages/projects/pvc/MountPathField.tsx b/frontend/src/pages/projects/pvc/MountPathField.tsx index fd66a9a3d7..bd26b47ee5 100644 --- a/frontend/src/pages/projects/pvc/MountPathField.tsx +++ b/frontend/src/pages/projects/pvc/MountPathField.tsx @@ -37,8 +37,8 @@ const MountPathField: React.FC = ({ onChange={(e, value) => { let error = ''; if (value.length === 0) { - error = 'Required'; - } else if (!/^[a-z-]+$/.test(value)) { + error = 'Enter a path to a model or folder. This path cannot point to a root folder.'; + } else if (!/^[a-z-]+\/?$/.test(value)) { error = 'Must only consist of lower case letters and dashes'; } else if (inUseMountPaths.includes(`/${value}`)) { error = 'Mount folder is already in use for this workbench'; @@ -50,10 +50,11 @@ const MountPathField: React.FC = ({ - - {mountPath.error - ? 'Enter a path to a model or folder. This path cannot point to a root folder.' - : 'Must consist of lower case letters and dashes.'} + + {mountPath.error ? mountPath.error : 'Must consist of lower case letters and dashes.'}