Fix/workaround for #7470 and #6041

[nitzanm]

#7470 happens because Traffic Manager Profile update is a PUT request. PUT, by definition, replaces the entire object. Since the list of Endpoints is part of the Profile object in Azure, it is cleared because the provider doesn't specify it in the PUT request.

Since our schema specifies Endpoints as separate objects than the Profile, we shouldn't be touching the endpoints when updating a Profile. There are two ways to do that:

1. Replace the PUT with a PATCH. I don't like this because that allows for config drift, especially if Traffic Manager Profiles have new attributes in the future that the provider doesn't know about yet.
2. Retrieve all endpoints and copy them into the PUT request so that they aren't affected. Is this beautiful? Absolutely not, but I don't see a better solution given the restrictions.

(fixes #6041)

[nitzanm]

@tombuildsstuff could you possibly re-trigger the build? Looks like it failed with an error unrelated to my changes (while installing tooling), but I don't have permission to re-run it.

[ArcturusZhang]

Hi @nitzanm thanks for your PR!

The root cause of issue #6041 should be we are using the PUT request for updating a resource, and as a separated virtual resource endpoint we will never specify those in the profile, therefore every time we apply, Azure will erase all the endpoints. Therefore, instead of making patches for the PUT request, it would be more elegant to update this resource using the PATCH request. And the profilesClient does have a function of Update which sends a PATCH request.

Could we please introduce a separated function for update and use the client.Update in it to fix this issue?
What do you think?

[IvanKirianov]

Hi guys,
When you are plan to merge this bugfix?
Thanks!

[nitzanm]

@ArcturusZhang I don't have a strong opinion on it either way; I'm OK with changing it to use PATCH instead. I'll try to get to it next week.

BTW: any idea why the CI build is failing? It looks to me like an environment error rather than something introduced by my code changes, but perhaps you have some insight?

[ArcturusZhang]

Hi @ArcturusZhang actually you can ignore that CI failure since it is caused by some random connection issues. When you make a new commit, the CI could automatically get triggered again.

[rohinz]

Hi @ArcturusZhang actually you can ignore that CI failure since it is caused by some random connection issues. When you make a new commit, the CI could automatically get triggered again.

@nitzanm Can you give it a try?

[katbyte]

Hi @nitzanm, thanks for this PR. As comments have been addressed by #7846 i'm going to close this in favour of it

[ghost]

This has been released in version 2.23.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.23.0"
}
# ... other configuration ...

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!