From 4ab8330c692295917386af93c4f819ee011641c1 Mon Sep 17 00:00:00 2001
From: Reid Vandewiele <reid@puppet.com>
Date: Tue, 22 Sep 2020 16:39:34 -0700
Subject: [PATCH] Don't re-issue certs if they have required exts

Basically, don't do work that isn't necessary. This commit changes how
shortname/OID conflicts are handled for the pp_auth_role extension by
just standardizing on always specifying OID, for now.
---
 CHANGELOG.md                      | 13 +++++++++++++
 plans/convert.pp                  |  8 +++-----
 plans/util/add_cert_extensions.pp | 13 +++++++++++--
 3 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7b0f115b..527fb158 100644
--- a/CHANGELOG.md
+++ b/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 --git a/plans/convert.pp b/plans/convert.pp
index bd15b588..526d3dc5 100644
--- a/plans/convert.pp
+++ b/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 --git a/plans/util/add_cert_extensions.pp b/plans/util/add_cert_extensions.pp
index a2fd4d44..66e1d35b 100644
--- a/plans/util/add_cert_extensions.pp
+++ b/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