Skip to content

Commit

Permalink
[UIServer] Add DISABLE_GKE_METADATA env flag to skip metadata retriev…
Browse files Browse the repository at this point in the history
…al. (#3118)

* Add DISABLE_GKE_METADATA env flag to skip metadata retrieval.

* node server build step fix and cleanup

* Update frontend dockerfile to use npm ci
  • Loading branch information
Bobgy authored Feb 20, 2020
1 parent 74a8178 commit 3b072b2
Show file tree
Hide file tree
Showing 8 changed files with 4,292 additions and 2,260 deletions.
2 changes: 1 addition & 1 deletion frontend/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ COPY . .

WORKDIR ./frontend

RUN npm install && npm run postinstall
RUN npm ci && npm run postinstall
RUN npm run build

RUN mkdir -p ./server/dist && \
Expand Down
2 changes: 1 addition & 1 deletion frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"lint": "tslint -c ./tslint.prod.json -p .",
"mock:api": "ts-node-dev -O '{\"module\": \"commonjs\"}' mock-backend/mock-api-server.ts 3001",
"mock:server": "node server/dist/server.js build",
"postinstall": "cd ./server && npm i && cd ../mock-backend && npm i",
"postinstall": "cd ./server && npm ci && cd ../mock-backend && npm ci",
"start:proxy-standalone": "./start-proxy-standalone.sh",
"start:proxy-standalone-and-server": "./start-proxy-standalone-and-server.sh",
"start": "react-scripts-ts start",
Expand Down
66 changes: 63 additions & 3 deletions frontend/server/app.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,17 @@ import { Storage as GCSStorage } from '@google-cloud/storage';
import { UIServer } from './app';
import { loadConfigs } from './configs';
import * as minioHelper from './minio-helper';
import * as k8sHelper from './k8s-helper';

jest.mock('minio');
jest.mock('node-fetch');
jest.mock('@google-cloud/storage');
jest.mock('./minio-helper');
jest.mock('./k8s-helper');

const mockedFetch: jest.Mock = fetch as any;
const mockedK8sHelper: jest.Mock = k8sHelper as any;

describe('UIServer apis', () => {
let app: UIServer;
const indexHtmlPath = path.resolve(os.tmpdir(), 'index.html');
Expand Down Expand Up @@ -269,7 +273,6 @@ describe('UIServer apis', () => {

it('responds with a http artifact if source=http', done => {
const artifactContent = 'hello world';
const mockedFetch: jest.Mock = fetch as any;
mockedFetch.mockImplementationOnce((url: string, opts: any) =>
url === 'http://foo.bar/ml-pipeline/hello/world.txt'
? Promise.resolve({ buffer: () => Promise.resolve(artifactContent) })
Expand All @@ -293,7 +296,6 @@ describe('UIServer apis', () => {

it('responds with a https artifact if source=https', done => {
const artifactContent = 'hello world';
const mockedFetch: jest.Mock = fetch as any;
mockedFetch.mockImplementationOnce((url: string, opts: any) =>
url === 'https://foo.bar/ml-pipeline/hello/world.txt' &&
opts.headers.Authorization === 'someToken'
Expand Down Expand Up @@ -322,7 +324,6 @@ describe('UIServer apis', () => {

it('responds with a https artifact using the inherited header if source=https and http authorization key is provided.', done => {
const artifactContent = 'hello world';
const mockedFetch: jest.Mock = fetch as any;
mockedFetch.mockImplementationOnce((url: string, _opts: any) =>
url === 'https://foo.bar/ml-pipeline/hello/world.txt'
? Promise.resolve({ buffer: () => Promise.resolve(artifactContent) })
Expand Down Expand Up @@ -370,6 +371,65 @@ describe('UIServer apis', () => {
});
});

describe('/system', () => {
describe('/cluster-name', () => {
it('responds with cluster name data from gke metadata', done => {
mockedFetch.mockImplementationOnce((url: string, _opts: any) =>
url === 'http://metadata/computeMetadata/v1/instance/attributes/cluster-name'
? Promise.resolve({ text: () => Promise.resolve('test-cluster') })
: Promise.reject('Unexpected request'),
);
mockedK8sHelper.isInCluster = true;
const configs = loadConfigs(argv, {});
app = new UIServer(configs);

const request = requests(app.start());
request
.get('/system/cluster-name')
.expect('Content-Type', 'text/html; charset=utf-8')
.expect(200, 'test-cluster', done);
});
it('responds with endpoint disabled if DISABLE_GKE_METADATA env is true', done => {
const configs = loadConfigs(argv, { DISABLE_GKE_METADATA: 'true' });
app = new UIServer(configs);

const request = requests(app.start());
request
.get('/system/cluster-name')
.expect('Content-Type', 'text/html; charset=utf-8')
.expect(500, 'GKE metadata endpoints are disabled.', done);
});
});
describe('/project-id', () => {
it('responds with project id data from gke metadata', done => {
mockedFetch.mockImplementationOnce((url: string, _opts: any) =>
url === 'http://metadata/computeMetadata/v1/project/project-id'
? Promise.resolve({ text: () => Promise.resolve('test-project') })
: Promise.reject('Unexpected request'),
);
mockedK8sHelper.isInCluster = true;
const configs = loadConfigs(argv, {});
app = new UIServer(configs);

const request = requests(app.start());
request
.get('/system/project-id')
.expect('Content-Type', 'text/html; charset=utf-8')
.expect(200, 'test-project', done);
});
it('responds with endpoint disabled if DISABLE_GKE_METADATA env is true', done => {
const configs = loadConfigs(argv, { DISABLE_GKE_METADATA: 'true' });
app = new UIServer(configs);

const request = requests(app.start());
request
.get('/system/project-id')
.expect('Content-Type', 'text/html; charset=utf-8')
.expect(500, 'GKE metadata endpoints are disabled.', done);
});
});
});

// TODO: refractor k8s helper module so that api that interact with k8s can be
// mocked and tested. There is currently no way to mock k8s APIs as
// `k8s-helper.isInCluster` is a constant that is generated when the module is
Expand Down
6 changes: 3 additions & 3 deletions frontend/server/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
deleteTensorboardHandler,
} from './handlers/tensorboard';
import { getPodLogsHandler } from './handlers/pod-logs';
import { clusterNameHandler, projectIdHandler } from './handlers/gke-metadata';
import { getClusterNameHandler, getProjectIdHandler } from './handlers/gke-metadata';
import { getAllowCustomVisualizationsHandler } from './handlers/vis';
import { getIndexHTMLHandler } from './handlers/index-html';

Expand Down Expand Up @@ -128,8 +128,8 @@ function createUIServer(options: UIConfigs) {
registerHandler(app.get, '/k8s/pod/logs', getPodLogsHandler(options.argo, options.artifacts));

/** Cluster metadata (GKE only) */
registerHandler(app.get, '/system/cluster-name', clusterNameHandler);
registerHandler(app.get, '/system/project-id', projectIdHandler);
registerHandler(app.get, '/system/cluster-name', getClusterNameHandler(options.gkeMetadata));
registerHandler(app.get, '/system/project-id', getProjectIdHandler(options.gkeMetadata));

/** Visualization */
registerHandler(
Expand Down
9 changes: 9 additions & 0 deletions frontend/server/configs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ export function loadConfigs(
ARGO_ARCHIVE_BUCKETNAME = 'mlpipeline',
/** Prefix to logs. */
ARGO_ARCHIVE_PREFIX = 'logs',
/** Disables GKE metadata endpoint. */
DISABLE_GKE_METADATA = 'false',
/** Deployment type. */
DEPLOYMENT: DEPLOYMENT_STR = '',
} = env;
Expand Down Expand Up @@ -149,6 +151,9 @@ export function loadConfigs(
visualizations: {
allowCustomVisualizations: asBool(ALLOW_CUSTOM_VISUALIZATIONS),
},
gkeMetadata: {
disabled: asBool(DISABLE_GKE_METADATA),
},
};
}

Expand Down Expand Up @@ -202,6 +207,9 @@ export interface ServerConfigs {
apiVersionPrefix: string;
deployment: Deployments;
}
export interface GkeMetadataConfigs {
disabled: boolean;
}
export interface UIConfigs {
server: ServerConfigs;
artifacts: {
Expand All @@ -214,4 +222,5 @@ export interface UIConfigs {
visualizations: VisualizationsConfigs;
viewer: ViewerConfigs;
pipeline: PipelineConfigs;
gkeMetadata: GkeMetadataConfigs;
}
23 changes: 21 additions & 2 deletions frontend/server/handlers/gke-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,20 @@
import { Handler } from 'express';
import * as k8sHelper from '../k8s-helper';
import fetch from 'node-fetch';
import { GkeMetadataConfigs } from '../configs';

export const clusterNameHandler: Handler = async (_, res) => {
const disabledHandler: Handler = async (_, res) => {
res.status(500).send('GKE metadata endpoints are disabled.');
};

export const getClusterNameHandler = (options: GkeMetadataConfigs) => {
if (options.disabled) {
return disabledHandler;
}
return clusterNameHandler;
};

const clusterNameHandler: Handler = async (_, res) => {
if (!k8sHelper.isInCluster) {
res.status(500).send('Not running in Kubernetes cluster.');
return;
Expand All @@ -28,7 +40,14 @@ export const clusterNameHandler: Handler = async (_, res) => {
res.send(await response.text());
};

export const projectIdHandler: Handler = async (_, res) => {
export const getProjectIdHandler = (options: GkeMetadataConfigs) => {
if (options.disabled) {
return disabledHandler;
}
return projectIdHandler;
};

const projectIdHandler: Handler = async (_, res) => {
if (!k8sHelper.isInCluster) {
res.status(500).send('Not running in Kubernetes cluster.');
return;
Expand Down
Loading

0 comments on commit 3b072b2

Please sign in to comment.