Skip to content

Commit

Permalink
[ASP-5745] Enable lazy evaluated settings on Jobbergate-cli (#639)
Browse files Browse the repository at this point in the history
* feat(cli): ensure there is no required setting

* fix(cli): implement ContextProtocol to avoid circular imports

* Update jobbergate-cli/jobbergate_cli/context.py

* feat: remove OIDC_AUDIENCE from core, cli, and agent, following d1fdaa7

* code review

* lint(cli): organize imports on subapps.applications.app
  • Loading branch information
fschuch authored Nov 4, 2024
1 parent ec236fe commit 3fde577
Show file tree
Hide file tree
Showing 28 changed files with 239 additions and 225 deletions.
5 changes: 0 additions & 5 deletions examples/login-with-api.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
- ARMADA_API_BASE
- OIDC_DOMAIN
- OIDC_CLIENT_ID
- OIDC_AUDIENCE
These can either be set in a `.env` file or as environment variables.
Expand All @@ -32,7 +31,6 @@
- OIDC_DOMAIN=http://keycloak.local:8080/realms/jobbergate-local
- OIDC_CLIENT_ID=cli
- OIDC_AUDIENCE=https://local.omnivector.solutions
- ARMADA_API_BASE=http://localhost:8000
Note: After logging in the first time, running this demo again will use the token saved
Expand All @@ -52,7 +50,6 @@

domain = os.getenv("OIDC_DOMAIN")
client_id = os.getenv("OIDC_CLIENT_ID")
audience = os.getenv("OIDC_AUDIENCE")
base_api_url = os.getenv("ARMADA_API_BASE")

access_token_file = pathlib.Path("./access.token")
Expand Down Expand Up @@ -84,7 +81,6 @@ def login():
data=dict(
client_id=client_id,
grant_type="client_credentials",
audience=audience,
),
)
device_code_data = response.json()
Expand Down Expand Up @@ -122,7 +118,6 @@ def refresh(refresh_token):
f"{domain}/protocol/openid-connect/token",
data=dict(
client_id=client_id,
audience=audience,
grant_type="refresh_token",
refresh_token=refresh_token,
),
Expand Down
1 change: 1 addition & 0 deletions jobbergate-agent/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ This file keeps track of all notable changes to jobbergate-agent
## Unreleased
- Changed auto-update task to reuse current scheduler instead of creating a new one
- Fixed environment variables from the machine running the agent propagating to slurm jobs (notice `--export=ALL` is the default behavior for sbatch)
- Removed the `OIDC_AUDIENCE` setting

## 5.3.0 -- 2024-09-09

Expand Down
1 change: 0 additions & 1 deletion jobbergate-agent/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ To install the package from Pypi simply run `pip install jobbergate-agent`.
JOBBERGATE_AGENT_X_SLURM_USER_NAME="<sbatch-user-name>"
JOBBERGATE_AGENT_SENTRY_DSN="<sentry-dsn-key>"
JOBBERGATE_AGENT_OIDC_DOMAIN="<OIDC-domain>"
JOBBERGATE_AGENT_OIDC_AUDIENCE="<OIDC-audience>"
JOBBERGATE_AGENT_OIDC_CLIENT_ID="<OIDC-app-client-id>"
JOBBERGATE_AGENT_OIDC_CLIENT_SECRET="<OIDC-app-client-secret>"
```
Expand Down
1 change: 0 additions & 1 deletion jobbergate-agent/jobbergate_agent/clients/cluster_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ def acquire_token(token: Token) -> Token:

logger.debug("Attempting to acquire token from OIDC")
oidc_body = dict(
audience=SETTINGS.OIDC_AUDIENCE,
client_id=SETTINGS.OIDC_CLIENT_ID,
client_secret=SETTINGS.OIDC_CLIENT_SECRET,
grant_type="client_credentials",
Expand Down
1 change: 0 additions & 1 deletion jobbergate-agent/jobbergate_agent/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ class Settings(BaseSettings):

# OIDC config for machine-to-machine security
OIDC_DOMAIN: str
OIDC_AUDIENCE: str = "https://apis.omnivector.solutions"
OIDC_CLIENT_ID: str
OIDC_CLIENT_SECRET: str
OIDC_USE_HTTPS: bool = True
Expand Down
2 changes: 2 additions & 0 deletions jobbergate-cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ This file keeps track of all notable changes to jobbergate-cli
## Unreleased

- Enabled positional arguments to select entries using the cli [ASP-5649]
- Modified internal details to ensure commands that require no authentication face no configuration errors [ASP-5745]
- Removed the `OIDC_AUDIENCE` setting

## 5.3.0 -- 2024-09-09

Expand Down
5 changes: 2 additions & 3 deletions jobbergate-cli/jobbergate_cli/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,8 @@ class Settings(BaseSettings):
JOBBERGATE_LEGACY_NAME_CONVENTION: Optional[bool] = False

# Auth0 config for machine-to-machine security
OIDC_DOMAIN: str
OIDC_AUDIENCE: str
OIDC_CLIENT_ID: str
OIDC_DOMAIN: Optional[str] = None
OIDC_CLIENT_ID: Optional[str] = None
OIDC_USE_HTTPS: bool = True
OIDC_CLIENT_SECRET: Optional[str] = None

Expand Down
62 changes: 62 additions & 0 deletions jobbergate-cli/jobbergate_cli/context.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
"""
Provides a data object for context passed from the main entry point.
"""

from dataclasses import dataclass
from functools import cached_property

from buzz import check_expressions
from httpx import Client
from jobbergate_core.auth.handler import JobbergateAuthHandler

from jobbergate_cli.auth import show_login_message, track_login_progress
from jobbergate_cli.exceptions import Abort
from jobbergate_cli.config import settings
from jobbergate_cli.schemas import ContextProtocol


@dataclass
class JobbergateContext(ContextProtocol):
"""
A data object describing context passed from the main entry point.
"""

raw_output: bool = False
full_output: bool = False

@cached_property
def client(self) -> Client:
"""
Client for making requests to the Jobbergate API.
"""
return Client(
base_url=settings.ARMADA_API_BASE,
auth=self.authentication_handler,
timeout=settings.JOBBERGATE_REQUESTS_TIMEOUT,
)

@cached_property
def authentication_handler(self) -> JobbergateAuthHandler:
"""
The authentication handler for the context.
This is a cached property to ensure that the handler is only created when needed,
so commands that require no authentication face no configuration errors.
"""
with check_expressions(
"The following settings are required to enable authenticated requests:",
raise_exc_class=Abort,
raise_kwargs=dict(support=True, subject="Configuration error"),
) as check:
check(settings.OIDC_DOMAIN, "OIDC_DOMAIN")
check(settings.OIDC_CLIENT_ID, "OIDC_CLIENT_ID")

protocol = "https" if settings.OIDC_USE_HTTPS else "http"
return JobbergateAuthHandler(
cache_directory=settings.JOBBERGATE_USER_TOKEN_DIR,
login_domain=f"{protocol}://{settings.OIDC_DOMAIN}",
login_client_id=settings.OIDC_CLIENT_ID,
login_client_secret=settings.OIDC_CLIENT_SECRET,
login_url_handler=show_login_message,
login_sequence_handler=track_login_progress,
)
31 changes: 6 additions & 25 deletions jobbergate-cli/jobbergate_cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,15 @@
import sys
from typing import Any

import httpx
import importlib_metadata
import typer
from jobbergate_core.auth.handler import JobbergateAuthHandler

from jobbergate_cli.auth import show_login_message, track_login_progress
from jobbergate_cli.config import settings
from jobbergate_cli.context import JobbergateContext
from jobbergate_cli.exceptions import Abort, handle_abort, handle_authentication_error
from jobbergate_cli.logging import init_logs, init_sentry
from jobbergate_cli.render import render_demo, render_json, terminal_message
from jobbergate_cli.schemas import JobbergateContext
from jobbergate_cli.schemas import ContextProtocol
from jobbergate_cli.subapps.applications.app import app as applications_app
from jobbergate_cli.subapps.job_scripts.app import app as job_scripts_app
from jobbergate_cli.subapps.job_submissions.app import app as job_submissions_app
Expand Down Expand Up @@ -68,27 +66,10 @@ def main(
init_logs(verbose=verbose)
init_sentry()

protocol = "https" if settings.OIDC_USE_HTTPS else "http"
authentication_handler = JobbergateAuthHandler(
cache_directory=settings.JOBBERGATE_USER_TOKEN_DIR,
login_domain=f"{protocol}://{settings.OIDC_DOMAIN}",
login_audience=settings.OIDC_AUDIENCE,
login_client_id=settings.OIDC_CLIENT_ID,
login_client_secret=settings.OIDC_CLIENT_SECRET,
login_url_handler=show_login_message,
login_sequence_handler=track_login_progress,
)

ctx.obj = JobbergateContext(
client=httpx.Client(
base_url=settings.ARMADA_API_BASE,
auth=authentication_handler,
timeout=settings.JOBBERGATE_REQUESTS_TIMEOUT,
),
authentication_handler=authentication_handler,
full_output=full,
raw_output=raw,
)
# Stored first as a local variable to enable type checking and make mypy happy with the syntax
# Then stored in the context object to be passed to the subcommands
context: ContextProtocol = JobbergateContext(full_output=full, raw_output=raw)
ctx.obj = context


@app.command(rich_help_panel="Authentication")
Expand Down
8 changes: 4 additions & 4 deletions jobbergate-cli/jobbergate_cli/render.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from rich.table import Table
from rich.markdown import Markdown

from jobbergate_cli.schemas import JobbergateContext, ListResponseEnvelope
from jobbergate_cli.schemas import ContextProtocol, ListResponseEnvelope
from jobbergate_cli.text_tools import dedent
from jobbergate_cli.text_tools import indent as indent_text

Expand Down Expand Up @@ -105,7 +105,7 @@ def render_json(data: Any):


def render_list_results(
ctx: JobbergateContext,
ctx: ContextProtocol,
envelope: ListResponseEnvelope,
style_mapper: StyleMapper | None = None,
hidden_fields: list[str] | None = None,
Expand Down Expand Up @@ -179,7 +179,7 @@ def render_dict(


def render_single_result(
ctx: JobbergateContext,
ctx: ContextProtocol,
result: dict[str, Any] | pydantic.BaseModel,
hidden_fields: list[str] | None = None,
title: str = "Result",
Expand Down Expand Up @@ -212,7 +212,7 @@ def render_single_result(


def render_paginated_list_results(
ctx: JobbergateContext,
ctx: ContextProtocol,
envelope: ListResponseEnvelope,
title: str = "Results List",
style_mapper: Optional[StyleMapper] = None,
Expand Down
23 changes: 15 additions & 8 deletions jobbergate-cli/jobbergate_cli/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@

from datetime import datetime
from pathlib import Path
from typing import Any, Dict, Generic, List, Optional, TypeVar
from typing import Any, Dict, Generic, List, Optional, Protocol, TypeVar

import httpx
import pydantic
import pydantic.generics
from jobbergate_core.auth.handler import JobbergateAuthHandler

from jobbergate_cli.constants import FileType
from jobbergate_core.auth.handler import JobbergateAuthHandler


class TokenSet(pydantic.BaseModel, extra="ignore"):
Expand Down Expand Up @@ -53,15 +53,22 @@ class DeviceCodeData(pydantic.BaseModel, extra="ignore"):
interval: int


class JobbergateContext(pydantic.BaseModel, arbitrary_types_allowed=True):
class ContextProtocol(Protocol):
"""
A data object describing context passed from the main entry point.
A protocol describing context passed from the main entry point.
It aims to help static type checkers at the same time that prevents
circular import issues on the actual implementation.
"""

client: httpx.Client
authentication_handler: JobbergateAuthHandler
raw_output: bool = False
full_output: bool = False
raw_output: bool
full_output: bool

@property
def client(self) -> httpx.Client: ...

@property
def authentication_handler(self) -> JobbergateAuthHandler: ...


class JobbergateConfig(pydantic.BaseModel, extra="allow"):
Expand Down
33 changes: 8 additions & 25 deletions jobbergate-cli/jobbergate_cli/subapps/applications/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from jobbergate_cli.exceptions import Abort
from jobbergate_cli.render import StyleMapper, render_single_result, terminal_message
from jobbergate_cli.requests import make_request
from jobbergate_cli.schemas import ApplicationResponse, JobbergateContext
from jobbergate_cli.schemas import ApplicationResponse, ContextProtocol
from jobbergate_cli.subapps.applications.tools import (
fetch_application_data,
save_application_files,
Expand All @@ -21,7 +21,6 @@
from jobbergate_cli.subapps.pagination import handle_pagination
from jobbergate_cli.subapps.tools import resolve_application_selection


# TODO: move hidden field logic to the API
HIDDEN_FIELDS = [
"cloned_from_id",
Expand Down Expand Up @@ -74,11 +73,7 @@ def list_all(
"""
Show available applications
"""
jg_ctx: JobbergateContext = ctx.obj

# Make static type checkers happy
assert jg_ctx is not None
assert jg_ctx.client is not None
jg_ctx: ContextProtocol = ctx.obj

params: Dict[str, Any] = dict(
include_null_identifier=show_all,
Expand Down Expand Up @@ -124,7 +119,7 @@ def get_one(
"""
Get a single application by id or identifier
"""
jg_ctx: JobbergateContext = ctx.obj
jg_ctx: ContextProtocol = ctx.obj
result = fetch_application_data(
jg_ctx, id_or_identifier=resolve_application_selection(id_or_identifier, id, identifier)
)
Expand Down Expand Up @@ -171,10 +166,7 @@ def create(
if application_desc:
req_data["description"] = application_desc

jg_ctx: JobbergateContext = ctx.obj

# Make static type checkers happy
assert jg_ctx.client is not None
jg_ctx: ContextProtocol = ctx.obj

result = cast(
Dict[str, Any],
Expand Down Expand Up @@ -268,10 +260,7 @@ def update(
if application_name:
req_data["name"] = application_name

jg_ctx: JobbergateContext = ctx.obj

# Make static type checkers happy
assert jg_ctx.client is not None
jg_ctx: ContextProtocol = ctx.obj

if req_data:
make_request(
Expand Down Expand Up @@ -330,10 +319,7 @@ def delete(
"""
Delete an existing application.
"""
jg_ctx: JobbergateContext = ctx.obj

# Make static type checkers happy
assert jg_ctx.client is not None
jg_ctx: ContextProtocol = ctx.obj

identification = resolve_application_selection(id_or_identifier, id, identifier)

Expand Down Expand Up @@ -376,7 +362,7 @@ def download_files(
"""
Download the files from an application to the current working directory.
"""
jg_ctx: JobbergateContext = ctx.obj
jg_ctx: ContextProtocol = ctx.obj

result = fetch_application_data(jg_ctx, resolve_application_selection(id_or_identifier, id, identifier))
saved_files = save_application_files(
Expand Down Expand Up @@ -444,10 +430,7 @@ def clone(
if application_name:
req_data["name"] = application_name

jg_ctx: JobbergateContext = ctx.obj

# Make static type checkers happy
assert jg_ctx.client is not None
jg_ctx: ContextProtocol = ctx.obj

result = cast(
ApplicationResponse,
Expand Down
Loading

0 comments on commit 3fde577

Please sign in to comment.