Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: php-voms-admin - Web based interface to control VOMS parameters written in PHP
https://bugzilla.redhat.com/show_bug.cgi?id=603346
Summary: Review Request: php-voms-admin - Web based interface to control VOMS parameters written in PHP Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: mattias.ellert@fysast.uu.se QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://www.grid.tsl.uu.se/review/php-voms-admin-spec SRPM URL: http://www.grid.tsl.uu.se/review/php-voms-admin-0.5-0.el5.svn18160.src.rpm
Description: PHP VOMS-Admin (PVA) originally implemented the same functions as the traditional JAVA-based VOMS-Admin (v.2.0.18) interface for Apache Tomcat. It was designed to be more flexible and stable, provide easy scalability and minimize resource usage. PVA is fully compatible with the vomsd mysql backend.
For an example of a server running php-voms-admin installed from this rpm, see http://grid.tsl.uu.se/pva/ and https://grid.tsl.uu.se/pva/ for the unauthenticated and authenticated interface respectively.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #1 from Jason Tibbitts tibbs@math.uh.edu 2010-12-03 15:16:58 EST --- The spec URL above is invalid.
Version 0.5 seems to have been released; is there any reason to continue to package a checkout? Also, you have the dist tag in the wrong place in the Release:; it goes at the end (except in one limited case which does not apply here).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Status Whiteboard| |StalledSubmitter
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
Mattias Ellert mattias.ellert@fysast.uu.se changed:
What |Removed |Added ---------------------------------------------------------------------------- Status Whiteboard|StalledSubmitter |
--- Comment #2 from Mattias Ellert mattias.ellert@fysast.uu.se 2011-05-15 08:34:58 EDT --- (In reply to comment #1)
The spec URL above is invalid.
Typo - the last dash should of course have been a full stop.
Version 0.5 seems to have been released
Updated to the released version:
SPEC: http://www.grid.tsl.uu.se/review/php-voms-admin.spec SRPM: http://www.grid.tsl.uu.se/review/php-voms-admin-0.5-1.fc14.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
Lev Shamardin shamardin@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |shamardin@gmail.com AssignedTo|nobody@fedoraproject.org |shamardin@gmail.com
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #3 from Lev Shamardin shamardin@gmail.com 2011-06-04 16:34:03 EDT --- 1. /etc/httpd/conf.d/pva.conf contains %DATADIR% macros inside, must be fixed.
2. I think you should either add
Requires: voms-server
or consider adding README.Fedora stating that voms-server package (possibly on some other host) is required for using this software.
3. Looks like /usr/share/pva/.htaccess is redundant, since pva.conf already contains these rewrites.
4. After first fix mentioned above, the package does NOT work on my Fedora 14 x86_64 box, with these errors in ssl_error_log:
[Sun Jun 05 00:30:26 2011] [error] [client 127.0.0.1] PHP Notice: Undefined variable: voms_str_unauth in /usr/share/pva/index.php on line 58 [Sun Jun 05 00:30:26 2011] [error] [client 127.0.0.1] PHP Notice: Undefined variable: voms_title in /usr/share/pva/index.php on line 90 [Sun Jun 05 00:30:26 2011] [error] [client 127.0.0.1] PHP Notice: Undefined variable: regex_digits in /usr/share/pva/index.php on line 112 [Sun Jun 05 00:30:26 2011] [error] [client 127.0.0.1] PHP Warning: preg_match(): Empty regular expression in /usr/share/pva/index.php on line 112
5. As the added bonus, would you consider upgrading to a newer SVN version, or at least include lang/ru.inc into the package. (I will not require this, but it would be very nice.)
Now, the formal part:
MUST items:
- MUST: rpmlint errors: php-voms-admin.noarch: E: htaccess-file /usr/share/pva/.htaccess + MUST: package is named according to guidelines. + MUST: spec file name is correct. + MUST: The package must meet the Packaging Guidelines . + MUST: package is licensed under ASL 2.0 + MUST: The License field in the package spec file does match the actual license. + MUST: The license text file is included into the source and the package. + MUST: The spec file must is written in American English. + MUST: The spec file for the package is legible. + MUST: Provided sources match the upstream SVN. + MUST: The package builds on noarch. + MUST: Package does not need any Build dependencies. + MUST: Source package does not use gettext locales. + MUST: Package does not contain shared libraries. + MUST: Package does not bundle copies of system libraries. + MUST: Package is not relocatable. + MUST: Package owns all directories it creates. + MUST: %files listings are fine. + MUST: Permissions on files are correct. + MUST: Package consistently uses macros. + MUST: The package contains code, or permissable content. + MUST: Package does not contain large documentation. + MUST: Package %doc does not affect the runtime. + MUST: Package does not own files or directories owned by other packages. + MUST: All filenames in rpm package are valid UTF-8.
SHOULD items:
+ SHOULD: Source package includes the license text. + SHOULD: Package builds in mock. - SHOULD: At the moment the package functionality seems to be broken, but this seems to be an upstream issue. Some svn revisions ago it did work. - SHOULD: Manual page for /usr/sbin/pva-addvo is missing in upstream.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #4 from Andrii Salnikov manf@grid.org.ua 2011-06-06 10:50:22 EDT --- (In reply to comment #3) Hello, Lev!
I have made required changes to stable branch and released 0.5.1 to fix drawbacks.
1. %DATADIR% problems occurs in code revision in subject here provided by Mattias due to old spec file that was not updated from SVN. Spec file from SVN tagged version does not have this issue.
Here you can find latest src.rpm and spec file for 0.5.1:
http://grid.org.ua/development/pva/packages/rpms/php-voms-admin-0.5.1.spec http://grid.org.ua/development/pva/packages/rpms/php-voms-admin-0.5.1-1.fc14...
2. I have added README.Fedora to 0.5.1, because voms-server in not required for PVA operation. PVA works even without it, but if you want to employ credentials signing you must have voms-server installed.
3. outdated spec file problem
4. outdated spec file problem
5. Upstream 0.6 language files contain several changes, that are incompatible with 0.5 (menu entries especially). Russian translation by Denis Patrikeev was provided for upstream 0.6 and cannot be easily added to 0.5 distribution. This summer I plan to release stable 0.6 and russian translation will be a part of it.
- SHOULD: Manual page for /usr/sbin/pva-addvo is missing in upstream.
Manual page for pva-addvo has been included in 0.5.1 release.
Regards, Andrii
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #5 from Lev Shamardin shamardin@gmail.com 2011-06-06 16:52:24 EDT --- Hi Andrii,
First of all, thanks for a prompt reply.
I have tested your spec with a checkout of pva-0.5.1 tag, the errors I mentioned in the comment above are still relevant for this version. I suppose, something is broken with php includes, since I can see the sources of the regex.php and langdetect.php in the .../pva/ page source. I'm not a php expert, but I suspect that ini_set directives should be avoided in the production code, and should be moved out to the configuration (probably to /etc/httpd/conf.d/pva.conf).
Second, there is one critical thing missing in your version of the spec-file, namely instructions how to get the sources from the upstream version control system (see http://fedoraproject.org/wiki/Packaging/SourceURL).
And the last question, do you have a FAS account, and, if you do, are you in the 'packager' group? If not, either Mattias should maintain and submit the spec for this package, or you should follow the sponsorship procedure described in http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #6 from Andrii Salnikov manf@grid.org.ua 2011-06-07 04:08:30 EDT --- (In reply to comment #5) Hi Lev,
I have forgot about fixes made in upstream for PHP 5.3.3 included into Fedora 14 distribution. When you mentioned ‘sources visibility’ I realized what the problem is.
Now I have finished with PHP 5.3.3 fixes for 0.5.1, update an SVN tagged version and src.rpm mentioned in previous post. You can find updated version following the same URLs.
And the last question, do you have a FAS account, and, if you do, are you in the 'packager' group? If not, either Mattias should maintain and submit the spec for this package, or you should follow the sponsorship procedure described in http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group.
No, I have not FAS account. My packages, that seems to work for me, was posted for you on bug request. If you confirm that now SVN version is woks on you Fedora 14, I will ask Mattias to make an official commit after his final spec corrections.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #7 from Mattias Ellert mattias.ellert@fysast.uu.se 2011-06-08 09:29:22 EDT --- Thanks Andrii for addressing the issues raised by the reviewer. I have updated the package with your latest changes:
SPEC: http://www.grid.tsl.uu.se/review/php-voms-admin.spec SRPM: http://www.grid.tsl.uu.se/review/php-voms-admin-0.5.1-1.fc14.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
Lev Shamardin shamardin@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ON_DEV Flag| |fedora-review+
--- Comment #8 from Lev Shamardin shamardin@gmail.com 2011-06-10 14:27:54 EDT --- The package now works correctly and the review issues are fixed.
Review items which were not ok, but are fixed now:
+ MUST: rpmlint errors: no errors, only spelling warnings. + SHOULD: Package core functionality appears to work in most cases. + SHOULD: Package contains manual pages for binaries/scripts.
Setting fedora-review+.
Please consider providing a package also for the EPEL tree. I could be a co-maintainer if you wish.
There are some future points which could be done better:
1. The package could provide a SELinux module/subpackage, conforming to https://fedoraproject.org/wiki/SELinux_Policy_Modules_Packaging_Draft. Under SELinux it needs two things to operate properly:
a) setsebool -P httpd_can_sendmail 1 b) proper context on /var/www/pva/mail-copies, setroubleshootd suggests
semanage fcontext -a -t httpd_sys_rw_content_t '/var/www/pva/mail-copies' restorecon -v '/var/www/pva/mail-copies'
2. Upstream provides no documentation on package configuration. Consider adding notices about editing /etc/pva/addvo.conf and /etc/pva/config.inc after installation into README.Fedora.
I have also found an upstream issue, but I haven't found the appropriate bug tracker, so filing it here:
pva-addvo fails to add a new vo, if there is more than one copy (or symlink) of VO administrator's CA in /etc/grid-security/certificates: this causes an incorrect CAID inside the pva-addvo, which can not only point at a wrong CA, but also to a missing CA id.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #9 from Lev Shamardin shamardin@gmail.com 2011-06-10 14:47:46 EDT --- And almost forgot another missing documentation/packaing(?) minor, but still important point: this package is useless without SSL with certificates peer authentication, so README.Fedora should include
1. Notice about this requirement. 2. Example configuration for mod_ssl.
Also probably it would be a good idea to include a universal configuration snippet into /etc/http/conf.d/pva.conf, something like
<Directory/usr/share/pva/ >
...
<IfModule mod_ssl.c> SSLRequireSSL SSLVerifyClient require SSLVerifyDepth 10 # or other appropriate number </IfModule> <IfModule mod_gnutls.c> GnuTLSClientVerify require </IfModule> <IfModule mod_nss.c> SSLVerifyDepth 10 # not sure about this directive </IfModule>
... </Directory>
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #10 from Mattias Ellert mattias.ellert@fysast.uu.se 2011-06-10 15:42:51 EDT --- (In reply to comment #8)
The package now works correctly and the review issues are fixed.
Setting fedora-review+.
Thank you for the review. I'll investigate the SELinux issues.
I have also found an upstream issue, but I haven't found the appropriate bug tracker, so filing it here:
Upstream bug tracker is http://bugzilla.nordugrid.org/ → Product: Contributed software → Component: pva
New Package SCM Request ======================= Package Name: php-voms-admin Short Description: Web based interface to control VOMS parameters written in PHP Owners: ellert Branches: f14 f15 el5 el6 InitialCC:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #11 from Andrii Salnikov manf@grid.org.ua 2011-06-11 06:22:27 EDT --- Hi Lev, First of all thanks a lot for deep review and posting hints for PHP VOMS-Admin enhancements.
As for documentation, now I am working on operation manual for 0.6 that already includes configuration description. I need a few weeks to finish it along with 0.6 stable (fixing some minor issues, while writing the documentation). But anyway I think this is a good idea to write manual pages for configuration files, so I’ll implement this too in 0.6.
Link to the upstream bug tracker is available on project web-site and already provided by Mattias. Mentioning the bug tracker in README directly will be also a good idea. I will appreciate if you report any found bugs to bugzilla, so I’ll be able to improve the software.
pva-addvo bug that you described seems to be already reported and fixed in 0.5. I have also made a test with grid-security/certificates directory containing 2 different hash symlinks for one CA (IGTF new-style distribution), and haven’t encountered any problems. Could you point on how to reproduce the bug?
About the SSL notice, I have also put it to documentation for 0.6 but I agree it must be mentioned in README directly. Universal configuration is also a good idea, thank you for hinting. But I think optional VerifyClient will be more useful, because PVA handles unauthenticated operations and allows to view vomses and mkgridmap configuration for a VO without authentication and maybe other things if ACL rules are configured accordingly. A choice to require client certificate is available on demand of site administrator that want to enforce more security restrictions.
Regards, Andrii
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
Mattias Ellert mattias.ellert@fysast.uu.se changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #12 from Jon Ciesla limb@jcomserv.net 2011-06-13 08:17:27 EDT --- Git done (by process-git-requests).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_DEV |MODIFIED
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #13 from Fedora Update System updates@fedoraproject.org 2011-06-18 02:36:27 EDT --- php-voms-admin-0.5.1-2.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/php-voms-admin-0.5.1-2.fc14
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #14 from Fedora Update System updates@fedoraproject.org 2011-06-18 02:36:42 EDT --- php-voms-admin-0.5.1-2.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/php-voms-admin-0.5.1-2.el6
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #15 from Fedora Update System updates@fedoraproject.org 2011-06-18 02:36:49 EDT --- php-voms-admin-0.5.1-2.el5 has been submitted as an update for Fedora EPEL 5. https://admin.fedoraproject.org/updates/php-voms-admin-0.5.1-2.el5
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #16 from Fedora Update System updates@fedoraproject.org 2011-06-18 02:36:57 EDT --- php-voms-admin-0.5.1-2.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/php-voms-admin-0.5.1-2.fc15
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #17 from Fedora Update System updates@fedoraproject.org 2011-06-21 13:12:14 EDT --- php-voms-admin-0.5.1-2.el6 has been pushed to the Fedora EPEL 6 testing repository.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #18 from Fedora Update System updates@fedoraproject.org 2011-07-08 14:05:11 EDT --- php-voms-admin-0.6-1.fc14 has been pushed to the Fedora 14 stable repository.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Fixed In Version| |php-voms-admin-0.6-1.fc14 Resolution| |ERRATA Last Closed| |2011-07-08 14:05:21
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #19 from Fedora Update System updates@fedoraproject.org 2011-07-08 14:12:06 EDT --- php-voms-admin-0.6-1.fc15 has been pushed to the Fedora 15 stable repository.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|php-voms-admin-0.6-1.fc14 |php-voms-admin-0.6-1.fc15
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #20 from Fedora Update System updates@fedoraproject.org 2011-07-14 20:00:34 EDT --- php-voms-admin-0.6-1.el6 has been pushed to the Fedora EPEL 6 stable repository.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|php-voms-admin-0.6-1.fc15 |php-voms-admin-0.6-1.el6
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
--- Comment #21 from Fedora Update System updates@fedoraproject.org 2011-07-14 20:03:27 EDT --- php-voms-admin-0.6-1.el5 has been pushed to the Fedora EPEL 5 stable repository.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=603346
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|php-voms-admin-0.6-1.el6 |php-voms-admin-0.6-1.el5
package-review@lists.fedoraproject.org