fix(terraform): check if the dynamic name is one of the resources blo…

…ck (#5607) * check if the dynamic name is one of the resources block * . * Update tests/terraform/graph/variable_rendering/test_renderer.py Co-authored-by: Barak Fatal <[email protected]> --------- Co-authored-by: Barak Fatal <[email protected]>
bridgecrewio · Oct 5, 2023 · 1927999 · 1927999
1 parent 1b66c38
commit 1927999
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 10 deletions.
diff --git a/checkov/terraform/parser_functions.py b/checkov/terraform/parser_functions.py
@@ -5,6 +5,7 @@
 from collections.abc import Hashable
 from typing import Dict, List, Union, Any, Callable
 
+from checkov.common.util.data_structures_utils import pickle_deepcopy
 from checkov.common.util.type_forcers import convert_str_to_bool
 from checkov.common.util.parser_utils import eval_string, split_merge_args, string_to_native, to_string
 
@@ -199,7 +200,16 @@ def process_dynamic_values(conf: Dict[str, List[Any]]) -> bool:
 
         for element_name, element_value in dynamic_element.items():
             if "content" in element_value:
-                conf[element_name] = element_value["content"]
+                if element_name in conf:
+                    if not isinstance(conf[element_name], list):
+                        conf[element_name] = [conf[element_name]]
+                    if isinstance(element_value["content"], list):
+                        conf[element_name].extend(element_value["content"])
+                    else:
+                        conf[element_name].append(element_value["content"])
+
+                else:
+                    conf[element_name] = pickle_deepcopy(element_value["content"])
             else:
                 # this should be the result of a successful dynamic block rendering
                 # in some cases a whole dict is added, which doesn't have a list around it

diff --git a/tests/terraform/checks/resource/aws/example_S3AbortIncompleteUploads/main.tf b/tests/terraform/checks/resource/aws/example_S3AbortIncompleteUploads/main.tf
@@ -197,3 +197,29 @@ resource "aws_s3_bucket_lifecycle_configuration" "pass3" {
     status = "Enabled"
   }
 }
+
+resource "aws_s3_bucket_lifecycle_configuration" "resource_with_dynamic_rule_pass4" {
+  bucket = aws_s3_bucket.main.bucket
+
+  rule {
+    id     = "abort_incomplete_multipart_upload"
+    status = "Enabled"
+
+    abort_incomplete_multipart_upload {
+      days_after_initiation = var.config.abort_incomplete_multipart_upload
+    }
+  }
+
+  dynamic "rule" {
+    for_each = local.lifecycle_rules.storage_class
+
+    content {
+      id     = "storage_class_is_${var.config.storage_class}"
+      status = "Enabled"
+
+      transition {
+        storage_class = var.config.storage_class
+      }
+    }
+  }
+}
diff --git a/tests/terraform/checks/resource/aws/test_S3AbortIncompleteUploads.py b/tests/terraform/checks/resource/aws/test_S3AbortIncompleteUploads.py
@@ -22,6 +22,7 @@ def test(self):
             "aws_s3_bucket_lifecycle_configuration.pass",
             "aws_s3_bucket_lifecycle_configuration.pass2",
             "aws_s3_bucket_lifecycle_configuration.pass3",
+            "aws_s3_bucket_lifecycle_configuration.resource_with_dynamic_rule_pass4"
         }
         failing_resources = {
             "aws_s3_bucket_lifecycle_configuration.fail",
@@ -40,6 +41,5 @@ def test(self):
         self.assertEqual(passing_resources, passed_check_resources)
         self.assertEqual(failing_resources, failed_check_resources)
 
-
 if __name__ == "__main__":
     unittest.main()
diff --git a/tests/terraform/checks/resource/gcp/example_GoogleCloudSqlDatabasePubliclyAccessible/main.tf b/tests/terraform/checks/resource/gcp/example_GoogleCloudSqlDatabasePubliclyAccessible/main.tf
@@ -59,22 +59,22 @@ resource "google_sql_database_instance" "instance4-fail" {
     ip_configuration {
 
       dynamic "authorized_networks" {
-        for_each = google_compute_instance.apps
-        iterator = apps
+        for_each = local.onprem
+        iterator = onprem
 
         content {
-          name  = apps.value.name
-          value = apps.value.network_interface.0.access_config.0.nat_ip
+          name  = "onprem-${onprem.key}"
+          value = "0.0.0.0/0"
         }
       }
 
       dynamic "authorized_networks" {
-        for_each = local.onprem
-        iterator = onprem
+        for_each = google_compute_instance.apps
+        iterator = apps
 
         content {
-          name  = "onprem-${onprem.key}"
-          value = "0.0.0.0/0"
+          name  = apps.value.name
+          value = apps.value.network_interface.0.access_config.0.nat_ip
         }
       }
     }

diff --git a/tests/terraform/checks/resource/test_base_resource_dynamic_value_check.py b/tests/terraform/checks/resource/test_base_resource_dynamic_value_check.py
@@ -135,3 +135,5 @@ def tearDown(self) -> None:
         super().tearDown()
         resource_registry.checks = self.check_list_before
         self.check_list_before = None
+
+
diff --git a/tests/terraform/graph/variable_rendering/test_renderer.py b/tests/terraform/graph/variable_rendering/test_renderer.py
@@ -470,3 +470,18 @@ def test_provider_alias(self):
 
         provider_alias = next(vertex for vertex in local_graph.vertices if vertex.block_type == BlockType.PROVIDER and vertex.name == "aws.test")
         assert provider_alias.config["aws"]["default_tags"] == [{"tags": [{"test": "Test"}]}]
+
+    def test_multiple_dynamic_blocks_value_not_supporting(self):
+        resource_paths = [
+            os.path.join(TEST_DIRNAME, 'test_resources', 'multiple_dynamic_blocks'),
+        ]
+        for path in resource_paths:
+            graph_manager = TerraformGraphManager('m', ['m'])
+            local_graph, _ = graph_manager.build_graph_from_source_directory(path, render_variables=True)
+
+            resources_vertex = list(filter(lambda v: v.block_type == BlockType.RESOURCE and v.has_dynamic_block, local_graph.vertices))
+            value_block_1 = resources_vertex[0].config['google_sql_database_instance']['instance4-should-fail']['settings'][0]['ip_configuration'][0]['dynamic'][0]['authorized_networks']['content'][0]['value']
+            value_block_2 = resources_vertex[0].config['google_sql_database_instance']['instance4-should-fail']['settings'][0]['ip_configuration'][0][
+                'dynamic'][1]['authorized_networks']['content'][0]['value']
+            # TODO - for now we don't support multiple dynamic blocks - the value_block_1 and value_block_2 needs to be diffrent and not overide each other
+            assert not value_block_1 != value_block_2
diff --git a/tests/terraform/graph/variable_rendering/test_resources/multiple_dynamic_blocks/main.tf b/tests/terraform/graph/variable_rendering/test_resources/multiple_dynamic_blocks/main.tf
@@ -0,0 +1,31 @@
+resource "google_sql_database_instance" "instance4-should-fail" {
+  name             = "instance"
+  database_version = "POSTGRES_11"
+
+  settings {
+    tier = "db-f1-micro"
+
+    ip_configuration {
+
+      dynamic "authorized_networks" {
+        for_each = google_compute_instance.apps
+        iterator = apps
+
+        content {
+          name  = apps.value.name
+          value = apps.value.network_interface.0.access_config.0.nat_ip
+        }
+      }
+
+      dynamic "authorized_networks" {
+        for_each = local.onprem
+        iterator = onprem
+
+        content {
+          name  = "onprem-${onprem.key}"
+          value = "0.0.0.0/0"
+        }
+      }
+    }
+  }
+}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -135,3 +135,5 @@ def tearDown(self) -> None:
		super().tearDown()
		resource_registry.checks = self.check_list_before
		self.check_list_before = None