Skip to content

Commit

Permalink
fix issue 20228
Browse files Browse the repository at this point in the history
  • Loading branch information
@sinbai
sinbai committed Mar 3, 2023
1 parent ec8e5eb commit 88e461f
Show file tree
Hide file tree
Showing 2 changed files with 159 additions and 21 deletions.
54 changes: 45 additions & 9 deletions internal/services/redis/redis_cache_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import (
"strings"
"time"

"github.com/hashicorp/terraform-provider-azurerm/internal/services/redis/migration"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/redis/validate"

"github.com/hashicorp/go-azure-helpers/lang/pointer"
"github.com/hashicorp/go-azure-helpers/lang/response"
"github.com/hashicorp/go-azure-helpers/resourcemanager/commonschema"
Expand All @@ -17,6 +20,8 @@ import (
"github.com/hashicorp/go-azure-helpers/resourcemanager/zones"
"github.com/hashicorp/go-azure-sdk/resource-manager/redis/2021-06-01/patchschedules"
"github.com/hashicorp/go-azure-sdk/resource-manager/redis/2021-06-01/redis"
"github.com/hashicorp/go-cty/cty"
"github.com/hashicorp/go-cty/cty/gocty"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-provider-azurerm/helpers/tf"
azValidate "github.com/hashicorp/terraform-provider-azurerm/helpers/validate"
Expand All @@ -25,8 +30,6 @@ import (
"github.com/hashicorp/terraform-provider-azurerm/internal/services/network"
networkParse "github.com/hashicorp/terraform-provider-azurerm/internal/services/network/parse"
networkValidate "github.com/hashicorp/terraform-provider-azurerm/internal/services/network/validate"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/redis/migration"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/redis/validate"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/suppress"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation"
Expand Down Expand Up @@ -818,11 +821,28 @@ func expandRedisConfiguration(d *pluginsdk.ResourceData) (*redis.RedisCommonProp
}

// RDB Backup
if v := raw["rdb_backup_enabled"].(bool); v {
if connStr := raw["rdb_storage_connection_string"].(string); connStr == "" {
return nil, fmt.Errorf("The rdb_storage_connection_string property must be set when rdb_backup_enabled is true")
// the "output.RdbBackupEnabled" should be set value when the rdb_backup_enabled is set to false in config since not setting false means it could not be disabled using patch to update the Redis Cache.
// d.GetOk cannot identify if user sets the property that is bool type and `rdb_backup_enabled` is set as `false`. So it has to identify it using `d.GetRawConfig()`
rc := make(map[string]cty.Value, 0)
if err := gocty.FromCtyValue(d.GetRawConfig(), &rc); err != nil {
// RDB Backup
// the "output.RdbBackupEnabled" should be set value when the rdb_backup_enabled is set to false in config since not setting false means it could not be disabled using patch to update the Redis Cache.
// d.GetOk cannot identify if user sets the property that is bool type and `rdb_backup_enabled` is set as `false`. So it has to identify it using `d.GetRawConfig()`
if c := d.GetRawConfig().AsValueMap(); len(c) > 0 {
rc := cty.Value{}
if err := gocty.FromCtyValue(c["redis_configuration"], &rc); err != nil {
if v := c["redis_configuration"].AsValueSlice(); v != nil && len(v) > 0 && !v[0].AsValueMap()["rdb_backup_enabled"].IsNull() {
if v[0].AsValueMap()["rdb_backup_enabled"].True() {
if connStr := raw["rdb_storage_connection_string"].(string); connStr == "" {
return nil, fmt.Errorf("The rdb_storage_connection_string property must be set when rdb_backup_enabled is true")
}
output.RdbBackupEnabled = utils.String("true")
} else {
output.RdbBackupEnabled = utils.String("false")
}
}
}
}
output.RdbBackupEnabled = utils.String(strconv.FormatBool(v))
}

if v := raw["rdb_backup_frequency"].(int); v > 0 {
Expand All @@ -842,8 +862,25 @@ func expandRedisConfiguration(d *pluginsdk.ResourceData) (*redis.RedisCommonProp
}

// AOF Backup
if v := raw["aof_backup_enabled"].(bool); v {
output.AofBackupEnabled = utils.String(strconv.FormatBool(v))
// the "output.AdditionalProperties["aof-backup-enabled"]" should be set value when the aof_backup_enabled is set to false in config since not setting false means it could not be disabled using patch to update the Redis Cache.
// d.GetOk cannot identify if user sets the property that is bool type and `aof_backup_enabled` is set as `false`. So it has to identify it using `d.GetRawConfig()`
rc = make(map[string]cty.Value, 0)
if err := gocty.FromCtyValue(d.GetRawConfig(), &rc); err != nil {
// AOF Backup
// the "output.AdditionalProperties["aof-backup-enabled"]" should be set value when the aof_backup_enabled is set to false in config since not setting false means it could not be disabled using patch to update the Redis Cache.
// d.GetOk cannot identify if user sets the property that is bool type and `aof_backup_enabled` is set as `false`. So it has to identify it using `d.GetRawConfig()`
if c := d.GetRawConfig().AsValueMap(); len(c) > 0 {
rc := cty.Value{}
if err := gocty.FromCtyValue(c["redis_configuration"], &rc); err != nil {
if v := d.GetRawConfig().AsValueMap()["redis_configuration"].AsValueSlice(); v != nil && len(v) > 0 && !v[0].AsValueMap()["aof_backup_enabled"].IsNull() {
if v[0].AsValueMap()["aof_backup_enabled"].True() {
output.AofBackupEnabled = utils.String("true")
} else {
output.AofBackupEnabled = utils.String("false")
}
}
}
}
}

if v := raw["aof_storage_connection_string_0"].(string); v != "" {
Expand All @@ -853,7 +890,6 @@ func expandRedisConfiguration(d *pluginsdk.ResourceData) (*redis.RedisCommonProp
if v := raw["aof_storage_connection_string_1"].(string); v != "" {
output.AofStorageConnectionString1 = utils.String(v)
}

authEnabled := raw["enable_authentication"].(bool)
// Redis authentication can only be disabled if it is launched inside a VNET.
if _, isPrivate := d.GetOk("subnet_id"); !isPrivate {
Expand Down
126 changes: 114 additions & 12 deletions internal/services/redis/redis_cache_resource_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@ func TestAccRedisCache_BackupDisabled(t *testing.T) {
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
// `redis_configuration.0.aof_storage_connection_string_0` and `redis_configuration.0.aof_storage_connection_string_1` are returned as:
// "...;AccountKey=[key hidden]" rather than "...;AccountKey=fsjfvjnfnf"
// TODO: remove this once the Bug's been fixed:
// https://github.com/Azure/azure-rest-api-specs/issues/3037
ExpectNonEmptyPlan: true,
},
})
}
Expand Down Expand Up @@ -178,7 +183,7 @@ func TestAccRedisCache_BackupEnabledDisabled(t *testing.T) {
check.That(data.ResourceName).ExistsInAzure(r),
),
// `redis_configuration.0.rdb_storage_connection_string` is returned as:
// "...;AccountKey=[key hidden]" rather than "...;AccountKey=fsjfvjnfnf"
// "...;AccountKey=[key hidden]" rather than "...;AccountKey=fsjfvjnfnf..."
// TODO: remove this once the Bug's been fixed:
// https://github.com/Azure/azure-rest-api-specs/issues/3037
ExpectNonEmptyPlan: true,
Expand All @@ -188,8 +193,8 @@ func TestAccRedisCache_BackupEnabledDisabled(t *testing.T) {
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
// `redis_configuration.0.rdb_storage_connection_string` is returned as:
// "...;AccountKey=[key hidden]" rather than "...;AccountKey=fsjfvjnfnf"
// `redis_configuration.0.aof_storage_connection_string_0` and `redis_configuration.0.aof_storage_connection_string_1` are returned as:
// "...;AccountKey=[key hidden]" rather than "...;AccountKey=fsjfvjnfnf..."
// TODO: remove this once the Bug's been fixed:
// https://github.com/Azure/azure-rest-api-specs/issues/3037
ExpectNonEmptyPlan: true,
Expand Down Expand Up @@ -224,13 +229,19 @@ func TestAccRedisCache_AOFBackupEnabledDisabled(t *testing.T) {
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
// `redis_configuration.0.aof_storage_connection_string_0` and `aof_storage_connection_string_1` are returned as:
// "...;AccountKey=[key hidden]" rather than "...;AccountKey=fsjfvjnfnf..."
// TODO: remove this once the Bug's been fixed:
ExpectNonEmptyPlan: true,
},
{
Config: r.aofBackupDisabled(data),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
// `redis_configuration.0.rdb_storage_connection_string` is returned as:
// "...;AccountKey=[key hidden]" rather than "...;AccountKey=fsjfvjnfnf..."
// TODO: remove this once the Bug's been fixed:
ExpectNonEmptyPlan: true,
},
})
Expand Down Expand Up @@ -710,6 +721,42 @@ resource "azurerm_resource_group" "test" {
location = "%s"
}
resource "azurerm_storage_account" "test" {
name = "acctestsa%s"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
account_tier = "Standard"
account_replication_type = "GRS"
tags = {
environment = "staging"
}
}
resource "azurerm_storage_account" "test2" {
name = "acctestsa2%s"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
account_tier = "Standard"
account_replication_type = "GRS"
tags = {
environment = "staging"
}
}
resource "azurerm_storage_account" "test3" {
name = "acctestsa3%s"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
account_tier = "Standard"
account_replication_type = "GRS"
tags = {
environment = "staging"
}
}
resource "azurerm_redis_cache" "test" {
name = "acctestRedis-%d"
location = azurerm_resource_group.test.location
Expand All @@ -720,10 +767,13 @@ resource "azurerm_redis_cache" "test" {
enable_non_ssl_port = false
redis_configuration {
rdb_backup_enabled = false
rdb_backup_enabled = false
aof_backup_enabled = true
aof_storage_connection_string_0 = azurerm_storage_account.test2.primary_connection_string
aof_storage_connection_string_1 = azurerm_storage_account.test3.primary_connection_string
}
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger)
`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomString, data.RandomString, data.RandomInteger)
}

func (RedisCacheResource) backupEnabled(data acceptance.TestData) string {
Expand Down Expand Up @@ -762,7 +812,7 @@ resource "azurerm_redis_cache" "test" {
rdb_backup_enabled = true
rdb_backup_frequency = 60
rdb_backup_max_snapshot_count = 1
rdb_storage_connection_string = "DefaultEndpointsProtocol=https;BlobEndpoint=${azurerm_storage_account.test.primary_blob_endpoint};AccountName=${azurerm_storage_account.test.name};AccountKey=${azurerm_storage_account.test.primary_access_key}"
rdb_storage_connection_string = azurerm_storage_account.test.primary_connection_string
}
}
`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger)
Expand All @@ -779,6 +829,42 @@ resource "azurerm_resource_group" "test" {
location = "%s"
}
resource "azurerm_storage_account" "test" {
name = "acctestsa%s"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
account_tier = "Standard"
account_replication_type = "GRS"
tags = {
environment = "staging"
}
}
resource "azurerm_storage_account" "test2" {
name = "acctestsa2%s"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
account_tier = "Standard"
account_replication_type = "GRS"
tags = {
environment = "staging"
}
}
resource "azurerm_storage_account" "test3" {
name = "acctestsa3%s"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
account_tier = "Standard"
account_replication_type = "GRS"
tags = {
environment = "staging"
}
}
resource "azurerm_redis_cache" "test" {
name = "acctestRedis-%d"
location = azurerm_resource_group.test.location
Expand All @@ -789,10 +875,14 @@ resource "azurerm_redis_cache" "test" {
enable_non_ssl_port = false
redis_configuration {
aof_backup_enabled = false
aof_backup_enabled = false
rdb_backup_enabled = true
rdb_backup_frequency = 60
rdb_backup_max_snapshot_count = 1
rdb_storage_connection_string = azurerm_storage_account.test3.primary_connection_string
}
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger)
`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomString, data.RandomString, data.RandomInteger)
}

func (RedisCacheResource) aofBackupEnabled(data acceptance.TestData) string {
Expand All @@ -807,7 +897,19 @@ resource "azurerm_resource_group" "test" {
}
resource "azurerm_storage_account" "test" {
name = "unlikely23exst2acct%s"
name = "acctestsa%s"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
account_tier = "Standard"
account_replication_type = "GRS"
tags = {
environment = "staging"
}
}
resource "azurerm_storage_account" "test2" {
name = "acctestsa2%s"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
account_tier = "Standard"
Expand All @@ -829,11 +931,11 @@ resource "azurerm_redis_cache" "test" {
redis_configuration {
aof_backup_enabled = true
aof_storage_connection_string_0 = "DefaultEndpointsProtocol=https;BlobEndpoint=${azurerm_storage_account.test.primary_blob_endpoint};AccountName=${azurerm_storage_account.test.name};AccountKey=${azurerm_storage_account.test.primary_access_key}"
aof_storage_connection_string_1 = "DefaultEndpointsProtocol=https;BlobEndpoint=${azurerm_storage_account.test.primary_blob_endpoint};AccountName=${azurerm_storage_account.test.name};AccountKey=${azurerm_storage_account.test.secondary_access_key}"
aof_storage_connection_string_0 = azurerm_storage_account.test.primary_connection_string
aof_storage_connection_string_1 = azurerm_storage_account.test2.primary_connection_string
}
}
`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger)
`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomString, data.RandomInteger)
}

func (RedisCacheResource) patchSchedule(data acceptance.TestData) string {
Expand Down

0 comments on commit 88e461f

Please sign in to comment.