From 1e8e6a7968548227b1bcd753d1673c47f8dfd7dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Soko=C5=82owski?= Date: Mon, 20 Feb 2023 22:21:31 +0100 Subject: [PATCH 1/4] backend: fix use of SSH private key for git ops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Primarily this is supposed to fix the `git pull` aciton triggered by webhooks from GitHub. But in addition to that the point is to simplify that committing wrapper which has far too much in it. Instead of passing everything as CLI arguments one should make use of already supported environment variables and the `env` argument to `subprocess` functions like `run()`. Writing extra logic in the wrapper only makes it unnecessarily complicated. By passing both user, email, and the SSH options in `run_shell_command` we avoid the need to repeat the same boilerplate to provide Git config and SSH credentials. Signed-off-by: Jakub Sokołowski --- .../bin/git_commit_bpmn_models_repo | 44 ++++--------------- .../spiffworkflow_backend/config/default.py | 10 +---- .../services/git_service.py | 37 ++++++---------- 3 files changed, 24 insertions(+), 67 deletions(-) diff --git a/spiffworkflow-backend/bin/git_commit_bpmn_models_repo b/spiffworkflow-backend/bin/git_commit_bpmn_models_repo index b475427a7..8e9276eea 100755 --- a/spiffworkflow-backend/bin/git_commit_bpmn_models_repo +++ b/spiffworkflow-backend/bin/git_commit_bpmn_models_repo @@ -12,17 +12,9 @@ set -o errtrace -o errexit -o nounset -o pipefail bpmn_models_absolute_dir="$1" git_commit_message="$2" git_branch="$3" -git_commit_username="$4" -git_commit_email="$5" -git_commit_password="$6" -if [[ -z "${5:-}" ]]; then - >&2 echo "usage: $(basename "$0") [bpmn_models_absolute_dir] [git_commit_message] [git_branch] [git_commit_username] [git_commit_email]" - exit 1 -fi - -if [[ -z "$git_commit_password" && -z "${SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY:-}" ]]; then - >&2 echo "ERROR: A git password or SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY must be provided" +if [[ -z "${3:-}" ]]; then + >&2 echo "usage: $(basename "${0}") [bpmn_models_absolute_dir] [git_commit_message] [git_branch]" exit 1 fi @@ -32,38 +24,20 @@ function failed_to_get_lock() { } function run() { - cd "$bpmn_models_absolute_dir" + cd "${bpmn_models_absolute_dir}" git add . # https://unix.stackexchange.com/a/155077/456630 if [ -z "$(git status --porcelain)" ]; then echo "No changes to commit" - else - - git config --local user.name "$git_commit_username" - git config --local user.email "$git_commit_email" - - if [[ -n "${SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY:-}" ]]; then - tmpfile=$(mktemp /tmp/tmp_git.XXXXXX) - chmod 600 "$tmpfile" - echo "$SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY" >"$tmpfile" - export GIT_SSH_COMMAND="ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -i ${tmpfile} -F /dev/null" - else - PAT="${git_commit_username}:${git_commit_password}" - AUTH=$(echo -n "$PAT" | openssl base64 | tr -d '\n') - git config --local http.extraHeader "Authorization: Basic $AUTH" - fi - - git commit -m "$git_commit_message" - git push --set-upstream origin "$git_branch" - - if [[ -z "${SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY:-}" ]]; then - git config --unset --local http.extraHeader - fi + return fi + + git commit -m "${git_commit_message}" + git push --set-upstream origin "${git_branch}" } exec {lock_fd}>/var/lock/mylockfile || failed_to_get_lock -flock --timeout 60 "$lock_fd" || failed_to_get_lock +flock --timeout 60 "${lock_fd}" || failed_to_get_lock run -flock -u "$lock_fd" +flock -u "${lock_fd}" diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/config/default.py b/spiffworkflow-backend/src/spiffworkflow_backend/config/default.py index c6994a7cf..d16dcd916 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/config/default.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/default.py @@ -96,9 +96,6 @@ SPIFFWORKFLOW_BACKEND_GIT_PUBLISH_CLONE_URL = environ.get( SPIFFWORKFLOW_BACKEND_GIT_COMMIT_ON_SAVE = ( environ.get("SPIFFWORKFLOW_BACKEND_GIT_COMMIT_ON_SAVE", default="false") == "true" ) -SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY = environ.get( - "SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY" -) SPIFFWORKFLOW_BACKEND_GIT_USERNAME = environ.get("SPIFFWORKFLOW_BACKEND_GIT_USERNAME") SPIFFWORKFLOW_BACKEND_GIT_USER_EMAIL = environ.get( "SPIFFWORKFLOW_BACKEND_GIT_USER_EMAIL" @@ -106,11 +103,8 @@ SPIFFWORKFLOW_BACKEND_GIT_USER_EMAIL = environ.get( SPIFFWORKFLOW_BACKEND_GITHUB_WEBHOOK_SECRET = environ.get( "SPIFFWORKFLOW_BACKEND_GITHUB_WEBHOOK_SECRET", default=None ) -SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY = environ.get( - "SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY", default=None -) -SPIFFWORKFLOW_BACKEND_GIT_USER_PASSWORD = environ.get( - "SPIFFWORKFLOW_BACKEND_GIT_USER_PASSWORD", default=None +SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY_PATH = environ.get( + "SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY_PATH", default=None ) # Database Configuration diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py index f8ea457d3..e6d924264 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py @@ -94,19 +94,7 @@ class GitService: raise ConfigurationError( "SPIFFWORKFLOW_BACKEND_BPMN_SPEC_ABSOLUTE_DIR config must be set" ) - if current_app.config["SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY"]: - os.environ["SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY"] = ( - current_app.config["SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY"] - ) - git_username = "" - git_email = "" - if ( - current_app.config["SPIFFWORKFLOW_BACKEND_GIT_USERNAME"] - and current_app.config["SPIFFWORKFLOW_BACKEND_GIT_USER_EMAIL"] - ): - git_username = current_app.config["SPIFFWORKFLOW_BACKEND_GIT_USERNAME"] - git_email = current_app.config["SPIFFWORKFLOW_BACKEND_GIT_USER_EMAIL"] shell_command_path = os.path.join( current_app.root_path, "..", "..", "bin", "git_commit_bpmn_models_repo" ) @@ -115,9 +103,6 @@ class GitService: repo_path_to_use, message, branch_name_to_use, - git_username, - git_email, - current_app.config["SPIFFWORKFLOW_BACKEND_GIT_USER_PASSWORD"], ] return cls.run_shell_command_to_get_stdout(shell_command) @@ -169,8 +154,17 @@ class GitService: cls, command: list[str], return_success_state: bool = False ) -> Union[subprocess.CompletedProcess[bytes], bool]: """Run_shell_command.""" + env = { + 'GIT_COMMITTER_NAME': current_app.config.get("SPIFFWORKFLOW_BACKEND_GIT_USERNAME", "unknown"), + 'GIT_COMMITTER_EMAIL': current_app.config.get("SPIFFWORKFLOW_BACKEND_GIT_USER_EMAIL", "unknown@example.org"), + } + # SSH authentication can be also provided via gitconfig. + ssh_key_path = current_app.config.get("SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY_PATH") + if ssh_key_path is not None: + env['GIT_SSH_COMMAND'] = 'ssh -F /dev/null -i %s' % ssh_key_path + # this is fine since we pass the commands directly - result = subprocess.run(command, check=False, capture_output=True) # noqa + result = subprocess.run(command, check=False, capture_output=True, env=env) # noqa if return_success_state: return result.returncode == 0 @@ -178,9 +172,9 @@ class GitService: 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} " + f"Failed to execute git command: {command}" + f"Stdout: {stdout}" + f"Stderr: {stderr}" ) return result @@ -252,11 +246,6 @@ class GitService: git_clone_url = current_app.config[ "SPIFFWORKFLOW_BACKEND_GIT_PUBLISH_CLONE_URL" ] - if git_clone_url.startswith("https://"): - git_clone_url = git_clone_url.replace( - "https://", - f"https://{current_app.config['SPIFFWORKFLOW_BACKEND_GIT_USERNAME']}:{current_app.config['SPIFFWORKFLOW_BACKEND_GIT_USER_PASSWORD']}@", - ) cmd = ["git", "clone", git_clone_url, destination_process_root] cls.run_shell_command(cmd) From 1bbf3a264cddb92ff24e5623a8a8aa0e32c90dde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Soko=C5=82owski?= Date: Mon, 20 Feb 2023 22:27:18 +0100 Subject: [PATCH 2/4] backend: specify --rebase when using git pull MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise it fails with: ``` Pulling without specifying how to reconcile divergent branches is discouraged. ``` Signed-off-by: Jakub Sokołowski --- .../src/spiffworkflow_backend/services/git_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py index e6d924264..0638cca9d 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/git_service.py @@ -225,7 +225,7 @@ class GitService: with FileSystemService.cd( current_app.config["SPIFFWORKFLOW_BACKEND_BPMN_SPEC_ABSOLUTE_DIR"] ): - cls.run_shell_command(["git", "pull"]) + cls.run_shell_command(["git", "pull", "--rebase"]) return True @classmethod From 56a913e33ca9834166208bad64145334fa9df638 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Soko=C5=82owski?= Date: Mon, 20 Feb 2023 22:28:35 +0100 Subject: [PATCH 3/4] backend: use sensible lock filename for git MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jakub Sokołowski --- spiffworkflow-backend/bin/git_commit_bpmn_models_repo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spiffworkflow-backend/bin/git_commit_bpmn_models_repo b/spiffworkflow-backend/bin/git_commit_bpmn_models_repo index 8e9276eea..215e0ac9d 100755 --- a/spiffworkflow-backend/bin/git_commit_bpmn_models_repo +++ b/spiffworkflow-backend/bin/git_commit_bpmn_models_repo @@ -37,7 +37,7 @@ function run() { git push --set-upstream origin "${git_branch}" } -exec {lock_fd}>/var/lock/mylockfile || failed_to_get_lock +exec {lock_fd}>/var/lock/spiff-workflow-git-lock || failed_to_get_lock flock --timeout 60 "${lock_fd}" || failed_to_get_lock run flock -u "${lock_fd}" From 819415d487299cef433e678012f2a3d76cd0963b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Soko=C5=82owski?= Date: Tue, 21 Feb 2023 20:02:16 +0100 Subject: [PATCH 4/4] backend: create SSH key file when contents provided MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jakub Sokołowski --- spiffworkflow-backend/bin/boot_server_in_docker | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spiffworkflow-backend/bin/boot_server_in_docker b/spiffworkflow-backend/bin/boot_server_in_docker index 2976e27d8..4ab98e324 100755 --- a/spiffworkflow-backend/bin/boot_server_in_docker +++ b/spiffworkflow-backend/bin/boot_server_in_docker @@ -55,6 +55,14 @@ if [[ "${SPIFFWORKFLOW_BACKEND_RUN_DATA_SETUP:-}" != "false" ]]; then SPIFFWORKFLOW_BACKEND_FAIL_ON_INVALID_PROCESS_MODELS=false poetry run python bin/save_all_bpmn.py fi +if [[ -n "${SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY:-}" ]]; then + if [[ -z "${SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY_PATH}" ]]; then + export SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY_PATH=$(mktemp /tmp/ssh_private_key.XXXXXX) + fi + chmod 600 "${SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY_PATH}" + echo "${SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY}" >"${SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY_PATH}" +fi + # Assure that the the Process Models Directory is initialized as a git repo git init "${SPIFFWORKFLOW_BACKEND_BPMN_SPEC_ABSOLUTE_DIR}" git config --global --add safe.directory "${SPIFFWORKFLOW_BACKEND_BPMN_SPEC_ABSOLUTE_DIR}"