From 5abfdfffab14b57d7011d567c0004682cc6d9c67 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?=
 <psuarezhernandez@suse.com>
Date: Wed, 29 May 2019 11:03:16 +0100
Subject: [PATCH] Do not break repo files with multiple line values on
 yumpkg (bsc#1135360)

Fix pylint issues

Do not fail when value is not string

Add integration test for del_mod_repo with multiline values

Properly check for string types
---
 salt/modules/yumpkg.py           | 16 +++++++---
 tests/integration/modules/pkg.py | 50 ++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/salt/modules/yumpkg.py b/salt/modules/yumpkg.py
index ef77e2a6bb..fdb5e3db8a 100644
--- a/salt/modules/yumpkg.py
+++ b/salt/modules/yumpkg.py
@@ -2566,7 +2566,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 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)
 
     with salt.utils.fopen(repofile, 'w') as fileout:
@@ -2702,11 +2707,14 @@ def mod_repo(repo, basedir=None, **kwargs):
             del filerepos[stanza]['comments']
         content += '\n[{0}]'.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 isinstance(value, six.string_types) and '\n' in value:
+                value = '\n '.join(value.split('\n'))
             content += '\n{0}={1}'.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 += '\n{0}\n'.format(comments)
 
diff --git a/tests/integration/modules/pkg.py b/tests/integration/modules/pkg.py
index cdda2e0957..083291f61a 100644
--- a/tests/integration/modules/pkg.py
+++ b/tests/integration/modules/pkg.py
@@ -106,6 +106,56 @@ class PkgModuleTest(integration.ModuleCase,
             if repo is not None:
                 self.run_function('pkg.del_repo', [repo])
 
+    @requires_network()
+    @destructiveTest
+    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])
+
     def test_owner(self):
         '''
         test finding the package owning a file
-- 
2.21.0


