[TECH DEBT] saltenv="base" audit

[whytewolf]

Description of the tech debt to be addressed, include links and screenshots

while working on cmdmod not taking into consideration saltenv setup in config. i ran a quick command to see just how many modules might have this same problem.

here are the results of that command

grep -r 'saltenv="base"' * | awk -F \: '{print $1}' | sort | uniq -c                                                                
      3 aptly.py
      3 aptpkg.py
      2 archive.py
      1 arista_pyeapi.py
      1 chroot.py
      8 ciscoconfparse_mod.py
      2 debconfmod.py
      4 debuild_pkgbuild.py
      5 dockermod.py
      1 dpkg_lowpkg.py
      2 file.py
      8 git.py
      8 heat.py
      7 iosconfig.py
      2 jenkinsmod.py
      1 jira_mod.py
      2 k8s.py
      4 kubernetesmod.py
      2 lxd.py
      6 napalm_mod.py
      3 napalm_network.py
      1 netmiko_mod.py
      1 nxos.py
      1 nxos_api.py
      2 pip.py
      1 pkg_resource.py
      1 rpm_lowpkg.py
      5 rpmbuild_pkgbuild.py
      1 rsync.py
      3 saltcheck.py
      1 saltutil.py
      1 scp_mod.py
      4 slsutil.py
      2 solarispkg.py
      3 ssh.py
      1 state.py
      2 textfsm_mod.py
      1 tomcat.py
      1 transactional_update.py
      3 virt.py
      2 virtualenv_mod.py
      3 win_certutil.py
      4 win_pkg.py
      1 win_pki.py
      2 winrepo.py
      1 yumpkg.py

Now it is entirely possible given the search simplicity that these are not an issue. however this is something that needs to be looked into to make sure we are not making the same mistake everywhere and that saltenv is not ignored when it is set in the config file.

Versions Report

This was run against the master branch of the gitrepo.

[OrangeDog]

You mean instead of

def _format_repo_args(
    comment=None, component=None, distribution=None, uploaders_file=None, saltenv="base"
):

it should be

def _format_repo_args(
    comment=None, component=None, distribution=None, uploaders_file=None, saltenv=None
):
  if saltenv is None:
    saltenv = __opts__.get("saltenv", "base")

?

[whytewolf]

personally i would prefer opts for this since saltenv is one of those important settings that controls how things operate globally and config.get pulls from many sources.

but in essence yes.

also need to make sure that it doesn't break by making that change. hence the audit and not all out change right now.

[OrangeDog]

Also need to check the documentation.
For example: https://docs.saltproject.io/en/latest/ref/renderers/index.html#writing-renderers
(that one also uses six)

[whytewolf]

agreed, documents should also make sure that the practice of using saltenv="base" is not seen as how it should be done.