Allow kwargs for fileserver roots update (bsc#1218482) (#618)

* Allow kwargs for fileserver roots update (bsc#1218482)

* Prevent exceptions with fileserver.update when called via state

* Fix wrong logic and enhance tests around fileserver.update

* Remove test which is not longer valid

---------

Co-authored-by: Pablo Suárez Hernández <[email protected]>

changelog/65819.fixed.md

@@ -0,0 +1 @@
+Prevent exceptions with fileserver.update when called via state

salt/runners/fileserver.py

@@ -350,6 +350,12 @@ def update(backend=None, **kwargs):
         salt-run fileserver.update backend=git remotes=myrepo,yourrepo
     """
     fileserver = salt.fileserver.Fileserver(__opts__)
+
+    # Remove possible '__pub_user' in kwargs as it is not expected
+    # on "update" function for the different fileserver backends.
+    if "__pub_user" in kwargs:
+        del kwargs["__pub_user"]
+
     fileserver.update(back=backend, **kwargs)
     return True
 

tests/integration/runners/test_fileserver.py

@@ -202,15 +202,31 @@ def test_update(self):
         fileserver.update
         """
         ret = self.run_run_plus(fun="fileserver.update")
-        self.assertTrue(ret["return"])
+        self.assertTrue(ret["return"] is True)
 
         # Backend submitted as a string
         ret = self.run_run_plus(fun="fileserver.update", backend="roots")
-        self.assertTrue(ret["return"])
+        self.assertTrue(ret["return"] is True)
 
         # Backend submitted as a list
         ret = self.run_run_plus(fun="fileserver.update", backend=["roots"])
-        self.assertTrue(ret["return"])
+        self.assertTrue(ret["return"] is True)
+
+        # Possible '__pub_user' is removed from kwargs
+        ret = self.run_run_plus(
+            fun="fileserver.update", backend=["roots"], __pub_user="foo"
+        )
+        self.assertTrue(ret["return"] is True)
+
+        # Unknown arguments
+        ret = self.run_run_plus(
+            fun="fileserver.update", backend=["roots"], unknown_arg="foo"
+        )
+        self.assertIn(
+            "Passed invalid arguments: update() got an unexpected keyword argument"
+            " 'unknown_arg'",
+            ret["return"],
+        )
 
         # Other arguments are passed to backend
         def mock_gitfs_update(remotes=None):
@@ -225,7 +241,23 @@ def mock_gitfs_update(remotes=None):
             ret = self.run_run_plus(
                 fun="fileserver.update", backend="gitfs", remotes="myrepo,yourrepo"
             )
-            self.assertTrue(ret["return"])
+            self.assertTrue(ret["return"] is True)
+            mock_backend_func.assert_called_once_with(remotes="myrepo,yourrepo")
+
+        # Possible '__pub_user' arguments are removed from kwargs
+        mock_backend_func = create_autospec(mock_gitfs_update)
+        mock_return_value = {
+            "gitfs.envs": None,  # This is needed to activate the backend
+            "gitfs.update": mock_backend_func,
+        }
+        with patch("salt.loader.fileserver", MagicMock(return_value=mock_return_value)):
+            ret = self.run_run_plus(
+                fun="fileserver.update",
+                backend="gitfs",
+                remotes="myrepo,yourrepo",
+                __pub_user="foo",
+            )
+            self.assertTrue(ret["return"] is True)
             mock_backend_func.assert_called_once_with(remotes="myrepo,yourrepo")
 
         # Unknown arguments are passed to backend