-
Notifications
You must be signed in to change notification settings - Fork 835
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Decryption: do not fail if no matching creation_rule is present in config file #1434
base: main
Are you sure you want to change the base?
Conversation
d00c23f
to
bf6d65b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One readability comment, but otherwise looks good to me (code-wise)
// Load configuration here for backwards compatibility (error out in case of bad config files), | ||
// but only when not just decrypting (https://github.com/getsops/sops/issues/868) | ||
needsCreationRule := isEncryptMode || isRotateMode || isSetMode || isEditMode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, this would only be false if
isEncryptMode = false
isRotateMode = false
isSetMode = false
isDecryptMode = true
If this is correct, it might be easier to reason about if this is written as
// Load configuration here for backwards compatibility (error out in case of bad config files), | |
// but only when not just decrypting (https://github.com/getsops/sops/issues/868) | |
needsCreationRule := isEncryptMode || isRotateMode || isSetMode || isEditMode | |
// Load configuration here for backwards compatibility (error out in case of bad config files), | |
// but only when not just decrypting (https://github.com/getsops/sops/issues/868) | |
needsCreationRule := !(isEncryptMode || isRotateMode || isSetMode) && isDecryptMode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, this would only be false if
isEncryptMode = false isRotateMode = false isSetMode = false isDecryptMode = true
Yes, due to the way isEditMode
works (it's true
if no other mode is set).
If this is correct, it might be easier to reason about if this is written as
I find the new expression harder to understand, and the intended behavior - the creation rule needs to be checked here if one of the modes is active that needs it later - less clear.
Also your expression has a boolean error, it evaluates to true
in case needsCreationRule
should be false
, so it would have to be needsCreationRule := isEncryptMode || isRotateMode || isSetMode || !isDecryptMode
.
…ent in config file. Signed-off-by: Felix Fontein <felix@fontein.de>
bf6d65b
to
8b803bb
Compare
Since the config file loading is only done for backwards compatibility (and the result isn't used), we can simply skip it when only decrypting.
(This is not a problem for new-style decryption -
sops decrypt
subcommand - since that doens't load the config for the file.)Fixes #868.