Salt: [FEATURE REQUEST] handle gpg rendering failures some other way in pillar

[oeuftete]

Is your feature request related to a problem? Please describe.

When gpg rendering of pillar data fails, the un-decrypted data is just passed along for that pillar entry, without error (though there is a warning log). This can lead to unexpected behaviour.

https://github.com/saltstack/salt/blob/8afbb8e00556e0d491f6052308469e9f4a0c7672/salt/renderers/gpg.py#L372-L376

Example, with key not imported:

root@master:~# salt-call -l warning pillar.item some_secret
local:
    ----------
    some_secret:
        -----BEGIN PGP MESSAGE-----
        Version: GnuPG v2.0.22 (GNU/Linux)

        hQEMA1yfYLgkgU17AQgAoRgwZpSyERF/FGVkWUHqeGwV1idKP/gIl6YTXdyFYbRx
        4eQAiwnW0RvrLBCfWJ0oqOv8HPh3XgPYwuF6YLxanohSeiHoiOwoBFT5myr7CBmY
        quzcHL4DjGLzdcQDVSPwvk9b5PrRLKwT0SfmE3l2QMqr3reV7eXSZ1XBmMLspb86
        p8jxwvc+J9sMsvTQdgb2ChVyrZdWM+xgxfalkeIaTQW37komv9fhHUaE+xIXW2A1
        dpaSsSqaHm+VOjliLS8OERjFBeHjQEkUzx+vslhL8crO5U5zryi4v6loYlCln5R5
        T3GPXuJVjti1pUqjZQ0xjS/0iJ0OOx2sjH2CfacggtJGAeaiS5xvzS692sNs30xl
        p2JuK2ms/CedbZP1TwzxVenRYMxLzsJuZpg76j02ZgBVSqQMxmyoszKfjcZvL0m9
        Do6mY5nRjw==
        =xC3e
        -----END PGP MESSAGE-----

root@master:~# salt-call gpg.import_key filename=/root/privkey.gpg user=salt
local:
    ----------
    message:
        Successfully imported key(s).
    res:
        True

root@master:~# salt-call -l warning pillar.item some_secret
local:
    ----------
    some_secret:
        supersecret

Describe the solution you'd like

Honestly, I'm not sure. Maybe status quo is the best compromise? Maybe that pillar item should just not be populated at all?

Additional context

This has come up in the context of ubuntu 18.04+ rendering failures, cf. https://github.com/saltstack/salt/issues/50882#issuecomment-716597901.

[oeuftete]

ZD-5962.

[wwalker]

My opinion is that if any part of pillar rendering fails, the pillar render should fail.

This is resulting in pushing invalid config files for us.

[oeuftete]

The aws_kms renderer has a similar issue in its typical path:

https://github.com/saltstack/salt/blob/5f91ccc5fb1529dfd613b378905a51e596dbdee7/salt/renderers/aws_kms.py#L234-L238

[wwalker]

Both of those result in generation of invalid pillar content which gets written to minions as if it is good.

That should be a hard fail. Like any other pillar data that is invalid (reference to a missing grain or pillar value - "data failed to compile")

[calve]

Unfortunately this make salt unreliable.

Worse, since we're talking about secrets here, most rendered files that includes GPG decrypted data only returns show_diff=False and the user have no way to know the decryption failed and the rendered file is corrupted.

I agree we should hard fail instead of injecting corrupted data. In case we do not want to makes a breaking change, I suggest this to be an option in salt-master.

I suggest to change the issue with

Describe the solution you'd like:

  - An exception is raised. No data returned. No file changed. No modules executed

[raddessi]

I've been thinking about implementing a gpg wrapper renderer to do exactly this, it would be great if like you said we could just toggle safety on and off (who would want it to pass unencrypted data though?) via the master config and have this as part of the package itself.