[Pki-devel] [PATCH] PKI Deployment Framework PKI TRAC issues (08/14/2012)

[Ade Lee]

1. As discussed on #irc, the correct fix is to add null as the last
argument for the outputError() function calls when status is sent in.
Please fix this for all of these calls.

2. Use dict function get(foo, default) rather than setdefault(foo,
default)

3. The line : nick = subsystemnick.split(' ', 2) is confusing and not
necessary.  Its better to use code like this:

if ':' in subsystemnick: 
    token_name = subsystemnick.split(':')[0]
else: 
    token_name = "internal"

4. Please use str.format() when constructing big strings like the sslget
command.

5. In the case where you check if the security domain is defined, you
should log that it does not and then return (NOT exit).

6. We should not exit in any cases here except if the sslget call has an
invocation error.  If there is an error, it should be prominently logged
but it should not stop the pkidestroy.

7. Check what happens if sslget fails to reach the server.  In this
case, it is likely that status will be set to None (along with error).
If this is the case, right now your code will throw an exception.

Ade


On Tue, 2012-08-14 at 18:21 -0700, Matthew Harmsen wrote:
> This patch documents continued implementation of the PKI Deployment
> Framework based upon the revised filesystem layout documented here:
>       * http://pki.fedoraproject.org/wiki/PKI_Instance_Deployment#CA_.2F_KRA_.2F_OCSP_.2F_RA_.2F_TKS_.2F_TPS
> This patch addresses the following issues:
>       * TRAC Ticket #266 - for non-master CA subsystems, pkidestroy
>         needs to contact the security domain to update the domain
>       * Made Fedora 17 rely upon tomcatjss 7.0.0 or later
> It has been tested and proven to work successfully to
> spawn/destroy/spawn a KRA as a separate instance on a 64-bit Fedora 17
> machine (using the appropriate 'tomcatjss.jar').
> 
> P. S. - While fixing the parameters passed via "outputError()" in
> 'base/common/src/com/netscape/cms/servlet/csadmin/UpdateDomainXML.java', I noticed that several of the other servlets in this directory also utilized the "AUTH_FAILURE" error value for the second argument of "outputError()" which gets passed as the string "2" --- while this string is technically acceptable, I believe that this may be old usage of some legacy parent method since "outputError()" is currently defined in "base/common/src/com/netscape/cms/servlet/base/CMSServlet.java" as:
>       * protected void outputError(HttpServletResponse httpResp,
>         String errorString)
>       * protected void outputError(HttpServletResponse httpResp,
>         String errorString, String requestId)
>       * protected void outputError(HttpServletResponse httpResp,
>         String status, String errorString, String requestId)
> so for all of my changes to "outputError()" in "UpdateDomainXML.java",
> I merely changed these incorrect three parameter call versions to the
> two parameter call version by removing the second parameter
> ("AUTH_FAILURE").  If I am correct about this seemingly legacy usage,
> please let me know if I need to file a TRAC ticket for this issue.
> 
> Thanks,
> -- Matt 
> _______________________________________________
> Pki-devel mailing list
> Pki-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel