Story #14813

[Workbench2] Use cluster config

Added by Peter Amstutz 7 months ago. Updated 12 days ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
07/31/2019
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
1.0

Description

Fetch the safe clusterwide config to bootstrap the app. Update apps to expect clusterwide config.

Workbench2 should go to a well known endpoint relative to the app's base URL, which should then redirect to or proxy for the public clusterwide config exported by controller (#15000).


Subtasks

Task #15387: Review 14813-cluster-configResolvedEric Biagiotti


Related issues

Related to Arvados - Story #13648: [Epic] Use one cluster configuration file for all componentsNew

Blocked by Arvados - Feature #15000: [controller] publish safe configResolved06/07/2019

Associated revisions

Revision db1fe4b4
Added by Eric Biagiotti 26 days ago

Merge remote-tracking branch 'origin/14813-wb2-config' into 14813-wb2-config

refs #14813
2
14813
2

Arvados-DCO-1.1-Signed-off-by: Eric Biagiotti <>

Revision adedc9a3
Added by Tom Clegg 23 days ago

Merge branch '14813-config-cors'

refs #14813

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <>

Revision 1d0a3328
Added by Eric Biagiotti 20 days ago

Merge branch '14813-wb2-config'

refs #14813
2

Arvados-DCO-1.1-Signed-off-by: Eric Biagiotti <>

History

#1 Updated by Peter Amstutz 7 months ago

  • Status changed from New to In Progress

#2 Updated by Peter Amstutz 7 months ago

  • Status changed from In Progress to New

#3 Updated by Peter Amstutz 7 months ago

  • Subject changed from [Workbench2] Use cluster config to [Workbench2, Composer] Use cluster config

#4 Updated by Peter Amstutz 7 months ago

  • Related to Story #13648: [Epic] Use one cluster configuration file for all components added

#5 Updated by Peter Amstutz 5 months ago

#6 Updated by Peter Amstutz 5 months ago

  • Description updated (diff)

#7 Updated by Peter Amstutz 5 months ago

  • Description updated (diff)

#8 Updated by Tom Morris 2 months ago

  • Story points set to 1.0
  • Target version changed from To Be Groomed to Arvados Future Sprints
  • Description updated (diff)
  • Subject changed from [Workbench2, Composer] Use cluster config to [Workbench2] Use cluster config
  • Tracker changed from Feature to Story

#9 Updated by Tom Morris 2 months ago

  • Target version changed from Arvados Future Sprints to 2019-07-03 Sprint

#10 Updated by Eric Biagiotti 2 months ago

  • Assigned To set to Eric Biagiotti

#11 Updated by Eric Biagiotti about 2 months ago

  • Target version changed from 2019-07-03 Sprint to 2019-07-17 Sprint

#12 Updated by Eric Biagiotti about 1 month ago

  • Target version changed from 2019-07-17 Sprint to 2019-07-31 Sprint

#13 Updated by Eric Biagiotti 29 days ago

A few notes on the approach for this:

  • The local config has API_HOST, VOCABULARY_URL, and FILEVIEWERS_CONFIG_URL defined. If found, give these priority and log a console message stating that the local config is deprecated and link to release notes.
  • Workbench2 will still need API_HOST defined somewhere to get the cluster config. Currently, there is an environment variable (REACT_APP_ARVADOS_CONFIG_URL) set to point to the local config. Seems to make more sense to have an ARVADOS_API_HOST environment variable instead of a 1 value local config and an environtment variable pointing to it. Arvbox will also need an update if we do this.
  • Workbench2 currently loads the entire discovery document, but only uses the attributes in the table below. The right column describes where we'd be getting this info when using the cluster config.
    Discovery attribute With cluster config
    baseURL Use ARVADOS_API_HOST env variable and append /arvados/v1
    keepWebServiceUrl From cluster config
    remoteHosts From cluster config
    rootUrl Use ARVADOS_API_HOST env variable
    uuidPrefix From RemoteClusters cluster config entry? I'm not seeing anything for this at https://172.17.0.2:8000/arvados/v1/config
    websocketUrl From cluster config
    workbenchUrl From cluster config
    workbench2Url From cluster config
    vocabularyUrl From cluster config
    fileViewersConfigUrl From cluster config
  • Need to add the following cluster config defaults:
    • vocabularyUrl - public/vocabulary-example.json
    • fileViewersConfigUrl - public/file-viewers-example.json
  • Update release notes to detail the order of precendence and how to update to the cluster config.

#14 Updated by Peter Amstutz 29 days ago

Eric Biagiotti wrote:

A few notes on the approach for this:

  • The local config has API_HOST, VOCABULARY_URL, and FILEVIEWERS_CONFIG_URL defined. If found, give these priority and log a console message stating that the local config is deprecated and link to release notes.
  • Workbench2 will still need API_HOST defined somewhere to get the cluster config. Currently, there is an environment variable (REACT_APP_ARVADOS_CONFIG_URL) set to point to the local config. Seems to make more sense to have an ARVADOS_API_HOST environment variable instead of a 1 value local config and an environtment variable pointing to it. Arvbox will also need an update if we do this.

Keep in mind that the way workbench2 works, when not using the React development server, it is a static lump of javascript that is loaded and executed in the browser. So the browser never sees anything like REACT_APP_ARVADOS_CONFIG_URL. The browser does have access to the URL that it was loaded from, which it uses the bootstrap configuration (by adding on the path to the config file from the URL where it loaded the base document, loading the config file, and then accessing the API server.)

  • Workbench2 currently loads the entire discovery document, but only uses the attributes in the table below. The right column describes where we'd be getting this info when using the cluster config.
    Discovery attribute With cluster config
    baseURL Use ARVADOS_API_HOST env variable and append /arvados/v1
    keepWebServiceUrl From cluster config
    remoteHosts From cluster config
    rootUrl Use ARVADOS_API_HOST env variable
    uuidPrefix From RemoteClusters cluster config entry? I'm not seeing anything for this at https://172.17.0.2:8000/arvados/v1/config
    websocketUrl From cluster config
    workbenchUrl From cluster config
    workbench2Url From cluster config
    vocabularyUrl From cluster config
    fileViewersConfigUrl From cluster config
  • Need to add the following cluster config defaults:
    • vocabularyUrl - public/vocabulary-example.json
    • fileViewersConfigUrl - public/file-viewers-example.json

So they would be resolved relative to the workbench2 URL (which is how it works currently)? They could be full URLs as well.

#16 Updated by Eric Biagiotti 27 days ago

  • Status changed from New to In Progress

#17 Updated by Eric Biagiotti 21 days ago

Latest at 94f1f08184b135d10245fa782e66d43247a107d3 and WB2 at de9713360b5bc04dc3586b332bc98db5d97650a5
Tests: https://ci.curoverse.com/view/Developer/job/developer-run-tests/1441/ - Seems to be the flaky python test that Tom tried to fix.

- WB2 now uses the cluster config instead of the discovery document. I had originally propagated the new config format throughout the WB2 code base, but realized that the safest way (considering some of the testing limitations of wb2) was to just map the cluster config json to the existing wb2 config object.
- Updated the readme to describe how to use the cluster config and now prints a warning when local config values are found.

Manual testing:

- I set up a federation and manually tested, cross site search, link account, cross cluster login, account menu, and link to wb1

#18 Updated by Eric Biagiotti 21 days ago

  • Target version changed from 2019-07-31 Sprint to 2019-08-14 Sprint

#19 Updated by Lucas Di Pentima 20 days ago

From chat:

I made a mistake modifying the config.js file to just leave AP_HOST, leaving the trailing comma (invalid JSON format) and wb2 tried to connect to the undefined host, erroring out on the JS console.

The rest LGTM, thanks!

#20 Updated by Eric Biagiotti 20 days ago

Lucas Di Pentima wrote:

From chat:

I made a mistake modifying the config.js file to just leave AP_HOST, leaving the trailing comma (invalid JSON format) and wb2 tried to connect to the undefined host, erroring out on the JS console.

The rest LGTM, thanks!

I have an update to error handling/logging in the WB2 repo at 61769345c78e04b0f756dcd15e39fe57ddb75c80.

#21 Updated by Lucas Di Pentima 20 days ago

Much better, thanks! 61769345c78e04b0f756dcd15e39fe57ddb75c80 LGTM, please merge.

#22 Updated by Eric Biagiotti 19 days ago

  • Status changed from In Progress to Resolved

Also available in: Atom PDF