Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: unstable response values may cause provider produced inconsistant results #687

Merged
merged 2 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
4 changes: 4 additions & 0 deletions internal/services/azapi_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions internal/services/azapi_resource_data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions internal/services/azapi_resource_list_data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions internal/services/azapi_update_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
17 changes: 17 additions & 0 deletions internal/services/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,20 @@ 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",
"lastUpdatedOn",
"lastUpdated",
"lastUpdatedTime",
"lastUpdatedTimeUtc",
"modifiedOn",
"lastModifiedUtc",
"lastModifiedTimeUtc",
"lastModifiedAt",
"lastModifiedBy",
"lastModifiedByType",
}
}
25 changes: 25 additions & 0 deletions utils/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
83 changes: 83 additions & 0 deletions utils/json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Loading