From 80b93767123be16c4863465a51c9e2e87e32cdaf Mon Sep 17 00:00:00 2001 From: Kenneth Lakin Date: Mon, 24 Oct 2022 18:49:29 -0500 Subject: [PATCH 1/2] Writes basic NATs creds file if UAA is not up If for some reason NATs sync cannot authenticate to use the BOSH API, then the user authentication file for NATs will be empty. This causes problems when UAA is slow to start in some installations. Our approach to fix this issue was to make NATS Sync write the credentials for the Director and the Monitor even if the BOSH API is not available to be consumed. We also made sure that NATs Sync is not going to overwrite any credentials if they were already written. [#183572544] Investigate possible NATS 2.0 race condition Signed-off-by: Daniel Felipe Ochoa --- .../lib/nats_sync/users_sync.rb | 38 ++++++++--- .../spec/nats_sync/users_sync_spec.rb | 63 ++++++++++++++++++- 2 files changed, 91 insertions(+), 10 deletions(-) diff --git a/src/bosh-nats-sync/lib/nats_sync/users_sync.rb b/src/bosh-nats-sync/lib/nats_sync/users_sync.rb index 7da6c98e1ac..4ef9a9c9987 100644 --- a/src/bosh-nats-sync/lib/nats_sync/users_sync.rb +++ b/src/bosh-nats-sync/lib/nats_sync/users_sync.rb @@ -14,14 +14,28 @@ def initialize(nats_config_file_path, bosh_config, nats_server_executable, nats_ def execute_users_sync NATSSync.logger.info 'Executing NATS Users Synchronization' - vms_uuids = query_all_running_vms - current_file_hash = nats_file_hash - write_nats_config_file(vms_uuids, read_subject_file(@bosh_config['director_subject_file']), - read_subject_file(@bosh_config['hm_subject_file'])) - new_file_hash = nats_file_hash - UsersSync.reload_nats_server_config(@nats_server_executable, @nats_server_pid_file) unless current_file_hash == new_file_hash + vms_uuids = [] + overwriteable_config_file = true + begin + vms_uuids = query_all_running_vms + rescue RuntimeError => e + NATSSync.logger.error "Could not query all running vms: #{e.message}" + overwriteable_config_file = is_the_user_file_overwritable? + if overwriteable_config_file + NATSSync.logger.info "NATS config file is empty, writing basic users config file." + else + NATSSync.logger.info "NATS config file is not empty, doing nothing." + end + end + + if overwriteable_config_file + current_file_hash = nats_file_hash + write_nats_config_file(vms_uuids, read_subject_file(@bosh_config['director_subject_file']), + read_subject_file(@bosh_config['hm_subject_file'])) + new_file_hash = nats_file_hash + UsersSync.reload_nats_server_config(@nats_server_executable, @nats_server_pid_file) unless current_file_hash == new_file_hash + end NATSSync.logger.info 'Finishing NATS Users Synchronization' - vms_uuids end def self.reload_nats_server_config(nats_server_executable, nats_server_pid_file) @@ -33,6 +47,12 @@ def self.reload_nats_server_config(nats_server_executable, nats_server_pid_file) private + def is_the_user_file_overwritable? + JSON.parse(File.read(@nats_config_file_path)).empty? + rescue + true + end + def read_subject_file(file_path) return nil unless File.exist?(file_path) return nil if File.empty?(file_path) @@ -45,10 +65,12 @@ def nats_file_hash end def call_bosh_api(endpoint) + auth_header = create_authentication_header + NATSSync.logger.debug 'auth_header is empty, next REST call could fail' if auth_header.nil? || auth_header.empty? response = RestClient::Request.execute( url: @bosh_config['url'] + endpoint, method: :get, - headers: { 'Authorization' => create_authentication_header }, + headers: { 'Authorization' => auth_header }, verify_ssl: false, ) NATSSync.logger.debug(response.inspect) diff --git a/src/bosh-nats-sync/spec/nats_sync/users_sync_spec.rb b/src/bosh-nats-sync/spec/nats_sync/users_sync_spec.rb index 15e6024c2fc..aa916a5b862 100644 --- a/src/bosh-nats-sync/spec/nats_sync/users_sync_spec.rb +++ b/src/bosh-nats-sync/spec/nats_sync/users_sync_spec.rb @@ -6,7 +6,9 @@ module NATSSync describe UsersSync do before do allow(NATSSync).to receive(:logger).and_return(logger) - allow(logger).to receive :info + allow(logger).to receive(:debug) + allow(logger).to receive(:error) + allow(logger).to receive(:info) allow(Open3).to receive(:capture2e).and_return(["Success", capture_status]) end @@ -149,7 +151,64 @@ module NATSSync .to_return(status: 200, body: deployments_json) allow(auth_provider).to receive(:new).and_return(auth_provider_double) allow(auth_provider_double).to receive(:auth_header).and_return('Bearer xyz') - allow(logger).to receive(:debug) + File.open(nats_config_file_path, 'w') do |f| + f.write('{}') + end + end + + describe 'when UAA is not deployed and the BOSH API is not available' do + before do + stub_request(:get, url + '/deployments') + .with(headers: { 'Authorization' => 'Bearer xyz' }) + .to_return(status: 401, body: "Unauthorized") + end + + describe "and the authentication file is empty" do + it 'should write the basic bosh configuration' do + expect(JSON.parse(File.read(nats_config_file_path)).empty?).to be true + subject.execute_users_sync + file = File.read(nats_config_file_path) + data_hash = JSON.parse(file) + expect(data_hash['authorization']['users']) + .to include(include('user' => director_subject)) + expect(data_hash['authorization']['users']) + .to include(include('user' => hm_subject)) + expect(data_hash['authorization']['users'].length).to eq(2) + end + end + + describe "and the authentication file is corrupted" do + before do + File.open(nats_config_file_path, 'w') do |f| + f.write('{invalidchar') + end + end + it 'should write the basic bosh configuration' do + subject.execute_users_sync + file = File.read(nats_config_file_path) + data_hash = JSON.parse(file) + expect(data_hash['authorization']['users']) + .to include(include('user' => director_subject)) + expect(data_hash['authorization']['users']) + .to include(include('user' => hm_subject)) + expect(data_hash['authorization']['users'].length).to eq(2) + end + end + + describe "and the authentication file is not empty" do + before do + File.open(nats_config_file_path, 'w') do |f| + f.write('{"authorization": {"users": [{"user": "foo"}]}}') + end + end + it 'should not overwrite the authentication file' do + subject.execute_users_sync + file = File.read(nats_config_file_path) + data_hash = JSON.parse(file) + expect(data_hash).to eq({ 'authorization' => { 'users' => [{ 'user' => 'foo' }] } }) + end + end + end describe 'when there are no deployments with running vms in Bosh' do From 23e6dc860f4cfd8aaa8473fa03d419f23ea2c8bb Mon Sep 17 00:00:00 2001 From: Daniel Felipe Ochoa Date: Mon, 24 Oct 2022 18:52:57 -0500 Subject: [PATCH 2/2] Fix for rubocop issues Fixing the rubocop issues introduced in the previous commit and other commits. [#183572544] Investigate possible NATS 2.0 race condition Signed-off-by: Kenneth Lakin --- .../lib/nats_sync/users_sync.rb | 20 ++++++++++++------ .../spec/nats_sync/users_sync_spec.rb | 21 +++++++++---------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/src/bosh-nats-sync/lib/nats_sync/users_sync.rb b/src/bosh-nats-sync/lib/nats_sync/users_sync.rb index 4ef9a9c9987..df8f46004d2 100644 --- a/src/bosh-nats-sync/lib/nats_sync/users_sync.rb +++ b/src/bosh-nats-sync/lib/nats_sync/users_sync.rb @@ -20,11 +20,11 @@ def execute_users_sync vms_uuids = query_all_running_vms rescue RuntimeError => e NATSSync.logger.error "Could not query all running vms: #{e.message}" - overwriteable_config_file = is_the_user_file_overwritable? + overwriteable_config_file = user_file_overwritable? if overwriteable_config_file - NATSSync.logger.info "NATS config file is empty, writing basic users config file." + NATSSync.logger.info 'NATS config file is empty, writing basic users config file.' else - NATSSync.logger.info "NATS config file is not empty, doing nothing." + NATSSync.logger.info 'NATS config file is not empty, doing nothing.' end end @@ -33,23 +33,31 @@ def execute_users_sync write_nats_config_file(vms_uuids, read_subject_file(@bosh_config['director_subject_file']), read_subject_file(@bosh_config['hm_subject_file'])) new_file_hash = nats_file_hash - UsersSync.reload_nats_server_config(@nats_server_executable, @nats_server_pid_file) unless current_file_hash == new_file_hash + unless current_file_hash == new_file_hash + UsersSync.reload_nats_server_config(@nats_server_executable, + @nats_server_pid_file) + end end NATSSync.logger.info 'Finishing NATS Users Synchronization' end def self.reload_nats_server_config(nats_server_executable, nats_server_pid_file) output, status = Open3.capture2e("#{nats_server_executable} --signal reload=#{nats_server_pid_file}") + + # rubocop:disable Style/GuardClause + # rubocop:disable Layout/LineLength unless status.success? raise("Cannot execute: #{nats_server_executable} --signal reload=#{nats_server_pid_file}, Status Code: #{status} \nError: #{output}") end + # rubocop:enable Style/GuardClause + # rubocop:enable Layout/LineLength end private - def is_the_user_file_overwritable? + def user_file_overwritable? JSON.parse(File.read(@nats_config_file_path)).empty? - rescue + rescue RuntimeError, JSON::ParserError true end diff --git a/src/bosh-nats-sync/spec/nats_sync/users_sync_spec.rb b/src/bosh-nats-sync/spec/nats_sync/users_sync_spec.rb index aa916a5b862..ad87a6d5549 100644 --- a/src/bosh-nats-sync/spec/nats_sync/users_sync_spec.rb +++ b/src/bosh-nats-sync/spec/nats_sync/users_sync_spec.rb @@ -9,7 +9,7 @@ module NATSSync allow(logger).to receive(:debug) allow(logger).to receive(:error) allow(logger).to receive(:info) - allow(Open3).to receive(:capture2e).and_return(["Success", capture_status]) + allow(Open3).to receive(:capture2e).and_return(['Success', capture_status]) end subject { UsersSync.new(nats_config_file_path, bosh_config, nats_executable, nats_server_pid_file) } @@ -144,9 +144,9 @@ module NATSSync describe '#execute_nats_sync' do before do - stub_request(:get, url + '/info') + stub_request(:get, "#{url}/info") .to_return(status: 200, body: info_json) - stub_request(:get, url + '/deployments') + stub_request(:get, "#{url}/deployments") .with(headers: { 'Authorization' => 'Bearer xyz' }) .to_return(status: 200, body: deployments_json) allow(auth_provider).to receive(:new).and_return(auth_provider_double) @@ -158,12 +158,12 @@ module NATSSync describe 'when UAA is not deployed and the BOSH API is not available' do before do - stub_request(:get, url + '/deployments') + stub_request(:get, "#{url}/deployments") .with(headers: { 'Authorization' => 'Bearer xyz' }) - .to_return(status: 401, body: "Unauthorized") + .to_return(status: 401, body: 'Unauthorized') end - describe "and the authentication file is empty" do + describe 'and the authentication file is empty' do it 'should write the basic bosh configuration' do expect(JSON.parse(File.read(nats_config_file_path)).empty?).to be true subject.execute_users_sync @@ -177,7 +177,7 @@ module NATSSync end end - describe "and the authentication file is corrupted" do + describe 'and the authentication file is corrupted' do before do File.open(nats_config_file_path, 'w') do |f| f.write('{invalidchar') @@ -195,7 +195,7 @@ module NATSSync end end - describe "and the authentication file is not empty" do + describe 'and the authentication file is not empty' do before do File.open(nats_config_file_path, 'w') do |f| f.write('{"authorization": {"users": [{"user": "foo"}]}}') @@ -208,7 +208,6 @@ module NATSSync expect(data_hash).to eq({ 'authorization' => { 'users' => [{ 'user' => 'foo' }] } }) end end - end describe 'when there are no deployments with running vms in Bosh' do @@ -228,7 +227,7 @@ module NATSSync describe 'when there are deployments with running vms in Bosh' do before do - stub_request(:get, url + '/deployments/deployment-1/vms') + stub_request(:get, "#{url}/deployments/deployment-1/vms") .with(headers: { 'Authorization' => 'Bearer xyz' }) .to_return(status: 200, body: vms_json) end @@ -323,7 +322,7 @@ module NATSSync describe 'when reloading the NATs server fails' do let(:capture_status) { instance_double(Process::Status, success?: false) } before do - allow(Open3).to receive(:capture2e).and_return(["Failed to reload NATs server", capture_status]) + allow(Open3).to receive(:capture2e).and_return(['Failed to reload NATs server', capture_status]) end it 'should raise an error' do