search

osquery / osquery-go

Go bindings for osquery
MIT License
388 stars 79 forks source link

Add context and lock functionality to client interface #108

Closed directionless closed 1 year ago

directionless commented 1 year ago

As discussed in #98, the osquery thrift socket is can only handle a single caller.

In #99, I tried adding a mutex. But, with a mutex, it didn't feel like there was a good timeout behavior. Looking round, I considered a rate.Limiter, but I find those very hard to understand. And eventually I found this suggestion for using single sized channels as a form of mutex.

This should allow for exclusive locking, timeouts, and ctx cancelation.

I have also stopped exporting the raw osquery thrift client. Using it would bypass these locks, and feels like an antipattern. However, it does make this a breaking change.

I also bumped the go version because CI was complaining

Fixes: #98 Closes: #99

directionless commented 1 year ago

@aleksmaus @RebeccaMahany I know you've both poked around here. How's this approach feel?

directionless commented 1 year ago

~I found a corner that doesn't seem great. I'm downgrading this to draft while I tinker.~

Update: Fixed. Calling Unlock when unlocked, now panics. As it does with mutex.

directionless commented 1 year ago

@zwass nudge nudge?

directionless commented 1 year ago

@zwass @lucasmrod I'd love a thumb from y'all. Mostly because I don't want to inadvertently break what you're doing. I think there's enough support for this, so I'm going to end up merging it. But I do want to give y'all time to object / approve / etc

directionless commented 1 year ago

Hearing no objections and tentative approval, I'm going to merge this.

zwass commented 1 year ago

Yes that makes sense. I don't think this will cause any issues for us. Thanks!