[BUG] Regression in redis cache handling for mine.update and mine.get in SaltStack introduced in v3005rc1

[stupiddr]

Description
After switching our SaltStack installation from the localfs cache backend to the Redis cache backend, we encountered issues with mine.update and mine.get. These problems were introduced by this commit.

Regression Issues:

1. When listing minions from Redis, Salt queries the incorrect Redis/Valkey SET ("$BANKEY@minions"), preventing minion retrieval. This is due to changes in how banks are referenced in the list function.
   

2. The _build_bank_hier function writes "." as the value, causing the Redis SET ("$BANK@minions") to contain only "." instead of the expected list of minions.
   
   According to SaltStack Documentation:

   • "$BANK_*" is a Redis SET containing the list of banks under the current bank, rather than always being ".".

Setup
SaltMaster configuration:

[root@salt-test ~]# cat /etc/salt/master
cache: redis
cache.redis.host: 127.0.0.1
cache.redis.port: 6379
cache.redis.db: '0'
cache.redis.password: ''
cache.redis.bank_prefix: $BANK
cache.redis.bank_keys_prefix: $BANKEYS
cache.redis.key_prefix: $KEY
cache.redis.separator: '@'

log_level_logfile: debug
[root@salt-test ~]#

SaltMinion configuration:

[root@salt-test ~]# cat /etc/salt/minion
hash_type: sha256
id: salt-test
log_level: info
master: salt-test.cloud.phx3.gdg
mine_functions:
  grains.item:
  - os
[root@salt-test ~]#

Please be as specific as possible and give set-up details.

• on-prem machine
• Install:
  • salt-3007.1-0.x86_64
  • redis or valkey (same issue observed with both)
  • python3-redis (dnf install python3-redis -y)
  • redis module (salt-pip install redis)

Steps to Reproduce the behavior

Using the contents above for /etc/salt/master for salt-master and /etc/salt/minion for salt-minion run the following command so the minions "mine" and "data" is written to the redis cache:

• salt 'salt-test' mine.update
• salt 'salt-test' saltutil.refresh_grains

Use mine.get targeting with grain or compound as the target type:

[root@salt-test ~]# salt 'salt-test' mine.get 'G@os:AlmaLinux' grains.item compound
salt-test:
    ----------
[root@salt-test ~]# salt 'salt-test' mine.get 'os:AlmaLinux' grains.item grain
salt-test:
    ----------
[root@salt-test ~]#

Expected behavior
The mine.get module to return back the mine data that matched the query:

[root@salt-test ~]# salt 'salt-test' mine.get 'G@os:AlmaLinux' grains.item compound
salt-test:
    ----------
    salt-test:
        ----------
        os:
            AlmaLinux
[root@salt-test ~]# salt 'salt-test' mine.get 'os:AlmaLinux' grains.item grain
salt-test:
    ----------
    salt-test:
        ----------
        os:
            AlmaLinux
[root@salt-test ~]#

Versions Report

salt --versions-report

Salt Version:
          Salt: 3007.1

Python Version:
        Python: 3.10.14 (main, Apr  3 2024, 21:30:09) [GCC 11.2.0]

Dependency Versions:
          cffi: 1.16.0
      cherrypy: unknown
      dateutil: 2.8.2
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.4
       libgit2: Not Installed
  looseversion: 1.3.0
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.7
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 23.1
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.19.1
        pygit2: Not Installed
  python-gnupg: 0.5.2
        PyYAML: 6.0.1
         PyZMQ: 25.1.2
        relenv: 0.16.0
         smmap: Not Installed
       timelib: 0.3.0
       Tornado: 6.3.3
           ZMQ: 4.3.4

Salt Package Information:
  Package Type: onedir

System Versions:
          dist: almalinux 9.5 Teal Serval
        locale: utf-8
       machine: x86_64
       release: 5.14.0-503.22.1.el9_5.x86_64
        system: Linux
       version: AlmaLinux 9.5 Teal Serval

Additional context

mine.get regression:
When either command:

• salt 'salt-test' mine.get 'os:AlmaLinux' grains.item grain
• salt 'salt-test' mine.get 'G@os:AlmaLinux' grains.item compound

Is ran the following is observed in redis/valkey:

127.0.0.1:6379> MONITOR
OK
[0 127.0.0.1:52364] "CLIENT" "SETINFO" "LIB-NAME" "redis-py"
[0 127.0.0.1:52364] "CLIENT" "SETINFO" "LIB-VER" "5.2.1"
[0 127.0.0.1:52364] "SELECT" "0"
[0 127.0.0.1:52364] "SMEMBERS" "$BANKEYS@minions"

The Redis SET queried should be "$BANK@minions" as "$BANKEY@minions" wouldn't even be an Redis SET to query based on the hierarchy that is built in redis: https://docs.saltproject.io/en/3007/ref/cache/all/salt.cache.redis_cache.html#redis

Reverting this line changed of the list function in redis_cache.py and restarting salt-master resolves the issue and the correct SET of "$BANK@minions" is then queried:

127.0.0.1:6379> MONITOR
OK
[0 127.0.0.1:58080] "CLIENT" "SETINFO" "LIB-NAME" "redis-py"
[0 127.0.0.1:58080] "CLIENT" "SETINFO" "LIB-VER" "5.2.1"
[0 127.0.0.1:58080] "SELECT" "0"
[0 127.0.0.1:58080] "SMEMBERS" "$BANK@minions"

mine.update regression:
This is observed in redis/valkey when salt 'salt-test' mine.update is ran:

[root@salt-test ~]# valkey-cli
127.0.0.1:6379> FLUSHDB
OK
127.0.0.1:6379> MONITOR
OK
[0 127.0.0.1:52616] "CLIENT" "SETINFO" "LIB-NAME" "redis-py"
[0 127.0.0.1:52616] "CLIENT" "SETINFO" "LIB-VER" "5.2.1"
[0 127.0.0.1:52616] "SELECT" "0"
[0 127.0.0.1:52616] "GET" "$KEY@minions/salt-test/mine"
[0 127.0.0.1:52616] "MULTI"
[0 127.0.0.1:52616] "SADD" "$BANK@minions" "."
[0 127.0.0.1:52616] "SADD" "$BANK@minions/salt-test" "."
[0 127.0.0.1:52616] "SET" "$KEY@minions/salt-test/mine" "\x81\xabgrains.item\x81\xa2os\xa9AlmaLinux"
[0 127.0.0.1:52616] "SADD" "$BANKEYS@minions/salt-test" "mine"
[0 127.0.0.1:52616] "SET" "$TSTAMP@{'minions/salt-test'}/{'mine'}" "\xceg\xa1\x0cu"
[0 127.0.0.1:52616] "EXEC"

The line in the redis output with the issue is:
[0 127.0.0.1:52616] "SADD" "$BANK@minions" "."
Which is inserted by this line of redis_cache.py

The salt-master debug logs for redis_cache.py inserting these items:

2025-02-03 19:54:03,760 [/opt/saltstack/salt/lib/python3.10/site-packages/salt/cache/redis_cache.py:345 ][DEBUG   ][144609] Adding minions/salt-test to $BANK@minions
2025-02-03 19:54:03,760 [/opt/saltstack/salt/lib/python3.10/site-packages/salt/cache/redis_cache.py:345 ][DEBUG   ][144609] Adding minions/salt-test to $BANK@minions/salt-test
2025-02-03 19:54:03,760 [/opt/saltstack/salt/lib/python3.10/site-packages/salt/cache/redis_cache.py:387 ][DEBUG   ][144609] Setting the value for mine under minions/salt-test ($KEY@minions/salt-test/mine)
2025-02-03 19:54:03,760 [/opt/saltstack/salt/lib/python3.10/site-packages/salt/cache/redis_cache.py:394 ][DEBUG   ][144609] Adding mine to $BANKEYS@minions/salt-test

According to https://docs.saltproject.io/en/3007/ref/cache/all/salt.cache.redis_cache.html#redis:
"$BANK_*" is a Redis SET containing the list of banks under the current bank, rather than always being "."

Reverting the code in the "_build_bank_hier" function to the code in prior to the commit that caused the regression and restarting salt-master results in the following observed in redis/valkey when salt 'salt-test' mine.update is ran:

[root@salt-test ~]# valkey-cli
127.0.0.1:6379> FLUSHDB
OK
127.0.0.1:6379> MONITOR
OK
[0 127.0.0.1:50170] "CLIENT" "SETINFO" "LIB-NAME" "redis-py"
[0 127.0.0.1:50170] "CLIENT" "SETINFO" "LIB-VER" "5.2.1"
[0 127.0.0.1:50170] "SELECT" "0"
[0 127.0.0.1:50170] "GET" "$KEY@minions/salt-test/mine"
[0 127.0.0.1:50170] "MULTI"
[0 127.0.0.1:50170] "SADD" "$BANK@minions" "salt-test"
[0 127.0.0.1:50170] "SET" "$KEY@minions/salt-test/mine" "\x81\xabgrains.item\x81\xa2os\xa9AlmaLinux"
[0 127.0.0.1:50170] "SADD" "$BANKEYS@minions/salt-test" "mine"
[0 127.0.0.1:50170] "SET" "$TSTAMP@{'minions/salt-test'}/{'mine'}" "\xceg\xa1\x10R"
[0 127.0.0.1:50170] "EXEC"

"[0 127.0.0.1:50170] "SADD" "$BANK@minions" "salt-test""
Should be the expected result so that "$BANK@minions" is a Redis SET containing the list of banks.

The salt-master debug logs for redis_cache.py inserting these items:

2025-02-03 20:04:47,899 [/opt/saltstack/salt/lib/python3.10/site-packages/salt/cache/redis_cache.py:345 ][DEBUG   ][150074] Adding salt-test to $BANK@minions
2025-02-03 20:04:47,899 [/opt/saltstack/salt/lib/python3.10/site-packages/salt/cache/redis_cache.py:390 ][DEBUG   ][150074] Setting the value for mine under minions/salt-test ($KEY@minions/salt-test/mine)
2025-02-03 20:04:47,899 [/opt/saltstack/salt/lib/python3.10/site-packages/salt/cache/redis_cache.py:397 ][DEBUG   ][150074] Adding mine to $BANKEYS@minions/salt-test

After reverting both of these changes:

• _build_bank_hier function
• list function
  Restarting salt-master, running mine.update, and saltutil.refresh_grains so data is present in redis.

The expected data is then returned from mine.get using compound or grain targeting:

[root@salt-test ~]# salt 'salt-test' mine.get 'G@os:AlmaLinux' grains.item compound
salt-test:
    ----------
    salt-test:
        ----------
        os:
            AlmaLinux
[root@salt-test ~]# salt 'salt-test' mine.get 'os:AlmaLinux' grains.item grain
salt-test:
    ----------
    salt-test:
        ----------
        os:
            AlmaLinux
[root@salt-test ~]#