Skip to content

feat(terraform): prefer write-only PostgreSQL password arguments#481

Open
davidspielmann wants to merge 5 commits into
microsoft:mainfrom
davidspielmann:feat/postgresql-write-only-admin-password
Open

feat(terraform): prefer write-only PostgreSQL password arguments#481
davidspielmann wants to merge 5 commits into
microsoft:mainfrom
davidspielmann:feat/postgresql-write-only-admin-password

Conversation

@davidspielmann
Copy link
Copy Markdown

@davidspielmann davidspielmann commented May 6, 2026

Addresses #355 (or a first attempt...)

Pull Request

Description

This PR improves the PostgreSQL Flexible Server admin password flow by using AzureRM write-only password arguments where supported.

The change reduces future persistence of the PostgreSQL admin password in Terraform artifacts by replacing standard password arguments with write-only alternatives.

It updates the PostgreSQL Terraform component to prefer AzureRM write-only password arguments where supported.

Related Issue

Relates to #355

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Blueprint modification or addition
  • Component modification or addition
  • Documentation update
  • CI/CD pipeline change
  • Other (please describe):

Implementation Details

This PR updates the PostgreSQL Terraform component to prefer write-only arguments for the admin password flow. I have changed the following:

  • Replaces the Key Vault secret value argument with value_wo
  • Adds value_wo_version for the Key Vault admin password secret
  • Replaces PostgreSQL Flexible Server administrator_password with administrator_password_wo
  • Adds administrator_password_wo_version
  • Adds admin_password_wo_version as a configurable version value for write-only password updates
  • Raises the Terraform minimum version from >= 1.9.8 to >= 1.12.0, because write-only arguments require Terraform 1.11 or later.

Testing Performed

  • Terraform plan/apply
  • Blueprint deployment test
  • Unit tests
  • Integration tests
  • Bug fix includes regression test (see Test Policy)
  • Manual validation
  • Other: Ran terraform fmt and terraform validate for src/000-cloud/035-postgresql/terraform

Validation Steps

Checklist

  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed
  • I have run terraform fmt on all Terraform code
  • I have run terraform validate on all Terraform code
  • I have run az bicep format on all Bicep code
  • I have run az bicep build to validate all Bicep code
  • I have checked for any sensitive data/tokens that should not be committed
  • Lint checks pass (run applicable linters for changed file types)

Security Review

  • No credentials, secrets, or tokens are hardcoded or logged
  • RBAC and identity changes follow least-privilege principles
  • No new network exposure or public endpoints introduced without justification
  • Dependency additions or updates have been reviewed for known vulnerabilities
  • Container image changes use pinned digests or SHA references

Additional Notes

There may be additional future potential to further reduce sensitive value persistence by adopting Terraform ephemeral values, such as replacing managed password generation with an ephemeral "random_password" block where appropriate. That is intentionally left out of this PR to keep the scope rather small.

@davidspielmann davidspielmann requested a review from a team as a code owner May 6, 2026 13:14
@davidspielmann
Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@katriendg
Copy link
Copy Markdown
Collaborator

@davidspielmann thank you so much for this PR and the details in the issue. This is definitely something we want to include, though given the changes in provider versions, we have some pre-work to do before your changes can merge. The issue #484 tracks this. Once that is closed, you will need to rebase this branch and update any remaining work.

One of the things you will need to do is ensure you run some of the commands to update generated docs like npm run tf-docs to expose the variable.

I believe for now you can wait for our sign to have the version upgrade done before you spend any time on this PR. Thank you!

@katriendg
Copy link
Copy Markdown
Collaborator

@davidspielmann thank you for your patience. The PR #487 has now been merged, and versions.tf upgraded. You can now rebase and add your proposed changes (small conflict you can rebase clean and go from there).
Then also ensure you run the npm run tf-docs and we should be good. Thank you!

@davidspielmann davidspielmann force-pushed the feat/postgresql-write-only-admin-password branch from 5d373f2 to 13fac81 Compare May 12, 2026 14:59
@davidspielmann
Copy link
Copy Markdown
Author

Thank you again for the guidance and for getting the version upgrade merged.

I have rebased this PR onto the current main branch, resolved the conflicts, and regenerated the Terraform docs with npm run tf-docs.

I believe that should be everything from my side for now? If not, please let me know :)

Copy link
Copy Markdown
Collaborator

@katriendg katriendg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your changes, and docs all look good.
No changes in blueprints, which is fine because of the default value. Looks good to merge, appreciate your contribution @davidspielmann

@rezatnoMsirhC
Copy link
Copy Markdown
Contributor

Thank you for this contribution and for your patience working through the version upgrade dependency.

A question about blueprint deployments: with the switch to administrator_password_wo, is there still a way for blueprint users to rotate the PostgreSQL admin password?

Before this change, updating postgresql_admin_password in a blueprint would cause Terraform to detect a diff on administrator_password in state and push the new value to the server. After this change, write-only values are never stored in state, so Terraform has nothing to compare. Changing postgresql_admin_password in a blueprint will produce no plan diff and the server password will not be updated.

The admin_password_wo_version increment is the only mechanism that triggers a re-apply of the write-only value, but that variable is not surfaced in any of the three blueprints that reference this component (full-single-node-cluster, full-multi-node-cluster, blueprints/modules/robotics). Adding a postgresql_admin_password_wo_version variable to each blueprint (defaulting to 1) and wiring it through to the module call would restore the rotation path without breaking existing deployments.

@github-actions
Copy link
Copy Markdown

📚 Documentation Health Report

Generated on: 2026-05-15 10:35:44 UTC

📈 Documentation Statistics

Category File Count
Main Documentation 220
Infrastructure Components 197
Blueprints 39
GitHub Resources 44
AI Assistant Guides (Copilot) 17
Total 517

🏗️ Three-Tree Architecture Status

  • ✅ Bicep Documentation Tree: Auto-generated navigation
  • ✅ Terraform Documentation Tree: Auto-generated navigation
  • ✅ README Documentation Tree: Manual README organization

🔍 Quality Metrics

  • Frontmatter Validation:
    success
  • Link Validation: success

This report is automatically generated by the Documentation Automation workflow.

@github-actions
Copy link
Copy Markdown

📚 Documentation Health Report

Generated on: 2026-05-15 10:42:32 UTC

📈 Documentation Statistics

Category File Count
Main Documentation 220
Infrastructure Components 197
Blueprints 39
GitHub Resources 44
AI Assistant Guides (Copilot) 17
Total 517

🏗️ Three-Tree Architecture Status

  • ✅ Bicep Documentation Tree: Auto-generated navigation
  • ✅ Terraform Documentation Tree: Auto-generated navigation
  • ✅ README Documentation Tree: Manual README organization

🔍 Quality Metrics

  • Frontmatter Validation:
    success
  • Link Validation: success

This report is automatically generated by the Documentation Automation workflow.

@davidspielmann
Copy link
Copy Markdown
Author

Thanks @rezatnoMsirhC, that makes sense. Using the write-only arguments turns out to be more involved than I initially expected.

I have added postgresql_admin_password_wo_version to the three blueprints you mentioned:

  • full-single-node-cluster
  • full-multi-node-cluster
  • blueprints/modules/robotics

The variable defaults to 1 and is now wired through to the PostgreSQL component as admin_password_wo_version, so blueprint users can rotate the PostgreSQL admin password by updating postgresql_admin_password and incrementing postgresql_admin_password_wo_version. I hope this makes sense? I also regenerated the Terraform docs with npm run tf-docs.
One follow-up question: What about blueprints/robotics/terraform and blueprints/azureml/terraform? Would it make sense to add this there as well?

@rezatnoMsirhC
Copy link
Copy Markdown
Contributor

Yes, please add postgresql_admin_password_wo_version to both blueprints/robotics/terraform and blueprints/azureml/terraform as well.

Both blueprints delegate their PostgreSQL configuration directly to blueprints/modules/robotics/terraform (the module "robotics" call in each main.tf). Since that module now exposes postgresql_admin_password_wo_version, the outer blueprints need to forward the variable or users of those blueprints will have no way to trigger a password rotation.

The pattern is the same as what you applied to the other three blueprints: declare a postgresql_admin_password_wo_version variable defaulting to 1 in each blueprint's variables.tf, then wire it through to the module call:

postgresql_admin_password_wo_version = var.postgresql_admin_password_wo_version

After making those changes, run npm run tf-docs once more to regenerate the READMEs for those two blueprints, and this should be ready to merge.

@davidspielmann
Copy link
Copy Markdown
Author

Thank you, I added postgresql_admin_password_wo_version to both outer blueprints (blueprints/robotics/terraform and blueprints/azureml/terraform). I also regenerated the Terraform docs.

@rezatnoMsirhC
Copy link
Copy Markdown
Contributor

Thanks @davidspielmann, the changes to both blueprints/robotics/terraform and blueprints/azureml/terraform look correct. The variable is declared consistently (type = number, default = 1) and wired through to the module "robotics" call in each blueprint. The regenerated docs are also in order. This PR is ready to merge from a review standpoint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants