fix(service): use temporary directory to clone project templates on project creation (#3243)

Co-authored-by: Ralf Grubenmann <[email protected]>

Author: Panaetius

renku/core/template/template.py

@@ -403,7 +403,7 @@ def get_latest_reference_and_version(
         else:
             return (self.reference, self.version) if current_version < Version(self.version) else (reference, version)
 
-    def get_template(self, id, reference: Optional[str]) -> Optional["Template"]:
+    def get_template(self, id, reference: Optional[str]) -> "Template":
         """Return all available versions for a template id."""
         try:
             return next(t for t in self.templates if t.id == id)
@@ -491,7 +491,7 @@ def _has_template_at(self, id: str, reference: str) -> bool:
         else:
             return any(t.id == id for t in manifest.templates)
 
-    def get_template(self, id, reference: Optional[str]) -> Optional["Template"]:
+    def get_template(self, id, reference: Optional[str]) -> "Template":
         """Return a template at a specific reference."""
         if reference is not None and reference != self.reference:
             try:

renku/domain_model/template.py

@@ -88,7 +88,7 @@ def get_latest_reference_and_version(
         raise NotImplementedError
 
     @abstractmethod
-    def get_template(self, id, reference: Optional[str]) -> Optional["Template"]:
+    def get_template(self, id, reference: Optional[str]) -> "Template":
         """Return a template at a specific reference."""
         raise NotImplementedError
 

renku/ui/service/controllers/templates_create_project.py

@@ -17,19 +17,19 @@
 # limitations under the License.
 """Renku service template create project controller."""
 import shutil
-from pathlib import Path
-from typing import Dict, Optional
+from typing import Any, Dict, Optional, cast
 
 from marshmallow import EXCLUDE
 
 from renku.command.init import create_from_template_local_command
+from renku.core import errors
+from renku.core.template.template import fetch_templates_source
 from renku.core.util.contexts import renku_project_context
-from renku.domain_model.template import TEMPLATE_MANIFEST, TemplatesManifest
+from renku.domain_model.template import Template
 from renku.infrastructure.repository import Repository
 from renku.ui.service.config import MESSAGE_PREFIX
 from renku.ui.service.controllers.api.abstract import ServiceCtrl
 from renku.ui.service.controllers.api.mixins import RenkuOperationMixin
-from renku.ui.service.controllers.utils.project_clone import user_project_clone
 from renku.ui.service.errors import UserProjectCreationError
 from renku.ui.service.serializers.templates import ProjectTemplateRequest, ProjectTemplateResponseRPC
 from renku.ui.service.utils import new_repo_push
@@ -45,11 +45,14 @@ class TemplatesCreateProjectCtrl(ServiceCtrl, RenkuOperationMixin):
 
     def __init__(self, cache, user_data, request_data):
         """Construct a templates read manifest controller."""
-        self.ctx = TemplatesCreateProjectCtrl.REQUEST_SERIALIZER.load({**user_data, **request_data}, unknown=EXCLUDE)
+        self.ctx = cast(
+            Dict[str, Any],
+            TemplatesCreateProjectCtrl.REQUEST_SERIALIZER.load({**user_data, **request_data}, unknown=EXCLUDE),
+        )
         self.ctx["commit_message"] = f"{MESSAGE_PREFIX} init {self.ctx['project_name']}"
         super(TemplatesCreateProjectCtrl, self).__init__(cache, user_data, request_data)
 
-        self.template: Optional[Dict] = None
+        self.template: Optional[Template] = None
 
     @property
     def context(self):
@@ -59,16 +62,12 @@ def context(self):
     @property
     def default_metadata(self):
         """Default metadata for project creation."""
-        automated_update = True
-        if self.template and "allow_template_update" in self.template:
-            automated_update = self.template["allow_template_update"]
 
         metadata = {
             "__template_source__": self.ctx["git_url"],
             "__template_ref__": self.ctx["ref"],
             "__template_id__": self.ctx["identifier"],
             "__namespace__": self.ctx["project_namespace"],
-            "__automated_update__": automated_update,
             "__repository__": self.ctx["project_repository"],
             "__sanitized_project_name__": self.ctx["project_name_stripped"],
             "__project_slug__": self.ctx["project_slug"],
@@ -114,50 +113,49 @@ def setup_new_project(self):
 
     def setup_template(self):
         """Reads template manifest."""
-        project = user_project_clone(self.user_data, self.ctx)
-        templates = TemplatesManifest.from_path(Path(project.abs_path) / TEMPLATE_MANIFEST).get_raw_content()
+        templates_source = fetch_templates_source(source=self.ctx["git_url"], reference=self.ctx["ref"])
         identifier = self.ctx["identifier"]
-        self.template = next((template for template in templates if template["folder"] == identifier), None)
-        if self.template is None:
+        try:
+            self.template = templates_source.get_template(id=identifier, reference=None)
+        except (errors.InvalidTemplateError, errors.TemplateNotFoundError) as e:
             raise UserProjectCreationError(
                 error_message=f"the template '{identifier}' does not exist in the target template's repository"
-            )
+            ) from e
 
-        repository = Repository(project.abs_path)
+        repository = Repository(templates_source.path)
         self.template_version = repository.head.commit.hexsha
 
         # Verify missing parameters
-        template_parameters = self.template.get("variables", {})
+        template_parameters = set(p.name for p in self.template.parameters)
         provided_parameters = {p["key"]: p["value"] for p in self.ctx["parameters"]}
-        missing_keys = list(template_parameters.keys() - provided_parameters.keys())
+        missing_keys = list(template_parameters - provided_parameters.keys())
         if len(missing_keys) > 0:
             raise UserProjectCreationError(error_message=f"the template requires a value for '${missing_keys[0]}'")
 
-        return project, provided_parameters
+        return provided_parameters
 
     def new_project_push(self, project_path):
         """Push new project to the remote."""
         return new_repo_push(project_path, self.ctx["new_project_url_with_auth"])
 
     def new_project(self):
         """Create new project from template."""
-        template_project, provided_parameters = self.setup_template()
+        provided_parameters = self.setup_template()
+        assert self.template is not None
         new_project = self.setup_new_project()
         new_project_path = new_project.abs_path
 
-        source_path = template_project.abs_path / self.ctx["identifier"]
-
         with renku_project_context(new_project_path):
             create_from_template_local_command().build().execute(
-                source_path,
+                self.template.path,
                 name=self.ctx["project_name"],
                 namespace=self.ctx["project_namespace"],
                 metadata=provided_parameters,
                 default_metadata=self.default_metadata,
                 custom_metadata=self.ctx["project_custom_metadata"],
                 template_version=self.template_version,
-                immutable_template_files=self.template.get("immutable_template_files", []),  # type: ignore[union-attr]
-                automated_template_update=self.template.get("allow_template_update", True),  # type: ignore[union-attr]
+                immutable_template_files=self.template.immutable_files,
+                automated_template_update=self.template.allow_update,
                 user=self.git_user,
                 initial_branch=self.ctx["initial_branch"],
                 commit_message=self.ctx["commit_message"],

tests/fixtures/templates.py

@@ -191,7 +191,7 @@ def get_latest_reference_and_version(
             version = str(max(self._versions))
             return version, version
 
-        def get_template(self, id, reference: Optional[str]) -> Optional[Template]:
+        def get_template(self, id, reference: Optional[str]) -> Template:
             """Return a template at a specific reference."""
             if not reference:
                 reference = self.reference

tests/service/controllers/test_templates_create_project.py

@@ -66,7 +66,6 @@ def test_template_create_project_ctrl(ctrl_init, svc_client_templates_creation,
         "ref",
         "new_project_url_with_auth",
         "url_with_auth",
-        "user_id",
     }
     assert expected_context.issubset(set(ctrl.context.keys()))
 
@@ -76,7 +75,6 @@ def test_template_create_project_ctrl(ctrl_init, svc_client_templates_creation,
         "__template_ref__",
         "__template_id__",
         "__namespace__",
-        "__automated_update__",
         "__repository__",
         "__sanitized_project_name__",
         "__project_slug__",

tests/service/views/test_templates_views.py

@@ -194,7 +194,7 @@ def test_create_project_from_template_failures(svc_client_templates_creation):
     response = svc_client.post("/templates.create_project", data=json.dumps(payload_missing_project), headers=headers)
     assert 200 == response.status_code
     assert {"error"} == set(response.json.keys())
-    assert UserProjectCreationError.code == response.json["error"]["code"]
+    assert UserProjectCreationError.code == response.json["error"]["code"], response.json
     assert "project name" in response.json["error"]["devMessage"].lower()
 
     # NOTE: fail on wrong git url - unexpected when invoked from the UI
@@ -204,7 +204,7 @@ def test_create_project_from_template_failures(svc_client_templates_creation):
     response = svc_client.post("/templates.create_project", data=json.dumps(payload_wrong_repo), headers=headers)
     assert 200 == response.status_code
     assert {"error"} == set(response.json.keys())
-    assert UserProjectCreationError.code == response.json["error"]["code"]
+    assert UserProjectCreationError.code == response.json["error"]["code"], response.json
     assert "git_url" in response.json["error"]["devMessage"]
 
     # NOTE: missing fields -- unlikely to happen. If that is the case, we should determine if it's a user error or not
@@ -225,7 +225,7 @@ def test_create_project_from_template_failures(svc_client_templates_creation):
     response = svc_client.post("/templates.create_project", data=json.dumps(payload_fake_id), headers=headers)
     assert 200 == response.status_code
     assert {"error"} == set(response.json.keys())
-    assert UserProjectCreationError.code == response.json["error"]["code"]
+    assert UserProjectCreationError.code == response.json["error"]["code"], response.json
     assert "does not exist" in response.json["error"]["devMessage"]
     assert fake_identifier in response.json["error"]["devMessage"]
 
@@ -239,6 +239,6 @@ def test_create_project_from_template_failures(svc_client_templates_creation):
 
         assert 200 == response.status_code
         assert {"error"} == set(response.json.keys())
-        assert UserProjectCreationError.code == response.json["error"]["code"]
+        assert UserProjectCreationError.code == response.json["error"]["code"], response.json
         assert "does not exist" in response.json["error"]["devMessage"]
         assert fake_identifier in response.json["error"]["devMessage"]

tests/utils.py

@@ -116,18 +116,18 @@ def modified_environ(*remove, **update):
     """
     env = os.environ
     update = update or {}
-    remove_list = list(remove) or []
+    remove_set = set(remove or [])
 
     # List of environment variables being updated or removed.
-    stomped = (set(update.keys()) | set(remove_list)) & set(env.keys())
+    stomped = (set(update.keys()) | remove_set) & set(env.keys())
     # Environment variables and values to restore on exit.
     update_after = {k: env[k] for k in stomped}
     # Environment variables and values to remove on exit.
     remove_after = frozenset(k for k in update if k not in env)
 
     try:
         env.update(update)
-        [env.pop(k, None) for k in remove_list]
+        [env.pop(k, None) for k in remove_set]
         yield
     finally:
         env.update(update_after)