diff --git a/CHANGELOG.md b/CHANGELOG.md index c783bcb..46e7b47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ DEPENDENCIES: - Updated `github.com/Azure/azure-sdk-for-go/sdk/azcore` from v1.16.0 to v1.19.1 BUG FIXES: +- Fixed an issue where `msgraph_resource` failed to update resources when external changes are detected, specifically when clearing array fields ([#58](https://github.com/microsoft/terraform-provider-msgraph/issues/58)) - Fixed an issue where `msgraph_resource` failed to track state for `$ref` resources (relationships), causing drift detection failures ([#68](https://github.com/microsoft/terraform-provider-msgraph/issues/68)) - Fixed an issue where `@odata.type` property was missing in PATCH requests for resources that require it (e.g. Named Locations) ([#59](https://github.com/microsoft/terraform-provider-msgraph/issues/59)) diff --git a/internal/services/msgraph_resource.go b/internal/services/msgraph_resource.go index 157fa1f..b3d9479 100644 --- a/internal/services/msgraph_resource.go +++ b/internal/services/msgraph_resource.go @@ -342,7 +342,7 @@ func (r *MSGraphResource) Update(ctx context.Context, req resource.UpdateRequest patchBody := utils.DiffObject(previousBody, requestBody, diffOption) // If there's something to update, send PATCH - if !utils.IsEmptyObject(patchBody) { + if patchBody != nil { _, err := r.client.Update(ctx, fmt.Sprintf("%s/%s", model.Url.ValueString(), model.Id.ValueString()), model.ApiVersion.ValueString(), patchBody, options) if err != nil { resp.Diagnostics.AddError("Failed to create resource", err.Error()) diff --git a/internal/services/msgraph_resource_test.go b/internal/services/msgraph_resource_test.go index 347197e..d2e4555 100644 --- a/internal/services/msgraph_resource_test.go +++ b/internal/services/msgraph_resource_test.go @@ -278,6 +278,29 @@ func TestAcc_ResourceImport_InvalidIDFormat(t *testing.T) { }) } +func TestAcc_ResourceApplicationRemovesPasswordCredentials(t *testing.T) { + data := acceptance.BuildTestData(t, "msgraph_resource", "test") + + r := MSGraphTestResource{} + + data.ResourceTest(t, r, []resource.TestStep{ + { + Config: r.applicationWithPasswordCredentials(true), + Check: resource.ComposeTestCheckFunc( + check.That(data.ResourceName).Exists(r), + check.That(data.ResourceName).Key("id").IsUUID(), + ), + }, + { + Config: r.applicationWithPasswordCredentials(false), + Check: resource.ComposeTestCheckFunc( + check.That(data.ResourceName).Exists(r), + check.That(data.ResourceName).Key("id").IsUUID(), + ), + }, + }) +} + func (r MSGraphTestResource) Exists(ctx context.Context, client *clients.Client, state *terraform.InstanceState) (*bool, error) { apiVersion := state.Attributes["api_version"] url := state.Attributes["url"] @@ -601,3 +624,26 @@ resource "msgraph_resource" "test" { } `, displayName) } + +func (r MSGraphTestResource) applicationWithPasswordCredentials(enabled bool) string { + passwordCredential := "" + if enabled { + passwordCredential = ` + { + displayName = "first credential" + }` + } + + return fmt.Sprintf(` +resource "msgraph_resource" "test" { + url = "applications" + + body = { + displayName = "My Application" + passwordCredentials = [ + %s + ] + } +} +`, passwordCredential) +} diff --git a/internal/utils/json.go b/internal/utils/json.go index 0d58a87..d1dccf6 100644 --- a/internal/utils/json.go +++ b/internal/utils/json.go @@ -221,7 +221,7 @@ func DiffObject(old interface{}, new interface{}, option UpdateJsonOption) inter // include keys present in new for key, newVal := range newMap { if oldVal, ok := oldValue[key]; ok { - if d := DiffObject(oldVal, newVal, option); !IsEmptyObject(d) { + if d := DiffObject(oldVal, newVal, option); d != nil { res[key] = d } } else { @@ -269,17 +269,3 @@ func DiffObject(old interface{}, new interface{}, option UpdateJsonOption) inter // primitives or differing types -> return new return new } - -// IsEmptyObject returns true if the input should be considered an empty patch -func IsEmptyObject(v interface{}) bool { - if v == nil { - return true - } - switch t := v.(type) { - case map[string]interface{}: - return len(t) == 0 - case []interface{}: - return len(t) == 0 - } - return false -} diff --git a/internal/utils/json_test.go b/internal/utils/json_test.go index baf0745..9969ca0 100644 --- a/internal/utils/json_test.go +++ b/internal/utils/json_test.go @@ -295,6 +295,20 @@ func TestDiffObject(t *testing.T) { "name": "changed", }, }, + { + name: "array changed to empty -> empty array returned (issue #58)", + old: []interface{}{1, 2, 3}, + newV: []interface{}{}, + opt: UpdateJsonOption{}, + want: []interface{}{}, + }, + { + name: "nested array changed to empty -> parent map with empty array returned (issue #58)", + old: map[string]interface{}{"a": []interface{}{1}}, + newV: map[string]interface{}{"a": []interface{}{}}, + opt: UpdateJsonOption{}, + want: map[string]interface{}{"a": []interface{}{}}, + }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { @@ -305,24 +319,3 @@ func TestDiffObject(t *testing.T) { }) } } - -func TestIsEmptyObject(t *testing.T) { - testcases := []struct { - name string - v interface{} - want bool - }{ - {name: "nil", v: nil, want: true}, - {name: "empty map", v: map[string]interface{}{}, want: true}, - {name: "empty slice", v: []interface{}{}, want: true}, - {name: "scalar", v: "x", want: false}, - } - for _, tc := range testcases { - t.Run(tc.name, func(t *testing.T) { - got := IsEmptyObject(tc.v) - if got != tc.want { - t.Fatalf("IsEmptyObject() = %v, want %v (input=%#v)", got, tc.want, tc.v) - } - }) - } -}