Bug: azurerm_dns_aaaa_record - Normalize IPv6 addresses

[phires]

This is a work in progress try to fix issue #5321 .
Sadly somehow the DiffSuppressFunc does not compare the correct set values with each other - and I have no idea why.
If adding a logline to the func I get something like this:

old 2607:f8b0:4005:800::1003, new 
old 2201:1234:1234::1, new 2201:1234:1234::1
old , new 2607:f8b0:4005:0800:0000:0000:0000:1003

Anyone any clues?

[katbyte]

Hi @phires,

Thanks for giving this a shot. I'm not sure we need the CompressIPv6Address function, a suppress should do the trick. Could we add an acctest with an IP address exhibing the error? once thats done we can dig into this further. thanks!

[katbyte]

And we should add a validate.IPv6Address

[katbyte]

👋 @phires, Is this reaady to be looked at outside the failing travis test?

[katbyte]

Pretty sure the validation function will allow the compressed form in, so it won't really help

[phires]

wave @phires, Is this reaady to be looked at outside the failing travis test?

I've removed the unused import (shame on me), would be great to have a look onto this, why the DiffSuppressFunc is not working correctly.

[katbyte]

Hey @phires, i believe this is because we are trying to diff suppress on a set and they behave differently then a list. I think what we'll need to do is setup a custom hash function an a state migration to switch to the new hashs.

[WodansSon]

@phires, I believe @katbyte is totally correct in her assessment of what is happening here. I was playing around with this too and added some debug spew to your code just to see what the values were when entering and exiting your function and what I saw is consistent with what @katbyte is saying:

*** = Enter function
--- = Exit function

*** Str Old: 2607:f8b0:4005:800::1003 :: Str New:  ***
--- Old: 2607:f8b0:4005:800::1003 :: New: <nil> :: Equal: false ---

*** Str Old:  :: Str New: 2607:f8b0:4005:0800:0000:0000:0000:1003 ***
--- Old: <nil> :: New: 2607:f8b0:4005:800::1003 :: Equal: false ---

[phires]

@phires, I believe @katbyte is totally correct in her assessment of what is happening here. I was playing around with this too and added some debug spew to your code just to see what the values were when entering and exiting your function and what I saw is consistent with what @katbyte is saying:

*** = Enter function
--- = Exit function

*** Str Old: 2607:f8b0:4005:800::1003 :: Str New:  ***
--- Old: 2607:f8b0:4005:800::1003 :: New: <nil> :: Equal: false ---

*** Str Old:  :: Str New: 2607:f8b0:4005:0800:0000:0000:0000:1003 ***
--- Old: <nil> :: New: 2607:f8b0:4005:800::1003 :: Equal: false ---

Yes, see my original PR text, I had added a similar func already ;)

So, a custom hash function it is. Any pointers where to start?

[WodansSon]

@phires sure, I played around with this a bit and here is what I did...

In the ./azurerm/helpers/azure directory I created a file called ipv6.go. I then wrote the below code for that file:

package azure

import (
	"net"

	"github.com/hashicorp/terraform-plugin-sdk/helper/hashcode"
)

func NormalizeIPv6Address(ipv6 interface{}) string {
	return net.ParseIP(ipv6.(string)).String()
}

func HashIPv6Address(ipv6 interface{}) int {
	return hashcode.String(NormalizeIPv6Address(ipv6))
}

Then in the resource I updated the "records" schema Set function to be:

Set:           azure.HashIPv6Address,

I also removed the DiffSuppressFunc and removed the github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress package import declaration.

Finally, I modified the expand function:

Orig code:
ipv6 := v.(string)

New code:
ipv6 := azure.NormalizeIPv6Address(v)

and the flatten function:

Orig code:
results = append(results, *record.Ipv6Address)

New code:
results = append(results, azure.NormalizeIPv6Address(*record.Ipv6Address))

That's it... I think that should work, now all we need to do is to move the test cases and update them to use the new normalize functions and I think we are gold. 😃

[WodansSon]

@phires doing it this way I don't think we will need the state migration either, but play with it and see how it goes.

[WodansSon]

👋 @phires just curious, did you want to make the changes to your code or are you ok with me pushing to your branch? Thanks again for the PR!

[phires]

Hi, I've implemented the changes and also build some tests for the new funcs.

[WodansSon]

@phires thanks much for pushing those changes and adding the test cases. LGTM! 🚀

[WodansSon]

[katbyte]

Thanks for this @phires, LGTM 👍

[ghost]

This has been released in version 1.43.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 = "~> 1.43.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!