From 3a947618c88ef88ec7b3753895340d23ee7d06a7 Mon Sep 17 00:00:00 2001 From: Andrew Longmore Date: Thu, 13 May 2021 11:23:22 +0100 Subject: [PATCH] feat: move jupyter and jupyterlab to single host --- ...7-remove-need-for-wildcard-certificates.md | 27 ++++ .../workspaces/common/src/config/catalogue.js | 4 +- code/workspaces/common/src/stackTypes.js | 11 ++ code/workspaces/common/src/stackTypes.spec.js | 15 ++ .../infrastructure-api/package.json | 2 +- .../resources/jupyter.deployment.template.yml | 9 +- .../infrastructure-api/src/config/config.js | 6 + .../deploymentGenerator.spec.js.snap | 9 +- .../ingressGenerator.spec.js.snap | 25 +++ .../src/kubernetes/deploymentGenerator.js | 8 +- .../kubernetes/deploymentGenerator.spec.js | 2 +- .../src/kubernetes/ingressGenerator.js | 23 ++- .../src/kubernetes/ingressGenerator.spec.js | 15 ++ .../__snapshots__/stackManager.spec.js.snap | 2 +- .../src/stacks/stackBuilders.js | 6 +- .../src/stacks/stackManager.js | 22 ++- .../src/stacks/stackManager.spec.js | 14 ++ .../assetRepo/AddRepoMetadataDetails.spec.js | 2 +- .../assetRepo/EditRepoMetadataForm.spec.js | 2 +- .../notebooks/CreateNotebookForm.js | 4 +- .../notebooks/CreateNotebookForm.spec.js | 17 ++ .../CreateNotebookForm.spec.js.snap | 150 ++++++++++++++++++ code/workspaces/web-app/src/core/urlHelper.js | 18 ++- .../web-app/src/core/urlHelper.spec.js | 21 ++- 24 files changed, 378 insertions(+), 36 deletions(-) create mode 100644 architecture/decisions/0047-remove-need-for-wildcard-certificates.md create mode 100644 code/workspaces/common/src/stackTypes.spec.js diff --git a/architecture/decisions/0047-remove-need-for-wildcard-certificates.md b/architecture/decisions/0047-remove-need-for-wildcard-certificates.md new file mode 100644 index 000000000..9a28daeed --- /dev/null +++ b/architecture/decisions/0047-remove-need-for-wildcard-certificates.md @@ -0,0 +1,27 @@ +# 47. Remove need for wildcard certificates + +Date: 2021-05-13 + +## Status + +Accepted + +## Context + +DataLabs makes extensive use of reverse proxying, to give users access to resources (such as Minio or JupyterLabs). These resources need individually from an external URL to an internal service URL. There are four design options for reverse proxying (): + +1. Subdomain - this allows the external path and internal path to be the same, probably with a default root base path (/). Different services are identified by the external URL's hostname. This has some disadvantages - multiple hostnames require a wildcard certificate, or multiple certificates if a wildcard certificate can not be acquired; and it makes the development environment more difficult, because you can not just use localhost. +2. Port - this also allows the external path and internal path to be the same, probably with a default root base path (/). Different services are identified by the external URL's port. This has the disadvantage that some organisational firewalls restrict http traffic to unusual hosts. +3. Symmetric Path - this allows the external path and internal path to be the same, but with that path configured. Different services are identified by the path. This is the best option, but the internal service must allow the path to be configurable. +4. Asymmetric Path - here the external and internal paths are different. Different services are identifiable by the external path. This requires a search-and-replace of the path on the rendered HTML and JavaScript, so unless these are simple, then this is too fragile. + +Historically DataLabs has used Subdomain proxying. + +## Decision + +Where possible, Symmetric Path or Asymmetric Path proxying should be used. If this is not possible, a ConfigMap option should determine whether the remaining proxying strategy should be Subdomain or Port proxying. + +## Consequences + +* Developer effort is required to develop the different proxying strategies. +* Portability will be improved and local development will be simplified. diff --git a/code/workspaces/common/src/config/catalogue.js b/code/workspaces/common/src/config/catalogue.js index bd6ae193f..3ad084c4d 100644 --- a/code/workspaces/common/src/config/catalogue.js +++ b/code/workspaces/common/src/config/catalogue.js @@ -1,5 +1,5 @@ import data from './catalogue_asset_repo_config.json'; export const catalogueAvailable = () => data.catalogue && data.catalogue.available; -export const catalogueServer = () => catalogueAvailable() && data.catalogue.storage.server; -export const catalogueFileLocation = () => catalogueAvailable() && data.catalogue.storage.rootDirectory; +export const catalogueServer = () => (catalogueAvailable() ? data.catalogue.storage.server : null); +export const catalogueFileLocation = () => (catalogueAvailable() ? data.catalogue.storage.rootDirectory : null); diff --git a/code/workspaces/common/src/stackTypes.js b/code/workspaces/common/src/stackTypes.js index 568f0a833..8f25612c4 100644 --- a/code/workspaces/common/src/stackTypes.js +++ b/code/workspaces/common/src/stackTypes.js @@ -9,6 +9,15 @@ const PROJECT = 'project'; const RSHINY = 'rshiny'; const RSTUDIO = 'rstudio'; const ZEPPELIN = 'zeppelin'; +const singleHostNameTypes = [JUPYTER, JUPYTERLAB]; + +// returns true if this stack type can be handled by a single host name +const isSingleHostName = type => singleHostNameTypes.includes(type); + +// base path for resources +const basePath = (type, projectKey, name) => (isSingleHostName(type) + ? `/resource/${projectKey}/${name}` + : '/'); export { DASK, @@ -19,4 +28,6 @@ export { RSHINY, RSTUDIO, ZEPPELIN, + isSingleHostName, + basePath, }; diff --git a/code/workspaces/common/src/stackTypes.spec.js b/code/workspaces/common/src/stackTypes.spec.js new file mode 100644 index 000000000..837903d30 --- /dev/null +++ b/code/workspaces/common/src/stackTypes.spec.js @@ -0,0 +1,15 @@ +import { JUPYTER, basePath } from './stackTypes'; + +describe('stackTypes', () => { + describe('basePath', () => { + const projectKey = 'project-key'; + const name = 'name'; + it('gives custom base path if type is for single hostname', () => { + expect(basePath(JUPYTER, projectKey, name)).toEqual('/resource/project-key/name'); + }); + + it('gives root base path if type is for multiple hostname', () => { + expect(basePath('assumed-to-be-multiple', projectKey, name)).toEqual('/'); + }); + }); +}); diff --git a/code/workspaces/infrastructure-api/package.json b/code/workspaces/infrastructure-api/package.json index 061a8a8b7..ffc91383e 100644 --- a/code/workspaces/infrastructure-api/package.json +++ b/code/workspaces/infrastructure-api/package.json @@ -22,8 +22,8 @@ "lodash": "4.17.19", "mongoose": "5.7.5", "mustache": "3.0.1", - "uuid": "3.3.2", "service-chassis": "^0.1.0", + "uuid": "3.3.2", "winston": "3.2.1" }, "devDependencies": { diff --git a/code/workspaces/infrastructure-api/resources/jupyter.deployment.template.yml b/code/workspaces/infrastructure-api/resources/jupyter.deployment.template.yml index b6056f38d..f36eaa2ad 100644 --- a/code/workspaces/infrastructure-api/resources/jupyter.deployment.template.yml +++ b/code/workspaces/infrastructure-api/resources/jupyter.deployment.template.yml @@ -46,9 +46,10 @@ spec: name: {{ name }} command: ["start.sh", "jupyter", "{{ startCmd }}", - "--NotebookApp.token='$(JUPYTER_TOKEN)'", - "--NotebookApp.allow_origin='{{ domain }}'", - "--NotebookApp.notebook_dir='/data/notebooks/{{ name }}'"] + "--NotebookApp.token=$(JUPYTER_TOKEN)", + "--NotebookApp.allow_origin={{ domain }}", + "--NotebookApp.notebook_dir=/data/notebooks/{{ name }}", + "--NotebookApp.base_url={{{ basePath }}}"] ports: - containerPort: 8888 protocol: TCP @@ -77,7 +78,7 @@ spec: memory: 8Gi livenessProbe: httpGet: - path: / + path: {{{ basePath }}} port: 8888 initialDelaySeconds: 5 periodSeconds: 10 diff --git a/code/workspaces/infrastructure-api/src/config/config.js b/code/workspaces/infrastructure-api/src/config/config.js index 31fb7e8ad..17cc57706 100644 --- a/code/workspaces/infrastructure-api/src/config/config.js +++ b/code/workspaces/infrastructure-api/src/config/config.js @@ -7,6 +7,12 @@ const config = convict({ default: 'info', env: 'LOG_LEVEL', }, + datalabName: { + doc: 'The name of the datalab', + format: 'String', + default: 'testlab', + env: 'DATALAB_NAME', + }, datalabDomain: { doc: 'The domain the datalabs instance is running on', format: 'String', diff --git a/code/workspaces/infrastructure-api/src/kubernetes/__snapshots__/deploymentGenerator.spec.js.snap b/code/workspaces/infrastructure-api/src/kubernetes/__snapshots__/deploymentGenerator.spec.js.snap index 8814bd19e..a7624b386 100644 --- a/code/workspaces/infrastructure-api/src/kubernetes/__snapshots__/deploymentGenerator.spec.js.snap +++ b/code/workspaces/infrastructure-api/src/kubernetes/__snapshots__/deploymentGenerator.spec.js.snap @@ -274,9 +274,10 @@ spec: name: deployment-name command: [\\"start.sh\\", \\"jupyter\\", \\"lab\\", - \\"--NotebookApp.token='$(JUPYTER_TOKEN)'\\", - \\"--NotebookApp.allow_origin='project-key-notebook-name.datalabs.localhost'\\", - \\"--NotebookApp.notebook_dir='/data/notebooks/deployment-name'\\"] + \\"--NotebookApp.token=$(JUPYTER_TOKEN)\\", + \\"--NotebookApp.allow_origin=project-key-notebook-name.datalabs.localhost\\", + \\"--NotebookApp.notebook_dir=/data/notebooks/deployment-name\\", + \\"--NotebookApp.base_url=/resource/project-key/notebook-name\\"] ports: - containerPort: 8888 protocol: TCP @@ -305,7 +306,7 @@ spec: memory: 8Gi livenessProbe: httpGet: - path: / + path: /resource/project-key/notebook-name port: 8888 initialDelaySeconds: 5 periodSeconds: 10 diff --git a/code/workspaces/infrastructure-api/src/kubernetes/__snapshots__/ingressGenerator.spec.js.snap b/code/workspaces/infrastructure-api/src/kubernetes/__snapshots__/ingressGenerator.spec.js.snap index 7f37f0c53..8d01edf00 100644 --- a/code/workspaces/infrastructure-api/src/kubernetes/__snapshots__/ingressGenerator.spec.js.snap +++ b/code/workspaces/infrastructure-api/src/kubernetes/__snapshots__/ingressGenerator.spec.js.snap @@ -191,3 +191,28 @@ spec: servicePort: 80 " `; + +exports[`Ingress generator should use datalab hostname and custom path for single hostname type 1`] = ` +"--- +apiVersion: networking.k8s.io/v1beta1 +kind: Ingress +metadata: + name: name-ingress + annotations: + nginx.ingress.kubernetes.io/auth-url: http://localhost:9000/auth + nginx.ingress.kubernetes.io/auth-signin: https://testlab.test-datalabs.nerc.ac.uk + nginx.ingress.kubernetes.io/proxy-body-size: 50g +spec: + tls: + - hosts: + - testlab.datalabs.localhost + rules: + - host: testlab.datalabs.localhost + http: + paths: + - path: /resource/project/name + backend: + serviceName: name-service + servicePort: 80 +" +`; diff --git a/code/workspaces/infrastructure-api/src/kubernetes/deploymentGenerator.js b/code/workspaces/infrastructure-api/src/kubernetes/deploymentGenerator.js index 68e74333e..e46dfbcf9 100644 --- a/code/workspaces/infrastructure-api/src/kubernetes/deploymentGenerator.js +++ b/code/workspaces/infrastructure-api/src/kubernetes/deploymentGenerator.js @@ -1,9 +1,12 @@ import { imageConfig, image, defaultImage } from 'common/src/config/images'; +import { stackTypes } from 'common'; import { DeploymentTemplates, ServiceTemplates, generateManifest, ConfigMapTemplates, NetworkPolicyTemplates, AutoScalerTemplates } from './manifestGenerator'; import nameGenerator from '../common/nameGenerators'; import config from '../config/config'; import logger from '../config/logger'; +const { basePath } = stackTypes; + const containerInfo = imageConfig(); function getImage(type, version) { @@ -15,13 +18,14 @@ function getImage(type, version) { } } -function createJupyterDeployment({ projectKey, deploymentName, notebookName, type, volumeMount, version }) { +function createJupyterDeployment({ projectKey, deploymentName, name, type, volumeMount, version }) { const startCmd = type === 'jupyterlab' ? 'lab' : 'notebook'; const img = getImage(type, version); const context = { name: deploymentName, grantSudo: 'yes', - domain: `${projectKey}-${notebookName}.${config.get('datalabDomain')}`, + domain: `${projectKey}-${name}.${config.get('datalabDomain')}`, + basePath: basePath(type, projectKey, name), jupyter: { image: img.image, }, diff --git a/code/workspaces/infrastructure-api/src/kubernetes/deploymentGenerator.spec.js b/code/workspaces/infrastructure-api/src/kubernetes/deploymentGenerator.spec.js index b6b7e98c1..f5e121883 100644 --- a/code/workspaces/infrastructure-api/src/kubernetes/deploymentGenerator.spec.js +++ b/code/workspaces/infrastructure-api/src/kubernetes/deploymentGenerator.spec.js @@ -93,7 +93,7 @@ describe('deploymentGenerator', () => { const params = { projectKey: 'project-key', deploymentName: 'deployment-name', - notebookName: 'notebook-name', + name: 'notebook-name', type: 'jupyterlab', volumeMount: 'volume-mount', }; diff --git a/code/workspaces/infrastructure-api/src/kubernetes/ingressGenerator.js b/code/workspaces/infrastructure-api/src/kubernetes/ingressGenerator.js index e0df6d5c8..6cf988ee7 100644 --- a/code/workspaces/infrastructure-api/src/kubernetes/ingressGenerator.js +++ b/code/workspaces/infrastructure-api/src/kubernetes/ingressGenerator.js @@ -1,10 +1,12 @@ +import { isSingleHostName, basePath } from 'common/src/stackTypes'; +import { join } from 'path'; import config from '../config/config'; import { IngressTemplates, generateManifest } from './manifestGenerator'; function createIngress({ name, projectKey, ingressName, serviceName, port, - connectPort, rewriteTarget, proxyTimeout, visible, proxyRequestBuffering }) { - const host = createSniInfo(name, projectKey); - const paths = createPathInfo(serviceName, port, connectPort); + connectPort, rewriteTarget, proxyTimeout, visible, proxyRequestBuffering, type }) { + const host = createSniInfo(name, projectKey, type); + const paths = createPathInfo(serviceName, port, connectPort, type, projectKey, name); const privateEndpoint = visible !== 'public'; const authServiceUrlRoot = config.get('authorisationServiceForIngress') || config.get('authorisationService'); const context = { @@ -22,21 +24,26 @@ function createIngress({ name, projectKey, ingressName, serviceName, port, return generateManifest(context, IngressTemplates.DEFAULT_INGRESS); } -function createSniInfo(name, projectKey) { - return `${projectKey}-${name}.${config.get('datalabDomain')}`; +function createSniInfo(name, projectKey, type) { + const domain = config.get('datalabDomain'); + const lab = config.get('datalabName'); + return isSingleHostName(type) + ? `${lab}.${domain}` + : `${projectKey}-${name}.${domain}`; } -function createPathInfo(serviceName, port, connectPort) { +function createPathInfo(serviceName, port, connectPort, type, projectKey, name) { const paths = []; + const base = basePath(type, projectKey, name); paths.push({ - path: '/', + path: base, serviceName, servicePort: port, }); if (connectPort) { paths.push({ - path: '/connect', + path: join(base, 'connect'), serviceName, servicePort: connectPort, }); diff --git a/code/workspaces/infrastructure-api/src/kubernetes/ingressGenerator.spec.js b/code/workspaces/infrastructure-api/src/kubernetes/ingressGenerator.spec.js index a96a51128..93132e1bf 100644 --- a/code/workspaces/infrastructure-api/src/kubernetes/ingressGenerator.spec.js +++ b/code/workspaces/infrastructure-api/src/kubernetes/ingressGenerator.spec.js @@ -1,3 +1,4 @@ +import { JUPYTER } from 'common/src/stackTypes'; import ingressGenerator from './ingressGenerator'; import config from '../config/config'; @@ -104,4 +105,18 @@ describe('Ingress generator', () => { return expect(template).resolves.toMatchSnapshot(); }); + + it('should use datalab hostname and custom path for single hostname type', () => { + const options = { + name: 'name', + projectKey: 'project', + ingressName: 'name-ingress', + serviceName: 'name-service', + port: 80, + type: JUPYTER, + }; + const template = ingressGenerator.createIngress(options); + + return expect(template).resolves.toMatchSnapshot(); + }); }); diff --git a/code/workspaces/infrastructure-api/src/stacks/__snapshots__/stackManager.spec.js.snap b/code/workspaces/infrastructure-api/src/stacks/__snapshots__/stackManager.spec.js.snap index 0551fe8b6..167f38784 100644 --- a/code/workspaces/infrastructure-api/src/stacks/__snapshots__/stackManager.spec.js.snap +++ b/code/workspaces/infrastructure-api/src/stacks/__snapshots__/stackManager.spec.js.snap @@ -30,7 +30,7 @@ Array [ "projectKey": "project", "status": "requested", "type": "jupyter", - "url": "https://project-expectedName.datalabs.localhost", + "url": "https://testlab.datalabs.localhost/resource/project/expectedName", }, ], ] diff --git a/code/workspaces/infrastructure-api/src/stacks/stackBuilders.js b/code/workspaces/infrastructure-api/src/stacks/stackBuilders.js index 388d600ac..36941811e 100644 --- a/code/workspaces/infrastructure-api/src/stacks/stackBuilders.js +++ b/code/workspaces/infrastructure-api/src/stacks/stackBuilders.js @@ -1,4 +1,5 @@ import chalk from 'chalk'; +import { stackTypes } from 'common'; import logger from '../config/logger'; import deploymentApi from '../kubernetes/deploymentApi'; import serviceApi from '../kubernetes/serviceApi'; @@ -10,6 +11,8 @@ import autoScalerApi from '../kubernetes/autoScalerApi'; import deploymentGenerator from '../kubernetes/deploymentGenerator'; import nameGenerator from '../common/nameGenerators'; +const { basePath } = stackTypes; + export const createDeployment = (params, generator) => () => { const { name, projectKey, type } = params; const deploymentName = nameGenerator.deploymentName(name, type); @@ -89,8 +92,9 @@ export const createIngressRule = (params, generator) => (service) => { const ingressName = nameGenerator.deploymentName(name, type); const serviceName = service.metadata.name; const { port } = service.spec.ports[0]; + const base = basePath(type, projectKey, name); - return generator({ ...params, ingressName, serviceName, port }) + return generator({ ...params, ingressName, serviceName, port, basePath: base }) .then((manifest) => { logger.info(`Creating ingress rule ${chalk.blue(ingressName)} with manifest:`); logger.debug(manifest.toString()); diff --git a/code/workspaces/infrastructure-api/src/stacks/stackManager.js b/code/workspaces/infrastructure-api/src/stacks/stackManager.js index 2e5d34053..2c6075c33 100644 --- a/code/workspaces/infrastructure-api/src/stacks/stackManager.js +++ b/code/workspaces/infrastructure-api/src/stacks/stackManager.js @@ -1,6 +1,7 @@ -import path from 'path'; +import { join } from 'path'; import { imageCategory } from 'common/src/config/images'; import { catalogueFileLocation, catalogueServer } from 'common/src/config/catalogue'; +import { stackTypes } from 'common'; import logger from '../config/logger'; import config from '../config/config'; import Stacks from './Stacks'; @@ -10,6 +11,8 @@ import nameGenerator from '../common/nameGenerators'; import deploymentApi from '../kubernetes/deploymentApi'; import centralAssetRepoRepository from '../dataaccess/centralAssetRepoRepository'; +const { isSingleHostName, basePath } = stackTypes; + async function createStack(user, params) { const { projectKey, name, type } = params; const stack = Stacks.getStack(type); @@ -30,13 +33,22 @@ async function createStack(user, params) { ...params, category: imageCategory(type), status: status.REQUESTED, - url: `https://${projectKey}-${name}.${config.get('datalabDomain')}`, + url: url(projectKey, name, type), internalEndpoint: `http://${params.type}-${name}.${projectKey}`, }, ); return creationResponse; } +function url(projectKey, name, type) { + const lab = config.get('datalabName'); + const domain = config.get('datalabDomain'); + const singleHostNameAndPath = join(`${lab}.${domain}`, basePath(type, projectKey, name)); + return isSingleHostName(type) + ? `https://${singleHostNameAndPath}` + : `https://${projectKey}-${name}.${domain}`; +} + function restartStack(params) { const { projectKey, name, type } = params; @@ -120,13 +132,13 @@ async function createAssetVolumeAndVolumeMountArrays(assetIds) { name: volumeName, nfs: { server: storageServer, - path: path.join(storageLocation, fileLocation), + path: join(storageLocation || '', fileLocation), readOnly: true, }, }); assetVolumeMounts.push({ name: volumeName, - mountPath: path.join('/assets', fileLocation), + mountPath: join('/assets', fileLocation), readOnly: true, }); }); @@ -144,4 +156,4 @@ function getContainerNonAssetVolumeMounts(container) { return volumeMounts.filter(volumeMount => !nameGenerator.isAssetVolume(volumeMount.name)); } -export default { createStack, restartStack, deleteStack, mountAssetsOnStack }; +export default { createStack, restartStack, deleteStack, mountAssetsOnStack, url }; diff --git a/code/workspaces/infrastructure-api/src/stacks/stackManager.spec.js b/code/workspaces/infrastructure-api/src/stacks/stackManager.spec.js index 83c3cea8e..acc8424bc 100644 --- a/code/workspaces/infrastructure-api/src/stacks/stackManager.spec.js +++ b/code/workspaces/infrastructure-api/src/stacks/stackManager.spec.js @@ -1,4 +1,5 @@ import * as catalogueConfig from 'common/src/config/catalogue'; +import { JUPYTER } from 'common/src/stackTypes'; import Stacks from './Stacks'; import * as stackRepository from '../dataaccess/stacksRepository'; import stackManager from './stackManager'; @@ -249,4 +250,17 @@ describe('Stack Controller', () => { } }); }); + + describe('url', () => { + const projectKey = 'project-key'; + const name = 'name'; + + it('gives correct url when single hostname', () => { + expect(stackManager.url(projectKey, name, JUPYTER)).toEqual('https://testlab.datalabs.localhost/resource/project-key/name'); + }); + + it('gives correct url when multiple hostname', () => { + expect(stackManager.url(projectKey, name, 'assumed-multiple-hostname')).toEqual('https://project-key-name.datalabs.localhost'); + }); + }); }); diff --git a/code/workspaces/web-app/src/components/assetRepo/AddRepoMetadataDetails.spec.js b/code/workspaces/web-app/src/components/assetRepo/AddRepoMetadataDetails.spec.js index 2855868bb..9d40e9af1 100644 --- a/code/workspaces/web-app/src/components/assetRepo/AddRepoMetadataDetails.spec.js +++ b/code/workspaces/web-app/src/components/assetRepo/AddRepoMetadataDetails.spec.js @@ -6,7 +6,7 @@ import * as assetRepoHooks from '../../hooks/assetRepoHooks'; jest.mock('../../hooks/reduxFormHooks'); jest.mock('../../hooks/assetRepoHooks'); -reduxFormHooks.useReduxFormValue = jest.fn().mockResolvedValue('value'); +reduxFormHooks.useReduxFormValue = jest.fn().mockReturnValue('value'); assetRepoHooks.useAssetRepo = jest.fn().mockReturnValue({ fetching: false, value: { createdAssetId: null } }); describe('AddRepoMetadataDetails', () => { diff --git a/code/workspaces/web-app/src/components/assetRepo/EditRepoMetadataForm.spec.js b/code/workspaces/web-app/src/components/assetRepo/EditRepoMetadataForm.spec.js index b201d1939..19fde6632 100644 --- a/code/workspaces/web-app/src/components/assetRepo/EditRepoMetadataForm.spec.js +++ b/code/workspaces/web-app/src/components/assetRepo/EditRepoMetadataForm.spec.js @@ -4,7 +4,7 @@ import { PureEditRepoMetadataForm } from './EditRepoMetadataForm'; import * as reduxFormHooks from '../../hooks/reduxFormHooks'; jest.mock('../../hooks/reduxFormHooks'); -reduxFormHooks.useReduxFormValue = jest.fn().mockResolvedValue('value'); +reduxFormHooks.useReduxFormValue = jest.fn().mockReturnValue('value'); describe('EditRepoMetadataForm', () => { const shallowRender = () => shallow( diff --git a/code/workspaces/web-app/src/components/notebooks/CreateNotebookForm.js b/code/workspaces/web-app/src/components/notebooks/CreateNotebookForm.js index 1c60d53cf..a5eafa09b 100644 --- a/code/workspaces/web-app/src/components/notebooks/CreateNotebookForm.js +++ b/code/workspaces/web-app/src/components/notebooks/CreateNotebookForm.js @@ -6,6 +6,7 @@ import { formatAndParseMultiSelect, CreateFormControls, renderAdornedTextField, import { getAsyncValidate, syncValidate } from './createNotebookFormValidator'; import getUrlNameStartEndText from '../../core/urlHelper'; import AssetMultiSelect from '../common/form/AssetMultiSelect'; +import { useReduxFormValue } from '../../hooks/reduxFormHooks'; export const FORM_NAME = 'createNotebook'; const NAME_FIELD_NAME = 'name'; @@ -20,7 +21,8 @@ const commonProps = { const CreateNotebookForm = ({ handleSubmit, cancel, submitting, dataStorageOptions, projectKey, typeOptions, versionOptions, }) => { - const { startText, endText } = getUrlNameStartEndText(projectKey, window.location); + const typeValue = useReduxFormValue(FORM_NAME, TYPE_FIELD_NAME); + const { startText, endText } = getUrlNameStartEndText(projectKey, window.location, typeValue); return (
diff --git a/code/workspaces/web-app/src/components/notebooks/CreateNotebookForm.spec.js b/code/workspaces/web-app/src/components/notebooks/CreateNotebookForm.spec.js index e5d9c36fe..1ad9049de 100644 --- a/code/workspaces/web-app/src/components/notebooks/CreateNotebookForm.spec.js +++ b/code/workspaces/web-app/src/components/notebooks/CreateNotebookForm.spec.js @@ -1,6 +1,11 @@ import React from 'react'; import { shallow } from 'enzyme'; +import { JUPYTER } from 'common/src/stackTypes'; import { PureCreateNotebookForm } from './CreateNotebookForm'; +import * as reduxFormHooks from '../../hooks/reduxFormHooks'; + +jest.mock('../../hooks/reduxFormHooks'); +reduxFormHooks.useReduxFormValue = jest.fn().mockReturnValue('value'); describe('CreateNotebookForm', () => { function shallowRender(props) { @@ -51,4 +56,16 @@ describe('CreateNotebookForm', () => { // Assert expect(output).toMatchSnapshot(); }); + + it('creates correct snapshot for create Notebook Form when the type supports single hostname', () => { + // Arrange + const props = generateProps(); + reduxFormHooks.useReduxFormValue = jest.fn().mockReturnValue(JUPYTER); + + // Act + const output = shallowRender(props); + + // Assert + expect(output).toMatchSnapshot(); + }); }); diff --git a/code/workspaces/web-app/src/components/notebooks/__snapshots__/CreateNotebookForm.spec.js.snap b/code/workspaces/web-app/src/components/notebooks/__snapshots__/CreateNotebookForm.spec.js.snap index ff889e276..83291e046 100644 --- a/code/workspaces/web-app/src/components/notebooks/__snapshots__/CreateNotebookForm.spec.js.snap +++ b/code/workspaces/web-app/src/components/notebooks/__snapshots__/CreateNotebookForm.spec.js.snap @@ -1,5 +1,155 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`CreateNotebookForm creates correct snapshot for create Notebook Form when the type supports single hostname 1`] = ` + +
+ +
+
+ +
+
+ +
+
+ +
+
+ +
+
+ , + "value": "private", + }, + Object { + "text": , + "value": "project", + }, + ] + } + /> +
+
+ +
+ + +`; + exports[`CreateNotebookForm creates correct snapshot for create Notebook Form when there are not version options 1`] = `
diff --git a/code/workspaces/web-app/src/core/urlHelper.js b/code/workspaces/web-app/src/core/urlHelper.js index aa000db55..05a1405e3 100644 --- a/code/workspaces/web-app/src/core/urlHelper.js +++ b/code/workspaces/web-app/src/core/urlHelper.js @@ -1,4 +1,20 @@ -export default function getUrlNameStartEndText(projectKey, windowLocation) { +import { stackTypes } from 'common'; +import { join } from 'path'; + +const { isSingleHostName, basePath } = stackTypes; + +export default function getUrlNameStartEndText(projectKey, windowLocation, type) { + return isSingleHostName(type) ? singleHostName(projectKey, windowLocation, type) : multiHostName(projectKey, windowLocation); +} + +function singleHostName(projectKey, windowLocation, type) { + const hostAndPath = join(windowLocation.hostname, basePath(type, projectKey, '')); + const startText = `${windowLocation.protocol}//${hostAndPath}`; + const endText = ''; + return { startText, endText }; +} + +function multiHostName(projectKey, windowLocation) { const separator = '.'; const restHostname = windowLocation.hostname.split(separator).slice(1); const startText = `${windowLocation.protocol}//${projectKey}-`; diff --git a/code/workspaces/web-app/src/core/urlHelper.spec.js b/code/workspaces/web-app/src/core/urlHelper.spec.js index d9cd02cf6..22ec1de75 100644 --- a/code/workspaces/web-app/src/core/urlHelper.spec.js +++ b/code/workspaces/web-app/src/core/urlHelper.spec.js @@ -1,3 +1,4 @@ +import { JUPYTER } from 'common/src/stackTypes'; import getUrlNameStartEndText from './urlHelper'; const projectKey = 'testproj'; @@ -8,7 +9,8 @@ describe('getUrlNameStartEndText', () => { protocol: 'https:', hostname: 'localhost', }; - const { startText, endText } = getUrlNameStartEndText(projectKey, windowLocation); + const type = 'assumed-multiple-host'; + const { startText, endText } = getUrlNameStartEndText(projectKey, windowLocation, type); expect(startText).toEqual('https://testproj-'); expect(endText).toEqual('.datalabs.localhost'); @@ -19,7 +21,8 @@ describe('getUrlNameStartEndText', () => { protocol: 'https:', hostname: 'testlab.datalabs.localhost', }; - const { startText, endText } = getUrlNameStartEndText(projectKey, windowLocation); + const type = 'assumed-multiple-host'; + const { startText, endText } = getUrlNameStartEndText(projectKey, windowLocation, type); expect(startText).toEqual('https://testproj-'); expect(endText).toEqual('.datalabs.localhost'); @@ -30,9 +33,21 @@ describe('getUrlNameStartEndText', () => { protocol: 'https:', hostname: 'datalab.datalabs.nerc.ac.uk', }; - const { startText, endText } = getUrlNameStartEndText(projectKey, windowLocation); + const type = 'assumed-multiple-host'; + const { startText, endText } = getUrlNameStartEndText(projectKey, windowLocation, type); expect(startText).toEqual('https://testproj-'); expect(endText).toEqual('.datalabs.nerc.ac.uk'); }); + + it('returns correct values when single hostname on datalab.datalabs.nerc.ac.uk', () => { + const windowLocation = { + protocol: 'https:', + hostname: 'datalab.datalabs.nerc.ac.uk', + }; + const { startText, endText } = getUrlNameStartEndText(projectKey, windowLocation, JUPYTER); + + expect(startText).toEqual('https://datalab.datalabs.nerc.ac.uk/resource/testproj/'); + expect(endText).toEqual(''); + }); });