cleaned up the git service and expanded the api git hook w/ burnettk

spiffworkflow-backend/src/spiffworkflow_backend/api.yml

@@ -494,6 +494,11 @@ paths:
     post:
       operationId: spiffworkflow_backend.routes.process_api_blueprint.github_webhook_receive
       summary: receives push webhooks from github so we can keep our process model repo up to date
+      requestBody:
+        content:
+          application/json:
+            schema:
+              $ref: "#/components/schemas/ProcessModelCategory"
       tags:
         - git
       responses:

spiffworkflow-backend/src/spiffworkflow_backend/config/default.py

@@ -27,8 +27,6 @@ CONNECTOR_PROXY_URL = environ.get(
     "CONNECTOR_PROXY_URL", default="http://localhost:7004"
 )
 
-GIT_COMMIT_ON_SAVE = environ.get("GIT_COMMIT_ON_SAVE", default="false") == "true"
-
 # Open ID server
 OPEN_ID_SERVER_URL = environ.get(
     "OPEN_ID_SERVER_URL", default="http://localhost:7002/realms/spiffworkflow"
@@ -63,7 +61,9 @@ SPIFFWORKFLOW_BACKEND_LOG_LEVEL = environ.get(
 
 # When a user clicks on the `Publish` button, this is the default branch this server merges into.
 # I.e., dev server could have `staging` here. Staging server might have `production` here.
-GIT_MERGE_BRANCH = environ.get("GIT_MERGE_BRANCH", default="staging")
+GIT_BRANCH_TO_PUBLISH_TO = environ.get("GIT_BRANCH_TO_PUBLISH_TO", default="staging")
+GIT_CLONE_URL_FOR_PUBLISHING = environ.get("GIT_CLONE_URL", default=None)
+GIT_COMMIT_ON_SAVE = environ.get("GIT_COMMIT_ON_SAVE", default="false") == "true"
 
 # Datbase Configuration
 SPIFF_DATABASE_TYPE = environ.get(

spiffworkflow-backend/src/spiffworkflow_backend/config/dev.py

@@ -1,7 +1,7 @@
 """Dev."""
 from os import environ
 
-GIT_MERGE_BRANCH = environ.get("GIT_MERGE_BRANCH", default="staging")
+GIT_BRANCH_TO_PUBLISH_TO = environ.get("GIT_BRANCH_TO_PUBLISH_TO", default="staging")
 GIT_USERNAME = environ.get("GIT_USERNAME", default="sartography-automated-committer")
 GIT_USER_EMAIL = environ.get(
     "GIT_USER_EMAIL", default="sartography-automated-committer@users.noreply.github.com"

spiffworkflow-backend/src/spiffworkflow_backend/config/development.py

@@ -12,3 +12,8 @@ SPIFFWORKFLOW_BACKEND_LOG_LEVEL = environ.get(
 RUN_BACKGROUND_SCHEDULER = (
     environ.get("RUN_BACKGROUND_SCHEDULER", default="true") == "true"
 )
+GIT_CLONE_URL_FOR_PUBLISHING = environ.get(
+    "GIT_CLONE_URL", default="https://github.com/sartography/sample-process-models.git"
+)
+GIT_USERNAME = "sartography-automated-committer"
+GIT_USER_EMAIL = f"{GIT_USERNAME}@users.noreply.github.com"

spiffworkflow-backend/src/spiffworkflow_backend/config/terraform_deployed_environment.py

@@ -5,8 +5,8 @@ from os import environ
 environment_identifier_for_this_config_file_only = environ["SPIFFWORKFLOW_BACKEND_ENV"]
 
 GIT_COMMIT_ON_SAVE = True
-GIT_USERNAME = environment_identifier_for_this_config_file_only
+GIT_USERNAME = "sartography-automated-committer"
-GIT_USER_EMAIL = f"{environment_identifier_for_this_config_file_only}@example.com"
+GIT_USER_EMAIL = f"{GIT_USERNAME}@users.noreply.github.com"
 SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_NAME = environ.get(
     "SPIFFWORKFLOW_BACKEND_PERMISSIONS_FILE_NAME",
     default="terraform_deployed_environment.yml",
@@ -24,3 +24,6 @@ SPIFFWORKFLOW_BACKEND_URL = (
     f"https://api.{environment_identifier_for_this_config_file_only}.spiffworkflow.org"
 )
 CONNECTOR_PROXY_URL = f"https://connector-proxy.{environment_identifier_for_this_config_file_only}.spiffworkflow.org"
+GIT_CLONE_URL_FOR_PUBLISHING = environ.get(
+    "GIT_CLONE_URL", default="https://github.com/sartography/sample-process-models.git"
+)

spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance.py

@@ -93,7 +93,7 @@ class ProcessInstanceModel(SpiffworkflowBaseDBModel):
     created_at_in_seconds: int = db.Column(db.Integer)
     status: str = db.Column(db.String(50))
 
-    bpmn_xml_file_contents: bytes | None = None
+    bpmn_xml_file_contents: str | None = None
     bpmn_version_control_type: str = db.Column(db.String(50))
     bpmn_version_control_identifier: str = db.Column(db.String(255))
     spiff_step: int = db.Column(db.Integer)
@@ -101,9 +101,6 @@ class ProcessInstanceModel(SpiffworkflowBaseDBModel):
     @property
     def serialized(self) -> dict[str, Any]:
         """Return object data in serializeable format."""
-        local_bpmn_xml_file_contents = ""
-        if self.bpmn_xml_file_contents:
-            local_bpmn_xml_file_contents = self.bpmn_xml_file_contents.decode("utf-8")
         return {
             "id": self.id,
             "process_model_identifier": self.process_model_identifier,
@@ -112,7 +109,7 @@ class ProcessInstanceModel(SpiffworkflowBaseDBModel):
             "start_in_seconds": self.start_in_seconds,
             "end_in_seconds": self.end_in_seconds,
             "process_initiator_id": self.process_initiator_id,
-            "bpmn_xml_file_contents": local_bpmn_xml_file_contents,
+            "bpmn_xml_file_contents": self.bpmn_xml_file_contents,
             "bpmn_version_control_identifier": self.bpmn_version_control_identifier,
             "bpmn_version_control_type": self.bpmn_version_control_type,
             "spiff_step": self.spiff_step,

spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py

@@ -375,7 +375,7 @@ def process_model_publish(
 ) -> flask.wrappers.Response:
     """Process_model_publish."""
     if branch_to_update is None:
-        branch_to_update = current_app.config["GIT_MERGE_BRANCH"]
+        branch_to_update = current_app.config["GIT_BRANCH_TO_PUBLISH_TO"]
     process_model_identifier = un_modify_modified_process_model_id(
         modified_process_model_identifier
     )
@@ -1817,13 +1817,13 @@ def get_spiff_task_from_process_instance(
 
 
 # sample body:
-# {'ref': 'refs/heads/main', 'repository': {'name': 'sample-process-models',
+# {"ref": "refs/heads/main", "repository": {"name": "sample-process-models",
-# 'full_name': 'sartography/sample-process-models', 'private': False .... }}
+# "full_name": "sartography/sample-process-models", "private": False .... }}
 # test with: ngrok http 7000
 # where 7000 is the port the app is running on locally
-def github_webhook_receive(body: dict) -> Response:
+def github_webhook_receive(body: Dict) -> Response:
     """Github_webhook_receive."""
-    print(f"body: {body}")
+    GitService.handle_web_hook(body)
     return Response(json.dumps({"ok": True}), status=200, mimetype="application/json")
 
 

spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py

@@ -1,54 +1,74 @@
 """Git_service."""
 import os
 import shutil
+import subprocess  # noqa we need the subprocess module to safely run the git commands
 import uuid
 from typing import Optional
+from typing import Union
 
 from flask import current_app
 from flask import g
 
+from spiffworkflow_backend.config import ConfigurationError
 from spiffworkflow_backend.models.process_model import ProcessModelInfo
 from spiffworkflow_backend.services.file_system_service import FileSystemService
 
 
+class MissingGitConfigsError(Exception):
+    """MissingGitConfigsError."""
+
+
+class InvalidGitWebhookBodyError(Exception):
+    """InvalidGitWebhookBodyError."""
+
+
+class GitCloneUrlMismatchError(Exception):
+    """GitCloneUrlMismatchError."""
+
+
+class GitCommandError(Exception):
+    """GitCommandError."""
+
+
+# TOOD: check for the existence of git and configs on bootup if publishing is enabled
 class GitService:
     """GitService."""
 
-    @staticmethod
+    @classmethod
-    def get_current_revision() -> str:
+    def get_current_revision(cls) -> str:
         """Get_current_revision."""
         bpmn_spec_absolute_dir = current_app.config["BPMN_SPEC_ABSOLUTE_DIR"]
         # The value includes a carriage return character at the end, so we don't grab the last character
-        current_git_revision = os.popen(  # noqa: S605
+        with FileSystemService.cd(bpmn_spec_absolute_dir):
-            f"cd {bpmn_spec_absolute_dir} && git rev-parse --short HEAD"
+            return cls.run_shell_command_to_get_stdout(
-        ).read()[
+                ["git", "rev-parse", "--short", "HEAD"]
-            :-1
+            )
-        ]  # noqa: S605
-        return current_git_revision
 
-    @staticmethod
+    @classmethod
     def get_instance_file_contents_for_revision(
-        process_model: ProcessModelInfo, revision: str
+        cls, process_model: ProcessModelInfo, revision: str
-    ) -> bytes:
+    ) -> str:
         """Get_instance_file_contents_for_revision."""
         bpmn_spec_absolute_dir = current_app.config["BPMN_SPEC_ABSOLUTE_DIR"]
         process_model_relative_path = FileSystemService.process_model_relative_path(
             process_model
         )
-        shell_cd_command = f"cd {bpmn_spec_absolute_dir}"
+        with FileSystemService.cd(bpmn_spec_absolute_dir):
-        shell_git_command = f"git show {revision}:{process_model_relative_path}/{process_model.primary_file_name}"
+            shell_command = [
-        shell_command = f"{shell_cd_command} && {shell_git_command}"
+                "git",
-        # git show 78ae5eb:category_number_one/script-task/script-task.bpmn
+                "show",
-        file_contents: str = os.popen(shell_command).read()[:-1]  # noqa: S605
+                f"{revision}:{process_model_relative_path}/{process_model.primary_file_name}",
-        assert file_contents  # noqa: S101
+            ]
-        return file_contents.encode("utf-8")
+            return cls.run_shell_command_to_get_stdout(shell_command)
 
-    @staticmethod
+    @classmethod
-    def commit(message: str, repo_path: Optional[str] = None) -> str:
+    def commit(cls, message: str, repo_path: Optional[str] = None) -> str:
         """Commit."""
         repo_path_to_use = repo_path
         if repo_path is None:
             repo_path_to_use = current_app.config["BPMN_SPEC_ABSOLUTE_DIR"]
+        if repo_path_to_use is None:
+            raise ConfigurationError("BPMN_SPEC_ABSOLUTE_DIR config must be set")
 
         git_username = ""
         git_email = ""
@@ -58,13 +78,90 @@ class GitService:
         shell_command_path = os.path.join(
             current_app.root_path, "..", "..", "bin", "git_commit_bpmn_models_repo"
         )
-        shell_command = f"{shell_command_path} '{repo_path_to_use}' '{message}' '{git_username}' '{git_email}'"
+        shell_command = [
-        output = os.popen(shell_command).read()  # noqa: S605
+            shell_command_path,
-        return output
+            repo_path_to_use,
+            message,
+            git_username,
+            git_email,
+        ]
+        return cls.run_shell_command_to_get_stdout(shell_command)
+
+    @classmethod
+    def check_for_configs(cls) -> None:
+        """Check_for_configs."""
+        if current_app.config["GIT_BRANCH_TO_PUBLISH_TO"] is None:
+            raise MissingGitConfigsError(
+                "Missing config for GIT_BRANCH_TO_PUBLISH_TO. "
+                "This is required for publishing process models"
+            )
+        if current_app.config["GIT_CLONE_URL_FOR_PUBLISHING"] is None:
+            raise MissingGitConfigsError(
+                "Missing config for GIT_CLONE_URL_FOR_PUBLISHING. "
+                "This is required for publishing process models"
+            )
+
+    @classmethod
+    def run_shell_command_as_boolean(cls, command: list[str]) -> bool:
+        """Run_shell_command_as_boolean."""
+        # we know result will be a bool here
+        result: bool = cls.run_shell_command(command, return_success_state=True)  # type: ignore
+        return result
+
+    @classmethod
+    def run_shell_command_to_get_stdout(cls, command: list[str]) -> str:
+        """Run_shell_command_to_get_stdout."""
+        # we know result will be a CompletedProcess here
+        result: subprocess.CompletedProcess[bytes] = cls.run_shell_command(
+            command, return_success_state=False
+        )  # type: ignore
+        return result.stdout.decode("utf-8")
+
+    @classmethod
+    def run_shell_command(
+        cls, command: list[str], return_success_state: bool = False
+    ) -> Union[subprocess.CompletedProcess[bytes], bool]:
+        """Run_shell_command."""
+        # this is fine since we pass the commands directly
+        result = subprocess.run(command, check=False, capture_output=True)  # noqa
+        if return_success_state:
+            return result.returncode == 0
+
+        if result.returncode != 0:
+            stdout = result.stdout.decode("utf-8")
+            stderr = result.stderr.decode("utf-8")
+            raise GitCommandError(
+                f"Failed to execute git command: {command} "
+                f"Stdout: {stdout} "
+                f"Stderr: {stderr} "
+            )
+
+        return result
+
+    # only supports github right now
+    @classmethod
+    def handle_web_hook(cls, webhook: dict) -> None:
+        """Handle_web_hook."""
+        cls.check_for_configs()
+
+        if "repository" not in webhook or "clone_url" not in webhook["repository"]:
+            raise InvalidGitWebhookBodyError(
+                f"Cannot find required keys of 'repository:clone_url' from webhook body: {webhook}"
+            )
+
+        clone_url = webhook["repository"]["clone_url"]
+        if clone_url != current_app.config["GIT_CLONE_URL_FOR_PUBLISHING"]:
+            raise GitCloneUrlMismatchError(
+                f"Configured clone url does not match clone url from webhook: {clone_url}"
+            )
+
+        with FileSystemService.cd(current_app.config["BPMN_SPEC_ABSOLUTE_DIR"]):
+            cls.run_shell_command(["git", "pull"])
 
     @classmethod
     def publish(cls, process_model_id: str, branch_to_update: str) -> str:
         """Publish."""
+        cls.check_for_configs()
         source_process_model_root = FileSystemService.root_path()
         source_process_model_path = os.path.join(
             source_process_model_root, process_model_id
@@ -76,21 +173,29 @@ class GitService:
         # we are adding a guid to this so the flake8 issue has been mitigated
         destination_process_root = f"/tmp/{clone_dir}"  # noqa
 
-        cmd = (
+        git_clone_url = current_app.config["GIT_CLONE_URL_FOR_PUBLISHING"].replace(
-            f"git clone https://{current_app.config['GIT_USERNAME']}:{current_app.config['GIT_USER_PASSWORD']}"
+            "https://",
-            f"@github.com/sartography/sample-process-models.git {destination_process_root}"
+            f"https://{current_app.config['GIT_USERNAME']}:{current_app.config['GIT_USER_PASSWORD']}@",
         )
-        os.system(cmd)  # noqa: S605
+        cmd = ["git", "clone", git_clone_url, destination_process_root]
+
+        cls.run_shell_command(cmd)
         with FileSystemService.cd(destination_process_root):
             # create publish branch from branch_to_update
-            os.system(f"git checkout {branch_to_update}")  # noqa: S605
+            cls.run_shell_command(["git", "checkout", branch_to_update])
-            publish_branch = f"publish-{process_model_id}"
+            branch_to_pull_request = f"publish-{process_model_id}"
-            command = f"git show-ref --verify refs/remotes/origin/{publish_branch}"
+
-            output = os.popen(command).read()  # noqa: S605
+            # check if branch exists and checkout appropriately
-            if output:
+            command = [
-                os.system(f"git checkout {publish_branch}")  # noqa: S605
+                "git",
+                "show-ref",
+                "--verify",
+                f"refs/remotes/origin/{branch_to_pull_request}",
+            ]
+            if cls.run_shell_command_as_boolean(command):
+                cls.run_shell_command(["git", "checkout", branch_to_pull_request])
             else:
-                os.system(f"git checkout -b {publish_branch}")  # noqa: S605
+                cls.run_shell_command(["git", "checkout", "-b", branch_to_pull_request])
 
             # copy files from process model into the new publish branch
             destination_process_model_path = os.path.join(
@@ -100,15 +205,20 @@ class GitService:
                 shutil.rmtree(destination_process_model_path)
             shutil.copytree(source_process_model_path, destination_process_model_path)
 
-            # add and commit files to publish_branch, then push
+            # add and commit files to branch_to_pull_request, then push
-            commit_message = f"Request to publish changes to {process_model_id}, from {g.user.username}"
+            commit_message = (
+                f"Request to publish changes to {process_model_id}, "
+                f"from {g.user.username} on {current_app.config['ENV_IDENTIFIER']}"
+            )
             cls.commit(commit_message, destination_process_root)
-            os.system("git push")  # noqa
+            cls.run_shell_command(["git", "push"])
 
             # build url for github page to open PR
-            output = os.popen("git config --get remote.origin.url").read()  # noqa
+            git_remote = cls.run_shell_command_to_get_stdout(
-            remote_url = output.strip().replace(".git", "")
+                ["git", "config", "--get", "remote.origin.url"]
-            pr_url = f"{remote_url}/compare/{publish_branch}?expand=1"
+            )
+            remote_url = git_remote.strip().replace(".git", "")
+            pr_url = f"{remote_url}/compare/{branch_to_update}...{branch_to_pull_request}?expand=1"
 
         # try to clean up
         if os.path.exists(destination_process_root):