From f12f3e38fc4c1bd8bba707bc2f1b3cad086922cd Mon Sep 17 00:00:00 2001 From: kaisecheng <69120390+kaisecheng@users.noreply.github.com> Date: Tue, 2 Mar 2021 14:30:44 +0100 Subject: [PATCH] GeoIP clean up database after new download (#12689) This commit adds a clean-up step to remove .gz and .mmdb which is not in the metadata after pointing to the new GeoIP database Fixed: #12560 --- x-pack/lib/filters/geoip/database_manager.rb | 17 +++++++---------- x-pack/lib/filters/geoip/download_manager.rb | 14 ++++++++------ .../filters/geoip/database_manager_spec.rb | 14 ++++++++++++++ .../filters/geoip/download_manager_spec.rb | 18 +++++++++--------- 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/x-pack/lib/filters/geoip/database_manager.rb b/x-pack/lib/filters/geoip/database_manager.rb index a35bed22388..c9c151a8ade 100644 --- a/x-pack/lib/filters/geoip/database_manager.rb +++ b/x-pack/lib/filters/geoip/database_manager.rb @@ -73,11 +73,12 @@ def execute_download_job # scheduler callback def call(job, time) - logger.debug "scheduler is running" + logger.debug "scheduler runs database update check" begin if execute_download_job @geoip.setup_filter(database_path) + clean_up_database end rescue DatabaseExpiryError => e logger.error(e.message, :cause => e.cause, :backtrace => e.backtrace) @@ -96,15 +97,9 @@ def database_path protected # return a valid database path or default database path def patch_database_path(database_path) - unless file_exist?(database_path) - database_path = get_file_path("GeoLite2-#{@database_type}.mmdb") - - unless file_exist?(database_path) - raise "You must specify 'database => ...' in your geoip filter (I looked for '#{database_path}')" - end - end - - database_path + return database_path if file_exist?(database_path) + return database_path if database_path = get_file_path("GeoLite2-#{@database_type}.mmdb") and file_exist?(database_path) + raise "You must specify 'database => ...' in your geoip filter (I looked for '#{database_path}')" end def check_age @@ -120,6 +115,8 @@ def check_age logger.warn("The MaxMind database has been used for #{days_without_update} days without update. "\ "Logstash will stop the GeoIP plugin in #{30 - days_without_update} days. "\ "Please check the network settings and allow Logstash accesses the internet to download the latest database ") + else + logger.debug("The MaxMind database hasn't updated", :days_without_update => days_without_update) end end diff --git a/x-pack/lib/filters/geoip/download_manager.rb b/x-pack/lib/filters/geoip/download_manager.rb index d7c7506c798..fe237d2da39 100644 --- a/x-pack/lib/filters/geoip/download_manager.rb +++ b/x-pack/lib/filters/geoip/download_manager.rb @@ -46,12 +46,14 @@ def fetch_database def check_update uuid = get_uuid res = rest_client.get("#{GEOIP_ENDPOINT}?key=#{uuid}&elastic_geoip_service_tos=agree") - logger.info "#{GEOIP_ENDPOINT} return #{res.status}" + logger.debug("check update", :endpoint => GEOIP_ENDPOINT, :response => res.status) - all_db = JSON.parse(res.body) - target_db = all_db.select { |info| info['name'].include?(@database_type) }.first + dbs = JSON.parse(res.body) + target_db = dbs.select { |db| db['name'].include?(@database_type) }.first + has_update = @metadata.gz_md5 != target_db['md5_hash'] + logger.info "new database version detected? #{has_update}" - [@metadata.gz_md5 != target_db['md5_hash'], target_db] + [has_update, target_db] end def download_database(server_db) @@ -60,7 +62,7 @@ def download_database(server_db) Down.download(server_db['url'], destination: new_database_zip_path) raise "the new download has wrong checksum" if md5(new_database_zip_path) != server_db['md5_hash'] - logger.debug("new database downloaded in #{new_database_zip_path}") + logger.debug("new database downloaded in ", :path => new_database_zip_path) new_database_zip_path end end @@ -82,8 +84,8 @@ def assert_database!(database_path) def rest_client @client ||= Faraday.new do |conn| - conn.adapter :net_http conn.use Faraday::Response::RaiseError + conn.adapter :net_http end end diff --git a/x-pack/spec/filters/geoip/database_manager_spec.rb b/x-pack/spec/filters/geoip/database_manager_spec.rb index dd5a38d9d36..86ab9aae766 100644 --- a/x-pack/spec/filters/geoip/database_manager_spec.rb +++ b/x-pack/spec/filters/geoip/database_manager_spec.rb @@ -11,15 +11,22 @@ module LogStash module Filters module Geoip let(:mock_geoip_plugin) { double("geoip_plugin") } let(:mock_metadata) { double("database_metadata") } let(:mock_download_manager) { double("download_manager") } + let(:mock_scheduler) { double("scheduler") } let(:db_manager) do manager = DatabaseManager.new(mock_geoip_plugin, DEFAULT_CITY_DB_PATH, "City", get_vendor_path) manager.instance_variable_set(:@metadata, mock_metadata) manager.instance_variable_set(:@download_manager, mock_download_manager) + manager.instance_variable_set(:@scheduler, mock_scheduler) manager end let(:logger) { double("Logger") } context "patch database" do + it "use input path" do + path = db_manager.send(:patch_database_path, DEFAULT_ASN_DB_PATH) + expect(path).to eq(DEFAULT_ASN_DB_PATH) + end + it "use CC license database as default" do path = db_manager.send(:patch_database_path, "") expect(path).to eq(DEFAULT_CITY_DB_PATH) @@ -112,6 +119,13 @@ module LogStash module Filters module Geoip allow(mock_geoip_plugin).to receive(:setup_filter).never db_manager.send(:call, nil, nil) end + + it "should call scheduler when has update" do + allow(db_manager).to receive(:execute_download_job).and_return(true) + allow(mock_geoip_plugin).to receive(:setup_filter).once + allow(db_manager).to receive(:clean_up_database).once + db_manager.send(:call, nil, nil) + end end context "clean up database" do diff --git a/x-pack/spec/filters/geoip/download_manager_spec.rb b/x-pack/spec/filters/geoip/download_manager_spec.rb index 6bdf1b55d63..f57b057f207 100644 --- a/x-pack/spec/filters/geoip/download_manager_spec.rb +++ b/x-pack/spec/filters/geoip/download_manager_spec.rb @@ -105,15 +105,15 @@ module LogStash module Filters module Geoip end end - # context "assert database" do - # it "should raise error if file is invalid" do - # expect{ download_manager.send(:assert_database!, "Gemfile") }.to raise_error /failed to load database/ - # end - # - # it "should pass validation" do - # expect(download_manager.send(:assert_database!, DEFAULT_CITY_DB_PATH)).to be_nil - # end - # end + context "assert database" do + it "should raise error if file is invalid" do + expect{ download_manager.send(:assert_database!, "Gemfile") }.to raise_error /failed to load database/ + end + + it "should pass validation" do + expect(download_manager.send(:assert_database!, DEFAULT_CITY_DB_PATH)).to be_nil + end + end context "fetch database" do it "should be false if no update" do