bugfix: unstable response values may cause provider produced inconsis…

…tant results (#687) * bugfix: unstable response values may cause provider produced inconsistant results * add more fields
Azure · Dec 5, 2024 · c2f6536 · c2f6536
1 parent ac4a733
commit c2f6536
Show file tree

Hide file tree

Showing 8 changed files with 144 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,9 @@
+## v2.2.0 (unreleased)
+
+BUG FIXES:
+- Fix a bug that the provider produced inconsistent result after apply when default output feature is enabled.  
+Notice: Terraform will detect the `output` field's changes made outside of Terraform since the last "terraform apply". You can run `terraform refresh` to update the state file with the latest values.
+
 ## v2.1.0
 FEATURES:
 - `azapi_resource` resource: Support resource move operation, it allows moving resources from `azurerm` provider.

diff --git a/internal/services/azapi_resource.go b/internal/services/azapi_resource.go
@@ -698,6 +698,7 @@ func (r *AzapiResource) CreateUpdate(ctx context.Context, requestPlan tfsdk.Plan
 				var defaultOutput interface{}
 				if !r.ProviderData.Features.DisableDefaultOutput {
 					defaultOutput = id.ResourceDef.GetReadOnly(responseBody)
+					defaultOutput = utils.RemoveFields(defaultOutput, volatileFieldList())
 				}
 				output, err := buildOutputFromBody(responseBody, plan.ResponseExportValues, defaultOutput)
 				if err != nil {
@@ -758,6 +759,7 @@ func (r *AzapiResource) CreateUpdate(ctx context.Context, requestPlan tfsdk.Plan
 	var defaultOutput interface{}
 	if !r.ProviderData.Features.DisableDefaultOutput {
 		defaultOutput = id.ResourceDef.GetReadOnly(responseBody)
+		defaultOutput = utils.RemoveFields(defaultOutput, volatileFieldList())
 	}
 	output, err := buildOutputFromBody(responseBody, plan.ResponseExportValues, defaultOutput)
 	if err != nil {
@@ -898,6 +900,7 @@ func (r *AzapiResource) Read(ctx context.Context, request resource.ReadRequest,
 	var defaultOutput interface{}
 	if !r.ProviderData.Features.DisableDefaultOutput {
 		defaultOutput = id.ResourceDef.GetReadOnly(responseBody)
+		defaultOutput = utils.RemoveFields(defaultOutput, volatileFieldList())
 	}
 	output, err := buildOutputFromBody(responseBody, model.ResponseExportValues, defaultOutput)
 	if err != nil {
@@ -1032,6 +1035,7 @@ func (r *AzapiResource) ImportState(ctx context.Context, request resource.Import
 	var defaultOutput interface{}
 	if !r.ProviderData.Features.DisableDefaultOutput {
 		defaultOutput = id.ResourceDef.GetReadOnly(responseBody)
+		defaultOutput = utils.RemoveFields(defaultOutput, volatileFieldList())
 	}
 	output, err := buildOutputFromBody(responseBody, state.ResponseExportValues, defaultOutput)
 	if err != nil {

diff --git a/internal/services/azapi_resource_data_source.go b/internal/services/azapi_resource_data_source.go
@@ -265,6 +265,7 @@ func (r *AzapiResourceDataSource) Read(ctx context.Context, request datasource.R
 	var defaultOutput interface{}
 	if !r.ProviderData.Features.DisableDefaultOutput {
 		defaultOutput = id.ResourceDef.GetReadOnly(responseBody)
+		defaultOutput = utils.RemoveFields(defaultOutput, volatileFieldList())
 	}
 	output, err := buildOutputFromBody(responseBody, model.ResponseExportValues, defaultOutput)
 	if err != nil {

diff --git a/internal/services/azapi_resource_list_data_source.go b/internal/services/azapi_resource_list_data_source.go
@@ -11,6 +11,7 @@ import (
 	"github.com/Azure/terraform-provider-azapi/internal/retry"
 	"github.com/Azure/terraform-provider-azapi/internal/services/myvalidator"
 	"github.com/Azure/terraform-provider-azapi/internal/services/parse"
+	"github.com/Azure/terraform-provider-azapi/utils"
 	"github.com/hashicorp/terraform-plugin-framework-timeouts/resource/timeouts"
 	"github.com/hashicorp/terraform-plugin-framework/datasource"
 	"github.com/hashicorp/terraform-plugin-framework/datasource/schema"
@@ -159,6 +160,7 @@ func (r *ResourceListDataSource) Read(ctx context.Context, request datasource.Re
 	var defaultOutput interface{}
 	if !r.ProviderData.Features.DisableDefaultOutput {
 		defaultOutput = responseBody
+		defaultOutput = utils.RemoveFields(defaultOutput, volatileFieldList())
 	}
 	output, err := buildOutputFromBody(responseBody, model.ResponseExportValues, defaultOutput)
 	if err != nil {

diff --git a/internal/services/azapi_update_resource.go b/internal/services/azapi_update_resource.go
@@ -416,6 +416,7 @@ func (r *AzapiUpdateResource) CreateUpdate(ctx context.Context, plan tfsdk.Plan,
 	var defaultOutput interface{}
 	if !r.ProviderData.Features.DisableDefaultOutput {
 		defaultOutput = id.ResourceDef.GetReadOnly(responseBody)
+		defaultOutput = utils.RemoveFields(defaultOutput, volatileFieldList())
 	}
 	output, err := buildOutputFromBody(responseBody, model.ResponseExportValues, defaultOutput)
 	if err != nil {
@@ -502,6 +503,7 @@ func (r *AzapiUpdateResource) Read(ctx context.Context, request resource.ReadReq
 	var defaultOutput interface{}
 	if !r.ProviderData.Features.DisableDefaultOutput {
 		defaultOutput = id.ResourceDef.GetReadOnly(responseBody)
+		defaultOutput = utils.RemoveFields(defaultOutput, volatileFieldList())
 	}
 	output, err := buildOutputFromBody(responseBody, model.ResponseExportValues, defaultOutput)
 	if err != nil {

diff --git a/internal/services/common.go b/internal/services/common.go
@@ -57,3 +57,24 @@ func buildOutputFromBody(responseBody interface{}, modelResponseExportValues typ
 		return types.DynamicNull(), errors.New("unsupported type for response_export_values, must be a list or map")
 	}
 }
+
+func volatileFieldList() []string {
+	return []string{
+		"etag",
+		"updatedBy",
+		"updated",
+		"updatedOn",
+		"updatedTimestamp",
+		"lastUpdatedOn",
+		"lastUpdated",
+		"lastUpdatedTime",
+		"lastUpdatedTimeUtc",
+		"lastUpdatedDateUTC",
+		"modifiedOn",
+		"lastModifiedUtc",
+		"lastModifiedTimeUtc",
+		"lastModifiedAt",
+		"lastModifiedBy",
+		"lastModifiedByType",
+	}
+}
diff --git a/utils/json.go b/utils/json.go
@@ -289,6 +289,31 @@ func NormalizeObject(input interface{}) interface{} {
 	return output
 }
 
+// RemoveFields is used to remove fields from input
+func RemoveFields(input interface{}, fields []string) interface{} {
+	if input == nil {
+		return input
+	}
+	switch v := input.(type) {
+	case map[string]interface{}:
+		for _, field := range fields {
+			delete(v, field)
+		}
+		for key, value := range v {
+			v[key] = RemoveFields(value, fields)
+		}
+		return v
+	case []interface{}:
+		res := make([]interface{}, 0)
+		for _, item := range v {
+			res = append(res, RemoveFields(item, fields))
+		}
+		return res
+	default:
+		return input
+	}
+}
+
 func isZeroValue(value interface{}) bool {
 	if value == nil {
 		return true

diff --git a/utils/json_test.go b/utils/json_test.go
@@ -1018,3 +1018,86 @@ func Test_UpdateObjectDuplicateIdentifiersWithInconsistentOrdering(t *testing.T)
 		t.Fatalf("Expected:\n%s\n\n but got\n%s", expectedJson, gotJson)
 	}
 }
+
+func Test_RemoveFields(t *testing.T) {
+	testcases := []struct {
+		OldJson    string
+		Fields     []string
+		ExpectJson string
+	}{
+		{
+			OldJson: `
+{
+    "apiVersion": "2024-05-01",
+    "id": "/subscriptions/00000000-0000-0000-0000-00000000000/resourceGroups/acctestheng125/providers/Microsoft.Network/routeTables/acctestheng125",
+    "name": "acctestheng125",
+    "type": "microsoft.network/routetables",
+    "location": "westus",
+    "properties": {
+        "provisioningState": "Succeeded",
+        "resourceGuid": "c7e6268d-eef3-4aa0-86a2-9a2cdedd59a8",
+        "disableBgpRoutePropagation": false,
+        "routes": [
+            {
+                "name": "route1",
+                "id": "/subscriptions/00000000-0000-0000-0000-00000000000/resourceGroups/acctestheng125/providers/Microsoft.Network/routeTables/acctestheng125/routes/route1",
+                "etag": "W/\"8cbb63f5-3125-4a0b-86d0-a4818311b154\"",
+                "properties": {
+                    "provisioningState": "Succeeded",
+                    "addressPrefix": "10.1.0.0/16",
+                    "nextHopType": "VnetLocal",
+                    "nextHopIpAddress": "",
+                    "hasBgpOverride": false
+                },
+                "type": "Microsoft.Network/routeTables/routes"
+            }
+        ]
+    },
+    "etag": "W/\"8cbb63f5-3125-4a0b-86d0-a4818311b154\""
+}
+`,
+			Fields: []string{"etag"},
+			ExpectJson: `
+{
+    "apiVersion": "2024-05-01",
+    "id": "/subscriptions/00000000-0000-0000-0000-00000000000/resourceGroups/acctestheng125/providers/Microsoft.Network/routeTables/acctestheng125",
+    "name": "acctestheng125",
+    "type": "microsoft.network/routetables",
+    "location": "westus",
+    "properties": {
+        "provisioningState": "Succeeded",
+        "resourceGuid": "c7e6268d-eef3-4aa0-86a2-9a2cdedd59a8",
+        "disableBgpRoutePropagation": false,
+        "routes": [
+            {
+                "name": "route1",
+                "id": "/subscriptions/00000000-0000-0000-0000-00000000000/resourceGroups/acctestheng125/providers/Microsoft.Network/routeTables/acctestheng125/routes/route1",
+                "properties": {
+                    "provisioningState": "Succeeded",
+                    "addressPrefix": "10.1.0.0/16",
+                    "nextHopType": "VnetLocal",
+                    "nextHopIpAddress": "",
+                    "hasBgpOverride": false
+                },
+                "type": "Microsoft.Network/routeTables/routes"
+            }
+        ]
+    }
+}
+`,
+		},
+	}
+
+	for _, testcase := range testcases {
+		var old, expected interface{}
+		_ = json.Unmarshal([]byte(testcase.OldJson), &old)
+		_ = json.Unmarshal([]byte(testcase.ExpectJson), &expected)
+
+		result := utils.RemoveFields(old, testcase.Fields)
+		if !reflect.DeepEqual(result, expected) {
+			expectedJson, _ := json.Marshal(expected)
+			resultJson, _ := json.Marshal(result)
+			t.Fatalf("Expected %s but got %s", expectedJson, resultJson)
+		}
+	}
+}