From 8c6f2a07ae10ed082865adb6eeb3e3f2fdb68e11 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Wed, 10 Jul 2013 11:40:48 +0200 Subject: [PATCH 1/4] Remove magic retrieval of configuration files This change leads to expected exceptions when bootstrapping the application since the \Icinga\Application\Config class was refactored but its usage not. refs #4354 refs #4353 --- doc/CONFIG.md | 24 +++ library/Icinga/Application/Config.php | 159 +++++++++++------- .../Application/Config/files/config.ini | 7 + .../Icinga/Application/Config/files/extra.ini | 2 + .../Config/files/modules/amodule/config.ini | 2 + .../Config/files/modules/amodule/extra.ini | 2 + .../library/Icinga/Application/ConfigTest.php | 100 +++++++---- test/php/library/Icinga/ConfigTest.php | 50 ------ 8 files changed, 200 insertions(+), 146 deletions(-) create mode 100644 doc/CONFIG.md create mode 100644 test/php/library/Icinga/Application/Config/files/config.ini create mode 100644 test/php/library/Icinga/Application/Config/files/extra.ini create mode 100644 test/php/library/Icinga/Application/Config/files/modules/amodule/config.ini create mode 100644 test/php/library/Icinga/Application/Config/files/modules/amodule/extra.ini delete mode 100755 test/php/library/Icinga/ConfigTest.php diff --git a/doc/CONFIG.md b/doc/CONFIG.md new file mode 100644 index 000000000..5ebaa607b --- /dev/null +++ b/doc/CONFIG.md @@ -0,0 +1,24 @@ +# Application and Module Configuration + +The \Icinga\Application\Config class is a general purpose service to help you find, load and save +configuration data. It is used both by the Icinga 2 Web modules and the framework itself. With +INI files as source it enables you to store configuration in a familiar format. Icinga 2 Web +defines some configuration files for its own purposes. Please note that both modules and framework +keep their main configuration in the INI file called config.ini. Here's some example code: + + global->get('defaultTimezone', 'Europe/Berlin'); + + // If you don't pass a configuration name to IcingaConfig::app it tries to load values from the + // application's config.ini. For using other files you have to pass this parameter though. + // The following example loads a section from the application's authentication.ini: + IcingaConfig::app('authentication')->get('ldap-authentication'); + + // If you don't pass a configuration name to IcingaConfig::module it tries to load values from + // the module's config.ini. For using other files you have to pass this parameter though. + // The following example loads values from the example module's extra.ini: + IcingaConfig::module('example', 'extra')->logging->get('enabled', true); + diff --git a/library/Icinga/Application/Config.php b/library/Icinga/Application/Config.php index e19d15345..b77861310 100755 --- a/library/Icinga/Application/Config.php +++ b/library/Icinga/Application/Config.php @@ -1,79 +1,110 @@ $what === null) { + if (!@is_readable($filename)) { + throw new \Exception('Cannot read config file: ' . $filename); + }; + $this->configFile = $filename; + parent::__construct($filename); + } + + /** + * Retrieve a application config instance. + * + * @param string $configname + * @return mixed + */ + public static function app($configname = 'config') + { + if (!isset(self::$app[$configname])) { + $filename = self::$configDir . '/' . $configname . '.ini'; + self::$app[$configname] = new Config($filename); + } + return self::$app[$configname]; + } + + /** + * Retrieve a module config instance. + * + * @param string $modulename + * @param string $configname + * @return Config + */ + public static function module($modulename, $configname = 'config') + { + if (!isset(self::$modules[$modulename])) { + self::$modules[$modulename] = array(); + } + $moduleConfigs = self::$modules[$modulename]; + if (!isset($moduleConfigs[$configname])) { + $filename = self::$configDir . '/modules/' . $modulename . '/' . $configname . '.ini'; + $moduleConfigs[$configname] = new Config($filename); + } + return $moduleConfigs[$configname]; + } + + /** + * Retrieve names of accessible sections or properties. + * + * @param $name + * @return array + */ + public function keys($name = null) + { + if ($name === null) { + return array_keys($this->toArray()); + } elseif ($this->$name === null) { return array(); } else { - return array_keys($this->$what->toArray()); + return array_keys($this->$name->toArray()); } } - - public function getConfigDir() - { - return $this->configDir; - } - - public function __construct($filename, $section = null, $options = false) - { - $options['allowModifications'] = true; - $this->configDir = dirname($filename); - return parent::__construct($filename, $section, $options); - } - - public static function module($name, $file = null) - { - if ($file === null) { - $file = $name . '.ini'; // TODO: default should be module/config.ini - } - $filename = Module::get($name)->getConfigDir() . '/' . $file; - if (file_exists($filename)) { - $config = new Config($filename); - // Compat: $config->$module->$whatever - self::getInstance()->$name = $config; - return $config; - } - return null; - } - - public function __get($key) - { - $res = parent::__get($key); - if ($res === null) { - $app = Icinga::app(); - if ($app->hasModule($key)) { - $filename = $app->getModule($key)->getConfigDir() . "/$key.ini"; - } else { - $filename = $this->configDir . '/' . $key . '.ini'; - } - if (file_exists($filename)) { - $res = $this->$key = new Config($filename); - } - } - return $res; - } - - public static function getInstance($configFile = null) - { - if (self::$instance === null) { - if ($configFile === null) { - $configFile = dirname(dirname(dirname(dirname(__FILE__)))) - . '/config/icinga.ini'; - } - self::$instance = new Config($configFile); - } - return self::$instance; - } } diff --git a/test/php/library/Icinga/Application/Config/files/config.ini b/test/php/library/Icinga/Application/Config/files/config.ini new file mode 100644 index 000000000..b74ea65c5 --- /dev/null +++ b/test/php/library/Icinga/Application/Config/files/config.ini @@ -0,0 +1,7 @@ +[logging] +enable = 1 + +[backend] +db.user = 'user' +db.password = 'password' +disable = 1 diff --git a/test/php/library/Icinga/Application/Config/files/extra.ini b/test/php/library/Icinga/Application/Config/files/extra.ini new file mode 100644 index 000000000..fc203052e --- /dev/null +++ b/test/php/library/Icinga/Application/Config/files/extra.ini @@ -0,0 +1,2 @@ +[meta] +version = 1 diff --git a/test/php/library/Icinga/Application/Config/files/modules/amodule/config.ini b/test/php/library/Icinga/Application/Config/files/modules/amodule/config.ini new file mode 100644 index 000000000..012a5188a --- /dev/null +++ b/test/php/library/Icinga/Application/Config/files/modules/amodule/config.ini @@ -0,0 +1,2 @@ +[menu] +breadcrumb = 1 diff --git a/test/php/library/Icinga/Application/Config/files/modules/amodule/extra.ini b/test/php/library/Icinga/Application/Config/files/modules/amodule/extra.ini new file mode 100644 index 000000000..38fd04b27 --- /dev/null +++ b/test/php/library/Icinga/Application/Config/files/modules/amodule/extra.ini @@ -0,0 +1,2 @@ +[ldap] +user.ldap_object_class = inetOrgPerson diff --git a/test/php/library/Icinga/Application/ConfigTest.php b/test/php/library/Icinga/Application/ConfigTest.php index 72cd2cfad..52967c220 100644 --- a/test/php/library/Icinga/Application/ConfigTest.php +++ b/test/php/library/Icinga/Application/ConfigTest.php @@ -1,50 +1,86 @@ markTestIncomplete('testListAll is not implemented yet'); + IcingaConfig::$configDir = dirname(__FILE__) . '/Config/files'; } - /** - * Test for Config::GetConfigDir() - * - **/ - public function testGetConfigDir() + public function testAppConfig() { - $this->markTestIncomplete('testGetConfigDir is not implemented yet'); + $config = IcingaConfig::app(); + $this->assertEquals(1, $config->logging->enable); + // Test non-existent property where null is the default value + $this->assertEquals(null, $config->logging->get('disable')); + // Test non-existent property using zero as the default value + $this->assertEquals(0, $config->logging->get('disable', 0)); + // Test retrieve full section + $this->assertEquals( + array( + 'disable' => 1, + 'db' => array( + 'user' => 'user', + 'password' => 'password' + ) + ), + $config->backend->toArray() + ); + // Test non-existent section using 'default' as default value + $this->assertEquals('default', $config->get('magic', 'default')); + // Test sub-properties + $this->assertEquals('user', $config->backend->db->user); + // Test non-existent sub-property using 'UTF-8' as the default value + $this->assertEquals('UTF-8', $config->backend->db->get('encoding', 'UTF-8')); + // Test invalid property names using false as default value + $this->assertEquals(false, $config->backend->get('.', false)); + $this->assertEquals(false, $config->backend->get('db.', false)); + $this->assertEquals(false, $config->backend->get('.user', false)); + // Test retrieve array of sub-properties + $this->assertEquals( + array( + 'user' => 'user', + 'password' => 'password' + ), + $config->backend->db->toArray() + ); + // Test singleton + $this->assertEquals($config, IcingaConfig::app()); + $this->assertEquals(array('logging', 'backend'), $config->keys()); + $this->assertEquals(array('enable'), $config->keys('logging')); } - /** - * Test for Config::__get() - * - **/ - public function test__get() + public function testAppExtraConfig() { - $this->markTestIncomplete('test__get is not implemented yet'); + $extraConfig = IcingaConfig::app('extra'); + $this->assertEquals(1, $extraConfig->meta->version); + $this->assertEquals($extraConfig, IcingaConfig::app('extra')); } - /** - * Test for Config::GetInstance() - * Note: This method is static! - * - **/ - public function testGetInstance() + public function testModuleConfig() { - $this->markTestIncomplete('testGetInstance is not implemented yet'); + $moduleConfig = IcingaConfig::module('amodule'); + $this->assertEquals(1, $moduleConfig->menu->get('breadcrumb')); + $this->assertEquals($moduleConfig, IcingaConfig::module('amodule')); } + public function testModuleExtraConfig() + { + $moduleExtraConfig = IcingaConfig::module('amodule', 'extra'); + $this->assertEquals( + 'inetOrgPerson', + $moduleExtraConfig->ldap->user->get('ldap_object_class') + ); + $this->assertEquals($moduleExtraConfig, IcingaConfig::module('amodule', 'extra')); + } } diff --git a/test/php/library/Icinga/ConfigTest.php b/test/php/library/Icinga/ConfigTest.php deleted file mode 100755 index 4d700a579..000000000 --- a/test/php/library/Icinga/ConfigTest.php +++ /dev/null @@ -1,50 +0,0 @@ -markTestIncomplete('testGet is not implemented yet'); - } - - /** - * Test for Config::__get() - * - **/ - public function test__get() - { - $this->markTestIncomplete('test__get is not implemented yet'); - } - - /** - * Test for Config::__set() - * - **/ - public function test__set() - { - $this->markTestIncomplete('test__set is not implemented yet'); - } - - /** - * Test for Config::Create() - * Note: This method is static! - * - **/ - public function testCreate() - { - $this->markTestIncomplete('testCreate is not implemented yet'); - } - -} From 51755209b649aafd1bb993e7f8b58fba9bf50a43 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Fri, 12 Jul 2013 15:37:36 +0200 Subject: [PATCH 2/4] Adapt usage of config class refs #4354 --- Vagrantfile | 4 +- application/controllers/ModulesController.php | 15 ++-- .../layouts/scripts/parts/navigation.phtml | 2 +- config/{icinga.ini => config.ini} | 0 .../modules/monitoring}/backends.ini | 4 +- .../modules/monitoring}/instances.ini | 0 .../modules/monitoring}/menu.ini | 0 .../Application/ApplicationBootstrap.php | 26 +++---- library/Icinga/Application/Config.php | 39 ++++++----- library/Icinga/Application/Modules/Module.php | 4 +- library/Icinga/Application/Web.php | 2 +- .../Backend/LdapUserBackend.php | 26 +++---- library/Icinga/Authentication/Manager.php | 68 +++++++++---------- library/Icinga/Backend.php | 10 +-- library/Icinga/Web/ActionController.php | 23 ++++++- library/Icinga/Web/ModuleActionController.php | 4 +- .../monitoring/library/Monitoring/Backend.php | 5 +- public/css.php | 2 +- public/index.php | 2 +- 19 files changed, 129 insertions(+), 107 deletions(-) rename config/{icinga.ini => config.ini} (100%) rename {modules/monitoring/config => config/modules/monitoring}/backends.ini (59%) rename {modules/monitoring/config => config/modules/monitoring}/instances.ini (100%) rename {modules/monitoring/config => config/modules/monitoring}/menu.ini (100%) diff --git a/Vagrantfile b/Vagrantfile index 5a6eaf508..79419bad3 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -48,6 +48,8 @@ Vagrant::Config.run do |config| # an identifier, the second is the path on the guest to mount the # folder, and the third is the path on the host to the actual folder. # # config.vm.share_folder "v-icinga2-web-pub", "/var/www/html/icinga2-web", "./pub" + config.vm.share_folder "v-test", "/vagrant/config", "./config", :owner => 'vagrant', :group => 'apache', :extra => 'dmode=775,fmode=775' + config.vm.customize ["setextradata", :id, "VBoxInternal2/SharedFoldersEnableSymlinksCreate/v-test", "1"] # Enable provisioning with Puppet stand alone. Puppet manifests # are contained in a directory path relative to this Vagrantfile. @@ -56,7 +58,7 @@ Vagrant::Config.run do |config| config.vm.provision :puppet do |puppet| puppet.module_path = ".vagrant-puppet/modules" puppet.manifests_path = ".vagrant-puppet/manifests" - #puppet.options = "-v -d" + # # puppet.options = "-v -d" end # The npm module jquery won't install via puppet because of an mysterious error diff --git a/application/controllers/ModulesController.php b/application/controllers/ModulesController.php index 8e28308ea..1a5dec426 100755 --- a/application/controllers/ModulesController.php +++ b/application/controllers/ModulesController.php @@ -4,21 +4,21 @@ /** * Icinga 2 Web - Head for multiple monitoring frontends * Copyright (C) 2013 Icinga Development Team - * + * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License * as published by the Free Software Foundation; either version 2 * of the License, or (at your option) any later version. - * + * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. - * + * * You should have received a copy of the GNU General Public License * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - * + * * @copyright 2013 Icinga Development Team * @author Icinga Development Team */ @@ -58,16 +58,15 @@ class ModulesController extends ActionController public function overviewAction() { $this->indexAction(); - + } public function enableAction() { $this->manager->enableModule($this->_getParam('name')); $this->manager->loadModule($this->_getParam('name')); - $this->getResponse()->setHeader('X-Icinga-Enable-Module', $this->_getParam('name')); - $this->replaceLayout = true; - $this->indexAction(); + $this->getResponse()->setHeader('X-Icinga-Enable-Module', $this->_getParam('name')); + $this->redirectNow('index?_render=body'); } diff --git a/application/layouts/scripts/parts/navigation.phtml b/application/layouts/scripts/parts/navigation.phtml index bb166d86c..04dbf801e 100755 --- a/application/layouts/scripts/parts/navigation.phtml +++ b/application/layouts/scripts/parts/navigation.phtml @@ -3,7 +3,7 @@ $url = Zend_Controller_Front::getInstance()->getRequest()->getRequestUri(); $currentKey = isset($this->navkey) ? $this->navkey : $url; - $item = $this->navigation->listAll("menu"); + $item = $this->navigation->keys("menu"); ?> auth()->isAuthenticated()): ?>