From 43f085352b7870721e066b88424c7023261e6892 Mon Sep 17 00:00:00 2001 From: isaac-hammes Date: Thu, 17 Aug 2023 13:41:15 -0700 Subject: [PATCH 01/77] (POD-10) Log reason for failed VM checks. --- lib/vmpooler/api/helpers.rb | 12 ------------ lib/vmpooler/pool_manager.rb | 12 +++++++----- spec/unit/pool_manager_spec.rb | 8 ++++---- 3 files changed, 11 insertions(+), 21 deletions(-) diff --git a/lib/vmpooler/api/helpers.rb b/lib/vmpooler/api/helpers.rb index c6351a9..c56834a 100644 --- a/lib/vmpooler/api/helpers.rb +++ b/lib/vmpooler/api/helpers.rb @@ -551,18 +551,6 @@ module Vmpooler end end end - - def vm_ready?(vm_name, domain = nil) - tracer.in_span("Vmpooler::API::Helpers.#{__method__}") do - begin - open_socket(vm_name, domain) - rescue StandardError => _e - return false - end - - true - end - end end end end diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 3bc020b..d6735c1 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -103,7 +103,7 @@ module Vmpooler mutex.synchronize do @redis.with_metrics do |redis| request_id = redis.hget("vmpooler__vm__#{vm}", 'request_id') - if provider.vm_ready?(pool, vm) + if provider.vm_ready?(pool, vm, redis) move_pending_vm_to_ready(vm, pool, redis, request_id) else fail_pending_vm(vm, pool, timeout, redis) @@ -130,6 +130,7 @@ module Vmpooler if exists request_id = redis.hget("vmpooler__vm__#{vm}", 'request_id') pool_alias = redis.hget("vmpooler__vm__#{vm}", 'pool_alias') if request_id + open_socket_error = redis.hget("vmpooler__vm__#{vm}", 'open_socket_error') redis.smove("vmpooler__pending__#{pool}", "vmpooler__completed__#{pool}", vm) if request_id ondemandrequest_hash = redis.hgetall("vmpooler__odrequest__#{request_id}") @@ -139,7 +140,7 @@ module Vmpooler end end $metrics.increment("errors.markedasfailed.#{pool}") - $logger.log('d', "[!] [#{pool}] '#{vm}' marked as 'failed' after #{timeout} minutes") + $logger.log('d', "[!] [#{pool}] '#{vm}' marked as 'failed' after #{timeout} minutes with error: #{open_socket_error}") else remove_nonexistent_vm(vm, pool, redis) end @@ -197,11 +198,12 @@ module Vmpooler def vm_still_ready?(pool_name, vm_name, provider, redis) # Check if the VM is still ready/available - return true if provider.vm_ready?(pool_name, vm_name) + return true if provider.vm_ready?(pool_name, vm_name, redis) raise("VM #{vm_name} is not ready") rescue StandardError - move_vm_queue(pool_name, vm_name, 'ready', 'completed', redis, "is unreachable, removed from 'ready' queue") + open_socket_error = redis.hget("vmpooler__vm__#{vm_name}", 'open_socket_error') + move_vm_queue(pool_name, vm_name, 'ready', 'completed', redis, "removed from 'ready' queue. vm unreachable with error: #{open_socket_error}") end def check_ready_vm(vm, pool_name, ttl, provider) @@ -318,7 +320,7 @@ module Vmpooler redis.hset("vmpooler__vm__#{vm}", 'user_tagged', 'true') if success end - throw :stop_checking if provider.vm_ready?(pool, vm) + throw :stop_checking if provider.vm_ready?(pool, vm, redis) throw :stop_checking if provider.get_vm(pool, vm) diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index 8561896..c0f293d 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/spec/unit/pool_manager_spec.rb @@ -203,8 +203,8 @@ EOT context 'host is in pool' do it 'calls move_pending_vm_to_ready if host is ready' do - expect(provider).to receive(:vm_ready?).with(pool,vm).and_return(true) redis_connection_pool.with do |redis| + expect(provider).to receive(:vm_ready?).with(pool, vm, redis).and_return(true) expect(subject).to receive(:move_pending_vm_to_ready).with(vm, pool, redis, nil) end @@ -212,8 +212,8 @@ EOT end it 'calls fail_pending_vm if host is not ready' do - expect(provider).to receive(:vm_ready?).with(pool,vm).and_return(false) redis_connection_pool.with do |redis| + expect(provider).to receive(:vm_ready?).with(pool, vm, redis).and_return(false) expect(subject).to receive(:fail_pending_vm).with(vm, pool, timeout, redis) end @@ -298,7 +298,7 @@ EOT it 'logs message if VM has exceeded timeout and exists' do redis_connection_pool.with do |redis| redis.hset("vmpooler__vm__#{vm}", 'clone',Date.new(2001,1,1).to_s) - expect(logger).to receive(:log).with('d', "[!] [#{pool}] '#{vm}' marked as 'failed' after #{timeout} minutes") + expect(logger).to receive(:log).with('d', "[!] [#{pool}] '#{vm}' marked as 'failed' after #{timeout} minutes with error: ") expect(subject.fail_pending_vm(vm, pool, timeout, redis, exists: true)).to eq(true) end end @@ -628,7 +628,7 @@ EOT end it 'should log messages about being unreachable' do - expect(logger).to receive(:log).with('d', "[!] [#{pool}] '#{vm}' is unreachable, removed from 'ready' queue") + expect(logger).to receive(:log).with('d', "[!] [#{pool}] '#{vm}' removed from 'ready' queue. vm unreachable with error: ") subject._check_ready_vm(vm, pool, ttl, provider) end From 089071b1b901b5fcf6ab2e7b6f504fa5cc9aa0cd Mon Sep 17 00:00:00 2001 From: isaac-hammes Date: Fri, 18 Aug 2023 08:58:37 -0700 Subject: [PATCH 02/77] (maint) Release prep for version 3.4.0 --- CHANGELOG.md | 12 ++++++++++++ Gemfile.lock | 2 +- lib/vmpooler/version.rb | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f09aed2..36ea514 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Changelog +## [3.4.0](https://github.com/puppetlabs/vmpooler/tree/3.4.0) (2023-08-18) + +[Full Changelog](https://github.com/puppetlabs/vmpooler/compare/3.3.0...3.4.0) + +**Implemented enhancements:** + +- \(POD-10\) Log reason for failed VM checks. [\#611](https://github.com/puppetlabs/vmpooler/pull/611) ([isaac-hammes](https://github.com/isaac-hammes)) + +**Closed issues:** + +- Log reason connection on port 22 of a failed VM [\#609](https://github.com/puppetlabs/vmpooler/issues/609) + ## [3.3.0](https://github.com/puppetlabs/vmpooler/tree/3.3.0) (2023-08-16) [Full Changelog](https://github.com/puppetlabs/vmpooler/compare/3.2.0...3.3.0) diff --git a/Gemfile.lock b/Gemfile.lock index 59fe3b6..b7d1865 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - vmpooler (3.3.0) + vmpooler (3.4.0) concurrent-ruby (~> 1.1) connection_pool (~> 2.4) deep_merge (~> 1.2) diff --git a/lib/vmpooler/version.rb b/lib/vmpooler/version.rb index af71ac2..1ffb7ab 100644 --- a/lib/vmpooler/version.rb +++ b/lib/vmpooler/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Vmpooler - VERSION = '3.3.0' + VERSION = '3.4.0' end From 5146d32bf3c5bee32949666b53fa65d30bbd3991 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 18 Aug 2023 22:16:46 +0000 Subject: [PATCH 03/77] Bump puma from 6.3.0 to 6.3.1 Bumps [puma](https://github.com/puma/puma) from 6.3.0 to 6.3.1. - [Release notes](https://github.com/puma/puma/releases) - [Changelog](https://github.com/puma/puma/blob/master/History.md) - [Commits](https://github.com/puma/puma/compare/v6.3.0...v6.3.1) --- updated-dependencies: - dependency-name: puma dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- Gemfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index b7d1865..ccec34f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -111,9 +111,9 @@ GEM coderay (~> 1.1) method_source (~> 1.0) spoon (~> 0.0) - puma (6.3.0) + puma (6.3.1) nio4r (~> 2.0) - puma (6.3.0-java) + puma (6.3.1-java) nio4r (~> 2.0) racc (1.7.1) racc (1.7.1-java) From fdbb0f3a77cf090ee0e88b1930668197fa7f2fa9 Mon Sep 17 00:00:00 2001 From: Jake Spain Date: Fri, 18 Aug 2023 17:36:12 -0400 Subject: [PATCH 04/77] Add ability to use bind_as with a service account --- Gemfile.lock | 1 + docs/configuration.md | 12 +++ lib/vmpooler/api/helpers.rb | 30 +++++- spec/unit/api/helpers_spec.rb | 182 ++++++++++++++++++++++------------ vmpooler.yaml.example | 26 +++++ 5 files changed, 185 insertions(+), 66 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index b7d1865..199c526 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -184,6 +184,7 @@ GEM rspec (~> 3) PLATFORMS + arm64-darwin-22 universal-java-11 x86_64-linux diff --git a/docs/configuration.md b/docs/configuration.md index e577025..560c328 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -246,6 +246,18 @@ This can be a string providing a single DN. For multiple DNs please specify the The LDAP object-type used to designate a user object. (optional) +### LDAP\_SERVICE_ACCOUNT\_HASH + +A hash containing the following parameters for a service account to perform the +initial bind. After the initial bind, then a search query is performed using the +'base' and 'user_object', then re-binds as the returned user. + +- :user_dn: The full distinguished name (DN) of the service account used to bind. + +- :password: The password for the service account used to bind. + +(optional) + ### SITE\_NAME The name of your deployment. diff --git a/lib/vmpooler/api/helpers.rb b/lib/vmpooler/api/helpers.rb index c56834a..4669b4c 100644 --- a/lib/vmpooler/api/helpers.rb +++ b/lib/vmpooler/api/helpers.rb @@ -68,7 +68,7 @@ module Vmpooler end end - def authenticate_ldap(port, host, encryption_hash, user_object, base, username_str, password_str) + def authenticate_ldap(port, host, encryption_hash, user_object, base, username_str, password_str, service_account_hash = nil) tracer.in_span( "Vmpooler::API::Helpers.#{__method__}", attributes: { @@ -79,6 +79,14 @@ module Vmpooler }, kind: :client ) do + if service_account_hash + username = service_account_hash[:user_dn] + password = service_account_hash[:password] + else + username = "#{user_object}=#{username_str},#{base}" + password = password_str + end + ldap = Net::LDAP.new( :host => host, :port => port, @@ -86,12 +94,22 @@ module Vmpooler :base => base, :auth => { :method => :simple, - :username => "#{user_object}=#{username_str},#{base}", - :password => password_str + :username => username, + :password => password } ) - return true if ldap.bind + if service_account_hash + return true if ldap.bind_as( + :base => base, + :filter => "(#{user_object}=#{username_str})", + :password => password_str + ) + elsif ldap.bind + return true + else + return false + end return false end @@ -116,6 +134,7 @@ module Vmpooler :method => :start_tls, :tls_options => { :ssl_version => 'TLSv1' } } + service_account_hash = auth[:ldap]['service_account_hash'] unless ldap_base.is_a? Array ldap_base = ldap_base.split @@ -134,7 +153,8 @@ module Vmpooler search_user_obj, search_base, username_str, - password_str + password_str, + service_account_hash ) return true if result end diff --git a/spec/unit/api/helpers_spec.rb b/spec/unit/api/helpers_spec.rb index a25ad61..27176e4 100644 --- a/spec/unit/api/helpers_spec.rb +++ b/spec/unit/api/helpers_spec.rb @@ -268,22 +268,62 @@ describe Vmpooler::API::Helpers do :tls_options => { :ssl_version => 'TLSv1' } } end - it 'should attempt ldap authentication' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base, username_str, password_str) - subject.authenticate(auth, username_str, password_str) + context 'without a service account' do + it 'should attempt ldap authentication' do + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base, username_str, password_str, nil) + + subject.authenticate(auth, username_str, password_str) + end + + it 'should return true when authentication is successful' do + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base, username_str, password_str, nil).and_return(true) + + expect(subject.authenticate(auth, username_str, password_str)).to be true + end + + it 'should return false when authentication fails' do + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base, username_str, password_str, nil).and_return(false) + + expect(subject.authenticate(auth, username_str, password_str)).to be false + end end - it 'should return true when authentication is successful' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base, username_str, password_str).and_return(true) + context 'with a service account' do + let(:service_account_hash) do + { + :user_dn => 'cn=Service Account,ou=users,dc=example,dc=com', + :password => 's3cr3t' + } + end + let(:auth) { + { + 'provider' => 'ldap', + ldap: { + 'host' => host, + 'base' => base, + 'user_object' => user_object, + 'service_account_hash' => service_account_hash + } + } + } + it 'should attempt ldap authentication' do + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base, username_str, password_str, service_account_hash) - expect(subject.authenticate(auth, username_str, password_str)).to be true - end + subject.authenticate(auth, username_str, password_str) + end - it 'should return false when authentication fails' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base, username_str, password_str).and_return(false) + it 'should return true when authentication is successful' do + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base, username_str, password_str, service_account_hash).and_return(true) - expect(subject.authenticate(auth, username_str, password_str)).to be false + expect(subject.authenticate(auth, username_str, password_str)).to be true + end + + it 'should return false when authentication fails' do + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base, username_str, password_str, service_account_hash).and_return(false) + + expect(subject.authenticate(auth, username_str, password_str)).to be false + end end context 'with an alternate ssl_version' do @@ -298,7 +338,7 @@ describe Vmpooler::API::Helpers do end it 'should specify the alternate ssl_version when authenticating' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, secure_encryption, user_object, base, username_str, password_str) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, secure_encryption, user_object, base, username_str, password_str, nil) subject.authenticate(auth, username_str, password_str) end @@ -311,7 +351,7 @@ describe Vmpooler::API::Helpers do end it 'should specify the alternate port when authenticating' do - expect(subject).to receive(:authenticate_ldap).with(alternate_port, host, default_encryption, user_object, base, username_str, password_str) + expect(subject).to receive(:authenticate_ldap).with(alternate_port, host, default_encryption, user_object, base, username_str, password_str, nil) subject.authenticate(auth, username_str, password_str) end @@ -331,7 +371,7 @@ describe Vmpooler::API::Helpers do end it 'should specify the secure port and encryption options when authenticating' do - expect(subject).to receive(:authenticate_ldap).with(secure_port, host, secure_encryption, user_object, base, username_str, password_str) + expect(subject).to receive(:authenticate_ldap).with(secure_port, host, secure_encryption, user_object, base, username_str, password_str, nil) subject.authenticate(auth, username_str, password_str) end @@ -349,36 +389,36 @@ describe Vmpooler::API::Helpers do end it 'should attempt to bind with each base' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[0], username_str, password_str) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[1], username_str, password_str) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[0], username_str, password_str, nil) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[1], username_str, password_str, nil) subject.authenticate(auth, username_str, password_str) end it 'should not search the second base when the first binds' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[0], username_str, password_str).and_return(true) - expect(subject).to_not receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[1], username_str, password_str) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[0], username_str, password_str, nil).and_return(true) + expect(subject).to_not receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[1], username_str, password_str, nil) subject.authenticate(auth, username_str, password_str) end it 'should search the second base when the first bind fails' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[0], username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[1], username_str, password_str) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[0], username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[1], username_str, password_str, nil) subject.authenticate(auth, username_str, password_str) end it 'should return true when any bind succeeds' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[0], username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[1], username_str, password_str).and_return(true) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[0], username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[1], username_str, password_str, nil).and_return(true) expect(subject.authenticate(auth, username_str, password_str)).to be true end it 'should return false when all bind attempts fail' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[0], username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[1], username_str, password_str).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[0], username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object, base[1], username_str, password_str, nil).and_return(false) expect(subject.authenticate(auth, username_str, password_str)).to be false end @@ -396,36 +436,36 @@ describe Vmpooler::API::Helpers do end it 'should attempt to bind with each user object' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base, username_str, password_str) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base, username_str, password_str) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base, username_str, password_str, nil) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base, username_str, password_str, nil) subject.authenticate(auth, username_str, password_str) end it 'should not search the second user object when the first binds' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base, username_str, password_str).and_return(true) - expect(subject).to_not receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base, username_str, password_str) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base, username_str, password_str, nil).and_return(true) + expect(subject).to_not receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base, username_str, password_str, nil) subject.authenticate(auth, username_str, password_str) end it 'should search the second user object when the first bind fails' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base, username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base, username_str, password_str) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base, username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base, username_str, password_str, nil) subject.authenticate(auth, username_str, password_str) end it 'should return true when any bind succeeds' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base, username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base, username_str, password_str).and_return(true) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base, username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base, username_str, password_str, nil).and_return(true) expect(subject.authenticate(auth, username_str, password_str)).to be true end it 'should return false when all bind attempts fail' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base, username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base, username_str, password_str).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base, username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base, username_str, password_str, nil).and_return(false) expect(subject.authenticate(auth, username_str, password_str)).to be false end @@ -450,64 +490,64 @@ describe Vmpooler::API::Helpers do end it 'should attempt to bind with each user object and base' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[0], username_str, password_str) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[0], username_str, password_str) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[1], username_str, password_str) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[1], username_str, password_str) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[0], username_str, password_str, nil) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[0], username_str, password_str, nil) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[1], username_str, password_str, nil) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[1], username_str, password_str, nil) subject.authenticate(auth, username_str, password_str) end it 'should not continue searching when the first combination binds' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[0], username_str, password_str).and_return(true) - expect(subject).to_not receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[0], username_str, password_str) - expect(subject).to_not receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[1], username_str, password_str) - expect(subject).to_not receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[1], username_str, password_str) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[0], username_str, password_str, nil).and_return(true) + expect(subject).to_not receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[0], username_str, password_str, nil) + expect(subject).to_not receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[1], username_str, password_str, nil) + expect(subject).to_not receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[1], username_str, password_str, nil) subject.authenticate(auth, username_str, password_str) end it 'should search the remaining combinations when the first bind fails' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[0], username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[0], username_str, password_str) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[1], username_str, password_str) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[1], username_str, password_str) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[0], username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[0], username_str, password_str, nil) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[1], username_str, password_str, nil) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[1], username_str, password_str, nil) subject.authenticate(auth, username_str, password_str) end it 'should search the remaining combinations when the first two binds fail' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[0], username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[0], username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[1], username_str, password_str) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[1], username_str, password_str) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[0], username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[0], username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[1], username_str, password_str, nil) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[1], username_str, password_str, nil) subject.authenticate(auth, username_str, password_str) end it 'should search the remaining combination when the first three binds fail' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[0], username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[0], username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[1], username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[1], username_str, password_str) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[0], username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[0], username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[1], username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[1], username_str, password_str, nil) subject.authenticate(auth, username_str, password_str) end it 'should return true when any bind succeeds' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[0], username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[0], username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[1], username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[1], username_str, password_str).and_return(true) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[0], username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[0], username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[1], username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[1], username_str, password_str, nil).and_return(true) expect(subject.authenticate(auth, username_str, password_str)).to be true end it 'should return false when all bind attempts fail' do - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[0], username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[0], username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[1], username_str, password_str).and_return(false) - expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[1], username_str, password_str).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[0], username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[0], username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[0], base[1], username_str, password_str, nil).and_return(false) + expect(subject).to receive(:authenticate_ldap).with(default_port, host, default_encryption, user_object[1], base[1], username_str, password_str, nil).and_return(false) expect(subject.authenticate(auth, username_str, password_str)).to be false end @@ -541,6 +581,12 @@ describe Vmpooler::API::Helpers do :tls_options => { :ssl_version => 'TLSv1' } } end + let(:service_account_hash) do + { + :user_dn => 'cn=Service Account,ou=users,dc=example,dc=com', + :password => 's3cr3t' + } + end let(:ldap) { double('ldap') } it 'should create a new ldap connection' do allow(ldap).to receive(:bind) @@ -572,6 +618,20 @@ describe Vmpooler::API::Helpers do expect(subject.authenticate_ldap(port, host, encryption, user_object, base, username_str, password_str)).to be false end + + it 'should return true when a bind_as is successful' do + expect(Net::LDAP).to receive(:new).and_return(ldap) + expect(ldap).to receive(:bind_as).and_return(true) + + expect(subject.authenticate_ldap(port, host, encryption, user_object, base, username_str, password_str, service_account_hash)).to be true + end + + it 'should return false when a bind_as fails' do + expect(Net::LDAP).to receive(:new).and_return(ldap) + expect(ldap).to receive(:bind_as).and_return(false) + + expect(subject.authenticate_ldap(port, host, encryption, user_object, base, username_str, password_str, service_account_hash)).to be false + end end end diff --git a/vmpooler.yaml.example b/vmpooler.yaml.example index efd0ba7..d1024c2 100644 --- a/vmpooler.yaml.example +++ b/vmpooler.yaml.example @@ -367,6 +367,15 @@ # - user_object # The LDAP object-type used to designate a user object. # +# - service_account_hash +# A hash containing the following parameters for a service account to perform the +# initial bind. After the initial bind, then a search query is performed using the +# 'base' and 'user_object', then re-binds as the returned user. +# - :user_dn +# The full distinguished name (DN) of the service account used to bind. +# - :password +# The password for the service account used to bind. +# # Example: # :auth: # provider: 'ldap' @@ -375,6 +384,23 @@ # port: 389 # base: 'ou=users,dc=company,dc=com' # user_object: 'uid' +# +# :auth: +# provider: 'ldap' +# :ldap: +# host: 'ldap.example.com' +# port: 636 +# service_account_hash: +# :user_dn: 'cn=Service Account,ou=Accounts,dc=ldap,dc=example,dc=com' +# :password: 'service-account-password' +# encryption: +# :method: :simple_tls +# :tls_options: +# :ssl_version: 'TLSv1_2' +# base: +# - 'ou=Accounts,dc=company,dc=com' +# user_object: +# - 'samAccountName' :auth: provider: 'ldap' From 17a1831952d70b95cb458e95e5515d9790a0a6b5 Mon Sep 17 00:00:00 2001 From: isaac-hammes Date: Tue, 22 Aug 2023 12:09:17 -0700 Subject: [PATCH 05/77] (POD-8) Add timeout_notification config to log warning before vm is destroyed. --- Gemfile.lock | 1 + .../vmpooler.yaml.dummy-example.aliasedpools | 6 ++ lib/vmpooler.rb | 1 + lib/vmpooler/pool_manager.rb | 73 ++++++++++++------- spec/unit/pool_manager_spec.rb | 51 ++++++------- vmpooler.yaml.dummy-example | 3 + vmpooler.yaml.example | 3 + 7 files changed, 86 insertions(+), 52 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 0d89963..4a94d9c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -186,6 +186,7 @@ GEM PLATFORMS arm64-darwin-22 universal-java-11 + x86_64-darwin-22 x86_64-linux DEPENDENCIES diff --git a/examples/vmpooler.yaml.dummy-example.aliasedpools b/examples/vmpooler.yaml.dummy-example.aliasedpools index ebece50..55bf9ff 100644 --- a/examples/vmpooler.yaml.dummy-example.aliasedpools +++ b/examples/vmpooler.yaml.dummy-example.aliasedpools @@ -17,6 +17,7 @@ logfile: '/Users/samuel/workspace/vmpooler/vmpooler.log' task_limit: 10 timeout: 15 + timeout_notification: 5 vm_checktime: 1 vm_lifetime: 12 vm_lifetime_auth: 24 @@ -38,6 +39,7 @@ datastore: 'vmstorage' size: 5 timeout: 15 + timeout_notification: 5 ready_ttl: 1440 provider: dummy dns_plugin: 'example' @@ -48,6 +50,7 @@ datastore: 'vmstorage' size: 5 timeout: 15 + timeout_notification: 5 ready_ttl: 1440 provider: dummy dns_plugin: 'example' @@ -58,6 +61,7 @@ datastore: 'vmstorage' size: 5 timeout: 15 + timeout_notification: 5 ready_ttl: 1440 provider: dummy dns_plugin: 'example' @@ -67,6 +71,7 @@ datastore: 'vmstorage' size: 5 timeout: 15 + timeout_notification: 5 ready_ttl: 1440 provider: dummy dns_plugin: 'example' @@ -77,6 +82,7 @@ datastore: 'other-vmstorage' size: 5 timeout: 15 + timeout_notification: 5 ready_ttl: 1440 provider: dummy dns_plugin: 'example' diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index 985a72b..2fcde30 100644 --- a/lib/vmpooler.rb +++ b/lib/vmpooler.rb @@ -82,6 +82,7 @@ module Vmpooler end parsed_config[:config]['clone_target'] = ENV['CLONE_TARGET'] if ENV['CLONE_TARGET'] parsed_config[:config]['timeout'] = string_to_int(ENV['TIMEOUT']) if ENV['TIMEOUT'] + parsed_config[:config]['timeout_notification'] = string_to_int(ENV['TIMEOUT_NOTIFICATION']) if ENV['TIMEOUT_NOTIFICATION'] parsed_config[:config]['vm_lifetime_auth'] = string_to_int(ENV['VM_LIFETIME_AUTH']) if ENV['VM_LIFETIME_AUTH'] parsed_config[:config]['max_tries'] = string_to_int(ENV['MAX_TRIES']) if ENV['MAX_TRIES'] parsed_config[:config]['retry_factor'] = string_to_int(ENV['RETRY_FACTOR']) if ENV['RETRY_FACTOR'] diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index d6735c1..d16db1a 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -82,21 +82,21 @@ module Vmpooler end # Check the state of a VM - def check_pending_vm(vm, pool, timeout, provider) + def check_pending_vm(vm, pool, timeout, timeout_notification, provider) Thread.new do begin - _check_pending_vm(vm, pool, timeout, provider) + _check_pending_vm(vm, pool, timeout, timeout_notification, provider) rescue StandardError => e $logger.log('s', "[!] [#{pool}] '#{vm}' #{timeout} #{provider} errored while checking a pending vm : #{e}") @redis.with_metrics do |redis| - fail_pending_vm(vm, pool, timeout, redis) + fail_pending_vm(vm, pool, timeout, timeout_notification, redis) end raise end end end - def _check_pending_vm(vm, pool, timeout, provider) + def _check_pending_vm(vm, pool, timeout, timeout_notification, provider) mutex = vm_mutex(vm) return if mutex.locked? @@ -106,7 +106,7 @@ module Vmpooler if provider.vm_ready?(pool, vm, redis) move_pending_vm_to_ready(vm, pool, redis, request_id) else - fail_pending_vm(vm, pool, timeout, redis) + fail_pending_vm(vm, pool, timeout, timeout_notification, redis) end end end @@ -122,35 +122,53 @@ module Vmpooler $logger.log('d', "[!] [#{pool}] '#{vm}' no longer exists. Removing from pending.") end - def fail_pending_vm(vm, pool, timeout, redis, exists: true) + def fail_pending_vm(vm, pool, timeout, timeout_notification, redis, exists: true) clone_stamp = redis.hget("vmpooler__vm__#{vm}", 'clone') - time_since_clone = (Time.now - Time.parse(clone_stamp)) / 60 - if time_since_clone > timeout - if exists - request_id = redis.hget("vmpooler__vm__#{vm}", 'request_id') - pool_alias = redis.hget("vmpooler__vm__#{vm}", 'pool_alias') if request_id - open_socket_error = redis.hget("vmpooler__vm__#{vm}", 'open_socket_error') - redis.smove("vmpooler__pending__#{pool}", "vmpooler__completed__#{pool}", vm) - if request_id - ondemandrequest_hash = redis.hgetall("vmpooler__odrequest__#{request_id}") - if ondemandrequest_hash && ondemandrequest_hash['status'] != 'failed' && ondemandrequest_hash['status'] != 'deleted' - # will retry a VM that did not come up as vm_ready? only if it has not been market failed or deleted - redis.zadd('vmpooler__odcreate__task', 1, "#{pool_alias}:#{pool}:1:#{request_id}") - end - end - $metrics.increment("errors.markedasfailed.#{pool}") - $logger.log('d', "[!] [#{pool}] '#{vm}' marked as 'failed' after #{timeout} minutes with error: #{open_socket_error}") - else + + already_timed_out = time_since_clone > timeout + timing_out_soon = time_since_clone > timeout_notification && !redis.hget("vmpooler__vm__#{vm}", 'timeout_notification') + + if already_timed_out + unless exists remove_nonexistent_vm(vm, pool, redis) + return true end + open_socket_error = handle_timed_out_vm(vm, pool, redis) end + + redis.hset("vmpooler__vm__#{vm}", 'timeout_notification', 1) if timing_out_soon + + nonexist_warning = if already_timed_out + "[!] [#{pool}] '#{vm}' marked as 'failed' after #{timeout} minutes with error: #{open_socket_error}" + elsif timing_out_soon + "[!] [#{pool}] '#{vm}' no longer exists when attempting to send notification of impending failure" + else + "[!] [#{pool}] '#{vm}' This error is wholly unexpected" + end + $logger.log('d', nonexist_warning) true rescue StandardError => e $logger.log('d', "Fail pending VM failed with an error: #{e}") false end + def handle_timed_out_vm(vm, pool, redis) + request_id = redis.hget("vmpooler__vm__#{vm}", 'request_id') + pool_alias = redis.hget("vmpooler__vm__#{vm}", 'pool_alias') if request_id + open_socket_error = redis.hget("vmpooler__vm__#{vm}", 'open_socket_error') + redis.smove("vmpooler__pending__#{pool}", "vmpooler__completed__#{pool}", vm) + if request_id + ondemandrequest_hash = redis.hgetall("vmpooler__odrequest__#{request_id}") + if ondemandrequest_hash && ondemandrequest_hash['status'] != 'failed' && ondemandrequest_hash['status'] != 'deleted' + # will retry a VM that did not come up as vm_ready? only if it has not been market failed or deleted + redis.zadd('vmpooler__odcreate__task', 1, "#{pool_alias}:#{pool}:1:#{request_id}") + end + end + $metrics.increment("errors.markedasfailed.#{pool}") + open_socket_error + end + def move_pending_vm_to_ready(vm, pool, redis, request_id = nil) clone_time = redis.hget("vmpooler__vm__#{vm}", 'clone') finish = format('%