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

JDBC driver should not be shade/relocate Bouncycastle #608

Closed
rafaelbey opened this issue Oct 13, 2021 · 8 comments
Closed

JDBC driver should not be shade/relocate Bouncycastle #608

rafaelbey opened this issue Oct 13, 2021 · 8 comments
Assignees
Labels
Backlog feature status-triage_done Initial triage done, will be further handled by the driver team

Comments

@rafaelbey
Copy link

Bouncycastle libraries are signed as required by JCA.

Given the snowflake relocates the library, we are seeing failures on our applications if snowflake JDBC driver is loaded first (adding the BC provider).

While we have a workaround, this library should not be shaded/relocated or the jdbc driver should be properly signed.

@jdimeo
Copy link

jdimeo commented Oct 14, 2021

More generally, can we make an unshaded .jar available for these use cases where we don't need a shaded jar? Something like:

<dependency>
    <groupId>net.snowflake</groupId>
    <artifactId>snowflake-jdbc</artifactId>
    <version>3.13.8</version>
    <classifier>unshaded</classifier>
</dependency>

@jdimeo
Copy link

jdimeo commented Oct 14, 2021

This is more or less duplicate with #174

@rafaelbey
Copy link
Author

That could do the trick.

Note that an unshaded jar could bring other challenges to large applications, with other libraries already competing for transitive library versions, like for Jackson among others. Hence, the unshaded would need to be kept lean as one of the proposals on #174 or could lead to other set of issues been raised.

@jdimeo
Copy link

jdimeo commented Oct 14, 2021

Of course, but for those of us who are doing it the "Maven way" we are used to overriding transitive versions, etc. to resolve those kinds of conflicts. For AWS Lambdas with file size limits, it's important to allow users to have only one (for example) Jackson version if their use case allows it.

@sfc-gh-kterada sfc-gh-kterada added the status-triage_needed This is a new issue, and initial triage is needed label Oct 21, 2021
@joshuamzm
Copy link

It's unfortunate the Snowflake team hasn't resolved on this or #174 issues. However, I'm still having problems using the JDBC with other libraries depending on Bouncycastle.

@rafaelbey Can you elaborate more on your workaround to make your projects work along with the Snowflake JDBC driver?

In my case, I'm seeing this error, maybe because of the problem you described originally in this issue.

Exception in thread "main" java.lang.NoClassDefFoundError: net/snowflake/client/jdbc/internal/org/bouncycastle/crypto/CipherParameters (wrong name: org/bouncycastle/crypto/CipherParameters)

@wesleyhillyext
Copy link

I found this thread while investigating #1193.

Snowflake offers a FIPS-compliant version of their driver, which does not bundle Bouncy Castle in its JAR at all, see https://docs.snowflake.com/en/release-notes/2020-04.html#snowflake-jdbc-driver-fips-compliant-driver-released

I believe switching to that version of the driver will resolve the issue @rafaelbey had.

@sfc-gh-anugupta sfc-gh-anugupta added Backlog and removed status-triage_needed This is a new issue, and initial triage is needed labels Apr 18, 2023
sfc-gh-wshangguan pushed a commit that referenced this issue May 26, 2023
- fix pom.xml url
- add sortpom plugin and sort pom.xml
- remove internal repository reference
- fix scm section
- add a pluginManagement section
- add version properties for plugins
- update plugin versions
- add dependency convergence and upper bounds checks
- fill out dependencyManagement section to converge all dependencies at the upper bound
- add unused declared and used undeclared dependency checks
- remove unused declared dependencies
- add used undeclared dependencies
- fix the <scope> declaration of several dependencies
- remove all dependency <exclusions> sections
- remove maven-install-plugin
- add japicmp plugin to enforce semantic versioning
- update maven required version to 3.6.3
- add linkage checker enforcer rule
- add linkage exclusions covering the current set of linkage issues
- add duplicate class enforcer check from extra-enforcer-rules
- adjust the java-9 profile to activate for jdks 9 or greater
- add JPMS line for tests with jdk>8 "--add-opens=java.base/java.nio=ALL-UNNAMED"
- update fmt-maven-plugin
- add maven wrapper, pin maven to 3.8 for compatibility with linkage checker
- disable shading by default, fixes #174 #608
- dependency fixes possibly fix #1211
- update json-smart to 2.4.9, related #1311
- httpclient to 4.5.14, #1273
sfc-gh-wshangguan added a commit that referenced this issue May 30, 2023
* Java library best practices

- fix pom.xml url
- add sortpom plugin and sort pom.xml
- remove internal repository reference
- fix scm section
- add a pluginManagement section
- add version properties for plugins
- update plugin versions
- add dependency convergence and upper bounds checks
- fill out dependencyManagement section to converge all dependencies at the upper bound
- add unused declared and used undeclared dependency checks
- remove unused declared dependencies
- add used undeclared dependencies
- fix the <scope> declaration of several dependencies
- remove all dependency <exclusions> sections
- remove maven-install-plugin
- add japicmp plugin to enforce semantic versioning
- update maven required version to 3.6.3
- add linkage checker enforcer rule
- add linkage exclusions covering the current set of linkage issues
- add duplicate class enforcer check from extra-enforcer-rules
- adjust the java-9 profile to activate for jdks 9 or greater
- add JPMS line for tests with jdk>8 "--add-opens=java.base/java.nio=ALL-UNNAMED"
- update fmt-maven-plugin
- add maven wrapper, pin maven to 3.8 for compatibility with linkage checker
- disable shading by default, fixes #174 #608
- dependency fixes possibly fix #1211
- update json-smart to 2.4.9, related #1311
- httpclient to 4.5.14, #1273

* Adjust for JDBC workflow

- Add include/exclude for japicmp
- downgrade sortpom's version to 3.0.1 for java8; 3.2.1 is compiled by java11
- use src, target flag instead of release flag in maven-compiler-plugin because java 8 in test infra doesn't support release flag.
- run new formatter as suggested
- disable enforce-linkage-checker in self-contained-jar
- change back slf4j to provided scope
- disable japicmp in jenkinsIT profile
- disable maven-dependency-plugin:analyze-only in jenkinsIT profile
- change default build back to fat jar
- add slf4j & logback version bumps

---------

Co-authored-by: Preston Bennes <[email protected]>
Co-authored-by: wshangguan <[email protected]>
Co-authored-by: igarish <[email protected]>
@sfc-gh-dszmolka
Copy link
Contributor

hi folks - just a quick update; starting from version 3.14.5, we release the (for now, experimental) thin version of snowflake-jdbc (https://mvnrepository.com/artifact/net.snowflake/snowflake-jdbc-thin) which could solve problems like this.

if you have a possibility, test it please

@sfc-gh-dszmolka sfc-gh-dszmolka added the status-triage_done Initial triage done, will be further handled by the driver team label Apr 26, 2024
@sfc-gh-dszmolka
Copy link
Contributor

closing this issue as snowflake-jdbc-thin should solve this, and similar issues - if you have further concerns, do comment please and can reopen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backlog feature status-triage_done Initial triage done, will be further handled by the driver team
Projects
None yet
Development

No branches or pull requests

9 participants