saltstack / saltstack/salt

[BUG] `reactor` config value type should be list or string

Open
#63,579 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug Reactor
Dominant language
Python
Stars
15.7k
Forks
5.6k
Avg merge
2d 44m
Merged PRs (30d)
80

Description

Description
It should be possible to specify reactor as a string to a YAML-formatted file, according to:
https://github.com/saltstack/salt/blob/3ddd7eb6ffe8059e2f889031c67a4e2605da1a4d/salt/utils/reactor.py#L87-L98

This avoids having to reload the master each time the reactor mappings are updated, therefore providing a piece of top file functionality for reactors. This behavior is not documented though.

When trying this functionality, one is blocked by the config type validation frequent warnings transpire, but the reactions work:
https://github.com/saltstack/salt/blob/3ddd7eb6ffe8059e2f889031c67a4e2605da1a4d/salt/config/__init__.py#L697-L698

Steps to Reproduce the behavior

# /etc/salt/master
reactor: /etc/salt/master.d/reactor.yaml
# /etc/salt/master.d/reactor.yaml 
- some/event:
  - react.to.some_event

Expected behavior
Load reactor mappings dynamically without complaint

Screenshots

[salt.config      :1929][WARNING ][230228] Config option 'reactor' with value /etc/salt/master.d/reactor.yaml has an invalid type of str, a list is required for this option

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)
Salt Version:
          Salt: 3005.1

Dependency Versions:
          cffi: 1.14.6
      cherrypy: unknown
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: 4.0.9
     gitpython: 3.1.29
        Jinja2: 3.1.0
       libgit2: Not Installed
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.9.8
        pygit2: Not Installed
        Python: 3.9.16 (main, Nov  1 2022, 00:00:00)
  python-gnupg: 0.4.8
        PyYAML: 5.4.1
         PyZMQ: 23.2.0
         smmap: 5.0.0
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4

System Versions:
          dist: rocky 9.1 Blue Onyx
        locale: utf-8
       machine: x86_64
       release: 5.14.0-162.6.1.el9_1.0.1.x86_64
        system: Linux
       version: Rocky Linux 9.1 Blue Onyx

Additional context
https://github.com/saltstack/salt/issues/8098
https://github.com/saltstack/salt/issues/570

Other parts of the reactor code (concerned with adding/removing reactors) assume a list though, so there is some discrepancy/potential for conflicts, but the functionality provides value. To synthesize both functionalities, maybe the transient additions/removals should be in a separate list, which is guaranteed to be one.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in salt/config/init.py at the validation referenced in the issue, then read salt/utils/reactor.py around the linked reactor configuration handling. Reproduce the YAML configuration with a string value and check how the add/remove reactor paths assume a list. Done means string-based reactor mappings load dynamically without the invalid-type warning while existing list behavior remains compatible.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
devops, infrastructure
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.