decrypt master key in parallel #638
Closed
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
SinisterMinister
force-pushed
the
parallel-decrypt
branch
2 times, most recently
from
ac0c439 to
f1a7822
Compare
Codecov Report
@@ Coverage Diff @@
## develop #638 +/- ##
===========================================
- Coverage 37.15% 36.33% -0.83%
===========================================
Files 21 21
Lines 2893 2967 +74
===========================================
+ Hits 1075 1078 +3
- Misses 1724 1795 +71
Partials 94 94
Continue to review full report at Codecov.
|
|
|
||
| // Capture service locally to prevent data races | ||
| kmsSvcMtx.RLock() | ||
| svc := kmsSvc |
There was a problem hiding this comment.
These changes were necessary to deal with the concurrent access of the kmsSvc by the decrypt calls.
Currently, Sops tries to decrypt the master key by iterating over each one in series. In practice, this can lead to significant performance issues if the working key is at the end of the list. The example case we encountered was using Sops with Ansible and the upcoming Sops plugin for it. During execution, Ansible calls sops for every task to decrypt the variables. In our case, we used KMS and PGP for our keys, and when running on a machine that relied on PGP, it would take over 30 seconds to decrypt a file as it iterated through providers. This would make PGP based builds take hours where it would take minutes for KMS based builds as it would fail the KMS key look up thousands of times throughout the run. This change makes the decryption efforts happen in parallel, returning the first to succeed, making the speed faster than KMS based builds without affecting the KMS based build times.
SinisterMinister
force-pushed
the
parallel-decrypt
branch
from
f1a7822 to
2c2cbf1
Compare
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.
Currently, Sops tries to decrypt the master key by iterating over each one in series. In practice, this can lead to significant performance issues if the working key is at the end of the list. The example case we encountered was using Sops with Ansible and the upcoming Sops plugin for it. During execution, Ansible calls sops for every task to decrypt the variables. In our case, we used KMS and PGP for our keys, and when running on a machine that relied on PGP, it would take over 30 seconds to decrypt a file as it iterated through providers. This would make PGP based builds take hours where it would take minutes for KMS based builds as it would fail the KMS key look up thousands of times throughout the run.
This change makes the decryption efforts happen in parallel, returning the first to succeed, making the speed faster than KMS based builds without affecting the KMS based build times.