KNOX-3094 - Update CM API swagger to 7.13.1 #992
Conversation
…ndency convergence for org.jetbrains:annotations.
| } | ||
| } | ||
| } | ||
| throw new IllegalStateException("Unexpected default trust managers:" + Arrays.toString(trustManagers)); |
There was a problem hiding this comment.
What is the consequence of throwing this exception? The DiscoveryApiClient#configureSsl() method does not catch it, so it will bubble up. Do you know where it will eventually be handled and how?
There was a problem hiding this comment.
I took this part of the migration from here:
https://square.github.io/okhttp/5.x/okhttp/okhttp3/-ok-http-client/-builder/ssl-socket-factory.html
I would expect this to work in a standard setup (and it's the case with JDK 8-17).
I have not tested it on a FIPS-enabled cluster though.
I throw and catch IllegalStateException and return null if the trust manager for the default algorithm and trust store is not an instance of X509TrustManager. The getTruststoreSSLContext() method also uses the same methods by calling SSLContextBuilder.loadTrustMaterial(): uses the default TrustManagerFactory algorithm, initializes and gets the trust managers; it does not check the instance type. But the okhttp API expects an implementation of javax.net.ssl.X509TrustManager to validate the server's certificates.
The deprecated method variant does not require it, okhttp would use reflection to get one from sun.security.ssl.SSLContextImpl:
https://github.com/square/okhttp/blob/4984568367caaf359b82c452bd28b5e192824d1c/okhttp/src/main/kotlin/okhttp3/internal/platform/Platform.kt#L88
But this was removed in Okhttp 5.
|
According to the PR description, this change is supposed to upgrade CM and OkHttp versions. However, I see that many others were touched: |
|
Okhttp 4.x is written in Kotlin and needs the kotlin-stdlib artifacts. The com.cloudera.api.swagger:cloudera-manager-api-swagger:7.13.1 brings in Okhttp 4.10.0 is not the latest, but the swagger-codegen library only has this option. From the CM part, they would have liked to set the okhttp3 version to 4.12.0, but swagger-codegen is bringing internally version 4.10.0: swagger-api/swagger-codegen#12337 So we need to bump okhttp to 4.12.0, and I was also asked to upgrade gson-fire to 1.9.0; the latter has a gson:2.10.1 dependency (we had 2.8.9). The kotlin libraries are upgraded to 1.9.10 because of dependency convergence issues: "OkHttp 4.12.0 expects you to use any version of kotlin greater than or equal to 1.8.21. We don't rush out a release each time okio releases with a newer Kotlin. This isn't a thing we worry about at all." ... "It's already fixed in the 5.0 alphas.". Due to these issues, teams either remain on the latest non-kotlin version or remove okhttp: org.jetbrains:annotations also needed to be resolved: I excluded it from kotlin-stdlib because by simply adding to dependencyManagement would cause it to become a compile-time dependency on pty4j. The okio dependency was pinned to 3.6.0 because of this: hadoop-hdfs-client is a test dependency (so the old okhttp:2.7.5 will not be in our release artifact dependencies) and okio is backwards-compatible: |
Isn't it possible to exclude them everywhere? I'd like to avoid pulling in more 3rd party dependencies that are not essential for Knox to work. They are expensive to maintain in the future (CVEs, possible incompatibility, etc...). |
What changes were proposed in this pull request?
Upgrade com.cloudera.api.swagger:cloudera-manager-api-swagger to 7.13.1.
Update code for okhttp3. Resolve dependency convergence issues.
How was this patch tested?
Unit and integration tests, deployed to a dev cluster and restarted Knox to verify discovery and knox ui works.