Add endpoint configuration query #2024

dbutenhof · 2020-12-04T16:46:41Z

The advertised endpoints are self-configuring based on reverse-proxy configuration as long as the reverse-proxy service advertises the configured external host address either via Forward: [...];host=<name>[;...] or by X-Forwarded-Host: <host>[,...] (note that in the latter case, the first hostname given is presumed to be the first and most appropriate proxy).

The following sample is generated by a GET <host>/api/v1/endpoints with the header X-Forwarded-Host: pbench.perf.lab.eng.bos.redhat.com:8902, for example:

{
  "api": {
    "controllers_list": "http://pbench.perf.lab.eng.bos.redhat.com:8902/api/v1/api/v1/controllers/list",
    "controllers_months": "http://pbench.perf.lab.eng.bos.redhat.com:8902/api/v1/api/v1/controllers/months",
    "elasticsearch": "http://pbench.perf.lab.eng.bos.redhat.com:8902/api/v1/api/v1/elasticsearch",
    "endpoints": "http://pbench.perf.lab.eng.bos.redhat.com:8902/api/v1/api/v1/endpoints",
    "graphql": "http://pbench.perf.lab.eng.bos.redhat.com:8902/api/v1/api/v1/graphql",
    "host_info": "http://pbench.perf.lab.eng.bos.redhat.com:8902/api/v1/api/v1/host_info",
    "login": "http://pbench.perf.lab.eng.bos.redhat.com:8902/api/v1/api/v1/login",
    "logout": "http://pbench.perf.lab.eng.bos.redhat.com:8902/api/v1/api/v1/logout",
    "register": "http://pbench.perf.lab.eng.bos.redhat.com:8902/api/v1/api/v1/register",
    "upload_ctrl": "http://pbench.perf.lab.eng.bos.redhat.com:8902/api/v1/api/v1/upload/ctrl/",
    "user": "http://pbench.perf.lab.eng.bos.redhat.com:8902/api/v1/api/v1/user/"
  },
  "identification": "Pbench server 0.71.0-11gdb60e9279",
  "indices": {
    "result_data_index": "drb.v5.result-data.",
    "result_index": "drb.v5.result-data-sample.",
    "run_index": "drb.v6.run-data.",
    "run_toc_index": "drb.v6.run-toc."
  }
}

This requires that the dashboard do a fetch back through the origin address with suffix /api/v1/endpoints, and stash the resulting JSON somewhere. I have a prototype dashboard branch where I've simply replaced the <script> in document.ejs with a more dynamic version that amounts to effectively the same HTTP and Javascript operations performed to load the static endpoints.js used by the current code:

  <!-- runtime endpoints config -->
  <script type="text/javascript">
    var response = await fetch(window.origin + "/api/v1/endpoints");
    window.endpoints = await response.json();
  </script>

(Though whether this is the "best" way to do it may be another question, it works... of course with appropriate changes to the endpoint references through the code, and I haven't yet gotten the e2e tests debugged enough to push a draft PR.)

NOTE

The user endpoint in Flask ends with a parameter template string; I decided to remove that from the endpoint, but leave the trailing / so the dashboard can simply append the username on the URI. The simple parsing here won't work if we end up with a template string that's not at the end, but that's agile.

portante

Instead of a separate API endpoint, why not render the web page served to the browser with the information embedded in the rendered page?

dbutenhof · 2020-12-07T12:56:18Z

Instead of a separate API endpoint, why not render the web page served to the browser with the information embedded in the rendered page?

One big reason is that this is part of the server, not the dashboard. It can be used by curl, or Postman, or any other client to learn the server configuration.

Now, we still need the client to know how to do that first query to find the configuration, and that needs to be known externally somehow. That's where your suggestion might come in, on the dashboard side.

It also occurs to me that a GET server:8001 probably should return this information rather than requiring the client to know server:8001/api/v1/endpoints. Then if we do a "v2" API the default unqualified endpoint would return the latest version info.

Maybe it should even identify itself as "server": "Pbench server 0.71-xxx" or some such.

gurbirkalsi · 2020-12-09T02:01:34Z

@dbutenhof With the endpoint configuration being integrated into the dashboard binary, is this server endpoint going to be a starting point for the deployment process to populate config/endpoints.js before running yarn build and eventually moving it to the target server?

gurbirkalsi · 2020-12-09T05:20:07Z

I've addressed these requirements in distributed-system-analysis/pbench-dashboard#110. Please let me know if these changes meet the requirements for deploying the dashboard binary with a standalone config.

lib/pbench/server/api/resources/endpoint_configure.py

webbnh

Looks good, but I found a couple of nits, and I have a bunch of pointed questions.

lib/pbench/server/api/resources/endpoint_configure.py

webbnh · 2021-03-24T17:37:36Z

lib/pbench/server/api/resources/endpoint_configure.py

+                # If the URI is parameterized with a Flask "<type:name>"
+                # template string, we don't want to report it, so we remove
+                # it from the URI. We derive an API name by converting the


Would it be more useful to report the template string with JS template arguments in it? E.g.,

param_template = re.compile(r"<(?P<type_name>\w+):(?P<param_name>\w+)>") [...] url = self.param_template.sub("${\g<param_name>}", url)

such that "/api/v1/foo/<string:name>/detail/<string:param>" would be reported as ".../api/v1/foo/${name}/detail/${param}".

The client has to remove/substitute the templates then and I figured that wasn't very generic. I didn't really want to make it so Javascript-specific, either, and Javascript will only do that sort of substitution in literal template strings (backticked), so far as I can tell; although it's just a .replace away, it's not quite that neat.

Anyway, just letting it append the username to the end of the URI seemed easiest, at least for now.

I second the idea of not making this Javascript specific.

I guess I was proposing two things:

make the URI available as a template

make it easy for the client to fill in the template (which is why I offered JS syntax)

It doesn't have to be Javascript format, but I'll point out that that format also works for shell, so it's not exactly JS-specific. ;-) We could use %s instead of ${...}, but I opted for the latter because it provides semantic information.

It's true that the client will likely need to have some foreknowledge in order to fill in the template, but it will need to have even closer coupling to produce the URI if we just lop it off at the first parameter. Providing a template removes the need for some of that coupling.

Apparently there is a (proposed) IETF standard for URI templates.

Let's track in issue #2154.

lib/pbench/server/api/resources/endpoint_configure.py

portante

Also needs a rebase.

lib/pbench/server/api/resources/endpoint_configure.py

portante · 2021-03-25T21:26:20Z

lib/pbench/server/api/resources/endpoint_configure.py

+                # If the URI is parameterized with a Flask "<type:name>"
+                # template string, we don't want to report it, so we remove
+                # it from the URI. We derive an API name by converting the


I second the idea of not making this Javascript specific.

This defines a new API which will supply a client (e.g., the UI dashboard) with all the information it needs to adapt to this particular server instance including full API URIs and metadata like index names.

This API was previously assuming direct access to the Pbench server and API port. While we have a default Apache reverse-proxy configuration in our Ansible config role, that means we expect the client to access our services through the http port (80) rather than the configured server listening port (e.g., 8001). However, with an external reverse proxy, we don't want to advertise access to the local host/port at all; instead we want to point clients to the configured reverse proxy host and port. This adds a new [pbench-server] config variable called proxy_host, which by default mirrors the default host name on port 80 but can be altered to direct the client to access the reverse-proxy port; for example, proxy_host=pbench.lab.example.com:8901 will report an endpoint configuration directing all endpoints through that host and port.

Note that the unit test remains as it was, as a check that we're generating the proper set; this will have to be maintained as our API expands, but that seems like a worthwhile validation.

webbnh

Looks good. I just have one question.

webbnh · 2021-03-26T16:59:19Z

lib/pbench/server/api/resources/endpoint_configure.py

 import re
-from flask.globals import current_app
+from logging import Logger

+from flask.globals import current_app
 from flask_restful import Resource, abort
 from flask import request, jsonify
 from urllib.parse import urljoin

+from pbench.server import PbenchServerConfig
 from pbench.server.api.resources.query_apis import get_index_prefix


What is our rubric for structuring import statements?

I get keeping the pbench ones separate and having them follow the "system" ones, but what's the distinction which sets re and logging apart from flask and urllib?

Typically what PEP 8 talks about, which is alphabetical order grouped by imports first, from next, where standard library are grouped, then external libraries, then project imports.

The Pbench server API listens on port 8001; we want to be able to access the API externally through normal HTTP/HTTPS. With an external reverse proxy gateway (e.g., NGINX) this can be accomplished by redirecting (e.g.) pbench.example.com:/api/ to the actual server at port 8001 and opening the 8001/tcp port in the server firewall. Our default Ansible deployment mechanism configures a local Apache which is directly accessed from outside. This PR adds an Apache reverse proxy to allow external API access on port 80. With these changes, the Pbench gunicorn wsgi server will still listen on port 8001, but this port won't need to be opened externally as Apache will proxy /api/ to port 8001. This means that all external accesses will go to port 80 (http) or (when we get there) 433 (https). In the short term this affects the pbench_server configuration in the dashboard endpoints.js. Once distributed-system-analysis#2024 (server-side endpoint configuration and metadata) is merged, I plan to change the dashboard to use that, simply fetch-ing that from window.origin and assigning it to window.endpoints within the launch html page, removing the need to manage a local static endpoints.js entirely.

dbutenhof requested review from portante, gurbirkalsi, FuqingWang and npalaska December 4, 2020 16:46

This comment has been minimized.

Sign in to view

dbutenhof linked an issue Dec 4, 2020 that may be closed by this pull request

Implement a server endpoint to return configuration data for the Dashboard #2018

Closed

portante requested changes Dec 5, 2020

View reviewed changes

portante assigned dbutenhof Dec 7, 2020

portante added enhancement Dashboard Of and relating to the Dashboard GUI Server labels Dec 7, 2020

portante added this to the v0.71 milestone Dec 7, 2020

This comment has been minimized.

Sign in to view

dbutenhof mentioned this pull request Dec 9, 2020

Update Ansible configuration for deploying dashboard using runtime environment variables distributed-system-analysis/pbench-dashboard#110

Merged

dbutenhof mentioned this pull request Jan 6, 2021

Dashboard support for ES7 / Pbench server 0.71 distributed-system-analysis/pbench-dashboard#114

Merged

dbutenhof force-pushed the config-api branch from 7c630b0 to 9612bb1 Compare January 7, 2021 22:03

portante previously approved these changes Jan 8, 2021

View reviewed changes

lib/pbench/server/api/resources/endpoint_configure.py Outdated Show resolved Hide resolved

dbutenhof dismissed portante’s stale review via 447c6fc January 8, 2021 13:58

dbutenhof force-pushed the config-api branch from 9612bb1 to 447c6fc Compare January 8, 2021 13:58

dbutenhof requested a review from portante January 8, 2021 15:23

dbutenhof force-pushed the config-api branch from 447c6fc to e5067eb Compare January 8, 2021 17:26

portante reviewed Jan 8, 2021

View reviewed changes

lib/pbench/server/api/resources/endpoint_configure.py Outdated Show resolved Hide resolved

portante previously approved these changes Jan 8, 2021

View reviewed changes

dbutenhof requested review from portante and webbnh March 23, 2021 13:24

portante approved these changes Mar 23, 2021

View reviewed changes

webbnh previously approved these changes Mar 24, 2021

View reviewed changes

portante requested changes Mar 25, 2021

View reviewed changes

dbutenhof added 6 commits March 25, 2021 17:32

Add endpoint configuration query

44dab29

This defines a new API which will supply a client (e.g., the UI dashboard) with all the information it needs to adapt to this particular server instance including full API URIs and metadata like index names.

Rebase

9057b8a

Move to automatic reverse-proxy configuration

3be2ed1

Change to dynamically construct API set from Flask

71578da

Note that the unit test remains as it was, as a check that we're generating the proper set; this will have to be maintained as our API expands, but that seems like a worthwhile validation.

Review comments & cleanup

9cbd99f

dbutenhof dismissed webbnh’s stale review via 9cbd99f March 25, 2021 21:32

dbutenhof force-pushed the config-api branch from f365ec0 to 9cbd99f Compare March 25, 2021 21:32

Tweak to log output

a7ddfc4

dbutenhof requested review from portante and webbnh March 26, 2021 13:15

webbnh approved these changes Mar 26, 2021

View reviewed changes

portante mentioned this pull request Mar 26, 2021

Report the template string with JS template arguments in pbench server config API #2154

Closed

portante approved these changes Mar 26, 2021

View reviewed changes

portante merged commit 3912bb7 into distributed-system-analysis:main Mar 26, 2021

portante mentioned this pull request Apr 12, 2021

Resolve reverse-proxy configuration issues in server setup #2099

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add endpoint configuration query #2024

Add endpoint configuration query #2024

dbutenhof commented Dec 4, 2020 •

edited

Loading

This comment has been minimized.

portante left a comment

dbutenhof commented Dec 7, 2020 •

edited

Loading

gurbirkalsi commented Dec 9, 2020 •

edited

Loading

This comment has been minimized.

gurbirkalsi commented Dec 9, 2020

webbnh left a comment

webbnh Mar 24, 2021

dbutenhof Mar 25, 2021 •

edited by portante

Loading

portante Mar 25, 2021

webbnh Mar 26, 2021

portante Mar 26, 2021

portante left a comment

portante Mar 25, 2021

webbnh left a comment

webbnh Mar 26, 2021

portante Mar 26, 2021

Add endpoint configuration query #2024

Add endpoint configuration query #2024

Conversation

dbutenhof commented Dec 4, 2020 • edited Loading

This comment has been minimized.

portante left a comment

Choose a reason for hiding this comment

dbutenhof commented Dec 7, 2020 • edited Loading

gurbirkalsi commented Dec 9, 2020 • edited Loading

This comment has been minimized.

gurbirkalsi commented Dec 9, 2020

webbnh left a comment

Choose a reason for hiding this comment

webbnh Mar 24, 2021

Choose a reason for hiding this comment

dbutenhof Mar 25, 2021 • edited by portante Loading

Choose a reason for hiding this comment

portante Mar 25, 2021

Choose a reason for hiding this comment

webbnh Mar 26, 2021

Choose a reason for hiding this comment

portante Mar 26, 2021

Choose a reason for hiding this comment

portante left a comment

Choose a reason for hiding this comment

portante Mar 25, 2021

Choose a reason for hiding this comment

webbnh left a comment

Choose a reason for hiding this comment

webbnh Mar 26, 2021

Choose a reason for hiding this comment

portante Mar 26, 2021

Choose a reason for hiding this comment

dbutenhof commented Dec 4, 2020 •

edited

Loading

dbutenhof commented Dec 7, 2020 •

edited

Loading

gurbirkalsi commented Dec 9, 2020 •

edited

Loading

dbutenhof Mar 25, 2021 •

edited by portante

Loading