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

[Endi Sukma Dewata]

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.

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.

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).

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

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

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.

7. Instead of overriding doGet() and doPost() it's also possible to just 
override service().

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

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?

10. Document root could be obtained using self.doc.getroot() instead of 
self.doc.find('.').

11. Element index could be obtained using self.root.index(mapping) 
instead of list(self.root).index(mapping).

12. There are some trailing whitespaces.

-- 
Endi S. Dewata