mirror of
https://github.com/saltstack/salt.git
synced 2025-04-17 10:10:20 +00:00
Better handling of enabled/disabled arguments in pkgrepo.managed (#39251)
This reverses the decision made in #35055 to deprecate the "enabled" parameter in this state. This was a poorly-conceived decision, likely made because the "enabled" param was not included in the docstring for the pkgrepo.managed state under the "YUM/ZYPPER" section, while the "disabled" param *was* listed under the "APT" section. "disabled" isn't a thing in yum/dnf, so there was never any reason to try to shoehorn it in. Not to mention the fact that the "disabled" argument isn't even supported in Zypper, which effectively breaks Zypper support. This commit normalizes enabled/disabled based on platform, so passing "enabled" in APT will pass the opposite value as "disabled", and vice-versa on the other platforms. This allows enabled/disabled to be used interchangeably. It also more gracefully handles booleans in yum/dnf/zypper, so that a bool can be properly compared to a 1/0 value.
This commit is contained in:
Erik Johnson
Nicole Thomas
parent
8e88f71dd9
commit
c1d16cc3d0
2 changed files with 72 additions and 48 deletions
|
|
@ -2265,18 +2265,12 @@ def mod_repo(repo, basedir=None, **kwargs):
|
|||
)
|
||||
|
||||
# Build a list of keys to be deleted
|
||||
todelete = ['disabled']
|
||||
todelete = []
|
||||
for key in repo_opts:
|
||||
if repo_opts[key] != 0 and not repo_opts[key]:
|
||||
del repo_opts[key]
|
||||
todelete.append(key)
|
||||
|
||||
# convert disabled to enabled respectively from pkgrepo state
|
||||
if 'enabled' not in repo_opts:
|
||||
repo_opts['enabled'] = int(str(repo_opts.pop('disabled', False)).lower() != 'true')
|
||||
else:
|
||||
repo_opts.pop('disabled', False)
|
||||
|
||||
# Add baseurl or mirrorlist to the 'todelete' list if the other was
|
||||
# specified in the repo_opts
|
||||
if 'mirrorlist' in repo_opts:
|
||||
|
|
@ -2348,6 +2342,7 @@ def mod_repo(repo, basedir=None, **kwargs):
|
|||
if key in six.iterkeys(filerepos[repo].copy()):
|
||||
del filerepos[repo][key]
|
||||
|
||||
_bool_to_str = lambda x: '1' if x else '0'
|
||||
# Old file or new, write out the repos(s)
|
||||
filerepos[repo].update(repo_opts)
|
||||
content = header
|
||||
|
|
@ -2358,7 +2353,12 @@ def mod_repo(repo, basedir=None, **kwargs):
|
|||
del filerepos[stanza]['comments']
|
||||
content += '\n[{0}]'.format(stanza)
|
||||
for line in six.iterkeys(filerepos[stanza]):
|
||||
content += '\n{0}={1}'.format(line, filerepos[stanza][line])
|
||||
content += '\n{0}={1}'.format(
|
||||
line,
|
||||
filerepos[stanza][line]
|
||||
if not isinstance(filerepos[stanza][line], bool)
|
||||
else _bool_to_str(filerepos[stanza][line])
|
||||
)
|
||||
content += '\n{0}\n'.format(comments)
|
||||
|
||||
with salt.utils.fopen(repofile, 'w') as fileout:
|
||||
|
|
@ -2404,9 +2404,6 @@ def _parse_repo_file(filename):
|
|||
'Failed to parse line in %s, offending line was '
|
||||
'\'%s\'', filename, line.rstrip()
|
||||
)
|
||||
# YUM uses enabled field - create the disabled field so that comparisons works correctly in state
|
||||
if comps[0].strip() == 'enabled':
|
||||
repos[repo]['disabled'] = comps[1] != "1"
|
||||
|
||||
return (header, repos)
|
||||
|
||||
|
|
|
|||
|
|
@ -112,7 +112,7 @@ def managed(name, ppa=None, **kwargs):
|
|||
<salt.modules.yumpkg>`, :mod:`apt <salt.modules.aptpkg>`, and :mod:`zypper
|
||||
<salt.modules.zypper>` repositories are supported.
|
||||
|
||||
**YUM OR ZYPPER-BASED SYSTEMS**
|
||||
**YUM/DNF/ZYPPER-BASED SYSTEMS**
|
||||
|
||||
.. note::
|
||||
One of ``baseurl`` or ``mirrorlist`` below is required. Additionally,
|
||||
|
|
@ -126,6 +126,16 @@ def managed(name, ppa=None, **kwargs):
|
|||
repo. Secondly, it will be the name of the file as stored in
|
||||
/etc/yum.repos.d (e.g. ``/etc/yum.repos.d/foo.conf``).
|
||||
|
||||
enabled : True
|
||||
Whether or not the repo is enabled. Can be specified as True/False or
|
||||
1/0.
|
||||
|
||||
disabled : False
|
||||
Included to reduce confusion due to APT's use of the ``disabled``
|
||||
argument. If this is passed for a yum/dnf/zypper-based distro, then the
|
||||
reverse will be passed as ``enabled``. For example passing
|
||||
``disabled=True`` will assume ``enabled=False``.
|
||||
|
||||
humanname
|
||||
This is used as the "name" value in the repo file in
|
||||
``/etc/yum.repos.d/`` (or ``/etc/zypp/repos.d`` for Suse distros).
|
||||
|
|
@ -203,10 +213,16 @@ def managed(name, ppa=None, **kwargs):
|
|||
'deb http://us.archive.ubuntu.com/ubuntu precise main':
|
||||
pkgrepo.managed
|
||||
|
||||
disabled
|
||||
disabled : False
|
||||
Toggles whether or not the repo is used for resolving dependencies
|
||||
and/or installing packages.
|
||||
|
||||
enabled : True
|
||||
Included to reduce confusion due to yum/dnf/zypper's use of the
|
||||
``enabled`` argument. If this is passed for an APT-based distro, then
|
||||
the reverse will be passed as ``disabled``. For example, passing
|
||||
``enabled=False`` will assume ``disabled=False``.
|
||||
|
||||
comps
|
||||
On apt-based systems, comps dictate the types of packages to be
|
||||
installed from the repository (e.g. main, nonfree, ...). For
|
||||
|
|
@ -281,14 +297,19 @@ def managed(name, ppa=None, **kwargs):
|
|||
'intended.')
|
||||
return ret
|
||||
|
||||
if 'enabled' in kwargs:
|
||||
salt.utils.warn_until(
|
||||
'Nitrogen',
|
||||
'The `enabled` argument has been deprecated in favor of '
|
||||
'`disabled`.'
|
||||
)
|
||||
enabled = kwargs.pop('enabled', None)
|
||||
disabled = kwargs.pop('disabled', None)
|
||||
|
||||
if enabled is not None and disabled is not None:
|
||||
ret['result'] = False
|
||||
ret['comment'] = 'Only one of enabled/disabled is allowed'
|
||||
return ret
|
||||
elif enabled is None and disabled is None:
|
||||
# If neither argument was passed we assume the repo will be enabled
|
||||
enabled = True
|
||||
|
||||
repo = name
|
||||
os_family = __grains__['os_family'].lower()
|
||||
if __grains__['os'] in ('Ubuntu', 'Mint'):
|
||||
if ppa is not None:
|
||||
# overload the name/repo value for PPAs cleanly
|
||||
|
|
@ -298,26 +319,26 @@ def managed(name, ppa=None, **kwargs):
|
|||
except TypeError:
|
||||
repo = ':'.join(('ppa', str(ppa)))
|
||||
|
||||
elif __grains__['os_family'].lower() in ('redhat', 'suse'):
|
||||
kwargs['disabled'] = not salt.utils.is_true(enabled) \
|
||||
if enabled is not None \
|
||||
else salt.utils.is_true(disabled)
|
||||
|
||||
elif os_family in ('redhat', 'suse'):
|
||||
if 'humanname' in kwargs:
|
||||
kwargs['name'] = kwargs.pop('humanname')
|
||||
_val = lambda x: '1' if salt.utils.is_true(x) else '0'
|
||||
if 'disabled' in kwargs:
|
||||
if 'enabled' in kwargs:
|
||||
ret['result'] = False
|
||||
ret['comment'] = 'Only one of enabled/disabled is permitted'
|
||||
return ret
|
||||
_reverse = lambda x: '1' if x == '0' else '0'
|
||||
kwargs['enabled'] = _reverse(_val(kwargs.pop('disabled')))
|
||||
elif 'enabled' in kwargs:
|
||||
kwargs['enabled'] = _val(kwargs['enabled'])
|
||||
if 'name' not in kwargs:
|
||||
# Fall back to the repo name if humanname not provided
|
||||
kwargs['name'] = repo
|
||||
|
||||
# Replace 'enabled' from kwargs with 'disabled'
|
||||
enabled = kwargs.pop('enabled', True)
|
||||
kwargs['disabled'] = not salt.utils.is_true(enabled)
|
||||
kwargs['enabled'] = not salt.utils.is_true(disabled) \
|
||||
if disabled is not None \
|
||||
else salt.utils.is_true(enabled)
|
||||
|
||||
elif os_family == 'nilinuxrt':
|
||||
# opkg is the pkg virtual
|
||||
kwargs['enabled'] = not salt.utils.is_true(disabled) \
|
||||
if disabled is not None \
|
||||
else salt.utils.is_true(enabled)
|
||||
|
||||
for kwarg in _STATE_INTERNAL_KEYWORDS:
|
||||
kwargs.pop(kwarg, None)
|
||||
|
|
@ -342,11 +363,10 @@ def managed(name, ppa=None, **kwargs):
|
|||
else:
|
||||
sanitizedkwargs = kwargs
|
||||
|
||||
if __grains__['os_family'] == 'Debian':
|
||||
if os_family == 'debian':
|
||||
repo = _strip_uri(repo)
|
||||
|
||||
if pre:
|
||||
needs_update = False
|
||||
for kwarg in sanitizedkwargs:
|
||||
if kwarg not in pre:
|
||||
if kwarg == 'enabled':
|
||||
|
|
@ -354,33 +374,40 @@ def managed(name, ppa=None, **kwargs):
|
|||
# not explicitly set, so we don't need to update the repo
|
||||
# if it's desired to be enabled and the 'enabled' key is
|
||||
# missing from the repo definition
|
||||
if __grains__['os_family'] == 'RedHat':
|
||||
if os_family == 'redhat':
|
||||
if not salt.utils.is_true(sanitizedkwargs[kwarg]):
|
||||
needs_update = True
|
||||
break
|
||||
else:
|
||||
needs_update = True
|
||||
break
|
||||
else:
|
||||
needs_update = True
|
||||
break
|
||||
elif kwarg == 'comps':
|
||||
if sorted(sanitizedkwargs[kwarg]) != sorted(pre[kwarg]):
|
||||
needs_update = True
|
||||
elif kwarg == 'line' and __grains__['os_family'] == 'Debian':
|
||||
break
|
||||
elif kwarg == 'line' and os_family == 'debian':
|
||||
# split the line and sort everything after the URL
|
||||
sanitizedsplit = sanitizedkwargs[kwarg].split()
|
||||
sanitizedsplit[3:] = sorted(sanitizedsplit[3:])
|
||||
reposplit = pre[kwarg].split()
|
||||
reposplit[3:] = sorted(reposplit[3:])
|
||||
if sanitizedsplit != reposplit:
|
||||
needs_update = True
|
||||
break
|
||||
if 'comments' in kwargs:
|
||||
_line = pre[kwarg].split('#')
|
||||
if str(kwargs['comments']) not in _line:
|
||||
needs_update = True
|
||||
break
|
||||
else:
|
||||
if str(sanitizedkwargs[kwarg]) != str(pre[kwarg]):
|
||||
needs_update = True
|
||||
|
||||
if not needs_update:
|
||||
if os_family in ('redhat', 'suse') \
|
||||
and any(isinstance(x, bool) for x in
|
||||
(sanitizedkwargs[kwarg], pre[kwarg])):
|
||||
# This check disambiguates 1/0 from True/False
|
||||
if salt.utils.is_true(sanitizedkwargs[kwarg]) != \
|
||||
salt.utils.is_true(pre[kwarg]):
|
||||
break
|
||||
else:
|
||||
if str(sanitizedkwargs[kwarg]) != str(pre[kwarg]):
|
||||
break
|
||||
else:
|
||||
ret['result'] = True
|
||||
ret['comment'] = ('Package repo \'{0}\' already configured'
|
||||
.format(name))
|
||||
|
|
@ -401,7 +428,7 @@ def managed(name, ppa=None, **kwargs):
|
|||
pass
|
||||
|
||||
try:
|
||||
if __grains__['os_family'] == 'Debian':
|
||||
if os_family == 'debian':
|
||||
__salt__['pkg.mod_repo'](repo, saltenv=__env__, **kwargs)
|
||||
else:
|
||||
__salt__['pkg.mod_repo'](repo, **kwargs)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue