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

Failure in Oracle/SQL Server modules due to useSSL query param #568

Closed
rnorth opened this issue Jan 30, 2018 · 7 comments
Closed

Failure in Oracle/SQL Server modules due to useSSL query param #568

rnorth opened this issue Jan 30, 2018 · 7 comments

Comments

@rnorth
Copy link
Member

rnorth commented Jan 30, 2018

After upgrading the Oracle and SQL Server modules to use 1.6.0 as their base, unfortunately we had some build failures. It seems that #561 has unfortunately led to JDBC URLs being created that Oracle and SQL Server consider invalid.

In order to progress the 1.6.0 release I'm going with an ugly but necessary workaround; overriding the createConnection method for these classes so that appendDisableSslConfig is not used.

We can come up with a cleaner workaround, but for now this works and unblocks the 1.6.0 release for these modules.

This underscores the value of having a single repository with wider tests to guard against regression, as described in #564!

rnorth added a commit to testcontainers/testcontainers-java-module-mssqlserver that referenced this issue Jan 30, 2018
rnorth added a commit to testcontainers/testcontainers-java-module-oracle-xe that referenced this issue Jan 30, 2018
rnorth added a commit to testcontainers/testcontainers-java-module-oracle-xe that referenced this issue Jan 30, 2018
* Bump POMs and Changelogs for 1.6.0 release

* Disable `useSSL` query param for Oracle/SQL Server modules
Temporary fix for testcontainers/testcontainers-java#568
rnorth added a commit to testcontainers/testcontainers-java-module-mssqlserver that referenced this issue Jan 30, 2018
* Bump POMs and Changelogs for 1.6.0 release

* Disable `useSSL` query param for Oracle/SQL Server modules
Temporary fix for testcontainers/testcontainers-java#568
@npetzall
Copy link

Is it too late to revert #561.
As this might mean that any custom built DatabaseContainers subclassing JdbcContainer now faces a breaking change.

@iNikem
Copy link
Contributor

iNikem commented Jan 31, 2018

I can fix this in a week or so. Unless somebody wants to do it faster, @rnorth , can you assign this to me?

@kiview
Copy link
Member

kiview commented Jan 31, 2018

I think GitHub only allows assigning to Members, but I think it would be nice if you fix this 🙂

@rnorth
Copy link
Member Author

rnorth commented Jan 31, 2018

Thanks @iNikem. And yes, @npetzall, you're right, this will likely be a problem for people subclassing JdbcContainer. I should have some time this evening, so will do a quick 1.6.1 release with a fix.

@npetzall
Copy link

Also note that argument separator in connectionString may differ some don't even use ?.

@iNikem
Copy link
Contributor

iNikem commented Jan 31, 2018

As the original problem was specific to MySQL I think I will rework my fix to be MySQL container specific.

rnorth added a commit that referenced this issue Jan 31, 2018
Add template method allowing JDBCContainer subclasses to have a specific
form of JDBC URL used for establishing Connections.

Fixes #568
@rnorth
Copy link
Member Author

rnorth commented Jan 31, 2018

I've made a quick PR to resolve this without a wholesale reversion - #569. Please could you take a look and let me know if it makes sense?

@rnorth rnorth closed this as completed in a6d8bf2 Feb 3, 2018
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

No branches or pull requests

4 participants