search

instaclustr / cassandra-exporter

Java agent for exporting Cassandra metrics to Prometheus
Apache License 2.0
73 stars 46 forks source link

Don't depend on sun.security package #99

Open multani opened 3 years ago

multani commented 3 years ago

This is not portable, and fails depending on the JVM used, see https://www.oracle.com/java/technologies/faq-sun-packages.html:

The sun. packages are not part of the supported, public interface. A Java program that directly calls into sun. packages is not guaranteed to work on all Java-compatible platforms. In fact, such a program is not guaranteed to work even in future versions on the same platform.

Also:

$ docker run --rm -ti -v $PWD:/src -w /src maven:3-openjdk-11 mvn package
[...]
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] /src/common/src/test/java/com/zegelin/cassandra/exporter/netty/ssl/TestSslContextFactory.java:[10,24] sun.security.ssl.SSLEngineImpl is not public in sun.security.ssl; cannot be accessed from outside package
[ERROR] /src/common/src/test/java/com/zegelin/cassandra/exporter/netty/ssl/TestSslContextFactory.java:[49,78] sun.security.ssl.SSLEngineImpl is not public in sun.security.ssl; cannot be accessed from outside package
[ERROR] /src/common/src/test/java/com/zegelin/cassandra/exporter/netty/ssl/TestSslContextFactory.java:[166,78] sun.security.ssl.SSLEngineImpl is not public in sun.security.ssl; cannot be accessed from outside package
[ERROR] /src/common/src/test/java/com/zegelin/cassandra/exporter/netty/ssl/TestSslContextFactory.java:[178,78] sun.security.ssl.SSLEngineImpl is not public in sun.security.ssl; cannot be accessed from outside package
[...]

For some reason, the tests pass with the CircleCI Java Docker image; I tried with various combination of the JDK (dockerized Maven's maven:3-openjdk-11 and maven:3-openjdk-8 and the JDK from Oracle's own website) and none work.

itskarlsson commented 1 year ago

I agree that depending on sun.* packages should be fixed, but the proposed fix you uploaded doesn't actually test anything since you are just checking the interface which both are a part of SSL implementations use. Those tests will pass even when the incorrect implementation is passed.