Bug 226214

Summary: Merge Review: openldap
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Gwyn Ciesla <gwync>
Status: CLOSED ERRATA QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fenlason, jsafrane
Target Milestone: ---Flags: gwync: fedora-review+
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 17:12:19 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:    
Bug Blocks: 426387    

Description Nobody's working on this, feel free to take it 2007-01-31 20:18:48 UTC
Fedora Merge Review: openldap

http://cvs.fedora.redhat.com/viewcvs/devel/openldap/
Initial Owner: fenlason

Comment 1 Gwyn Ciesla 2008-01-23 20:46:20 UTC
rpmlint on SRPM:

openldap.src:351: E: use-of-RPM_SOURCE_DIR
You use $RPM_SOURCE_DIR or %{_sourcedir} in your spec file. If you have to
use a directory for building, use $RPM_BUILD_ROOT instead.

openldap.src:750: W: macro-in-%changelog _sbindir
Macros are expanded in %changelog too, which can in unfortunate cases lead
to the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally
odd entries eg. in source rpms, which is rarely wanted.  Avoid use of macros
in %changelog altogether, or use two '%'s to escape them, like '%%foo'.

openldap.src:821: W: macro-in-%changelog _sysconfdir
Macros are expanded in %changelog too, which can in unfortunate cases lead
to the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally
odd entries eg. in source rpms, which is rarely wanted.  Avoid use of macros
in %changelog altogether, or use two '%'s to escape them, like '%%foo'.

openldap.src:1073: W: macro-in-%changelog _prefix
Macros are expanded in %changelog too, which can in unfortunate cases lead
to the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally
odd entries eg. in source rpms, which is rarely wanted.  Avoid use of macros
in %changelog altogether, or use two '%'s to escape them, like '%%foo'.

openldap.src: W: mixed-use-of-spaces-and-tabs (spaces: line 158, tab: line 177)
The specfile mixes use of spaces and tabs for indentation, which is a
cosmetic annoyance.  Use either spaces or tabs for indentation, not both.

All easily correctable.


rpmlint on rpms:

openldap.i386: W: obsolete-not-provided compat-openldap
If a package is obsoleted by a compatible replacement, the obsoleted package
must also be provided in order to provide clean upgrade paths and not cause
unnecessary dependency breakage.  If the obsoleting package is not a compatible
replacement for the old one, leave out the provides.

Fix.

openldap-clients.i386: W: summary-ended-with-dot Client programs for OpenLDAP.
Summary ends with a dot.

openldap-devel.i386: W: file-not-utf8
/usr/share/doc/openldap-devel-2.4.7/drafts/draft-ietf-ldapext-ldapv3-vlv-xx.txt
The character encoding of this file is not UTF-8.  Consider converting it
in the specfile for example using iconv(1).

openldap-devel.i386: W: file-not-utf8
/usr/share/doc/openldap-devel-2.4.7/drafts/draft-ietf-ldapext-acl-model-xx.txt
The character encoding of this file is not UTF-8.  Consider converting it
in the specfile for example using iconv(1).

openldap-devel.i386: W: summary-ended-with-dot OpenLDAP development libraries
and header files.
Summary ends with a dot.

All fixable.

openldap-devel.i386: W: one-line-command-in-%post /sbin/ldconfig
You should use %post -p <command> instead of using:

%post
<command>

It will avoid the fork of a shell interpreter to execute your command as
well as allows rpm to automatically mark the dependency on your command
for the excecution of the scriptlet.

openldap-devel.i386: W: one-line-command-in-%postun /sbin/ldconfig
You should use %postun -p <command> instead of using:

%postun
<command>

It will avoid the fork of a shell interpreter to execute your command as
well as allows rpm to automatically mark the dependency on your command
for the excecution of the scriptlet.

Should fix.

openldap-servers.i386: W: non-conffile-in-etc /etc/openldap/schema/README
A non-executable file in your package is being installed in /etc, but is not
a configuration file. All non-executable files in /etc should be configuration
files. Mark the file as %config in the spec file.

Possibly move to %doc, rename schema-README?

openldap-servers.i386: E: non-readable /etc/sysconfig/ldap 0640
The file can't be read by everybody. If this is expected (for security
reasons), contact your rpmlint distributor to get it added to the list of
exceptions for your distro (or add it to your local configuration if you
installed rpmlint from the source tarball).

openldap-servers.i386: E: non-standard-gid /etc/openldap/slapd.conf ldap
A file in this package is owned by a non standard group.
Standard groups are:
root, bin, daemon, sys, adm, tty, disk, lp, mem, kmem, wheel, mail,
news, uucp, man, games, gopher, dip, ftp, lock, nobody, users

openldap-servers.i386: E: non-readable /etc/openldap/slapd.conf 0640
The file can't be read by everybody. If this is expected (for security
reasons), contact your rpmlint distributor to get it added to the list of
exceptions for your distro (or add it to your local configuration if you
installed rpmlint from the source tarball).

Not problems.

openldap-servers.i386: E: executable-marked-as-config-file /etc/rc.d/init.d/ldap
Executables must not be marked as config files because that may
prevent upgrades from working correctly. If you need to be able to
customize an executable, make it for example read a config file in
/etc/sysconfig.

Problem.

openldap-servers.i386: E: non-standard-gid /etc/openldap/DB_CONFIG.example ldap
A file in this package is owned by a non standard group.
Standard groups are:
root, bin, daemon, sys, adm, tty, disk, lp, mem, kmem, wheel, mail,
news, uucp, man, games, gopher, dip, ftp, lock, nobody, users

openldap-servers.i386: E: non-readable /etc/openldap/DB_CONFIG.example 0640
The file can't be read by everybody. If this is expected (for security
reasons), contact your rpmlint distributor to get it added to the list of
exceptions for your distro (or add it to your local configuration if you
installed rpmlint from the source tarball).

Ok.

openldap-servers.i386: W: non-conffile-in-etc /etc/openldap/DB_CONFIG.example
A non-executable file in your package is being installed in /etc, but is not
a configuration file. All non-executable files in /etc should be configuration
files. Mark the file as %config in the spec file.

Move to %doc?

openldap-servers.i386: E: non-standard-uid /var/lib/ldap ldap
A file in this package is owned by a non standard user.
Standard users are:
root, bin, daemon, adm, lp, sync, shutdown, halt, mail, news, uucp,
operator, games, gopher, ftp, nobody

openldap-servers.i386: E: non-standard-gid /var/lib/ldap ldap
A file in this package is owned by a non standard group.
Standard groups are:
root, bin, daemon, sys, adm, tty, disk, lp, mem, kmem, wheel, mail,
news, uucp, man, games, gopher, dip, ftp, lock, nobody, users

openldap-servers.i386: E: non-standard-dir-perm /var/lib/ldap 0700
A standard directory should have permission set to 0755. If you get this
message, it means that you have wrong directory permissions in some dirs
included in your package.

openldap-servers.i386: E: non-standard-uid /var/run/openldap ldap
A file in this package is owned by a non standard user.
Standard users are:
root, bin, daemon, adm, lp, sync, shutdown, halt, mail, news, uucp,
operator, games, gopher, ftp, nobody

openldap-servers.i386: E: non-standard-gid /var/run/openldap ldap
A file in this package is owned by a non standard group.
Standard groups are:
root, bin, daemon, sys, adm, tty, disk, lp, mem, kmem, wheel, mail,
news, uucp, man, games, gopher, dip, ftp, lock, nobody, users

Ok.

openldap-servers.i386: W: summary-ended-with-dot OpenLDAP servers and related files.
Summary ends with a dot.

Fix.

openldap-servers.i386: W: conffile-without-noreplace-flag
/etc/pki/tls/certs/slapd.pem
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


openldap-servers.i386: W: conffile-without-noreplace-flag /etc/rc.d/init.d/ldap
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here

Fix.

openldap-servers.i386: E: file-in-usr-marked-as-conffile
/usr/share/openldap/migration/migrate_common.ph
A file in /usr is marked as being a configuration file.
Store your conf files in /etc/ instead.

Why is this marked conf and not in etc?

openldap-servers.i386: W: spurious-bracket-in-%pre
The %pre scriptlet contains an "if []" construct without a space before
the "]".

Fix.

openldap-servers.i386: W: dangerous-command-in-%pre chown
openldap-servers.i386: W: dangerous-command-in-%post rm
openldap-servers.i386: W: spurious-bracket-in-%preun
The %preun scriptlet contains an "if []" construct without a space before
the "]".

openldap-servers.i386: W: dangerous-command-in-%preun rm
openldap-servers.i386: W: no-reload-entry /etc/rc.d/init.d/ldap
In your init script (/etc/rc.d/init.d/your_file), you don't
have a 'reload' entry, which is necessary for good functionality.

Could be replaced by %exclude and %attr in %files.

openldap-servers.i386: W: incoherent-init-script-name ldap
The init script name should be the same as the package name in lower case,
or one with 'd' appended if it invokes a process by that name.

What would be broken if this was fixed?

openldap-servers-sql.i386: W: spurious-executable-perm
/usr/share/doc/openldap-servers-sql-2.4.7/rdbms_depend/timesten/create_schema.sh
The file is installed with executable permissions, but was identified as one
that probably should not be executable.  Verify if the executable bits are
desired, and remove if not.

openldap-servers-sql.i386: W: spurious-executable-perm
/usr/share/doc/openldap-servers-sql-2.4.7/rdbms_depend/timesten/ttcreate_schema.sh
The file is installed with executable permissions, but was identified as one
that probably should not be executable.  Verify if the executable bits are
desired, and remove if not.

openldap-servers-sql.i386: W: summary-ended-with-dot OpenLDAP server SQL support
module.
Summary ends with a dot.

Fix.

Should .a files be in a -static subpackage?

Otherwise, no blockers.

Comment 2 Jan Safranek 2008-01-25 15:08:15 UTC
Thanks for review!

> rpmlint on SRPM:
> 
> openldap.src:351: E: use-of-RPM_SOURCE_DIR
> You use $RPM_SOURCE_DIR or %{_sourcedir} in your spec file. If you have to
> use a directory for building, use $RPM_BUILD_ROOT instead.

Fixed

> openldap.src:750: W: macro-in-%changelog _sbindir
> Macros are expanded in %changelog too, which can in unfortunate cases lead

All macros in changelog are fixed.

> openldap.src: W: mixed-use-of-spaces-and-tabs

Fixed

> rpmlint on rpms:
> 
> openldap.i386: W: obsolete-not-provided compat-openldap
> If a package is obsoleted by a compatible replacement, the obsoleted package
> must also be provided in order to provide clean upgrade paths and not cause
> unnecessary dependency breakage.  If the obsoleting package is not a compatible
> replacement for the old one, leave out the provides.
> 

The compat-openldap is *not* obsoleted by compatible replacement. It just does
not exists anymore and I want it to be removed on update (otherwise openldap
cannot be updated by yum, because compat-openldap will require the same version
of openldap to be installed).

> openldap-clients.i386: W: summary-ended-with-dot Client programs for OpenLDAP.
> Summary ends with a dot.
>
> openldap-devel.i386: W: summary-ended-with-dot OpenLDAP development libraries
> and header files.
> Summary ends with a dot.

Both fixed.

> openldap-devel.i386: W: file-not-utf8
> /usr/share/doc/openldap-devel-2.4.7/drafts/draft-ietf-ldapext-ldapv3-vlv-xx.txt
> The character encoding of this file is not UTF-8.  
> 
> openldap-devel.i386: W: file-not-utf8
> /usr/share/doc/openldap-devel-2.4.7/drafts/draft-ietf-ldapext-acl-model-xx.txt
> The character encoding of this file is not UTF-8.  

It's some sort of official document and I dont' think it's appropriate to
convert it to UTF-8. Apart from that, there are only 3 non-UTF-* characters in
these documents.

> openldap-devel.i386: W: one-line-command-in-%post /sbin/ldconfig
> You should use %post -p <command>

Fixed

> openldap-servers.i386: W: non-conffile-in-etc /etc/openldap/schema/README
> A non-executable file in your package is being installed in /etc, but is not
> a configuration file. All non-executable files in /etc should be configuration
> files. Mark the file as %config in the spec file.
> 
> Possibly move to %doc, rename schema-README?

Moved to %doc as README.schema

> openldap-servers.i386: E: non-readable /etc/sysconfig/ldap 0640
> The file can't be read by everybody.

It is readable now.

> openldap-servers.i386: E: non-standard-gid /etc/openldap/slapd.conf ldap
> A file in this package is owned by a non standard group.

Filed bug #430206 (together with other guid/uid reports)

> openldap-servers.i386: E: executable-marked-as-config-file /etc/rc.d/init.d/ldap
> Executables must not be marked as config files because that may
> prevent upgrades from working correctly.

Fixed

> openldap-servers.i386: W: non-conffile-in-etc /etc/openldap/DB_CONFIG.example
> A non-executable file in your package is being installed in /etc, but is not
> a configuration file. All non-executable files in /etc should be configuration
> files. Mark the file as %config in the spec file.
> 
> Move to %doc?

Not sure about this. People are used that this file is in /etc. I'll keep it
there as %config. 

> openldap-servers.i386: E: non-standard-dir-perm /var/lib/ldap 0700
> A standard directory should have permission set to 0755. If you get this
> message, it means that you have wrong directory permissions in some dirs
> included in your package.
>
> Ok.

I'd like to keep it 0700 too, users should not read ldap database files unless
admin explicitly allows it

> openldap-servers.i386: W: summary-ended-with-dot OpenLDAP servers and related
files.
> Summary ends with a dot.
 
Fixed

> openldap-servers.i386: W: conffile-without-noreplace-flag
> /etc/pki/tls/certs/slapd.pem
> A configuration file is stored in your package without the noreplace flag.
> A way to resolve this is to put the following in your SPEC file:

Fixed.

> %config(noreplace) /etc/your_config_file_here
> 
> openldap-servers.i386: W: conffile-without-noreplace-flag /etc/rc.d/init.d/ldap
> A configuration file is stored in your package without the noreplace flag.
> A way to resolve this is to put the following in your SPEC file:
> 
> %config(noreplace) /etc/your_config_file_here

Not fixed - init.d/ldap is not config file anymore.
 
> openldap-servers.i386: E: file-in-usr-marked-as-conffile
> /usr/share/openldap/migration/migrate_common.ph
> A file in /usr is marked as being a configuration file.
> Store your conf files in /etc/ instead.
> 
> Why is this marked conf and not in etc?

It contains configuration of migration tools. Whole concept of migration tools
stored in /usr/share is somewhat weird, see bug #236697. I'll remove the %config
for now and try to separate it to standalone package later.

> openldap-servers.i386: W: spurious-bracket-in-%pre
> The %pre scriptlet contains an "if []" construct without a space before
> the "]".
> openldap-servers.i386: W: spurious-bracket-in-%preun
> The %preun scriptlet contains an "if []" construct without a space before
> the "]".

Can't find it - %pre/%preun servers has all brackets correct. Rpmlint is maybe
confused by '/var/lib/ldap/[a]lock'???
 
> openldap-servers.i386: W: dangerous-command-in-%pre chown
> openldap-servers.i386: W: dangerous-command-in-%post rm
> openldap-servers.i386: W: dangerous-command-in-%preun rm

This is ok, there is some magic to upgrade database to new version when the
package is being updated.

> openldap-servers.i386: W: no-reload-entry /etc/rc.d/init.d/ldap
> In your init script (/etc/rc.d/init.d/your_file), you don't
> have a 'reload' entry, which is necessary for good functionality.

To be fixed as part of bug #247012.
 
> openldap-servers.i386: W: incoherent-init-script-name ldap
> The init script name should be the same as the package name in lower case,
> or one with 'd' appended if it invokes a process by that name.
> 
> What would be broken if this was fixed?

It would probably break nothing, but people are used to it. I'd like to keep it
as it is.

> openldap-servers-sql.i386: W: spurious-executable-perm
> /usr/share/doc/openldap-servers-sql-2.4.7/rdbms_depend/timesten/create_schema.sh
> /usr/share/doc/openldap-servers-sql-2.4.7/rdbms_depend/timesten/ttcreate_schema.sh
> The file is installed with executable permissions, but was identified as one
> that probably should not be executable.  Verify if the executable bits are
> desired, and remove if not.

Executability removed.
 
> openldap-servers-sql.i386: W: summary-ended-with-dot OpenLDAP server SQL support
> module.
> Summary ends with a dot.
 
Fixed.

> Should .a files be in a -static subpackage?

Is there any .a file? I hope not. If you mean .la files, these are necessary to
load openldap modules. I did not find any way how to make modules work without
them :(.

I fixed the glitches mentioned above and created openldap-2.4.7-4, which has
following rpmlint problems, all commented above.

openldap.i386: W: obsolete-not-provided compat-openldap
openldap-devel.i386: W: file-not-utf8
/usr/share/doc/openldap-devel-2.4.7/drafts/draft-ietf-ldapext-ldapv3-vlv-xx.txt
openldap-devel.i386: W: file-not-utf8
/usr/share/doc/openldap-devel-2.4.7/drafts/draft-ietf-ldapext-acl-model-xx.txt
openldap-servers.i386: E: non-standard-gid /etc/openldap/slapd.conf ldap
openldap-servers.i386: E: non-readable /etc/openldap/slapd.conf 0640
openldap-servers.i386: E: non-standard-uid /var/lib/ldap ldap
openldap-servers.i386: E: non-standard-gid /var/lib/ldap ldap
openldap-servers.i386: E: non-standard-dir-perm /var/lib/ldap 0700
openldap-servers.i386: E: non-standard-uid /var/run/openldap ldap
openldap-servers.i386: E: non-standard-gid /var/run/openldap ldap
openldap-servers.i386: W: spurious-bracket-in-%pre
openldap-servers.i386: W: dangerous-command-in-%pre chown
openldap-servers.i386: W: dangerous-command-in-%post rm
openldap-servers.i386: W: spurious-bracket-in-%preun
openldap-servers.i386: W: dangerous-command-in-%preun rm
openldap-servers.i386: W: no-reload-entry /etc/rc.d/init.d/ldap
openldap-servers.i386: W: incoherent-init-script-name ldap


Comment 3 Gwyn Ciesla 2008-01-25 17:12:19 UTC
Ok, looks good. Everything out of the ordinary is either fixed or done
intentially and documented here.

APRROVED.

Comment 4 Jan Safranek 2008-02-28 14:37:21 UTC
Jon, I separated migrationtools from openldap package and created new package
for it. Could you please do a review of the new package? It's quite simple and
you already know openldap internals, so it should not be anything complicated.

You can find the review request at
https://bugzilla.redhat.com/show_bug.cgi?id=435279

Thanks in advance.