diff --git a/leverage/logger.py b/leverage/logger.py index febd971..e40ef66 100644 --- a/leverage/logger.py +++ b/leverage/logger.py @@ -64,9 +64,15 @@ def _configure_logger(logger, show_level=True): logger (logging.Logger): Logger to be configured show_level (bool): Whether to display the logging level in the record. Defaults to True """ - state = get_current_context().obj + click_context = get_current_context(silent=True) - level = state.verbosity + if click_context: + state = click_context.obj + else: + state = None + + # Defaults to DEBUG if there is no click context (unit tests normally) + level = state.verbosity if state else "DEBUG" logger.setLevel(level) logger.propagate = False diff --git a/leverage/modules/terraform.py b/leverage/modules/terraform.py index 7e769c4..357ae08 100644 --- a/leverage/modules/terraform.py +++ b/leverage/modules/terraform.py @@ -1,5 +1,6 @@ import os import re +from typing import Sequence import click import dockerpty @@ -282,7 +283,7 @@ def invoke_for_all_commands(tf, layers, command, args, skip_validation=True): # change to original dir and set it in the container tf.paths.cwd = original_location - # change to original workgindir + # change to original working dir tf.container_config["working_dir"] = original_working_dir return layers @@ -351,55 +352,60 @@ def _plan(tf, args): raise Exit(exit_code) -def handle_apply_arguments_parsing(args): - """Parse and process the arguments for the 'apply' command.""" - # Initialize new_args to handle both '-key=value' and '-key value' - new_args = [] - skip_next = False # Flag to skip the next argument if it's part of '-key value' - - for i, arg in enumerate(args): - if skip_next: - skip_next = False # Reset flag and skip this iteration - continue - - if arg.startswith("-") and not arg.startswith("-var"): - if i + 1 < len(args) and not args[i + 1].startswith("-"): - # Detected '-key value' pair; append them without merging - new_args.append(arg) - new_args.append(args[i + 1]) - skip_next = True # Mark to skip the next item as it's already processed - logger.debug(f"Detected '-key value' pair: {arg}, {args[i + 1]}") - else: - # Either '-key=value' or a standalone '-key'; just append - new_args.append(arg) - logger.debug(f"Appending standard -key=value or standalone argument: {arg}") - else: - # Handles '-var' and non '-' starting arguments - new_args.append(arg) - logger.debug(f"Appending argument (non '-' or '-var'): {arg}") +def has_a_plan_file(args: Sequence[str]) -> bool: + """Determine whether the list of arguments has a plan file at the end. + + Terraform apply arguments have the form "-target ADDRESS" or "-target=ADDRESS" + in one case "-var 'NAME=value'" or "-var='NAME=value'". There are also flags + with the form "-flag". + We just need to know if there is or not a plan file as a last argument to + decide if we prepend our default terraform arguments or not. - return new_args + Cases to consider: + Args | Plan file present + -------------------------------------|------------------- + () | False + ("-flag") | False + ("-var=value") | False + ("plan_file") | True + (..., "-var", "value") | False + (..., "-flag", "plan_file") | True + (..., "-var=value", "plan_file") | True + (..., "-var", "value", "plan_file") | True + + """ + + # Valid 'terraform apply' flags: + # https://developer.hashicorp.com/terraform/cli/commands/apply + tf_flags = [ + "-destroy", + "-refresh-only", + "-detailed-exitcode", + "-auto-approve", + "-compact-warnings", + "-json", + "-no-color", + ] + + if not args or args[-1].startswith("-"): + return False + + if len(args) > 1: + second_last = args[-2] + if second_last.startswith("-"): + if not "=" in second_last and second_last not in tf_flags: + return False + + return True @pass_container -def _apply(tf, args): +def _apply(tf, args: Sequence[str]) -> None: """Build or change the infrastructure in this layer.""" - # if there is a plan, remove all "-var" from the default args - # Preserve the original `-var` removal logic and modify tf_default_args if necessary - tf_default_args = tf.tf_default_args - for arg in args: - if not arg.startswith("-"): - tf_default_args = [arg for index, arg in enumerate(tf_default_args) if not arg.startswith("-var")] - break - - # Process arguments using the new parsing logic - processed_args = handle_apply_arguments_parsing(args) - - logger.debug(f"Original tf_default_args: {tf_default_args}") - logger.debug(f"Processed argument list for execution: {processed_args}") - - # Execute the command with the modified arguments list - exit_code = tf.start_in_layer("apply", *tf_default_args, *processed_args) + default_args = [] if has_a_plan_file(args) else tf.tf_default_args + logger.debug(f"Default args passed to apply command: {default_args}") + + exit_code = tf.start_in_layer("apply", *default_args, *args) if exit_code: logger.error(f"Command execution failed with exit code: {exit_code}") diff --git a/tests/test_modules/terraform/__init__.py b/tests/test_modules/terraform/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/tests/test_modules/terraform/test_handle_apply_arguments_parsing.py b/tests/test_modules/terraform/test_handle_apply_arguments_parsing.py deleted file mode 100644 index e61dd1d..0000000 --- a/tests/test_modules/terraform/test_handle_apply_arguments_parsing.py +++ /dev/null @@ -1,35 +0,0 @@ -import pytest - -from leverage.modules.terraform import handle_apply_arguments_parsing - - -class TestHandleApplyArgumentsParsing: - @pytest.mark.parametrize( - "input_args,expected_output", - [ - # Test case: Single '-key=value' - (["-target=kubernetes_manifest.irfq"], ["-target=kubernetes_manifest.irfq"]), - # Test case: '-key value' - (["-target", "kubernetes_manifest.irfq"], ["-target", "kubernetes_manifest.irfq"]), - # Test case: Multiple mixed arguments - ( - ["-target", "kubernetes_manifest.irfq", "-lock=false"], - ["-target", "kubernetes_manifest.irfq", "-lock=false"], - ), - # Test case: '-var' arguments should be included as is - (["-var", "name=value"], ["-var", "name=value"]), - # Test case: Non-flag argument - (["some_value"], ["some_value"]), - # Test case: Mixed '-key=value' and '-key value' with '-var' - ( - ["-var", "name=value", "-target", "kubernetes_manifest.irfq", "-lock=false"], - ["-var", "name=value", "-target", "kubernetes_manifest.irfq", "-lock=false"], - ), - # Test case: No arguments - ([], []), - # Test case: '-key=value' format with '-var' - (["-var", "name=value", "-lock=false"], ["-var", "name=value", "-lock=false"]), - ], - ) - def test_handle_apply_arguments_parsing(self, input_args, expected_output): - assert handle_apply_arguments_parsing(input_args) == expected_output diff --git a/tests/test_modules/test_terraform.py b/tests/test_modules/test_terraform.py index d8030bc..2ccc5fb 100644 --- a/tests/test_modules/test_terraform.py +++ b/tests/test_modules/test_terraform.py @@ -7,6 +7,7 @@ from leverage._utils import AwsCredsContainer from leverage.container import TerraformContainer from leverage.modules.terraform import _init +from leverage.modules.terraform import has_a_plan_file from tests.test_containers import container_fixture_factory @@ -55,3 +56,32 @@ def test_init_with_args(terraform_container): mocked_pty.call_args_list[0].kwargs["command"] == f"terraform init -migrate-state -backend-config=/project/./config/backend.tfvars" ) + + +@pytest.mark.parametrize( + "args, expected_output", + [ + # No arguments, there's no plan file + ([], False), + # One argument that doesn't begin with '-', it is a plan file + (["plan_file"], True), + # A single flag/mode, no plan file + (["-no-color"], False), + # A single argument that has -key=value form, no plan file + (["-val='NAME=value'"], False), + # One key value argument, no plan file + (["-target", "aws_iam_role.example_role"], False), + # One flag before a plan file + (["-compact-warnings", "plan_file"], True), + # One -key=value argument before a plan file + (["-lock=false", "plan_file"], True), + # One key value argument before a plan file + (["-lock-timeout", "5s", "plan_file"], True), + # Some other options + (["-no-color", "-auto-approve"], False), + (["-destroy", "-target", "aws_iam_role.example.role"], False), + (["-target=aws_iam_role.example_role", "-destroy"], False), + ], +) +def test_apply_arguments_have_plan_file(args, expected_output): + assert has_a_plan_file(tuple(args)) == expected_output