Make frontend backend_pool_settings computed

[VenelinMartinov]

Community Note

• Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
• Please do not leave "+1" or "me too" comments, they generate extra noise for PR followers and do not help prioritize for review

Description

The provider sets a value for this property so it should be computed, otherwise the resource always produces a diff.

PR Checklist

• I have followed the guidelines in our Contributing Documentation.
• I have checked to ensure there aren't other open Pull Requests for the same update/change.
• I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
• I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.
• I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
  For example: “resource_name_here - description of change e.g. adding property new_property_name_here”

Changes to existing Resource / Data Source

• I have added an explanation of what my changes do and why I'd like you to include them (This may be covered by linking to an issue above, but may benefit from additional explanation).
• I have written new tests for my resource or datasource changes & updated any relevent documentation.
• I have successfully run tests with my changes locally. If not, please provide details on testing challenges that prevented you running the tests.
• (For changes that include a state migration only). I have manually tested the migration path between relevant versions of the provider.

Testing

• My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

• azurerm_frontdoor - make backend_settings correctly computed

This is a (please select all that apply):

• Bug Fix
• New Feature (ie adding a service, resource, or data source)
• Enhancement
• Breaking Change

Related Issue(s)

Fixes #25911

Note

If this PR changes meaningfully during the course of review please update the title and description as required.

[stephybun]

Thanks for this PR @VenelinMartinov.

We're moving away from the pattern of setting Computed on Optional properties that are causing a diff because the API is returning some default values for them. This is explained in more detail over in our Contributor Docs.

I believe you should be able to add backend_pool_settings to ignore_changes in the resource's lifecycle which will suppress the diff. Could you give that a go?

[VenelinMartinov]

Thanks for your response @stephybun.

I imagine ignore_changes would fix the issue but that requires that every user add that to their program - I wanted to improve the default behaviour of the resource.

Can you help me understand why you'e made the change to move away from Computed for resources set by the API? Thank you for the link to the docs but it's not clear to me which use case is not possible if the property is marked Optional + Computed. Perhaps an example program would help?

[stephybun]

@VenelinMartinov sure I'll try and go into the specifics for this case.

This particular instance of adding Computed to backend_pool_settings is problematic since it is a List of properties (a block) that a user can and may want to remove/empty/reset after it has been set in the Terraform configuration.

The reason being is that Terraform is unable to tell when a Computed property is being deliberately removed from a user's configuration or when a user is trying to unset the values in the block e.g. by setting

backend_pool_settings {}

This is the "side-effect" that's mentioned in the linked documentation.

In other words Computed cannot be set in order to enable users to unset Optional blocks which is why the standard in the AzureRM Provider is to explicitly ignore these properties with ignore_changes. This is behaviour that is going to become more prevalent as time goes on and we continue to work through removing Computed on blocks like this.

I hope that clarifies things a bit better. Let me know if you still have questions.

[VenelinMartinov]

Thanks for the explanation @stephybun.

If I understand correctly the issue is that, if the property is Computed, the provider can't know if the property in the old state was set by the user or by the API?

If the property isn't Computed the provider always assumes the property is set by the user, even when it was actually set by the API.

Could this be solved by utilising provider private state? https://developer.hashicorp.com/terraform/plugin/framework/resources/private-state

Perhaps when a default value is applied, the provider can tag the field as being defaulted in the private state. Once that happens, on the next apply, if the user has removed the property in their input, the provider can look up if the value comes from a default in the private state, and if not, re-apply the default?

This would solve both issues without any need for users to utilise ignore_changes in their programs.

Have I understood the issue correctly and does this sound like something which might be feasible?

[stephybun]

@VenelinMartinov your understanding is correct.

Without having looked into provider private state in great detail, it sounds like it could be a promising way to improve behaviours like this.

Unfortunately private state is a framework feature and as mentioned in the docs that I linked, the provider currently exclusively uses hashicorp/terraform-plugin-sdk@v2 and there is no support for hashicorp/terraform-plugin-framework, so at the moment this suggestion isn't viable.

We are working on adding support for hashicorp/terraform-plugin-framework, which is tracked in this issue but development and migration to it is still in it's infancy.

For the moment the standard remains to add this property to ignore_changes but we will certainly be revisiting improvements that we can make to cases like this once we have access to framework features on the provider.

[stephybun]

Closing based on discussion above. When framework becomes available within the provider we can investigate alternative solutions.

[VenelinMartinov]

@stephybun many thanks for your explanations here 🙏