From 129b5e98088e240a142be10d012beaa49a6e5df8 Mon Sep 17 00:00:00 2001 From: Vallari Agrawal Date: Fri, 10 May 2024 17:42:40 +0530 Subject: [PATCH] kill: add tests + fix run owner logic Signed-off-by: Vallari Agrawal --- src/teuthology_api/services/kill.py | 2 +- tests/test_kill.py | 53 ++++++++++++++++++++++++++--- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/src/teuthology_api/services/kill.py b/src/teuthology_api/services/kill.py index c72af9e..3c6872d 100644 --- a/src/teuthology_api/services/kill.py +++ b/src/teuthology_api/services/kill.py @@ -33,7 +33,7 @@ async def run(args, send_logs: bool, token: dict, request: Request): log.error("teuthology-kill is missing --run") raise HTTPException(status_code=400, detail="--run is a required argument") - if (run_owner.lower() != username.lower()) or ( + if (run_owner.lower() != username.lower()) and ( run_owner.lower() != f"scheduled_{username.lower()}@teuthology" ): isUserAdmin = await isAdmin(username, token["access_token"]) diff --git a/tests/test_kill.py b/tests/test_kill.py index 7a30953..8e0b059 100644 --- a/tests/test_kill.py +++ b/tests/test_kill.py @@ -10,7 +10,7 @@ async def override_get_token(): - return {"access_token": "token_123", "token_type": "bearer"} + return {"access_token": "token_123", "token_type": "bearer", "isUserAdmin": False} app.dependency_overrides[get_token] = override_get_token @@ -30,16 +30,22 @@ async def override_get_token(): } +@patch("teuthology_api.services.kill.isAdmin") @patch("subprocess.Popen") @patch("teuthology_api.services.kill.get_run_details") @patch("teuthology_api.services.kill.get_username") -def test_kill_run_success(m_get_username, m_get_run_details, m_popen): +def test_kill_run_success(m_get_username, m_get_run_details, m_popen, m_isAdmin): m_get_username.return_value = "user1" - m_get_run_details.return_value = {"id": "7451978", "user": "user1"} + m_isAdmin.return_value = False + m_get_run_details.return_value = { + "id": "7451978", + "user": "user1", + "jobs": [{"owner": "user1"}], + } mock_process = m_popen.return_value mock_process.communicate.return_value = ("logs", "") mock_process.wait.return_value = 0 - response = client.post("/kill", data=json.dumps(mock_kill_args)) + response = client.post("kill/", data=json.dumps(mock_kill_args)) assert response.status_code == 200 assert response.json() == {"kill": "success"} @@ -48,3 +54,42 @@ def test_kill_run_fail(): response = client.post("/kill", data=json.dumps(mock_kill_args)) assert response.status_code == 401 assert response.json() == {"detail": "You need to be logged in"} + + +@patch("teuthology_api.services.kill.isAdmin") +@patch("subprocess.Popen") +@patch("teuthology_api.services.kill.get_run_details") +@patch("teuthology_api.services.kill.get_username") +def test_admin_kill_run_success(m_get_username, m_get_run_details, m_popen, m_isAdmin): + m_get_username.return_value = "user1" + m_isAdmin.return_value = True + m_get_run_details.return_value = { + "id": "7451978", + "user": "user1", + "jobs": [{"owner": "someone_else"}], + } + mock_process = m_popen.return_value + mock_process.communicate.return_value = ("logs", "") + mock_process.wait.return_value = 0 + response = client.post("kill/", data=json.dumps(mock_kill_args)) + assert response.status_code == 200 + assert response.json() == {"kill": "success"} + + +@patch("teuthology_api.services.kill.isAdmin") +@patch("subprocess.Popen") +@patch("teuthology_api.services.kill.get_run_details") +@patch("teuthology_api.services.kill.get_username") +def test_non_admin_kill_run_fail(m_get_username, m_get_run_details, m_popen, m_isAdmin): + m_get_username.return_value = "user1" + m_isAdmin.return_value = False + m_get_run_details.return_value = { + "id": "7451978", + "user": "user1", + "jobs": [{"owner": "someone_else"}], + } + mock_process = m_popen.return_value + mock_process.communicate.return_value = ("logs", "") + mock_process.wait.return_value = 0 + response = client.post("kill/", data=json.dumps(mock_kill_args)) + assert response.status_code == 401 # run doesn't belong to user + user is not admin