kill: add tests + fix run owner logic

Signed-off-by: Vallari Agrawal <[email protected]>

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"])

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