From 22619fc78c1d1b6208c96ca8d7b66d58f4049dc1 Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Fri, 19 Feb 2021 15:38:31 +0100 Subject: [PATCH 1/5] clean up database after new download --- x-pack/lib/filters/geoip/database_manager.rb | 1 + x-pack/lib/filters/geoip/download_manager.rb | 2 +- x-pack/spec/filters/geoip/database_manager_spec.rb | 9 +++++++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/x-pack/lib/filters/geoip/database_manager.rb b/x-pack/lib/filters/geoip/database_manager.rb index a35bed22388..fb7c7056b0d 100644 --- a/x-pack/lib/filters/geoip/database_manager.rb +++ b/x-pack/lib/filters/geoip/database_manager.rb @@ -78,6 +78,7 @@ def call(job, time) 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) diff --git a/x-pack/lib/filters/geoip/download_manager.rb b/x-pack/lib/filters/geoip/download_manager.rb index d7c7506c798..3952950a1c2 100644 --- a/x-pack/lib/filters/geoip/download_manager.rb +++ b/x-pack/lib/filters/geoip/download_manager.rb @@ -82,8 +82,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..27cb0511ca9 100644 --- a/x-pack/spec/filters/geoip/database_manager_spec.rb +++ b/x-pack/spec/filters/geoip/database_manager_spec.rb @@ -11,10 +11,12 @@ 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") } @@ -112,6 +114,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 From b870fb0c551d35abc76b3ab52a56294805eaeb8c Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Sat, 20 Feb 2021 22:55:21 +0100 Subject: [PATCH 2/5] add log --- x-pack/lib/filters/geoip/database_manager.rb | 16 ++++++---------- x-pack/lib/filters/geoip/download_manager.rb | 12 +++++++----- .../spec/filters/geoip/database_manager_spec.rb | 5 +++++ 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/x-pack/lib/filters/geoip/database_manager.rb b/x-pack/lib/filters/geoip/database_manager.rb index fb7c7056b0d..c9c151a8ade 100644 --- a/x-pack/lib/filters/geoip/database_manager.rb +++ b/x-pack/lib/filters/geoip/database_manager.rb @@ -73,7 +73,7 @@ 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 @@ -97,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 @@ -121,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 3952950a1c2..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 diff --git a/x-pack/spec/filters/geoip/database_manager_spec.rb b/x-pack/spec/filters/geoip/database_manager_spec.rb index 27cb0511ca9..86ab9aae766 100644 --- a/x-pack/spec/filters/geoip/database_manager_spec.rb +++ b/x-pack/spec/filters/geoip/database_manager_spec.rb @@ -22,6 +22,11 @@ module LogStash module Filters module Geoip 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) From 6a92a73c5fe0bb7505e79f671073c3b92b5d983e Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Sat, 20 Feb 2021 23:03:45 +0100 Subject: [PATCH 3/5] cache metadata --- x-pack/lib/filters/geoip/database_metadata.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/lib/filters/geoip/database_metadata.rb b/x-pack/lib/filters/geoip/database_metadata.rb index c71c296517c..7928833324f 100644 --- a/x-pack/lib/filters/geoip/database_metadata.rb +++ b/x-pack/lib/filters/geoip/database_metadata.rb @@ -28,11 +28,13 @@ def save_timestamp(database_path) metadata.each { |row| csv << row } end + @cache = metadata logger.debug("metadata updated", :metadata => metadata) end def get_all - file_exist?(@metadata_path)? ::CSV.read(@metadata_path, headers: false) : Array.new + @cache ||= file_exist?(@metadata_path)? + ::CSV.read(@metadata_path, headers: false) : Array.new end # Give rows of metadata in default database type, or empty array From 298d58fa1c0b83cc6809b11a9d2d29fcdc57ce45 Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Mon, 1 Mar 2021 14:18:58 +0100 Subject: [PATCH 4/5] unblock test case relying on latest geoip filter gem --- .../filters/geoip/download_manager_spec.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) 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 From 43ea2ce03642a41202f3d337044fff081f1b5365 Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Mon, 1 Mar 2021 14:25:10 +0100 Subject: [PATCH 5/5] Revert "cache metadata" This reverts commit c1d9332044fca464193df7e8ebe099e77f1ec2af. --- x-pack/lib/filters/geoip/database_metadata.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/x-pack/lib/filters/geoip/database_metadata.rb b/x-pack/lib/filters/geoip/database_metadata.rb index 7928833324f..c71c296517c 100644 --- a/x-pack/lib/filters/geoip/database_metadata.rb +++ b/x-pack/lib/filters/geoip/database_metadata.rb @@ -28,13 +28,11 @@ def save_timestamp(database_path) metadata.each { |row| csv << row } end - @cache = metadata logger.debug("metadata updated", :metadata => metadata) end def get_all - @cache ||= file_exist?(@metadata_path)? - ::CSV.read(@metadata_path, headers: false) : Array.new + file_exist?(@metadata_path)? ::CSV.read(@metadata_path, headers: false) : Array.new end # Give rows of metadata in default database type, or empty array