Skip to content
This repository has been archived by the owner on Jun 6, 2024. It is now read-only.

Corruption on simultanous calls to set config token #725

Closed
BlythMeister opened this issue Jul 18, 2022 · 10 comments
Closed

Corruption on simultanous calls to set config token #725

BlythMeister opened this issue Jul 18, 2022 · 10 comments
Assignees
Labels
bug Something isn't working WIP Working in progress (will be deployed soon)

Comments

@BlythMeister
Copy link
Contributor

Describe the bug
Multiple simultanious calls to set token result in a corrupt yaml file

To Reproduce
Steps to reproduce the behavior:

  1. Run config set token multiple times at the exact same time.
  2. Review the config file output
  3. There is a chance (not garentee) of corruption

Expected behavior
Locking over access to write configuration to allow multi threading to not corrupt the config file

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: Windows

Datree version (run datree version):

  • Version: 1.5.30

Client ID (cat ~/.datree/config.yaml):

  • client_id: [e.g. C5fiX5Tm4SvxwkcfXVSHu5]

Additional context
Add any other context about the problem here.
If possible, include a YAML file to reproduce the bug.

@BlythMeister BlythMeister added the bug Something isn't working label Jul 18, 2022
@amustaque97
Copy link
Contributor

I can probably take a look at this issue if no one is working on it

@myishay
Copy link
Contributor

myishay commented Jul 18, 2022

@amustaque97 go for it 🚀🚀

@adifayer adifayer added the WIP Working in progress (will be deployed soon) label Jul 19, 2022
@amustaque97
Copy link
Contributor

How are you executing simultaneously calls? I’m assuming these are different processes invocations. Example running command every time in separate shell instance. If that is the case I don’t think we can control it. Inter process communication is hard to handle. If it would be some threading then it’s a different case.
cc @BlythMeister @myishay

@BlythMeister
Copy link
Contributor Author

Yes it is.

I'm a c# Dev, in c# I would use mutex to control access to a file system to prevent simultaneous write (or something built into the system IO)

it seems like go does have support for mutex.
I wonder if the set config cli tasks could use a mutex for locking. A mutex is a system wide lock which will mean only 1 instance of the tool can write at any one time.

@amustaque97
Copy link
Contributor

I agree mutex is present in golang std lib. It fails to fix the issue if there are different program execution processes. Mutex is per-process based AFAIK

@BlythMeister
Copy link
Contributor Author

No a mutex is a system lock.

In c# this is done by creating a file on disk and using it to signify the lock in is place.
Other methods can also be used.

But from my research on mutex in go.
These are not process, but system locks.

@BlythMeister
Copy link
Contributor Author

Though the existence of https://github.com/alexflint/go-filemutex library seems to hint otherwise....

But a simple "lock" file approach like git uses when writing config file will suffice really to prevent simultaneous writes.

@amustaque97
Copy link
Contributor

Here is an issue on those lines

Datree is using a package to write config file called ‘viper’ and we cannot modify 3rd party package ‘viper’ even if we follow this

@BlythMeister
Copy link
Contributor Author

I have managed to work around this as my callers to datree is powershell so I can muted lock there instead.

https://github.com/15below/Ensconce/blob/develop/src/Scripts/kubernetesHelper.ps1#L82-L111

@myishay
Copy link
Contributor

myishay commented Jul 24, 2022

@amustaque97 As you mentioned, it seems like there isn't an easy use for mutex in go currently, and since @BlythMeister managed to make a workaround, I will close this issue for now. If you find any solutions for this problem feel free to reopen and comment here. Cheers!

@myishay myishay closed this as completed Jul 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working WIP Working in progress (will be deployed soon)
Projects
None yet
Development

No branches or pull requests

4 participants