fix ok http client explicit dependency

[wind57]

No description provided.

[wind57]

My reasoning are based on 2.0.x branch - but the same applies to main.

The changes are small, but it will take a significant amount of words explaining it, so please bear with me.

We need to first read carefully this part of maven documentation (the second example with Project X, Y and Z and what a dependency ends up being resolved). After we transpose that example to our code base (removed all irrelevant parts), we have this in spring-cloud-kubernetes-dependencies:

....
<dependencyManagement>
    <dependencies>
        ....
        <dependency>
		<groupId>io.kubernetes</groupId>
		<artifactId>client-java</artifactId>
		<version>${kubernetes-java-client.version}</version>
	</dependency>

         .....

        <dependency>
		<groupId>com.squareup.okhttp3</groupId>
		<artifactId>okhttp</artifactId>
		<version>${okhttp.version}</version>
	</dependency>
	<dependency>
		<groupId>com.squareup.okhttp3</groupId>
		<artifactId>logging-interceptor</artifactId>
		<version>${okhttp.version}</version>
	</dependency>

         .....

Let's look now at one of our examples here. So in spring-cloud-kubernetes-client-config we require io.kubernetes:client-java:11.0.2. This internally has a dependency on com.squareup.okhttp3:okhttp:3.14.9.

Now suppose I create such a simple example. You would expect that the version of com.squareup.okhttp3:okhttp is 3.14.9 - but that is not the case. Since that maven documentation kind of explains it, the version that we end up with is going to be 3.14.4. (README in that project shows steps to reproduce). That means:

• if someone uses any bom from spring cloud to import its dependencies
• we are always overriding the okhttp library version, no matter what io.kubernetes really requires.

We should definitely get rid of managing that dependency ourselves, at least imo.

The downside is that if k8s-client ever drops okhttp client, some of our tests will fail, because the only place where we use okhttp is in tests or examples:

$ cd spring-cloud-kubernetes
$ grep -r "com.squareup.okhttp3" --exclude .flattened-pom.xml . | sort --unique  
./spring-cloud-kubernetes-examples/kubernetes-loadbalancer-example/greeting-service/pom.xml:                    <groupId>com.squareup.okhttp3</groupId>
./spring-cloud-kubernetes-integration-tests/discovery/tests/pom.xml:                    <groupId>com.squareup.okhttp3</groupId>
./spring-cloud-kubernetes-integration-tests/pom.xml:                            <groupId>com.squareup.okhttp3</groupId>
./spring-cloud-kubernetes-integration-tests/simple-configmap/pom.xml:                   <groupId>com.squareup.okhttp3</groupId>
./spring-cloud-kubernetes-integration-tests/simple-core/pom.xml:                        <groupId>com.squareup.okhttp3</groupId>

But that is a minor thing to worry about, because if the tests fail (and kubernetes-client) removed that dependency, we can declare it explicitly in the those projects (not via bom). On the other hand, there is another solution : we explicitly manage the version of okhttp that we need in the projects above, without managing them in bom. That is, for example in : spring-cloud-kubernetes-examples/kubernetes-loadbalancer-example/greeting-service/pom.xml we would have:

<!-- not tracked via bom ON PURPOSE, see defect 849 -->
<dependency>
      <groupId>com.squareup.okhttp3</groupId>
     <artifactId>okhttp</artifactId>
    <version>5.0.0-alpha.2</version>
</dependency>

instead of:

<dependency>
      <groupId>com.squareup.okhttp3</groupId>
     <artifactId>okhttp</artifactId>
</dependency>

what we currently have.

@ryanjbaxter ready to review, but more importantly ready to expand on what your thoughts are here. thank you.

[wind57]

@ryanjbaxter leaving a note here also - as this is ready for review too. It should address this

[ryanjbaxter]

Thanks for the detailed explanation!

So the reason why everything (the tests and examples) still compiles is because of the transitive dependency from kubernetes-client?

[wind57]

sort of. currently, even if kubernetes-client still drops ok http, our code will still compile and work, but only because we have that dependency in dependencyManagement and require it explicitly in those projects.

At the same time, if we drop it, it will still work (as this PR proves), because of the transitive dependency. The problem is that we override that transitive dependency version. To try and simplify this : dependencyManagement will tell what version of transitive dependency to use.

So, if we drop it like in this PR : we rely on the transitive one.
At the same time, if we do not want to rely on that transitive one, we should not manage ok-http via bom; because in that case it simply overrides the dependency version that kubernetes client really requires. It's like a vicious circle.

[ryanjbaxter]

Right I agree. I think its fine to drop the explicit dependency in the BOM and rely on kubernetes-client. If they do drop it we can address what approach to take if/when that happens.

[wind57]

in that case, this is ready to be merged and the defect that this triggered, also. thank you for looking into this.