Make requests sent by the dashboard reverse proxy compatible

[niole]

This PR needs manual testing and tests in general.

Why are these changes needed?

Many users serve the ray dashboard behind a reverse proxy. At the moment, not all of the requests sent by the ray dashboard are formatted such that they will find the ray dashboard server when it is served behind a reverse proxy.

The kinds of users that want to do this are those that want to integrate ray into a larger web application. Large web applications often have complex architectures, lots of routes of their own, and reverse proxying in order to send requests to the right place. This change is key to enabling these types of web applications to integrate with ray.

key changes

requestHandlers.ts

This file formats urls such that they are "reverse proxy compatible". These changes should "just work".
I chose to create an HTTP request helper file because this will help safeguard future developers from accidentally writing requests that don't work behind a reverse proxy. This file should be treated as the canonical way to send HTTP requests in the dashboard.

working with api.ts

In order to not disrupt the entire dashboard app too much, I opted to not replace the usage of fetch in this file and instead export a url formatting function from the requestHandlers.ts file. The dashboard to use the centralized handlers only in the future.

I had to remove the use of URL, because URL relies on the use of an absolute base url in order to instantiate the URL object. This PRs changes are directly in conflict with how the URL object is instantiated. I manually verified this in Chrome. My changes to this file will need testing and extra attention.

how the dashboard is build

From now on, the dashboard will be built like PUBLIC_URL=".". The published ray package will be built with this env var. This makes it so that the static assets request URLs are relative to the path at which the dashboard is served.

user facing doc changes

The user facing docs for building ray from source should include the PUBLIC_URL="." setting.

Changelog

The changelog should mention that requests sent by the out-of-the-box dashboard will now be relative to the path at which the dashboard is served. It should warn that this may break reverse proxy rules and that there may no longer be a need for per-path reverse proxying. It should probably also give an example of what requests will look like now.

Related issue number

#8432

Checks

• I've run scripts/format.sh to lint the changes in this PR.
• I've included any doc changes needed for https://docs.ray.io/en/master/.
• I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
• Testing Strategy
  • Unit tests
  • Release tests
  • This PR is not tested :(

[simon-mo]

Awesome! This is definitely much much better than my failed attempt here: #7156

To clarify, if a user wants to add custom prefix, they would have to re-build the dashboard manually right?

[niole]

@simon-mo yes, the user has to rebuild the dashboard in order to get the api prefix changes.

[fyrestone]

cc @mxz96102

[rkooo567]

Please convert this to a regular PR and ping me!

[simon-mo]

Actually, @architkulkarni and I were discussing this and we actually have the use case to support arbitrary path prefix when spawning the dashboard server (not at js build time). @niole do you think #7156 can satisfy your requirement? If so we can revive that PR instead.

[niole]

@simon-mo the general idea of https://github.com/ray-project/ray/pull/7156/files looks like it would work. There are some things in my PR that i think are missing in that PR, like api calls in the services directory need the prefix too. Also there is an assumption that the prefix should be an absolute path, which in many cases isn't necessary and might make the solution a little inflexible.

Maybe I can cherry pick some commits from that PR into this one so that we can supply the path prefix and runtime instead of build time.

[simon-mo]

@niole that would be awesome!!

[mxz96102]

In my opinion, there is no need to let the user define the public path in front end, as long as we can use relative paths for the request and use hash history for the frontend route.

[mxz96102]

In our case, we access the dashboard through a platform proxy service with access control. By using relative paths and hash history, we allow proxy service using their arbitrary path and all the functions still work well. The point is, frontend isn't a single service, it binds with the backend API Server. So the API path root is always the same as the frontend path root.

[niole]

@mxz96102 can you link an example of the "platform proxy service" that you use and how you use it?

[niole]

I think we can just make this thing work behind a reverse proxy out of the box. The requests don't need to be told exactly where to go by using a path prefix. In order to achieve this, I think we do need to build the frontend with PUBLIC_URL="." as well, though. That will make it so that requests send by the create react app index.html can also be hit behind a reverse proxy. I think it is the most sane way to accomplish this, as well.

@simon-mo If I make a solution that doesn't depend on a path prefix, is that ok? The solution would also set an environment variable when the frontend is built for the shipped ray library and should also "just work" for all users.

[simon-mo]

@niole that sounds good. And just in case user actually wants path prefix out of the box, we can add that in our HTTP server layer. To clarify, what does PUBLIC_URL="." do and why would it help frontend do the path resolution? I can't find any documentation about it.

[niole]

@simon-mo https://create-react-app.dev/docs/using-the-public-folder/, the PUBLIC_URL environment variable will be taken into consideration by create-react-app at build time. It is used to set the value of process.env.PUBLIC_URL. create-react-app uses it to replace %PUBLIC_URL%, which you can see in the pre-build index.html: https://github.com/ray-project/ray/blob/master/dashboard/client/public/index.html#L5.
It is also interpolated as a prefix for other static assets like the chunk.js's that create-react-app generates at build time and interpolates into the final version of the index.html, which will be used at runtime.

setting it to "." makes it so that when the index.html is parsed by your browser, the browser will send GET requests that are relative to the path at which the dashboard is served. This makes this setting the right choice for making the asset GET requests work behind a reverse proxy.

[niole]

@simon-mo can we include .env.production.local in the git repo? https://github.com/ray-project/ray/blob/master/dashboard/client/.gitignore#L19. I could set PUBLIC_URL in there so that npm run build would use the value during the production build process. Does this make sense according the ray repo best practices? The variable could also be provided at each place where npm run build is called, for example: PUBLIC_URL="." npm run build.

[simon-mo]

Thanks for explaining! Including a .env.production.local sounds good. We should definitely set PUBLIC_URL="." by default.

[niole]

@simon-mo is there a way to write integration tests for this repo? Where would I write a test for confirming that the dashboard works behind a reverse proxy?

[simon-mo]

We are adding rendering test right here. #14253
For this PR I don't think we need to add a test, as long as we can test it manually showing it working. After the render test PR merged, we can open a separate PR to test it behind reverse proxy.

[simon-mo]

I'm testing it manually, seems like the static assets fail to load.

My testing steps, using traefik proxy:

1. Download the binary given your platform https://github.com/traefik/traefik/releases For mac: wget https://github.com/traefik/traefik/releases/download/v2.4.5/traefik_v2.4.5_darwin_amd64.tar.gz
2. tar zxf traefik_v2.4.5_darwin_amd64.tar.gz
3. write the following to a file config.toml

# http routing section
[http]
  [http.routers]
     [http.routers.to-dashboard]
      rule = "PathPrefix(`/ray/dashboard`)"
      middlewares = ["strip"]
      service = "dashboard"
  [http.middlewares]
    [http.middlewares.strip.stripPrefix]
      prefixes = ["/ray/dashboard"]
  [http.services]
    [http.services.dashboard.loadBalancer]
      [[http.services.dashboard.loadBalancer.servers]]
        url = "http://localhost:8265"

4. start traefik ./traefik --providers.file.filename=./conf.toml --entryPoints.web.address=:8081 --accesslog=true
5. ray start --head
6. visit localhost:8265, working
7. visit localhost:8001/ray/dashboard, not working.

[niole]

@simon-mo does it work if you add a / at the end of http://localhost:8081/ray/dashboard? for some reason it failed for me with no trailing slash, but worked if I added the /

if the requests still 404 with the /, please post the index.html file so I can take a look at the paths in the links/scripts (a screenshot of the elements tab in your browser also works)

[simon-mo]

it worked for me with trailing slash. Can you add a short section in the end of doc/source/ray-dashboard.rst about properly setting up reverse proxy and this edge case? (so users know how to set up proxy serving with trailing /) You can build the doc locally with https://docs.ray.io/en/latest/development.html#building-the-docs

[niole]

@simon-mo I added a section that explains the trailing / problem and gives an example reverse proxy config that works around the issue. Please give feedback on understandability and content

[niole]

@simon-mo Got this error when building the docs: https://docs.ray.io/en/latest/development.html#building-the-docs.

ERROR: sphinx-tabs 2.0.1 has requirement sphinx<4,>=2, but you'll have sphinx 1.8.5 which is incompatible.
ERROR: sphinx-book-theme 0.0.39 has requirement docutils>=0.15, but you'll have docutils 0.14 which is incompatible.

So I updated the version of the offending sphynx dependency.

[simon-mo]

Looking good. Thanks for the contribution @niole!

[niole]

@simon-mo do you know what the release plan is for ray 2.0.0?

[rkooo567]

Just lyk the next version will be 1.3.0, not 2.0 (2.0 is the master version). We are doing a monthly release, so that will be in about 3 weeks!

[niole]

@rkooo567 Ok, thank you. Also, sounds like I referenced the wrong version in the docs changes that I made for the ray dashboard. I will open a PR to change it to 1.3.0 or remove it entirely