Skip to content

Commit

Permalink
Make the API-server return HTTP status 404 on non-existing endpoints …
Browse files Browse the repository at this point in the history
…instead of 200.

Fixes #9860.
  • Loading branch information
fniessink committed Feb 7, 2025
1 parent 9eb0edc commit 23b475b
Show file tree
Hide file tree
Showing 19 changed files with 222 additions and 152 deletions.
4 changes: 2 additions & 2 deletions components/api_server/src/healthcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
api_server_port = getenv("API_SERVER_PORT", "5001")
exit_code = 1
try:
with urlopen(f"http://localhost:{api_server_port}/api/health") as response: # nosec B310
exit_code = 0 if response.status == HTTPStatus.OK else 1
with urlopen(f"http://localhost:{api_server_port}/api/internal/health") as response: # nosec B310
exit_code = 0 if response.status == HTTPStatus.OK and response.read() == b'{"healthy": true}' else 1
finally:
sys.exit(exit_code)
3 changes: 2 additions & 1 deletion components/api_server/src/routes/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
get_report_changelog,
)
from .datamodel import get_data_model
from .documentation import get_api
from .documentation import get_api_docs
from .health import get_health
from .logo import get_logo
from .measurement import get_metric_measurements, get_measurements, set_entity_attribute, stream_nr_measurements
from .metric import (
Expand Down
11 changes: 4 additions & 7 deletions components/api_server/src/routes/documentation.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,15 @@
import bottle


@bottle.get("/api", authentication_required=False)
@bottle.get("/api/<fragment>", authentication_required=False)
@bottle.get("/api/<version>/<fragment>", authentication_required=False)
def get_api(version="", fragment=""):
"""Return the API. Use version and/or fragment to limit the routes returned."""
@bottle.get("/api/v3/docs", authentication_required=False)
def get_api_docs() -> dict[str, dict[str, str]]:
"""Return the API docs."""
port = os.environ.get("API_SERVER_PORT", "5001")
routes = [route for route in bottle.default_app().routes if version in route.rule and fragment in route.rule]
return {
route.rule: {
"url": f"https://www.quality-time.example.org:{port}{route.rule}",
"method": route.method,
"description": route.callback.__doc__,
}
for route in routes
for route in bottle.default_app().routes
}
9 changes: 9 additions & 0 deletions components/api_server/src/routes/health.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
"""Health end-point."""

import bottle


@bottle.get("/api/internal/health", authentication_required=False)
def get_health():
"""Return the API-server health."""
return {"healthy": True} # For now, server being up means it is healthy
19 changes: 2 additions & 17 deletions components/api_server/tests/routes/test_documentation.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import unittest

from routes import get_api
from routes import get_api_docs

EXTERNAL_API_VERSION = "v3"

Expand All @@ -16,22 +16,7 @@ class DocumentationTest(unittest.TestCase):

def test_get_api(self):
"""Test that the API can be retrieved."""
api_json = get_api()
self.assertIn("/api", api_json)
api_json = get_api_docs()
self.assertIn(self.login_internal, api_json)
self.assertIn(self.login_external, api_json)
self.assertIn(self.server_external, api_json)

def test_get_api_by_version(self):
"""Test that the API can be filtered by version."""
api_json = get_api(EXTERNAL_API_VERSION)
self.assertNotIn(self.login_internal, api_json)
self.assertIn(self.login_external, api_json)
self.assertIn(self.server_external, api_json)

def test_get_api_by_fragment(self):
"""Test that the API can be filtered by version and a fragment."""
api_json = get_api(EXTERNAL_API_VERSION, "login")
self.assertNotIn(self.login_internal, api_json)
self.assertIn(self.login_external, api_json)
self.assertNotIn(self.server_external, api_json)
13 changes: 13 additions & 0 deletions components/api_server/tests/routes/test_health.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
"""Unit tests for the health route."""

import unittest

from routes import get_health


class HealthTest(unittest.TestCase):
"""Unit tests for the health route."""

def test_health(self):
"""Test that the health status can be retrieved."""
self.assertEqual({"healthy": True}, get_health())
37 changes: 31 additions & 6 deletions components/api_server/tests/test_healthcheck.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,46 @@
"""Unit tests for the API-server healthcheck script."""

import sys
import unittest
from http import HTTPStatus
from unittest.mock import patch, MagicMock


@patch("sys.exit")
@patch("urllib.request.urlopen")
class APIServerHealthcheckTestCase(unittest.TestCase):
"""Unit tests for the API-server healthcheck."""

@patch("sys.exit")
@patch("urllib.request.urlopen")
def test_healthy(self, mock_urlopen, mock_exit):
"""Test that the server is healthy."""
def create_response(self, healthy: bool = True, status=HTTPStatus.OK) -> MagicMock:
"""Create a API-server response."""
response = MagicMock()
response.status = HTTPStatus.OK
response.status = status
response.read.return_value = b'{"healthy": true}' if healthy else b'{"healthy": false}'
response.__enter__.return_value = response
mock_urlopen.return_value = response
return response

def run_haelthcheck(self) -> None:
"""Run the healthcheck."""
import healthcheck # noqa: F401

def tearDown(self) -> None:
"""Remove the healthcheck module."""
del sys.modules["healthcheck"]

def test_healthy(self, mock_urlopen, mock_exit):
"""Test that the server is healthy."""
mock_urlopen.return_value = self.create_response()
self.run_haelthcheck()
mock_exit.assert_called_once_with(0)

def test_unhealthy_json(self, mock_urlopen, mock_exit):
"""Test that the server is unhealthy."""
mock_urlopen.return_value = self.create_response(healthy=False)
self.run_haelthcheck()
mock_exit.assert_called_once_with(1)

def test_unhealthy_status(self, mock_urlopen, mock_exit):
"""Test that the server is unhealthy."""
mock_urlopen.return_value = self.create_response(status=HTTPStatus.INTERNAL_SERVER_ERROR)
self.run_haelthcheck()
mock_exit.assert_called_once_with(1)
6 changes: 6 additions & 0 deletions docs/src/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ If your currently installed *Quality-time* version is not the latest version, pl

<!-- The line "## <square-bracket>Unreleased</square-bracket>" is replaced by the release/release.py script with the new release version and release date. -->

## [Unreleased]

### Fixed

- Make the API-server return HTTP status 404 on non-existing endpoints instead of 200. Fixes [#9860](https://github.com/ICTU/quality-time/issues/9860).

## v5.24.0 - 2025-02-06

### Added
Expand Down
6 changes: 6 additions & 0 deletions docs/src/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@ python src/quality_time_server.py

The API of the API-server is served at [http://localhost:5001](http://localhost:5001), e.g. access [http://localhost:5001/api/internal/report](http://localhost:5001/api/internal/report) to get the available reports combined with their recent measurements.

To quickly see all endpoints, use the `/api/v3/docs` endpoint:

```console
curl http://localhost:5001/api/v3/docs | jq .
```

```{note}
If you're new to Python virtual environments, note that:
- Creating a virtual environment (`uv venv`) has to be done once. Only when the Python version changes, you want to recreate the virtual environment.
Expand Down
2 changes: 1 addition & 1 deletion tests/application_tests/src/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ class ApiTest(unittest.TestCase):

def test_documentation(self):
"""Test that the documentation API is available."""
apis = requests.get("http://www:8080/api", timeout=10).json().keys()
apis = requests.get("http://www:8080/api/v3/docs", timeout=10).json().keys()
self.assertIn("/api/internal/login", apis)
self.assertIn("/api/v3/login", apis)
121 changes: 61 additions & 60 deletions tests/feature_tests/.vulture_ignore_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,19 @@
logged_in_client # unused function (src/steps/auth.py:8)
logged_out_client # unused function (src/steps/auth.py:16)
log_in_with_incorrect_credentials # unused function (src/steps/auth.py:23)
check_invalid_credentials # unused function (src/steps/auth.py:29)
check_unauthenticated # unused function (src/steps/auth.py:35)
check_unauthorized # unused function (src/steps/auth.py:41)
get_public_key # unused function (src/steps/auth.py:47)
check_public_key # unused function (src/steps/auth.py:54)
access_non_existing_endpoint # unused function (src/steps/auth.py:29)
check_invalid_credentials # unused function (src/steps/auth.py:35)
check_unauthenticated # unused function (src/steps/auth.py:41)
check_unauthorized # unused function (src/steps/auth.py:47)
check_endpoint_does_not_exist # unused function (src/steps/auth.py:53)
get_public_key # unused function (src/steps/auth.py:59)
check_public_key # unused function (src/steps/auth.py:66)
check_changelog # unused function (src/steps/changelog.py:8)
get_data_model # unused function (src/steps/data_model.py:15)
get_old_data_model # unused function (src/steps/data_model.py:22)
_.report_date # unused attribute (src/steps/data_model.py:25)
check_data_model # unused function (src/steps/data_model.py:29)
check_too_old_data_model # unused function (src/steps/data_model.py:35)
healthy_server # unused function (src/steps/healthcheck.py:8)
get_health # unused function (src/steps/healthcheck.py:13)
check_health # unused function (src/steps/healthcheck.py:19)
add_item # unused function (src/steps/item.py:11)
copy_item # unused function (src/steps/item.py:45)
move_item # unused function (src/steps/item.py:55)
Expand All @@ -34,67 +33,69 @@
measure_issue_status # unused function (src/steps/measurement.py:66)
set_entity_attribute # unused function (src/steps/measurement.py:81)
connect_to_nr_of_measurements_stream # unused function (src/steps/measurement.py:90)
check_nr_of_measurements # unused function (src/steps/measurement.py:104)
_.report_date # unused attribute (src/steps/measurement.py:109)
check_nr_of_measurements_stream # unused function (src/steps/measurement.py:117)
get_reports_overview_measurements # unused function (src/steps/measurement.py:130)
_.min_report_date # unused attribute (src/steps/measurement.py:135)
_.report_date # unused attribute (src/steps/measurement.py:138)
_.report_date # unused attribute (src/steps/measurement.py:140)
check_reports_overview_measurements # unused function (src/steps/measurement.py:144)
assert_issue_status # unused function (src/steps/metric.py:12)
assert_issue_id_suggestions # unused function (src/steps/metric.py:23)
retrieve_issue_tracker_options # unused function (src/steps/metric.py:30)
assert_issue_tracker_options # unused function (src/steps/metric.py:36)
create_new_issue # unused function (src/steps/metric.py:42)
new_issue_response # unused function (src/steps/metric.py:48)
accept_technical_debt # unused function (src/steps/metric.py:56)
do_not_accept_technical_debt # unused function (src/steps/metric.py:62)
measurement_request # unused function (src/steps/metric.py:68)
assert_metric_is_being_measured # unused function (src/steps/metric.py:76)
assert_metric_technical_debt_end_date_is_empty # unused function (src/steps/metric.py:83)
assert_metric_technical_debt_end_date_is_not_empty # unused function (src/steps/metric.py:90)
check_nr_of_measurements # unused function (src/steps/measurement.py:105)
_.report_date # unused attribute (src/steps/measurement.py:110)
check_nr_of_measurements_stream # unused function (src/steps/measurement.py:118)
get_reports_overview_measurements # unused function (src/steps/measurement.py:131)
_.min_report_date # unused attribute (src/steps/measurement.py:136)
_.report_date # unused attribute (src/steps/measurement.py:139)
_.report_date # unused attribute (src/steps/measurement.py:141)
check_reports_overview_measurements # unused function (src/steps/measurement.py:145)
assert_issue_status # unused function (src/steps/metric.py:10)
assert_issue_id_suggestions # unused function (src/steps/metric.py:21)
retrieve_issue_tracker_options # unused function (src/steps/metric.py:28)
assert_issue_tracker_options # unused function (src/steps/metric.py:34)
create_new_issue # unused function (src/steps/metric.py:40)
new_issue_response # unused function (src/steps/metric.py:46)
accept_technical_debt # unused function (src/steps/metric.py:54)
do_not_accept_technical_debt # unused function (src/steps/metric.py:60)
assert_metric_technical_debt_end_date_is_empty # unused function (src/steps/metric.py:66)
assert_metric_technical_debt_end_date_is_not_empty # unused function (src/steps/metric.py:73)
add_notification_destination # unused function (src/steps/notification_destination.py:7)
add_notification_destination_to_non_existing_report # unused function (src/steps/notification_destination.py:16)
delete_notification_destination_of_non_existing_report # unused function (src/steps/notification_destination.py:24)
change_notification_destination_of_non_existing_report # unused function (src/steps/notification_destination.py:31)
get_report_metric_status_summary # unused function (src/steps/report.py:13)
check_report_metric_status_summary # unused function (src/steps/report.py:21)
download_report_as_pdf # unused function (src/steps/report.py:29)
download_report_as_json # unused function (src/steps/report.py:37)
download_report_as_json_with_key # unused function (src/steps/report.py:48)
re_import_report # unused function (src/steps/report.py:56)
import_report # unused function (src/steps/report.py:64)
time_travel_long_ago # unused function (src/steps/report.py:73)
_.report_date # unused attribute (src/steps/report.py:76)
time_travel_future # unused function (src/steps/report.py:79)
_.report_date # unused attribute (src/steps/report.py:82)
reset_report_date # unused function (src/steps/report.py:85)
_.report_date # unused attribute (src/steps/report.py:88)
time_travel # unused function (src/steps/report.py:91)
_.report_date # unused attribute (src/steps/report.py:95)
check_pdf # unused function (src/steps/report.py:99)
check_json # unused function (src/steps/report.py:105)
check_no_json # unused function (src/steps/report.py:113)
get_non_existing_report # unused function (src/steps/report.py:119)
copy_non_existing_report # unused function (src/steps/report.py:126)
delete_non_existing_report # unused function (src/steps/report.py:133)
change_title_of_non_existing_report # unused function (src/steps/report.py:140)
import_failed # unused function (src/steps/report.py:147)
get_measurements # unused function (src/steps/report.py:154)
_.report_date # unused attribute (src/steps/report.py:158)
_.min_report_date # unused attribute (src/steps/report.py:159)
change_non_existing_report_tracker_type # unused function (src/steps/report.py:165)
get_non_existing_report_tracker_options # unused function (src/steps/report.py:172)
get_non_existing_report_tracker_suggestions # unused function (src/steps/report.py:179)
set_technical_debt_desired_response_time # unused function (src/steps/report.py:186)
get_report_metric_status_summary # unused function (src/steps/report.py:15)
check_report_metric_status_summary # unused function (src/steps/report.py:23)
download_report_as_pdf # unused function (src/steps/report.py:31)
download_report_as_json # unused function (src/steps/report.py:39)
download_report_as_json_with_key # unused function (src/steps/report.py:50)
re_import_report # unused function (src/steps/report.py:58)
import_report # unused function (src/steps/report.py:66)
time_travel_long_ago # unused function (src/steps/report.py:75)
_.report_date # unused attribute (src/steps/report.py:78)
time_travel_future # unused function (src/steps/report.py:81)
_.report_date # unused attribute (src/steps/report.py:84)
reset_report_date # unused function (src/steps/report.py:87)
_.report_date # unused attribute (src/steps/report.py:90)
time_travel # unused function (src/steps/report.py:93)
_.report_date # unused attribute (src/steps/report.py:97)
check_pdf # unused function (src/steps/report.py:101)
check_json # unused function (src/steps/report.py:108)
check_no_json # unused function (src/steps/report.py:116)
get_non_existing_report # unused function (src/steps/report.py:122)
copy_non_existing_report # unused function (src/steps/report.py:129)
delete_non_existing_report # unused function (src/steps/report.py:136)
change_title_of_non_existing_report # unused function (src/steps/report.py:143)
import_failed # unused function (src/steps/report.py:150)
get_measurements # unused function (src/steps/report.py:157)
_.report_date # unused attribute (src/steps/report.py:161)
_.min_report_date # unused attribute (src/steps/report.py:162)
change_non_existing_report_tracker_type # unused function (src/steps/report.py:168)
get_non_existing_report_tracker_options # unused function (src/steps/report.py:175)
get_non_existing_report_tracker_suggestions # unused function (src/steps/report.py:182)
set_technical_debt_desired_response_time # unused function (src/steps/report.py:189)
search # unused function (src/steps/search.py:8)
search_invalid # unused function (src/steps/search.py:15)
check_search_results # unused function (src/steps/search.py:22)
check_no_search_results # unused function (src/steps/search.py:28)
check_invalid_search # unused function (src/steps/search.py:36)
get_server_info # unused function (src/steps/server.py:8)
check_http_status_code # unused function (src/steps/server.py:21)
get_server_info # unused function (src/steps/server_info.py:8)
check_http_status_code # unused function (src/steps/server_info.py:21)
healthy_server # unused function (src/steps/server_info.py:27)
get_health # unused function (src/steps/server_info.py:32)
check_health # unused function (src/steps/server_info.py:38)
get_server_docs # unused function (src/steps/server_info.py:44)
post_settings # unused function (src/steps/settings.py:10)
check_post_settings # unused function (src/steps/settings.py:16)
get_settings # unused function (src/steps/settings.py:22)
Expand Down
4 changes: 4 additions & 0 deletions tests/feature_tests/src/features/auth.feature
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ Feature: authentication
When the client tries to log in with incorrect credentials
Then the server tells the client the credentials are incorrect

Scenario: non-existing endpoint
When the client tries to access a non-existing endpoint
Then the server tells the client the endpoint does not exist

Scenario: change editors
When jadoe logs in
And the client changes the reports_overview permissions to "{"edit_reports": ["jadoe", "other_user"], "edit_entities": []}"
Expand Down
6 changes: 0 additions & 6 deletions tests/feature_tests/src/features/healthcheck.feature

This file was deleted.

6 changes: 0 additions & 6 deletions tests/feature_tests/src/features/server.feature

This file was deleted.

15 changes: 15 additions & 0 deletions tests/feature_tests/src/features/server_info.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
Feature: server info
Information about the server can be retrieved

Scenario: get the server information
When the client gets the server information
Then the server information is returned

Scenario: healthcheck API-server
Given a healthy server
When a client checks the server health
Then the server answers

Scenario: API-endpoints documentation
When the client gets the server documentation
Then the server documentation is returned
Loading

0 comments on commit 23b475b

Please sign in to comment.