From 3a30432fcaa2e53eec8ea6a8f756d348b115c99e Mon Sep 17 00:00:00 2001
From: Ben Roberts <ben.roberts@gsacapital.com>
Date: Tue, 28 Jun 2022 22:46:05 +0100
Subject: [PATCH] Converge quorum member auth

The current code for authenticating to quorum members generates a
change event for every puppet run due to the exec having not being
`refreshonly` or having any conditionals. This cases the pcsd tokens
file to be updated regularly as well.

The proposed change splits up the authentication so that it's done
once per quorum member, rather than doing them all in one go. It
also adds a conditional check to see if any authentication token
is already present in the pcsd tokens file, and skips the exec if so.

This is convergent, but comes with two minor costs:
- if quorum member hostnames overlap (e.g. `foo` and `foobar`),
  then the condition will match perhaps incorrectly and one
  hostname may not get added
- If for any reason the authentication token becomes invalid,
  puppet will not correct it, and manual intervention will be
  required (in the form of a `pcs cluster auth` or `pcs host auth`
  command)

A `pcs host deauth` command would be handled correctly, and puppet
would do a corrective re-auth.

Fixes #500
---
 manifests/init.pp | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/manifests/init.pp b/manifests/init.pp
index 1b4fe15f..e4c8c844 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -589,7 +589,6 @@
       # list the members to join.
       # TODO - verify that this is safe when quorum_members is a list of IP
       # addresses
-      $node_string = join($quorum_members, ' ')
 
       # Define the pcs host command, this changed with 0.10.0 as per #513
       $pcs_auth_command = versioncmp($pcs_version, '0.10.0') ? {
@@ -597,17 +596,17 @@
         default => 'pcs host auth',
       }
 
-      # Attempt to authorize all members. The command will return successfully
-      # if they were already authenticated so it's safe to run every time this
-      # is applied.
-      # TODO - make it run only once
-      exec { 'authorize_members':
-        command => "${pcs_auth_command} ${node_string} ${auth_credential_string}",
-        path    => $exec_path,
-        require => [
-          Service['pcsd'],
-          User['hacluster'],
-        ],
+      # Attempt to authorize each member
+      $quorum_members.each |$node| {
+        exec { "authorize_member_${node}":
+          command => "${pcs_auth_command} ${node} ${auth_credential_string}",
+          unless  => "grep '${node}' /var/lib/pcsd/tokens",
+          path    => $exec_path,
+          require => [
+            Service['pcsd'],
+            User['hacluster'],
+          ],
+        }
       }
     }
 
@@ -636,7 +635,7 @@
         command => "pcs cluster setup --force ${pcs_cluster_setup_namearg} ${cluster_name} ${node_string}",
         path    => $exec_path,
         onlyif  => 'test ! -f /etc/corosync/corosync.conf',
-        require => Exec['authorize_members'],
+        require => Exec[$quorum_members.map |$node| {"authorize_member_${node}"}],
       }
       # We need to do this so the temporary cluster doesn't delete our authkey
       if $enable_secauth {
@@ -655,7 +654,7 @@
         onlyif  => $qdevice_token_check,
         require => [
           Package[$package_quorum_device],
-          Exec['authorize_members'],
+          Exec[$quorum_members.map |$node| {"authorize_member_${node}"}],
           Exec['pcs_cluster_temporary'],
         ],
       }