adding support for new list options

Closes #5368
fabric8io · Aug 1, 2023 · 3051d11 · 3051d11
1 parent a64f5ff
commit 3051d11
Show file tree

Hide file tree

Showing 14 changed files with 113 additions and 111 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,12 +5,14 @@
 #### Bugs
 
 #### Improvements
+* Fix #5368: added support for additional ListOptions fields
 
 #### Dependency Upgrade
 
 #### New Features
 
 #### _**Note**_: Breaking changes
+* Fix #5368: ListOptions parameter ordering is now alphabetical.  If you are using non-crud mocking for lists with options, you may need to update your parameter order.
 
 ### 6.8.0 (2023-07-24)
 

diff --git a/...-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/AbstractWatchManager.java b/...-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/AbstractWatchManager.java
@@ -301,7 +301,7 @@ void updateResourceVersion(final String newResourceVersion) {
    */
   protected void startWatch() {
     listOptions.setResourceVersion(resourceVersion.get());
-    URL url = BaseOperation.appendListOptionParams(requestUrl, listOptions);
+    URL url = this.baseOperation.appendListOptionParams(requestUrl, listOptions);
 
     String origin = requestUrl.getProtocol() + "://" + requestUrl.getHost();
     if (requestUrl.getPort() != -1) {

diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/BaseOperation.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/BaseOperation.java
@@ -78,6 +78,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.TreeMap;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.Executor;
@@ -1015,41 +1016,17 @@ private DefaultSharedIndexInformer<T, L> createInformer(long resync, Executor ex
     return informer;
   }
 
-  public static URL appendListOptionParams(URL base, ListOptions listOptions) {
+  public URL appendListOptionParams(URL base, ListOptions listOptions) {
     if (listOptions == null) {
       return base;
     }
     URLBuilder urlBuilder = new URLBuilder(base);
-    if (listOptions.getLimit() != null) {
-      urlBuilder.addQueryParameter("limit", listOptions.getLimit().toString());
-    }
-    if (listOptions.getContinue() != null) {
-      urlBuilder.addQueryParameter("continue", listOptions.getContinue());
-    }
-
-    if (listOptions.getFieldSelector() != null) {
-      urlBuilder.addQueryParameter("fieldSelector", listOptions.getFieldSelector());
-    }
-
-    if (listOptions.getLabelSelector() != null) {
-      urlBuilder.addQueryParameter("labelSelector", listOptions.getLabelSelector());
-    }
 
-    if (listOptions.getResourceVersion() != null) {
-      urlBuilder.addQueryParameter("resourceVersion", listOptions.getResourceVersion());
-    }
-
-    if (listOptions.getTimeoutSeconds() != null) {
-      urlBuilder.addQueryParameter("timeoutSeconds", listOptions.getTimeoutSeconds().toString());
-    }
+    Map<String, ?> values = getKubernetesSerialization().convertValue(listOptions, TreeMap.class);
+    values.remove("apiVersion");
+    values.remove("kind");
+    values.forEach((k, v) -> urlBuilder.addQueryParameter(k, v.toString()));
 
-    if (listOptions.getAllowWatchBookmarks() != null) {
-      urlBuilder.addQueryParameter("allowWatchBookmarks", listOptions.getAllowWatchBookmarks().toString());
-    }
-
-    if (listOptions.getWatch() != null) {
-      urlBuilder.addQueryParameter(WATCH, listOptions.getWatch().toString());
-    }
     return urlBuilder.build();
   }
 

diff --git a/...ent/src/test/java/io/fabric8/kubernetes/client/dsl/internal/AbstractWatchManagerTest.java b/...ent/src/test/java/io/fabric8/kubernetes/client/dsl/internal/AbstractWatchManagerTest.java
@@ -22,6 +22,7 @@
 import io.fabric8.kubernetes.client.WatcherException;
 import io.fabric8.kubernetes.client.dsl.internal.AbstractWatchManager.WatchRequestState;
 import io.fabric8.kubernetes.client.utils.CommonThreadPool;
+import io.fabric8.kubernetes.client.utils.KubernetesSerialization;
 import io.fabric8.kubernetes.client.utils.Utils;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
@@ -285,6 +286,8 @@ public void onClose() {
   static BaseOperation mockOperation() {
     BaseOperation operation = mock(BaseOperation.class, Mockito.RETURNS_DEEP_STUBS);
     Mockito.when(operation.getOperationContext().getExecutor()).thenReturn(Runnable::run);
+    Mockito.when(operation.getKubernetesSerialization()).thenReturn(new KubernetesSerialization());
+    Mockito.when(operation.appendListOptionParams(Mockito.any(), Mockito.any())).thenCallRealMethod();
     return operation;
   }
 

diff --git a/...tes-client/src/test/java/io/fabric8/kubernetes/client/dsl/internal/BaseOperationTest.java b/...tes-client/src/test/java/io/fabric8/kubernetes/client/dsl/internal/BaseOperationTest.java
@@ -34,6 +34,7 @@
 import io.fabric8.kubernetes.client.http.TestHttpResponse;
 import io.fabric8.kubernetes.client.impl.BaseClient;
 import io.fabric8.kubernetes.client.utils.CommonThreadPool;
+import io.fabric8.kubernetes.client.utils.KubernetesSerialization;
 import io.fabric8.kubernetes.client.utils.Serialization;
 import io.fabric8.kubernetes.client.utils.URLUtils;
 import io.fabric8.kubernetes.client.utils.Utils;
@@ -132,7 +133,13 @@ void testChainingGracePeriodAndPropagationPolicy() {
   void testListOptions() throws MalformedURLException {
     // Given
     URL url = new URL("https://172.17.0.2:8443/api/v1/namespaces/default/pods");
-    final BaseOperation<Pod, PodList, Resource<Pod>> operation = new BaseOperation<>(new OperationContext());
+    final BaseOperation<Pod, PodList, Resource<Pod>> operation = new BaseOperation<Pod, PodList, Resource<Pod>>(
+        new OperationContext()) {
+      @Override
+      public KubernetesSerialization getKubernetesSerialization() {
+        return new KubernetesSerialization();
+      }
+    };
 
     // When and Then
     assertEquals(URLUtils.join(url.toString(), "?limit=5"),
@@ -141,36 +148,36 @@ void testListOptions() throws MalformedURLException {
             .build()).toString());
     assertEquals(
         URLUtils.join(url.toString(),
-            "?limit=5&continue=eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ"),
+            "?continue=eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ&limit=5"),
         operation.fetchListUrl(url, new ListOptionsBuilder()
             .withLimit(5L)
             .withContinue("eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ")
             .build()).toString());
     assertEquals(URLUtils.join(url.toString(),
-        "?limit=5&continue=eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ&fieldSelector=status.phase%3DRunning"),
+        "?continue=eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ&fieldSelector=status.phase%3DRunning&limit=5"),
         operation.fetchListUrl(url, new ListOptionsBuilder()
             .withLimit(5L)
             .withContinue("eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ")
             .withFieldSelector("status.phase=Running")
             .build()).toString());
     assertEquals(URLUtils.join(url.toString(),
-        "?limit=5&continue=eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ&fieldSelector=status.phase%3DRunning&resourceVersion=210448"),
+        "?continue=eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ&fieldSelector=status.phase%3DRunning&limit=5&resourceVersion=210448"),
         operation.fetchListUrl(url, new ListOptionsBuilder()
             .withLimit(5L)
             .withContinue("eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ")
             .withFieldSelector("status.phase=Running")
             .withResourceVersion("210448")
             .build()).toString());
     assertEquals(URLUtils.join(url.toString(),
-        "?limit=5&continue=eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ&labelSelector=%21node-role.kubernetes.io%2Fmaster&resourceVersion=210448"),
+        "?continue=eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ&labelSelector=%21node-role.kubernetes.io%2Fmaster&limit=5&resourceVersion=210448"),
         operation.fetchListUrl(url, new ListOptionsBuilder()
             .withLimit(5L)
             .withContinue("eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ")
             .withLabelSelector("!node-role.kubernetes.io/master")
             .withResourceVersion("210448")
             .build()).toString());
     assertEquals(URLUtils.join(url.toString(),
-        "?limit=5&continue=eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ&labelSelector=%21node-role.kubernetes.io%2Fmaster&resourceVersion=210448&timeoutSeconds=10"),
+        "?continue=eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ&labelSelector=%21node-role.kubernetes.io%2Fmaster&limit=5&resourceVersion=210448&timeoutSeconds=10"),
         operation.fetchListUrl(url, new ListOptionsBuilder()
             .withLimit(5L)
             .withContinue("eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ")
@@ -179,7 +186,7 @@ void testListOptions() throws MalformedURLException {
             .withTimeoutSeconds(10L)
             .build()).toString());
     assertEquals(URLUtils.join(url.toString(),
-        "?limit=5&continue=eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ&labelSelector=%21node-role.kubernetes.io%2Fmaster&resourceVersion=210448&timeoutSeconds=10&allowWatchBookmarks=true"),
+        "?allowWatchBookmarks=true&continue=eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ&labelSelector=%21node-role.kubernetes.io%2Fmaster&limit=5&resourceVersion=210448&timeoutSeconds=10"),
         operation.fetchListUrl(url, new ListOptionsBuilder()
             .withLimit(5L)
             .withContinue("eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ")
@@ -189,7 +196,7 @@ void testListOptions() throws MalformedURLException {
             .withAllowWatchBookmarks(true)
             .build()).toString());
     assertEquals(URLUtils.join(url.toString(),
-        "?limit=5&continue=eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ&labelSelector=%21node-role.kubernetes.io%2Fmaster&resourceVersion=210448&timeoutSeconds=10&allowWatchBookmarks=true&watch=true"),
+        "?allowWatchBookmarks=true&continue=eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ&labelSelector=%21node-role.kubernetes.io%2Fmaster&limit=5&resourceVersion=210448&timeoutSeconds=10&watch=true"),
         operation.fetchListUrl(url, new ListOptionsBuilder()
             .withLimit(5L)
             .withContinue("eyJ2IjoibWV0YS5rOHMuaW8vdjEiLCJydiI6MjE0NDUzLCJzdGFydCI6ImV0Y2QtbWluaWt1YmVcdTAwMDAifQ")
@@ -208,6 +215,17 @@ void testListOptions() throws MalformedURLException {
     assertEquals(URLUtils.join(url.toString(), "?watch=true"), operation.fetchListUrl(url, new ListOptionsBuilder()
         .withWatch(true)
         .build()).toString());
+    // taken from the example showing how to use send initial events
+    assertEquals(
+        URLUtils.join(url.toString(),
+            "?allowWatchBookmarks=true&resourceVersion=&resourceVersionMatch=NotOlderThan&sendInitialEvents=true&watch=true"),
+        operation.fetchListUrl(url, new ListOptionsBuilder()
+            .withWatch(true)
+            .withSendInitialEvents()
+            .withAllowWatchBookmarks()
+            .withResourceVersion("")
+            .withResourceVersionMatch("NotOlderThan")
+            .build()).toString());
   }
 
   @Test

diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/CustomResourceTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/CustomResourceTest.java
@@ -430,8 +430,8 @@ public void onClose(WatcherException cause) {
   void testWatchSingleResource() throws IOException, InterruptedException {
     // Given
     server.expect()
-        .withPath("/apis/test.fabric8.io/v1alpha1/namespaces/ns1/hellos" + "?fieldSelector="
-            + Utils.toUrlEncoded("metadata.name=example-hello") + "&allowWatchBookmarks=true&watch=true")
+        .withPath("/apis/test.fabric8.io/v1alpha1/namespaces/ns1/hellos" + "?allowWatchBookmarks=true&fieldSelector="
+            + Utils.toUrlEncoded("metadata.name=example-hello") + "&watch=true")
         .andUpgradeToWebSocket()
         .open()
         .waitFor(WATCH_EVENT_PERIOD)
@@ -464,8 +464,8 @@ public void onClose(WatcherException cause) {
   void testWatchWithLabels() throws IOException, InterruptedException {
     // Given
     server.expect()
-        .withPath("/apis/test.fabric8.io/v1alpha1/namespaces/ns1/hellos?labelSelector=" + Utils.toUrlEncoded("foo=bar")
-            + "&allowWatchBookmarks=true&watch=true")
+        .withPath("/apis/test.fabric8.io/v1alpha1/namespaces/ns1/hellos?allowWatchBookmarks=true&labelSelector="
+            + Utils.toUrlEncoded("foo=bar") + "&watch=true")
         .andUpgradeToWebSocket()
         .open()
         .waitFor(WATCH_EVENT_PERIOD)
@@ -498,8 +498,9 @@ void testWatchSomeResourceVersion() throws IOException, InterruptedException {
     // Given
     String watchResourceVersion = "1001";
     server.expect()
-        .withPath("/apis/test.fabric8.io/v1alpha1/namespaces/ns1/hellos?resourceVersion=" + watchResourceVersion
-            + "&allowWatchBookmarks=true&watch=true")
+        .withPath("/apis/test.fabric8.io/v1alpha1/namespaces/ns1/hellos?allowWatchBookmarks=true&resourceVersion="
+            + watchResourceVersion
+            + "&watch=true")
         .andUpgradeToWebSocket()
         .open()
         .waitFor(WATCH_EVENT_PERIOD)
@@ -533,7 +534,7 @@ public void onClose(WatcherException cause) {
   void testWatchWithListOptions() throws IOException, InterruptedException {
     // Given
     server.expect().withPath(
-        "/apis/test.fabric8.io/v1alpha1/namespaces/ns1/hellos?resourceVersion=1003&timeoutSeconds=30&allowWatchBookmarks=true&watch=true")
+        "/apis/test.fabric8.io/v1alpha1/namespaces/ns1/hellos?allowWatchBookmarks=true&resourceVersion=1003&timeoutSeconds=30&watch=true")
         .andUpgradeToWebSocket()
         .open()
         .waitFor(WATCH_EVENT_PERIOD)
@@ -570,7 +571,7 @@ public void onClose(WatcherException cause) {
   void testWatchWithNamespaceAndListOptions() throws IOException, InterruptedException {
     // Given
     server.expect().withPath(
-        "/apis/test.fabric8.io/v1alpha1/namespaces/ns1/hellos?resourceVersion=1003&timeoutSeconds=30&allowWatchBookmarks=true&watch=true")
+        "/apis/test.fabric8.io/v1alpha1/namespaces/ns1/hellos?allowWatchBookmarks=true&resourceVersion=1003&timeoutSeconds=30&watch=true")
         .andUpgradeToWebSocket()
         .open()
         .waitFor(WATCH_EVENT_PERIOD)