Skip to content

Commit

Permalink
Update modals and generate meaningful k8s name for images
Browse files Browse the repository at this point in the history
  • Loading branch information
DaoDaoNoCode committed Jul 27, 2023
1 parent 2c849f3 commit 10f9b48
Show file tree
Hide file tree
Showing 28 changed files with 15,027 additions and 7,276 deletions.
1 change: 1 addition & 0 deletions backend/.eslintignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
package.json
jest.config.js
7 changes: 7 additions & 0 deletions backend/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module.exports = {
transform: {
'node_modules': 'ts-jest',
},
preset: 'ts-jest',
testEnvironment: 'node',
};
20,814 changes: 14,126 additions & 6,688 deletions backend/package-lock.json

Large diffs are not rendered by default.

10 changes: 7 additions & 3 deletions backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@
"build-only": "tsc -p . && node ./dist/server.js --log=1 --registry=localhost:50051 --buildonly",
"build": "npm run build:clean; npm run tsc",
"build:clean": "rimraf ./dist",
"test": "npm run test:lint; npm run test:type-check",
"test": "npm run test:lint; npm run test:type-check; npm run test:jest",
"test:lint": "eslint --max-warnings 0 --ext .json,.js,.ts src/plugins src/routes src/utils",
"test:fix": "eslint --ext .json,.js,.ts src/plugins src/routes src/utils --fix",
"test:type-check": "tsc --noEmit",
"test:jest": "jest",
"server": "NODE_ENV=production node ./dist/server.js",
"tsc": "tsc -p .",
"lint": "eslint ./src/",
Expand Down Expand Up @@ -66,13 +67,16 @@
"typescript": "^4.0.3"
},
"optionalDependencies": {
"@types/jest": "^29.5.3",
"eslint": "^6.8.0",
"eslint-config-esnext": "^4.1.0",
"eslint-config-node": "^4.1.0",
"eslint-config-prettier": "^6.15.0",
"eslint-plugin-babel": "^5.3.1",
"eslint-plugin-import": "^2.22.1",
"eslint-plugin-node": "^11.1.0",
"eslint-plugin-prettier": "^3.4.0"
"eslint-plugin-prettier": "^3.4.0",
"jest": "^29.6.1",
"ts-jest": "^29.1.1"
}
}
}
77 changes: 77 additions & 0 deletions backend/src/__tests__/dockerRepositoryURL.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// https://cloud.google.com/artifact-registry/docs/docker/names
// The full name for a container image is one of the following formats:
// LOCATION-docker.pkg.dev/PROJECT-ID/REPOSITORY/IMAGE
// LOCATION-docker.pkg.dev/PROJECT-ID/REPOSITORY/IMAGE:TAG
// LOCATION-docker.pkg.dev/PROJECT-ID/REPOSITORY/IMAGE@IMAGE-DIGEST

import { parseImageURL } from '../routes/api/images/imageUtils';

test('Invalid URL: space string', () => {
const url = ' ';
const { fullURL, host } = parseImageURL(url);
expect(fullURL).toBe('');
expect(host).toBeUndefined();
});

test('Invalid URL: no match', () => {
const url = '/';
const { host, tag } = parseImageURL(url);
expect(host).toBeUndefined();
expect(tag).toBeUndefined();
});

test('Invalid URL: host only', () => {
const url = 'docker.io';
const { host } = parseImageURL(url);
expect(host).toBe('');
});

test('Invalid URL: host and repo, no image', () => {
const url = 'docker.io/opendatahub';
const { host } = parseImageURL(url);
expect(host).toBe('');
});

test('Valid URL with spaces on both sides', () => {
const url = ' docker.io/library/mysql:test ';
const { fullURL, host, tag } = parseImageURL(url);
expect(fullURL).toBe('docker.io/library/mysql:test');
expect(host).toBe('docker.io');
expect(tag).toBe('test');
});

test('Docker container URL without tag', () => {
const url = 'docker.io/library/mysql';
const { host, tag } = parseImageURL(url);
expect(host).toBe('docker.io');
expect(tag).toBeUndefined();
});

test('Docker container URL with tag', () => {
const url = 'docker.io/library/mysql:test-tag';
const { host, tag } = parseImageURL(url);
expect(host).toBe('docker.io');
expect(tag).toBe('test-tag');
});

test('OpenShift internal registry URL without tag', () => {
const url = 'image-registry.openshift-image-registry.svc:5000/opendatahub/s2i-minimal-notebook';
const { host, tag } = parseImageURL(url);
expect(host).toBe('image-registry.openshift-image-registry.svc:5000');
expect(tag).toBeUndefined();
});

test('OpenShift internal registry URL with tag', () => {
const url =
'image-registry.openshift-image-registry.svc:5000/opendatahub/s2i-minimal-notebook:v0.3.0-py36';
const { host, tag } = parseImageURL(url);
expect(host).toBe('image-registry.openshift-image-registry.svc:5000');
expect(tag).toBe('v0.3.0-py36');
});

test('Quay URL with port and tag', () => {
const url = 'quay.io:443/opendatahub/odh-dashboard:main-55e19fa';
const { host, tag } = parseImageURL(url);
expect(host).toBe('quay.io:443');
expect(tag).toBe('main-55e19fa');
});
73 changes: 58 additions & 15 deletions backend/src/routes/api/images/imageUtils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { IMAGE_ANNOTATIONS, imageUrlRegex } from '../../../utils/constants';
import { IMAGE_ANNOTATIONS } from '../../../utils/constants';
import { convertLabelsToString } from '../../../utils/componentUtils';
import {
ImageStreamTag,
Expand All @@ -13,6 +13,42 @@ import {
import { FastifyRequest } from 'fastify';
import createError from 'http-errors';

const translateDisplayNameForK8s = (name: string): string =>
name
.trim()
.toLowerCase()
.replace(/\s/g, '-')
.replace(/[^A-Za-z0-9-]/g, '');

/**
* This function uses a regex to match the image location string
* The match result will return an array of 4 elements:
* Full URL, host, repo/image and tag(if any)
* @param imageString
*/
export const parseImageURL = (
imageString: string,
): { fullURL: string; host: string; image: string; tag: string } => {
const imageUrlRegex =
/^([\w.\-_]+(?::\d+|)(?=\/[a-z0-9._-]+\/[a-z0-9._-]+)|)(?:\/|)([a-z0-9.\-_]+(?:\/[a-z0-9.\-_]+|))(?::([\w.\-_]{1,127})|)/;
const trimmedString = imageString.trim();
const result = trimmedString.match(imageUrlRegex);
if (!result) {
return {
fullURL: trimmedString,
host: undefined,
image: undefined,
tag: undefined,
};
}
return {
fullURL: result[0],
host: result[1],
image: result[2],
tag: result[3],
};
};

export const getImageList = async (
fastify: KubeFastifyInstance,
labels: { [key: string]: string },
Expand Down Expand Up @@ -189,8 +225,14 @@ const packagesToString = (packages: BYONImagePackage[]): string => {
const mapImageStreamToBYONImage = (is: ImageStream): BYONImage => ({
id: is.metadata.uid,
name: is.metadata.name,
display_name: is.metadata.annotations['opendatahub.io/notebook-image-name'] || is.metadata.name,
description: is.metadata.annotations['opendatahub.io/notebook-image-desc'] || '',
display_name:
is.metadata.annotations['opendatahub.io/notebook-image-name'] ||
is.metadata.annotations['openshift.io/display-name'] ||
is.metadata.name,
description:
is.metadata.annotations['opendatahub.io/notebook-image-desc'] ||
is.metadata.annotations['openshift.io/description'] ||
'',
visible: is.metadata.labels['opendatahub.io/notebook-image'] === 'true',
error: getBYONImageErrorMessage(is),
packages: JSON.parse(
Expand All @@ -209,14 +251,12 @@ export const postImage = async (
const customObjectsApi = fastify.kube.customObjectsApi;
const namespace = fastify.kube.namespace;
const body = request.body as BYONImage;
const fullUrl = body.url;
const matchArray = fullUrl.match(imageUrlRegex);
// check if the host is valid
if (!matchArray[1]) {
const inputURL = body.url;
const { fullURL, host, tag } = parseImageURL(inputURL);
if (!host) {
fastify.log.error('Invalid repository URL unable to add notebook image');
return { success: false, error: 'Invalid repository URL: ' + fullUrl };
return { success: false, error: 'Invalid repository URL: ' + fullURL };
}
const imageTag = matchArray[4];
const labels = {
'app.kubernetes.io/created-by': 'byon',
'opendatahub.io/notebook-image': 'true',
Expand All @@ -227,7 +267,10 @@ export const postImage = async (

if (validName.length > 0) {
fastify.log.error('Duplicate name unable to add notebook image');
return { success: false, error: 'Unable to add notebook image: ' + body.display_name };
return {
success: false,
error: 'Duplicated name. Unable to add notebook image: ' + body.display_name,
};
}

const payload: ImageStream = {
Expand All @@ -237,10 +280,10 @@ export const postImage = async (
annotations: {
'opendatahub.io/notebook-image-desc': body.description || '',
'opendatahub.io/notebook-image-name': body.display_name,
'opendatahub.io/notebook-image-url': fullUrl,
'opendatahub.io/notebook-image-url': fullURL,
'opendatahub.io/notebook-image-creator': body.provider,
},
name: `byon-${Date.now()}`,
name: `custom-${translateDisplayNameForK8s(body.display_name)}`,
namespace: namespace,
labels: labels,
},
Expand All @@ -253,13 +296,13 @@ export const postImage = async (
annotations: {
'opendatahub.io/notebook-software': packagesToString(body.software),
'opendatahub.io/notebook-python-dependencies': packagesToString(body.packages),
'openshift.io/imported-from': fullUrl,
'openshift.io/imported-from': fullURL,
},
from: {
kind: 'DockerImage',
name: fullUrl,
name: fullURL,
},
name: imageTag || 'latest',
name: tag || 'latest',
},
],
},
Expand Down
1 change: 1 addition & 0 deletions backend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,7 @@ export type ODHSegmentKey = {

export type BYONImage = {
id: string;
// FIXME: This shouldn't be a user defined value consumed from the request payload but should be a controlled value from an authentication middleware.
provider: string;
imported_time: string;
error: string;
Expand Down
3 changes: 0 additions & 3 deletions backend/src/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,3 @@ export const DEFAULT_NOTEBOOK_SIZES: NotebookSize[] = [
},
},
];

export const imageUrlRegex =
/^([\w.\-_]+((?::\d+|)(?=\/[a-z0-9._-]+\/[a-z0-9._-]+))|)(?:\/|)([a-z0-9.\-_]+(?:\/[a-z0-9.\-_]+|))(?::([\w.\-_]{1,127})|)/;
49 changes: 0 additions & 49 deletions frontend/src/__tests__/dockerRepositoryURL.spec.ts

This file was deleted.

56 changes: 56 additions & 0 deletions frontend/src/concepts/dashboard/DashboardModalFooter.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import * as React from 'react';
import {
ActionList,
ActionListItem,
Alert,
Button,
Stack,
StackItem,
} from '@patternfly/react-core';

type DashboardModalFooterProps = {
submitLabel: string;
onSubmit: () => void;
onCancel: () => void;
isSubmitDisabled: boolean;
isCancelDisabled: boolean;
alertTitle: string;
error?: Error;
};

const DashboardModalFooter: React.FC<DashboardModalFooterProps> = ({
submitLabel,
onSubmit,
onCancel,
isSubmitDisabled,
isCancelDisabled,
error,
alertTitle,
}) => (
// make sure alert uses the full width
<Stack hasGutter style={{ flex: 'auto' }}>
{error && (
<StackItem>
<Alert isInline variant="danger" title={alertTitle}>
{error.message}
</Alert>
</StackItem>
)}
<StackItem>
<ActionList>
<ActionListItem>
<Button key="submit" variant="primary" isDisabled={isSubmitDisabled} onClick={onSubmit}>
{submitLabel}
</Button>
</ActionListItem>
<ActionListItem>
<Button key="cancel" variant="link" isDisabled={isCancelDisabled} onClick={onCancel}>
Cancel
</Button>
</ActionListItem>
</ActionList>
</StackItem>
</Stack>
);

export default DashboardModalFooter;
Loading

0 comments on commit 10f9b48

Please sign in to comment.