Converting storage for template parameters, kernel options, and repo lists (basically anything that's not really a string or a number) to a list or hash, as appropriate. This will allow the cobbler API to accept list/hash input as appropriate in addition to strings, allowing for more advanced use of the templating engine. This also extends more power to the user to add their own entries in /var/lib/cobbler files for ksmeta, as opposed to having to enter in --ksmeta options on the command line, which previously did not tolerate newlines.

All of this is backwards compatible with the old format (both should load fine).  Files will convert over
to the new format once any add commands are run.

8 files changed, 162 insertions, 105 deletions

diff --git a/cobbler/action_sync.py b/cobbler/action_sync.py
index 5a6b6d4..80fb90e 100644
--- a/cobbler/action_sync.py
+++ b/cobbler/action_sync.py
@@ -27,7 +27,9 @@ import cexceptions
 import traceback
 import errno
 
+import item_distro
 import item_profile
+import item_system
 
 from Cheetah.Template import Template
 
@@ -356,12 +358,9 @@ class BootSync:
            )
            self.mkdir(copy_path)
            dest = os.path.join(copy_path, "ks.cfg")
-           # print "ks copy: %s -> %s" % (kickstart_path, dest)
            try:
-                meta = self.blend_options(False, (   
-                    distro.ks_meta,
-                    g.ks_meta,
-                ))  
+                will_it_blend = (distro.ks_meta, g.ks_meta)
+                meta = self.blend_options(False, will_it_blend) 
                 meta["yum_repo_stanza"] = self.generate_repo_stanza(g)
                 meta["yum_config_stanza"] = self.generate_config_stanza(g)
                 meta["kickstart_done"] = self.generate_kickstart_signal(g, is_system=False)
@@ -386,7 +385,6 @@ class BootSync:
         # will replace "TEMPLATE::yum_repo_stanza" in a cobbler kickstart file.
         buf = ""
         repos = profile.repos
-        repos = repos.split(" ")
         for r in repos:
             repo = self.repos.find(r)
             if repo is None:
@@ -398,7 +396,6 @@ class BootSync:
     def generate_config_stanza(self, profile):
         # returns the line in post that would configure yum to use repos added with "cobbler repo add"
         repos = profile.repos
-        repos = repos.split(" ")
         buf = ""
         for r in repos:
             repo = self.repos.find(r)
@@ -607,7 +604,7 @@ class BootSync:
         ))
 
         # the kernel options line is common to elilo and pxelinux
-        append_line = "%s" % kopts
+        append_line = "%s" % self.hash_to_string(kopts)
 
         # if not ia64, include initrd on this line
         # for ia64, it's already done
@@ -658,16 +655,23 @@ class BootSync:
 
         NOTE: relevant to http only
         """
-        filename = os.path.join(self.settings.webdir,"distros",distro.name)
-        distro.kernel_options = self.blend_options(True,(
-           self.settings.kernel_options,
-           distro.kernel_options
-        ))
+
+        clone = item_distro.Distro(self.config)
+        clone.from_datastruct(distro.to_datastruct())
+
+        filename = os.path.join(self.settings.webdir,"distros",clone.name)
+        will_it_blend = (self.settings.kernel_options, distro.kernel_options)
+        clone.kernel_options = self.blend_options(True,will_it_blend)
         fd = self.open_file(filename,"w+")
         # resolve to current values
-        distro.kernel = utils.find_kernel(distro.kernel)
-        distro.initrd = utils.find_initrd(distro.initrd)
-        self.tee(fd,yaml.dump(distro.to_datastruct()))
+        clone.kernel = utils.find_kernel(clone.kernel)
+        clone.initrd = utils.find_initrd(clone.initrd)
+
+        # convert storage to something that's koan readable        
+        clone.kernel_options = self.hash_to_string(clone.kernel_options)
+        clone.ks_meta = self.hash_to_string(clone.ks_meta)
+
+        self.tee(fd,yaml.dump(clone.to_datastruct()))
         self.close_file(fd)
 
     def write_profile_file(self,profile):
@@ -676,14 +680,16 @@ class BootSync:
 
         NOTE: relevant to http only
         """
-        filename = os.path.join(self.settings.webdir,"profiles",profile.name)
-        distro = self.distros.find(profile.distro)
+
+        clone = item_profile.Profile(self.config)
+        clone.from_datastruct(profile.to_datastruct())
+
+
+        filename = os.path.join(self.settings.webdir,"profiles",clone.name)
+        distro = self.distros.find(clone.distro)
         if distro is not None:
-            profile.kernel_options = self.blend_options(True,(
-               self.settings.kernel_options,
-               distro.kernel_options,
-               profile.kernel_options
-            ))
+            will_it_blend = (self.settings.kernel_options, distro.kernel_options, clone.kernel_options)
+            clone.kernel_options = self.blend_options(True,will_it_blend)
         # yaml file: http only
         fd = self.open_file(filename,"w+")
         # if kickstart path is local, we've already copied it into
@@ -691,10 +697,12 @@ class BootSync:
 
         # NOTE: we only want to write this to the webdir, not the settings
         # file, so we must make a clone, outside of the collection.
- 
-        clone = item_profile.Profile(self.config)
-        clone.from_datastruct(profile.to_datastruct())
 
+        # convert storage to something that's koan readable                   
+        clone.kernel_options = self.hash_to_string(clone.kernel_options)
+        clone.ks_meta = self.hash_to_string(clone.ks_meta)
+
+        # make URLs for koan if the kickstart files are locally managed (which is preferred)
         if clone.kickstart and clone.kickstart.startswith("/"):
             clone.kickstart = "http://%s/cobbler_track/kickstarts/%s/ks.cfg" % (self.settings.server, clone.name)
         self.tee(fd,yaml.dump(clone.to_datastruct()))
@@ -707,8 +715,16 @@ class BootSync:
 
         NOTE: relevant to http only
         """
+
+        # no real reason to clone this yet, but in case changes come later, might as well be safe.
+        clone = item_system.System(self.config)
+        clone.from_datastruct(system.to_datastruct())
+        # koan expects strings, not cobbler's storage format
+        clone.kernel_options = self.hash_to_string(clone.kernel_options)
+        clone.ks_meta = self.hash_to_string(clone.ks_meta)
+
         fd = self.open_file(filename,"w+")
-        self.tee(fd,yaml.dump(system.to_datastruct()))
+        self.tee(fd,yaml.dump(clone.to_datastruct()))
         self.close_file(fd)
 
     def tee(self,fd,text):
@@ -730,14 +746,6 @@ class BootSync:
         except IOError, ioe:
             raise cexceptions.CobblerException("need_perms2",src,dst)
 
-
-    def copy(self,src,dst):
-        print "%s -> %s" % (src,dst)
-        try:
-            return shutil.copy(src,dst)
-        except IOError, ioe:
-            raise cexceptions.CobblerException("need_perms2",src,dst)
-
     def rmfile(self,path):
         try:
             os.unlink(path)
@@ -783,50 +791,26 @@ class BootSync:
         """
         Given a list of options, take the values used by the
         first argument in the list unless overridden by those in the
-        second (or further on), according to --key=value formats.
-
-        This is used such that we can have default kernel options
-        in /etc and then distro, profile, and system options with various
-        levels of configurability overriding them.  This also works
-        for template metadata (--ksopts)
-
-        The output when is_for_kernel is true is a space delimited list.
-        When is_for_kernel is false, it's just a hash.
-        """
-        # FIXME: needs to be smart enough to remove duplicate options.
-        internal = {}
-        results = []
-        # for all list of kernel options
-        for items in list_of_opts:
-           # get each option
-           if items is not None:
-               tokens=items.split(" ")
-               # deal with key/value pairs and single options alike
-               for token in tokens:
-                   key_value = token.split("=")
-                   if len(key_value) == 1:
-                       internal[key_value[0]] = ""
-                   else:
-                       internal[key_value[0]] = key_value[1]
-        if not is_for_kernel:
-            return internal
-        # the kernel requires a flat string for options, and we want
-        # to remove certain invalid options.
-        # go back through the final list and render the single
-        # items AND key/value items
-        for key in internal.keys():
-           data = internal[key]
-           if (key == "ks" or key == "initrd" or key == "append"):
-               # the user REALLY doesn't want to do this...
-               continue
-           if data == "":
-               results.append(key)
-           else:
-               results.append("%s=%s" % (key,internal[key]))
-        # end result is a new fragment of an options string
-    
-        # now add the remote syslogging stuff
-        results.append("syslog=%s:%s" % (self.settings.server, self.settings.syslog_port))
+        second (or further on).
+        """
+        results = {}
+        buffer = ""
+        for optslist in list_of_opts:
+           for key in optslist:
+               results[key] = optslist[key]
 
-        return " ".join(results)
+        if is_for_kernel and self.settings.syslog_port != 0:
+            results["syslog"] = "%s:%s" % (self.settings.server, self.settings.syslog_port)
+
+        return results
+
+    def hash_to_string(self, hash):
+        buffer = ""
+        for key in hash:
+           value = hash[key]
+           if value is None:
+               buffer = buffer + key + " "
+           else:
+               buffer = buffer + key + "=" + value + " "
+        return buffer
 
diff --git a/cobbler/cobbler.py b/cobbler/cobbler.py
index 4a57919..14a8f56 100755
--- a/cobbler/cobbler.py
+++ b/cobbler/cobbler.py
@@ -88,6 +88,7 @@ class BootCLI:
             'clobber'      : self.enchant,
             'transmogrify' : self.enchant,
             'status'       : self.status,
+            'reserialize'  : self.reserialize,
             'help'         : self.usage,
             '--help'       : self.usage,
             '/?'           : self.usage
@@ -422,6 +423,16 @@ class BootCLI:
             raise cexceptions.CobblerException("unknown_cmd", args[0])
         return True
 
+    def reserialize(self, args):
+        """
+        This command is intentionally not documented in the manpage.
+        Basically it loads the cobbler config and then reserialize's it's internal state.
+        It can be used for testing config format upgrades and other development related things.
+        It has very little purpose in the real world.
+        """
+        self.api.serialize()
+        return True
+
     def sync(self, args):
         """
         Sync the config file with the system config: 'cobbler sync'
diff --git a/cobbler/item.py b/cobbler/item.py
index 01392ec..1249229 100644
--- a/cobbler/item.py
+++ b/cobbler/item.py
@@ -13,8 +13,8 @@ Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 """
 
 import exceptions
-
 import serializable
+import utils
 
 class Item(serializable.Serializable):
 
@@ -26,29 +26,30 @@ class Item(serializable.Serializable):
         self.name = name
         return True
 
-    def set_kernel_options(self,options_string):
+    def set_kernel_options(self,options):
         """
-	Kernel options are a comma delimited list of key value pairs,
-	like 'a=b,c=d,e=f'
+	Kernel options are a space delimited list,
+	like 'a=b,c=d,e=f' or as a hash.
 	"""
-        self.kernel_options = options_string
-        return True
+        (success, value) = utils.input_string_or_hash(options," ")
+        if not success:
+            return False
+        else:
+            self.kernel_options = value
+            return True
 
-    def set_ksmeta(self,options_string):
+    def set_ksmeta(self,options):
         """
-        A comma delimited list of key value pairs, like 'a=b,c=d,e=f'.
+        A comma delimited list of key value pairs, like 'a=b,c=d,e=f' or a hash.
         The meta tags are used as input to the templating system
         to preprocess kickstart files
         """
-        if options_string is None:
-            options_string = ""
-        self.ks_meta = options_string
-        tokens = self.ks_meta.split(",")
-        for t in tokens:
-            tokens2 = t.split("=")
-            if len(tokens2) != 2:
-                return False
-        return True
+        (success, value) = utils.input_string_or_hash(options,",")
+        if not success:
+            return False
+        else:
+            self.ks_meta = value
+            return True
 
     def load_item(self,datastruct,key,default=''):
         """
diff --git a/cobbler/item_distro.py b/cobbler/item_distro.py
index 1d28920..48e28ea 100644
--- a/cobbler/item_distro.py
+++ b/cobbler/item_distro.py
@@ -36,8 +36,8 @@ class Distro(item.Item):
         self.name = None
         self.kernel = None
         self.initrd = None
-        self.kernel_options = ""
-        self.ks_meta = ""
+        self.kernel_options = {}
+        self.ks_meta = {}
         self.arch = "x86"
         self.breed = "redhat"
 
@@ -52,6 +52,11 @@ class Distro(item.Item):
         self.ks_meta        = self.load_item(seed_data,'ks_meta')
         self.arch           = self.load_item(seed_data,'arch','x86')
         self.breed          = self.load_item(seed_data,'breed','redhat')
+        # backwards compatibility -- convert string entries to dicts for storage
+        if type(self.kernel_options) != dict:
+            self.set_kernel_options(self.kernel_options)
+        if type(self.ks_meta) != dict:
+            self.set_ksmeta(self.ks_meta)
         return self
 
     def set_kernel(self,kernel):
diff --git a/cobbler/item_profile.py b/cobbler/item_profile.py
index 0b310c0..a666bb4 100644
--- a/cobbler/item_profile.py
+++ b/cobbler/item_profile.py
@@ -33,8 +33,8 @@ class Profile(item.Item):
         self.name = None
         self.distro = None # a name, not a reference
         self.kickstart = self.settings.default_kickstart
-        self.kernel_options = ''
-        self.ks_meta = ''
+        self.kernel_options = {}
+        self.ks_meta = {} 
         self.virt_name = 'virtguest'
         self.virt_file_size = 5    # GB.  5 = Decent _minimum_ default for FC5.
         self.virt_ram = 512        # MB.  Install with 256 not likely to pass
@@ -51,6 +51,9 @@ class Profile(item.Item):
         self.kernel_options  = self.load_item(seed_data,'kernel_options')
         self.ks_meta         = self.load_item(seed_data,'ks_meta')
         self.repos           = self.load_item(seed_data,'repos', "")       
+        # backwards compatibility
+        if type(self.repos) != list:
+            self.set_repos(self.repos)
 
         # virt specific 
         self.virt_name       = self.load_item(seed_data,'virt_name')
@@ -60,6 +63,12 @@ class Profile(item.Item):
         self.virt_file_size  = self.load_item(seed_data,'virt_file_size')
         self.virt_paravirt   = self.load_item(seed_data,'virt_paravirt')
 
+        # backwards compatibility -- convert string entries to dicts for storage
+        if type(self.kernel_options) != dict:
+            self.set_kernel_options(self.kernel_options)
+        if type(self.ks_meta) != dict:
+            self.set_ksmeta(self.ks_meta)
+
         return self
 
     def set_distro(self,distro_name):
@@ -73,8 +82,13 @@ class Profile(item.Item):
         raise cexceptions.CobblerException("no_distro")
 
     def set_repos(self,repos):
-        repolist = repos.split(" ")
+        if type(repos) != list:
+            # allow backwards compatibility support of string input
+            repolist = repos.split(" ")
+        else:
+            repolist = repos
         ok = True
+        repolist.remove('')
         for r in repolist:
             if not self.config.repos().find(r):
                 ok = False 
diff --git a/cobbler/item_system.py b/cobbler/item_system.py
index 03a4032..7409f11 100644
--- a/cobbler/item_system.py
+++ b/cobbler/item_system.py
@@ -25,8 +25,8 @@ class System(item.Item):
     def clear(self):
         self.name = None
         self.profile = None # a name, not a reference
-        self.kernel_options = ""
-        self.ks_meta = ""
+        self.kernel_options = {}
+        self.ks_meta = {}
         self.pxe_address = ""
 
     def from_datastruct(self,seed_data):
@@ -35,6 +35,13 @@ class System(item.Item):
         self.kernel_options = self.load_item(seed_data,'kernel_options')
         self.ks_meta = self.load_item(seed_data,'ks_meta')
         self.pxe_address = self.load_item(seed_data,'pxe_address')
+
+        # backwards compatibility -- convert string entries to dicts for storage
+        if type(self.kernel_options) != dict:
+            self.set_kernel_options(self.kernel_options)
+        if type(self.ks_meta) != dict:
+            self.set_ksmeta(self.ks_meta)
+
         return self
 
     def set_name(self,name):
diff --git a/cobbler/settings.py b/cobbler/settings.py
index af62766..d7fcd8c 100644
--- a/cobbler/settings.py
+++ b/cobbler/settings.py
@@ -24,7 +24,14 @@ DEFAULTS = {
     "server"            : "127.0.0.1",
     "next_server"       : "127.0.0.1",
     "dhcpd_bin"         : "/usr/sbin/dhcpd",
-    "kernel_options"    : "append devfs=nomount ramdisk_size=16438 lang= text ksdevice=eth0",
+    "kernel_options"    : {
+        "append"        : None,
+        "devfs"         : "nomount",
+        "ramdisk_size"  : 16438,
+        "lang="         : " ",
+        "text"          : None,
+        "ksdevice"      : "eth0",
+    },
     "tftpd_conf"        : "/etc/xinetd.d/tftp",
     "tftpboot"          : "/tftpboot",
     "webdir"            : "/var/www/cobbler",
@@ -87,6 +94,11 @@ class Settings(serializable.Serializable):
 
    def __getattr__(self,name):
        if self._attributes.has_key(name):
+           if name == "kernel_options":
+               # backwards compatibility -- convert possible string value to hash
+               (success, result) = utils.input_string_or_hash(self._attributes[name], " ")
+               self._attributes[name] = result
+               return result
            return self._attributes[name]
        elif DEFAULTS.has_key(name):
            lookup = DEFAULTS[name]
diff --git a/cobbler/utils.py b/cobbler/utils.py
index 4333393..add034f 100644
--- a/cobbler/utils.py
+++ b/cobbler/utils.py
@@ -173,4 +173,27 @@ def find_kickstart(url):
            return url
     return None
 
+def input_string_or_hash(options,delim=","):
+    """
+    Older cobbler files stored configurations in a flat way, such that all values for strings.
+    Newer versions of cobbler allow dictionaries.  This function is used to allow loading
+    of older value formats so new users of cobbler aren't broken in an upgrade.
+    """
+    if options is None:
+        return (True, {})
+    elif type(options) != dict:
+        new_dict = {}
+        tokens = options.split(delim)
+        for t in tokens:
+            tokens2 = t.split("=")
+            if len(tokens2) == 1 and tokens2[0] != '':
+                new_dict[tokens2[0]] = None
+            elif len(tokens2) == 2 and tokens2[0] != '':
+                new_dict[tokens2[0]] = tokens2[1]
+            else:
+                return (False, {})
+        return (True, new_dict)
+    else:
+        options.pop('',None)
+        return (True, options)