Skip to content

Commit

Permalink
Support edit model registry after it's created (#3572)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ppadti
ppadti authored Dec 17, 2024
1 parent 1896f34 commit 5d3efb2
Show file tree
Hide file tree
Showing 10 changed files with 376 additions and 296 deletions.
5 changes: 4 additions & 1 deletion backend/src/routes/api/modelRegistries/modelRegistryUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ const patchModelRegistry = async (
dryRun ? 'All' : undefined,
undefined,
undefined,
{ headers: { 'Content-type': PatchUtils.PATCH_FORMAT_JSON_PATCH } },
{ headers: { 'Content-type': PatchUtils.PATCH_FORMAT_JSON_MERGE_PATCH } },
// patchNamespacedCustomObject doesn't support TS generics and returns body as `object`, so we assert its real type
) as Promise<{ body: ModelRegistryKind }>);
return response.body;
Expand Down Expand Up @@ -279,6 +279,9 @@ const updateDatabasePassword = async (
},
undefined,
dryRun ? 'All' : undefined,
undefined,
undefined,
{ headers: { 'Content-type': PatchUtils.PATCH_FORMAT_JSON_MERGE_PATCH } },
);
} else {
await deleteDatabasePasswordSecret(fastify, modelRegistry, modelRegistryNamespace);
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/__tests__/cypress/cypress/pages/modelRegistrySettings.ts
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { appChrome } from './appChrome';
import { K8sNameDescriptionField } from './components/subComponents/K8sNameDescriptionField';

export enum FormFieldSelector {
NAME = '#mr-name',
RESOURCENAME = '#resource-mr-name',
HOST = '#mr-host',
PORT = '#mr-port',
USERNAME = '#mr-username',
Expand All @@ -27,6 +27,8 @@ export enum DatabaseDetailsTestId {
}

class ModelRegistrySettings {
k8sNameDescription = new K8sNameDescriptionField('mr');

visit(wait = true) {
cy.visitWithLogin('/modelRegistrySettings');
if (wait) {
Expand Down
7 changes: 7 additions & 0 deletions frontend/src/__tests__/cypress/cypress/support/commands/odh.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,13 @@ declare global {
type: 'GET /api/modelRegistries',
response: OdhResponse<K8sResourceListResult<ModelRegistryKind>>,
) => Cypress.Chainable<null>) &
((
type: 'PATCH /api/modelRegistries/:modelRegistryName',
options: {
path: { modelRegistryName: string };
},
response: OdhResponse<{ modelRegistry: ModelRegistryKind; databasePassword?: string }>,
) => Cypress.Chainable<null>) &
((
type: 'GET /api/modelRegistries/:modelRegistryName',
options: {
Expand Down
107 changes: 87 additions & 20 deletions .../__tests__/cypress/cypress/tests/mocked/modelRegistrySettings/modelRegistrySettings.cy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { StackCapability, StackComponent } from '~/concepts/areas/types';
import {
FormFieldSelector,
modelRegistrySettings,
DatabaseDetailsTestId,
} from '~/__tests__/cypress/cypress/pages/modelRegistrySettings';
import { pageNotfound } from '~/__tests__/cypress/cypress/pages/pageNotFound';
import {
Expand All @@ -26,8 +25,10 @@ const groupSubjects: RoleBindingSubject[] = [

const setupMocksForMRSettingAccess = ({
hasModelRegistries = true,
hasDatabasePassword = true,
}: {
hasModelRegistries?: boolean;
hasDatabasePassword?: boolean;
}) => {
asProductAdminUser();
cy.interceptOdh(
Expand Down Expand Up @@ -82,7 +83,7 @@ const setupMocksForMRSettingAccess = ({
},
{
modelRegistry: mockModelRegistry({ name: 'test-registry-1' }),
databasePassword: 'test-password',
databasePassword: hasDatabasePassword ? 'test-password' : undefined,
},
);
cy.interceptOdh(
Expand All @@ -95,6 +96,17 @@ const setupMocksForMRSettingAccess = ({
},
);

cy.interceptOdh(
'PATCH /api/modelRegistries/:modelRegistryName',
{
path: { modelRegistryName: 'test-registry-1' },
},
{
modelRegistry: mockModelRegistry({ name: 'test-registry-1' }),
databasePassword: 'test-password',
},
);

cy.interceptOdh(
'GET /api/modelRegistryRoleBindings',
mockK8sResourceList([
Expand Down Expand Up @@ -161,6 +173,21 @@ describe('CreateModal', () => {
modelRegistrySettings.findFormField(FormFieldSelector.PASSWORD).type('strongPassword');
modelRegistrySettings.findFormField(FormFieldSelector.DATABASE).type('myDatabase');
modelRegistrySettings.findFormField(FormFieldSelector.DATABASE).blur();
modelRegistrySettings.findSubmitButton().should('be.enabled');

// test resource name validation
modelRegistrySettings.k8sNameDescription.findResourceEditLink().click();
modelRegistrySettings.k8sNameDescription
.findResourceNameInput()
.should('have.attr', 'aria-invalid', 'false');
// Invalid character k8s names fail
modelRegistrySettings.k8sNameDescription.findResourceNameInput().clear().type('InVaLiD vAlUe!');
modelRegistrySettings.k8sNameDescription
.findResourceNameInput()
.should('have.attr', 'aria-invalid', 'true');
modelRegistrySettings.findSubmitButton().should('be.disabled');
modelRegistrySettings.k8sNameDescription.findResourceNameInput().clear().type('image');

modelRegistrySettings.findSubmitButton().should('be.enabled');
modelRegistrySettings.shouldHaveNoErrors();
});
Expand All @@ -176,40 +203,80 @@ describe('ModelRegistriesTable', () => {
});
});

describe('ViewDatabaseConfigModal', () => {
it('Shows database details for a registry', () => {
describe('EditModelRegistry', () => {
it('Update model registry', () => {
setupMocksForMRSettingAccess({});
modelRegistrySettings.visit(true);
modelRegistrySettings
.findModelRegistryRow('test-registry-1')
.findKebabAction('View database configuration')
.findKebabAction('Edit model registry')
.click();
modelRegistrySettings
.findDatabaseDetail(DatabaseDetailsTestId.HOST)
.should('contain.text', 'model-registry-db');
.findFormField(FormFieldSelector.NAME)
.should('have.value', 'test-registry-1');
modelRegistrySettings.findFormField(FormFieldSelector.NAME).clear().type('test-2');
modelRegistrySettings
.findDatabaseDetail(DatabaseDetailsTestId.PORT)
.should('contain.text', '5432');
.findFormField(FormFieldSelector.HOST)
.should('have.value', 'model-registry-db');
modelRegistrySettings.findFormField(FormFieldSelector.PORT).should('have.value', '5432');
modelRegistrySettings
.findDatabaseDetail(DatabaseDetailsTestId.USERNAME)
.should('contain.text', 'mlmduser');
modelRegistrySettings.findDatabasePasswordHiddenButton().click();
.findFormField(FormFieldSelector.USERNAME)
.should('have.value', 'mlmduser');
modelRegistrySettings
.findDatabaseDetail(DatabaseDetailsTestId.PASSWORD)
.should('contain.text', 'test-password');
.findFormField(FormFieldSelector.PASSWORD)
.should('have.value', 'test-password');
modelRegistrySettings
.findDatabaseDetail(DatabaseDetailsTestId.DATABASE)
.should('contain.text', 'model-registry');
.findFormField(FormFieldSelector.DATABASE)
.should('have.value', 'model-registry');
modelRegistrySettings.findSubmitButton().should('be.enabled');
modelRegistrySettings.findSubmitButton().click();
});

it('Shows error loading password when secret fails to decode', () => {
setupMocksForMRSettingAccess({});
it('Shows skeleton, when password is loading', () => {
setupMocksForMRSettingAccess({ hasDatabasePassword: false });
modelRegistrySettings.visit(true);
modelRegistrySettings
.findModelRegistryRow('test-registry-1')
.findKebabAction('Edit model registry')
.click();
modelRegistrySettings
.findFormField(FormFieldSelector.NAME)
.should('have.value', 'test-registry-1');
modelRegistrySettings
.findFormField(FormFieldSelector.HOST)
.should('have.value', 'model-registry-db');
modelRegistrySettings.findFormField(FormFieldSelector.PORT).should('have.value', '5432');
modelRegistrySettings
.findFormField(FormFieldSelector.USERNAME)
.should('have.value', 'mlmduser');
modelRegistrySettings
.findFormField(FormFieldSelector.DATABASE)
.should('have.value', 'model-registry');
modelRegistrySettings.findSubmitButton().should('be.disabled');
});

it('Shows erros, when password fails to load', () => {
setupMocksForMRSettingAccess({ hasDatabasePassword: false });
modelRegistrySettings.visit(true);
modelRegistrySettings
.findModelRegistryRow('test-registry-2')
.findKebabAction('View database configuration')
.findKebabAction('Edit model registry')
.click();
cy.findByText('Error loading password').should('exist');
modelRegistrySettings
.findFormField(FormFieldSelector.NAME)
.should('have.value', 'test-registry-2');
modelRegistrySettings
.findFormField(FormFieldSelector.HOST)
.should('have.value', 'model-registry-db');
modelRegistrySettings.findFormField(FormFieldSelector.PORT).should('have.value', '5432');
modelRegistrySettings
.findFormField(FormFieldSelector.USERNAME)
.should('have.value', 'mlmduser');
cy.findByText('Failed to load the password. The Secret file is missing.').should('exist');
modelRegistrySettings
.findFormField(FormFieldSelector.DATABASE)
.should('have.value', 'model-registry');
modelRegistrySettings.findSubmitButton().should('be.disabled');
});
});

Expand Down
Loading

0 comments on commit 5d3efb2

Please sign in to comment.