Add option to have gpg decrypt failures treated as errors

[jfindlay]

What does this PR do?

This is defaulted false for compatibility, and is controlled via a gpg_decrypt_must_succeed option.

Previous Behavior

If this is True- and the ciphertext couldn't be decrypted- then it's treated as an error rather than just passing the raw ciphertext through.

Sending the ciphertext through basically is never desired- the point of the gpg renderer is to decrypt the secret, not send the cipher text through.

New Behavior

Explicitly error if gpg does not decrypt the ciphertext.

Merge requirements satisfied?

• Docs
• Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
• Tests written/updated

Commits signed with GPG?

Yes

[MKLeb]

Something like the following in tests/pytest/unit/renderers/test_gpg.py may help with test failures...

@pytest.fixture
def configure_loader_modules():
    """
    GPG renderer configuration
    """
    return {
        gpg: {
            "__opts__": {
                "gpg_decrypt_must_succeed": True
            }
        }
    }

[jfindlay]

@Ch3LL anything else that needs to be done on this?

[Ch3LL]

Apologies for the delay, I was out sick most of last week. Thanks it looks good :)

[jfindlay]

Apologies for the delay, I was out sick most of last week. Thanks it looks good :)

No problem, thanks for the review.

[max-arnold]

I guess it is too late to fix, but Salt has two configuration namespaces for feature toggles:

1. features
2. use_superseded

This means that instead of a global gpg_decrypt_must_succeed option, the toggle could be implemented as:

features:
  strict_gpg_decrypt

from salt.features import features
if features.get("strict_gpg_decrypt", False):
    ...

or

use_superseded:
  - module.foo

from salt.utils.decorators import with_deprecated

@with_deprecated(globals(), "Beryllium")
def foo():
    "This is a new function"

def _foo():
    "This is a deprecated function"

P.S. It would be nice to deprecate the non-strict way in future (or flip the default). These feature flags tend to accumulate in various master/minion bootstrap scripts I'm working on. I'm always torn between enabling better defaults (module.run, enable_slsvars_fixes, enable_legacy_startup_events: false, etc) and simpler bootstrap scripts.

[jfindlay]

It would be nice to deprecate the non-strict way in future (or flip the default).

@max-arnold, this was my intention. It's clearly the obvious thing to do. Let the option bake a couple of releases and then there's no reason to change the default behavior. You're welcome to submit the pull request, otherwise I will get around to actioning it from my backlog at some point.

These feature flags tend to accumulate.

Yes, change the default and burn the config.