fix(core): fix bug where remote_cache caused project ids to leak (#1618)

* fix(core): fix bug where remote_cache caused project ids to leak between projects

* add test for instance attributes

Author: Panaetius

renku/core/management/repository.py

@@ -113,11 +113,11 @@ class RepositoryApiMixin(GitCore):
         "requirements\\.txt",
     ]
 
-    _commit_activity_cache = {}
+    _commit_activity_cache = attr.ib(factory=dict)
 
-    _activity_index = None
+    _activity_index = attr.ib(default=None)
 
-    _remote_cache = {}
+    _remote_cache = attr.ib(factory=dict)
 
     def __attrs_post_init__(self):
         """Initialize computed attributes."""

tests/core/commands/test_client.py

@@ -17,6 +17,8 @@
 # limitations under the License.
 """Test Python SDK client."""
 
+import inspect
+
 import pytest
 
 
@@ -41,3 +43,53 @@ def test_local_client(tmpdir):
 def test_ignored_paths(paths, ignored, client):
     """Test resolution of ignored paths."""
     assert client.find_ignored_paths(*paths) == ignored
+
+
+def test_safe_class_attributes(tmpdir):
+    """Test that there are no unsafe class attributes on the client.
+
+    This prevents us from adding class attributes that might leak in a threaded environment.
+    If you do add a class attribute and want to add it to the list of safe_attributes,
+    make sure that it's not something that can leak between calls, e.g. in the service.
+    """
+    from renku.core.management.client import LocalClient
+
+    # NOTE: attributes that are allowed on LocalClient
+    safe_attributes = [
+        "ACTIVITY_INDEX",
+        "CACHE",
+        "CONFIG_NAME",
+        "DATASETS",
+        "DATA_DIR_CONFIG_KEY",
+        "LOCK_SUFFIX",
+        "METADATA",
+        "POINTERS",
+        "RENKU_LFS_IGNORE_PATH",
+        "RENKU_PROTECTED_PATHS",
+        "WORKFLOW",
+        "_CMD_STORAGE_CHECKOUT",
+        "_CMD_STORAGE_CLEAN",
+        "_CMD_STORAGE_INSTALL",
+        "_CMD_STORAGE_LIST",
+        "_CMD_STORAGE_MIGRATE_INFO",
+        "_CMD_STORAGE_PULL",
+        "_CMD_STORAGE_STATUS",
+        "_CMD_STORAGE_TRACK",
+        "_CMD_STORAGE_UNTRACK",
+        "_LFS_HEADER",
+        "_global_config_dir",
+    ]
+
+    client1 = LocalClient(str(tmpdir.mkdir("project1")))
+
+    client2 = LocalClient(str(tmpdir.mkdir("project2")))
+    class_attributes = inspect.getmembers(LocalClient, lambda a: not (inspect.isroutine(a)))
+    class_attributes = [a for a in class_attributes if not a[0].startswith("__") and not a[0].endswith("__")]
+    identical_attributes = []
+    for a, v in class_attributes:
+        if a in safe_attributes or isinstance(v, property):
+            continue
+        if id(getattr(client1, a)) == id(getattr(client2, a)):
+            identical_attributes.append(a)
+
+    assert not identical_attributes