libnetwork: make resolvconf more self-contained#42755
Conversation
Includes the changes from moby/moby#42755 Signed-off-by: Sebastiaan van Stijn <[email protected]>
|
relates to moby/buildkit#2317 |
ddd02f3 to
ebc554b
Compare
Includes the changes from moby/moby#42755 Signed-off-by: Sebastiaan van Stijn <[email protected]>
USING https://github.com/thaJeztah/docker/tree/TEMP_CLEANED_DOCKER branch, which includes moby/moby#42755 and moby/moby#42757 Signed-off-by: Sebastiaan van Stijn <[email protected]>
|
Something broke, but I'm confused what. I suspect it's related to the pkg/ioutils.HashData() utility, but that was just moved 🤔 ; |
22d5787 to
5102ae7
Compare
|
Opened opencontainers/go-digest#64 to fix go-digest |
5102ae7 to
049c8ab
Compare
There was a problem hiding this comment.
Included a temporary fix here (until opencontainers/go-digest#64 is merged)
There was a problem hiding this comment.
Isn't this different since it could be an ipv6 loopback?
There was a problem hiding this comment.
Ah, yes. Brainfart; I was thinking this was to skip loopback IP-addresses (as they won't work as DNS for container), but this is the bit where we forward requests to them. Yup, that shouldn't use IPv6 likely. Or maybe it should, but we can do that separately.
This allows using the package without having to import the "types" package, and without having to consume github.com/ishidawataru/sctp. Signed-off-by: Sebastiaan van Stijn <[email protected]>
The IsLocalhost utility was not used, which only leaves the IsIPv4Localhost utility. Go's "net" package provides a `IsLoopBack()` check, but it checks for both IPv4 and IPv6 loopback interfaces. We likely should also do IPv6 here, but that's better left for a separate change, so instead, I replicated the IPv4 bits from Go's net.IP.IsLoopback(). Signed-off-by: Sebastiaan van Stijn <[email protected]>
It's the only location it's used, so we might as well move it there. Signed-off-by: Sebastiaan van Stijn <[email protected]>
Previously, ioutils imported the crypty/sha256 package, because it was
used by the HashData() utility. As a side-effect of that import, the
sha256 algorithm was registered through its `init()` function.
Now that the HashData() utility is removed, the import is no longer needed
in this package, but some parts of our code depended on the side-effect, and
without this, it fail to recognise the algorithms, unless something else
happens to import crypto/sha256 / crypto/sha512, which made our
tests fail:
```
=== Failed
=== FAIL: reference TestLoad (0.00s)
store_test.go:53: failed to parse reference: unsupported digest algorithm
=== FAIL: reference TestSave (0.00s)
store_test.go:82: failed to parse reference: unsupported digest algorithm
=== FAIL: reference TestAddDeleteGet (0.00s)
store_test.go:174: could not parse reference: unsupported digest algorithm
=== FAIL: reference TestInvalidTags (0.00s)
store_test.go:355: assertion failed: error is not nil: unsupported digest algorithm
```
While it would be better to do the import in the actual locations where it's
expected, there may be code-paths we overlook, so instead adding the import
here temporarily. Until the PR in go-digest has been merged and released.
Signed-off-by: Sebastiaan van Stijn <[email protected]>
This information was already documented in the package's GoDoc, so no need to repeat it. Signed-off-by: Sebastiaan van Stijn <[email protected]>
049c8ab to
d215d34
Compare
|
@cpuguy83 updated; PTAL |
| // isIPv4Loopback checks if the given IP address is an IPv4 loopback address. | ||
| // It's based on the logic in Go's net.IP.IsLoopback(), but only the IPv4 part: | ||
| // https://github.com/golang/go/blob/go1.16.6/src/net/ip.go#L120-L126 | ||
| func isIPv4Loopback(ipAddress string) bool { |
There was a problem hiding this comment.
This is a lot less eyebrow raising than the regular expressions above for parsing IP addresses in a language with a pretty decent standard library for parsing IP addresses. 😅 👍
There was a problem hiding this comment.
Ha! True. Yes, I didn't compare performance between this and the original approach, but I assume the stdlib code is usually optimised.
|
Failure is unrelated, and a known flaky test ; #36907 |
libnetwork: move resolvconf consts into the resolvconf package
This allows using the package without having to import the "types" package, and without having to consume github.com/ishidawataru/sctp.
libnetwork: remove resolvconf/dns package
The IsLocalhost utility was not used, and go's "net" package provides a
IsLoopBack()check, which can be used in stead ofIsIPv4Localhost.move pkg/ioutils.HashData() to libnetwork/resolvconf
It's the only location it's used, so we might as well move it there. This also removes the "crypto/sha256" and "encoding/hex" dependencies from pkg/ioutils..
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)