[Pki-devel] [PATCH] 192 Removed RA and TPS theme packages.

[Ade Lee]

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.