Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DASH_PROXY #1289

Merged
merged 4 commits into from
Jun 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).

## [UNRELEASED]
### Added
- [#1289](https://github.com/plotly/dash/pull/1289) Supports `DASH_PROXY` env var to tell `app.run_server` to report the correct URL to view your app, when it's being proxied. Throws an error if the proxy is incompatible with the host and port you've given the server.
- [#1240](https://github.com/plotly/dash/pull/1240) Adds `callback_context` to clientside callbacks (e.g. `dash_clientside.callback_context.triggered`). Supports `triggered`, `inputs`, `inputs_list`, `states`, and `states_list`, all of which closely resemble their serverside cousins.

### Changed
Expand Down
75 changes: 55 additions & 20 deletions dash/dash.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
from __future__ import print_function

import itertools
import os
import random
import sys
import collections
import importlib
Expand All @@ -14,6 +12,7 @@
import mimetypes

from functools import wraps
from future.moves.urllib.parse import urlparse

import flask
from flask_compress import Compress
Expand All @@ -25,7 +24,7 @@
from .fingerprint import build_fingerprint, check_fingerprint
from .resources import Scripts, Css
from .development.base_component import ComponentRegistry
from .exceptions import PreventUpdate, InvalidResourceError
from .exceptions import PreventUpdate, InvalidResourceError, ProxyError
from .version import __version__
from ._configs import get_combined_config, pathname_configs
from ._utils import (
Expand Down Expand Up @@ -1332,7 +1331,8 @@ def enable_dev_tools(

if dev_tools.silence_routes_logging:
logging.getLogger("werkzeug").setLevel(logging.ERROR)
self.logger.setLevel(logging.INFO)

self.logger.setLevel(logging.INFO)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't do all that much with self.logger but what we do we always want the user to see - not just when silencing built-in route logging.


if dev_tools.hot_reload:
_reload = self._hot_reload
Expand Down Expand Up @@ -1449,6 +1449,7 @@ def run_server(
self,
host=os.getenv("HOST", "127.0.0.1"),
port=os.getenv("PORT", "8050"),
proxy=os.getenv("DASH_PROXY", None),
debug=False,
dev_tools_ui=None,
dev_tools_props_check=None,
Expand All @@ -1475,6 +1476,14 @@ def run_server(
env: ``PORT``
:type port: int

:param proxy: If this application will be served to a different URL
via a proxy configured outside of Python, you can list it here
as a string of the form ``"{input}::{output}"``, for example:
``"http://0.0.0.0:8050::https://my.domain.com"``
so that the startup message will display an accurate URL.
env: ``DASH_PROXY``
:type proxy: string

:param debug: Set Flask debug mode and enable dev tools.
env: ``DASH_DEBUG``
:type debug: bool
Expand Down Expand Up @@ -1555,25 +1564,51 @@ def run_server(
]
raise

if self._dev_tools.silence_routes_logging:
# Since it's silenced, the address doesn't show anymore.
# so we only see the "Running on" message once with hot reloading
# https://stackoverflow.com/a/57231282/9188800
if os.getenv("WERKZEUG_RUN_MAIN") != "true":
ssl_context = flask_run_options.get("ssl_context")
self.logger.info(
"Running on %s://%s:%s%s",
"https" if ssl_context else "http",
host,
port,
self.config.requests_pathname_prefix,
)
protocol = "https" if ssl_context else "http"
path = self.config.requests_pathname_prefix

if proxy:
served_url, proxied_url = map(urlparse, proxy.split("::"))

def verify_url_part(served_part, url_part, part_name):
if served_part != url_part:
raise ProxyError(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we happy with proxy errors throwing, or would it be better to just show a warning but keep going?

Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand this: If we throw an error when the served_url portion of DASH_PROXY does not match what's already available in protocol/host/port either through defaults or env variables, why force the user to provide it in the first place? Why not just reuse what we already have and use DASH_PROXY only for the proxied url part?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DASH_PROXY presumably comes from some system configuration at a level that's inaccessible to Dash itself, it's just being reported by the environment as something Dash needs to obey.

But I guess that brings up the option to do the reverse of what you're suggesting: ignore host and port when we have a DASH_PROXY - maybe throw a warning or an error if the user tries to override those and they conflict with the proxy, otherwise just run with the values indicated by the proxy.

I don't think we can do that with the protocol, because if you're using https you also need to provide the cert and key. But I get the impression (@gzork can you confirm this?) that at least in our own use of such proxies, encryption is handled by the proxy itself so DASH_PROXY will look like http://<local>::https://<public>. So if the user did provide ssl_context when the proxy indicates http it would be an error but that's unlikely to happen accidentally. And the reverse won't happen if the proxy is handling encryption and this is correctly reflected in DASH_PROXY.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at least in our own use of such proxies, encryption is handled by the proxy itself so DASH_PROXY will look like http://::https://.

That's correct, in our case the encryption is handled by haproxy on the Dash Enterprise server, and I think it's safe to generally also assume that it will be handled by the proxy itself, a note either in the docs or elsewhere would still be good though.

"""
{0}: {1} is incompatible with the proxy:
{3}
To see your app at {4},
you must use {0}: {2}
""".format(
part_name,
url_part,
served_part,
proxy,
proxied_url.geturl(),
)
)

verify_url_part(served_url.scheme, protocol, "protocol")
verify_url_part(served_url.hostname, host, "host")
verify_url_part(served_url.port, port, "port")

# Generate a debugger pin and log it to the screen.
debugger_pin = os.environ["WERKZEUG_DEBUG_PIN"] = "-".join(
itertools.chain(
"".join([str(random.randint(0, 9)) for _ in range(3)])
for _ in range(3)
display_url = (
proxied_url.scheme,
proxied_url.hostname,
(":{}".format(proxied_url.port) if proxied_url.port else ""),
path,
)
)
else:
display_url = (protocol, host, ":{}".format(port), path)

self.logger.info("Dash is running on %s://%s%s%s\n", *display_url)
self.logger.info(" Warning: This is a development server. Do not use app.run_server")
self.logger.info(" in production, use a production WSGI server like gunicorn instead.\n")

self.logger.info("Debugger PIN: %s", debugger_pin)
if not os.environ.get("FLASK_ENV"):
os.environ["FLASK_ENV"] = "development"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The complete message you get now (ours plus the built-in Flask message) looks like:

$ python app1.py
Dash is running on http://127.0.0.1:8050/

 Warning: This is a development server. Do not use app.run_server
 in production, use a production WSGI server like gunicorn instead.

 * Serving Flask app "app1" (lazy loading)
 * Environment: development
 * Debug mode: on


self.server.run(host=host, port=port, debug=debug, **flask_run_options)
4 changes: 4 additions & 0 deletions dash/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,7 @@ class MissingCallbackContextException(CallbackException):

class UnsupportedRelativePath(CallbackException):
pass


class ProxyError(DashException):
pass
60 changes: 60 additions & 0 deletions tests/unit/test_configs.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,3 +254,63 @@ def test_port_env_fail_range(empty_environ):
excinfo.exconly()
== "AssertionError: Expecting an integer from 1 to 65535, found port=65536"
)


def test_no_proxy_success(mocker, caplog, empty_environ):
app = Dash()

# mock out the run method so we don't actually start listening forever
mocker.patch.object(app.server, "run")

app.run_server(port=8787)

assert "Dash is running on http://127.0.0.1:8787/\n" in caplog.text


@pytest.mark.parametrize(
"proxy, host, port, path",
[
("https://daash.plot.ly", "127.0.0.1", 8050, "/"),
("https://daaash.plot.ly", "0.0.0.0", 8050, "/a/b/c/"),
("https://daaaash.plot.ly", "127.0.0.1", 1234, "/"),
("http://go.away", "127.0.0.1", 8050, "/now/"),
("http://my.server.tv:8765", "0.0.0.0", 80, "/"),
],
)
def test_proxy_success(mocker, caplog, empty_environ, proxy, host, port, path):
proxystr = "http://{}:{}::{}".format(host, port, proxy)
app = Dash(url_base_pathname=path)
mocker.patch.object(app.server, "run")

app.run_server(proxy=proxystr, host=host, port=port)

assert "Dash is running on {}{}\n".format(proxy, path) in caplog.text


def test_proxy_failure(mocker, empty_environ):
app = Dash()

# if the tests work we'll never get to server.run, but keep the mock
# in case something is amiss and we don't get an exception.
mocker.patch.object(app.server, "run")

with pytest.raises(_exc.ProxyError) as excinfo:
app.run_server(
proxy="https://127.0.0.1:8055::http://plot.ly", host="127.0.0.1", port=8055
)
assert "protocol: http is incompatible with the proxy" in excinfo.exconly()
assert "you must use protocol: https" in excinfo.exconly()

with pytest.raises(_exc.ProxyError) as excinfo:
app.run_server(
proxy="http://0.0.0.0:8055::http://plot.ly", host="127.0.0.1", port=8055
)
assert "host: 127.0.0.1 is incompatible with the proxy" in excinfo.exconly()
assert "you must use host: 0.0.0.0" in excinfo.exconly()

with pytest.raises(_exc.ProxyError) as excinfo:
app.run_server(
proxy="http://0.0.0.0:8155::http://plot.ly", host="0.0.0.0", port=8055
)
assert "port: 8055 is incompatible with the proxy" in excinfo.exconly()
assert "you must use port: 8155" in excinfo.exconly()