Merge ~smoser/cloud-init:bug/1621180 into cloud-init:master

[Scott Moser]

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

[Scott Moser (smoser)]

$ python ./mytest.py
before deepcopy
f1 is f: True
f1 == f: True
after deepcopy
f1 is f: False
f1 == f: True

$ cat mytest.py
#!/usr/bin/python

import copy

f = {'a': {'b': {'c': {1}}}}
f1 = f

print("before deepcopy")
print("f1 is f: %s" % (f1 is f))
print("f1 == f: %s" % (f1 == f))

f = copy.deepcopy(f)

print("after deepcopy")
print("f1 is f: %s" % (f1 is f))
print("f1 == f: %s" % (f1 == f))

[Ryan Harper (raharper)]

I need to go read some with deeper call-by-reference/value in python to un-confuse myself.

review: Approve

[Christian Ehrhardt  (paelzer)]

review: Approve

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