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 5 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
19 changes: 14 additions & 5 deletions hooks/_common.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,21 +163,30 @@ 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"
declare -i args_array_length=$2
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved
shift 2
declare -a args=()
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved
# 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

local -a -r files=("$@")
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved

# 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 +212,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
10 changes: 6 additions & 4 deletions hooks/terraform_checkov.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,16 @@ 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[*]}" "${FILES[@]}"
}

#######################################################################
Expand Down
10 changes: 5 additions & 5 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]/__GIT_WORKING_DIR__/$(pwd)\/}
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved
done
# shellcheck disable=SC2153 # False positive
terraform_docs_ "${HOOK_CONFIG[*]}" "$ARGS" "${FILES[@]}"
terraform_docs_ "${HOOK_CONFIG[*]}" "${ARGS[*]}" "${FILES[@]}"
}

#######################################################################
Expand Down
2 changes: 1 addition & 1 deletion hooks/terraform_fmt.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ function main {
fi

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

#######################################################################
Expand Down
2 changes: 1 addition & 1 deletion hooks/terraform_providers_lock.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ 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[*]}" "${FILES[@]}"
}

#######################################################################
Expand Down
9 changes: 5 additions & 4 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,8 +31,8 @@ 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[*]}" "${FILES[@]}"
}

#######################################################################
Expand Down
10 changes: 5 additions & 5 deletions hooks/terraform_tfsec.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ 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[*]}" "${FILES[@]}"
}

#######################################################################
Expand Down
2 changes: 1 addition & 1 deletion 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[*]}" "${FILES[@]}"
}

#######################################################################
Expand Down
2 changes: 1 addition & 1 deletion hooks/terragrunt_fmt.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ function main {
# JFYI: terragrunt hclfmt color already suppressed via PRE_COMMIT_COLOR=never

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

#######################################################################
Expand Down
2 changes: 1 addition & 1 deletion hooks/terragrunt_validate.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ function main {
# JFYI: terragrunt validate color already suppressed via PRE_COMMIT_COLOR=never

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

#######################################################################
Expand Down
2 changes: 1 addition & 1 deletion hooks/terrascan.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ function main {
# JFYI: terrascan color already suppressed via PRE_COMMIT_COLOR=never

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

#######################################################################
Expand Down
32 changes: 21 additions & 11 deletions hooks/tfupdate.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,32 @@ function main {
# JFYI: suppress color for `tfupdate` 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"
# shellcheck disable=SC2034 # Unused var.
local -r dir_path="$2"
local -r dir_path="$1"
shift 1
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved
declare -a -r args=("$@")
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved

local expand_args=()
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved

while read -r -d '' ARG; do
expand_args+=("$ARG")
done < <(echo "${args[@]}" | xargs printf '%s\0')

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

# return exit code to common::per_dir_hook
local exit_code=$?
Expand All @@ -48,12 +54,16 @@ function per_dir_hook_unique_part {
# args (string with array) arguments that configure wrapped tool behavior
#######################################################################
function run_hook_on_whole_repo {
local -r args="$1"
declare -a -r args=("$@")
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved

local expand_args=()
MaxymVlasov marked this conversation as resolved.
Show resolved Hide resolved

while read -r -d '' ARG; do
expand_args+=("$ARG")
done < <(echo "${args[@]}" | xargs printf '%s\0')

# pass the arguments to hook
# shellcheck disable=SC2068 # hook fails when quoting is used ("$arg[@]")
# shellcheck disable=SC2048 # Use "${array[@]}" (with quotes) to prevent whitespace problems.
# shellcheck disable=SC2086 # Double quote to prevent globbing and word splitting.
tfupdate ${args[*]} --recursive .
tfupdate "${expand_args[@]}" .

# return exit code to common::per_dir_hook
local exit_code=$?
Expand Down