Merge pull request #813 from Forst/merge-config

Add support for merging lists when merging user-data

subiquity/common/resources.py

@@ -33,7 +33,8 @@ def get_users_and_groups(chroot_prefix=[]):
     users_and_groups_path = resource_path('users-and-groups')
     groups = ['admin']
     if os.path.exists(users_and_groups_path):
-        groups = open(users_and_groups_path).read().split()
+        with open(users_and_groups_path) as f:
+            groups = f.read().split()
     groups.append('sudo')
 
     command = chroot_prefix + ['getent', 'group']

subiquity/models/subiquity.py

@@ -14,7 +14,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import asyncio
-import copy
 from collections import OrderedDict
 import functools
 import logging
@@ -49,6 +48,27 @@ from .updates import UpdatesModel
 log = logging.getLogger('subiquity.models.subiquity')
 
 
+def merge_cloud_init_config(target, source):
+    # type: (dict, dict) -> None
+    """
+    Merges the ``source`` dictionary into the ``target`` dictionary:
+
+    * If both items are dictionaries, they are merged recursively.
+    * If both items are lists, contents of the source list are appended
+    to the target list.
+    * Otherwise, the source item overwrites the target item.
+
+    Based on the ``curtin.config.merge_config`` function.
+    """
+    for k, v in source.items():
+        if isinstance(v, dict) and isinstance(target.get(k, None), dict):
+            merge_cloud_init_config(target[k], v)
+        elif isinstance(v, list) and isinstance(target.get(k, None), list):
+            target[k].extend(v)
+        else:
+            target[k] = v
+
+
 def setup_yaml():
     """ http://stackoverflow.com/a/8661021 """
     represent_dict_order = (
@@ -249,9 +269,8 @@ class SubiquityModel:
             model = getattr(self, model_name)
             if getattr(model, 'make_cloudconfig', None):
                 merge_config(config, model.make_cloudconfig())
-        userdata = copy.deepcopy(self.userdata)
-        merge_config(userdata, config)
-        return userdata
+        merge_cloud_init_config(config, self.userdata)
+        return config
 
     async def target_packages(self):
         packages = list(self.packages)

subiquity/models/tests/test_subiquity.py

@@ -20,6 +20,7 @@ import yaml
 from subiquitycore.pubsub import MessageHub
 from subiquitycore.tests.util import run_coro
 
+from subiquity.common.types import IdentityData
 from subiquity.models.subiquity import ModelNames, SubiquityModel
 from subiquity.server.server import (
     INSTALL_MODEL_NAMES,
@@ -189,3 +190,26 @@ class TestSubiquityModel(unittest.TestCase):
         self.assertEqual(
             get_mirror(config["apt"], "primary", get_architecture()),
             mirror_val)
+
+    def test_cloud_init_user_list_merge(self):
+        main_user = IdentityData(
+            username='mainuser',
+            crypted_password='dummy_value',
+            hostname='somehost')
+        secondary_user = {'name': 'user2'}
+
+        with self.subTest('Main user + secondary user'):
+            model = self.make_model()
+            model.identity.add_user(main_user)
+            model.userdata = {'users': [secondary_user]}
+            cloud_init_config = model._cloud_init_config()
+            self.assertEqual(len(cloud_init_config['users']), 2)
+            self.assertEqual(cloud_init_config['users'][0]['name'], 'mainuser')
+            self.assertEqual(cloud_init_config['users'][1]['name'], 'user2')
+
+        with self.subTest('Secondary user only'):
+            model = self.make_model()
+            model.userdata = {'users': [secondary_user]}
+            cloud_init_config = model._cloud_init_config()
+            self.assertEqual(len(cloud_init_config['users']), 1)
+            self.assertEqual(cloud_init_config['users'][0]['name'], 'user2')