feat(terraform): add the Azure postgres module - #14101
Merged
Merged
Conversation
This was referenced Aug 19, 2026
Contributor
Greptile SummaryThe PR adds an Azure PostgreSQL Flexible Server Terraform module with private networking, authentication options, storage and backup validation, high availability, and monitoring alerts.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
Caller[Terraform caller] --> Module[Azure PostgreSQL module]
Module --> DNS[Private DNS zone and VNet link]
Module --> Server[PostgreSQL Flexible Server]
Server --> Subnet[Delegated subnet]
Module --> Admin[Optional Entra administrator]
Server --> Database[Protected PostgreSQL database]
Server --> Alerts[CPU memory storage connections and IOPS alerts]
Alerts --> Actions[Optional action groups]
Reviews (4): Last reviewed commit: "feat(terraform): add the Azure postgres ..." | Re-trigger Greptile |
Contributor
There was a problem hiding this comment.
3 issues found across 5 files
Confidence score: 2/5
deployment/terraform/modules/azure/postgres/main.tfcan produce an invalid default PostgreSQL server because password authentication remains enabled whilepasswordis null, causing deployment failure; require a password whenever password authentication is enabled.deployment/terraform/modules/azure/postgres/main.tfenables Entra authentication without configuring the required Entra administrator, so Entra identities cannot log in; add the administrator resource and object ID input or explicitly reject the configuration.deployment/terraform/modules/azure/postgres/main.tfacceptsbackup_retention_daysvalues from 1–6 but forwards unsupported values to Azure, causing deployment failure; enforce Azure’s 7-day minimum in validation.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="deployment/terraform/modules/azure/postgres/main.tf">
<violation number="1" location="deployment/terraform/modules/azure/postgres/main.tf:53">
P1: When callers use the module defaults, password authentication remains enabled but this argument passes a null password, which Azure requires for a default server. Require `password` whenever password authentication is enabled, or make it a required input.</violation>
<violation number="2" location="deployment/terraform/modules/azure/postgres/main.tf:55">
P2: When `backup_retention_days` is between 1 and 6, this resource forwards a value Azure does not support, so the module fails after accepting its own validation. Enforce Azure's 7-day minimum before assigning this argument.</violation>
<violation number="3" location="deployment/terraform/modules/azure/postgres/main.tf:61">
P1: When `enable_entra_authentication` is true, this module enables the flag but never configures the Entra administrator required for any Entra identity to log in. Add an administrator resource and object ID input, or explicitly require and document that callers provision this dependency.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
justin-tahara
force-pushed
the
jtahara/azure-tf-postgres
branch
from
August 19, 2026 22:47
6ec2275 to
1020f11
Compare
Contributor
There was a problem hiding this comment.
1 issue found across 5 files
Confidence score: 3/5
deployment/terraform/modules/azure/postgres/variables.tfvalidatesstorage_tieronly against the global tier list, allowing unsupportedstorage_tier/storage_gbcombinations to pass planning and fail during deployment — validate the tier against the selected storage size.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="deployment/terraform/modules/azure/postgres/variables.tf">
<violation number="1" location="deployment/terraform/modules/azure/postgres/variables.tf:67">
P2: Validate `storage_tier` against `storage_gb`, not only against the global tier list. Azure supports tiers such as `P60` and `P80` only for specific storage sizes, so invalid combinations currently pass plan and fail during apply.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
justin-tahara
force-pushed
the
jtahara/azure-tf-postgres
branch
from
August 19, 2026 23:14
1020f11 to
95fcaa7
Compare
justin-tahara
force-pushed
the
jtahara/azure-tf-postgres
branch
from
August 19, 2026 23:31
95fcaa7 to
21e8fbb
Compare
jmelahman
approved these changes
Aug 20, 2026
Mirrors deployment/terraform/modules/aws/s3. Backs the Onyx file store, which already speaks Azure Blob through backend/onyx/file_store/azure_blob_file_store.py. The outputs line up with the environment the app reads: storage_account_name, primary_blob_endpoint and container_name feed AZURE_STORAGE_ACCOUNT_NAME, AZURE_STORAGE_ACCOUNT_URL and AZURE_FILE_STORE_CONTAINER_NAME. Three places where Azure does not map cleanly onto the S3 module: - Shared access keys are off by default. The app authenticates with DefaultAzureCredential, so a key never has to exist, and the account defaults to Entra ID authentication. - Network rules replace the bucket policy. With no allowlist the account stays reachable and is gated on Entra ID alone, which is how the s3 module behaves with no policy attached. Supplying subnets or IPs flips it to deny-first. - Cool tiering waits 30 days rather than the s3 module's 7. Azure has no Intelligent-Tiering, and Cool bills a 30-day minimum per blob, so moving earlier costs more than it saves.
Mirrors deployment/terraform/modules/aws/postgres: a private database server, one database on it, and the same five alerts, silent until a caller supplies somewhere to send them. The server always joins a delegated subnet, so it has no public endpoint. That also means it resolves only through a private DNS zone, which the module creates and links to the virtual network unless the caller supplies one. Where the interface has to differ from the AWS module: - Storage comes off a fixed ladder of sizes rather than an arbitrary GiB count, so an off-ladder value is rejected here instead of at apply. Azure offers auto-grow as a switch with no ceiling, replacing max_storage_gb. - Memory and storage alerts are inverted. Azure publishes percent used where CloudWatch publishes bytes free, so the free-storage floor of 15% becomes a storage_percent ceiling of 85. IOPS alerts against the provisioned limit, which is more useful than an absolute count. - Zone-redundant high availability replaces multi-AZ, and Azure does not offer it on burstable SKUs. The module rejects that combination rather than letting apply fail. - Backups cannot be turned off; the retention floor is one day, not zero. Entra ID authentication is the analogue of RDS IAM auth, and is what will let a workload identity reach the database without a password once the aks module lands. prevent_destroy guards the server and the database, matching AWS.
justin-tahara
force-pushed
the
jtahara/azure-tf-postgres
branch
from
August 20, 2026 21:06
21e8fbb to
b88112a
Compare
Contributor
|
Preview Deployment
|
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
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
Mirrors
deployment/terraform/modules/aws/postgres: a private database server, one database on it, and the same five alerts, silent until a caller supplies somewhere to send them.The server always joins a delegated subnet, so it has no public endpoint. That also means it resolves only through a private DNS zone, which the module creates and links to the virtual network unless the caller supplies one.
Where the interface has to differ from the AWS module:
max_storage_gb.storage_percentceiling of 85. IOPS alerts against the provisioned limit, which is more useful than an absolute count.Entra ID authentication is the analogue of RDS IAM auth, and is what will let a workload identity reach the database without a password once the
aksmodule lands.prevent_destroyguards the server and the database, matching AWS.How Has This Been Tested?
The suite covers the GiB-to-MB conversion, the private DNS zone name Azure requires, the Entra-only path dropping password logins, and six input validations — including the storage-ladder and burstable-plus-HA combinations that would otherwise only fail at apply.
terraform validateand the repo's terraform hooks pass. Not applied against a live subscription.Additional Options
Changes from review (greptile)
entra_authentication_onlyis set. The default path left password authentication on with a null password, which Azure rejects at create.azurerm_postgresql_flexible_server_active_directory_administratorplus the three variables it needs, and a validation that refuses Entra-only without one.storage_tiervalidated against the tiers Azure offers. The regex acceptedP2,P12andP99, none of which exist.Tests: 12 → 18.
Round 2
storage_tieris now validated againststorage_gb, not just against the global tier list. Each size starts at its own default tier and can only be raised within a set list; P60 and above exist only for the 8192 and 16384 GiB volumes. A tier that is valid on its own could still be one Azure refuses at that size.public_network_access_enabled = falseis now stated outright rather than left to Azure defaulting it for a VNet-integrated server. I checked the provider accepts it alongsidedelegated_subnet_id.backup_retention_daysfloor raised to 7. Flexible Server has no shorter retention; 1–6 passed validation and failed the plan.32768from the storage sizes. Azure's largest volume is 33553408 MB, not32768 * 1024, so that entry produced a value the provider rejects.rejects_entra_only_without_entrainherited a password, so two rules fired and the run did not prove what it claimed.Tests: 18 → 23.
Round 3
The storage-tier rule no longer asserts a ceiling. greptile was right that the capacity-specific allowlist rejected valid configurations —
8192withP70among them. I did not take the suggested edit, because it still asserts ceilings ("8192" = ["P60", "P70"]excludesP80) that I cannot verify any better than the ones it replaces.Instead the rule now checks only what I am confident about: a size cannot go below its own default tier. Raising the tier to buy IOPS is the documented feature, so no upper bound is asserted at all. Under-constraining costs a little catch; over-constraining rejects deployments that would have worked, which is worse in a published module.
One nuance worth noting:
index()throws rather than failing validation when handed a value that is not in the list, so a bogus tier likeP12produced a function error instead of a clean message. It is wrapped intry()so the tier-list rule reports that case.Tests: 23 (one rewritten from a rejection case into an acceptance case).