Skip to content

Commit

Permalink
Converge quorum member auth
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@optiz0r
optiz0r committed Jun 29, 2022
1 parent 5f48a6c commit 302162a
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 34 deletions.
26 changes: 13 additions & 13 deletions manifests/init.pp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -597,17 +597,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'],
],
}
}
}

Expand Down Expand Up @@ -636,7 +636,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 {
Expand All @@ -655,7 +655,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'],
],
}
Expand Down
56 changes: 35 additions & 21 deletions spec/classes/corosync_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,7 @@
let(:node) { 'node2.test.org' }

it 'does not perform the auth' do
is_expected.not_to contain_exec('authorize_members')
is_expected.not_to contain_exec('authorize_member_node2.test.org')
end
end

Expand All @@ -805,14 +805,17 @@
end

it 'authorizes all nodes' do
is_expected.to contain_exec('authorize_members').with(
command: "pcs #{auth_command} node1.test.org node2.test.org node3.test.org -u hacluster -p some-secret-sauce",
path: '/sbin:/bin:/usr/sbin:/usr/bin',
require: [
'Service[pcsd]',
'User[hacluster]'
]
)
['node1.test.org', 'node2.test.org', 'node3.test.org'].each do |node|
is_expected.to contain_exec("authorize_member_#{node}").with(
command: "pcs #{auth_command} #{node} -u hacluster -p some-secret-sauce",
unless: "grep '#{node}' /var/lib/pcsd/tokens",
path: '/sbin:/bin:/usr/sbin:/usr/bin',
require: [
'Service[pcsd]',
'User[hacluster]'
]
)
end
end
end

Expand All @@ -836,15 +839,18 @@

let(:facts) { override_facts(super(), networking: { ip: '192.168.0.10' }) }

it 'match ip and auth nodes by member names' do
is_expected.to contain_exec('authorize_members').with(
command: "pcs #{auth_command} 192.168.0.10 192.168.0.12 192.168.0.13 -u hacluster -p some-secret-sauce",
path: '/sbin:/bin:/usr/sbin:/usr/bin',
require: [
'Service[pcsd]',
'User[hacluster]'
]
)
['192.168.0.10', '192.168.0.12', '192.168.0.13'].each do |node|
it 'match ip and auth nodes by member names' do
is_expected.to contain_exec("authorize_member_#{node}").with(
command: "pcs #{auth_command} #{node} -u hacluster -p some-secret-sauce",
unless: "grep '#{node}' /var/lib/pcsd/tokens",
path: '/sbin:/bin:/usr/sbin:/usr/bin',
require: [
'Service[pcsd]',
'User[hacluster]'
]
)
end
end

context 'where the auth-node IP is not the default IP' do
Expand All @@ -864,7 +870,9 @@
end

it 'still detects that this is the auth-node' do
is_expected.to contain_exec('authorize_members')
is_expected.to contain_exec('authorize_member_192.168.0.10')
is_expected.to contain_exec('authorize_member_192.168.0.12')
is_expected.to contain_exec('authorize_member_192.168.0.13')
end
end
end
Expand Down Expand Up @@ -1017,7 +1025,11 @@
command: "pcs cluster setup --force #{cluster_name_arg} cluster_test node1.test.org node2.test.org node3.test.org",
path: '/sbin:/bin:/usr/sbin:/usr/bin',
onlyif: 'test ! -f /etc/corosync/corosync.conf',
require: 'Exec[authorize_members]'
require: [
'Exec[authorize_member_node1.test.org]',
'Exec[authorize_member_node2.test.org]',
'Exec[authorize_member_node3.test.org]',
]
)
end

Expand All @@ -1028,7 +1040,9 @@
onlyif: 'test 0 -ne $(grep quorum1.test.org /var/lib/pcsd/tokens >/dev/null 2>&1; echo $?)',
require: [
'Package[corosync-qdevice]',
'Exec[authorize_members]',
'Exec[authorize_member_node1.test.org]',
'Exec[authorize_member_node2.test.org]',
'Exec[authorize_member_node3.test.org]',
'Exec[pcs_cluster_temporary]'
]
)
Expand Down

0 comments on commit 302162a

Please sign in to comment.