Optimize imports & cleanup some codes

Icinga · Dec 9, 2022 · 7587a35 · 7587a35
1 parent d2b5622
commit 7587a35
Show file tree

Hide file tree

Showing 10 changed files with 85 additions and 77 deletions.
diff --git a/application/clicommands/CheckCommand.php b/application/clicommands/CheckCommand.php
@@ -6,8 +6,8 @@
 
 use Icinga\Application\Logger;
 use Icinga\Module\X509\Command;
-use Icinga\Module\X509\Job;
 use Icinga\Module\X509\Model\X509Certificate;
+use Icinga\Module\X509\Model\X509CertificateChain;
 use Icinga\Module\X509\Model\X509Target;
 use ipl\Sql\Expression;
 use ipl\Stdlib\Filter;
@@ -89,36 +89,28 @@ public function hostAction()
         ]);
 
         // Sub queries for (valid_from, valid_to) columns
-        $validFrom = X509Certificate::on($conn)
-            ->with(['chain', 'issuer_certificate'])
-            ->columns([
-                new Expression('MAX(GREATEST(%s, %s))', ['valid_from', 'issuer_certificate.valid_from'])
-            ]);
-
-        $validFrom->getResolver()->setAliasPrefix('sub_');
+        $validFrom = $targets->createSubQuery(new X509Certificate(), 'chain.certificate');
         $validFrom
+            ->columns([new Expression('MAX(GREATEST(%s, %s))', ['valid_from', 'issuer_certificate.valid_from'])])
             ->getSelectBase()
-            ->where(new Expression(
-                'sub_certificate_link.certificate_chain_id = target_chain.id'
-            ));
+            ->resetWhere()
+            ->where(new Expression('sub_certificate_link.certificate_chain_id = target_chain.id'));
 
         $validTo = clone $validFrom;
-        $validTo->columns([
-            new Expression('MIN(LEAST(%s, %s))', ['valid_to', 'issuer_certificate.valid_to'])
-        ]);
+        $validTo->columns([new Expression('MIN(LEAST(%s, %s))', ['valid_to', 'issuer_certificate.valid_to'])]);
 
-        list($validFromSelect, $validFromValues) = $validFrom->dump();
-        list($validToSelect, $validToValues) = $validTo->dump();
+        list($validFromSelect, $_) = $validFrom->dump();
+        list($validToSelect, $_) = $validTo->dump();
         $targets
             ->withColumns([
-                'valid_from' => new Expression("$validFromSelect", null, ...$validFromValues),
-                'valid_to'   => new Expression("$validToSelect", null, ...$validToValues)
+                'valid_from' => new Expression($validFromSelect),
+                'valid_to'   => new Expression($validToSelect)
             ])
             ->getSelectBase()
             ->where(new Expression('target_chain_link.order = 0'));
 
         if ($ip !== null) {
-            $targets->filter(Filter::equal('ip', Job::binary($ip)));
+            $targets->filter(Filter::equal('ip', $ip));
         }
         if ($hostname !== null) {
             $targets->filter(Filter::equal('hostname', $hostname));

diff --git a/application/controllers/CertificateController.php b/application/controllers/CertificateController.php
@@ -8,7 +8,6 @@
 use Icinga\Module\X509\CertificateDetails;
 use Icinga\Module\X509\Controller;
 use Icinga\Module\X509\Model\X509Certificate;
-use ipl\Sql;
 use ipl\Stdlib\Filter;
 
 class CertificateController extends Controller

diff --git a/application/controllers/CertificatesController.php b/application/controllers/CertificatesController.php
@@ -62,6 +62,7 @@ public function indexAction()
             } else {
                 $this->addControl($searchBar);
                 $this->sendMultipartUpdate();
+
                 return;
             }
         } else {
@@ -77,14 +78,7 @@ public function indexAction()
         $this->addControl($limitControl);
         $this->addControl($searchBar);
 
-        // List of allowed columns to be exported
-        $exportable = array_flip([
-            'id', 'subject', 'issuer', 'version', 'self_signed', 'ca', 'trusted',
-            'pubkey_algo', 'pubkey_bits', 'signature_algo', 'signature_hash_algo',
-            'valid_from', 'valid_to'
-        ]);
-
-        $this->handleFormatRequest($certificates, function (Query $certificates) use ($exportable) {
+        $this->handleFormatRequest($certificates, function (Query $certificates) {
             /** @var X509Certificate $cert */
             foreach ($certificates as $cert) {
                 $cert['valid_from'] = (new \DateTime())
@@ -94,7 +88,7 @@ public function indexAction()
                     ->setTimestamp($cert['valid_to'])
                     ->format('l F jS, Y H:i:s e');
 
-                yield array_intersect_key(iterator_to_array($cert), $exportable);
+                yield array_intersect_key(iterator_to_array($cert), array_flip($cert->getExportableColumns()));
             }
         });
 
@@ -107,10 +101,11 @@ public function indexAction()
 
     public function completeAction()
     {
-        $suggestions = new ObjectSuggestions();
-        $suggestions->setModel(X509Certificate::class);
-        $suggestions->forRequest($this->getServerRequest());
-        $this->getDocument()->add($suggestions);
+        $this->getDocument()->add(
+            (new ObjectSuggestions())
+                ->setModel(X509Certificate::class)
+                ->forRequest($this->getServerRequest())
+        );
     }
 
     public function searchEditorAction()

diff --git a/application/controllers/UsageController.php b/application/controllers/UsageController.php
@@ -78,6 +78,7 @@ public function indexAction()
             } else {
                 $this->addControl($searchBar);
                 $this->sendMultipartUpdate();
+
                 return;
             }
         } else {
@@ -93,13 +94,7 @@ public function indexAction()
         $this->addControl($limitControl);
         $this->addControl($searchBar);
 
-        $exportable = array_flip([
-            'valid', 'hostname', 'ip', 'port', 'subject', 'issuer', 'version',
-            'self_signed', 'ca', 'trusted', 'pubkey_algo', 'pubkey_bits',
-            'signature_algo', 'signature_hash_algo', 'valid_from', 'valid_to'
-        ]);
-
-        $this->handleFormatRequest($targets, function (Query $targets) use ($conn, $exportable) {
+        $this->handleFormatRequest($targets, function (Query $targets) {
             foreach ($targets as $usage) {
                 $usage['valid_from'] = (new \DateTime())
                     ->setTimestamp($usage['valid_from'])
@@ -113,7 +108,10 @@ public function indexAction()
                 $usage->port = $usage->chain->target->port;
                 $usage->valid = $usage->chain->valid;
 
-                yield array_intersect_key(iterator_to_array($usage), $exportable);
+                yield array_intersect_key(
+                    iterator_to_array($usage),
+                    array_flip(array_merge(['valid', 'hostname', 'ip', 'port'], $usage->getExportableColumns()))
+                );
             }
         });
 
@@ -126,10 +124,11 @@ public function indexAction()
 
     public function completeAction()
     {
-        $suggestions = new ObjectSuggestions();
-        $suggestions->setModel(X509Certificate::class);
-        $suggestions->forRequest($this->getServerRequest());
-        $this->getDocument()->add($suggestions);
+        $this->getDocument()->add(
+            (new ObjectSuggestions())
+                ->setModel(X509Certificate::class)
+                ->forRequest($this->getServerRequest())
+        );
     }
 
     public function searchEditorAction()

diff --git a/library/X509/CertificateDetails.php b/library/X509/CertificateDetails.php
@@ -5,9 +5,9 @@
 namespace Icinga\Module\X509;
 
 use DateTime;
+use Icinga\Module\X509\Model\X509Certificate;
 use ipl\Html\BaseHtmlElement;
 use ipl\Html\Html;
-use ipl\Orm\Model;
 
 /**
  * Widget to display X.509 certificate details
@@ -23,7 +23,7 @@ class CertificateDetails extends BaseHtmlElement
      */
     protected $cert;
 
-    public function setCert(Model $cert)
+    public function setCert(X509Certificate $cert)
     {
         $this->cert = $cert;
 

diff --git a/library/X509/DataTable.php b/library/X509/DataTable.php
@@ -84,7 +84,7 @@ protected function renderRow($row)
         $cells = [];
 
         foreach ($this->columns as $key => $column) {
-            if (! is_int($key) && isset($row->$key)) {
+            if (! is_int($key) && property_exists($row, $key)) {
                 $data = $row[$key];
             } else {
                 $data = null;

diff --git a/library/X509/Job.php b/library/X509/Job.php
@@ -199,7 +199,7 @@ protected function getScanTargets(): Generator
                 // The filter processor doesn't allow us to filter using expressions
                 $targets
                     ->getSelectBase()
-                    ->where(new Expression('last_scan < %d', [$this->sinceLastScan->getTimestamp()]));
+                    ->where(['last_scan < ?' => $this->sinceLastScan->getTimestamp()]);
             }
 
             foreach ($targets as $target) {
@@ -438,10 +438,10 @@ protected function processChain($target, $chain)
         $this->db->transaction(function () use ($target, $chain) {
             $row = X509Target::on($this->db)->columns(['id']);
 
-            $filter = Filter::all();
-            $filter->add(Filter::equal('ip', static::binary($target->ip)));
-            $filter->add(Filter::equal('port', $target->port));
-            $filter->add(Filter::equal('hostname', $target->hostname));
+            $filter = Filter::all()
+                ->add(Filter::equal('ip', $target->ip))
+                ->add(Filter::equal('port', $target->port))
+                ->add(Filter::equal('hostname', $target->hostname));
 
             $row->filter($filter);
 
@@ -463,13 +463,14 @@ protected function processChain($target, $chain)
 
             $chainUptodate = false;
 
-            $lastChain = X509CertificateChain::on($this->db)->columns(['id']);
-            $lastChain
+            $lastChain = X509CertificateChain::on($this->db)
+                ->columns(['id'])
                 ->filter(Filter::equal('target_id', $targetId))
                 ->orderBy('id', SORT_DESC)
-                ->limit(1);
+                ->limit(1)
+                ->first();
 
-            if (($lastChain = $lastChain->first())) {
+            if ($lastChain) {
                 $lastFingerprints = X509Certificate::on($this->db)->utilize('chain');
                 $lastFingerprints
                     ->columns(['fingerprint'])
@@ -495,7 +496,7 @@ protected function processChain($target, $chain)
             }
 
             if ($chainUptodate) {
-                $chainId = (int) $lastChain->id;
+                $chainId = $lastChain->id;
             } else {
                 // TODO: https://github.com/Icinga/ipl-orm/pull/78
                 $this->db->insert(

diff --git a/library/X509/Model/X509Certificate.php b/library/X509/Model/X509Certificate.php
@@ -86,6 +86,30 @@ public function getSearchColumns()
         return ['subject', 'issuer'];
     }
 
+    /**
+     * Get list of allowed columns to be exported
+     *
+     * @return string[]
+     */
+    public function getExportableColumns(): array
+    {
+        return [
+            'id',
+            'subject',
+            'issuer',
+            'version',
+            'self_signed',
+            'ca',
+            'trusted',
+            'pubkey_algo',
+            'pubkey_bits',
+            'signature_algo',
+            'signature_hash_algo',
+            'valid_from',
+            'valid_to'
+        ];
+    }
+
     public function createBehaviors(Behaviors $behaviors)
     {
         $behaviors->add(new Binary([

diff --git a/library/X509/ProvidedHook/HostsImportSource.php b/library/X509/ProvidedHook/HostsImportSource.php
@@ -4,11 +4,9 @@
 
 namespace Icinga\Module\X509\ProvidedHook;
 
-use Icinga\Module\X509\DbTool;
 use Icinga\Module\X509\Job;
 use Icinga\Module\X509\Model\X509Target;
 use ipl\Sql;
-use ipl\Stdlib\Filter;
 
 class HostsImportSource extends X509ImportSource
 {
@@ -29,11 +27,11 @@ public function fetchData()
 
         if ($this->getDb()->getAdapter() instanceof Sql\Adapter\Pgsql) {
             $targets->withColumns([
-                'host_ports' => new Sql\Expression('ARRAY_TO_STRING(ARRAY_AGG(DISTINCT port),  \',\')')
+                'host_ports' => new Sql\Expression("ARRAY_TO_STRING(ARRAY_AGG(DISTINCT port), ',')")
             ]);
         } else {
             $targets->withColumns([
-                'host_ports' => new Sql\Expression('GROUP_CONCAT(DISTINCT port SEPARATOR \',\')')
+                'host_ports' => new Sql\Expression("GROUP_CONCAT(DISTINCT port SEPARATOR ',')")
             ]);
         }
 
@@ -60,12 +58,12 @@ public function fetchData()
                 $target->host_name_or_ip = $target->host_ip;
             }
 
-            unset($target->ip); // Isn't needed any more!!
-            unset($target->chain); // We don't need any relation properties anymore
+            // Target ip is now obsolete and must not be included in the results.
+            // The relation is only used to utilize the query and must not be in the result set as well.
+            unset($target->ip);
+            unset($target->chain);
 
-            $properties = iterator_to_array($target);
-
-            $results[$target->host_name_or_ip] = (object) $properties;
+            $results[$target->host_name_or_ip] = (object) iterator_to_array($target);
         }
 
         return $results;

diff --git a/library/X509/ProvidedHook/ServicesImportSource.php b/library/X509/ProvidedHook/ServicesImportSource.php
@@ -50,11 +50,11 @@ public function fetchData()
         if ($this->getDb()->getAdapter() instanceof Sql\Adapter\Pgsql) {
             $targets
                 ->withColumns([
-                    'cert_fingerprint' => new Sql\Expression('ENCODE(%s, \'hex\')', [
+                    'cert_fingerprint' => new Sql\Expression("ENCODE(%s, 'hex')", [
                         'chain.certificate.fingerprint'
                     ]),
                     'cert_dn'          => new Sql\Expression(
-                        'ARRAY_TO_STRING(ARRAY_AGG(CONCAT(%s, \'=\', %s)), \',\')',
+                        "ARRAY_TO_STRING(ARRAY_AGG(CONCAT(%s, '=', %s)), ',')",
                         [
                             'chain.certificate.dn.key',
                             'chain.certificate.dn.value'
@@ -65,13 +65,13 @@ public function fetchData()
                 ->groupBy(['target_chain_certificate.id', 'target_chain_certificate_issuer_certificate.id']);
 
             $certAltName->columns([
-                new Sql\Expression('ARRAY_TO_STRING(ARRAY_AGG(CONCAT(%s, \':\', %s)), \',\')', ['type', 'value'])
+                new Sql\Expression("ARRAY_TO_STRING(ARRAY_AGG(CONCAT(%s, ':', %s)), ',')", ['type', 'value'])
             ]);
         } else {
             $targets->withColumns([
                 'cert_fingerprint' => new Sql\Expression('HEX(%s)', ['chain.certificate.fingerprint']),
                 'cert_dn'          => new Sql\Expression(
-                    'GROUP_CONCAT(CONCAT(%s, \'=\', %s) SEPARATOR \',\')',
+                    "GROUP_CONCAT(CONCAT(%s, '=', %s) SEPARATOR ',')",
                     [
                         'chain.certificate.dn.key',
                         'chain.certificate.dn.value'
@@ -80,7 +80,7 @@ public function fetchData()
             ]);
 
             $certAltName->columns([
-                new Sql\Expression('GROUP_CONCAT(CONCAT(%s, \':\', %s) SEPARATOR \',\')', ['type', 'value'])
+                new Sql\Expression("GROUP_CONCAT(CONCAT(%s, ':', %s) SEPARATOR ',')", ['type', 'value'])
             ]);
         }
 
@@ -101,12 +101,12 @@ public function fetchData()
                 $target->host_port
             );
 
-            unset($target->ip); // Isn't needed any more!!
-            unset($target->chain); // We don't need any relation properties anymore
+            // Target ip is now obsolete and must not be included in the results.
+            // The relation is only used to utilize the query and must not be in the result set as well.
+            unset($target->ip);
+            unset($target->chain);
 
-            $properties = iterator_to_array($target);
-
-            $results[$target->host_name_ip_and_port] = (object) $properties;
+            $results[$target->host_name_ip_and_port] = (object) iterator_to_array($target);
         }
 
         return $results;