feat(terraform): import psql module#2221
Open
ootneim wants to merge 5 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a feature-rich Terraform module to deploy Azure PostgreSQL Flexible Server with provider declarations, many snake_case input variables/validations, data lookups (Azure AD, VNet), networking/private DNS and private endpoint support, UAMI and AD admin bindings, diagnostics, AzAPI storage read, outputs, and documentation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Terraform as "Terraform"
participant AzureAD as "Azure AD (data sources)"
participant ARM as "Azure Resource Manager"
participant LA as "Log Analytics"
participant AzAPI as "AzAPI"
Terraform->>AzureAD: data.azurerm_client_config.current
Terraform->>AzureAD: data.azuread_service_principal.current
Terraform->>AzureAD: data.azuread_service_principal.terraform
Terraform->>AzureAD: data.azuread_group.psql_admin_groups (for_each)
Terraform->>ARM: data.azurerm_virtual_network.psql
alt psql_enable_private_access = true
Terraform->>ARM: azurerm_subnet.psql (delegation)
Terraform->>ARM: azurerm_private_dns_zone.psql (conditional)
Terraform->>ARM: azurerm_private_dns_zone_virtual_network_link.psql (conditional)
Terraform->>ARM: azurerm_postgresql_flexible_server.psql (private access, UAMI, AD admin)
else
Terraform->>ARM: azurerm_postgresql_flexible_server.psql (public access)
Terraform->>ARM: azurerm_postgresql_flexible_server_firewall_rule.psql (for_each)
end
Terraform->>ARM: azurerm_user_assigned_identity.psql_identity
Terraform->>ARM: azurerm_postgresql_flexible_server_active_directory_administrator.* (terraform + per-group)
Terraform->>ARM: azurerm_postgresql_flexible_server_configuration.* (pgbouncer/extensions/custom)
opt diagnostics enabled
Terraform->>ARM: azurerm_monitor_diagnostic_setting.postgresql
ARM->>LA: send logs & metrics
end
opt track actual storage
Terraform->>AzAPI: data.azapi_resource.psql_actual
end
ARM-->>Terraform: outputs (ids, fqdn, dns zone, storage)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧠 Learnings (16)📓 Common learnings📚 Learning: 2025-05-19T07:53:37.021ZApplied to files:
📚 Learning: 2025-09-19T05:36:32.158ZApplied to files:
📚 Learning: 2025-05-15T10:56:41.891ZApplied to files:
📚 Learning: 2025-05-15T10:56:41.891ZApplied to files:
📚 Learning: 2025-01-15T08:33:27.165ZApplied to files:
📚 Learning: 2025-08-12T12:19:56.075ZApplied to files:
📚 Learning: 2024-11-27T09:21:33.566ZApplied to files:
📚 Learning: 2025-08-12T12:18:52.207ZApplied to files:
📚 Learning: 2025-08-12T12:18:52.207ZApplied to files:
📚 Learning: 2025-07-31T11:01:28.814ZApplied to files:
📚 Learning: 2025-05-15T11:04:30.495ZApplied to files:
📚 Learning: 2025-07-31T11:01:28.814ZApplied to files:
📚 Learning: 2024-12-05T13:35:28.894ZApplied to files:
📚 Learning: 2025-09-25T12:52:58.967ZApplied to files:
📚 Learning: 2025-08-20T12:49:09.790ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (11)
infrastructure/modules/psql/readme.md (2)
80-91: Fix output names to match module outputs.Apply:
-output "psql_id" { - value = module.psql.psql_server_id -} -output "psql_fqdn" { - value = module.psql.psql_server_fqdn -} -output "psql_UserAssignedIdentity_id" { - value = module.psql.psql_identity_id -} +output "psql_server_id" { + value = module.psql.psql_server_id +} +output "psql_server_fqdn" { + value = module.psql.psql_server_fqdn +} +output "psql_identity_id" { + value = module.psql.psql_identity_id +}
31-34: Suspicious firewall example range.
start_ip51.120.0.114 →end_ip251.120.0.114 spans a massive range and likely unintended (and unusual first octet change). Consider a tighter example.infrastructure/modules/psql/variables.tf (6)
139-143: Add validation for maintenance hour/day.Apply:
variable "psql_maintenance_start_hour" { type = number default = 1 description = "0–23." + validation { + condition = var.psql_maintenance_start_hour >= 0 && var.psql_maintenance_start_hour <= 23 + error_message = "psql_maintenance_start_hour must be between 0 and 23." + } } variable "psql_maintenance_day_of_week" { type = number default = 2 description = "1=Monday ... 7=Sunday (Azure spec)." + validation { + condition = var.psql_maintenance_day_of_week >= 1 && var.psql_maintenance_day_of_week <= 7 + error_message = "psql_maintenance_day_of_week must be between 1 and 7." + } }
106-109: Constrain and normalize PgBouncer pool mode.Provider typically expects lowercase values:
session|transaction|statement. Add validation and consider lowercasing at use.Apply:
variable "psql_pgbouncer_pool_mode" { type = string - description = "pgbouncer pool mode (SESSION, TRANSACTION, STATEMENT)" + description = "pgbouncer pool mode: session | transaction | statement" + validation { + condition = contains(["session","transaction","statement"], lower(var.psql_pgbouncer_pool_mode)) + error_message = "psql_pgbouncer_pool_mode must be one of: session, transaction, statement." + } }
26-34: Incorrect description of resource group variable.
psql_NetworkResourceGroupdescription says “psql network name”; should be “resource group of the virtual network.”variable "psql_NetworkResourceGroup" { type = string - description = "psql network name" + description = "Resource group of the existing virtual network." }
274-299: Diagnostics lists: language/wording and validation are fine, but standardize English.Adjust error message text and “Empty list” phrasing for consistency.
- description = "Friendly diagnostic log categories to enable. Empty liste = ingen logger." + description = "Friendly diagnostic log categories to enable. Empty list = no logs." ... - error_message = "En eller flere oppgitte kategorier er ikke gyldige friendly navn." + error_message = "One or more provided categories are not valid friendly names."
117-121: Consider validating supported PostgreSQL versions.Optional: restrict
psql_Versionto known provider-supported values to fail fast.
258-265: Optional: validate firewall IPs format.You can regex-check IPv4 format for
start_ip/end_ipto catch typos early; ordering checks are harder in HCL but formatting catch helps.infrastructure/modules/psql/data.tf (1)
3-5: Unused data source.
azuread_service_principal.currentisn’t referenced. Remove it or use it in main.tf to avoid hardcoding an object id.infrastructure/modules/psql/main.tf (2)
137-146: Normalize PgBouncer pool mode to lowercase.Matches your new validation and provider expectations.
value = var.psql_pgbouncer_pool_mode + # ensure lowercase per provider expectations + value = lower(var.psql_pgbouncer_pool_mode)
274-280: Firewall rules only when public access is on — good.Consider optional validation on IP formats in variables to fail early.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
infrastructure/modules/psql/data.tf(1 hunks)infrastructure/modules/psql/diagnostic.tf(1 hunks)infrastructure/modules/psql/main.tf(1 hunks)infrastructure/modules/psql/output.tf(1 hunks)infrastructure/modules/psql/providers.tf(1 hunks)infrastructure/modules/psql/readme.md(1 hunks)infrastructure/modules/psql/variables.tf(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-05-07T17:01:34.104Z
Learnt from: sduranc
PR: Altinn/altinn-platform#1607
File: infrastructure/modules/observability/providers.tf:1-6
Timestamp: 2025-05-07T17:01:34.104Z
Learning: In Terraform modules, it's a best practice to only declare required providers (with source and version) but not include provider configurations like `provider "azurerm" { features {} }`. The actual provider configuration should be added by the consumer of the module at the root level.
Applied to files:
infrastructure/modules/psql/providers.tf
📚 Learning: 2024-12-05T13:35:28.894Z
Learnt from: monteiro-renato
PR: Altinn/altinn-platform#1162
File: infrastructure/adminservices-test/altinn-monitor-test-rg/k6_tests_rg_providers.tf:25-32
Timestamp: 2024-12-05T13:35:28.894Z
Learning: In Terraform, it's recommended to specify provider version constraints in the `required_providers` block within the `terraform` configuration rather than in the `provider` configuration block.
Applied to files:
infrastructure/modules/psql/providers.tf
📚 Learning: 2024-11-27T09:21:33.566Z
Learnt from: monteiro-renato
PR: Altinn/altinn-platform#1138
File: infrastructure/adminservices-test/altinn-monitor-test-rg/lasttest.tf:1-10
Timestamp: 2024-11-27T09:21:33.566Z
Learning: In the `infrastructure/adminservices-test/altinn-monitor-test-rg` Terraform configuration, it's acceptable to add resources like `azurerm_resource_group` and `azurerm_monitor_workspace` without including additional variables, outputs, provider, or backend configurations.
Applied to files:
infrastructure/modules/psql/providers.tfinfrastructure/modules/psql/data.tf
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
infrastructure/modules/psql/variables.tf (1)
248-256: Backup retention validation LGTM.Clear range check with sensible default.
infrastructure/modules/psql/diagnostic.tf (2)
20-40: Diagnostics resource setup looks solid.Conditional creation, dynamic logs/metrics, and workspace wiring are correct given the locals.
3-18: Verified — category mappings match Azure Monitor supported logs.
The friendly→API names in infrastructure/modules/psql/diagnostic.tf match Azure Monitor's supported log categories for Microsoft.DBforPostgreSQL/flexibleServers: PostgreSQLLogs, PostgreSQLFlexSessions, PostgreSQLFlexQueryStoreRuntime, PostgreSQLFlexQueryStoreWaitStats, PostgreSQLFlexTableStats, PostgreSQLFlexDatabaseXacts.infrastructure/modules/psql/data.tf (1)
12-20: LAW and VNet lookups LGTM.Names/RG-based lookups are straightforward.
infrastructure/modules/psql/output.tf (1)
1-4: Outputs otherwise look correct and useful.Also applies to: 6-9, 11-14, 16-20, 28-31, 33-39, 41-45
infrastructure/modules/psql/main.tf (2)
71-88: Server block generally solid; good use of ignore_changes and VNet toggles.Confirm whether
private_endpoint_network_policies = "Enabled"is desired for a delegated subnet, and whetherservice_endpoints = ["Microsoft.Storage"]is needed for this subnet.Also applies to: 91-97
248-255: Custom configuration filtering is clean.Reserved keys are excluded; uses
for_eachto apply safely.
Comment on lines
+1
to
+27
| terraform { | ||
| required_providers { | ||
| azurerm = { | ||
| source = "hashicorp/azurerm" | ||
| } | ||
| azuread = { | ||
| source = "hashicorp/azuread" | ||
| } | ||
| null = { | ||
| source = "hashicorp/null" | ||
| } | ||
| time = { | ||
| source = "hashicorp/time" | ||
| } | ||
| random = { | ||
| source = "hashicorp/random" | ||
| } | ||
| postgresql = { | ||
| source = "cyrilgdn/postgresql" | ||
| version = "__postgresql_provider__" | ||
| } | ||
| azapi = { | ||
| source = "azure/azapi" | ||
| version = "~> 1.13" | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Remove unused providers and replace placeholder version.
postgresqlprovider is not used in this module and has a placeholder version__postgresql_provider__that will breakinit. Drop it or pin a real constraint.- Consider removing
null,time, andrandomif unused to avoid unnecessary downloads. - Add minimal version floors for actually used providers (
azurerm,azuread,azapi) to prevent accidental downgrades; leave tight pins to root to avoid conflicts.
Run to confirm unused providers:
🏁 Script executed:
#!/bin/bash
# Search for any resource/data using these providers in this module
rg -nP '^(resource|data)\s+"(null_resource|time_|random_|postgresql_)' infrastructure/modules/psql --type tf -C1 || trueLength of output: 0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== providers.tf ==="
if [ -f infrastructure/modules/psql/providers.tf ]; then
sed -n '1,240p' infrastructure/modules/psql/providers.tf || true
else
echo "providers.tf not found"
fi
echo
echo "=== .tf files in module ==="
find infrastructure/modules/psql -type f -name '*.tf' -print || true
echo
echo "=== Search for provider-specific resource/data types (postgresql/null/time/random) ==="
rg -nS 'postgresql_|null_resource\b|time_[A-Za-z0-9_]+|random_[A-Za-z0-9_]+' infrastructure/modules/psql -C2 || true
echo
echo "=== Search for provider blocks referencing these providers ==="
rg -nS 'provider\s+"(postgresql|null|time|random)"' infrastructure/modules/psql -C2 || trueLength of output: 12472
Remove unused providers and fix placeholder version
- In infrastructure/modules/psql/providers.tf, drop the unused
postgresql,null,time, andrandomentries. - Remove the
__postgresql_provider__placeholder (or replace it with a real version) if you ever intend to add PostgreSQL-specific resources. - Add minimum version constraints for the actually used providers (
azurerm,azuread,azapi) underrequired_providersto prevent inadvertent downgrades.
🤖 Prompt for AI Agents
in infrastructure/modules/psql/providers.tf lines 1-27: the file currently
declares unused providers (postgresql, null, time, random) and contains a
placeholder version "__postgresql_provider__"; remove the unused provider blocks
(postgresql, null, time, random) entirely, delete the placeholder version entry
(or replace it with a concrete version only if you will add postgres-specific
resources), and add explicit minimum version constraints for the actually used
providers (azurerm, azuread, azapi) under required_providers to lock reasonable
minimums (e.g., set version = ">= X.Y.Z" or "~> X.Y" for each) to prevent
accidental downgrades.
tjololo
requested changes
Sep 19, 2025
tjololo
left a comment
tjololo
left a comment
Member
There was a problem hiding this comment.
Great work! I have some comments about casing. Please note that I have only made suggestion/comments in the variables.tf not the all the places they are used so just applying them would break the code
| data "azurerm_virtual_network" "psql" { | ||
| name = var.psql_NetworkName | ||
| resource_group_name = var.psql_NetworkResourceGroup | ||
| } No newline at end of file |
Comment on lines
+12
to
+15
| data "azurerm_log_analytics_workspace" "workspace" { | ||
| name = var.log_analytics_workspace_name | ||
| resource_group_name = var.log_analytics_workspace_rg | ||
| } |
Member
There was a problem hiding this comment.
Can we instead of having the name and rg as variables just have the workspace.id as a variable, since its the only value we use
| count = var.psql_diagnostics_enabled ? 1 : 0 | ||
| name = "psql-diag" | ||
| target_resource_id = azurerm_postgresql_flexible_server.psql.id | ||
| log_analytics_workspace_id = data.azurerm_log_analytics_workspace.workspace.id |
Member
There was a problem hiding this comment.
suggest changing this reference to a new variable that is the log analytics workspace id
| default = true | ||
| } | ||
|
|
||
| variable "psql_Version" { |
Member
There was a problem hiding this comment.
Keep to one type of formating, for terraform i prefer snake_case
Suggested change
| variable "psql_Version" { | |
| variable "psql_version" { |
|
|
||
| variable "psql_maintenance_start_minute" { | ||
| type = number | ||
| default = 4 |
Member
There was a problem hiding this comment.
See coderabbits comment, default does not follow what is said in the description. Either the default or the description is erroneous
| default = false | ||
| } | ||
|
|
||
| variable "psql_StorageTier" { |
Member
There was a problem hiding this comment.
Keep to one type of formating, for terraform i prefer snake_case
Suggested change
| variable "psql_StorageTier" { | |
| variable "psql_storage_tier" { |
| description = "Optional explicit name for the PostgreSQL subnet. Defaults to \"<psql_ServerName>-subnet\" when null." | ||
| type = string | ||
| default = null | ||
| } No newline at end of file |
Comment on lines
+282
to
+306
| #Example of peering | ||
| /* | ||
| resource "azurerm_virtual_network_peering" "psql_to_aks" { | ||
| name = var.aks_vnet_name | ||
| resource_group_name = azurerm_resource_group.psql.name | ||
| virtual_network_name = azurerm_virtual_network.psql.name | ||
| remote_virtual_network_id = var.aks_vnet_id | ||
|
|
||
| allow_virtual_network_access = true | ||
| allow_forwarded_traffic = true | ||
| allow_gateway_transit = false | ||
| use_remote_gateways = false | ||
| } | ||
| resource "azurerm_virtual_network_peering" "aks_to_psql" { | ||
| name = azurerm_virtual_network.psql.name | ||
| resource_group_name = var.aks_resource_group_name | ||
| virtual_network_name = var.aks_vnet_name | ||
| remote_virtual_network_id = azurerm_virtual_network.psql.id | ||
|
|
||
| allow_virtual_network_access = true | ||
| allow_forwarded_traffic = true | ||
| allow_gateway_transit = false | ||
| use_remote_gateways = false | ||
| } | ||
| */ No newline at end of file |
Member
There was a problem hiding this comment.
I think we should remove this. Do we expose enough with outputs to make it easy for uses of this module to peer networks?
This should probably be a part of the readme.md if we think its nice to have for the users of this module
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
infrastructure/modules/psql/providers.tf (1)
3-15: Trim unused provider and pin real versions.
nullisn’t referenced anywhere in this module, so keeping it just slowsterraform init. Also, azurerm/azuread should carry minimum versions like azapi to avoid accidental downgrades. Please drop the unused provider and declare sensible floor versions for the ones you actually use.Apply something like:
terraform { required_providers { azurerm = { source = "hashicorp/azurerm" - } + version = ">= 3.90.0" + } azuread = { source = "hashicorp/azuread" - } - null = { - source = "hashicorp/null" + version = ">= 2.48.0" } azapi = { source = "azure/azapi" version = "~> 1.13" } } }(Bump the version floors to whatever you’ve validated in
main.)
Based on learningsinfrastructure/modules/psql/main.tf (2)
128-132: Pass numbers tomaintenance_window.
maintenance_windowexpects numeric types; thetostring(...)calls coerce them to strings and the provider rejects the plan. Please drop the casts and feed the numbers directly.maintenance_window { - day_of_week = tostring(var.psql_maintenance_day_of_week) - start_hour = tostring(var.psql_maintenance_start_hour) - start_minute = tostring(var.psql_maintenance_start_minute) + day_of_week = var.psql_maintenance_day_of_week + start_hour = var.psql_maintenance_start_hour + start_minute = var.psql_maintenance_start_minute }
164-165: Remove hard-coded Terraform SP object ID.The GUID ties the module to one tenant/service principal and breaks reuse elsewhere. Use the current principal (from
data.azuread_service_principal.current) or accept an input instead.- object_id = "641fc568-3e2f-4174-a7ce-d91f50c8e6d6" - principal_name = "altinn-platform-terraform" + object_id = data.azuread_service_principal.current.object_id + principal_name = data.azuread_service_principal.current.display_name
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
infrastructure/modules/psql/data.tf(1 hunks)infrastructure/modules/psql/diagnostic.tf(1 hunks)infrastructure/modules/psql/main.tf(1 hunks)infrastructure/modules/psql/output.tf(1 hunks)infrastructure/modules/psql/providers.tf(1 hunks)infrastructure/modules/psql/readme.md(1 hunks)infrastructure/modules/psql/variables.tf(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- infrastructure/modules/psql/readme.md
- infrastructure/modules/psql/diagnostic.tf
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-09-19T05:36:32.158Z
Learnt from: tjololo
PR: Altinn/altinn-platform#2221
File: infrastructure/modules/psql/main.tf:20-37
Timestamp: 2025-09-19T05:36:32.158Z
Learning: Azure PostgreSQL Flexible Server private DNS zones must end with ".postgres.database.azure.com". Valid patterns are "privatelink.postgres.database.azure.com" (shared standard zone) or "[name].postgres.database.azure.com" (custom zones). The pattern "${server_name}.privatelink.postgres.database.azure.com" is incorrect and doesn't follow Azure's documented conventions.
Applied to files:
infrastructure/modules/psql/main.tf
📚 Learning: 2025-05-15T10:56:41.891Z
Learnt from: tjololo
PR: Altinn/altinn-platform#1607
File: infrastructure/modules/observability/app.tf:18-20
Timestamp: 2025-05-15T10:56:41.891Z
Learning: The Terraform resource `azuread_service_principal` should use the `client_id` parameter (not `application_id`) to link to an Azure AD application in current versions of the azuread provider.
Applied to files:
infrastructure/modules/psql/main.tf
📚 Learning: 2025-05-15T10:56:41.891Z
Learnt from: tjololo
PR: Altinn/altinn-platform#1607
File: infrastructure/modules/observability/app.tf:18-20
Timestamp: 2025-05-15T10:56:41.891Z
Learning: The Terraform resource `azuread_service_principal` should use the `client_id` parameter (not `application_id`) to link to an Azure AD application in the azuread provider.
Applied to files:
infrastructure/modules/psql/main.tf
📚 Learning: 2025-05-07T17:01:34.104Z
Learnt from: sduranc
PR: Altinn/altinn-platform#1607
File: infrastructure/modules/observability/providers.tf:1-6
Timestamp: 2025-05-07T17:01:34.104Z
Learning: In Terraform modules, it's a best practice to only declare required providers (with source and version) but not include provider configurations like `provider "azurerm" { features {} }`. The actual provider configuration should be added by the consumer of the module at the root level.
Applied to files:
infrastructure/modules/psql/providers.tf
📚 Learning: 2024-12-05T13:35:28.894Z
Learnt from: monteiro-renato
PR: Altinn/altinn-platform#1162
File: infrastructure/adminservices-test/altinn-monitor-test-rg/k6_tests_rg_providers.tf:25-32
Timestamp: 2024-12-05T13:35:28.894Z
Learning: In Terraform, it's recommended to specify provider version constraints in the `required_providers` block within the `terraform` configuration rather than in the `provider` configuration block.
Applied to files:
infrastructure/modules/psql/providers.tf
📚 Learning: 2024-11-27T09:21:33.566Z
Learnt from: monteiro-renato
PR: Altinn/altinn-platform#1138
File: infrastructure/adminservices-test/altinn-monitor-test-rg/lasttest.tf:1-10
Timestamp: 2024-11-27T09:21:33.566Z
Learning: In the `infrastructure/adminservices-test/altinn-monitor-test-rg` Terraform configuration, it's acceptable to add resources like `azurerm_resource_group` and `azurerm_monitor_workspace` without including additional variables, outputs, provider, or backend configurations.
Applied to files:
infrastructure/modules/psql/providers.tf
Comment on lines
+97
to
+109
| delegated_subnet_id = var.psql_enable_private_access ? azurerm_subnet.psql[0].id : null | ||
| private_dns_zone_id = var.psql_enable_private_access ? azurerm_private_dns_zone.psql[0].id : null | ||
| public_network_access_enabled = var.psql_enable_private_access ? false : true | ||
| backup_retention_days = var.psql_backup_retention_days | ||
| geo_redundant_backup_enabled = var.psql_geo_redundant_backup_enabled | ||
|
|
||
| storage_mb = var.psql_storage_size | ||
| auto_grow_enabled = var.psql_storage_auto_grow | ||
|
|
||
| sku_name = var.psql_compute_size | ||
| storage_tier = var.psql_storage_tier == null ? null : var.psql_storage_tier | ||
| depends_on = [azurerm_private_dns_zone_virtual_network_link.psql] | ||
|
|
Contributor
There was a problem hiding this comment.
Fix private_dns_zone_id when reusing an existing zone.
If existing_private_dns_zone_id is set, azurerm_private_dns_zone.psql has count = 0, so azurerm_private_dns_zone.psql[0].id throws “index out of range” even though private access is enabled. You already compute local.effective_private_dns_zone_id; please use it and guard the dependency accordingly.
- private_dns_zone_id = var.psql_enable_private_access ? azurerm_private_dns_zone.psql[0].id : null
- public_network_access_enabled = var.psql_enable_private_access ? false : true
+ private_dns_zone_id = var.psql_enable_private_access ? local.effective_private_dns_zone_id : null
+ public_network_access_enabled = var.psql_enable_private_access ? false : true
backup_retention_days = var.psql_backup_retention_days
geo_redundant_backup_enabled = var.psql_geo_redundant_backup_enabled
@@
- depends_on = [azurerm_private_dns_zone_virtual_network_link.psql]
+ depends_on = var.psql_enable_private_access && local.create_private_dns_zone ? [
+ azurerm_private_dns_zone_virtual_network_link.psql
+ ] : []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| delegated_subnet_id = var.psql_enable_private_access ? azurerm_subnet.psql[0].id : null | |
| private_dns_zone_id = var.psql_enable_private_access ? azurerm_private_dns_zone.psql[0].id : null | |
| public_network_access_enabled = var.psql_enable_private_access ? false : true | |
| backup_retention_days = var.psql_backup_retention_days | |
| geo_redundant_backup_enabled = var.psql_geo_redundant_backup_enabled | |
| storage_mb = var.psql_storage_size | |
| auto_grow_enabled = var.psql_storage_auto_grow | |
| sku_name = var.psql_compute_size | |
| storage_tier = var.psql_storage_tier == null ? null : var.psql_storage_tier | |
| depends_on = [azurerm_private_dns_zone_virtual_network_link.psql] | |
| delegated_subnet_id = var.psql_enable_private_access ? azurerm_subnet.psql[0].id : null | |
| private_dns_zone_id = var.psql_enable_private_access ? local.effective_private_dns_zone_id : null | |
| public_network_access_enabled = var.psql_enable_private_access ? false : true | |
| backup_retention_days = var.psql_backup_retention_days | |
| geo_redundant_backup_enabled = var.psql_geo_redundant_backup_enabled | |
| storage_mb = var.psql_storage_size | |
| auto_grow_enabled = var.psql_storage_auto_grow | |
| sku_name = var.psql_compute_size | |
| storage_tier = var.psql_storage_tier == null ? null : var.psql_storage_tier | |
| depends_on = var.psql_enable_private_access && local.create_private_dns_zone ? [ | |
| azurerm_private_dns_zone_virtual_network_link.psql | |
| ] : [] |
🤖 Prompt for AI Agents
in infrastructure/modules/psql/main.tf around lines 97 to 109, when reusing an
existing private DNS zone the code indexes azurerm_private_dns_zone.psql[0].id
which will fail if that resource has count = 0; replace the direct index with
local.effective_private_dns_zone_id for private_dns_zone_id, and update the
depends_on so it only references
azurerm_private_dns_zone_virtual_network_link.psql when the module actually
creates the zone (e.g., use a conditional depends_on list that is empty when
existing_private_dns_zone_id is set, otherwise includes
azurerm_private_dns_zone_virtual_network_link.psql) so no out-of-range indexing
or invalid dependency occurs.
tjololo
reviewed
Oct 20, 2025
tjololo
approved these changes
Dec 9, 2025
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This pull request introduces a new Terraform module for managing PostgreSQL (psql) resources within the infrastructure. The module is located at psql and is designed to standardize and simplify the provisioning and management of PostgreSQL databases and related resources.
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.