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

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

Open
swanandt opened this issue Feb 12, 2019 · 1 comment

Comments

@swanandt
Copy link

[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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant