Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add support for version constraints in tfupdate #437

Merged
merged 17 commits into from
Oct 6, 2022
Merged

feat: Add support for version constraints in tfupdate #437

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
92e64df
Swap args positions as preparation to passing arrays as arrays to func
MaxymVlasov Oct 4, 2022
2f88e60
Pass ARGS to functions as array
MaxymVlasov Oct 4, 2022
3233c37
Expand args to true array
MaxymVlasov Oct 4, 2022
dd470ad
Crutch to make work tfupdate as expected
MaxymVlasov Oct 4, 2022
d699d24
Remove crutch, add normal solution, tnx to yermulnik
MaxymVlasov Oct 4, 2022
031296a
Apply review suggestions
MaxymVlasov Oct 5, 2022
68a7f51
Apply suggestions from code review
MaxymVlasov Oct 5, 2022
7866b64
Apply review suggestions
MaxymVlasov Oct 5, 2022
41b0489
Not work. Revert "Apply review suggestions"
MaxymVlasov Oct 5, 2022
7eccb5a
Add comment about how while loop work
MaxymVlasov Oct 5, 2022
cb9c726
Move options parsing to right place
MaxymVlasov Oct 6, 2022
d289774
Fix --all
MaxymVlasov Oct 6, 2022
d51b470
Stop tfupdate hook execution with wrong args order
MaxymVlasov Oct 6, 2022
e57286b
Fix shellcheck violations
MaxymVlasov Oct 6, 2022
6321039
Refactor hooks to new style
MaxymVlasov Oct 6, 2022
22fa93a
Restore right behavior
MaxymVlasov Oct 6, 2022
8e379da
Update hooks/terraform_docs.sh
MaxymVlasov Oct 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 32 additions & 7 deletions hooks/_common.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,22 @@ function common::parse_cmdline {
case $argv in
-a | --args)
shift
ARGS+=("$1")
# `argv` is an string from array with content like:
# ('provider aws' '--version "> 0.14"' '--ignore-path "some/path"')
# where each element is the value of each `--args` from hook config.
# `echo` prints contents of `argv` as an expanded string
# `xargs` passes expanded string to `printf`
# `printf` which splits it into NUL-separated elements,
# NUL-separated elements read by `read` using empty separator
# (`-d ''` or `-d $'\0'`)
# into an `ARGS` array

# This allows to "rebuild" initial `args` array of sort of grouped elements
# into a proper array, where each element is a standalone array slice
# with quoted elements being treated as a standalone slice of array as well.
while read -r -d '' ARG; do
ARGS+=("$ARG")
done < <(echo "$1" | xargs printf '%s\0')
shift
;;
-h | --hook-config)
Expand Down Expand Up @@ -129,7 +144,7 @@ function common::parse_and_export_env_vars {
#######################################################################
function common::is_hook_run_on_whole_repo {
local -r hook_id="$1"
shift 1
shift
local -a -r files=("$@")
# get directory containing `.pre-commit-hooks.yaml` file
local -r root_config_dir="$(dirname "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd -P)")"
Expand Down Expand Up @@ -163,21 +178,31 @@ function common::is_hook_run_on_whole_repo {
# 2.1. If at least 1 check failed - change exit code to non-zero
# 3. Complete hook execution and return exit code
# Arguments:
# args (string with array) arguments that configure wrapped tool behavior
# hook_id (string) hook ID, see `- id` for details in .pre-commit-hooks.yaml file
# args_array_length (integer) Count of arguments in args array.
# args (array) arguments that configure wrapped tool behavior
# files (array) filenames to check
#######################################################################
function common::per_dir_hook {
local -r args="$1"
local -r hook_id="$2"
local -r hook_id="$1"
local -i args_array_length=$2
shift 2
local -a args=()
# Expand args to a true array.
# Based on https://stackoverflow.com/a/10953834
while ((args_array_length-- > 0)); do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would a simpler code work here? This also makes no need for args_array_length var.

Suggested change
while ((args_array_length-- > 0)); do
while [[ $@ ]]; do

On the other hand the whole loop looks quite like simple args=("$@") 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I seem to think I understand why args_array_length is needed.
Though given common::per_dir_hook "$HOOK_ID" "${ARGS[*]}" "${FILES[@]}", which is used across this PR, the 2nd ARG to this current function has ARGS array rather than number of its elements — is this expected? So it's either common::per_dir_hook "$HOOK_ID" "${#ARGS[*]}" "${ARGS[*]}" "${FILES[@]}", or I am missing something essential 🤔 (probably it's right about time to leave this for tomorrow 😛)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I couldn't leave it for tomorrow.
This while thing doesn't work as expected for me, and given common::per_dir_hook "$HOOK_ID" "${ARGS[*]}" "${FILES[@]}" this should be that all same thing from our discussion in Slack:

  while read -r -d '' ARG; do
    args+=("$ARG")
  done < <(echo "$1" | xargs printf '%s\0')
  shift

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea why, but the suggested solution does not work here 🤔

7866b64 (#437)

I try to replace common::per_dir_hook "${ARGS[*]}" with common::per_dir_hook "${ARGS[@]}" but got a weird error

dirname: unrecognized option '--version__REPLACED__SPACE__">__REPLACED__SPACE__1.1.8"'
Try 'dirname --help' for more information.

Also, no luck with changing per_dir_hook_unique_part "${args[@]}" to per_dir_hook_unique_part "${args[*]}"

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm-m-m, that's odd. By the way how come args come in with __REPLACED__SPACE__ thing in them? Is the __REPLACED__SPACE__ to be replaced with actual space later down the code? 🤔
I didn't check the code with __REPLACED__SPACE__ in args instead of spaces...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the REPLACED__SPACE to be replaced with actual space later down the code?

Should be. Firstly, it replaces spaces with placeholders and then placeholders with spaces. Check file for __REPLACED__SPACE__

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, I mean is this expected that at this place the __REPLACED__SPACE__ isn't replaced with spaces yet? This may influence how we should be parsing array elements since elements values are not space-separated string, but __REPLACED__SPACE__-separated string.

args+=("$1")
shift
done
# assign rest of function's positional ARGS into `files` array,
# despite there's only one positional ARG left
local -a -r files=("$@")

# check is (optional) function defined
if [ "$(type -t run_hook_on_whole_repo)" == function ] &&
# check is hook run via `pre-commit run --all`
common::is_hook_run_on_whole_repo "$hook_id" "${files[@]}"; then
run_hook_on_whole_repo "$args"
run_hook_on_whole_repo "${args[@]}"
exit 0
fi

Expand All @@ -203,7 +228,7 @@ function common::per_dir_hook {
dir_path="${dir_path//__REPLACED__SPACE__/ }"
pushd "$dir_path" > /dev/null || continue

per_dir_hook_unique_part "$args" "$dir_path"
per_dir_hook_unique_part "$dir_path" "${args[@]}"

local exit_code=$?
if [ $exit_code -ne 0 ]; then
Expand Down
28 changes: 14 additions & 14 deletions hooks/terraform_checkov.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,34 +13,35 @@ function main {
common::export_provided_env_vars "${ENV_VARS[@]}"
common::parse_and_export_env_vars
# Support for setting PATH to repo root.
# shellcheck disable=SC2178 # It's the simplest syntax for that case
ARGS=${ARGS[*]/__GIT_WORKING_DIR__/$(pwd)\/}
for i in "${!ARGS[@]}"; do
ARGS[i]=${ARGS[i]/__GIT_WORKING_DIR__/$(pwd)\/}
done

# Suppress checkov color
if [ "$PRE_COMMIT_COLOR" = "never" ]; then
export ANSI_COLORS_DISABLED=true
fi
# shellcheck disable=SC2128 # It's the simplest syntax for that case
common::per_dir_hook "$ARGS" "$HOOK_ID" "${FILES[@]}"

common::per_dir_hook "$HOOK_ID" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
}

#######################################################################
# Unique part of `common::per_dir_hook`. The function is executed in loop
# on each provided dir path. Run wrapped tool with specified arguments
# Arguments:
# args (string with array) arguments that configure wrapped tool behavior
# dir_path (string) PATH to dir relative to git repo root.
# Can be used in error logging
# args (array) arguments that configure wrapped tool behavior
# Outputs:
# If failed - print out hook checks status
#######################################################################
function per_dir_hook_unique_part {
# common logic located in common::per_dir_hook
local -r args="$1"
# shellcheck disable=SC2034 # Unused var.
local -r dir_path="$2"
local -r dir_path="$1"
shift
local -a -r args=("$@")

# shellcheck disable=SC2068 # hook fails when quoting is used ("$arg[@]")
checkov -d . ${args[@]}
checkov -d . "${args[@]}"

# return exit code to common::per_dir_hook
local exit_code=$?
Expand All @@ -51,14 +52,13 @@ function per_dir_hook_unique_part {
# Unique part of `common::per_dir_hook`. The function is executed one time
# in the root git repo
# Arguments:
# args (string with array) arguments that configure wrapped tool behavior
# args (array) arguments that configure wrapped tool behavior
#######################################################################
function run_hook_on_whole_repo {
local -r args="$1"
local -a -r args=("$@")

# pass the arguments to hook
# shellcheck disable=SC2068 # hook fails when quoting is used ("$arg[@]")
checkov -d "$(pwd)" ${args[@]}
checkov -d "$(pwd)" "${args[@]}"

# return exit code to common::per_dir_hook
local exit_code=$?
Expand Down
12 changes: 6 additions & 6 deletions hooks/terraform_docs.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ function main {
common::parse_cmdline "$@"
common::export_provided_env_vars "${ENV_VARS[@]}"
common::parse_and_export_env_vars
# Support for setting relative PATH to .terraform-docs.yml config.
# shellcheck disable=SC2178 # It's the simplest syntax for that case
ARGS=${ARGS[*]/--config=/--config=$(pwd)\/}
# shellcheck disable=SC2128 # It's the simplest syntax for that case
# Support for setting PATH to repo root.
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved
for i in "${!ARGS[@]}"; do
ARGS[i]=${ARGS[i]/--config=/--config=$(pwd)\/}
done
# shellcheck disable=SC2153 # False positive
terraform_docs_ "${HOOK_CONFIG[*]}" "$ARGS" "${FILES[@]}"
terraform_docs_ "${HOOK_CONFIG[*]}" "${ARGS[*]}" "${FILES[@]}"
}

#######################################################################
Expand Down Expand Up @@ -91,7 +91,7 @@ function terraform_docs {
shift 3
local -a -r files=("$@")

declare -a paths
local -a paths

local index=0
local file_with_path
Expand Down
12 changes: 6 additions & 6 deletions hooks/terraform_fmt.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,27 @@ function main {
fi

# shellcheck disable=SC2153 # False positive
common::per_dir_hook "${ARGS[*]}" "$HOOK_ID" "${FILES[@]}"
common::per_dir_hook "$HOOK_ID" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
}

#######################################################################
# Unique part of `common::per_dir_hook`. The function is executed in loop
# on each provided dir path. Run wrapped tool with specified arguments
# Arguments:
# args (string with array) arguments that configure wrapped tool behavior
# dir_path (string) PATH to dir relative to git repo root.
# Can be used in error logging
# args (array) arguments that configure wrapped tool behavior
# Outputs:
# If failed - print out hook checks status
#######################################################################
function per_dir_hook_unique_part {
local -r args="$1"
# shellcheck disable=SC2034 # Unused var.
local -r dir_path="$2"
local -r dir_path="$1"
shift
local -a -r args=("$@")

# pass the arguments to hook
# shellcheck disable=SC2068 # hook fails when quoting is used ("$arg[@]")
terraform fmt ${args[@]}
terraform fmt "${args[@]}"

# return exit code to common::per_dir_hook
local exit_code=$?
Expand Down
12 changes: 6 additions & 6 deletions hooks/terraform_providers_lock.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,23 @@ function main {
# JFYI: suppress color for `terraform providers lock` is N/A`

# shellcheck disable=SC2153 # False positive
common::per_dir_hook "${ARGS[*]}" "$HOOK_ID" "${FILES[@]}"
common::per_dir_hook "$HOOK_ID" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
}

#######################################################################
# Unique part of `common::per_dir_hook`. The function is executed in loop
# on each provided dir path. Run wrapped tool with specified arguments
# Arguments:
# args (string with array) arguments that configure wrapped tool behavior
# dir_path (string) PATH to dir relative to git repo root.
# Can be used in error logging
# args (array) arguments that configure wrapped tool behavior
# Outputs:
# If failed - print out hook checks status
#######################################################################
function per_dir_hook_unique_part {
local -r args="$1"
local -r dir_path="$2"
local -r dir_path="$1"
shift
local -a -r args=("$@")

local exit_code

Expand All @@ -41,8 +42,7 @@ function per_dir_hook_unique_part {
}

# pass the arguments to hook
# shellcheck disable=SC2068 # hook fails when quoting is used ("$arg[@]")
terraform providers lock ${args[@]}
terraform providers lock "${args[@]}"

# return exit code to common::per_dir_hook
exit_code=$?
Expand Down
19 changes: 10 additions & 9 deletions hooks/terraform_tflint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ function main {
common::export_provided_env_vars "${ENV_VARS[@]}"
common::parse_and_export_env_vars
# Support for setting PATH to repo root.
# shellcheck disable=SC2178 # It's the simplest syntax for that case
ARGS=${ARGS[*]/__GIT_WORKING_DIR__/$(pwd)\/}
for i in "${!ARGS[@]}"; do
ARGS[i]=${ARGS[i]/__GIT_WORKING_DIR__/$(pwd)\/}
done
# JFYI: tflint color already suppressed via PRE_COMMIT_COLOR=never

# Run `tflint --init` for check that plugins installed.
Expand All @@ -30,31 +31,31 @@ function main {
echo "${TFLINT_INIT}"
return ${exit_code}
}
# shellcheck disable=SC2128 # It's the simplest syntax for that case
common::per_dir_hook "$ARGS" "$HOOK_ID" "${FILES[@]}"

common::per_dir_hook "$HOOK_ID" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
}

#######################################################################
# Unique part of `common::per_dir_hook`. The function is executed in loop
# on each provided dir path. Run wrapped tool with specified arguments
# Arguments:
# args (string with array) arguments that configure wrapped tool behavior
# dir_path (string) PATH to dir relative to git repo root.
# Can be used in error logging
# args (array) arguments that configure wrapped tool behavior
# Outputs:
# If failed - print out hook checks status
#######################################################################
function per_dir_hook_unique_part {
local -r args="$1"
local -r dir_path="$2"
local -r dir_path="$1"
shift
local -a -r args=("$@")

# Print checked PATH **only** if TFLint have any messages
# shellcheck disable=SC2091,SC2068 # Suppress error output
$(tflint ${args[@]} 2>&1) 2> /dev/null || {
common::colorify "yellow" "TFLint in $dir_path/:"

# shellcheck disable=SC2068 # hook fails when quoting is used ("$arg[@]")
tflint ${args[@]}
tflint "${args[@]}"
}

# return exit code to common::per_dir_hook
Expand Down
28 changes: 13 additions & 15 deletions hooks/terraform_tfsec.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,37 +13,36 @@ function main {
common::export_provided_env_vars "${ENV_VARS[@]}"
common::parse_and_export_env_vars
# Support for setting PATH to repo root.
# shellcheck disable=SC2178 # It's the simplest syntax for that case
ARGS=${ARGS[*]/__GIT_WORKING_DIR__/$(pwd)\/}
for i in "${!ARGS[@]}"; do
ARGS[i]=${ARGS[i]/__GIT_WORKING_DIR__/$(pwd)\/}
done

# Suppress tfsec color
if [ "$PRE_COMMIT_COLOR" = "never" ]; then
# shellcheck disable=SC2178,SC2128 # It's the simplest syntax for that case
ARGS+=" --no-color"
ARGS+=("--no-color")
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved
fi

# shellcheck disable=SC2128 # It's the simplest syntax for that case
common::per_dir_hook "$ARGS" "$HOOK_ID" "${FILES[@]}"
common::per_dir_hook "$HOOK_ID" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
}

#######################################################################
# Unique part of `common::per_dir_hook`. The function is executed in loop
# on each provided dir path. Run wrapped tool with specified arguments
# Arguments:
# args (string with array) arguments that configure wrapped tool behavior
# dir_path (string) PATH to dir relative to git repo root.
# Can be used in error logging
# args (array) arguments that configure wrapped tool behavior
# Outputs:
# If failed - print out hook checks status
#######################################################################
function per_dir_hook_unique_part {
local -r args="$1"
# shellcheck disable=SC2034 # Unused var.
local -r dir_path="$2"
local -r dir_path="$1"
shift
local -a -r args=("$@")

# pass the arguments to hook
# shellcheck disable=SC2068 # hook fails when quoting is used ("$arg[@]")
tfsec ${args[@]}
tfsec "${args[@]}"

# return exit code to common::per_dir_hook
local exit_code=$?
Expand All @@ -54,14 +53,13 @@ function per_dir_hook_unique_part {
# Unique part of `common::per_dir_hook`. The function is executed one time
# in the root git repo
# Arguments:
# args (string with array) arguments that configure wrapped tool behavior
# args (array) arguments that configure wrapped tool behavior
#######################################################################
function run_hook_on_whole_repo {
local -r args="$1"
local -a -r args=("$@")

# pass the arguments to hook
# shellcheck disable=SC2068 # hook fails when quoting is used ("$arg[@]")
tfsec "$(pwd)" ${args[@]}
tfsec "$(pwd)" "${args[@]}"

# return exit code to common::per_dir_hook
local exit_code=$?
Expand Down
14 changes: 6 additions & 8 deletions hooks/terraform_validate.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function main {
ARGS+=("-no-color")
fi
# shellcheck disable=SC2153 # False positive
common::per_dir_hook "${ARGS[*]}" "$HOOK_ID" "${FILES[@]}"
common::per_dir_hook "$HOOK_ID" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
}

#######################################################################
Expand All @@ -31,17 +31,16 @@ function main {
# 2. Run `terraform validate`
# 3. If at least 1 check failed - change the exit code to non-zero
# Arguments:
# args (string with array) arguments that configure wrapped tool behavior
# dir_path (string) PATH to dir relative to git repo root.
# Can be used in error logging
# ENV_VARS (array) environment variables that will be used with
# `terraform` commands
# args (array) arguments that configure wrapped tool behavior
# Outputs:
# If failed - print out hook checks status
#######################################################################
function per_dir_hook_unique_part {
local -r args="$1"
local -r dir_path="$2"
local -r dir_path="$1"
shift
local -a -r args=("$@")

local exit_code
local validate_output
Expand All @@ -52,8 +51,7 @@ function per_dir_hook_unique_part {
}

# pass the arguments to hook
# shellcheck disable=SC2068 # hook fails when quoting is used ("$arg[@]")
validate_output=$(terraform validate ${args[@]} 2>&1)
validate_output=$(terraform validate "${args[@]}" 2>&1)
exit_code=$?

if [ $exit_code -ne 0 ]; then
Expand Down
Loading