fix: Updated firewall module with resource prefix naming and added feature #3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…isted resource in main.tf
Summary of ChangesHello @ravimalvia10, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the Azure Firewall Terraform module by introducing advanced naming flexibility through a new 'resource_position_prefix' variable and enhancing configurability with dynamic blocks for IP and rule management. It integrates comprehensive resource definitions for Azure Firewall, its policies, and rule collections, along with diagnostic settings. The accompanying documentation and examples have been thoroughly updated and expanded to guide users through the new features and demonstrate various deployment scenarios, including isolated rule sets and public IP prefix usage. Additionally, Dependabot has been set up to maintain the module's dependencies. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🛡️ Checkov Security Scan: 4 Issues Found🔴 [CKV_AZURE_216] Ensure DenyIntelMode is set to Deny for Azure Firewalls resource "azurerm_firewall" "firewall" {
count = var.enabled && var.firewall_enable ? 1 : 0
name = format(var.resource_position_prefix ? "firewall-%s" : "%s-firewall", local.name)
location = var.location
resource_group_name = var.resource_group_name
threat_intel_mode = var.threat_intel_mode
sku_tier = var.sku_tier
sku_name = var.sku_name
firewall_policy_id = join("", azurerm_firewall_policy.policy.*.id)
tags = module.labels.tags
private_ip_ranges = var.firewall_private_ip_ranges
dns_servers = var.dns_servers
# Primary ip_configuration (requires subnet_id)
ip_configuration {
name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, var.primary_public_ip_name)
subnet_id = var.subnet_id
public_ip_address_id = azurerm_public_ip.primary_public_ip[0].id #azurerm_public_ip.primary_public_ip[0].id
}
# Additional ip_configurations for the remaining PIPs
dynamic "ip_configuration" {
for_each = { for k, v in azurerm_public_ip.public_ip : k => v if k != var.primary_public_ip_name }
content {
name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, ip_configuration.key)
public_ip_address_id = ip_configuration.value.id
# subnet_id omitted by design for additional PIPsn
}
}
lifecycle {
ignore_changes = [tags]
}
}🔴 [CKV_AZURE_220] Ensure Firewall policy has IDPS mode as deny resource "azurerm_firewall_policy" "policy" {
count = var.enabled && var.firewall_enable ? 1 : 0
name = format(var.resource_position_prefix ? "firewall-Policy-%s" : "%s-firewall-Policy", local.name)
resource_group_name = var.resource_group_name
location = var.location
sku = var.sku_policy
dynamic "identity" {
for_each = var.identity_type != null && var.sku_policy == "Premium" && var.sku_tier == "Premium" ? [1] : []
content {
type = var.identity_type
identity_ids = var.identity_type == "UserAssigned" ? [join("", azurerm_user_assigned_identity.identity.*.id)] : null
}
}
}🔴 [CKV_AZURE_216] Ensure DenyIntelMode is set to Deny for Azure Firewalls resource "azurerm_firewall" "firewall" {
count = var.enabled && var.firewall_enable ? 1 : 0
name = format(var.resource_position_prefix ? "firewall-%s" : "%s-firewall", local.name)
location = var.location
resource_group_name = var.resource_group_name
threat_intel_mode = var.threat_intel_mode
sku_tier = var.sku_tier
sku_name = var.sku_name
firewall_policy_id = join("", azurerm_firewall_policy.policy.*.id)
tags = module.labels.tags
private_ip_ranges = var.firewall_private_ip_ranges
dns_servers = var.dns_servers
# Primary ip_configuration (requires subnet_id)
ip_configuration {
name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, var.primary_public_ip_name)
subnet_id = var.subnet_id
public_ip_address_id = azurerm_public_ip.primary_public_ip[0].id #azurerm_public_ip.primary_public_ip[0].id
}
# Additional ip_configurations for the remaining PIPs
dynamic "ip_configuration" {
for_each = { for k, v in azurerm_public_ip.public_ip : k => v if k != var.primary_public_ip_name }
content {
name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, ip_configuration.key)
public_ip_address_id = ip_configuration.value.id
# subnet_id omitted by design for additional PIPsn
}
}
lifecycle {
ignore_changes = [tags]
}
}🔴 [CKV_AZURE_220] Ensure Firewall policy has IDPS mode as deny resource "azurerm_firewall_policy" "policy" {
count = var.enabled && var.firewall_enable ? 1 : 0
name = format(var.resource_position_prefix ? "firewall-Policy-%s" : "%s-firewall-Policy", local.name)
resource_group_name = var.resource_group_name
location = var.location
sku = var.sku_policy
dynamic "identity" {
for_each = var.identity_type != null && var.sku_policy == "Premium" && var.sku_tier == "Premium" ? [1] : []
content {
type = var.identity_type
identity_ids = var.identity_type == "UserAssigned" ? [join("", azurerm_user_assigned_identity.identity.*.id)] : null
}
}
} |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
116
to
129
| resource "azurerm_firewall_policy" "policy" { | ||
| count = var.enabled && var.firewall_enable ? 1 : 0 | ||
| name = format(var.resource_position_prefix ? "firewall-Policy-%s" : "%s-firewall-Policy", local.name) | ||
| resource_group_name = var.resource_group_name | ||
| location = var.location | ||
| sku = var.sku_policy | ||
| dynamic "identity" { | ||
| for_each = var.identity_type != null && var.sku_policy == "Premium" && var.sku_tier == "Premium" ? [1] : [] | ||
| content { | ||
| type = var.identity_type | ||
| identity_ids = var.identity_type == "UserAssigned" ? [join("", azurerm_user_assigned_identity.identity.*.id)] : null | ||
| } | ||
| } | ||
| } |
Check failure
Code scanning / checkov
Ensure Firewall policy has IDPS mode as deny Error
Comment on lines
116
to
129
| resource "azurerm_firewall_policy" "policy" { | ||
| count = var.enabled && var.firewall_enable ? 1 : 0 | ||
| name = format(var.resource_position_prefix ? "firewall-Policy-%s" : "%s-firewall-Policy", local.name) | ||
| resource_group_name = var.resource_group_name | ||
| location = var.location | ||
| sku = var.sku_policy | ||
| dynamic "identity" { | ||
| for_each = var.identity_type != null && var.sku_policy == "Premium" && var.sku_tier == "Premium" ? [1] : [] | ||
| content { | ||
| type = var.identity_type | ||
| identity_ids = var.identity_type == "UserAssigned" ? [join("", azurerm_user_assigned_identity.identity.*.id)] : null | ||
| } | ||
| } | ||
| } |
Check failure
Code scanning / checkov
Ensure Firewall policy has IDPS mode as deny Error
diffDiff--- a/examples/complete/plan-target.txt
+++ b/examples/complete/plan-pr.txt
@@ -1,5 +1,437 @@
-No changes. Your infrastructure matches the configuration.
+Terraform used the selected providers to generate the following execution
+plan. Resource actions are indicated with the following symbols:
+ + create
-Terraform has compared your real infrastructure against your configuration
-and found no differences, so no changes are needed.
+Terraform will perform the following actions:
+
+ # module.firewall.azurerm_firewall.firewall[0] will be created
+ + resource "azurerm_firewall" "firewall" {
+ + dns_proxy_enabled = (known after apply)
+ + firewall_policy_id = (known after apply)
+ + id = (known after apply)
+ + location = "eastus"
+ + name = "firewall-app-test-eus"
+ + resource_group_name = "rg-app-test"
+ + sku_name = "AZFW_VNet"
+ + sku_tier = "Standard"
+ + tags = {
+ + "Business_unit" = "corp"
+ + "Deployment_mode" = "terraform"
+ + "Environment" = "test"
+ + "Managedby" = "terraform-az-modules"
+ + "Name" = "app-test-eus"
+ + "Repository" = "https://github.com/terraform-az-modules/terraform-azure-firewall"
+ }
+ + threat_intel_mode = "Alert"
+
+ + ip_configuration {
+ + name = "ipconfig-app-test-eus-ingress"
+ + private_ip_address = (known after apply)
+ + public_ip_address_id = (known after apply)
+ + subnet_id = (known after apply)
+ }
+ + ip_configuration {
+ + name = "ipconfig-app-test-eus-app"
+ + private_ip_address = (known after apply)
+ + public_ip_address_id = (known after apply)
+ }
+ + ip_configuration {
+ + name = "ipconfig-app-test-eus-vnet"
+ + private_ip_address = (known after apply)
+ + public_ip_address_id = (known after apply)
+ }
+ }
+
+ # module.firewall.azurerm_firewall_policy.policy[0] will be created
+ + resource "azurerm_firewall_policy" "policy" {
+ + child_policies = (known after apply)
+ + firewalls = (known after apply)
+ + id = (known after apply)
+ + location = "eastus"
+ + name = "firewall-Policy-app-test-eus"
+ + resource_group_name = "rg-app-test"
+ + rule_collection_groups = (known after apply)
+ + sku = "Standard"
+ + threat_intelligence_mode = "Alert"
+ }
+
+ # module.firewall.azurerm_firewall_policy_rule_collection_group.app_policy_rule_collection_group[0] will be created
+ + resource "azurerm_firewall_policy_rule_collection_group" "app_policy_rule_collection_group" {
+ + firewall_policy_id = (known after apply)
+ + id = (known after apply)
+ + name = "DefaultApplicationRuleCollectionGroup"
+ + priority = 300
+
+ + application_rule_collection {
+ + action = "Allow"
+ + name = "example_app_policy"
+ + priority = 200
+
+ + rule {
+ + destination_fqdns = [
+ + "*",
+ ]
+ + name = "app_test"
+ + source_addresses = [
+ + "*",
+ ]
+ + source_ip_groups = []
+
+ + protocols {
+ + port = 443
+ + type = "Https"
+ }
+ + protocols {
+ + port = 80
+ + type = "Http"
+ }
+ }
+ }
+ }
+
+ # module.firewall.azurerm_firewall_policy_rule_collection_group.nat_policy_rule_collection_group[0] will be created
+ + resource "azurerm_firewall_policy_rule_collection_group" "nat_policy_rule_collection_group" {
+ + firewall_policy_id = (known after apply)
+ + id = (known after apply)
+ + name = "DefaultDnatRuleCollectionGroup"
+ + priority = 100
+
+ + nat_rule_collection {
+ + action = "Dnat"
+ + name = "web_server_nat_policy"
+ + priority = 100
+
+ + rule {
+ + destination_address = (known after apply)
+ + destination_ports = [
+ + "8080",
+ ]
+ + name = "web_server_nat"
+ + protocols = [
+ + "TCP",
+ ]
+ + source_addresses = [
+ + "*",
+ ]
+ + translated_address = "10.0.1.20"
+ + translated_port = 80
+ }
+ }
+ }
+
+ # module.firewall.azurerm_firewall_policy_rule_collection_group.network_policy_rule_collection_group[0] will be created
+ + resource "azurerm_firewall_policy_rule_collection_group" "network_policy_rule_collection_group" {
+ + firewall_policy_id = (known after apply)
+ + id = (known after apply)
+ + name = "DefaultNetworkRuleCollectionGroup"
+ + priority = 200
+
+ + network_rule_collection {
+ + action = "Allow"
+ + name = "example_network_policy"
+ + priority = 100
+
+ + rule {
+ + destination_addresses = [
+ + "*",
+ ]
+ + destination_fqdns = []
+ + destination_ip_groups = []
+ + destination_ports = [
+ + "22",
+ ]
+ + name = "ssh"
+ + protocols = [
+ + "TCP",
+ ]
+ + source_addresses = [
+ + "*",
+ ]
+ + source_ip_groups = []
+ }
+ }
+ + network_rule_collection {
+ + action = "Allow"
+ + name = "example_network_policy-2"
+ + priority = 101
+
+ + rule {
+ + destination_addresses = [
+ + "*",
+ ]
+ + destination_fqdns = []
+ + destination_ip_groups = []
+ + destination_ports = [
+ + "587",
+ ]
+ + name = "smtp"
+ + protocols = [
+ + "TCP",
+ ]
+ + source_addresses = [
+ + "*",
+ ]
+ + source_ip_groups = []
+ }
+ }
+ }
+
+ # module.firewall.azurerm_monitor_diagnostic_setting.firewall_diagnostic_setting[0] will be created
+ + resource "azurerm_monitor_diagnostic_setting" "firewall_diagnostic_setting" {
+ + id = (known after apply)
+ + log_analytics_destination_type = "AzureDiagnostics"
+ + log_analytics_workspace_id = (known after apply)
+ + name = "firewall-diagnostic-log-app-test-eus"
+ + target_resource_id = (known after apply)
+
+ + enabled_log {
+ + category = "AzureFirewallApplicationRule"
+ # (1 unchanged attribute hidden)
+ }
+ + enabled_log {
+ + category = "AzureFirewallDnsProxy"
+ # (1 unchanged attribute hidden)
+ }
+ + enabled_log {
+ + category = "AzureFirewallNetworkRule"
+ # (1 unchanged attribute hidden)
+ }
+
+ + enabled_metric (known after apply)
+
+ + metric (known after apply)
+ }
+
+ # module.firewall.azurerm_public_ip.primary_public_ip[0] will be created
+ + resource "azurerm_public_ip" "primary_public_ip" {
+ + allocation_method = "Static"
+ + ddos_protection_mode = "VirtualNetworkInherited"
+ + fqdn = (known after apply)
+ + id = (known after apply)
+ + idle_timeout_in_minutes = 4
+ + ip_address = (known after apply)
+ + ip_version = "IPv4"
+ + location = "eastus"
+ + name = "ip-app-test-eus-ingress"
+ + resource_group_name = "rg-app-test"
+ + sku = "Standard"
+ + sku_tier = "Regional"
+ + tags = {
+ + "Business_unit" = "corp"
+ + "Deployment_mode" = "terraform"
+ + "Environment" = "test"
+ + "Managedby" = "terraform-az-modules"
+ + "Name" = "app-test-eus"
+ + "Repository" = "https://github.com/terraform-az-modules/terraform-azure-firewall"
+ }
+ }
+
+ # module.firewall.azurerm_public_ip.public_ip["app"] will be created
+ + resource "azurerm_public_ip" "public_ip" {
+ + allocation_method = "Static"
+ + ddos_protection_mode = "VirtualNetworkInherited"
+ + fqdn = (known after apply)
+ + id = (known after apply)
+ + idle_timeout_in_minutes = 4
+ + ip_address = (known after apply)
+ + ip_version = "IPv4"
+ + location = "eastus"
+ + name = "pip-app-test-eus-app"
+ + public_ip_prefix_id = (known after apply)
+ + resource_group_name = "rg-app-test"
+ + sku = "Standard"
+ + sku_tier = "Regional"
+ + tags = {
+ + "Business_unit" = "corp"
+ + "Deployment_mode" = "terraform"
+ + "Environment" = "test"
+ + "Managedby" = "terraform-az-modules"
+ + "Name" = "app-test-eus"
+ + "Repository" See the full diff from https://github.com/terraform-az-modules/terraform-azure-firewall/actions/runs/18016338540 |
ravimalvia10
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly refactors the firewall module, introducing a standardized naming convention with resource_position_prefix, adding support for public IP prefixes, and expanding the documentation with new examples. These are great improvements for usability and consistency. However, the review identified a critical issue in the public IP creation logic that could lead to resource duplication. There are also several medium-severity issues, including numerous copy-paste errors in variable descriptions and documentation (referencing 'NSG' or 'Key Vault'), incorrect default values in descriptions, and errors within the new example code that would prevent them from executing successfully. Addressing these issues will be crucial for ensuring the module is robust, reliable, and easy for others to use.
main.tf
Outdated
Comment on lines
42
to
54
| resource "azurerm_public_ip" "primary_public_ip" { | ||
| count = var.enabled && var.firewall_enable ? 1 : 0 | ||
| name = format(var.resource_position_prefix ? "ip-%s-%s" : "%s-%s-ip", local.name, var.primary_public_ip_name) | ||
| location = var.location | ||
| resource_group_name = var.resource_group_name | ||
| allocation_method = var.public_ip_allocation_method | ||
| sku = var.public_ip_sku | ||
| ddos_protection_mode = "VirtualNetworkInherited" | ||
| tags = module.labels.tags | ||
| lifecycle { | ||
| create_before_destroy = true | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic for creating public IPs is flawed. This azurerm_public_ip resource for the primary IP is created separately from the IPs defined in var.public_ip_names. If var.primary_public_ip_name is also included in var.public_ip_names (as the variable description implies), you will create a duplicate public IP, leading to unnecessary costs and confusion.
A better approach is to create all public IPs using a single resource block with for_each over var.public_ip_names, and then select the primary IP from the resulting map of resources. This avoids duplication and simplifies the logic.
I recommend removing this resource block and modifying the azurerm_firewall resource to select the primary IP from the main azurerm_public_ip.public_ip map.
Comment on lines
161
to
176
| nat_rule_collection = [ | ||
| { | ||
| name = "example_nat_policy-1" | ||
| priority = "101" | ||
| rules = [ | ||
| { | ||
| name = "nat_rule_collection1_rule1" | ||
| protocols = ["TCP", "UDP"] | ||
| source_addresses = ["10.0.0.1", "10.0.0.2"] | ||
| destination_ports = ["80"] | ||
| translated_address = "192.168.0.1" | ||
| translated_port = "8080" | ||
| }, | ||
| ] | ||
| }, | ||
| ] |
There was a problem hiding this comment.
The nat_rule_collection in this example is missing the destination_address for its rule. A DNAT rule requires a public destination IP address to translate traffic from. Without it, the rule is ineffective. Please add the destination_address, likely referencing one of the firewall's public IPs (e.g., module.firewall.primary_public_ip_address).
nat_rule_collection = [
{
name = "example_nat_policy-1"
priority = 101
rules = [
{
name = "nat_rule_collection1_rule1"
protocols = ["TCP", "UDP"]
source_addresses = ["10.0.0.1", "10.0.0.2"]
destination_address = module.firewall.primary_public_ip_address
destination_ports = ["80"]
translated_address = "192.168.0.1"
translated_port = "8080"
},
]
},
]
| network_rule_collection = [ | ||
| { | ||
| name = "example_network_policy" | ||
| priority = "100" |
There was a problem hiding this comment.
The priority for this network_rule_collection is set as a string ("100"), but the module's variable definition expects a number. This will cause a type mismatch error when the example is used. Please remove the quotes.
priority = 100
Comment on lines
+73
to
+74
| # Allow up to 3 open pull requests for pip dependencies | ||
| open-pull-requests-limit: 3 |
There was a problem hiding this comment.
This comment appears to be a copy-paste from a pip dependency configuration. It should be updated to be relevant for Terraform dependencies or removed for clarity.
# Allow up to 3 open pull requests for terraform dependencies
open-pull-requests-limit: 3
Comment on lines
+59
to
+60
| # Allow up to 3 open pull requests for pip dependencies | ||
| open-pull-requests-limit: 3 |
There was a problem hiding this comment.
This comment appears to be a copy-paste from a pip dependency configuration. It should be updated to be relevant for Terraform dependencies or removed for clarity.
# Allow up to 3 open pull requests for terraform dependencies
open-pull-requests-limit: 3
Comment on lines
56
to
66
| variable "repository" { | ||
| type = string | ||
| default = "https://github.com/terraform-az-modules/terraform-azure-key-vault" | ||
| description = "Terraform current module repo" | ||
|
|
||
| validation { | ||
| # regex(...) fails if it cannot find a match | ||
| condition = can(regex("^https://", var.repository)) | ||
| error_message = "The module-repo value must be a valid Git repo link." | ||
| } | ||
| } |
There was a problem hiding this comment.
The default value for the repository variable points to the terraform-azure-key-vault repository. This should be updated to point to the correct terraform-azure-firewall repository.
variable "repository" {
type = string
default = "https://github.com/terraform-az-modules/terraform-azure-firewall"
description = "Terraform current module repo"
validation {
# regex(...) fails if it cannot find a match
condition = can(regex("^https://", var.repository))
error_message = "The module-repo value must be a valid Git repo link."
}
}
Comment on lines
129
to
133
| variable "public_ip_sku" { | ||
| description = "The SKU of the Public IP. Accepted values are Basic and Standard. Defaults to Basic" | ||
| default = "Standard" | ||
| type = string | ||
| } |
There was a problem hiding this comment.
The description for public_ip_sku states that it 'Defaults to Basic', but the actual default value is "Standard". The description should be corrected to match the default value to avoid confusion.
variable "public_ip_sku" {
description = "The SKU of the Public IP. Accepted values are Basic and Standard. Defaults to Standard"
default = "Standard"
type = string
}
Comment on lines
147
to
151
| variable "public_ip_prefix_length" { | ||
| type = number | ||
| default = 31 | ||
| description = "Specifies the number of bits of the prefix. The value can be set between 0 (4,294,967,296 addresses) and 31 (2 addresses). Defaults to 28(16 addresses). Changing this forces a new resource to be created." | ||
| } |
There was a problem hiding this comment.
The description for public_ip_prefix_length states that it 'Defaults to 28(16 addresses)', but the actual default value is 31. Please correct the description to match the default value.
variable "public_ip_prefix_length" {
type = number
default = 31
description = "Specifies the number of bits of the prefix. The value can be set between 0 (4,294,967,296 addresses) and 31 (2 addresses). Defaults to 31. Changing this forces a new resource to be created."
}
Comment on lines
173
to
177
| variable "sku_name" { | ||
| type = string | ||
| default = "AZFW_VNet" | ||
| description = "(optional) describe your variable" | ||
| } |
There was a problem hiding this comment.
The description for the sku_name variable is a placeholder. Please provide a meaningful description that explains what the variable is for and what its possible values are.
variable "sku_name" {
type = string
default = "AZFW_VNet"
description = "SKU name of the Firewall. Possible values are `AZFW_VNet` and `AZFW_Hub`."
}
Comment on lines
323
to
351
| variable "enable_diagnostic" { | ||
| type = bool | ||
| default = false | ||
| description = "Set to false to prevent the module from creating the diagnosys setting for the NSG Resource.." | ||
| } | ||
|
|
||
| variable "storage_account_id" { | ||
| type = string | ||
| default = null | ||
| description = "Storage account id to pass it to destination details of diagnosys setting of NSG." | ||
| } | ||
|
|
||
| variable "eventhub_name" { | ||
| type = string | ||
| default = null | ||
| description = "Eventhub Name to pass it to destination details of diagnosys setting of NSG." | ||
| } | ||
|
|
||
| variable "eventhub_authorization_rule_id" { | ||
| type = string | ||
| default = null | ||
| description = "Eventhub authorization rule id to pass it to destination details of diagnosys setting of NSG." | ||
| } | ||
|
|
||
| variable "log_analytics_workspace_id" { | ||
| type = string | ||
| default = null | ||
| description = "log analytics workspace id to pass it to destination details of diagnosys setting of NSG." | ||
| } |
There was a problem hiding this comment.
The descriptions for several diagnostic-related variables incorrectly refer to 'NSG'. This appears to be a copy-paste error and should be updated to 'Firewall'. This affects the following variables:
enable_diagnosticstorage_account_ideventhub_nameeventhub_authorization_rule_idlog_analytics_workspace_id
🛡️ Checkov Security Scan: 4 Issues Found🔴 [CKV_AZURE_216] Ensure DenyIntelMode is set to Deny for Azure Firewalls resource "azurerm_firewall" "firewall" {
count = var.enabled && var.firewall_enable ? 1 : 0
name = format(var.resource_position_prefix ? "firewall-%s" : "%s-firewall", local.name)
location = var.location
resource_group_name = var.resource_group_name
threat_intel_mode = var.threat_intel_mode
sku_tier = var.sku_tier
sku_name = var.sku_name
firewall_policy_id = join("", azurerm_firewall_policy.policy.*.id)
tags = module.labels.tags
private_ip_ranges = var.firewall_private_ip_ranges
dns_servers = var.dns_servers
# Primary ip_configuration (requires subnet_id)
ip_configuration {
name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, var.primary_public_ip_name)
subnet_id = var.subnet_id
public_ip_address_id = azurerm_public_ip.primary_public_ip[0].id #azurerm_public_ip.primary_public_ip[0].id
}
# Additional ip_configurations for the remaining PIPs
dynamic "ip_configuration" {
for_each = { for k, v in azurerm_public_ip.public_ip : k => v if k != var.primary_public_ip_name }
content {
name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, ip_configuration.key)
public_ip_address_id = ip_configuration.value.id
# subnet_id omitted by design for additional PIPsn
}
}
lifecycle {
ignore_changes = [tags]
}
}🔴 [CKV_AZURE_220] Ensure Firewall policy has IDPS mode as deny resource "azurerm_firewall_policy" "policy" {
count = var.enabled && var.firewall_enable ? 1 : 0
name = format(var.resource_position_prefix ? "firewall-Policy-%s" : "%s-firewall-Policy", local.name)
resource_group_name = var.resource_group_name
location = var.location
sku = var.sku_policy
dynamic "identity" {
for_each = var.identity_type != null && var.sku_policy == "Premium" && var.sku_tier == "Premium" ? [1] : []
content {
type = var.identity_type
identity_ids = var.identity_type == "UserAssigned" ? [join("", azurerm_user_assigned_identity.identity.*.id)] : null
}
}
}🔴 [CKV_AZURE_216] Ensure DenyIntelMode is set to Deny for Azure Firewalls resource "azurerm_firewall" "firewall" {
count = var.enabled && var.firewall_enable ? 1 : 0
name = format(var.resource_position_prefix ? "firewall-%s" : "%s-firewall", local.name)
location = var.location
resource_group_name = var.resource_group_name
threat_intel_mode = var.threat_intel_mode
sku_tier = var.sku_tier
sku_name = var.sku_name
firewall_policy_id = join("", azurerm_firewall_policy.policy.*.id)
tags = module.labels.tags
private_ip_ranges = var.firewall_private_ip_ranges
dns_servers = var.dns_servers
# Primary ip_configuration (requires subnet_id)
ip_configuration {
name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, var.primary_public_ip_name)
subnet_id = var.subnet_id
public_ip_address_id = azurerm_public_ip.primary_public_ip[0].id #azurerm_public_ip.primary_public_ip[0].id
}
# Additional ip_configurations for the remaining PIPs
dynamic "ip_configuration" {
for_each = { for k, v in azurerm_public_ip.public_ip : k => v if k != var.primary_public_ip_name }
content {
name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, ip_configuration.key)
public_ip_address_id = ip_configuration.value.id
# subnet_id omitted by design for additional PIPsn
}
}
lifecycle {
ignore_changes = [tags]
}
}🔴 [CKV_AZURE_220] Ensure Firewall policy has IDPS mode as deny resource "azurerm_firewall_policy" "policy" {
count = var.enabled && var.firewall_enable ? 1 : 0
name = format(var.resource_position_prefix ? "firewall-Policy-%s" : "%s-firewall-Policy", local.name)
resource_group_name = var.resource_group_name
location = var.location
sku = var.sku_policy
dynamic "identity" {
for_each = var.identity_type != null && var.sku_policy == "Premium" && var.sku_tier == "Premium" ? [1] : []
content {
type = var.identity_type
identity_ids = var.identity_type == "UserAssigned" ? [join("", azurerm_user_assigned_identity.identity.*.id)] : null
}
}
} |
🛡️ Checkov Security Scan: 4 Issues Found🔴 [CKV_AZURE_216] Ensure DenyIntelMode is set to Deny for Azure Firewalls resource "azurerm_firewall" "firewall" {
count = var.enabled && var.firewall_enable ? 1 : 0
name = format(var.resource_position_prefix ? "firewall-%s" : "%s-firewall", local.name)
location = var.location
resource_group_name = var.resource_group_name
threat_intel_mode = var.threat_intel_mode
sku_tier = var.sku_tier
sku_name = var.sku_name
firewall_policy_id = join("", azurerm_firewall_policy.policy.*.id)
tags = module.labels.tags
private_ip_ranges = var.firewall_private_ip_ranges
dns_servers = var.dns_servers
# Primary ip_configuration (requires subnet_id)
ip_configuration {
name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, var.primary_public_ip_name)
subnet_id = var.subnet_id
public_ip_address_id = azurerm_public_ip.primary_public_ip[0].id #azurerm_public_ip.primary_public_ip[0].id
}
# Additional ip_configurations for the remaining PIPs
dynamic "ip_configuration" {
for_each = { for k, v in azurerm_public_ip.public_ip : k => v if k != var.primary_public_ip_name }
content {
name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, ip_configuration.key)
public_ip_address_id = ip_configuration.value.id
# subnet_id omitted by design for additional PIPsn
}
}
lifecycle {
ignore_changes = [tags]
}
}🔴 [CKV_AZURE_220] Ensure Firewall policy has IDPS mode as deny resource "azurerm_firewall_policy" "policy" {
count = var.enabled && var.firewall_enable ? 1 : 0
name = format(var.resource_position_prefix ? "firewall-Policy-%s" : "%s-firewall-Policy", local.name)
resource_group_name = var.resource_group_name
location = var.location
sku = var.sku_policy
dynamic "identity" {
for_each = var.identity_type != null && var.sku_policy == "Premium" && var.sku_tier == "Premium" ? [1] : []
content {
type = var.identity_type
identity_ids = var.identity_type == "UserAssigned" ? [join("", azurerm_user_assigned_identity.identity.*.id)] : null
}
}
}🔴 [CKV_AZURE_216] Ensure DenyIntelMode is set to Deny for Azure Firewalls resource "azurerm_firewall" "firewall" {
count = var.enabled && var.firewall_enable ? 1 : 0
name = format(var.resource_position_prefix ? "firewall-%s" : "%s-firewall", local.name)
location = var.location
resource_group_name = var.resource_group_name
threat_intel_mode = var.threat_intel_mode
sku_tier = var.sku_tier
sku_name = var.sku_name
firewall_policy_id = join("", azurerm_firewall_policy.policy.*.id)
tags = module.labels.tags
private_ip_ranges = var.firewall_private_ip_ranges
dns_servers = var.dns_servers
# Primary ip_configuration (requires subnet_id)
ip_configuration {
name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, var.primary_public_ip_name)
subnet_id = var.subnet_id
public_ip_address_id = azurerm_public_ip.primary_public_ip[0].id #azurerm_public_ip.primary_public_ip[0].id
}
# Additional ip_configurations for the remaining PIPs
dynamic "ip_configuration" {
for_each = { for k, v in azurerm_public_ip.public_ip : k => v if k != var.primary_public_ip_name }
content {
name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, ip_configuration.key)
public_ip_address_id = ip_configuration.value.id
# subnet_id omitted by design for additional PIPsn
}
}
lifecycle {
ignore_changes = [tags]
}
}🔴 [CKV_AZURE_220] Ensure Firewall policy has IDPS mode as deny resource "azurerm_firewall_policy" "policy" {
count = var.enabled && var.firewall_enable ? 1 : 0
name = format(var.resource_position_prefix ? "firewall-Policy-%s" : "%s-firewall-Policy", local.name)
resource_group_name = var.resource_group_name
location = var.location
sku = var.sku_policy
dynamic "identity" {
for_each = var.identity_type != null && var.sku_policy == "Premium" && var.sku_tier == "Premium" ? [1] : []
content {
type = var.identity_type
identity_ids = var.identity_type == "UserAssigned" ? [join("", azurerm_user_assigned_identity.identity.*.id)] : null
}
}
} |
ravimalvia10
force-pushed
the
firewall-final-1.0
branch
from
02aa7be to
0092ba2
Compare
Comment on lines
+53
to
+87
| resource "azurerm_firewall" "firewall" { | ||
| depends_on = [azurerm_public_ip.public_ip, azurerm_public_ip_prefix.pip_prefix] | ||
| count = var.enabled && var.firewall_enable ? 1 : 0 | ||
| name = format(var.resource_position_prefix ? "firewall-%s" : "%s-firewall", local.name) | ||
| location = var.location | ||
| resource_group_name = var.resource_group_name | ||
| threat_intel_mode = var.threat_intel_mode | ||
| sku_tier = var.sku_tier | ||
| sku_name = var.sku_name | ||
| firewall_policy_id = join("", azurerm_firewall_policy.policy.*.id) | ||
| tags = module.labels.tags | ||
| private_ip_ranges = var.firewall_private_ip_ranges | ||
| dns_servers = var.dns_servers | ||
|
|
||
| # Primary ip_configuration (first in the list) | ||
| ip_configuration { | ||
| name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, var.public_ip_names[0]) | ||
| subnet_id = var.subnet_id | ||
| public_ip_address_id = azurerm_public_ip.public_ip[var.public_ip_names[0]].id | ||
| } | ||
|
|
||
| # Additional ip_configurations (skip first one) | ||
| dynamic "ip_configuration" { | ||
| for_each = length(var.public_ip_names) > 1 ? toset(slice(var.public_ip_names, 1, length(var.public_ip_names))) : [] | ||
| content { | ||
| name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, ip_configuration.value) | ||
| public_ip_address_id = azurerm_public_ip.public_ip[ip_configuration.value].id | ||
| # subnet_id omitted here! | ||
| } | ||
| } | ||
|
|
||
| lifecycle { | ||
| ignore_changes = [tags] | ||
| } | ||
| } |
Check failure
Code scanning / checkov
Ensure DenyIntelMode is set to Deny for Azure Firewalls Error
Comment on lines
+53
to
+87
| resource "azurerm_firewall" "firewall" { | ||
| depends_on = [azurerm_public_ip.public_ip, azurerm_public_ip_prefix.pip_prefix] | ||
| count = var.enabled && var.firewall_enable ? 1 : 0 | ||
| name = format(var.resource_position_prefix ? "firewall-%s" : "%s-firewall", local.name) | ||
| location = var.location | ||
| resource_group_name = var.resource_group_name | ||
| threat_intel_mode = var.threat_intel_mode | ||
| sku_tier = var.sku_tier | ||
| sku_name = var.sku_name | ||
| firewall_policy_id = join("", azurerm_firewall_policy.policy.*.id) | ||
| tags = module.labels.tags | ||
| private_ip_ranges = var.firewall_private_ip_ranges | ||
| dns_servers = var.dns_servers | ||
|
|
||
| # Primary ip_configuration (first in the list) | ||
| ip_configuration { | ||
| name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, var.public_ip_names[0]) | ||
| subnet_id = var.subnet_id | ||
| public_ip_address_id = azurerm_public_ip.public_ip[var.public_ip_names[0]].id | ||
| } | ||
|
|
||
| # Additional ip_configurations (skip first one) | ||
| dynamic "ip_configuration" { | ||
| for_each = length(var.public_ip_names) > 1 ? toset(slice(var.public_ip_names, 1, length(var.public_ip_names))) : [] | ||
| content { | ||
| name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, ip_configuration.value) | ||
| public_ip_address_id = azurerm_public_ip.public_ip[ip_configuration.value].id | ||
| # subnet_id omitted here! | ||
| } | ||
| } | ||
|
|
||
| lifecycle { | ||
| ignore_changes = [tags] | ||
| } | ||
| } |
Check failure
Code scanning / checkov
Ensure DenyIntelMode is set to Deny for Azure Firewalls Error
🛡️ Checkov Security Scan: 4 Issues Found🔴 [CKV_AZURE_216] Ensure DenyIntelMode is set to Deny for Azure Firewalls resource "azurerm_firewall" "firewall" {
depends_on = [azurerm_public_ip.public_ip, azurerm_public_ip_prefix.pip_prefix]
count = var.enabled && var.firewall_enable ? 1 : 0
name = format(var.resource_position_prefix ? "firewall-%s" : "%s-firewall", local.name)
location = var.location
resource_group_name = var.resource_group_name
threat_intel_mode = var.threat_intel_mode
sku_tier = var.sku_tier
sku_name = var.sku_name
firewall_policy_id = join("", azurerm_firewall_policy.policy.*.id)
tags = module.labels.tags
private_ip_ranges = var.firewall_private_ip_ranges
dns_servers = var.dns_servers
# Primary ip_configuration (first in the list)
ip_configuration {
name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, var.public_ip_names[0])
subnet_id = var.subnet_id
public_ip_address_id = azurerm_public_ip.public_ip[var.public_ip_names[0]].id
}
# Additional ip_configurations (skip first one)
dynamic "ip_configuration" {
for_each = length(var.public_ip_names) > 1 ? toset(slice(var.public_ip_names, 1, length(var.public_ip_names))) : []
content {
name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, ip_configuration.value)
public_ip_address_id = azurerm_public_ip.public_ip[ip_configuration.value].id
# subnet_id omitted here!
}
}
lifecycle {
ignore_changes = [tags]
}
}🔴 [CKV_AZURE_220] Ensure Firewall policy has IDPS mode as deny resource "azurerm_firewall_policy" "policy" {
count = var.enabled && var.firewall_enable ? 1 : 0
name = format(var.resource_position_prefix ? "firewall-policy-%s" : "%s-firewall-policy", local.name)
resource_group_name = var.resource_group_name
location = var.location
sku = var.sku_policy
dynamic "identity" {
for_each = var.identity_type != null && var.sku_policy == "Premium" && var.sku_tier == "Premium" ? [1] : []
content {
type = var.identity_type
identity_ids = var.identity_type == "UserAssigned" ? [join("", azurerm_user_assigned_identity.identity.*.id)] : null
}
}
}🔴 [CKV_AZURE_216] Ensure DenyIntelMode is set to Deny for Azure Firewalls resource "azurerm_firewall" "firewall" {
depends_on = [azurerm_public_ip.public_ip, azurerm_public_ip_prefix.pip_prefix]
count = var.enabled && var.firewall_enable ? 1 : 0
name = format(var.resource_position_prefix ? "firewall-%s" : "%s-firewall", local.name)
location = var.location
resource_group_name = var.resource_group_name
threat_intel_mode = var.threat_intel_mode
sku_tier = var.sku_tier
sku_name = var.sku_name
firewall_policy_id = join("", azurerm_firewall_policy.policy.*.id)
tags = module.labels.tags
private_ip_ranges = var.firewall_private_ip_ranges
dns_servers = var.dns_servers
# Primary ip_configuration (first in the list)
ip_configuration {
name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, var.public_ip_names[0])
subnet_id = var.subnet_id
public_ip_address_id = azurerm_public_ip.public_ip[var.public_ip_names[0]].id
}
# Additional ip_configurations (skip first one)
dynamic "ip_configuration" {
for_each = length(var.public_ip_names) > 1 ? toset(slice(var.public_ip_names, 1, length(var.public_ip_names))) : []
content {
name = format(var.resource_position_prefix ? "ipconfig-%s-%s" : "%s-%s-ipconfig", local.name, ip_configuration.value)
public_ip_address_id = azurerm_public_ip.public_ip[ip_configuration.value].id
# subnet_id omitted here!
}
}
lifecycle {
ignore_changes = [tags]
}
}🔴 [CKV_AZURE_220] Ensure Firewall policy has IDPS mode as deny resource "azurerm_firewall_policy" "policy" {
count = var.enabled && var.firewall_enable ? 1 : 0
name = format(var.resource_position_prefix ? "firewall-policy-%s" : "%s-firewall-policy", local.name)
resource_group_name = var.resource_group_name
location = var.location
sku = var.sku_policy
dynamic "identity" {
for_each = var.identity_type != null && var.sku_policy == "Premium" && var.sku_tier == "Premium" ? [1] : []
content {
type = var.identity_type
identity_ids = var.identity_type == "UserAssigned" ? [join("", azurerm_user_assigned_identity.identity.*.id)] : null
}
}
} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes #
Closes #
Type of Change
Checklist