Merge pull request #50921 from terminalmage/issue49894

Add support for architectures for APT pkgrepo.managed states

salt/modules/aptpkg.py

@@ -1582,7 +1582,7 @@ def _split_repo_str(repo):
     Return APT source entry as a tuple.
     '''
     split = sourceslist.SourceEntry(repo)
-    return split.type, split.uri, split.dist, split.comps
+    return split.type, split.architectures, split.uri, split.dist, split.comps
 
 
 def _consolidate_repo_sources(sources):
@@ -1771,7 +1771,7 @@ def get_repo(repo, **kwargs):
 
     if repos:
         try:
-            repo_type, repo_uri, repo_dist, repo_comps = _split_repo_str(repo)
+            repo_type, repo_architectures, repo_uri, repo_dist, repo_comps = _split_repo_str(repo)
             if ppa_auth:
                 uri_match = re.search('(http[s]?://)(.+)', repo_uri)
                 if uri_match:
@@ -1844,7 +1844,11 @@ def del_repo(repo, **kwargs):
     if repos:
         deleted_from = dict()
         try:
-            repo_type, repo_uri, repo_dist, repo_comps = _split_repo_str(repo)
+            repo_type, \
+                repo_architectures, \
+                repo_uri, \
+                repo_dist, \
+                repo_comps = _split_repo_str(repo)
         except SyntaxError:
             raise SaltInvocationError(
                 'Error: repo \'{0}\' not a well formatted definition'
@@ -1852,8 +1856,10 @@ def del_repo(repo, **kwargs):
             )
 
         for source in repos:
-            if (source.type == repo_type and source.uri == repo_uri and
-                    source.dist == repo_dist):
+            if (source.type == repo_type
+                    and source.architectures == repo_architectures
+                    and source.uri == repo_uri
+                    and source.dist == repo_dist):
 
                 s_comps = set(source.comps)
                 r_comps = set(repo_comps)
@@ -2319,7 +2325,11 @@ def mod_repo(repo, saltenv='base', **kwargs):
     repos = [s for s in sources if not s.invalid]
     mod_source = None
     try:
-        repo_type, repo_uri, repo_dist, repo_comps = _split_repo_str(repo)
+        repo_type, \
+            repo_architectures, \
+            repo_uri, \
+            repo_dist, \
+            repo_comps = _split_repo_str(repo)
     except SyntaxError:
         raise SyntaxError(
             'Error: repo \'{0}\' not a well formatted definition'.format(repo)
@@ -2387,6 +2397,8 @@ def mod_repo(repo, saltenv='base', **kwargs):
 
     if 'architectures' in kwargs:
         kwargs['architectures'] = kwargs['architectures'].split(',')
+    else:
+        kwargs['architectures'] = repo_architectures
 
     if 'disabled' in kwargs:
         kwargs['disabled'] = salt.utils.data.is_true(kwargs['disabled'])
@@ -2408,6 +2420,8 @@ def mod_repo(repo, saltenv='base', **kwargs):
                     mod_source = source
             if not source.comps:
                 mod_source = source
+            if kwargs['architectures'] != source.architectures:
+                mod_source = source
             if mod_source:
                 break
 

salt/states/pkgrepo.py

@@ -457,6 +457,9 @@ def managed(name, ppa=None, **kwargs):
                         sanitizedkwargs[kwarg])
                 if precomments != kwargcomments:
                     break
+            elif kwarg == 'architectures' and sanitizedkwargs[kwarg]:
+                if set(sanitizedkwargs[kwarg]) != set(pre[kwarg]):
+                    break
             else:
                 if __grains__['os_family'] in ('RedHat', 'Suse') \
                         and any(isinstance(x, bool) for x in
@@ -476,11 +479,18 @@ def managed(name, ppa=None, **kwargs):
 
     if __opts__['test']:
         ret['comment'] = (
-            'Package repo \'{0}\' will be configured. This may cause pkg '
+            'Package repo \'{0}\' would be configured. This may cause pkg '
             'states to behave differently than stated if this action is '
             'repeated without test=True, due to the differences in the '
             'configured repositories.'.format(name)
         )
+        if pre:
+            for kwarg in sanitizedkwargs:
+                if sanitizedkwargs.get(kwarg) != pre.get(kwarg):
+                    ret['changes'][kwarg] = {'new': sanitizedkwargs.get(kwarg),
+                                             'old': pre.get(kwarg)}
+        else:
+            ret['changes']['repo'] = name
         return ret
 
     # empty file before configure
@@ -509,9 +519,8 @@ def managed(name, ppa=None, **kwargs):
         if pre:
             for kwarg in sanitizedkwargs:
                 if post.get(kwarg) != pre.get(kwarg):
-                    change = {'new': post[kwarg],
-                              'old': pre.get(kwarg)}
-                    ret['changes'][kwarg] = change
+                    ret['changes'][kwarg] = {'new': post.get(kwarg),
+                                             'old': pre.get(kwarg)}
         else:
             ret['changes'] = {'repo': repo}
 

tests/integration/states/test_pkgrepo.py

@@ -5,6 +5,7 @@ tests for pkgrepo states
 
 # Import Python libs
 from __future__ import absolute_import, print_function, unicode_literals
+import os
 
 # Import Salt Testing libs
 from tests.support.case import ModuleCase
@@ -12,7 +13,7 @@ from tests.support.mixins import SaltReturnAssertsMixin
 from tests.support.unit import skipIf
 from tests.support.helpers import (
     destructiveTest,
-    requires_system_grains
+    requires_system_grains,
 )
 
 # Import Salt libs
@@ -128,3 +129,137 @@ class PkgrepoTest(ModuleCase, SaltReturnAssertsMixin):
         finally:
             # Clean up
             self.run_state('pkgrepo.absent', name=kwargs['name'])
+
+    @requires_system_grains
+    def test_pkgrepo_04_apt_with_architectures(self, grains):
+        '''
+        Test managing a repo with architectures specified
+        '''
+        if grains['os_family'].lower() != 'debian':
+            self.skipTest('APT-only test')
+
+        name = 'deb {{arch}}http://foo.com/bar/latest {oscodename} main'.format(oscodename=grains['oscodename'])
+
+        def _get_arch(arch):
+            return '[arch={0}] '.format(arch) if arch else ''
+
+        def _run(arch='', test=False):
+            ret = self.run_state(
+                'pkgrepo.managed',
+                name=name.format(arch=_get_arch(arch)),
+                file=fn_,
+                refresh=False,
+                test=test)
+            return ret[next(iter(ret))]
+
+        fn_ = salt.utils.files.mkstemp(dir='/etc/apt/sources.list.d', suffix='.list')
+
+        try:
+            # Run with test=True
+            ret = _run(test=True)
+            assert ret['changes'] == {'repo': name.format(arch='')}, ret['changes']
+            assert 'would be' in ret['comment'], ret['comment']
+            assert ret['result'] is None, ret['result']
+
+            # Run for real
+            ret = _run()
+            assert ret['changes'] == {'repo': name.format(arch='')}, ret['changes']
+            assert ret['comment'].startswith('Configured'), ret['comment']
+            assert ret['result'] is True, ret['result']
+
+            # Run again with test=True, should exit with no changes and a True
+            # result.
+            ret = _run(test=True)
+            assert not ret['changes'], ret['changes']
+            assert 'already' in ret['comment'], ret['comment']
+            assert ret['result'] is True, ret['result']
+
+            # Run for real again, results should be the same as above (i.e. we
+            # should never get to the point where we exit with a None result).
+            ret = _run()
+            assert not ret['changes'], ret['changes']
+            assert 'already' in ret['comment'], ret['comment']
+            assert ret['result'] is True, ret['result']
+
+            expected_changes = {
+                'line': {
+                    'new': name.format(arch=_get_arch('amd64')),
+                    'old': name.format(arch=''),
+                },
+                'architectures': {
+                    'new': ['amd64'],
+                    'old': [],
+                },
+            }
+
+            # Run with test=True and the architecture set. We should get a None
+            # result with some expected changes.
+            ret = _run(arch='amd64', test=True)
+            assert ret['changes'] == expected_changes, ret['changes']
+            assert 'would be' in ret['comment'], ret['comment']
+            assert ret['result'] is None, ret['result']
+
+            # Run for real, with the architecture set. We should get a True
+            # result with the same changes.
+            ret = _run(arch='amd64')
+            assert ret['changes'] == expected_changes, ret['changes']
+            assert ret['comment'].startswith('Configured'), ret['comment']
+            assert ret['result'] is True, ret['result']
+
+            # Run again with test=True, should exit with no changes and a True
+            # result.
+            ret = _run(arch='amd64', test=True)
+            assert not ret['changes'], ret['changes']
+            assert 'already' in ret['comment'], ret['comment']
+            assert ret['result'] is True, ret['result']
+
+            # Run for real again, results should be the same as above (i.e. we
+            # should never get to the point where we exit with a None result).
+            ret = _run(arch='amd64')
+            assert not ret['changes'], ret['changes']
+            assert 'already' in ret['comment'], ret['comment']
+            assert ret['result'] is True, ret['result']
+
+            expected_changes = {
+                'line': {
+                    'new': name.format(arch=''),
+                    'old': name.format(arch=_get_arch('amd64')),
+                },
+                'architectures': {
+                    'new': [],
+                    'old': ['amd64'],
+                },
+            }
+
+            # Run with test=True and the architecture set back to the original
+            # value. We should get a None result with some expected changes.
+            ret = _run(test=True)
+            assert ret['changes'] == expected_changes, ret['changes']
+            assert 'would be' in ret['comment'], ret['comment']
+            assert ret['result'] is None, ret['result']
+
+            # Run for real, with the architecture set. We should get a True
+            # result with the same changes.
+            ret = _run()
+            assert ret['changes'] == expected_changes, ret['changes']
+            assert ret['comment'].startswith('Configured'), ret['comment']
+            assert ret['result'] is True, ret['result']
+
+            # Run again with test=True, should exit with no changes and a True
+            # result.
+            ret = _run(test=True)
+            assert not ret['changes'], ret['changes']
+            assert 'already' in ret['comment'], ret['comment']
+            assert ret['result'] is True, ret['result']
+
+            # Run for real again, results should be the same as above (i.e. we
+            # should never get to the point where we exit with a None result).
+            ret = _run()
+            assert not ret['changes'], ret['changes']
+            assert 'already' in ret['comment'], ret['comment']
+            assert ret['result'] is True, ret['result']
+        finally:
+            try:
+                os.remove(fn_)
+            except OSError:
+                pass