containers / gvisor-tap-vsock

A new network stack based on gVisor
Apache License 2.0
265 stars 49 forks source link

Panics on systems with empty /etc/resolv.conf #417

Open siretart opened 1 week ago

siretart commented 1 week ago

While working on the Debian package of this project, I noticed a panic while running the testsuite of this package:

in Spec Setup (BeforeEach) [0.000 seconds]
dns add test
/<<PKGBUILDDIR>>/_build/src/github.com/containers/gvisor-tap-vsock/pkg/services/dns/dns_test.go:22
  should add dns zone with ip [BeforeEach]
  /<<PKGBUILDDIR>>/_build/src/github.com/containers/gvisor-tap-vsock/pkg/services/dns/dns_test.go:29

  Test Panicked
  runtime error: index out of range [0] with length 0
  /usr/lib/go-1.23/src/runtime/panic.go:115

  Full Stack Trace
  github.com/containers/gvisor-tap-vsock/pkg/services/dns.getDNSHostAndPort()
    /<<PKGBUILDDIR>>/_build/src/github.com/containers/gvisor-tap-vsock/pkg/services/dns/dns_config_unix.go:15 +0x6c
  github.com/containers/gvisor-tap-vsock/pkg/services/dns.newDNSHandler({0xbb0d80, 0x0, 0x0})
    /<<PKGBUILDDIR>>/_build/src/github.com/containers/gvisor-tap-vsock/pkg/services/dns/dns.go:29 +0x37
  github.com/containers/gvisor-tap-vsock/pkg/services/dns.New({0x0, 0x0}, {0x0, 0x0}, {0xbb0d80?, 0x10?, 0x1?})
    /<<PKGBUILDDIR>>/_build/src/github.com/containers/gvisor-tap-vsock/pkg/services/dns/dns.go:166 +0x39
  github.com/containers/gvisor-tap-vsock/pkg/services/dns.init.func1.1()
    /<<PKGBUILDDIR>>/_build/src/github.com/containers/gvisor-tap-vsock/pkg/services/dns/dns_test.go:26 +0x33
  github.com/containers/gvisor-tap-vsock/pkg/services/dns.TestSuite(0xc0000a8b60)
    /<<PKGBUILDDIR>>/_build/src/github.com/containers/gvisor-tap-vsock/pkg/services/dns/dns_test.go:19 +0x3c
  testing.tRunner(0xc0000a8b60, 0x8825b0)
    /usr/lib/go-1.23/src/testing/testing.go:1690 +0xf4
  created by testing.(*T).Run in goroutine 1
    /usr/lib/go-1.23/src/testing/testing.go:1743 +0x390
------------------------------
•! Panic in Spec Setup (BeforeEach) [0.000 seconds]

This panic starts happening when upgrading to version 0.8.0. It does not occur with version 0.7.3

Looking at the code, I believe this is caused by 58eb054369873855afdc329ae724db779800cf74, which introduces https://github.com/containers/gvisor-tap-vsock/blob/72b102d1d443d6d02e4c04f5c2af809e4c2e0910/pkg/services/dns/dns_config_unix.go#L9-L17

The panic occurs in line 15 when the test machine does have an empty /etc/resolv.conf. This makes sense for a build machine to attempt to disable connections to the internet.

Arguably, this can also happen outside of the test suite. Having the code raise an error seems preferable over a runtime error: index out of range [0] with length 0

gbraad commented 1 week ago

An empty resolv.conf file is possible, as in environments that only depend on IP addresses. It is unlikely for us to work as expected; though a warning is more appropriate than hard failure (crash) ;-)

siretart commented 1 week ago

For the Debian package, I've come up with this patch:

https://salsa.debian.org/go-team/packages/golang-github-containers-gvisor-tap-vsocks/-/blob/debian/sid/debian/patches/0002-Avoid-crash-with-empty-resolv.conf.patch?ref_type=heads

let me know if you prefer me to send this is a PR.

evidolob commented 1 week ago

@siretart It would be nice if you make a PR with your fix

siretart commented 1 week ago

@evidolob see #420