[PATCH] 192 Removed RA and TPS theme packages.
The RA and TPS theme packages are no longer necessary due to the reorganization of the theme files. The build and deployment scripts have been updated accordingly. Ticket #407 -- Endi S. Dewata
1. There are cgi scripts which are explicitly set to have execute permissions by the build script. Is this not needed? Can you confirm this? 2. As mentioned in a previous review, cfg.pl is needed. 3. Are there really no changes required in pkicreate and pkiremove? Surely there must be code that copies stuff from /usr/share/pki/tps-ui and /usr/share/pki/ra-ui. There may even be a programmatic check for the ui package. And don't we need code to copy the relevant bits from common-ui? 4. How was this tested? Do you pkicreate and configure RA and TPS instances? When the instances are created, are all the relevant files where you expect them to be ?-- ie. compare the layout of old/new instances. Did you configure the instances and check the UI to make sure it looks the same? Did you look at the security officer workstation stuff ? Or configure a token? Ade On Tue, 2012-11-27 at 16:15 -0600, Endi Sukma Dewata wrote:
The RA and TPS theme packages are no longer necessary due to the reorganization of the theme files. The build and deployment scripts have been updated accordingly.
Ticket #407
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel
On 11/30/2012 12:25 AM, Ade Lee wrote:
1. There are cgi scripts which are explicitly set to have execute permissions by the build script. Is this not needed? Can you confirm this?
Are you referring to the removal of this code? chmod 755 %{buildroot}%{_datadir}/pki/tps-ui/cgi-bin/sow/cfg.pl There are codes in pkicreate:2076-2090 that set the permissions: set_permissions("${cgibin_instance_path}/sow/*.pl", $default_exe_permissions); So the above chmod code is not necessary.
2. As mentioned in a previous review, cfg.pl is needed.
This is discussed in my response for #190.
3. Are there really no changes required in pkicreate and pkiremove? Surely there must be code that copies stuff from /usr/share/pki/tps-ui and /usr/share/pki/ra-ui. There may even be a programmatic check for the ui package. And don't we need code to copy the relevant bits from common-ui?
The original code already has some codes to copy the subsystem theme files and common theme files (see pkicreate:2064): # Copy /usr/share/pki/<ra/tps>-ui to <instance> return 0 if !copy_directory($ui_subsystem_path, $pki_instance_path, $default_dir_permissions, $default_file_permissions, $pki_user, $pki_group); # Copy /usr/share/pki/common-ui to <instance>/docroot/pki return 0 if !copy_directory( $common_ui_subsystem_path, "$docroot_instance_path/pki", $default_dir_permissions, $default_file_permissions, $pki_user, $pki_group); In the attached revised patch the code to copy <ra/tps>-ui has been removed because the RA/TPS themes don't exist anymore. The copy_directory() would ignore missing folders, but it's better to remove unused code. The code to copy the common-ui is left intact.
4. How was this tested? Do you pkicreate and configure RA and TPS instances?
Yes, the UI seems to be working fine, I didn't see any broken page/links/images, but I'm not familiar with these subsystems to test the functionality.
When the instances are created, are all the relevant files where you expect them to be ?-- ie. compare the layout of old/new instances.
Yes, they seem to be deployed to the same locations.
Did you configure the instances and check the UI to make sure it looks the same?
Yes, they looked fine.
Did you look at the security officer workstation stuff ? Or configure a token?
No, I'm not familiar with this UI, please let me know how to test it. -- Endi S. Dewata
On Sun, 2012-12-02 at 20:56 -0600, Endi Sukma Dewata wrote:
On 11/30/2012 12:25 AM, Ade Lee wrote:
1. There are cgi scripts which are explicitly set to have execute permissions by the build script. Is this not needed? Can you confirm this?
Are you referring to the removal of this code?
chmod 755 %{buildroot}%{_datadir}/pki/tps-ui/cgi-bin/sow/cfg.pl
There are codes in pkicreate:2076-2090 that set the permissions:
set_permissions("${cgibin_instance_path}/sow/*.pl", $default_exe_permissions);
So the above chmod code is not necessary.
2. As mentioned in a previous review, cfg.pl is needed.
This is discussed in my response for #190.
3. Are there really no changes required in pkicreate and pkiremove? Surely there must be code that copies stuff from /usr/share/pki/tps-ui and /usr/share/pki/ra-ui. There may even be a programmatic check for the ui package. And don't we need code to copy the relevant bits from common-ui?
The original code already has some codes to copy the subsystem theme files and common theme files (see pkicreate:2064):
# Copy /usr/share/pki/<ra/tps>-ui to <instance> return 0 if !copy_directory($ui_subsystem_path, $pki_instance_path, $default_dir_permissions, $default_file_permissions, $pki_user, $pki_group);
# Copy /usr/share/pki/common-ui to <instance>/docroot/pki return 0 if !copy_directory( $common_ui_subsystem_path, "$docroot_instance_path/pki", $default_dir_permissions, $default_file_permissions, $pki_user, $pki_group);
In the attached revised patch the code to copy <ra/tps>-ui has been removed because the RA/TPS themes don't exist anymore. The copy_directory() would ignore missing folders, but it's better to remove unused code.
The code to copy the common-ui is left intact.
4. How was this tested? Do you pkicreate and configure RA and TPS instances?
Yes, the UI seems to be working fine, I didn't see any broken page/links/images, but I'm not familiar with these subsystems to test the functionality.
When the instances are created, are all the relevant files where you expect them to be ?-- ie. compare the layout of old/new instances.
Yes, they seem to be deployed to the same locations.
Did you configure the instances and check the UI to make sure it looks the same?
Yes, they looked fine.
Did you look at the security officer workstation stuff ? Or configure a token?
No, I'm not familiar with this UI, please let me know how to test it.
As noted for the other patch, the Security Officer workstation is probably broken (for reasons not related to your patch). ACK.
participants (2)
-
Ade Lee -
Endi Sukma Dewata