Revert BackwardCompatibilityInterceptor's behavior of changing /apis to /oapi URLs

[rohanKanojia]

Revert #2373

This commit reverts 293ab9d which was added in order
to fix #2373. This behavior was causing problems with Strimzi's Kubernetes Client
upgrade: strimzi/strimzi-kafka-operator#3553 . On bisecting
the failing test, I found this commit as culprit. Git bisect log:

kubernetes-client : $ git bisect log
git bisect start
# bad: [ea09419284b32e6da70aadfaf9022485f2bec594] Merge pull request #2478 from rohanKanojia/pr/migrate-github-actions-java8
git bisect bad ea09419284b32e6da70aadfaf9022485f2bec594
# good: [c5fe5488dc15ed3d4795a3b3d676549c24c2e744] Post v4.10.3 release chores
git bisect good c5fe5488dc15ed3d4795a3b3d676549c24c2e744
# bad: [262e35f09be835bf5f3b5fbc4c16791224fbc390] Merge pull request #2381 from rohanKanojia/pr/issue2308
git bisect bad 262e35f09be835bf5f3b5fbc4c16791224fbc390
# bad: [d13a86480f699735dcc8e0ece90c24d4e49079e8] Merge pull request #2393 from rohanKanojia/pr/fix-failing-resource-it
git bisect bad d13a86480f699735dcc8e0ece90c24d4e49079e8
# good: [f1d695d59ce50a9067639df0b79855f03096adc8] Merge pull request #2371 from manusa/fix/mockserver-post
git bisect good f1d695d59ce50a9067639df0b79855f03096adc8
# good: [0b1e0668788aaf2d7f0e706f111ca67456cbb36c] Merge pull request #2361 from fabric8io/dependabot/maven/org.jboss-jandex-2.2.1.Final
git bisect good 0b1e0668788aaf2d7f0e706f111ca67456cbb36c
# bad: [14cae945ee20ce1ac980d492e53d388aa93eb371] Merge pull request #2372 from rohanKanojia/pr/issue2292
git bisect bad 14cae945ee20ce1ac980d492e53d388aa93eb371
# bad: [f6d4e8fedd2d7c36a5608e5f3a4fd73070e51559] Merge pull request #2375 from rohanKanojia/pr/issue2373
git bisect bad f6d4e8fedd2d7c36a5608e5f3a4fd73070e51559
# bad: [293ab9d445291de389d438f4a857928722230dba] Fix #2373: Unable to create a Template on OCP3
git bisect bad 293ab9d445291de389d438f4a857928722230dba
# first bad commit: [293ab9d445291de389d438f4a857928722230dba] Fix #2373: Unable to create a Template on OCP3

Handling OpenShift old /oapi requests seem to be handled in OpenShiftOperation where we check config.isOpenshiftApiGroupsEnabled() to modify OperationContext.

#2373 can be fixed by adding apiVersion in OpenShiftOperation.

Description

Type of change

• Bug fix (non-breaking change which fixes an issue)
• Feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change
• Chore (non-breaking change which doesn't affect codebase;
  test, version modification, documentation, etc.)

Checklist

• Code contributed by me aligns with current project license: Apache 2.0
• I Added CHANGELOG entry regarding this change
• I have implemented unit tests to cover my changes
• I have added/updated the javadocs and other documentation accordingly
• No new bugs, code smells, etc. in SonarCloud report
• I tested my code in Kubernetes
• I tested my code in OpenShift

[manusa]

You're "reverting" too much, some of the fixes provided after (like the one for createOrReplace #2467) are reverted too.

[rohanKanojia]

I reverted BackwardCompatibilityInterceptor to it's 4.10.3 state. I think #2467 was also caused by my change in #2375 . I haven't reverted tests for #2467 I added(they're passing with this change):

kubernetes-client/kubernetes-itests/src/test/java/io/fabric8/openshift/DeploymentConfigIT.java

Lines 93 to 97 in 323be1c

	public void createOrReplace() {
	// Given
	DeploymentConfig deploymentConfig = client.deploymentConfigs().inNamespace(session.getNamespace()).withName("dc-createorreplace").get();
	// When

kubernetes-client/kubernetes-itests/src/test/java/io/fabric8/openshift/RouteIT.java

Lines 99 to 107 in 323be1c

	public void createOrReplace() {
	// Given
	Route route = client.routes().inNamespace(currentNamespace).withName("route-createorreplace").get();
	// When
	route.getMetadata().setAnnotations(Collections.singletonMap("foo", "bar"));
	route.getSpec().setHost("test.fabric8.io");
	route = client.routes().inNamespace(currentNamespace).createOrReplace(route);

[manusa]

I reverted BackwardCompatibilityInterceptor to it's 4.10.3 state. I think #2467 was also caused by my change in #2375 .

I'm not so sure about that.

The thing is that with the "new" createOrReplace procedure, we are relying on a 409 Conflict HTTP status code response to trigger the replace request.

If you don't keep this check in the interceptor:

 if (!response.isSuccessful() && response.code() == HttpURLConnection.HTTP_NOT_FOUND)

The interceptor will "kick in" whenever a resource needs to be replaced (too).

Regarding tests not failing, I'm guessing the tests you implemented in #2467 fix did not include an OpenShift resource specific replace case scenario.

[rohanKanojia]

I think you're right. Right now BackwardsCompatibiityInterceptor(what is here in this PR) seems to be only converting /oapi URLs to /apis. Client sends /apis URLs for both OpenShift 3.10.0 and 3.11.0, this is the reason tests are passing on both. I should add that 404 check.

[sonarqubecloud]

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

17.6% Coverage
0.0% Duplication

The version of Java (1.8.0_265) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

[rohanKanojia]

[merge]

[tombentley]

@rohanKanojia please could you tell me when there might be a release with this change in?

[manusa]

Hi @tombentley
If there are no impediments, we'll be cutting a release today.

[tombentley]

Awesome, thanks!

[manusa]

Hi @tombentley
In case you missed it, new release is available https://github.com/fabric8io/kubernetes-client/releases/tag/v4.12.0 on Maven Central.