From a1664a6dfa8dadfc03652deda2f13bfe1bca2c58 Mon Sep 17 00:00:00 2001 From: Barak Fatal <35402131+bo156@users.noreply.github.com> Date: Mon, 14 Aug 2023 13:31:20 +0300 Subject: [PATCH] feat(terraform): Removed most usages of enable_nested_modules (#5415) * Removed most usages of enable_nested_modules * Removed other mocks with True * flake8 fixes * added extra values to ignore in dangerfile --- checkov/common/util/env_vars_config.py | 1 - .../checks/utils/dependency_path_handler.py | 6 +- .../graph_builder/graph_components/blocks.py | 10 +- .../graph_builder/graph_components/module.py | 49 ++---- .../graph_builder/graph_to_tf_definitions.py | 2 +- .../terraform/graph_builder/local_graph.py | 2 - checkov/terraform/plan_runner.py | 12 +- checkov/terraform/runner.py | 102 +++-------- dangerfile.ts | 2 +- .../graph/graph_builder/test_local_graph.py | 159 +----------------- .../test_terraform_graph_parser.py | 1 - .../test_render_scenario.py | 28 --- .../graph/variable_rendering/test_renderer.py | 2 - .../parser_scenarios/bad_tf/expected.json | 32 ---- .../bad_tf/skip_bad_tf_example.tf | 25 --- .../module_matryoshka/bucket1/bucket.tf | 7 - .../bucket1/bucket2/bucket.tf | 7 - .../bucket1/bucket2/bucket3/bucket.tf | 4 - .../module_matryoshka/buckets.tf | 7 - .../module_matryoshka/expected.json | 96 ----------- .../expected.json | 120 ------------- .../tf_module/main.tf | 23 --- .../tf_module/module/main.tf | 4 - .../tf_module/module/module2/main.tf | 11 -- .../tf_module/module/module2/variable.tf | 3 - .../tf_module/module/variable.tf | 3 - tests/terraform/parser/test_parser_modules.py | 19 +-- tests/terraform/runner/test_plan_runner.py | 26 +-- tests/terraform/runner/test_runner.py | 26 +-- 29 files changed, 50 insertions(+), 739 deletions(-) delete mode 100644 tests/terraform/parser/resources/parser_scenarios/bad_tf/expected.json delete mode 100644 tests/terraform/parser/resources/parser_scenarios/bad_tf/skip_bad_tf_example.tf delete mode 100644 tests/terraform/parser/resources/parser_scenarios/module_matryoshka/bucket1/bucket.tf delete mode 100644 tests/terraform/parser/resources/parser_scenarios/module_matryoshka/bucket1/bucket2/bucket.tf delete mode 100644 tests/terraform/parser/resources/parser_scenarios/module_matryoshka/bucket1/bucket2/bucket3/bucket.tf delete mode 100644 tests/terraform/parser/resources/parser_scenarios/module_matryoshka/buckets.tf delete mode 100644 tests/terraform/parser/resources/parser_scenarios/module_matryoshka/expected.json delete mode 100644 tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/expected.json delete mode 100644 tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/main.tf delete mode 100644 tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/module/main.tf delete mode 100644 tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/module/module2/main.tf delete mode 100644 tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/module/module2/variable.tf delete mode 100644 tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/module/variable.tf diff --git a/checkov/common/util/env_vars_config.py b/checkov/common/util/env_vars_config.py index b49f19dddc2..272dde419c1 100644 --- a/checkov/common/util/env_vars_config.py +++ b/checkov/common/util/env_vars_config.py @@ -34,7 +34,6 @@ def __init__(self) -> None: self.ENABLE_MODULES_FOREACH_HANDLING = convert_str_to_bool( os.getenv("CHECKOV_ENABLE_MODULES_FOREACH_HANDLING", True) ) - self.ENABLE_NESTED_MODULES = convert_str_to_bool(os.getenv("CHECKOV_ENABLE_NESTED_MODULES", True)) self.EXPERIMENTAL_GRAPH_DEBUG = convert_str_to_bool(os.getenv("CHECKOV_EXPERIMENTAL_GRAPH_DEBUG", False)) self.EXPIRATION_TIME_IN_SEC = force_int(os.getenv("CHECKOV_EXPIRATION_TIME_IN_SEC", 604800)) self.GITHUB_CONF_DIR_NAME = os.getenv("CKV_GITHUB_CONF_DIR_NAME", "github_conf") diff --git a/checkov/terraform/checks/utils/dependency_path_handler.py b/checkov/terraform/checks/utils/dependency_path_handler.py index a6284bae9f6..2452baf6ae3 100644 --- a/checkov/terraform/checks/utils/dependency_path_handler.py +++ b/checkov/terraform/checks/utils/dependency_path_handler.py @@ -1,6 +1,4 @@ -import os from typing import List -from checkov.common.runners.base_runner import strtobool PATH_SEPARATOR = "->" @@ -8,6 +6,4 @@ def unify_dependency_path(dependency_path: List[str]) -> str: if not dependency_path: return '' - if strtobool(os.getenv('CHECKOV_ENABLE_NESTED_MODULES', 'True')): - return dependency_path[-1] - return PATH_SEPARATOR.join(dependency_path) + return dependency_path[-1] diff --git a/checkov/terraform/graph_builder/graph_components/blocks.py b/checkov/terraform/graph_builder/graph_components/blocks.py index 8a5ab1de12c..0b3401efd0a 100644 --- a/checkov/terraform/graph_builder/graph_components/blocks.py +++ b/checkov/terraform/graph_builder/graph_components/blocks.py @@ -11,7 +11,6 @@ from checkov.common.graph.graph_builder.graph_components.blocks import Block from checkov.common.util.consts import RESOLVED_MODULE_ENTRY_NAME from checkov.terraform.graph_builder.graph_components.block_types import BlockType -from checkov.terraform.graph_builder.utils import remove_module_dependency_in_path if TYPE_CHECKING: from checkov.terraform import TFModule @@ -64,14 +63,7 @@ def __init__( self.module_dependency: TFDefinitionKeyType | None = "" self.module_dependency_num: str | None = "" if path: - if strtobool(os.getenv('CHECKOV_ENABLE_NESTED_MODULES', 'True')): - self.path = path # type:ignore[assignment] # Block class would need to be a Generic type to make this pass - else: - self.path, module_dependency, num = remove_module_dependency_in_path(path) # type:ignore[arg-type] - self.path = os.path.realpath(self.path) - if module_dependency: - self.module_dependency = module_dependency - self.module_dependency_num = num + self.path = path # type:ignore[assignment] # Block class would need to be a Generic type to make this pass if attributes.get(RESOLVED_MODULE_ENTRY_NAME): del attributes[RESOLVED_MODULE_ENTRY_NAME] self.attributes = attributes diff --git a/checkov/terraform/graph_builder/graph_components/module.py b/checkov/terraform/graph_builder/graph_components/module.py index 83851fb2650..817ef6b377e 100644 --- a/checkov/terraform/graph_builder/graph_components/module.py +++ b/checkov/terraform/graph_builder/graph_components/module.py @@ -7,7 +7,6 @@ from checkov.common.runners.base_runner import strtobool from checkov.common.util.data_structures_utils import pickle_deepcopy from checkov.common.util.parser_utils import get_abs_path, get_module_from_full_path -from checkov.terraform.checks.utils.dependency_path_handler import unify_dependency_path from checkov.terraform.graph_builder.graph_components.block_types import BlockType from checkov.terraform.graph_builder.graph_components.blocks import TerraformBlock from checkov.terraform.parser_functions import handle_dynamic_values @@ -42,7 +41,6 @@ def __init__( self.resources_types: Set[str] = set() self.source_dir = source_dir self.render_dynamic_blocks_env_var = os.getenv('CHECKOV_RENDER_DYNAMIC_MODULES', 'True') - self.enable_nested_modules = strtobool(os.getenv('CHECKOV_ENABLE_NESTED_MODULES', 'True')) self.use_new_tf_parser = strtobool(os.getenv('CHECKOV_NEW_TF_PARSER', 'True')) def __eq__(self, other: object) -> bool: @@ -73,7 +71,6 @@ def to_dict(self) -> dict[str, Any]: 'resources_types': self.resources_types, 'source_dir': self.source_dir, 'render_dynamic_blocks_env_var': self.render_dynamic_blocks_env_var, - 'enable_nested_modules': self.enable_nested_modules, 'use_new_tf_parser': self.use_new_tf_parser, 'blocks': [block.to_dict() for block in self.blocks] } @@ -93,7 +90,6 @@ def from_dict(module_dict: dict[str, Any]) -> Module: module.resources_types = module_dict.get('resources_types', set()) module.source_dir = module_dict.get('source_dir', '') module.render_dynamic_blocks_env_var = module_dict.get('render_dynamic_blocks_env_var', '') - module.enable_nested_modules = module_dict.get('enable_nested_modules', False) module.use_new_tf_parser = module_dict.get('use_new_tf_parser', False) return module @@ -105,41 +101,18 @@ def add_blocks( self._block_type_to_func[block_type](self, blocks, path) def _add_to_blocks(self, block: TerraformBlock) -> None: - if self.enable_nested_modules: - if self.use_new_tf_parser: - if isinstance(block.path, str): - block.source_module_object = None - block.path = block.path - else: - block.source_module_object = block.path.tf_source_modules - block.path = block.path.file_path + if self.use_new_tf_parser: + if isinstance(block.path, str): + block.source_module_object = None + block.path = block.path else: - block.module_dependency, block.module_dependency_num = get_module_from_full_path(block.path) - block.path = get_abs_path(block.path) - self.blocks.append(block) - return - - dependencies = self.module_dependency_map.get(os.path.dirname(block.path), - []) if self.module_dependency_map else [] - module_dependency_num = "" - if not dependencies: - dependencies = [[]] - for dep_idx, dep_trail in enumerate(dependencies): - if dep_idx > 0: - block = pickle_deepcopy(block) - block.module_dependency = unify_dependency_path(dep_trail) - - if block.module_dependency: - module_dependency_numbers = self.dep_index_mapping.get((block.path, dep_trail[-1]), - []) if self.dep_index_mapping else [] - for mod_idx, module_dep_num in enumerate(module_dependency_numbers): - if mod_idx > 0: - block = pickle_deepcopy(block) - block.module_dependency_num = module_dep_num - self.blocks.append(block) - else: - block.module_dependency_num = module_dependency_num - self.blocks.append(block) + block.source_module_object = block.path.tf_source_modules + block.path = block.path.file_path + else: + block.module_dependency, block.module_dependency_num = get_module_from_full_path(block.path) + block.path = get_abs_path(block.path) + self.blocks.append(block) + return def _add_provider(self, blocks: List[Dict[str, Dict[str, Any]]], path: str | TFDefinitionKey) -> None: for provider_dict in blocks: diff --git a/checkov/terraform/graph_builder/graph_to_tf_definitions.py b/checkov/terraform/graph_builder/graph_to_tf_definitions.py index 39d3f949dd3..cbd6719356d 100644 --- a/checkov/terraform/graph_builder/graph_to_tf_definitions.py +++ b/checkov/terraform/graph_builder/graph_to_tf_definitions.py @@ -43,5 +43,5 @@ def convert_graph_vertices_to_tf_definitions( def add_breadcrumbs(vertex: TerraformBlock, breadcrumbs: Dict[str, Dict[str, Any]], relative_block_path: str) -> None: vertex_breadcrumbs = vertex.breadcrumbs if vertex_breadcrumbs: - vertex_key = vertex.name if not strtobool(os.getenv('CHECKOV_ENABLE_NESTED_MODULES', 'True')) else vertex.attributes.get(CustomAttributes.TF_RESOURCE_ADDRESS, vertex.name) + vertex_key = vertex.attributes.get(CustomAttributes.TF_RESOURCE_ADDRESS, vertex.name) breadcrumbs.setdefault(relative_block_path, {})[vertex_key] = vertex_breadcrumbs diff --git a/checkov/terraform/graph_builder/local_graph.py b/checkov/terraform/graph_builder/local_graph.py index bb54030b948..5d4dc76f1e5 100644 --- a/checkov/terraform/graph_builder/local_graph.py +++ b/checkov/terraform/graph_builder/local_graph.py @@ -759,8 +759,6 @@ def update_list_attribute( def get_path_with_nested_modules(block: TerraformBlock) -> str: if not block.module_dependency: return block.path - if not strtobool(os.getenv('CHECKOV_ENABLE_NESTED_MODULES', 'True')): - return unify_dependency_path([block.module_dependency, block.path]) # type:ignore[no-any-return] # will be fixed when removing terraform/checks from mypy exclusion return get_tf_definition_key_from_module_dependency(block.path, block.module_dependency, block.module_dependency_num) # type:ignore[arg-type] # will be fixed when removing terraform/checks from mypy exclusion diff --git a/checkov/terraform/plan_runner.py b/checkov/terraform/plan_runner.py index 3061b4221b4..495aa526402 100644 --- a/checkov/terraform/plan_runner.py +++ b/checkov/terraform/plan_runner.py @@ -28,8 +28,7 @@ from checkov.terraform.checks.resource.registry import resource_registry from checkov.terraform.context_parsers.registry import parser_registry from checkov.terraform.plan_parser import TF_PLAN_RESOURCE_ADDRESS -from checkov.terraform.plan_utils import create_definitions, build_definitions_context, \ - get_resource_id_without_nested_modules +from checkov.terraform.plan_utils import create_definitions, build_definitions_context from checkov.terraform.runner import Runner as TerraformRunner, merge_reports from checkov.terraform.deep_analysis_plan_graph_manager import DeepAnalysisGraphManager @@ -121,11 +120,7 @@ def run( for vertex in self.tf_plan_local_graph.vertices: if vertex.block_type == BlockType.RESOURCE: address = vertex.attributes.get(CustomAttributes.TF_RESOURCE_ADDRESS) - if self.enable_nested_modules: - report.add_resource(f'{vertex.path}:{address}') - else: - resource_id = get_resource_id_without_nested_modules(address) - report.add_resource(f'{vertex.path}:{resource_id}') + report.add_resource(f'{vertex.path}:{address}') self.graph_manager.save_graph(self.tf_plan_local_graph) if self._should_run_deep_analysis: tf_local_graph = self._create_terraform_graph(runner_filter) @@ -226,7 +221,6 @@ def run_block(self, entities, entity_lines_range = [entity_context.get('start_line'), entity_context.get('end_line')] entity_code_lines = entity_context.get('code_lines') entity_address = entity_context.get('address') - entity_id = entity_address if self.enable_nested_modules else get_resource_id_without_nested_modules(entity_address) _, _, entity_config = registry.extract_entity_details(entity) results = registry.scan(scanned_file, entity, [], runner_filter, report_type=CheckType.TERRAFORM_PLAN) @@ -249,7 +243,7 @@ def run_block(self, entities, code_block=censored_code_lines, file_path=scanned_file, file_line_range=entity_lines_range, - resource=entity_id, + resource=entity_address, resource_address=entity_address, evaluations=None, check_class=check.__class__.__module__, diff --git a/checkov/terraform/runner.py b/checkov/terraform/runner.py index 3535cdac2ad..335e29f66a3 100644 --- a/checkov/terraform/runner.py +++ b/checkov/terraform/runner.py @@ -25,7 +25,7 @@ from checkov.common.util.consts import RESOLVED_MODULE_ENTRY_NAME from checkov.common.util.data_structures_utils import pickle_deepcopy from checkov.common.util.parser_utils import get_module_from_full_path, get_abs_path, \ - get_tf_definition_key_from_module_dependency, TERRAFORM_NESTED_MODULE_INDEX_SEPARATOR, get_module_name, \ + get_tf_definition_key_from_module_dependency, get_module_name, \ strip_terraform_module_referrer from checkov.common.util.secrets import omit_secret_value_from_checks, omit_secret_value_from_graph_checks from checkov.common.variables.context import EvaluationContext @@ -35,7 +35,6 @@ from checkov.terraform.checks.module.registry import module_registry from checkov.terraform.checks.provider.registry import provider_registry from checkov.terraform.checks.resource.registry import resource_registry -from checkov.terraform.checks.utils.dependency_path_handler import PATH_SEPARATOR from checkov.terraform.context_parsers.registry import parser_registry from checkov.terraform.evaluation.base_variable_evaluation import BaseVariableEvaluation from checkov.common.graph.graph_builder.graph_components.attribute_names import CustomAttributes @@ -46,7 +45,6 @@ from checkov.terraform.image_referencer.manager import TerraformImageReferencerManager from checkov.terraform.parser import Parser from checkov.terraform.tf_parser import TFParser -from checkov.terraform.plan_utils import get_resource_id_without_nested_modules from checkov.terraform.tag_providers import get_resource_tags from checkov.common.runners.base_runner import strtobool @@ -89,7 +87,6 @@ def __init__( self.definitions_with_modules: dict[str, dict[str, Any]] = {} self.referrer_cache: Dict[str, str] = {} self.non_referred_cache: Set[str] = set() - self.enable_nested_modules = strtobool(os.getenv('CHECKOV_ENABLE_NESTED_MODULES', 'True')) block_type_registries = { # noqa: CCE003 # a static attribute 'resource': resource_registry, @@ -205,10 +202,7 @@ def _update_definitions_and_breadcrumbs(self, all_graphs, local_graph, report, r for graph in local_graph: for vertex in graph.vertices: if vertex.block_type == BlockType.RESOURCE: - if self.enable_nested_modules: - vertex_id = vertex.attributes.get(CustomAttributes.TF_RESOURCE_ADDRESS) - else: - vertex_id = vertex.id + vertex_id = vertex.attributes.get(CustomAttributes.TF_RESOURCE_ADDRESS) report.add_resource(f'{vertex.path}:{vertex_id}') igraph_graph = self.graph_manager.save_graph(graph) all_graphs.append(igraph_graph) @@ -271,20 +265,11 @@ def get_graph_checks_report(self, root_folder: str, runner_filter: RunnerFilter, module_dependency_num = entity.get(CustomAttributes.MODULE_DEPENDENCY_NUM) definition_context_file_path = full_file_path if module_dependency and module_dependency_num: - if self.enable_nested_modules: - resource = entity.get(CustomAttributes.TF_RESOURCE_ADDRESS, resource_id) - else: - module_dependency_path = module_dependency.split(PATH_SEPARATOR)[-1] - tf_path = get_tf_definition_key_from_module_dependency(full_file_path, module_dependency_path, module_dependency_num) - referrer_id = self._find_id_for_referrer(tf_path) - if referrer_id: - resource = f'{referrer_id}.{resource_id}' + resource = entity.get(CustomAttributes.TF_RESOURCE_ADDRESS, resource_id) definition_context_file_path = get_tf_definition_key_from_module_dependency(full_file_path, module_dependency, module_dependency_num) elif entity.get(CustomAttributes.TF_RESOURCE_ADDRESS) and entity.get(CustomAttributes.TF_RESOURCE_ADDRESS) != resource_id: # for plan resources resource = entity[CustomAttributes.TF_RESOURCE_ADDRESS] - if not self.enable_nested_modules: - resource = get_resource_id_without_nested_modules(resource) entity_config = self.get_graph_resource_entity_config(entity) censored_code_lines = omit_secret_value_from_graph_checks( check=check, @@ -315,10 +300,7 @@ def get_graph_checks_report(self, root_folder: str, runner_filter: RunnerFilter, definition_context_file_path=definition_context_file_path ) if self.breadcrumbs: - if self.enable_nested_modules: - breadcrumb = self.breadcrumbs.get(record.file_path, {}).get(resource) - else: - breadcrumb = self.breadcrumbs.get(record.file_path, {}).get(resource_id) + breadcrumb = self.breadcrumbs.get(record.file_path, {}).get(resource) if breadcrumb: record = GraphRecord(record, breadcrumb) record.set_guideline(check.guideline) @@ -366,17 +348,13 @@ def check_tf_definition( self.context = definitions_context logging.debug('Created definitions context') - if self.enable_nested_modules: - self.push_skipped_checks_down_from_modules(self.context) + self.push_skipped_checks_down_from_modules(self.context) for full_file_path, definition in self.definitions.items(): self.pbar.set_additional_data({'Current File Scanned': os.path.relpath( full_file_path.file_path if isinstance(full_file_path, TFDefinitionKey) else full_file_path, root_folder)}) - if self.enable_nested_modules: - abs_scanned_file = get_abs_path(full_file_path) - abs_referrer = None - else: - abs_scanned_file, abs_referrer = strip_terraform_module_referrer(file_path=full_file_path) + abs_scanned_file = get_abs_path(full_file_path) + abs_referrer = None scanned_file = f"/{os.path.relpath(abs_scanned_file, root_folder)}" logging.debug(f"Scanning file: {scanned_file}") self.run_all_blocks(definition, self.context, full_file_path, root_folder, report, @@ -426,51 +404,29 @@ def run_block( context_parser = parser_registry.context_parsers[block_type] definition_path = context_parser.get_entity_context_path(entity) (entity_type, entity_name, entity_config) = registry.extract_entity_details(entity) - entity_id = ".".join(definition_path) # example: aws_s3_bucket.my_bucket caller_file_path = None caller_file_line_range = None - if self.enable_nested_modules: - entity_id = entity_config.get(CustomAttributes.TF_RESOURCE_ADDRESS) - module_full_path, _ = get_module_from_full_path(full_file_path) - if module_full_path: - module_name = get_module_name(full_file_path) - if not module_name: - full_definition_path = entity_id.split('.') - try: - module_name_index = len(full_definition_path) - full_definition_path[::-1][1:].index(BlockType.MODULE) - 1 # the next item after the last 'module' prefix is the module name - except ValueError as e: - # TODO handle multiple modules with the same name in repo - logging.warning(f'Failed to get module name for resource {entity_id}. {str(e)}') - continue - module_name = full_definition_path[module_name_index] - caller_context = definition_context[module_full_path].get(BlockType.MODULE, {}).get(module_name) - if not caller_context: - continue - caller_file_line_range = [caller_context.get('start_line'), caller_context.get('end_line')] - abs_caller_file = get_abs_path(module_full_path) - caller_file_path = f"/{os.path.relpath(abs_caller_file, root_folder)}" - elif module_referrer is not None: - referrer_id = self._find_id_for_referrer(full_file_path) - - if referrer_id: - entity_id = f"{referrer_id}.{entity_id}" # ex: module.my_module.aws_s3_bucket.my_bucket - abs_caller_file = module_referrer[:module_referrer.rindex(TERRAFORM_NESTED_MODULE_INDEX_SEPARATOR)] - caller_file_path = f"/{os.path.relpath(abs_caller_file, root_folder)}" - + entity_id = entity_config.get(CustomAttributes.TF_RESOURCE_ADDRESS) + module_full_path, _ = get_module_from_full_path(full_file_path) + if module_full_path: + module_name = get_module_name(full_file_path) + if not module_name: + full_definition_path = entity_id.split('.') try: - caller_context = definition_context[abs_caller_file] - for part in referrer_id.split("."): - caller_context = caller_context[part] - except KeyError: - logging.debug("Unable to find caller context for: %s", abs_caller_file) - caller_context = None - - if caller_context: - caller_file_line_range = [caller_context.get('start_line'), caller_context.get('end_line')] - else: - logging.debug(f"Unable to find referrer ID for full path: {full_file_path}") + module_name_index = len(full_definition_path) - full_definition_path[::-1][1:].index(BlockType.MODULE) - 1 # the next item after the last 'module' prefix is the module name + except ValueError as e: + # TODO handle multiple modules with the same name in repo + logging.warning(f'Failed to get module name for resource {entity_id}. {str(e)}') + continue + module_name = full_definition_path[module_name_index] + caller_context = definition_context[module_full_path].get(BlockType.MODULE, {}).get(module_name) + if not caller_context: + continue + caller_file_line_range = [caller_context.get('start_line'), caller_context.get('end_line')] + abs_caller_file = get_abs_path(module_full_path) + caller_file_path = f"/{os.path.relpath(abs_caller_file, root_folder)}" if entity_context_path_header is None: entity_context_path = [block_type] + definition_path @@ -495,9 +451,6 @@ def run_block( entity_code_lines = None skipped_checks = None - if not self.enable_nested_modules and block_type == "module": - self.push_skipped_checks_down_old(definition_context, context_path, skipped_checks) - if full_file_path in self.evaluations_context: variables_evaluations = {} for var_name, context_info in self.evaluations_context.get(full_file_path, {}).items(): @@ -543,10 +496,7 @@ def run_block( definition_context_file_path=full_file_path ) if CHECKOV_CREATE_GRAPH: - if self.enable_nested_modules: - entity_key = entity_id - else: - entity_key = f"{entity_type}.{entity_name}" + entity_key = entity_id breadcrumb = self.breadcrumbs.get(record.file_path, {}).get(entity_key) if breadcrumb: record = GraphRecord(record, breadcrumb) diff --git a/dangerfile.ts b/dangerfile.ts index aa1b7ed55d8..e0cc7f914e3 100644 --- a/dangerfile.ts +++ b/dangerfile.ts @@ -6,7 +6,7 @@ const IGNORE_VAR = [ 'definitions_context_object_path', 'root_folder', 'bucket', 'source_id', 'num_vertices', 'num_edges', 'file_name', 'tmp_folder', 'self.bucket_name', 'repository_zip_path', 'file_size_in_mb', 'repository_zip_path', 'event', 'block_type', 'block_name', 'graph_framework', 'custom_policies', 'checkov_check_id', - 'start_time', 'datetime.now()', 'framework.name', 'str(framework)' + 'start_time', 'datetime.now()', 'framework.name', 'str(framework)', 'entity_id', 'full_file_path' ]; const START_END_IGNORE = [ diff --git a/tests/terraform/graph/graph_builder/test_local_graph.py b/tests/terraform/graph/graph_builder/test_local_graph.py index ef5b618d07e..963b72955f1 100644 --- a/tests/terraform/graph/graph_builder/test_local_graph.py +++ b/tests/terraform/graph/graph_builder/test_local_graph.py @@ -250,27 +250,7 @@ def test_vertices_from_local_graph(self): self.assertIsNotNone(tf_definitions) self.assertIsNotNone(breadcrumbs) - @mock.patch.dict(os.environ, {"CHECKOV_ENABLE_NESTED_MODULES": "False"}) def test_module_dependencies(self): - resources_dir = os.path.realpath(os.path.join(TEST_DIRNAME, '../resources/modules/stacks')) - hcl_config_parser = Parser() - module, _ = hcl_config_parser.parse_hcl_module(resources_dir, self.source) - self.assertEqual(module.module_dependency_map[f'{resources_dir}/prod'], [[]]) - self.assertEqual(module.module_dependency_map[f'{resources_dir}/stage'], [[]]) - self.assertEqual(module.module_dependency_map[f'{resources_dir}/test'], [[]]) - self.assertEqual(module.module_dependency_map[f'{resources_dir}/prod/sub-prod'], [[f'{resources_dir}/prod/main.tf']]) - expected_inner_modules = [ - [f'{resources_dir}/prod/main.tf', f'{resources_dir}/prod/sub-prod/main.tf'], - [f'{resources_dir}/stage/main.tf'], - [f'{resources_dir}/test/main.tf'] - ] - self.assertEqual(module.module_dependency_map[f'{os.path.dirname(resources_dir)}/s3_inner_modules'], expected_inner_modules) - self.assertEqual(module.module_dependency_map[f'{os.path.dirname(resources_dir)}/s3_inner_modules/inner'], - list(map(lambda dep_list: dep_list + [f'{os.path.dirname(resources_dir)}/s3_inner_modules/main.tf'], - expected_inner_modules))) - - @mock.patch.dict(os.environ, {"CHECKOV_ENABLE_NESTED_MODULES": "True"}) - def test_module_dependencies_nested_module_enable(self): resources_dir = os.path.realpath(os.path.join(TEST_DIRNAME, '../resources/modules/stacks')) hcl_config_parser = Parser() module, _ = hcl_config_parser.parse_hcl_module(resources_dir, self.source) @@ -318,7 +298,6 @@ def test_blocks_from_local_graph_module(self): self.assertEqual(len(list(filter(lambda block: block.block_type == BlockType.MODULE and block.name == 's3', module.blocks))), 3) self.assertEqual(len(list(filter(lambda block: block.block_type == BlockType.MODULE and block.name == 'sub-module', module.blocks))), 1) - @mock.patch.dict(os.environ, {"CHECKOV_ENABLE_NESTED_MODULES": "False"}) @mock.patch.dict(os.environ, {"CHECKOV_NEW_TF_PARSER": "False"}) def test_vertices_from_local_graph_module(self): parent_dir = Path(TEST_DIRNAME).parent @@ -330,142 +309,6 @@ def test_vertices_from_local_graph_module(self): self.assertEqual(12, len(local_graph.edges)) - # check vertex breadcrumbs - bucket_vertex_1 = next( - vertex - for vertex in local_graph.vertices - if vertex.name == "aws_s3_bucket.inner_s3" and vertex.source_module == {4} - ) - bucket_vertex_2 = next( - vertex - for vertex in local_graph.vertices - if vertex.name == "aws_s3_bucket.inner_s3" and vertex.source_module == {5} - ) - bucket_vertex_3 = next( - vertex - for vertex in local_graph.vertices - if vertex.name == "aws_s3_bucket.inner_s3" and vertex.source_module == {6} - ) - self.assertDictEqual( - { - "versioning.enabled": [ - { - "type": "module", - "name": "inner_module_call", - "path": str(parent_dir / "resources/modules/s3_inner_modules/main.tf"), - "module_connection": False, - }, - { - "type": "variable", - "name": "versioning", - "path": str(parent_dir / "resources/modules/s3_inner_modules/inner/variables.tf"), - "module_connection": False, - }, - ], - "source_module_": [ - { - "type": "module", - "name": "sub-module", - "path": str(parent_dir / "resources/modules/stacks/prod/main.tf"), - "idx": 0 - }, - { - "type": "module", - "name": "s3", - "path": str(parent_dir / "resources/modules/stacks/prod/sub-prod/main.tf"), - "idx": 3 - }, - { - "type": "module", - "name": "inner_module_call", - "path": str(parent_dir / "resources/modules/s3_inner_modules/main.tf"), - "idx": 4 - }, - ], - }, - bucket_vertex_1.breadcrumbs, - ) - - self.assertDictEqual( - { - "versioning.enabled": [ - { - "type": "module", - "name": "inner_module_call", - "path": str(parent_dir / "resources/modules/s3_inner_modules/main.tf"), - "module_connection": False, - }, - { - "type": "variable", - "name": "versioning", - "path": str(parent_dir / "resources/modules/s3_inner_modules/inner/variables.tf"), - "module_connection": False, - }, - ], - "source_module_": [ - { - "type": "module", - "name": "s3", - "path": str(parent_dir / "resources/modules/stacks/stage/main.tf"), - "idx": 1 - }, - { - "type": "module", - "name": "inner_module_call", - "path": str(parent_dir / "resources/modules/s3_inner_modules/main.tf"), - "idx": 5 - }, - ], - }, - bucket_vertex_2.breadcrumbs, - ) - - self.assertDictEqual( - { - "versioning.enabled": [ - { - "type": "module", - "name": "inner_module_call", - "path": str(parent_dir / "resources/modules/s3_inner_modules/main.tf"), - "module_connection": False, - }, - { - "type": "variable", - "name": "versioning", - "path": str(parent_dir / "resources/modules/s3_inner_modules/inner/variables.tf"), - "module_connection": False, - }, - ], - "source_module_": [ - { - "type": "module", - "name": "s3", - "path": str(parent_dir / "resources/modules/stacks/test/main.tf"), - "idx": 2 - }, - { - "type": "module", - "name": "inner_module_call", - "path": str(parent_dir / "resources/modules/s3_inner_modules/main.tf"), - "idx": 6 - }, - ], - }, - bucket_vertex_3.breadcrumbs, - ) - - @mock.patch.dict(os.environ, {"CHECKOV_ENABLE_NESTED_MODULES": "True"}) - @mock.patch.dict(os.environ, {"CHECKOV_NEW_TF_PARSER": "False"}) - def test_vertices_from_local_graph_module_nested_module_enable(self): - parent_dir = Path(TEST_DIRNAME).parent - resources_dir = str(parent_dir / "resources/modules/stacks") - hcl_config_parser = Parser() - module, _ = hcl_config_parser.parse_hcl_module(resources_dir, self.source) - local_graph = TerraformLocalGraph(module) - local_graph.build_graph(render_variables=True) - - self.assertEqual(12, len(local_graph.edges)) - # check vertex breadcrumbs bucket_vertex_1 = next( vertex @@ -684,7 +527,7 @@ def test_variables_same_name_different_modules_with_new_tf_parser(self): self.assertEqual(2, len(module_variable_edges)) self.assertNotEqual(local_graph.vertices[module_variable_edges[0].origin], local_graph.vertices[module_variable_edges[1].origin]) - @mock.patch.dict(os.environ, {"CHECKOV_ENABLE_NESTED_MODULES": "True", "CHECKOV_NEW_TF_PARSER": "False"}) + @mock.patch.dict(os.environ, {"CHECKOV_NEW_TF_PARSER": "False"}) def test_nested_modules_instances(self): resources_dir = os.path.realpath(os.path.join(TEST_DIRNAME, '../resources/modules/nested_modules_instances')) hcl_config_parser = Parser() diff --git a/tests/terraform/graph/graph_builder/test_terraform_graph_parser.py b/tests/terraform/graph/graph_builder/test_terraform_graph_parser.py index 6d98362f68c..633f890a695 100644 --- a/tests/terraform/graph/graph_builder/test_terraform_graph_parser.py +++ b/tests/terraform/graph/graph_builder/test_terraform_graph_parser.py @@ -173,7 +173,6 @@ def test_hcl_parsing_sorting(self): result_resource = tf_definitions[source_dir + '/main.tf']['resource'][0]['google_compute_instance']['tfer--test3']['service_account'][0]['scopes'][0] self.assertListEqual(result_resource, expected) - @mock.patch.dict(os.environ, {"CHECKOV_ENABLE_NESTED_MODULES": "True"}) def test_build_graph_with_linked_modules(self): source_dir = os.path.realpath(os.path.join(TEST_DIRNAME, '../resources/nested_modules_double_call')) diff --git a/tests/terraform/graph/variable_rendering/test_render_scenario.py b/tests/terraform/graph/variable_rendering/test_render_scenario.py index 81ac0004c38..279cd518b01 100644 --- a/tests/terraform/graph/variable_rendering/test_render_scenario.py +++ b/tests/terraform/graph/variable_rendering/test_render_scenario.py @@ -97,28 +97,6 @@ def test_nested_modules_instances_enable(self): expected = expected.replace(resources_dir, '') assert result == expected - @mock.patch.dict(os.environ, {"CHECKOV_ENABLE_NESTED_MODULES": "False"}) - def test_nested_modules_instances_disable(self): - dir_name = 'nested_modules_instances_disable' - resources_dir = os.path.realpath(os.path.join(TEST_DIRNAME, '../../parser/resources/parser_scenarios', dir_name)) - - from checkov.terraform.parser import Parser - parser = Parser() - tf_definitions = {} - parser.parse_directory(directory=resources_dir, out_definitions=tf_definitions) - - with open(f'{resources_dir}/expected.json') as fp: - expected = json.load(fp) - result, expected = json.dumps(tf_definitions, sort_keys=True), json.dumps(expected, sort_keys=True) - result = result.replace(resources_dir, '') - expected = expected.replace(resources_dir, '') - assert result == expected - - @mock.patch.dict(os.environ, {"CHECKOV_NEW_TF_PARSER": "False"}) - @mock.patch.dict(os.environ, {"CHECKOV_ENABLE_NESTED_MODULES": "False"}) - def test_module_matryoshka(self): - self.go("module_matryoshka") - def test_module_matryoshka_nested_module_enable(self): self.go("module_matryoshka_nested_module_enable") @@ -155,12 +133,6 @@ def test_bad_ref_fallbacks(self): def test_doc_evaluations_verify(self): self.go("doc_evaluations_verify", replace_expected=True) - @mock.patch.dict(os.environ, {"CHECKOV_NEW_TF_PARSER": "False"}) - @mock.patch.dict(os.environ, {"CHECKOV_ENABLE_NESTED_MODULES": "False"}) - def test_bad_tf(self): - # Note: this hits the _clean_bad_definitions internal function - self.go("bad_tf") - def test_bad_tf_nested_modules_enable(self): # Note: this hits the _clean_bad_definitions internal function self.go("bad_tf_nested_modules_enable") diff --git a/tests/terraform/graph/variable_rendering/test_renderer.py b/tests/terraform/graph/variable_rendering/test_renderer.py index 7eb81e36f5c..eafed68c3f7 100644 --- a/tests/terraform/graph/variable_rendering/test_renderer.py +++ b/tests/terraform/graph/variable_rendering/test_renderer.py @@ -177,7 +177,6 @@ def test_dict_tfvar(self): self.assertEqual(expected_value, actual_value, f'error during comparing {v.block_type} in attribute key: {attribute_key}') - @mock.patch.dict(os.environ, {"CHECKOV_ENABLE_NESTED_MODULES": "False"}) @mock.patch.dict(os.environ, {"CHECKOV_NEW_TF_PARSER": "False"}) def test_graph_rendering_order(self): resource_path = os.path.join(TEST_DIRNAME, "..", "resources", "module_rendering", "example") @@ -200,7 +199,6 @@ def test_graph_rendering_order(self): count += 1 self.assertEqual(found, count, f"Expected all instances to have the same value, found {found} instances but only {count} correct values") - @mock.patch.dict(os.environ, {"CHECKOV_ENABLE_NESTED_MODULES": "True"}) def test_graph_rendering_order_nested_module_enable(self): resource_path = os.path.realpath(os.path.join(TEST_DIRNAME, "..", "resources", "module_rendering", "example")) graph_manager = TerraformGraphManager('m', ['m']) diff --git a/tests/terraform/parser/resources/parser_scenarios/bad_tf/expected.json b/tests/terraform/parser/resources/parser_scenarios/bad_tf/expected.json deleted file mode 100644 index a475dca0901..00000000000 --- a/tests/terraform/parser/resources/parser_scenarios/bad_tf/expected.json +++ /dev/null @@ -1,32 +0,0 @@ -{ - "skip_bad_tf_example.tf": { - "module": [ - { - "bar": { - "memory": ["1G"], - "source": ["baz"], - "__start_line__": 11, - "__end_line__": 14, - "__address__": "bar" - } - }, - { - "okay": { - "source": ["./okay", "baz2"], - "__start_line__": 16, - "__end_line__": 19, - "__address__": "okay" - } - } - ], - "variable": [ - { - "okay": { - "__start_line__": 1, - "__end_line__": 2, - "__address__": "okay" - } - } - ] - } -} \ No newline at end of file diff --git a/tests/terraform/parser/resources/parser_scenarios/bad_tf/skip_bad_tf_example.tf b/tests/terraform/parser/resources/parser_scenarios/bad_tf/skip_bad_tf_example.tf deleted file mode 100644 index 329a594e325..00000000000 --- a/tests/terraform/parser/resources/parser_scenarios/bad_tf/skip_bad_tf_example.tf +++ /dev/null @@ -1,25 +0,0 @@ -variable "okay" { -} - -// Variable is missing a name, not valid terraform syntex -variable { - name = "test" - default = "test_value" - type = "string" -} - -module "bar" { - memory = "1G" - source = "baz" -} - -module "okay" { - source = "./okay" - source = "baz2" -} - -// Module is missing a name, can't be referenced or deployed -module { - source = "./not-okay" - memory = "far" -} diff --git a/tests/terraform/parser/resources/parser_scenarios/module_matryoshka/bucket1/bucket.tf b/tests/terraform/parser/resources/parser_scenarios/module_matryoshka/bucket1/bucket.tf deleted file mode 100644 index c9f8d90d694..00000000000 --- a/tests/terraform/parser/resources/parser_scenarios/module_matryoshka/bucket1/bucket.tf +++ /dev/null @@ -1,7 +0,0 @@ -module "bucket2" { - source = "./bucket2" -} - -resource "aws_s3_bucket" "example1" { - bucket = "bucket1" -} \ No newline at end of file diff --git a/tests/terraform/parser/resources/parser_scenarios/module_matryoshka/bucket1/bucket2/bucket.tf b/tests/terraform/parser/resources/parser_scenarios/module_matryoshka/bucket1/bucket2/bucket.tf deleted file mode 100644 index a9b04444a30..00000000000 --- a/tests/terraform/parser/resources/parser_scenarios/module_matryoshka/bucket1/bucket2/bucket.tf +++ /dev/null @@ -1,7 +0,0 @@ -module "bucket3" { - source = "./bucket3" -} - -resource "aws_s3_bucket" "example2" { - bucket = "bucket2" -} \ No newline at end of file diff --git a/tests/terraform/parser/resources/parser_scenarios/module_matryoshka/bucket1/bucket2/bucket3/bucket.tf b/tests/terraform/parser/resources/parser_scenarios/module_matryoshka/bucket1/bucket2/bucket3/bucket.tf deleted file mode 100644 index 6aca14b0480..00000000000 --- a/tests/terraform/parser/resources/parser_scenarios/module_matryoshka/bucket1/bucket2/bucket3/bucket.tf +++ /dev/null @@ -1,4 +0,0 @@ -resource "aws_s3_bucket" "example3" { - bucket = "bucket3" - acl = "public-read" # used by test_runner.py -} \ No newline at end of file diff --git a/tests/terraform/parser/resources/parser_scenarios/module_matryoshka/buckets.tf b/tests/terraform/parser/resources/parser_scenarios/module_matryoshka/buckets.tf deleted file mode 100644 index 7c654144c4f..00000000000 --- a/tests/terraform/parser/resources/parser_scenarios/module_matryoshka/buckets.tf +++ /dev/null @@ -1,7 +0,0 @@ -module "bucket1" { - source = "./bucket1" -} - -resource "aws_s3_bucket" "example0" { - bucket = "bucket0" -} \ No newline at end of file diff --git a/tests/terraform/parser/resources/parser_scenarios/module_matryoshka/expected.json b/tests/terraform/parser/resources/parser_scenarios/module_matryoshka/expected.json deleted file mode 100644 index 00f165af676..00000000000 --- a/tests/terraform/parser/resources/parser_scenarios/module_matryoshka/expected.json +++ /dev/null @@ -1,96 +0,0 @@ -{ - "buckets.tf": { - "module": [ - { - "bucket1": { - "source": ["./bucket1"], - "__resolved__": ["bucket1/bucket.tf([{buckets.tf#*#0}])"], - "__start_line__": 1, - "__end_line__": 3, - "__address__": "bucket1" - } - } - ], - "resource": [ - { - "aws_s3_bucket": { - "example0": { - "bucket": ["bucket0"], - "__start_line__": 5, - "__end_line__": 7, - "__address__": "aws_s3_bucket.example0" - } - } - } - ] - }, - "bucket1/bucket2/bucket3/bucket.tf([{buckets.tf->bucket1/bucket.tf->bucket1/bucket2/bucket.tf#*#0#*#0}])": { - "resource": [ - { - "aws_s3_bucket": { - "example3": { - "bucket": ["bucket3"], - "acl": ["public-read"], - "__start_line__": 1, - "__end_line__": 4, - "__address__": "module.bucket1.module.bucket2.module.bucket3.aws_s3_bucket.example3" - } - } - } - ] - }, - "bucket1/bucket2/bucket.tf([{buckets.tf->bucket1/bucket.tf#*#0}])": { - "module": [ - { - "bucket3": { - "source": ["./bucket3"], - "__resolved__": ["bucket1/bucket2/bucket3/bucket.tf([{bucket1/bucket2/bucket.tf#*#0}])"], - "__start_line__": 1, - "__end_line__": 3, - "__address__": "module.bucket1.module.bucket2.bucket3" - } - } - ], - "resource": [ - { - "aws_s3_bucket": { - "example2": { - "bucket": [ - "bucket2" - ], - "__start_line__": 5, - "__end_line__": 7, - "__address__": "module.bucket1.module.bucket2.aws_s3_bucket.example2" - } - } - } - ] - }, - "bucket1/bucket.tf([{buckets.tf#*#0}])": { - "module": [ - { - "bucket2": { - "source": ["./bucket2"], - "__resolved__": ["bucket1/bucket2/bucket.tf([{bucket1/bucket.tf#*#0}])"], - "__start_line__": 1, - "__end_line__": 3, - "__address__": "module.bucket1.bucket2" - } - } - ], - "resource": [ - { - "aws_s3_bucket": { - "example1": { - "bucket": [ - "bucket1" - ], - "__start_line__": 5, - "__end_line__": 7, - "__address__": "module.bucket1.aws_s3_bucket.example1" - } - } - } - ] - } -} \ No newline at end of file diff --git a/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/expected.json b/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/expected.json deleted file mode 100644 index 82ce6934c82..00000000000 --- a/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/expected.json +++ /dev/null @@ -1,120 +0,0 @@ -{ - "/tf_module/main.tf": { - "provider": [ - {"aws": {"region": ["us-west-2"], "__start_line__": 1, "__end_line__": 3}} - ], - "module": [ - { - "s3_module": { - "source": ["./module"], - "bucket": ["${aws_s3_bucket.example.id}"], - "__start_line__": 5, - "__end_line__": 9, - "__resolved__": [ - "/tf_module/module/main.tf([{/tf_module/main.tf#*#0}])", - "/tf_module/module/variable.tf([{/tf_module/main.tf#*#0}])" - ] - } - }, - { - "s3_module2": { - "source": ["./module"], - "bucket": ["${aws_s3_bucket.example2.id}"], - "__start_line__": 11, - "__end_line__": 15, - "__resolved__": [ - "/tf_module/module/main.tf([{/tf_module/main.tf#*#1}])", - "/tf_module/module/variable.tf([{/tf_module/main.tf#*#1}])" - ] - } - } - ], - "resource": [ - { - "aws_s3_bucket": { - "example": { - "bucket": ["example"], - "__start_line__": 17, - "__end_line__": 19 - } - } - }, - { - "aws_s3_bucket": { - "example2": { - "bucket": ["example"], - "__start_line__": 21, - "__end_line__": 23 - } - } - } - ] - }, - "/tf_module/module/module2/main.tf([{/tf_module/module/main.tf#*#0}])": { - "locals": [ - {"bucket2": ["${var.bucket2}"], "__start_line__": 1, "__end_line__": 3} - ], - "resource": [ - { - "aws_s3_bucket_public_access_block": { - "var_bucket": { - "bucket": ["${local.bucket2}"], - "block_public_acls": [true], - "block_public_policy": [true], - "ignore_public_acls": [true], - "restrict_public_buckets": [true], - "__start_line__": 5, - "__end_line__": 11 - } - } - } - ] - }, - "/tf_module/module/module2/variable.tf([{/tf_module/module/main.tf#*#0}])": { - "variable": [ - {"bucket2": {"type": ["${string}"], "__start_line__": 1, "__end_line__": 3}} - ] - }, - "/tf_module/module/main.tf([{/tf_module/main.tf#*#0}])": { - "module": [ - { - "inner_s3_module": { - "source": ["./module2"], - "bucket2": ["${var.bucket}"], - "__start_line__": 1, - "__end_line__": 4, - "__resolved__": [ - "/tf_module/module/module2/main.tf([{/tf_module/module/main.tf#*#0}])", - "/tf_module/module/module2/variable.tf([{/tf_module/module/main.tf#*#0}])" - ] - } - } - ] - }, - "/tf_module/module/variable.tf([{/tf_module/main.tf#*#0}])": { - "variable": [ - {"bucket": {"type": ["${string}"], "__start_line__": 1, "__end_line__": 3}} - ] - }, - "/tf_module/module/main.tf([{/tf_module/main.tf#*#1}])": { - "module": [ - { - "inner_s3_module": { - "source": ["./module2"], - "bucket2": ["${var.bucket}"], - "__start_line__": 1, - "__end_line__": 4, - "__resolved__": [ - "/tf_module/module/module2/main.tf([{/tf_module/module/main.tf#*#0}])", - "/tf_module/module/module2/variable.tf([{/tf_module/module/main.tf#*#0}])" - ] - } - } - ] - }, - "/tf_module/module/variable.tf([{/tf_module/main.tf#*#1}])": { - "variable": [ - {"bucket": {"type": ["${string}"], "__start_line__": 1, "__end_line__": 3}} - ] - } -} diff --git a/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/main.tf b/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/main.tf deleted file mode 100644 index 8ee87ebba41..00000000000 --- a/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/main.tf +++ /dev/null @@ -1,23 +0,0 @@ -provider "aws" { - region = "us-west-2" -} - -module "s3_module" { - source = "./module" - - bucket = aws_s3_bucket.example.id -} - -module "s3_module2" { - source = "./module" - - bucket = aws_s3_bucket.example2.id -} - -resource "aws_s3_bucket" "example" { - bucket = "example" -} - -resource "aws_s3_bucket" "example2" { - bucket = "example" -} diff --git a/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/module/main.tf b/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/module/main.tf deleted file mode 100644 index 6a502f56875..00000000000 --- a/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/module/main.tf +++ /dev/null @@ -1,4 +0,0 @@ -module "inner_s3_module" { - source = "./module2" - bucket2 = var.bucket -} diff --git a/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/module/module2/main.tf b/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/module/module2/main.tf deleted file mode 100644 index 7ce60788c36..00000000000 --- a/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/module/module2/main.tf +++ /dev/null @@ -1,11 +0,0 @@ -locals { - bucket2 = var.bucket2 -} - -resource "aws_s3_bucket_public_access_block" "var_bucket" { - bucket = local.bucket2 - block_public_acls = true - block_public_policy = true - ignore_public_acls = true - restrict_public_buckets = true -} diff --git a/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/module/module2/variable.tf b/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/module/module2/variable.tf deleted file mode 100644 index 69df070abad..00000000000 --- a/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/module/module2/variable.tf +++ /dev/null @@ -1,3 +0,0 @@ -variable "bucket2" { - type = string -} \ No newline at end of file diff --git a/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/module/variable.tf b/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/module/variable.tf deleted file mode 100644 index fd81546e48e..00000000000 --- a/tests/terraform/parser/resources/parser_scenarios/nested_modules_instances_disable/tf_module/module/variable.tf +++ /dev/null @@ -1,3 +0,0 @@ -variable "bucket" { - type = string -} \ No newline at end of file diff --git a/tests/terraform/parser/test_parser_modules.py b/tests/terraform/parser/test_parser_modules.py index 4d0eab8720f..ad6441855ab 100644 --- a/tests/terraform/parser/test_parser_modules.py +++ b/tests/terraform/parser/test_parser_modules.py @@ -48,15 +48,7 @@ def test_load_registry_module(self): external_aws_modules_path = os.path.join(self.external_module_path, 'github.com/terraform-aws-modules/terraform-aws-security-group/v3.18.0') assert os.path.exists(external_aws_modules_path) - @mock.patch.dict(os.environ, {"CHECKOV_ENABLE_NESTED_MODULES": "True"}) def test_load_inner_registry_module_with_nested_modules(self): - self.load_inner_registry_module(True) - - @mock.patch.dict(os.environ, {"CHECKOV_ENABLE_NESTED_MODULES": "False"}) - def test_load_inner_registry_module_without_nested_modules(self): - self.load_inner_registry_module(False) - - def load_inner_registry_module(self, nested_modules): parser = Parser() directory = os.path.join(self.resources_dir, "registry_security_group_inner_module") self.external_module_path = os.path.join(self.tmp_path, DEFAULT_EXTERNAL_MODULES_DIR) @@ -85,14 +77,9 @@ def load_inner_registry_module(self, nested_modules): os.path.join(directory, expected_remote_module_path, f'versions.tf{TERRAFORM_NESTED_MODULE_PATH_PREFIX}{expected_inner_main_file}{TERRAFORM_NESTED_MODULE_INDEX_SEPARATOR}0{TERRAFORM_NESTED_MODULE_PATH_ENDING}'), ] - if not nested_modules: - for expected_file_name in expected_file_names: - if expected_file_name not in list(out_definitions.keys()): - self.fail(f"expected file {expected_file_name} to be in out_definitions") - else: - for expected_file_name in expected_file_names: - if not any(definition for definition in out_definitions.keys() if definition.startswith(expected_file_name[:-3])): - self.fail(f"expected file {expected_file_name} to be in out_definitions") + for expected_file_name in expected_file_names: + if not any(definition for definition in out_definitions.keys() if definition.startswith(expected_file_name[:-3])): + self.fail(f"expected file {expected_file_name} to be in out_definitions") def test_invalid_module_sources(self): parser = Parser() diff --git a/tests/terraform/runner/test_plan_runner.py b/tests/terraform/runner/test_plan_runner.py index bb1abdd924b..b4b4efa9de0 100644 --- a/tests/terraform/runner/test_plan_runner.py +++ b/tests/terraform/runner/test_plan_runner.py @@ -728,7 +728,7 @@ def test_runner_with_iam_data_block(self): failed_check_resources = {c.resource for c in report.failed_checks} self.assertEqual(failing_resources, failed_check_resources) - @mock.patch.dict(os.environ, {'CHECKOV_ENABLE_NESTED_MODULES': 'True', 'CHECKOV_EXPERIMENTAL_CROSS_VARIABLE_EDGES': 'True'}) + @mock.patch.dict(os.environ, {'CHECKOV_EXPERIMENTAL_CROSS_VARIABLE_EDGES': 'True'}) def test_plan_and_tf_combine_graph(self): tf_file_path = Path(__file__).parent / "resources/plan_and_tf_combine_graph/tfplan.json" @@ -795,30 +795,7 @@ def test_plan_and_tf_combine_graph_with_missing_resources(self): report_addresses = [report.failed_checks[0].resource_address, report.failed_checks[1].resource_address] assert sorted(expected_addresses) == sorted(report_addresses) - @mock.patch.dict(os.environ, {'CHECKOV_ENABLE_NESTED_MODULES': 'False'}) - @mock.patch.dict(os.environ, {"CHECKOV_NEW_TF_PARSER": "False"}) def test_plan_resources_ids(self): - current_dir = os.path.dirname(os.path.realpath(__file__)) - valid_plan_path = current_dir + "/resources/plan_resources_ids/tfplan.json" - valid_resources_ids = ["module.child_1_c.aws_eks_cluster.cluster", "module.child_1_b.aws_eks_cluster.cluster", - "module.child_1_a.aws_eks_cluster.cluster"] - runner = Runner() - runner.graph_registry.checks = [] - report = runner.run( - root_folder=None, - files=[valid_plan_path], - external_checks_dir=[current_dir + "/extra_yaml_checks"], - runner_filter=RunnerFilter(framework=["terraform_plan"]), - ) - self.assertGreater(report.get_summary()["failed"] + report.get_summary()["passed"], 0) - - for check in itertools.chain(report.failed_checks, report.passed_checks): - self.assertIn(check.resource, valid_resources_ids) - - self.assertEqual(len(report.resources), 3) - - @mock.patch.dict(os.environ, {'CHECKOV_ENABLE_NESTED_MODULES': 'True'}) - def test_plan_resources_ids_with_nested_modules(self): current_dir = os.path.dirname(os.path.realpath(__file__)) valid_plan_path = current_dir + "/resources/plan_resources_ids_with_nested_modules/tfplan.json" valid_resources_ids = ["module.child_0.module.child_1_c.aws_eks_cluster.cluster", @@ -839,7 +816,6 @@ def test_plan_resources_ids_with_nested_modules(self): self.assertEqual(len(report.resources), 3) - @mock.patch.dict(os.environ, {'CHECKOV_ENABLE_NESTED_MODULES': 'True'}) def test_plan_resources_created_by_modules(self): current_dir = os.path.dirname(os.path.realpath(__file__)) valid_plan_path = current_dir + "/extra_tf_plan_checks/modules.json" diff --git a/tests/terraform/runner/test_runner.py b/tests/terraform/runner/test_runner.py index 3c9b3749e92..e162222a6f5 100644 --- a/tests/terraform/runner/test_runner.py +++ b/tests/terraform/runner/test_runner.py @@ -978,7 +978,7 @@ def test_external_definitions_context(self): def test_failure_in_resolved_module(self): current_dir = os.path.dirname(os.path.realpath(__file__)) - valid_dir_path = os.path.join(current_dir, "../parser/resources/parser_scenarios/module_matryoshka") + valid_dir_path = os.path.join(current_dir, "../parser/resources/parser_scenarios/module_matryoshka_nested_module_enable") valid_dir_path = os.path.normpath(valid_dir_path) runner = Runner(db_connector=self.db_connector()) checks_allowlist = ['CKV_AWS_20'] @@ -1111,29 +1111,7 @@ def test_record_relative_path_with_abs_file(self): # no need to join with a '/' because the TF runner adds it to the start of the file path self.assertEqual(record.repo_file_path, f'/{file_rel_path}') - @mock.patch.dict(os.environ, {"CHECKOV_ENABLE_NESTED_MODULES": "False"}) def test_record_definition_context_path(self): - if self.use_new_tf_parser == "True": - # We assume CHECKOV_ENABLE_NESTED_MODULES is True for the case of the new TF parser - return - resources_path = os.path.join( - os.path.dirname(os.path.realpath(__file__)), "resources", "definition_context_path_nested_modules") - checks_allow_list = ['CKV_AWS_20'] - main_path = os.path.join(resources_path, 'main.tf') - expected_definition_context_paths = [main_path, - f'{os.path.join(resources_path, "module/main.tf")}{TERRAFORM_NESTED_MODULE_PATH_PREFIX}{main_path}{TERRAFORM_NESTED_MODULE_INDEX_SEPARATOR}0{TERRAFORM_NESTED_MODULE_PATH_ENDING}', - f'{os.path.join(resources_path, "module/module2/main.tf")}{TERRAFORM_NESTED_MODULE_PATH_PREFIX}{main_path}->{os.path.join(resources_path, "module/main.tf")}{TERRAFORM_NESTED_MODULE_INDEX_SEPARATOR}0{TERRAFORM_NESTED_MODULE_PATH_ENDING}'] - expected_definition_context_paths.sort() - - runner = Runner(db_connector=self.db_connector()) - report = runner.run(root_folder=resources_path, external_checks_dir=None, - runner_filter=RunnerFilter(framework=["terraform"], checks=checks_allow_list)) - definition_context_paths = [f.definition_context_file_path for f in report.failed_checks] - definition_context_paths.sort() - self.assertEqual(expected_definition_context_paths, definition_context_paths) - - @mock.patch.dict(os.environ, {"CHECKOV_ENABLE_NESTED_MODULES": "True"}) - def test_record_definition_context_path_with_nested_module(self): resources_path = os.path.join( os.path.dirname(os.path.realpath(__file__)), "resources", "definition_context_path_nested_modules") checks_allow_list = ['CKV_AWS_20'] @@ -1204,7 +1182,6 @@ def test_module_skip(self): self.assertFalse(found_inside) self.assertFalse(found_outside) - @mock.patch.dict(os.environ, {"CHECKOV_ENABLE_NESTED_MODULES": "True"}) def test_nested_modules_caller_file(self): current_dir = os.path.dirname(os.path.realpath(__file__)) report = Runner(db_connector=self.db_connector()).run( @@ -1357,7 +1334,6 @@ def test_entity_context_fetching(self): assert entity_context is not None assert entity_context['start_line'] == 1 and entity_context['end_line'] == 7 - @mock.patch.dict(os.environ, {"CHECKOV_ENABLE_NESTED_MODULES": "True"}) def test_resource_ids_nested_modules(self): resources_path = os.path.join( os.path.dirname(os.path.realpath(__file__)), "resources", "resource_ids_nested_modules")