This is an automated email from the ASF dual-hosted git repository.
henry3260 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 84719cfb009 Fix airflowctl tracebacks on an unusable credentials file
(#72468)
84719cfb009 is described below
commit 84719cfb009f6286cdbb2883ba9615955dd14e17
Author: Y-C <[email protected]>
AuthorDate: Sun Nov 1 19:06:37 2026 +0800
Fix airflowctl tracebacks on an unusable credentials file (#72468)
---
airflow-ctl/src/airflowctl/api/client.py | 124 +++++++++++++--------
airflow-ctl/src/airflowctl/ctl/cli_config.py | 15 +--
.../src/airflowctl/ctl/commands/auth_command.py | 10 +-
airflow-ctl/src/airflowctl/exceptions.py | 4 +
airflow-ctl/tests/airflow_ctl/api/test_client.py | 52 ++++++++-
.../airflow_ctl/ctl/commands/test_auth_command.py | 22 ++++
.../tests/airflow_ctl/ctl/test_cli_config.py | 10 +-
7 files changed, 174 insertions(+), 63 deletions(-)
diff --git a/airflow-ctl/src/airflowctl/api/client.py
b/airflow-ctl/src/airflowctl/api/client.py
index 99a506b9cb2..b692c2f2e2d 100644
--- a/airflow-ctl/src/airflowctl/api/client.py
+++ b/airflow-ctl/src/airflowctl/api/client.py
@@ -64,6 +64,7 @@ from airflowctl.api.operations import (
XComOperations,
)
from airflowctl.exceptions import (
+ AirflowCtlCredentialInvalidException,
AirflowCtlCredentialNotFoundException,
AirflowCtlException,
AirflowCtlKeyringException,
@@ -162,6 +163,41 @@ 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, json.JSONDecodeError):
+ # Callers already treat an absent file and malformed JSON as missing
credentials.
+ raise
+ except OSError as e:
+ # `auth login` writes to the same path, so logging in again cannot fix
this.
+ raise AirflowCtlCredentialInvalidException(
+ f"Credentials file {path} could not be read: {e}. Check its
permissions."
+ ) from e
+ except ValueError as e:
+ # Non-UTF-8 bytes; malformed JSON was re-raised above.
+ 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
+
+
+def _read_api_url(path: str) -> str:
+ """Read the API URL from a credentials file; the debug-credentials file
has none, so it is not in _read_cli_config."""
+ api_url = _read_cli_config(path).get("api_url")
+ if not isinstance(api_url, str):
+ raise AirflowCtlCredentialInvalidException(
+ f"Credentials file {path} does not contain an api_url. Log in
again to recreate it."
+ )
+ return api_url
+
+
# Credentials for the API
class Credentials:
"""Credentials for the API."""
@@ -253,57 +289,57 @@ class 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}"
+ self.api_url = _read_api_url(config_path)
+ 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, json.JSONDecodeError):
# This is expected during the auth login command.
# Also allow token-only usage without local config (for commands
like `version --remote`).
# JSONDecodeError handles the case where the credentials file
exists but is empty or corrupted.
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:
+ # `auth login` has to be able to replace an unusable file, and it
is the recovery the
+ # message recommends. Every other kind would silently fall back to
the default API URL.
+ if self.client_kind != ClientKind.AUTH:
+ raise
return self
diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py
b/airflow-ctl/src/airflowctl/ctl/cli_config.py
index 455454fc271..25d5d6c870f 100755
--- a/airflow-ctl/src/airflowctl/ctl/cli_config.py
+++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py
@@ -46,12 +46,7 @@ from airflowctl.api.operations import BaseOperations,
ServerResponseError
from airflowctl.ctl.console_formatting import AirflowConsole, is_data_sequence
from airflowctl.ctl.utils.validation import split_validation_errors
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
@@ -98,12 +93,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, httpx.WriteError) as e:
diff --git a/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py
b/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py
index a2e87b307d2..9cbe48e9f81 100644
--- a/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py
+++ b/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py
@@ -32,11 +32,13 @@ from airflowctl.api.client import (
NEW_API_CLIENT,
ClientKind,
Credentials,
+ _read_api_url,
get_default_api_url,
provide_api_client,
)
from airflowctl.api.datamodels.auth_generated import LoginBody
from airflowctl.ctl.console_formatting import AirflowConsole
+from airflowctl.exceptions import AirflowCtlCredentialInvalidException
@provide_api_client(kind=ClientKind.AUTH)
@@ -187,11 +189,9 @@ def list_envs(args) -> None:
config_status = "ok"
try:
- with open(config_path) as f:
- config = json.load(f)
- api_url = config.get("api_url", "unknown")
- except (OSError, json.JSONDecodeError, KeyError) as e:
- config_status = f"config error: {str(e)[:50]}"
+ api_url = _read_api_url(config_path)
+ except (OSError, ValueError, AirflowCtlCredentialInvalidException) as
e:
+ config_status = f"config error: {e}"
api_url = "error reading config"
# Try to get token from keyring
diff --git a/airflow-ctl/src/airflowctl/exceptions.py
b/airflow-ctl/src/airflowctl/exceptions.py
index 0af8b9fa593..dc52d09ca52 100644
--- a/airflow-ctl/src/airflowctl/exceptions.py
+++ b/airflow-ctl/src/airflowctl/exceptions.py
@@ -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."""
diff --git a/airflow-ctl/tests/airflow_ctl/api/test_client.py
b/airflow-ctl/tests/airflow_ctl/api/test_client.py
index 511428c2135..17d1f634888 100644
--- a/airflow-ctl/tests/airflow_ctl/api/test_client.py
+++ b/airflow-ctl/tests/airflow_ctl/api/test_client.py
@@ -50,6 +50,7 @@ from airflowctl.api.client import (
)
from airflowctl.api.operations import ServerResponseError
from airflowctl.exceptions import (
+ AirflowCtlCredentialInvalidException,
AirflowCtlCredentialNotFoundException,
AirflowCtlException,
AirflowCtlKeyringException,
@@ -381,7 +382,7 @@ class TestCredentials:
mock_keyring.set_password = MagicMock()
mock_keyring.get_password = MagicMock()
- Credentials(client_kind=cli_client).save(skip_keyring=True)
+ Credentials(api_url="http://localhost:8080";,
client_kind=cli_client).save(skip_keyring=True)
config_dir = os.environ.get("AIRFLOW_HOME",
os.path.expanduser("~/airflow"))
assert os.path.exists(config_dir)
@@ -508,6 +509,55 @@ class TestCredentials:
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(b"\xff\xfe", None, "could not be read.*Log in again",
id="not-utf8"),
+ pytest.param(
+ b'{"api_url": "http://x"}', 0o000, "could not be read.*Check
its permissions", id="unreadable"
+ ),
+ pytest.param(json.dumps(["a"]).encode(), None, "is not a JSON
object", id="not-an-object"),
+ pytest.param(
+ json.dumps({"other": 1}).encode(), None, "does not contain an
api_url", id="no-api-url"
+ ),
+ pytest.param(
+ json.dumps({"api_url": None}).encode(), None, "does not
contain an api_url", id="null-api-url"
+ ),
+ ],
+ )
+ @pytest.mark.parametrize("client_kind", [ClientKind.CLI,
ClientKind.NO_AUTH])
+ def test_load_unusable_credentials_file_reports_an_error(
+ self, monkeypatch, tmp_path, contents, mode, expected, client_kind
+ ):
+ """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 mode is not None:
+ config.chmod(mode)
+
+ with pytest.raises(AirflowCtlCredentialInvalidException,
match=expected):
+ Credentials(client_kind=client_kind).load()
+
+ def test_load_unusable_debug_credentials_file_reports_an_error(self,
monkeypatch, tmp_path):
+ monkeypatch.setenv("AIRFLOW_HOME", str(tmp_path))
+ monkeypatch.setenv("AIRFLOW_CLI_ENVIRONMENT", "TEST_BAD_CONFIG")
+ monkeypatch.setenv("AIRFLOW_CLI_DEBUG_MODE", "true")
+ (tmp_path / "TEST_BAD_CONFIG.json").write_text(json.dumps({"api_url":
"http://x"}), encoding="utf-8")
+ (tmp_path /
"debug_creds_TEST_BAD_CONFIG.json").write_text(json.dumps(["a"]),
encoding="utf-8")
+
+ with pytest.raises(AirflowCtlCredentialInvalidException, match="is not
a JSON object"):
+ Credentials(client_kind=ClientKind.CLI).load()
+
+ def test_load_tolerates_an_unusable_file_for_auth_login(self, monkeypatch,
tmp_path):
+ """`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(json.dumps({"other":
1}), encoding="utf-8")
+
+ assert Credentials(client_kind=ClientKind.AUTH).load().api_url is None
+
class TestBoundedGetNewPassword:
@patch("airflowctl.api.client.getpass.getpass")
diff --git a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py
b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py
index 9cd7cf8d16d..d7331e6975b 100644
--- a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py
+++ b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py
@@ -546,6 +546,28 @@ class TestListEnvs:
args = self.parser.parse_args(["auth", "list-envs"])
auth_command.list_envs(args)
+ @pytest.mark.parametrize(
+ ("contents", "expected"),
+ [
+ pytest.param(b"\xff\xfe", "codec can't decode", id="not-utf8"),
+ pytest.param(b"[]", "not a JSON object", id="not-an-object"),
+ pytest.param(b'{"api_url": null}', "does not contain an api_url",
id="null-api-url"),
+ ],
+ )
+ @patch("airflowctl.ctl.commands.auth_command.AirflowConsole",
autospec=True)
+ @patch("keyring.get_password")
+ def test_list_envs_unusable_config(
+ self, mock_get_password, mock_console, monkeypatch, tmp_path,
contents, expected
+ ):
+ monkeypatch.setenv("AIRFLOW_HOME", str(tmp_path))
+ (tmp_path / "production.json").write_bytes(contents)
+
+ auth_command.list_envs(self.parser.parse_args(["auth", "list-envs"]))
+
+ [env] = mock_console.return_value.print_as.call_args.kwargs["data"]
+ assert env["api_url"] == "error reading config"
+ assert expected in env["status"]
+
def test_list_envs_debug_mode(self, monkeypatch):
"""Test list-envs in debug mode."""
with tempfile.TemporaryDirectory() as temp_airflow_home:
diff --git a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
index 3d09bef569e..961a41c8c77 100644
--- a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
+++ b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
@@ -70,6 +70,7 @@ from airflowctl.ctl.console_formatting import AirflowConsole
from airflowctl.ctl.utils.yaml import safe_load
from airflowctl.exceptions import (
AirflowCtlConnectionException,
+ AirflowCtlCredentialInvalidException,
AirflowCtlCredentialNotFoundException,
AirflowCtlKeyringException,
AirflowCtlNotFoundException,
@@ -722,11 +723,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):
