From 068fe0df24740772c1a588ec62d5673383a8e880 Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Thu, 23 Aug 2007 12:12:42 -0400 Subject: Some code to fix a reference issue that allowed for hash value propogation up the graph, in ksmeta and possibly kopts. Also added some code to check to make sure an object of the wrong type never gets added to the wrong collection. --- cobbler/collection.py | 4 ++++ cobbler/item_distro.py | 14 ++++++-------- cobbler/item_profile.py | 3 ++- cobbler/item_repo.py | 1 + cobbler/item_system.py | 5 +++-- cobbler/utils.py | 7 +++++-- tests/tests.py | 30 +++++++++++++++++------------- 7 files changed, 38 insertions(+), 26 deletions(-) diff --git a/cobbler/collection.py b/cobbler/collection.py index c113481..98e841c 100644 --- a/cobbler/collection.py +++ b/cobbler/collection.py @@ -154,6 +154,10 @@ class Collection(serializable.Serializable): """ if ref is None or not ref.is_valid(): raise CX(_("insufficient or invalid arguments supplied")) + + if ref.COLLECTION_TYPE != self.collection_type(): + raise CX(_("API error: storing wrong data type in collection")) + if not with_copy: # don't need to run triggers, so add it already ... self.listing[ref.name.lower()] = ref diff --git a/cobbler/item_distro.py b/cobbler/item_distro.py index 4a97fcf..fdec9c8 100644 --- a/cobbler/item_distro.py +++ b/cobbler/item_distro.py @@ -25,6 +25,7 @@ from rhpl.translate import _, N_, textdomain, utf8 class Distro(item.Item): TYPE_NAME = _("distro") + COLLECTION_TYPE = "distro" def clear(self,is_subobject=False): """ @@ -49,13 +50,9 @@ class Distro(item.Item): def get_parent(self): """ Return object next highest up the tree. - NOTE: conceptually there is no need for subdistros, but it's implemented - anyway for testing purposes + NOTE: conceptually there is no need for subdistros """ - if self.parent is None or self.parent == '': - return None - else: - return self.config.distros().find(name=self.parent) + return None def from_datastruct(self,seed_data): """ @@ -73,10 +70,11 @@ class Distro(item.Item): self.depth = self.load_item(seed_data,'depth',0) # backwards compatibility -- convert string entries to dicts for storage - if type(self.kernel_options) != dict: + if self.kernel_options != "<>" and type(self.kernel_options) != dict: self.set_kernel_options(self.kernel_options) - if type(self.ks_meta) != dict: + if self.ks_meta != "<>" and 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 4062826..3df3113 100644 --- a/cobbler/item_profile.py +++ b/cobbler/item_profile.py @@ -21,7 +21,8 @@ from rhpl.translate import _, N_, textdomain, utf8 class Profile(item.Item): TYPE_NAME = _("profile") - + COLLECTION_TYPE = "profile" + def make_clone(self): ds = self.to_datastruct() cloned = Profile(self.config) diff --git a/cobbler/item_repo.py b/cobbler/item_repo.py index 726fa13..447e057 100644 --- a/cobbler/item_repo.py +++ b/cobbler/item_repo.py @@ -20,6 +20,7 @@ from rhpl.translate import _, N_, textdomain, utf8 class Repo(item.Item): TYPE_NAME = _("repo") + COLLECTION_TYPE = "repo" def make_clone(self): ds = self.to_datastruct() diff --git a/cobbler/item_system.py b/cobbler/item_system.py index 3575d79..445cf73 100644 --- a/cobbler/item_system.py +++ b/cobbler/item_system.py @@ -21,6 +21,7 @@ from rhpl.translate import _, N_, textdomain, utf8 class System(item.Item): TYPE_NAME = _("system") + COLLECTION_TYPE = "system" def make_clone(self): ds = self.to_datastruct() @@ -70,9 +71,9 @@ class System(item.Item): # backwards compatibility -- convert string entries to dicts for storage # this allows for better usage from the API. - if type(self.kernel_options) != dict: + if self.kernel_options != "<>" and type(self.kernel_options) != dict: self.set_kernel_options(self.kernel_options) - if type(self.ks_meta) != dict: + if self.ks_meta != "<>" and type(self.ks_meta) != dict: self.set_ksmeta(self.ks_meta) # backwards compatibility -- if name is an IP or a MAC, set appropriate fields diff --git a/cobbler/utils.py b/cobbler/utils.py index 34b77e8..7845960 100644 --- a/cobbler/utils.py +++ b/cobbler/utils.py @@ -307,7 +307,10 @@ def __consolidate(node,results): for key in node_data: value = node_data[key] if value != "<>": - node_data_copy[key] = value + if type(value) == type({}): + node_data_copy[key] = value.copy() + else: + node_data_copy[key] = value for field in node_data_copy: @@ -328,7 +331,7 @@ def __consolidate(node,results): # or scalar. if type(data_item) == dict: # interweave hash results - results[field].update(data_item) + results[field].update(data_item.copy()) elif type(data_item) == list or type(data_item) == tuple: # add to lists (cobbler doesn't have many lists) # FIXME: should probably uniqueify list after doing this diff --git a/tests/tests.py b/tests/tests.py index 616ac8a..71862c2 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -222,6 +222,7 @@ class Additions(BootTest): self.assertTrue(profile.set_distro("testdistro0")) self.assertTrue(profile.set_kickstart("http://127.0.0.1/foo")) self.assertTrue(self.api.profiles().add(profile)) + self.api.sync() system = self.api.new_system() self.assertTrue(system.set_name("foo")) self.assertTrue(system.set_profile("testprofile12b2")) @@ -240,6 +241,7 @@ class Additions(BootTest): profile2.set_name("testprofile12b3") profile2.set_parent("testprofile12b2") self.assertTrue(self.api.profiles().add(profile2)) + self.api.sync() # FIXME: now add a system to the inherited profile # and set a attribute on it that we will later check for @@ -249,6 +251,7 @@ class Additions(BootTest): self.assertTrue(system2.set_profile("testprofile12b3")) self.assertTrue(system2.set_ksmeta({"narf" : "troz"})) self.assertTrue(self.api.systems().add(system2)) + self.api.sync() # now see if the profile does NOT have the ksmeta attribute # this is part of our test against upward propogation @@ -261,16 +264,17 @@ class Additions(BootTest): profile = self.api.profiles().find("testprofile12b2") self.assertTrue(type(profile.ks_meta) == type({})) - self.assertFalse(profile.ks_meta.has_key("narf")) + print "DEBUG2: %s" % profile.ks_meta + self.api.sync() + self.assertFalse(profile.ks_meta.has_key("narf"), "profile does not have the system ksmeta") - # be extra careful self.api.sync() # verify that the distro did not acquire the property # we just set on the leaf system distro = self.api.distros().find("testdistro0") self.assertTrue(type(distro.ks_meta) == type({})) - self.assertFalse(distro.ks_meta.has_key("narf")) + self.assertFalse(distro.ks_meta.has_key("narf"), "distro does not have the system ksmeta") # STEP THREE: verify that inheritance appears to work # by setting ks_meta on the subprofile and seeing @@ -280,10 +284,10 @@ class Additions(BootTest): profile2 = self.api.profiles().find("testprofile12b3") profile2.set_ksmeta({"canyouseethis" : "yes" }) self.assertTrue(self.api.profiles().add(profile2)) - system2 = self.api.systems().get("foo2") + system2 = self.api.systems().find("foo2") data = utils.blender(False, system2) - assertTrue(data.has_key("ks_meta")) - assertTrue(data["ks_meta"].has_key("canyouseethis")) + self.assertTrue(data.has_key("ks_meta")) + self.assertTrue(data["ks_meta"].has_key("canyouseethis")) # STEP FOUR: do the same on the superprofile and see # if that propogates @@ -291,20 +295,20 @@ class Additions(BootTest): profile = self.api.profiles().find("testprofile12b2") profile.set_ksmeta({"canyouseethisalso" : "yes" }) self.assertTrue(self.api.profiles().add(profile)) - system2 = self.api.systems().get("foo2") + system2 = self.api.systems().find("foo2") data = utils.blender(False, system2) - assertTrue(data.has_key("ks_meta")) - assertTrue(data["ks_meta"].has_key("canyouseethisalso")) + self.assertTrue(data.has_key("ks_meta")) + self.assertTrue(data["ks_meta"].has_key("canyouseethisalso")) # STEP FIVE: see if distro attributes propogate distro = self.api.distros().find("testdistro0") distro.set_ksmeta({"alsoalsowik" : "moose" }) - self.assertTrue(self.api.systems().add(distro)) - system2 = self.api.systems().get("foo2") + self.assertTrue(self.api.distros().add(distro)) + system2 = self.api.systems().find("foo2") data = utils.blender(False, system2) - assertTrue(data.has_key("ks_meta")) - assertTrue(data["ks_meta"].has_key("alsoalsowik")) + self.assertTrue(data.has_key("ks_meta")) + self.assertTrue(data["ks_meta"].has_key("alsoalsowik")) # STEP SIX: see if settings changes also propogate # TBA -- cgit