From 64f29b2fc6b63b52a88bbf39efb9948b79e72de0 Mon Sep 17 00:00:00 2001
From: Ad Schellevis <ad@opnsense.org>
Date: Thu, 12 Nov 2020 16:02:31 +0100
Subject: [PATCH] FRR: uniform logging, ditch old file log. closes
 https://github.com/opnsense/plugins/issues/2104

---
 .../Quagga/Api/DiagnosticsController.php      |  4 --
 .../OPNsense/Quagga/DiagnosticsController.php |  4 --
 .../OPNsense/Quagga/forms/general.xml         | 16 +-----
 .../app/models/OPNsense/Quagga/ACL/ACL.xml    |  1 +
 .../app/models/OPNsense/Quagga/General.xml    | 23 +-------
 .../app/models/OPNsense/Quagga/Menu/Menu.xml  |  2 +-
 .../OPNsense/Quagga/Migrations/M1_0_2.php     | 53 +++++++++++++++++++
 .../mvc/app/views/OPNsense/Quagga/log.volt    | 27 ----------
 net/frr/src/opnsense/scripts/quagga/quagga.rb | 11 ----
 .../conf/actions.d/actions_quagga.conf        |  6 ---
 .../templates/OPNsense/Quagga/bgpd.conf       |  3 --
 .../templates/OPNsense/Quagga/ospf6d.conf     |  3 --
 .../templates/OPNsense/Quagga/ospfd.conf      |  3 --
 .../templates/OPNsense/Quagga/ripd.conf       |  3 --
 .../templates/OPNsense/Quagga/zebra.conf      |  3 --
 .../OPNsense/Syslog/local/routing_frr.conf    |  6 +++
 16 files changed, 65 insertions(+), 103 deletions(-)
 create mode 100644 net/frr/src/opnsense/mvc/app/models/OPNsense/Quagga/Migrations/M1_0_2.php
 delete mode 100644 net/frr/src/opnsense/mvc/app/views/OPNsense/Quagga/log.volt
 create mode 100644 net/frr/src/opnsense/service/templates/OPNsense/Syslog/local/routing_frr.conf

diff --git a/net/frr/src/opnsense/mvc/app/controllers/OPNsense/Quagga/Api/DiagnosticsController.php b/net/frr/src/opnsense/mvc/app/controllers/OPNsense/Quagga/Api/DiagnosticsController.php
index 6dd944141..a6d8da8f4 100644
--- a/net/frr/src/opnsense/mvc/app/controllers/OPNsense/Quagga/Api/DiagnosticsController.php
+++ b/net/frr/src/opnsense/mvc/app/controllers/OPNsense/Quagga/Api/DiagnosticsController.php
@@ -129,10 +129,6 @@ class DiagnosticsController extends ApiControllerBase
     {
         return $this->get_general_information('routes');
     }
-    public function logAction()
-    {
-        return $this->get_general_information('log')['response']['general_log'];
-    }
     public function generalroutes6Action()
     {
         return $this->get_general_information('routes6');
diff --git a/net/frr/src/opnsense/mvc/app/controllers/OPNsense/Quagga/DiagnosticsController.php b/net/frr/src/opnsense/mvc/app/controllers/OPNsense/Quagga/DiagnosticsController.php
index 0c0ccd21b..4b7431184 100644
--- a/net/frr/src/opnsense/mvc/app/controllers/OPNsense/Quagga/DiagnosticsController.php
+++ b/net/frr/src/opnsense/mvc/app/controllers/OPNsense/Quagga/DiagnosticsController.php
@@ -44,8 +44,4 @@ class DiagnosticsController extends \OPNsense\Base\IndexController
     {
         $this->view->pick('OPNsense/Quagga/diagnosticsgeneral');
     }
-    public function logAction()
-    {
-        $this->view->pick('OPNsense/Quagga/log');
-    }
 }
diff --git a/net/frr/src/opnsense/mvc/app/controllers/OPNsense/Quagga/forms/general.xml b/net/frr/src/opnsense/mvc/app/controllers/OPNsense/Quagga/forms/general.xml
index fbb34f396..2fcfbc2b9 100644
--- a/net/frr/src/opnsense/mvc/app/controllers/OPNsense/Quagga/forms/general.xml
+++ b/net/frr/src/opnsense/mvc/app/controllers/OPNsense/Quagga/forms/general.xml
@@ -11,27 +11,15 @@
         <type>checkbox</type>
         <help>This will activate the routing service only on the master device.</help>
     </field>
-    <field>
-        <id>general.enablelogfile</id>
-        <label>Create a logfile</label>
-        <type>checkbox</type>
-        <help>If you check this, a log file will be written to disk.</help>
-    </field>
-    <field>
-        <id>general.logfilelevel</id>
-        <label>Logfile level</label>
-        <type>dropdown</type>
-        <help>This is the detail level of the log. A higher level means more data is logged.</help>
-    </field>
     <field>
         <id>general.enablesyslog</id>
-        <label>Send log messages to syslog</label>
+        <label>Enable logging</label>
         <type>checkbox</type>
         <help>Syslog is a service which is made to collect log messages from different software and maybe to a central logging server. Check this box if you have such a setup.</help>
     </field>
     <field>
         <id>general.sysloglevel</id>
-        <label>Syslog level</label>
+        <label>log level</label>
         <type>dropdown</type>
         <help>This is the detail level of the log. A higher level means more data is logged.</help>
     </field>
diff --git a/net/frr/src/opnsense/mvc/app/models/OPNsense/Quagga/ACL/ACL.xml b/net/frr/src/opnsense/mvc/app/models/OPNsense/Quagga/ACL/ACL.xml
index 3fa7a13e5..a99328b13 100644
--- a/net/frr/src/opnsense/mvc/app/models/OPNsense/Quagga/ACL/ACL.xml
+++ b/net/frr/src/opnsense/mvc/app/models/OPNsense/Quagga/ACL/ACL.xml
@@ -4,6 +4,7 @@
       <patterns>
          <pattern>ui/quagga/*</pattern>
          <pattern>api/quagga/*</pattern>
+         <pattern>ui/diagnostics/log/routing/frr</pattern>
       </patterns>
    </page-routing>
 </acl>
diff --git a/net/frr/src/opnsense/mvc/app/models/OPNsense/Quagga/General.xml b/net/frr/src/opnsense/mvc/app/models/OPNsense/Quagga/General.xml
index dc5530304..80d9576eb 100644
--- a/net/frr/src/opnsense/mvc/app/models/OPNsense/Quagga/General.xml
+++ b/net/frr/src/opnsense/mvc/app/models/OPNsense/Quagga/General.xml
@@ -1,7 +1,7 @@
 <model>
     <mount>//OPNsense/quagga/general</mount>
     <description>Quagga Routing configuration</description>
-    <version>1.0.1</version>
+    <version>1.0.2</version>
     <items>
         <enabled type="BooleanField">
             <default>0</default>
@@ -11,27 +11,8 @@
             <default>0</default>
             <Required>Y</Required>
         </enablecarp>
-        <enablelogfile type="BooleanField">
-            <default>0</default>
-            <Required>Y</Required>
-        </enablelogfile>
-        <logfilelevel type="OptionField">
-            <Required>Y</Required>
-            <multiple>N</multiple>
-            <default>notifications</default>
-            <OptionValues>
-                <critical>Critical</critical>
-                <emergencies>Emergencies</emergencies>
-                <errors>Errors</errors>
-                <alerts>Alerts</alerts>
-                <warnings>Warnings</warnings>
-                <notifications>Notifications</notifications>
-                <informational>Informational</informational>
-                <debugging>Debugging</debugging>
-            </OptionValues>
-        </logfilelevel>
         <enablesyslog type="BooleanField">
-            <default>0</default>
+            <default>1</default>
             <Required>Y</Required>
         </enablesyslog>
         <sysloglevel type="OptionField">
diff --git a/net/frr/src/opnsense/mvc/app/models/OPNsense/Quagga/Menu/Menu.xml b/net/frr/src/opnsense/mvc/app/models/OPNsense/Quagga/Menu/Menu.xml
index d7da7259a..03dca32de 100644
--- a/net/frr/src/opnsense/mvc/app/models/OPNsense/Quagga/Menu/Menu.xml
+++ b/net/frr/src/opnsense/mvc/app/models/OPNsense/Quagga/Menu/Menu.xml
@@ -11,7 +11,7 @@
       <OSPF VisibleName="OSPF" url="/ui/quagga/diagnostics/ospf" order="20" />
       <OSPFv3 VisibleName="OSPFv3" url="/ui/quagga/diagnostics/ospfv3" order="25" />
       <BGP VisibleName="BGPv4" url="/ui/quagga/diagnostics/bgp" order="40" />
-      <LOG VisibleName="Log" url="/ui/quagga/diagnostics/log" order="100" />
+      <LOG VisibleName="Log" url="/ui/diagnostics/log/routing/frr" order="100" />
     </Diagnostics>
   </Routing>
 </menu>
diff --git a/net/frr/src/opnsense/mvc/app/models/OPNsense/Quagga/Migrations/M1_0_2.php b/net/frr/src/opnsense/mvc/app/models/OPNsense/Quagga/Migrations/M1_0_2.php
new file mode 100644
index 000000000..f97735288
--- /dev/null
+++ b/net/frr/src/opnsense/mvc/app/models/OPNsense/Quagga/Migrations/M1_0_2.php
@@ -0,0 +1,53 @@
+<?php
+
+/*
+ * Copyright (C) 2020 Deciso B.V.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ *    this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
+ * OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+namespace OPNsense\Quagga\Migrations;
+
+use OPNsense\Core\Config;
+use OPNsense\Base\BaseModelMigration;
+use OPNSense\Quagga\General;
+
+class M1_0_2 extends BaseModelMigration
+{
+    /**
+     * @param Quagga $model
+     */
+    public function run($model)
+    {
+        // XXX: since migrations act per (general) version, we need to check which sub model is calling us.
+        if ($model instanceof General) {
+            $cfgObj = Config::getInstance()->object();
+            if (!empty($cfgObj->OPNsense->quagga->general->enablelogfile)) {
+                if ((string)$model->enablesyslog != "1") {
+                    $model->sysloglevel = (string)$cfgObj->OPNsense->quagga->general->logfilelevel;
+                }
+                $model->enablesyslog = "1";
+            }
+        }
+    }
+}
diff --git a/net/frr/src/opnsense/mvc/app/views/OPNsense/Quagga/log.volt b/net/frr/src/opnsense/mvc/app/views/OPNsense/Quagga/log.volt
deleted file mode 100644
index 6f129beb9..000000000
--- a/net/frr/src/opnsense/mvc/app/views/OPNsense/Quagga/log.volt
+++ /dev/null
@@ -1,27 +0,0 @@
-<div class="content-box">
-<table id="logtable" class="table table-condensed table-hover table-striped" style="table-layout: initial;">
-    <thead>
-        <tr>
-            <th data-column-id="date" data-sortable="false" data-type="string">{{ lang._('Date') }}</th>
-            <th data-column-id="time" data-sortable="false" data-type="string">{{ lang._('Time') }}</th>
-            <th data-column-id="service" data-sortable="false" data-type="string">{{ lang._('Service') }}</th>
-            <th data-column-id="message" data-sortable="false" data-type="string">{{ lang._('Message') }}</th>
-        </tr>
-    </thead>
-</table>
-</div>
-
-<script>
-$(document).ready(function() {
-    updateServiceControlUI('quagga');
-});
-
-$("#logtable").bootgrid({
-    ajax: true,
-    navigation: 0,
-    url: "/api/quagga/diagnostics/log",
-    ajaxSettings: { "method": "GET", cache: true },
-    responseHandler: function(resp) { return {"rows": resp}; },
-    sortable: false
-});
-</script>
diff --git a/net/frr/src/opnsense/scripts/quagga/quagga.rb b/net/frr/src/opnsense/scripts/quagga/quagga.rb
index e8881a4d0..a59427855 100755
--- a/net/frr/src/opnsense/scripts/quagga/quagga.rb
+++ b/net/frr/src/opnsense/scripts/quagga/quagga.rb
@@ -129,14 +129,6 @@ class General
     routes(true)
   end
 
-  def log
-    File.read('/var/log/frr.log').lines.select {|l| l.strip.length > 10}.map do |line|
-      date, time, service, message = line.split(' ', 4)
-      date = date.split('/').reverse.join(".") # format dd.mm.yyyy
-      service = service.split(':').first if service
-      {date: date, time:time, service: service, message: message }
-    end
-  end
 end
 
 class OSPF
@@ -709,9 +701,6 @@ OptionParser.new do |opts|
   opts.on("-6", "--general-routes6", "Print Routing Table (IPv6)") do |od|
     options[:general_routes6] = od
   end
-  opts.on("-l", "--general-log", "Print Logs") do |od|
-    options[:general_log] = od
-  end
   ### BGP
   opts.on("-B", "--bgp-overview", "Print an overview of BGP") do |od|
     options[:bgp_overview] = od
diff --git a/net/frr/src/opnsense/service/conf/actions.d/actions_quagga.conf b/net/frr/src/opnsense/service/conf/actions.d/actions_quagga.conf
index 66a793e04..1b3075f56 100644
--- a/net/frr/src/opnsense/service/conf/actions.d/actions_quagga.conf
+++ b/net/frr/src/opnsense/service/conf/actions.d/actions_quagga.conf
@@ -106,12 +106,6 @@ parameters:
 type:script_output
 message: Print IPv6 Routing Table
 
-[general-log]
-command:/usr/local/opnsense/scripts/quagga/quagga.rb --general-log
-parameters:
-type:script_output
-message: Show Quagga logs
-
 [general-runningconfig]
 command:/usr/local/bin/vtysh -c "show run"
 parameters:
diff --git a/net/frr/src/opnsense/service/templates/OPNsense/Quagga/bgpd.conf b/net/frr/src/opnsense/service/templates/OPNsense/Quagga/bgpd.conf
index abdf1f8d9..e5d05085b 100644
--- a/net/frr/src/opnsense/service/templates/OPNsense/Quagga/bgpd.conf
+++ b/net/frr/src/opnsense/service/templates/OPNsense/Quagga/bgpd.conf
@@ -5,9 +5,6 @@
 !   2017/03/03 20:21:04
 !
 {% if helpers.exists('OPNsense.quagga.general') %}
-{%   if helpers.exists('OPNsense.quagga.general.enablelogfile') and OPNsense.quagga.general.enablelogfile == '1' %}
-log file /var/log/frr.log {{ OPNsense.quagga.general.logfilelevel }}
-{%   endif %}
 {%   if helpers.exists('OPNsense.quagga.general.enablesyslog') and OPNsense.quagga.general.enablesyslog == '1' %}
 log syslog {{ OPNsense.quagga.general.sysloglevel }}
 {%   endif %}
diff --git a/net/frr/src/opnsense/service/templates/OPNsense/Quagga/ospf6d.conf b/net/frr/src/opnsense/service/templates/OPNsense/Quagga/ospf6d.conf
index 607316523..b58d95986 100644
--- a/net/frr/src/opnsense/service/templates/OPNsense/Quagga/ospf6d.conf
+++ b/net/frr/src/opnsense/service/templates/OPNsense/Quagga/ospf6d.conf
@@ -8,9 +8,6 @@
 !   2017/03/03 20:21:04
 !
 {% if helpers.exists('OPNsense.quagga.general') %}
-{%   if helpers.exists('OPNsense.quagga.general.enablelogfile') and OPNsense.quagga.general.enablelogfile == '1' %}
-log file /var/log/frr.log {{ OPNsense.quagga.general.logfilelevel }}
-{%   endif %}
 {%   if helpers.exists('OPNsense.quagga.general.enablesyslog') and OPNsense.quagga.general.enablesyslog == '1' %}
 log syslog {{ OPNsense.quagga.general.sysloglevel }}
 {%   endif %}
diff --git a/net/frr/src/opnsense/service/templates/OPNsense/Quagga/ospfd.conf b/net/frr/src/opnsense/service/templates/OPNsense/Quagga/ospfd.conf
index 8b7729c0e..72c880d87 100644
--- a/net/frr/src/opnsense/service/templates/OPNsense/Quagga/ospfd.conf
+++ b/net/frr/src/opnsense/service/templates/OPNsense/Quagga/ospfd.conf
@@ -8,9 +8,6 @@
 !   2017/03/03 20:21:04
 !
 {% if helpers.exists('OPNsense.quagga.general') %}
-{%   if helpers.exists('OPNsense.quagga.general.enablelogfile') and OPNsense.quagga.general.enablelogfile == '1' %}
-log file /var/log/frr.log {{ OPNsense.quagga.general.logfilelevel }}
-{%   endif %}
 {%   if helpers.exists('OPNsense.quagga.general.enablesyslog') and OPNsense.quagga.general.enablesyslog == '1' %}
 log syslog {{ OPNsense.quagga.general.sysloglevel }}
 {%   endif %}
diff --git a/net/frr/src/opnsense/service/templates/OPNsense/Quagga/ripd.conf b/net/frr/src/opnsense/service/templates/OPNsense/Quagga/ripd.conf
index 6c5756f02..ffb74650e 100644
--- a/net/frr/src/opnsense/service/templates/OPNsense/Quagga/ripd.conf
+++ b/net/frr/src/opnsense/service/templates/OPNsense/Quagga/ripd.conf
@@ -5,9 +5,6 @@
 !   2017/03/26 22:40:16
 !
 {% if helpers.exists('OPNsense.quagga.general') %}
-{%   if helpers.exists('OPNsense.quagga.general.enablelogfile') and OPNsense.quagga.general.enablelogfile == '1' %}
-log file /var/log/frr.log {{ OPNsense.quagga.general.logfilelevel }}
-{%   endif %}
 {%   if helpers.exists('OPNsense.quagga.general.enablesyslog') and OPNsense.quagga.general.enablesyslog == '1' %}
 log syslog {{ OPNsense.quagga.general.sysloglevel }}
 {%   endif %}
diff --git a/net/frr/src/opnsense/service/templates/OPNsense/Quagga/zebra.conf b/net/frr/src/opnsense/service/templates/OPNsense/Quagga/zebra.conf
index 13d12b88c..01a97a6f4 100644
--- a/net/frr/src/opnsense/service/templates/OPNsense/Quagga/zebra.conf
+++ b/net/frr/src/opnsense/service/templates/OPNsense/Quagga/zebra.conf
@@ -3,9 +3,6 @@
 ! Zebra configuration saved from vty
 !   2017/03/03 20:21:04
 !
-{% if helpers.exists('OPNsense.quagga.general.enablelogfile') and OPNsense.quagga.general.enablelogfile == '1' %}
-log file /var/log/frr.log {{ OPNsense.quagga.general.logfilelevel }}
-{% endif %}
 {% if helpers.exists('OPNsense.quagga.general.enablesyslog') and OPNsense.quagga.general.enablesyslog == '1' %}
 log syslog {{ OPNsense.quagga.general.sysloglevel }}
 {% endif %}
diff --git a/net/frr/src/opnsense/service/templates/OPNsense/Syslog/local/routing_frr.conf b/net/frr/src/opnsense/service/templates/OPNsense/Syslog/local/routing_frr.conf
new file mode 100644
index 000000000..84bcfee81
--- /dev/null
+++ b/net/frr/src/opnsense/service/templates/OPNsense/Syslog/local/routing_frr.conf
@@ -0,0 +1,6 @@
+###################################################################
+# Local syslog-ng configuration filter definition [FRR].
+###################################################################
+filter f_local_routing_frr {
+     program("bgpd") or program("ospfd") or program("ospf6d") or program("ripd") or program("zebra");
+};