Changes to Auth configurations (#68)

I realized while testing things a bit more locally that just having one endpoint was insufficient - flyte-cli and pyflyte currently leverage the gRPC Python Flyte Admin client.  However, discovery needs the HTTP endpoint.  Even though in production both are served on the same endpoint and traffic is handled through ingress, this is not always the case, and we want to support the use-case where gRPC Admin and HTTP Admin are hosted separately.

We were debating just adding a separate config for discovery alone, but ultimately it makes more sense to split out the existing URL config into two objects, especially if the above pattern is something we want to continue to support.  However, in the interest of not making people migrate, and not writing migration code in this PR, we're just introducing the HTTP one for now.

Also, we should lowercase the metadata header before sending it to Admin in the gRPC calls.

flytekit/__init__.py

@@ -1,4 +1,4 @@
 from __future__ import absolute_import
 import flytekit.plugins
 
-__version__ = '0.4.1'
+__version__ = '0.4.2'

flytekit/clients/raw.py

@@ -129,7 +129,9 @@ def __init__(self, url, insecure=False, credentials=None, options=None):
             self.force_auth_flow()
 
     def set_access_token(self, access_token):
-        self._metadata = [(_creds_config.AUTHORIZATION_METADATA_KEY.get(), "Bearer {}".format(access_token))]
+        # Always set the header to lower-case regardless of what the config is. The grpc libraries that Admin uses
+        # to parse the metadata don't change the metadata, but they do automatically lower the key you're looking for.
+        self._metadata = [(_creds_config.AUTHORIZATION_METADATA_KEY.get().lower(), "Bearer {}".format(access_token))]
 
     def force_auth_flow(self):
         refresh_handler_fn = _get_refresh_handler(_creds_config.AUTH_MODE.get())

flytekit/clis/auth/credentials.py

@@ -1,26 +1,42 @@
 from __future__ import absolute_import
+
+import logging as _logging
+
 from flytekit.clis.auth.auth import AuthorizationClient as _AuthorizationClient
 from flytekit.clis.auth.discovery import DiscoveryClient as _DiscoveryClient
 
 from flytekit.configuration.creds import (
     REDIRECT_URI as _REDIRECT_URI,
-    CLIENT_ID as _CLIENT_ID
+    CLIENT_ID as _CLIENT_ID,
 )
-from flytekit.configuration.platform import URL as _URL, INSECURE as _INSECURE
+from flytekit.configuration.platform import URL as _URL, INSECURE as _INSECURE, HTTP_URL as _HTTP_URL
 
 try:  # Python 3
     import urllib.parse as _urlparse
 except ImportError:  # Python 2
     import urlparse as _urlparse
 
 # Default, well known-URI string used for fetching JSON metadata. See https://tools.ietf.org/html/rfc8414#section-3.
-discovery_endpoint_path = ".well-known/oauth-authorization-server"
+discovery_endpoint_path = "./.well-known/oauth-authorization-server"
+
+
+def _get_discovery_endpoint(http_config_val, platform_url_val, insecure_val):
 
+    if http_config_val:
+        scheme, netloc, path, _, _, _ = _urlparse.urlparse(http_config_val)
+        if not scheme:
+            scheme = 'http' if insecure_val else 'https'
+    else:  # Use the main _URL config object effectively
+        scheme = 'http' if insecure_val else 'https'
+        netloc = platform_url_val
+        path = ''
 
-def _get_discovery_endpoint():
-    if _INSECURE.get():
-        return _urlparse.urljoin('http://{}/'.format(_URL.get()), discovery_endpoint_path)
-    return _urlparse.urljoin('https://{}/'.format(_URL.get()), discovery_endpoint_path)
+    computed_endpoint = _urlparse.urlunparse((scheme, netloc, path, None, None, None))
+    # The urljoin function needs a trailing slash in order to append things correctly. Also, having an extra slash
+    # at the end is okay, it just gets stripped out.
+    computed_endpoint = _urlparse.urljoin(computed_endpoint + '/', discovery_endpoint_path)
+    _logging.info('Using {} as discovery endpoint'.format(computed_endpoint))
+    return computed_endpoint
 
 
 # Lazy initialized authorization client singleton
@@ -41,6 +57,6 @@ def get_client():
 
 
 def get_authorization_endpoints():
-    discovery_endpoint = _get_discovery_endpoint()
+    discovery_endpoint = _get_discovery_endpoint(_HTTP_URL.get(), _URL.get(), _INSECURE.get())
     discovery_client = _DiscoveryClient(discovery_url=discovery_endpoint)
     return discovery_client.get_authorization_endpoints()

flytekit/configuration/platform.py

@@ -4,6 +4,16 @@
 from flytekit.common import constants as _constants
 
 URL = _config_common.FlyteRequiredStringConfigurationEntry('platform', 'url')
+
+HTTP_URL = _config_common.FlyteStringConfigurationEntry('platform', 'http_url', default=None)
+"""
+If not starting with either http or https, this setting should begin with // as per the urlparse library and
+https://tools.ietf.org/html/rfc1808.html, otherwise the netloc will not be properly parsed.
+
+Currently the only use-case for this configuration setting is for Auth discovery. This setting supports the case where
+Flyte Admin's gRPC and HTTP points are deployed on different ports.
+"""
+
 INSECURE = _config_common.FlyteBoolConfigurationEntry('platform', 'insecure', default=False)
 
 CLOUD_PROVIDER = _config_common.FlyteStringConfigurationEntry(

tests/flytekit/unit/cli/auth/test_credentials.py

@@ -0,0 +1,41 @@
+from __future__ import absolute_import
+
+from flytekit.clis.auth import credentials as _credentials
+
+
+def test_get_discovery_endpoint():
+    endpoint = _credentials._get_discovery_endpoint('//localhost:8088', 'localhost:8089', True)
+    assert endpoint == 'http://localhost:8088/.well-known/oauth-authorization-server'
+
+    endpoint = _credentials._get_discovery_endpoint('//localhost:8088', 'localhost:8089', False)
+    assert endpoint == 'https://localhost:8088/.well-known/oauth-authorization-server'
+
+    endpoint = _credentials._get_discovery_endpoint('//localhost:8088/path', 'localhost:8089', True)
+    assert endpoint == 'http://localhost:8088/path/.well-known/oauth-authorization-server'
+
+    endpoint = _credentials._get_discovery_endpoint('//localhost:8088/path', 'localhost:8089', False)
+    assert endpoint == 'https://localhost:8088/path/.well-known/oauth-authorization-server'
+
+    endpoint = _credentials._get_discovery_endpoint('//flyte.corp.com', 'localhost:8089', False)
+    assert endpoint == 'https://flyte.corp.com/.well-known/oauth-authorization-server'
+
+    endpoint = _credentials._get_discovery_endpoint('//flyte.corp.com/path', 'localhost:8089', False)
+    assert endpoint == 'https://flyte.corp.com/path/.well-known/oauth-authorization-server'
+
+    endpoint = _credentials._get_discovery_endpoint(None, 'localhost:8089', True)
+    assert endpoint == 'http://localhost:8089/.well-known/oauth-authorization-server'
+
+    endpoint = _credentials._get_discovery_endpoint(None, 'localhost:8089', False)
+    assert endpoint == 'https://localhost:8089/.well-known/oauth-authorization-server'
+
+    endpoint = _credentials._get_discovery_endpoint(None, 'flyte.corp.com', True)
+    assert endpoint == 'http://flyte.corp.com/.well-known/oauth-authorization-server'
+
+    endpoint = _credentials._get_discovery_endpoint(None, 'flyte.corp.com', False)
+    assert endpoint == 'https://flyte.corp.com/.well-known/oauth-authorization-server'
+
+    endpoint = _credentials._get_discovery_endpoint(None, 'localhost:8089', True)
+    assert endpoint == 'http://localhost:8089/.well-known/oauth-authorization-server'
+
+    endpoint = _credentials._get_discovery_endpoint(None, 'localhost:8089', False)
+    assert endpoint == 'https://localhost:8089/.well-known/oauth-authorization-server'