Skip to content

Commit bfc4af9

Browse files
add changes requested
Signed-off-by: Philip Colares Carneiro <philip.colares@gmail.com>
1 parent 8ea3ded commit bfc4af9

4 files changed

Lines changed: 34 additions & 21 deletions

File tree

frontend/src/concepts/k8s/K8sNameDescriptionField/utils.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,14 @@ export enum LimitNameResourceType {
3333
/** Model deployments create routes */
3434
MODEL_DEPLOYMENT,
3535
PVC,
36+
/** Model Registry names are capped at 40 characters */
37+
MODEL_REGISTRY,
3638
}
3739
/** K8s max DNS subdomain name length */
3840
const MAX_RESOURCE_NAME_LENGTH = 253;
3941

4042
const MAX_PVC_NAME_LENGTH = 63;
43+
const MAX_MODEL_REGISTRY_NAME_LENGTH = 40;
4144

4245
/** KServe InferenceService names must start with a lowercase letter */
4346
export const INFERENCE_SERVICE_NAME_REGEX = /^[a-z]([-a-z0-9]*[a-z0-9])?$/;
@@ -49,6 +52,7 @@ export const resourceTypeLimits: Record<LimitNameResourceType, number> = {
4952
[LimitNameResourceType.WORKBENCH]: ROUTE_BASED_NAME_LENGTH,
5053
[LimitNameResourceType.MODEL_DEPLOYMENT]: ROUTE_BASED_NAME_LENGTH,
5154
[LimitNameResourceType.PVC]: MAX_PVC_NAME_LENGTH,
55+
[LimitNameResourceType.MODEL_REGISTRY]: MAX_MODEL_REGISTRY_NAME_LENGTH,
5256
};
5357

5458
export const isK8sNameDescriptionType = (

frontend/src/pages/modelRegistrySettings/CreateModal.tsx

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,17 @@ import {
2020
createModelRegistryBackend,
2121
updateModelRegistryBackend,
2222
} from '#~/services/modelRegistrySettingsService';
23-
import { isValidK8sName, kindApiVersion, translateDisplayNameForK8s } from '#~/concepts/k8s/utils';
23+
import { kindApiVersion } from '#~/concepts/k8s/utils';
2424
import FormSection from '#~/components/pf-overrides/FormSection';
2525
import { AreaContext } from '#~/concepts/areas/AreaContext';
2626
import { SupportedArea, useIsAreaAvailable } from '#~/concepts/areas';
2727
import K8sNameDescriptionField, {
2828
useK8sNameDescriptionFieldData,
2929
} from '#~/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField';
30+
import {
31+
isK8sNameDescriptionDataValid,
32+
LimitNameResourceType,
33+
} from '#~/concepts/k8s/K8sNameDescriptionField/utils';
3034
import useModelRegistryCertificateNames from '#~/concepts/modelRegistrySettings/useModelRegistryCertificateNames';
3135
import {
3236
buildDatabaseSpec,
@@ -68,6 +72,7 @@ const CreateModal: React.FC<CreateModalProps> = ({ onClose, refresh, modelRegist
6872
const [error, setError] = React.useState<Error>();
6973
const { data: nameDesc, onDataChange: setNameDesc } = useK8sNameDescriptionFieldData({
7074
initialData: mr,
75+
limitNameResourceType: LimitNameResourceType.MODEL_REGISTRY,
7176
});
7277
const [databaseSource, setDatabaseSource] = React.useState<DatabaseSource>(
7378
DatabaseSource.DEFAULT,
@@ -231,8 +236,6 @@ const CreateModal: React.FC<CreateModalProps> = ({ onClose, refresh, modelRegist
231236
setDatabaseType(newType);
232237
};
233238

234-
const effectiveK8sName = nameDesc.k8sName.value || translateDisplayNameForK8s(nameDesc.name);
235-
236239
const onSubmit = async () => {
237240
setIsSubmitting(true);
238241
setError(undefined);
@@ -322,7 +325,7 @@ const CreateModal: React.FC<CreateModalProps> = ({ onClose, refresh, modelRegist
322325
apiVersion: kindApiVersion(ModelRegistryModel),
323326
kind: 'ModelRegistry',
324327
metadata: {
325-
name: effectiveK8sName,
328+
name: nameDesc.k8sName.value,
326329
namespace: modelRegistryNamespace,
327330
annotations: {
328331
'openshift.io/description': nameDesc.description,
@@ -389,15 +392,12 @@ const CreateModal: React.FC<CreateModalProps> = ({ onClose, refresh, modelRegist
389392
const hasContent = (value: string): boolean => !!value.trim().length;
390393

391394
const canSubmit = () => {
392-
const isValidName =
393-
isValidK8sName(effectiveK8sName) && effectiveK8sName.length <= MAX_MODEL_REGISTRY_NAME_LENGTH;
395+
const isValidName = isK8sNameDescriptionDataValid(nameDesc);
394396

395397
if (databaseSource === DatabaseSource.DEFAULT) {
396-
// For default database, only name is required
397398
return !isSubmitting && isValidName;
398399
}
399400

400-
// For external database, all connection fields are required
401401
return (
402402
!isSubmitting &&
403403
isValidName &&

packages/cypress/cypress/pages/modelRegistrySettings.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { Contextual } from './components/Contextual';
33
import { K8sNameDescriptionField } from './components/subComponents/K8sNameDescriptionField';
44
import { SearchSelector } from './components/subComponents/SearchSelector';
55

6+
export const MAX_MODEL_REGISTRY_NAME_LENGTH = 40;
7+
68
export enum FormFieldSelector {
79
NAME = '#mr-name',
810
DESCRIPTION = '#mr-description',

packages/cypress/cypress/tests/mocked/modelRegistrySettings/modelRegistrySettings.cy.ts

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { mockRoleBindingK8sResource } from '@odh-dashboard/internal/__mocks__/mo
1212
import {
1313
DatabaseType,
1414
FormFieldSelector,
15+
MAX_MODEL_REGISTRY_NAME_LENGTH,
1516
modelRegistrySettings,
1617
} from '../../../pages/modelRegistrySettings';
1718
import { pageNotfound } from '../../../pages/pageNotFound';
@@ -336,51 +337,57 @@ describe('CreateModal', () => {
336337
modelRegistrySettings.shouldHaveAllErrors();
337338
});
338339

339-
it('should enforce maxLength of 40 on the name input field', () => {
340+
it('should enforce maxLength on the name input field', () => {
340341
modelRegistrySettings.visit(true);
341342
modelRegistrySettings.findCreateButton().click();
342343
modelRegistrySettings
343344
.findFormField(FormFieldSelector.NAME)
344-
.should('have.attr', 'maxLength', '40');
345+
.should('have.attr', 'maxLength', String(MAX_MODEL_REGISTRY_NAME_LENGTH));
345346
});
346347

347-
it('should disable submit when resource name exceeds 40 characters', () => {
348+
it('should disable submit when resource name exceeds the character limit', () => {
348349
modelRegistrySettings.visit(true);
349350
modelRegistrySettings.findCreateButton().click();
350351
modelRegistrySettings.findDatabaseSourceDefaultRadio().should('be.checked');
351352

352-
modelRegistrySettings.findFormField(FormFieldSelector.NAME).type('a'.repeat(40));
353+
modelRegistrySettings
354+
.findFormField(FormFieldSelector.NAME)
355+
.type('a'.repeat(MAX_MODEL_REGISTRY_NAME_LENGTH));
353356
modelRegistrySettings.findSubmitButton().should('be.enabled');
354357

355358
modelRegistrySettings.k8sNameDescription.findResourceEditLink().click();
356-
modelRegistrySettings.k8sNameDescription.findResourceNameInput().clear().type('a'.repeat(41));
359+
modelRegistrySettings.k8sNameDescription
360+
.findResourceNameInput()
361+
.clear()
362+
.type('a'.repeat(MAX_MODEL_REGISTRY_NAME_LENGTH + 1));
357363
modelRegistrySettings.findSubmitButton().should('be.disabled');
358364
});
359365

360-
it('should show character count warning when name approaches 40 limit', () => {
366+
it('should show character count warning when name approaches the limit', () => {
361367
modelRegistrySettings.visit(true);
362368
modelRegistrySettings.findCreateButton().click();
363369

364-
const nearLimitName = 'a'.repeat(31);
370+
const nearLimitName = 'a'.repeat(MAX_MODEL_REGISTRY_NAME_LENGTH - 9);
365371
modelRegistrySettings.findFormField(FormFieldSelector.NAME).type(nearLimitName);
366-
cy.contains('Cannot exceed 40 characters').should('be.visible');
372+
cy.contains(`Cannot exceed ${MAX_MODEL_REGISTRY_NAME_LENGTH} characters`).should('be.visible');
367373
cy.contains('remaining').should('be.visible');
368374
});
369375

370-
it('should allow creation with name at exactly 40 characters using default database', () => {
376+
it('should allow creation with name at exactly the character limit using default database', () => {
371377
modelRegistrySettings.visit(true);
372378
modelRegistrySettings.findCreateButton().click();
373379
modelRegistrySettings.findDatabaseSourceDefaultRadio().should('be.checked');
374380

375-
const exactName = 'a'.repeat(40);
381+
const exactName = 'a'.repeat(MAX_MODEL_REGISTRY_NAME_LENGTH);
376382
modelRegistrySettings.findFormField(FormFieldSelector.NAME).type(exactName);
377383
modelRegistrySettings.findSubmitButton().should('be.enabled');
378384
modelRegistrySettings.findSubmitButton().click();
379385

380386
cy.wait('@createModelRegistry').then((interception) => {
381-
const expectedName = 'a'.repeat(40);
382-
expect(interception.request.body.modelRegistry.metadata.name).to.equal(expectedName);
383-
expect(interception.request.body.modelRegistry.metadata.name).to.have.length.at.most(40);
387+
expect(interception.request.body.modelRegistry.metadata.name).to.equal(exactName);
388+
expect(interception.request.body.modelRegistry.metadata.name).to.have.length.at.most(
389+
MAX_MODEL_REGISTRY_NAME_LENGTH,
390+
);
384391
});
385392
});
386393

0 commit comments

Comments
 (0)