Once 'pkispawn' and 'pkidestroy' have completed their initial implementation, the entire code base should be re-factored.
Things to look for include the following:
- moving individual items into lists as appropriate and traversing the lists - improve exception handling
Instead of using a static list (which still needs to be maintained) the pkispawn can use several template/source folders (e.g. tomcat, apache, ca, kra) and then copy all files in it while doing customization/slot substitutions. So when pkispawn creates a new Tomcat instance it will copy & customize all files from the 'tomcat' template folder into the deployment folder. Similarly, when pkispawn deploys a new subsystem it will copy & customize all files from the corresponding template folder into the deployment folder. So the code may look something like this:
sub deploy { $tomcat_deploy_dir = "/var/lib/pki/$instance_name"; $subsystem_deploy_dir = "$tomcat_deploy_dir/$subsystem_name"; %dict = ... slot substitutions ...; @exceptions = [ '*.jpg', '*.gif' ]; if (! $tomcat_deployed) { copy_and_customize( $tomcat_template_dir, $tomcat_deploy_dir, %dict, @exceptions); } copy_and_customize( $subsystem_template_dir, $subsystem_deploy_dir, %dict, @exceptions); }
This way any new files will be picked up automatically by pkispawn, and it's possible to maintain different set of files for each subsystem template folder without requiring any specific customization code.
Currently pkispawn & pkidestroy use a loop to load and execute the deployment scriptlets:
for pki_scriptlet in pki_subsystem_scriptlets: scriptlet = __import__("pki.deployment" +\ "." + pki_scriptlet[4:], fromlist = [pki_scriptlet[4:]]) instance = scriptlet.PkiScriptlet() if not config.pki_update_flag: rv = instance.spawn() else: rv = instance.respawn() if rv != 0: sys.exit(1)
All scriptlets are warpped in spawn/respawn/destroy() methods which take no parameters and return an error code. If we need to pass some objects from one scriptlet to another we'd have to use global variables which will make the code harder to read.
It might be better to convert these scriptlets into methods that will be called by a Deployer object. This way we will have more flexibility to use method parameters and return values to pass objects between scriptlets.
So in pkispawn it will call the deployer:
deployer = <TomcatDeployer, ApacheDeployer, or custom Deployer> deployer.spawn() if not config.pki_update_flag: rv = deployer.spawn() else: rv = deployer.respawn()
In Tomcat deployer it will call the scriptlets which have been converted into methods:
class TomcatDeployer(Deployer): def spawn(self): initialize() configure() deploy_wars() ... finalize()
There should be a mechanism to register a custom Deployer.
As currently designed, scriptlets are well-defined basic pluggable classes which always consist of spawn(), respawn(), and destroy() methods - see http://pki.fedoraproject.org/wiki/PKI_Instance_Deployment#PKI_Scriptlets.
While it is true that they utilize a global dictionary for passing data, they were designed to be very easy to customize and re-order, I therefore disagree with the https://fedorahosted.org/pki/ticket/228#comment:3 proposal to throw-away this notion of scriptlets and turn them into methods within a class as this hard-codes the order of execution (unless multiple customized deployers are utilize) which may not always be the same for every PKI subsystem/instance, and is somewhat reminiscent of the "pkicreate" and "pkiremove" Perl scripts (albeit more object-oriented rather than procedural).
However, I agree with the https://fedorahosted.org/pki/ticket/228#comment:3 observation that the existing looping infrastructure used to process these servlets could be made far more maintainable.
Therefore, for ease of maintenance and customizability, I would propose that the current implementation of ordering which utilizes numbered symlinks should be replaced to use an ordered list that is read in from three new parameters located within each PKI Subsystem's section within the 'pkideployment.cfg' file.
This approach would still use a modified loop and global dictionary to process individual scriptlets, but would greatly simplify customization and maintenance by removing any need for symlinks.
For example, the [CA] section might look something like this:
* pki_spawn_scriptlet_order=initialization, infrastructure_layout, instance_layout, subsystem_layout, selinux_setup, webapp_deployment, slot_substitution, security_databases, configuration, finalization * pki_respawn_scriptlet_order=initialization, infrastructure_layout, instance_layout, subsystem_layout, selinux_setup, webapp_deployment, slot_substitution, security_databases, configuration, finalization * pki_destroy_scriptlet_order=initialization, configuration, webapp_deployment, subsystem_layout, security_databases, instance_layout, selinux_setup, infrastructure_layout, finalization
This alternative would allow an administrator to more easily change the order, delete, and/or add customized scriptlets to suit their specific needs while at the same time suggesting a default ordering useful for the vast majority of deployments.
Create utility classes for the ldap ds functions and security domain functions that are currently under pkiparser.py, and move them to pkihelper.py.
Clean up the which calls them in pkispawn encapsulating them in some "verify()" functions:
def verify_ds_availability(self): if not config.str2bool(self.pki_master_dict['pki_spawn_interactive'])\ and not\ config.str2bool(self.pki_master_dict['pki_skip_configuration']): try: self.ds_connect() self.ds_bind() if self.ds_base_dn_exists() and not\ config.str2bool(self.pki_master_dict['pki_ds_remove_data']): config.pki_log.error(log.PKI_LDAP_BASE_DN_EXISTS, extra=config.PKI_INDENTATION_LEVEL_2) raise self.ds_close() except ldap.LDAPError as e: config.pki_log.error(log.PKI_LDAP_UNREACHABLE_1, e.message['desc'], extra=config.PKI_INDENTATION_LEVEL_2) raise def verify_sd_availability(self): if not config.str2bool(self.pki_master_dict['pki_spawn_interactive'])\ and not\ config.str2bool(self.pki_master_dict['pki_skip_configuration']): if config.pki_subsystem != "CA" or\ config.str2bool(self.pki_master_dict['pki_clone']) or\ config.str2bool(self.pki_master_dict['pki_subordinate']): try: self.sd_connect() info = self.sd_get_info() self.set_property(config.pki_subsystem, 'pki_security_domain_name', info.name) self.sd_authenticate() except requests.exceptions.ConnectionError as e: config.pki_log.error( log.PKI_SECURITY_DOMAIN_INACCESSIBLE_1, str(e), extra=config.PKI_INDENTATION_LEVEL_2) raise except requests.exceptions.HTTPError as e: config.pki_log.error( log.PKI_SECURITY_DOMAIN_INACCESSIBLE_1, str(e), extra=config.PKI_INDENTATION_LEVEL_2) raise
and place them as calls at the end of spawn() in the initialization.py scriptlet:
# verify selinux context of selected ports deployer.configuration_file.populate_non_default_ports() deployer.configuration_file.verify_selinux_ports() + # verify availability of LDAP server + deployer.ds.verify_ds_availability() + # verify availability of security domain + deployer.sd.verify_sd_availability() return self.rv
Metadata Update from @mharmsen: - Issue assigned to mharmsen - Issue set to the milestone: UNTRIAGED
Dogtag PKI is moving from Pagure issues to GitHub issues. This means that existing or new issues will be reported and tracked through Dogtag PKI's GitHub Issue tracker.
This issue has been cloned to GitHub and is available here: https://github.com/dogtagpki/pki/issues/799
If you want to receive further updates on the issue, please navigate to the GitHub issue and click on Subscribe button.
Subscribe
Thank you for understanding, and we apologize for any inconvenience.
Metadata Update from @dmoluguw: - Issue close_status updated to: migrated - Issue status updated to: Closed (was: Open)
Login to comment on this ticket.