Fix driver class loading

The changes to class loading intended to fix usage of :jdbc_driver_library with
Java 11, introduced in `v4.3.14`, appear to have some issues
(elastic/logstash#11268, #355). This commit attempts
to resolve those issues, and reworks the integration tests to work with
docker and ensure that the :jdbc_driver_library configuration option works.

This has been tested with postgresql using Java 8 and Java 11.

Author: robbavey

ci/unit/Dockerfile

@@ -3,6 +3,7 @@ FROM docker.elastic.co/logstash/logstash:$ELASTIC_STACK_VERSION
 COPY --chown=logstash:logstash Gemfile /usr/share/plugins/plugin/Gemfile
 COPY --chown=logstash:logstash *.gemspec /usr/share/plugins/plugin/
 RUN cp /usr/share/logstash/logstash-core/versions-gem-copy.yml /usr/share/logstash/versions.yml
+RUN curl -o /usr/share/logstash/postgresql.jar https://jdbc.postgresql.org/download/postgresql-42.2.8.jar
 ENV PATH="${PATH}:/usr/share/logstash/vendor/jruby/bin"
 ENV LOGSTASH_SOURCE="1"
 RUN gem install bundler -v '< 2'

ci/unit/docker-compose.yml

@@ -15,3 +15,10 @@ services:
       LS_JAVA_OPTS: "-Xmx256m -Xms256m"
       LOGSTASH_SOURCE: 1
     tty: true
+
+  postgresql:
+    image: postgres:latest
+    volumes:
+    - ./setup.sql:/docker-entrypoint-initdb.d/init.sql
+    ports:
+      - 5432:5432

ci/unit/run.sh

@@ -5,4 +5,4 @@ set -ex
 
 export USER='logstash'
 
-bundle exec rspec -fd 2>/dev/null
+bundle exec rspec spec && bundle exec rspec spec/inputs/integ_spec.rb --tag integration -fd 2>/dev/null

ci/unit/setup.sql

@@ -0,0 +1,12 @@
+create DATABASE jdbc_input_db;
+
+\c jdbc_input_db;
+
+CREATE TABLE employee (
+   emp_no integer NOT NULL,
+   first_name VARCHAR (50) NOT NULL,
+   last_name VARCHAR (50) NOT NULL
+);
+
+INSERT INTO employee VALUES (1, 'David', 'Blenkinsop');
+INSERT INTO employee VALUES (2, 'Mark', 'Guckenheimer');

lib/logstash/plugin_mixins/jdbc/jdbc.rb

@@ -140,28 +140,18 @@ def jdbc_connect
 
     private
 
-    def load_drivers
-      return if @jdbc_driver_library.nil? || @jdbc_driver_library.empty?
-
-      driver_jars = @jdbc_driver_library.split(",")
-
-      # Needed for JDK 11 as the DriverManager has a different ClassLoader than Logstash
-      urls = java.net.URL[driver_jars.length].new
-
-      driver_jars.each_with_index do |driver, idx|
-        urls[idx] = java.io.File.new(driver).toURI().toURL()
-      end
-      ucl = java.net.URLClassLoader.new_instance(urls)
-      begin
-        klass = java.lang.Class.forName(@jdbc_driver_class.to_java(:string), true, ucl);
-      rescue Java::JavaLang::ClassNotFoundException => e
-        raise LogStash::Error, "Unable to find driver class via URLClassLoader in given driver jars: #{@jdbc_driver_class}"
-      end
-      begin
-        driver = klass.getConstructor().newInstance();
-        java.sql.DriverManager.register_driver(WrappedDriver.new(driver.to_java(java.sql.Driver)).to_java(java.sql.Driver))
-      rescue Java::JavaSql::SQLException => e
-        raise LogStash::Error, "Unable to register driver with java.sql.DriverManager using WrappedDriver: #{@jdbc_driver_class}"
+    def load_driver_jars
+      unless @jdbc_driver_library.nil? || @jdbc_driver_library.empty?
+        @jdbc_driver_library.split(",").each do |driver_jar|
+          begin
+            @logger.debug("loading #{driver_jar}")
+            # Use https://github.com/jruby/jruby/wiki/CallingJavaFromJRuby#from-jar-files to make classes from jar
+            # available
+            require driver_jar
+          rescue LoadError => e
+            raise LogStash::PluginLoadingError, "unable to load #{driver_jar} from :jdbc_driver_library, #{e.message}"
+          end
+        end
       end
     end
 
@@ -174,7 +164,7 @@ def open_jdbc_connection
       Sequel.application_timezone = @plugin_timezone.to_sym
       if @drivers_loaded.false?
         begin
-          load_drivers
+          load_driver_jars
           Sequel::JDBC.load_driver(@jdbc_driver_class)
         rescue LogStash::Error => e
           # raised in load_drivers, e.cause should be the caught Java exceptions

logstash-input-jdbc.gemspec

@@ -28,5 +28,4 @@ Gem::Specification.new do |s|
   s.add_development_dependency 'logstash-devutils'
   s.add_development_dependency 'timecop'
   s.add_development_dependency 'jdbc-derby'
-  s.add_development_dependency 'jdbc-postgres'
 end

spec/inputs/integ_spec.rb

@@ -1,5 +1,7 @@
 require "logstash/devutils/rspec/spec_helper"
 require "logstash/inputs/jdbc"
+require "sequel"
+require "sequel/adapters/jdbc"
 
 # This test requires: Firebird installed to Mac OSX, it uses the built-in example database `employee`
 
@@ -8,15 +10,23 @@
   # is picked up.  It could be arbitrarily set to any timezone, but then the test
   # would have to compensate differently.  That's why UTC is chosen.
   ENV["TZ"] = "Etc/UTC"
-  let(:mixin_settings) do
-    { "jdbc_user" => "SYSDBA", "jdbc_driver_class" => "org.firebirdsql.jdbc.FBDriver", "jdbc_driver_library" => "/elastic/tmp/jaybird-full-3.0.4.jar",
-    "jdbc_connection_string" => "jdbc:firebirdsql://localhost:3050//Library/Frameworks/Firebird.framework/Versions/A/Resources/examples/empbuild/employee.fdb", "jdbc_password" => "masterkey"}
+  # For Travis and CI based on docker, we source from ENV
+  jdbc_connection_string = ENV.fetch("PG_CONNECTION_STRING",
+                                     "jdbc:postgresql://postgresql:5432") + "/jdbc_input_db?user=postgres"
+
+  let(:settings) do
+    { "jdbc_driver_class" => "org.postgresql.Driver",
+      "jdbc_connection_string" => jdbc_connection_string,
+      "jdbc_driver_library" => "/usr/share/logstash/postgresql.jar",
+      "jdbc_user" => "postgres",
+      "statement" => 'SELECT FIRST_NAME, LAST_NAME FROM "employee" WHERE EMP_NO = 2'
+    }
   end
-  let(:settings) { {"statement" => "SELECT FIRST_NAME, LAST_NAME FROM EMPLOYEE WHERE EMP_NO > 144"} }
-  let(:plugin) { LogStash::Inputs::Jdbc.new(mixin_settings.merge(settings)) }
+
+  let(:plugin) { LogStash::Inputs::Jdbc.new(settings) }
   let(:queue) { Queue.new }
 
-  context "when passing no parameters" do
+  context "when connecting to a postgres instance" do
     before do
       plugin.register
     end
@@ -25,12 +35,44 @@
       plugin.stop
     end
 
-    it "should retrieve params correctly from Event" do
+    it "should populate the event with database entries" do
       plugin.run(queue)
       event = queue.pop
       expect(event.get('first_name')).to eq("Mark")
       expect(event.get('last_name')).to eq("Guckenheimer")
     end
   end
+
+  context "when supplying a non-existent library" do
+    let(:settings) do
+      super.merge(
+          "jdbc_driver_library" => "/no/path/to/postgresql.jar"
+      )
+    end
+
+    it "should not register correctly" do
+      plugin.register
+      q = Queue.new
+      expect do
+        plugin.run(q)
+      end.to raise_error(::LogStash::PluginLoadingError)
+    end
+  end
+
+  context "when connecting to a non-existent server" do
+    let(:settings) do
+      super.merge(
+          "jdbc_connection_string" => "jdbc:postgresql://localhost:65000/somedb"
+      )
+    end
+
+    it "should not register correctly" do
+      plugin.register
+      q = Queue.new
+      expect do
+        plugin.run(q)
+      end.to raise_error(::Sequel::DatabaseConnectionError)
+    end
+  end
 end
 

spec/inputs/jdbc_spec.rb

@@ -2,8 +2,6 @@
 require "logstash/devutils/rspec/spec_helper"
 require "logstash/inputs/jdbc"
 require "jdbc/derby"
-require 'jdbc/postgres'
-Jdbc::Postgres.load_driver
 require "sequel"
 require "sequel/adapters/jdbc"
 require "timecop"
@@ -78,24 +76,6 @@
     end
   end
 
-  context "when connecting to a non-existent server", :no_connection => true do
-    let(:mixin_settings) do
-      super.merge(
-        "jdbc_driver_class" => "org.postgresql.Driver",
-        "jdbc_connection_string" => "jdbc:postgresql://localhost:65000/somedb"
-      )
-    end
-    let(:settings) { super.merge("statement" => "SELECT 1 as col1 FROM test_table", "jdbc_user" => "foo", "jdbc_password" => "bar") }
-
-    it "should not register correctly" do
-        plugin.register
-        q = Queue.new
-        expect do
-          plugin.run(q)
-        end.to raise_error(::Sequel::DatabaseConnectionError)
-    end
-  end
-
   context "when both jdbc_password and jdbc_password_filepath arguments are passed" do
     let(:statement) { "SELECT * from test_table" }
     let(:jdbc_password) { "secret" }