jvalue / ods

Open Data Service - Make consuming open data easy, safe, and reliable
GNU Affero General Public License v3.0
36 stars 23 forks source link

Routing issue on /api/storage and /api/storage-mq #280

Closed sonallux closed 3 years ago

sonallux commented 3 years ago

Issue description

Traefik is routing the requests based on a PathPrefix. The path prefix for the storage service is /api/storage and for the StorageMQ service /api/storage-mq. The issue is that every request to the StorageMQ service starts with /api/storage-mq and obviously does match the route of the StorageMQ and the Storage service. Because Traefik prioritises the routes based on descending order using rules length the requests are routed to the correct service in normal operation. But if the StorageMQ service has crashed, all further requests to the StorageMQ service are actually routed to the Storage service. This does not cause issues currently, as the Storage service can not find a database table with name -mq (The prefix /api/storage is removed from the path by Treafik) and will return a 404 error, which is equivalent to what Traefik is returning if there is no matching rule.

Possible Solutions

  1. Change routing rule for the Storage service to Path(`/api/storage`) || PathPrefix(`/api/storage/`). This resolves the issue because /api/storage-mq is no longer matched.
  2. Rename one of the two routes, so that the Storage route is not a prefix of the StorageMQ route. I can not think of a good route name right now.

I prefer option 1 as it is a very simple change.

But the Kubernetes PR #218 might has the same issue @georg-schwarz and @nxmyoz. If the routing rule can not be adjusted like in option 1 for Kubernetes, we should go with option 2 and rename the routes. This will ensure consistency between the docker-compose and Kubernetes environment.

Edit: I am going to provide a PR with option 1, so we can get the issue fixed soon. If we later come to the conclusion that option 2 is better, we can do another PR.


To not run into this issue again in the future, we should never add new routes that are a prefix of another route.

georg-schwarz commented 3 years ago

Closed by #281