diff --git a/airflow-core/newsfragments/67060.bugfix.rst b/airflow-core/newsfragments/67060.bugfix.rst new file mode 100644 index 0000000000000..819998a53e624 --- /dev/null +++ b/airflow-core/newsfragments/67060.bugfix.rst @@ -0,0 +1 @@ +Fix variable import handling for structured falsy values. diff --git a/airflow-core/src/airflow/cli/commands/variable_command.py b/airflow-core/src/airflow/cli/commands/variable_command.py index 85166cb6e79d3..ba1cf4c6ee410 100644 --- a/airflow-core/src/airflow/cli/commands/variable_command.py +++ b/airflow-core/src/airflow/cli/commands/variable_command.py @@ -151,7 +151,7 @@ def variables_import(args, session): try: value = v description = None - if isinstance(v, dict) and v.get("value"): # verify that var configuration has value + if isinstance(v, dict) and "value" in v: # verify that var configuration has value value, description = v["value"], v.get("description") Variable.set(k, value, description, serialize_json=not isinstance(value, str)) except Exception as e: diff --git a/airflow-core/tests/unit/cli/commands/test_variable_command.py b/airflow-core/tests/unit/cli/commands/test_variable_command.py index 21d2fb66822b5..c7890d9c1a960 100644 --- a/airflow-core/tests/unit/cli/commands/test_variable_command.py +++ b/airflow-core/tests/unit/cli/commands/test_variable_command.py @@ -474,6 +474,37 @@ def test_variables_import_with_descriptions( ) assert session.scalar(select(Variable.description).where(Variable.key == "var3")) is None + @pytest.mark.parametrize( + ("value", "deserialize_json"), + [ + ("", False), + (0, True), + (False, True), + (None, True), + ], + ids=["empty_string", "zero", "false", "null"], + ) + def test_variables_import_with_structured_falsy_values( + self, create_variable_file, value, deserialize_json + ): + """Test variables_import preserves structured falsy values and descriptions.""" + file = create_variable_file( + {"falsy_key": {"value": value, "description": "Falsy value description"}}, + format="json", + ) + + with create_session() as session: + variable_command.variables_import( + self.parser.parse_args(["variables", "import", os.fspath(file)]), session=session + ) + + assert Variable.get("falsy_key", deserialize_json=deserialize_json) == value + with create_session() as session: + assert ( + session.scalar(select(Variable.description).where(Variable.key == "falsy_key")) + == "Falsy value description" + ) + def test_variables_import_env(self, create_variable_file, env_variable_data): """Test variables_import with ENV format""" env_file = create_variable_file(env_variable_data, format="env") diff --git a/airflow-ctl/src/airflowctl/ctl/commands/variable_command.py b/airflow-ctl/src/airflowctl/ctl/commands/variable_command.py index 88bf33a0f0197..09f3e1f949ffb 100644 --- a/airflow-ctl/src/airflowctl/ctl/commands/variable_command.py +++ b/airflow-ctl/src/airflowctl/ctl/commands/variable_command.py @@ -47,6 +47,10 @@ def import_(args, api_client=NEW_API_CLIENT) -> list[str]: rich.print(f"[red]Invalid variable file: {args.file}") sys.exit(1) + if not isinstance(var_json, dict): + rich.print(f"[red]Invalid variable file: {args.file}") + sys.exit(1) + action_on_existence = BulkActionOnExistence(args.action_on_existing_key) vars_to_update = [] for k, v in var_json.items(): diff --git a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_variable_command.py b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_variable_command.py index a0598d03459f8..f573585935fd4 100644 --- a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_variable_command.py +++ b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_variable_command.py @@ -17,6 +17,7 @@ from __future__ import annotations import json +from types import SimpleNamespace import pytest @@ -89,17 +90,20 @@ def test_import_success(self, api_client_maker, tmp_path, monkeypatch): "", 0, False, + None, ], - ids=["empty_string", "zero", "false"], + ids=["empty_string", "zero", "false", "null"], ) - def test_import_falsy_values(self, api_client_maker, tmp_path, monkeypatch, falsy_value): + def test_import_falsy_values(self, tmp_path, monkeypatch, falsy_value): """Test that falsy values (empty string, 0, False) are correctly imported.""" - api_client = api_client_maker( - path="/api/v2/variables", - response_json=self.bulk_response_success.model_dump(), - expected_http_status_code=200, - kind=ClientKind.CLI, - ) + captured_variables = None + + def bulk(variables): + nonlocal captured_variables + captured_variables = variables + return self.bulk_response_success + + api_client = SimpleNamespace(variables=SimpleNamespace(bulk=bulk)) monkeypatch.chdir(tmp_path) expected_json_path = tmp_path / self.export_file_name @@ -113,6 +117,24 @@ def test_import_falsy_values(self, api_client_maker, tmp_path, monkeypatch, fals api_client=api_client, ) assert response == [self.key] + entity = captured_variables.actions[0].entities[0] + assert entity.value.root == falsy_value + assert entity.description == "test falsy value" + + def test_import_rejects_non_object_json(self, tmp_path, monkeypatch, capsys): + monkeypatch.chdir(tmp_path) + expected_json_path = tmp_path / self.export_file_name + expected_json_path.write_text(json.dumps([self.key])) + + with pytest.raises(SystemExit) as exit_info: + variable_command.import_( + self.parser.parse_args(["variables", "import", expected_json_path.as_posix()]), + ) + + assert exit_info.value.code == 1 + output = capsys.readouterr().out + assert "Invalid variable file:" in output + assert expected_json_path.as_posix() in output def test_import_error(self, api_client_maker, tmp_path, monkeypatch): api_client = api_client_maker(