Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

connection type compatibility column #3225

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions frontend/src/__mocks__/mockConnectionType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -542,3 +542,38 @@ const mockFields: ConnectionTypeField[] = [
},
},
];

export const mockModelServingFields: ConnectionTypeField[] = [
{
type: 'short-text',
name: 'Access key',
description: '',
envVar: 'AWS_ACCESS_KEY_ID',
required: true,
properties: {},
},
{
type: 'hidden',
name: 'Secret key',
description: '',
envVar: 'AWS_SECRET_ACCESS_KEY',
required: true,
properties: {},
},
{
type: 'short-text',
name: 'Endpoint',
description: '',
envVar: 'AWS_S3_ENDPOINT',
required: true,
properties: {},
},
{
type: 'short-text',
name: 'Bucket',
description: '',
envVar: 'AWS_S3_BUCKET',
required: true,
properties: {},
},
];
24 changes: 20 additions & 4 deletions frontend/src/__tests__/cypress/cypress/pages/connectionTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ class CreateConnectionTypePage {
rowNames.map((name, index) =>
this.getFieldsTableRow(index).findName().should('contain.text', name),
);
return this;
}

getCategorySection() {
Expand Down Expand Up @@ -136,7 +137,8 @@ class ConnectionTypeRow extends TableRow {
}

shouldHaveName(name: string) {
return this.findConnectionTypeName().should('have.text', name);
this.findConnectionTypeName().should('have.text', name);
return this;
}

findConnectionTypeDescription() {
Expand All @@ -147,16 +149,28 @@ class ConnectionTypeRow extends TableRow {
return this.find().findByTestId('connection-type-creator');
}

findConnectionTypeCompatibility() {
return this.find().findByTestId('connection-type-compatibility');
}

shouldHaveDescription(description: string) {
return this.findConnectionTypeDescription().should('contain.text', description);
this.findConnectionTypeDescription().should('contain.text', description);
return this;
}

shouldHaveModelServingCompatibility() {
this.findConnectionTypeCompatibility().should('have.text', 'Model serving');
return this;
}

shouldHaveCreator(creator: string) {
return this.findConnectionTypeCreator().should('have.text', creator);
this.findConnectionTypeCreator().should('have.text', creator);
return this;
}

shouldShowPreInstalledLabel() {
return this.find().findByTestId('connection-type-user-label').should('exist');
this.find().findByTestId('connection-type-user-label').should('exist');
return this;
}

findEnabled() {
Expand All @@ -173,10 +187,12 @@ class ConnectionTypeRow extends TableRow {

shouldBeEnabled() {
this.findEnabled().should('be.checked');
return this;
}

shouldBeDisabled() {
this.findEnabled().should('not.be.checked');
return this;
}

findEnableStatus() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import {
} from '~/__tests__/cypress/cypress/utils/mockUsers';
import { connectionTypesPage } from '~/__tests__/cypress/cypress/pages/connectionTypes';
import { mockDashboardConfig } from '~/__mocks__';
import { mockConnectionTypeConfigMap } from '~/__mocks__/mockConnectionType';
import {
mockConnectionTypeConfigMap,
mockModelServingFields,
} from '~/__mocks__/mockConnectionType';
import { deleteModal } from '~/__tests__/cypress/cypress/pages/components/DeleteModal';

it('Connection types should not be available for non product admins', () => {
Expand Down Expand Up @@ -49,6 +52,7 @@ describe('Connection types', () => {
description: 'description 2',
enabled: false,
preInstalled: true,
fields: mockModelServingFields,
}),
mockConnectionTypeConfigMap({
name: 'test-2',
Expand Down Expand Up @@ -76,6 +80,7 @@ describe('Connection types', () => {
row2.shouldHaveDescription('description 2');
row2.shouldShowPreInstalledLabel();
row2.shouldBeDisabled();
row2.shouldHaveModelServingCompatibility();
});

it('should delete connection type', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,14 @@ describe('Connections', () => {
initIntercepts();
projectDetails.visitSection('test-project', 'connections');
projectDetails.shouldBeEmptyState('Connections', 'connections', false);
connectionsPage.findTable().findByText('test1').should('exist');
connectionsPage.findTable().findByText('s3').should('exist');
connectionsPage.findTable().findByText('test2').should('exist');
connectionsPage.findTable().findByText('postgres').should('exist');
const row1 = connectionsPage.getConnectionRow('test1');
row1.find().findByText('test1').should('exist');
row1.find().findByText('s3').should('exist');
row1.find().findByText('Model serving').should('exist');
const row2 = connectionsPage.getConnectionRow('test2');
row2.find().findByText('test2').should('exist');
row2.find().findByText('postgres').should('exist');
row1.find().findByText('Model serving').should('exist');
});

it('Delete a connection', () => {
Expand Down
11 changes: 11 additions & 0 deletions frontend/src/concepts/connectionTypes/CompatibilityLabel.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import * as React from 'react';
import { Label } from '@patternfly/react-core';
import { CompatibleTypes } from '~/concepts/connectionTypes/utils';

type Props = {
type: CompatibleTypes;
};

const CompatibilityLabel: React.FC<Props> = ({ type }) => <Label color="blue">{type}</Label>;

export default CompatibilityLabel;
50 changes: 50 additions & 0 deletions frontend/src/concepts/connectionTypes/__tests__/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@ import {
TextField,
} from '~/concepts/connectionTypes/types';
import {
CompatibleTypes,
defaultValueToString,
fieldNameToEnvVar,
fieldTypeToString,
getCompatibleTypes,
isModelServingCompatible,
isValidEnvVar,
toConnectionTypeConfigMap,
toConnectionTypeConfigMapObj,
Expand Down Expand Up @@ -262,3 +265,50 @@ describe('isValidEnvVar', () => {
expect(isValidEnvVar('has space')).toBe(false);
});
});

describe('isModelServingCompatible', () => {
it('should identify model serving compatible env vars', () => {
expect(isModelServingCompatible([])).toBe(false);
expect(
isModelServingCompatible([
'AWS_ACCESS_KEY_ID',
'AWS_SECRET_ACCESS_KEY',
'AWS_S3_ENDPOINT',
'AWS_S3_BUCKET',
]),
).toBe(true);
expect(isModelServingCompatible(['AWS_ACCESS_KEY_ID', 'AWS_SECRET_ACCESS_KEY'])).toBe(false);
expect(
isModelServingCompatible([
'AWS_ACCESS_KEY_ID',
'AWS_SECRET_ACCESS_KEY',
'AWS_S3_ENDPOINT',
'AWS_S3_BUCKET',
'URI',
]),
).toBe(true);
});
});

describe('getCompatibleTypes', () => {
it('should return compatible types', () => {
expect(getCompatibleTypes(['AWS_ACCESS_KEY_ID'])).toEqual([]);
expect(
getCompatibleTypes([
'AWS_ACCESS_KEY_ID',
'AWS_SECRET_ACCESS_KEY',
'AWS_S3_ENDPOINT',
'AWS_S3_BUCKET',
]),
).toEqual([CompatibleTypes.ModelServing]);
expect(
getCompatibleTypes([
'AWS_ACCESS_KEY_ID',
'AWS_SECRET_ACCESS_KEY',
'AWS_S3_ENDPOINT',
'AWS_S3_BUCKET',
'URI',
]),
).toEqual([CompatibleTypes.ModelServing]);
});
});
2 changes: 1 addition & 1 deletion frontend/src/concepts/connectionTypes/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ export type Connection = SecretKind & {
'opendatahub.io/connection-type': string;
};
};
data: {
data?: {
[key: string]: string;
};
};
Expand Down
22 changes: 22 additions & 0 deletions frontend/src/concepts/connectionTypes/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
ConnectionTypeFieldType,
ConnectionTypeFieldTypeUnion,
} from '~/concepts/connectionTypes/types';
import { enumIterator } from '~/utilities/utils';

export const toConnectionTypeConfigMapObj = (
configMap: ConnectionTypeConfigMap,
Expand Down Expand Up @@ -85,3 +86,24 @@ export const fieldNameToEnvVar = (name: string): string => {

const ENV_VAR_NAME_REGEX = new RegExp('^[_a-zA-Z][_a-zA-Z0-9]*$');
export const isValidEnvVar = (name: string): boolean => ENV_VAR_NAME_REGEX.test(name);

export const isModelServingCompatible = (envVars: string[]): boolean =>
['AWS_ACCESS_KEY_ID', 'AWS_SECRET_ACCESS_KEY', 'AWS_S3_ENDPOINT', 'AWS_S3_BUCKET'].every(
(envVar) => envVars.includes(envVar),
);

export enum CompatibleTypes {
ModelServing = 'Model serving',
}

const compatibilities: Record<CompatibleTypes, (envVars: string[]) => boolean> = {
[CompatibleTypes.ModelServing]: isModelServingCompatible,
};

export const getCompatibleTypes = (envVars: string[]): CompatibleTypes[] =>
enumIterator(CompatibleTypes).reduce<CompatibleTypes[]>((acc, [, value]) => {
if (compatibilities[value](envVars)) {
acc.push(value);
}
return acc;
}, []);
30 changes: 26 additions & 4 deletions frontend/src/pages/connectionTypes/ConnectionTypesTableRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ import {
TimestampTooltipVariant,
Truncate,
} from '@patternfly/react-core';
import { ConnectionTypeConfigMapObj } from '~/concepts/connectionTypes/types';
import {
ConnectionTypeConfigMapObj,
isConnectionTypeDataField,
} from '~/concepts/connectionTypes/types';
import { relativeTime } from '~/utilities/time';
import { updateConnectionTypeEnabled } from '~/services/connectionTypesService';
import useNotification from '~/utilities/useNotification';
Expand All @@ -22,6 +25,8 @@ import {
} from '~/concepts/k8s/utils';
import { connectionTypeColumns } from '~/pages/connectionTypes/columns';
import CategoryLabel from '~/concepts/connectionTypes/CategoryLabel';
import { getCompatibleTypes } from '~/concepts/connectionTypes/utils';
import CompatibilityLabel from '~/concepts/connectionTypes/CompatibilityLabel';

type ConnectionTypesTableRowProps = {
obj: ConnectionTypeConfigMapObj;
Expand Down Expand Up @@ -78,6 +83,12 @@ const ConnectionTypesTableRow: React.FC<ConnectionTypesTableRowProps> = ({
setIsEnabled(obj.metadata.annotations?.['opendatahub.io/enabled'] === 'true');
}, [obj.metadata.annotations]);

const compatibleTypes = getCompatibleTypes(
obj.data?.fields
?.filter(isConnectionTypeDataField)
.filter((field) => field.required)
.map((field) => field.envVar) ?? [],
);
return (
<Tr>
<Td dataLabel={connectionTypeColumns[0].label}>
Expand All @@ -99,23 +110,34 @@ const ConnectionTypesTableRow: React.FC<ConnectionTypesTableRowProps> = ({
'-'
)}
</Td>
<Td dataLabel={connectionTypeColumns[2].label} data-testid="connection-type-creator">
<Td dataLabel={connectionTypeColumns[2].label} data-testid="connection-type-compatibility">
{compatibleTypes.length ? (
<LabelGroup>
{compatibleTypes.map((compatibleType) => (
<CompatibilityLabel key={compatibleType} type={compatibleType} />
))}
</LabelGroup>
) : (
'-'
)}
</Td>
<Td dataLabel={connectionTypeColumns[3].label} data-testid="connection-type-creator">
{ownedByDSC(obj) ? (
<Label data-testid="connection-type-user-label">{creator}</Label>
) : (
creator
)}
</Td>
<Td
dataLabel={connectionTypeColumns[3].label}
dataLabel={connectionTypeColumns[4].label}
data-testid="connection-type-created"
modifier="nowrap"
>
<Timestamp date={createdDate} tooltip={{ variant: TimestampTooltipVariant.default }}>
{createdDate ? relativeTime(Date.now(), createdDate.getTime()) : 'Unknown'}
</Timestamp>
</Td>
<Td dataLabel={connectionTypeColumns[4].label}>
<Td dataLabel={connectionTypeColumns[5].label}>
<Switch
isChecked={isEnabled}
aria-label="toggle enabled"
Expand Down
10 changes: 8 additions & 2 deletions frontend/src/pages/connectionTypes/columns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,19 @@ export const connectionTypeColumns: SortableData<ConnectionTypeConfigMapObj>[] =
label: 'Name',
field: 'name',
sortable: sorter,
width: 30,
width: 20,
},
{
label: 'Category',
field: 'category',
sortable: false,
width: 25,
width: 20,
},
{
label: 'Compatibility',
field: 'compatibility',
sortable: false,
width: 15,
},
{
label: 'Creator',
Expand Down
Loading
Loading