GeoIP clean up database after new download

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
 

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
 

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

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