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
pybind/mgr/dashboard: fix reverse proxy support #19758
Conversation
src/pybind/mgr/dashboard/module.py
Outdated
@@ -65,12 +65,24 @@ def recurse_refs(root, path): | |||
def get_prefixed_url(url): | |||
return global_instance().url_prefix + url | |||
|
|||
def prepare_url_prefix(url_prefix): | |||
if url_prefix == None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, might want to prefer is
over ==
, see http://legacy.python.org/dev/peps/pep-0008/#programming-recommendations
src/pybind/mgr/dashboard/module.py
Outdated
else: | ||
if len(url_prefix) != 0: | ||
if url_prefix[0] != '/': | ||
url_prefix = '/'+url_prefix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could use
url_prefix = urlparse.urljoin('/', url_prefix)
src/pybind/mgr/dashboard/module.py
Outdated
if len(url_prefix) != 0: | ||
if url_prefix[0] != '/': | ||
url_prefix = '/'+url_prefix | ||
if url_prefix[-1] == '/': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could just put:
url_prefix = url_prefix.rstrip('/')
src/pybind/mgr/dashboard/module.py
Outdated
@@ -65,12 +65,24 @@ def recurse_refs(root, path): | |||
def get_prefixed_url(url): | |||
return global_instance().url_prefix + url | |||
|
|||
def prepare_url_prefix(url_prefix): | |||
if url_prefix == None: | |||
url_prefix = '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably we can use #19948 to simplify this a little bit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll change this as soon as #19948 is merged.
Thank you for your feedback! I added most of your suggestions, see my other comment. |
This fixes http redirection for reverse http proxies Fixes: http://tracker.ceph.com/issues/22557 Signed-off-by: Nick Erdmann <n@nirf.de>
Signed-off-by: Nick Erdmann <n@nirf.de>
@tchaikov @nrdmn yeah, that looks like it was broken by this PR -- the call to set_uri on an active mgr should be setting something that matches what the standby redirects to (i.e. both should to have the trailing slash, or neither should) |
👍 👍 👍 |
Changes in 4f7007d break support for reverse http proxies by always redirecting to / on standby MGRs. This patch should fix it.
Fixes: http://tracker.ceph.com/issues/22557
Signed-off-by: Nick Erdmann n@nirf.de