From bfde1c7d79a6c81749424d0c7e46ea5854f4d081 Mon Sep 17 00:00:00 2001 From: Matthias Van Woensel Date: Thu, 13 Feb 2020 16:50:32 +0100 Subject: [PATCH 01/16] initial pre-commit test --- resources/hooks/pre-commit | 89 +++++++++++++++++++++++ src/Command/Project/ProjectGetCommand.php | 4 + src/Service/Git.php | 7 ++ 3 files changed, 100 insertions(+) create mode 100755 resources/hooks/pre-commit diff --git a/resources/hooks/pre-commit b/resources/hooks/pre-commit new file mode 100755 index 000000000..e653c2303 --- /dev/null +++ b/resources/hooks/pre-commit @@ -0,0 +1,89 @@ +#!/bin/sh +# +# The hook should exit with non-zero status +# after issuing an appropriate message if it wants to stop the commit. +# + +C_NO='\033[0m' # No Color +C_RED='\033[0;31m' +C_BRIGHT_RED='\033[1;31m' +C_GREEN='\033[0;32m' +C_BRIGHT_GREEN='\033[1;32m' +C_BLUE='\033[0;34m' +C_BRIGHT_BLUE='\033[1;34m' +C_GRAY='\033[0;37m' +C_WHITE='\033[1;37m' + +if git rev-parse --verify HEAD >/dev/null 2>&1 +then + against=HEAD +else + # Initial commit: diff against an empty tree object + against=$(git hash-object -t tree /dev/null) +fi + +if test $(git config -l | grep hooks.psh_allow_container_rename | wc -l) != 0 +then + psh_allow_container_rename=$(git config --bool hooks.psh_allow_container_rename) +else + psh_allow_container_rename="false" +fi + +# Redirect output to stderr. +exec 1>&2 + +cmd_to_test="git diff --cached --diff-filter=M -z $against --color --word-diff=plain" + +# Prevent changing the name of containers to avoid dataloss +if [ "$psh_allow_container_rename" != "true" ] && + test $($cmd_to_test | grep 'name' | wc -c) != 0 +then + relevant_changes=$($cmd_to_test | grep 'name') + + echo "**************************************************" + echo "* ${C_BRIGHT_RED}Error${C_NO}: Attempt to rename a container detected! *" + echo "**************************************************" + echo "" + echo "Changing the name of your application after it has been deployed will destroy all storage volumes and result in the loss of all persistent data. This is typically a Very Bad Thing to do. It could be useful under certain circumstances in the early stages of development but you almost certainly don't want to change it on a live project." + echo "For more information see: https://docs.platform.sh/configuration/app/name.html" + echo "" + echo "-----------------------------" + echo "| Relevant change |" + echo "-----------------------------" + echo "" + echo "$relevant_changes" + echo "" + echo "-----------------------------" + echo "" + echo "If you know what you are doing you can permanently disable this check using:" + echo "" + echo " ${C_GRAY}git config hooks.psh_allow_container_rename true${C_NO}" + echo "" + + + exec < /dev/tty + + #allow the user to commit anyway + while true; do + read -p "Proceed anyway? (y/n) " yn + if [ "$yn" = "" ]; then + yn='n' + fi + echo "" + case $yn in + [Yy] ) exit 0;; + [Nn] ) echo "Commit cancelled."; exit 1;; + * ) echo "Please answer y for yes or n for no.";; + esac + done +fi + + + + + + + + + +exec git diff-index --check --cached $against -- diff --git a/src/Command/Project/ProjectGetCommand.php b/src/Command/Project/ProjectGetCommand.php index 30731b96f..860a4db49 100644 --- a/src/Command/Project/ProjectGetCommand.php +++ b/src/Command/Project/ProjectGetCommand.php @@ -198,6 +198,10 @@ protected function execute(InputInterface $input, OutputInterface $output) $projectRootRelative )); + if ($this->getOption('init-hooks')) { + $git->createPreCommitHook($projectRoot); + } + // Return early if there is no code in the repository. if (!glob($projectRoot . '/*', GLOB_NOSORT)) { return 0; diff --git a/src/Service/Git.php b/src/Service/Git.php index 2f2972fda..411bff71d 100644 --- a/src/Service/Git.php +++ b/src/Service/Git.php @@ -431,6 +431,13 @@ public function cloneRepo($url, $destination = null, array $args = [], $mustRun return (bool) $this->execute($args, false, $mustRun, false); } + public function createPreCommitHook() { + $root = $this->getRoot(); + if(file_exists($root.'/hooks/pre-commit')) { + throw new \RuntimeException('pre-commit already exists'); + } + + } /** * Find the root directory of a Git repository. * From 9b50db4c413a99201d8af3658a591ff5fbc7befd Mon Sep 17 00:00:00 2001 From: Matthias Van Woensel Date: Fri, 14 Feb 2020 12:01:10 +0100 Subject: [PATCH 02/16] Cleaning up and adding the type check --- resources/hooks/pre-commit | 130 +++++++++++++++++++++++++++---------- 1 file changed, 95 insertions(+), 35 deletions(-) diff --git a/resources/hooks/pre-commit b/resources/hooks/pre-commit index e653c2303..b4c631a11 100755 --- a/resources/hooks/pre-commit +++ b/resources/hooks/pre-commit @@ -13,54 +13,30 @@ C_BLUE='\033[0;34m' C_BRIGHT_BLUE='\033[1;34m' C_GRAY='\033[0;37m' C_WHITE='\033[1;37m' +C_BOLD='\033[0;1;4m' -if git rev-parse --verify HEAD >/dev/null 2>&1 -then - against=HEAD -else - # Initial commit: diff against an empty tree object - against=$(git hash-object -t tree /dev/null) -fi - -if test $(git config -l | grep hooks.psh_allow_container_rename | wc -l) != 0 -then - psh_allow_container_rename=$(git config --bool hooks.psh_allow_container_rename) -else - psh_allow_container_rename="false" -fi - -# Redirect output to stderr. -exec 1>&2 - -cmd_to_test="git diff --cached --diff-filter=M -z $against --color --word-diff=plain" - -# Prevent changing the name of containers to avoid dataloss -if [ "$psh_allow_container_rename" != "true" ] && - test $($cmd_to_test | grep 'name' | wc -c) != 0 -then - relevant_changes=$($cmd_to_test | grep 'name') - - echo "**************************************************" - echo "* ${C_BRIGHT_RED}Error${C_NO}: Attempt to rename a container detected! *" - echo "**************************************************" - echo "" - echo "Changing the name of your application after it has been deployed will destroy all storage volumes and result in the loss of all persistent data. This is typically a Very Bad Thing to do. It could be useful under certain circumstances in the early stages of development but you almost certainly don't want to change it on a live project." - echo "For more information see: https://docs.platform.sh/configuration/app/name.html" +print_changes() { + relevant_changes=$1 echo "" echo "-----------------------------" - echo "| Relevant change |" + echo "| Relevant change(s) |" echo "-----------------------------" echo "" echo "$relevant_changes" echo "" echo "-----------------------------" echo "" +} + +print_permanently_disable_notice() { + hook_config_name=$1 echo "If you know what you are doing you can permanently disable this check using:" echo "" - echo " ${C_GRAY}git config hooks.psh_allow_container_rename true${C_NO}" + echo " ${C_GRAY}git config hooks.$hook_config_name true${C_NO}" echo "" +} - +confirm_or_exit(){ exec < /dev/tty #allow the user to commit anyway @@ -76,6 +52,24 @@ then * ) echo "Please answer y for yes or n for no.";; esac done +} + +get_config() { + hook_config_name=$1 + if test $(git config -l | grep hooks.$hook_config_name | wc -l) != 0 + then + git config --bool hooks.$hook_config_name | cat + else + echo "false" + fi +} + +if git rev-parse --verify HEAD >/dev/null 2>&1 +then + against=HEAD +else + # Initial commit: diff against an empty tree object + against=$(git hash-object -t tree /dev/null) fi @@ -83,6 +77,72 @@ fi +# Redirect output to stderr. +exec 1>&2 + +cmd_to_test="git diff --diff-filter=M -z $against --word-diff=plain" + + + + + +echo "Checking name change of containers..." +hook_config_name="psh_allow_container_rename" +config_value=$(get_config $hook_config_name) +if [ "$config_value" != "true" ] && + test $($cmd_to_test --no-color | grep -E 'name\:|\[.+\:.+\}$' | wc -c) != 0 +then + relevant_changes="$($cmd_to_test --color | grep 'name:')\n $($cmd_to_test | grep -E '\[.+\:.+\}$')" + echo "**************************************************" + echo "* ${C_BRIGHT_RED}Error${C_NO}: Attempt to rename a container detected! *" + echo "**************************************************" + echo "" + echo "Changing the name of your application after it has been deployed will destroy all storage volumes and result in the loss of all persistent data. This is typically a Very Bad Thing to do. It could be useful under certain circumstances in the early stages of development but you almost certainly don't want to change it on a live project." + echo "For more information see: https://docs.platform.sh/configuration/app/name.html" + print_changes "$relevant_changes" + print_permanently_disable_notice $hook_config_name + + confirm_or_exit +else + echo "OK" +fi + + + + + + +echo "Checking service type changes..." +hook_config_name="psh_allow_service_type_change"; +config_value=$(get_config $hook_config_name) +if [ "$config_value" != "true" ] && + test $($cmd_to_test | grep 'type:' | wc -c) != 0 +then + relevant_changes=$($cmd_to_test | grep 'type:') + + echo "**************************************************" + echo "* ${C_BRIGHT_RED}Warning${C_NO}: Change of service type detected! *" + echo "**************************************************" + echo "" + echo "Persistent services can ${C_BOLD}not${C_NO} be upgraded (e.g. MySQL v10.3 -> MySQL v10.2). Only non-persistent containers like chrome-headless, redis, memcached can be downgraded." + echo "" + echo "Please verify your changes before proceeding:" + echo "- ${C_BOLD}Downgrading${C_NO} to an older version will break your service and has the potential to cause dataloss." + echo "- ${C_BOLD}Upgrading${C_NO} to a newer version should work flawlessly(*). But please do verify that this is working correctly for your application by branching your production/master environment first." + echo "${C_BOLD}Downgrading again later is not possible!${C_NO}" + echo "" + echo "${C_GRAY}* There are limitations regarding which service supports big version jumps while keeping the data (e.g.: Elasticsearch 1.7 -> 6.5). These are upstream limitations not specific to platform.sh. Check the documentation relevant to your service.${C_NO}" + print_changes "$relevant_changes" + print_permanently_disable_notice $hook_config_name + + confirm_or_exit +else + echo "OK" +fi + + + + From 455894c7ae8ee05d9f5236c4786a56c54e77d0b1 Mon Sep 17 00:00:00 2001 From: Matthias Van Woensel Date: Fri, 14 Feb 2020 12:09:02 +0100 Subject: [PATCH 03/16] adding failed and ok print messages --- resources/hooks/pre-commit | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/resources/hooks/pre-commit b/resources/hooks/pre-commit index b4c631a11..7bf6c7b18 100755 --- a/resources/hooks/pre-commit +++ b/resources/hooks/pre-commit @@ -36,6 +36,14 @@ print_permanently_disable_notice() { echo "" } +print_ok() { + echo " [${C_BRIGHT_GREEN}OK${C_NO}]" +} + +print_fail() { + echo " [${C_BRIGHT_RED}FAILED${C_NO}]" +} + confirm_or_exit(){ exec < /dev/tty @@ -64,13 +72,15 @@ get_config() { fi } -if git rev-parse --verify HEAD >/dev/null 2>&1 -then - against=HEAD -else - # Initial commit: diff against an empty tree object - against=$(git hash-object -t tree /dev/null) -fi +get_git_head() { + if git rev-parse --verify HEAD >/dev/null 2>&1 + then + echo "HEAD" + else + git hash-object -t tree /dev/null + fi +} + @@ -80,18 +90,21 @@ fi # Redirect output to stderr. exec 1>&2 + +against=$(get_git_head) cmd_to_test="git diff --diff-filter=M -z $against --word-diff=plain" -echo "Checking name change of containers..." +echo -n "Checking name change of containers..." hook_config_name="psh_allow_container_rename" config_value=$(get_config $hook_config_name) if [ "$config_value" != "true" ] && test $($cmd_to_test --no-color | grep -E 'name\:|\[.+\:.+\}$' | wc -c) != 0 then + print_fail relevant_changes="$($cmd_to_test --color | grep 'name:')\n $($cmd_to_test | grep -E '\[.+\:.+\}$')" echo "**************************************************" echo "* ${C_BRIGHT_RED}Error${C_NO}: Attempt to rename a container detected! *" @@ -104,7 +117,7 @@ then confirm_or_exit else - echo "OK" + print_ok fi @@ -112,12 +125,13 @@ fi -echo "Checking service type changes..." +echo -n "Checking service type changes..." hook_config_name="psh_allow_service_type_change"; config_value=$(get_config $hook_config_name) if [ "$config_value" != "true" ] && test $($cmd_to_test | grep 'type:' | wc -c) != 0 then + print_fail relevant_changes=$($cmd_to_test | grep 'type:') echo "**************************************************" @@ -137,7 +151,7 @@ then confirm_or_exit else - echo "OK" + print_ok fi From ad1d823b9c5bd60d52d7a9b6eaec06b6e22c69e4 Mon Sep 17 00:00:00 2001 From: Matthias Van Woensel Date: Fri, 14 Feb 2020 12:52:34 +0100 Subject: [PATCH 04/16] less is more --- resources/hooks/pre-commit | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/resources/hooks/pre-commit b/resources/hooks/pre-commit index 7bf6c7b18..3d1ccab90 100755 --- a/resources/hooks/pre-commit +++ b/resources/hooks/pre-commit @@ -105,14 +105,14 @@ if [ "$config_value" != "true" ] && test $($cmd_to_test --no-color | grep -E 'name\:|\[.+\:.+\}$' | wc -c) != 0 then print_fail - relevant_changes="$($cmd_to_test --color | grep 'name:')\n $($cmd_to_test | grep -E '\[.+\:.+\}$')" + echo "**************************************************" echo "* ${C_BRIGHT_RED}Error${C_NO}: Attempt to rename a container detected! *" echo "**************************************************" echo "" echo "Changing the name of your application after it has been deployed will destroy all storage volumes and result in the loss of all persistent data. This is typically a Very Bad Thing to do. It could be useful under certain circumstances in the early stages of development but you almost certainly don't want to change it on a live project." echo "For more information see: https://docs.platform.sh/configuration/app/name.html" - print_changes "$relevant_changes" + print_changes "$($cmd_to_test --color | grep 'name:')\n $($cmd_to_test | grep -E '\[.+\:.+\}$')" print_permanently_disable_notice $hook_config_name confirm_or_exit @@ -132,7 +132,6 @@ if [ "$config_value" != "true" ] && test $($cmd_to_test | grep 'type:' | wc -c) != 0 then print_fail - relevant_changes=$($cmd_to_test | grep 'type:') echo "**************************************************" echo "* ${C_BRIGHT_RED}Warning${C_NO}: Change of service type detected! *" @@ -146,7 +145,7 @@ then echo "${C_BOLD}Downgrading again later is not possible!${C_NO}" echo "" echo "${C_GRAY}* There are limitations regarding which service supports big version jumps while keeping the data (e.g.: Elasticsearch 1.7 -> 6.5). These are upstream limitations not specific to platform.sh. Check the documentation relevant to your service.${C_NO}" - print_changes "$relevant_changes" + print_changes "$($cmd_to_test | grep 'type:')" print_permanently_disable_notice $hook_config_name confirm_or_exit From 774c343f9b60e176deaf69a412c0ff4692ae5592 Mon Sep 17 00:00:00 2001 From: Matthias Van Woensel Date: Fri, 14 Feb 2020 17:27:31 +0100 Subject: [PATCH 05/16] Adding the calls to make it copy the resource to the correct folder --- src/Command/Project/ProjectGetCommand.php | 16 ++++++++++++---- src/Service/Git.php | 21 +++++++++++++++++---- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/Command/Project/ProjectGetCommand.php b/src/Command/Project/ProjectGetCommand.php index 860a4db49..f38d89209 100644 --- a/src/Command/Project/ProjectGetCommand.php +++ b/src/Command/Project/ProjectGetCommand.php @@ -12,7 +12,7 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; - +use Symfony\Component\Console\Question\ConfirmationQuestion; class ProjectGetCommand extends CommandBase { protected function configure() @@ -25,7 +25,8 @@ protected function configure() ->addArgument('directory', InputArgument::OPTIONAL, 'The directory to clone to. Defaults to the project title') ->addOption('environment', 'e', InputOption::VALUE_REQUIRED, "The environment ID to clone. Defaults to 'master' or the first available environment") ->addOption('depth', null, InputOption::VALUE_REQUIRED, 'Create a shallow clone: limit the number of commits in the history') - ->addOption('build', null, InputOption::VALUE_NONE, 'Build the project after cloning'); + ->addOption('build', null, InputOption::VALUE_NONE, 'Build the project after cloning') + ->addOption('no-create-hooks', null, InputOption::VALUE_NONE, 'Do not create hooks in your git repository to help you avoid common mistakes'); $this->addProjectOption(); Ssh::configureInput($this->getDefinition()); $this->addExample('Clone the project "abc123" into the directory "my-project"', 'abc123 my-project'); @@ -198,8 +199,15 @@ protected function execute(InputInterface $input, OutputInterface $output) $projectRootRelative )); - if ($this->getOption('init-hooks')) { - $git->createPreCommitHook($projectRoot); + if (!$input->getOption('no-create-hooks')) { + $this->debug('Creating hooks'); + + $helper = $this->getHelper('question'); + $question = new ConfirmationQuestion('It looks like you already have a pre-commit hook, do you want to overwrite your existing pre-commit hook with ours?', false); + + if(!$git->hasHook('pre-commit', $projectRoot) || $helper->ask($input, $output, $question)) { + $git->createHook('pre-commit', $projectRoot); + } } // Return early if there is no code in the repository. diff --git a/src/Service/Git.php b/src/Service/Git.php index 411bff71d..a75fe857b 100644 --- a/src/Service/Git.php +++ b/src/Service/Git.php @@ -431,12 +431,25 @@ public function cloneRepo($url, $destination = null, array $args = [], $mustRun return (bool) $this->execute($args, false, $mustRun, false); } - public function createPreCommitHook() { - $root = $this->getRoot(); - if(file_exists($root.'/hooks/pre-commit')) { - throw new \RuntimeException('pre-commit already exists'); + public function hasHook($hookName,$projectRoot) { + $gitRoot = $projectRoot . '/.git'; + $dest_file="$gitRoot/hooks/$hookName"; + + return file_exists($dest_file); + } + + public function createHook($hookName, $projectRoot) { + $source_file="resources/hooks/$hookName"; + $gitRoot = $projectRoot . '/.git'; + $dest_file="$gitRoot/hooks/$hookName"; + + if(!file_exists($source_file)) { + throw new \RuntimeException('Hook does not exist'); } + if(copy($source_file,$dest_file)) { + chmod($dest_file,0755); + } } /** * Find the root directory of a Git repository. From dac8eee3bb2b0ee563221ecde3b46ddab15a7f1a Mon Sep 17 00:00:00 2001 From: Matthias Van Woensel Date: Mon, 17 Feb 2020 13:22:34 +0100 Subject: [PATCH 06/16] Adding check for large files --- resources/hooks/pre-commit | 62 +++++++++++++++++++++++++++++++++----- 1 file changed, 54 insertions(+), 8 deletions(-) diff --git a/resources/hooks/pre-commit b/resources/hooks/pre-commit index 3d1ccab90..5ad150535 100755 --- a/resources/hooks/pre-commit +++ b/resources/hooks/pre-commit @@ -17,9 +17,10 @@ C_BOLD='\033[0;1;4m' print_changes() { relevant_changes=$1 + display=${2:-"Relevant_change(s)"} echo "" echo "-----------------------------" - echo "| Relevant change(s) |" + echo "| $display |" echo "-----------------------------" echo "" echo "$relevant_changes" @@ -47,15 +48,15 @@ print_fail() { confirm_or_exit(){ exec < /dev/tty - #allow the user to commit anyway + #allow the user to proceed even after we warned him while true; do - read -p "Proceed anyway? (y/n) " yn + read -p "Proceed? (y/n) " yn if [ "$yn" = "" ]; then yn='n' fi echo "" case $yn in - [Yy] ) exit 0;; + [Yy] ) return;; [Nn] ) echo "Commit cancelled."; exit 1;; * ) echo "Please answer y for yes or n for no.";; esac @@ -87,6 +88,7 @@ get_git_head() { + # Redirect output to stderr. exec 1>&2 @@ -102,7 +104,7 @@ echo -n "Checking name change of containers..." hook_config_name="psh_allow_container_rename" config_value=$(get_config $hook_config_name) if [ "$config_value" != "true" ] && - test $($cmd_to_test --no-color | grep -E 'name\:|\[.+\:.+\}$' | wc -c) != 0 + test $($cmd_to_test --no-color | grep -E 'name\:|\[.+\:.+\}$' | grep '\[' | wc -c) != 0 then print_fail @@ -112,7 +114,7 @@ then echo "" echo "Changing the name of your application after it has been deployed will destroy all storage volumes and result in the loss of all persistent data. This is typically a Very Bad Thing to do. It could be useful under certain circumstances in the early stages of development but you almost certainly don't want to change it on a live project." echo "For more information see: https://docs.platform.sh/configuration/app/name.html" - print_changes "$($cmd_to_test --color | grep 'name:')\n $($cmd_to_test | grep -E '\[.+\:.+\}$')" + print_changes "$($cmd_to_test --color | grep 'name:' | grep '\[')\n $($cmd_to_test | grep -E '\[.+\:.+\}$')" print_permanently_disable_notice $hook_config_name confirm_or_exit @@ -129,7 +131,7 @@ echo -n "Checking service type changes..." hook_config_name="psh_allow_service_type_change"; config_value=$(get_config $hook_config_name) if [ "$config_value" != "true" ] && - test $($cmd_to_test | grep 'type:' | wc -c) != 0 + test $($cmd_to_test | grep 'type\:' | grep '\[' | wc -c) != 0 then print_fail @@ -145,7 +147,7 @@ then echo "${C_BOLD}Downgrading again later is not possible!${C_NO}" echo "" echo "${C_GRAY}* There are limitations regarding which service supports big version jumps while keeping the data (e.g.: Elasticsearch 1.7 -> 6.5). These are upstream limitations not specific to platform.sh. Check the documentation relevant to your service.${C_NO}" - print_changes "$($cmd_to_test | grep 'type:')" + print_changes "$($cmd_to_test | grep 'type\:' | grep '\[')" print_permanently_disable_notice $hook_config_name confirm_or_exit @@ -158,5 +160,49 @@ fi +echo -n "Checking large files..." + +hook_config_name="psh_allow_large_files"; +config_value=$(get_config $hook_config_name) + +#get all files that were modified/added, +#get their size on disk with `stat` +#get all sizes that are 1MB (999999=6chars, 1000000=7chars) or higher +cmd_to_test="git diff --diff-filter=MA --name-only -z $against" +cmd_stat="xargs -0 sh -c 'for arg do echo -n \"\$arg|\";stat -c \"%s\" \"\$arg\";done' _" +cmd_grep="grep -E '^.+\\|[0-9]{7,}+\$'" +cmd="${cmd_to_test} | ${cmd_stat} | ${cmd_grep}" + +if [ "$config_value" != "true" ] && + test $(eval $cmd | wc -c) != 0 +then + print_fail + + echo "**************************************************" + echo "* ${C_BRIGHT_RED}Warning${C_NO}: Large file commit detected! *" + echo "**************************************************" + echo "" + echo "It looks like you are attempting to commit a file larger than ~1MB." + echo "This will slow down builds/clones, and other operations we would rather not slow down." + echo "" + echo "The maximum total size of your git environment is 4GB." + echo "Therefore we strongly recommend against commit large files." + echo "" + echo "Please verify that you want to commit the following:" + + print_changes "$(eval $cmd)" "Filename / MB " + print_permanently_disable_notice $hook_config_name + + confirm_or_exit + +else + print_ok +fi + + + + + + exec git diff-index --check --cached $against -- From f48625f0bf6a45204854d7651d17acbb85046cd2 Mon Sep 17 00:00:00 2001 From: Matthias Van Woensel Date: Tue, 18 Feb 2020 11:46:39 +0100 Subject: [PATCH 07/16] Detecting service change name should be done differently, not sure how yet. --- resources/hooks/pre-commit | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/resources/hooks/pre-commit b/resources/hooks/pre-commit index 5ad150535..5f993b1a3 100755 --- a/resources/hooks/pre-commit +++ b/resources/hooks/pre-commit @@ -104,7 +104,7 @@ echo -n "Checking name change of containers..." hook_config_name="psh_allow_container_rename" config_value=$(get_config $hook_config_name) if [ "$config_value" != "true" ] && - test $($cmd_to_test --no-color | grep -E 'name\:|\[.+\:.+\}$' | grep '\[' | wc -c) != 0 + test $($cmd_to_test --no-color | grep -E 'name\:$' | grep '\[' | wc -c) != 0 #|\[.+\:.+\} then print_fail @@ -114,7 +114,8 @@ then echo "" echo "Changing the name of your application after it has been deployed will destroy all storage volumes and result in the loss of all persistent data. This is typically a Very Bad Thing to do. It could be useful under certain circumstances in the early stages of development but you almost certainly don't want to change it on a live project." echo "For more information see: https://docs.platform.sh/configuration/app/name.html" - print_changes "$($cmd_to_test --color | grep 'name:' | grep '\[')\n $($cmd_to_test | grep -E '\[.+\:.+\}$')" + #print_changes "$($cmd_to_test --color | grep 'name:' | grep '\[')\n $($cmd_to_test | grep -E '\[.+\:.+\}$')" + print_changes "$($cmd_to_test --color | grep 'name:' | grep '\[')')" print_permanently_disable_notice $hook_config_name confirm_or_exit From 09d784b3b3d8b304b850df3dc02cb8ff5aee9f50 Mon Sep 17 00:00:00 2001 From: Matthias Van Woensel Date: Wed, 26 Feb 2020 09:20:51 +0100 Subject: [PATCH 08/16] Adding the ability to get parse the local configuration file e.g. platform app:config-get --local --property hooks.build --- src/Command/App/AppConfigGetCommand.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Command/App/AppConfigGetCommand.php b/src/Command/App/AppConfigGetCommand.php index 13ab54d5a..f3c57604c 100644 --- a/src/Command/App/AppConfigGetCommand.php +++ b/src/Command/App/AppConfigGetCommand.php @@ -4,6 +4,8 @@ use Platformsh\Cli\Command\CommandBase; use Platformsh\Cli\Model\AppConfig; use Platformsh\Cli\Model\Host\LocalHost; +use Platformsh\Cli\Local\LocalApplication; + use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; @@ -19,7 +21,8 @@ protected function configure() ->setName('app:config-get') ->setDescription('View the configuration of an app') ->addOption('property', 'P', InputOption::VALUE_REQUIRED, 'The configuration property to view') - ->addOption('refresh', null, InputOption::VALUE_NONE, 'Whether to refresh the cache'); + ->addOption('refresh', null, InputOption::VALUE_NONE, 'Whether to refresh the cache') + ->addOption('local','L', InputOption::VALUE_NONE, 'Use the local configuration'); $this->addProjectOption(); $this->addEnvironmentOption(); $this->addAppOption(); @@ -31,6 +34,7 @@ protected function configure() */ protected function execute(InputInterface $input, OutputInterface $output) { + // Allow override via PLATFORM_APPLICATION. $prefix = $this->config()->get('service.env_prefix'); if (getenv($prefix . 'APPLICATION') && !LocalHost::conflictsWithCommandLineOptions($input, $prefix)) { @@ -40,6 +44,9 @@ protected function execute(InputInterface $input, OutputInterface $output) throw new \RuntimeException('Failed to decode: ' . $prefix . 'APPLICATION'); } $appConfig = new AppConfig($decoded); + } elseif((bool) $input->getOption('local')) { + $local = new LocalApplication($this->getProjectRoot()); + $appConfig = new AppConfig($local->getConfig()); } else { $this->validateInput($input); $this->warnAboutDeprecatedOptions(['identity-file']); From 0b2598c582d99cd173209be3ce494f2d6e4243ed Mon Sep 17 00:00:00 2001 From: Matthias Van Woensel Date: Wed, 26 Feb 2020 12:24:42 +0100 Subject: [PATCH 09/16] Add a check to the pre-commit to check for common build hook failures e.g. npm run serve --- resources/hooks/pre-commit | 36 ++++++++++++++++++++++- src/Command/Project/ProjectGetCommand.php | 2 +- src/Service/Git.php | 12 ++++++-- 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/resources/hooks/pre-commit b/resources/hooks/pre-commit index 5f993b1a3..8e3eb4ead 100755 --- a/resources/hooks/pre-commit +++ b/resources/hooks/pre-commit @@ -82,7 +82,9 @@ get_git_head() { fi } - +get_build_hook() { + $APPLICATION_EXECUTABLE app:config --local --property hooks.build +} @@ -205,5 +207,37 @@ fi +#check common words that shouldn't be found inside the .yaml file (e.g npm run serve) +echo -n "Checking build hook for known faulty commands..." + +hook_config_name="psh_disable_build_hook_check"; +config_value=$(get_config $hook_config_name) +cmd_to_test="get_build_hook | grep -E '(npm run serve)|(.+&$)'" + +if [ "$config_value" != "true" ] && + test $(eval $cmd_to_test | wc -c) != 0 +then + print_fail + + echo "**************************************************" + echo "* ${C_BRIGHT_RED}Warning${C_NO}: Possible faulty build hook command detected! *" + echo "**************************************************" + echo "" + echo "If a command in the build hook does not finish, the build is unable to finish and it will have to be killed manually." + echo "" + echo "It looks like your build hook contains a command that is known to cause this issue." + echo "Please verify that this is indeed correct." + echo "" + + print_changes "$(eval ${cmd_to_test})" " Command(s) " + print_permanently_disable_notice $hook_config_name + + confirm_or_exit + +else + print_ok +fi + + exec git diff-index --check --cached $against -- diff --git a/src/Command/Project/ProjectGetCommand.php b/src/Command/Project/ProjectGetCommand.php index f38d89209..a9527c365 100644 --- a/src/Command/Project/ProjectGetCommand.php +++ b/src/Command/Project/ProjectGetCommand.php @@ -206,7 +206,7 @@ protected function execute(InputInterface $input, OutputInterface $output) $question = new ConfirmationQuestion('It looks like you already have a pre-commit hook, do you want to overwrite your existing pre-commit hook with ours?', false); if(!$git->hasHook('pre-commit', $projectRoot) || $helper->ask($input, $output, $question)) { - $git->createHook('pre-commit', $projectRoot); + $git->createHook('pre-commit', $projectRoot, $this->config()->get('application.executable')); } } diff --git a/src/Service/Git.php b/src/Service/Git.php index a75fe857b..9ce089cb8 100644 --- a/src/Service/Git.php +++ b/src/Service/Git.php @@ -438,7 +438,7 @@ public function hasHook($hookName,$projectRoot) { return file_exists($dest_file); } - public function createHook($hookName, $projectRoot) { + public function createHook($hookName, $projectRoot, $application_executable) { $source_file="resources/hooks/$hookName"; $gitRoot = $projectRoot . '/.git'; $dest_file="$gitRoot/hooks/$hookName"; @@ -446,8 +446,14 @@ public function createHook($hookName, $projectRoot) { if(!file_exists($source_file)) { throw new \RuntimeException('Hook does not exist'); } - - if(copy($source_file,$dest_file)) { + + if(file_put_contents($dest_file, + str_replace( + '$APPLICATION_EXECUTABLE', + $application_executable, + file_get_contents($source_file) + ) + )) { chmod($dest_file,0755); } } From d4b93e6f393b949fca7598e1f7f646564818c010 Mon Sep 17 00:00:00 2001 From: Matthias Van Woensel Date: Wed, 26 Feb 2020 14:47:38 +0100 Subject: [PATCH 10/16] Adding ability to check services.yaml file for database name changes --- resources/hooks/pre-commit | 42 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/resources/hooks/pre-commit b/resources/hooks/pre-commit index 8e3eb4ead..db0c8b4d4 100755 --- a/resources/hooks/pre-commit +++ b/resources/hooks/pre-commit @@ -20,7 +20,7 @@ print_changes() { display=${2:-"Relevant_change(s)"} echo "" echo "-----------------------------" - echo "| $display |" + echo "| $display |" echo "-----------------------------" echo "" echo "$relevant_changes" @@ -97,7 +97,7 @@ exec 1>&2 against=$(get_git_head) cmd_to_test="git diff --diff-filter=M -z $against --word-diff=plain" - +files_changed=$(git diff --diff-filter=M -z $against --name-only | xargs --null) @@ -129,6 +129,44 @@ fi +echo -n "Checking name change of service containers..." +hook_config_name="psh_allow_service_container_rename" +config_value=$(get_config $hook_config_name) + +if [ "$config_value" != "true" ] && + test $(git diff -z --name-only $against | xargs -0 -IREPLACE echo REPLACE | grep '/services.yaml') != "0" #check if we have changes to services.yaml file +then + services_yaml=$(git diff -z --name-only $against | xargs -0 -IREPLACE echo REPLACE | grep '/services.yaml') + + #check only the changes in that services.yaml file + if test $($cmd_to_test --no-color $services_yaml | grep -E '^\[.+:\-]' | grep '\[' | wc -c) != 0 + then + print_fail + + echo "**************************************************" + echo "* ${C_BRIGHT_RED}Error${C_NO}: Attempt to rename a service container detected! *" + echo "**************************************************" + echo "" + echo "Changing the name of your application after it has been deployed will destroy all storage volumes and result in the loss of all persistent data. This is typically a Very Bad Thing to do. It could be useful under certain circumstances in the early stages of development but you almost certainly don't want to change it on a live project." + echo "For more information see: https://docs.platform.sh/configuration/app/name.html" + echo "" + echo "Please check your services.yaml file" + + print_changes "$($cmd_to_test --no-color $services_yaml | grep -E '^\[.+:\-]' | grep '\[')" + print_permanently_disable_notice $hook_config_name + + confirm_or_exit + else + print_ok + fi +else + print_ok +fi + + + + + echo -n "Checking service type changes..." hook_config_name="psh_allow_service_type_change"; From c3e9a6e5102cd3e7e751c13c5b067d9d228ee14b Mon Sep 17 00:00:00 2001 From: Matthias Van Woensel Date: Wed, 26 Feb 2020 14:59:48 +0100 Subject: [PATCH 11/16] addding services.yaml specific checks --- resources/hooks/pre-commit | 78 +++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 35 deletions(-) diff --git a/resources/hooks/pre-commit b/resources/hooks/pre-commit index db0c8b4d4..2428b7eb9 100755 --- a/resources/hooks/pre-commit +++ b/resources/hooks/pre-commit @@ -129,17 +129,16 @@ fi -echo -n "Checking name change of service containers..." -hook_config_name="psh_allow_service_container_rename" -config_value=$(get_config $hook_config_name) - -if [ "$config_value" != "true" ] && - test $(git diff -z --name-only $against | xargs -0 -IREPLACE echo REPLACE | grep '/services.yaml') != "0" #check if we have changes to services.yaml file +if test $(git diff -z --name-only $against | xargs -0 -IREPLACE echo REPLACE | grep '/services.yaml') != "0" #check if we have changes to services.yaml file then + echo -n "Checking name change of service containers..." + hook_config_name="psh_allow_service_container_rename" + config_value=$(get_config $hook_config_name) services_yaml=$(git diff -z --name-only $against | xargs -0 -IREPLACE echo REPLACE | grep '/services.yaml') #check only the changes in that services.yaml file - if test $($cmd_to_test --no-color $services_yaml | grep -E '^\[.+:\-]' | grep '\[' | wc -c) != 0 + if [ "$config_value" != "true" ] && + test $($cmd_to_test --no-color $services_yaml | grep -E '^\[.+:\-]' | grep '\[' | wc -c) != 0 then print_fail @@ -159,42 +158,51 @@ then else print_ok fi -else - print_ok -fi + echo -n "Checking service type changes..." + hook_config_name="psh_allow_service_type_change"; + config_value=$(get_config $hook_config_name) + if [ "$config_value" != "true" ] && + test $($cmd_to_test --no-color $services_yaml | grep 'type\:' | grep '\[' | wc -c) != 0 + then + print_fail + + echo "**************************************************" + echo "* ${C_BRIGHT_RED}Warning${C_NO}: Change of service type detected! *" + echo "**************************************************" + echo "" + echo "Persistent services can ${C_BOLD}not${C_NO} be downgraded (e.g. MySQL v10.3 -> MySQL v10.2). Only non-persistent containers like chrome-headless, redis, memcached can be downgraded." + echo "" + echo "Please verify your changes before proceeding:" + echo "- ${C_BOLD}Downgrading${C_NO} to an older version will break your service and has the potential to cause dataloss." + echo "- ${C_BOLD}Upgrading${C_NO} to a newer version should work flawlessly(*). But please do verify that this is working correctly for your application by branching your production/master environment first." + echo "${C_BOLD}Downgrading again later is not possible!${C_NO}" + echo "" + echo "${C_GRAY}* There are limitations regarding which service supports big version jumps while keeping the data (e.g.: Elasticsearch 1.7 -> 6.5). These are upstream limitations not specific to platform.sh. Check the documentation relevant to your service.${C_NO}" + echo "" + echo "For your convenience, here are the links to documentation of the most common services:" + echo "- MySQL https://docs.platform.sh/configuration/services/mysql.html#supported-versions" + echo "- PostgreSQL https://docs.platform.sh/configuration/services/postgresql.html#upgrading" + echo "- MongoDB https://docs.platform.sh/configuration/services/mongodb.html#supported-versions" + print_changes "$($cmd_to_test $services_yaml | grep 'type\:' | grep '\[')" + print_permanently_disable_notice $hook_config_name + + confirm_or_exit + else + print_ok + fi + + +fi #end of services.yaml specific changes + + -echo -n "Checking service type changes..." -hook_config_name="psh_allow_service_type_change"; -config_value=$(get_config $hook_config_name) -if [ "$config_value" != "true" ] && - test $($cmd_to_test | grep 'type\:' | grep '\[' | wc -c) != 0 -then - print_fail - echo "**************************************************" - echo "* ${C_BRIGHT_RED}Warning${C_NO}: Change of service type detected! *" - echo "**************************************************" - echo "" - echo "Persistent services can ${C_BOLD}not${C_NO} be upgraded (e.g. MySQL v10.3 -> MySQL v10.2). Only non-persistent containers like chrome-headless, redis, memcached can be downgraded." - echo "" - echo "Please verify your changes before proceeding:" - echo "- ${C_BOLD}Downgrading${C_NO} to an older version will break your service and has the potential to cause dataloss." - echo "- ${C_BOLD}Upgrading${C_NO} to a newer version should work flawlessly(*). But please do verify that this is working correctly for your application by branching your production/master environment first." - echo "${C_BOLD}Downgrading again later is not possible!${C_NO}" - echo "" - echo "${C_GRAY}* There are limitations regarding which service supports big version jumps while keeping the data (e.g.: Elasticsearch 1.7 -> 6.5). These are upstream limitations not specific to platform.sh. Check the documentation relevant to your service.${C_NO}" - print_changes "$($cmd_to_test | grep 'type\:' | grep '\[')" - print_permanently_disable_notice $hook_config_name - confirm_or_exit -else - print_ok -fi From f5e8739d94f2098a45ddd91a4d84a3a90f96b968 Mon Sep 17 00:00:00 2001 From: Matthias Van Woensel Date: Thu, 5 Mar 2020 17:46:54 +0100 Subject: [PATCH 12/16] Adding ability to check disk sizes before they push and cause a cryptic error + a bit of Refactoring --- resources/hooks/pre-commit | 250 ++++++++++++++++++++++++------------- 1 file changed, 161 insertions(+), 89 deletions(-) diff --git a/resources/hooks/pre-commit b/resources/hooks/pre-commit index 2428b7eb9..aa9c0155c 100755 --- a/resources/hooks/pre-commit +++ b/resources/hooks/pre-commit @@ -83,54 +83,171 @@ get_git_head() { } get_build_hook() { - $APPLICATION_EXECUTABLE app:config --local --property hooks.build + $APPLICATION_EXECUTABLE app:config --local --property hooks.build 2> /dev/null } +get_plan_storage_size() { + $APPLICATION_EXECUTABLE subscription:info storage 2> /dev/null +} + +check_container_name_change() { + echo -n "Checking name change of containers..." + hook_config_name="psh_allow_container_rename" + config_value=$(get_config $hook_config_name) + if [ "$config_value" != "true" ] && + test $($cmd_to_test --no-color | grep -E 'name\:$' | grep '\[' | wc -c) != 0 #|\[.+\:.+\} + then + print_fail + + echo "**************************************************" + echo "* ${C_BRIGHT_RED}Error${C_NO}: Attempt to rename a container detected! *" + echo "**************************************************" + echo "" + echo "Changing the name of your application after it has been deployed will destroy all storage volumes and result in the loss of all persistent data. This is typically a Very Bad Thing to do. It could be useful under certain circumstances in the early stages of development but you almost certainly don't want to change it on a live project." + echo "For more information see: https://docs.platform.sh/configuration/app/name.html" + #print_changes "$($cmd_to_test --color | grep 'name:' | grep '\[')\n $($cmd_to_test | grep -E '\[.+\:.+\}$')" + print_changes "$($cmd_to_test --color | grep 'name:' | grep '\[')')" + print_permanently_disable_notice $hook_config_name + confirm_or_exit + else + print_ok + fi +} +#end container name change +check_large_files() { + echo -n "Checking large files..." + hook_config_name="psh_allow_large_files"; + config_value=$(get_config $hook_config_name) -# Redirect output to stderr. -exec 1>&2 + #get all files that were modified/added, + #get their size on disk with `stat` + #get all sizes that are 1MB (999999=6chars, 1000000=7chars) or higher + cmd_to_test="git diff --diff-filter=MA --name-only -z $against" + cmd_stat="xargs -0 sh -c 'for arg do echo -n \"\$arg|\";stat -c \"%s\" \"\$arg\";done' _" + cmd_grep="grep -E '^.+\\|[0-9]{7,}+\$'" + cmd="${cmd_to_test} | ${cmd_stat} | ${cmd_grep}" + if [ "$config_value" != "true" ] && + test $(eval $cmd | wc -c) != 0 + then + print_fail -against=$(get_git_head) -cmd_to_test="git diff --diff-filter=M -z $against --word-diff=plain" -files_changed=$(git diff --diff-filter=M -z $against --name-only | xargs --null) + echo "**************************************************" + echo "* ${C_BRIGHT_RED}Warning${C_NO}: Large file commit detected! *" + echo "**************************************************" + echo "" + echo "It looks like you are attempting to commit a file larger than ~1MB." + echo "This will slow down builds/clones, and other operations we would rather not slow down." + echo "" + echo "The maximum total size of your git environment is 4GB." + echo "Therefore we strongly recommend against commit large files." + echo "" + echo "Please verify that you want to commit the following:" + + print_changes "$(eval $cmd)" "Filename / MB " + print_permanently_disable_notice $hook_config_name + confirm_or_exit + + else + print_ok + fi +} +#end large file commit +check_on_yaml_changes() { + #check common words that shouldn't be found inside the .yaml file (e.g npm run serve) + echo -n "Checking build hook for known faulty commands..." -echo -n "Checking name change of containers..." -hook_config_name="psh_allow_container_rename" -config_value=$(get_config $hook_config_name) -if [ "$config_value" != "true" ] && - test $($cmd_to_test --no-color | grep -E 'name\:$' | grep '\[' | wc -c) != 0 #|\[.+\:.+\} -then - print_fail + hook_config_name="psh_disable_build_hook_check"; + config_value=$(get_config $hook_config_name) + cmd_to_test="get_build_hook | grep -E '(npm run serve)|(.+&$)'" + #yaml_files=$(git diff -z --name-only $against | xargs -0 -IREPLACE echo REPLACE | grep '.yaml') - echo "**************************************************" - echo "* ${C_BRIGHT_RED}Error${C_NO}: Attempt to rename a container detected! *" - echo "**************************************************" - echo "" - echo "Changing the name of your application after it has been deployed will destroy all storage volumes and result in the loss of all persistent data. This is typically a Very Bad Thing to do. It could be useful under certain circumstances in the early stages of development but you almost certainly don't want to change it on a live project." - echo "For more information see: https://docs.platform.sh/configuration/app/name.html" - #print_changes "$($cmd_to_test --color | grep 'name:' | grep '\[')\n $($cmd_to_test | grep -E '\[.+\:.+\}$')" - print_changes "$($cmd_to_test --color | grep 'name:' | grep '\[')')" - print_permanently_disable_notice $hook_config_name - - confirm_or_exit -else - print_ok -fi + if [ "$config_value" != "true" ] && + test $(eval $cmd_to_test | wc -c) != 0 + then + print_fail + echo "**************************************************" + echo "* ${C_BRIGHT_RED}Warning${C_NO}: Possible faulty build hook command detected! *" + echo "**************************************************" + echo "" + echo "If a command in the build hook does not finish, the build is unable to finish and it will have to be killed manually." + echo "" + echo "It looks like your build hook contains a command that is known to cause this issue." + echo "Please verify that this is indeed correct." + echo "" + + print_changes "$(eval ${cmd_to_test})" " Command(s) " + print_permanently_disable_notice $hook_config_name + confirm_or_exit + + else + print_ok + fi + #check if disksize is higher than you are allowed on your plan (this is not perfect, so it won't catch all issues, but it will catch the obvious mistakes) + echo -n "Checking plan size hook for known faulty commands..." + hook_config_name="psh_disable_plan_size_check"; + config_value=$(get_config $hook_config_name) -if test $(git diff -z --name-only $against | xargs -0 -IREPLACE echo REPLACE | grep '/services.yaml') != "0" #check if we have changes to services.yaml file -then + plan_size="$(get_plan_storage_size)" + sum_cmd="find . -name '*.yaml' -exec grep -h 'disk:' {} \; | awk -F: '{sum+=\$2} END {print sum}'" + summed_disk_size="$(eval ${sum_cmd})" + + if [ "$config_value" != "true" ] && + [ "$plan_size" -lt "$summed_disk_size" ] + then + print_fail + + echo "***************************************************************" + echo "* ${C_BRIGHT_RED}Warning${C_NO}: Disk size seems higher than what your plan allows! *" + echo "***************************************************************" + echo "" + echo "We did a check on your yaml files and it looks like you are asking for more disk than your plan allows." + echo "" + echo "Summed disk size ${C_BOLD}$summed_disk_size${C_NO} MB is greater than the current plan limit ${C_BOLD}$plan_size${C_NO} MB" + echo "" + echo "Please check the ${C_BOLD}disk:${C_NO} properties in your .yaml files." + echo "Alternatively, ask the owner of your project to increase the storage of your plan." + echo "" + echo "For more information see: https://docs.platform.sh/configuration/app/storage.html#disk" + echo "" + echo "The push might fail with the same error should you continue." + + + print_permanently_disable_notice $hook_config_name + + confirm_or_exit + + else + print_ok + fi + + + + if test $(git diff -z --name-only $against | xargs -0 -IREPLACE echo REPLACE | grep '/services.yaml' | wc -l) != "0" #check if we have changes to services.yaml file + then + check_on_service_yaml_changes + fi +} +#end check_on_yaml_changes + + + + + + + +check_on_service_yaml_changes() { echo -n "Checking name change of service containers..." hook_config_name="psh_allow_service_container_rename" config_value=$(get_config $hook_config_name) @@ -193,9 +310,10 @@ then else print_ok fi +} +#end check_on_service_yaml_changes -fi #end of services.yaml specific changes @@ -209,80 +327,34 @@ fi #end of services.yaml specific changes -echo -n "Checking large files..." -hook_config_name="psh_allow_large_files"; -config_value=$(get_config $hook_config_name) +# Redirect output to stderr. +exec 1>&2 -#get all files that were modified/added, -#get their size on disk with `stat` -#get all sizes that are 1MB (999999=6chars, 1000000=7chars) or higher -cmd_to_test="git diff --diff-filter=MA --name-only -z $against" -cmd_stat="xargs -0 sh -c 'for arg do echo -n \"\$arg|\";stat -c \"%s\" \"\$arg\";done' _" -cmd_grep="grep -E '^.+\\|[0-9]{7,}+\$'" -cmd="${cmd_to_test} | ${cmd_stat} | ${cmd_grep}" -if [ "$config_value" != "true" ] && - test $(eval $cmd | wc -c) != 0 -then - print_fail +against=$(get_git_head) +cmd_to_test="git diff --diff-filter=M -z $against --word-diff=plain" +files_changed=$(git diff --diff-filter=M -z $against --name-only | xargs --null) - echo "**************************************************" - echo "* ${C_BRIGHT_RED}Warning${C_NO}: Large file commit detected! *" - echo "**************************************************" - echo "" - echo "It looks like you are attempting to commit a file larger than ~1MB." - echo "This will slow down builds/clones, and other operations we would rather not slow down." - echo "" - echo "The maximum total size of your git environment is 4GB." - echo "Therefore we strongly recommend against commit large files." - echo "" - echo "Please verify that you want to commit the following:" - - print_changes "$(eval $cmd)" "Filename / MB " - print_permanently_disable_notice $hook_config_name - confirm_or_exit - -else - print_ok -fi +check_container_name_change +if test $(git diff -z --name-only $against | xargs -0 -IREPLACE echo REPLACE | grep '.yaml' | wc -l) != "0" #check if we have changes to any .yaml files +then + check_on_yaml_changes +fi + + +check_large_files -#check common words that shouldn't be found inside the .yaml file (e.g npm run serve) -echo -n "Checking build hook for known faulty commands..." -hook_config_name="psh_disable_build_hook_check"; -config_value=$(get_config $hook_config_name) -cmd_to_test="get_build_hook | grep -E '(npm run serve)|(.+&$)'" -if [ "$config_value" != "true" ] && - test $(eval $cmd_to_test | wc -c) != 0 -then - print_fail - echo "**************************************************" - echo "* ${C_BRIGHT_RED}Warning${C_NO}: Possible faulty build hook command detected! *" - echo "**************************************************" - echo "" - echo "If a command in the build hook does not finish, the build is unable to finish and it will have to be killed manually." - echo "" - echo "It looks like your build hook contains a command that is known to cause this issue." - echo "Please verify that this is indeed correct." - echo "" - - print_changes "$(eval ${cmd_to_test})" " Command(s) " - print_permanently_disable_notice $hook_config_name - confirm_or_exit - -else - print_ok -fi From d7ff077b9ebe5b8511a265b90343fdcfbcf9ef25 Mon Sep 17 00:00:00 2001 From: Matthias Van Woensel Date: Wed, 11 Mar 2020 17:21:54 +0100 Subject: [PATCH 13/16] Converting bash pre-commit into php code --- resources/hooks/pre-commit | 357 +--------------- src/Application.php | 1 + src/Command/Commit/CommitValidateCommand.php | 410 +++++++++++++++++++ src/Command/Project/ProjectGetCommand.php | 6 +- src/Service/Git.php | 18 + 5 files changed, 443 insertions(+), 349 deletions(-) create mode 100644 src/Command/Commit/CommitValidateCommand.php diff --git a/resources/hooks/pre-commit b/resources/hooks/pre-commit index aa9c0155c..0fe907f16 100755 --- a/resources/hooks/pre-commit +++ b/resources/hooks/pre-commit @@ -1,78 +1,4 @@ -#!/bin/sh -# -# The hook should exit with non-zero status -# after issuing an appropriate message if it wants to stop the commit. -# - -C_NO='\033[0m' # No Color -C_RED='\033[0;31m' -C_BRIGHT_RED='\033[1;31m' -C_GREEN='\033[0;32m' -C_BRIGHT_GREEN='\033[1;32m' -C_BLUE='\033[0;34m' -C_BRIGHT_BLUE='\033[1;34m' -C_GRAY='\033[0;37m' -C_WHITE='\033[1;37m' -C_BOLD='\033[0;1;4m' - -print_changes() { - relevant_changes=$1 - display=${2:-"Relevant_change(s)"} - echo "" - echo "-----------------------------" - echo "| $display |" - echo "-----------------------------" - echo "" - echo "$relevant_changes" - echo "" - echo "-----------------------------" - echo "" -} - -print_permanently_disable_notice() { - hook_config_name=$1 - echo "If you know what you are doing you can permanently disable this check using:" - echo "" - echo " ${C_GRAY}git config hooks.$hook_config_name true${C_NO}" - echo "" -} - -print_ok() { - echo " [${C_BRIGHT_GREEN}OK${C_NO}]" -} - -print_fail() { - echo " [${C_BRIGHT_RED}FAILED${C_NO}]" -} - -confirm_or_exit(){ - exec < /dev/tty - - #allow the user to proceed even after we warned him - while true; do - read -p "Proceed? (y/n) " yn - if [ "$yn" = "" ]; then - yn='n' - fi - echo "" - case $yn in - [Yy] ) return;; - [Nn] ) echo "Commit cancelled."; exit 1;; - * ) echo "Please answer y for yes or n for no.";; - esac - done -} - -get_config() { - hook_config_name=$1 - if test $(git config -l | grep hooks.$hook_config_name | wc -l) != 0 - then - git config --bool hooks.$hook_config_name | cat - else - echo "false" - fi -} - +#!/usr/bin/env sh get_git_head() { if git rev-parse --verify HEAD >/dev/null 2>&1 then @@ -82,280 +8,19 @@ get_git_head() { fi } -get_build_hook() { - $APPLICATION_EXECUTABLE app:config --local --property hooks.build 2> /dev/null -} - -get_plan_storage_size() { - $APPLICATION_EXECUTABLE subscription:info storage 2> /dev/null -} - -check_container_name_change() { - echo -n "Checking name change of containers..." - hook_config_name="psh_allow_container_rename" - config_value=$(get_config $hook_config_name) - if [ "$config_value" != "true" ] && - test $($cmd_to_test --no-color | grep -E 'name\:$' | grep '\[' | wc -c) != 0 #|\[.+\:.+\} - then - print_fail - - echo "**************************************************" - echo "* ${C_BRIGHT_RED}Error${C_NO}: Attempt to rename a container detected! *" - echo "**************************************************" - echo "" - echo "Changing the name of your application after it has been deployed will destroy all storage volumes and result in the loss of all persistent data. This is typically a Very Bad Thing to do. It could be useful under certain circumstances in the early stages of development but you almost certainly don't want to change it on a live project." - echo "For more information see: https://docs.platform.sh/configuration/app/name.html" - #print_changes "$($cmd_to_test --color | grep 'name:' | grep '\[')\n $($cmd_to_test | grep -E '\[.+\:.+\}$')" - print_changes "$($cmd_to_test --color | grep 'name:' | grep '\[')')" - print_permanently_disable_notice $hook_config_name - - confirm_or_exit - else - print_ok - fi -} -#end container name change - -check_large_files() { - - echo -n "Checking large files..." - - hook_config_name="psh_allow_large_files"; - config_value=$(get_config $hook_config_name) - - #get all files that were modified/added, - #get their size on disk with `stat` - #get all sizes that are 1MB (999999=6chars, 1000000=7chars) or higher - cmd_to_test="git diff --diff-filter=MA --name-only -z $against" - cmd_stat="xargs -0 sh -c 'for arg do echo -n \"\$arg|\";stat -c \"%s\" \"\$arg\";done' _" - cmd_grep="grep -E '^.+\\|[0-9]{7,}+\$'" - cmd="${cmd_to_test} | ${cmd_stat} | ${cmd_grep}" - - if [ "$config_value" != "true" ] && - test $(eval $cmd | wc -c) != 0 - then - print_fail - - echo "**************************************************" - echo "* ${C_BRIGHT_RED}Warning${C_NO}: Large file commit detected! *" - echo "**************************************************" - echo "" - echo "It looks like you are attempting to commit a file larger than ~1MB." - echo "This will slow down builds/clones, and other operations we would rather not slow down." - echo "" - echo "The maximum total size of your git environment is 4GB." - echo "Therefore we strongly recommend against commit large files." - echo "" - echo "Please verify that you want to commit the following:" - - print_changes "$(eval $cmd)" "Filename / MB " - print_permanently_disable_notice $hook_config_name - - confirm_or_exit - - else - print_ok - fi -} -#end large file commit - - -check_on_yaml_changes() { - #check common words that shouldn't be found inside the .yaml file (e.g npm run serve) - echo -n "Checking build hook for known faulty commands..." - - hook_config_name="psh_disable_build_hook_check"; - config_value=$(get_config $hook_config_name) - cmd_to_test="get_build_hook | grep -E '(npm run serve)|(.+&$)'" - #yaml_files=$(git diff -z --name-only $against | xargs -0 -IREPLACE echo REPLACE | grep '.yaml') - - if [ "$config_value" != "true" ] && - test $(eval $cmd_to_test | wc -c) != 0 - then - print_fail - - echo "**************************************************" - echo "* ${C_BRIGHT_RED}Warning${C_NO}: Possible faulty build hook command detected! *" - echo "**************************************************" - echo "" - echo "If a command in the build hook does not finish, the build is unable to finish and it will have to be killed manually." - echo "" - echo "It looks like your build hook contains a command that is known to cause this issue." - echo "Please verify that this is indeed correct." - echo "" - - print_changes "$(eval ${cmd_to_test})" " Command(s) " - print_permanently_disable_notice $hook_config_name - - confirm_or_exit - - else - print_ok - fi - - #check if disksize is higher than you are allowed on your plan (this is not perfect, so it won't catch all issues, but it will catch the obvious mistakes) - echo -n "Checking plan size hook for known faulty commands..." - - hook_config_name="psh_disable_plan_size_check"; - config_value=$(get_config $hook_config_name) - - plan_size="$(get_plan_storage_size)" - sum_cmd="find . -name '*.yaml' -exec grep -h 'disk:' {} \; | awk -F: '{sum+=\$2} END {print sum}'" - summed_disk_size="$(eval ${sum_cmd})" - - if [ "$config_value" != "true" ] && - [ "$plan_size" -lt "$summed_disk_size" ] - then - print_fail - - echo "***************************************************************" - echo "* ${C_BRIGHT_RED}Warning${C_NO}: Disk size seems higher than what your plan allows! *" - echo "***************************************************************" - echo "" - echo "We did a check on your yaml files and it looks like you are asking for more disk than your plan allows." - echo "" - echo "Summed disk size ${C_BOLD}$summed_disk_size${C_NO} MB is greater than the current plan limit ${C_BOLD}$plan_size${C_NO} MB" - echo "" - echo "Please check the ${C_BOLD}disk:${C_NO} properties in your .yaml files." - echo "Alternatively, ask the owner of your project to increase the storage of your plan." - echo "" - echo "For more information see: https://docs.platform.sh/configuration/app/storage.html#disk" - echo "" - echo "The push might fail with the same error should you continue." - - - print_permanently_disable_notice $hook_config_name - - confirm_or_exit - - else - print_ok - fi - - - - if test $(git diff -z --name-only $against | xargs -0 -IREPLACE echo REPLACE | grep '/services.yaml' | wc -l) != "0" #check if we have changes to services.yaml file - then - check_on_service_yaml_changes - fi -} -#end check_on_yaml_changes - - - - - - - -check_on_service_yaml_changes() { - echo -n "Checking name change of service containers..." - hook_config_name="psh_allow_service_container_rename" - config_value=$(get_config $hook_config_name) - services_yaml=$(git diff -z --name-only $against | xargs -0 -IREPLACE echo REPLACE | grep '/services.yaml') - - #check only the changes in that services.yaml file - if [ "$config_value" != "true" ] && - test $($cmd_to_test --no-color $services_yaml | grep -E '^\[.+:\-]' | grep '\[' | wc -c) != 0 - then - print_fail - - echo "**************************************************" - echo "* ${C_BRIGHT_RED}Error${C_NO}: Attempt to rename a service container detected! *" - echo "**************************************************" - echo "" - echo "Changing the name of your application after it has been deployed will destroy all storage volumes and result in the loss of all persistent data. This is typically a Very Bad Thing to do. It could be useful under certain circumstances in the early stages of development but you almost certainly don't want to change it on a live project." - echo "For more information see: https://docs.platform.sh/configuration/app/name.html" - echo "" - echo "Please check your services.yaml file" - - print_changes "$($cmd_to_test --no-color $services_yaml | grep -E '^\[.+:\-]' | grep '\[')" - print_permanently_disable_notice $hook_config_name - - confirm_or_exit - else - print_ok - fi - - - - echo -n "Checking service type changes..." - hook_config_name="psh_allow_service_type_change"; - config_value=$(get_config $hook_config_name) - if [ "$config_value" != "true" ] && - test $($cmd_to_test --no-color $services_yaml | grep 'type\:' | grep '\[' | wc -c) != 0 - then - print_fail - - echo "**************************************************" - echo "* ${C_BRIGHT_RED}Warning${C_NO}: Change of service type detected! *" - echo "**************************************************" - echo "" - echo "Persistent services can ${C_BOLD}not${C_NO} be downgraded (e.g. MySQL v10.3 -> MySQL v10.2). Only non-persistent containers like chrome-headless, redis, memcached can be downgraded." - echo "" - echo "Please verify your changes before proceeding:" - echo "- ${C_BOLD}Downgrading${C_NO} to an older version will break your service and has the potential to cause dataloss." - echo "- ${C_BOLD}Upgrading${C_NO} to a newer version should work flawlessly(*). But please do verify that this is working correctly for your application by branching your production/master environment first." - echo "${C_BOLD}Downgrading again later is not possible!${C_NO}" - echo "" - echo "${C_GRAY}* There are limitations regarding which service supports big version jumps while keeping the data (e.g.: Elasticsearch 1.7 -> 6.5). These are upstream limitations not specific to platform.sh. Check the documentation relevant to your service.${C_NO}" - echo "" - echo "For your convenience, here are the links to documentation of the most common services:" - echo "- MySQL https://docs.platform.sh/configuration/services/mysql.html#supported-versions" - echo "- PostgreSQL https://docs.platform.sh/configuration/services/postgresql.html#upgrading" - echo "- MongoDB https://docs.platform.sh/configuration/services/mongodb.html#supported-versions" - print_changes "$($cmd_to_test $services_yaml | grep 'type\:' | grep '\[')" - print_permanently_disable_notice $hook_config_name - - confirm_or_exit - else - print_ok - fi -} -#end check_on_service_yaml_changes - - - - - - - - - - - - - - - - -# Redirect output to stderr. -exec 1>&2 - - against=$(get_git_head) -cmd_to_test="git diff --diff-filter=M -z $against --word-diff=plain" -files_changed=$(git diff --diff-filter=M -z $against --name-only | xargs --null) - +#@TODO: add check to see if CLI is actually installed... +exec < /dev/tty #redirect keyboard input so we can do interactivity in symfony +$APPLICATION_EXECUTABLE commit:validate $against 1>/dev/null - -check_container_name_change - - -if test $(git diff -z --name-only $against | xargs -0 -IREPLACE echo REPLACE | grep '.yaml' | wc -l) != "0" #check if we have changes to any .yaml files +if [ $? -eq 0 ] then - check_on_yaml_changes + echo "CLI says OK, allow commit." + exit 0 +else + echo "CLI failed, cancelling commit." + exit 1 fi - - -check_large_files - - - - - - - - -exec git diff-index --check --cached $against -- +exec <&- \ No newline at end of file diff --git a/src/Application.php b/src/Application.php index 48be678b5..7d2a300e8 100644 --- a/src/Application.php +++ b/src/Application.php @@ -115,6 +115,7 @@ protected function getCommands() $commands[] = new Command\Certificate\CertificateListCommand(); $commands[] = new Command\Commit\CommitGetCommand(); $commands[] = new Command\Commit\CommitListCommand(); + $commands[] = new Command\Commit\CommitValidateCommand(); $commands[] = new Command\Db\DbSqlCommand(); $commands[] = new Command\Db\DbDumpCommand(); $commands[] = new Command\Db\DbSizeCommand(); diff --git a/src/Command/Commit/CommitValidateCommand.php b/src/Command/Commit/CommitValidateCommand.php new file mode 100644 index 000000000..878e7c984 --- /dev/null +++ b/src/Command/Commit/CommitValidateCommand.php @@ -0,0 +1,410 @@ +setName('commit:validate') + ->setAliases(['pre-commit-validate']) + ->setDescription('This will validate the commit you are about to make (used by the pre-commit hook)') + ->addArgument('revision', InputArgument::OPTIONAL, 'The revision to ckeck against','HEAD') + + ; + } + + protected function getHookConfigValue($config) { + return (bool)$this->git->getConfig($config); + } + + protected function checkDiff($strDiff, $regex, &$matches) { + return preg_match_all($regex, $strDiff, $matches); + } + + protected function writeErrorMessage($title, $message) { + $line_of_stars = str_repeat("*",strlen($title)+4); + $this->stdErr->writeLn($line_of_stars); + $this->stdErr->writeLn("* $title *"); + $this->stdErr->writeLn($line_of_stars); + $this->stdErr->writeLn(""); + $this->stdErr->writeLn($message); + } + + protected function printRelevantChanges(array $changes,$title="") { + if(!$changes) return; + if(!$title) $title = "Relevant changes(s)"; + $message = implode(PHP_EOL, $changes); + + $line_of_dashes = str_repeat("-",strlen($title)+4); + $this->stdErr->writeLn($line_of_dashes); + $this->stdErr->writeLn("| $title |"); + $this->stdErr->writeLn($line_of_dashes); + $this->stdErr->writeLn(""); + $this->stdErr->writeLn("$message"); + $this->stdErr->writeLn(""); + $this->stdErr->writeLn($line_of_dashes); + + } + + protected function printPermanentlyDisableNotice($hook_config_name){ + $this->stdErr->writeLn( + [ + "If you know what you are doing you can permanently disable this check using:", + "", + " git config hooks.$hook_config_name true", + "", + ] + ); + } + + protected function confirmOrExit($question="Proceed?") { + $questionHelper = $this->getService('question_helper'); + return $questionHelper->confirm($question); + } + + protected function checkContainerNameChange($strDiff) { + $this->stdErr->write("Checking name change of containers..."); + $hookConfigName="psh_allow_container_rename"; + $hookConfigValue=(bool)$this->getHookConfigValue($hookConfigName); + if(!$hookConfigValue && preg_match('/name\: ?\[.+\}/', $strDiff, $matches) ){ + $this->stdErr->writeLn("[FAIL]"); + $this->writeErrorMessage("Attempt to rename a container detected!", + [ + "Changing the name of your application after it has been deployed will destroy all storage volumes and result in the loss of all persistent data. This is typically a Very Bad Thing to do. It could be useful under certain circumstances in the early stages of development but you almost certainly don't want to change it on a live project.", + "For more information see: https://docs.platform.sh/configuration/app/name.html" + ] + ); + $this->printRelevantChanges($matches); + $this->printPermanentlyDisableNotice($hookConfigName); + return false; + } + + $this->stdErr->writeLn("[OK]"); + return true; + } + + protected function checkLargeFiles($arrFilesToCheck) { + + $this->stdErr->write("Checking for large files... "); + $hookConfigName="psh_allow_large_files"; + $hookConfigValue=(bool)$this->getHookConfigValue($hookConfigName); + if(!$hookConfigValue){ + + $max_byte_size = 1;//get all files larger than x MB + $large_files = []; + foreach($arrFilesToCheck as $fileName) { + $size=round(filesize($fileName) / 1024/1024, 2);//always round down + if($size >= $max_byte_size) { + $large_files[] = "$fileName : $size MB"; + } + } + + if(count($large_files)) { + $this->stdErr->writeLn("[FAIL]"); + $this->writeErrorMessage("Large file commit detected!", + [ + "It looks like you are attempting to commit a file larger than ~1 MB.", + "This will slow down builds/clones, and other operations we would rather not slow down.", + "", + "The maximum total size of your git environment is 4GB.", + "Therefore we strongly recommend against commit large files.", + "", + "Please verify that you want to commit the following:" + ] + ); + $this->printRelevantChanges($large_files, " Filename : MB "); + $this->printPermanentlyDisableNotice($hookConfigName); + + return false; + } + } + + $this->stdErr->writeLn("[OK]"); + return true; + } + + protected function checkCommonMistakesInBuildHook() { + $this->stdErr->write("Checking build hook for known faulty commands... "); + $hookConfigName="psh_disable_build_hook_check"; + $hookConfigValue=(bool)$this->getHookConfigValue($hookConfigName); + if(!$hookConfigValue){ + $arrFaultyCommands=['npm run serve']; + $buildHookContents=$this->getBuildHookContents(); + + foreach($arrFaultyCommands as $faultyCommand) { + if(stripos($buildHookContents,$faultyCommand)!==FALSE) { + $this->stdErr->writeLn("[FAIL]"); + $this->writeErrorMessage("Possible faulty build hook command detected!", + [ + "If a command in the build hook does not finish, the build is unable to finish and it will have to be killed manually.", + "", + "It looks like your build hook contains a command that is known to cause this issue.", + "Please verify that this is indeed correct.", + "", + ] + ); + $this->printRelevantChanges([$faultyCommand], " Command "); + $this->printPermanentlyDisableNotice($hookConfigName); + + return false; + } + } + } + + $this->stdErr->writeLn("[OK]"); + return true; + } + + protected function checkDiskSizeVsPlanSize($project) { + $this->stdErr->write("Checking plan size hook for known faulty commands... "); + $hookConfigName="psh_disable_plan_size_check"; + $hookConfigValue=(bool)$this->getHookConfigValue($hookConfigName); + if(!$hookConfigValue){ + + $planSize = $this->getSubscriptionPlanSize($project); + $summedDiskSize = $this->getSummedDiskSize(); + if($planSize < $summedDiskSize) { + + $this->stdErr->writeLn("[FAIL]"); + $this->writeErrorMessage("Disk size seems higher than what your plan allows!", + [ + "We did a check on your yaml files and it looks like you are asking for more disk than your plan allows.", + "", + "Summed disk size $summedDiskSize MB is greater than the current plan limit $planSize MB", + "", + "Please check the disk: properties in your .yaml files.", + "Alternatively, ask the owner of your project to increase the storage of your plan.", + "", + "For more information see: https://docs.platform.sh/configuration/app/storage.html#disk", + "", + "The push might fail with the same error should you continue." + ] + ); + + $this->printPermanentlyDisableNotice($hookConfigName); + + return false; + + } + } + + $this->stdErr->writeLn("[OK]"); + return true; + } + + protected function checkServiceNameChange($strDiff) { + $this->stdErr->write("Checking name change of service containers..."); + $hookConfigName="psh_allow_service_container_rename"; + $hookConfigValue=(bool)$this->getHookConfigValue($hookConfigName); + + if(!$hookConfigValue && preg_match('/\[\-(.+):\-]\{\+.+:\+}/', $strDiff, $matches) && !in_array($matches[1],['disk','type']) ){ + unset($matches[1]); + $this->stdErr->writeLn("[FAIL]"); + $this->writeErrorMessage("Attempt to rename a service container detected!", + [ + "Changing the name of your application after it has been deployed will destroy all storage volumes and result in the loss of all persistent data. This is typically a Very Bad Thing to do. It could be useful under certain circumstances in the early stages of development but you almost certainly don't want to change it on a live project.", + "For more information see: https://docs.platform.sh/configuration/app/name.html", + "", + "Please check your services.yaml file" + ] + ); + $this->printRelevantChanges($matches); + $this->printPermanentlyDisableNotice($hookConfigName); + return false; + } + + $this->stdErr->writeLn("[OK]"); + return true; + } + + protected function checkServiceTypeChanges($strDiff) { + $this->stdErr->write("Checking service type changes..."); + $hookConfigName="psh_allow_service_type_change"; + $hookConfigValue=(bool)$this->getHookConfigValue($hookConfigName); + + if(!$hookConfigValue && preg_match('/type: ?\[\-.+\-]\{\+.+\+}/', $strDiff, $matches) ){ + $this->stdErr->writeLn("[FAIL]"); + $this->writeErrorMessage("Change of service type detected!", + [ + "Persistent services can not be downgraded (e.g. MySQL v10.3 -> MySQL v10.2). Only non-persistent containers like chrome-headless, redis, memcached can be downgraded.", + "", + "Please verify your changes before proceeding:", + "- Downgrading to an older version will break your service and has the potential to cause dataloss.", + "- Upgrading to a newer version should work flawlessly(*). But please do verify that this is working correctly for your application by branching your production/master environment first.", + "Downgrading again later is not possible!", + "", + "* There are limitations regarding which service supports big version jumps while keeping the data (e.g.: Elasticsearch 1.7 -> 6.5). These are upstream limitations not specific to platform.sh. Check the documentation relevant to your service.", + "", + "For your convenience, here are the links to documentation of the most common services:", + "- MySQL https://docs.platform.sh/configuration/services/mysql.html#supported-versions", + "- PostgreSQL https://docs.platform.sh/configuration/services/postgresql.html#upgrading", + "- MongoDB https://docs.platform.sh/configuration/services/mongodb.html#supported-versions", + ] + ); + $this->printRelevantChanges($matches); + $this->printPermanentlyDisableNotice($hookConfigName); + return false; + } + + $this->stdErr->writeLn("[OK]"); + return true; + } + + + + + + + + + protected function getModifiedFiles($revision) { + return explode("\0", $this->git->diff($revision,["--diff-filter=M","-z","--name-only"])); + } + + protected function getAddedFiles($revision) { + return explode("\0", $this->git->diff($revision,["--diff-filter=A","-z","--name-only"])); + } + + protected function hasFileChanged($arrModifiedFiles, $patternToLookFor='/.yaml$/') { + foreach($arrModifiedFiles as $fileName) { + if(preg_match($patternToLookFor, $fileName) == 1) { + return true; + } + } + + return false; + } + + protected function hasYamlChanges($arrModifiedFiles) { + return $this->hasFileChanged($arrModifiedFiles,'/\.yaml$/'); + } + + protected function hasServiceYamlChanges($arrModifiedFiles) { + return $this->hasFileChanged($arrModifiedFiles,'/services\.yaml$/'); + } + + protected function getSummedDiskSize() { + $sum=0; + + /** @var \Platformsh\Cli\Local\LocalProject $localProject */ + $localProject = $this->getService('local.project'); + $serviceConfig = $localProject->readProjectConfigFile($this->getProjectRoot(), 'services.yaml'); + foreach($serviceConfig as $service) { + $sum+= isset($service['disk']) ? $service['disk'] : self::MIN_SERVICE_DISK_SIZE; + } + + + $appConfig = $this->getNormalizedAppConfig(); + if(isset($appConfig['disk'])) { + $sum+=$appConfig['disk']; + } + return $sum; + } + + protected function getNormalizedAppConfig() { + $local = new LocalApplication($this->getProjectRoot()); + $appConfig = new AppConfig($local->getConfig()); + return $appConfig->getNormalized(); + } + + protected function getBuildHookContents() { + $appConfig = $this->getNormalizedAppConfig(); + if(isset($appConfig['hooks']['build'])) { + return $appConfig['hooks']['build']; + } + return ""; + + } + + protected function getSubscriptionPlanSize($project) { + return $this->getSubscriptionInfo($project, 'storage') ?:PHP_INT_MAX; + } + + protected function getSubscriptionInfo($project, $property) { + $id = $project->getSubscriptionId(); + + $subscription = $this->api()->getClient() + ->getSubscription($id); + if ($subscription) { + return (int)$this->api()->getNestedProperty($subscription, $property); + } + + } + + + + + /** + * {@inheritdoc} + */ + protected function execute(InputInterface $input, OutputInterface $output) + { + $project = $this->getCurrentProject(); + $projectRoot = $this->getProjectRoot(); + if (!$project || !$projectRoot) { + throw new RootNotFoundException(); + } + + /** @var \Platformsh\Cli\Service\Git $git */ + $this->git = $this->getService('git'); + /** @var \Platformsh\Cli\Service\Ssh $ssh */ + $ssh = $this->getService('ssh'); + $this->git->setDefaultRepositoryDir($projectRoot); + $this->git->setSshCommand($ssh->getSshCommand()); + + + $strDiff = $this->git->diff($input->getArgument('revision'),["--diff-filter=M","-z","--word-diff=plain"]); + $arrModifiedFiles = $this->getModifiedFiles($input->getArgument('revision')); + $arrAddedFiles = $this->getAddedFiles($input->getArgument('revision')); + + $questionHelper = $this->getService('question_helper'); + + //check the global diff + if(!$this->checkContainerNameChange($strDiff) && !$questionHelper->confirm('Proceed with commit?')){return self::EXIT_CODE_1;} + if(!$this->checkLargeFiles(array_merge($arrModifiedFiles,$arrAddedFiles)) && !$questionHelper->confirm('Proceed with commit?') ){return self::EXIT_CODE_1;}; + + + //only do these checks when there are .yaml file changes + if($this->hasYamlChanges($arrModifiedFiles)) { + if(!$this->checkCommonMistakesInBuildHook() && !$questionHelper->confirm('Proceed with commit?') ){return self::EXIT_CODE_1;} + if(!$this->checkDiskSizeVsPlanSize($project) && !$questionHelper->confirm('Proceed with commit?') ){return self::EXIT_CODE_1;} + + if($this->hasServiceYamlChanges($arrModifiedFiles)){ + $strServiceDiff = $this->git->diff($input->getArgument('revision'),["--diff-filter=M","-z","--word-diff=plain", $this->config()->get('service.project_config_dir') . '/services.yaml']); + + if(!$this->checkServiceNameChange($strServiceDiff) && !$questionHelper->confirm('Proceed with commit?')){ return self::EXIT_CODE_1;} + if(!$this->checkServiceTypeChanges($strServiceDiff) && !$questionHelper->confirm('Proceed with commit?')){ return self::EXIT_CODE_1;} + }//end hasServiceYamlChanges + }//end hasYamlChanges + + //if we get down here, all good. + $this->stdErr->writeln("All good"); + return self::EXIT_CODE_0; + } +} diff --git a/src/Command/Project/ProjectGetCommand.php b/src/Command/Project/ProjectGetCommand.php index a9527c365..c280d69a8 100644 --- a/src/Command/Project/ProjectGetCommand.php +++ b/src/Command/Project/ProjectGetCommand.php @@ -202,10 +202,10 @@ protected function execute(InputInterface $input, OutputInterface $output) if (!$input->getOption('no-create-hooks')) { $this->debug('Creating hooks'); - $helper = $this->getHelper('question'); - $question = new ConfirmationQuestion('It looks like you already have a pre-commit hook, do you want to overwrite your existing pre-commit hook with ours?', false); + $questionHelper = $this->getService('question_helper'); + $question = 'It looks like you already have a pre-commit hook, do you want to overwrite your existing pre-commit hook with ours?'; - if(!$git->hasHook('pre-commit', $projectRoot) || $helper->ask($input, $output, $question)) { + if(!$git->hasHook('pre-commit', $projectRoot) || $questionHelper->confirm($question)) { $git->createHook('pre-commit', $projectRoot, $this->config()->get('application.executable')); } } diff --git a/src/Service/Git.php b/src/Service/Git.php index 9ce089cb8..4903a279c 100644 --- a/src/Service/Git.php +++ b/src/Service/Git.php @@ -285,6 +285,24 @@ public function fetch($remote, $branch = null, $dir = null, $mustRun = false) return (bool) $this->execute($args, $dir, $mustRun, false); } + /** + * diff local against the Git remote. + * + * @param string $remote + * @param string|null $branch + * @param string|null $dir + * @param bool $mustRun + * + * @return string + */ + public function diff($remote, $additional_args = null, $dir = null, $mustRun = false) + { + $args = ['diff', $remote]; + if($additional_args) { + $args = array_merge($args, $additional_args); + } + return $this->execute($args, $dir, $mustRun, false); + } /** * Pull a ref from a repository. * From c8690e81c0bfdc6e1f5f447309815ec39ad3b552 Mon Sep 17 00:00:00 2001 From: Matthias Van Woensel Date: Thu, 12 Mar 2020 11:43:42 +0100 Subject: [PATCH 14/16] Cleaning up --- resources/hooks/pre-commit | 2 +- src/Command/Commit/CommitValidateCommand.php | 25 +++----------------- 2 files changed, 4 insertions(+), 23 deletions(-) diff --git a/resources/hooks/pre-commit b/resources/hooks/pre-commit index 0fe907f16..12cb8e121 100755 --- a/resources/hooks/pre-commit +++ b/resources/hooks/pre-commit @@ -20,7 +20,7 @@ then echo "CLI says OK, allow commit." exit 0 else - echo "CLI failed, cancelling commit." + echo "Cancelling commit." exit 1 fi exec <&- \ No newline at end of file diff --git a/src/Command/Commit/CommitValidateCommand.php b/src/Command/Commit/CommitValidateCommand.php index 878e7c984..e8a42836f 100644 --- a/src/Command/Commit/CommitValidateCommand.php +++ b/src/Command/Commit/CommitValidateCommand.php @@ -3,17 +3,11 @@ namespace Platformsh\Cli\Command\Commit; use Platformsh\Cli\Command\CommandBase; -use Platformsh\Cli\Service\GitDataApi; -use Platformsh\Client\Model\Environment; -use Platformsh\Client\Model\Git\Commit; use Platformsh\Cli\Local\LocalApplication; -use Platformsh\Cli\Local\LocalProject; use Platformsh\Cli\Model\AppConfig; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputArgument; -use Symfony\Component\Console\Input\InputOption; -use Symfony\Component\Console\Output\NullOutput; use Symfony\Component\Console\Output\OutputInterface; class CommitValidateCommand extends CommandBase @@ -34,8 +28,7 @@ protected function configure() ->setName('commit:validate') ->setAliases(['pre-commit-validate']) ->setDescription('This will validate the commit you are about to make (used by the pre-commit hook)') - ->addArgument('revision', InputArgument::OPTIONAL, 'The revision to ckeck against','HEAD') - + ->addArgument('revision', InputArgument::OPTIONAL, 'The revision to ckeck against','HEAD') ; } @@ -68,8 +61,7 @@ protected function printRelevantChanges(array $changes,$title="") { $this->stdErr->writeLn(""); $this->stdErr->writeLn("$message"); $this->stdErr->writeLn(""); - $this->stdErr->writeLn($line_of_dashes); - + $this->stdErr->writeLn($line_of_dashes); } protected function printPermanentlyDisableNotice($hook_config_name){ @@ -276,13 +268,6 @@ protected function checkServiceTypeChanges($strDiff) { return true; } - - - - - - - protected function getModifiedFiles($revision) { return explode("\0", $this->git->diff($revision,["--diff-filter=M","-z","--name-only"])); } @@ -297,7 +282,6 @@ protected function hasFileChanged($arrModifiedFiles, $patternToLookFor='/.yaml$/ return true; } } - return false; } @@ -319,7 +303,6 @@ protected function getSummedDiskSize() { $sum+= isset($service['disk']) ? $service['disk'] : self::MIN_SERVICE_DISK_SIZE; } - $appConfig = $this->getNormalizedAppConfig(); if(isset($appConfig['disk'])) { $sum+=$appConfig['disk']; @@ -339,7 +322,6 @@ protected function getBuildHookContents() { return $appConfig['hooks']['build']; } return ""; - } protected function getSubscriptionPlanSize($project) { @@ -352,9 +334,8 @@ protected function getSubscriptionInfo($project, $property) { $subscription = $this->api()->getClient() ->getSubscription($id); if ($subscription) { - return (int)$this->api()->getNestedProperty($subscription, $property); + return $this->api()->getNestedProperty($subscription, $property); } - } From a66cd15dc6b6da90510bc176c5321ad0a36d83b5 Mon Sep 17 00:00:00 2001 From: Matthias Van Woensel Date: Thu, 12 Mar 2020 11:53:41 +0100 Subject: [PATCH 15/16] Checking if the CLI exists --- resources/hooks/pre-commit | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/resources/hooks/pre-commit b/resources/hooks/pre-commit index 12cb8e121..40f74750f 100755 --- a/resources/hooks/pre-commit +++ b/resources/hooks/pre-commit @@ -1,4 +1,9 @@ #!/usr/bin/env sh + +if [ ! -f "$APPLICATION_EXECUTABLE" ]; then #check if the cli is installed + exit 0 +fi + get_git_head() { if git rev-parse --verify HEAD >/dev/null 2>&1 then @@ -10,8 +15,6 @@ get_git_head() { against=$(get_git_head) -#@TODO: add check to see if CLI is actually installed... - exec < /dev/tty #redirect keyboard input so we can do interactivity in symfony $APPLICATION_EXECUTABLE commit:validate $against 1>/dev/null @@ -23,4 +26,4 @@ else echo "Cancelling commit." exit 1 fi -exec <&- \ No newline at end of file +exec <&- #restore keyboard input \ No newline at end of file From ce1592d2c859accc48833bcd421eca3de92c0cfa Mon Sep 17 00:00:00 2001 From: Matthias Van Woensel Date: Mon, 16 Mar 2020 12:12:53 +0100 Subject: [PATCH 16/16] Adding ./mercure to commands we don't want to see in build hook --- src/Command/Commit/CommitValidateCommand.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Command/Commit/CommitValidateCommand.php b/src/Command/Commit/CommitValidateCommand.php index e8a42836f..3a06142cc 100644 --- a/src/Command/Commit/CommitValidateCommand.php +++ b/src/Command/Commit/CommitValidateCommand.php @@ -146,7 +146,10 @@ protected function checkCommonMistakesInBuildHook() { $hookConfigName="psh_disable_build_hook_check"; $hookConfigValue=(bool)$this->getHookConfigValue($hookConfigName); if(!$hookConfigValue){ - $arrFaultyCommands=['npm run serve']; + $arrFaultyCommands=[ + 'npm run serve', + './mercure' + ]; $buildHookContents=$this->getBuildHookContents(); foreach($arrFaultyCommands as $faultyCommand) {