Bug 908116 - Review Request: openshift-origin-console - The OpenShift Management Console
Summary: Review Request: openshift-origin-console - The OpenShift Management Console
Keywords:
Status: CLOSED DEFERRED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Michael S.
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 894524
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-02-05 22:26 UTC by Troy Dawson
Modified: 2014-10-03 17:48 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-10-03 17:48:40 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Troy Dawson 2013-02-05 22:26:25 UTC
Spec URL: http://tdawson.fedorapeople.org/openshift-origin/openshift-origin-console.spec
SRPM URL: http://tdawson.fedorapeople.org/openshift-origin/openshift-origin-console-0.4.2-2.fc18.src.rpm
Description: This contains the console configuration components of OpenShift.  This includes the configuration necessary to run the console with mod_passenger.
Fedora Account System Username: tdawson

Comment 1 Troy Dawson 2013-02-05 22:38:06 UTC
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.

Comment 2 Michael S. 2013-02-23 14:55:24 UTC
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.

Comment 3 Michael S. 2013-02-23 14:57:19 UTC
Mhh, in fact, i am not sure now, what is the use of /var/www/.openshift/api.yml ?

Comment 4 Michael S. 2013-02-23 15:07:08 UTC
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.

Comment 5 Troy Dawson 2013-03-21 22:48:55 UTC
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.

Comment 6 Michael S. 2013-03-24 19:56:05 UTC
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.

Comment 7 Michael S. 2013-03-24 20:07:19 UTC
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.

Comment 8 Troy Dawson 2013-06-13 20:22:14 UTC
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."

Comment 9 Marek Mahut 2013-10-11 09:23:42 UTC
What's the latest on this review?

Comment 10 Michael S. 2013-10-11 18:55:28 UTC
Oups, damn, I forgot about it :/

Will try to do it during the weekend.

Comment 11 Michael S. 2013-10-11 19:06:16 UTC
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}

Comment 12 Troy Dawson 2013-10-14 20:36:10 UTC
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

Comment 13 Troy Dawson 2013-10-24 15:47:54 UTC
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

Comment 14 Troy Dawson 2014-04-02 22:11:54 UTC
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.

Comment 15 Troy Dawson 2014-10-03 17:48:40 UTC
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.


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