fix #4924: allows non-okhttp clients to handle invalid status

Author: shawkins

CHANGELOG.md

@@ -20,6 +20,7 @@
 * Fix #4899: BuildConfigs.instantiateBinary().fromFile() does not time out
 * Fix #4908: using the response headers in the vertx response
 * Fix #4947: typo in HttpClient.Factory scoring system logic
+* Fix #4928: allows non-okhttp clients to handle invalid status
 
 #### Improvements
 * Fix #4675: adding a fully client side timeout for informer watches

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

@@ -642,28 +642,27 @@ public CompletableFuture<AbstractWatchManager<T>> submitWatch(ListOptions option
           if (t instanceof KubernetesClientException) {
             KubernetesClientException ke = (KubernetesClientException) t;
             List<Integer> furtherProcessedCodes = Arrays.asList(200, 503);
-            if (!furtherProcessedCodes.contains(ke.getCode())) {
-              throw ke;
-            }
-
-            //release the watch after disabling the watcher (to avoid premature call to onClose)
-            watcherToggle.disable();
-
-            // If the HTTP return code is 200 or 503, we retry the watch again using a persistent hanging
-            // HTTP GET. This is meant to handle cases like kubectl local proxy which does not support
-            // websockets. Issue: https://github.com/kubernetes/kubernetes/issues/25126
-            try {
-              return new WatchHTTPManager<>(
-                  httpClient,
-                  this,
-                  optionsToUse,
-                  watcher,
-                  getRequestConfig().getWatchReconnectInterval(),
-                  getRequestConfig().getWatchReconnectLimit());
-            } catch (MalformedURLException e) {
-              throw KubernetesClientException.launderThrowable(forOperationType(WATCH), e);
+            if (furtherProcessedCodes.contains(ke.getCode())) {
+              //release the watch after disabling the watcher (to avoid premature call to onClose)
+              watcherToggle.disable();
+
+              // If the HTTP return code is 200 or 503, we retry the watch again using a persistent hanging
+              // HTTP GET. This is meant to handle cases like kubectl local proxy which does not support
+              // websockets. Issue: https://github.com/kubernetes/kubernetes/issues/25126
+              try {
+                return new WatchHTTPManager<>(
+                    httpClient,
+                    this,
+                    optionsToUse,
+                    watcher,
+                    getRequestConfig().getWatchReconnectInterval(),
+                    getRequestConfig().getWatchReconnectLimit());
+              } catch (MalformedURLException e) {
+                throw KubernetesClientException.launderThrowable(forOperationType(WATCH), e);
+              }
             }
           }
+          throw KubernetesClientException.launderThrowable(t);
         } finally {
           watch.close();
         }

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

@@ -651,10 +651,12 @@ public static Status createStatus(HttpResponse<?> response) {
         String bodyString = response.bodyString();
         if (Utils.isNotNullOrEmpty(bodyString)) {
           Status status = JSON_MAPPER.readValue(bodyString, Status.class);
-          if (status.getCode() == null) {
-            status = new StatusBuilder(status).withCode(statusCode).build();
+          if (status != null) {
+            if (status.getCode() == null) {
+              status = new StatusBuilder(status).withCode(statusCode).build();
+            }
+            return status;
           }
-          return status;
         }
       } catch (IOException e) {
         // ignored

kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/WatchTest.java

@@ -19,6 +19,7 @@
 import io.fabric8.kubernetes.api.model.ListOptionsBuilder;
 import io.fabric8.kubernetes.api.model.Pod;
 import io.fabric8.kubernetes.api.model.PodBuilder;
+import io.fabric8.kubernetes.api.model.PodList;
 import io.fabric8.kubernetes.api.model.StatusBuilder;
 import io.fabric8.kubernetes.api.model.WatchEvent;
 import io.fabric8.kubernetes.api.model.WatchEventBuilder;
@@ -27,6 +28,8 @@
 import io.fabric8.kubernetes.client.Watch;
 import io.fabric8.kubernetes.client.Watcher;
 import io.fabric8.kubernetes.client.WatcherException;
+import io.fabric8.kubernetes.client.dsl.MixedOperation;
+import io.fabric8.kubernetes.client.dsl.PodResource;
 import io.fabric8.kubernetes.client.dsl.Watchable;
 import io.fabric8.kubernetes.client.server.mock.EnableKubernetesMockClient;
 import io.fabric8.kubernetes.client.server.mock.KubernetesMockServer;
@@ -48,6 +51,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
@@ -431,14 +435,38 @@ public void onClose(WatcherException cause) {
     assertTrue(latch.await(10, TimeUnit.SECONDS));
   }
 
+  @Test
+  void testUnknownResponse() throws InterruptedException {
+    // Given
+
+    // trigger the usage of the http watch
+    server.expect()
+        .withPath("/api/v1/namespaces/test/pods?allowWatchBookmarks=true&watch=true")
+        .andReturn(404, "null")
+        .times(2);
+
+    MixedOperation<Pod, PodList, PodResource> pods = client.pods();
+
+    assertThrows(KubernetesClientException.class, () -> pods.watch(new Watcher<Pod>() {
+
+      @Override
+      public void eventReceived(Action action, Pod resource) {
+      }
+
+      @Override
+      public void onClose(WatcherException cause) {
+      }
+    }));
+  }
+
   @Test
   void testHttpWatch() throws InterruptedException {
     // Given
 
     // trigger the usage of the http watch
     server.expect()
         .withPath("/api/v1/namespaces/test/pods?allowWatchBookmarks=true&watch=true")
-        .andReturn(200, null)
+        .andReturn(200, "")
         .once();
 
     String dummyEvent = Serialization.asJson(new WatchEventBuilder().withType("MODIFIED")