Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
115 changes: 71 additions & 44 deletions airflow-ctl/src/airflowctl/api/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
XComOperations,
)
from airflowctl.exceptions import (
AirflowCtlCredentialInvalidException,
AirflowCtlCredentialNotFoundException,
AirflowCtlException,
AirflowCtlKeyringException,
Expand Down Expand Up @@ -160,6 +161,26 @@ def _safe_path_under_airflow_home(airflow_home: str, filename: str) -> str:
return str(target)


def _read_cli_config(path: str) -> dict:
"""Read a credentials file, so an unusable one reaches the user as a message and not a traceback."""
try:
with open(path, encoding="utf-8") as f:
config = json.load(f)
except FileNotFoundError:
# An absent file is a distinct case the callers already handle.
raise
except (OSError, ValueError) as e:
# OSError covers unreadable paths; ValueError covers both malformed JSON and non-UTF-8 bytes.
raise AirflowCtlCredentialInvalidException(
f"Credentials file {path} could not be read: {e}. Log in again to recreate it."
) from e
if not isinstance(config, dict):
raise AirflowCtlCredentialInvalidException(
f"Credentials file {path} is not a JSON object. Log in again to recreate it."
)
return config


# Credentials for the API
class Credentials:
"""Credentials for the API."""
Expand Down Expand Up @@ -253,56 +274,62 @@ def load(self) -> Credentials:
default_config_dir = os.environ.get("AIRFLOW_HOME", os.path.expanduser("~/airflow"))
config_path = _safe_path_under_airflow_home(default_config_dir, self.input_cli_config_file)
try:
with open(config_path) as f:
credentials = json.load(f)
self.api_url = credentials["api_url"]
if self.client_kind == ClientKind.NO_AUTH:
return self
if self.api_token is not None:
return self
if os.getenv("AIRFLOW_CLI_DEBUG_MODE") == "true":
debug_creds_path = _safe_path_under_airflow_home(
default_config_dir, f"debug_creds_{self.input_cli_config_file}"
credentials = _read_cli_config(config_path)
if "api_url" not in credentials:
raise AirflowCtlCredentialInvalidException(
f"Credentials file {config_path} does not contain an api_url. "
"Log in again to recreate it."
)
self.api_url = credentials["api_url"]
if self.client_kind == ClientKind.NO_AUTH:
return self
if self.api_token is not None:
return self
if os.getenv("AIRFLOW_CLI_DEBUG_MODE") == "true":
debug_creds_path = _safe_path_under_airflow_home(
default_config_dir, f"debug_creds_{self.input_cli_config_file}"
)
try:
debug_credentials = _read_cli_config(debug_creds_path)
self.api_token = debug_credentials.get(
self.token_key_for_environment(self.api_environment)
)
except FileNotFoundError as e:
if self.client_kind == ClientKind.CLI:
raise AirflowCtlCredentialNotFoundException(
f"Debug credentials file not found: {debug_creds_path}. "
"Set AIRFLOW_CLI_DEBUG_MODE=false or log in with debug mode enabled first."
) from e
self.api_token = None
else:
try:
self.api_token = keyring.get_password(
"airflowctl", self.token_key_for_environment(self.api_environment)
)
try:
with open(debug_creds_path) as df:
debug_credentials = json.load(df)
self.api_token = debug_credentials.get(
self.token_key_for_environment(self.api_environment)
)
except FileNotFoundError as e:
if self.client_kind == ClientKind.CLI:
raise AirflowCtlCredentialNotFoundException(
f"Debug credentials file not found: {debug_creds_path}. "
"Set AIRFLOW_CLI_DEBUG_MODE=false or log in with debug mode enabled first."
) from e
self.api_token = None
else:
try:
self.api_token = keyring.get_password(
"airflowctl", self.token_key_for_environment(self.api_environment)
)
except ValueError as e:
# Incorrect keyring password
log.warning(
"Could not access keyring for environment %s: %s", self.api_environment, e
)
if self.client_kind == ClientKind.CLI:
raise AirflowCtlKeyringException(
f"Incorrect keyring password for environment {self.api_environment}"
) from e
self.api_token = None
except NoKeyringError as e:
# No keyring backend available
log.error("No keyring backend available: %s", e)
if self.client_kind == ClientKind.CLI:
raise AirflowCtlKeyringException("Keyring backend is not available") from e
self.api_token = None
except ValueError as e:
# Incorrect keyring password
log.warning("Could not access keyring for environment %s: %s", self.api_environment, e)
if self.client_kind == ClientKind.CLI:
raise AirflowCtlKeyringException(
f"Incorrect keyring password for environment {self.api_environment}"
) from e
self.api_token = None
except NoKeyringError as e:
# No keyring backend available
log.error("No keyring backend available: %s", e)
if self.client_kind == ClientKind.CLI:
raise AirflowCtlKeyringException("Keyring backend is not available") from e
self.api_token = None
except FileNotFoundError:
# This is expected during the auth login command.
# Also allow token-only usage without local config (for commands like `version --remote`).
if self.client_kind not in (ClientKind.AUTH, ClientKind.NO_AUTH) and self.api_token is None:
raise AirflowCtlCredentialNotFoundException("No credentials file found. Please login first.")
except AirflowCtlCredentialInvalidException:
# Same tolerance as a missing file: `auth login` has to be able to replace an unusable one,
# and it is the recovery the message recommends.
if self.client_kind not in (ClientKind.AUTH, ClientKind.NO_AUTH):
raise

return self

Expand Down
15 changes: 3 additions & 12 deletions airflow-ctl/src/airflowctl/ctl/cli_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,7 @@
from airflowctl.api.operations import BaseOperations, ServerResponseError
from airflowctl.ctl.console_formatting import AirflowConsole
from airflowctl.ctl.utils.yaml import safe_load
from airflowctl.exceptions import (
AirflowCtlConnectionException,
AirflowCtlCredentialNotFoundException,
AirflowCtlKeyringException,
AirflowCtlNotFoundException,
)
from airflowctl.exceptions import AirflowCtlException
from airflowctl.utils.module_loading import import_string

BUILD_DOCS = "BUILDING_AIRFLOW_DOCS" in os.environ
Expand Down Expand Up @@ -76,12 +71,8 @@ def safe_call_command(function: Callable, args: Iterable[Arg]) -> None:

try:
function(args)
except (
AirflowCtlCredentialNotFoundException,
AirflowCtlConnectionException,
AirflowCtlKeyringException,
AirflowCtlNotFoundException,
) as e:
except AirflowCtlException as e:
# The base class, so a new AirflowCtl error never regresses to a raw traceback.
rich.print(f"command failed due to {e}")
sys.exit(1)
except (httpx.RemoteProtocolError, httpx.ReadError) as e:
Expand Down
4 changes: 4 additions & 0 deletions airflow-ctl/src/airflowctl/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ class AirflowCtlCredentialNotFoundException(AirflowCtlNotFoundException):
"""Raise when a credential couldn't be found while performing an operation."""


class AirflowCtlCredentialInvalidException(AirflowCtlException):
"""Raise when a credentials file exists but cannot be used."""


class AirflowCtlConnectionException(AirflowCtlException):
"""Raise when a connection error occurs while performing an operation."""

Expand Down
37 changes: 37 additions & 0 deletions airflow-ctl/tests/airflow_ctl/api/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
)
from airflowctl.api.operations import ServerResponseError
from airflowctl.exceptions import (
AirflowCtlCredentialInvalidException,
AirflowCtlCredentialNotFoundException,
AirflowCtlException,
AirflowCtlKeyringException,
Expand Down Expand Up @@ -343,6 +344,42 @@ def test_load_no_keyring_backend_token_provided(self, mock_keyring):
credentials = Credentials(client_kind=cli_client, api_token="TEST_TOKEN").load()
assert credentials.api_token == "TEST_TOKEN"

@pytest.mark.parametrize(
("contents", "mode", "expected"),
[
pytest.param("{not json", None, "could not be read", id="corrupt"),
pytest.param("", None, "could not be read", id="empty"),
pytest.param(b"\xff\xfe", None, "could not be read", id="not-utf8"),
pytest.param('{"api_url": "http://x"}', 0o000, "could not be read", id="unreadable"),
pytest.param(json.dumps(["a"]), None, "is not a JSON object", id="not-an-object"),
pytest.param(json.dumps({"other": 1}), None, "does not contain an api_url", id="no-api-url"),
],
)
def test_load_unusable_credentials_file_reports_an_error(
self, monkeypatch, tmp_path, contents, mode, expected
):
"""A file airflowctl cannot use must reach the user as a message, not a traceback."""
monkeypatch.setenv("AIRFLOW_HOME", str(tmp_path))
monkeypatch.setenv("AIRFLOW_CLI_ENVIRONMENT", "TEST_BAD_CONFIG")
config = tmp_path / "TEST_BAD_CONFIG.json"
config.write_bytes(contents) if isinstance(contents, bytes) else config.write_text(
contents, encoding="utf-8"
)
if mode is not None:
config.chmod(mode)

with pytest.raises(AirflowCtlCredentialInvalidException, match=expected):
Credentials(client_kind=ClientKind.CLI).load()

@pytest.mark.parametrize("client_kind", [ClientKind.AUTH, ClientKind.NO_AUTH])
def test_load_tolerates_an_unusable_file_for_auth_kinds(self, monkeypatch, tmp_path, client_kind):
"""`auth login` is the recovery the error recommends, so it must survive an unusable file."""
monkeypatch.setenv("AIRFLOW_HOME", str(tmp_path))
monkeypatch.setenv("AIRFLOW_CLI_ENVIRONMENT", "TEST_BAD_CONFIG")
(tmp_path / "TEST_BAD_CONFIG.json").write_text("{not json", encoding="utf-8")

assert Credentials(client_kind=client_kind).load().api_url is None


class TestBoundedGetNewPassword:
@patch("airflowctl.api.client.getpass.getpass")
Expand Down
10 changes: 9 additions & 1 deletion airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
)
from airflowctl.exceptions import (
AirflowCtlConnectionException,
AirflowCtlCredentialInvalidException,
AirflowCtlCredentialNotFoundException,
AirflowCtlKeyringException,
AirflowCtlNotFoundException,
Expand Down Expand Up @@ -470,11 +471,18 @@ class TestCliConfigMethods:
"raised_exception",
[
AirflowCtlCredentialNotFoundException("missing credentials"),
AirflowCtlCredentialInvalidException("unusable credentials"),
AirflowCtlConnectionException("connection failed"),
AirflowCtlKeyringException("keyring failure"),
AirflowCtlNotFoundException("resource not found"),
],
ids=["credential-not-found", "connection-error", "keyring-error", "not-found"],
ids=[
"credential-not-found",
"credential-invalid",
"connection-error",
"keyring-error",
"not-found",
],
)
def test_safe_call_command_exits_non_zero_for_airflowctl_exceptions(self, raised_exception):
def raise_error(_args):
Expand Down