Bugzilla will be upgraded to version 5.0 on a still to be determined date in the near future. The original upgrade date has been delayed.
Bug 589867 - Review Request: logcheck - analyzes logfiles and sends email
Review Request: logcheck - analyzes logfiles and sends email
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Peter Robinson
Fedora Extras Quality Assurance
:
Depends On: 601115 602587
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-07 03:14 EDT by Matthias Runge
Modified: 2015-01-05 10:08 EST (History)
8 users (show)

See Also:
Fixed In Version: logcheck-1.3.13-5.el5
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-10-05 05:40:22 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
pbrobinson: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Matthias Runge 2010-05-07 03:14:36 EDT
Spec URL: http://www.matthias-runge.de/fedora/logcheck.spec
SRPM URL: http://www.matthias-runge.de/fedora/logcheck-1.3.8-1.fc13.src.rpm
Description: 
Logcheck is a simple utility which is designed to allow a system administrator
to view the log-files which are produced upon hosts under their control.

It does this by mailing summaries of the log-files to them, after first
filtering out "normal" entries.

Normal entries are entries which match one of the many included regular
expression files contain in the database.
Comment 1 Matthias Runge 2010-05-07 03:17:26 EDT
[mrunge@mrungexp SPECS]$ rpmlint logcheck.spec ../RPMS/noarch/logcheck-1.3.8-1.fc13.noarch.rpm ../SRPMS/logcheck-1.3.8-1.fc13.src.rpm 
2 packages and 1 specfiles checked; 0 errors, 0 warnings.
Comment 2 Mohammed Imran 2010-05-07 10:00:55 EDT
Package Review
==============

Key:
- = N/A
x = Check
! = Problem
? = Not evaluated

=== REQUIRED ITEMS ===
[x]  Package is named according to the Package Naming Guidelines.
[x]  Spec file name must match the base package %{name}, in the format
%{name}.spec.
[x]  Package meets the Packaging Guidelines.
[x]  Rpmlint is silent
[x]  Package is not relocatable.

[!]  Buildroot is correct
(%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n))
its not needed anymore, you can remove it 

[x]  Package is licensed with an open-source compatible license and meets other
legal requirements as defined in the legal section of Packaging Guidelines.
[x]  License field in the package spec file matches the actual license.
License type:GPLv2
[x]  If (and only if) the source package includes the text of the license(s) in
its own file, then that file, containing the text of the license(s) for the
package is included in %doc.
[x]  Spec file is legible and written in American English.
[x]  Sources used to build the package matches the upstream source, as provided
in the spec URL.
MD5SUM this package    :86ea9f35183f28f95deb0aba509efb61
MD5SUM upstream package:86ea9f35183f28f95deb0aba509efb61

[x]  Package is not known to require ExcludeArch, OR:
[x]  All build dependencies are listed in BuildRequires, except for any that
[-]  The spec file handles locales properly.
[-]  ldconfig called in %post and %postun if required.
[-]  Package must own all directories that it creates.
[-]  Package requires other packages for directories it uses.
[x]  Package does not contain duplicates in %files.

[!]  Permissions on files are set properly.
retain %config(noreplace) lines,remove %attr(0755,root,root) lines,also group can be logcheck ?
your makefile handles all this

no need to list %dir %{_datadir}/logtail, just %{_datadir}/logtail/* is enough

replace
%{_sbindir}/logcheck
%{_sbindir}/logtail
%{_sbindir}/logtail2

with %{_sbindir}/*


[x]  Package has a %clean section, which contains rm -rf %{buildroot} (or
$RPM_BUILD_ROOT).
[x]  Package consistently uses macros.
[x]  Package contains code, or permissable content.

[!]  Large documentation files are in a -doc subpackage, if required.
make -doc package,also there is man page which you need to install in man directory

[x]  Package uses nothing in %doc for runtime.
[-]  Header files in -devel subpackage, if present.
[-]  Static libraries in -devel subpackage, if present.
[-]  Package requires pkgconfig, if .pc files are present.
[-]  Development .so files in -devel subpackage, if present.
[-]  Fully versioned dependency in subpackages, if present.
[x]  Package does not contain any libtool archives (.la).
[-]  Package contains a properly installed %{name}.desktop file if it is a GUI
application.
[x]  Package does not own files or directories owned by other packages.


Needs to be Fixed
==================
According to install instructins
#-Extract logcheck and run make install.
-Add an unpriviliged user for running logcheck. (typicallly named "logcheck")
-chown -R logcheck /etc/logcheck /var/lock/logcheck /var/lib/logcheck
-Be sure this user can access your log files
-Edit logcheck configuration files in /etc/logcheck.  Most importantly
logcheck.conf.  logcheck.logfiles contains a list of logfiles to be scanned.
-Install logcheck cron job.  There is a sample in debian/logcheck.cron.d

where you are performing these steps ?
you may use use scriptlets
see http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
Please Fix the above issues 
please do read
http://fedoraproject.org/wiki/Packaging/Guidelines
Comment 3 Matthias Runge 2010-05-08 16:12:53 EDT
Thank you for your review.

I did not perform the install routine from debian, mainly because it creates lots of rpmlint warnings and some errors regarding wrong dir-permissions.

I changed my mind about this. You'll find the corrected spec and srpm under the following urls.

Spec URL: http://www.matthias-runge.de/fedora/logcheck.spec
SRPM: http://www.matthias-runge.de/fedora/logcheck-1.3.8-2.fc13.src.rpm

changelog
* Sat May 8 2010 Matthias Runge <mrunge@matthias-runge.de> 1.3.8-2
- install man page
- clean up permissions and files section
- user logcheck added

Even if Fedora does not require the BuildRoot tag, I'll leave it there; I'd like to include logcheck in EPEL, too.
Comment 4 Matthias Runge 2010-05-10 03:22:54 EDT
Version 1.3.8-3 contains a modified list of logfiles to watch, since fedora does not use 
-/var/log/syslog
-/var/log/auth.log

Spec URL: http://www.matthias-runge.de/fedora/logcheck.spec
SRPM: http://www.matthias-runge.de/fedora/logcheck-1.3.8-3.fc13.src.rpm
Comment 5 Matthias Runge 2010-06-07 05:22:19 EDT
Update:

SRPM: http://www.matthias-runge.de/fedora/logcheck-1.3.8-5.fc13.src.rpm
SPEC: http://www.matthias-runge.de/fedora/logcheck.spec

Version 1.3.8-5 contains some fixes:
- add lockfile-progs as requirement 
- debians run-parts accepts --list parameter, fedoras doesn't need it

lockfile-progs is a new review request
https://bugzilla.redhat.com/show_bug.cgi?id=601115
Comment 6 Pavel Alexeev 2010-06-07 05:35:41 EDT
Mohammed Imran, do you plan make this review? No flag set there.
Comment 7 Mohammed Imran 2010-06-07 05:51:17 EDT
Hi pavel,

         If you are interested, you can take the review ;).Im bit busy now,so wont be able to make the review till weekend.
Comment 8 Pavel Alexeev 2010-06-08 05:29:10 EDT
Mohammed, no, I do not want take it form you! If you plan do it - please.
Now I see it have long tail of dependencies instead I simple help you with it.
Comment 9 Matthias Runge 2010-06-10 05:22:09 EDT
I see, it looks like much work to review logcheck or it's dependencies.
Luckily those dependants are really small and any help is greately appreciated.

Thanks anyway!
Comment 10 Matthias Runge 2010-06-10 05:41:45 EDT
I've submitted all dependand packages for review, in total 5 dependant packages. As I already wrote in #c9 , those are really small and any help is grateful taken
Comment 11 Paul Howarth 2010-06-10 06:19:45 EDT
(In reply to comment #2)
> no need to list %dir %{_datadir}/logtail, just %{_datadir}/logtail/* is enough

Not quite. You actually need:

%{_datadir}/logtail/

This includes the directory and its contents. Using just %{_datadir}/logtail/* would mean that the package did not own the directory itself, which would be wrong since none of the package's dependencies own it either.

Similarly you should add:
%dir %{_sysconfdir}/%{name}/
for ownership of this directory. The %dir is needed here because you want to mark the contents of the directory as %config(noreplace).

> replace
> %{_sbindir}/logcheck
> %{_sbindir}/logtail
> %{_sbindir}/logtail2
> 
> with %{_sbindir}/*

That's a matter of style and I would personally prefer the original form.

I wouldn't use "logcheck" as the default group in %defattr as it will make it the default group for everything, including the binaries, documentation, cron scripts etc., which is probably not what you want.

I would have your package create and own the /var/lib/logcheck directory rather than having useradd create it in %pre, so that the directory gets removed if the package is removed from the system.
Comment 12 Matthias Runge 2010-06-10 07:12:40 EDT
> %{_datadir}/logtail/

That suits me better.
> 

> 
> I wouldn't use "logcheck" as the default group in %defattr as it will make it
> the default group for everything, including the binaries, documentation, cron
> scripts etc., which is probably not what you want.
> 
> I would have your package create and own the /var/lib/logcheck directory rather
> than having useradd create it in %pre, so that the directory gets removed if
> the package is removed from the system.    

That's a little difficult. First of all, I fully agree with you, but what happens with the "logcheck"-user when the package is removed. Created users should be left on the system, afaik. When the user is left, why remove his home dir?
Comment 13 Paul Howarth 2010-06-10 07:47:48 EDT
(In reply to comment #12)
> > I wouldn't use "logcheck" as the default group in %defattr as it will make it
> > the default group for everything, including the binaries, documentation, cron
> > scripts etc., which is probably not what you want.
> > 
> > I would have your package create and own the /var/lib/logcheck directory rather
> > than having useradd create it in %pre, so that the directory gets removed if
> > the package is removed from the system.    
> 
> That's a little difficult. First of all, I fully agree with you, but what
> happens with the "logcheck"-user when the package is removed. Created users
> should be left on the system, afaik. When the user is left, why remove his home
> dir?    

The user would be left on the system, but wouldn't be actively used for anything - it would just own any files that may have been left around from the running of the program - if any. The absence of a home directory wouldn't matter for that purpose.
Comment 14 Matthias Runge 2010-08-05 04:03:39 EDT
I've updated from 1.3.8 to version 1.3.11. I took the hint from Paul and let install section create homedir for logcheck, the package owns homedir, too.

To enhance readability, I substituted the two mini-patches by sed-invocations.

Updated files:
SPEC: http://www.matthias-runge.de/fedora/logcheck.spec
SRPM: http://www.matthias-runge.de/fedora/logcheck-1.3.11-1.fc13.src.rpm
Comment 15 Matthias Runge 2010-08-28 17:59:15 EDT
lockfile-progs (as last remaining dependency) is in -testing now. All other dependencies are approved and hit the repositories.

Can we proceed with the review now?
Comment 16 Peter Robinson 2010-08-29 10:01:47 EDT
(In reply to comment #15)
> lockfile-progs (as last remaining dependency) is in -testing now. All other
> dependencies are approved and hit the repositories.
> 
> Can we proceed with the review now?

Yes, I'll do it in the next day or two.
Comment 17 Peter Robinson 2010-09-06 15:03:40 EDT
Sorry for the delay. There's a few issues that need to be addressed

+ rpmlint output

 rpmlint logcheck*
logcheck.noarch: W: non-standard-uid /var/lib/logcheck logcheck
logcheck.noarch: W: non-standard-gid /var/lib/logcheck logcheck
logcheck.noarch: E: non-standard-dir-perm /var/lib/logcheck 0700L
logcheck.noarch: E: non-standard-dir-perm /etc/logcheck/cracking.d 02750L
logcheck.noarch: E: non-standard-dir-perm /etc/logcheck/violations.ignore.d 02750L
logcheck.noarch: E: non-readable /etc/logcheck/logcheck.logfiles 0640L
logcheck.noarch: E: non-standard-dir-perm /etc/logcheck/ignore.d.server 02750L
logcheck.noarch: E: non-standard-dir-perm /etc/logcheck/ignore.d.paranoid 02750L
logcheck.noarch: E: non-readable /etc/logcheck/logcheck.conf 0640L
logcheck.noarch: W: non-standard-uid /var/lock/logcheck logcheck
logcheck.noarch: W: non-standard-gid /var/lock/logcheck logcheck
logcheck.noarch: E: non-standard-dir-perm /var/lock/logcheck 0700L
logcheck.noarch: W: non-standard-gid /etc/logcheck logcheck
logcheck.noarch: E: non-standard-dir-perm /etc/logcheck 0750L
logcheck.noarch: E: non-standard-dir-perm /etc/logcheck/ignore.d.workstation 02750L
logcheck.noarch: E: non-standard-dir-perm /etc/logcheck/violations.d 02750L
logcheck.noarch: E: non-standard-dir-perm /etc/logcheck/cracking.ignore.d 02750L
logcheck.noarch: W: manual-page-warning /usr/share/man/man1/Logcheck.8.gz 1: warning: `\"' not defined
logcheck.noarch: W: no-manual-page-for-binary logcheck
logcheck.src: W: invalid-url Source0: http://ftp.de.debian.org/debian/pool/main/l/logcheck/logcheck_1.3.11.tar.gz HTTP Error 404: Not Found
logcheck.spec: W: invalid-url Source0: http://ftp.de.debian.org/debian/pool/main/l/logcheck/logcheck_1.3.11.tar.gz HTTP Error 404: Not Found
2 packages and 1 specfiles checked; 12 errors, 9 warnings.

+ package name satisfies the packaging naming guidelines
+ specfile name matches the package base name
+ package should satisfy packaging guidelines
+ license meets guidelines and is acceptable to Fedora
+ license matches the actual package license
- latest version packaged
  version 1.3.13 is available

+ %doc includes license file
+ spec file written in American English
+ spec file is legible
- upstream sources match sources in the srpm
  packaged version doesn't seem to be available any longer
+ package successfully builds on at least one architecture
  tested using koji scratch build
+ BuildRequires list all build dependencies
n/a %find_lang instead of %{_datadir}/locale/*
n/a binary RPM with shared library files must call ldconfig in %post and %postun+ does not use Prefix: /usr
+ package owns all directories it creates
+ no duplicate files in %files
+ Package perserves timestamps on install
- Permissions on files must be set properly
  rpmlint complains about a number of permissions
+ %defattr line
+ consistent use of macros
+ package must contain code or permissible content
n/a large documentation files should go in -doc subpackage
+ files marked %doc should not affect package runtime 
n/a header files should be in -devel
n/a static libraries should be in -static
n/a packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
n/a libfoo.so must go in -devel
n/a devel must require the fully versioned base
n/a packages should not contain libtool .la files
n/a packages containing GUI apps must include %{name}.desktop file
+ packages must not own files or directories owned by other packages
+ filenames must be valid UTF-8

Optional:

+ if there is no license file, packager should query upstream to include it
n/a translations of description and summary for non-English languages, if
available
+ reviewer should build the package in mock/koji
+ the package should build into binary RPMs on all supported architectures
  package built using koji scratch build
n/a review should test the package functions as described
+ scriptlets should be sane
n/a non -devel packages should require fully versioned base
n/a pkgconfig files should go in -devel
  shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or
/usr/sbin
+ Package should have man files
Comment 18 Matthias Runge 2010-09-07 15:57:51 EDT
Thanks for the review.

Some of those errors resp. warnings are not so easy to fix. I decided to set the permissions more fedora-like. INSTALL-file says, logcheck-user should own the config-files. I cannot see a reason for this, or why root may not own them, esp. why those config files may not me read by other users. Information hiding does not work in most cases. 

mrunge@sofja noarch]$ rpmlint logcheck-1.3.13-1.fc14.noarch.rpm
logcheck.noarch: W: non-standard-uid /var/lib/logcheck logcheck
logcheck.noarch: W: non-standard-gid /var/lib/logcheck logcheck
logcheck.noarch: E: non-standard-dir-perm /var/lib/logcheck 0700L
logcheck.noarch: W: manual-page-warning /usr/share/man/man8/logcheck.8.gz 1: warning: macro `\"' not defined
logcheck.noarch: W: non-standard-uid /var/lock/logcheck logcheck
logcheck.noarch: W: non-standard-gid /var/lock/logcheck logcheck
logcheck.noarch: E: non-standard-dir-perm /var/lock/logcheck 0700L

Adding a new user and group makes such messages. logcheck should own /var/lib/logcheck and /var/lock/logcheck. Those permissions are sane to me.

Fixed files:
SPEC: http://www.matthias-runge.de/fedora/logcheck.spec
SRPM: http://www.matthias-runge.de/fedora/logcheck-1.3.13-1.fc14.src.rpm
Comment 19 Peter Robinson 2010-09-09 13:34:25 EDT
Seems sane. APPROVED!
Comment 20 Matthias Runge 2010-09-10 02:41:47 EDT
Thank you Peter!

New Package SCM Request
=======================
Package Name: logcheck
Short Description: analyzes logfiles and sends email
Owners: mrunge
Branches: f13 f14 devel el5 el6
Comment 21 Kevin Fenzi 2010-09-10 14:47:03 EDT
Git done (by process-git-requests).
Comment 22 Fedora Update System 2010-09-11 15:19:24 EDT
logcheck-1.3.13-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/logcheck-1.3.13-1.fc14
Comment 23 Fedora Update System 2010-09-11 15:21:30 EDT
logcheck-1.3.13-1.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/logcheck-1.3.13-1.fc13
Comment 24 Fedora Update System 2010-09-11 15:22:03 EDT
logcheck-1.3.13-2.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/logcheck-1.3.13-2.el5
Comment 25 Fedora Update System 2010-09-12 14:24:49 EDT
logcheck-1.3.13-2.el5 has been pushed to the Fedora EPEL 5 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update logcheck'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/logcheck-1.3.13-2.el5
Comment 26 Fedora Update System 2010-09-14 04:46:51 EDT
logcheck-1.3.13-2.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/logcheck-1.3.13-2.fc14
Comment 27 Fedora Update System 2010-09-14 04:48:23 EDT
logcheck-1.3.13-2.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/logcheck-1.3.13-2.fc13
Comment 28 Fedora Update System 2010-09-14 05:10:29 EDT
logcheck-1.3.13-3.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/logcheck-1.3.13-3.el5
Comment 29 Fedora Update System 2010-09-14 13:29:56 EDT
logcheck-1.3.13-3.el5 has been pushed to the Fedora EPEL 5 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update logcheck'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/logcheck-1.3.13-3.el5
Comment 30 Fedora Update System 2010-10-05 05:40:16 EDT
logcheck-1.3.13-2.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 31 Fedora Update System 2010-10-05 09:26:20 EDT
logcheck-1.3.13-2.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 32 chris 2010-10-13 05:44:52 EDT
Thanks very much for adding logcheck to the repository - great tool.  But it doesn't seem to be working at all: it's not filtering any entries out on my system.

I've tested using log messages from a few daemons with /usr/bin/logcheck-test, and the ignore rules are matching correctly, but a run of logcheck then spews out all those log messages without filtering any out.

Here's what I think is happening: logcheck uses /usr/bin/run-parts to pick up the filter rule files from subdirectories of /etc/logcheck.  run-parts looks for executables, and runs them.  But that's not what logcheck wants it to do - logcheck wants it to spit out the names of matching files, not actually execute them.

The Debian version of run-parts has a --list parameter, which does exactly this.  When the logcheck package has been ported across to Fedora, the --list parameter has been removed from the invocation of run-parts (because our version of run-parts doesn't have that parameter), but that simply results in run-parts producing no output.  Which means that logcheck thinks it has no filter rule files.  Which means no log messages ever get filtered out.

I'm surprised that this is the case, because the package should never have been pushed to stable with such a fundamental bug in it.  But I'm fairly sure that's what's going on (on my system, at least - Fedora 13 with logcheck-1.3.13-2.fc13.noarch and crontabs-1.10-32.fc13.noarch).
Comment 33 chris 2010-10-13 06:03:29 EDT
In confirmation of the above: simply replacing the invocation of run-parts with /bin/ls in the cleanrules() function of logcheck fixes the problem.

We don't want to do this, because it loses the benefits of run-parts (ignoring .rpmsave files, etc.) - but it does demonstrate that that's where the problem is.
Comment 34 Matthias Runge 2010-10-13 06:06:10 EDT
Thank you for your report. I'll take a deeper look into this later this day.

Could you please open a new bug instead of reopening an existing one next time?
Comment 35 chris 2010-10-13 11:46:09 EDT
Thanks for the speedy attention.

Sure - I thought about opening a new bug, but was just obeying the instruction in comment 30: "If problems still persist, please make note of it in this bug report."
Comment 36 Fedora Update System 2011-08-17 08:23:47 EDT
logcheck-1.3.13-4.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/logcheck-1.3.13-4.el5
Comment 37 Fedora Update System 2011-09-24 17:55:50 EDT
logcheck-1.3.13-5.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 38 Matthias Runge 2015-01-05 08:42:35 EST
Package Change Request
======================
Package Name: logcheck
New Branches: epel7
Owners: mrunge
Comment 39 Gwyn Ciesla 2015-01-05 10:08:00 EST
Git done (by process-git-requests).

Note You need to log in before you can comment on or make changes to this bug.