`azurerm_app_configuration` - support for `replica` #22452

teowa · 2023-07-11T06:57:33Z

support inline replica

resolves #21634
references:

teowa · 2023-07-12T02:39:08Z

please see #21689 (comment)

teowa · 2023-08-25T01:46:18Z

Hi @mbfrahry , should we split this into a seperate resource? if so, I will reopen #21689

tombuildsstuff · 2023-08-25T08:34:40Z

@teowa as discussed offline, this should remain a part of this resource rather than being a separate resource

stephybun

Thanks for this PR @teowa.

I've left some comments in-line. Once those are fixed up we can take another look through. Thanks~!

stephybun · 2023-09-14T13:14:32Z

internal/services/appconfiguration/app_configuration_data_source.go

@@ -93,6 +94,29 @@ func dataSourceAppConfiguration() *pluginsdk.Resource {
 				Computed: true,
 			},

+			"replica": {
+				Type:     pluginsdk.TypeSet,


We can't use sets in data sources because every item gets the same hashcode. This can just be a computed list and should be fine.

fixed, thanks.

stephybun · 2023-09-14T13:15:34Z

internal/services/appconfiguration/app_configuration_resource.go

+							Required:     true,
+							ValidateFunc: validate.ConfigurationStoreReplicaName,
+						},
+						"location": {


This needs to be added to the hash code since this can be updated. Also this should be using the location schema from the commonschema packages. There's a location schema without ForceNew.

fixed, thanks.

stephybun · 2023-09-14T13:19:17Z

internal/services/appconfiguration/app_configuration_resource.go

+
+	replicaClient := meta.(*clients.Client).AppConfiguration.ReplicasClient
+	for _, replica := range expandedReplicas {
+		replicaId := replicas.NewReplicaID(subscriptionId, resourceGroup, name, *replica.Name)


To be consistent we should use the values from the parsed id

Suggested change

replicaId := replicas.NewReplicaID(subscriptionId, resourceGroup, name, *replica.Name)

replicaId := replicas.NewReplicaID(id.SubscriptionId, id.ResourceGroupName, id.Name, *replica.Name)

fixed, thanks.

stephybun · 2023-09-14T13:20:52Z

internal/services/appconfiguration/app_configuration_resource.go

+		existingReplica, err := replicaClient.Get(ctx, replicaId)
+		if err != nil {
+			if !response.WasNotFound(existingReplica.HttpResponse) {
+				return fmt.Errorf("retrieving %s: %+v", replicaId, err)
+			}
+		}


We probably don't need to check if any replica's already exist here since this is in the create and we check this when we get the resource?

Suggested change

existingReplica, err := replicaClient.Get(ctx, replicaId)

if err != nil {

if !response.WasNotFound(existingReplica.HttpResponse) {

return fmt.Errorf("retrieving %s: %+v", replicaId, err)

}

}

fixed, thanks.

stephybun · 2023-09-14T13:25:50Z

internal/services/appconfiguration/app_configuration_resource.go

+			for _, newReplica := range newReplicas.(*pluginsdk.Set).List() {
+				newReplicaMap := newReplica.(map[string]interface{})
+
+				if strings.EqualFold(oldReplicaMap["name"].(string), newReplicaMap["name"].(string)) && strings.EqualFold(oldReplicaMap["location"].(string), newReplicaMap["location"].(string)) {


To ensure we're comparing these consistently and to prevent bugs we should normalise both location values since it isn't just the casing that can differ in these.

fixed, thanks.

stephybun · 2023-09-14T13:41:51Z

website/docs/d/app_configuration.html.markdown

+
+* `location` - The supported Azure location where the replica exists.
+
+* `name` - (Optional) The name of the replica.


Suggested change

* `name` - (Optional) The name of the replica.

* `name` - The name of the replica.

fixed, thanks.

stephybun · 2023-09-14T13:43:17Z