search

saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.1k stars 5.47k forks source link

[EPIC] Documented options in minion/master config files don't match minion/master config documentation, and vice versa #58112

Open ScriptAutomate opened 4 years ago

ScriptAutomate commented 4 years ago

Description

There are options listed in the conf/minion file that are undocumented in the minion configuration documentation, doc/ref/configuration/minion.rst, and vice versa.

Setup

Clone the latest version of this repository.

Steps to Reproduce the behavior

See the information in Additional context below.

Expected behavior

The options listed in the minion configuration file and the minion configuration documentation should match, or at a minimum the minion configuration documentation should include information regarding all options listed in the conf/minion file. This is the bug.

An improvement/enhancement could be for the conf/minion file to also include all the options in the minion configuration documentation that are currently missing. This would be helpful for users, such as what was experienced in #57931

Additional context

I whipped up these scripts to find all options missing from the conf/minion file that are listed doc/ref/configuration/minion.rst, and vice versa.

Minion audit

#!/usr/bin/env bash
# Format for option names
cat doc/ref/configuration/minion.rst | \
    grep '\-\-\-' -B1 | \
    grep -v '\-\-' | \
    sed 's/``//g' | \
    sort > minion-options-docs.txt

# Grab all config options for minion conf from conf
cat conf/minion | \
    grep ':' | \
    egrep -v "^# |\." | \
    sed 's:#::g' | \
    cut -d':' -f1 | \
    sort | \
    uniq > minion-options-conf.txt

# What's potentially missing from minion conf docs?
diff minion-options-docs.txt minion-options-conf.txt | \
    grep '>' | \
    sed 's:>\ ::g' > settings-missing-from-minion-options-docs.txt

# What's missing from minion conf file?
diff minion-options-docs.txt minion-options-conf.txt | \
    grep '<' | \
    sed 's:<\ ::g' > settings-missing-from-minion-options-conf.txt

This ends up creating two files:

$ cat settings-missing-from-minion-options-docs.txt | wc -l
8
$ cat settings-missing-from-minion-options-conf.txt | wc -l
71

This seems to say that quite a few options are missing from the minion conf file, and some are missing from the minion configuration documentation.

Missing from conf/minion

Source file: https://github.com/saltstack/salt/blob/master/conf/minion

always_verify_signature
cache_sreqs
cmd_blacklist_glob
cmd_whitelist_glob
color_theme
decrypt_pillar
decrypt_pillar_default
decrypt_pillar_delimiter
decrypt_pillar_renderers
default_include
default_top
docker.compare_container_networks
docker.update_mine
enable_fqdns_grains
enable_gpu_grains
enable_zip_modules
env_order
extmod_whitelist/extmod_blacklist
fibre_channel_grains
grains_blacklist
grains_cache_expiration
grains_dirs
http_connect_timeout
http_request_timeout
iscsi_grains
lock_saltenv
log_rotate_backup_count
log_rotate_max_bytes
master_sign_key_name
master_tops_first
master_type
master_uri_format
metadata_server_grains
minion_id_remove_domain
modules_max_memory
nvme_grains
on_demand_ext_pillar
optimization_order
pass_to_ext_pillars
pidfile
publish_port
recon_default
recon_max
recon_randomize
retry_dns
retry_dns_count
return_retry_timer
return_retry_timer_max
saltenv
snapper_states
snapper_states_config
source_address
source_interface_name
source_publish_port
source_ret_port
ssh_merge_pillar
state_top
state_top_saltenv
top_file_merging_strategy
transport
use_master_when_local
use_yamlloader_old
verify_master_pubkey_sign
winrepo_cache_expire_max
winrepo_cache_expire_min
winrepo_cachefile
winrepo_dir
winrepo_dir_ng
winrepo_remotes
winrepo_remotes_ng
winrepo_source_dir

Missing from doc/ref/configuration/minion.rst

Source file: https://github.com/saltstack/salt/blob/master/doc/ref/configuration/minion.rst

color
environment
event_match_type
key_logfile
output
return
state_aggregate
state_output_profile

Other Thoughts

Are the minion configuration options auto-populated anywhere? If not, it would be a good idea to introduce automation that would autopopulate content in either the minion conf file, or the rst documentation page (or both, ideally). That, or audit to see whether there are currently conf options that aren't included in the minion conf, nor the rst documentation page.

ScriptAutomate commented 4 years ago

I also decided to audit the master related conf and docs, in a similar fashion, after seeing that an issue was previously opened back in 2016 concerning documenting both minion and master configs: #32400

Master audit

#!/usr/bin/env bash
# Format for option names
sed -n 1,5818p doc/ref/configuration/master.rst | \
    egrep '\-\-\-|\*\*\*' -B1 | \
    grep -v ':' | \
    egrep -v '\-\-|\*\*| ' | \
    sed 's/``//g' | \
    sort > master-options-docs.txt

# Grab all config options for master conf from conf
cat conf/master | \
    grep ':' | \
    egrep -v "^# |\." | \
    sed 's:#::g' | \
    cut -d':' -f1 | \
    sort | \
    uniq > master-options-conf.txt

# What's potentially missing from master conf docs?
diff master-options-docs.txt master-options-conf.txt | \
    grep '>' | \
    sed 's:>\ ::g' > settings-missing-from-master-options-docs.txt

# What's missing from master conf file?
diff master-options-docs.txt master-options-conf.txt | \
    grep '<' | \
    sed 's:<\ ::g' > settings-missing-from-master-options-conf.txt

This ends up creating two files:

$ cat settings-missing-from-master-options-docs.txt | wc -l
30
$ cat settings-missing-from-master-options-conf.txt | wc -l
114

This seems to say that quite a few options are missing from the master conf file, and some are missing from the master configuration documentation.

Missing from conf/master

Source file: https://github.com/saltstack/salt/blob/master/conf/master

api_logfile
api_pidfile
auth_events
autoreject_file
autosign_file
autosign_timeout
azurefs_update_interval
color_theme
con_cache
default_include
enable_gpu_grains
enforce_mine_cache
event_publisher_niceness
event_return_niceness
ext_job_cache
extmod_whitelist/extmod_blacklist
file_ignore_glob
fileserver_limit_traversal
fileserver_list_cache_time
fileserver_update_niceness
fileserver_verify_config
gitfs_base
gitfs_disable_saltenv_mapping
gitfs_global_lock
gitfs_mountpoint
gitfs_ref_types
gitfs_saltenv
gitfs_saltenv_blacklist
gitfs_saltenv_whitelist
gitfs_update_interval
git_pillar_includes
git_pillar_update_interval
git_pillar_verify_config
hgfs_base
hgfs_branch_method
hgfs_mountpoint
hgfs_remotes
hgfs_root
hgfs_saltenv_blacklist
hgfs_saltenv_whitelist
hgfs_update_interval
http_connect_timeout
http_request_timeout
include
interface
jinja_lstrip_blocks
jinja_trim_blocks
job_cache_store_endtime
log_rotate_backup_count
log_rotate_max_bytes
maintenance_niceness
master_id
master_job_cache
master_pubkey_signature
master_sign_key_name
master_sign_pubkey
master_use_pubkey_signature
max_minions
minion_data_cache_events
minionfs_blacklist
minionfs_env
minionfs_mountpoint
minionfs_update_interval
minionfs_whitelist
mworker_niceness
mworker_queue_niceness
optimization_order
pidfile
pillar_includes_override_sls
ping_on_rotate
presence_events
publish_session
pub_server_niceness
reactor_niceness
req_server_niceness
rest_timeout
roots_update_interval
roster_defaults
rotate_aes_key
s3fs_update_interval
skip_grains
sock_pool_size
ssh_priv_passwd
ssh_scan_timeout
state_top
state_top_saltenv
svnfs_branches
svnfs_mountpoint
svnfs_remotes
svnfs_root
svnfs_saltenv_blacklist
svnfs_saltenv_whitelist
svnfs_tags
svnfs_trunk
svnfs_update_interval
syndic_forward_all_events
syndic_pidfile
tcp_master_publish_pull
tcp_master_workers
transport
transport_opts
userdata_template
use_yamlloader_old
winrepo_branch
winrepo_cachefile
winrepo_insecure_auth
winrepo_passphrase
winrepo_password
winrepo_privkey
winrepo_provider
winrepo_pubkey
winrepo_ssl_verify
winrepo_user
yaml_utf8

Missing from doc/ref/configuration/master.rst

Source file: https://github.com/saltstack/salt/blob/master/doc/ref/configuration/master.rst

batch_safe_limit
batch_safe_size
client_acl_verify
default_top
event_match_type
event_return_queue_max_seconds
fileserver_events
gitfs_insecure_auth
gitfs_passphrase
gitfs_password
gitfs_privkey
gitfs_pubkey
gitfs_refspecs
git_pillar_insecure_auth
git_pillar_passphrase
git_pillar_password
git_pillar_privkey
git_pillar_pubkey
git_pillar_refspecs
git_pillar_user
gpg_cache
gpg_cache_backend
gpg_cache_ttl
key_cache
key_logfile
netapi_allow_raw_shell
pillar_gitfs_ssl_verify
return
ssh_run_pre_flight
ssh_update_roster
ScriptAutomate commented 4 years ago

Also: according to #32400, the following are completely undocumented at the moment and aren't to be found in the above audits of master/minion config files or config documentation:

auth_mode
bootstrap_delay
cluster_masters
cluster_mode
enable_lspci
http_max_body
http_request_timeout
pillar_version
queue_dirs
search
search_index_interval
serial
sign_pub_messages
sqlite_queue_dir
syndic_event_forward_timeout
syndic_jid_forward_cache_hwm
syndic_max_event_process_time
zmq_filtering

It looks like the best source for all available options is:

Which, running an audit against all default options for master/minion here led to the following 49 options potentially needing to be documented (or removed from salt/config/__init__.py if they are deprecated?). I do not know whether these can be defined within master/minion conf files:

archive_jobs
beacons_before_connect
cmd_safe
disabled_requisites
discovery
django_auth_path
django_auth_settings
drop_messages_signature_fail
dummy_pub
eauth_tokens
engines
extmod_blacklist
extmod_whitelist
gitfs_fallback
gitfs_user
git_pillar_fallback
id_function
ipc_write_buffer
key_pass
local
log_fmt_jid
minion_jid_queue_hwm
minion_restart_command
minion_sign_messages
multifunc_ordered
password
permissive_acl
python2_bin
python3_bin
regen_thin
renderer_blacklist
renderer_whitelist
require_minion_sign_messages
resolve_dns_fallback
restart_on_error
salt_cp_chunk_size
schedule
scheduler_before_connect
signing_key_pass
ssh_config_file
ssh_sudo_user
state_auto_order
thoriumenv
thorium_interval
thorium_roots
thorium_top
unique_jid
username
winrepo_fallback
nicholasmhughes commented 4 years ago

ran into the need for sqlite_queue_dir today due to use case similar to #45854

ScriptAutomate commented 3 years ago

Options like token_dir and syndic_dir aren't listed in the VALID_OPTS or DEFAULT_MASTER_OPTS in salt/config/__init__.py. They appear in salt/config/__init__.py, but are hidden as a kind of post-processing catch when applying master and syndic configs.

https://github.com/saltstack/salt/blob/3f3010415d7e12b9bfad15efa592ed024e314a6a/salt/config/__init__.py#L3854-L3855 They should be included in VALID_OPTS and in DEFAULT_MASTER_OPTS (as empty strings). They need to be included like pki_dir is (as an str), with a default value in default master opts as "" :

https://github.com/saltstack/salt/blob/3f3010415d7e12b9bfad15efa592ed024e314a6a/salt/config/__init__.py#L180-L181

A messy audit shows the following options that may or may not be undocumented:

egrep -n "opts\[\".*\"\] \=" salt/config/__init__.py | grep -v "opts\[\"\_"
1860:        opts["return"] = ",".join(opts["return"])
1944:            conf_opts["id"] = str(conf_opts["id"])
1946:            conf_opts["id"] = salt.utils.data.decode(conf_opts["id"])
2026:            opts["conf_file"] = path
2613:    opts["providers"] = providers_config
2619:    opts["profiles"] = profiles_config
3420:        opts["grains"] = loader.grains(opts)
3544:        opts["ssl"] = None
3547:        opts["ssl"] = {}
3613:            opts["environment"] = opts["saltenv"]
3620:            opts["saltenv"] = opts["environment"]
3636:            opts["id"] = minion_id
3642:        opts["id"] = _append_domain(opts)
3659:        opts["pidfile"] = os.path.join(
3664:        opts["sock_dir"] = os.path.join(opts["cachedir"], ".salt-unix")
3668:    opts["open_mode"] = opts["open_mode"] is True
3669:    opts["file_roots"] = _validate_file_roots(opts["file_roots"])
3670:    opts["pillar_roots"] = _validate_pillar_roots(opts["pillar_roots"])
3673:    opts["extension_modules"] = opts.get("extension_modules") or os.path.join(
3677:    opts["utils_dirs"] = opts.get("utils_dirs") or [
3702:        opts["beacons"] = {}
3705:        opts["ipc_write_buffer"] = _DFLT_IPC_WBUFFER
3707:        opts["ipc_write_buffer"] = 0
3710:    opts["hash_type"] = opts["hash_type"].lower()
3728:            opts["discovery"] = {}
3744:        opts["discovery"] = salt.utils.dictupdate.update(
3798:        opts["nodegroups"] = DEFAULT_MASTER_OPTS.get("nodegroups", {})
3800:        opts["nodegroups"] = salt.utils.data.repack_dictlist(opts["nodegroups"])
3831:            opts["environment"] = opts["saltenv"]
3838:            opts["saltenv"] = opts["environment"]
3849:        opts["sock_dir"] = os.path.join(opts["cachedir"], ".salt-unix")
3851:    opts["token_dir"] = os.path.join(opts["cachedir"], "tokens")
3852:    opts["syndic_dir"] = os.path.join(opts["cachedir"], "syndics")
3855:    opts["extension_modules"] = opts.get("extension_modules") or os.path.join(
3859:    opts["utils_dirs"] = opts.get("utils_dirs") or [
3867:        opts["ipc_write_buffer"] = _DFLT_IPC_WBUFFER
3869:        opts["ipc_write_buffer"] = 0
3878:        opts["id"] = _append_domain(opts)
3910:    opts["open_mode"] = opts["open_mode"] is True
3911:    opts["auto_accept"] = opts["auto_accept"] is True
3912:    opts["file_roots"] = _validate_file_roots(opts["file_roots"])
3913:    opts["pillar_roots"] = _validate_file_roots(opts["pillar_roots"])
3923:        opts["file_ignore_regex"] = []
3936:            opts["file_ignore_glob"] = [opts["file_ignore_glob"]]
3946:        opts["worker_threads"] = 3
3951:    opts["hash_type"] = opts["hash_type"].lower()
4005:        opts["token_file"] = os.path.abspath(os.path.expanduser(opts["token_file"]))
4014:                opts["token"] = fp_.read().strip()
4016:    if opts["interface"] == "0.0.0.0":
4017:        opts["interface"] = "127.0.0.1"
4021:        opts["master_uri"] = "tcp://{ip}:{port}".format(

Further filtered:

egrep "opts\[\".*\"\] \=" salt/config/__init__.py | grep -v "opts\[\"\_" | cut -d'"' -f2 | sort | uniq
auto_accept
beacons
conf_file
discovery
environment
extension_modules
file_ignore_glob
file_ignore_regex
file_roots
grains
hash_type
id
interface
ipc_write_buffer
master_uri
nodegroups
open_mode
pidfile
pillar_roots
profiles
providers
return
saltenv
sock_dir
ssl
syndic_dir
token
token_dir
token_file
utils_dirs
worker_threads

These would require further research by SME's to learn whether other options here - if not already picked up in the other audits within this issue - need to be included in places like VALID_OPTS and documented. I have not audited this entire list against what already exists in the rest of the init config or the master/minion config files to see if they may already be documented as expected.

max-arnold commented 3 years ago

This is a really good research!

My two cents:

I'm not sure if it is practical, but maybe it makes sense to wrap the options dictionary into a class (only in unit tests) that logs every key access attempt. That (and some log/docs parsing automation) could help reveal other undocumented options as well as prevent adding new ones.

max-arnold commented 3 years ago

By the way, many minion options are also useful in the MasterMinion context (orchestration states). For example, you can add the use_superseded: [module.run] option to a master config file to use the new module.run syntax in orchestrations.

root@master:~# cat /srv/salt/myorch.sls
Version test:
  module.run:
    - test.version:

root@master:~# salt-run state.orch myorch
[WARNING ] The function "module.run" is using its deprecated version and will expire in version "Phosphorus".
[ERROR   ] Module function Version test is not available
master_master:
----------
          ID: Version test
    Function: module.run
      Result: False
     Comment: Module function Version test is not available
     Started: 21:51:03.568205
    Duration: 1.771 ms
     Changes:

Summary for master_master
------------
Succeeded: 0
Failed:    1
------------
Total states run:     1
Total run time:   1.771 ms

root@master:~# echo "use_superseded: [module.run]" > /etc/salt/master.d/opts.conf

root@master:~# salt-run state.orch myorch
master_master:
----------
          ID: Version test
    Function: module.run
      Result: True
     Comment: test.version: 3002.5
     Started: 21:51:36.216013
    Duration: 1.97 ms
     Changes:
              ----------
              test.version:
                  3002.5

Summary for master_master
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:   1.970 ms
max-arnold commented 3 years ago

But the leaky bucket needs to be fixed first, otherwise it would be a never-ending job... Another undocumented option has been added: https://github.com/saltstack/salt/pull/59833