From 393589b755ef8c7401ddb1159483c9f3979a0f11 Mon Sep 17 00:00:00 2001 From: Osvaldo Demo Date: Wed, 19 Jun 2024 12:34:52 -0300 Subject: [PATCH] [159] Fix issue with argument parsing logic for tf apply --- leverage/logger.py | 10 +- leverage/modules/terraform.py | 109 ++++++++---- .../test_handle_apply_arguments_parsing.py | 165 +++++++++++++++--- 3 files changed, 223 insertions(+), 61 deletions(-) diff --git a/leverage/logger.py b/leverage/logger.py index febd9719..e40ef66b 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 7e769c42..45911791 100644 --- a/leverage/modules/terraform.py +++ b/leverage/modules/terraform.py @@ -1,5 +1,6 @@ import os import re +from typing import List import click import dockerpty @@ -351,55 +352,89 @@ 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' +def handle_apply_arguments_parsing(default_args: List[str], args: List[str]) -> List[str]: + """Parse and process arguments for the 'apply' command.""" + parsed_args = [] + plan_filename = None + skip_next = False + + valid_single_flags = {"-auto-approve", "-json", "-compact-warnings", "-no-color"} + + i = 0 + while i < len(args): + arg = args[i] - for i, arg in enumerate(args): if skip_next: - skip_next = False # Reset flag and skip this iteration + skip_next = False + i += 1 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 + if arg.startswith("-") and arg not in {"-var", "-var-file"}: + if arg == "-json" and i + 1 < len(args) and not args[i + 1].startswith("-"): + # Handle -json + parsed_args.append(arg) + parsed_args.append(args[i + 1]) + logger.debug(f"Detected -json argument with plan: {arg} {args[i + 1]}") + plan_filename = args[i + 1] + skip_next = True + elif arg in valid_single_flags: + # Handle valid single flags + parsed_args.append(arg) + logger.debug(f"Appending valid single flag: {arg}") + elif i + 1 < len(args) and not args[i + 1].startswith("-"): + # Handle '-key value' pair + parsed_args.append(arg) + parsed_args.append(args[i + 1]) logger.debug(f"Detected '-key value' pair: {arg}, {args[i + 1]}") + skip_next = True 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}") + # Standalone flag or unknown format (treat as standalone flag) + parsed_args.append(arg) + logger.debug(f"Appending standalone flag: {arg}") + elif arg.startswith("-var=") or arg.startswith("-var-file="): + logger.debug(f"Detected var or varfile '-var=name=value' pair: {arg}, {args[i + 1]}") + parsed_args.append(arg) + elif arg == "-var" or arg == "-var-file": + logger.debug(f"Detected var or varfile '-var name=value' pair: {arg}, {args[i + 1]}") + if i + 1 < len(args): + parsed_args.append(args[i]) + parsed_args.append(f"{args[i + 1]}") + skip_next = True + elif not arg.startswith("-"): + # Assume it's a plan filename if it's not a flag + plan_filename = arg + parsed_args.append(arg) + logger.debug(f"Detected plan filename: {arg}") else: - # Handles '-var' and non '-' starting arguments - new_args.append(arg) - logger.debug(f"Appending argument (non '-' or '-var'): {arg}") + # Handle unexpected format (should not occur with valid Terraform apply command usage) + logger.warning(f"Ignoring unexpected argument format: {arg}") + + i += 1 + + # If a plan filename is detected, retain only valid single flags and the plan filename + if plan_filename: + parsed_args = [arg for arg in parsed_args if arg in valid_single_flags or arg == plan_filename] - return new_args + # If no plan filename detected, prepend default_args + if not plan_filename: + parsed_args = default_args + parsed_args + + logger.debug(f"Final parsed_args: {parsed_args}") + return parsed_args @pass_container -def _apply(tf, args): +def _apply(tf, args: List[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 = tf.tf_default_args + logger.debug(f"Original tf_default_args: {default_args}") + + # Parse arguments for apply command + parsed_args = handle_apply_arguments_parsing(default_args, args) + logger.debug(f"Processed argument list for execution: {parsed_args}") + + # Execute the command with the parsed arguments list + exit_code = tf.start_in_layer("apply", *parsed_args) if exit_code: logger.error(f"Command execution failed with exit code: {exit_code}") diff --git a/tests/test_modules/terraform/test_handle_apply_arguments_parsing.py b/tests/test_modules/terraform/test_handle_apply_arguments_parsing.py index e61dd1d3..568cf333 100644 --- a/tests/test_modules/terraform/test_handle_apply_arguments_parsing.py +++ b/tests/test_modules/terraform/test_handle_apply_arguments_parsing.py @@ -1,35 +1,156 @@ import pytest - +from typing import List from leverage.modules.terraform import handle_apply_arguments_parsing class TestHandleApplyArgumentsParsing: @pytest.mark.parametrize( - "input_args,expected_output", + "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 + # Test case: No arguments ( - ["-target", "kubernetes_manifest.irfq", "-lock=false"], - ["-target", "kubernetes_manifest.irfq", "-lock=false"], + [], + [ + "-var-file=/project/apps-devstg/config/backend.tfvars", + "-var-file=/project/apps-devstg/config/account.tfvars", + ], ), - # 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' + # Test case: -var="my_variable=test" ( - ["-var", "name=value", "-target", "kubernetes_manifest.irfq", "-lock=false"], - ["-var", "name=value", "-target", "kubernetes_manifest.irfq", "-lock=false"], + ["-var=my_variable=test"], + [ + "-var-file=/project/apps-devstg/config/backend.tfvars", + "-var-file=/project/apps-devstg/config/account.tfvars", + "-var=my_variable=test", + ], + ), + # Test case: -var-file="varfile.tfvars" + ( + ["-var-file=varfile.tfvars"], + [ + "-var-file=/project/apps-devstg/config/backend.tfvars", + "-var-file=/project/apps-devstg/config/account.tfvars", + "-var-file=varfile.tfvars", + ], + ), + # Test case: -target="module.appgw.0" + ( + ["-target=module.appgw.0"], + [ + "-var-file=/project/apps-devstg/config/backend.tfvars", + "-var-file=/project/apps-devstg/config/account.tfvars", + "-target=module.appgw.0", + ], + ), + # Test case: -target "module.appgw.0" + ( + ["-target", "module.appgw.0"], + [ + "-var-file=/project/apps-devstg/config/backend.tfvars", + "-var-file=/project/apps-devstg/config/account.tfvars", + "-target", + "module.appgw.0", + ], + ), + # Test case: -lock=false + ( + ["-lock=false"], + [ + "-var-file=/project/apps-devstg/config/backend.tfvars", + "-var-file=/project/apps-devstg/config/account.tfvars", + "-lock=false", + ], + ), + # Test case: -parallelism=4 + ( + ["-parallelism=4"], + [ + "-var-file=/project/apps-devstg/config/backend.tfvars", + "-var-file=/project/apps-devstg/config/account.tfvars", + "-parallelism=4", + ], + ), + # Test case: -compact-warnings + ( + ["-compact-warnings"], + [ + "-var-file=/project/apps-devstg/config/backend.tfvars", + "-var-file=/project/apps-devstg/config/account.tfvars", + "-compact-warnings", + ], + ), + # Test case: -input=false + ( + ["-input=false"], + [ + "-var-file=/project/apps-devstg/config/backend.tfvars", + "-var-file=/project/apps-devstg/config/account.tfvars", + "-input=false", + ], + ), + # Test case: -json + ( + ["-json"], + [ + "-var-file=/project/apps-devstg/config/backend.tfvars", + "-var-file=/project/apps-devstg/config/account.tfvars", + "-json", + ], + ), + # Test case: -lock-timeout=60 + ( + ["-lock-timeout=60"], + [ + "-var-file=/project/apps-devstg/config/backend.tfvars", + "-var-file=/project/apps-devstg/config/account.tfvars", + "-lock-timeout=60", + ], + ), + # Test case: -json -auto-approve + ( + ["-json", "-auto-approve"], + [ + "-var-file=/project/apps-devstg/config/backend.tfvars", + "-var-file=/project/apps-devstg/config/account.tfvars", + "-json", + "-auto-approve", + ], + ), + # Test case: -no-color + ( + ["-no-color"], + [ + "-var-file=/project/apps-devstg/config/backend.tfvars", + "-var-file=/project/apps-devstg/config/account.tfvars", + "-no-color", + ], + ), + # Test case: -json -no-color my_plan + (["-json", "-no-color", "my_plan"], ["-json", "-no-color", "my_plan"]), + # Test case: -target=helm_release.cluster_autoscaling + ( + ["-target=helm_release.cluster_autoscaling"], + [ + "-var-file=/project/apps-devstg/config/backend.tfvars", + "-var-file=/project/apps-devstg/config/account.tfvars", + "-target=helm_release.cluster_autoscaling", + ], + ), + # Test case: -target helm_release.cluster_autoscaling + ( + ["-target", "helm_release.cluster_autoscaling"], + [ + "-var-file=/project/apps-devstg/config/backend.tfvars", + "-var-file=/project/apps-devstg/config/account.tfvars", + "-target", + "helm_release.cluster_autoscaling", + ], ), - # 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 + def test_handle_apply_arguments_parsing(self, input_args: List[str], expected_output: List[str]): + default_args = [ + "-var-file=/project/apps-devstg/config/backend.tfvars", + "-var-file=/project/apps-devstg/config/account.tfvars", + ] + assert handle_apply_arguments_parsing(default_args, input_args) == expected_output