Merge ~bowenfan/snapstore-client:SN2279-model_service_update_commands into snapstore-client:main

[Bowen Fan]

Add model service update CLI for updating model API key

Model API key is the only updatable attribute in the on-prem model service
for now. Signing key names cannot be updated in a straightforward way because
the user would have to manually update the new corresponding account-key assertion.

Also add associated tests, fixtures, and CLI runner config.

Solves: https://warthogs.atlassian.net/browse/SN-2279

Part of 5 snapstore-client MPs to add model service support

1: Add publishergw support and a skeleton for model service < Done [1]
2: Add remaining create CLI functions < Done [2]
3: List
4: Update < This MP
5: Delete

CLI commands have been implemented, as much as possible, to follow this command structure guidance doc: https://discourse.ubuntu.com/t/command-structure/18556

On-prem model service spec: https://docs.google.com/document/d/1uxJ0Z1hCN_-6aJRwy9RhM3KblSuMgHqPwYEyP_88Ijk/edit

-----

[1] https://code.launchpad.net/~bowenfan/snapstore-client/+git/snapstore-client/+merge/457208
[2] https://code.launchpad.net/~bowenfan/snapstore-client/+git/snapstore-client/+merge/457241

[Maximiliano Bertacchini (maxiberta)]

LGTM +1

review: Approve

diff --git a/store_admin/cli/model_service.py b/store_admin/cli/model_service.py
index 2f990b3..8929a65 100644
--- a/store_admin/cli/model_service.py
+++ b/store_admin/cli/model_service.py
@@ -43,3 +43,11 @@ def create_signing_key(key_name):
 def create_serial_policy(model_name, signing_key_sha3_384):
     """Create a serial policy in the model service."""
     model_service.create_serial_policy(model_name, signing_key_sha3_384)
+
+
[e-mail](name="model")
[e-mail]("model_name")
[e-mail]("--api-key", help="New API key")
+def update_model(model_name, api_key):
+    """Update an existing model in the model service."""
+    model_service.update_model(model_name, api_key)
diff --git a/store_admin/cli/runner.py b/store_admin/cli/runner.py
index 7ff337d..88a6ce9 100644
--- a/store_admin/cli/runner.py
+++ b/store_admin/cli/runner.py
@@ -56,6 +56,11 @@ def lst():
 
 
 @run.group()
+def update():
+    "Update various artifacts on the managed proxy."
+
+
[e-mail]()
 def upload():
     "Upload various artifacts to the managed proxy."
 
@@ -83,6 +88,9 @@ create.add_command(model_service.create_serial_policy)
 # Commands under list
 lst.add_command(overrides.list_overrides)
 
+# Commands under update
+update.add_command(model_service.update_model)
+
 # Commands under upload
 upload.add_command(overrides.upload_overrides)
 upload.add_command(snaps.upload_snap)
diff --git a/store_admin/logic/model_service.py b/store_admin/logic/model_service.py
index 8dae87d..de7a084 100644
--- a/store_admin/logic/model_service.py
+++ b/store_admin/logic/model_service.py
@@ -75,3 +75,16 @@ def create_serial_policy(model_name, signing_key_sha3_384):
         f"Model '{model_name}' configured to sign serials "
         f"with key '{signing_key_sha3_384}'."
     )
+
+
+def update_model(model_name, api_key):
+    cfg = config.Config()
+    store = _check_default_store(cfg)
+    brand_account_id = _get_or_set_brand_account_id(cfg)
+    if not store or not brand_account_id:
+        return 1
+
+    json = {"api-key": api_key}
+    path = f"/publishergw/v1/brand/{brand_account_id}/model/{model_name}"
+    ws.model_service_update(store, path, json)
+    logger.info(f"Model '{model_name}' updated with new API key '{api_key}'.")
diff --git a/store_admin/logic/tests/conftest.py b/store_admin/logic/tests/conftest.py
index b0b08ac..2c7dc58 100644
--- a/store_admin/logic/tests/conftest.py
+++ b/store_admin/logic/tests/conftest.py
@@ -215,35 +215,84 @@ def _create_model_service_user_response(display_name=None, id=None, username=Non
     }
 
 
[e-mail]
-def model_service_model_apis(requests_mock, brand_account_id):
-    def _make_model_response(brand_account_id, **kwargs):
-        model = {}
+def _make_model_response(brand_account_id, **kwargs):
+    model = {}
 
-        model["brand_account_id"] = brand_account_id
-        model["name"] = kwargs.get("name") or "test-model"
-        model["series"] = "16"
-        model["api-key"] = kwargs.get("api_key") or "test-api-key"
+    model["brand_account_id"] = brand_account_id
+    model["name"] = kwargs.get("name") or "test-model"
+    model["series"] = "16"
+    model["api-key"] = kwargs.get("api_key") or "test-api-key"
+
+    model["created-by"] = (
+        kwargs.get("created_by") or _create_model_service_user_response()
+    )
+    model["created-at"] = (
+        kwargs.get("created_at") or datetime.utcnow().isoformat() + "Z",
+    )
+    model["modified-by"] = (
+        _create_model_service_user_response()
+        if "modified_by" not in kwargs
+        else kwargs["modified_by"]
+    )
+    model["modified-at"] = (
+        datetime.utcnow().isoformat() + "Z"
+        if "modified_at" not in kwargs
+        else kwargs["modified_at"]
+    )
+
+    return model
 
-        model["created-by"] = (
-            kwargs.get("created_by") or _create_model_service_user_response()
-        )
-        model["created-at"] = (
-            kwargs.get("created_at") or datetime.utcnow().isoformat() + "Z",
-        )
-        model["modified-by"] = (
-            _create_model_service_user_response()
-            if "modified_by" not in kwargs
-            else kwargs["modified_by"]
-        )
-        model["modified-at"] = (
-            datetime.utcnow().isoformat() + "Z"
-            if "modified_at" not in kwargs
-            else kwargs["modified_at"]
-        )
 
-        return model
+def _make_signing_key_response(brand_account_id, **kwargs):
+    key = {}
 
+    key["brand_account_id"] = brand_account_id
+    key["name"] = kwargs.get("name") or "test-signing-key"
+    key["fingerprint"] = kwargs.get("fingerprint") or "test-fingerprint"
+    key["sha3-384"] = kwargs.get("sha3_384") or "test-sha3-384"
+
+    key["created-by"] = (
+        kwargs.get("created_by") or _create_model_service_user_response()
+    )
+    key["created-at"] = (
+        kwargs.get("created_at") or datetime.utcnow().isoformat() + "Z",
+    )
+    key["modified-by"] = (
+        _create_model_service_user_response()
+        if "modified_by" not in kwargs
+        else kwargs["modified_by"]
+    )
+    key["modified-at"] = (
+        datetime.utcnow().isoformat() + "Z"
+        if "modified_at" not in kwargs
+        else kwargs["modified_at"]
+    )
+
+    return key
+
+
+def _make_serial_policy_response(model_name, **kwargs):
+    policy = {}
+
+    policy["model-name"] = model_name
+    policy["revision"] = kwargs.get("revision") or 1
+    policy["signing-key-sha3-384"] = (
+        kwargs.get("signing_key_sha3_384") or "test-key-sha3-384"
+    )
+    policy["authority"] = kwargs.get("authority") or "test-authority"
+
+    policy["created-by"] = (
+        kwargs.get("created_by") or _create_model_service_user_response()
+    )
+    policy["created-at"] = (
+        kwargs.get("created_at") or datetime.utcnow().isoformat() + "Z",
+    )
+
+    return policy
+
+
[e-mail]
+def create_model_api(requests_mock, brand_account_id):
     def setup(gw_url):
         create_response = _make_model_response(brand_account_id)
         requests_mock.add(
@@ -258,33 +307,22 @@ def model_service_model_apis(requests_mock, brand_account_id):
 
 
 @pytest.fixture
-def model_service_signing_key_apis(requests_mock, brand_account_id):
-    def _make_signing_key_response(brand_account_id, **kwargs):
-        key = {}
+def update_model_api(requests_mock, brand_account_id):
+    def setup(gw_url, model_name):
+        update_response = _make_model_response(brand_account_id)
+        requests_mock.add(
+            "PATCH",
+            gw_url + f"publishergw/v1/brand/{brand_account_id}/model/{model_name}",
+            json=update_response,
+            status=200,
+        )
+        return (update_response, requests_mock)
 
-        key["brand_account_id"] = brand_account_id
-        key["name"] = kwargs.get("name") or "test-signing-key"
-        key["fingerprint"] = kwargs.get("fingerprint") or "test-fingerprint"
-        key["sha3-384"] = kwargs.get("sha3_384") or "test-sha3-384"
+    return setup
 
-        key["created-by"] = (
-            kwargs.get("created_by") or _create_model_service_user_response()
-        )
-        key["created-at"] = (
-            kwargs.get("created_at") or datetime.utcnow().isoformat() + "Z",
-        )
-        key["modified-by"] = (
-            _create_model_service_user_response()
-            if "modified_by" not in kwargs
-            else kwargs["modified_by"]
-        )
-        key["modified-at"] = (
-            datetime.utcnow().isoformat() + "Z"
-            if "modified_at" not in kwargs
-            else kwargs["modified_at"]
-        )
-        return key
 
[e-mail]
+def create_signing_key_api(requests_mock, brand_account_id):
     def setup(gw_url):
         create_response = _make_signing_key_response(brand_account_id)
         requests_mock.add(
@@ -300,25 +338,7 @@ def model_service_signing_key_apis(requests_mock, brand_account_id):
 
 
 @pytest.fixture
-def model_service_serial_policy_apis(requests_mock, brand_account_id, model_name):
-    def _make_serial_policy_response(model_name, **kwargs):
-        policy = {}
-
-        policy["model-name"] = model_name
-        policy["revision"] = kwargs.get("revision") or 1
-        policy["signing-key-sha3-384"] = (
-            kwargs.get("signing_key_sha3_384") or "test-key-sha3-384"
-        )
-        policy["authority"] = kwargs.get("authority") or "test-authority"
-
-        policy["created-by"] = (
-            kwargs.get("created_by") or _create_model_service_user_response()
-        )
-        policy["created-at"] = (
-            kwargs.get("created_at") or datetime.utcnow().isoformat() + "Z",
-        )
-        return policy
-
+def create_serial_policy_api(requests_mock, brand_account_id, model_name):
     def setup(gw_url):
         create_response = _make_serial_policy_response(model_name)
         requests_mock.add(
diff --git a/store_admin/logic/tests/test_model_service.py b/store_admin/logic/tests/test_model_service.py
index 4496bfd..a07d5d3 100644
--- a/store_admin/logic/tests/test_model_service.py
+++ b/store_admin/logic/tests/test_model_service.py
@@ -7,6 +7,7 @@ from store_admin.logic.model_service import (
     create_model,
     create_signing_key,
     create_serial_policy,
+    update_model,
 )
 from testtools.matchers import (
     ContainsDict,
@@ -23,8 +24,8 @@ class TestModelService:
     gw_url = "http://offline.local/"
 
     @mock.patch.dict(os.environ, {BRAND_ACCOUNT_ID_ENV_KEY: "test-brand-account-id"})
-    def test_set_brand_account_id(self, model_service_model_apis):
-        model_service_model_apis(self.gw_url)
+    def test_set_brand_account_id(self, create_model_api):
+        create_model_api(self.gw_url)
         create_model("test-name", "test-key", "16")
 
         assert (
@@ -63,8 +64,8 @@ class TestModelService:
         )
 
     @mock.patch.dict(os.environ, {BRAND_ACCOUNT_ID_ENV_KEY: "test-brand-account-id"})
-    def test_create_model_success(self, model_service_model_apis, caplog):
-        _, api_mock = model_service_model_apis(self.gw_url)
+    def test_create_model_success(self, create_model_api, caplog):
+        _, api_mock = create_model_api(self.gw_url)
         create_model("test-name", "test-key", "16")
 
         assert len(api_mock.calls) == 1
@@ -76,8 +77,8 @@ class TestModelService:
         assert caplog.messages[-1] == "Model 'test-name' created."
 
     @mock.patch.dict(os.environ, {BRAND_ACCOUNT_ID_ENV_KEY: "test-brand-account-id"})
-    def test_create_signing_key_success(self, model_service_signing_key_apis, caplog):
-        _, api_mock = model_service_signing_key_apis(self.gw_url)
+    def test_create_signing_key_success(self, create_signing_key_api, caplog):
+        _, api_mock = create_signing_key_api(self.gw_url)
         create_signing_key("test-name")
 
         assert len(api_mock.calls) == 1
@@ -91,10 +92,8 @@ class TestModelService:
         assert caplog.messages[-1] == "Signing key 'test-name' created."
 
     @mock.patch.dict(os.environ, {BRAND_ACCOUNT_ID_ENV_KEY: "test-brand-account-id"})
-    def test_create_serial_policy_success(
-        self, model_service_serial_policy_apis, caplog
-    ):
-        _, api_mock = model_service_serial_policy_apis(self.gw_url)
+    def test_create_serial_policy_success(self, create_serial_policy_api, caplog):
+        _, api_mock = create_serial_policy_api(self.gw_url)
         create_serial_policy("test-model-name", "test-sha3-384")
 
         assert len(api_mock.calls) == 1
@@ -106,3 +105,17 @@ class TestModelService:
             "Model 'test-model-name' configured to "
             "sign serials with key 'test-sha3-384'."
         )
+
+    @mock.patch.dict(os.environ, {BRAND_ACCOUNT_ID_ENV_KEY: "test-brand-account-id"})
+    def test_update_model_success(self, update_model_api, caplog):
+        _, api_mock = update_model_api(self.gw_url, "test-model")
+        update_model("test-model", "updated-api-key")
+
+        assert len(api_mock.calls) == 1
+        assert {
+            "api-key": "updated-api-key",
+        } == json.loads(api_mock.calls[-1].request.body.decode())
+        assert (
+            caplog.messages[-1]
+            == "Model 'test-model' updated with new API key 'updated-api-key'."
+        )
diff --git a/store_admin/webservices.py b/store_admin/webservices.py
index a2196cb..b219ecf 100644
--- a/store_admin/webservices.py
+++ b/store_admin/webservices.py
@@ -183,6 +183,25 @@ def model_service_create(store, create_path, json):
     return resp.json()
 
 
+def model_service_update(store, update_path, json):
+    """Send a PATCH update request for model service entities to the proxy URL."""
+    read_url = urllib.parse.urljoin(store.get("gw_url"), update_path)
+    pubgw_admin_token = store.get("pubgw_token")
+    headers = {"Authorization": f"Macaroon {pubgw_admin_token}"}
+
+    resp = requests.patch(read_url, headers=headers, json=json)
+
+    if resp.status_code != 200:
+        if resp.status_code == 401:
+            logger.error(
+                "Authentication error. Please (re-)login to the offline store."
+            )
+        else:
+            _print_error_message("update model service entity", resp)
+        resp.raise_for_status()
+    return resp.json()
+
+
 def _print_error_message(action, response):
     """Print failure messages from other services in a standard way."""
     logger.error("Failed to %s:", action)