Skip to content

Commit

Permalink
[hailtop] Remove namespace options in hailctl auth commands (#14069)
Browse files Browse the repository at this point in the history
I'm trying to move our codebase away from relying on the notion of a K8s
namespace for routing because this model does not hold in alternative
deployment environments such as Terra. Further, having a `-n` option in
`hailctl auth` commands is awkward because it can only be used for dev
environments yet is visible to users in the help menu. As such, this is
a breaking change only for developers. The functionality is not really
gone though because you can replace any `hailctl auth … -n dgoldste`
with `HAIL_DEFAULT_NAMESPACE=dgoldste hailctl auth …`.
  • Loading branch information
daniel-goldstein authored Dec 16, 2023
1 parent 7fab886 commit c4aed60
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 112 deletions.
10 changes: 5 additions & 5 deletions build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1872,9 +1872,9 @@ steps:
--retry 3 \
--retry-delay 5 \
-XPOST)
hailctl auth copy-paste-login "$COPY_PASTE_TOKEN" --namespace {{ default_ns.name }}
hailctl auth copy-paste-login "$COPY_PASTE_TOKEN"
if hailctl auth copy-paste-login "$COPY_PASTE_TOKEN" --namespace {{ default_ns.name }}
if hailctl auth copy-paste-login "$COPY_PASTE_TOKEN"
then
echo "reusing a token should not work, but did"
exit 1
Expand All @@ -1888,14 +1888,14 @@ steps:
-XPOST)
python3 -c '
from hailtop.auth import copy_paste_login;
copy_paste_login("'$COPY_PASTE_TOKEN'", "{{ default_ns.name }}")
copy_paste_login("'$COPY_PASTE_TOKEN'")
'
python3 -c '
from hailtop.auth import copy_paste_login;
import aiohttp
try:
copy_paste_login("'$COPY_PASTE_TOKEN'", "{{ default_ns.name }}")
copy_paste_login("'$COPY_PASTE_TOKEN'")
print("reusing a token should not work, but did")
sys.exit(1)
except aiohttp.client_exceptions.ClientResponseError as exc:
Expand Down Expand Up @@ -1945,7 +1945,7 @@ steps:
import aiohttp
from hailtop.auth import copy_paste_login;
try:
copy_paste_login("'$COPY_PASTE_TOKEN'", "{{ default_ns.name }}")
copy_paste_login("'$COPY_PASTE_TOKEN'")
print("using an expired token should not work, but did")
sys.exit(1)
except aiohttp.client_exceptions.ClientResponseError as exc:
Expand Down
104 changes: 38 additions & 66 deletions hail/python/hailtop/auth/auth.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from typing import Any, Optional, Dict, Tuple, List
from contextlib import asynccontextmanager
from dataclasses import dataclass
from enum import Enum
import os
Expand Down Expand Up @@ -34,19 +35,19 @@ def from_json(config: Dict[str, Any]):


class HailCredentials(CloudCredentials):
def __init__(self, tokens: Tokens, cloud_credentials: Optional[CloudCredentials], namespace: str, authorize_target: bool):
def __init__(self, tokens: Tokens, cloud_credentials: Optional[CloudCredentials], deploy_config: DeployConfig, authorize_target: bool):
self._tokens = tokens
self._cloud_credentials = cloud_credentials
self._namespace = namespace
self._deploy_config = deploy_config
self._authorize_target = authorize_target

async def auth_headers_with_expiration(self) -> Tuple[Dict[str, str], Optional[float]]:
headers = {}
expiration = None
if self._authorize_target:
token, expiration = await self._get_idp_access_token_or_hail_token(self._namespace)
token, expiration = await self._get_idp_access_token_or_hail_token(self._deploy_config.default_namespace())
headers['Authorization'] = f'Bearer {token}'
if get_deploy_config().location() == 'external' and self._namespace != 'default':
if get_deploy_config().location() == 'external' and self._deploy_config.default_namespace() != 'default':
# We prefer an extant hail token to an access token for the internal auth token
# during development of the idp access token feature because the production auth
# is not yet configured to accept access tokens. This can be changed to always prefer
Expand All @@ -61,7 +62,7 @@ async def auth_headers_with_expiration(self) -> Tuple[Dict[str, str], Optional[f
return headers, expiration

async def access_token_with_expiration(self) -> Tuple[str, Optional[float]]:
return await self._get_idp_access_token_or_hail_token(self._namespace)
return await self._get_idp_access_token_or_hail_token(self._deploy_config.default_namespace())

async def _get_idp_access_token_or_hail_token(self, namespace: str) -> Tuple[str, Optional[float]]:
if self._cloud_credentials is not None:
Expand All @@ -88,13 +89,12 @@ def hail_credentials(
*,
tokens_file: Optional[str] = None,
cloud_credentials_file: Optional[str] = None,
namespace: Optional[str] = None,
deploy_config: Optional[DeployConfig] = None,
authorize_target: bool = True
) -> HailCredentials:
tokens = get_tokens(tokens_file)
deploy_config = get_deploy_config()
ns = namespace or deploy_config.default_namespace()
return HailCredentials(tokens, get_cloud_credentials_scoped_for_hail(credentials_file=cloud_credentials_file), ns, authorize_target=authorize_target)
deploy_config = deploy_config or get_deploy_config()
return HailCredentials(tokens, get_cloud_credentials_scoped_for_hail(credentials_file=cloud_credentials_file), deploy_config, authorize_target=authorize_target)


def get_cloud_credentials_scoped_for_hail(credentials_file: Optional[str] = None) -> Optional[CloudCredentials]:
Expand Down Expand Up @@ -138,21 +138,6 @@ def load_identity_spec() -> Optional[IdentityProviderSpec]:
return None


async def deploy_config_and_headers_from_namespace(namespace: Optional[str] = None, *, authorize_target: bool = True) -> Tuple[DeployConfig, Dict[str, str], str]:
deploy_config = get_deploy_config()

if namespace is not None:
deploy_config = deploy_config.with_default_namespace(namespace)
else:
namespace = deploy_config.default_namespace()


async with hail_credentials(namespace=namespace, authorize_target=authorize_target) as credentials:
headers = await credentials.auth_headers()

return (deploy_config, headers, namespace)


async def async_get_userinfo():
deploy_config = get_deploy_config()
userinfo_url = deploy_config.url('auth', '/api/v1alpha/userinfo')
Expand All @@ -172,29 +157,29 @@ def get_userinfo():
return async_to_blocking(async_get_userinfo())


def copy_paste_login(copy_paste_token: str, namespace: Optional[str] = None):
return async_to_blocking(async_copy_paste_login(copy_paste_token, namespace))
def copy_paste_login(copy_paste_token: str) -> str:
return async_to_blocking(async_copy_paste_login(copy_paste_token))


async def async_copy_paste_login(copy_paste_token: str, namespace: Optional[str] = None):
deploy_config, headers, namespace = await deploy_config_and_headers_from_namespace(namespace, authorize_target=False)
async with httpx.client_session(headers=headers) as session:
async def async_copy_paste_login(copy_paste_token: str) -> str:
deploy_config = get_deploy_config()
async with httpx.client_session() as session:
data = await retry_transient_errors(
session.post_read_json,
deploy_config.url('auth', '/api/v1alpha/copy-paste-login'),
params={'copy_paste_token': copy_paste_token}
params={'copy_paste_token': copy_paste_token},
)
token = data['token']
username = data['username']

tokens = get_tokens()
tokens[namespace] = token
tokens[deploy_config.default_namespace()] = token
dot_hail_dir = os.path.expanduser('~/.hail')
if not os.path.exists(dot_hail_dir):
os.mkdir(dot_hail_dir, mode=0o700)
tokens.write()

return namespace, username
return username


async def async_logout():
Expand Down Expand Up @@ -236,20 +221,21 @@ async def logout_oauth2_credentials(identity_spec: IdentityProviderSpec):
await AzureFlow.logout_installed_app(identity_spec.oauth2_credentials)


def get_user(username: str, namespace: Optional[str] = None) -> dict:
return async_to_blocking(async_get_user(username, namespace))
@asynccontextmanager
async def hail_session(**session_kwargs):
async with hail_credentials() as credentials:
async with Session(credentials=credentials, **session_kwargs) as session:
yield session

def get_user(username: str) -> dict:
return async_to_blocking(async_get_user(username))

async def async_get_user(username: str, namespace: Optional[str] = None) -> dict:
deploy_config, headers, _ = await deploy_config_and_headers_from_namespace(namespace)

async with httpx.client_session(
timeout=aiohttp.ClientTimeout(total=30),
headers=headers) as session:
return await retry_transient_errors(
session.get_read_json,
deploy_config.url('auth', f'/api/v1alpha/users/{username}')
)
async def async_get_user(username: str) -> dict:
async with hail_session(timeout=aiohttp.ClientTimeout(total=30)) as session:
url = get_deploy_config().url('auth', f'/api/v1alpha/users/{username}')
async with await session.get(url) as resp:
return await resp.json()


async def async_create_user(
Expand All @@ -259,11 +245,7 @@ async def async_create_user(
is_service_account: bool,
hail_identity: Optional[str],
hail_credentials_secret_name: Optional[str],
*,
namespace: Optional[str] = None
):
deploy_config, headers, _ = await deploy_config_and_headers_from_namespace(namespace)

body = {
'login_id': login_id,
'is_developer': is_developer,
Expand All @@ -272,26 +254,16 @@ async def async_create_user(
'hail_credentials_secret_name': hail_credentials_secret_name,
}

async with httpx.client_session(
timeout=aiohttp.ClientTimeout(total=30),
headers=headers) as session:
await retry_transient_errors(
session.post,
deploy_config.url('auth', f'/api/v1alpha/users/{username}/create'),
json=body
)
url = get_deploy_config().url('auth', f'/api/v1alpha/users/{username}/create')
async with hail_session(timeout=aiohttp.ClientTimeout(total=30)) as session:
await session.post(url, json=body)


def delete_user(username: str, namespace: Optional[str] = None):
return async_to_blocking(async_delete_user(username, namespace=namespace))
def delete_user(username: str):
return async_to_blocking(async_delete_user(username))


async def async_delete_user(username: str, namespace: Optional[str] = None):
deploy_config, headers, _ = await deploy_config_and_headers_from_namespace(namespace)
async with httpx.client_session(
timeout=aiohttp.ClientTimeout(total=300),
headers=headers) as session:
await retry_transient_errors(
session.delete,
deploy_config.url('auth', f'/api/v1alpha/users/{username}')
)
async def async_delete_user(username: str):
url = get_deploy_config().url('auth', f'/api/v1alpha/users/{username}')
async with hail_session(timeout=aiohttp.ClientTimeout(total=300)) as session:
await session.delete(url)
5 changes: 3 additions & 2 deletions hail/python/hailtop/hailctl/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,11 @@ async def _curl(
from hailtop.auth import hail_credentials # pylint: disable=import-outside-toplevel
from hailtop.config import get_deploy_config # pylint: disable=import-outside-toplevel

async with hail_credentials(namespace=namespace) as credentials:
deploy_config = get_deploy_config().with_default_namespace(namespace)
async with hail_credentials(deploy_config=deploy_config) as credentials:
headers_dict = await credentials.auth_headers()
headers = [x for k, v in headers_dict.items() for x in ['-H', f'{k}: {v}']]
path = get_deploy_config().url(service, path)
path = deploy_config.url(service, path)
os.execvp('curl', ['curl', *headers, *ctx.args, path])


Expand Down
28 changes: 9 additions & 19 deletions hail/python/hailtop/hailctl/auth/cli.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import asyncio
import sys
import typer
from typer import Option as Opt, Argument as Arg
from typer import Argument as Arg
import json

from typing import Optional, Annotated as Ann
Expand All @@ -15,29 +15,21 @@
)


NamespaceOption = Ann[
Optional[str],
Opt('--namespace', '-n', help='Namespace for the auth server (default: from deploy configuration).'),
]


@app.command()
def login(namespace: NamespaceOption = None):
def login():
'''Obtain Hail credentials.'''
from .login import async_login # pylint: disable=import-outside-toplevel
asyncio.run(async_login(namespace))
asyncio.run(async_login())


@app.command()
def copy_paste_login(copy_paste_token: str, namespace: NamespaceOption = None):
def copy_paste_login(copy_paste_token: str):
'''Obtain Hail credentials with a copy paste token.'''
from hailtop.auth import copy_paste_login # pylint: disable=import-outside-toplevel
from hailtop.config import get_deploy_config # pylint: disable=import-outside-toplevel

auth_ns, username = copy_paste_login(copy_paste_token, namespace)
if auth_ns == 'default':
print(f'Logged in as {username}.')
else:
print(f'Logged into namespace {auth_ns} as {username}.')
username = copy_paste_login(copy_paste_token)
print(f'Logged into {get_deploy_config().base_url("auth")} as {username}.')


@app.command()
Expand Down Expand Up @@ -92,26 +84,24 @@ def create_user(
service_account: bool = False,
hail_identity: Optional[str] = None,
hail_credentials_secret_name: Optional[str] = None,
namespace: NamespaceOption = None,
wait: bool = False,
):
'''
Create a new Hail user with username USERNAME and login ID LOGIN_ID.
'''
from .create_user import polling_create_user # pylint: disable=import-outside-toplevel

asyncio.run(polling_create_user(username, login_id, developer, service_account, hail_identity, hail_credentials_secret_name, namespace=namespace, wait=wait))
asyncio.run(polling_create_user(username, login_id, developer, service_account, hail_identity, hail_credentials_secret_name, wait=wait))


@app.command()
def delete_user(
username: str,
namespace: NamespaceOption = None,
wait: bool = False,
):
'''
Delete the Hail user with username USERNAME.
'''
from .delete_user import polling_delete_user # pylint: disable=import-outside-toplevel

asyncio.run(polling_delete_user(username, namespace, wait))
asyncio.run(polling_delete_user(username, wait))
5 changes: 2 additions & 3 deletions hail/python/hailtop/hailctl/auth/create_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,18 @@ async def polling_create_user(
hail_identity: Optional[str],
hail_credentials_secret_name: Optional[str],
*,
namespace: Optional[str] = None,
wait: bool = False,
):
try:
await async_create_user(username, login_id, developer, service_account, hail_identity, hail_credentials_secret_name, namespace=namespace)
await async_create_user(username, login_id, developer, service_account, hail_identity, hail_credentials_secret_name)

if not wait:
return

async def _poll():
tries = 0
while True:
user = await async_get_user(username, namespace)
user = await async_get_user(username)
if user['state'] == 'active':
print(f"Created user '{username}'")
return
Expand Down
7 changes: 2 additions & 5 deletions hail/python/hailtop/hailctl/auth/delete_user.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
from typing import Optional

from hailtop.utils import sleep_before_try
from hailtop.auth import async_delete_user, async_get_user

Expand All @@ -10,19 +8,18 @@ class DeleteUserException(Exception):

async def polling_delete_user(
username: str,
namespace: Optional[str],
wait: bool,
):
try:
await async_delete_user(username, namespace)
await async_delete_user(username)

if not wait:
return

async def _poll():
tries = 1
while True:
user = await async_get_user(username, namespace)
user = await async_get_user(username)
if user['state'] == 'deleted':
print(f"Deleted user '{username}'")
return
Expand Down
Loading

0 comments on commit c4aed60

Please sign in to comment.