From 6435e8cffa4832ebd3ae6f89c62429899a413174 Mon Sep 17 00:00:00 2001 From: Rob Bavey Date: Fri, 25 Oct 2019 15:26:37 -0400 Subject: [PATCH 1/2] 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 (https://github.com/elastic/logstash/issues/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. --- ci/unit/Dockerfile | 1 + ci/unit/docker-compose.yml | 7 ++++ ci/unit/run.sh | 2 +- ci/unit/setup.sql | 12 ++++++ lib/logstash/plugin_mixins/jdbc/jdbc.rb | 36 ++++++---------- logstash-input-jdbc.gemspec | 1 - spec/inputs/integ_spec.rb | 56 +++++++++++++++++++++---- spec/inputs/jdbc_spec.rb | 20 --------- 8 files changed, 83 insertions(+), 52 deletions(-) create mode 100644 ci/unit/setup.sql diff --git a/ci/unit/Dockerfile b/ci/unit/Dockerfile index 387a4e8..2434383 100644 --- a/ci/unit/Dockerfile +++ b/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' diff --git a/ci/unit/docker-compose.yml b/ci/unit/docker-compose.yml index edc5f6a..ffedde2 100644 --- a/ci/unit/docker-compose.yml +++ b/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 diff --git a/ci/unit/run.sh b/ci/unit/run.sh index f0347b6..92f0b01 100755 --- a/ci/unit/run.sh +++ b/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 diff --git a/ci/unit/setup.sql b/ci/unit/setup.sql new file mode 100644 index 0000000..9f2eb32 --- /dev/null +++ b/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'); diff --git a/lib/logstash/plugin_mixins/jdbc/jdbc.rb b/lib/logstash/plugin_mixins/jdbc/jdbc.rb index 877d31a..b5fbaf0 100644 --- a/lib/logstash/plugin_mixins/jdbc/jdbc.rb +++ b/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 diff --git a/logstash-input-jdbc.gemspec b/logstash-input-jdbc.gemspec index 4f294b3..6dbbcf3 100755 --- a/logstash-input-jdbc.gemspec +++ b/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 diff --git a/spec/inputs/integ_spec.rb b/spec/inputs/integ_spec.rb index 9497a8d..f99c633 100644 --- a/spec/inputs/integ_spec.rb +++ b/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 diff --git a/spec/inputs/jdbc_spec.rb b/spec/inputs/jdbc_spec.rb index 702a4be..02597e0 100755 --- a/spec/inputs/jdbc_spec.rb +++ b/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" } From f0d5945ba7b8f0c7a9b58e905216a461ae40b7e3 Mon Sep 17 00:00:00 2001 From: Rob Bavey Date: Fri, 25 Oct 2019 18:53:45 -0400 Subject: [PATCH 2/2] Version Bump and changelog entry --- CHANGELOG.md | 3 +++ logstash-input-jdbc.gemspec | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8916d30..58a5f29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +## 4.3.18 + - Fix issue with driver loading [#356](https://github.com/logstash-plugins/logstash-input-jdbc/pull/356) + ## 4.3.17 - Added documentation to provide more info about jdbc driver loading [#352](https://github.com/logstash-plugins/logstash-input-jdbc/pull/352) diff --git a/logstash-input-jdbc.gemspec b/logstash-input-jdbc.gemspec index 6dbbcf3..0b3745f 100755 --- a/logstash-input-jdbc.gemspec +++ b/logstash-input-jdbc.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |s| s.name = 'logstash-input-jdbc' - s.version = '4.3.17' + s.version = '4.3.18' s.licenses = ['Apache License (2.0)'] s.summary = "Creates events from JDBC data" s.description = "This gem is a Logstash plugin required to be installed on top of the Logstash core pipeline using $LS_HOME/bin/logstash-plugin install gemname. This gem is not a stand-alone program"