cloud-init-dev team mailing list archive

[Scott Moser <smoser@xxxxxxxxxx>]

Scott Moser has proposed merging ~smoser/cloud-init:bug/1621180 into cloud-init:master.

Commit message:
apt config conversion: treat empty string as not provided.

Old behavior allowed a user to provide:
  apt_mirror: ""

And that was the same as:
  apt_mirror: null

and the same as having not specified apt_mirror at all.  This maintains
that behavior for all old string values.

LP: #1621180

Requested reviews:
  cloud init development team (cloud-init-dev)
Related bugs:
  Bug #1621180 in cloud-init: "specifying apt_mirror of '' renders empty entries in /etc/apt/sources.list for uri"
  https://bugs.launchpad.net/cloud-init/+bug/1621180

For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/305137
-- 
Your team cloud init development team is requested to review the proposed merge of ~smoser/cloud-init:bug/1621180 into cloud-init:master.

diff --git a/cloudinit/config/cc_apt_configure.py b/cloudinit/config/cc_apt_configure.py
index 42c5641..76b8d64 100644
--- a/cloudinit/config/cc_apt_configure.py
+++ b/cloudinit/config/cc_apt_configure.py
@@ -18,6 +18,7 @@
 #    You should have received a copy of the GNU General Public License
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import copy
 import glob
 import os
 import re
@@ -476,9 +477,13 @@ def convert_v2_to_v3_apt_format(oldcfg):
                   'apt_custom_sources_list': 'sources_list',
                   'add_apt_repo_match': 'add_apt_repo_match'}
     needtoconvert = []
+    oldcfg = copy.deepcopy(oldcfg)
     for oldkey in mapoldkeys:
-        if oldcfg.get(oldkey, None) is not None:
-            needtoconvert.append(oldkey)
+        if oldkey in oldcfg:
+            if oldcfg[oldkey] in (None, ""):
+                del oldcfg[oldkey]
+            else:
+                needtoconvert.append(oldkey)
 
     # no old config, so no new one to be created
     if not needtoconvert:
diff --git a/tests/unittests/test_handler/test_handler_apt_conf_v1.py b/tests/unittests/test_handler/test_handler_apt_conf_v1.py
index 95fd1da..45714ef 100644
--- a/tests/unittests/test_handler/test_handler_apt_conf_v1.py
+++ b/tests/unittests/test_handler/test_handler_apt_conf_v1.py
@@ -3,6 +3,7 @@ from cloudinit import util
 
 from ..helpers import TestCase
 
+import copy
 import os
 import re
 import shutil
@@ -106,4 +107,25 @@ class TestAptProxyConfig(TestCase):
         self.assertFalse(os.path.isfile(self.cfile))
 
 
+class TestConversion(TestCase):
+    def test_convert_with_apt_mirror_as_empty_string(self):
+        # an empty apt_mirror is the same as no apt_mirror
+        empty_m_found = cc_apt_configure.convert_to_v3_apt_format(
+            {'apt_mirror': ''})
+        default_found = cc_apt_configure.convert_to_v3_apt_format({})
+        self.assertEqual(default_found, empty_m_found)
+
+    def test_convert_with_apt_mirror(self):
+        mirror = 'http://my.mirror/ubuntu'
+        f = cc_apt_configure.convert_to_v3_apt_format({'apt_mirror': mirror})
+        self.assertIn(mirror, {m['uri'] for m in f['apt']['primary']})
+
+    def test_no_old_content(self):
+        mirror = 'http://my.mirror/ubuntu'
+        mydata = {'apt': {'primary': {'arches': ['default'], 'uri': mirror}}}
+        expected = copy.deepcopy(mydata)
+        self.assertEqual(expected,
+                         cc_apt_configure.convert_to_v3_apt_format(mydata))
+
+
 # vi: ts=4 expandtab