Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the API-server return HTTP status 404 on non-existing endpoints … #10791

Merged
merged 1 commit into from
Feb 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
fniessink marked this conversation as resolved.
Show resolved Hide resolved
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"]
fniessink marked this conversation as resolved.
Show resolved Hide resolved

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