rpms / rubygem-mysql2

Clone
Source Code
GIT

#14 Fix the broken SSL tests with MariaDB 10.5.18.
Merged a year ago by jaruga. Opened a year ago by jaruga.
rpms/ jaruga/rubygem-mysql2 wip/fix-ssl-tests  into  rawhide

file added
+29 
@@ -0,0 +1,29 @@ 
 

+ From 06512d47dc1491bf5686b2bd89a8555de9f2acc9 Mon Sep 17 00:00:00 2001
 

+ From: Jun Aruga <jaruga@redhat.com>
 

+ Date: Thu, 22 Dec 2022 16:14:39 +0100
 

+ Subject: [PATCH] Use the SSL pem files in the Git repository.
 

+ 

+ ---
 

+  spec/mysql2/client_spec.rb | 6 +++---
 

+  1 file changed, 3 insertions(+), 3 deletions(-)
 

+ 

+ diff --git a/spec/mysql2/client_spec.rb b/spec/mysql2/client_spec.rb
 

+ index 5861882..3f5cda8 100644
 

+ --- a/spec/mysql2/client_spec.rb
 

+ +++ b/spec/mysql2/client_spec.rb
 

+ @@ -153,9 +153,9 @@ RSpec.describe Mysql2::Client do # rubocop:disable Metrics/BlockLength
 

+      let(:option_overrides) do
 

+        {
 

+          'host'     => 'mysql2gem.example.com', # must match the certificates
 

+ -        :sslkey    => '/etc/mysql/client-key.pem',
 

+ -        :sslcert   => '/etc/mysql/client-cert.pem',
 

+ -        :sslca     => '/etc/mysql/ca-cert.pem',
 

+ +        :sslkey    => 'spec/ssl/client-key.pem',
 

+ +        :sslcert   => 'spec/ssl/client-cert.pem',
 

+ +        :sslca     => 'spec/ssl/ca-cert.pem',
 

+          :sslcipher => 'DHE-RSA-AES256-SHA',
 

+          :sslverify => true,
 

+        }
 

+ -- 

+ 2.38.1
 

+

file modified
+32 -1 
@@ -6,7 +6,7 @@ 
 

  

  Name: rubygem-%{gem_name}
 

  Version: 0.5.4
 

- Release: 2%{?dist}
 

+ Release: 3%{?dist}
 

  Summary: A simple, fast Mysql library for Ruby, binding to libmysql
 

  License: MIT
 

  URL: https://github.com/brianmario/mysql2
 
@@ -14,6 +14,9 @@ 
 

  # git clone --no-checkout https://github.com/brianmario/mysql2.git
 

  # cd mysql2 && git archive -v -o mysql2-0.5.3-tests.txz 0.5.3 spec/
 

  Source1: %{gem_name}-%{version}-tests.txz
 

+ # Use the SSL pem files in the upstream repositry for the SSL tests.
 

+ # https://github.com/brianmario/mysql2/pull/1293
 

+ Patch0: rubygem-mysql2-0.5.4-use-ssl-pem-files-in-repo.patch
 

  

  # Required in lib/mysql2.rb
 

  Requires: rubygem(bigdecimal)
 
@@ -30,6 +33,8 @@ 
 

  BuildRequires: rubygem(bigdecimal)
 

  # Used in spec/em/em_spec.rb
 

  BuildRequires: rubygem(eventmachine)
 

+ # Used in spec/ssl/gen_certs.sh
 

+ BuildRequires: %{_bindir}/openssl
 

  %endif
 

  

  %description
 
@@ -50,6 +55,10 @@ 
 

  %prep
 

  %setup -q -n %{gem_name}-%{version} -b 1
 

  

+ pushd %{_builddir}/spec
 

+ %patch0 -p2
 

+ popd
 

+ 

  %build
 

  gem build ../%{gem_name}-%{version}.gemspec
 

  %gem_install
 
@@ -75,6 +84,24 @@ 
 

  ln -s %{_builddir}/spec spec
 

  

  TOP_DIR=$(pwd)
 

+ 

+ # Regenerate the SSL certification files from the localhost, as we cannot set
 

+ # the host mysql2gem.example.com required for the SSL tests.
 

+ # https://github.com/brianmario/mysql2/pull/1296
 

+ sed -i '/host/ s/mysql2gem\.example\.com/localhost/' spec/mysql2/client_spec.rb
 

+ sed -i '/commonName_default/ s/mysql2gem\.example\.com/localhost/' spec/ssl/gen_certs.sh
 

+ pushd spec/ssl
 

+ bash gen_certs.sh
 

+ popd
 

+ 

+ # See https://github.com/brianmario/mysql2/blob/master/ci/ssl.sh
 

+ echo "
 

+ [mysqld]
 

+ ssl-ca=${TOP_DIR}/spec/ssl/ca-cert.pem
 

+ ssl-cert=${TOP_DIR}/spec/ssl/server-cert.pem
 

+ ssl-key=${TOP_DIR}/spec/ssl/server-key.pem
 

+ " > ~/.my.cnf
 

+ 

  # Use testing port because the standard mysqld port 3306 is occupied.
 

  # Assign a random port to consider a case of multi builds in parallel in a host.
 

  # https://src.fedoraproject.org/rpms/rubygem-pg/pull-request/3
 
@@ -168,6 +195,10 @@ 
 

  

  

  %changelog
 

+ * Fri Dec 16 2022 Jun Aruga <jaruga@redhat.com> - 0.5.4-3
 

+ - Fix the broken SSL tests with MariaDB 10.5.18.
 

+   Resolves: rhbz#2144488
 

+ 

  * Sat Jul 23 2022 Fedora Release Engineering <releng@fedoraproject.org> - 0.5.4-2
 

  - Rebuilt for https://fedoraproject.org/wiki/Fedora_37_Mass_Rebuild

This PR fixes the BZ ticket: https://bugzilla.redhat.com/show_bug.cgi?id=2144488.

Based on the mschorm's investigation and the https://src.fedoraproject.org/rpms/rubygem-mysql2/pull-request/13#. I was able to reproduce and fix the issue on the upstream tests.
https://github.com/brianmario/mysql2/pull/1290
Thanks for that!

The commit message

This commit fixes the broken SSL tests below.

The rubygem-mysql2 build started to fail with the error below by the mysql
that can't connect to the MariaDB server, when a dependency mariadb was upgraded
from the version 3:10.5.16-3.fc37 to 3:10.5.18-1.fc38.
https://koschei.fedoraproject.org/build/14086115

build.log

+ /usr/libexec/mysqld --datadir=/builddir/build/BUILD/mysql2-0.5.4/usr/share/gems/gems/mysql2-0.5.4/data --log-error=/builddir/build/BUILD/mysql2-0.5.4/usr/share/gems/gems/mysql2-0.5.4/mysql.log --socket=/builddir/build/BUILD/mysql2-0.5.4/usr/share/gems/gems/mysql2-0.5.4/mysql.sock --pid-file=/builddir/build/BUILD/mysql2-0.5.4/usr/share/gems/gems/mysql2-0.5.4/mysql.pid --port=13471 --ssl
++ seq 10
+ for i in $(seq 10)
+ sleep 1
2022-11-17 11:20:48 0 [Note] /usr/libexec/mysqld (mysqld 10.5.18-MariaDB) starting as process 1297 ...
+ grep -q 'ready for connections.' /builddir/build/BUILD/mysql2-0.5.4/usr/share/gems/gems/mysql2-0.5.4/mysql.log
+ echo 'Waiting connections... 1'
...
+ echo 'Waiting connections... 10'
Waiting connections... 10
+ mysql -u mockbuild -e 'ALTER USER '\''root'\''@'\''localhost'\'' IDENTIFIED VIA mysql_native_password USING PASSWORD('\'''\'')' -S /builddir/build/BUILD/mysql2-0.5.4/usr/share/gems/gems/mysql2-0.5.4/mysql.sock -P 13471
ERROR 2002 (HY000): Can't connect to local MySQL server through socket '/builddir/build/BUILD/mysql2-0.5.4/usr/share/gems/gems/mysql2-0.' (2)

After Michal Schorm mschorm@redhat.com's investigation (thanks!), we found
the error comes from the not appropriate SSL configurations.

It was reported that in Fedora, the MariaDB 10.5.16 used downstream OpenSSL 3
patch, and the MariaDB 10.5.18 started to use upstream OpenSSL 3 patch.
And there are some differences between these two patch files. And the difference
triggered this issue.

See also https://github.com/brianmario/mysql2/pull/1290.

Resolves: rhbz#2144488

Note

Note that the ci/ssl.sh is creating the /etc/my.cnf on the upstream PR to fix this issue.
https://github.com/brianmario/mysql2/pull/1290
https://github.com/brianmario/mysql2/blob/ba4d46551d132492b34205cdb8fa224c92765bef/ci/ssl.sh#L12

However, as we don't have the permission to edit the /etc/my.conf with the mockbuild user in the mock build, the ~/.my.cnf is created in this PR.

According to the current MariaDB setting, we can use the following configuration files.

<mock-chroot> sh-5.2$ my_print_defaults --help | grep my.cnf | xargs find 2>/dev/null
/etc/my.cnf

<mock-chroot> sh-5.2$ my_print_defaults --help | grep my.cnf
/etc/my.cnf ~/.my.cnf

I wrote the details in the commit message with the upstream PR link.

Tests

I confirmed the scratch build passing.
https://koji.fedoraproject.org/koji/taskinfo?taskID=95450973

rebased onto c7122c9
a year ago

rebased onto 6457a21
a year ago

Build succeeded.

I am not convinced you have fixed the subject of the PR (Fix the broken SSL tests). The tests are still broken, as seen by the "pending" status of SSL-related tests. However, the workaround allows the build to finish.

And they are "broken" (in quotes as it depends on the point of view) in the upstream.
The tests are skipped (see here) as the relevant /etc/mysql/* files (defined here) do not exist in our configuration.

MariaDB server correctly picks up certs from the configuration file ~/.my.cnf, which is probably why we can start the server. But the tests are too naive in detecting the configuration.

IOW: Pending status of tests 3-8 means SSL tests are still broken and not fixed.

Pending: (Failures listed here are expected and do not affect your suite's status)
  1) Mysql2::Client should set default program_name in connect_attrs
     # DON'T WORRY, THIS TEST PASSES - but PERFORMANCE SCHEMA is not enabled in your MySQL daemon.
     # ./spec/mysql2/client_spec.rb:510
  2) Mysql2::Client should set custom connect_attrs
     # DON'T WORRY, THIS TEST PASSES - but PERFORMANCE SCHEMA is not enabled in your MySQL daemon.
     # ./spec/mysql2/client_spec.rb:520
  3) Mysql2::Client SSL should set ssl_mode option disabled
     # DON'T WORRY, THIS TEST PASSES - but /etc/mysql/client-key.pem does not exist.
     # ./spec/mysql2/client_spec.rb:169
  4) Mysql2::Client SSL should set ssl_mode option preferred
     # DON'T WORRY, THIS TEST PASSES - but /etc/mysql/client-key.pem does not exist.
     # ./spec/mysql2/client_spec.rb:169
  5) Mysql2::Client SSL should set ssl_mode option required
     # DON'T WORRY, THIS TEST PASSES - but /etc/mysql/client-key.pem does not exist.
     # ./spec/mysql2/client_spec.rb:169
  6) Mysql2::Client SSL should set ssl_mode option verify_ca
     # DON'T WORRY, THIS TEST PASSES - but /etc/mysql/client-key.pem does not exist.
     # ./spec/mysql2/client_spec.rb:169
  7) Mysql2::Client SSL should set ssl_mode option verify_identity
     # DON'T WORRY, THIS TEST PASSES - but /etc/mysql/client-key.pem does not exist.
     # ./spec/mysql2/client_spec.rb:169
  8) Mysql2::Client SSL should be able to connect via SSL options
     # DON'T WORRY, THIS TEST PASSES - but /etc/mysql/client-key.pem does not exist.
     # ./spec/mysql2/client_spec.rb:182
Finished in 10.67 seconds (files took 0.23867 seconds to load)
340 examples, 0 failures, 8 pending

While we're on the topic of tests, I think we should replace skip with fail in RSpec where relevant. Upstream allows for tests to run on various platforms with different configurations, which is correct. However, we know our configuration and environment, and we should be able to run SSL tests successfully. Therefore, the keyword should be fail and not skip at least for SSL.

Additionally, if we manage to make the PERFORMANCE SCHEMA tests work, then according to the logic I described, those tests should then fail and not be skipped over.

Oh, good catch! Thanks for finding the issue. I will check it, and also check if the upstream CI tests for Fedora cases work.

While we're on the topic of tests, I think we should replace skip with fail in RSpec where relevant.

I agree on your point. I will check it, and I will contribute to the upstream if we need to modify the tests.
Edited a year ago by jaruga

Just note that here is the current status on the upstream CI Fedora rawhide case.

https://github.com/brianmario/mysql2/actions/runs/3717583638/jobs/6305085271

Pending: (Failures listed here are expected and do not affect your suite's status)

  1) Mysql2::Client should set custom connect_attrs
     # DON'T WORRY, THIS TEST PASSES - but PERFORMANCE SCHEMA is not enabled in your MySQL daemon.
     # ./spec/mysql2/client_spec.rb:522

  2) Mysql2::Client should set default program_name in connect_attrs
     # DON'T WORRY, THIS TEST PASSES - but PERFORMANCE SCHEMA is not enabled in your MySQL daemon.
     # ./spec/mysql2/client_spec.rb:512

Finished in 10.45 seconds (files took 0.2104

Additionally, if we manage to make the PERFORMANCE SCHEMA tests work, then according to the logic I described, those tests should then fail and not be skipped over.

Possibly, there is a way to enable PERFORMANCE SCHEMA by adding the configurations to .my.cnf.
https://github.com/brianmario/mysql2/issues/965#issuecomment-819874949

rebased onto 46c96e1
a year ago

I rebased adding one patch, and updating the commit message.

And here is a PR to reflect our Fedora case to the upstream.
https://github.com/brianmario/mysql2/pull/1293

Oh, good catch! Thanks for finding the issue. I will check it, and also check if the upstream CI tests for Fedora cases work.

I confirmed that the SSL tests are not skipped on the mysql2 upstream CI Fedora cases. Because the pem files are copied into /etc/mysql in the used ci/ssl.sh script.

I agree on your point. I will check it, and I will contribute to the upstream if we need to modify the tests.

This can be improved in another upstream PR. I plan to send it to give the option that we check the tests without skipping.

Possibly, there is a way to enable PERFORMANCE SCHEMA by adding the configurations to .my.cnf.
https://github.com/brianmario/mysql2/issues/965#issuecomment-819874949

This enhancement can be in another PR. I want to make this build green first in this PR.

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

I rebased adding one patch, and updating the commit message.

Thanks! Looking at the scratch build, there seem to be failures:

Failures:
  1) Mysql2::Client SSL should set ssl_mode option disabled
     Failure/Error:
       expect do
         new_client(options)
       end.not_to raise_error
       expected no Exception, got #<Mysql2::Error::ConnectionError: Unknown server host 'mysql2gem.example.com' (-3)> with backtrace:
         # ./lib/mysql2/client.rb:95:in `connect'
         # ./lib/mysql2/client.rb:95:in `initialize'
         # /builddir/build/BUILD/spec/spec_helper.rb:31:in `new'
         # /builddir/build/BUILD/spec/spec_helper.rb:31:in `new_client'
         # ./spec/mysql2/client_spec.rb:177:in `block (5 levels) in <top (required)>'
         # ./spec/mysql2/client_spec.rb:176:in `block (4 levels) in <top (required)>'
     # ./spec/mysql2/client_spec.rb:176:in `block (4 levels) in <top (required)>'
  2) Mysql2::Client SSL should set ssl_mode option preferred
     Failure/Error:
       expect do
         new_client(options)
       end.not_to raise_error
       expected no Exception, got #<Mysql2::Error::ConnectionError: Unknown server host 'mysql2gem.example.com' (-3)> with backtrace:
         # ./lib/mysql2/client.rb:95:in `connect'
         # ./lib/mysql2/client.rb:95:in `initialize'
         # /builddir/build/BUILD/spec/spec_helper.rb:31:in `new'
         # /builddir/build/BUILD/spec/spec_helper.rb:31:in `new_client'
         # ./spec/mysql2/client_spec.rb:177:in `block (5 levels) in <top (required)>'
         # ./spec/mysql2/client_spec.rb:176:in `block (4 levels) in <top (required)>'
     # ./spec/mysql2/client_spec.rb:176:in `block (4 levels) in <top (required)>'
  3) Mysql2::Client SSL should set ssl_mode option required
     Failure/Error:
       expect do
         new_client(options)
       end.not_to raise_error
       expected no Exception, got #<Mysql2::Error::ConnectionError: Unknown server host 'mysql2gem.example.com' (-3)> with backtrace:
         # ./lib/mysql2/client.rb:95:in `connect'
         # ./lib/mysql2/client.rb:95:in `initialize'
         # /builddir/build/BUILD/spec/spec_helper.rb:31:in `new'
         # /builddir/build/BUILD/spec/spec_helper.rb:31:in `new_client'
         # ./spec/mysql2/client_spec.rb:177:in `block (5 levels) in <top (required)>'
         # ./spec/mysql2/client_spec.rb:176:in `block (4 levels) in <top (required)>'
     # ./spec/mysql2/client_spec.rb:176:in `block (4 levels) in <top (required)>'
  4) Mysql2::Client SSL should set ssl_mode option verify_ca
     Failure/Error:
       expect do
         new_client(options)
       end.not_to raise_error
       expected no Exception, got #<Mysql2::Error::ConnectionError: Unknown server host 'mysql2gem.example.com' (-3)> with backtrace:
         # ./lib/mysql2/client.rb:95:in `connect'
         # ./lib/mysql2/client.rb:95:in `initialize'
         # /builddir/build/BUILD/spec/spec_helper.rb:31:in `new'
         # /builddir/build/BUILD/spec/spec_helper.rb:31:in `new_client'
         # ./spec/mysql2/client_spec.rb:177:in `block (5 levels) in <top (required)>'
         # ./spec/mysql2/client_spec.rb:176:in `block (4 levels) in <top (required)>'
     # ./spec/mysql2/client_spec.rb:176:in `block (4 levels) in <top (required)>'
  5) Mysql2::Client SSL should set ssl_mode option verify_identity
     Failure/Error:
       expect do
         new_client(options)
       end.not_to raise_error
       expected no Exception, got #<Mysql2::Error::ConnectionError: Unknown server host 'mysql2gem.example.com' (-3)> with backtrace:
         # ./lib/mysql2/client.rb:95:in `connect'
         # ./lib/mysql2/client.rb:95:in `initialize'
         # /builddir/build/BUILD/spec/spec_helper.rb:31:in `new'
         # /builddir/build/BUILD/spec/spec_helper.rb:31:in `new_client'
         # ./spec/mysql2/client_spec.rb:177:in `block (5 levels) in <top (required)>'
         # ./spec/mysql2/client_spec.rb:176:in `block (4 levels) in <top (required)>'
     # ./spec/mysql2/client_spec.rb:176:in `block (4 levels) in <top (required)>'
  6) Mysql2::Client SSL should be able to connect via SSL options
     Failure/Error: connect user, pass, host, port, database, socket, flags, conn_attrs
     Mysql2::Error::ConnectionError:
       Unknown server host 'mysql2gem.example.com' (-3)
     # ./lib/mysql2/client.rb:95:in `connect'
     # ./lib/mysql2/client.rb:95:in `initialize'
     # /builddir/build/BUILD/spec/spec_helper.rb:31:in `new'
     # /builddir/build/BUILD/spec/spec_helper.rb:31:in `new_client'
     # ./spec/mysql2/client_spec.rb:165:in `block (3 levels) in <top (required)>'
     # ./spec/mysql2/client_spec.rb:184:in `block (3 levels) in <top (required)>'
Finished in 13 minutes 37 seconds (files took 0.22865 seconds to load)
340 examples, 6 failures, 2 pending

The culprit seems to be that the tests try to connect to the host specified in the options: 'host' => 'mysql2gem.example.com': https://github.com/brianmario/mysql2/blob/master/spec/mysql2/client_spec.rb#L156

The explanation on the line: # must match the certificates makes a bit of sense. I wonder if we can get away using a different hostname. I'll experiment and see if I can get it to build in regular mock.

The container in the CI run has option --add-host to help resolve it
docker run --add-host=mysql2gem.example.com:127.0.0.1 -t --cap-add=SYS_PTRACE --security-opt seccomp=unconfined mysql2

This enhancement can be in another PR. I want to make this build green first in this PR.

I agree.

Also, I see this in the logs of SSL tests:

  SSL
/builddir/build/BUILD/mysql2-0.5.4/usr/share/gems/gems/mysql2-0.5.4/lib/mysql2/client.rb:56: warning: Your mysql client library does not support setting ssl_mode; full support comes with 5.7.11.
    should set ssl_mode option disabled (FAILED - 1)
/builddir/build/BUILD/mysql2-0.5.4/usr/share/gems/gems/mysql2-0.5.4/lib/mysql2/client.rb:56: warning: Your mysql client library does not support setting ssl_mode; full support comes with 5.7.11.
    should set ssl_mode option preferred (FAILED - 2)
/builddir/build/BUILD/mysql2-0.5.4/usr/share/gems/gems/mysql2-0.5.4/lib/mysql2/client.rb:56: warning: Your mysql client library does not support setting ssl_mode; full support comes with 5.7.11.
    should set ssl_mode option required (FAILED - 3)
/builddir/build/BUILD/mysql2-0.5.4/usr/share/gems/gems/mysql2-0.5.4/lib/mysql2/client.rb:56: warning: Your mysql client library does not support setting ssl_mode; full support comes with 5.7.11.
    should set ssl_mode option verify_ca (FAILED - 4)
/builddir/build/BUILD/mysql2-0.5.4/usr/share/gems/gems/mysql2-0.5.4/lib/mysql2/client.rb:56: warning: Your mysql client library does not support setting ssl_mode; full support comes with 5.7.11.
    should set ssl_mode option verify_identity (FAILED - 5)
    should be able to connect via SSL options (FAILED - 6)

I do not know what the MySQL client library refers to.

Hmm, I see another failure in the tests. Can we set up the host setting mysql2gem.example.com - 127.0.0.1? Or skip the tests.

https://kojipkgs.fedoraproject.org//work/tasks/4087/95614087/build.log

expected no Exception, got #<Mysql2::Error::ConnectionError: Unknown server host 'mysql2gem.example.com' (-3)> with backtrace:

https://github.com/brianmario/mysql2/blob/7f6f33a6e0bd652d5e7087edb6e5d00df248d35b/.github/workflows/build.yml#L66

https://github.com/brianmario/mysql2/blob/7f6f33a6e0bd652d5e7087edb6e5d00df248d35b/.github/workflows/container.yml#L30
Edited a year ago by jaruga

@jackorp I noticed your responses after I commented above. Thanks for review. I will check how to fix it.

I do not know what the MySQL client library refers to.

I think it is below.

$ grep -rn 'full support comes with 5.7.11.' ext/
ext/mysql2/client.c:125:    rb_warn( "Your mysql client library does not support setting ssl_mode; full support comes with 5.7.11." );
ext/mysql2/client.c:166:  rb_warn( "Your mysql client library does not support setting ssl_mode; full support comes with 5.7.11." );

https://github.com/brianmario/mysql2/blob/7f6f33a6e0bd652d5e7087edb6e5d00df248d35b/ext/mysql2/client.c#L125
or
https://github.com/brianmario/mysql2/blob/7f6f33a6e0bd652d5e7087edb6e5d00df248d35b/ext/mysql2/client.c#L166

Hmm, I see another failure in the tests. Can we set up the host setting mysql2gem.example.com - 127.0.0.1? Or skip the tests.

We probably could set the host somewhere, somehow.

Also, keep in mind that we do not have root, which certainly complicates matters as the guides I am aware of use root to edit system files or service restarts.

However, I was able to go with a bit of cert magic. The spec/ssl/gen_certs.sh script can be used to regenerate the scripts. All we need to do is change this line: https://github.com/brianmario/mysql2/blob/7f6f33a6e0bd652d5e7087edb6e5d00df248d35b/spec/ssl/gen_certs.sh#L33
and on that line basically run: sed -i "s|mysql2gem.example.com|localhost|" and edit the host option Hash in the SSL client test to be localhost. Then run the script, restart the server (if it was running) or just start it, then the tests work for me in local mock properly.

IMO this solution is better than upstream's as it only requires the loopback network interface, which is on most environments. This should perhaps become the default, it makes local testing easier and the tests setup one bit less difficult.

As the tests pass with this change for me, maybe it is worth bringing upstream. What do you think of this approach with regenerated certs?

If the upstream would be interested more in the case of using a proper hostname, I'd recommend splitting this out to local only and then the need to edit the /etc/hosts. The latter can be skipped if not supported by the runtime environment as some of the network-related tests are skipped in the Ruby test suite.

I think it is below.

Ok, I see that it counts with either MySQL versions or MariaDB, but we should not be getting an error. I am currently not sure what the code path is with all the ifdefs...

Trying to print out the client object in Ruby indicates to me that the option is being set.

For example, the :verify_ca test case sets the :ssl_mode key in the hash as expected.
See this snippet from local testing:

#<Mysql2::Client:0x00007fcc58fe72f8
 @query_options=
  {:as=>:hash,
   :async=>false,
   :cast_booleans=>false,
   :symbolize_keys=>false,
   :database_timezone=>:local,
   :application_timezone=>nil,
   :cache_rows=>true,
   :connect_flags=>2148573700,
   :cast=>true,
   :default_file=>nil,
   :default_group=>nil,
   :host=>"localhost",
   :username=>"root",
   :password=>nil,
   :database=>"test",
   :port=>14129,
   :socket=>"/builddir/build/BUILD/mysql2-0.5.4/usr/share/gems/gems/mysql2-0.5.4/mysql.sock",
   :ssl_mode=>:verify_ca,
   :sslkey=>"spec/ssl/client-key.pem",
   :sslcert=>"spec/ssl/client-cert.pem",
   :sslca=>"spec/ssl/ca-cert.pem",
   :sslcipher=>"DHE-RSA-AES256-SHA",
   :sslverify=>true},
 @read_timeout=nil>
    should set ssl_mode option verify_ca

Thanks for checking!

As the tests pass with this change for me, maybe it is worth bringing upstream. What do you think of this approach with regenerated certs?

That's good idea. I am positive on your way that we create the own pem files from the gen_certs.sh scripts.

IMO this solution is better than upstream's as it only requires the loopback network interface, which is on most environments. This should perhaps become the default, it makes local testing easier and the tests setup one bit less difficult.

In my understanding, using the looppack network interface means using the IP 127.0.0.1. And using a domain is another thing. I meant the "mysql2gem.example.com" was defined as 127.0.0.1.

$ ip a
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
    inet6 ::1/128 scope host 
       valid_lft forever preferred_lft forever
...

I think we need to check if the localhost is defined in the /etc/hosts on the build environment just in case. Then if it is defined, we can use the way.

If the upstream would be interested more in the case of using a proper hostname, I'd recommend splitting this out to local only and then the need to edit the /etc/hosts. The latter can be skipped if not supported by the runtime environment as some of the network-related tests are skipped in the Ruby test suite.

I think if the upstream would be interested in the proper hostname rather than localhost, maybe we can suggest managing the SSL cert pem files for both the proper hostname (mysql2gem.example.com) and localhost in the repository, and give an option to execute one of them.
Edited a year ago by jaruga

Ok, I see that it counts with either MySQL versions or MariaDB, but we should not be getting an error. I am currently not sure what the code path is with all the ifdefs...

I investigated the issue now. It seems the condition to print the warning is a bug. The condition is checking the client library version. It seems it considers the case of the MySQL and the case of the MariaDB bundled client library case (as far as I know, MariaDB-bundled the client library printed the version of the MariaDB as a client version). However I don't think that the client library mariadb-connector-c's case is considered.

I reported the issue with an improvement of the warning message now.
https://github.com/brianmario/mysql2/pull/1294

That's good idea. I am positive on your way that we create the own pem files from the gen_certs.sh scripts.

What I was hinting at is that I'd rather keep this in the upstream and ideally would not go anywhere near touching these files in specfile unless absolutely necessary. It brings /usr/bin/openssl and its dependencies as BuildRequires as well as bringing even more complexity to our %check section. Or it brings more files to manage with the package if we want to pregenerate them once and then provide them in the package using the SourceN: ...

Having it on our side in one way or the other, for the time being, might be necessary if we go this way before appropriate patches are available in a future mysql2 gem, but I would not like it to be a permanent solution.

In my understanding, using the looppack network interface means using the IP 127.0.0.1. And using a domain is another thing. I meant the "mysql2gem.example.com" was defined as 127.0.0.1.

Right, domain and loopback can be different. I mean to say that for our case, both localhost and mysql2gem.example.com are going through the loopback (127.0.0.1). docker with run --add-host=foobar:127.0.0.1 adds a line to /etc/hosts that will resolve foobar to 127.0.0.1. And localhost does work in a similar way (an entry in /etc/hosts).

From the upstream tests' point of view, there is no difference between localhost and mysql2gem.example.com, this is underlined by the need to have the latter in /etc/hosts.

I think we need to check if the localhost is defined in the /etc/hosts just in case. Then if it is defined, we can use the way.

That's a good thing to check for, but I do not think catching localhost from /etc/hosts is a good way. Suppose a default container image or a mock build, for that matter, does not have localhost-related entries in /etc/hosts, or localhost does not resolve to anything. In that case, I'd consider it a broken environment (or significantly more limited).

Rather than checking a text file, perhaps checking the IP that Ruby resolves is better. Something like the following ruby code has to pass:

require 'socket'

Addrinfo.foreach(nil, 80).map(&:ip_address).any? Addrinfo.ip("localhost").ip_address

If that returns false, that means that localhost resolves to nothing or something that is not loopback. In that case, the environment cannot use localhost at all, or that localhost does not point to 127.0.0.1, and we cannot use localhost reliably.

An interesting bit, for some reason, it seems we do not necessarily need the localhost entry to appear in the /etc/hosts to have it resolve localhost correctly:

$ podman run -it --rm registry.fedoraproject.org/fedora:37 bash
[root@ff229b258a77 project]# cat /etc/hosts # Empty hosts file
[root@ff229b258a77 project]# jekyll serve -B -H 0.0.0.0
Resolving dependencies...
Configuration file: /opt/project/_config.yml
            Source: /opt/project
       Destination: /opt/project/_site
 Incremental build: disabled. Enable with --incremental
      Generating...
       Jekyll Feed: Generating feed for posts
                    done in 0.228 seconds.
 Auto-regeneration: disabled when running server detached.
    Server address: http://0.0.0.0:4000/
Server detached with pid '91'. Run `pkill -f jekyll' or `kill -9 91' to stop the server.
[root@ff229b258a77 project]# curl localhost:4000
<!DOCTYPE html>
<html lang="en"><head>
  <meta charset="utf-8">
  <meta http-equiv="X-UA-Compatible" content="IE=edge">
<...snip...>

Here I used Jekyll as a webserver that listens on localhost on a container with empty /etc/hosts. When I curl localhost:4000, I received a response from Jekyll.

I think if the upstream would be interested in the proper hostname rather than localhost, maybe we can suggest managing the SSL cert pem files for both the proper hostname (mysql2gem.example.com) and localhost in the repository, and give an option to execute one of them.

Yes, that sounds good. Having written out this response, I do not feel there is added value in differentiating localhost and proper hostname if both live in /etc/hosts. If I understand correctly why the tests are set up that way, it is because certificates require a hostname. I found out that using localhost is a valid value for that certificate field and SSL tests seem to pass. I'd like to validate this assumption on the wider range upstream tests for. Some ubuntu from the test matrix and CentOS 7 should be enough for a proof of concept.

I investigated the issue now. It seems the condition to print the warning is a bug.

Great, thanks!

rebased onto c33b1cf
a year ago

Build succeeded.

I sent a PR to run the SSL tests with the the SSL certification files generated from the host localhost. I rebased the PR with updating the spec file and commit message again.
https://github.com/brianmario/mysql2/pull/1296

Scratch build: ok (finally)
https://koji.fedoraproject.org/koji/taskinfo?taskID=95625801

The result is only 2 pendings related to the performance schema.

Pending: (Failures listed here are expected and do not affect your suite's status)
  1) Mysql2::Client should set default program_name in connect_attrs
     # DON'T WORRY, THIS TEST PASSES - but PERFORMANCE SCHEMA is not enabled in your MySQL daemon.
     # ./spec/mysql2/client_spec.rb:510
  2) Mysql2::Client should set custom connect_attrs
     # DON'T WORRY, THIS TEST PASSES - but PERFORMANCE SCHEMA is not enabled in your MySQL daemon.
     # ./spec/mysql2/client_spec.rb:520
Finished in 10.69 seconds (files took 0.25351 seconds to load)
340 examples, 0 failures, 2 pending

Let me comment below.

What I was hinting at is that I'd rather keep this in the upstream and ideally would not go anywhere near touching these files in specfile unless absolutely necessary. It brings /usr/bin/openssl and its dependencies as BuildRequires as well as bringing even more complexity to our %check section. Or it brings more files to manage with the package if we want to pregenerate them once and then provide them in the package using the SourceN: ...

Having it on our side in one way or the other, for the time being, might be necessary if we go this way before appropriate patches are available in a future mysql2 gem, but I would not like it to be a permanent solution.

I agree on your points. Having openssl as BuildRequires is the downside of the first option. And However, managing some files for tar.gz file is also hard in the second option. So, I chose the first option. This is a temporary workaround. If the upstream PR above is merged, we don't need to generate the files in the spec file.

From the upstream tests' point of view, there is no difference between localhost and mysql2gem.example.com, this is underlined by the need to have the latter in /etc/hosts.

Yes, that sounds good. Having written out this response, I do not feel there is added value in differentiating localhost and proper hostname if both live in /etc/hosts. If I understand correctly why the tests are set up that way, it is because certificates require a hostname. I found out that using localhost is a valid value for that certificate field and SSL tests seem to pass. I'd like to validate this assumption on the wider range upstream tests for. Some ubuntu from the test matrix and CentOS 7 should be enough for a proof of concept.

First, I tried to apply the certification files generated from the host localhost to the every cases in the upstream CI. However I saw a failure of the sudo service mysql restart and a failure of the specific SSL test in only Ubuntu cases. So, am applying the certification files from the localhost to the Fedora rawhide case on the upstream CI on the PR above. Baby step.

https://github.com/junaruga/mysql2/commit/b56345f5fdb5e51336b9e3f664a506ac70c4c9ac
https://github.com/junaruga/mysql2/actions/runs/3766171111/jobs/6402394799
https://github.com/junaruga/mysql2/actions/runs/3766171111/jobs/6402393909

That's a good thing to check for, but I do not think catching localhost from /etc/hosts is a good way.

I intended to check the availability of the localhost manually. It seems that the localhost is available in the building environment.

An interesting bit, for some reason, it seems we do not necessarily need the localhost entry to appear in the /etc/hosts to have it resolve localhost correctly:
...
Here I used Jekyll as a webserver that listens on localhost on a container with empty /etc/hosts. When I curl localhost:4000, I received a response from Jekyll.

OK. Thanks for checking. I suspect the host environment's host setting affects the container's hosts setting.

Ci passed. :tada:

As both the upstream and our CI passed with the expected results, you are free to merge from my point of view. We can take a look at the rest of the action items later async from this change.

Pull-Request has been merged by jaruga
a year ago

Thanks for the review! Yes, we can take a look at the rest of the action items later.
I merged this PR, and built now. The koji is here.
https://koji.fedoraproject.org/koji/buildinfo?buildID=2105858

First, I tried to apply the certification files generated from the host localhost to the every cases in the upstream CI. However I saw a failure of the sudo service mysql restart and a failure of the specific SSL test in only Ubuntu cases. So, am applying the certification files from the localhost to the Fedora rawhide case on the upstream CI on the PR above. Baby step.

https://github.com/junaruga/mysql2/commit/b56345f5fdb5e51336b9e3f664a506ac70c4c9ac
https://github.com/junaruga/mysql2/actions/runs/3766171111/jobs/6402394799
https://github.com/junaruga/mysql2/actions/runs/3766171111/jobs/6402393909

Thx for elaborating here and sharing the test runs. From the long term perspective, I think that the biggest issue is this: "sudo service mysql restart". IOW I think the way we execute the DB with regular user is superior, precisely because we don't need root privileges and fiddle with system services and what not. While upstream is open to accept our patches (nice), the amount of branches to handle the upstream and our DB setup makes the code quite noncomprehensive.

Thx for elaborating here and sharing the test runs. From the long term perspective, I think that the biggest issue is this: "sudo service mysql restart". IOW I think the way we execute the DB with regular user is superior, precisely because we don't need root privileges and fiddle with system services and what not. While upstream is open to accept our patches (nice), the amount of branches to handle the upstream and our DB setup makes the code quite noncomprehensive.

Good point. I agree. Right now the MySQL or MariaDB servers are executed by root (sudo) in all the cases of the upstream CI, and systemd (service) is used in the upstream Ubuntu cases. I plan to contribute to make MySQL or MariaDB run by regular user in all the cases of the upstream CI to align the way with the rubygem-mysql2.spec.
Metadata
None
No Tags
Changes Summary 2
+29
file added
rubygem-mysql2-0.5.4-use-ssl-pem-files-in-repo.patch
+32 -1
file changed
rubygem-mysql2.spec
Powered by Pagure 5.13.3
Documentation • File an Issue • About this Instance • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.