Extract the transport functions from the CommandPipe

- The CommandPipe class now delegates submission of commands to
  the Transport classes (LocalPipe or SecureShell)
- Added SSH options for non-interactive mode
- Refactored tests

refs #4441

library/Icinga/Protocol/Commandpipe/CommandPipe.php

@@ -30,41 +30,20 @@ namespace Icinga\Protocol\Commandpipe;
 
 use Icinga\Application\Logger as IcingaLogger;
 
+use Icinga\Protocol\Commandpipe\Transport\Transport;
+use Icinga\Protocol\Commandpipe\Transport\LocalPipe;
+use Icinga\Protocol\Commandpipe\Transport\SecureShell;
+
 /**
  * Class CommandPipe
  * @package Icinga\Protocol\Commandpipe
  */
 class CommandPipe
 {
-    /**
-     * @var mixed
-     */
-    private $path;
 
-    /**
+    private $name = "";
-     * @var mixed
-     */
-    private $name;
 
-    /**
+    private $transport = null;
-     * @var bool|mixed
-     */
-    private $user = false;
-
-    /**
-     * @var bool|mixed
-     */
-    private $host = false;
-
-    /**
-     * @var int|mixed
-     */
-    private $port = 22;
-
-    /**
-     * @var string
-     */
-    public $fopen_mode = "w";
 
     /**
      *
@@ -91,16 +70,20 @@ class CommandPipe
      */
     public function __construct(\Zend_Config $config)
     {
-        $this->path = $config->path;
+        $this->getTransportForConfiguration($config);
         $this->name = $config->name;
-        if (isset($config->host)) {
+    }
-            $this->host = $config->host;
+
-        }
+    private function getTransportForConfiguration(\Zend_Config $config, $transport = null)
-        if (isset($config->port)) {
+    {
-            $this->port = $config->port;
+        if ($transport != null) {
-        }
+            $this->transport = $transport;
-        if (isset($config->user)) {
+        } else if (isset($config->host)) {
-            $this->user = $config->user;
+            $this->transport = new SecureShell();
+            $this->transport->setEndpoint($config);
+        } else {
+            $this->transport = new LocalPipe();
+            $this->transport->setEndpoint($config);
         }
     }
 
@@ -110,56 +93,7 @@ class CommandPipe
      */
     public function send($command)
     {
-        if (!$this->host) {
+        $this->transport->send($command);
-            IcingaLogger::debug(
-                "Attempting to send external icinga command $command to local command file {$this->path}"
-            );
-            $file = @fopen($this->path, $this->fopen_mode);
-            if (!$file) {
-                throw new \RuntimeException("Could not open icinga pipe at $file : " . print_r(error_get_last(), true));
-            }
-            fwrite($file, "[" . time() . "] " . $command . PHP_EOL);
-            IcingaLogger::debug('Writing [' . time() . '] ' . $command . PHP_EOL);
-            fclose($file);
-        } else {
-            // send over ssh
-            $retCode = 0;
-            $output = array();
-            IcingaLogger::debug(
-                'Icinga instance is on different host, attempting to send command %s via ssh to %s:%s/%s',
-                $command,
-                $this->host,
-                $this->port,
-                $this->path
-            );
-            $hostConnector = $this->user ? $this->user . "@" . $this->host : $this->host;
-            exec(
-                "ssh $hostConnector -p{$this->port} \"echo '[" . time() . "] "
-                . escapeshellcmd(
-                    $command
-                )
-                . "' > {$this->path}\"",
-                $output,
-                $retCode
-            );
-            IcingaLogger::debug(
-                "$:ssh $hostConnector -p{$this->port} \"echo '[" . time() . "] " . escapeshellcmd(
-                    $command
-                ) . "' > {$this->path}\""
-            );
-            IcingaLogger::debug("Code code %s: %s ", $retCode, $output);
-
-            if ($retCode != 0) {
-                throw new \RuntimeException(
-                    'Could not send command to remote icinga host: '
-                    . implode(
-                        "\n",
-                        $output
-                    )
-                    . " (returncode $retCode)"
-                );
-            }
-        }
     }
 
     /**
@@ -601,4 +535,14 @@ class CommandPipe
             )
         );
     }
+
+    /**
+     * Return the transport handler that handles actual sending of commands
+     *
+     * @return Transport
+     */
+    public function getTransport()
+    {
+        return $this->transport;
+    }
 }

library/Icinga/Protocol/Commandpipe/Transport/LocalPipe.php

@@ -0,0 +1,32 @@
+<?php
+
+namespace Icinga\Protocol\Commandpipe\Transport;
+use Icinga\Application\Logger;
+
+class LocalPipe implements Transport
+{
+    private $path;
+    private $openMode = "w";
+
+    public function setEndpoint(\Zend_Config $config)
+    {
+        $this->path = isset($config->path) ? $config->path : '/usr/local/icinga/var/rw/icinga.cmd';
+    }
+
+    public function send($message)
+    {
+        Logger::debug('Attempting to send external icinga command %s to local command file ', $message, $this->path);
+        $file = @fopen($this->path, $this->openMode);
+        if (!$file) {
+            throw new \RuntimeException('Could not open icinga pipe at $file : ' . print_r(error_get_last(), true));
+        }
+        fwrite($file, '[' . time() . '] ' . $message . PHP_EOL);
+        Logger::debug('Writing [' . time() . '] ' . $message . PHP_EOL);
+        fclose($file);
+    }
+
+    public function setOpenMode($mode)
+    {
+        $this->openMode = $mode;
+    }
+}

library/Icinga/Protocol/Commandpipe/Transport/SecureShell.php

@@ -0,0 +1,60 @@
+<?php
+
+namespace Icinga\Protocol\Commandpipe\Transport;
+
+use Icinga\Application\Logger;
+
+class SecureShell implements Transport
+{
+    private $host = 'localhost';
+    private $path = "/usr/local/icinga/var/rw/icinga.cmd";
+    private $port = 22;
+    private $user = null;
+    private $password = null;
+
+    public function setEndpoint(\Zend_Config $config)
+    {
+        $this->host = isset($config->host) ? $config->host : "localhost";
+        $this->port = isset($config->port) ? $config->port : 22;
+        $this->user = isset($config->user) ? $config->user : null;
+        $this->password = isset($config->password) ? $config->password : null;
+        $this->path = isset($config->path) ? $config->path : "/usr/local/icinga/var/rw/icinga.cmd";
+    }
+
+    public function send($command)
+    {
+        $retCode = 0;
+        $output = array();
+        Logger::debug(
+            'Icinga instance is on different host, attempting to send command %s via ssh to %s:%s/%s',
+            $command,
+            $this->host,
+            $this->port,
+            $this->path
+        );
+        $hostConnector = $this->user ? $this->user . "@" . $this->host : $this->host;
+        exec(
+            'ssh -o BatchMode=yes -o KbdInteractiveAuthentication=no'.$hostConnector.' -p'.$this->port.' "echo \'['. time() .'] '
+            . escapeshellcmd(
+                $command
+            )
+            . '\' > '.$this->path.'" > /dev/null 2> /dev/null & ',
+            $output,
+            $retCode
+        );
+        Logger::debug(
+            'ssh '.$hostConnector.' -p'.$this->port.' "echo \'['. time() .'] '
+            . escapeshellcmd(
+                $command
+            )
+            . '\' > '.$this->path.'"'
+        );
+        Logger::debug("Return code %s: %s ", $retCode, $output);
+
+        if ($retCode != 0) {
+            $msg =  'Could not send command to remote icinga host: '. implode("\n",$output). " (returncode $retCode)";
+            Logger::error($msg);
+            throw new \RuntimeException($msg);
+        }
+    }
+}

library/Icinga/Protocol/Commandpipe/Transport/Transport.php

@@ -0,0 +1,9 @@
+<?php
+
+namespace Icinga\Protocol\Commandpipe\Transport;
+
+interface Transport
+{
+    public function setEndpoint(\Zend_Config $config);
+    public function send($message);
+}

test/php/library/Icinga/Protocol/Commandpipe/CommandPipeLoader.php

@@ -0,0 +1,34 @@
+<?php
+/**
+ * Created by JetBrains PhpStorm.
+ * User: moja
+ * Date: 7/31/13
+ * Time: 1:29 PM
+ * To change this template use File | Settings | File Templates.
+ */
+
+namespace Tests\Icinga\Protocol\Commandpipe;
+
+require_once("./library/Icinga/LibraryLoader.php");
+use Test\Icinga\LibraryLoader;
+
+class CommandPipeLoader extends LibraryLoader {
+
+    public static function requireLibrary()
+    {
+        require_once("Zend/Config.php");
+        require_once("Zend/Log.php");
+        require_once("../../library/Icinga/Application/Logger.php");
+
+        require_once("../../library/Icinga/Protocol/Commandpipe/IComment.php");
+        require_once("../../library/Icinga/Protocol/Commandpipe/Comment.php");
+        require_once("../../library/Icinga/Protocol/Commandpipe/CommandPipe.php");
+        require_once("../../library/Icinga/Protocol/Commandpipe/Acknowledgement.php");
+        require_once("../../library/Icinga/Protocol/Commandpipe/Downtime.php");
+        require_once("../../library/Icinga/Protocol/Commandpipe/PropertyModifier.php");
+        require_once("../../library/Icinga/Protocol/Commandpipe/Exception/InvalidCommandException.php");
+        require_once("../../library/Icinga/Protocol/Commandpipe/Transport/Transport.php");
+        require_once("../../library/Icinga/Protocol/Commandpipe/Transport/SecureShell.php");
+        require_once("../../library/Icinga/Protocol/Commandpipe/Transport/LocalPipe.php");
+    }
+}

test/php/library/Icinga/Protocol/Commandpipe/CommandPipeTest.php

@@ -1,35 +1,44 @@
 <?php
 namespace Tests\Icinga\Protocol\Commandpipe;
 
+require_once(__DIR__.'/CommandPipeLoader.php');
+CommandPipeLoader::requireLibrary();
+
 use Icinga\Protocol\Commandpipe\Comment as Comment;
 use Icinga\Protocol\Commandpipe\Acknowledgement as Acknowledgement;
 use Icinga\Protocol\Commandpipe\Downtime as Downtime;
 use Icinga\Protocol\Commandpipe\Commandpipe as Commandpipe;
 use \Icinga\Protocol\Commandpipe\PropertyModifier as MONFLAG;
 
-require_once("Zend/Config.php");
-require_once("Zend/Log.php");
-require_once("../../library/Icinga/Application/Logger.php");
-
-require_once("../../library/Icinga/Protocol/Commandpipe/IComment.php");
-require_once("../../library/Icinga/Protocol/Commandpipe/Comment.php");
-require_once("../../library/Icinga/Protocol/Commandpipe/CommandPipe.php");
-require_once("../../library/Icinga/Protocol/Commandpipe/Acknowledgement.php");
-require_once("../../library/Icinga/Protocol/Commandpipe/Downtime.php");
-require_once("../../library/Icinga/Protocol/Commandpipe/PropertyModifier.php");
-require_once("../../library/Icinga/Protocol/Commandpipe/Exception/InvalidCommandException.php");
-
 if(!defined("EXTCMD_TEST_BIN"))
     define("EXTCMD_TEST_BIN", "./bin/extcmd_test");
 
+/**
+ * Several tests for the command pipe component
+ *
+ * Uses the helper script extcmd_test, which is basically the extracted command
+ * parser functions from the icinga core
+ *
+ *
+ */
 class CommandPipeTest extends \PHPUnit_Framework_TestCase
 {
-
+    /**
+     * Return the path of the test pipe used in these tests
+     *
+     * @return string
+     */
     public function getPipeName()
     {
         return sys_get_temp_dir()."/icinga_test_pipe";
     }
-    private function getTestPipe()
+
+    /**
+     * Return a @see Icinga\Protocal\CommandPipe\CommandPipe instance set up for the local test pipe
+     *
+     * @return Commandpipe
+     */
+    private function getLocalTestPipe()
     {
         $tmpPipe = $this->getPipeName();
         $this->cleanup();
@@ -45,26 +54,76 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase
         return $pipe;
     }
 
+    /**
+     * Return a @see Icinga\Protocal\CommandPipe\CommandPipe instance set up for the local test pipe, but with ssh as the transport layer
+     *
+     * @return Commandpipe
+     */
+    private function getSSHTestPipe()
+    {
+        $tmpPipe = $this->getPipeName();
+        $this->cleanup();
+        touch($tmpPipe);
+
+        $cfg = new \Zend_Config(array(
+            "path" => $tmpPipe,
+            "user"  => "vagrant",
+            "password" => "vagrant",
+            "host" => 'localhost',
+            "port"  => 22,
+            "name" => "test"
+        ));
+        $comment = new Comment("Autor","Comment");
+        $pipe = new Commandpipe($cfg);
+
+        return $pipe;
+    }
+
+    /**
+     * Remove the testpipe if it exists
+     *
+     */
     private function cleanup() {
         if(file_exists($this->getPipeName()))
             unlink($this->getPipeName());
     }
 
+    /**
+     * Query the extcmd_test script with $command or the command pipe and test whether the result is $exceptedString and
+     * has a return code of 0.
+     *
+     * Note:
+     * - if no string is given, only the return code is tested
+     * - if no command is given, the content of the test commandpipe is used
+     *
+     * This helps testing whether commandpipe serialization works correctly
+     *
+     * @param bool $expectedString      The string that is expected to be returned from the extcmd_test binary
+     *                                  (optional, leave it to just test for the return code)
+     * @param bool $command             The commandstring to send (optional, leave it for using the command pipe content)
+     */
     private function assertCommandSucceeded($expectedString = false,$command = false) {
         $resultCode = null;
         $resultArr = array();
         $receivedCmd = exec(EXTCMD_TEST_BIN." ".escapeshellarg($command ? $command : file_get_contents($this->getPipeName())),$resultArr,$resultCode);
-        $this->assertEquals(0,$resultCode,"Submit of external command returned error : ".$receivedCmd);
+        $this->assertEquals(0, $resultCode, "Submit of external command returned error : ".$receivedCmd);
         if (!$expectedString)
            return;
         $this->assertEquals(
             $expectedString,
-            $receivedCmd
+            $receivedCmd,
+            'Asserting that the command icinga received matches the command we send'
         );
     }
+
+    /**
+     * Test whether a single host acknowledgment is serialized and send correctly
+     *
+     * @throws \Exception|Exception
+     */
     public function testAcknowledgeSingleHost()
     {
-        $pipe = $this->getTestPipe();
+        $pipe = $this->getLocalTestPipe();
         try {
             $ack = new Acknowledgement(new Comment("I can","sends teh ack"));
             $pipe->acknowledge(array(
@@ -80,12 +139,17 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase
         $this->cleanup();
     }
 
+    /**
+     * Test whether multiple host and service acknowledgments are serialized and send correctly
+     *
+     * @throws \Exception|Exception
+     */
     public function testAcknowledgeMultipleObjects()
     {
-        $pipe = $this->getTestPipe();
+        $pipe = $this->getLocalTestPipe();
         try {
             $ack = new Comment("I can","sends teh ack");
-            $pipe->fopen_mode = "a";
+            $pipe->getTransport()->setOpenMode("a");
             $pipe->acknowledge(array(
                 (object) array(
                     "host_name" => "hostA"
@@ -100,8 +164,7 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase
             ),$ack);
 
             $result = explode("\n",file_get_contents($this->getPipeName()));
-
+            $this->assertCount(5, $result, "Asserting the correct number of commands being written to the command pipe");
-            $this->assertCount(5,$result);
 
             $this->assertCommandSucceeded("ACKNOWLEDGE_HOST_PROBLEM;hostA;0;0;0;I can;sends teh ack",$result[0]);
             $this->assertCommandSucceeded("ACKNOWLEDGE_HOST_PROBLEM;hostB;0;0;0;I can;sends teh ack",$result[1]);
@@ -115,9 +178,14 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase
         $this->cleanup();
     }
 
+    /**
+     * Test whether a single host comment is correctly serialized and send to the command pipe
+     *
+     * @throws \Exception|Exception
+     */
     public function testAddHostComment()
     {
-        $pipe = $this->getTestPipe();
+        $pipe = $this->getLocalTestPipe();
         try {
             $pipe->addComment(array((object) array("host_name" => "hostA")),
                 new Comment("Autor","Comment")
@@ -130,9 +198,14 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase
         $this->cleanup();
     }
 
+    /**
+     * Test whether removing all hostcomments is correctly serialized and send to the command pipe
+     *
+     * @throws \Exception|Exception
+     */
     public function testRemoveAllHostComment()
     {
-        $pipe = $this->getTestPipe();
+        $pipe = $this->getLocalTestPipe();
         try {
             $pipe->removeComment(array(
                 (object) array(
@@ -147,9 +220,14 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase
         $this->cleanup();
     }
 
+    /**
+     * Test whether removing a single host comment is correctly serialized and send to the command pipe
+     *
+     * @throws \Exception|Exception
+     */
     public function testRemoveSpecificComment()
     {
-        $pipe = $this->getTestPipe();
+        $pipe = $this->getLocalTestPipe();
         try {
             $pipe->removeComment(array((object) array("comment_id"=>34,"host_name"=>"test")));
             $this->assertCommandSucceeded("DEL_HOST_COMMENT;34");
@@ -160,11 +238,16 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase
         $this->cleanup();
     }
 
+    /**
+     * Test whether a multiple reschedules for services and hosts are correctly serialized and send to the commandpipe
+     *
+     * @throws \Exception|Exception
+     */
     public function testScheduleChecks()
     {
-        $pipe = $this->getTestPipe();
+        $pipe = $this->getLocalTestPipe();
         try {
-            $pipe->fopen_mode = "a"; // append so we have multiple results
+            $pipe->getTransport()->setOpenMode("a"); // append so we have multiple results
             $t = time();
             // normal reschedule
             $pipe->scheduleCheck(array(
@@ -182,7 +265,7 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase
             ),$t,true);
 
             $result = explode("\n",file_get_contents($this->getPipeName()));
-            $this->assertCount(6,$result);
+            $this->assertCount(6,$result, "Asserting a correct number of commands being written to the commandpipe");
 
             $this->assertCommandSucceeded("SCHEDULE_HOST_CHECK;test;".$t,$result[0]);
             $this->assertCommandSucceeded("SCHEDULE_SVC_CHECK;test;svc1;".$t,$result[1]);
@@ -198,11 +281,16 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase
         $this->cleanup();
     }
 
+    /**
+     * Test whether modifying monitoringflags of a host and service is correctly serialized and send to the command pipe
+     *
+     * @throws \Exception|Exception
+     */
     public function testObjectStateModifications()
     {
-        $pipe = $this->getTestPipe();
+        $pipe = $this->getLocalTestPipe();
         try {
-            $pipe->fopen_mode = "a";
+            $pipe->getTransport()->setOpenMode("a");
             $pipe->setMonitoringProperties(array(
                     (object) array(
                         "host_name" => "Testhost"
@@ -223,7 +311,7 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase
 
             $result = explode("\n",file_get_contents($this->getPipeName()));
             array_pop($result); // remove empty last line
-            $this->assertCount(12,$result);
+            $this->assertCount(12,$result, "Asserting a correct number of commands being written to the commandpipe");
             foreach ($result as $command) {
                 $this->assertCommandSucceeded(false,$command);
             }
@@ -235,9 +323,14 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase
         $this->cleanup();
     }
 
+    /**
+     * Test whether enabling and disabling global notifications are send correctly to the pipe
+     *
+     * @throws \Exception|Exception
+     */
     public function testGlobalNotificationTrigger()
     {
-        $pipe = $this->getTestPipe();
+        $pipe = $this->getLocalTestPipe();
         try {
             $pipe->enableGlobalNotifications();
             $this->assertCommandSucceeded("ENABLE_NOTIFICATIONS;");
@@ -250,9 +343,14 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase
         $this->cleanup();
     }
 
+    /**
+     * Test whether host and servicedowntimes are correctly scheduled
+     *
+     * @throws \Exception|Exception
+     */
     public function testScheduleDowntime()
     {
-        $pipe = $this->getTestPipe();
+        $pipe = $this->getLocalTestPipe();
         try {
             $downtime = new Downtime(25,26,new Comment("me","test"));
             $pipe->scheduleDowntime(array(
@@ -277,11 +375,16 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase
         $this->cleanup();
     }
 
+    /**
+     * Test whether the removal of downtimes is correctly serialized and send to the commandpipe for hosts and services
+     *
+     * @throws \Exception|Exception
+     */
     public function testRemoveDowntime()
     {
-        $pipe = $this->getTestPipe();
+        $pipe = $this->getLocalTestPipe();
         try {
-            $pipe->fopen_mode = "a";
+            $pipe->getTransport()->setOpenMode("a");
             $pipe->removeDowntime(array(
                 (object) array(
                     "host_name" => "Testhost"
@@ -298,7 +401,7 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase
             ));
             $result = explode("\n",file_get_contents($this->getPipeName()));
             array_pop($result); // remove empty last line
-            $this->assertCount(3,$result);
+            $this->assertCount(3,$result, "Asserting a correct number of commands being written to the commandpipe");
             $this->assertCommandSucceeded("DEL_DOWNTIME_BY_HOST_NAME;Testhost",$result[0]);
             $this->assertCommandSucceeded("DEL_DOWNTIME_BY_HOST_NAME;host;svc",$result[1]);
             $this->assertCommandSucceeded("DEL_SVC_DOWNTIME;123",$result[2]);
@@ -310,4 +413,30 @@ class CommandPipeTest extends \PHPUnit_Framework_TestCase
         $this->cleanup();
     }
 
+    /**
+     * Test sending of commands via SSH (currently disabled)
+     *
+     * @throws \Exception|Exception
+     */
+    public function testSSHCommands()
+    {
+        $this->markTestSkipped("This test assumes running in a vagrant VM with key-auth");
+
+        if (!is_dir("/vagrant")) {
+        }
+        $pipe = $this->getSSHTestPipe();
+        try {
+            $ack = new Acknowledgement(new Comment("I can","sends teh ack"));
+            $pipe->acknowledge(array(
+                (object) array(
+                    "host_name" => "hostA"
+                )
+            ),$ack);
+            $this->assertCommandSucceeded("ACKNOWLEDGE_HOST_PROBLEM;hostA;0;0;0;I can;sends teh ack");
+        } catch(Exception $e) {
+            $this->cleanup();
+            throw $e;
+        }
+        $this->cleanup();
+    }
 }