Bug 908116

rpmlint output, with comments:

$ rpmlint openshift-origin-console.spec openshift-origin-console-0.4.2-2.fc18.src.rpm
1 packages and 1 specfiles checked; 0 errors, 0 warnings.

$ rpmlint openshift-origin-console-0.4.2-2.fc18.noarch.rpm
openshift-origin-console.noarch: E: devel-dependency v8-devel
# Not sure when that because an error, it depends on it

openshift-origin-console.noarch: W: obsolete-not-provided openshift-console
openshift-origin-console.noarch: E: useless-provides openshift-origin-console
# openshift-console was the original name, only changed last week.
# There are plenty of handbuilt rpm's out there with that name
#  so we need to obsolete it for now.  Will remove in the future

openshift-origin-console.noarch: W: only-non-binary-in-usr-lib
# don't know where it's seeing a binary

openshift-origin-console.noarch: W: conffile-without-noreplace-flag /var/www/openshift/console/config/environments/production.rb
openshift-origin-console.noarch: W: conffile-without-noreplace-flag /var/www/openshift/console/config/environments/development.rb
# These were set this way on purpose by upstream.

openshift-origin-console.noarch: W: dangling-symlink /var/www/openshift/console/httpd/modules /usr/lib64/httpd/modules
openshift-origin-console.noarch: W: dangling-symlink /var/www/openshift/console/httpd/conf/magic /etc/httpd/conf/magic
# Ya, but they link to real directories.

openshift-origin-console.noarch: E: script-without-shebang /var/www/openshift/console/script/console_ruby
# Noted, but that's the way the script works

openshift-origin-console.noarch: W: hidden-file-or-dir /var/www/.openshift
openshift-origin-console.noarch: W: hidden-file-or-dir /var/www/.openshift
# These are supposed to be there.

1 packages and 0 specfiles checked; 3 errors, 9 warnings.

A few ( for now ) remarks :

- Provides:  openshift-origin-console = %{version} 

I think you mean :
Provides:  openshift-console = %{version} 

( as this would solve obsolete-not-provided and useless-provides, because providing the package name is likely not what you want )

- why does it need gcc-c++ at run time ? does it compile something ?

- conffile-without-noreplace-flag mean that if someone modify them, then they will be replaced on upgrade. So that's not really what we want from a configuration file. So either they are configuration and i can modify them, or they are not. 

- W: only-non-binary-in-usr-lib
# don't know where it's seeing a binary

the message is "only non binary", so indeed, it doesn't see a binary there :) FHS would mandate to put in /usr/share but if that's ruby module, it can go to /usr/lib

- I see the console is running under the uid of apache, so you need /var/www/.openshift/api.yml. However, having a config file outside of /etc, in a hidden directory, is pretty bad. The config file is not marked as such, so if I install the console somewhere, change the location of the broker and upgrade, my modification will be removed. That's a pretty serious problem.

Mhh, in fact, i am not sure now, what is the use of /var/www/.openshift/api.yml ?

Also, why make the code owned by apache, since this mean the console can erase its own code, which is usually bad from a security point of view. I would recommend to run it under a separate uid.


Also /var/www/openshift/console/httpd/console.conf would be better in /etc, as all configuration file ( so a backup of /etc/ will take care of this ). i also think we want to get ride of /etc/sysconfig file, so I would to merge it with the systemd file directly ( like for the init file )

Last :
%{consoledir}/log/production.log  
do not seems to be rotated by logrotate, so would grow after some time.

Spec URL: http://tdawson.fedorapeople.org/openshift-origin/openshift-origin-console.spec
SRPM URL: http://tdawson.fedorapeople.org/openshift-origin/openshift-origin-console-1.5.17-2.fc18.src.rpm

- Updated to the latest stable upstream version
- Fixed provides, now is openshift-console
- gcc-c++ dependancy - Still need to talk to upstream about that.  Leaving it in until I hear back from them.
- The config files (without noreplace) are also ruby scripts.  If you modify them, your saved changes will be saved as *.rpmsave.  But upstream might need to put functionality in that config file, which might break otherwise.  In short, they need to be config without replace.
- api.yml ... I don't know why it's where it is.  But it works there.  That would need to be changed upstream, not in this spec file.
- Put apache permissions back to what they were upstream.  Upstream has things 640 and 750 so that general users cannot mess with things.
- logs not rotating.  You are correct.  Bug has been filed.

I do not get it, if the file is marked as %config file, that mean I should be able to touch it and not have it broken on a security upgrade. If I shouldn't touch it, then just do not mark it as %config.

Either this is a file for upstream, or a file for the admin. Having it both way is looking for problem.


Permission as 640 and 750 are wrong. That produces useless rpmlint warnings and protect nothing. And the point on having %{consoledir}/config/environments/production.rb writable by apache ( among others ) and the security issue it poses are still valid.

The init script does thing rather strange such as running bundle --install :
          rm -rf Gemfile.lock
          scl enable ruby193 "bundle install --local" > /dev/null
          chown apache:apache Gemfile.lock
        popd > /dev/null

Which basically mean that after restarting the service, it will download stuff from the web, and install code who is outside of the package manager, that do not seems like a good idea security wise :/


And i am not sure again that we need a /etc/sysconfig/ file especially if there is no added value ( ie, there is nothing a admin should really modify there.

There is also a spurious requires on v8-devel, and there is no reason for that IMHO. Maybe that's needed due to the bundle install in the initscript, but then, that's really more problematic.

Ok so bundle install --local do not seems to download anything from the web. yet, i do not see why it does this on initscript.

Spec URL: http://tdawson.fedorapeople.org/openshift-origin/openshift-origin-console.spec
SRPM URL: http://tdawson.fedorapeople.org/openshift-origin/openshift-origin-console-1.5.18-2.fc20.src.rpm

- Removed v8-devel dependancy
- Removed gcc-c++ dependancy
- Changed production.rb and development.rb to %config(noreplace)
-- Upstream is working on them not being config files at all, instead just using console.conf only.
- running "bundle install --local"
-- As you noted, this doesn't pull anything down, only rebuilds.
-- After working on a F19 machine, where this doesn't happen (systemd script doesn't have it), I've seen twice where things fail because it's needed.  If you update any gems that the console is running, and don't rerun that bundle command, then restarting the service fails.
- openshift-console.env - why have it?
-- According to comments "To pass additional options (for instance, -D definitions) to the httpd binary at startup."

What's the latest on this review?

Oups, damn, I forgot about it :/

Will try to do it during the weekend.

Doesn't build, it is missing a requires on systemd :

+ /usr/lib/rpm/redhat/brp-java-repack-jars
Traitement des fichiers : openshift-origin-console-1.5.18-2.fc19.noarch
erreur : Le fichier doit commencer par « / » : %{_unitdir}/openshift-console.service
Erreur de construction de RPM :
    Le fichier doit commencer par « / » : %{_unitdir}/openshift-console.service
Child return code was: 1

( ie, it doesn't expand %{_unitdir}

Spec URL: http://tdawson.fedorapeople.org/openshift-origin/openshift-origin-console.spec
SRPM URL: http://tdawson.fedorapeople.org/openshift-origin/openshift-origin-console-1.10.2.2-2.fc20.src.rpm

- Updated to latest stable version
- Added Requires and BuildRequires for systemd functions
- Cleaned up odd permissions on apache owned files and directories
- Tested to make sure it built on rawhide

Spec URL: http://tdawson.fedorapeople.org/openshift-origin/openshift-origin-console.spec
SRPM URL: http://tdawson.fedorapeople.org/openshift-origin/openshift-origin-console-1.10.2.2-3.fc20.src.rpm

- Added tests and test requirements, but don't enforce yet because all the requirements aren't in Fedora.
- Patched Gemfile to work with Fedora's newer gem versions

Spec URL: http://tdawson.fedorapeople.org/openshift-origin/openshift-origin-console.spec
SRPM URL: http://tdawson.fedorapeople.org/openshift-origin/openshift-origin-console-1.15.1.3-1.fc20.src.rpm

- Updated to the latest stable release.
- Upsteam fixes
-- Changed link that was pointing to /usr/lib64/ to {libdir}
-- Clean extra dependencies out of Gemfile
-- Various other bugs.

Closing Review Request.
The latest versions of this package is no longer supported on Fedora, and is not expected to be in the future.
Thank you for everyone's efforts in reviewing this package up to this point.