Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Salt minion deployed by open-vm-tools creates broken minion configuration from guestinfo./vmware.components.salt_minion.args #739

Open
stanimir-kozarev opened this issue Oct 1, 2024 · 8 comments
Labels

Comments

@stanimir-kozarev
Copy link

Describe the bug

Salt minion deployed by open-vm-tools with SaltStack component has broken minion configuration with all custom arguments to VMware Tools salt-minion setup script svtminion.sh including service arguments with "-" or "--". The bug is present in version 12.3.x and 12.4.x. The affected svtminion.sh version is 1.6.

Reproduction steps

  1. Add VM configuration parameter "guestinfo.vmware.components.available" with value
    "salt_minion"
  2. Add VM configuration parameter "guestinfo./vmware.components.salt_minion.args" with value that contain script arguments starting with "-" or "--" and applicable minion configuration lines separated by space
    "--minionversion 3007.1 --source http://saltmaster01.corp.local/salt/py3/onedir --loglevel debug master=saltmaster01.corp.local id=testminion minion_id_caching=False minion_id_lowercase=True minion_id_remove_domain=True"
  3. Add VM configuration parameter "guestinfo./vmware.components.salt_minion.desiredstate" with value "present"
  4. Monitor VM configuration parameter "guestinfo.vmware.components.salt_minion.laststatus" until its value is "100"
  5. List the content of the produced by svtminion.sh script Salt minion configuration file at /etc/salt/minion.

cat /etc/salt/minion

Minion configuration file - created by VMTools Salt script

enable_fqdns_grains: False
--minionversion: --minionversion
3007.1: 3007.1
--source: --source
http://saltmaster01.corp.local/salt/py3/onedir: http://saltmaster01.corp.local/salt/py3/onedir
--loglevel: --loglevel
debug: debug
master: saltmaster01.corp.local
id: testminion
minion_id_caching: False
minion_id_lowercase: True
minion_id_remove_domain: True

Expected behavior

Salt minion deployed by open-vm-tools has minion configuration only with custom arguments applicable to salt-minion configuration without salt-minion setup script svtminion.sh service arguments like --minionversion

Additional context

Enable Salt Minion Using VMware Tools

@vmwkruti1111
Copy link
Contributor

For deploying the salt-minion using open-vm-tools, please note only standard configuration settings applicable to /etc/salt/minion should be configured to the guest variable.

Custom arguments like "--loglevel" , "--minionversion", "--source" should not be set/configured to the guest variable "guestinfo./vmware.components.salt_minion.args",
instead you can configure the guest var with "log_level=debug " as its a standard setting (https://docs.saltproject.io/en/latest/ref/configuration/minion.html#log-level)

you can see in the below script, for installation its fetching the config info from guestvars and info provided through command line args separately.

Code snip from installation script: https://github.com/vmware/open-vm-tools/blob/master/open-vm-tools/services/plugins/componentMgr/svtminion.sh#L726

_fetch_vmtools_salt_minion_conf() {
# fetch the current configuration for section salt_minion
# from vmtoolsd configuration file

_debug_log "$0:${FUNCNAME[0]} retrieving minion configuration parameters"
_fetch_vmtools_salt_minion_conf_guestvars || {
    _error_log "$0:${FUNCNAME[0]} failed to process guest variable "\
        "arguments, retcode '$?'";
}
...
_fetch_vmtools_salt_minion_conf_cli_args "$*" || {
    _error_log "$0:${FUNCNAME[0]} failed to process command line "\
        "arguments, retcode '$?'";
}

@stanimir-kozarev
Copy link
Author

stanimir-kozarev commented Oct 2, 2024 via email

@dmurphy18
Copy link

dmurphy18 commented Oct 17, 2024

The salt-minion configuration file is incorrect and does not conform to a salt-minion configuration file.
The salt-minion configuration file has entries, such as, --loglevel, those belong the svtminion.sh script argument parameters and do not get written to the salt-minion configuration file, typically located at /etc/salt/minion, by the script. Similarly for --source.

Can you supply the actual command line generated, that was used, this is required to duplicate the issue.
Noting that command line handing is exhaustively tested for the Linux script in CI/CD.

And as stated in #739 (comment), these are not "guestinfo./vmware.components" items, unless something has been done there to support them recently, in which case, they need to generate the correct command line to feed to the script. That is, a messed up guestinfo is not the concern of the script, especially if changes have been made there without consulting the Salt Team, as to those changes.

Note: the script version 1.6 has been in the field for over a year, 1.7 is the latest version of the script.

@twangboy
Copy link

twangboy commented Oct 17, 2024

That erroneous minion config was created by the script itself... I think that's the bug. Unless --source and --minionversion are being passed in guestVars. I guess we'd need to see what's in guestVars as well.

@stanimir-kozarev
Copy link
Author

The easiest way to reproduce the bug is by PowerShell commands to vCenter like it is described here
https://www.stevenbright.com/2022/03/deploy-salt-minions-automatically-using-vmware-tools/

The requred change for fixing this is just add 3 lines in do section of _fetch_vmtools_salt_minion_conf_guestvars() of svtminion.sh script:

        # check for start of next option, idx starts with '--' or does not contain '='
        if [[ "${idx}" = --* || "${idx}" != *"="* ]]; then
            continue
        fi

Example code change that will create a valid minion configuration and allow customization by arguments of minion binary source, version, and etc. during the minion deployment by svtminion.sh script.

_fetch_vmtools_salt_minion_conf_guestvars() {
    # fetch the current configuration for section salt_minion
    # from guest variables args

    local _retn=0
    local gvar_args=""

    gvar_args=$(vmtoolsd --cmd "info-get ${guestvars_salt_args}" 2>/dev/null)\
        || { _warning_log "$0:${FUNCNAME[0]} unable to retrieve arguments "\
            "from guest variables location ${guestvars_salt_args}, "\
            "retcode '$?'";
    }

    if [[ -z "${gvar_args}" ]]; then return ${_retn}; fi

    _debug_log "$0:${FUNCNAME[0]} processing arguments from guest variables "\
        "location ${guestvars_salt_args}"

    for idx in ${gvar_args}
    do
        # check for start of next option, idx starts with '--' or does not contain '='
        if [[ "${idx}" = --* || "${idx}" != *"="* ]]; then
            continue
        fi

        cfg_key=$(echo "${idx}" | cut -d '=' -f 1)
        cfg_value=$(echo "${idx}" | cut -d '=' -f 2)
        _update_minion_conf_ary "${cfg_key}" "${cfg_value}" || {
            _error_log "$0:${FUNCNAME[0]} error updating minion configuration "\
                "array with key '${cfg_key}' and value '${cfg_value}', "\
                "retcode '$?'";
        }
    done

    return ${_retn}
}

@dmurphy18
Copy link

dmurphy18 commented Oct 18, 2024 via email

@dmurphy18
Copy link

dmurphy18 commented Oct 18, 2024 via email

@stanimir-kozarev
Copy link
Author

The proposed change is about exactly that "the guestinfo vars was only supposed to pass salt-minion configuration". It will create minion configuration only from "key=value" pairs of minion configuration as per design and it will not allow minion misconfiguration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants