Fix 8768 corrupt ip_restrictions in App Service and add tests

[sdebruyn]

• Added acceptance tests to cover the error from azurerm_app_service ip_restriction doesn't remove subnet_id when unset #8768 (the second one didn't succeed at first)
• Fixed an if condition which should have alerted about the error
• Fixed the issue itself by setting the other two of subnet/IP/tag to empty when a restriction is being reused for a new subnet/IP/tag is used

The root cause was that IP restriction lines are being reused. When a reused one had a subnet and it would be changed to an IP, then the subnet was not removed. Thus, to know what has changed, I had to pass ResourceData down the tree.

Any kind of feedback is welcome, I don't regularly use Go.

[jackofallops]

Hi @sdebruyn - I've taken a quick look over this and I think we may be addressing the wrong problem here? It appears that when the subnet_id property was deprecated, it was done so with a bug in there that forced users to also specify virtual_network_subnet_id to use it, which was not the intent as this property was meant to replace it. (this can be seen in the example config in the linked issue). I'm going to do a little more investigation, but I think it can be resolved relatively simply. If that's the case, I will open a new PR to supersede this one (with some more complex test cases), if that's OK with you?

[sdebruyn]

Hi @jackofallops, certainly! Whatever gets the issue fixed! It's a real dealbreaker for us.

In my findings the extra subnet property was not the cause of this issue (although it was my first thought too).

I would merge the tests anyway so that this issue can be tracked.

[jackofallops]

Thanks @sdebruyn - You are correct, there's an issue with the way the flatten and expand functions are working there too... I've got a branch running through the full regression suite at the moment that appears to resolve the initial issue, I'll add some more complex scenarios too for extra confidence. Typically we phase out deprecated properties from tests, but I think in this case it'll be worth having some coverage there. As soon as I'm confident I'll open the PR and close here with references.

[katbyte]

I'm going to close this in favour of #11170 - thanks for starting this @sdebruyn

[ghost]

This has been released in version 2.56.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.56.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 👉 [email protected]. Thanks!