From 67658a67c5d72fb8eb916451f33fbdb5f597c549 Mon Sep 17 00:00:00 2001 From: pierrejeambrun Date: Tue, 20 May 2025 17:38:58 +0200 Subject: [PATCH] API handle slashes in variable keys --- .../core_api/routes/public/variables.py | 6 +- .../core_api/routes/public/test_variables.py | 105 +++++++++++++++--- 2 files changed, 94 insertions(+), 17 deletions(-) diff --git a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/variables.py b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/variables.py index e55a6f540a725..36ee71c17ecd5 100644 --- a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/variables.py +++ b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/variables.py @@ -47,7 +47,7 @@ @variables_router.delete( - "/{variable_key}", + "/{variable_key:path}", status_code=status.HTTP_204_NO_CONTENT, responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]), dependencies=[Depends(action_logging()), Depends(requires_access_variable("DELETE"))], @@ -64,7 +64,7 @@ def delete_variable( @variables_router.get( - "/{variable_key}", + "/{variable_key:path}", responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]), dependencies=[Depends(requires_access_variable("GET"))], ) @@ -121,7 +121,7 @@ def get_variables( @variables_router.patch( - "/{variable_key}", + "/{variable_key:path}", responses=create_openapi_http_exception_doc( [ status.HTTP_400_BAD_REQUEST, diff --git a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_variables.py b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_variables.py index fd235b9758edf..ee40c785673b3 100644 --- a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_variables.py +++ b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_variables.py @@ -44,6 +44,10 @@ TEST_VARIABLE_VALUE3 = '{"password": "some_password"}' TEST_VARIABLE_DESCRIPTION3 = "Some description for the variable" +TEST_VARIABLE_KEY4 = "test_variable_key/with_slashes" +TEST_VARIABLE_VALUE4 = "test_variable_value" +TEST_VARIABLE_DESCRIPTION4 = "Some description for the variable" + TEST_VARIABLE_SEARCH_KEY = "test_variable_search_key" TEST_VARIABLE_SEARCH_VALUE = "random search value" @@ -78,6 +82,13 @@ def _create_variables(session) -> None: session=session, ) + Variable.set( + key=TEST_VARIABLE_KEY4, + value=TEST_VARIABLE_VALUE4, + description=TEST_VARIABLE_DESCRIPTION4, + session=session, + ) + Variable.set( key=TEST_VARIABLE_SEARCH_KEY, value=TEST_VARIABLE_SEARCH_VALUE, @@ -102,9 +113,11 @@ class TestDeleteVariable(TestVariableEndpoint): def test_delete_should_respond_204(self, test_client, session): self.create_variables() variables = session.query(Variable).all() - assert len(variables) == 4 + assert len(variables) == 5 response = test_client.delete(f"/variables/{TEST_VARIABLE_KEY}") assert response.status_code == 204 + response = test_client.delete(f"/variables/{TEST_VARIABLE_KEY4}") + assert response.status_code == 204 variables = session.query(Variable).all() assert len(variables) == 3 check_last_log(session, dag_id=None, event="delete_variable", logical_date=None) @@ -156,6 +169,15 @@ class TestGetVariable(TestVariableEndpoint): "is_encrypted": True, }, ), + ( + TEST_VARIABLE_KEY4, + { + "key": TEST_VARIABLE_KEY4, + "value": TEST_VARIABLE_VALUE4, + "description": TEST_VARIABLE_DESCRIPTION4, + "is_encrypted": True, + }, + ), ( TEST_VARIABLE_SEARCH_KEY, { @@ -194,35 +216,75 @@ class TestGetVariables(TestVariableEndpoint): "query_params, expected_total_entries, expected_keys", [ # Filters - ({}, 4, [TEST_VARIABLE_KEY, TEST_VARIABLE_KEY2, TEST_VARIABLE_KEY3, TEST_VARIABLE_SEARCH_KEY]), - ({"limit": 1}, 4, [TEST_VARIABLE_KEY]), - ({"limit": 1, "offset": 1}, 4, [TEST_VARIABLE_KEY2]), + ( + {}, + 5, + [ + TEST_VARIABLE_KEY, + TEST_VARIABLE_KEY2, + TEST_VARIABLE_KEY3, + TEST_VARIABLE_KEY4, + TEST_VARIABLE_SEARCH_KEY, + ], + ), + ({"limit": 1}, 5, [TEST_VARIABLE_KEY]), + ({"limit": 1, "offset": 1}, 5, [TEST_VARIABLE_KEY2]), # Sort ( {"order_by": "id"}, - 4, - [TEST_VARIABLE_KEY, TEST_VARIABLE_KEY2, TEST_VARIABLE_KEY3, TEST_VARIABLE_SEARCH_KEY], + 5, + [ + TEST_VARIABLE_KEY, + TEST_VARIABLE_KEY2, + TEST_VARIABLE_KEY3, + TEST_VARIABLE_KEY4, + TEST_VARIABLE_SEARCH_KEY, + ], ), ( {"order_by": "-id"}, - 4, - [TEST_VARIABLE_SEARCH_KEY, TEST_VARIABLE_KEY3, TEST_VARIABLE_KEY2, TEST_VARIABLE_KEY], + 5, + [ + TEST_VARIABLE_SEARCH_KEY, + TEST_VARIABLE_KEY4, + TEST_VARIABLE_KEY3, + TEST_VARIABLE_KEY2, + TEST_VARIABLE_KEY, + ], ), ( {"order_by": "key"}, - 4, - [TEST_VARIABLE_KEY3, TEST_VARIABLE_KEY2, TEST_VARIABLE_KEY, TEST_VARIABLE_SEARCH_KEY], + 5, + [ + TEST_VARIABLE_KEY3, + TEST_VARIABLE_KEY2, + TEST_VARIABLE_KEY, + TEST_VARIABLE_KEY4, + TEST_VARIABLE_SEARCH_KEY, + ], ), ( {"order_by": "-key"}, - 4, - [TEST_VARIABLE_SEARCH_KEY, TEST_VARIABLE_KEY, TEST_VARIABLE_KEY2, TEST_VARIABLE_KEY3], + 5, + [ + TEST_VARIABLE_SEARCH_KEY, + TEST_VARIABLE_KEY4, + TEST_VARIABLE_KEY, + TEST_VARIABLE_KEY2, + TEST_VARIABLE_KEY3, + ], ), # Search ( {"variable_key_pattern": "~"}, - 4, - [TEST_VARIABLE_KEY, TEST_VARIABLE_KEY2, TEST_VARIABLE_KEY3, TEST_VARIABLE_SEARCH_KEY], + 5, + [ + TEST_VARIABLE_KEY, + TEST_VARIABLE_KEY2, + TEST_VARIABLE_KEY3, + TEST_VARIABLE_KEY4, + TEST_VARIABLE_SEARCH_KEY, + ], ), ({"variable_key_pattern": "search"}, 1, [TEST_VARIABLE_SEARCH_KEY]), ], @@ -282,6 +344,21 @@ class TestPatchVariable(TestVariableEndpoint): "is_encrypted": True, }, ), + ( + TEST_VARIABLE_KEY4, + { + "key": TEST_VARIABLE_KEY4, + "value": "The new value", + "description": "The new description", + }, + {"update_mask": ["value"]}, + { + "key": TEST_VARIABLE_KEY4, + "value": "The new value", + "description": TEST_VARIABLE_DESCRIPTION4, + "is_encrypted": True, + }, + ), ( TEST_VARIABLE_KEY2, {