lyft / presto-gateway

A load balancer / proxy / gateway for prestodb
Apache License 2.0
358 stars 156 forks source link

Add ExternalUrl to Active Backends Page #105

Closed axelsteingrimsson closed 4 years ago

axelsteingrimsson commented 4 years ago

When running Presto Gateway on Kubernetes pods in Google Cloud Platform the backend URL uses an internal endpoint for requests. This works fine until a user tries to use the URL on the Active Backends page in the UI. This uses the internal hostname and therefore cannot be reached by users outside of the network. To work around this I'd like to add an optional externalUrl field that can be used to point to an external hostname. The field could be passed in like this:

curl -X POST http://localhost:8080/entity\?entityType\=GATEWAY_BACKEND \
-d '{"name":"presto1", \
     "proxyTo": "presto1.lyft.com", \
     "externalUrl": "http://my-external-url.com", \
     "active": true, \
     "routingGroup": "adhoc"}'

curl -X POST http://localhost:8080/entity\?entityType\=GATEWAY_BACKEND \
-d '{"name":"presto2", \
     "proxyTo": "presto2.lyft.com", \
     "active": true, \
     "routingGroup": "adhoc"}'

If the externalUrl field is passed in then it will appear in the Active Backends UI under the URL heading instead of the proxyTo value. If it is not passed in then the value passed into proxyTo will appear in it's place, just as normal.

Here is an example (on my local development) using the externalUrl field to override the proxyTo field in the Active Backends page using the requests from above:

Screen Shot 2020-04-16 at 11 29 44 AM

I didn't see a reason to include both proxyTo and externalUrl if both are present, but if there is any reason they are both needed I can change it so that both are presented in the page.

puneetjaiswal commented 4 years ago

Though it is a nice feature, we did not have a use case where a user had to open the individual clusters (this page was meant for informative purposes and for admin use only) in the browser. Users can still click on their respective query id and that will open the query UI pointing to respective presto-cluster via the gateway.

axelsteingrimsson commented 4 years ago

In the way that we have Presto Gateway, as specified in the PR description, the URL in the active backends doesn't point to anywhere useful. Because we use the gateway with scheduled and ad-hoc clusters, it would be ideal for our users to also be able to use the URL to connect to the appropriate cluster. As it's not breaking any functionality, is it blocking from adding a feature like this in?

puneetjaiswal commented 4 years ago

In the way that we have Presto Gateway, as specified in the PR description, the URL in the active backends doesn't point to anywhere useful. Because we use the gateway with scheduled and ad-hoc clusters, it would be ideal for our users to also be able to use the URL to connect to the appropriate cluster. As it's not breaking any functionality, is it blocking from adding a feature like this in?

it would be ok to have this merged.

axelsteingrimsson commented 4 years ago

Alright, that would be great. Sorry for the delay on the CLA. Would you be able to review it?

puneetjaiswal commented 4 years ago

+1

puneetjaiswal commented 4 years ago

PTAL @hitejinder

axelsteingrimsson commented 4 years ago

+1