voxpupuli/puppet-corosync

When Managing PCS Auth, pcs_cluster_auth Executes Every Puppet Run

Opened this issue · 1 comments

Affected Puppet, Ruby, OS and module versions/distributions

  • Puppet: 5.5.20
  • Ruby: 2.0.0.648
  • Distribution: RHEL 7.9
  • Module version: v7.0.0

How to reproduce (e.g Puppet code you use)

class { 'corosync':
  manage_pcsd_service          => true,
  manage_pcsd_auth             => true,
  sensitive_hacluster_password => Sensitive('this-is-the-actual-password'),
  sensitive_hacluster_hash     => Sensitive('a-hash-of-the-passwd-for-the-user-resource'),
}

What are you seeing

When managing pcs auth via this module, puppet runs Exec/pcs_cluster_auth to authenticate pcs nodes on every run. This seems to be by design based on the comments in the code snippet below. Puppet best practice however expects configuration to converge to a stable state. This can never happen if a "change" happens every run. This causes problems for compliance reporting and deployment pipelines that expect clean Puppet runs.

manifests/init.pp

      # 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 { 'pcs_cluster_auth':
        command => "pcs cluster auth ${node_string} ${auth_credential_string}",
        path    => $exec_path,
        require => [
          Service['pcsd'],
          User['hacluster'],
        ],
      }
    }

What behavior did you expect instead

Cluster auth should only run once or as needed and not every Puppet run. This will allow Puppet to converge to a stable state.

Output log

Notice: /Stage[main]/Corosync/Exec[pcs_cluster_auth]/returns: executed successfully (corrective)

Any additional information you'd like to impart

I've just run into this myself. The pcs docs indicate the var/lib/pcsd/tokens is used to store auth tokens when run as root, which it is when run under puppet using this module. This file is currently a json document containing

{
  "format_version": 3,
  "data_version": 8,
  "tokens": {
    "hostname": "token"
  }
  "ports": {
    "hostname": 2224
  }
}

The simplest naive fix for this issue would be to add an unless clause that checks for the presence of all hostnames in this file. I've just done a quick test of this code, and it appears to do the right thing in my case, even if it is rather hacky.

$auth_check_command = $quorum_members.map |$node| {
  "grep '${node}' /var/lib/pcsd/tokens"
}.join(" && ")

exec { 'authorize_members':
  command => "pcs cluster auth ${node_string} ${auth_credential_string}",
  unless => $auth_check_command,
  path    => $exec_path,
  require => [
    Service['pcsd'],
    User['hacluster'],
    ],
}

Since what goes in the auth file is a token, not the original password, this won't validate that the auth is correct, just that it has previously been authenticated. If for any reason the authentication is invalidated, it would have to be corrected by manually re-running the pcs cluster auth or pcs host auth commands.

This code will also fail if the hostnames used overlap with each other (like foo and foobar), or match any of the terms in the file.

The use of multiple greps pipelined together is because currently a single auth command is run for all quorum members in one go. Alternatively the auth exec could be repeated separately for each quorum member, and then a single grep could check that each specific quorum member is present, which is a bit cleaner.

$quorum_members.each |$node| {
  exec { "authorize_member_$node":
    command => "pcs cluster auth ${node} ${auth_credential_string}",
    unless => "grep '$node' /var/lib/pcsd/tokens",
    path    => $exec_path,
    require => [
      Service['pcsd'],
      User['hacluster'],
      ],
  }
}

A more technically correct check for a single hostname in the tokens hash would be this, but it depends on jq, which may not be available.

jq -e </var/lib/pcsd/tokens ".tokens[\"hostname\"] | if .==null then false else true end"