Add support for serving additional metrics provider JS in the UI

[banks]

This PR builds on #8694 and implements the JS provider serving component for the UI.

TODO

• All documentation for the new config and features is deferred until they are implemented as things might change but I want to keep PRs small and incremental.

[banks]

I realised this needs to be conditional on environment === production otherwise it prints this junk into the page in UI dev mode.

[dnephin]

I have not had a chance to read the RFC, so I apologize if this is already documented somewhere.

If I am understanding this PR correctly, we are merging a list of javascript files into a single file and serving it at a single URL. Could we have also used a plain https://golang.org/pkg/net/http/#FileServer to serve these files?

[dnephin]

I believe this file is never closed. It would probably have a defer f.Close(), right? Since this is in a loop, I guess we need to extract most of this out into a new function so that the defer happens before the next iteration of the loop.

A separate function should also simplify this error handling by returning an error and allowing us to only write the error response in one place.

[banks]

Separate func makes this way cleaner thanks. Great suggestion.

[banks]

Thanks Daniel. Good catches.

Could we have also used a plain https://golang.org/pkg/net/http/#FileServer to serve these files?

We could. It seemed a little cleaner to bundle them server side so that the UI only has to have a single script tag injected to load them rather than being passed a whole list of URLs, you'd also have to worry about naming, validating that only JS is being served from that prefix if you allow full directory serving, directory traversals on the server etc.

[dnephin]

FileServer accepts a https://golang.org/pkg/net/http/#FileSystem. It seems like we could probably restrict the files provided by the http.FileSystem by doing exact path matches. What kind of naming problems would we need to handle?

It seemed a little cleaner to bundle them server side so that the UI only has to have a single script tag injected to load them rather than being passed a whole list of URLs

Isn't it currently doing a range over a list of urls (ExtraScripts) ?

[banks]

What kind of naming problems would we need to handle?

For example if the user specified three files in three different directories: /foo/bar.js, /bar/baz.js, /home/users/sillyusername/bar.js what URL paths should the be served at?

1. Option 1: /ui/assets/:full_absolute_path. It seems gross to serve them at full absolute URL paths as that exposes the underlying filesystem structure publicly which is not great. e.g. /ui/assets/home/users/sillyusername/bar.js would be both surprising and unpleasant for users I think.

2. Option 2: flatten them in some way onto a single path: e.g. /ui/assets/bar.js except now we have two differnt bar.js files so we'd have to add some random string or something to disambiguate again and thread those to the script tags which seems like unnecessary complication.

So having considered those two I thought that it was simpler all round just having a single well-known file for the script tag and serving them all without exposing anything additional from the filesystem.

Isn't it currently doing a range over a list of urls (ExtraScripts) ?

It is - that was a later thought that since i was adding the machinery I might as well make it generic and an array so if we ever need to do anything else similar (inject a JS dep conditionally at runtime) we can re-use it. I'd be fine with making it more specific again if you think that's cleaner though.

Honestly I don't have a strong opinion here but this seemed simplest thing that would work without tricky decisions about name mangling or exposing too much!

FWIW we already us http.FileServer here for the underlying serving whether it's from AssetFS or an external UI dir. I choose not to plumb multiple additional separate FileServers at different paths for the above reasons.

Does that make sense or do you still think there is significant benefit revisiting this design choice? I don't think what we are doing is a one-way-door either. It's only internal UI components that are affected so we can always change later if there turns out to be other downsides?

[dnephin]

Makes sense. My thinking is that while there may be some issues to work around with the separate files approach, at least those issues are pretty well know. Serving static files from a web server is a common practice.

On the other hand merging a bunch of JS files into one seems less common (other than frameworks which can impose strict requirements on the contents of the files). I'm not sure if there are rough edges or potential risks with merging the files. At first I thought maybe variable name collisions, but I guess those would exist anyway if all the files end up on the same page. So maybe there aren't any hidden problems here, I'm not sure.

Option 2 from above sounds pretty appealing to me, but as you say, this should be relatively easy to change if we need to, so this approach of merging files is probably safe for now.

[rboyer]

LGTM