[BUG] salt-minion 3006.0 fails to get secret from HashiCorp Vault

[thrashwerk]

Description
salt-minion 3006.0 can no longer retrieve secrets from HashiCorp Vault.

Old minions upgraded from 3005.1 and new servers without prior Salt installations experience the same problem on 3006.0.

Setup
Salt master Vault config /etc/salt/master.d/vault.conf

vault:
  url: https://vault.REDACTED:8200
  auth:
    uses: 30
    method: token
    token: REDACTED
  policies:
    - saltstack/minions

• on-prem machine
• VM (Virtualbox, KVM, etc. please specify)
• VM running on a cloud service, please be explicit and add details
• container (Kubernetes, Docker, containerd, etc. please specify)
• or a combination, please be explicit
• jails if it is FreeBSD
• classic packaging
• onedir packaging
• used bootstrap to install

Steps to Reproduce the behavior
Running vault.read_secret module from the Salt master on a minion fails with an error ERROR: Failed to read secret! KeyError: 'vault'

root@salt-master ~ # salt 'scrap-k8s-node115' vault.read_secret salt/services/scrap-k8s
scrap-k8s-node115:
    ERROR: Failed to read secret! KeyError: 'vault'
ERROR: Minions returned with non-zero exit code

Expected behavior
vault.read_secret returns the key-value pairs from Vault

root@salt-master ~ # salt 'scrap-k8s-node114' vault.read_secret salt/services/scrap-k8s
scrap-k8s-node114:
    ----------
    rke2_backup_s3_access_key:
        REDACTED

Versions Report

salt --versions-report

Salt master is Debian 11.
Minions are either Debian 11 or Ubuntu 22.04, both OS' experience the same problem.

Salt Version:
          Salt: 3006.0
 
Python Version:
        Python: 3.10.11 (main, Apr 14 2023, 05:57:16) [GCC 11.2.0]
 
Dependency Versions:
          cffi: 1.14.6
      cherrypy: unknown
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: 4.0.10
     gitpython: 3.1.31
        Jinja2: 3.1.2
       libgit2: Not Installed
  looseversion: 1.0.2
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 22.0
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.9.8
        pygit2: Not Installed
  python-gnupg: 0.4.8
        PyYAML: 5.4.1
         PyZMQ: 23.2.0
        relenv: 0.11.2
         smmap: 5.0.0
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4
 
System Versions:
          dist: debian 11 bullseye
        locale: utf-8
       machine: x86_64
       release: 5.10.0-18-amd64
        system: Linux
       version: Debian GNU/Linux 11 bullseye

[welcome]

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

• Community Wiki
• Salt’s Contributor Guide
• Join our Community Slack
• IRC on LiberaChat
• Salt Project YouTube channel
• Salt Project Twitch channel

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at saltproject@vmware.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

[thrashwerk]

Creating /etc/salt/minion.d/vault.conf on the minions fixes this

vault:
  config_location: master

Seems it went for the local config but since no Vault config existed there was no vault key therefore KeyError: 'vault'.

The error is somewhat vague and could use some improvement, maybe appending Vault configuration missing to it would be clearer.

The more interesting question is - why did it default to the local config?

config_location was added in 3006.0 - salt/modules/vault.py but I can't understand why it goes for local in salt/utils/vault.py.

[lkubb]

This is just a bug, it accesses __opts__["vault"] directly as the first step of deciding whether to use local or remote configuration. It should be opts.get("vault", {}).get("config_location") or something alike to not break without any vault configuration.

[dmurphy18]

@thrashwerk There is a major re-write of Salt's support for vault by this PR #62684 which is being moved to a salt extension for the Salt 3007 release.

Once the salt extension is ready I shall review the code that was in salt/utils/vault.py has been fixed, given @lkubb wrote the original PR

[markusschuh]

Additional note:

In case the vault config on master uses the verify option, this is no longer read and used by the minion as well. But the verify can be added to minion as well, if needed, e.g.:

vault:
  config_location: master
  verify: /etc/pki/tls/cert.pen

[ipaqmaster]

This is still a problem on 3006.1. Every new guest automatically provisioned cannot do any vault.read_secret calls until:

vault:
config_location: master

Gets set at a minimum.

I've worked around this problem by adding a quirks.sls salt state which sets the above but the nature of this issue requires the salt-minion service then be restarted before it checks for that config again. Then I can call our salt.highstate. A major inconvenience.

Doing this via the salt-master is annoying as sending a cmd.run 'systemctl restart salt-minion' cannot return given the service gets restarted - and the minion doesn't always come back either. I frequently experience the same issue with minion.restart. All Layer 2.