fix: correctly compare compressed/expanded ipv6 in targets

[GGabriele]

When syncing a target with a compressed IPv6 (for example [2001:db8:fd73::e]:1326) decK can successfully create it but it fails to detect its existence when pulling the config from the API because the CP expands the address (for example to [2001:0db8:fd73:0000:0000:0000:0000:000e]:1326) causing decK to attempt to recreate the target and failing because the target already exists.

This commit fixes this by using some helpers to expand the ipv6 before trying to sync/dump.

Summary

SUMMARY_GOES_HERE

Full changelog

• [Implement ...]
• [Fix ...]

Issues resolved

Fix #XXX

Documentation

• Link to the website documentation PR

Testing

• Unit tests
• E2E tests
• Manual testing on Universal
• Manual testing on Kubernetes

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 87.23404% with 6 lines in your changes missing coverage. Please review.

Project coverage is 41.42%. Comparing base (c70e4ec) to head (6764fd2).

Files	Patch %	Lines
pkg/file/builder.go	87.23%	4 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #109      +/-   ##
==========================================
+ Coverage   41.23%   41.42%   +0.19%     
==========================================
  Files          75       75              
  Lines       10793    10840      +47     
==========================================
+ Hits         4450     4491      +41     
- Misses       5881     5885       +4     
- Partials      462      464       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pmalek]

This can be prone to errors or to be misused. Why not use https://pkg.go.dev/net/netip#ParseAddr and https://pkg.go.dev/net/netip#Addr.Is6 ?

[GGabriele]

As silly as it may look, this is enough to catch all combinations of targets in IPv6 format, including the one with port and brackets like [2600:1f16:1784:fa03:fd73::e]:1326, which the library wouldn't consider as v6

[pmalek]

Wouldn't https://pkg.go.dev/net/netip#ParseAddrPort work for this then?

I'm just worried that this func is very easy to abuse.

isIPv6("this:is:nuts:!") will also return true. It will also return true for 1:2:3:4 which http://sqa.fyicenter.com/1000334_IPv6_Address_Validator.html#Result reports as invalid.

[GGabriele]

ParseAddrPort works if the port is always there, but it would fail to parse things like ::1 (would work with [::1]:8000 though).

If you really feel strong about this we can improve this validation, but generally decK (or this library) doesn't do much input validation other than what's part of the JSON schemas from go-kong.

For the records though, those bogus entries would still fail validation at a later stage:

$ ./deck gateway sync kong.yaml
Error: building state: invalid ipv6 address 1:2:3:4

$ ./deck gateway sync kong.yaml
Error: building state: invalid ipv6 address this:is:nuts:!

[pmalek]

Understood that this is validated on other levels.

I'm just suggesting to consider improving the reliability of this while we're at it

For instance something like this should work just fine and will work for addresses with and without ports

func IsIPv6(str string) bool {
	addr, err := netip.ParseAddrPort(str)
	if err != nil {
		addr, err := netip.ParseAddr(str)
		fmt.Printf("error parsing address: %v\n", err)
		if err != nil {
			fmt.Printf("error parsing address port: %v\n", err)
			return false
		}
		return addr.Is6()
	}

	return addr.Addr().Is6()
}

[GGabriele]

This still would fail to parse something like [::1], as well as detecting bogus patterns like [invalid:ipv6::address]:1326: the function would rightly return false, but this is not just false, it's invalid. At this point we would need add extra handling and validation for these cases too. I think this is going to become more complex and fragile overall.

I'm renaming this function to hasIPv6Format which better represent its purpose: 6764fd2

[pmalek]

Couldn't we also use StringExpanded() here? Possibly with a port?

[pmalek]

Wouldn't https://pkg.go.dev/net/netip#ParseAddrPort work for this then?

I'm just worried that this func is very easy to abuse.

isIPv6("this:is:nuts:!") will also return true. It will also return true for 1:2:3:4 which http://sqa.fyicenter.com/1000334_IPv6_Address_Validator.html#Result reports as invalid.

[pmalek]

Code looks good and serves it's purpose 👍

Feel free to address the comments or merge as is if time is of the essence.