azurerm_postgresql_flexible_server: add support for cluster#31315
azurerm_postgresql_flexible_server: add support for cluster#31315mbfrahry merged 12 commits intohashicorp:mainfrom
azurerm_postgresql_flexible_server: add support for cluster#31315Conversation
internal/services/postgres/postgresql_flexible_server_resource.go
Outdated
Show resolved
Hide resolved
internal/services/postgres/postgresql_flexible_server_resource_test.go
Outdated
Show resolved
Hide resolved
|
Thanks @wuxu92 , LGTM! |
magodo
left a comment
There was a problem hiding this comment.
Thanks for this PR!
I have left some additional mostly minor comments that once addressed this should be good to merge 👍
internal/services/postgres/postgresql_flexible_server_resource.go
Outdated
Show resolved
Hide resolved
|
Thanks! LGTM! |
|
@wuxu92 |
|
|
||
| versionOld, versionNew := diff.GetChange("version") | ||
| // version should be set since cluster only support pg17+ | ||
| if version, err := strconv.ParseInt(versionNew.(string), 10, 32); err != nil { |
There was a problem hiding this comment.
This is great since the API errors but doesn't really explain why
│ Message: "Unsupported value \"12\" for parameter PGServerVersion"
| // Confirmed with service team: the max size will be 32 whilst current is 20 | ||
| ValidateFunc: validation.IntBetween(1, 32), | ||
| }, | ||
| "default_database_name": { |
There was a problem hiding this comment.
It looks like a database name is provided for this if not specifed. Let's Default this to postgres
~ cluster {
- default_database_name = "postgres" -> null
# (1 unchanged attribute hidden)
}
There was a problem hiding this comment.
Thanks for the heads-up, schema updated
|
|
||
| "cluster": { | ||
| Type: pluginsdk.TypeList, | ||
| Optional: true, |
There was a problem hiding this comment.
This should be ForceNew since we can't add it after it's created or remove it.
| newClusterObj := newCluster.([]interface{}) | ||
| if len(oldClusterObj) > 0 && len(newClusterObj) > 0 { | ||
| // below type assertions are safe since schema ensure the types | ||
| oldClusterSize := oldClusterObj[0].(map[string]interface{})["size"].(int) |
There was a problem hiding this comment.
These can panic since we're only checking the length of oldClusterObj. Unfortunately, the following type assertions aren't safe because Terraform can set any of the following to nil. So we should take the time to ensure it's not nil before casting.
| if len(input) == 0 { | ||
| return nil | ||
| } | ||
| v := input[0].(map[string]interface{}) |
There was a problem hiding this comment.
We should a nil check when we check the len(input) to prevent a potential panic
There was a problem hiding this comment.
right, nil check added.
| "size": { | ||
| Type: pluginsdk.TypeInt, | ||
| Required: true, | ||
| // Confirmed with service team: the max size will be 32 whilst current is 20 |
There was a problem hiding this comment.
Since this causes an error from the api side, we should look to lock this to 20 until they add it
| } | ||
|
|
||
| // can't downgrade cluster size | ||
| oldCluster, newCluster := diff.GetChange("cluster") |
There was a problem hiding this comment.
We should wrap this block in diff.HasChange("cluster") so we don't do these checks unless the cluster has changed
|
Hi @mbfrahry, Is this gonna be available in which version of the azurerm provider? Thanks! |
…icorp#31315) [ENHANCEMENT] `azurerm_postgresql_flexible_server` - add support for `cluster`
Community Note
Description
https://learn.microsoft.com/en-us/azure/postgresql/flexible-server/concepts-elastic-clusters
some items confirmed from service team but not mentioned in swagger or product doc:
PR Checklist
For example: “
resource_name_here- description of change e.g. adding propertynew_property_name_here”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_postgresql_flexible_server- support for theclusterpropertyThis is a (please select all that apply):
Related Issue(s)
Fixes #31212
AI Assistance Disclosure
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the provider.
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
Note
If this PR changes meaningfully during the course of review please update the title and description as required.