saltstack / saltstack/salt

[BUG] modules fail to load silently when there is a conflict with an extension

Open
#62,003 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

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

Description

Description

From here; https://github.com/saltstack/salt-ext-modules-vmware/issues/259#issuecomment-1113347370

The vmware sddc modules implement grains and execution modules with the same names as files inside of the core salt project. Jobs fail, depending on the type of minion you use (std vs proxy).

Setup

on your salt master...

Install the sddc modules and do some basic setup

pip3 install saltext.vmware
mkdir -p /srv/salt /srv/pillar

Configure the saltmaster to communicate with vsphere (new-style modules)

cat <<EOF > /srv/salt/vmware_config.sls
vmware_config:
  host: ****vcenterhostname*****
  password: *********
  user: ***********
  verify_ssl: False
EOF

Configure the saltmaster to run some salt-proxy processes for the esxi hosts to run the legacy modules...

# add some pillar data to the saltmaster with details about the esxi connections/environment
cat <<EOF > /srv/pillar/esxi-connections.sls
esxi-connections:
  cluster1:
    user: root
    pass: ************
    hosts:
      - esx01.domain.tld
      - esx02.domain.tld
      - esx03.domain.tld
      - esx04.domain.tld
      - esx05.domain.tld
EOF

Add the pillar data to the top file...

cat <<EOF > /srv/pillar/top.sls
base:
  'saltmaster':
    - vmware_config
    - esxi-proxies

  # Dynamically load the proxy minion configs from the files in /srv/pillar/proxy
  {% set minions = salt.file.find('/srv/pillar/proxy/*.sls') %}
  {% for minion in minions %}
    {% set minion_id = minion.split('/')[-1].replace('.sls','') %}
    {% set pillarfile = '.'.join(['proxy', minion_id]) %}

  {{ minion_id }}:
    - {{ pillarfile }}
  {% endfor %}
EOF

Create a simply state file to manage the proxy configs...

cat <<EOF > /srv/salt/proxy/configure.sls
ensure proxy master configured:
  file.managed:
    - name: /etc/salt/proxy
    - contents: "master: 127.0.0.1"
    - create: True
    - overwrite: True
    - makedirs: True
    - user: root
    - mode: 0644

{% set config = pillar.get('esxi-connections', {}) %}
{% for cluster in config %}
  {% for host in config[cluster].hosts %}
  {% set shorthost = host.split(".")[0] %}
  {%- set minion_id = '_'.join(['esxi', cluster, shorthost]) %}

proxy configured for {{ minion_id }}:
  file.serialize:
    - name: /srv/pillar/proxy/{{ minion_id }}.sls
    - serializer: yaml
    - dataset: { veryify_ssl: False, proxy: { verify_ssl: False, proxytype: esxi, host: {{host}}, username: {{ config[cluster].user }}, passwords: [ "{{ config[cluster].pass }}" ] } }

  {%- endfor %}
{% endfor %}
EOF

Configure the proxies, start them, accept their keys...

salt-call state.apply proxy.configure
salt saltmaster saltutil.refresh_pillar

for x in $(ls /srv/pillar/proxy/esxi*.sls | sed -e 's/.sls//g'); do
  salt-proxy --proxyid `basename $x` -d
done

salt-key -yA

# for now, shut everything down....
pkill salt-proxy

Obviously... you'll need a vSphere environment with root access to be able to test this...

Steps to Reproduce the behavior

  1. Set your environment up per the above
  2. Using new-style esxi code: salt saltmaster vmware_esxi.power_state; this will work
  3. Try starting your salt proxies
for x in $(ls /srv/pillar/proxy/esxi*.sls | sed -e 's/.sls//g'); do 
  salt-proxy --proxyid `basename $x` -d; 
done
  1. The above fails so apply the patch from https://github.com/saltstack/salt-ext-modules-vmware/issues/259#issuecomment-1113330577

If you go through the "Long Version", you'll see that the proxy doesn't load the legacy esxi still module.

Expected behavior
I'm not really sure what we should do here. Per this comment, https://github.com/saltstack/salt-ext-modules-vmware/issues/259#issuecomment-1113347370, I think we should do something - but I'm not sure what. I'm a big believer in the idea that a framework/interface should protect the users against programmer error and that's clearly what's happened here.

You could argue that we could still allow duplicate module names and simply change the SDDC module to not load if it's a proxy minion and vice versa if it's a normal minion. The problem is that when someone DOES create a saltext in the future, and it conflicts with some core code (because they want to make it better) we'll see silent failures and unstable behaviors.

We're still dealing with the whole module.run vs module.xrun fiasco, years later. I'd like to see similar situations avoided in the future.

Screenshots
If applicable, add screenshots to help explain your problem.

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)
PASTE HERE

Additional context
Add any other context about the problem here.

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 by reproducing the conflict with the salt-proxy commands and the saltext.vmware setup described in the issue, then trace how proxy and standard-minion modules are loaded. Compare the core Salt modules with the conflicting extension modules referenced in the report. Done should include a decided behavior for duplicate module names and verification that conflicts no longer fail silently.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, infrastructure
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.