Skip to content

Commit 062313c

Browse files
rohanKanojiamanusa
authored andcommitted
Fix #3189: VersionInfo contains null data in OpenShift 4.x
assert response code after getting response from apiServer rather than depending upon fragile null check
1 parent 1a61f0c commit 062313c

File tree

4 files changed

+59
-6
lines changed

4 files changed

+59
-6
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
* Retry only Non-Restful Create-only resources in OpenShiftOAuthInterceptor
88
* Fix #3126: a KubernetesClientException will be thrown from patch/replace rather than a null being returned when the item does not exist
99
* Fix #3121: ServiceOperationImpl replace will throw a KubernetesClientException rather than a NPE if the item doesn't exist
10+
* Fix #3189: VersionInfo contains null data in OpenShift 4.6
1011

1112
#### Improvements
1213
* Fix #3149: replace(item) will consult the item's resourceVersion for the first PUT attempt when not specifically locked on a resourceVersion

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/ClusterOperationsImpl.java

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,10 @@ public ClusterOperationsImpl(OkHttpClient client, Config config, String item) {
4343

4444
public VersionInfo fetchVersion() {
4545
try {
46-
Response response = handleVersionGet(versionEndpoint);
46+
Request request = getRequest(versionEndpoint);
47+
Response response = handleVersionGet(request);
48+
assertResponseCode(request, response);
49+
4750
Map<String, String> myMap = new HashMap<>();
4851

4952
if (response.body() != null) {
@@ -55,11 +58,15 @@ public VersionInfo fetchVersion() {
5558
}
5659
}
5760

58-
protected Response handleVersionGet(String versionEndpointToBeUsed) throws IOException {
59-
Request.Builder requestBuilder = new Request.Builder()
61+
protected Response handleVersionGet(Request request) throws IOException {
62+
return client.newCall(request).execute();
63+
}
64+
65+
protected Request getRequest(String versionEndpointToBeUsed) {
66+
return new Request.Builder()
6067
.get()
61-
.url(URLUtils.join(config.getMasterUrl(), versionEndpointToBeUsed));
62-
return client.newCall(requestBuilder.build()).execute();
68+
.url(URLUtils.join(config.getMasterUrl(), versionEndpointToBeUsed))
69+
.build();
6370
}
6471

6572
protected static VersionInfo fetchVersionInfoFromResponse(Map<String, String> responseAsMap) throws ParseException {
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/**
2+
* Copyright (C) 2015 Red Hat, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.fabric8.openshift;
17+
18+
import io.fabric8.kubernetes.client.VersionInfo;
19+
import io.fabric8.openshift.client.OpenShiftClient;
20+
import org.arquillian.cube.openshift.impl.requirement.RequiresOpenshift;
21+
import org.arquillian.cube.requirement.ArquillianConditionalRunner;
22+
import org.jboss.arquillian.test.api.ArquillianResource;
23+
import org.junit.Test;
24+
import org.junit.runner.RunWith;
25+
26+
import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
27+
28+
@RunWith(ArquillianConditionalRunner.class)
29+
@RequiresOpenshift
30+
public class VersionIT {
31+
@ArquillianResource
32+
OpenShiftClient client;
33+
34+
@Test
35+
public void getVersionShouldReturnValidVersion() {
36+
// Given + When
37+
VersionInfo versionInfo = client.getVersion();
38+
39+
// Then
40+
assertThat(versionInfo).isNotNull();
41+
assertThat(versionInfo.getMajor()).isNotNull();
42+
assertThat(versionInfo.getMinor()).isNotNull();
43+
assertThat(versionInfo.getBuildDate()).isNotNull();
44+
}
45+
}

openshift-client/src/main/java/io/fabric8/openshift/client/internal/OpenShiftClusterOperationsImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public VersionInfo fetchVersion() {
5353
}
5454

5555
private VersionInfo fetchOpenshift4Version() throws IOException, ParseException {
56-
Response response = handleVersionGet(OPENSHIFT4_VERSION_ENDPOINT);
56+
Response response = handleVersionGet(getRequest(OPENSHIFT4_VERSION_ENDPOINT));
5757
if (response.isSuccessful() && response.body() != null) {
5858
ClusterVersionList clusterVersionList = Serialization.jsonMapper().readValue(response.body().string(), ClusterVersionList.class);
5959
if (!clusterVersionList.getItems().isEmpty()) {

0 commit comments

Comments
 (0)