[Pki-devel] [PATCH] 127 - add servlet to return 501 or d9 style instances

[Ade Lee]

On Fri, 2013-04-19 at 23:20 -0500, Endi Sukma Dewata wrote:
> On 4/19/2013 2:32 PM, Ade Lee wrote:
> >
> >      Added servlet to return 501 for rest operations for d9 instances
> >
> >      D9 instances run on tomcat6, which does not have support for the
> >      autheticator and realm.  We are not supporting the REST operations
> >      on D9 style instances.  They will need to be migrated.
> >
> >      The migration framework has been modified to process d9 or d10
> >      style instances, and a migration script has been added to add the new
> >      servlet to existing d9 instances.
> 
> Some comments (some of which have been discussed over IRC):
> 
> 1. To be consistent the pki_subsystem and pki_instance classes should be 
> called PKISubsystem and PKIInstance.

done
> 
> 2. In PKIUpgrader's constructor the instance/subsystem objects are only 
> created for validation then discarded. It might be better to move the 
> validation inside the instances() or subsystems() where the objects are 
> actually created and stored.
> 
done - we now have a validate() method which is called by the
constructor.

> 3. There seems to be a bug in these lines:
> 
>    inst = pki.pki_instance(instance, instance_type)
>    ...
>    subs = pki.pki_instance(instance, subsystem, instance_type)
> 
> The second line probably should have been calling pki.pki_subsystem() 
> and the parameter should have been inst (the instance object, not the name).
> 

Done. This is validation code that has been removed.

> 4. To improve clarity, the subsystem/instance name variable should be 
> called 'name' inside the corresponding class, and 'subsystemName' or 
> 'instanceName' outside the class.
> 
Done

> 5. The PKISubsystem constructor shouldn't need to take the type 
> parameter because it can be obtained from the instance.
> 
Done

> 6. The message in RESTServlet could be simplified into something like this:
> 
>    The REST services are not available because this server is a legacy
>    Dogtag 9 server. To access the REST services this server must be
>    migrated into a new Dogtag 10 server.
> 
Done
> 7. Instead of overriding doGet() and doPost() it's also possible to just 
> override service().
> 
Done

> 8. The --instance-type parameter works differently from the --instance 
> and --subsystem parameters. By default pki-upgrade will upgrade all 
> instances and subsystem on the system. If the --instance or --subsystem 
> is specified, it will narrow the scope to the specified 
> instance/subsystem only. The --instance-type, on the other hand, works 
> as an additional parameter to --instance and has a default value of 10.
> 
> Suppose there is a mix of Dogtag 9 and Dogtag 10 instances and some may 
> have the same names (e.g. D9 pki-ca and D10 pki-ca). The parameters will 
> work like this:
> 
>    pki-upgrade  ==> upgrade all instances
> 
>    pki-upgrade --instance pki-ca  ==> upgrade D10 pki-ca only
> 
>    pki-upgrade --instance pki-ca --instance-type 9
>      ==> upgrade D9 pki-ca only
> 
> To be consistent it should work like this:
> 
>    pki-upgrade --instance pki-ca  ==> upgrade both pki-ca instances
> 
>    pki-upgrade --instance pki-ca --instance-type 10
>      ==> upgrade D10 pki-ca only
> 
>    pki-upgrade --instance-type 9  ==> upgrade all D9 instances
> 
Done

> 9. We discussed about putting the context XML in 
> <instance>/conf/Catalina/localhost instead of META-INF. Is this going to 
> be handled by a separate upgrade script?
> 
To be handled as a separate upgrade script when we do the 10.1
customization work.

> 10. Document root could be obtained using self.doc.getroot() instead of 
> self.doc.find('.').
> 
Done
> 11. Element index could be obtained using self.root.index(mapping) 
> instead of list(self.root).index(mapping).
> 
Done

> 12. There are some trailing whitespaces.
> 
Fixed
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pki-vakwetu-0127-2-Added-servlet-to-return-501-for-rest-operations-for-.patch
Type: text/x-patch
Size: 34271 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20130422/69b1371c/attachment.bin>