From 184fe957b4f13194f15264307e3f80e2aafbc600 Mon Sep 17 00:00:00 2001 From: Robert Chipperfield Date: Thu, 29 Jul 2021 14:56:26 +0100 Subject: [PATCH] Fix issues with race conditions connecting to Vault (#65) Co-authored-by: Peter Souter --- lib/puppet/functions/hiera_vault.rb | 157 ++++++++++++++++------------ 1 file changed, 88 insertions(+), 69 deletions(-) diff --git a/lib/puppet/functions/hiera_vault.rb b/lib/puppet/functions/hiera_vault.rb index 5d9ca95..ed74b06 100644 --- a/lib/puppet/functions/hiera_vault.rb +++ b/lib/puppet/functions/hiera_vault.rb @@ -15,6 +15,11 @@ rescue LoadError => e raise Puppet::DataBinding::LookupError, "[hiera-vault] Must install debouncer gem to use hiera-vault backend" end + begin + require 'thread' + rescue LoadError => e + raise Puppet::DataBinding::LookupError, "[hiera-vault] Must install thread gem to use hiera-vault backend" + end dispatch :lookup_key do @@ -23,8 +28,14 @@ param 'Puppet::LookupContext', :context end - $vault = Vault::Client.new - $shutdown = Debouncer.new(10) { $vault.shutdown() } + $hiera_vault_mutex = Mutex.new + $hiera_vault_client = Vault::Client.new + $hiera_vault_shutdown = Debouncer.new(10) { + $hiera_vault_mutex.synchronize do + $hiera_vault_client.shutdown() + $hiera_vault_client = nil + end + } def vault_token(options) token = nil @@ -98,76 +109,83 @@ def vault_get(key, options, context) raise ArgumentError, "[hiera-vault] invalid value for default_field_behavior: '#{options['default_field_behavior']}', should be one of 'ignore','only'" end - begin - $vault.configure do |config| - config.address = options['address'] unless options['address'].nil? - config.token = vault_token(options) - config.ssl_pem_file = options['ssl_pem_file'] unless options['ssl_pem_file'].nil? - config.ssl_verify = options['ssl_verify'] unless options['ssl_verify'].nil? - config.ssl_ca_cert = options['ssl_ca_cert'] if config.respond_to? :ssl_ca_cert - config.ssl_ca_path = options['ssl_ca_path'] if config.respond_to? :ssl_ca_path - config.ssl_ciphers = options['ssl_ciphers'] if config.respond_to? :ssl_ciphers + $hiera_vault_mutex.synchronize do + # If our Vault client has got cleaned up by a previous shutdown call, reinstate it + if $hiera_vault_client.nil? + $hiera_vault_client = Vault::Client.new end - if $vault.sys.seal_status.sealed? - raise Puppet::DataBinding::LookupError, "[hiera-vault] vault is sealed" + + begin + $hiera_vault_client.configure do |config| + config.address = options['address'] unless options['address'].nil? + config.token = vault_token(options) + config.ssl_pem_file = options['ssl_pem_file'] unless options['ssl_pem_file'].nil? + config.ssl_verify = options['ssl_verify'] unless options['ssl_verify'].nil? + config.ssl_ca_cert = options['ssl_ca_cert'] if config.respond_to? :ssl_ca_cert + config.ssl_ca_path = options['ssl_ca_path'] if config.respond_to? :ssl_ca_path + config.ssl_ciphers = options['ssl_ciphers'] if config.respond_to? :ssl_ciphers + end + + if $hiera_vault_client.sys.seal_status.sealed? + raise Puppet::DataBinding::LookupError, "[hiera-vault] vault is sealed" + end + + context.explain { "[hiera-vault] Client configured to connect to #{$hiera_vault_client.address}" } + rescue StandardError => e + $hiera_vault_shutdown.call + $hiera_vault_client = nil + raise Puppet::DataBinding::LookupError, "[hiera-vault] Skipping backend. Configuration error: #{e}" end - context.explain { "[hiera-vault] Client configured to connect to #{$vault.address}" } - rescue StandardError => e - $shutdown.call - $vault = nil - raise Puppet::DataBinding::LookupError, "[hiera-vault] Skipping backend. Configuration error: #{e}" - end + answer = nil - answer = nil + if options['mounts']['generic'] + raise ArgumentError, "[hiera-vault] generic is no longer valid - change to kv" + else + kv_mounts = options['mounts'].dup + end - if options['mounts']['generic'] - raise ArgumentError, "[hiera-vault] generic is no longer valid - change to kv" - else - kv_mounts = options['mounts'].dup - end + # Only kv mounts supported so far + kv_mounts.each_pair do |mount, paths| + interpolate(context, paths).each do |path| + secretpath = context.interpolate(File.join(mount, path)) - # Only kv mounts supported so far - kv_mounts.each_pair do |mount, paths| - interpolate(context, paths).each do |path| - secretpath = context.interpolate(File.join(mount, path)) + context.explain { "[hiera-vault] Looking in path #{secretpath} for #{key}" } - context.explain { "[hiera-vault] Looking in path #{secretpath} for #{key}" } + secret = nil - secret = nil + paths = [] - paths = [] + if options.fetch("v2_guess_mount", true) + paths << [:v2, File.join(mount, path, 'data', key).chomp('/')] + paths << [:v2, File.join(mount, 'data', path, key).chomp('/')] + else + paths << [:v2, File.join(mount, path, key).chomp('/')] + paths << [:v2, File.join(mount, key).chomp('/')] if key.start_with?(path) + end - if options.fetch("v2_guess_mount", true) - paths << [:v2, File.join(mount, path, 'data', key).chomp('/')] - paths << [:v2, File.join(mount, 'data', path, key).chomp('/')] - else - paths << [:v2, File.join(mount, path, key).chomp('/')] - paths << [:v2, File.join(mount, key).chomp('/')] if key.start_with?(path) - end + paths << [:v1, File.join(mount, path, key)] if options.fetch("v1_lookup", true) - paths << [:v1, File.join(mount, path, key)] if options.fetch("v1_lookup", true) - - paths.each do |version_path| - begin - version, path = version_path[0], version_path[1] - context.explain { "[hiera-vault] Checking path: #{path}" } - response = $vault.logical.read(path) - next if response.nil? - secret = version == :v1 ? response.data : response.data[:data] - rescue Vault::HTTPConnectionError - context.explain { "[hiera-vault] Could not connect to read secret: #{secretpath}" } - rescue Vault::HTTPError => e - context.explain { "[hiera-vault] Could not read secret #{secretpath}: #{e.errors.join("\n").rstrip}" } + paths.each do |version_path| + begin + version, path = version_path[0], version_path[1] + context.explain { "[hiera-vault] Checking path: #{path}" } + response = $hiera_vault_client.logical.read(path) + next if response.nil? + secret = version == :v1 ? response.data : response.data[:data] + rescue Vault::HTTPConnectionError + context.explain { "[hiera-vault] Could not connect to read secret: #{secretpath}" } + rescue Vault::HTTPError => e + context.explain { "[hiera-vault] Could not read secret #{secretpath}: #{e.errors.join("\n").rstrip}" } + end end - end - next if secret.nil? + next if secret.nil? - context.explain { "[hiera-vault] Read secret: #{key}" } - if (options['default_field'] and ( ['ignore', nil].include?(options['default_field_behavior']) || - (secret.has_key?(options['default_field'].to_sym) && secret.length == 1) ) ) + context.explain { "[hiera-vault] Read secret: #{key}" } + if (options['default_field'] and ( ['ignore', nil].include?(options['default_field_behavior']) || + (secret.has_key?(options['default_field'].to_sym) && secret.length == 1) ) ) return nil if ! secret.has_key?(options['default_field'].to_sym) @@ -182,23 +200,24 @@ def vault_get(key, options, context) end else - # Turn secret's hash keys into strings allow for nested arrays and hashes - # this enables support for create resources etc - new_answer = secret.inject({}) { |h, (k, v)| h[k.to_s] = stringify_keys v; h } - end + # Turn secret's hash keys into strings allow for nested arrays and hashes + # this enables support for create resources etc + new_answer = secret.inject({}) { |h, (k, v)| h[k.to_s] = stringify_keys v; h } + end - unless new_answer.nil? - answer = new_answer - break + unless new_answer.nil? + answer = new_answer + break + end end + + break unless answer.nil? end - break unless answer.nil? + answer = context.not_found if answer.nil? + $hiera_vault_shutdown.call + return answer end - - answer = context.not_found if answer.nil? - $shutdown.call - return answer end # Stringify key:values so user sees expected results and nested objects