search

motiv-labs / janus

An API Gateway written in Go
https://hellofresh.gitbooks.io/janus
MIT License
2.78k stars 318 forks source link

[BUG] : Hot Reloading removes API end-point adding using Admin API POST operation #390

Open swanandt opened 5 years ago

swanandt commented 5 years ago

[Short description of problem here] Hi We are using Janus Server and using File-system as storage. One issue we identified with hot -reload configuration is "New API end-points added using POST and passing the file are lost when some-one changes the existing API end-point which Janus server has loaded as part of initial start-up.

=============================================== We checked the code and seems during hot-reload when comparison is made and if there are any changes in new config that is built as part of repository and APIs added in between using POST and admin API port are lost...... " This is very BIG DISADVANTAGE as what modifications are added as part of POST are LOST "

Reproduction Steps:

  1. Start a server with any simple API defs in /etc/janus/apis/example.json
  2. Check the API end-point using GET or even during start up visible in Log or using admin token GET operation.
  3. Now using admin token add one more API end-point to Repo using POST operation.
  4. Now change something in example.json , upon saving the Reload Config will happen and this new API added using POST to Repo is lost.

Expected behavior: Ideally the new API added using POST command shall be retained. The logic in building the new Repo during HOT-Reload feature is "if the new config is different , it reloads all of it " in that case it lose out on API end-point added by POST.

Thought on how this can be FIX Approach 1: During the HOT-Reload it shall ignore APIs added during POST maybe adding some identifier to it. Approach 2: Perform initial exercise of adding respective files to /etc/janus/apis/

There are few more but I need to formulate well and check more code [ BEST possible suggestion ] but you know better so you can address it.

We are also thinking now only write event is being watched but also other events like CREATE can be added and it can then be added to watch list.

Thanks n Regards Swan.

Janus version: [3.8.7] OS and version: [Linux version 4.13.9-300.fc27.x86_64 ]

swanandt commented 5 years ago

Hi Any Update

We just check that for File System which is loaded as part of initial configuration and changing just file removes the other end-points [ which is loaded as part of initial start-up ]

Ideally it shall only append what is change to current or existing configuration.

The Change NOTE clearly mention:

3.5.0

78 79 ## Added 80 81 - Check GitHub permissions. Sets is_admin into the jwt token when the chosen provider is Github 82 - Jaeger support as distributed tracing backend 83 - Added Proxy Listen Path validation to prevent chi from panicking in case of invalid listen path 84 - Added load balancing for upstream targets. Now you can add multiple upstream targets and Janus will balance the requests. 85 - Added support for url parameters both in listen path and upstreams. 86 87 ## Fixed 88 89 - Monitor health check endpoints only of active proxies. Reported on #203 90 - Fix hot reload was not working when using in memory storage implementation 91 - Fix oauth servers post endpoint incorrect behaviour. Reported on #234 92 - Add constant time compare to basic auth password. Reported on #194

But somehow this seems to be broken....

Any Pointers how to fix it and also if you can share how code flows then we can check more as well.

Thanks Swanand