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

[stanimir-kozarev]

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 _"saltminion"
2. Add VM configuration parameter _"guestinfo./vmware.components.saltminion.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_removedomain=True"
3. Add VM configuration parameter _"guestinfo./vmware.components.saltminion.desiredstate" with value "present"
4. Monitor VM configuration parameter _"guestinfo.vmware.components.saltminion.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]

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]

Linux and Windows implementation should match in that feature and custom arguments like "--loglevel" , "--minionversion", "--source" must be allowed like in the Windows implementation of VM Tools.

Currently custom arguments like "--minionversion" and "--source" work properly as script arguments and they must be filtered out. The rationale for this is that the PowerShell or vCenter API automation that set the attributes at VM level must be able to set minion version and custom minion binaries repo source. The Salt minion version must be controlled at deployment time and it must not be higher than the Salt masters version. The custom location must be available as option because most of the production servers or VM do not have Internet access to pull binaries from https://repo.saltproject.io. The bug can be fixed with simple filter of arguments that start with "-" or "--" in the _fetch_vmtools_salt_minion_conf().

On Wed, Oct 2, 2024 at 2:07 PM vmwkruti1111 @.***> wrote:

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 '$?'"; }

— Reply to this email directly, view it on GitHub https://github.com/vmware/open-vm-tools/issues/739#issuecomment-2388377953, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFOCIFABRBV7BV2FOOA76HTZZPHX7AVCNFSM6AAAAABPE645B6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOBYGM3TOOJVGM . You are receiving this because you authored the thread.Message ID: @.***>

[dmurphy18]

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 https://github.com/vmware/open-vm-tools/issues/739#issuecomment-2389011109, 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]

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]

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_confguestvars() 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]

I disagree, the Linux script handles '--' and '-' correctly already.

You are asking for processing of guestinfo vars which were never part of the task when originally producing the script.

Changes to values placed in the guestinfo vars from those initially defined require a change of the input specification to process and despite requests for meeting to discuss handling changes and exposing functionality in version 1.7 of the scripts, none has been forthcoming. Note 1.7 is the most current version of the script and release 1.6 of the script is being used in the bug definition.

An email thread is no place to make design decisions. The script is conformant with the stated handling of the guestinfo variables when it was created.

This reminds me of the misplaced handling of the 'source' option where the VM Tools team incorrectly handled the 'source' parameter to the script, and wanted the script to change to handle their mistake rather than correct their processing. If changes have been made to the format of parameters in guestinfo vars then they need to be documented and discussed, before any implementation is made to support those changes.

Best Regards David Murphy

On Fri, Oct 18, 2024 at 1:48 AM stanimir-kozarev @.***> wrote:

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}

}

— Reply to this email directly, view it on GitHub https://github.com/vmware/open-vm-tools/issues/739#issuecomment-2421693370, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACL3RJE7RWON7G3Z4UGIX2TZ4C4KVAVCNFSM6AAAAABPE645B6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRRGY4TGMZXGA . You are receiving this because you commented.Message ID: @.***>

-- Best Regards

David Murphy @.***

-- This electronic communication and the information and any files transmitted with it, or attached to it, are confidential and are intended solely for the use of the individual or entity to whom it is addressed and may contain information that is confidential, legally privileged, protected by privacy laws, or otherwise restricted from disclosure to anyone else. If you are not the intended recipient or the person responsible for delivering the e-mail to the intended recipient, you are hereby notified that any use, copying, distributing, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited. If you received this e-mail in error, please return the e-mail to the sender, delete it from your computer, and destroy any printed copy of it.

[dmurphy18]

One further thing, the guestinfo vars was only supposed to pass salt-minion configuration, that is, any setting that is in a typically /etc/salt/minion configuration, not items such as 'source', those are for the script CLI arguments.

Please include a sample of the guestinfo vars setting that the user is using and I quill quickly identify invalid salt-minion configuration parameters.

David

On Fri, Oct 18, 2024 at 9:20 AM David Murphy @.***> wrote:

I disagree, the Linux script handles '--' and '-' correctly already.

You are asking for processing of guestinfo vars which were never part of the task when originally producing the script.

Changes to values placed in the guestinfo vars from those initially defined require a change of the input specification to process and despite requests for meeting to discuss handling changes and exposing functionality in version 1.7 of the scripts, none has been forthcoming. Note 1.7 is the most current version of the script and release 1.6 of the script is being used in the bug definition.

An email thread is no place to make design decisions. The script is conformant with the stated handling of the guestinfo variables when it was created.

This reminds me of the misplaced handling of the 'source' option where the VM Tools team incorrectly handled the 'source' parameter to the script, and wanted the script to change to handle their mistake rather than correct their processing. If changes have been made to the format of parameters in guestinfo vars then they need to be documented and discussed, before any implementation is made to support those changes.

Best Regards David Murphy

On Fri, Oct 18, 2024 at 1:48 AM stanimir-kozarev @.***> wrote:

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}

}

— Reply to this email directly, view it on GitHub https://github.com/vmware/open-vm-tools/issues/739#issuecomment-2421693370, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACL3RJE7RWON7G3Z4UGIX2TZ4C4KVAVCNFSM6AAAAABPE645B6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRRGY4TGMZXGA . You are receiving this because you commented.Message ID: @.***>

-- Best Regards

David Murphy @.***

-- Best Regards

David Murphy @.***

-- This electronic communication and the information and any files transmitted with it, or attached to it, are confidential and are intended solely for the use of the individual or entity to whom it is addressed and may contain information that is confidential, legally privileged, protected by privacy laws, or otherwise restricted from disclosure to anyone else. If you are not the intended recipient or the person responsible for delivering the e-mail to the intended recipient, you are hereby notified that any use, copying, distributing, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited. If you received this e-mail in error, please return the e-mail to the sender, delete it from your computer, and destroy any printed copy of it.

[stanimir-kozarev]

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.