fix: delete storage config fields (#416)

Fixes deleting storage configuration fields by setting them to `null`.

Author: leafty

.devcontainer/devcontainer.json

@@ -14,7 +14,8 @@
 		"ghcr.io/mpriscella/features/kind:1": {},
 		"ghcr.io/devcontainers-contrib/features/gh-release:1": {
 			"repo": "authzed/zed",
-			"binaryNames": "zed"
+			"binaryNames": "zed",
+			"version": "v0.21.1"
 		},
 		"ghcr.io/devcontainers-contrib/features/spicedb:1": {},
 		"ghcr.io/devcontainers/features/kubectl-helm-minikube:1": {

components/renku_data_services/data_connectors/core.py

@@ -80,7 +80,8 @@ def validate_storage_patch(
     if patch.configuration is not None:
         # we need to apply the patch to the existing storage to properly validate it
         patch.configuration = {**storage.configuration, **patch.configuration}
-        for k, v in patch.configuration.items():
+        dict_items = list(patch.configuration.items())
+        for k, v in dict_items:
             if v is None:
                 # delete fields that were unset
                 del patch.configuration[k]

test/bases/renku_data_services/data_api/test_data_connectors.py

@@ -248,6 +248,42 @@ async def test_patch_data_connector(sanic_client: SanicASGITestClient, create_da
     assert set(data_connector.get("keywords")) == {"keyword 1", "keyword 2"}
 
 
+@pytest.mark.asyncio
+async def test_patch_data_connector_can_unset_storage_field(
+    sanic_client: SanicASGITestClient, create_data_connector, user_headers
+) -> None:
+    initial_storage = {
+        "configuration": {
+            "provider": "AWS",
+            "type": "s3",
+            "region": "us-east-1",
+            "access_key_id": "ACCESS KEY",
+            "secret_access_key": "SECRET",
+        },
+        "source_path": "my-bucket",
+        "target_path": "my_data",
+    }
+    data_connector = await create_data_connector("My data connector", storage=initial_storage)
+
+    headers = merge_headers(user_headers, {"If-Match": data_connector["etag"]})
+    data_connector_id = data_connector["id"]
+    patch = {"storage": {"configuration": {"region": None, "access_key_id": None, "secret_access_key": None}}}
+    _, response = await sanic_client.patch(
+        f"/api/data/data_connectors/{data_connector_id}", headers=headers, json=patch
+    )
+
+    assert response.status_code == 200, response.text
+    assert response.json is not None
+    new_configuration = response.json["storage"]["configuration"]
+    assert new_configuration is not None
+    assert new_configuration["provider"] == "AWS"
+    assert new_configuration["type"] == "s3"
+    assert "region" not in new_configuration
+    assert "access_key_id" not in new_configuration
+    assert "secret_access_key" not in new_configuration
+    assert len(response.json["storage"]["sensitive_fields"]) == 0
+
+
 @pytest.mark.asyncio
 async def test_delete_data_connector(sanic_client: SanicASGITestClient, create_data_connector, user_headers) -> None:
     await create_data_connector("Data connector 1")