From 2fb07c402653dc9e9ce2b98c1af7a841da6e3f51 Mon Sep 17 00:00:00 2001 From: Humair Khan Date: Thu, 9 May 2024 08:54:19 -0400 Subject: [PATCH] chore: prettier fixes Signed-off-by: Humair Khan --- frontend/server/handlers/artifacts.ts | 224 ++++++++------- .../integration-tests/artifact-get.test.ts | 258 ++++++++++-------- frontend/server/minio-helper.test.ts | 35 ++- frontend/server/minio-helper.ts | 91 +++--- frontend/server/workflow-helper.ts | 20 +- frontend/src/components/ArtifactPreview.tsx | 15 +- .../src/components/MinioArtifactPreview.tsx | 2 +- .../src/components/tabs/InputOutputTab.tsx | 35 ++- .../components/tabs/RuntimeNodeDetailsV2.tsx | 10 +- .../viewers/MetricsVisualizations.tsx | 4 +- frontend/src/lib/Apis.test.ts | 6 +- frontend/src/lib/Apis.ts | 9 +- frontend/src/lib/OutputArtifactLoader.test.ts | 2 +- frontend/src/lib/OutputArtifactLoader.ts | 7 +- frontend/src/mlmd/MlmdUtils.ts | 7 +- 15 files changed, 407 insertions(+), 318 deletions(-) diff --git a/frontend/server/handlers/artifacts.ts b/frontend/server/handlers/artifacts.ts index 5ccbbe6e36bc..d5bea2cbf6ae 100644 --- a/frontend/server/handlers/artifacts.ts +++ b/frontend/server/handlers/artifacts.ts @@ -15,7 +15,7 @@ import fetch from 'node-fetch'; import { AWSConfigs, HttpConfigs, MinioConfigs, ProcessEnv } from '../configs'; import { Client as MinioClient } from 'minio'; import { PreviewStream, findFileOnPodVolume, parseJSONString } from '../utils'; -import {createMinioClient, getObjectStream} from '../minio-helper'; +import { createMinioClient, getObjectStream } from '../minio-helper'; import * as serverInfo from '../helpers/server-info'; import { Handler, Request, Response } from 'express'; import { Storage } from '@google-cloud/storage'; @@ -24,9 +24,9 @@ import { HACK_FIX_HPM_PARTIAL_RESPONSE_HEADERS } from '../consts'; import * as fs from 'fs'; import { isAllowedDomain } from './domain-checker'; -import { getK8sSecret } from "../k8s-helper"; -import { StorageOptions } from "@google-cloud/storage/build/src/storage"; -import { CredentialBody } from "google-auth-library/build/src/auth/credentials"; +import { getK8sSecret } from '../k8s-helper'; +import { StorageOptions } from '@google-cloud/storage/build/src/storage'; +import { CredentialBody } from 'google-auth-library/build/src/auth/credentials'; /** * ArtifactsQueryStrings describes the expected query strings key value pairs @@ -77,10 +77,10 @@ export interface GCSProviderInfo { * @param tryExtract whether the handler try to extract content from *.tar.gz files. */ export function getArtifactsHandler({ - artifactsConfigs, - useParameter, - tryExtract, - }: { + artifactsConfigs, + useParameter, + tryExtract, +}: { artifactsConfigs: { aws: AWSConfigs; http: HttpConfigs; @@ -95,7 +95,9 @@ export function getArtifactsHandler({ const source = useParameter ? req.params.source : req.query.source; const bucket = useParameter ? req.params.bucket : req.query.bucket; const key = useParameter ? req.params[0] : req.query.key; - const { peek = 0, providerInfo = "", namespace = ""} = req.query as Partial; + const { peek = 0, providerInfo = '', namespace = '' } = req.query as Partial< + ArtifactsQueryStrings + >; if (!source) { res.status(500).send('Storage source is missing from artifact request'); return; @@ -110,10 +112,10 @@ export function getArtifactsHandler({ } console.log(`Getting storage artifact at: ${source}: ${bucket}/${key}`); - let client : MinioClient; + let client: MinioClient; switch (source) { case 'gcs': - await getGCSArtifactHandler({bucket, key}, peek, providerInfo, namespace)(req, res); + await getGCSArtifactHandler({ bucket, key }, peek, providerInfo, namespace)(req, res); break; case 'minio': try { @@ -123,13 +125,13 @@ export function getArtifactsHandler({ return; } await getMinioArtifactHandler( - { - bucket, - client, - key, - tryExtract, - }, - peek, + { + bucket, + client, + key, + tryExtract, + }, + peek, )(req, res); break; case 's3': @@ -140,30 +142,30 @@ export function getArtifactsHandler({ return; } await getMinioArtifactHandler( - { - bucket, - client, - key, - }, - peek, + { + bucket, + client, + key, + }, + peek, )(req, res); break; case 'http': case 'https': await getHttpArtifactsHandler( - allowedDomain, - getHttpUrl(source, http.baseUrl || '', bucket, key), - http.auth, - peek, + allowedDomain, + getHttpUrl(source, http.baseUrl || '', bucket, key), + http.auth, + peek, )(req, res); break; case 'volume': await getVolumeArtifactsHandler( - { - bucket, - key, - }, - peek, + { + bucket, + key, + }, + peek, )(req, res); break; default: @@ -202,7 +204,7 @@ function getHttpArtifactsHandler( if (auth.key.length > 0) { // inject original request's value if exists, otherwise default to provided default value headers[auth.key] = - req.headers[auth.key] || req.headers[auth.key.toLowerCase()] || auth.defaultValue; + req.headers[auth.key] || req.headers[auth.key.toLowerCase()] || auth.defaultValue; } if (!isAllowedDomain(url, allowedDomain)) { res.status(500).send(`Domain not allowed.`); @@ -210,9 +212,9 @@ function getHttpArtifactsHandler( } const response = await fetch(url, { headers }); response.body - .on('error', err => res.status(500).send(`Unable to retrieve artifact: ${err}`)) - .pipe(new PreviewStream({ peek })) - .pipe(res); + .on('error', err => res.status(500).send(`Unable to retrieve artifact: ${err}`)) + .pipe(new PreviewStream({ peek })) + .pipe(res); }; } @@ -234,31 +236,45 @@ function getMinioArtifactHandler( }; } -async function parseGCSProviderInfo(providerInfo: GCSProviderInfo, namespace: string): Promise { +async function parseGCSProviderInfo( + providerInfo: GCSProviderInfo, + namespace: string, +): Promise { if (!providerInfo.Params.tokenKey || !providerInfo.Params.secretName) { - throw new Error('Provider info with fromEnv:false supplied with incomplete secret credential info.'); + throw new Error( + 'Provider info with fromEnv:false supplied with incomplete secret credential info.', + ); } let configGCS: StorageOptions; try { - const tokenString = await getK8sSecret(providerInfo.Params.secretName, providerInfo.Params.tokenKey, namespace); + const tokenString = await getK8sSecret( + providerInfo.Params.secretName, + providerInfo.Params.tokenKey, + namespace, + ); const credentials = parseJSONString(tokenString); - configGCS = {credentials}; - configGCS.scopes = "https://www.googleapis.com/auth/devstorage.read_write"; + configGCS = { credentials }; + configGCS.scopes = 'https://www.googleapis.com/auth/devstorage.read_write'; return configGCS; } catch (err) { throw new Error('Failed to parse GCS Provider config. Error: ' + err); } } -function getGCSArtifactHandler(options: { key: string; bucket: string }, peek: number = 0, providerInfoString?: string, namespace?: string) { +function getGCSArtifactHandler( + options: { key: string; bucket: string }, + peek: number = 0, + providerInfoString?: string, + namespace?: string, +) { const { key, bucket } = options; return async (_: Request, res: Response) => { try { - let storageOptions : StorageOptions | undefined; - if(providerInfoString) { - const providerInfo = parseJSONString(providerInfoString); - if (providerInfo && providerInfo.Params.fromEnv === "false") { - if (!namespace){ + let storageOptions: StorageOptions | undefined; + if (providerInfoString) { + const providerInfo = parseJSONString(providerInfoString); + if (providerInfo && providerInfo.Params.fromEnv === 'false') { + if (!namespace) { res.status(500).send('Failed to parse provider info. Reason: No namespace provided'); } else { storageOptions = await parseGCSProviderInfo(providerInfo, namespace); @@ -279,11 +295,11 @@ function getGCSArtifactHandler(options: { key: string; bucket: string }, peek: n // Build a RegExp object that only recognizes asterisks ('*'), and // escapes everything else. const regex = new RegExp( - '^' + + '^' + key - .split(/\*+/) - .map(escapeRegexChars) - .join('.*') + + .split(/\*+/) + .map(escapeRegexChars) + .join('.*') + '$', ); return regex.test(f.name); @@ -295,16 +311,16 @@ function getGCSArtifactHandler(options: { key: string; bucket: string }, peek: n return; } console.log( - `Found ${matchingFiles.length} matching files: `, - matchingFiles.map(file => file.name).join(','), + `Found ${matchingFiles.length} matching files: `, + matchingFiles.map(file => file.name).join(','), ); let contents = ''; // TODO: support peek for concatenated matching files if (peek) { matchingFiles[0] - .createReadStream() - .pipe(new PreviewStream({ peek })) - .pipe(res); + .createReadStream() + .pipe(new PreviewStream({ peek })) + .pipe(res); return; } @@ -312,17 +328,17 @@ function getGCSArtifactHandler(options: { key: string; bucket: string }, peek: n matchingFiles.forEach((f, i) => { const buffer: Buffer[] = []; f.createReadStream() - .on('data', data => buffer.push(Buffer.from(data))) - .on('end', () => { - contents += - Buffer.concat(buffer) - .toString() - .trim() + '\n'; - if (i === matchingFiles.length - 1) { - res.send(contents); - } - }) - .on('error', () => res.status(500).send('Failed to read file: ' + f.name)); + .on('data', data => buffer.push(Buffer.from(data))) + .on('end', () => { + contents += + Buffer.concat(buffer) + .toString() + .trim() + '\n'; + if (i === matchingFiles.length - 1) { + res.send(contents); + } + }) + .on('error', () => res.status(500).send('Failed to read file: ' + f.name)); }); } catch (err) { res.status(500).send('Failed to download GCS file(s). Error: ' + err); @@ -362,14 +378,14 @@ function getVolumeArtifactsHandler(options: { bucket: string; key: string }, pee const stat = await fs.promises.stat(filePath); if (stat.isDirectory()) { res - .status(400) - .send(`Failed to open volume file ${filePath} is directory, does not support now`); + .status(400) + .send(`Failed to open volume file ${filePath} is directory, does not support now`); return; } fs.createReadStream(filePath) - .pipe(new PreviewStream({ peek })) - .pipe(res); + .pipe(new PreviewStream({ peek })) + .pipe(res); } catch (err) { console.log(`Failed to open volume: ${err}`); res.status(500).send(`Failed to open volume.`); @@ -405,10 +421,10 @@ const QUERIES = { }; export function getArtifactsProxyHandler({ - enabled, - allowedDomain, - namespacedServiceGetter, - }: { + enabled, + allowedDomain, + namespacedServiceGetter, +}: { enabled: boolean; allowedDomain: string; namespacedServiceGetter: NamespacedServiceGetter; @@ -417,36 +433,36 @@ export function getArtifactsProxyHandler({ return (req, res, next) => next(); } return proxy( - (_pathname, req) => { - // only proxy requests with namespace query parameter - return !!getNamespaceFromUrl(req.url || ''); + (_pathname, req) => { + // only proxy requests with namespace query parameter + return !!getNamespaceFromUrl(req.url || ''); + }, + { + changeOrigin: true, + onProxyReq: proxyReq => { + console.log('Proxied artifact request: ', proxyReq.path); }, - { - changeOrigin: true, - onProxyReq: proxyReq => { - console.log('Proxied artifact request: ', proxyReq.path); - }, - pathRewrite: (pathStr, req) => { - const url = new URL(pathStr || '', DUMMY_BASE_PATH); - url.searchParams.delete(QUERIES.NAMESPACE); - return url.pathname + url.search; - }, - router: req => { - const namespace = getNamespaceFromUrl(req.url || ''); - if (!namespace) { - console.log(`namespace query param expected in ${req.url}.`); - throw new Error(`namespace query param expected.`); - } - const urlStr = namespacedServiceGetter(namespace!); - if (!isAllowedDomain(urlStr, allowedDomain)) { - console.log(`Domain is not allowed.`); - throw new Error(`Domain is not allowed.`); - } - return namespacedServiceGetter(namespace!); - }, - target: '/artifacts', - headers: HACK_FIX_HPM_PARTIAL_RESPONSE_HEADERS, + pathRewrite: (pathStr, req) => { + const url = new URL(pathStr || '', DUMMY_BASE_PATH); + url.searchParams.delete(QUERIES.NAMESPACE); + return url.pathname + url.search; + }, + router: req => { + const namespace = getNamespaceFromUrl(req.url || ''); + if (!namespace) { + console.log(`namespace query param expected in ${req.url}.`); + throw new Error(`namespace query param expected.`); + } + const urlStr = namespacedServiceGetter(namespace!); + if (!isAllowedDomain(urlStr, allowedDomain)) { + console.log(`Domain is not allowed.`); + throw new Error(`Domain is not allowed.`); + } + return namespacedServiceGetter(namespace!); }, + target: '/artifacts', + headers: HACK_FIX_HPM_PARTIAL_RESPONSE_HEADERS, + }, ); } diff --git a/frontend/server/integration-tests/artifact-get.test.ts b/frontend/server/integration-tests/artifact-get.test.ts index 87aa3bfe34f3..d104e1a8e635 100644 --- a/frontend/server/integration-tests/artifact-get.test.ts +++ b/frontend/server/integration-tests/artifact-get.test.ts @@ -23,7 +23,7 @@ import { UIServer } from '../app'; import { loadConfigs } from '../configs'; import * as serverInfo from '../helpers/server-info'; import { commonSetup, mkTempDir } from './test-helper'; -import {getK8sSecret} from "../k8s-helper"; +import { getK8sSecret } from '../k8s-helper'; const MinioClient = minio.Client; jest.mock('minio'); @@ -100,17 +100,17 @@ describe('/artifacts', () => { process.env.AWS_SECRET_ACCESS_KEY = 'awsSecret123'; const request = requests(app.start()); request - .get('/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt') - .expect(200, artifactContent, err => { - expect(mockedMinioClient).toBeCalledWith({ - accessKey: 'aws123', - endPoint: 's3.amazonaws.com', - region: 'us-east-1', - secretKey: 'awsSecret123', - useSSL: true, - }); - done(err); + .get('/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt') + .expect(200, artifactContent, err => { + expect(mockedMinioClient).toBeCalledWith({ + accessKey: 'aws123', + endPoint: 's3.amazonaws.com', + region: 'us-east-1', + secretKey: 'awsSecret123', + useSSL: true, }); + done(err); + }); }); it('responds with artifact if source is AWS S3, and creds are sourced from Load Configs', done => { @@ -145,39 +145,43 @@ describe('/artifacts', () => { app = new UIServer(configs); const request = requests(app.start()); const providerInfo = { - "Params": { - "accessKeyKey": "someSecret", + Params: { + accessKeyKey: 'someSecret', // this not set and default is used (tls=true) // since aws connections are always tls secured - "disableSSL": "false", - "endpoint": "s3.amazonaws.com", - "fromEnv": "false", + disableSSL: 'false', + endpoint: 's3.amazonaws.com', + fromEnv: 'false', // this not set and default is used // since aws connections always have the same port - "port": "0001", - "region": "us-east-2", - "secretKeyKey": "someSecret", - "secretName": "aws-s3-creds" + port: '0001', + region: 'us-east-2', + secretKeyKey: 'someSecret', + secretName: 'aws-s3-creds', }, - "Provider": "s3", + Provider: 's3', }; - const namespace = "test"; + const namespace = 'test'; request - .get(`/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=${namespace}&providerInfo=${JSON.stringify(providerInfo)}`) - .expect(200, artifactContent, err => { - expect(mockedMinioClient).toBeCalledWith({ - accessKey: 'someSecret', - endPoint: 's3.amazonaws.com', - port: undefined, - region: 'us-east-2', - secretKey: 'someSecret', - useSSL: undefined, - }); - expect(mockedMinioClient).toBeCalledTimes(1); - expect(mockedGetK8sSecret).toBeCalledWith("aws-s3-creds", "someSecret", `${namespace}`); - expect(mockedGetK8sSecret).toBeCalledTimes(2); - done(err); + .get( + `/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=${namespace}&providerInfo=${JSON.stringify( + providerInfo, + )}`, + ) + .expect(200, artifactContent, err => { + expect(mockedMinioClient).toBeCalledWith({ + accessKey: 'someSecret', + endPoint: 's3.amazonaws.com', + port: undefined, + region: 'us-east-2', + secretKey: 'someSecret', + useSSL: undefined, }); + expect(mockedMinioClient).toBeCalledTimes(1); + expect(mockedGetK8sSecret).toBeCalledWith('aws-s3-creds', 'someSecret', `${namespace}`); + expect(mockedGetK8sSecret).toBeCalledTimes(2); + done(err); + }); }); it('responds error when source is s3, and creds are sourced from Provider Configs, but no namespace is provided', done => { @@ -186,23 +190,31 @@ describe('/artifacts', () => { app = new UIServer(configs); const request = requests(app.start()); const providerInfo = { - "Params": { - "accessKeyKey": "AWS_ACCESS_KEY_ID", - "disableSSL": "false", - "endpoint": "s3.amazonaws.com", - "fromEnv": "false", - "region": "us-east-2", - "secretKeyKey": "AWS_SECRET_ACCESS_KEY", - "secretName": "aws-s3-creds" + Params: { + accessKeyKey: 'AWS_ACCESS_KEY_ID', + disableSSL: 'false', + endpoint: 's3.amazonaws.com', + fromEnv: 'false', + region: 'us-east-2', + secretKeyKey: 'AWS_SECRET_ACCESS_KEY', + secretName: 'aws-s3-creds', }, - "Provider": "s3", + Provider: 's3', }; request - .get(`/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt$&providerInfo=${JSON.stringify(providerInfo)}`) - .expect(500, 'Failed to initialize Minio Client for S3 Provider: Error: Artifact Store provider given, but no namespace provided.', err => { + .get( + `/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt$&providerInfo=${JSON.stringify( + providerInfo, + )}`, + ) + .expect( + 500, + 'Failed to initialize Minio Client for S3 Provider: Error: Artifact Store provider given, but no namespace provided.', + err => { expect(mockedGetK8sSecret).toBeCalledTimes(0); done(err); - }); + }, + ); }); it('responds with artifact if source is s3-compatible, and creds are sourced from Provider Configs', done => { @@ -213,34 +225,38 @@ describe('/artifacts', () => { app = new UIServer(configs); const request = requests(app.start()); const providerInfo = { - "Params": { - "accessKeyKey": "someSecret", - "disableSSL": "false", - "endpoint": "https://mys3.com", - "fromEnv": "false", - "region": "auto", - "secretKeyKey": "someSecret", - "secretName": "my-secret" + Params: { + accessKeyKey: 'someSecret', + disableSSL: 'false', + endpoint: 'https://mys3.com', + fromEnv: 'false', + region: 'auto', + secretKeyKey: 'someSecret', + secretName: 'my-secret', }, - "Provider": "s3", + Provider: 's3', }; - const namespace = "test"; + const namespace = 'test'; request - .get(`/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=${namespace}&providerInfo=${JSON.stringify(providerInfo)}`) - .expect(200, artifactContent, err => { - expect(mockedMinioClient).toBeCalledWith({ - accessKey: 'someSecret', - endPoint: 'mys3.com', - port: undefined, - region: 'auto', - secretKey: 'someSecret', - useSSL: true, - }); - expect(mockedMinioClient).toBeCalledTimes(1); - expect(mockedGetK8sSecret).toBeCalledWith("my-secret", "someSecret", `${namespace}`); - expect(mockedGetK8sSecret).toBeCalledTimes(2); - done(err); + .get( + `/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=${namespace}&providerInfo=${JSON.stringify( + providerInfo, + )}`, + ) + .expect(200, artifactContent, err => { + expect(mockedMinioClient).toBeCalledWith({ + accessKey: 'someSecret', + endPoint: 'mys3.com', + port: undefined, + region: 'auto', + secretKey: 'someSecret', + useSSL: true, }); + expect(mockedMinioClient).toBeCalledTimes(1); + expect(mockedGetK8sSecret).toBeCalledWith('my-secret', 'someSecret', `${namespace}`); + expect(mockedGetK8sSecret).toBeCalledTimes(2); + done(err); + }); }); it('responds with artifact if source is s3-compatible, and creds are sourced from Provider Configs, with endpoint port', done => { @@ -251,34 +267,38 @@ describe('/artifacts', () => { app = new UIServer(configs); const request = requests(app.start()); const providerInfo = { - "Params": { - "accessKeyKey": "someSecret", - "disableSSL": "false", - "endpoint": "https://mys3.ns.svc.cluster.local:1234", - "fromEnv": "false", - "region": "auto", - "secretKeyKey": "someSecret", - "secretName": "my-secret" + Params: { + accessKeyKey: 'someSecret', + disableSSL: 'false', + endpoint: 'https://mys3.ns.svc.cluster.local:1234', + fromEnv: 'false', + region: 'auto', + secretKeyKey: 'someSecret', + secretName: 'my-secret', }, - "Provider": "s3", + Provider: 's3', }; - const namespace = "test"; + const namespace = 'test'; request - .get(`/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=${namespace}&providerInfo=${JSON.stringify(providerInfo)}`) - .expect(200, artifactContent, err => { - expect(mockedMinioClient).toBeCalledWith({ - accessKey: 'someSecret', - endPoint: 'mys3.ns.svc.cluster.local', - port: 1234, - region: 'auto', - secretKey: 'someSecret', - useSSL: true, - }); - expect(mockedMinioClient).toBeCalledTimes(1); - expect(mockedGetK8sSecret).toBeCalledWith("my-secret", "someSecret", `${namespace}`); - expect(mockedGetK8sSecret).toBeCalledTimes(2); - done(err); + .get( + `/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=${namespace}&providerInfo=${JSON.stringify( + providerInfo, + )}`, + ) + .expect(200, artifactContent, err => { + expect(mockedMinioClient).toBeCalledWith({ + accessKey: 'someSecret', + endPoint: 'mys3.ns.svc.cluster.local', + port: 1234, + region: 'auto', + secretKey: 'someSecret', + useSSL: true, }); + expect(mockedMinioClient).toBeCalledTimes(1); + expect(mockedGetK8sSecret).toBeCalledWith('my-secret', 'someSecret', `${namespace}`); + expect(mockedGetK8sSecret).toBeCalledTimes(2); + done(err); + }); }); it('responds with artifact if source is gcs, and creds are sourced from Provider Configs', done => { @@ -292,36 +312,40 @@ describe('/artifacts', () => { mockedGcsStorage.mockImplementationOnce(() => ({ bucket: () => ({ getFiles: () => - Promise.resolve([[{ name: 'hello/world.txt', createReadStream: () => stream }]]), + Promise.resolve([[{ name: 'hello/world.txt', createReadStream: () => stream }]]), }), })); const configs = loadConfigs(argv, {}); app = new UIServer(configs); const request = requests(app.start()); const providerInfo = { - "Params": { - "fromEnv": "false", - "secretName": "someSecret", - "tokenKey": 'somekey' + Params: { + fromEnv: 'false', + secretName: 'someSecret', + tokenKey: 'somekey', }, - "Provider": "gs", + Provider: 'gs', }; - const namespace = "test"; + const namespace = 'test'; request - .get(`/artifacts/get?source=gcs&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=${namespace}&providerInfo=${JSON.stringify(providerInfo)}`) - .expect(200, artifactContent + '\n', err => { - const expectedArg = { - "credentials": { - "client_email": "testemail", - "private_key": "testkey", - }, - "scopes": "https://www.googleapis.com/auth/devstorage.read_write" - }; - expect(mockedGcsStorage).toBeCalledWith(expectedArg); - expect(mockedGetK8sSecret).toBeCalledWith("someSecret", "somekey", `${namespace}`); - expect(mockedGetK8sSecret).toBeCalledTimes(1); - done(err); - }); + .get( + `/artifacts/get?source=gcs&bucket=ml-pipeline&key=hello%2Fworld.txt&namespace=${namespace}&providerInfo=${JSON.stringify( + providerInfo, + )}`, + ) + .expect(200, artifactContent + '\n', err => { + const expectedArg = { + credentials: { + client_email: 'testemail', + private_key: 'testkey', + }, + scopes: 'https://www.googleapis.com/auth/devstorage.read_write', + }; + expect(mockedGcsStorage).toBeCalledWith(expectedArg); + expect(mockedGetK8sSecret).toBeCalledWith('someSecret', 'somekey', `${namespace}`); + expect(mockedGetK8sSecret).toBeCalledTimes(1); + done(err); + }); }); it('responds with partial s3 artifact if peek=5 flag is set', done => { diff --git a/frontend/server/minio-helper.test.ts b/frontend/server/minio-helper.test.ts index 3e139d71658a..554c81020e56 100644 --- a/frontend/server/minio-helper.test.ts +++ b/frontend/server/minio-helper.test.ts @@ -30,11 +30,14 @@ describe('minio-helper', () => { describe('createMinioClient', () => { it('creates a minio client with the provided configs.', async () => { - const client = await createMinioClient({ - accessKey: 'accesskey', - endPoint: 'minio.kubeflow:80', - secretKey: 'secretkey', - }, 's3'); + const client = await createMinioClient( + { + accessKey: 'accesskey', + endPoint: 'minio.kubeflow:80', + secretKey: 'secretkey', + }, + 's3', + ); expect(client).toBeInstanceOf(MinioClient); expect(MockedMinioClient).toHaveBeenCalledWith({ @@ -45,9 +48,12 @@ describe('minio-helper', () => { }); it('fallbacks to the provided configs if EC2 metadata is not available.', async () => { - const client = await createMinioClient({ - endPoint: 'minio.kubeflow:80', - }, 's3'); + const client = await createMinioClient( + { + endPoint: 'minio.kubeflow:80', + }, + 's3', + ); expect(client).toBeInstanceOf(MinioClient); expect(MockedMinioClient).toHaveBeenCalledWith({ @@ -56,13 +62,12 @@ describe('minio-helper', () => { }); it('uses EC2 metadata credentials if access key are not provided.', async () => { - (fromNodeProviderChain as jest.Mock).mockImplementation(() => - () => - Promise.resolve({ - accessKeyId: 'AccessKeyId', - secretAccessKey: 'SecretAccessKey', - sessionToken: 'SessionToken', - }) + (fromNodeProviderChain as jest.Mock).mockImplementation(() => () => + Promise.resolve({ + accessKeyId: 'AccessKeyId', + secretAccessKey: 'SecretAccessKey', + sessionToken: 'SessionToken', + }), ); const client = await createMinioClient({ endPoint: 's3.amazonaws.com' }, 's3'); expect(client).toBeInstanceOf(MinioClient); diff --git a/frontend/server/minio-helper.ts b/frontend/server/minio-helper.ts index fae0da14c3a5..5f244800f2a9 100644 --- a/frontend/server/minio-helper.ts +++ b/frontend/server/minio-helper.ts @@ -19,9 +19,9 @@ import gunzip from 'gunzip-maybe'; import { URL } from 'url'; import { Client as MinioClient, ClientOptions as MinioClientOptions } from 'minio'; import { isAWSS3Endpoint } from './aws-helper'; -import { S3ProviderInfo } from "./handlers/artifacts"; -import { getK8sSecret } from "./k8s-helper"; -import { parseJSONString } from "./utils"; +import { S3ProviderInfo } from './handlers/artifacts'; +import { getK8sSecret } from './k8s-helper'; +import { parseJSONString } from './utils'; const { fromNodeProviderChain } = require('@aws-sdk/credential-providers'); /** MinioRequestConfig describes the info required to retrieve an artifact. */ export interface MinioRequestConfig { @@ -56,16 +56,21 @@ export interface MinioClientOptionsWithOptionalSecrets extends Partial(providerInfoString); + const providerInfo = parseJSONString(providerInfoString); if (!providerInfo) { - throw new Error("Failed to parse provider info."); + throw new Error('Failed to parse provider info.'); } // If fromEnv == false, we rely on the default credentials or env to provide credentials (e.g. IRSA) - if (providerInfo.Params.fromEnv === "false") { - if (!namespace){ - throw new Error("Artifact Store provider given, but no namespace provided."); + if (providerInfo.Params.fromEnv === 'false') { + if (!namespace) { + throw new Error('Artifact Store provider given, but no namespace provided.'); } else { config = await parseS3ProviderInfo(config, providerInfo, namespace); } @@ -73,7 +78,7 @@ export async function createMinioClient(config: MinioClientOptionsWithOptionalSe } // If using s3 and sourcing credentials from environment (currently only aws is supported) - if (providerType === "s3" && (!config.accessKey || !config.secretKey)) { + if (providerType === 's3' && (!config.accessKey || !config.secretKey)) { // AWS S3 with credentials from provider chain if (isAWSS3Endpoint(config.endPoint)) { try { @@ -91,12 +96,14 @@ export async function createMinioClient(config: MinioClientOptionsWithOptionalSe console.error('Unable to get aws instance profile credentials: ', e); } } else { - console.error('Encountered S3-compatible provider type with no provided credentials, and unsupported environment based credential support.'); + console.error( + 'Encountered S3-compatible provider type with no provided credentials, and unsupported environment based credential support.', + ); } } // If using any AWS or S3 compatible store (e.g. minio, aws s3 when using manual creds, ceph, etc.) - let mc : MinioClient; + let mc: MinioClient; try { mc = await new MinioClient(config as MinioClientOptions); } catch (err) { @@ -106,21 +113,41 @@ export async function createMinioClient(config: MinioClientOptionsWithOptionalSe } // Parse provider info for any s3 compatible store that's not AWS S3 -async function parseS3ProviderInfo(config: MinioClientOptionsWithOptionalSecrets, providerInfo: S3ProviderInfo, namespace: string) : Promise { - if (!providerInfo.Params.accessKeyKey || !providerInfo.Params.secretKeyKey || !providerInfo.Params.secretName) { - throw new Error('Provider info with fromEnv:false supplied with incomplete secret credential info.'); +async function parseS3ProviderInfo( + config: MinioClientOptionsWithOptionalSecrets, + providerInfo: S3ProviderInfo, + namespace: string, +): Promise { + if ( + !providerInfo.Params.accessKeyKey || + !providerInfo.Params.secretKeyKey || + !providerInfo.Params.secretName + ) { + throw new Error( + 'Provider info with fromEnv:false supplied with incomplete secret credential info.', + ); } try { - config.accessKey = await getK8sSecret(providerInfo.Params.secretName, providerInfo.Params.accessKeyKey, namespace); - config.secretKey = await getK8sSecret(providerInfo.Params.secretName, providerInfo.Params.secretKeyKey, namespace); + config.accessKey = await getK8sSecret( + providerInfo.Params.secretName, + providerInfo.Params.accessKeyKey, + namespace, + ); + config.secretKey = await getK8sSecret( + providerInfo.Params.secretName, + providerInfo.Params.secretKeyKey, + namespace, + ); } catch (e) { - throw new Error(`Encountered error when trying to fetch provider secret ${providerInfo.Params.secretName}.`); + throw new Error( + `Encountered error when trying to fetch provider secret ${providerInfo.Params.secretName}.`, + ); } if (isAWSS3Endpoint(providerInfo.Params.endpoint)) { if (providerInfo.Params.endpoint) { - if(providerInfo.Params.endpoint.startsWith("https")){ + if (providerInfo.Params.endpoint.startsWith('https')) { const parseEndpoint = new URL(providerInfo.Params.endpoint); config.endPoint = parseEndpoint.hostname; } else { @@ -153,7 +180,7 @@ async function parseS3ProviderInfo(config: MinioClientOptionsWithOptionalSecrets config.region = providerInfo.Params.region ? providerInfo.Params.region : undefined; if (providerInfo.Params.disableSSL) { - config.useSSL = !(providerInfo.Params.disableSSL.toLowerCase() === "true"); + config.useSSL = !(providerInfo.Params.disableSSL.toLowerCase() === 'true'); } else { config.useSSL = undefined; } @@ -178,8 +205,8 @@ export function isTarball(buf: Buffer) { const v0 = [0x75, 0x73, 0x74, 0x61, 0x72, 0x20, 0x20, 0x00]; return ( - v1.reduce((res, curr, i) => res && curr === buf[offset + i], true) || - v0.reduce((res, curr, i) => res && curr === buf[offset + i], true as boolean) + v1.reduce((res, curr, i) => res && curr === buf[offset + i], true) || + v0.reduce((res, curr, i) => res && curr === buf[offset + i], true as boolean) ); } @@ -189,11 +216,11 @@ export function isTarball(buf: Buffer) { */ export function maybeTarball(): Transform { return peek( - { newline: false, maxBuffer: 264 }, - (data: Buffer, swap: (error?: Error, parser?: Transform) => void) => { - if (isTarball(data)) swap(undefined, extractFirstTarRecordAsStream()); - else swap(undefined, new PassThrough()); - }, + { newline: false, maxBuffer: 264 }, + (data: Buffer, swap: (error?: Error, parser?: Transform) => void) => { + if (isTarball(data)) swap(undefined, extractFirstTarRecordAsStream()); + else swap(undefined, new PassThrough()); + }, ); } @@ -235,11 +262,11 @@ function extractFirstTarRecordAsStream() { * */ export async function getObjectStream({ - bucket, - key, - client, - tryExtract = true, - }: MinioRequestConfig): Promise { + bucket, + key, + client, + tryExtract = true, +}: MinioRequestConfig): Promise { const stream = await client.getObject(bucket, key); return tryExtract ? stream.pipe(gunzip()).pipe(maybeTarball()) : stream.pipe(new PassThrough()); } diff --git a/frontend/server/workflow-helper.ts b/frontend/server/workflow-helper.ts index 2acefbf181fa..d6ad124684be 100644 --- a/frontend/server/workflow-helper.ts +++ b/frontend/server/workflow-helper.ts @@ -138,7 +138,7 @@ export function createPodLogsMinioRequestConfig( // different bucket/prefix for diff namespace? return async (podName: string, _namespace?: string): Promise => { // create a new client each time to ensure session token has not expired - const client = await createMinioClient(minioOptions, "s3"); + const client = await createMinioClient(minioOptions, 's3'); const workflowName = workflowNameFromPodName(podName); return { bucket, @@ -190,14 +190,16 @@ export async function getPodLogsMinioRequestConfigfromWorkflow( const { host, port } = urlSplit(s3Artifact.endpoint, s3Artifact.insecure); const { accessKey, secretKey } = await getMinioClientSecrets(s3Artifact); - - const client = await createMinioClient({ - accessKey, - endPoint: host, - port, - secretKey, - useSSL: !s3Artifact.insecure, - }, "s3"); + const client = await createMinioClient( + { + accessKey, + endPoint: host, + port, + secretKey, + useSSL: !s3Artifact.insecure, + }, + 's3', + ); return { bucket: s3Artifact.bucket, client, diff --git a/frontend/src/components/ArtifactPreview.tsx b/frontend/src/components/ArtifactPreview.tsx index 43deb93338c1..1799c1b9f4f3 100644 --- a/frontend/src/components/ArtifactPreview.tsx +++ b/frontend/src/components/ArtifactPreview.tsx @@ -24,7 +24,7 @@ import { stylesheet } from 'typestyle'; import Banner from './Banner'; import { ValueComponentProps } from './DetailsTable'; import { logger } from 'src/lib/Utils'; -import { URIToSessionInfo } from "./tabs/InputOutputTab"; +import { URIToSessionInfo } from './tabs/InputOutputTab'; const css = stylesheet({ root: { @@ -66,12 +66,12 @@ const ArtifactPreview: React.FC = ({ maxbytes = 255, maxlines = 20, }) => { - let storage: StoragePath | undefined - let providerInfo: string | undefined + let storage: StoragePath | undefined; + let providerInfo: string | undefined; if (value) { try { - providerInfo = sessionMap?.get(value) + providerInfo = sessionMap?.get(value); storage = WorkflowParser.parseStoragePath(value); } catch (error) { logger.error(error); @@ -140,7 +140,12 @@ async function getPreview( return ``; } // TODO how to handle binary data (can probably use magic number to id common mime types) - let data = await Apis.readFile({path: storagePath, providerInfo: providerInfo, namespace: namespace, peek: maxbytes +1}); + let data = await Apis.readFile({ + path: storagePath, + providerInfo: providerInfo, + namespace: namespace, + peek: maxbytes + 1, + }); // is preview === data and no maxlines if (data.length <= maxbytes && (!maxlines || data.split('\n').length < maxlines)) { return data; diff --git a/frontend/src/components/MinioArtifactPreview.tsx b/frontend/src/components/MinioArtifactPreview.tsx index f27ccc9e9afc..ac4e15f8677c 100644 --- a/frontend/src/components/MinioArtifactPreview.tsx +++ b/frontend/src/components/MinioArtifactPreview.tsx @@ -78,7 +78,7 @@ async function getPreview( maxlines?: number, ): Promise<{ data: string; hasMore: boolean }> { // TODO how to handle binary data (can probably use magic number to id common mime types) - let data = await Apis.readFile({path: storagePath, namespace: namespace, peek: maxbytes +1}); + let data = await Apis.readFile({ path: storagePath, namespace: namespace, peek: maxbytes + 1 }); // is preview === data and no maxlines if (data.length <= maxbytes && !maxlines) { return { data, hasMore: false }; diff --git a/frontend/src/components/tabs/InputOutputTab.tsx b/frontend/src/components/tabs/InputOutputTab.tsx index c4e1b213fa04..f000f1a1f6e6 100644 --- a/frontend/src/components/tabs/InputOutputTab.tsx +++ b/frontend/src/components/tabs/InputOutputTab.tsx @@ -42,7 +42,7 @@ export type ParamList = Array>; export type URIToSessionInfo = Map; export interface ArtifactParamsWithSessionInfo { params: ParamList; - sessionMap: URIToSessionInfo + sessionMap: URIToSessionInfo; } export interface ArtifactLocation { @@ -93,15 +93,15 @@ export function InputOutputTab({ execution, namespace }: IOTabProps) { ); } - let inputArtifacts : ParamList = [] - let outputArtifacts : ParamList = [] + let inputArtifacts: ParamList = []; + let outputArtifacts: ParamList = []; - if (inputArtifactsWithSessionInfo){ - inputArtifacts = inputArtifactsWithSessionInfo.params + if (inputArtifactsWithSessionInfo) { + inputArtifacts = inputArtifactsWithSessionInfo.params; } if (outputArtifactsWithSessionInfo) { - outputArtifacts = outputArtifactsWithSessionInfo.params + outputArtifacts = outputArtifactsWithSessionInfo.params; } let isIoEmpty = false; @@ -215,34 +215,33 @@ function extractParamFromExecution(execution: Execution, name: string): KeyValue export function getArtifactParamList( inputArtifacts: LinkedArtifact[], artifactTypeNames: string[], -): (ArtifactParamsWithSessionInfo) { - - let sessMap : URIToSessionInfo = new Map() +): ArtifactParamsWithSessionInfo { + let sessMap: URIToSessionInfo = new Map(); let params = Object.values(inputArtifacts).map((linkedArtifact, index) => { let key = getArtifactName(linkedArtifact); if ( - key && - (artifactTypeNames[index] === 'system.Metrics' || - artifactTypeNames[index] === 'system.ClassificationMetrics') + key && + (artifactTypeNames[index] === 'system.Metrics' || + artifactTypeNames[index] === 'system.ClassificationMetrics') ) { key += ' (This is an empty file by default)'; } const artifactId = linkedArtifact.artifact.getId(); const artifactElement = RoutePageFactory.artifactDetails(artifactId) ? ( - - {key} - + + {key} + ) : ( - key + key ); const uri = linkedArtifact.artifact.getUri(); const sessInfo = getStoreSessionInfoFromArtifact(linkedArtifact); - sessMap.set(uri, sessInfo) + sessMap.set(uri, sessInfo); return [artifactElement, uri]; }); - return {params: params, sessionMap: sessMap} + return { params: params, sessionMap: sessMap }; } diff --git a/frontend/src/components/tabs/RuntimeNodeDetailsV2.tsx b/frontend/src/components/tabs/RuntimeNodeDetailsV2.tsx index c9ada66e430e..8a1cac7faa47 100644 --- a/frontend/src/components/tabs/RuntimeNodeDetailsV2.tsx +++ b/frontend/src/components/tabs/RuntimeNodeDetailsV2.tsx @@ -49,7 +49,7 @@ import { MetricsVisualizations } from 'src/components/viewers/MetricsVisualizati import { ArtifactTitle } from 'src/components/tabs/ArtifactTitle'; import InputOutputTab, { getArtifactParamList, - ParamList + ParamList, } from 'src/components/tabs/InputOutputTab'; import { convertYamlToPlatformSpec, convertYamlToV2PipelineSpec } from 'src/lib/v2/WorkflowUtils'; import { PlatformDeploymentConfig } from 'src/generated/pipeline_spec/pipeline_spec'; @@ -180,7 +180,7 @@ function TaskNodeDetail({ {selectedTab === 0 && (() => { if (execution) { - return ; + return ; } return NODE_STATE_UNAVAILABLE; })()} @@ -422,10 +422,10 @@ function ArtifactInfo({ ]; let artifactParamsWithSessionInfo = getArtifactParamList([linkedArtifact], artifactTypeName); - let artifactParams : ParamList = [] + let artifactParams: ParamList = []; - if (artifactParamsWithSessionInfo){ - artifactParams = artifactParamsWithSessionInfo.params + if (artifactParamsWithSessionInfo) { + artifactParams = artifactParamsWithSessionInfo.params; } return ( diff --git a/frontend/src/components/viewers/MetricsVisualizations.tsx b/frontend/src/components/viewers/MetricsVisualizations.tsx index 22ec593af029..f43992241c67 100644 --- a/frontend/src/components/viewers/MetricsVisualizations.tsx +++ b/frontend/src/components/viewers/MetricsVisualizations.tsx @@ -891,7 +891,7 @@ export async function getHtmlViewerConfig( const providerInfo = getStoreSessionInfoFromArtifact(linkedArtifact); // TODO(zijianjoy): Limit the size of HTML file fetching to prevent UI frozen. - let data = await Apis.readFile({path: storagePath, providerInfo, namespace: namespace}); + let data = await Apis.readFile({ path: storagePath, providerInfo, namespace: namespace }); return { htmlContent: data, type: PlotType.WEB_APP } as HTMLViewerConfig; }); return Promise.all(htmlViewerConfigs); @@ -919,7 +919,7 @@ export async function getMarkdownViewerConfig( const providerInfo = getStoreSessionInfoFromArtifact(linkedArtifact); // TODO(zijianjoy): Limit the size of Markdown file fetching to prevent UI frozen. - let data = await Apis.readFile({path: storagePath, providerInfo, namespace: namespace}); + let data = await Apis.readFile({ path: storagePath, providerInfo, namespace: namespace }); return { markdownContent: data, type: PlotType.MARKDOWN } as MarkdownViewerConfig; }); return Promise.all(markdownViewerConfigs); diff --git a/frontend/src/lib/Apis.test.ts b/frontend/src/lib/Apis.test.ts index ffb3fc571922..8c4c2184c529 100644 --- a/frontend/src/lib/Apis.test.ts +++ b/frontend/src/lib/Apis.test.ts @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {Apis} from './Apis'; -import {StorageService} from './WorkflowParser'; +import { Apis } from './Apis'; +import { StorageService } from './WorkflowParser'; const fetchSpy = (response: string) => { const spy = jest.fn(() => @@ -128,7 +128,7 @@ describe('Apis', () => { const spy = fetchSpy('file contents'); expect( await Apis.readFile({ - path: {source: StorageService.GCS, key:"testkey", bucket:'testbucket'}, + path: { source: StorageService.GCS, key: 'testkey', bucket: 'testbucket' }, }), ).toEqual('file contents'); expect(spy).toHaveBeenCalledWith('artifacts/get?source=gcs&bucket=testbucket&key=testkey', { diff --git a/frontend/src/lib/Apis.ts b/frontend/src/lib/Apis.ts index 497e9b751046..c87fa6d5078d 100644 --- a/frontend/src/lib/Apis.ts +++ b/frontend/src/lib/Apis.ts @@ -265,13 +265,18 @@ export class Apis { /** * Reads file from storage using server. */ - public static readFile({path, providerInfo, namespace, peek} : { + public static readFile({ + path, + providerInfo, + namespace, + peek, + }: { path: StoragePath; namespace?: string; providerInfo?: string; peek?: number; }): Promise { - let query = this.buildReadFileUrl({ path, namespace, providerInfo, peek, isDownload: false }) + let query = this.buildReadFileUrl({ path, namespace, providerInfo, peek, isDownload: false }); return this._fetch(query); } diff --git a/frontend/src/lib/OutputArtifactLoader.test.ts b/frontend/src/lib/OutputArtifactLoader.test.ts index 100e9a940efa..819c8ee10511 100644 --- a/frontend/src/lib/OutputArtifactLoader.test.ts +++ b/frontend/src/lib/OutputArtifactLoader.test.ts @@ -88,7 +88,7 @@ describe('OutputArtifactLoader', () => { fileToRead = JSON.stringify({ outputs: [metadata] }); await OutputArtifactLoader.load(storagePath, 'ns1'); expect(readFileSpy).toHaveBeenCalledTimes(2); - expect(readFileSpy.mock.calls.map(([{path, namespace}]) => namespace)) + expect(readFileSpy.mock.calls.map(([{ path, namespace }]) => namespace)) .toMatchInlineSnapshot(` Array [ "ns1", diff --git a/frontend/src/lib/OutputArtifactLoader.ts b/frontend/src/lib/OutputArtifactLoader.ts index 8a23d71841a0..fa1182cb4f5d 100644 --- a/frontend/src/lib/OutputArtifactLoader.ts +++ b/frontend/src/lib/OutputArtifactLoader.ts @@ -58,7 +58,7 @@ export class OutputArtifactLoader { public static async load(outputPath: StoragePath, namespace?: string): Promise { let plotMetadataList: PlotMetadata[] = []; try { - const metadataFile = await Apis.readFile({path: outputPath, namespace: namespace}); + const metadataFile = await Apis.readFile({ path: outputPath, namespace: namespace }); if (metadataFile) { try { plotMetadataList = OutputArtifactLoader.parseOutputMetadataInJson( @@ -516,7 +516,10 @@ async function readSourceContent( if (storage === 'inline') { return source; } - return await Apis.readFile({path: WorkflowParser.parseStoragePath(source), namespace: namespace}); + return await Apis.readFile({ + path: WorkflowParser.parseStoragePath(source), + namespace: namespace, + }); } export const TEST_ONLY = { diff --git a/frontend/src/mlmd/MlmdUtils.ts b/frontend/src/mlmd/MlmdUtils.ts index a6614108eb00..32ae10fd1d60 100644 --- a/frontend/src/mlmd/MlmdUtils.ts +++ b/frontend/src/mlmd/MlmdUtils.ts @@ -287,8 +287,11 @@ export interface LinkedArtifact { artifact: Artifact; } -export function getStoreSessionInfoFromArtifact(artifact : LinkedArtifact) : string | undefined { - return artifact.artifact.getCustomPropertiesMap().get("store_session_info")?.getStringValue(); +export function getStoreSessionInfoFromArtifact(artifact: LinkedArtifact): string | undefined { + return artifact.artifact + .getCustomPropertiesMap() + .get('store_session_info') + ?.getStringValue(); } export async function getLinkedArtifactsByEvents(events: Event[]): Promise {