Fail if record cannot be decrypted with any keys in multi-decryptor#2632
Fail if record cannot be decrypted with any keys in multi-decryptor#2632justincormack wants to merge 1 commit intomoby:masterfrom
Conversation
Previously this would return nil, nil in the final fallthrough case. Signed-off-by: Justin Cormack <justin.cormack@docker.com>
022604e to
52f18e8
Compare
Codecov Report
@@ Coverage Diff @@
## master #2632 +/- ##
==========================================
- Coverage 62.59% 61.98% -0.61%
==========================================
Files 51 134 +83
Lines 7180 21822 +14642
==========================================
+ Hits 4494 13526 +9032
- Misses 2231 6839 +4608
- Partials 455 1457 +1002 |
|
Thanks! I definitely do not have a test for this case. I'm not entirely sure I see where it'd be These were the two test cases I could think of: diff --git a/manager/encryption/encryption_test.go b/manager/encryption/encryption_test.go
index 2556ef8a..1b566b21 100644
--- a/manager/encryption/encryption_test.go
+++ b/manager/encryption/encryption_test.go
@@ -107,6 +107,15 @@ func TestMultiDecryptor(t *testing.T) {
require.IsType(t, ErrCannotDecrypt{}, err)
}
}
+
+ // Test multidecryptor where it does not have a decryptor with the right key
+ for _, d := range []MultiDecrypter{m, NewMultiDecrypter()} {
+ plaintext := []byte("message")
+ ciphertext, err := Encrypt(plaintext, NewNACLSecretbox([]byte("other")))
+ require.NoError(t, err)
+ _, err = Decrypt(ciphertext, d)
+ require.IsType(t, ErrCannotDecrypt{}, err)
+ }
}The first one returns The error returned in your change is much more friendly, however. |
|
Oh, actually I am being confused, yes it will error... |
|
@justincormack I actually find named return vars kinda confusing - I'm not sure why I used it. I'll submit a new PR with the test and with explicit returns instead, and your much nicer error. Thanks again! |
Previously this would return nil, nil in the final fallthrough case.
Signed-off-by: Justin Cormack justin.cormack@docker.com
cc @cyli