Skip to content

Commit f5c2c10

Browse files
manaswinidasmanaswinidas
authored
fix(model-registry): surface K8s name validation errors on MCP deploy name field (opendatahub-io#7192)
* fix(model-registry): surface K8s name validation errors on the deployment name field The K8sNameDescriptionField component silently disabled the submit button when the auto-generated resource name was empty (symbols-only input) or exceeded the 253-character limit, with no visible feedback. - Show inline error when the display name produces an empty resource name (e.g. only special characters entered). - Surface invalidLength and invalidCharacters errors directly below the name field when the "Edit resource name" section is collapsed. - Add red validation border on the name input when errors are present. - Extract duplicated validation message strings into shared constants in utils.ts (DRY). Made-with: Cursor * fix(model-registry): use dashboard K8s name utilities in MCP deploy modal to reject invalid names * fix(model-registry): fix prettier formatting in ResourceNameField and McpDeployModal Made-with: Cursor * fix(model-registry): select project in deploy modal Cypress tests The namespace selector resolves an extension-provided project dropdown in CI (Module Federation), so it does not auto-select. Manually select the project before asserting the deploy button enables. Made-with: Cursor * fix(model-registry): use display name 'Test Project' for project selector in tests Made-with: Cursor * fix(model-registry): align selectProject with mcpCatalog page object pattern Made-with: Cursor * fix(model-registry): unwrap assembleModArchBody data wrapper in deploy modal test assertions Made-with: Cursor * Remove alias and export value from utils
1 parent cc83365 commit f5c2c10

6 files changed

Lines changed: 170 additions & 34 deletions

File tree

‎packages/cypress/cypress/pages/mcpDeployments.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,23 @@ class McpDeployModal {
178178
return this.findModal().findByTestId('modal-cancel-button');
179179
}
180180

181+
findResourceNameHelperText(): Cypress.Chainable<JQuery<HTMLElement>> {
182+
return this.findModal().findByTestId('mcp-deploy-name-helper');
183+
}
184+
185+
findProjectSelectorToggle(): Cypress.Chainable<JQuery<HTMLElement>> {
186+
return this.findModal().findByTestId('project-selector-toggle');
187+
}
188+
189+
findProjectSelectorOption(name: string): Cypress.Chainable<JQuery<HTMLElement>> {
190+
return cy.findByTestId('project-selector-menuList').findByRole('menuitem', { name });
191+
}
192+
193+
selectProject(name: string): void {
194+
this.findProjectSelectorToggle().click();
195+
this.findProjectSelectorOption(name).click();
196+
}
197+
181198
findSubmitError(): Cypress.Chainable<JQuery<HTMLElement>> {
182199
return this.findModal().findByTestId('error-message-alert');
183200
}

‎packages/cypress/cypress/tests/mocked/mcpCatalog/mcpDeployments.cy.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,4 +487,54 @@ describe('MCP Deploy from Catalog', () => {
487487
.should('be.visible')
488488
.and('contain.text', 'Failed to load MCP server configuration');
489489
});
490+
491+
it('should enable deploy button when a valid name is entered', () => {
492+
initDeployIntercepts();
493+
494+
cy.intercept('POST', `${MCP_DEPLOYMENTS_API}*`, {
495+
body: { data: mockRunningDeployment() },
496+
}).as('createDeployment');
497+
498+
cy.visitWithLogin(`/ai-hub/mcp-servers/catalog/${TEST_SERVER_ID}`);
499+
mcpServerDetailsPage.findDeployButton().click();
500+
cy.wait('@getConverter');
501+
mcpDeployModal.shouldBeOpen();
502+
mcpDeployModal.findSubmitButton().should('be.disabled');
503+
504+
mcpDeployModal.findNameInput().type('My Server');
505+
mcpDeployModal.selectProject('Test Project');
506+
mcpDeployModal.findSubmitButton().should('not.be.disabled');
507+
mcpDeployModal.findSubmitButton().click();
508+
509+
cy.wait('@createDeployment').then((interception) => {
510+
const body = interception.request.body.data || interception.request.body;
511+
expect(body).to.have.property('name', 'my-server');
512+
expect(body).to.have.property('displayName', 'My Server');
513+
});
514+
});
515+
516+
it('should auto-generate a valid K8s name for dash-only display name input', () => {
517+
initDeployIntercepts();
518+
519+
cy.intercept('POST', `${MCP_DEPLOYMENTS_API}*`, {
520+
body: { data: mockRunningDeployment() },
521+
}).as('createDeployment');
522+
523+
cy.visitWithLogin(`/ai-hub/mcp-servers/catalog/${TEST_SERVER_ID}`);
524+
mcpServerDetailsPage.findDeployButton().click();
525+
cy.wait('@getConverter');
526+
mcpDeployModal.shouldBeOpen();
527+
528+
mcpDeployModal.findNameInput().type('----');
529+
mcpDeployModal.selectProject('Test Project');
530+
mcpDeployModal.findResourceNameHelperText().should('contain.text', 'The resource name will be');
531+
mcpDeployModal.findSubmitButton().should('not.be.disabled');
532+
mcpDeployModal.findSubmitButton().click();
533+
534+
cy.wait('@createDeployment').then((interception) => {
535+
const body = interception.request.body.data || interception.request.body;
536+
expect(body.name).to.match(/^gen-[a-z0-9]+$/);
537+
expect(body.name).to.not.equal('----');
538+
});
539+
});
490540
});

‎packages/model-registry/upstream/frontend/src/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField.tsx‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
HelperTextItem,
77
TextArea,
88
TextInput,
9+
ValidatedOptions,
910
} from '@patternfly/react-core';
1011
import ResourceNameDefinitionTooltip from '~/concepts/k8s/ResourceNameDefinitionTooltip';
1112
import FormFieldset from '~/app/pages/modelRegistry/screens/components/FormFieldset';
@@ -15,7 +16,12 @@ import {
1516
UseK8sNameDescriptionDataConfiguration,
1617
UseK8sNameDescriptionFieldData,
1718
} from './types';
18-
import { handleUpdateLogic, setupDefaults } from './utils';
19+
import {
20+
handleUpdateLogic,
21+
setupDefaults,
22+
getMaxLengthErrorMessage,
23+
INVALID_K8S_NAME_CHARACTERS_MESSAGE,
24+
} from './utils';
1925
import ResourceNameField from './ResourceNameField';
2026

2127
/** Companion data hook */
@@ -60,6 +66,13 @@ const K8sNameDescriptionField: React.FC<K8sNameDescriptionFieldProps> = ({
6066

6167
const { name, description, k8sName } = data;
6268

69+
const hasEmptyResourceName = !!name && !k8sName.value && !k8sName.state.immutable;
70+
const hasHiddenValidationError =
71+
!showK8sField &&
72+
!k8sName.state.immutable &&
73+
(k8sName.state.invalidLength || k8sName.state.invalidCharacters);
74+
const hasNameError = hasEmptyResourceName || hasHiddenValidationError;
75+
6376
const nameInput = (
6477
<TextInput
6578
aria-readonly={!onDataChange}
@@ -69,6 +82,7 @@ const K8sNameDescriptionField: React.FC<K8sNameDescriptionFieldProps> = ({
6982
value={name}
7083
onChange={(_e, value) => onDataChange?.('name', value)}
7184
isRequired
85+
validated={hasNameError ? ValidatedOptions.error : ValidatedOptions.default}
7286
/>
7387
);
7488

@@ -91,14 +105,28 @@ const K8sNameDescriptionField: React.FC<K8sNameDescriptionFieldProps> = ({
91105
<FormGroup label={nameLabel} isRequired fieldId={`${dataTestId}-name`}>
92106
<FormFieldset component={nameInput} field="Name" />
93107
{nameHelperText || (!showK8sField && !k8sName.state.immutable) ? (
94-
<HelperText>
108+
<HelperText data-testid={`${dataTestId}-name-helper`}>
95109
{nameHelperText && <HelperTextItem>{nameHelperText}</HelperTextItem>}
96110
{!showK8sField && !k8sName.state.immutable && (
97111
<>
98-
{k8sName.value && (
112+
{hasEmptyResourceName ? (
113+
<HelperTextItem variant="error">
114+
The name must contain at least one alphanumeric character.
115+
</HelperTextItem>
116+
) : k8sName.value ? (
99117
<HelperTextItem>
100118
The resource name will be <b>{k8sName.value}</b>.
101119
</HelperTextItem>
120+
) : null}
121+
{k8sName.state.invalidLength && (
122+
<HelperTextItem variant="error">
123+
{getMaxLengthErrorMessage(k8sName.state.maxLength)}
124+
</HelperTextItem>
125+
)}
126+
{k8sName.state.invalidCharacters && (
127+
<HelperTextItem variant="error">
128+
{INVALID_K8S_NAME_CHARACTERS_MESSAGE}
129+
</HelperTextItem>
102130
)}
103131
<HelperTextItem>
104132
<Button

‎packages/model-registry/upstream/frontend/src/concepts/k8s/K8sNameDescriptionField/ResourceNameField.tsx‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
} from '@patternfly/react-core';
99
import FormFieldset from '~/app/pages/modelRegistry/screens/components/FormFieldset';
1010
import { K8sNameDescriptionFieldData, K8sNameDescriptionFieldUpdateFunction } from './types';
11+
import { getMaxLengthErrorMessage, INVALID_K8S_NAME_CHARACTERS_MESSAGE } from './utils';
1112

1213
type ResourceNameFieldProps = {
1314
allowEdit: boolean;
@@ -69,14 +70,11 @@ const ResourceNameField: React.FC<ResourceNameFieldProps> = ({
6970
<HelperText>
7071
{k8sName.state.invalidLength && (
7172
<HelperTextItem variant="error">
72-
Cannot exceed {k8sName.state.maxLength} characters
73+
{getMaxLengthErrorMessage(k8sName.state.maxLength)}
7374
</HelperTextItem>
7475
)}
7576
{k8sName.state.invalidCharacters && (
76-
<HelperTextItem variant="error">
77-
Must start and end with a lowercase letter or number. Valid characters include lowercase
78-
letters, numbers, and hyphens (-).
79-
</HelperTextItem>
77+
<HelperTextItem variant="error">{INVALID_K8S_NAME_CHARACTERS_MESSAGE}</HelperTextItem>
8078
)}
8179
{!k8sName.state.invalidLength && !k8sName.state.invalidCharacters && (
8280
<HelperTextItem>

‎packages/model-registry/upstream/frontend/src/concepts/k8s/K8sNameDescriptionField/utils.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,13 @@ import {
44
UseK8sNameDescriptionDataConfiguration,
55
} from './types';
66

7-
const MAX_K8S_NAME_LENGTH = 253;
7+
export const MAX_K8S_NAME_LENGTH = 253;
8+
9+
export const getMaxLengthErrorMessage = (maxLength: number): string =>
10+
`Cannot exceed ${maxLength} characters`;
11+
12+
export const INVALID_K8S_NAME_CHARACTERS_MESSAGE =
13+
'Must start and end with a lowercase letter or number. Valid characters include lowercase letters, numbers, and hyphens (-).';
814

915
/**
1016
* Translates a name to a k8s-safe value.

‎packages/model-registry/upstream/frontend/src/odh/components/McpDeployModal.tsx‎

Lines changed: 62 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,15 @@ import { CodeEditor, Language } from '@patternfly/react-code-editor';
1616
import { APIOptions, useQueryParamNamespaces } from 'mod-arch-core';
1717
import { DashboardModalFooter, FieldGroupHelpLabelIcon } from 'mod-arch-shared';
1818
import { useThemeContext } from '@odh-dashboard/internal/app/ThemeContext';
19+
import {
20+
translateDisplayNameForK8s,
21+
isValidK8sName,
22+
} from '@odh-dashboard/internal/concepts/k8s/utils';
1923
import NamespaceSelectorFieldWrapper from '~/odh/components/NamespaceSelectorFieldWrapper';
2024
import useMcpServerConverter from '~/app/hooks/mcpCatalogDeployment/useMcpServerConverter';
21-
import K8sNameDescriptionField, {
22-
useK8sNameDescriptionFieldData,
23-
} from '~/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField';
25+
import K8sNameDescriptionField from '~/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField';
26+
import { K8sNameDescriptionFieldData } from '~/concepts/k8s/K8sNameDescriptionField/types';
27+
import { MAX_K8S_NAME_LENGTH } from '~/concepts/k8s/K8sNameDescriptionField/utils';
2428
import { createMcpDeployment, updateMcpDeployment } from '~/app/api/mcpCatalogDeployment/service';
2529
import { mcpDeploymentsUrl } from '~/app/routes/mcpCatalog/mcpCatalog';
2630
import { mcpServerCRToYaml } from '~/app/utils/mcpServerYaml';
@@ -43,15 +47,51 @@ const McpDeployModal: React.FC<McpDeployModalProps> = ({
4347
const { theme } = useThemeContext();
4448
const [crData, crLoaded, crError] = useMcpServerConverter(existingDeployment ? '' : serverId);
4549

46-
const { data: nameDescData, onDataChange: onNameDescChange } = useK8sNameDescriptionFieldData(
47-
existingDeployment
48-
? {
49-
initialData: {
50-
name: existingDeployment.displayName ?? existingDeployment.name,
51-
k8sName: existingDeployment.name,
52-
},
53-
}
54-
: {},
50+
const [displayNameValue, setDisplayNameValue] = React.useState(
51+
existingDeployment ? (existingDeployment.displayName ?? existingDeployment.name) : '',
52+
);
53+
const [k8sNameManual, setK8sNameManual] = React.useState('');
54+
const [k8sTouched, setK8sTouched] = React.useState(false);
55+
56+
const autoK8sName = React.useMemo(
57+
() => translateDisplayNameForK8s(displayNameValue),
58+
[displayNameValue],
59+
);
60+
const effectiveK8sName = existingDeployment
61+
? existingDeployment.name
62+
: k8sTouched
63+
? k8sNameManual
64+
: autoK8sName;
65+
66+
const nameDescData = React.useMemo<K8sNameDescriptionFieldData>(
67+
() => ({
68+
name: displayNameValue,
69+
description: '',
70+
k8sName: {
71+
value: effectiveK8sName,
72+
state: {
73+
immutable: !!existingDeployment,
74+
invalidCharacters:
75+
effectiveK8sName.length > 0 ? !isValidK8sName(effectiveK8sName) : false,
76+
invalidLength: effectiveK8sName.length > MAX_K8S_NAME_LENGTH,
77+
maxLength: MAX_K8S_NAME_LENGTH,
78+
touched: k8sTouched,
79+
},
80+
},
81+
}),
82+
[displayNameValue, effectiveK8sName, k8sTouched, existingDeployment],
83+
);
84+
85+
const onNameDescChange = React.useCallback(
86+
(key: keyof K8sNameDescriptionFieldData, value: string) => {
87+
if (key === 'name') {
88+
setDisplayNameValue(value);
89+
} else if (key === 'k8sName') {
90+
setK8sNameManual(value);
91+
setK8sTouched(true);
92+
}
93+
},
94+
[],
5595
);
5696

5797
const [selectedNamespace, setSelectedNamespace] = React.useState(
@@ -84,11 +124,8 @@ const McpDeployModal: React.FC<McpDeployModalProps> = ({
84124
setSelectedNamespace(projectName);
85125
}, []);
86126

87-
const displayName = nameDescData.name;
88-
const k8sName = nameDescData.k8sName.value;
89-
90127
const handleDeploy = React.useCallback(async () => {
91-
if (!ociImageValue || !selectedNamespace || !k8sName) {
128+
if (!ociImageValue || !selectedNamespace || !effectiveK8sName) {
92129
return;
93130
}
94131

@@ -106,16 +143,16 @@ const McpDeployModal: React.FC<McpDeployModalProps> = ({
106143
opts,
107144
existingDeployment.name,
108145
{
109-
displayName,
146+
displayName: displayNameValue,
110147
image: ociImageValue,
111148
yaml: yamlContent,
112149
},
113150
);
114151
onClose(true);
115152
} else {
116153
await createMcpDeployment('', { ...queryParams, namespace: selectedNamespace })(opts, {
117-
name: k8sName,
118-
displayName,
154+
name: effectiveK8sName,
155+
displayName: displayNameValue,
119156
serverName: crData?.metadata.name || undefined,
120157
image: ociImageValue,
121158
yaml: yamlContent,
@@ -135,8 +172,8 @@ const McpDeployModal: React.FC<McpDeployModalProps> = ({
135172
}, [
136173
ociImageValue,
137174
selectedNamespace,
138-
k8sName,
139-
displayName,
175+
effectiveK8sName,
176+
displayNameValue,
140177
yamlContent,
141178
crData,
142179
queryParams,
@@ -146,10 +183,10 @@ const McpDeployModal: React.FC<McpDeployModalProps> = ({
146183
]);
147184

148185
const hasValidName =
149-
!!displayName &&
150-
!!k8sName &&
151-
!nameDescData.k8sName.state.invalidCharacters &&
152-
!nameDescData.k8sName.state.invalidLength;
186+
!!displayNameValue &&
187+
!!effectiveK8sName &&
188+
isValidK8sName(effectiveK8sName) &&
189+
effectiveK8sName.length <= MAX_K8S_NAME_LENGTH;
153190

154191
const dataReady = !!existingDeployment || crLoaded;
155192

0 commit comments

Comments
 (0)