Hello Michael,
thank you for you're detailed review. I apologize for the line wrap issue... I will answer you're questions inline below.
-------- Original-Nachricht --------
*Von:* michael.tremer@ipfire.org *Gesendet:* Tue, 5 Feb 2019 12:44:54 +0000 *An:* ipfire@starkstromkonsument.de *CC:* development@lists.ipfire.org
*Betreff:* Re: New addon: zabbix_agentd
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.
The zabbix_agentd provides "items" for gathering data for monitoring by the zabbix_server. These out-of-the-box-items can be extended by UserParameter's consisting of one-liner's or entire scripts (e.g. the pakfire_updates.pl). These extensions of the agent have to be maintained by the user himself (and are typically shipped with zabbix templates). Some of the commands run by these extensions may require to be run as root. The sudoers-includefile has to be modified by the user in order to fit the needs of his UserParameters. This is why I consider this a config file that should be included in the backup. Otherwise the changes of the user will be overwritten on updates and lost when a system crashes. Do you agree?
Shouldn’t the whole /etc/zabbix directory be in the backup?
The Zabbix-Ecosystem consist of several daemons: zabbix_server, zabbix_proxy, zabbix_agentd and additionally it's php-frontend. By default they all share this config-directory. I'm planing to build another addon for the zabbix_proxy and wanted to separate the backups. Thinking about it again, it would be better to use separate directories like /etc/zabbix_agentd and /etc/zabbix_proxy in the case of IPFire, although other distributions like e.g. debian don't change this ... what do you think?
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.
It appeared in the log and I don't know why. I will remove the line.
+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.
I will set it to 'usr/lib/zabbix'.
+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.
yep.
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?
It doe not, because the daemon performs a open/write/close-operation when writing to the logfile.
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.
Ok, this will be fixed.
+# 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?
You are right. I didn't want to mess around with a corefile for the beginning. As this is a UserParameter-Extension for the agent and therefore not essentially nescesarry for the addon in the first place, I will remove this file and config/zabbix_agentd/userparameter_pakfire.conf from the patch and provide it separately as an update for the addon or only togehter with the apropriate Zabbix-Tamplate later on.
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.
You are right. But these are just examples. I will remove these. The user has to take care of this file anyway, as explained above. The reason for shipping this file is to have it there and in the backup with this filename. If I don't provide this placeholder, I'm afraid of the users choosing different filenames and ending up not having it in the backup of the addon. Is there a more elegant way to dispel this doubt?
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?
Yes it does, sorry I missed this. I will change it to 0.
+### 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?
There is no rational default for this, because the IP/DNS-Name of the zabbix_server (or proxy) will be different in every environment. Exception: the agent on the host of the server itsself. This is the default chosen in the sources and by the maintainers of other distributions. The agent does not start without this parameter beeing set though. Setting this as a default prevents the agent from beeing accessible by unauthorized zabbix servers or proxys and assures the damenon starting without errors anyways. Users of zabbix should be aware of his though.
+### 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.
It's yust for convenience actually. I personally aprecieate this parameter and the file already existing as very handy when configuring a new host. But thinking about it, actually it might be more consequent not providing this, because I don't provide a default for the other TLS-parameters and files and the file ends up residing on the system without any use if PSK is not the chosen method of encryption. I will remove both.
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.
Ok, i will change this. But will this not result in the user & group existing allways and not only if the addon is installed?
- # 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.
The install-routine allways creates this and I could not figure out how to disable / change this in the lfs. I know this is a dirty workaround, but I don't know a better way to solve this by today. Does anybody have a hint for me?
- -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.plWhy should this script not be allowed to be executed by other users than root and those in the zabbix group?
There is no reason besides my paranoia... The functionallity of the file will be integrated in the original one anyways (see above).
- touch /etc/zabbix/zabbix_agentd.psk
This file is not being used in the configuration file.
See above.
- # 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 existsif [ ! -d $RUNDIR ]; thenboot_mesg "Creating Directory $RUNDIR ..."mkdir $RUNDIRchown zabbix.zabbix $RUNDIRfiboot_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/nullevaluate_retval;;- stop)
boot_mesg "Stopping $NAME ..."killproc $DAEMON;;- restart)
${0} stopsleep 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.
Ok, I'll change it.
Why is the output of loadproc being thrown away? You won’t have to call evaluate_retval if you didn’t do that.
I actually just coppied this part from the netsnmpd-addon without thinking about it ...
Is it not better to have /var/run/zabbix being created in src/initscripts/sysconfig/createfiles?
Yes, it is. I did not know this script yet.
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.
The "-U" takes care of this.
+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.
I will check / fix these issues.
+# 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?
No and no. I will change it to S65 & K02, if you agree.
+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.
Ok.
+# Remove user and group +userdel zabbix
Do you delete the group here?
I tested this an the group seems to be deleted automatically, because it's the users primary group.
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
I hope I cloud clarify you're questions. I'm looking forward to some more feedback to my few questions above and I will be preparing a modified patch within a few days.
Best regards, Alex