This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 180092 - Review Request: NRPE - Monitoring agent for Nagios
Review Request: NRPE - Monitoring agent for Nagios
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Dennis Gilmore
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-02-05 13:13 EST by Mike McGrath
Modified: 2007-11-30 17:11 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-12-05 23:23:44 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Mike McGrath 2006-02-05 13:13:50 EST
Spec: http://mmcgrath.net/~mmcgrath/nrpe/nrpe.spec
SRPM Name or Url: http://mmcgrath.net/~mmcgrath/nrpe/nrpe-2.3-1.src.rpm

Description: 
Nrpe is a system daemon that will execute various Nagios plugins
locally on behalf of a remote (monitoring) host that uses the
check_nrpe plugin.  Various plugins that can be executed by the 
daemon are available at:
http://sourceforge.net/projects/nagiosplug

This package provides the core agent.
Comment 1 Mike McGrath 2006-03-05 12:20:20 EST
Upstream has a newer version
SPEC: https://mmcgrath.net/~mmcgrath/nrpe/nrpe.spec
SRPM: http://mmcgrath.net/~mmcgrath/nrpe/nrpe-2.4-1.src.rpm
Comment 2 Jochen Schmitt 2006-03-05 16:24:37 EST
Good:
+ Local build worked fine.
+ Mock build wored fine.


Bad:
- Missing SMP flags. If it doesn't build with it, please add a comment
  (wiki: PackagingGuidelines#parallelmake)
- %{?dist} missing.
- init script doesn't have a reload action.

Comment 3 Mike McGrath 2006-03-05 17:04:32 EST
SRPM: http://mmcgrath.net/~mmcgrath/nrpe/nrpe-2.4-2.src.rpm
SPEC: http://mmcgrath.net/~mmcgrath/nrpe/nrpe.spec

- Added %{?dist}
- Parallel build added and works.
- Added reload section (via HUP) to the init script
Comment 4 Jochen Schmitt 2006-03-06 13:25:53 EST
Good:
+ Local build worked fine.
+ Mock build worked fine.


TODO:
1. Please register a fixed uid on
http://fedoraproject.org/wiki/Packaging/UserRegistry?action=show&redirect=PackageUserRegistry
and use it in your rpm. Please see:
http://fedoraproject.org/wiki/Packaging/UserCreation
Comment 5 Mike McGrath 2006-03-06 13:58:03 EST
"Using 'fedora-usermgmt' is optional and not required by packaging guidelines."
Comment 6 Jochen Schmitt 2006-03-12 14:38:12 EST
Yes, put you should assign a fixed uid to the user which will created by your
package.
Comment 7 Mike McGrath 2006-03-12 17:21:19 EST
Sorry, I think I'm missing something.  I don't see in the packaging guidelines
nor in the review checklist a UID registration requirement.  I have thought
about switching this to the "damon" user.  Perhaps I should do that?
Comment 8 Paul Howarth 2006-03-13 03:53:53 EST
(In reply to comment #7)
> Sorry, I think I'm missing something.  I don't see in the packaging guidelines
> nor in the review checklist a UID registration requirement.  I have thought
> about switching this to the "damon" user.  Perhaps I should do that?

There is no requirement to register or assign a fixed UID. Doing so only makes
sense for applications that may share data over NFS or a similar arrangement
that uses the same UIDs on multiple machines.

There are many packages in Extras that create users but do not assign fixed UIDs
(or attempt to do so using fedora-usermgmt).

I don't think reusing a different UID (did you mean "daemon"?) is a good idea
though.

Can't say more without looking at the spec, and mmcgrath.net is unreachable from
here at the moment.
Comment 9 Mike McGrath 2006-03-13 09:31:44 EST
mmcgrath.net should be back up now.  Terrible storm blew through central
Illinois last night.  I did mean daemon but will stick with useradd -r nrpe in
my spec file.  I don't really even know what services use daemon.
Comment 10 Paul Howarth 2006-03-13 09:56:35 EST
(In reply to comment #9)
> mmcgrath.net should be back up now.  Terrible storm blew through central
> Illinois last night.  I did mean daemon but will stick with useradd -r nrpe in
> my spec file.

Looks reasonable to me. I'd just add a '-c "Description of what account is for"'
option to the useradd command so that people will know what the nrpe account in
their passwd file is for.
Comment 11 Mike McGrath 2006-03-13 10:11:14 EST
SPEC: http://mmcgrath.net/~mmcgrath/nrpe/nrpe.spec
SRPM: http://mmcgrath.net/~mmcgrath/nrpe/nrpe-2.4-3.src.rpm

Updated useradd with a -c
Comment 12 Jochen Schmitt 2006-03-13 10:42:27 EST
Bad:

- You don't allocate a fixed uid/gid.

Comment 13 Jochen Schmitt 2006-03-13 10:43:07 EST
Bad:

- You don't allocate a fixed uid/gid.

Comment 14 Paul Howarth 2006-03-13 10:54:18 EST
Looks like you need a new reviewer Mike, unless Jochen can justify his
requirement for a fixed uid/gid.
Comment 15 Ralf Corsepius 2006-03-13 10:59:21 EST
(In reply to comment #13)
> Bad:
> 
> - You don't allocate a fixed uid/gid.
Elaborate in depth why this package requires a fixed uid/gid.
Comment 17 Ralf Corsepius 2006-03-15 12:28:50 EST
(In reply to comment #16)
> Please read:
> 
>
http://fedoraproject.org/wiki/PackageDynamicUserCreationConsideredBad?highlight=%28user%29

This answer of yours is simply inacceptable.
Comment 18 Mike McGrath 2006-03-15 12:57:24 EST
A) Fedora User Managment is optionsl
B) Removing users after removing a package is bad.  Notice NRPE does not remove
the user.  This prevents other people from becoming that user.   Many packages
(including httpd) don't remove the user they create when they get uninstalled.
Comment 19 Jochen Schmitt 2006-03-26 14:31:58 EST
But nobody prohihed root for deleting such an user.

It may be right, that in older package the system user didn't assigned to a 
fixed user if, but this is from by point of view an argument for new packages.
Comment 20 Mike McGrath 2006-03-26 15:53:27 EST
Thats fine, but its not required for Fedora Extras packages.  If you'd like to
make it a requirement you'll have to convince the FESCo to make it manditory. 
In the meantime its optional and not in the review guidelines anywhere.
Comment 22 Mike McGrath 2006-06-14 20:14:35 EDT
Last comment from Warren (Due to bz db loss)

During configure:

checking for Kerberos include files... could not find include files
Comment 23 Mike McGrath 2006-06-18 18:15:37 EDT
SPEC: http://mmcgrath.net/~mmcgrath/nrpe/nrpe.spec
SRPM: http://mmcgrath.net/~mmcgrath/nrpe/nrpe-2.4-4.src.rpm

- Added --with-kerberos-inc to configure

Comment 24 Jose Pedro Oliveira 2006-06-24 09:32:17 EDT
Mike,

Could you update to version 2.5.1 ?

tia,
jpo
Comment 25 Mike McGrath 2006-07-03 15:43:00 EDT
SPEC: http://mmcgrath.net/~mmcgrath/nrpe/nrpe.spec
SRPM: http://mmcgrath.net/~mmcgrath/nrpe/nrpe-2.5.2-1.src.rpm

Sorry for the delay, here's 2.5.2-1
Comment 26 Jochen Schmitt 2006-07-06 14:44:56 EDT
Good:
+ Local build works fine.
+ Mock build works fine.
+ rpmlint quite on binary rpm.


Bad:
- rpmlint complaints the binary rpm:
E: nrpe configure-without-libdir-spec
- Can't sheck source tarball agains upstream during a technical problem on
download the upstream source.
Comment 27 Mike McGrath 2006-07-07 10:06:59 EDT
The RPM lint error is safe to ignore because there is nothing that would go in a
lib dir.
Comment 28 Mike McGrath 2006-07-23 18:55:05 EDT
No more rpmlint errors:

SPEC: http://mmcgrath.net/~mmcgrath/nrpe/nrpe.spec
SRPM: http://mmcgrath.net/~mmcgrath/nrpe/nrpe-2.5.2-2.src.rpm
Comment 29 Jose Pedro Oliveira 2006-07-31 21:15:24 EDT
Mike,
I've found two more problems:

* missing build requirement: tcp_wrappers

  configure output
  ----------------
  ...
  checking for socket in -lsocket... no
  checking for main in -lwrap... yes
  checking for strdup... yes
  ...

  nrpe linking
  ------------
  gcc ... -o nrpe nrpe.c utils.c -L/usr/lib  -lssl -lcrypto -lnsl -lwrap

* package nagios-plugins-nrpe shouldn't own 
    /usr/lib/nagios
    /usr/lib/nagios/plugins
  as they are owned by the nagios-plugins package (which is listed as a
  requirement)

     $ rpm -qf /usr/lib/nagios
     nagios-plugins-1.4.3-14.fc5

     $ rpm -qf /usr/lib/nagios/plugins
     nagios-plugins-1.4.3-14.fc5

jpo
Comment 30 Enrico Scholz 2006-08-25 02:23:44 EDT
Blocker:

* should not require nagios-plugins; see bug #203689
Comment 31 Mike McGrath 2006-08-25 10:43:59 EDT
This is by no means blocked.  The comments listed in 203689 are not required in
the packaging guidelines anywhere (that I found anyway)  If I'm mistaken please
let me know.
Comment 32 Mike McGrath 2006-08-25 10:52:19 EDT
Sorry, I thought I posted these:

SPEC: http://mmcgrath.net/~mmcgrath/nrpe/nrpe.spec
SRPM: http://mmcgrath.net/~mmcgrath/nrpe/nrpe-2.5.2-3.src.rpm

Changelog:
- no longer owns libdir/nagios
- buildrequires tcp_wrappers
Comment 33 Dennis Gilmore 2006-10-19 11:13:50 EDT
ill take this over
Comment 34 Dennis Gilmore 2006-11-19 22:14:31 EST
package meets naming and packaging guidelines.
 specfile is properly named, is cleanly written and uses macros consistently.
 dist tag is present.
 build root is correct.
      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 license field matches the actual license.
 license is open source-compatible. GPL License text is not included in 
package.
 source files match upstream:
     22afa197db8e4e5b13fac48636917b6d  ../SOURCES/nrpe-2.5.2.tar.gz
     22afa197db8e4e5b13fac48636917b6d  nrpe-2.5.2.tar.gz
 latest version is being packaged.
 BuildRequires are proper.
 package builds in mock ( fc5,fc6,devel on x86_64 and i386 ).
 rpmlint is silent.
 final provides and requires are sane:
nagios-plugins-nrpe-2.5.2-3.fc6.x86_64.rpm
check_nrpe
nagios-plugins-nrpe = 2.5.2-3.fc6
libcrypto.so.6()(64bit)
libnsl.so.1()(64bit)
libssl.so.6()(64bit)
nagios-plugins

nrpe-2.5.2-3.fc6.x86_64.rpm
config(nrpe) = 2.5.2-3.fc6
nrpe = 2.5.2-3.fc6
/sbin/chkconfig
/sbin/service
/usr/sbin/useradd
config(nrpe) = 2.5.2-3.fc6
libcrypto.so.6()(64bit)
libnsl.so.1()(64bit)
libssl.so.6()(64bit)
libwrap.so.0()(64bit)
 no shared libraries are present.
 package is not relocatable.
 owns the directories it creates.
 doesn't own any directories it shouldn't.
 no duplicates in %files.
 file permissions are appropriate.
 %clean is present.
 code, not content.
 documentation is small, so no -docs subpackage is necessary.
 %docs are not necessary for the proper functioning of the package.
 no headers.
 no pkgconfig files.
 no libtool .la droppings.
 not a GUI app.
 not a web app.


APPROVED
Comment 35 Kevin Fenzi 2006-12-05 22:40:47 EST
Hey Mike. 

I don't see this package in owners.list. Can you please add it?

See: 

http://fedoraproject.org/wiki/Extras/Contributors#head-f6f080b4c48fe519c98a29364a740953f90179e7
Comment 36 Mike McGrath 2006-12-05 23:23:44 EST
I'm terrible about that, its added now.  Also imported and build.  Closing bug.
Comment 37 Jose Pedro Oliveira 2006-12-08 12:17:39 EST
Mike,

Could you also build it for FC-5 and FC-6 ?

Tia,
jpo
Comment 38 Mike McGrath 2006-12-08 12:36:39 EST
Sure thing, I'll have them built by tonight.  Should be on the mirrors in the
next couple of days.

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