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

Améliorations techniques à considérer pour le proxy SIRI avant ouverture au public #2476

Closed
1 of 15 tasks
thbar opened this issue Jun 27, 2022 · 2 comments
Closed
1 of 15 tasks
Labels

Comments

@thbar
Copy link
Contributor

thbar commented Jun 27, 2022

Durant mon travail sur #2459, j'avais mis plusieurs TODOs pour penser aux points à traiter avant l'ouverture réelle au public.

Je déplace ces TODOs ici, pour pouvoir clôturer la PR (Credo etc), sans les oublier pour autant.

  • ⭐ Choisir un RequestorRef public qui soit clair (on valide transport-data-gouv-fr avec @ChristinaLaumond et @Miryad3108 pour l'instant)
  • ⏸️ Retourner quelque chose d'aidant sur le 403 "Forbidden" (lien vers le requêteur SIRI, doc etc ?)
  • ⭐ (priorisé en version simple) Tracer les requêtes dans la télémétrie (externe et interne) de façon à pouvoir produire des métriques. Je ne l'ai pas activé pour l'instant car les identifiants contiennent une clé secrète pour protéger l'accès durant la beta privée.
  • ⭐ Mettre en place un rate-limit (exemple: pas plus d'une requête par seconde)
  • ⭐ Autoriser les requêtes permises (exemple: les abonnements seront interdits car ils ne pourront pas être bien traités tout le temps, ou en tout cas nécessiteront d'étudier le fonctionnement de plus près) Filtrage simple des requêtes SIRI #2728. Vu avec @Miryad3108 on va chercher à contacter des réutilisateurs potentiels pour avoir leur avis sur la priorisation et l'intérêt de chaque requête, et sous quelles modalités elles seront intéressantes
    • LinesDiscovery (qui n'existe pas partout mais est très utile pour ensuite avec les "line refs"
    • StopPointsDiscovery (idem)
    • GetEstimatedTimetable (avec refs, ou sans refs ; certains serveurs répondent la totalité sans ref, d'autres ne retournent rien)
    • GetStopMonitoring (il existe des versions batchées, et non batchées, et il faut faire attention à la charge générée par les serveurs, au nombre d'arrêts demandés etc)
    • CheckStatus
    • GetGeneralMessage (perturbations etc)
  • ⭐ Analyser la payload de retour: elle peut parfois apparemment contenir le "requestor_ref". Il faudrait voir si c'est un usage répandu, et le cas échéant, décider éventuellement de modifier cette information (voir Is streaming encoding possible with Saxy? qcam/saxy#109 pour la discussion technique associée à une modification streamée de XML)
  • ⭐ Protéger du memory overload: selon la taille des réponses XML, et les traitements réalisés au final, il pourrait être sain de capper la mémoire utilisée par la requête. Cela peut se faire avec Process.flag(:max_heap_size, heap_size()) (voir ici), qui pourrait peut-être suffire pour agir comme fusible (à vérifier, selon le fonctionnement de Saxy)
  • Envisager de re-compresser la réponse avant envoi au client (selon Accept-Encoding)
  • Envisager de mettre en cache des requêtes fréquentes (avec leurs paramètres)
@thbar thbar added the SIRI label Jun 27, 2022
thbar added a commit that referenced this issue Jun 27, 2022
thbar added a commit that referenced this issue Jul 21, 2022
* Prepare todos for next round

* Add TODO

* Add simple POST mock server

* Add POST route to support SIRI

* Parse body as XML

* Extract testing code before reuse

* Format code

* Save live replacement of requestor ref via DOM model

* Make line_refs optional

Without line refs, everything is going to be returned (as I learned).

* Rebuild body from unmodified request for now

This is useful to print it, and I will work on requestor_ref replacement right after.

* Add TODO

* Replace GET test by POST for SIRI

* Move code to lib

* Add reproduction & fix for crash seen during my real-life testing

* Remove left-over

* Remove left-over

* Replace requestor_ref in query

* Adapt code

* Fix test

* Remove TODO

* Add support for POST in Finch wrapper

* Make a first query

* Add moduledoc

* Add relevant bits

* Fix more credo warnings

* Add helpful logger

* Improve SIRI CLI usage for myself

* Add helper to retrieve specific HTTP header

* Implement gzip decompression

We'll need to remove sensitive information (e.g. requestor_ref in particular), so this step is useful.

* Fix typo

* Add tooling to run batch of queries on all resources

* Support HTTP headers for SIRI

This is helpful to avoid HTTP 415 return (etalab/transport-private#5 (comment)) on CARENE.

* Remove completed & irrelevant todos

* Replace TODOs by #2476

* Add a comment pointing to #2476 so that things do not get lost

* Remove TODO

Removing this call may be required later when handling #1817

* Bubble up Mox/ExUnit exceptions

Without this, it is hard to write tests.

* Implement first fully POST SIRI test

* Return HTTP 405 for inadequate calls

POST on a GTFS-RT item and GET on a SIRI item are now forbidden for clarity.

I have added tests for this.

* Add todo

* Remove now unused code

* Fix incorrect tests & add notes for context

* Refactor method to extract seen requestor refs

This is going to be useful to verify the input payload from the controller.

* Update doc

* Start adapting code for requestor ref verification

* Return 403 Forbidden on incorrect incoming requestor_ref

* Move public requestor ref to config

* Delete mock_server.exs

* Try to fix Dialyzer issue

* Apply fix for Dialyzer

See qcam/saxy#111

* Revert "Try to fix Dialyzer issue"

This reverts commit b27a1fe.

* Add test about namespace & improve doc

* Use shorter form

Thanks @fchabouis for the idea

* Improve naming

* Add typespec

* Extract function for clarity & add tests

* Fix credo error

* Add a bit of typing for clarity

* Add spec to ensure we cover HEAD automatically for GTFS-RT

* Move code to single-line

* Extract method to improve understanding of code

* Always include XML version (1.0)

While the prolog is optional and 1.0 is the default, given the differences https://www.w3.org/TR/2006/REC-xml11-20060816/#sec-xml11 and the legacy servers we may target, we have decided to be explicit.

Co-authored-by: Francis Chabouis <fchabouis@gmail.com>

* Mix format

* Make prolog management more explicit

* Make test a bit more clear with variable on headers

* Add SIRI headers tests & improve GTFS-RT headers tests

Co-authored-by: Antoine Augusti <antoine.augusti@beta.gouv.fr>
Co-authored-by: Francis Chabouis <fchabouis@gmail.com>
@thbar
Copy link
Contributor Author

thbar commented Aug 1, 2022

Mis à jour et priorisé avec des étoiles aujourd'hui avec @Miryad3108 que je remercie !

@thbar
Copy link
Contributor Author

thbar commented May 2, 2023

Au final aujourd'hui on encourage plutôt les acteurs à ouvrir directement, ce que certains commencent à faire. Je clôture.

@thbar thbar closed this as completed May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant