fix: Workaround Go Inet marshal bug #410
Conversation
yevgenypats
changed the title
yevgenypats
force-pushed
the
fix/inet_marshal
branch
from
6e5f973
to
1dba1a7
Compare
schema/inet.go
Outdated
| @@ -146,6 +153,22 @@ func maybeGetIPv4(input string, ip net.IP) net.IP { | |||
| return ip.To4() | |||
| } | |||
|
|
|||
| // workaround this Golang bug: https://github.com/golang/go/issues/35727 | |||
| func (dst *Inet) UnmarshalJSON(b []byte) error { | |||
| tmp := inetIntermidate{} | |||
There was a problem hiding this comment.
| tmp := inetIntermidate{} | |
| tmp := inetIntermediate{} |
yevgenypats
requested review from
disq,
shimonp21,
hermanschaaf and
erezrokah
and removed request for
erezrokah
|
@shimonp21 this now works. @hermanschaaf added tests. |
schema/inet_test.go
Outdated
| } | ||
|
|
||
| // workaround this Golang bug: https://github.com/golang/go/issues/35727 | ||
| if len(r.IPNet.Mask) != len(r2.IPNet.Mask) { |
There was a problem hiding this comment.
Why compare lengths here, and not the values?
There was a problem hiding this comment.
because we compare the values in the line before that. the bug was that the bytes are in different length. I can also add a byte comparision.
There was a problem hiding this comment.
Ah I see. No all good, just curious
There was a problem hiding this comment.
but good call. Added more thorugh tests and also checked that it failed before the fix and after
schema/inet.go
Outdated
| return err | ||
| } | ||
| *dst = Inet{Status: tmp.Status} | ||
| dst.Status = tmp.Status |
| @@ -12,6 +13,12 @@ type InetTransformer interface { | |||
| TransformInet(*Inet) interface{} | |||
| } | |||
|
|
|||
| // workaround this Golang bug: https://github.com/golang/go/issues/35727 | |||
| type inetWrapper struct { | |||
There was a problem hiding this comment.
Maybe add a comment that this is only used in Unmarshal.
Maybe order structs so that exported struct Inet is at the top.
yevgenypats
force-pushed
the
fix/inet_marshal
branch
from
6192d8d
to
552ede9
Compare
There was a problem hiding this comment.
Approving at @yevgenypats request.
We may still have a bug in handling ipv6 though - testing that right now
🤖 I have created a release *beep* *boop* --- ## [1.5.3](v1.5.2...v1.5.3) (2022-11-15) ### Bug Fixes * Workaround Go Inet marshal bug ([#410](#410)) ([bd7718c](bd7718c)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Fix nasty bug triggered by Go bug
golang/go#35727
@shimonp21 can you please confirm this solves the bug triggered by the k8s cidr plugin.