Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
siretart opened this issue Nov 6, 2024 · 4 comments · Fixed by #420
Closed

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

siretart opened this issue Nov 6, 2024 · 4 comments · Fixed by #420
Assignees
Labels
bug Something isn't working

Comments

@siretart
Copy link

siretart commented Nov 6, 2024

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]�[0m
dns add test
�[90m/<<PKGBUILDDIR>>/_build/src/github.com/containers/gvisor-tap-vsock/pkg/services/dns/dns_test.go:22�[0m
  �[91m�[1mshould add dns zone with ip [BeforeEach]�[0m
  �[90m/<<PKGBUILDDIR>>/_build/src/github.com/containers/gvisor-tap-vsock/pkg/services/dns/dns_test.go:29�[0m

  �[91m�[1mTest Panicked�[0m
  �[91mruntime error: index out of range [0] with length 0�[0m
  /usr/lib/go-1.23/src/runtime/panic.go:115

  �[91mFull Stack Trace�[0m
  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
�[90m------------------------------�[0m
�[91m�[1m•! Panic in Spec Setup (BeforeEach) [0.000 seconds]�[0m

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 58eb054, which introduces

func getDNSHostAndPort() (string, string, error) {
conf, err := dns.ClientConfigFromFile("/etc/resolv.conf")
if err != nil {
return "", "", err
}
// TODO: use all configured nameservers, instead just first one
nameserver := conf.Servers[0]
return nameserver, conf.Port, nil

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
Copy link
Member

gbraad commented Nov 6, 2024

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) ;-)

@gbraad gbraad added the bug Something isn't working label Nov 6, 2024
@siretart
Copy link
Author

siretart commented Nov 6, 2024

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
Copy link
Contributor

evidolob commented Nov 6, 2024

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

siretart pushed a commit to siretart/gvisor-tap-vsock that referenced this issue Nov 8, 2024
@siretart
Copy link
Author

siretart commented Nov 8, 2024

@evidolob see #420

siretart pushed a commit to siretart/gvisor-tap-vsock that referenced this issue Nov 18, 2024
siretart pushed a commit to siretart/gvisor-tap-vsock that referenced this issue Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants