Merge pull request #128 from puppetlabs/better-idempotency-for-convert

Don't re-issue certs if they have required exts

Author: reidmv

Diff for: CHANGELOG.md

@@ -1,5 +1,18 @@
 # PEADM module
 
+## Unreleased
+### Summary
+
+Readme updates and further convert plan efficiency improvements
+
+### Features
+
+- In the peadm::convert plan, certificates which already contain requested extensions will not be re-issued. This will accelerate the convert process, or allow re-runs of the convert process to move more quickly.
+
+### Improvements
+
+- The README now provides more detailed information on how customers using the peadm module should go about getting support for it.
+
 ## 2.3.0
 ### Summary
 

Diff for: plans/convert.pp

@@ -177,9 +177,8 @@
   peadm::plan_step('convert-compilers-a') || {
     run_plan('peadm::util::add_cert_extensions', $compiler_a_targets,
       master_host => $master_target,
-      remove      => ['1.3.6.1.4.1.34380.1.3.13'], # OID form of pp_auth_role
       extensions  => {
-        'pp_auth_role'                         => 'pe_compiler',
+        peadm::oid('pp_auth_role')             => 'pe_compiler',
         peadm::oid('peadm_availability_group') => 'A',
       },
     )
@@ -188,9 +187,8 @@
   peadm::plan_step('convert-compilers-b') || {
     run_plan('peadm::util::add_cert_extensions', $compiler_b_targets,
       master_host => $master_target,
-      remove      => ['1.3.6.1.4.1.34380.1.3.13'], # OID form of pp_auth_role
       extensions  => {
-        'pp_auth_role'                         => 'pe_compiler',
+        peadm::oid('pp_auth_role')             => 'pe_compiler',
         peadm::oid('peadm_availability_group') => 'B',
       },
     )
@@ -236,5 +234,5 @@
     run_task('peadm::puppet_runonce', $all_targets - $master_target)
   }
 
-  return("Conversion to peadm Puppet Enterprise ${arch['architecture']} succeeded.")
+  return("Conversion to peadm Puppet Enterprise ${arch['architecture']} completed.")
 }

Diff for: plans/util/add_cert_extensions.pp

@@ -38,11 +38,20 @@
 
   # Loop through and recert each target one at at time, because Bolt lacks
   # real parallelism
-  $all_targets.map |$target| {
+  $all_targets.each |$target| {
     $certname = $certdata[$target]['certname']
+    $existing_exts = $certdata[$target]['extensions']
 
     # This will be the new trusted fact data for this node
-    $extension_requests = $certdata[$target]['extensions'] + $extensions
+    $extension_requests = $existing_exts + $extensions
+
+    # If the existing certificate meets all the requirements, there's no need
+    # to regenerate it. Skip it and move on to the next.
+    if (($extension_requests.all |$key,$val| { $existing_exts[$key] == $val }) and
+        !($remove.any |$key| { $key in $existing_exts.keys })) {
+      out::message("${certname} already has requested extensions; certificate will not be re-issued")
+      next()
+    }
 
     # Everything starts the same; we always stop the agent and revoke the
     # existing cert. We use `run_command` in case the master is 2019.x but