This is an automated email from the ASF dual-hosted git repository.
shahar1 pushed a commit to branch airflow-ctl/v0-1-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/airflow-ctl/v0-1-test by this
push:
new c55f756f034 [airflow-ctl/v0-1-test] Stop swallowing unexpected keyring
errors when saving airflowctl credentials (#70991) (#76047)
c55f756f034 is described below
commit c55f756f034c9aa4e4f820a69cc1e98d51634736
Author: Shahar Epstein <[email protected]>
AuthorDate: Fri Oct 30 17:56:09 2026 +0200
[airflow-ctl/v0-1-test] Stop swallowing unexpected keyring errors when
saving airflowctl credentials (#70991) (#76047)
* 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
* 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.
---------
(cherry picked from commit 2bb32e370cce86b1c422a249b95af1a0f743517b)
Co-authored-by: rjgoyln <[email protected]>
Co-authored-by: Henry Chen <[email protected]>
Co-authored-by: Y-C <[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 dfd961aee0f..5ca17140357 100644
--- a/airflow-ctl/src/airflowctl/api/client.py
+++ b/airflow-ctl/src/airflowctl/api/client.py
@@ -231,10 +231,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)
@@ -245,10 +247,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 46a45f054d0..049c384b652 100644
--- a/airflow-ctl/tests/airflow_ctl/api/test_client.py
+++ b/airflow-ctl/tests/airflow_ctl/api/test_client.py
@@ -280,7 +280,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")
