From 72aad07e0001a37a91a854b3ba9f8b0e9aeb1743 Mon Sep 17 00:00:00 2001
From: clux <sszynrae@gmail.com>
Date: Sun, 28 Feb 2021 23:44:56 +0000
Subject: [PATCH] ListParams now default enables bookmarks - closes #226

Since this flag does nothing for older apiservers it's safe to default
enable it. This will also close #219 in the process.

Note that ListParams does not pick up on the flag for non-watch calls.
---
 CHANGELOG.md                  |  4 ++++
 examples/configmap_watcher.rs |  2 +-
 kube/src/api/params.rs        | 28 +++++++++++++++++++++++-----
 kube/src/api/resource.rs      |  4 ++--
 4 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a143794d1..15f51d3ae 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,10 @@
 UNRELEASED
 ===================
  * see https://github.com/clux/kube-rs/compare/0.51.0...master
+ * `kube`: BREAKING: ListParams bookmarks default enabled-  #226 via
+   - renames `ListParams::allow_bookmarks` to `ListParams::bookmarks`
+   - `ListParams::default()` sets `bookmark` to true to avoid bad bad defaults
+   - `ListParams::allow_bookmarks` replaced by `ListParams::disable_bookmarks`
 
 0.51.0 / 2021-02-28
 ===================
diff --git a/examples/configmap_watcher.rs b/examples/configmap_watcher.rs
index ea72d1281..a3b3703e9 100644
--- a/examples/configmap_watcher.rs
+++ b/examples/configmap_watcher.rs
@@ -15,7 +15,7 @@ async fn main() -> anyhow::Result<()> {
     let namespace = std::env::var("NAMESPACE").unwrap_or("default".into());
 
     let cms: Api<ConfigMap> = Api::namespaced(client, &namespace);
-    let lp = ListParams::default().allow_bookmarks();
+    let lp = ListParams::default();
 
     let mut w = watcher(cms, lp).boxed();
     while let Some(event) = w.try_next().await? {
diff --git a/kube/src/api/params.rs b/kube/src/api/params.rs
index 7b8d834a8..8bfb7cf75 100644
--- a/kube/src/api/params.rs
+++ b/kube/src/api/params.rs
@@ -3,7 +3,7 @@ use crate::{Error, Result};
 use serde::Serialize;
 
 /// Common query parameters used in watch/list/delete calls on collections
-#[derive(Default, Clone)]
+#[derive(Clone)]
 #[allow(missing_docs)]
 pub struct ListParams {
     /// A selector to restrict the list of returned objects by their labels.
@@ -32,7 +32,7 @@ pub struct ListParams {
     /// If this is not a watch, this field is ignored.
     /// If the feature gate WatchBookmarks is not enabled in apiserver,
     /// this field is ignored.
-    pub allow_bookmarks: bool,
+    pub bookmarks: bool,
 
     /// Limit the number of results.
     ///
@@ -47,6 +47,21 @@ pub struct ListParams {
     pub continue_token: Option<String>,
 }
 
+impl Default for ListParams {
+    fn default() -> Self {
+        Self {
+            // bookmarks stable since 1.17, and backwards compatible
+            bookmarks: true,
+
+            label_selector: None,
+            field_selector: None,
+            timeout: None,
+            limit: None,
+            continue_token: None,
+        }
+    }
+}
+
 impl ListParams {
     pub(crate) fn validate(&self) -> Result<()> {
         if let Some(to) = &self.timeout {
@@ -99,9 +114,12 @@ impl ListParams {
         self
     }
 
-    /// Enables watch bookmarks from the api server if supported
-    pub fn allow_bookmarks(mut self) -> Self {
-        self.allow_bookmarks = true;
+    /// Disables watch bookmarks to simplify watch handling
+    ///
+    /// This is not recommended to use with production watchers as it can cause desyncs.
+    /// See [#219](https://github.com/clux/kube-rs/issues/219) for details.
+    pub fn disable_bookmarks(mut self) -> Self {
+        self.bookmarks = false;
         self
     }
 
diff --git a/kube/src/api/resource.rs b/kube/src/api/resource.rs
index eac22de28..7fd165780 100644
--- a/kube/src/api/resource.rs
+++ b/kube/src/api/resource.rs
@@ -140,7 +140,7 @@ impl Resource {
         if let Some(labels) = &lp.label_selector {
             qp.append_pair("labelSelector", &labels);
         }
-        if lp.allow_bookmarks {
+        if lp.bookmarks {
             qp.append_pair("allowWatchBookmarks", "true");
         }
 
@@ -474,7 +474,7 @@ mod test {
         let req = r.watch(&gp, "0").unwrap();
         assert_eq!(
             req.uri(),
-            "/api/v1/namespaces/ns/pods?&watch=true&resourceVersion=0&timeoutSeconds=290"
+            "/api/v1/namespaces/ns/pods?&watch=true&resourceVersion=0&timeoutSeconds=290&allowWatchBookmarks=true"
         );
     }
     #[test]