From 2f88ba3013d87bf86d8fcd3d49eae255220e4176 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 b475427a..8e9276ee 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 c6994a7c..d16dcd91 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 f8ea457d..e6d92426 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 72a29aa32777bc8651cb64f233856fc082b49cc7 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 e6d92426..0638cca9 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 e54965f8ecf65701c447d29427f002d17f3742dd 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 8e9276ee..215e0ac9 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 b0a9f440cbcb1c99bc954831bd4c1187af7c9e92 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 2976e27d..4ab98e32 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}"