Merge pull request #13191 from mheon/resolvconf_fixes

Modify /etc/resolv.conf when connecting/disconnecting

3 files changed, 190 insertions, 23 deletions

diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index 95f1634a8..afa351c17 100644
--- a/libpod/container_internal_linux.go
+++ b/libpod/container_internal_linux.go
@@ -2045,19 +2045,8 @@ func (c *Container) generateResolvConf() (string, error) {
 		}
 	}
 
-	ipv6 := false
-	// If network status is set check for ipv6 and dns namesevers
 	netStatus := c.getNetworkStatus()
 	for _, status := range netStatus {
-		for _, netInt := range status.Interfaces {
-			for _, netAddress := range netInt.Subnets {
-				// Note: only using To16() does not work since it also returns a valid ip for ipv4
-				if netAddress.IPNet.IP.To4() == nil && netAddress.IPNet.IP.To16() != nil {
-					ipv6 = true
-				}
-			}
-		}
-
 		if status.DNSServerIPs != nil {
 			for _, nsIP := range status.DNSServerIPs {
 				networkNameServers = append(networkNameServers, nsIP.String())
@@ -2070,16 +2059,9 @@ func (c *Container) generateResolvConf() (string, error) {
 		}
 	}
 
-	if c.config.NetMode.IsSlirp4netns() {
-		ctrNetworkSlipOpts := []string{}
-		if c.config.NetworkOptions != nil {
-			ctrNetworkSlipOpts = append(ctrNetworkSlipOpts, c.config.NetworkOptions["slirp4netns"]...)
-		}
-		slirpOpts, err := parseSlirp4netnsNetworkOptions(c.runtime, ctrNetworkSlipOpts)
-		if err != nil {
-			return "", err
-		}
-		ipv6 = slirpOpts.enableIPv6
+	ipv6, err := c.checkForIPv6(netStatus)
+	if err != nil {
+		return "", err
 	}
 
 	// Ensure that the container's /etc/resolv.conf is compatible with its
@@ -2160,6 +2142,116 @@ func (c *Container) generateResolvConf() (string, error) {
 	return destPath, nil
 }
 
+// Check if a container uses IPv6.
+func (c *Container) checkForIPv6(netStatus map[string]types.StatusBlock) (bool, error) {
+	for _, status := range netStatus {
+		for _, netInt := range status.Interfaces {
+			for _, netAddress := range netInt.Subnets {
+				// Note: only using To16() does not work since it also returns a valid ip for ipv4
+				if netAddress.IPNet.IP.To4() == nil && netAddress.IPNet.IP.To16() != nil {
+					return true, nil
+				}
+			}
+		}
+	}
+
+	if c.config.NetMode.IsSlirp4netns() {
+		ctrNetworkSlipOpts := []string{}
+		if c.config.NetworkOptions != nil {
+			ctrNetworkSlipOpts = append(ctrNetworkSlipOpts, c.config.NetworkOptions["slirp4netns"]...)
+		}
+		slirpOpts, err := parseSlirp4netnsNetworkOptions(c.runtime, ctrNetworkSlipOpts)
+		if err != nil {
+			return false, err
+		}
+		return slirpOpts.enableIPv6, nil
+	}
+
+	return false, nil
+}
+
+// Add a new nameserver to the container's resolv.conf, ensuring that it is the
+// first nameserver present.
+// Usable only with running containers.
+func (c *Container) addNameserver(ips []string) error {
+	// Take no action if container is not running.
+	if !c.ensureState(define.ContainerStateRunning, define.ContainerStateCreated) {
+		return nil
+	}
+
+	// Do we have a resolv.conf at all?
+	path, ok := c.state.BindMounts["/etc/resolv.conf"]
+	if !ok {
+		return nil
+	}
+
+	// Read in full contents, parse out existing nameservers
+	contents, err := ioutil.ReadFile(path)
+	if err != nil {
+		return err
+	}
+	ns := resolvconf.GetNameservers(contents)
+	options := resolvconf.GetOptions(contents)
+	search := resolvconf.GetSearchDomains(contents)
+
+	// We could verify that it doesn't already exist
+	// but extra nameservers shouldn't harm anything.
+	// Ensure we are the first entry in resolv.conf though, otherwise we
+	// might be after user-added servers.
+	ns = append(ips, ns...)
+
+	// We're rewriting the container's resolv.conf as part of this, but we
+	// hold the container lock, so there should be no risk of parallel
+	// modification.
+	if _, err := resolvconf.Build(path, ns, search, options); err != nil {
+		return errors.Wrapf(err, "error adding new nameserver to container %s resolv.conf", c.ID())
+	}
+
+	return nil
+}
+
+// Remove an entry from the existing resolv.conf of the container.
+// Usable only with running containers.
+func (c *Container) removeNameserver(ips []string) error {
+	// Take no action if container is not running.
+	if !c.ensureState(define.ContainerStateRunning, define.ContainerStateCreated) {
+		return nil
+	}
+
+	// Do we have a resolv.conf at all?
+	path, ok := c.state.BindMounts["/etc/resolv.conf"]
+	if !ok {
+		return nil
+	}
+
+	// Read in full contents, parse out existing nameservers
+	contents, err := ioutil.ReadFile(path)
+	if err != nil {
+		return err
+	}
+	ns := resolvconf.GetNameservers(contents)
+	options := resolvconf.GetOptions(contents)
+	search := resolvconf.GetSearchDomains(contents)
+
+	toRemove := make(map[string]bool)
+	for _, ip := range ips {
+		toRemove[ip] = true
+	}
+
+	newNS := make([]string, 0, len(ns))
+	for _, server := range ns {
+		if !toRemove[server] {
+			newNS = append(newNS, server)
+		}
+	}
+
+	if _, err := resolvconf.Build(path, newNS, search, options); err != nil {
+		return errors.Wrapf(err, "error removing nameservers from container %s resolv.conf", c.ID())
+	}
+
+	return nil
+}
+
 // updateHosts updates the container's hosts file
 func (c *Container) updateHosts(path string) error {
 	var hosts string
diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go
index e55e9d114..19d5c7f76 100644
--- a/libpod/networking_linux.go
+++ b/libpod/networking_linux.go
@@ -1170,6 +1170,7 @@ func (c *Container) NetworkDisconnect(nameOrID, netName string, force bool) erro
 	}
 
 	// update network status if container is running
+	oldStatus, statusExist := networkStatus[netName]
 	delete(networkStatus, netName)
 	c.state.NetworkStatus = networkStatus
 	err = c.save()
@@ -1180,8 +1181,26 @@ func (c *Container) NetworkDisconnect(nameOrID, netName string, force bool) erro
 	// Reload ports when there are still connected networks, maybe we removed the network interface with the child ip.
 	// Reloading without connected networks does not make sense, so we can skip this step.
 	if rootless.IsRootless() && len(networkStatus) > 0 {
-		return c.reloadRootlessRLKPortMapping()
+		if err := c.reloadRootlessRLKPortMapping(); err != nil {
+			return err
+		}
+	}
+
+	// Update resolv.conf if required
+	if statusExist {
+		stringIPs := make([]string, 0, len(oldStatus.DNSServerIPs))
+		for _, ip := range oldStatus.DNSServerIPs {
+			stringIPs = append(stringIPs, ip.String())
+		}
+		if len(stringIPs) == 0 {
+			return nil
+		}
+		logrus.Debugf("Removing DNS Servers %v from resolv.conf", stringIPs)
+		if err := c.removeNameserver(stringIPs); err != nil {
+			return err
+		}
 	}
+
 	return nil
 }
 
@@ -1263,11 +1282,36 @@ func (c *Container) NetworkConnect(nameOrID, netName string, netOpts types.PerNe
 	if err != nil {
 		return err
 	}
+
 	// The first network needs a port reload to set the correct child ip for the rootlessport process.
 	// Adding a second network does not require a port reload because the child ip is still valid.
 	if rootless.IsRootless() && len(networks) == 0 {
-		return c.reloadRootlessRLKPortMapping()
+		if err := c.reloadRootlessRLKPortMapping(); err != nil {
+			return err
+		}
 	}
+
+	ipv6, err := c.checkForIPv6(networkStatus)
+	if err != nil {
+		return err
+	}
+
+	// Update resolv.conf if required
+	stringIPs := make([]string, 0, len(results[netName].DNSServerIPs))
+	for _, ip := range results[netName].DNSServerIPs {
+		if (ip.To4() == nil) && !ipv6 {
+			continue
+		}
+		stringIPs = append(stringIPs, ip.String())
+	}
+	if len(stringIPs) == 0 {
+		return nil
+	}
+	logrus.Debugf("Adding DNS Servers %v to resolv.conf", stringIPs)
+	if err := c.addNameserver(stringIPs); err != nil {
+		return err
+	}
+
 	return nil
 }
 
diff --git a/test/e2e/network_connect_disconnect_test.go b/test/e2e/network_connect_disconnect_test.go
index baef142ba..a0716c84d 100644
--- a/test/e2e/network_connect_disconnect_test.go
+++ b/test/e2e/network_connect_disconnect_test.go
@@ -2,6 +2,7 @@ package integration
 
 import (
 	"os"
+	"strings"
 
 	. "github.com/containers/podman/v4/test/utils"
 	"github.com/containers/storage/pkg/stringid"
@@ -77,6 +78,11 @@ var _ = Describe("Podman network connect and disconnect", func() {
 		Expect(session).Should(Exit(0))
 		defer podmanTest.removeNetwork(netName)
 
+		gw := podmanTest.Podman([]string{"network", "inspect", netName, "--format", "{{(index .Subnets 0).Gateway}}"})
+		gw.WaitWithDefaultTimeout()
+		Expect(gw).Should(Exit(0))
+		ns := gw.OutputToString()
+
 		ctr := podmanTest.Podman([]string{"run", "-dt", "--name", "test", "--network", netName, ALPINE, "top"})
 		ctr.WaitWithDefaultTimeout()
 		Expect(ctr).Should(Exit(0))
@@ -85,6 +91,11 @@ var _ = Describe("Podman network connect and disconnect", func() {
 		exec.WaitWithDefaultTimeout()
 		Expect(exec).Should(Exit(0))
 
+		exec2 := podmanTest.Podman([]string{"exec", "-it", "test", "cat", "/etc/resolv.conf"})
+		exec2.WaitWithDefaultTimeout()
+		Expect(exec2).Should(Exit(0))
+		Expect(strings.Contains(exec2.OutputToString(), ns)).To(BeTrue())
+
 		dis := podmanTest.Podman([]string{"network", "disconnect", netName, "test"})
 		dis.WaitWithDefaultTimeout()
 		Expect(dis).Should(Exit(0))
@@ -98,6 +109,11 @@ var _ = Describe("Podman network connect and disconnect", func() {
 		exec = podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth0"})
 		exec.WaitWithDefaultTimeout()
 		Expect(exec).Should(ExitWithError())
+
+		exec3 := podmanTest.Podman([]string{"exec", "-it", "test", "cat", "/etc/resolv.conf"})
+		exec3.WaitWithDefaultTimeout()
+		Expect(exec3).Should(Exit(0))
+		Expect(strings.Contains(exec3.OutputToString(), ns)).To(BeFalse())
 	})
 
 	It("bad network name in connect should result in error", func() {
@@ -182,6 +198,16 @@ var _ = Describe("Podman network connect and disconnect", func() {
 		Expect(session).Should(Exit(0))
 		defer podmanTest.removeNetwork(newNetName)
 
+		gw := podmanTest.Podman([]string{"network", "inspect", newNetName, "--format", "{{(index .Subnets 0).Gateway}}"})
+		gw.WaitWithDefaultTimeout()
+		Expect(gw).Should(Exit(0))
+		ns := gw.OutputToString()
+
+		exec2 := podmanTest.Podman([]string{"exec", "-it", "test", "cat", "/etc/resolv.conf"})
+		exec2.WaitWithDefaultTimeout()
+		Expect(exec2).Should(Exit(0))
+		Expect(strings.Contains(exec2.OutputToString(), ns)).To(BeFalse())
+
 		ip := "10.11.100.99"
 		mac := "44:11:44:11:44:11"
 		connect := podmanTest.Podman([]string{"network", "connect", "--ip", ip, "--mac-address", mac, newNetName, "test"})
@@ -206,6 +232,11 @@ var _ = Describe("Podman network connect and disconnect", func() {
 		Expect(exec.OutputToString()).Should(ContainSubstring(ip))
 		Expect(exec.OutputToString()).Should(ContainSubstring(mac))
 
+		exec3 := podmanTest.Podman([]string{"exec", "-it", "test", "cat", "/etc/resolv.conf"})
+		exec3.WaitWithDefaultTimeout()
+		Expect(exec3).Should(Exit(0))
+		Expect(strings.Contains(exec3.OutputToString(), ns)).To(BeTrue())
+
 		// make sure no logrus errors are shown https://github.com/containers/podman/issues/9602
 		rm := podmanTest.Podman([]string{"rm", "--time=0", "-f", "test"})
 		rm.WaitWithDefaultTimeout()