New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Terraform IaC for Azure PostgreSQL Single Server with Database and Private Endpoint #98
Terraform IaC for Azure PostgreSQL Single Server with Database and Private Endpoint #98
Conversation
…from CIBCTO/fdx-cloud-service-certification-working:release/az-postgresql-tf to release/80-az-postgresql-tf Squashed commit of the following: commit 0aa57e2dbc3bee6d556345050c3effee1cd65293 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Wed Jan 27 14:46:31 2021 +0000 resource provider commit 8e4db96551f0b7327c3b274d0a569eb362eefdf6 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Wed Jan 27 14:45:06 2021 +0000 files dropped out of git when renaming module commit 28d18649f6b35dac0f721788b542dceaf45fe459 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Wed Jan 27 14:43:53 2021 +0000 PR changes commit d7a7bc6d8cbb63b20eeaeccecdf85501273cec25 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Mon Jan 25 10:33:16 2021 +0000 syntax commit a5368995af0ab0480fc54ae558b8d1f35ea4db31 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Mon Jan 25 10:32:30 2021 +0000 add description to vars2 commit e44bf0022f47fd9866d5e5700f20ec98384a692e Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Mon Jan 25 10:31:05 2021 +0000 add description to vars commit b9ce96a1fed3439f2bda734f7c9ddfb79db3e0a1 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Mon Jan 25 10:30:04 2021 +0000 delete non module variables commit 18bbf2c7cec469e317f67e98da8fb5305c58499c Merge: 21ba7c5 44ae054 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Mon Jan 25 09:54:39 2021 +0000 Merge branches 'master' and 'release/az-postgresql-tf' of ssh://stash.gto.intranet.db.com:7998/cibcto/fdx-cloud-service-certification-working into release/az-postgresql-tf commit 21ba7c55535c5165653aa858d786e4543878bd2d Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Fri Jan 15 15:23:44 2021 +0000 readme clarification commit a2ec5615cf65220c754129f31eac5a50fa86b856 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Fri Jan 15 14:54:36 2021 +0000 edit readme commit cb760a4cb117c282da23211e97aa4614db942941 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Fri Jan 15 14:25:48 2021 +0000 remove .idea commit bfe591fea9fe63ae1fcd440153eca05c7cb25afd Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Fri Jan 15 14:22:01 2021 +0000 add accidentally deleted files commit 1a921dcbda925264ab0e6fd81c06aae93e41a945 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Fri Jan 15 14:19:39 2021 +0000 copy tf over
Amazing pull request and contribution 🚀 Thank you @peterrhysthomas, @daniela-g-zheleva-db, @fleadsom and Deutsche Bank ..! 👏 💯 💯 💯 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great work. All of my comments are nits or minor suggestions.
Concerning the repo structure- perhaps it would be beneficial to segment the azure tf modules from their implementation logic? A common directory layout I've seen would look something like this:
azure/
aks/
postgresql/
modules/
aks/
postgresql/
| postgres\_resource\_group\_name | Name of resource group to hold postgres | `string` | n/a | yes | | ||
| postgres\_resource\_group\_tags | Tags to be added to the postgres resource group | `map` | n/a | yes | | ||
| postgres\_tags | n/a | `map` | n/a | yes | | ||
| postgres\_version | Version of porstgres | `string` | n/a | yes | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: porstgres
server_name = azurerm_postgresql_server.postgres-server.name | ||
value = "on" | ||
} | ||
#Configure postgresql |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: The spacing between resources here is sometimes 1 space and sometimes 0
default = 7 | ||
} | ||
#Enable geo redundency for Azure postgres | ||
#Enable geo redundancy for Azure postgres |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Duplicate line here
#Enable geo redundancy for Azure postgres | ||
variable "geo_redundent_enabled" { | ||
type = bool | ||
description = "Geo redundent postgres enablement, currently supports only: true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious, why does this support only true
? I noticed this below as well. Are these intended to be improved upon later- or why have a variable with only one supported value?
#Password for the database | ||
variable "database_password" { | ||
type = string | ||
description = "Password for the database in postgres" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps use the sensitive
value for the credentials variables?
https://learn.hashicorp.com/tutorials/terraform/sensitive-variables#refactor-database-credentials
@@ -0,0 +1,16 @@ | |||
output "server_name" { | |||
value = "${var.postgres_name}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
Perhaps this file doesn't need the interpolation syntax?
Possible improvement: value = var.postgres_name
… from CIBCTO/fdx-cloud-service-certification-working:release/az-postgresql-tf to release/80-az-postgresql-tf Squashed commit of the following: commit fa9baf162be8cbd4945088cf6a15757d0336120c Merge: c7e274c 3281d3e Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Fri Feb 12 18:03:07 2021 +0000 Merge branches 'release/80-az-postgresql-tf' and 'release/az-postgresql-tf' of ssh://stash.gto.intranet.db.com:7998/cibcto/fdx-cloud-service-certification-working into release/az-postgresql-tf � Conflicts: � azure/postgresql/terraform/README.md commit c7e274c33c3fecaa94b4f468cb67dbcfed8686af Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Fri Feb 12 17:59:00 2021 +0000 Add missing file and readme comment commit 0aa57e2dbc3bee6d556345050c3effee1cd65293 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Wed Jan 27 14:46:31 2021 +0000 resource provider commit 8e4db96551f0b7327c3b274d0a569eb362eefdf6 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Wed Jan 27 14:45:06 2021 +0000 files dropped out of git when renaming module commit 28d18649f6b35dac0f721788b542dceaf45fe459 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Wed Jan 27 14:43:53 2021 +0000 PR changes commit d7a7bc6d8cbb63b20eeaeccecdf85501273cec25 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Mon Jan 25 10:33:16 2021 +0000 syntax commit a5368995af0ab0480fc54ae558b8d1f35ea4db31 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Mon Jan 25 10:32:30 2021 +0000 add description to vars2 commit e44bf0022f47fd9866d5e5700f20ec98384a692e Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Mon Jan 25 10:31:05 2021 +0000 add description to vars commit b9ce96a1fed3439f2bda734f7c9ddfb79db3e0a1 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Mon Jan 25 10:30:04 2021 +0000 delete non module variables commit 18bbf2c7cec469e317f67e98da8fb5305c58499c Merge: 21ba7c5 44ae054 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Mon Jan 25 09:54:39 2021 +0000 Merge branches 'master' and 'release/az-postgresql-tf' of ssh://stash.gto.intranet.db.com:7998/cibcto/fdx-cloud-service-certification-working into release/az-postgresql-tf commit 21ba7c55535c5165653aa858d786e4543878bd2d Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Fri Jan 15 15:23:44 2021 +0000 readme clarification commit a2ec5615cf65220c754129f31eac5a50fa86b856 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Fri Jan 15 14:54:36 2021 +0000 edit readme commit cb760a4cb117c282da23211e97aa4614db942941 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Fri Jan 15 14:25:48 2021 +0000 remove .idea commit bfe591fea9fe63ae1fcd440153eca05c7cb25afd Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Fri Jan 15 14:22:01 2021 +0000 add accidentally deleted files commit 1a921dcbda925264ab0e6fd81c06aae93e41a945 Author: Daniela-G Zheleva <daniela-g.zheleva@db.com> Date: Fri Jan 15 14:19:39 2021 +0000 copy tf over
Thanks @eddie-knight for the comments! I thought I had spotted all the 'porstgres' typos 😄 I will make these changes and push them into the PR as soon as possible. |
Thanks @eddie-knight and @daniela-g-zheleva-db for collaborating on this PR. It's great to see the Cloud Service Certification project coming together in such an affective way 👏 💯 💯 💯 |
Per our conversation on the call today, I'm of the opinion that the best bet is to apply the suggested changes in a follow-up PR. FYI, whichever maintainer approves/merges this can use the Web UI to correct the |
Thanks Eddie, @peter-thomas-db Could you please approve this PR and merge. Unfortunately I accidentally deleted the branch internally, and so cannot push changes to this PR. Once it is merged to main I will raise another PR to fix the spelling/spacing issues as pointed out by @eddie-knight |
👏 💯💯💯 |
This PR is part of the epic #80. It consists of code to deploy PostgreSQL to Azure, in compliance with the security concerns described in the Service Accelerator Template already in the repo.
The tf code provisions a PostgreSQL Single Server with database and Private Endpoint.
The tf is dependant on there being a virtual network already provisioned in the Azure environment which can be passed in as an input variable.