Cannot destroy `azuread_synchronization_secret` when complex JSON type is value of secret

[tagur87]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritise this request
• Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritise the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform (and AzureAD Provider) Version

Latest on both

Affected Resource(s)

• azuread_synchronization_secret

Terraform Configuration Files

resource "azuread_synchronization_secret" "this" {
  service_principal_id = azuread_service_principal.this.id

  credential {
    key   = "BaseAddress"
    value = var.scim_base_address
  }
  credential {
    key   = "SecretToken"
    value = var.scim_secret_token
  }
    credential {
    key   = "SyncNotificationSettings"
    value = jsonencode({
      Enabled                                                        = false
      DeleteThresholdEnabled                              = false
      HumanResourcesLookaheadQueryEnabled = false
    })
  }
  credential {
    key   = "SyncAll"
    value = "false"
  }
}

Debug Output

https://gist.github.com/tagur87/c3d186e30f70f754c6867613bd20d702

Expected Behavior

All secrets, including ones with complex value types, are removed when destroying the resource.

Actual Behavior

When destroying a secret with a complex JSON type as a value of a secret, we receive an error from the Graph API.

Steps to Reproduce

1. terraform apply
2. terraform destroy

References

Added comment to MSGraph Metadata Issue about this API endpoint:

https://github.com/microsoftgraph/msgraph-metadata/issues/127#issuecomment-1379210820

[tagur87]

@JoostvDoorn - FYI since I know you created this resource.

[manicminer]

@tagur87 Thanks for reporting this. At first glance this looks like it might be an API bug and/or design fault. We'll wait to hear back on the linked upstream issue 🤞

[tagur87]

@tagur87 Thanks for reporting this. At first glance this looks like it might be an API bug and/or design fault. We'll wait to hear back on the linked upstream issue 🤞

Thanks @manicminer. Is this the best place to report it upstream? Is there another repo that's better?

[manicminer]

@tagur87 I would suggest pasting your report into a new issue on that repo

[lossanarch]

This is not an upstream issue (well, it's definitely a design issue, but ignoring that...) and is due to the way the provider runs the delete, which is to try to set all the keys in the credential payload to null with a PUT. From the provider logs in json mode (prettified and redacted):

============================ Begin AzureAD Request ============================
Request ID: REDACTED

PUT /beta/servicePrincipals/REDACTED/synchronization/secrets HTTP/1.1
Host: graph.microsoft.com
User-Agent: HashiCorp Terraform/1.5.0 (+https://www.terraform.io) Terraform Plugin SDK/2.10.1 terraform-provider-azuread/dev Hamilton (Go-http-client/1.1) pid-REDACTED
Content-Length: 152
Accept: application/json; charset=utf-8; IEEE754Compatible=false
Content-Type: application/json; charset=utf-8
Odata-Maxversion: 4.0
Odata-Version: 4.0
Accept-Encoding: gzip

{
  "value": [
    {
      "key": "BaseAddress",
      "value": ""
    },
    {
      "key": "SecretToken",
      "value": ""
    },
    {
      "key": "SyncNotificationSettings",
      "value": ""
    },
    {
      "key": "SyncAll",
      "value": ""
    }
  ]
}
============================= End AzureAD Request =============================

~~There is a super easy fix for this, just pass an empty array as the value of value:~~

{
  "value": []
}

This is accepted by the api and will allow the ~~delete.~~

EDIT: On closer inspection, I'm not sure it deletes anything. It's accepted by the api, but it appears more like a no-op, and the previously applied settings are still available. I still think essentially a no-op and successful destroy of this resource is better than failed destroy runs only fixable by manually removing the resource from state, maybe the docs would need an update to include a warning that the config can't be removed unless you delete the service principal as well.

You can test this with the az cli by running:

az rest --method PUT --uri "https://graph.microsoft.com/v1.0/servicePrincipals/{id}/synchronization/secrets" --body '{"value":[]}'

Yes, the upstream api does not allow a proper delete and that's a design flaw, but given that many providers have to work around significant api flaws and terraform has had an open issue for ignoring deletes of resources for 4 years and the fix is so easy here I think some effort should be made to ensure the inability to destroy this resource doesn't prevent destroy runs from succeeding. EDIT: The above isn't meant to sound hostile, many apologies if it does. Thanks for your great work manicminer.