From f04dadd761db3306877a56e3872dc12f1bed8a1f Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Sun, 2 Oct 2022 02:18:05 +0200
Subject: [PATCH] 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 <github@gone.nl>
---
 executor/oci/resolvconf.go      | 38 +++++++++++++++------------------
 executor/oci/resolvconf_test.go | 13 ++++++-----
 2 files changed, 23 insertions(+), 28 deletions(-)

diff --git a/executor/oci/resolvconf.go b/executor/oci/resolvconf.go
index 3ac0feda7aea..aa4c27c50b23 100644
--- a/executor/oci/resolvconf.go
+++ b/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
 		}
diff --git a/executor/oci/resolvconf_test.go b/executor/oci/resolvconf_test.go
index ec073885eb34..d17e706d623f 100644
--- a/executor/oci/resolvconf_test.go
+++ b/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 := `