Hello Alexander,
Thank you very much for submitting this patch.
I guess that you have been involved in the forum thread about getting this thing into IPFire. Happy to see that that is now making its way to the list.
However, there are some issues with the patch:
First of all, some lines are wrapped (presumably by your email program). You can see this at the end of the patch where the #s are not in the same line any more.
Therefore it won’t merge.
Also it is quite a large patch and could have been broken down into smaller parts to make it easier to review it.
I will go through the rest inline...
On 3 Feb 2019, at 19:37, Alexander Koch ipfire@starkstromkonsument.de wrote:
Hello,
I would like to contribute a new addon for monitoring hosts running IPFire by Zabbix Monitoring (https://www.zabbix.com/features) to IPFire. Topic in the forum: https://forum.ipfire.org/viewtopic.php?f=52&t=22039
I'm not a professional software developer and this is going to be my first patch for IPFire. I hope I did not make any stupid mistakes and I'm not wasting you're time. I've built and tested (only for/on x86_64) this package for/with core126, core127 (testing) and core128 (Development Build: zabbix_agentd/b72540bc) so far.
Before I finally submit this as a Patch, I've got two questions I could not figure out reading the wiki/forum:
1: How are logfiles (/var/log/zabbix) supposed to be treated by the backup- and uninstall-scripts of an addon? Are logs supposed to be included in the addon-backup? Is the log-directory supposed to be deleted by the uninstall.sh of the addon? If I do not include them in the backup, but delete the log-directory in uninstall.sh, the logs will be flushed on every update of the addon. This is probably not what the users expects to happen.
2: How is the original source-code of zabbix (https://www.zabbix.com/download_sources) supposed to be shipped with the patch? A patch only includes the lfs, config etc. and I did not find a place to provide a download URL for it. Did I miss something?
Best, Alex
P.S. Just in case you want to check what I achieved so far, I attached my current patchfile below:
Subject: [PATCH] zabbix_agentd: New addon for monitoring IPFire Hosts by Zabbix Monitoring (https://www.zabbix.com/features). See https://forum.ipfire.org/viewtopic.php?f=52&t=22039 for further details.
Signed-off-by: Alexander Koch ipfire@starkstromkonsument.de
config/backup/includes/zabbix_agentd | 3 + config/rootfiles/packages/zabbix_agentd | 21 ++ config/zabbix_agentd/logrotate | 9 + config/zabbix_agentd/pakfire_updates.pl | 100 ++++++ config/zabbix_agentd/sudoers | 17 + config/zabbix_agentd/userparameter_pakfire.conf | 4 + config/zabbix_agentd/zabbix_agentd.conf | 394 ++++++++++++++++++++++++ lfs/zabbix_agentd | 128 ++++++++ make.sh | 1 + src/initscripts/packages/zabbix_agentd | 61 ++++ src/paks/zabbix_agentd/install.sh | 45 +++ src/paks/zabbix_agentd/uninstall.sh | 38 +++ src/paks/zabbix_agentd/update.sh | 26 ++ 13 files changed, 847 insertions(+) create mode 100644 config/backup/includes/zabbix_agentd create mode 100644 config/rootfiles/packages/zabbix_agentd create mode 100644 config/zabbix_agentd/logrotate create mode 100644 config/zabbix_agentd/pakfire_updates.pl create mode 100644 config/zabbix_agentd/sudoers create mode 100644 config/zabbix_agentd/userparameter_pakfire.conf create mode 100644 config/zabbix_agentd/zabbix_agentd.conf create mode 100755 lfs/zabbix_agentd create mode 100755 src/initscripts/packages/zabbix_agentd create mode 100644 src/paks/zabbix_agentd/install.sh create mode 100644 src/paks/zabbix_agentd/uninstall.sh create mode 100644 src/paks/zabbix_agentd/update.sh
diff --git a/config/backup/includes/zabbix_agentd b/config/backup/includes/zabbix_agentd new file mode 100644 index 0000000..d6a2b49 --- /dev/null +++ b/config/backup/includes/zabbix_agentd @@ -0,0 +1,3 @@ +/etc/sudoers.d/zabbix +/etc/zabbix/zabbix_agentd.* +/etc/zabbix/scripts
I would say that /etc/sudoers.d/zabbix is not a configuration file for the user here and therefore should not be in the backup. It is a system configuration file that comes with the package.
Shouldn’t the whole /etc/zabbix directory be in the backup?
diff --git a/config/rootfiles/packages/zabbix_agentd b/config/rootfiles/packages/zabbix_agentd new file mode 100644 index 0000000..f12c46d --- /dev/null +++ b/config/rootfiles/packages/zabbix_agentd @@ -0,0 +1,21 @@ +#etc/group-
This file should not be in here and probably this is a mistake.
+etc/logrotate.d/zabbix_agentd +etc/rc.d/init.d/zabbix_agentd +etc/sudoers.d/zabbix +#etc/zabbix +#etc/zabbix/scripts +etc/zabbix/scripts/pakfire_updates.pl +etc/zabbix/zabbix_agentd.conf +#etc/zabbix/zabbix_agentd.conf.d +#etc/zabbix/zabbix_agentd.d +etc/zabbix/zabbix_agentd.d/userparameter_pakfire.conf +etc/zabbix/zabbix_agentd.psk +usr/bin/zabbix_get +usr/bin/zabbix_sender +#usr/lib/modules
This also does not seem to be a very well named directory.
+usr/sbin/zabbix_agentd +#usr/share/man/man1/zabbix_get.1 +#usr/share/man/man1/zabbix_sender.1 +#usr/share/man/man8/zabbix_agentd.8 +var/ipfire/backup/addons/includes/zabbix_agentd +#var/log/zabbix
The log directory should probably be shipped in this package.
diff --git a/config/zabbix_agentd/logrotate b/config/zabbix_agentd/logrotate new file mode 100644 index 0000000..83bbca9 --- /dev/null +++ b/config/zabbix_agentd/logrotate @@ -0,0 +1,9 @@ +/var/log/zabbix/zabbix_agentd.log {
- monthly
- rotate 12
- compress
- delaycompress
- missingok
- notifempty
- create 0640 zabbix zabbix
+}
Does the daemon not need to be notified when the log file is being rotated?
diff --git a/config/zabbix_agentd/pakfire_updates.pl b/config/zabbix_agentd/pakfire_updates.pl new file mode 100644 index 0000000..875df40 --- /dev/null +++ b/config/zabbix_agentd/pakfire_updates.pl @@ -0,0 +1,100 @@ +#!/usr/bin/perl +# +# Script for fetching available updates and "need reboot"-status for userparameter of zabbix_agentd +# +# This script is based on /opt/pakfire/lib/functions.pl +# +# Created on 09.07.2017 by Alexander Koch (ipfire@starkstromkonsument.de) +# Last modified on 24.01.19 by Alexander Koch (ipfire@starkstromkonsument.de) +#
This script is missing a license header. Presumably you want a GPLv3 or some similar header here. Please check the appropriate license that you would like to use.
+# Inculde Pakfire-Functions +require "/opt/pakfire/lib/functions.pl";
+# Check for passed options +unless (@ARGV) {
print "No options given!\n";
print "Possible options: updatescount, coreupdate_avail, need_reboot\n";
exit 2;
+}
+# Count packets +if ("$ARGV[0]" eq "updatescount") {
- # The following lines have been copied from
/opt/pakfire/lib/functions.pl with minor modifications.
- my @meta;
- my $file;
- my $line;
- my $prog;
- my ($name, $version, $release);
- my @templine;
- my $updatecount = 0;
- # Get list of packets
- open(FILE, "<$Conf::dbdir/lists/packages_list.db");
- my @db = <FILE>;
- close(FILE);
- # Get installed addons
- opendir(DIR,"$Conf::dbdir/installed");
- my @files = readdir(DIR);
- closedir(DIR);
- foreach $file (@files) {
next if ( $file eq "." );
next if ( $file eq ".." );
next if ( $file =~ /^old/ );
open(FILE, "<$Conf::dbdir/installed/$file");
@meta = <FILE>;
close(FILE);
foreach $line (@meta) {
@templine = split(/\: /,$line);
if ("$templine[0]" eq "Name") {
$name = $templine[1];
chomp($name);
} elsif ("$templine[0]" eq "ProgVersion") {
$version = $templine[1];
chomp($version);
} elsif ("$templine[0]" eq "Release") {
$release = $templine[1];
chomp($release);
}
}
foreach $prog (@db) {
@templine = split(/\;/,$prog);
if (("$name" eq "$templine[0]") && ("$release" < "$templine[2]")) {
$updatecount++;
}
}
- }
- print $updatecount;
- exit 0;
+}
+elsif ("$ARGV[0]" eq "coreupdate_avail") {
- eval(`grep "core_" $Conf::dbdir/lists/core-list.db`);
- if ("$core_release" > "$Conf::core_mine") {
print 1;
exit 0;
- }
- else {
print 0;
exit 0;
- }
+}
+elsif ("$ARGV[0]" eq "need_reboot") {
- if ( -e "/var/run/need_reboot" ) {
print 1;
exit 0;
- }
- else {
print 0;
exit 0;
- }
+}
+else {
- print "Wrong options!\n";
- print "Possible options: updatescount, coreupdate_avail, need_reboot\n";
- exit 2;
+}
Would it not have been a good idea to have the functions live in the pakfire code and just have a convenient script to call them? Or even extend the pakfire command to return whether there are updates or not?
diff --git a/config/zabbix_agentd/sudoers b/config/zabbix_agentd/sudoers new file mode 100644 index 0000000..d6049f3 --- /dev/null +++ b/config/zabbix_agentd/sudoers @@ -0,0 +1,17 @@ +# Include file for sudoers file +# +# This is needed for some userparameters to be able to execute commands that only run as root (using sudo) +# e.g. /usr/bin/openssl or /usr/sbin/smartctl +# +# USE AT YOU'RE OWN RISK. USING THIS WRONG CAN RESULT IN A SECURITY BREACH! +# +# Some hints: +# - It is strongly recommended to edit this file only using the visudo -f <filename> command. If you mess up this file, +# you might end up locking yourself out of your system! +# - Append the full path to each command, using "," as separator. +# - Only add commands you really need. Zabbix should not have more rights than it has to. +# +# Uncomment the following two lines and edit the example of commands to fit your needs: +# +#Defaults:zabbix !requiretty +#zabbix ALL=(ALL) NOPASSWD: /usr/bin/openssl, /usr/sbin/smartctl
You might want to limit the options to be given to smartctl. Potentially you can send commands to the hard drives but I assume that you only want to read information.
diff --git a/config/zabbix_agentd/userparameter_pakfire.conf b/config/zabbix_agentd/userparameter_pakfire.conf new file mode 100644 index 0000000..4fc4265 --- /dev/null +++ b/config/zabbix_agentd/userparameter_pakfire.conf @@ -0,0 +1,4 @@ +# Provide additional items for Pakfire-Updates +UserParameter=pakfire.updatescount,/etc/zabbix/scripts/pakfire_updates.pl updatescount +UserParameter=pakfire.coreupdate_avail,/etc/zabbix/scripts/pakfire_updates.pl coreupdate_avail +UserParameter=pakfire.need_reboot,/etc/zabbix/scripts/pakfire_updates.pl need_reboot diff --git a/config/zabbix_agentd/zabbix_agentd.conf b/config/zabbix_agentd/zabbix_agentd.conf new file mode 100644 index 0000000..e60af19 --- /dev/null +++ b/config/zabbix_agentd/zabbix_agentd.conf @@ -0,0 +1,394 @@ +# This is a configuration file for Zabbix agent daemon (Unix) +# To get more information about Zabbix, visit http://www.zabbix.com
+############ GENERAL PARAMETERS #################
+### Option: PidFile +# Name of PID file. +# +# Mandatory: no +# Default: +# PidFile=/tmp/zabbix_agentd.pid
+PidFile=/var/run/zabbix/zabbix_agentd.pid
+### Option: LogType +# Specifies where log messages are written to: +# system - syslog +# file - file specified with LogFile parameter +# console - standard output +# +# Mandatory: no +# Default: +# LogType=file
+### Option: LogFile +# Log file name for LogType 'file' parameter. +# +# Mandatory: yes, if LogType is set to file, otherwise no +# Default: +# LogFile=
+LogFile=/var/log/zabbix/zabbix_agentd.log
+### Option: LogFileSize +# Maximum size of log file in MB. +# 0 - disable automatic log rotation. +# +# Mandatory: no +# Range: 0-1024 +# Default: +# LogFileSize=1
Default seems to be enabled. Doesn’t this collide with logrotate?
+### Option: DebugLevel +# Specifies debug level: +# 0 - basic information about starting and stopping of Zabbix processes +# 1 - critical information +# 2 - error information +# 3 - warnings +# 4 - for debugging (produces lots of information) +# 5 - extended debugging (produces even more information) +# +# Mandatory: no +# Range: 0-5 +# Default: +# DebugLevel=3
+### Option: SourceIP +# Source IP address for outgoing connections. +# +# Mandatory: no +# Default: +# SourceIP=
+### Option: EnableRemoteCommands +# Whether remote commands from Zabbix server are allowed. +# 0 - not allowed +# 1 - allowed +# +# Mandatory: no +# Default: +# EnableRemoteCommands=0
+### Option: LogRemoteCommands +# Enable logging of executed shell commands as warnings. +# 0 - disabled +# 1 - enabled +# +# Mandatory: no +# Default: +# LogRemoteCommands=0
+##### Passive checks related
+### Option: Server +# List of comma delimited IP addresses, optionally in CIDR notation, or DNS names of Zabbix servers and Zabbix proxies. +# Incoming connections will be accepted only from the hosts listed here. +# If IPv6 support is enabled then '127.0.0.1', '::127.0.0.1', '::ffff:127.0.0.1' are treated equally +# and '::/0' will allow any IPv4 or IPv6 address. +# '0.0.0.0/0' can be used to allow any IPv4 address. +# Example: Server=127.0.0.1,192.168.1.0/24,::1,2001:db8::/32,zabbix.example.com +# +# Mandatory: yes, if StartAgents is not explicitly set to 0 +# Default: +# Server=
+Server=127.0.0.1
What is the rationale behind this default?
+### Option: ListenPort +# Agent will listen on this port for connections from the server. +# +# Mandatory: no +# Range: 1024-32767 +# Default: +# ListenPort=10050
+### Option: ListenIP +# List of comma delimited IP addresses that the agent should listen on. +# First IP address is sent to Zabbix server if connecting to it to retrieve list of active checks. +# +# Mandatory: no +# Default: +# ListenIP=0.0.0.0
+### Option: StartAgents +# Number of pre-forked instances of zabbix_agentd that process passive checks. +# If set to 0, disables passive checks and the agent will not listen on any TCP port. +# +# Mandatory: no +# Range: 0-100 +# Default: +# StartAgents=3
+##### Active checks related
+### Option: ServerActive +# List of comma delimited IP:port (or DNS name:port) pairs of Zabbix servers and Zabbix proxies for active checks. +# If port is not specified, default port is used. +# IPv6 addresses must be enclosed in square brackets if port for that host is specified. +# If port is not specified, square brackets for IPv6 addresses are optional. +# If this parameter is not specified, active checks are disabled. +# Example: ServerActive=127.0.0.1:20051,zabbix.domain,[::1]:30051,::1,[12fc::1] +# +# Mandatory: no +# Default: +# ServerActive=
+ServerActive=127.0.0.1
See above.
+### Option: Hostname +# Unique, case sensitive hostname. +# Required for active checks and must match hostname as configured on the server. +# Value is acquired from HostnameItem if undefined. +# +# Mandatory: no +# Default: +# Hostname=
+### Option: HostnameItem +# Item used for generating Hostname if it is undefined. Ignored if Hostname is defined. +# Does not support UserParameters or aliases. +# +# Mandatory: no +# Default: +# HostnameItem=system.hostname
+### Option: HostMetadata +# Optional parameter that defines host metadata. +# Host metadata is used at host auto-registration process. +# An agent will issue an error and not start if the value is over limit of 255 characters. +# If not defined, value will be acquired from HostMetadataItem. +# +# Mandatory: no +# Range: 0-255 characters +# Default: +# HostMetadata=
+### Option: HostMetadataItem +# Optional parameter that defines an item used for getting host metadata. +# Host metadata is used at host auto-registration process. +# During an auto-registration request an agent will log a warning message if +# the value returned by specified item is over limit of 255 characters. +# This option is only used when HostMetadata is not defined. +# +# Mandatory: no +# Default: +# HostMetadataItem=
+### Option: RefreshActiveChecks +# How often list of active checks is refreshed, in seconds. +# +# Mandatory: no +# Range: 60-3600 +# Default: +# RefreshActiveChecks=120
+### Option: BufferSend +# Do not keep data longer than N seconds in buffer. +# +# Mandatory: no +# Range: 1-3600 +# Default: +# BufferSend=5
+### Option: BufferSize +# Maximum number of values in a memory buffer. The agent will send +# all collected data to Zabbix Server or Proxy if the buffer is full. +# +# Mandatory: no +# Range: 2-65535 +# Default: +# BufferSize=100
+### Option: MaxLinesPerSecond +# Maximum number of new lines the agent will send per second to Zabbix Server +# or Proxy processing 'log' and 'logrt' active checks. +# The provided value will be overridden by the parameter 'maxlines', +# provided in 'log' or 'logrt' item keys. +# +# Mandatory: no +# Range: 1-1000 +# Default: +# MaxLinesPerSecond=20
+############ ADVANCED PARAMETERS #################
+### Option: Alias +# Sets an alias for an item key. It can be used to substitute long and complex item key with a smaller and simpler one. +# Multiple Alias parameters may be present. Multiple parameters with the same Alias key are not allowed. +# Different Alias keys may reference the same item key. +# For example, to retrieve the ID of user 'zabbix': +# Alias=zabbix.userid:vfs.file.regexp[/etc/passwd,^zabbix:.:([0-9]+),,,,\1] +# Now shorthand key zabbix.userid may be used to retrieve data. +# Aliases can be used in HostMetadataItem but not in HostnameItem parameters. +# +# Mandatory: no +# Range: +# Default:
+### Option: Timeout +# Spend no more than Timeout seconds on processing +# +# Mandatory: no +# Range: 1-30 +# Default: +# Timeout=3
+### Option: AllowRoot +# Allow the agent to run as 'root'. If disabled and the agent is started by 'root', the agent +# will try to switch to the user specified by the User configuration option instead. +# Has no effect if started under a regular user. +# 0 - do not allow +# 1 - allow +# +# Mandatory: no +# Default: +# AllowRoot=0
+### Option: User +# Drop privileges to a specific, existing user on the system. +# Only has effect if run as 'root' and AllowRoot is disabled. +# +# Mandatory: no +# Default: +# User=zabbix
+### Option: Include +# You may include individual files or all files in a directory in the configuration file. +# Installing Zabbix will create include directory in /usr/local/etc, unless modified during the compile time. +# +# Mandatory: no +# Default: +# Include=
+Include=/etc/zabbix/zabbix_agentd.d/*.conf
+####### USER-DEFINED MONITORED PARAMETERS #######
+### Option: UnsafeUserParameters +# Allow all characters to be passed in arguments to user-defined parameters. +# The following characters are not allowed: +# \ ' " ` * ? [ ] { } ~ $ ! & ; ( ) < > | # @ +# Additionally, newline characters are not allowed. +# 0 - do not allow +# 1 - allow +# +# Mandatory: no +# Range: 0-1 +# Default: +# UnsafeUserParameters=0
+### Option: UserParameter +# User-defined parameter to monitor. There can be several user-defined parameters. +# Format: UserParameter=<key>,<shell command> +# See 'zabbix_agentd' directory for examples. +# +# Mandatory: no +# Default: +# UserParameter=
+####### LOADABLE MODULES #######
+### Option: LoadModulePath +# Full path to location of agent modules. +# Default depends on compilation options. +# To see the default path run command "zabbix_agentd --help". +# +# Mandatory: no +# Default: +# LoadModulePath=/usr/lib/modules
See above.
+### Option: LoadModule +# Module to load at agent startup. Modules are used to extend functionality of the agent. +# Format: LoadModule=<module.so> +# The modules must be located in directory specified by LoadModulePath. +# It is allowed to include multiple LoadModule parameters. +# +# Mandatory: no +# Default: +# LoadModule=
+####### TLS-RELATED PARAMETERS #######
+### Option: TLSConnect +# How the agent should connect to server or proxy. Used for active checks. +# Only one value can be specified: +# unencrypted - connect without encryption +# psk - connect using TLS and a pre-shared key +# cert - connect using TLS and a certificate +# +# Mandatory: yes, if TLS certificate or PSK parameters are defined (even for 'unencrypted' connection) +# Default: +# TLSConnect=unencrypted
+### Option: TLSAccept +# What incoming connections to accept. +# Multiple values can be specified, separated by comma: +# unencrypted - accept connections without encryption +# psk - accept connections secured with TLS and a pre-shared key +# cert - accept connections secured with TLS and a certificate +# +# Mandatory: yes, if TLS certificate or PSK parameters are defined (even for 'unencrypted' connection) +# Default: +# TLSAccept=unencrypted
+### Option: TLSCAFile +# Full pathname of a file containing the top-level CA(s) certificates for +# peer certificate verification. +# +# Mandatory: no +# Default: +# TLSCAFile=
+### Option: TLSCRLFile +# Full pathname of a file containing revoked certificates. +# +# Mandatory: no +# Default: +# TLSCRLFile=
+### Option: TLSServerCertIssuer +# Allowed server certificate issuer. +# +# Mandatory: no +# Default: +# TLSServerCertIssuer=
+### Option: TLSServerCertSubject +# Allowed server certificate subject. +# +# Mandatory: no +# Default: +# TLSServerCertSubject=
+### Option: TLSCertFile +# Full pathname of a file containing the agent certificate or certificate chain. +# +# Mandatory: no +# Default: +# TLSCertFile=
+### Option: TLSKeyFile +# Full pathname of a file containing the agent private key. +# +# Mandatory: no +# Default: +# TLSKeyFile=
+### Option: TLSPSKIdentity +# Unique, case sensitive string used to identify the pre-shared key. +# +# Mandatory: no +# Default: +# TLSPSKIdentity=
+### Option: TLSPSKFile +# Full pathname of a file containing the pre-shared key. +# +# Mandatory: no +# Default: +# TLSPSKFile=
+#TLSPSKFile=/etc/zabbix/zabbix_agentd.psk
This line doesn’t do anything.
diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd new file mode 100755 index 0000000..fba24f1 --- /dev/null +++ b/lfs/zabbix_agentd @@ -0,0 +1,128 @@ +############################################################################### +# # +# IPFire.org - A linux based firewall # +# Copyright (C) 2007-2019 IPFire Team info@ipfire.org # +# # +# 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 3 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, see http://www.gnu.org/licenses/. # +# # +###############################################################################
+############################################################################### +# Definitions +###############################################################################
+include Config
+VER = 4.0.3
+THISAPP = zabbix-$(VER) +DL_FILE = $(THISAPP).tar.gz +DL_FROM = $(URL_IPFIRE) +DIR_APP = $(DIR_SRC)/$(THISAPP) +TARGET = $(DIR_INFO)/$(THISAPP) +PROG = zabbix_agentd +PAK_VER = 0.4 +DEPS = ""
+############################################################################### +# Top-level Rules +###############################################################################
+objects = $(DL_FILE)
+$(DL_FILE) = $(DL_FROM)/$(DL_FILE)
+$(DL_FILE)_MD5 = 917d7303c248a9d1c49b8883c01ab2d9
+install : $(TARGET)
+check : $(patsubst %,$(DIR_CHK)/%,$(objects))
+download :$(patsubst %,$(DIR_DL)/%,$(objects))
+md5 : $(subst %,%_MD5,$(objects))
+dist:
- @$(PAK)
+############################################################################### +# Downloading, checking, md5sum +###############################################################################
+$(patsubst %,$(DIR_CHK)/%,$(objects)) :
- @$(CHECK)
+$(patsubst %,$(DIR_DL)/%,$(objects)) :
- @$(LOAD)
+$(subst %,%_MD5,$(objects)) :
- @$(MD5)
+############################################################################### +# Installation Details +###############################################################################
+$(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
- @$(PREBUILD)
- @rm -rf $(DIR_APP) && cd $(DIR_SRC) && tar axvf $(DIR_DL)/$(DL_FILE)
- cd $(DIR_APP) && ./configure \
--prefix=/usr \
--enable-agent \
--sysconfdir="/etc/zabbix" \
--with-openssl
- cd $(DIR_APP) && make + cd $(DIR_APP) && make install
- # Add User Zabbix if it does not exist
- id -u zabbix &>/dev/null || useradd -r -U -s /bin/false -M -d
/var/empty -c "Zabbix Monitoring” zabbix
You are checking if the user exists, but expect to create a user *and* a group. This could potentially go wrong.
This will also randomly select a user ID. Therefore it would be better to have this in config/etc/passwd and config/etc/group so it will be persistent for every time the build is run.
- # Create config directory and create files.
- -rmdir zabbix_agentd.conf.d
You are trying to delete /usr/src/zabbit_agentd.conf.d here. This should not exist anyways.
- -mkdir -pv /etc/zabbix/zabbix_agentd.d
- -mkdir -pv /etc/zabbix/scripts
- install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/zabbix_agentd.conf \
/etc/zabbix/zabbix_agentd.conf
- install -v -m 644
$(DIR_SRC)/config/zabbix_agentd/userparameter_pakfire.conf \
/etc/zabbix/zabbix_agentd.d/userparameter_pakfire.conf
- install -v -m 754 -g zabbix
$(DIR_SRC)/config/zabbix_agentd/pakfire_updates.pl \
/etc/zabbix/scripts/pakfire_updates.pl
Why should this script not be allowed to be executed by other users than root and those in the zabbix group?
- touch /etc/zabbix/zabbix_agentd.psk
This file is not being used in the configuration file.
- # Create directory and file for logging.
- -mkdir -pv /var/log/zabbix
- chown zabbix.zabbix /var/log/zabbix -R
- # Create directory for pid.
- -mkdir -pv /var/run/zabbix
- chown zabbix.zabbix /var/run/zabbix
- # Install initscripts
- $(call INSTALL_INITSCRIPT,zabbix_agentd)
- # Install sudoers include file
- install -v -m 440 $(DIR_SRC)/config/zabbix_agentd/sudoers \
/etc/sudoers.d/zabbix
- # Install include file for backup
- install -v -m 644 $(DIR_SRC)/config/backup/includes/zabbix_agentd \
/var/ipfire/backup/addons/includes/zabbix_agentd
- # Install include file for Logrotate
- -mkdir -pv /etc/logrotate.d
- install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/logrotate \
/etc/logrotate.d/zabbix_agentd
- @rm -rf $(DIR_APP)
- @$(POSTBUILD)
diff --git a/make.sh b/make.sh index f96b74b..dadae3c 100755 --- a/make.sh +++ b/make.sh @@ -1588,6 +1588,7 @@ buildipfire() { lfsmake2 dehydrated lfsmake2 shairport-sync lfsmake2 borgbackup
- lfsmake2 zabbix_agentd
} buildinstaller() { diff --git a/src/initscripts/packages/zabbix_agentd b/src/initscripts/packages/zabbix_agentd new file mode 100755 index 0000000..e50b56c --- /dev/null +++ b/src/initscripts/packages/zabbix_agentd @@ -0,0 +1,61 @@ +#!/bin/sh +######################################################################## +# Begin $rc_base/init.d/zabbix_agentd +# +# Description : This is a script that starts zabbix_agent as deamon +# +# Authors : Alexander Koch (ipfire@starkstromkonsument.de) +# +# Version : 01.00 +# +# Notes : +# +########################################################################
+. /etc/sysconfig/rc +. ${rc_functions}
+NAME=zabbix_agentd +DAEMON=/usr/sbin/$NAME +DESC="Zabbix agent" +RUNDIR=/var/run/zabbix +CONF=/etc/zabbix/zabbix_agentd.conf
+test -x $DAEMON || exit 0
+case "${1}" in
- start)
# Make sure RUNDIR exists
if [ ! -d $RUNDIR ]; then
boot_mesg "Creating Directory $RUNDIR ..."
mkdir $RUNDIR
chown zabbix.zabbix $RUNDIR
fi
boot_mesg "Starting $NAME …"
We usually use a descriptive name here and not the name of the binary here.
Also no space before the ellipsis.
loadproc $DAEMON -c $CONF > /dev/null
evaluate_retval
;;
- stop)
boot_mesg "Stopping $NAME ..."
killproc $DAEMON
;;
- restart)
${0} stop
sleep 1
${0} start
;;
- status)
statusproc $DAEMON
;;
- *)
echo "Usage: ${0} {start|stop|restart|status}"
exit 1
;;
+esac
+# End $rc_base/init.d/zabbix_agentd
This script is a bit different than the others. Variables are being used instead of using the command names directly. Not sure if that is necessary.
Why is the output of loadproc being thrown away? You won’t have to call evaluate_retval if you didn’t do that.
Is it not better to have /var/run/zabbix being created in src/initscripts/sysconfig/createfiles?
diff --git a/src/paks/zabbix_agentd/install.sh b/src/paks/zabbix_agentd/install.sh new file mode 100644 index 0000000..7264a08 --- /dev/null +++ b/src/paks/zabbix_agentd/install.sh @@ -0,0 +1,45 @@ +#!/bin/bash +############################################################################ +# # +# This file is part of the IPFire Firewall. # +# # +# IPFire 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. # +# # +# IPFire 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 IPFire; if not, write to the Free Software # +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA # +# # +# Copyright (C) 2007 IPFire-Team info@ipfire.org. # +# # +############################################################################ +# +. /opt/pakfire/lib/functions.sh
+# Add User Zabbix if it does not exist +id -u zabbix &>/dev/null || useradd -r -U -s /bin/false -M -d /var/empty -c "Zabbix Monitoring” zabbix
See above. If the group has been lost, it won’t be recreated again.
+extract_files
+# Create additonal Directories and set permissions +mkdir -pv /etc/zabbix/zabbix_agentd.d +mkdir -pv /etc/zabbix/scripts
These should be in the tarball.
+mkdir -pv /var/run/zabbix +chown zabbix.zabbix /var/run/zabbix
This is being created in the initscript.
+mkdir -pv /var/log/zabbix +chown zabbix.zabbix /var/log/zabbix -R
This should also be in the tarball.
+# Create symlinks for runlevel interaction. +ln -sf ../init.d/zabbix_agentd /etc/rc.d/rc3.d/S14zabbix_agentd +ln -sf ../init.d/zabbix_agentd /etc/rc.d/rc0.d/K71zabbix_agentd +ln -sf ../init.d/zabbix_agentd /etc/rc.d/rc6.d/K71zabbix_agentd
You are starting this very early in the boot process. Even before the network is being started.
Is that deliberate or could this be moved to a later time?
Can zabbix bind to IP addresses if those are not assigned to the network interfaces, yet?
+restore_backup ${NAME} +start_service --background ${NAME} diff --git a/src/paks/zabbix_agentd/uninstall.sh b/src/paks/zabbix_agentd/uninstall.sh new file mode 100644 index 0000000..ae8f815 --- /dev/null +++ b/src/paks/zabbix_agentd/uninstall.sh @@ -0,0 +1,38 @@ +#!/bin/bash +############################################################################ +# # +# This file is part of the IPFire Firewall. # +# # +# IPFire 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. # +# # +# IPFire 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 IPFire; if not, write to the Free Software # +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA # +# # +# Copyright (C) 2007 IPFire-Team info@ipfire.org. # +# # +############################################################################ +# +. /opt/pakfire/lib/functions.sh +stop_service ${NAME} +make_backup ${NAME} +remove_files
+# Remove init-scripts and symlinks +rm -rfv /etc/rc.d/rc*.d/*zabbix_agentd
+# Remove directorys +rm -rfv /etc/zabbix +rm -rfv /var/log/zabbix +rm -rfv /var/run/zabbix
See above. Log files should not be removed I think. We do not do that anywhere else as far as I know.
+# Remove user and group +userdel zabbix
Do you delete the group here?
diff --git a/src/paks/zabbix_agentd/update.sh b/src/paks/zabbix_agentd/update.sh new file mode 100644 index 0000000..89c40d0 --- /dev/null +++ b/src/paks/zabbix_agentd/update.sh @@ -0,0 +1,26 @@ +#!/bin/bash +############################################################################ +# # +# This file is part of the IPFire Firewall. # +# # +# IPFire 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. # +# # +# IPFire 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 IPFire; if not, write to the Free Software # +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA # +# # +# Copyright (C) 2007 IPFire-Team info@ipfire.org. # +# # +############################################################################ +# +. /opt/pakfire/lib/functions.sh +./uninstall.sh
+./install.sh
2.7.4
So, those are a lot of comments. Most of them are just questions. Hope you can clarify those for me.
Looking forward to hearing from you soon. Apologies for taking a couple of days to review this. Where are the other people on this list?
-Michael