fix(terraform): Update CKV_AZURE_167 to correct check on retention po…

…licy (#6758) * fix: Update CKV_AZURE_167 to correct check on retention policy * Include old method --------- Co-authored-by: Taylor <[email protected]> Co-authored-by: Taylor <[email protected]>
bridgecrewio · Nov 15, 2024 · 3e46e49 · 3e46e49
1 parent 25333ca
commit 3e46e49
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 20 deletions.
diff --git a/checkov/terraform/checks/resource/azure/ACREnableRetentionPolicy.py b/checkov/terraform/checks/resource/azure/ACREnableRetentionPolicy.py
@@ -1,17 +1,30 @@
-from checkov.common.models.enums import CheckCategories
-from checkov.terraform.checks.resource.base_resource_value_check import BaseResourceValueCheck
+from typing import Dict, List, Any
 
+from checkov.common.models.enums import CheckResult, CheckCategories
+from checkov.terraform.checks.resource.base_resource_check import BaseResourceCheck
 
-class ACREnableRetentionPolicy(BaseResourceValueCheck):
+
+class ACREnableRetentionPolicy(BaseResourceCheck):
     def __init__(self) -> None:
         name = "Ensure a retention policy is set to cleanup untagged manifests."
         id = "CKV_AZURE_167"
         supported_resources = ("azurerm_container_registry",)
         categories = (CheckCategories.GENERAL_SECURITY,)
         super().__init__(name=name, id=id, categories=categories, supported_resources=supported_resources)
 
-    def get_inspected_key(self) -> str:
-        return "retention_policy/enabled"
+    def scan_resource_conf(self, conf: Dict[str, List[Any]]) -> CheckResult:
+        if 'retention_policy_in_days' in conf:
+            return CheckResult.PASSED
+
+        if 'retention_policy' in conf:
+            retention_policy = conf['retention_policy'][0]
+            if isinstance(retention_policy, dict) and retention_policy.get('enabled') == [True]:
+                return CheckResult.PASSED
+
+        return CheckResult.FAILED
+
+    def get_evaluated_keys(self) -> List[str]:
+        return ['retention_policy_in_days', 'retention_policy/enabled']
 
 
 check = ACREnableRetentionPolicy()
diff --git a/tests/terraform/checks/resource/azure/example_ACREnableRetentionPolicy/main.tf b/tests/terraform/checks/resource/azure/example_ACREnableRetentionPolicy/main.tf
@@ -1,5 +1,4 @@
-
-resource "azurerm_container_registry" "pass" {
+resource "azurerm_container_registry" "pass_old" {
   name                      = "containerRegistry1"
   resource_group_name       = azurerm_resource_group.rg.name
   location                  = azurerm_resource_group.rg.location
@@ -11,22 +10,31 @@ resource "azurerm_container_registry" "pass" {
   }
 }
 
-
-resource "azurerm_container_registry" "fail" {
-  name                = "containerRegistry1"
-  resource_group_name = azurerm_resource_group.rg.name
-  location            = azurerm_resource_group.rg.location
-  sku                 = "Premium"
+resource "azurerm_container_registry" "pass_new" {
+  name                      = "containerRegistry1"
+  resource_group_name       = azurerm_resource_group.rg.name
+  location                  = azurerm_resource_group.rg.location
+  sku                       = "Premium"
+  anonymous_pull_enabled    = false
+  quarantine_policy_enabled = true
+  retention_policy_in_days  = 7
 }
 
-
-resource "azurerm_container_registry" "fail2" {
+resource "azurerm_container_registry" "fail_old" {
   name                      = "containerRegistry1"
   resource_group_name       = azurerm_resource_group.rg.name
   location                  = azurerm_resource_group.rg.location
-  sku                       = "Standard"
-  quarantine_policy_enabled = false
+  sku                       = "Premium"
+  anonymous_pull_enabled    = false
+  quarantine_policy_enabled = true
   retention_policy {
     enabled = false
   }
 }
+
+resource "azurerm_container_registry" "fail" {
+  name                = "containerRegistry1"
+  resource_group_name = azurerm_resource_group.rg.name
+  location            = azurerm_resource_group.rg.location
+  sku                 = "Premium"
+}
diff --git a/tests/terraform/checks/resource/azure/test_ACREnableRetentionPolicy.py b/tests/terraform/checks/resource/azure/test_ACREnableRetentionPolicy.py
@@ -18,11 +18,12 @@ def test(self):
         summary = report.get_summary()
 
         passing_resources = {
-            'azurerm_container_registry.pass',
+            'azurerm_container_registry.pass_old',
+            'azurerm_container_registry.pass_new',
         }
         failing_resources = {
             'azurerm_container_registry.fail',
-            'azurerm_container_registry.fail2'
+            'azurerm_container_registry.fail_old',
         }
         skipped_resources = {}
 
@@ -39,4 +40,4 @@ def test(self):
 
 
 if __name__ == '__main__':
-    unittest.main()
+    unittest.main()