Description
The Terraform module for PostgreSQL needs to be updated to incorporate feedback provided by Vemund (GitHub username: tjololo) in Pull Request #2221: feat(terraform): import psql module.
This feedback includes corrections to DNS zone naming, variable consistency, resource configurations, and other improvements to ensure the module follows best practices and Azure conventions.
Feedback Points from PR #2221
Here is a summary of the key comments and suggestions from Vemund:
-
DNS Zone Naming: Azure PostgreSQL Flexible Server private DNS zones must end with ".postgres.database.azure.com". Use "privatelink.postgres.database.azure.com" (shared) or custom zones like "[name].postgres.database.azure.com". Avoid patterns like "${server_name}.privatelink.postgres.database.azure.com" as they don't follow Azure conventions. Concern about broad zones potentially breaking other lookups if multiple privatelinks are in the same VNet.
-
AzureAD Service Principal: Use client_id instead of application_id in the azuread_service_principal resource to link to an Azure AD application.
-
Admin Groups: Use object_id as an input for data "azuread_group" "psql_admin_groups" to avoid accidentally granting admin rights to the wrong group.
-
Log Analytics Workspace: Replace separate log_analytics_workspace_name and log_analytics_workspace_rg variables with a single log_analytics_workspace_id variable. Update references accordingly, e.g., in azurerm_monitor_diagnostic_setting.
-
Variable Naming Consistency: Use snake_case for variables (e.g., psql_version instead of psql_Version, psql_storage_tier instead of psql_StorageTier).
-
Maintenance Configuration: Fix mismatch between default value and description for psql_maintenance_start_minute.
-
Commented-Out Resources: Remove commented-out peering resources from the module; document in README.md if useful for users.
-
Storage Tier Logic: Review and simplify conditional logic for storage_tier as it may be redundant.
-
PgBouncer Configuration: Use a boolean value instead of count for enabling/disabling configurations.
-
AD Admin Resource: Make object_id and principal_name configurable via variables instead of hardcoding.
-
Outputs and Iteration: Confirm handling of list outputs; for-each should work as is.
-
Provider Versions: Add major version constraints to required_providers (e.g., minimum versions).
-
Variable Descriptions: Improve description for psql_storage_size to mention it as the initial size when auto-grow is enabled.
-
Nits: Add newlines at the end of files where missing (e.g., data blocks for virtual network, subnet, diagnostics, outputs, providers).
Proposed Solution
Implement the suggested changes in the PostgreSQL Terraform module located at infrastructure/modules/psql/.
- Update variable names, descriptions, and defaults for consistency.
- Refactor resource configurations as recommended.
- Remove unnecessary code and add documentation where appropriate.
- Test the updated module to ensure compatibility and functionality.
Description
The Terraform module for PostgreSQL needs to be updated to incorporate feedback provided by Vemund (GitHub username: tjololo) in Pull Request #2221: feat(terraform): import psql module.
This feedback includes corrections to DNS zone naming, variable consistency, resource configurations, and other improvements to ensure the module follows best practices and Azure conventions.
Feedback Points from PR #2221
Here is a summary of the key comments and suggestions from Vemund:
DNS Zone Naming: Azure PostgreSQL Flexible Server private DNS zones must end with ".postgres.database.azure.com". Use "privatelink.postgres.database.azure.com" (shared) or custom zones like "[name].postgres.database.azure.com". Avoid patterns like "${server_name}.privatelink.postgres.database.azure.com" as they don't follow Azure conventions. Concern about broad zones potentially breaking other lookups if multiple privatelinks are in the same VNet.
AzureAD Service Principal: Use
client_idinstead ofapplication_idin theazuread_service_principalresource to link to an Azure AD application.Admin Groups: Use
object_idas an input fordata "azuread_group" "psql_admin_groups"to avoid accidentally granting admin rights to the wrong group.Log Analytics Workspace: Replace separate
log_analytics_workspace_nameandlog_analytics_workspace_rgvariables with a singlelog_analytics_workspace_idvariable. Update references accordingly, e.g., inazurerm_monitor_diagnostic_setting.Variable Naming Consistency: Use snake_case for variables (e.g.,
psql_versioninstead ofpsql_Version,psql_storage_tierinstead ofpsql_StorageTier).Maintenance Configuration: Fix mismatch between default value and description for
psql_maintenance_start_minute.Commented-Out Resources: Remove commented-out peering resources from the module; document in README.md if useful for users.
Storage Tier Logic: Review and simplify conditional logic for
storage_tieras it may be redundant.PgBouncer Configuration: Use a boolean value instead of
countfor enabling/disabling configurations.AD Admin Resource: Make
object_idandprincipal_nameconfigurable via variables instead of hardcoding.Outputs and Iteration: Confirm handling of list outputs; for-each should work as is.
Provider Versions: Add major version constraints to
required_providers(e.g., minimum versions).Variable Descriptions: Improve description for
psql_storage_sizeto mention it as the initial size when auto-grow is enabled.Nits: Add newlines at the end of files where missing (e.g., data blocks for virtual network, subnet, diagnostics, outputs, providers).
Proposed Solution
Implement the suggested changes in the PostgreSQL Terraform module located at
infrastructure/modules/psql/.