feat: Allow explicit null and empty string

changes/.feature.md

@@ -0,0 +1 @@
+Allows explicit `null` values and empty string for updating ContainerRegistry.

src/ai/backend/manager/config.py

@@ -357,7 +357,7 @@
         t.Key(""): tx.URL,
         t.Key("type", default="docker"): t.String,
         t.Key("username", default=None): t.Null | t.String,
-        t.Key("password", default=None): t.Null | t.String,
+        t.Key("password", default=None): t.Null | t.String(allow_blank=True),
         t.Key("project", default=None): (
             t.Null | t.List(t.String) | tx.StringList(empty_str_as_empty_list=True)
         ),
@@ -711,6 +711,13 @@ async def modify_container_registry(
         raw_hostname = urllib.parse.quote(hostname, safe="")
         await self.etcd.delete_prefix(f"{self.ETCD_CONTAINER_REGISTRY_KEY}/{raw_hostname}")
 
+        # Exclude `None` values.
+        unset_password = False
+        if "password" in config_updated and config_updated["password"] is None:
+            # _ = config_updated.pop("password")
+            unset_password = True
+            config_updated["password"] = ""
+
         # Re-add the "accidentally" deleted items
         updates: dict[str, str] = {}
         for key, raw_item in registries.items():
@@ -741,6 +748,9 @@ async def modify_container_registry(
         )
         await self.etcd.put_dict(updates)
 
+        if unset_password:
+            await self.etcd.delete(f"{self.ETCD_CONTAINER_REGISTRY_KEY}/{raw_hostname}/password")
+
     async def delete_container_registry(self, hostname: str) -> None:
         # Fetch the raw registries data and make it a mutable dict.
         registries = dict(await self.etcd.get_prefix(self.ETCD_CONTAINER_REGISTRY_KEY))

src/ai/backend/manager/models/etcd.py

@@ -159,10 +159,14 @@ async def mutate(
         props: ModifyContainerRegistryInput,
     ) -> ModifyContainerRegistry:
         ctx: GraphQueryContext = info.context
-        input_config: Dict[str, Any] = {"": props.url, "type": props.type}
+        input_config: Dict[str, Any] = {}
+        # 1. 해당 필드 자체가 없으면 건드리지 말 것
+        # 2. 해당 필드가 ""면 에러 나지 않고 ""로 덮어쓸 것
+        # 3. 해당 필드가 null이면 unset 할 것
         set_if_set(props, input_config, "project")
         set_if_set(props, input_config, "username")
-        set_if_set(props, input_config, "password")
+        # set_if_set(props, input_config, "password")  # None
+        input_config["password"] = props.password
         set_if_set(props, input_config, "ssl_verify")
         log.info(
             "ETCD.MODIFY_CONTAINER_REGISTRY (ak:{}, hostname:{}, config:{})",

tests/client/integration/test_kernel.py

@@ -49,7 +49,7 @@ def exec_loop(kernel, mode, code, opts=None, user_inputs=None):
     return aggregate_console(console), num_queries
 
 
-@pytest.yield_fixture
+@pytest.fixture
 def py3_kernel():
     with Session() as sess:
         kernel = sess.Kernel.get_or_create("python:3.6-ubuntu18.04")

tests/manager/models/test_etcd.py

@@ -0,0 +1,163 @@
+import pytest
+
+from ai.backend.client.session import APIConfig, Session
+
+CONTAINER_REGISTRY_FIELDS = """
+    container_registry {
+        hostname
+        config {
+            url
+            type
+            project
+            username
+            password
+            ssl_verify
+        }
+    }
+"""
+
+
+def _admin_session():
+    api_config = APIConfig(
+        endpoint="http://127.0.0.1:8091",
+        endpoint_type="api",
+        access_key="AKIAIOSFODNN7EXAMPLE",
+        secret_key="wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
+        skip_sslcert_validation=True,
+    )
+    return Session(config=api_config)
+
+
+@pytest.mark.dependency()
+def test_create_container_registry():
+    query = """
+        mutation CreateContainerRegistry($hostname: String!, $props: CreateContainerRegistryInput!) {
+            create_container_registry(hostname: $hostname, props: $props) {
+                $CONTAINER_REGISTRY_FIELDS
+            }
+        }
+    """.replace("$CONTAINER_REGISTRY_FIELDS", CONTAINER_REGISTRY_FIELDS)
+
+    variables = {
+        "hostname": "cr.example.com",
+        "props": {
+            "url": "http://cr.example.com",
+            "type": "harbor2",
+            "project": ["default"],
+            "username": "username",
+            "password": "password",
+            "ssl_verify": False,
+        },
+    }
+
+    with _admin_session() as sess:
+        response = sess.Admin.query(query=query, variables=variables)
+        container_registry = response["create_container_registry"]["container_registry"]
+        assert container_registry["hostname"] == "cr.example.com"
+        assert container_registry["config"] == {
+            "url": "http://cr.example.com",
+            "type": "harbor2",
+            "project": ["default"],
+            "username": "username",
+            "password": "*****",
+            "ssl_verify": False,
+        }
+
+
+@pytest.mark.dependency(depends=["test_create_container_registry"])
+def test_modify_container_registry():
+    query = """
+        mutation ModifyContainerRegistry($hostname: String!, $props: ModifyContainerRegistryInput!) {
+            modify_container_registry(hostname: $hostname, props: $props) {
+                $CONTAINER_REGISTRY_FIELDS
+            }
+        }
+    """.replace("$CONTAINER_REGISTRY_FIELDS", CONTAINER_REGISTRY_FIELDS)
+
+    variables = {
+        "hostname": "cr.example.com",
+        "props": {
+            "username": "username2",
+        },
+    }
+
+    with _admin_session() as sess:
+        response = sess.Admin.query(query=query, variables=variables)
+        container_registry = response["modify_container_registry"]["container_registry"]
+        assert container_registry["config"]["url"] == "http://cr.example.com"
+        assert container_registry["config"]["type"] == "harbor2"
+        assert container_registry["config"]["project"] == ["default"]
+        assert container_registry["config"]["username"] == "username2"
+        assert container_registry["config"]["ssl_verify"] is False
+
+
+@pytest.mark.dependency(depends=["test_modify_container_registry"])
+def test_modify_container_registry_allows_empty_string():
+    query = """
+        mutation ModifyContainerRegistry($hostname: String!, $props: ModifyContainerRegistryInput!) {
+            modify_container_registry(hostname: $hostname, props: $props) {
+                $CONTAINER_REGISTRY_FIELDS
+            }
+        }
+    """.replace("$CONTAINER_REGISTRY_FIELDS", CONTAINER_REGISTRY_FIELDS)
+
+    variables = {
+        "hostname": "cr.example.com",
+        "props": {
+            "password": "",
+        },
+    }
+
+    with _admin_session() as sess:
+        response = sess.Admin.query(query=query, variables=variables)
+        container_registry = response["modify_container_registry"]["container_registry"]
+        assert container_registry["config"]["url"] == "http://cr.example.com"
+        assert container_registry["config"]["type"] == "harbor2"
+        assert container_registry["config"]["project"] == ["default"]
+        # assert container_registry["config"]["username"] == "username2"
+        assert container_registry["config"]["password"] == "*****"
+        assert container_registry["config"]["ssl_verify"] is False
+
+
+@pytest.mark.dependency(depends=["test_modify_container_registry_allows_empty_string"])
+def test_modify_container_registry_allows_null_for_unset():
+    query = """
+        mutation ModifyContainerRegistry($hostname: String!, $props: ModifyContainerRegistryInput!) {
+            modify_container_registry(hostname: $hostname, props: $props) {
+                $CONTAINER_REGISTRY_FIELDS
+            }
+        }
+    """.replace("$CONTAINER_REGISTRY_FIELDS", CONTAINER_REGISTRY_FIELDS)
+
+    variables = {
+        "hostname": "cr.example.com",
+        "props": {
+            "password": None,
+        },
+    }
+
+    with _admin_session() as sess:
+        response = sess.Admin.query(query=query, variables=variables)
+        container_registry = response["modify_container_registry"]["container_registry"]
+        assert container_registry["config"]["url"] == "http://cr.example.com"
+        assert container_registry["config"]["type"] == "harbor2"
+        assert container_registry["config"]["project"] == ["default"]
+        # assert container_registry["config"]["username"] == "username2"
+        assert container_registry["config"]["password"] is None
+        assert container_registry["config"]["ssl_verify"] is False
+
+
+@pytest.mark.dependency(depends=["test_modify_container_registry_allows_null_for_unset"])
+def test_delete_container_registry():
+    query = """
+        mutation DeleteContainerRegistry($hostname: String!) {
+            delete_container_registry(hostname: $hostname) {
+                $CONTAINER_REGISTRY_FIELDS
+            }
+        }
+    """.replace("$CONTAINER_REGISTRY_FIELDS", CONTAINER_REGISTRY_FIELDS)
+
+    with _admin_session() as sess:
+        response = sess.Admin.query(query=query, variables={"hostname": "cr.example.com"})
+        container_registry = response["delete_container_registry"]["container_registry"]
+        assert container_registry["hostname"] == "cr.example.com"