From 3d8f8a47ad41b22e657ad4b952900298eedb6dd3 Mon Sep 17 00:00:00 2001 From: Kairo Araujo Date: Tue, 23 Jul 2024 13:28:54 +0200 Subject: [PATCH] fix: status code from OK to NOT_FOUND (#651) This commit fixes the status code from OK (200) to NOT_FOUND (404) when the resource is not available or not in expected state when requested. Fixes: #467 Signed-off-by: Kairo Araujo --- repository_service_tuf_api/artifacts.py | 4 ++-- repository_service_tuf_api/bootstrap.py | 2 +- repository_service_tuf_api/metadata.py | 16 ++++++++-------- tests/unit/api/test_bootstrap.py | 6 +++--- tests/unit/api/test_metadata.py | 22 +++++++++++----------- tests/unit/api/test_targets.py | 8 ++++---- 6 files changed, 29 insertions(+), 29 deletions(-) diff --git a/repository_service_tuf_api/artifacts.py b/repository_service_tuf_api/artifacts.py index 3f139267..2743ad72 100644 --- a/repository_service_tuf_api/artifacts.py +++ b/repository_service_tuf_api/artifacts.py @@ -159,7 +159,7 @@ def post(payload: AddPayload) -> ResponsePostAdd: bs_state = bootstrap_state() if bs_state.bootstrap is False: raise HTTPException( - status.HTTP_200_OK, + status.HTTP_404_NOT_FOUND, detail={ "message": "Task not accepted.", "error": ( @@ -217,7 +217,7 @@ def delete(payload: DeletePayload) -> ResponsePostDelete: bs_state = bootstrap_state() if bs_state.bootstrap is False: raise HTTPException( - status.HTTP_200_OK, + status.HTTP_404_NOT_FOUND, detail={ "message": "Task not accepted.", "error": ( diff --git a/repository_service_tuf_api/bootstrap.py b/repository_service_tuf_api/bootstrap.py index cbd6fba0..87755492 100644 --- a/repository_service_tuf_api/bootstrap.py +++ b/repository_service_tuf_api/bootstrap.py @@ -198,7 +198,7 @@ def post_bootstrap(payload: BootstrapPayload) -> BootstrapPostResponse: # or is in the process of DAS signing ("signing") we consider it as locked. if bs_state.bootstrap is True or bs_state.state in ["pre", "signing"]: raise HTTPException( - status_code=status.HTTP_200_OK, + status_code=status.HTTP_404_NOT_FOUND, detail=BaseErrorResponse( error=( "System already has a Metadata. " diff --git a/repository_service_tuf_api/metadata.py b/repository_service_tuf_api/metadata.py index 09ab0984..466349d6 100644 --- a/repository_service_tuf_api/metadata.py +++ b/repository_service_tuf_api/metadata.py @@ -59,7 +59,7 @@ def post_metadata(payload: MetadataPostPayload) -> MetadataPostResponse: bs_state = bootstrap_state() if bs_state.bootstrap is False: raise HTTPException( - status.HTTP_200_OK, + status.HTTP_404_NOT_FOUND, detail={ "message": "Task not accepted.", "error": ( @@ -119,7 +119,7 @@ def post_metadata_online( bs_state = bootstrap_state() if bs_state.bootstrap is False: raise HTTPException( - status.HTTP_200_OK, + status.HTTP_404_NOT_FOUND, detail={ "message": "Task not accepted.", "error": ( @@ -134,7 +134,7 @@ def post_metadata_online( targets_online = settings_repository.get_fresh("TARGETS_ONLINE_KEY", True) if targets_in and not targets_online: raise HTTPException( - status.HTTP_200_OK, + status.HTTP_404_NOT_FOUND, detail={ "message": "Task not accepted.", "error": ( @@ -152,7 +152,7 @@ def post_metadata_online( # This indicates succinct hash bins are used if len(roles) > 0 and any(not Roles.is_role(role) for role in roles): raise HTTPException( - status.HTTP_200_OK, + status.HTTP_404_NOT_FOUND, detail={ "message": "Task not accepted.", "error": ( @@ -164,7 +164,7 @@ def post_metadata_online( else: if Roles.BINS.value in roles: raise HTTPException( - status.HTTP_200_OK, + status.HTTP_404_NOT_FOUND, detail={ "message": "Task not accepted.", "error": ( @@ -227,7 +227,7 @@ def get_metadata_sign() -> MetadataSignGetResponse: # Adds support only when bootstrap is signing state if bs_state.bootstrap is False and bs_state.state != "signing": raise HTTPException( - status.HTTP_200_OK, + status.HTTP_404_NOT_FOUND, detail={ "message": "No metadata pending signing available", "error": ( @@ -319,7 +319,7 @@ def post_metadata_sign( bs_state = bootstrap_state() if bs_state.bootstrap is False and bs_state.state != "signing": raise HTTPException( - status.HTTP_200_OK, + status.HTTP_404_NOT_FOUND, detail={ "message": "No signing pending.", "error": ( @@ -377,7 +377,7 @@ def delete_metadata_sign(payload: MetadataSignDeletePayload): signing_status = settings_repository.get_fresh(f"{role.upper()}_SIGNING") if signing_status is None: raise HTTPException( - status.HTTP_200_OK, + status.HTTP_404_NOT_FOUND, detail={ "message": f"No signing process for {role}.", "error": f"The {role} role is not in a signing process.", diff --git a/tests/unit/api/test_bootstrap.py b/tests/unit/api/test_bootstrap.py index 24b60f12..dd7f42fc 100644 --- a/tests/unit/api/test_bootstrap.py +++ b/tests/unit/api/test_bootstrap.py @@ -373,7 +373,7 @@ def test_post_bootstrap_already_bootstrap(self, test_client, monkeypatch): payload = json.loads(f_data) response = test_client.post(BOOTSTRAP_URL, json=payload) - assert response.status_code == status.HTTP_200_OK + assert response.status_code == status.HTTP_404_NOT_FOUND assert response.url == f"{test_client.base_url}{BOOTSTRAP_URL}" assert response.json() == { "detail": { @@ -399,7 +399,7 @@ def test_post_bootstrap_already_bootstrap_in_pre( payload = json.loads(f_data) response = test_client.post(BOOTSTRAP_URL, json=payload) - assert response.status_code == status.HTTP_200_OK + assert response.status_code == status.HTTP_404_NOT_FOUND assert response.url == f"{test_client.base_url}{BOOTSTRAP_URL}" assert response.json() == { "detail": {"error": "System already has a Metadata. State: pre"} @@ -423,7 +423,7 @@ def test_post_bootstrap_already_bootstrap_in_signing( payload = json.loads(f_data) response = test_client.post(BOOTSTRAP_URL, json=payload) - assert response.status_code == status.HTTP_200_OK + assert response.status_code == status.HTTP_404_NOT_FOUND assert response.url == f"{test_client.base_url}{BOOTSTRAP_URL}" assert response.json() == { "detail": { diff --git a/tests/unit/api/test_metadata.py b/tests/unit/api/test_metadata.py index 9a9a3522..f787db5f 100644 --- a/tests/unit/api/test_metadata.py +++ b/tests/unit/api/test_metadata.py @@ -69,7 +69,7 @@ def test_post_metadata_without_bootstrap(self, test_client, monkeypatch): payload = json.loads(f_data) response = test_client.post(METADATA_URL, json=payload) - assert response.status_code == status.HTTP_200_OK, response.text + assert response.status_code == status.HTTP_404_NOT_FOUND, response.text assert response.url == f"{test_client.base_url}{METADATA_URL}" assert response.json() == { "detail": { @@ -96,7 +96,7 @@ def test_post_metadata_bootstrap_intermediate_state( payload = json.loads(f_data) response = test_client.post(METADATA_URL, json=payload) - assert response.status_code == status.HTTP_200_OK + assert response.status_code == status.HTTP_404_NOT_FOUND assert response.url == f"{test_client.base_url}{METADATA_URL}" assert response.json() == { "detail": { @@ -317,7 +317,7 @@ def test_post_metadata_online_bootstrap_not_ready( ) response = test_client.post(METADATA_ONLINE_URL, json=payload) - assert response.status_code == status.HTTP_200_OK, response.text + assert response.status_code == status.HTTP_404_NOT_FOUND, response.text assert response.json() == { "detail": { "message": "Task not accepted.", @@ -345,7 +345,7 @@ def test_post_metadata_online_targets_offline_role_cannot_update( payload = {"roles": ["snapshot", "targets"]} response = test_client.post(METADATA_ONLINE_URL, json=payload) - assert response.status_code == status.HTTP_200_OK, response.text + assert response.status_code == status.HTTP_404_NOT_FOUND, response.text err_msg = "Targets is an offline role - use other endpoint to update" assert response.json() == { "detail": { @@ -386,7 +386,7 @@ def fake_get_fresh(attr: str) -> bool: payload = {"roles": ["snapshot", "targets", "abcsdaw"]} response = test_client.post(METADATA_ONLINE_URL, json=payload) - assert response.status_code == status.HTTP_200_OK, response.text + assert response.status_code == status.HTTP_404_NOT_FOUND, response.text roles = common_models.Roles.all_str() err_msg = ( f"Hash bin delegation is used and only {roles} roles can be bumped" @@ -433,7 +433,7 @@ def fake_get_fresh(attr: str) -> bool: payload = {"roles": ["snapshot", "targets", "bins"]} response = test_client.post(METADATA_ONLINE_URL, json=payload) - assert response.status_code == status.HTTP_200_OK, response.text + assert response.status_code == status.HTTP_404_NOT_FOUND, response.text err_msg = "Custom target delegation used and bins cannot be bumped" assert response.json() == { "detail": { @@ -591,7 +591,7 @@ def test_get_metadata_sign_no_bootstrap(self, test_client, monkeypatch): ) response = test_client.get(SIGN_URL) - assert response.status_code == status.HTTP_200_OK, response.text + assert response.status_code == status.HTTP_404_NOT_FOUND, response.text assert response.json() == { "detail": { "message": "No metadata pending signing available", @@ -609,7 +609,7 @@ def test_get_metadata_sign_bootstrap_pre(self, test_client, monkeypatch): ) response = test_client.get(SIGN_URL) - assert response.status_code == status.HTTP_200_OK, response.text + assert response.status_code == status.HTTP_404_NOT_FOUND, response.text assert response.json() == { "detail": { "message": "No metadata pending signing available", @@ -673,7 +673,7 @@ def test_post_metadata_no_bootstrap(self, test_client, monkeypatch): payload = {"role": "root", "signature": {"keyid": "k1", "sig": "s1"}} response = test_client.post(SIGN_URL, json=payload) - assert response.status_code == status.HTTP_200_OK, response.text + assert response.status_code == status.HTTP_404_NOT_FOUND, response.text assert response.json() == { "detail": { "message": "No signing pending.", @@ -692,7 +692,7 @@ def test_post_metadata_bootstrap_finished(self, test_client, monkeypatch): payload = {"role": "root", "signature": {"keyid": "k1", "sig": "s1"}} response = test_client.post(SIGN_URL, json=payload) - assert response.status_code == status.HTTP_200_OK, response.text + assert response.status_code == status.HTTP_404_NOT_FOUND, response.text assert response.json() == { "detail": { "message": "No signing pending.", @@ -766,7 +766,7 @@ def test_metadata_sign_delete_role_not_in_signing_status( payload = {"role": "root"} response = test_client.post(DELETE_SIGN_URL, json=payload) - assert response.status_code == status.HTTP_200_OK, response.text + assert response.status_code == status.HTTP_404_NOT_FOUND, response.text assert response.json() == { "detail": { "message": "No signing process for root.", diff --git a/tests/unit/api/test_targets.py b/tests/unit/api/test_targets.py index 5f8f339a..b0bcf5ca 100644 --- a/tests/unit/api/test_targets.py +++ b/tests/unit/api/test_targets.py @@ -197,7 +197,7 @@ def test_post_without_bootstrap(self, monkeypatch, test_client): payload = json.loads(f_data) response = test_client.post(ARTIFACTS_URL, json=payload) - assert response.status_code == status.HTTP_200_OK + assert response.status_code == status.HTTP_404_NOT_FOUND assert response.json() == { "detail": { "message": "Task not accepted.", @@ -222,7 +222,7 @@ def test_post_with_bootstrap_intermediate_state( payload = json.loads(f_data) response = test_client.post(ARTIFACTS_URL, json=payload) - assert response.status_code == status.HTTP_200_OK + assert response.status_code == status.HTTP_404_NOT_FOUND assert response.json() == { "detail": { "message": "Task not accepted.", @@ -358,7 +358,7 @@ def test_post_without_bootstrap_delete(self, monkeypatch, test_client): ) response = test_client.post(ARTIFACTS_DELETE_URL, json=payload) - assert response.status_code == status.HTTP_200_OK + assert response.status_code == status.HTTP_404_NOT_FOUND assert response.json() == { "detail": { "message": "Task not accepted.", @@ -381,7 +381,7 @@ def test_post_with_bootstrap_intermediate_state_delete( ) response = test_client.post(ARTIFACTS_DELETE_URL, json=payload) - assert response.status_code == status.HTTP_200_OK + assert response.status_code == status.HTTP_404_NOT_FOUND assert response.json() == { "detail": { "message": "Task not accepted.",