Load auth from kube config. #1443
Conversation
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: IronPan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold wait until sample test pass. |
|
/hold cancel Both e2e and sample tests are passed. |
|
/retest |
| @@ -34,9 +34,10 @@ class Client(object): | |||
| """ | |||
|
|
|||
| # in-cluster DNS name of the pipeline service | |||
| IN_CLUSTER_DNS_NAME = 'ml-pipeline.kubeflow.svc.cluster.local:8888' | |||
| IN_CLUSTER_DNS_NAME = 'ml-pipeline.{}.svc.cluster.local:8888' | |||
There was a problem hiding this comment.
Maybe rename to IN_CLUSTER_DNS_TEMPLATE and KUBE_PROXY_PATH_TEMPLATE
There was a problem hiding this comment.
Will do in a follow up PR.
| api_client = kfp_server_api.api_client.ApiClient(config) | ||
| self._run_api = kfp_server_api.api.run_service_api.RunServiceApi(api_client) | ||
| self._experiment_api = kfp_server_api.api.experiment_service_api.ExperimentServiceApi(api_client) | ||
|
|
||
| def _configure_auth(self, config, token): | ||
| def _load_config(self, host, client_id, namespace): |
There was a problem hiding this comment.
is this a @staticmethod?
| token = None | ||
| if host and client_id: | ||
| # fetch IAP auth token | ||
| token = get_auth_token(client_id) |
There was a problem hiding this comment.
it's a bit out of scope for this PR but maybe this should be renamed to get_iap_auth_token
| return config | ||
|
|
||
| if config.host: | ||
| config.host = os.path.join(config.host, Client.KUBE_PROXY_PATH.format(namespace)) |
There was a problem hiding this comment.
we probably want https://docs.python.org/2/library/urlparse.html#urlparse.urljoin here
|
The e2e and sample test covers in cluster use case. |
* update monitoring pointers * Update README.md
This PR supports:
Fix #1318
This PR can kind of fix #1104 as well, except that it doesn't go through IAP.
This change is