executor/oci: GetResolvConf(): simplify handling of resolv.conf

We only need the content here, not the checksum, so simplifying the code by
just using os.ReadFile(), using resolvconf.Path() for the location.

Also reversing the logic for custom options; The existing code was always
parsing the host's resolv.conf to read the nameservers, searchdomain and
options, but those options were only needed if they were not configured in
DNSConfig. This patch reverses the logic to only parse the resolv.conf if
no options are present in the passed DNSConfig.

There's some more optimisations to make (but changes in libnetwork are needed
for that); resolvconf.FilterResolvDNS() still parse the resolv.conf file, even
if we just generated it (in which case we're generating a resolv.conf, after
which we're parsing the generated file again to update it, which is not ideal).

Signed-off-by: Sebastiaan van Stijn <[email protected]>

Author: thaJeztah

executor/oci/resolvconf.go

@@ -16,7 +16,7 @@ var notFirstRun bool
 var lastNotEmpty bool
 
 // overridden by tests
-var resolvconfGet = resolvconf.Get
+var resolvconfPath = resolvconf.Path
 
 type DNSConfig struct {
 	Nameservers   []string
@@ -39,7 +39,7 @@ func GetResolvConf(ctx context.Context, stateDir string, idmap *idtools.Identity
 				generate = true
 			}
 			if !generate {
-				fiMain, err := os.Stat(resolvconf.Path())
+				fiMain, err := os.Stat(resolvconfPath())
 				if err != nil {
 					if !errors.Is(err, os.ErrNotExist) {
 						return nil, err
@@ -60,33 +60,30 @@ func GetResolvConf(ctx context.Context, stateDir string, idmap *idtools.Identity
 			return "", nil
 		}
 
-		var dt []byte
-		f, err := resolvconfGet()
-		if err != nil {
-			if !errors.Is(err, os.ErrNotExist) {
-				return "", err
-			}
-		} else {
-			dt = f.Content
+		dt, err := os.ReadFile(resolvconfPath())
+		if err != nil && !errors.Is(err, os.ErrNotExist) {
+			return "", err
 		}
 
+		var f *resolvconf.File
+		tmpPath := p + ".tmp"
 		if dns != nil {
 			var (
-				dnsNameservers   = resolvconf.GetNameservers(dt, resolvconf.IP)
-				dnsSearchDomains = resolvconf.GetSearchDomains(dt)
-				dnsOptions       = resolvconf.GetOptions(dt)
+				dnsNameservers   = dns.Nameservers
+				dnsSearchDomains = dns.SearchDomains
+				dnsOptions       = dns.Options
 			)
-			if len(dns.Nameservers) > 0 {
-				dnsNameservers = dns.Nameservers
+			if len(dns.Nameservers) == 0 {
+				dnsNameservers = resolvconf.GetNameservers(dt, resolvconf.IP)
 			}
-			if len(dns.SearchDomains) > 0 {
-				dnsSearchDomains = dns.SearchDomains
+			if len(dns.SearchDomains) == 0 {
+				dnsSearchDomains = resolvconf.GetSearchDomains(dt)
 			}
-			if len(dns.Options) > 0 {
-				dnsOptions = dns.Options
+			if len(dns.Options) == 0 {
+				dnsOptions = resolvconf.GetOptions(dt)
 			}
 
-			f, err = resolvconf.Build(p+".tmp", dnsNameservers, dnsSearchDomains, dnsOptions)
+			f, err = resolvconf.Build(tmpPath, dnsNameservers, dnsSearchDomains, dnsOptions)
 			if err != nil {
 				return "", err
 			}
@@ -98,7 +95,6 @@ func GetResolvConf(ctx context.Context, stateDir string, idmap *idtools.Identity
 			return "", err
 		}
 
-		tmpPath := p + ".tmp"
 		if err := os.WriteFile(tmpPath, f.Content, 0644); err != nil {
 			return "", err
 		}

executor/oci/resolvconf_test.go

@@ -5,19 +5,18 @@ import (
 	"os"
 	"testing"
 
-	"github.com/docker/docker/libnetwork/resolvconf"
 	"github.com/stretchr/testify/require"
 )
 
 // TestResolvConfNotExist modifies a global variable
 // It must not run in parallel.
 func TestResolvConfNotExist(t *testing.T) {
-	oldResolvconfGet := resolvconfGet
-	defer func() {
-		resolvconfGet = oldResolvconfGet
-	}()
-	resolvconfGet = func() (*resolvconf.File, error) {
-		return nil, os.ErrNotExist
+	oldResolvconfPath := resolvconfPath
+	t.Cleanup(func() {
+		resolvconfPath = oldResolvconfPath
+	})
+	resolvconfPath = func() string {
+		return "no-such-file"
 	}
 
 	defaultResolvConf := `