Add cache functionality to improve lookup executing times #18
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds cache decorator using Python dictionary. Now each value gathered from bw via the CLI is store in the cache. If the same exact field and key is requested, then initiall the data from the cache is returned instead of executing BW again, which is a lot slower.
For the caching functionality to work properly, the Bitwarden object should live as long as the Lookup module exists, instead of creating a new object, and thus a new cache, for each lookup. Therefore the Bitwarden object is persisted in the initial lookup and reused after that.
The `LookupModule` is initialised during every lookup. This results in a new Bitwarden object for every lookup removing the benefit of the cache. However, importing the bitwarden Lookup module is only done at specific stages during a playbook and not run for every lookup itself. So changing the cache to a global variable makes it persistent for as long as possible and thus making the cache actually benefical for improving the executing time. Fixes c0sco#11
During every lookup the Bitwarden object verifies whether it is logged in. This executes CLI and HTTP calls which take time, which is unnecessary for most of the time. Therefore I've implemented a global variable which is undefined initially, thus ensuring that `logged_in` function is executed and otherwise use the previous versions. Manual verification shows that this decreases overall execution time of lookup plugin
By setting the BW object as a global instance, everything can be implemented in the Bitwarden class itself, thus reducing the number of global variables and making it more object oriented.
|
@c0sco any indication when this could be merged? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds caching functionality and improves executing times by remove unnecessary logged_in verification. As mentioned in #11, I've also encountered really long executing times in Ansible due to this plugin looking up passwords used in become.
Due to how lookups are being handled internally by Ansible, the only way to implement a caching method is to use global variables. Implementing a cache in the actual
LookModuleorBitwardenmodule is not beneficiary because these objects are recreated for every lookup. However importing the actual Lookup plugin happens rarely and thus the cache exists for a long time within a playbook.Because the plugin check for every lookup whether BW is still logged in, this is also now improved to only check it initially and then trust that result. This also prevents a lot of unnecessary HTTP and CLI lookups.
Caching specific values greatly increases executing times because now certain passwords does not have to be looked up every time. E.g. in my situation the sudo password is looked up for every task, even for every item in a loop, this greatly increases execution time. With this new modification now a cache lookup is used instead of BW lookups. Manual verification shows that this has a extreme benefit on the execution time.
This PR will fix #11.