Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add client SSL configuration #214

Merged
merged 3 commits into from Nov 25, 2020
Merged

Add client SSL configuration #214

merged 3 commits into from Nov 25, 2020

Conversation

rockwotj
Copy link
Contributor

@rockwotj rockwotj commented Nov 20, 2020

This is a minimal change to introduce client SSL configuration to the postgres driver.

This additionally upgrades the testcontainers dependency because it wouldn't start up otherwise on MacOS.

Fixes #213

@oshai
Copy link
Contributor

oshai commented Nov 22, 2020

Some mysql tests are failing. I suspect it is related to the testcontainers upgrade but need to further check that.

@oshai
Copy link
Contributor

oshai commented Nov 22, 2020

Unfortunately I can't run tests locally with the old version, probably due to this issue: testcontainers/testcontainers-java#1623
I will try to check it on travis.

@rockwotj
Copy link
Contributor Author

Unfortunately I can't run tests locally with the old version, probably due to this issue: testcontainers/testcontainers-java#1623
I will try to check it on travis.

That was the issue I had to upgrade for. I can also debug tomorrow morning

@oshai
Copy link
Contributor

oshai commented Nov 23, 2020

I tried without test containers upgrade and it seems to be working: https://travis-ci.org/github/jasync-sql/jasync-sql/builds/745282931

@rockwotj
Copy link
Contributor Author

rockwotj commented Nov 23, 2020

@oshai I got it working. It was an annoying behavior change from this PR: testcontainers/testcontainers-java#857

The fix was to override configure and change it back to "test".

Also the "create the docker image during class initialization" is a not fun pattern to debug (especially when they swallow errors)... Would you be willing to accept some more PRs to clean that up and instead use JUnit's builtin ExternalResource?

@oshai
Copy link
Contributor

oshai commented Nov 23, 2020

@rockwotj thanks for all the effort!
Another PR with external resource is welcome.
Before we merge this, Please note that some mysql tests are still failing in CI.

@rockwotj
Copy link
Contributor Author

@rockwotj thanks for all the effort!
Another PR with external resource is welcome.
Before we merge this, Please note that some mysql tests are still failing in CI.

Oops, I was just running the connection spec, now they should pass now I've fixed the default config values.

@codecov
Copy link

codecov bot commented Nov 24, 2020

Codecov Report

Merging #214 (07c09cd) into master (3ead882) will increase coverage by 80.22%.
The diff coverage is 77.77%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master     #214       +/-   ##
=============================================
+ Coverage          0   80.22%   +80.22%     
- Complexity        0     1032     +1032     
=============================================
  Files             0      259      +259     
  Lines             0     3792     +3792     
  Branches          0      474      +474     
=============================================
+ Hits              0     3042     +3042     
- Misses            0      531      +531     
- Partials          0      219      +219     
Impacted Files Coverage Δ Complexity Δ
.../java/com/github/jasync/sql/db/SSLConfiguration.kt 88.88% <75.00%> (ø) 8.00 <5.00> (?)
...db/postgresql/codec/PostgreSQLConnectionHandler.kt 84.15% <100.00%> (ø) 34.00 <0.00> (?)
...async/sql/db/mysql/binary/encoder/StringEncoder.kt 100.00% <0.00%> (ø) 3.00% <0.00%> (?%)
...a/com/github/jasync/sql/db/column/ColumnEncoder.kt 100.00% <0.00%> (ø) 0.00% <0.00%> (?%)
...l/db/exceptions/ConnectionNotConnectedException.kt 100.00% <0.00%> (ø) 1.00% <0.00%> (?%)
...l/message/server/ParamProcessingFinishedMessage.kt 100.00% <0.00%> (ø) 1.00% <0.00%> (?%)
...va/com/github/jasync/sql/db/util/BufferDumper.java 95.55% <0.00%> (ø) 13.00% <0.00%> (?%)
...gresql/encoders/PreparedStatementOpeningEncoder.kt 95.65% <0.00%> (ø) 2.00% <0.00%> (?%)
...ql/db/exceptions/CanceledChannelFutureException.kt 0.00% <0.00%> (ø) 0.00% <0.00%> (?%)
...om/github/jasync/sql/db/mysql/decoder/OkDecoder.kt 100.00% <0.00%> (ø) 2.00% <0.00%> (?%)
... and 251 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ead882...07c09cd. Read the comment docs.

@oshai oshai merged commit 9f821e2 into jasync-sql:master Nov 25, 2020
@oshai
Copy link
Contributor

oshai commented Nov 25, 2020

Thanks @rockwotj !

oshai pushed a commit that referenced this pull request Nov 26, 2020
* Add client SSL configuration

* Fix mysql tests after testcontainers upgrade

* Fix up the default config for mysql tests
oshai pushed a commit that referenced this pull request Nov 29, 2020
* Add client SSL configuration

* Fix mysql tests after testcontainers upgrade

* Fix up the default config for mysql tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support client SSL (mutual auth)
2 participants