azurerm_redis_cache - Check if redis_configuration.aof_backup_enabled configured

[xuzhang3]

Check if aof_backup_enabled configured.
Relate PR: #22309 and issue #21967

[stephybun]

Thanks @xuzhang3. I think we can simplify the logic for setting aof_backup_enabled but other than this should be good to go.

[stephybun]

This can be simplified

Suggested change

	if strings.EqualFold(skuName, string(redis.SkuNamePremium)) {
	output.AofBackupEnabled = utils.String(strconv.FormatBool(v.(bool)))
	} else if v.(bool) && !strings.EqualFold(skuName, string(redis.SkuNamePremium)) {
	return nil, fmt.Errorf("The `aof_backup_enabled` property requires a `Premium` sku to be set")
	}
	if v.(bool) && !strings.EqualFold(skuName, string(redis.SkuNamePremium)) {
	return nil, fmt.Errorf("the `aof_backup_enabled` property requires a `Premium` sku to be set")
	}
	output.AofBackupEnabled = pointer.To(strconv.FormatBool(v.(bool)))

[xuzhang3]

When the SKU is not "Premium", the default "false" will be sent to the service but this is not allowed by service

[stephybun]

From your explanation it sounds like the only thing we need to check then is that the right sku is set?

Suggested change

	if strings.EqualFold(skuName, string(redis.SkuNamePremium)) {
	output.AofBackupEnabled = utils.String(strconv.FormatBool(v.(bool)))
	} else if v.(bool) && !strings.EqualFold(skuName, string(redis.SkuNamePremium)) {
	return nil, fmt.Errorf("The `aof_backup_enabled` property requires a `Premium` sku to be set")
	}
	if !strings.EqualFold(skuName, string(redis.SkuNamePremium)) {
	return nil, fmt.Errorf("the `aof_backup_enabled` property requires a `Premium` sku to be set")
	}
	output.AofBackupEnabled = pointer.To(strconv.FormatBool(v.(bool)))

[xuzhang3]

yes, the SKU check is required.

[xuzhang3]

We need block the request when the SKU is Premium and aof_backup_enabled configured and also need tohandle the default values scenario as the bool type will default sets to false

[stephybun]

@xuzhang3

We need block the request when the SKU is Premium and aof_backup_enabled configured

What is said here contradicts what's being done in this change.

I'm sorry but I'm not following anymore. Local testing isn't showing up any errors and the linked issue and PR have not given me any further insight.

Can you please add an acceptance test that demonstrates the problem this is trying to fix?

[xuzhang3]

sure， I will add the test to cover this scenario

[xuzhang3]

The issue here is redis_configuration.aof_backup_enabled is type bool. Even if not configured, the default value "false" will be set after creation.

Take one scenario for example:

1. Create an Redis instance with SKU Basic
2. Update maxmemory_policy in the redis_configuration.
3. Apply the Terraform command.
   In step 3, redis_configuration.aof_backup_enabled will get the false value though if is not configured. AzureRM will send the request with redis_configuration.aof_backup_enabled=false but service will reject the request because redis_configuration.aof_backup_enabled can only be set when SKU is Premium. So we need check t he SKU type and ignore redis_configuration.aof_backup_enabled if the SKU type is not Premium

Test TF:

resource "azurerm_redis_cache" "example" {
  name                = "xz3redistest"
  location            = azurerm_resource_group.example.location
  resource_group_name = azurerm_resource_group.example.name
  capacity            = 1
  family              = "C"
  sku_name            = "Basic"
  enable_non_ssl_port = false

  redis_configuration {

    # aof_backup_enabled = false
    maxmemory_reserved = 125
    maxmemory_delta    = 125
    maxmemory_policy   = "allkeys-lru"
  }
}

[xuzhang3]

@stephybun Code updated and add a test case.

[stephybun]

Thanks for adding the acceptance test @xuzhang3. LGTM 🎉

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.