From 3e46e49582b38729a5c8f290a52f5b8cba90aedb Mon Sep 17 00:00:00 2001 From: Paddy Morgan Date: Fri, 15 Nov 2024 22:39:21 +0000 Subject: [PATCH] fix(terraform): Update CKV_AZURE_167 to correct check on retention policy (#6758) * fix: Update CKV_AZURE_167 to correct check on retention policy * Include old method --------- Co-authored-by: Taylor Co-authored-by: Taylor <28880387+tsmithv11@users.noreply.github.com> --- .../azure/ACREnableRetentionPolicy.py | 23 ++++++++++--- .../example_ACREnableRetentionPolicy/main.tf | 32 ++++++++++++------- .../azure/test_ACREnableRetentionPolicy.py | 7 ++-- 3 files changed, 42 insertions(+), 20 deletions(-) diff --git a/checkov/terraform/checks/resource/azure/ACREnableRetentionPolicy.py b/checkov/terraform/checks/resource/azure/ACREnableRetentionPolicy.py index bb1a62af838..11091abe65b 100644 --- a/checkov/terraform/checks/resource/azure/ACREnableRetentionPolicy.py +++ b/checkov/terraform/checks/resource/azure/ACREnableRetentionPolicy.py @@ -1,8 +1,10 @@ -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" @@ -10,8 +12,19 @@ def __init__(self) -> None: 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 index 7120aaefd5d..2261a566035 100644 --- 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 index 19f36d0d9b8..5b48b58dd87 100644 --- 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() \ No newline at end of file + unittest.main()