From bd9008794c5f620ae3957af78881887c9e85d3cc Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Fri, 10 Jun 2016 07:50:07 +0200 Subject: [PATCH 1/2] Conflicting requirements bugfix. --- cdist/emulator.py | 37 +++++++++++++++++++--- cdist/test/emulator/__init__.py | 55 +++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/cdist/emulator.py b/cdist/emulator.py index 3f553412..f5a9f645 100644 --- a/cdist/emulator.py +++ b/cdist/emulator.py @@ -77,6 +77,9 @@ class Emulator(object): self.type_name = os.path.basename(argv[0]) self.cdist_type = core.CdistType(self.type_base_path, self.type_name) + # if set then object already exists and this var holds existing + # requirements + self._existing_reqs = None self.__init_log() @@ -150,10 +153,18 @@ class Emulator(object): self.parameters[key] = value if self.cdist_object.exists and not 'CDIST_OVERRIDE' in self.env: + # make existing requirements a set, so we can compare it + # later with new requirements + self._existing_reqs = set(self.cdist_object.requirements) if self.cdist_object.parameters != self.parameters: - raise cdist.Error("Object %s already exists with conflicting parameters:\n%s: %s\n%s: %s" - % (self.cdist_object.name, " ".join(self.cdist_object.source), self.cdist_object.parameters, self.object_source, self.parameters) - ) + errmsg = ("Object %s already exists with conflicting " + "parameters:\n%s: %s\n%s: %s" % (self.cdist_object.name, + " ".join(self.cdist_object.source), + self.cdist_object.parameters, + self.object_source, + self.parameters)) + self.log.error(errmsg) + raise cdist.Error(errmsg) else: if self.cdist_object.exists: self.log.debug('Object %s override forced with CDIST_OVERRIDE',self.cdist_object.name) @@ -210,7 +221,7 @@ class Emulator(object): # if no second last line, we are on the first type, so do not set a requirement pass - + reqs = set() if "require" in self.env: requirements = self.env['require'] self.log.debug("reqs = " + requirements) @@ -234,6 +245,24 @@ class Emulator(object): # (__file//bar => __file/bar) # This ensures pattern matching is done against sanitised list self.cdist_object.requirements.append(cdist_object.name) + reqs.add(cdist_object.name) + if self._existing_reqs is not None: + # if object exists then compare existing and new requirements + self.log.debug("OBJ: {} {}".format(self.cdist_type, self.object_id)) + self.log.debug("EXISTING REQS: {}".format(self._existing_reqs)) + self.log.debug("REQS: {}".format(reqs)) + + if self._existing_reqs != reqs: + errmsg = ("Object {} already exists with conflicting " + "requirements:\n{}: {}\n{}: {}".format( + self.cdist_object.name, + " ".join(self.cdist_object.source), + self._existing_reqs, + self.object_source, + reqs)) + self.log.error(errmsg) + raise cdist.Error(errmsg) + def record_auto_requirements(self): """An object shall automatically depend on all objects that it defined in it's type manifest. diff --git a/cdist/test/emulator/__init__.py b/cdist/test/emulator/__init__.py index f90e5320..b91c1e8f 100644 --- a/cdist/test/emulator/__init__.py +++ b/cdist/test/emulator/__init__.py @@ -133,6 +133,56 @@ class EmulatorTestCase(test.CdistTestCase): self.assertEqual(list(file_object.requirements), ['__planet/mars']) # if we get here all is fine +class EmulatorConflictingRequirementsTestCase(test.CdistTestCase): + + def setUp(self): + self.temp_dir = self.mkdtemp() + handle, self.script = self.mkstemp(dir=self.temp_dir) + os.close(handle) + base_path = self.temp_dir + + self.local = local.Local( + target_host=self.target_host, + base_path=base_path, + exec_path=test.cdist_exec_path, + add_conf_dirs=[conf_dir]) + self.local.create_files_dirs() + + self.manifest = core.Manifest(self.target_host, self.local) + self.env = self.manifest.env_initial_manifest(self.script) + self.env['__cdist_object_marker'] = self.local.object_marker_name + + def tearDown(self): + shutil.rmtree(self.temp_dir) + + def test_object_conflicting_requirements_req_none(self): + argv = ['__directory', 'spam'] + emu = emulator.Emulator(argv, env=self.env) + emu.run() + argv = ['__file', 'eggs'] + self.env['require'] = '__directory/spam' + emu = emulator.Emulator(argv, env=self.env) + emu.run() + argv = ['__file', 'eggs'] + if 'require' in self.env: + del self.env['require'] + emu = emulator.Emulator(argv, env=self.env) + self.assertRaises(cdist.Error, emu.run) + + def test_object_conflicting_requirements_none_req(self): + argv = ['__directory', 'spam'] + emu = emulator.Emulator(argv, env=self.env) + emu.run() + argv = ['__file', 'eggs'] + if 'require' in self.env: + del self.env['require'] + emu = emulator.Emulator(argv, env=self.env) + emu.run() + argv = ['__file', 'eggs'] + self.env['require'] = '__directory/spam' + emu = emulator.Emulator(argv, env=self.env) + self.assertRaises(cdist.Error, emu.run) + class AutoRequireEmulatorTestCase(test.CdistTestCase): @@ -366,3 +416,8 @@ class StdinTestCase(test.CdistTestCase): stdin_saved_by_emulator = fd.read() self.assertEqual(random_string, stdin_saved_by_emulator) + + +if __name__ == '__main__': + import unittest + unittest.main() From 5ee20f7886d81be81407c33cc205d82d1dc51c2e Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Wed, 22 Jun 2016 12:23:31 +0200 Subject: [PATCH 2/2] Update changelog. --- docs/changelog | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog b/docs/changelog index 8ad8e953..31f2bb35 100644 --- a/docs/changelog +++ b/docs/changelog @@ -2,6 +2,7 @@ Changelog --------- next: + * Core: Fix conflicting requirements (Darko Poljak) * New type: __jail_freebsd9: Handle jail management on FreeBSD <= 9.X (Jake Guffey) * New type: __jail_freebsd10: Handle jail management on FreeBSD >= 10.0 (Jake Guffey) * Type __jail: Dynamically select the correct jail subtype based on target host OS (Jake Guffey)