Normalize global git_pillar/winrepo config items

We were only normalizing the global version of a config parameter if it
had a per-remote counterpart. This was all well and good for gitfs,
which doesn't have any config values that are global-only, but
git_pillar and winrepo have config params which are global-only, and
they were therefore not being normalized.

This fixes that oversight, and also makes a few other changes in
salt.utils.gitfs to ensure that we're not pulling from the un-normalized
values from the opts dict, when we need to fall back to global defaults.

Additionally, it fixes an issue in which the `git_pillar_branch` config
item was overriding the branch set in a git_pillar remote.
This commit is contained in:
Erik Johnson 2018-02-28 13:07:11 -06:00
parent 633e1208e4
commit b1ce2501fd
No known key found for this signature in database
GPG key ID: 5E5583C437808F3F
9 changed files with 44 additions and 23 deletions

View file

@ -4852,11 +4852,10 @@ branch/tag.
winrepo_branch: winrepo
ext_pillar:
- git:
- https://mygitserver/winrepo1.git
- https://mygitserver/winrepo2.git:
- foo https://mygitserver/winrepo3.git
winrepo_remotes:
- https://mygitserver/winrepo1.git
- https://mygitserver/winrepo2.git:
- foo https://mygitserver/winrepo3.git
.. conf_master:: winrepo_ssl_verify

View file

@ -94,8 +94,8 @@ def init_git_pillar(opts):
pillar.init_remotes(
opts_dict['git'],
git_pillar.PER_REMOTE_OVERRIDES,
git_pillar.PER_REMOTE_ONLY
)
git_pillar.PER_REMOTE_ONLY,
git_pillar.GLOBAL_ONLY)
ret.append(pillar)
except FileserverConfigError:
if opts.get('git_pillar_verify_config', True):

View file

@ -504,7 +504,8 @@ class Master(SMaster):
git_pillar.init_remotes(
repo['git'],
salt.pillar.git_pillar.PER_REMOTE_OVERRIDES,
salt.pillar.git_pillar.PER_REMOTE_ONLY)
salt.pillar.git_pillar.PER_REMOTE_ONLY,
salt.pillar.git_pillar.GLOBAL_ONLY)
except FileserverConfigError as exc:
critical_errors.append(exc.strerror)
finally:

View file

@ -792,7 +792,8 @@ class Pillar(object):
git_pillar.init_remotes(
self.ext['git'],
salt.pillar.git_pillar.PER_REMOTE_OVERRIDES,
salt.pillar.git_pillar.PER_REMOTE_ONLY)
salt.pillar.git_pillar.PER_REMOTE_ONLY,
salt.pillar.git_pillar.GLOBAL_ONLY)
git_pillar.fetch_remotes()
except TypeError:
# Handle malformed ext_pillar

View file

@ -491,6 +491,7 @@ except ImportError:
PER_REMOTE_OVERRIDES = ('env', 'root', 'ssl_verify', 'refspecs')
PER_REMOTE_ONLY = ('name', 'mountpoint')
GLOBAL_ONLY = ('base', 'branch')
# Fall back to default per-remote-only. This isn't technically needed since
# salt.utils.gitfs.GitBase.init_remotes() will default to
@ -550,7 +551,11 @@ def ext_pillar(minion_id, repo, pillar_dirs):
opts['pillar_roots'] = {}
opts['__git_pillar'] = True
pillar = salt.utils.gitfs.GitPillar(opts)
pillar.init_remotes(repo, PER_REMOTE_OVERRIDES, PER_REMOTE_ONLY)
pillar.init_remotes(
repo,
PER_REMOTE_OVERRIDES,
PER_REMOTE_ONLY,
GLOBAL_ONLY)
if __opts__.get('__role') == 'minion':
# If masterless, fetch the remotes. We'll need to remove this once
# we make the minion daemon able to run standalone.

View file

@ -342,7 +342,8 @@ def clear_git_lock(role, remote=None, **kwargs):
obj.init_remotes(
ext_pillar['git'],
salt.pillar.git_pillar.PER_REMOTE_OVERRIDES,
salt.pillar.git_pillar.PER_REMOTE_ONLY)
salt.pillar.git_pillar.PER_REMOTE_ONLY,
salt.pillar.git_pillar.GLOBAL_ONLY)
git_objects.append(obj)
elif role == 'winrepo':
winrepo_dir = __opts__['winrepo_dir']
@ -357,7 +358,8 @@ def clear_git_lock(role, remote=None, **kwargs):
obj.init_remotes(
remotes,
salt.runners.winrepo.PER_REMOTE_OVERRIDES,
salt.runners.winrepo.PER_REMOTE_ONLY)
salt.runners.winrepo.PER_REMOTE_ONLY,
salt.runners.winrepo.GLOBAL_ONLY)
git_objects.append(obj)
else:
raise SaltInvocationError('Invalid role \'{0}\''.format(role))

View file

@ -87,7 +87,8 @@ def update(branch=None, repo=None):
pillar = salt.utils.gitfs.GitPillar(__opts__)
pillar.init_remotes(pillar_conf,
salt.pillar.git_pillar.PER_REMOTE_OVERRIDES,
salt.pillar.git_pillar.PER_REMOTE_ONLY)
salt.pillar.git_pillar.PER_REMOTE_ONLY,
salt.pillar.git_pillar.GLOBAL_ONLY)
for remote in pillar.remotes:
# Skip this remote if it doesn't match the search criteria
if branch is not None:

View file

@ -36,6 +36,7 @@ PER_REMOTE_OVERRIDES = ('ssl_verify', 'refspecs')
# salt.utils.gitfs.PER_REMOTE_ONLY for this value, so this is mainly for
# runners and other modules that import salt.runners.winrepo.
PER_REMOTE_ONLY = salt.utils.gitfs.PER_REMOTE_ONLY
GLOBAL_ONLY = ('branch',)
def genrepo(opts=None, fire_event=True):
@ -218,7 +219,10 @@ def update_git_repos(opts=None, clean=False, masterless=False):
try:
winrepo = salt.utils.gitfs.WinRepo(opts, base_dir)
winrepo.init_remotes(
remotes, PER_REMOTE_OVERRIDES, PER_REMOTE_ONLY)
remotes,
PER_REMOTE_OVERRIDES,
PER_REMOTE_ONLY,
GLOBAL_ONLY)
winrepo.fetch_remotes()
# Since we're not running update(), we need to manually call
# clear_old_remotes() to remove directories from remotes that

View file

@ -41,6 +41,9 @@ import salt.ext.six as six
# Optional per-remote params that can only be used on a per-remote basis, and
# thus do not have defaults in salt/config.py.
PER_REMOTE_ONLY = ('name',)
# Params which are global only and cannot be overridden for a single remote.
GLOBAL_ONLY = ()
SYMLINK_RECURSE_DEPTH = 100
# Auth support (auth params can be global or per-remote, too)
@ -307,7 +310,7 @@ class GitProvider(object):
salt.utils.url.strip_proto(saltenv_ptr['mountpoint'])
for key, val in six.iteritems(self.conf):
if key not in PER_SALTENV_PARAMS:
if key not in PER_SALTENV_PARAMS and not hasattr(self, key):
setattr(self, key, val)
for key in PER_SALTENV_PARAMS:
@ -770,13 +773,13 @@ class GitProvider(object):
'''
Resolve dynamically-set branch
'''
if self.branch == '__env__':
if self.role == 'git_pillar' and self.branch == '__env__':
target = self.opts.get('pillarenv') \
or self.opts.get('environment') \
or 'base'
return self.opts['{0}_base'.format(self.role)] \
return self.base \
if target == 'base' \
else target
else six.text_type(target)
return self.branch
def get_refspecs(self):
@ -824,7 +827,7 @@ class GitProvider(object):
try:
self.branch, self.url = self.id.split(None, 1)
except ValueError:
self.branch = self.opts['{0}_branch'.format(self.role)]
self.branch = self.conf['branch']
self.url = self.id
else:
self.url = self.id
@ -1913,7 +1916,8 @@ class GitBase(object):
self.opts['cachedir'], 'file_lists', self.role)
def init_remotes(self, remotes, per_remote_overrides,
per_remote_only=PER_REMOTE_ONLY):
per_remote_only=PER_REMOTE_ONLY,
global_only=GLOBAL_ONLY):
'''
Initialize remotes
'''
@ -1946,7 +1950,9 @@ class GitBase(object):
failhard(self.role)
per_remote_defaults = {}
for param in override_params:
global_values = set(override_params)
global_values.update(set(global_only))
for param in global_values:
key = '{0}_{1}'.format(self.role, param)
if key not in self.opts:
log.critical(
@ -2768,8 +2774,7 @@ class GitPillar(GitBase):
if repo.env:
env = repo.env
else:
base_branch = self.opts['{0}_base'.format(self.role)]
env = 'base' if repo.branch == base_branch else repo.branch
env = 'base' if repo.branch == repo.base else repo.branch
if repo._mountpoint:
if self.link_mountpoint(repo, cachedir):
self.pillar_dirs[repo.linkdir] = env
@ -2856,6 +2861,9 @@ class WinRepo(GitBase):
def __init__(self, opts, winrepo_dir):
self.role = 'winrepo'
super(WinRepo, self).__init__(opts, cache_root=winrepo_dir)
# Need to define this in case we try to reference it before checking
# out the repos.
self.winrepo_dirs = {}
def checkout(self):
'''