From 759203a79d7ae5250d6330bca6e8453cd3f5fc09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?= Date: Wed, 29 May 2019 11:03:16 +0100 Subject: [PATCH 1/6] Do not break repo files with multiple line values on yumpkg --- salt/modules/yumpkg.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/salt/modules/yumpkg.py b/salt/modules/yumpkg.py index c250b94f0ec0..1ce8410c2a52 100644 --- a/salt/modules/yumpkg.py +++ b/salt/modules/yumpkg.py @@ -2733,7 +2733,12 @@ def del_repo(repo, basedir=None, **kwargs): # pylint: disable=W0613 del filerepos[stanza]['comments'] content += '\n[{0}]'.format(stanza) for line in filerepos[stanza]: - content += '\n{0}={1}'.format(line, filerepos[stanza][line]) + # A whitespace is needed at the begining of the new line in order + # to avoid breaking multiple line values allowed on repo files. + value = filerepos[stanza][line] + if '\n' in value: + value = '\n '.join(value.split('\n')) + content += '\n{0}={1}'.format(line, value) content += '\n{0}\n'.format(comments) with salt.utils.files.fopen(repofile, 'w') as fileout: @@ -2868,11 +2873,14 @@ def mod_repo(repo, basedir=None, **kwargs): ) content += '[{0}]\n'.format(stanza) for line in six.iterkeys(filerepos[stanza]): + # A whitespace is needed at the begining of the new line in order + # to avoid breaking multiple line values allowed on repo files. + value = filerepos[stanza][line] + if '\n' in value: + value = '\n '.join(value.split('\n')) content += '{0}={1}\n'.format( line, - filerepos[stanza][line] - if not isinstance(filerepos[stanza][line], bool) - else _bool_to_str(filerepos[stanza][line]) + value if not isinstance(value, bool) else _bool_to_str(value) ) content += comments + '\n' From 60657ddc1315a8992c4ee1cd8db4e0787dba83fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?= Date: Wed, 29 May 2019 12:10:58 +0100 Subject: [PATCH 2/6] Fix pylint issues --- salt/modules/yumpkg.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/salt/modules/yumpkg.py b/salt/modules/yumpkg.py index 1ce8410c2a52..b2364b5e26c0 100644 --- a/salt/modules/yumpkg.py +++ b/salt/modules/yumpkg.py @@ -2737,7 +2737,7 @@ def del_repo(repo, basedir=None, **kwargs): # pylint: disable=W0613 # to avoid breaking multiple line values allowed on repo files. value = filerepos[stanza][line] if '\n' in value: - value = '\n '.join(value.split('\n')) + value = '\n '.join(value.split('\n')) content += '\n{0}={1}'.format(line, value) content += '\n{0}\n'.format(comments) @@ -2877,7 +2877,7 @@ def mod_repo(repo, basedir=None, **kwargs): # to avoid breaking multiple line values allowed on repo files. value = filerepos[stanza][line] if '\n' in value: - value = '\n '.join(value.split('\n')) + value = '\n '.join(value.split('\n')) content += '{0}={1}\n'.format( line, value if not isinstance(value, bool) else _bool_to_str(value) From 8ebc0035bcad7b14bd4ac69ca5bd1300dc58bb95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?= Date: Mon, 3 Jun 2019 12:39:08 +0100 Subject: [PATCH 3/6] Do not fail when value is not string --- salt/modules/yumpkg.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/salt/modules/yumpkg.py b/salt/modules/yumpkg.py index b2364b5e26c0..e42b83b87c21 100644 --- a/salt/modules/yumpkg.py +++ b/salt/modules/yumpkg.py @@ -2736,7 +2736,7 @@ def del_repo(repo, basedir=None, **kwargs): # pylint: disable=W0613 # A whitespace is needed at the begining of the new line in order # to avoid breaking multiple line values allowed on repo files. value = filerepos[stanza][line] - if '\n' in value: + if isinstance(value, str) and '\n' in value: value = '\n '.join(value.split('\n')) content += '\n{0}={1}'.format(line, value) content += '\n{0}\n'.format(comments) @@ -2876,7 +2876,7 @@ def mod_repo(repo, basedir=None, **kwargs): # A whitespace is needed at the begining of the new line in order # to avoid breaking multiple line values allowed on repo files. value = filerepos[stanza][line] - if '\n' in value: + if isinstance(value, str) and '\n' in value: value = '\n '.join(value.split('\n')) content += '{0}={1}\n'.format( line, From e5d1a14c330e7868ef5d707626571c1be1e0e264 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?= Date: Mon, 3 Jun 2019 13:06:34 +0100 Subject: [PATCH 4/6] Add integration test for del_mod_repo with multiline values --- tests/integration/modules/test_pkg.py | 48 +++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/integration/modules/test_pkg.py b/tests/integration/modules/test_pkg.py index dc3cb6eee897..a9ffbd3ea2da 100644 --- a/tests/integration/modules/test_pkg.py +++ b/tests/integration/modules/test_pkg.py @@ -133,6 +133,54 @@ def test_mod_del_repo(self, grains): if repo is not None: self.run_function('pkg.del_repo', [repo]) + def test_mod_del_repo_multiline_values(self): + ''' + test modifying and deleting a software repository defined with multiline values + ''' + os_grain = self.run_function('grains.item', ['os'])['os'] + repo = None + try: + if os_grain in ['CentOS', 'RedHat', 'SUSE']: + my_baseurl = 'http://my.fake.repo/foo/bar/\n http://my.fake.repo.alt/foo/bar/' + expected_get_repo_baseurl = 'http://my.fake.repo/foo/bar/\nhttp://my.fake.repo.alt/foo/bar/' + major_release = int( + self.run_function( + 'grains.item', + ['osmajorrelease'] + )['osmajorrelease'] + ) + repo = 'fakerepo' + name = 'Fake repo for RHEL/CentOS/SUSE' + baseurl = my_baseurl + gpgkey = 'https://my.fake.repo/foo/bar/MY-GPG-KEY.pub' + failovermethod = 'priority' + gpgcheck = 1 + enabled = 1 + ret = self.run_function( + 'pkg.mod_repo', + [repo], + name=name, + baseurl=baseurl, + gpgkey=gpgkey, + gpgcheck=gpgcheck, + enabled=enabled, + failovermethod=failovermethod, + ) + # return data from pkg.mod_repo contains the file modified at + # the top level, so use next(iter(ret)) to get that key + self.assertNotEqual(ret, {}) + repo_info = ret[next(iter(ret))] + self.assertIn(repo, repo_info) + self.assertEqual(repo_info[repo]['baseurl'], my_baseurl) + ret = self.run_function('pkg.get_repo', [repo]) + self.assertEqual(ret['baseurl'], expected_get_repo_baseurl) + self.run_function('pkg.mod_repo', [repo]) + ret = self.run_function('pkg.get_repo', [repo]) + self.assertEqual(ret['baseurl'], expected_get_repo_baseurl) + finally: + if repo is not None: + self.run_function('pkg.del_repo', [repo]) + @requires_salt_modules('pkg.owner') def test_owner(self): ''' From 7a178b8b2ad6ff7f90aa67bcb3f2789bb8307a3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?= Date: Wed, 5 Jun 2019 12:54:21 +0100 Subject: [PATCH 5/6] Properly check for string types --- salt/modules/yumpkg.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/salt/modules/yumpkg.py b/salt/modules/yumpkg.py index e42b83b87c21..16814c958610 100644 --- a/salt/modules/yumpkg.py +++ b/salt/modules/yumpkg.py @@ -2736,7 +2736,7 @@ def del_repo(repo, basedir=None, **kwargs): # pylint: disable=W0613 # A whitespace is needed at the begining of the new line in order # to avoid breaking multiple line values allowed on repo files. value = filerepos[stanza][line] - if isinstance(value, str) and '\n' in value: + if isinstance(value, six.string_types) and '\n' in value: value = '\n '.join(value.split('\n')) content += '\n{0}={1}'.format(line, value) content += '\n{0}\n'.format(comments) @@ -2876,7 +2876,7 @@ def mod_repo(repo, basedir=None, **kwargs): # A whitespace is needed at the begining of the new line in order # to avoid breaking multiple line values allowed on repo files. value = filerepos[stanza][line] - if isinstance(value, str) and '\n' in value: + if isinstance(value, six.string_types) and '\n' in value: value = '\n '.join(value.split('\n')) content += '{0}={1}\n'.format( line, From 761c61da9b1f161c25cf1932e687723123103931 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?= Date: Fri, 10 Jan 2020 09:22:17 +0000 Subject: [PATCH 6/6] Exclude test_mod_del_repo_multiline_values for SUSE distros --- tests/integration/modules/test_pkg.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/modules/test_pkg.py b/tests/integration/modules/test_pkg.py index 91a58e05fd9e..ab6c44d3e327 100644 --- a/tests/integration/modules/test_pkg.py +++ b/tests/integration/modules/test_pkg.py @@ -141,7 +141,7 @@ def test_mod_del_repo_multiline_values(self): os_grain = self.run_function('grains.item', ['os'])['os'] repo = None try: - if os_grain in ['CentOS', 'RedHat', 'SUSE']: + if os_grain in ['CentOS', 'RedHat']: my_baseurl = 'http://my.fake.repo/foo/bar/\n http://my.fake.repo.alt/foo/bar/' expected_get_repo_baseurl = 'http://my.fake.repo/foo/bar/\nhttp://my.fake.repo.alt/foo/bar/' major_release = int(