diff --git a/airflow-ctl-tests/tests/airflowctl_tests/conftest.py b/airflow-ctl-tests/tests/airflowctl_tests/conftest.py index 9e6ef75b7fa5a..cbcea44ec3629 100644 --- a/airflow-ctl-tests/tests/airflowctl_tests/conftest.py +++ b/airflow-ctl-tests/tests/airflowctl_tests/conftest.py @@ -21,6 +21,7 @@ import sys import pytest +import requests from python_on_whales import DockerClient, docker from airflowctl_tests import console @@ -35,16 +36,31 @@ from tests_common.test_utils.fernet import generate_fernet_key_string +@pytest.fixture(scope="module") +def api_token(): + url = "http://localhost:8080/auth/token" + payload = {"username": "airflow", "password": "airflow"} + try: + response = requests.post(url, json=payload) + response.raise_for_status() + token = response.json().get("access_token") + if not token: + raise ValueError("Response did not contain an access_token") + return token + except requests.exceptions.RequestException as e: + pytest.fail(f"Failed to obtain token: {e}") + + @pytest.fixture def run_command(): """Fixture that provides a helper to run airflowctl commands.""" - def _run_command(command: str, skip_login: bool = False) -> str: + def _run_command(command: str, env_vars: dict, skip_login: bool = False) -> str: import os from subprocess import PIPE, STDOUT, Popen host_envs = os.environ.copy() - host_envs["AIRFLOW_CLI_DEBUG_MODE"] = "true" + host_envs.update(env_vars) command_from_config = f"airflowctl {command}" @@ -231,8 +247,6 @@ def docker_compose_up(tmp_path_factory): dot_env_file = tmp_dir / ".env" dot_env_file.write_text( f"AIRFLOW_UID={os.getuid()}\n" - # To enable debug mode for airflowctl CLI - "AIRFLOW_CTL_CLI_DEBUG_MODE=true\n" # To enable config operations to work "AIRFLOW__API__EXPOSE_CONFIG=true\n" ) diff --git a/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py b/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py index 228321ac7e89d..3f89a7c969818 100644 --- a/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py +++ b/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py @@ -18,8 +18,6 @@ import pytest -from airflowctl_tests.constants import LOGIN_COMMAND - def date_param(): import random @@ -44,11 +42,12 @@ def date_param(): return random_dt.isoformat() -ONE_DATE_PARAM = date_param() +# Passing password via command line is insecure but acceptable for testing purposes +# Please do not do this in production, it enables possibility of exposing your credentials +LOGIN_COMMAND = "auth login --username airflow --password airflow" +LOGIN_COMMAND_SKIP_KEYRING = "auth login --skip-keyring" +LOGIN_OUTPUT = "Login successful! Welcome to airflowctl!" TEST_COMMANDS = [ - # Passing password via command line is insecure but acceptable for testing purposes - # Please do not do this in production, it enables possibility of exposing your credentials - LOGIN_COMMAND, # Assets commands "assets list", "assets get --asset-id=1", @@ -80,22 +79,22 @@ def date_param(): "dags list-version --dag-id=example_bash_operator", "dags list-warning", # Order of trigger and pause/unpause is important for test stability because state checked - f"dags trigger --dag-id=example_bash_operator --logical-date={ONE_DATE_PARAM} --run-after={ONE_DATE_PARAM}", + "dags trigger --dag-id=example_bash_operator --logical-date={date_param} --run-after={date_param}", # Test trigger without logical-date (should default to now) "dags trigger --dag-id=example_bash_operator", "dags pause example_bash_operator", "dags unpause example_bash_operator", # DAG Run commands - f'dagrun get --dag-id=example_bash_operator --dag-run-id="manual__{ONE_DATE_PARAM}"', + 'dagrun get --dag-id=example_bash_operator --dag-run-id="manual__{date_param}"', "dags update --dag-id=example_bash_operator --no-is-paused", # DAG Run commands "dagrun list --dag-id example_bash_operator --state success --limit=1", # XCom commands - need a DAG run with completed tasks - f'xcom add --dag-id=example_bash_operator --dag-run-id="manual__{ONE_DATE_PARAM}" --task-id=runme_0 --key=test_xcom_key --value=\'{{"test": "value"}}\'', - f'xcom get --dag-id=example_bash_operator --dag-run-id="manual__{ONE_DATE_PARAM}" --task-id=runme_0 --key=test_xcom_key', - f'xcom list --dag-id=example_bash_operator --dag-run-id="manual__{ONE_DATE_PARAM}" --task-id=runme_0', - f'xcom edit --dag-id=example_bash_operator --dag-run-id="manual__{ONE_DATE_PARAM}" --task-id=runme_0 --key=test_xcom_key --value=\'{{"updated": "value"}}\'', - f'xcom delete --dag-id=example_bash_operator --dag-run-id="manual__{ONE_DATE_PARAM}" --task-id=runme_0 --key=test_xcom_key', + 'xcom add --dag-id=example_bash_operator --dag-run-id="manual__{date_param}" --task-id=runme_0 --key=test_xcom_key --value=\'{{"test": "value"}}\'', + 'xcom get --dag-id=example_bash_operator --dag-run-id="manual__{date_param}" --task-id=runme_0 --key=test_xcom_key', + 'xcom list --dag-id=example_bash_operator --dag-run-id="manual__{date_param}" --task-id=runme_0', + 'xcom edit --dag-id=example_bash_operator --dag-run-id="manual__{date_param}" --task-id=runme_0 --key=test_xcom_key --value=\'{{"updated": "value"}}\'', + 'xcom delete --dag-id=example_bash_operator --dag-run-id="manual__{date_param}" --task-id=runme_0 --key=test_xcom_key', # Jobs commands "jobs list", # Pools commands @@ -124,11 +123,38 @@ def date_param(): "version --remote", ] +DATE_PARAM_1 = date_param() +DATE_PARAM_2 = date_param() +TEST_COMMANDS_DEBUG_MODE = [LOGIN_COMMAND] + [test.format(date_param=DATE_PARAM_1) for test in TEST_COMMANDS] +TEST_COMMANDS_SKIP_KEYRING = [LOGIN_COMMAND_SKIP_KEYRING] + [ + test.format(date_param=DATE_PARAM_2) for test in TEST_COMMANDS +] + @pytest.mark.flaky(reruns=3, reruns_delay=1) @pytest.mark.parametrize( - "command", TEST_COMMANDS, ids=[" ".join(command.split(" ", 2)[:2]) for command in TEST_COMMANDS] + "command", + TEST_COMMANDS_DEBUG_MODE, + ids=[" ".join(command.split(" ", 2)[:2]) for command in TEST_COMMANDS_DEBUG_MODE], ) def test_airflowctl_commands(command: str, run_command): """Test airflowctl commands using docker-compose environment.""" - run_command(command) + env_vars = {"AIRFLOW_CLI_DEBUG_MODE": "true"} + + run_command(command, env_vars, skip_login=True) + + +@pytest.mark.flaky(reruns=3, reruns_delay=1) +@pytest.mark.parametrize( + "command", + TEST_COMMANDS_SKIP_KEYRING, + ids=[" ".join(command.split(" ", 2)[:2]) for command in TEST_COMMANDS_SKIP_KEYRING], +) +def test_airflowctl_commands_skip_keyring(command: str, api_token: str, run_command): + """Test airflowctl commands using docker-compose environment without using keyring.""" + env_vars = {} + env_vars["AIRFLOW_CLI_TOKEN"] = api_token + env_vars["AIRFLOW_CLI_DEBUG_MODE"] = "false" + env_vars["AIRFLOW_CLI_ENVIRONMENT"] = "nokeyring" + + run_command(command, env_vars, skip_login=True) diff --git a/airflow-ctl-tests/tests/airflowctl_tests/test_config_sensitive_masking.py b/airflow-ctl-tests/tests/airflowctl_tests/test_config_sensitive_masking.py index 84719bb4cc3cf..776629da3a26e 100644 --- a/airflow-ctl-tests/tests/airflowctl_tests/test_config_sensitive_masking.py +++ b/airflow-ctl-tests/tests/airflowctl_tests/test_config_sensitive_masking.py @@ -41,7 +41,9 @@ def test_config_sensitive_masking(command: str, run_command): Airflow API, sensitive values (like fernet_key, sql_alchemy_conn) appear masked as '< hidden >' and do not leak actual secret values. """ - stdout_result = run_command(command) + + env_vars = {"AIRFLOW_CLI_DEBUG_MODE": "true"} + stdout_result = run_command(command, env_vars) # CRITICAL: Verify that sensitive values are masked # The Airflow API returns masked values as "< hidden >" for sensitive configs diff --git a/airflow-ctl/README.md b/airflow-ctl/README.md index 28f1b47381224..19e11b1369dcb 100644 --- a/airflow-ctl/README.md +++ b/airflow-ctl/README.md @@ -33,7 +33,8 @@ A command-line tool for interacting with Apache Airflow instances through the Ai - Python 3.10 or later (compatible with Python >= 3.10 and < 3.13) - Network access to an Apache Airflow instance with REST API enabled -- Keyring backend installed in operating system for secure token storage +- \[Recommended\] Keyring backend installed in operating system for secure token storage. + - In case there's no keyring available (common in headless environments) you can provide the token to each command. See the [Security page](https://airflow.apache.org/docs/apache-airflow-ctl/stable/security.html) for more information. ## Usage diff --git a/airflow-ctl/docs/howto/index.rst b/airflow-ctl/docs/howto/index.rst index 12946aed4498b..155dab5fc04d4 100644 --- a/airflow-ctl/docs/howto/index.rst +++ b/airflow-ctl/docs/howto/index.rst @@ -53,6 +53,7 @@ airflowctl needs to be able to connect to the Airflow API. You should pass API U You can also set the environment variable ``AIRFLOW_CLI_TOKEN`` to the token to use for authentication. There are two ways to authenticate with the Airflow API: + 1. Using a token acquired from the Airflow API .. code-block:: bash @@ -61,17 +62,18 @@ There are two ways to authenticate with the Airflow API: 2. Using a username and password - .. code-block:: bash airflowctl auth login --api-url --username --password --env -3. (optional) Using a token acquired from the Airflow API and username and password +If there's no keyring available, common in headless systems like docker images, you can still use the tool by setting +the environment variable ``AIRFLOW_CLI_TOKEN``. .. code-block:: bash export AIRFLOW_CLI_TOKEN= - airflowctl auth login --api-url --env + airflowctl auth login --api-url --env --skip-keyring + In both cases token is securely stored in the keyring backend. Only configuration persisted in ``~/.config/airflow`` file is the API URL and the environment name. The token is stored in the keyring backend and is not persisted in the @@ -108,6 +110,10 @@ If you provide a username via ``--username`` this is the required password to au The name of the environment to create or update. The default value is ``production``. This parameter is useful when you want to manage multiple Airflow environments. +**--skip-keyring**: This parameter is optional. +Useful when there's no keyring available in the system where airflowctl is running. +Set ``AIRFLOW_CLI_TOKEN`` or use the ``--api-token`` flag for next commands. + More Usage and Help Pictures '''''''''''''''''''''''''''' For more information use diff --git a/airflow-ctl/docs/images/command_hashes.txt b/airflow-ctl/docs/images/command_hashes.txt index 7f990f14b1be1..46f6f42baf000 100644 --- a/airflow-ctl/docs/images/command_hashes.txt +++ b/airflow-ctl/docs/images/command_hashes.txt @@ -10,5 +10,5 @@ jobs:7f8680afff230eb9940bc7fca727bd52 pools:03fc7d948cbecf16ff8d640eb8f0ce43 providers:1c0afb2dff31d93ab2934b032a2250ab variables:0354f8f4b0dde1c3771ed1568692c6ae -version:d4a7a6229b3a204f114283b62eac789b -auth login:5277c653ff6dce51f37472dc0bda9775 +version:31f4efdf8de0dbaaa4fac71ff7efecc3 +auth login:f85e04072626ab4ae17ad17e4a077bf2 diff --git a/airflow-ctl/docs/images/output_auth_login.svg b/airflow-ctl/docs/images/output_auth_login.svg index afa17cb9b56b9..006ed5bc2664d 100644 --- a/airflow-ctl/docs/images/output_auth_login.svg +++ b/airflow-ctl/docs/images/output_auth_login.svg @@ -1,4 +1,4 @@ - + - - + + - - + + - - + + - - + + - - + + - - + + - - + + - - + + - - + + - - + + - - + + - - + + - - + + - - + + - - + + - - + + + + + - + - + - - Usage:airflowctl auth login [-h] [--api-tokenAPI_TOKEN] -                             [--api-urlAPI_URL] [-eENV] -                             [--password [PASSWORD]] -                             [--usernameUSERNAME] - -Login to the metadata database - -Options: --h, --helpshow this help message and exit ---api-tokenAPI_TOKEN -The token to use for authentication ---api-urlAPI_URLThe URL of the metadata database API --e, --envENVThe environment to run the command in ---password [PASSWORD] -The password to use for authentication ---usernameUSERNAMEThe username to use for authentication + + Usage:airflowctl auth login [-h] [--api-tokenAPI_TOKEN] +                             [--api-urlAPI_URL] [-eENV] +                             [--password [PASSWORD]] [--skip-keyring] +                             [--usernameUSERNAME] + +Login to the metadata database + +Options: +-h, --helpshow this help message and exit +--api-tokenAPI_TOKEN +The token to use for authentication +--api-urlAPI_URLThe URL of the metadata database API +-e, --envENVThe environment to run the command in +--password [PASSWORD] +The password to use for authentication +--skip-keyringSkip storing credentials in keyring +--usernameUSERNAMEThe username to use for authentication diff --git a/airflow-ctl/docs/images/output_version.svg b/airflow-ctl/docs/images/output_version.svg index a7ce436c8b8aa..ade6b57810e6a 100644 --- a/airflow-ctl/docs/images/output_version.svg +++ b/airflow-ctl/docs/images/output_version.svg @@ -1,4 +1,4 @@ - + - - + + - - + + - - + + - - + + - - + + - - + + - - + + - - + + - - + + + + + + + + + + + - + - + - - Usage:airflowctl version [-h] [-eENV] [--remote] - -Show version information - -Options: --h, --helpshow this help message and exit --e, --envENVThe environment to run the command in ---remoteFetch the Airflow version in remote server,  -otherwise only shows the local airflowctl version + + Usage:airflowctl version [-h] [--api-tokenAPI_TOKEN] [-eENV] +                          [--remote] + +Show version information + +Options: +-h, --helpshow this help message and exit +--api-tokenAPI_TOKEN +The token to use for authentication +-e, --envENVThe environment to run the command in +--remoteFetch the Airflow version in remote server,  +otherwise only shows the local airflowctl version diff --git a/airflow-ctl/docs/installation/prerequisites.rst b/airflow-ctl/docs/installation/prerequisites.rst index a155a8debd828..b01ff4c3cc41b 100644 --- a/airflow-ctl/docs/installation/prerequisites.rst +++ b/airflow-ctl/docs/installation/prerequisites.rst @@ -25,8 +25,8 @@ The minimum memory required we recommend airflowctl to run with is 200MB, but th wildly on the deployment options you have. The Keyring backend needs to be installed separately into your operating system. This will enhance security. See :doc:`/security` for more information. -Keyring Backend -''''''''''''''' +Keyring Backend \[Recommended\] +''''''''''''''''''''''''''''''' airflowctl uses keyring to store the API token securely. This ensures that the token is not stored in plain text and is only accessible to authorized users. Recommended keyring backends are: @@ -36,6 +36,8 @@ Recommended keyring backends are: * `KDE4 & KDE5 KWallet `_ (requires `dbus `_) * `Windows Credential Locker `_ +In case there's no keyring available (common in headless environments) you can provide the token to each command. See :doc:`/security` for more information. + Third-Party Backends ==================== diff --git a/airflow-ctl/docs/security.rst b/airflow-ctl/docs/security.rst index 8ec5ea6d6f419..fb08f4f68b3e9 100644 --- a/airflow-ctl/docs/security.rst +++ b/airflow-ctl/docs/security.rst @@ -23,6 +23,7 @@ airflowctl facilitates the seamless deployment of CLI and API features together, - **Authentication**: airflowctl uses authentication to ensure that only authorized users can access the system. This is done using an API Token. See more on https://airflow.apache.org/docs/apache-airflow/stable/security/api.html - **Keyring**: airflowctl uses keyring to store the API Token securely. This ensures that the Token is not stored in plain text and is only accessible to authorized users. + - In case no keyring is available, you can set the ``AIRFLOW_CLI_TOKEN`` environment variable or the ``--api-token`` flag for each command. Be cautious of not exposing this token to others. airflowctl API Token has its own expiration time. The default is 1 hour. You can change it in the Airflow configuration file (airflow.cfg) by setting the ``jwt_cli_expiration_time`` parameter under the ``[api_auth]`` section. The value is in seconds. This will impact all users using ``airflowctl``. diff --git a/airflow-ctl/src/airflowctl/api/client.py b/airflow-ctl/src/airflowctl/api/client.py index ec703ebf69c6c..7c03dae30554a 100644 --- a/airflow-ctl/src/airflowctl/api/client.py +++ b/airflow-ctl/src/airflowctl/api/client.py @@ -53,7 +53,6 @@ ) from airflowctl.exceptions import ( AirflowCtlCredentialNotFoundException, - AirflowCtlException, AirflowCtlKeyringException, AirflowCtlNotFoundException, ) @@ -161,8 +160,13 @@ def input_cli_config_file(self) -> str: """Generate path for the CLI config file.""" return f"{self.api_environment}.json" - def save(self): - """Save the credentials to keyring and URL to disk as a file.""" + def save(self, skip_keyring: bool = False): + """ + Save the credentials to keyring and URL to disk as a file. + + Skip saving the token to keyring if skip_keyring is True, in this case, + only the config file with the API URL is created. + """ default_config_dir = os.environ.get("AIRFLOW_HOME", os.path.expanduser("~/airflow")) os.makedirs(default_config_dir, exist_ok=True) with open(os.path.join(default_config_dir, self.input_cli_config_file), "w") as f: @@ -175,6 +179,8 @@ def save(self): ) as f: json.dump({f"api_token_{self.api_environment}": self.api_token}, f) else: + if skip_keyring: + return # Replace the upstream EncryptedKeyring's unbounded password # prompt with a bounded one before set_password can trigger it. # The active backend may be a ChainerBackend that delegates to @@ -184,11 +190,15 @@ def save(self): for candidate in candidates: if hasattr(candidate, "_get_new_password"): candidate._get_new_password = _bounded_get_new_password - keyring.set_password("airflowctl", f"api_token_{self.api_environment}", self.api_token) - except NoKeyringError as e: + keyring.set_password("airflowctl", f"api_token_{self.api_environment}", self.api_token) # type: ignore[arg-type] + except (NoKeyringError, NotImplementedError) as e: log.error(e) raise AirflowCtlKeyringException( - "Keyring backend is not available. Cannot save credentials." + "Keyring backend is not available. Cannot save credentials.\n" + "The api url config was saved and you can still use airflowctl " + "by setting the AIRFLOW_CLI_TOKEN environment variable or passing " + "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 @@ -203,6 +213,8 @@ def load(self) -> Credentials: with open(config_path) as f: credentials = json.load(f) self.api_url = credentials["api_url"] + if self.api_token is not None: + return self if os.getenv("AIRFLOW_CLI_DEBUG_MODE") == "true": debug_creds_path = os.path.join( default_config_dir, f"debug_creds_{self.input_cli_config_file}" @@ -232,13 +244,9 @@ def load(self) -> Credentials: raise AirflowCtlKeyringException("Keyring backend is not available") from e self.api_token = None except FileNotFoundError: - if self.client_kind == ClientKind.AUTH: - # Saving the URL set from the Auth Commands if Kind is AUTH - self.save() - elif self.client_kind == ClientKind.CLI: + # This is expected during the auth login command + if self.client_kind != ClientKind.AUTH: raise AirflowCtlCredentialNotFoundException("No credentials file found. Please login first.") - else: - raise AirflowCtlException(f"Unknown client kind: {self.client_kind}") return self @@ -371,20 +379,21 @@ def xcom(self): # API Client Decorator for CLI Actions @contextlib.contextmanager -def get_client(kind: Literal[ClientKind.CLI, ClientKind.AUTH] = ClientKind.CLI): +def get_client(kind: Literal[ClientKind.CLI, ClientKind.AUTH] = ClientKind.CLI, api_token: str | None = None): """ Get CLI API client. Don't call this method, please use @provide_api_client decorator instead. """ api_client = None + api_token = api_token or os.getenv("AIRFLOW_CLI_TOKEN", None) try: # API URL always loaded from the config file, please save with it if you are using other than ClientKind.CLI - credentials = Credentials(client_kind=kind).load() + credentials = Credentials(client_kind=kind, api_token=api_token).load() api_client = Client( base_url=credentials.api_url or "http://localhost:8080", limits=httpx.Limits(max_keepalive_connections=1, max_connections=1), - token=credentials.api_token or str(os.getenv("AIRFLOW_CLI_TOKEN", "")), + token=str(api_token or credentials.api_token), kind=kind, ) yield api_client @@ -412,7 +421,8 @@ def decorator(func: Callable[PS, RT]) -> Callable[PS, RT]: @wraps(func) def wrapper(*args, **kwargs) -> RT: if "api_client" not in kwargs: - with get_client(kind=kind) as api_client: + api_token = getattr(args[0], "api_token", None) if args else None + with get_client(kind=kind, api_token=api_token) as api_client: return func(*args, api_client=api_client, **kwargs) # The CLI API Client should be only passed for Mocking and Testing return func(*args, **kwargs) diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py b/airflow-ctl/src/airflowctl/ctl/cli_config.py old mode 100644 new mode 100755 index 5f7114480c2df..fcc0762b9f653 --- a/airflow-ctl/src/airflowctl/ctl/cli_config.py +++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py @@ -243,6 +243,13 @@ def __call__(self, parser, namespace, values, option_string=None): dest="username", help="The username to use for authentication", ) +ARG_AUTH_SKIP_KEYRING = Arg( + flags=("--skip-keyring",), + dest="skip_keyring", + default=False, + action="store_true", + help="Skip storing credentials in keyring", +) ARG_AUTH_PASSWORD = Arg( flags=("--password",), type=str, @@ -749,6 +756,23 @@ def group_commands(self) -> list[CLICommand]: return self.group_commands_list +def add_auth_token_to_all_commands(commands: Iterable[CLICommand]) -> list[CLICommand]: + """Add ARG_AUTH_TOKEN to all ActionCommands.""" + new_commands: list[CLICommand] = [] + for command in commands: + if isinstance(command, ActionCommand): + new_args = list(command.args) + if ARG_AUTH_TOKEN not in new_args: + new_args.append(ARG_AUTH_TOKEN) + new_commands.append(command._replace(args=new_args)) + elif isinstance(command, GroupCommand): + new_subcommands = add_auth_token_to_all_commands(command.subcommands) + new_commands.append(command._replace(subcommands=new_subcommands)) + else: + new_commands.append(command) + return new_commands + + def merge_commands( base_commands: list[CLICommand], commands_will_be_merged: list[CLICommand] ) -> list[CLICommand]: @@ -812,7 +836,13 @@ def merge_commands( help="Login to the metadata database for personal usage. JWT Token must be provided via parameter.", description="Login to the metadata database", func=lazy_load_command("airflowctl.ctl.commands.auth_command.login"), - args=(ARG_AUTH_URL, ARG_AUTH_TOKEN, ARG_AUTH_ENVIRONMENT, ARG_AUTH_USERNAME, ARG_AUTH_PASSWORD), + args=( + ARG_AUTH_URL, + ARG_AUTH_ENVIRONMENT, + ARG_AUTH_USERNAME, + ARG_AUTH_PASSWORD, + ARG_AUTH_SKIP_KEYRING, + ), ), ActionCommand( name="list-envs", @@ -943,3 +973,5 @@ def merge_commands( core_commands = merge_commands( base_commands=command_factory.group_commands, commands_will_be_merged=core_commands ) +# Add ARG_AUTH_TOKEN to all commands +core_commands = add_auth_token_to_all_commands(core_commands) diff --git a/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py b/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py index 8cc55ff69e815..722219f52ba35 100644 --- a/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py +++ b/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py @@ -38,7 +38,9 @@ def login(args, api_client=NEW_API_CLIENT) -> None: success_message = "[green]Login successful! Welcome to airflowctl![/green]" # Check is username and password are passed if args.username and args.password: - # Generate empty credentials with the api_url and env + if args.skip_keyring: + rich.print("[red]The --skip-keyring is not compatible with username and password login.") + sys.exit(1) credentials = Credentials( api_url=args.api_url, api_token="", @@ -47,7 +49,6 @@ def login(args, api_client=NEW_API_CLIENT) -> None: ) # After logging in, the token will be saved in the credentials file try: - credentials.save() api_client.refresh_base_url(base_url=args.api_url, kind=ClientKind.AUTH) login_response = api_client.login.login_with_username_and_password( LoginBody( @@ -78,7 +79,7 @@ def login(args, api_client=NEW_API_CLIENT) -> None: api_url=args.api_url, api_token=token, api_environment=args.env, - ).save() + ).save(args.skip_keyring) rich.print(success_message) diff --git a/airflow-ctl/tests/airflow_ctl/api/test_client.py b/airflow-ctl/tests/airflow_ctl/api/test_client.py index 8d4a24c28b740..f79322d16fb74 100644 --- a/airflow-ctl/tests/airflow_ctl/api/test_client.py +++ b/airflow-ctl/tests/airflow_ctl/api/test_client.py @@ -126,6 +126,38 @@ def test_save(self, mock_keyring): "api_url": credentials.api_url, } + @patch.dict(os.environ, {"AIRFLOW_CLI_ENVIRONMENT": "TEST_SAVE_NO_KEYRING"}) + @patch("airflowctl.api.client.keyring") + def test_save_no_keyring(self, mock_keyring): + from keyring.errors import NoKeyringError + + cli_client = ClientKind.CLI + 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() + + @patch.dict(os.environ, {"AIRFLOW_CLI_ENVIRONMENT": "TEST_SAVE_SKIP_KEYRING"}) + @patch("airflowctl.api.client.keyring") + def test_save_no_keyring_backend_skip_keyring(self, mock_keyring): + + env = "TEST_SAVE_SKIP_KEYRING" + cli_client = ClientKind.CLI + mock_keyring.set_password = MagicMock() + mock_keyring.get_password = MagicMock() + + Credentials(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) + with open(os.path.join(config_dir, f"{env}.json")) as f: + credentials = Credentials(client_kind=cli_client, api_token="TEST_TOKEN").load() + assert json.load(f) == { + "api_url": credentials.api_url, + } + mock_keyring.set_password.assert_not_called() + mock_keyring.get_password.assert_not_called() + @patch.dict(os.environ, {"AIRFLOW_CLI_ENVIRONMENT": "TEST_LOAD"}) @patch.dict(os.environ, {"AIRFLOW_CLI_TOKEN": "TEST_TOKEN"}) @patch("airflowctl.api.client.keyring") @@ -196,6 +228,21 @@ def test_load_no_keyring_backend(self, mock_keyring): with pytest.raises(AirflowCtlKeyringException, match="Keyring backend is not available"): Credentials(client_kind=cli_client).load() + @patch.dict(os.environ, {"AIRFLOW_CLI_ENVIRONMENT": "TEST_NO_KEYRING_BACKEND"}) + @patch("airflowctl.api.client.keyring") + def test_load_no_keyring_backend_token_provided(self, mock_keyring): + from keyring.errors import NoKeyringError + + cli_client = ClientKind.CLI + config_dir = os.environ.get("AIRFLOW_HOME", os.path.expanduser("~/airflow")) + os.makedirs(config_dir, exist_ok=True) + with open(os.path.join(config_dir, "TEST_NO_KEYRING_BACKEND.json"), "w") as f: + json.dump({"api_url": "http://localhost:8080"}, f) + mock_keyring.get_password.side_effect = NoKeyringError("no backend") + + credentials = Credentials(client_kind=cli_client, api_token="TEST_TOKEN").load() + assert credentials.api_token == "TEST_TOKEN" + 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 54fb83901ee9e..70d19d26657f1 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 @@ -70,6 +70,52 @@ def test_login(self, mock_keyring, api_client_maker, monkeypatch): "airflowctl", "api_token_TEST_AUTH_LOGIN", "TEST_TOKEN" ) + @patch.dict(os.environ, {"AIRFLOW_CLI_TOKEN": "TEST_TOKEN"}) + @patch("airflowctl.api.client.keyring") + def test_login_with_skip_keyring(self, mock_keyring, api_client_maker): + from keyring.errors import NoKeyringError + + api_client = api_client_maker( + path="/auth/token/cli", + response_json=self.login_response.model_dump(), + expected_http_status_code=201, + kind=ClientKind.AUTH, + ) + + mock_keyring.set_password.side_effect = NoKeyringError("no backend") + with ( + patch("sys.stdin", io.StringIO("test_password")), + patch("airflowctl.ctl.cli_config.getpass.getpass", return_value="test_password"), + ): + auth_command.login( + self.parser.parse_args( + ["auth", "login", "--skip-keyring", "--api-url", "http://localhost:8080"] + ), + api_client=api_client, + ) + + @patch("airflowctl.api.client.keyring") + def test_login_without_skip_keyring_raises_on_no_keyring(self, mock_keyring, api_client_maker): + from keyring.errors import NoKeyringError + + api_client = api_client_maker( + path="/auth/token/cli", + response_json=self.login_response.model_dump(), + expected_http_status_code=201, + kind=ClientKind.AUTH, + ) + + mock_keyring.set_password.side_effect = NoKeyringError("no backend") + with ( + patch("sys.stdin", io.StringIO("test_password")), + patch("airflowctl.ctl.cli_config.getpass.getpass", return_value="test_password"), + pytest.raises(SystemExit, match="1"), + ): + auth_command.login( + self.parser.parse_args(["auth", "login", "--api-url", "http://localhost:8080"]), + api_client=api_client, + ) + # Test auth login with username and password @patch("airflowctl.api.client.keyring") def test_login_with_username_and_password(self, mock_keyring, api_client_maker): @@ -102,7 +148,6 @@ def test_login_with_username_and_password(self, mock_keyring, api_client_maker): ) mock_keyring.set_password.assert_has_calls( [ - mock.call("airflowctl", "api_token_production", ""), mock.call("airflowctl", "api_token_production", "TEST_TOKEN"), ] ) diff --git a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_pool_command.py b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_pool_command.py index 44cc341ee1c36..5ebfdc869c874 100644 --- a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_pool_command.py +++ b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_pool_command.py @@ -48,21 +48,21 @@ def test_import_missing_file(self, mock_client, tmp_path): """Test import with missing file.""" non_existent = tmp_path / "non_existent.json" with pytest.raises(SystemExit, match=f"Missing pools file {non_existent}"): - pool_command.import_(args=mock.MagicMock(file=non_existent)) + pool_command.import_(mock.MagicMock(file=non_existent)) def test_import_invalid_json(self, mock_client, tmp_path): """Test import with invalid JSON file.""" invalid_json = tmp_path / "invalid.json" invalid_json.write_text("invalid json") with pytest.raises(SystemExit, match="Invalid json file"): - pool_command.import_(args=mock.MagicMock(file=invalid_json)) + pool_command.import_(mock.MagicMock(file=invalid_json)) def test_import_invalid_pool_config(self, mock_client, tmp_path): """Test import with invalid pool configuration.""" invalid_pool = tmp_path / "invalid_pool.json" invalid_pool.write_text(json.dumps([{"invalid": "config"}])) with pytest.raises(SystemExit, match="Invalid pool configuration: {'invalid': 'config'}"): - pool_command.import_(args=mock.MagicMock(file=invalid_pool)) + pool_command.import_(mock.MagicMock(file=invalid_pool)) def test_import_success(self, mock_client, tmp_path, capsys): """Test successful pool import.""" @@ -87,7 +87,7 @@ def test_import_success(self, mock_client, tmp_path, capsys): mock_client.pools.bulk.return_value = mock_bulk_builder - pool_command.import_(args=mock.MagicMock(file=pools_file)) + pool_command.import_(mock.MagicMock(file=pools_file)) # Verify bulk operation was called with correct parameters mock_client.pools.bulk.assert_called_once() @@ -134,7 +134,7 @@ def test_export_json_to_file(self, mock_client, tmp_path, capsys): mock_pools.total_entries = 1 mock_client.pools.list.return_value = mock_pools - pool_command.export(args=mock.MagicMock(file=export_file, output="json")) + pool_command.export(mock.MagicMock(file=export_file, output="json")) # Verify the exported file content exported_data = json.loads(export_file.read_text()) @@ -170,7 +170,7 @@ def test_export_non_json_output(self, mock_client, tmp_path, capsys): mock_pools.total_entries = 1 mock_client.pools.list.return_value = mock_pools - pool_command.export(args=mock.MagicMock(file=tmp_path / "unused.json", output="table")) + pool_command.export(mock.MagicMock(file=tmp_path / "unused.json", output="table")) # Verify console output contains the raw dict captured = capsys.readouterr() @@ -185,4 +185,4 @@ def test_export_failure(self, mock_client, tmp_path): mock_client.pools.list.side_effect = Exception("API Error") with pytest.raises(SystemExit, match="Failed to export pools: API Error"): - pool_command.export(args=mock.MagicMock(file=export_file, output="json")) + pool_command.export(mock.MagicMock(file=export_file, output="json")) 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 3173aa4d353fd..6d5bf39e7ab61 100644 --- a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py +++ b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py @@ -22,7 +22,15 @@ import pytest -from airflowctl.ctl.cli_config import ActionCommand, CommandFactory, GroupCommand, merge_commands +from airflowctl.ctl.cli_config import ( + ARG_AUTH_TOKEN, + ActionCommand, + Arg, + CommandFactory, + GroupCommand, + add_auth_token_to_all_commands, + merge_commands, +) @pytest.fixture @@ -356,6 +364,68 @@ def test_merge_commands(self, no_op_method): assert "subcommand3" in sub_command_names assert "subcommand4" in sub_command_names + def test_add_auth_token_to_all_commands(self, no_op_method): + """Test the add_auth_token_to_all_commands method.""" + ARG_1 = Arg( + flags=("--arg1",), + ) + ARG_2 = Arg( + flags=("--arg1",), + ) + action_commands_1 = ( + ActionCommand( + name="subcommand1", + help="This is subcommand 1", + func=no_op_method, + args=(), + ), + ActionCommand( + name="subcommand2", + help="This is subcommand 2", + func=no_op_method, + args=(ARG_1,), + ), + ) + command_list = [ + GroupCommand( + name="command1", + help="This is command 1 new help", + description="This is command 1 new description", + subcommands=action_commands_1, + ), + ActionCommand( + name="command2", + help="This is command 2", + func=no_op_method, + args=(ARG_1, ARG_2), + ), + ] + + command_list = add_auth_token_to_all_commands(command_list) + + merged_command_names = [command.name for command in command_list] + assert "command1" in merged_command_names + assert "command2" in merged_command_names + assert len(command_list) == 2 + + expected_subcommand_1_args = [ARG_AUTH_TOKEN] + expected_subcommand_2_args = [ARG_1, ARG_AUTH_TOKEN] + expected_command_2_args = [ARG_1, ARG_2, ARG_AUTH_TOKEN] + + for command in command_list: + if command.name == "command1": + sub_command_names = [sc.name for sc in list(command.subcommands)] + assert "subcommand1" in sub_command_names + assert "subcommand2" in sub_command_names + assert len(sub_command_names) == 2 + for sub_command in command.subcommands: + if sub_command.name == "subcommand1": + assert sub_command.args == expected_subcommand_1_args + if sub_command.name == "subcommand2": + assert sub_command.args == expected_subcommand_2_args + if command.name == "command2": + assert command.args == expected_command_2_args + def test_trigger_dag_run_defaults_logical_date_to_now(self): """Test that trigger command defaults logical_date to now when not provided.""" from datetime import datetime, timezone