Bug 427162

Summary:	Review Request: sagator - antivir/antispam gateway for smtp server
Product:	[Fedora] Fedora	Reporter:	Jan ONDREJ <ondrejj>
Component:	Package Review	Assignee:	Peter Lemenkov <lemenkov>
Status:	CLOSED NEXTRELEASE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	danms, fedora-package-review, notting
Target Milestone:	---	Flags:	lemenkov: fedora-review+ kevin: fedora-cvs+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2008-01-25 20:48:47 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:	430295
Bug Blocks:

Description Jan ONDREJ 2008-01-01 18:18:51 UTC

Spec URL: http://www.salstar.sk/pub/fedora/SPECS/sagator.spec
SRPM URL: http://www.salstar.sk/pub/sagator/fedora/testing/8/SRPMS/sagator-1.0.0-0beta31.fc8.src.rpm
Description:
This program is an email antivirus/antispam gateway. It is an interface to
the postfix, sendmail, or any other smtpd, which runs antivirus and/or
spamchecker. Its modular architecture can use any combination of
antivirus/spamchecker according to configuration.

It has some internal checkers (string_scanner and regexp_scanner). Sagator
can parse MIME mails and decompress archives, if it is configured so.

Features:
    * modular antivirus/spamchecker support
    * you don't need any perl modules or any other modules, only python
    * you can return any quarantined mail to mailq/user mailbox
    * mailbox/maildir scanning and mailbox cleaning
    * nice statistics via RRDTOOL or MRTG
    * web-quarantine access for users
    * daily reports for users
    * SQL database logging and configuration
    * simple chroot support

-----------------------------------------------------------
rpmlint displays some errors and warnings, but I think there are not resolvable
in this package.

I can fix only spurious-executable-perm problems, but with reduced functionality. These are scripts in documentation, which can help user to configure services for sagator and test sagator's functionality.

The non-standard-* errors are caused because sagator requires a new user (vscan here), under which runs. This user is created in post script.

Version of this package is beta, because I (as author of this program) want to add final fedora spec file in a stable release. This version is perfectly stable for me and there are no known problems now. It fixes some issues found in latest stable and removes obsolete scanners. I don't want to push into fedora current stable, because users can use obsolete functions, which was removed in this release.

Comment 1 Dan Smith 2008-01-03 17:20:24 UTC

From rpmlint on the base package:

  sagator.noarch: W: spurious-executable-perm
/usr/share/doc/sagator-1.0.0/test/policytest
  sagator.noarch: W: file-not-utf8 /usr/share/doc/sagator-1.0.0/test/pack/rtest.zip
  sagator.noarch: W: spurious-executable-perm
/usr/share/doc/sagator-1.0.0/test/bigtest

Putting these scripts in %{_datadir}%{name} instead of %doc will quiet a lot of
these complaints.  Perhaps an %{datadir}/%{name}/examples/ (or util) directory
would be appropriate?

  sagator.noarch: W: symlink-should-be-relative /usr/share/sagator/etc/sgconf.py
/etc/sagator.conf

Why does the version in share link to the version in etc?  If it's an example,
it should be duplicated in %doc and /etc.

Comment 2 Dan Smith 2008-01-03 17:28:58 UTC

One other thing: your %install should rm -Rf %{buildroot}, per
http://fedoraproject.org/wiki/Packaging/Guidelines (Prepping BuildRoot for %install)

Comment 3 Jan ONDREJ 2008-01-03 20:23:18 UTC

Thank you for review.

(In reply to comment #1)
>   sagator.noarch: W: spurious-executable-perm
> /usr/share/doc/sagator-1.0.0/test/policytest
>   sagator.noarch: W: file-not-utf8
/usr/share/doc/sagator-1.0.0/test/pack/rtest.zip
>   sagator.noarch: W: spurious-executable-perm
> /usr/share/doc/sagator-1.0.0/test/bigtest
> 
> Putting these scripts in %{_datadir}%{name} instead of %doc will quiet a lot of
> these complaints.  Perhaps an %{datadir}/%{name}/examples/ (or util) directory
> would be appropriate?

These files are used to test sagator's configuration after installation
(configuration). I think an user does not find them in
%{datadir}/%{name}/examples/ and therefore they are not useful.

I think these warnings are not fatal and can be ignored.
I can move whole "test" directory from docs to /usr/share, but with reduced
functionality.

It is better to remove them like move them to an hidden place.

>   sagator.noarch: W: symlink-should-be-relative /usr/share/sagator/etc/sgconf.py
> /etc/sagator.conf

Symlink updated to relative in next release.

> Why does the version in share link to the version in etc?  If it's an example,
> it should be duplicated in %doc and /etc.

Sagator uses python script for configuration. It is an modular system, which
defines more scanners with many parameters in an array. To use this file in
sagator, it need to be imported into python. That symlink adds sagator's
configuration file to it's project, to be easy to include it.

(In reply to comment #2)
> One other thing: your %install should rm -Rf %{buildroot}, per
> http://fedoraproject.org/wiki/Packaging/Guidelines (Prepping BuildRoot for
%install)

Thank you, fixed.
Last changelog:
* Thu Jan 3 2008 Jan ONDREJ (SAL) <ondrejj(at)salstar.sk> - 1.0.0-0beta32
- clean buildroot before install
- sagator.conf symlink is now relative

http://www.salstar.sk/pub/fedora/SPECS/sagator.spec
http://www.salstar.sk/pub/sagator/fedora/testing/8/SRPMS/sagator-1.0.0-0beta32.fc8.src.rpm

Comment 4 Jan ONDREJ 2008-01-06 20:48:36 UTC

http://www.salstar.sk/pub/fedora/SPECS/sagator.spec
http://www.salstar.sk/pub/sagator/fedora/testing/8/SRPMS/sagator-1.0.0-0beta33.fc8.src.rpm

Changes:
- more macros used, I think all which can be used

Comment 5 Peter Lemenkov 2008-01-10 09:34:36 UTC

I'll review it.

Comment 6 Peter Lemenkov 2008-01-24 16:54:45 UTC

REVIEW:

[-] rpmlint is not silent however I don't think this is a blocker. See comments
above:

petro@localhost SPECS $ rpmlint ../RPMS/noarch/sagator-*
sagator.noarch: W: file-not-utf8 /usr/share/doc/sagator-1.0.0/test/pack/rtest.zip
sagator.noarch: W: spurious-executable-perm
/usr/share/doc/sagator-1.0.0/test/policytest
sagator.noarch: E: non-standard-gid /usr/share/sagator/etc vscan
sagator.noarch: E: non-standard-dir-perm /usr/share/sagator/etc 0750
sagator.noarch: W: spurious-executable-perm
/usr/share/doc/sagator-1.0.0/test/pattern
sagator.noarch: E: non-standard-gid /etc/sagator.conf vscan
sagator.noarch: E: non-readable /etc/sagator.conf 0640
sagator.noarch: E: non-standard-uid /var/spool/vscan/tmp vscan
sagator.noarch: E: non-standard-gid /var/spool/vscan/tmp vscan
sagator.noarch: E: non-standard-dir-perm /var/spool/vscan/tmp 01777
sagator.noarch: W: spurious-executable-perm /usr/share/doc/sagator-1.0.0/db/pgsql.sh
sagator.noarch: E: non-standard-uid /var/www/html/sagator vscan
sagator.noarch: E: non-standard-gid /var/www/html/sagator vscan
sagator.noarch: E: non-standard-dir-perm /var/www/html/sagator 0775
sagator.noarch: W: spurious-executable-perm
/usr/share/doc/sagator-1.0.0/test/bigtest
sagator.noarch: W: spurious-executable-perm /usr/share/doc/sagator-1.0.0/db/mysql.sh
sagator.noarch: W: spurious-executable-perm
/usr/share/doc/sagator-1.0.0/test/smtptest
sagator.noarch: W: spurious-executable-perm
/usr/share/doc/sagator-1.0.0/db/sqlite.sh
sagator.noarch: W: spurious-executable-perm
/usr/share/doc/sagator-1.0.0/test/mimeattach.py
sagator.noarch: E: non-standard-uid /var/spool/vscan/tmp/quarantine vscan
sagator.noarch: E: non-standard-gid /var/spool/vscan/tmp/quarantine vscan
sagator.noarch: E: non-standard-dir-perm /var/spool/vscan/tmp/quarantine 0770
sagator.noarch: W: doc-file-dependency
/usr/share/doc/sagator-1.0.0/test/mimeattach.py /usr/bin/python2
sagator.noarch: W: service-default-enabled /etc/init.d/sagator
sagator.noarch: W: service-default-enabled /etc/init.d/sagator
sagator.noarch: W: uncompressed-zip /usr/share/doc/sagator-1.0.0/test/pack/rtest.zip
sagator-webq.noarch: W: no-documentation

[+] The package named according to the Package Naming Guidelines.
[+] The spec file name matches the base package %{name}, in the format %{name}.spec.
[+] The package meets the Packaging Guidelines.
[+] The package licensed with a Fedora approved license and meet the Licensing
Guidelines.
[+] The License field in the package spec file matches the actual license.
[+] The source package includes the text of the license(s) in its own file in %doc.
[+] The spec file written in American English.
[+] The spec file for the package is legible.
[?] The sources used to build the package matches the upstream source. However
there is an error in source path in spec-file and there is more recent snapshot.
[+] The package successfully compile and build into binary rpms on at least one
supported architecture.
[+] All build dependencies are listed in BuildRequires.
[+] A package owns all directories that it creates.
[+] A package does not contain any duplicate files in the %files listing.
[+] Permissions on files are set properly instead of those described in the
above comments.
[+] Package has a %clean section.
[+] Package consistently use macros, as described in the macros section of
Packaging Guidelines.
[+] The package contains code, or permissable content.
[+] Packages does not own files or directories already owned by other packages.
      [+] The package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT). 
[+] All filenames in rpm packages are valid UTF-8.

OK. You should fix the source path in the spec-file and it's APROVED.

Comment 7 Jan ONDREJ 2008-01-24 17:40:03 UTC

New Package CVS Request
=======================
Package Name: sagator
Short Description: antivir/antispam gateway for smtp server
Owners: ondrejj
Branches: F-7 F-8 EL-4 EL-5
Cvsextras Commits: yes

Comment 8 Kevin Fenzi 2008-01-24 22:25:47 UTC

cvs done.

Comment 9 Jan ONDREJ 2008-01-25 20:48:47 UTC

Packages build in koji and waiting to testing.

There is another part of sagator still waiting for review:
  https://bugzilla.redhat.com/show_bug.cgi?id=430295

It's simpler. If you have time, please take it.

Thank you for help.