diff options
-rw-r--r-- | cloudinit/config/cc_apt_configure.py | 61 | ||||
-rw-r--r-- | tests/unittests/test_handler/test_handler_apt_source_v1.py | 87 |
2 files changed, 117 insertions, 31 deletions
diff --git a/cloudinit/config/cc_apt_configure.py b/cloudinit/config/cc_apt_configure.py index 609dbb51..42c56418 100644 --- a/cloudinit/config/cc_apt_configure.py +++ b/cloudinit/config/cc_apt_configure.py @@ -464,13 +464,19 @@ def convert_mirror(oldcfg, aptcfg): def convert_v2_to_v3_apt_format(oldcfg): """convert old to new keys and adapt restructured mirror spec""" - oldkeys = ['apt_sources', 'apt_mirror', 'apt_mirror_search', - 'apt_mirror_search_dns', 'apt_proxy', 'apt_http_proxy', - 'apt_ftp_proxy', 'apt_https_proxy', - 'apt_preserve_sources_list', 'apt_custom_sources_list', - 'add_apt_repo_match'] + mapoldkeys = {'apt_sources': 'sources', + 'apt_mirror': None, + 'apt_mirror_search': None, + 'apt_mirror_search_dns': None, + 'apt_proxy': 'proxy', + 'apt_http_proxy': 'http_proxy', + 'apt_ftp_proxy': 'https_proxy', + 'apt_https_proxy': 'ftp_proxy', + 'apt_preserve_sources_list': 'preserve_sources_list', + 'apt_custom_sources_list': 'sources_list', + 'add_apt_repo_match': 'add_apt_repo_match'} needtoconvert = [] - for oldkey in oldkeys: + for oldkey in mapoldkeys: if oldcfg.get(oldkey, None) is not None: needtoconvert.append(oldkey) @@ -480,32 +486,37 @@ def convert_v2_to_v3_apt_format(oldcfg): LOG.debug("apt config: convert V2 to V3 format for keys '%s'", ", ".join(needtoconvert)) - if oldcfg.get('apt', None) is not None: - msg = ("Error in apt configuration: " - "old and new format of apt features are mutually exclusive " - "('apt':'%s' vs '%s' key)" % (oldcfg.get('apt', None), - ", ".join(needtoconvert))) - LOG.error(msg) - raise ValueError(msg) + # if old AND new config are provided, prefer the new one (LP #1616831) + newaptcfg = oldcfg.get('apt', None) + if newaptcfg is not None: + LOG.debug("apt config: V1/2 and V3 format specified, preferring V3") + for oldkey in needtoconvert: + newkey = mapoldkeys[oldkey] + verify = oldcfg[oldkey] # drop, but keep a ref for verification + del oldcfg[oldkey] + if newkey is None or newaptcfg.get(newkey, None) is None: + # no simple mapping or no collision on this particular key + continue + if verify != newaptcfg[newkey]: + raise ValueError("Old and New apt format defined with unequal " + "values %s vs %s @ %s" % (verify, + newaptcfg[newkey], + oldkey)) + # return conf after clearing conflicting V1/2 keys + return oldcfg # create new format from old keys aptcfg = {} - # renames / moves under the apt key - convert_key(oldcfg, aptcfg, 'add_apt_repo_match', 'add_apt_repo_match') - convert_key(oldcfg, aptcfg, 'apt_proxy', 'proxy') - convert_key(oldcfg, aptcfg, 'apt_http_proxy', 'http_proxy') - convert_key(oldcfg, aptcfg, 'apt_https_proxy', 'https_proxy') - convert_key(oldcfg, aptcfg, 'apt_ftp_proxy', 'ftp_proxy') - convert_key(oldcfg, aptcfg, 'apt_custom_sources_list', 'sources_list') - convert_key(oldcfg, aptcfg, 'apt_preserve_sources_list', - 'preserve_sources_list') - # dict format not changed since v2, just renamed and moved - convert_key(oldcfg, aptcfg, 'apt_sources', 'sources') + # simple renames / moves under the apt key + for oldkey in mapoldkeys: + if mapoldkeys[oldkey] is not None: + convert_key(oldcfg, aptcfg, oldkey, mapoldkeys[oldkey]) + # mirrors changed in a more complex way convert_mirror(oldcfg, aptcfg) - for oldkey in oldkeys: + for oldkey in mapoldkeys: if oldcfg.get(oldkey, None) is not None: raise ValueError("old apt key '%s' left after conversion" % oldkey) diff --git a/tests/unittests/test_handler/test_handler_apt_source_v1.py b/tests/unittests/test_handler/test_handler_apt_source_v1.py index d96779c5..ddff4341 100644 --- a/tests/unittests/test_handler/test_handler_apt_source_v1.py +++ b/tests/unittests/test_handler/test_handler_apt_source_v1.py @@ -528,6 +528,7 @@ class TestAptSourceConfig(TestCase): 'filename': self.aptlistfile2} cfg3 = {'source': 'deb $MIRROR $RELEASE universe', 'filename': self.aptlistfile3} + cfg = {'apt_sources': [cfg1, cfg2, cfg3]} checkcfg = {self.aptlistfile: {'filename': self.aptlistfile, 'source': 'deb $MIRROR $RELEASE ' 'multiverse'}, @@ -537,15 +538,89 @@ class TestAptSourceConfig(TestCase): 'source': 'deb $MIRROR $RELEASE ' 'universe'}} - newcfg = cc_apt_configure.convert_v1_to_v2_apt_format([cfg1, cfg2, - cfg3]) - self.assertEqual(newcfg, checkcfg) + newcfg = cc_apt_configure.convert_to_v3_apt_format(cfg) + self.assertEqual(newcfg['apt']['sources'], checkcfg) - newcfg2 = cc_apt_configure.convert_v1_to_v2_apt_format(newcfg) - self.assertEqual(newcfg2, checkcfg) + # convert again, should stay the same + newcfg2 = cc_apt_configure.convert_to_v3_apt_format(newcfg) + self.assertEqual(newcfg2['apt']['sources'], checkcfg) + # should work without raising an exception + cc_apt_configure.convert_to_v3_apt_format({}) + + with self.assertRaises(ValueError): + cc_apt_configure.convert_to_v3_apt_format({'apt_sources': 5}) + + def test_convert_to_new_format_collision(self): + """Test the conversion of old to new format with collisions + That matches e.g. the MAAS case specifying old and new config""" + cfg_1_and_3 = {'apt': {'proxy': 'http://192.168.122.1:8000/'}, + 'apt_proxy': 'http://192.168.122.1:8000/'} + cfg_3_only = {'apt': {'proxy': 'http://192.168.122.1:8000/'}} + cfgconflict = {'apt': {'proxy': 'http://192.168.122.1:8000/'}, + 'apt_proxy': 'ftp://192.168.122.1:8000/'} + + # collision (equal) + newcfg = cc_apt_configure.convert_to_v3_apt_format(cfg_1_and_3) + self.assertEqual(newcfg, cfg_3_only) + # collision (equal, so ok to remove) + newcfg = cc_apt_configure.convert_to_v3_apt_format(cfg_3_only) + self.assertEqual(newcfg, cfg_3_only) + # collision (unequal) + with self.assertRaises(ValueError): + cc_apt_configure.convert_to_v3_apt_format(cfgconflict) + + def test_convert_to_new_format_dict_collision(self): + cfg1 = {'source': 'deb $MIRROR $RELEASE multiverse', + 'filename': self.aptlistfile} + cfg2 = {'source': 'deb $MIRROR $RELEASE main', + 'filename': self.aptlistfile2} + cfg3 = {'source': 'deb $MIRROR $RELEASE universe', + 'filename': self.aptlistfile3} + fullv3 = {self.aptlistfile: {'filename': self.aptlistfile, + 'source': 'deb $MIRROR $RELEASE ' + 'multiverse'}, + self.aptlistfile2: {'filename': self.aptlistfile2, + 'source': 'deb $MIRROR $RELEASE main'}, + self.aptlistfile3: {'filename': self.aptlistfile3, + 'source': 'deb $MIRROR $RELEASE ' + 'universe'}} + cfg_3_only = {'apt': {'sources': fullv3}} + cfg_1_and_3 = {'apt_sources': [cfg1, cfg2, cfg3]} + cfg_1_and_3.update(cfg_3_only) + + # collision (equal, so ok to remove) + newcfg = cc_apt_configure.convert_to_v3_apt_format(cfg_1_and_3) + self.assertEqual(newcfg, cfg_3_only) + # no old spec (same result) + newcfg = cc_apt_configure.convert_to_v3_apt_format(cfg_3_only) + self.assertEqual(newcfg, cfg_3_only) + + diff = {self.aptlistfile: {'filename': self.aptlistfile, + 'source': 'deb $MIRROR $RELEASE ' + 'DIFFERENTVERSE'}, + self.aptlistfile2: {'filename': self.aptlistfile2, + 'source': 'deb $MIRROR $RELEASE main'}, + self.aptlistfile3: {'filename': self.aptlistfile3, + 'source': 'deb $MIRROR $RELEASE ' + 'universe'}} + cfg_3_only = {'apt': {'sources': diff}} + cfg_1_and_3_different = {'apt_sources': [cfg1, cfg2, cfg3]} + cfg_1_and_3_different.update(cfg_3_only) + + # collision (unequal by dict having a different entry) + with self.assertRaises(ValueError): + cc_apt_configure.convert_to_v3_apt_format(cfg_1_and_3_different) + + missing = {self.aptlistfile: {'filename': self.aptlistfile, + 'source': 'deb $MIRROR $RELEASE ' + 'multiverse'}} + cfg_3_only = {'apt': {'sources': missing}} + cfg_1_and_3_missing = {'apt_sources': [cfg1, cfg2, cfg3]} + cfg_1_and_3_missing.update(cfg_3_only) + # collision (unequal by dict missing an entry) with self.assertRaises(ValueError): - cc_apt_configure.convert_v1_to_v2_apt_format(5) + cc_apt_configure.convert_to_v3_apt_format(cfg_1_and_3_missing) # vi: ts=4 expandtab |