From 9f87bcfb3b67cbad9ee967ac574484938447f448 Mon Sep 17 00:00:00 2001
From: Alexander Scheel <alex.scheel@hashicorp.com>
Date: Wed, 7 Jun 2023 08:10:16 -0400
Subject: [PATCH] Fix race in DNS test server configs (#21024)

When writing DNS configs, make sure to push the zone file prior to
writing the reference to the zone in the named.conf.options file.
Otherwise, when adding the initial domain (or any subsequent domains,
which isn't really used by these tests), a race occurs between Docker,
writing the updated config files, and the bind daemon, detecting that
mtime has changed on the named.conf.options file and reloading it
and any referenced zone files.

This should fix the error seen in some tests:

> /etc/bind/named.conf:9: parsing failed: file not found

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
---
 builtin/logical/pki/dnstest/server.go | 29 ++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/builtin/logical/pki/dnstest/server.go b/builtin/logical/pki/dnstest/server.go
index 8e2450b640..87091ed5eb 100644
--- a/builtin/logical/pki/dnstest/server.go
+++ b/builtin/logical/pki/dnstest/server.go
@@ -171,10 +171,7 @@ func (ts *TestServer) buildZoneFile(target string) string {
 	return zone
 }
 
-func (ts *TestServer) PushConfig() {
-	ts.lock.Lock()
-	defer ts.lock.Unlock()
-
+func (ts *TestServer) pushNamedConf() {
 	contents := docker.NewBuildContext()
 	cfgPath := "/etc/bind/named.conf.options"
 	namedCfg := ts.buildNamedConf()
@@ -183,6 +180,13 @@ func (ts *TestServer) PushConfig() {
 
 	ts.t.Logf("Generated bind9 config (%s):\n%v\n", cfgPath, namedCfg)
 
+	err := ts.runner.CopyTo(ts.startup.Container.ID, "/", contents)
+	require.NoError(ts.t, err, "failed pushing updated named.conf.options to container")
+}
+
+func (ts *TestServer) pushZoneFiles() {
+	contents := docker.NewBuildContext()
+
 	for _, domain := range ts.domains {
 		path := "/var/cache/bind/" + domain + ".zone"
 		zoneFile := ts.buildZoneFile(domain)
@@ -193,7 +197,22 @@ func (ts *TestServer) PushConfig() {
 	}
 
 	err := ts.runner.CopyTo(ts.startup.Container.ID, "/", contents)
-	require.NoError(ts.t, err, "failed pushing updated configuration to container")
+	require.NoError(ts.t, err, "failed pushing updated named.conf.options to container")
+}
+
+func (ts *TestServer) PushConfig() {
+	ts.lock.Lock()
+	defer ts.lock.Unlock()
+
+	// There's two cases here:
+	//
+	// 1. We've added a new top-level domain name. Here, we want to make
+	//    sure the new zone file is pushed before we push the reference
+	//    to it.
+	// 2. We've just added a new. Here, the order doesn't matter, but
+	//    mostly likely the second push will be a no-op.
+	ts.pushZoneFiles()
+	ts.pushNamedConf()
 
 	// Wait until our config has taken.
 	corehelpers.RetryUntil(ts.t, 15*time.Second, func() error {