Skip to content

Commit

Permalink
PR: #12294
Browse files Browse the repository at this point in the history 
- use server_id instead of server_name in postgresql_flexible_server_configuration resource
- construct id for d.SetId(id.ID()) by FlexibleServerConfigurationId in
update func
- remove error prefix of error messages
- improve error messages while update, delete, wait
- use state.ID in tests and rename test func according to convention
- update documentation remove resource group and server_name, add server_id
- validate name and value is not empty
- remove checking resource not found at deletion

Signed-off-by: Nick Metz <nick.metz@ptvgroup.com>
  • Loading branch information
Nick Metz committed Jul 10, 2021
1 parent b1bf89f commit d9b070c
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 47 deletions.
44 changes: 19 additions & 25 deletions azurerm/internal/services/postgres/postgresql_flexible_server_configuration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

"github.com/Azure/azure-sdk-for-go/services/preview/postgresql/mgmt/2020-02-14-preview/postgresqlflexibleservers"
"github.com/hashicorp/go-azure-helpers/response"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/postgres/parse"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/postgres/validate"
Expand Down Expand Up @@ -42,13 +41,11 @@ func resourcePostgresqlFlexibleServerConfiguration() *pluginsdk.Resource {
ForceNew: true,
},

"resource_group_name": azure.SchemaResourceGroupName(),

"server_name": {
"server_id": {
Type: pluginsdk.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validate.FlexibleServerName,
ValidateFunc: validate.FlexibleServerID,
},

"value": {
Expand All @@ -60,15 +57,20 @@ func resourcePostgresqlFlexibleServerConfiguration() *pluginsdk.Resource {
}

func resourceFlexibleServerConfigurationUpdate(d *pluginsdk.ResourceData, meta interface{}) error {
subscriptionId := meta.(*clients.Client).Account.SubscriptionId
client := meta.(*clients.Client).Postgres.FlexibleServersConfigurationsClient
ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d)
defer cancel()

log.Printf("[INFO] preparing arguments for Azure Postgresql Flexible Server configuration creation.")

name := d.Get("name").(string)
resGroup := d.Get("resource_group_name").(string)
serverName := d.Get("server_name").(string)
serverId, err := parse.FlexibleServerID(d.Get("server_id").(string))
if err != nil {
return err
}

id := parse.NewFlexibleServerConfigurationID(subscriptionId, serverId.ResourceGroup, serverId.Name, name)

props := postgresqlflexibleservers.Configuration{
ConfigurationProperties: &postgresqlflexibleservers.ConfigurationProperties{
Expand All @@ -77,29 +79,22 @@ func resourceFlexibleServerConfigurationUpdate(d *pluginsdk.ResourceData, meta i
},
}

future, err := client.Update(ctx, resGroup, serverName, name, props)
future, err := client.Update(ctx, serverId.ResourceGroup, serverId.Name, name, props)
if err != nil {
return err
return fmt.Errorf("update Azure Postgresql Flexible Server configuration %q (Resource Group %q): %+v", id.ConfigurationName, id.ResourceGroup, err)
}

if err = future.WaitForCompletionRef(ctx, client.Client); err != nil {
return err
return fmt.Errorf("waiting for the Azure Postgresql Flexible Server configuration update %q (Resource Group %q): %+v", id.ConfigurationName, id.ResourceGroup, err)
}

resp, err := client.Get(ctx, resGroup, serverName, name)
if err != nil {
return err
}
if resp.ID == nil {
return fmt.Errorf("Cannot read Azure Postgresql Flexible Server configuration %s (resource group %s) ID", name, resGroup)
}

d.SetId(*resp.ID)
d.SetId(id.ID())

return resourceFlexibleServerConfigurationRead(d, meta)
}

func resourceFlexibleServerConfigurationRead(d *pluginsdk.ResourceData, meta interface{}) error {
subscriptionId := meta.(*clients.Client).Account.SubscriptionId
client := meta.(*clients.Client).Postgres.FlexibleServersConfigurationsClient
ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d)
defer cancel()
Expand All @@ -117,12 +112,11 @@ func resourceFlexibleServerConfigurationRead(d *pluginsdk.ResourceData, meta int
return nil
}

return fmt.Errorf("Error making Read request on Azure Postgresql Flexible Server configuration %s: %+v", id.ConfigurationName, err)
return fmt.Errorf("making Read request on Azure Postgresql Flexible Server configuration %s: %+v", id.ConfigurationName, err)
}

d.Set("name", id.ConfigurationName)
d.Set("resource_group_name", id.ResourceGroup)
d.Set("server_name", id.FlexibleServerName)
d.Set("server_id", parse.NewFlexibleServerID(subscriptionId, id.ResourceGroup, id.FlexibleServerName).ID())

if props := resp.ConfigurationProperties; props != nil {
d.Set("value", props.Value)
Expand All @@ -143,7 +137,7 @@ func resourceFlexibleServerConfigurationDelete(d *pluginsdk.ResourceData, meta i

resp, err := client.Get(ctx, id.ResourceGroup, id.FlexibleServerName, id.ConfigurationName)
if err != nil {
return fmt.Errorf("Error retrieving Azure Postgresql Flexible Server configuration '%s': %+v", id.ConfigurationName, err)
return fmt.Errorf("retrieving Azure Postgresql Flexible Server configuration '%s': %+v", id.ConfigurationName, err)
}

props := postgresqlflexibleservers.Configuration{
Expand All @@ -158,14 +152,14 @@ func resourceFlexibleServerConfigurationDelete(d *pluginsdk.ResourceData, meta i
if response.WasNotFound(future.Response()) {
return nil
}
return err
return fmt.Errorf("deleting Azure Postgresql Flexible Server configuration %q (Resource Group %q): %+v", id.ConfigurationName, id.ResourceGroup, err)
}

if err = future.WaitForCompletionRef(ctx, client.Client); err != nil {
if response.WasNotFound(future.Response()) {
return nil
}
return err
return fmt.Errorf("waiting for Azure Postgresql Flexible Server configuration deletion %q (Resource Group %q): %+v", id.ConfigurationName, id.ResourceGroup, err)
}

return nil
Expand Down
29 changes: 14 additions & 15 deletions azurerm/internal/services/postgres/postgresql_flexible_server_configuration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestAccFlexibleServerConfiguration_backslashQuote(t *testing.T) {
name := "backslash_quote"
data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.template(data, name, "on"),
Config: r.basic(data, name, "on"),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("name").HasValue(name),
Expand All @@ -31,7 +31,7 @@ func TestAccFlexibleServerConfiguration_backslashQuote(t *testing.T) {
},
data.ImportStep(),
{
Config: r.empty(data),
Config: r.template(data),
Check: acceptance.ComposeTestCheckFunc(
data.CheckWithClientForResource(r.checkReset(name), "azurerm_postgresql_flexible_server.test"),
),
Expand All @@ -45,7 +45,7 @@ func TestAccFlexibleServerConfiguration_pgbouncerEnabled(t *testing.T) {
name := "pgbouncer.enabled"
data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.template(data, name, "true"),
Config: r.basic(data, name, "true"),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("name").HasValue(name),
Expand All @@ -54,7 +54,7 @@ func TestAccFlexibleServerConfiguration_pgbouncerEnabled(t *testing.T) {
},
data.ImportStep(),
{
Config: r.empty(data),
Config: r.template(data),
Check: acceptance.ComposeTestCheckFunc(
data.CheckWithClientForResource(r.checkReset(name), "azurerm_postgresql_flexible_server.test"),
),
Expand All @@ -68,7 +68,7 @@ func TestAccFlexibleServerConfiguration_updateApplicationName(t *testing.T) {
name := "application_name"
data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.template(data, name, "Test APP before"),
Config: r.basic(data, name, "Test APP before"),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("name").HasValue(name),
Expand All @@ -77,7 +77,7 @@ func TestAccFlexibleServerConfiguration_updateApplicationName(t *testing.T) {
},
data.ImportStep(),
{
Config: r.template(data, name, "Test APP after"),
Config: r.basic(data, name, "Test APP after"),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("name").HasValue(name),
Expand All @@ -89,7 +89,7 @@ func TestAccFlexibleServerConfiguration_updateApplicationName(t *testing.T) {

func (r PostgresqlFlexibleServerConfigurationResource) checkReset(configurationName string) acceptance.ClientCheckFunc {
return func(ctx context.Context, clients *clients.Client, state *pluginsdk.InstanceState) error {
id, err := parse.FlexibleServerID(state.Attributes["id"])
id, err := parse.FlexibleServerID(state.ID)
if err != nil {
return err
}
Expand Down Expand Up @@ -120,27 +120,26 @@ func (t PostgresqlFlexibleServerConfigurationResource) Exists(ctx context.Contex
return nil, err
}

resp, err := clients.Postgres.FlexibleServersConfigurationsClient.Get(ctx, id.ResourceGroup, id.FlexibleServerName, id.ConfigurationName)
resp, err := clients.Postgres.FlexibleServersConfigurationsClient.Get(ctx, id.ResourceGroup, id.FlexibleServerName, state.Attributes["name"])
if err != nil {
return nil, fmt.Errorf("reading Postgresql Configuration (%s): %+v", id.String(), err)
}

return utils.Bool(resp.ID != nil), nil
}

func (PostgresqlFlexibleServerConfigurationResource) empty(data acceptance.TestData) string {
func (PostgresqlFlexibleServerConfigurationResource) template(data acceptance.TestData) string {
return PostgresqlFlexibleServerResource{}.basic(data)
}

func (r PostgresqlFlexibleServerConfigurationResource) template(data acceptance.TestData, name, value string) string {
func (r PostgresqlFlexibleServerConfigurationResource) basic(data acceptance.TestData, name, value string) string {
return fmt.Sprintf(`
%s
resource "azurerm_postgresql_flexible_server_configuration" "test" {
name = "%s"
resource_group_name = azurerm_resource_group.test.name
server_name = azurerm_postgresql_flexible_server.test.name
value = "%s"
name = "%s"
server_id = azurerm_postgresql_flexible_server.test.id
value = "%s"
}
`, r.empty(data), name, value)
`, r.template(data), name, value)
}
11 changes: 4 additions & 7 deletions website/docs/r/postgresql_flexible_server_configuration.html.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,9 @@ resource "azurerm_postgresql_flexible_server" "example" {
}
resource "azurerm_postgresql_flexible_server_configuration" "example" {
name = "backslash_quote"
resource_group_name = azurerm_resource_group.example.name
server_name = azurerm_postgresql_flexible_server.example.name
value = "on"
name = "backslash_quote"
server_id = azurerm_postgresql_flexible_server.example.id
value = "on"
}
```

Expand All @@ -49,9 +48,7 @@ The following arguments are supported:

* `name` - (Required) Specifies the name of the PostgreSQL Configuration, which needs [to be a valid PostgreSQL configuration name](https://www.postgresql.org/docs/current/static/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIER). Changing this forces a new resource to be created.

* `server_name` - (Required) Specifies the name of the Azure PostgreSQL Flexible Server. Changing this forces a new resource to be created.

* `resource_group_name` - (Required) The name of the resource group in which the Azure PostgreSQL Flexible Server exists. Changing this forces a new resource to be created.
* `server_id` - (Required) The ID of the PostgreSQL Flexible Server where we want to change configuration. Changing this forces a new PostgreSQL Flexible Server Configuration resource.

* `value` - (Required) Specifies the value of the PostgreSQL Configuration. See the PostgreSQL documentation for valid values.

Expand Down

0 comments on commit d9b070c

Please sign in to comment.