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.

Sign up for GitHub

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

add hub_url_local as hub_url currently represents both public and local #994

Merged
merged 5 commits into from
Oct 31, 2020
Merged

add hub_url_local as hub_url currently represents both public and local #994

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6b1dce9
allow different internal and public hub_url
manics Nov 2, 2019
181efcd
user hub_url_public for auth config if set
manics Nov 2, 2019
a23550c
Remove hub_url_public, add hub_url_local
manics Oct 30, 2020
3d10c9f
Use hub_url_local for internal BinderHub➡️ JupyterHub traffic
manics Oct 30, 2020
50e56a0
deployment: check hub_url_local for JUPYTERHUB_API_URL
manics Oct 30, 2020
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
20 changes: 18 additions & 2 deletions binderhub/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,22 @@ def _default_hub_token(self):
""",
config=True,
)
@validate('hub_url')

hub_url_local = Unicode(
help="""
The base URL of the JupyterHub instance for local/internal traffic

If local/internal network connections from the BinderHub process should access
JupyterHub using a different URL than public/external traffic set this, default
is hub_url
""",
config=True,
)
@default('hub_url_local')
def _default_hub_url_local(self):
return self.hub_url

@validate('hub_url', 'hub_url_local')
def _add_slash(self, proposal):
"""trait validator to ensure hub_url ends with a trailing slash"""
if proposal.value is not None and not proposal.value.endswith('/'):
Expand Down Expand Up @@ -577,6 +592,7 @@ def initialize(self, *args, **kwargs):
self.launcher = Launcher(
parent=self,
hub_url=self.hub_url,
hub_url_local=self.hub_url_local,
hub_api_token=self.hub_api_token,
create_user=not self.auth_enabled,
)
Expand Down Expand Up @@ -665,7 +681,7 @@ def initialize(self, *args, **kwargs):
tornado.web.StaticFileHandler,
{'path': os.path.join(self.tornado_settings['static_path'], 'images')}),
(r'/about', AboutHandler),
(r'/health', HealthHandler, {'hub_url': self.hub_url}),
(r'/health', HealthHandler, {'hub_url': self.hub_url_local}),
(r'/', MainHandler),
(r'.*', Custom404),
]
Expand Down
8 changes: 6 additions & 2 deletions binderhub/launcher.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from tornado import web, gen
from tornado.httpclient import AsyncHTTPClient, HTTPRequest, HTTPError
from traitlets.config import LoggingConfigurable
from traitlets import Integer, Unicode, Bool
from traitlets import Integer, Unicode, Bool, default
from jupyterhub.traitlets import Callable
from jupyterhub.utils import maybe_future

Expand All @@ -34,6 +34,10 @@ class Launcher(LoggingConfigurable):

hub_api_token = Unicode(help="The API token for the Hub")
hub_url = Unicode(help="The URL of the Hub")
hub_url_local = Unicode(help="The internal URL of the Hub if different")
@default('hub_url_local')
def _default_hub_url_local(self):
return self.hub_url
create_user = Bool(True, help="Create a new Hub user")
allow_named_servers = Bool(
os.getenv('JUPYTERHUB_ALLOW_NAMED_SERVERS', "false") == "true",
Expand Down Expand Up @@ -81,7 +85,7 @@ async def api_request(self, url, *args, **kwargs):
"""Make an API request to JupyterHub"""
headers = kwargs.setdefault('headers', {})
headers.update({'Authorization': 'token %s' % self.hub_api_token})
hub_api_url = os.getenv('JUPYTERHUB_API_URL', '') or self.hub_url + 'hub/api/'
hub_api_url = os.getenv('JUPYTERHUB_API_URL', '') or self.hub_url_local + 'hub/api/'
request_url = hub_api_url + url
req = HTTPRequest(request_url, *args, **kwargs)
retry_delay = self.retry_delay
Expand Down
2 changes: 1 addition & 1 deletion helm-chart/binderhub/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ spec:
key: "binder.hub-token"
{{- if .Values.config.BinderHub.auth_enabled }}
- name: JUPYTERHUB_API_URL
value: {{ (print (.Values.config.BinderHub.hub_url | trimSuffix "/") "/hub/api/") }}
value: {{ (print (.Values.config.BinderHub.hub_url_local | default .Values.config.BinderHub.hub_url | trimSuffix "/") "/hub/api/") }}
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch adding this as well!

Copy link
Member

Choose a reason for hiding this comment

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

If auth_enabled, this URL is required to be mounted as an env var? Hmmm? Is this because the JupyterHub imported HubOAuth classes looks for this URL or similar?

Ah yes it seems so, this could been configured with c.HubAuth.api_url = c.BinderHub.hub_url_local someplace instead, because JUPYTERHUB_API_URL serves the purpose of a default value for the c.HubAuth.api_url traitlet.

It seems like...

  • JUPYTERHUB_API_URL can be configured with c.HubAuth.api_url
  • JUPYTERHUB_BASE_URL can be configured with c.HubAuth.hub_prefix
  • JUPYTERHUB_CLIENT_ID can be configured with c.HubOAuth.oauth_client_id
  • JUPYTERHUB_OAUTH_CALLBACK_URL can be configured with c.HubOAuth.oauth_redirect_uri

Well, okay, let's configure them through environment variables.

Copy link
Member Author

Choose a reason for hiding this comment

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

@consideRatio I was confused by the apparent duplication between config and env-vars too. I only discovered the problem when deploying it to a test system.

- name: JUPYTERHUB_BASE_URL
value: {{ .Values.jupyterhub.hub.baseUrl | quote }}
- name: JUPYTERHUB_CLIENT_ID
Expand Down