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 2bb32e370cc Stop swallowing unexpected keyring errors when saving
airflowctl credentials (#70991)
2bb32e370cc is described below
commit 2bb32e370cce86b1c422a249b95af1a0f743517b
Author: rjgoyln <[email protected]>
AuthorDate: Thu Sep 10 21:59:27 2026 +0800
Stop swallowing unexpected keyring errors when saving airflowctl
credentials (#70991)
* Stop swallowing unexpected keyring errors when saving airflowctl
credentials
The TypeError handler in Credentials.save() only produced a meaningful
error when api_token was None and client_kind was ClientKind.CLI at the
same time. No caller can satisfy both: the two save() call sites in
auth_command.py pass ClientKind.AUTH or leave client_kind at its default
of None, and each already guarantees a non-None token. The branch was
therefore unreachable, leaving the handler as an unconditional swallow
that let a failed credential save return as if it had succeeded.
* Update airflow-ctl/src/airflowctl/api/client.py
Co-authored-by: Y-C <[email protected]>
* Repair the duplicated keyring call left by the applied suggestion
Applying the review suggestion from the web UI kept the original
set_password call alongside the replacement, leaving the module
unparseable. The accompanying test constructed Credentials without a
token and only passed because the mocked backend accepted None.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
---------
Co-authored-by: Henry Chen <[email protected]>
Co-authored-by: Y-C <[email protected]>
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
airflow-ctl/src/airflowctl/api/client.py | 8 +++-----
airflow-ctl/tests/airflow_ctl/api/test_client.py | 22 +++++++++++++++++++++-
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/airflow-ctl/src/airflowctl/api/client.py
b/airflow-ctl/src/airflowctl/api/client.py
index e0c9c60f0ee..e160cf295b7 100644
--- a/airflow-ctl/src/airflowctl/api/client.py
+++ b/airflow-ctl/src/airflowctl/api/client.py
@@ -229,10 +229,12 @@ class Credentials:
for candidate in candidates:
if hasattr(candidate, "_get_new_password"):
candidate._get_new_password = _bounded_get_new_password
+ if self.api_token is None:
+ raise AirflowCtlCredentialNotFoundException("No API token
found. Please login first.")
keyring.set_password(
"airflowctl",
self.token_key_for_environment(self.api_environment),
- self.api_token, # type: ignore[arg-type]
+ self.api_token,
)
except (NoKeyringError, NotImplementedError) as e:
log.error(e)
@@ -243,10 +245,6 @@ class Credentials:
"the --api-token flag to any command.\n"
"Use `airflowctl auth login --skip-keyring ...` to dismiss
this error."
) from e
- except TypeError as e:
- # This happens when the token is None, which is not allowed by
keyring
- if self.api_token is None and self.client_kind == ClientKind.CLI:
- raise AirflowCtlCredentialNotFoundException("No API token
found. Please login first.") from e
def load(self) -> Credentials:
"""Load the credentials from keyring and URL from disk file."""
diff --git a/airflow-ctl/tests/airflow_ctl/api/test_client.py
b/airflow-ctl/tests/airflow_ctl/api/test_client.py
index b7e50073851..3db2a46a896 100644
--- a/airflow-ctl/tests/airflow_ctl/api/test_client.py
+++ b/airflow-ctl/tests/airflow_ctl/api/test_client.py
@@ -216,7 +216,27 @@ class TestCredentials:
mock_keyring.set_password.side_effect = NoKeyringError("no backend")
with pytest.raises(AirflowCtlKeyringException, match="Keyring backend
is not available"):
- Credentials(client_kind=cli_client).save()
+ Credentials(client_kind=cli_client, api_token="TEST_TOKEN").save()
+
+ @patch.dict(os.environ, {"AIRFLOW_CLI_ENVIRONMENT":
"TEST_SAVE_KEYRING_TYPE_ERROR"})
+ @patch("airflowctl.api.client.keyring")
+ def test_save_propagates_unexpected_keyring_error(self, mock_keyring):
+ mock_keyring.set_password.side_effect = TypeError("password must be a
string")
+
+ with pytest.raises(TypeError, match="password must be a string"):
+ Credentials(
+ api_url="http://localhost:8080";,
+ api_token="TEST_TOKEN",
+ client_kind=ClientKind.AUTH,
+ ).save()
+
+ @patch.dict(os.environ, {"AIRFLOW_CLI_ENVIRONMENT": "TEST_SAVE_NO_TOKEN"})
+ @patch("airflowctl.api.client.keyring")
+ def test_save_without_token(self, mock_keyring):
+ with pytest.raises(AirflowCtlCredentialNotFoundException, match="No
API token found"):
+ Credentials(api_url="http://localhost:8080";,
client_kind=ClientKind.AUTH).save()
+
+ mock_keyring.set_password.assert_not_called()
@patch.dict(os.environ, {"AIRFLOW_CLI_ENVIRONMENT":
"TEST_SAVE_SKIP_KEYRING"})
@patch("airflowctl.api.client.keyring")
