Skip unchanged zonefiles

Do not reload zonefiles which are entirely unchanged as determined
by SHA256 hash. In an environment where (e.g.) zones are PUT using
a REST API the same zone may be delivered in a situation where it
is entirely unchanged. In this circumstance is it useful to avoid
reloading it.

Signed-off-by: Alex Bligh <[email protected]>

zones.go

@@ -1,6 +1,8 @@
 package main
 
 import (
+	"crypto/sha256"
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -21,7 +23,12 @@ import (
 // Zones maps domain names to zone data
 type Zones map[string]*Zone
 
-var lastRead = map[string]time.Time{}
+type ZoneReadRecord struct {
+	time time.Time
+	hash string
+}
+
+var lastRead = map[string]*ZoneReadRecord{}
 
 func zonesReader(dirName string, zones Zones) {
 	for {
@@ -58,23 +65,54 @@ func zonesReadDir(dirName string, zones Zones) error {
 
 		seenZones[zoneName] = true
 
-		if _, ok := lastRead[zoneName]; !ok || file.ModTime().After(lastRead[zoneName]) {
+		if _, ok := lastRead[zoneName]; !ok || file.ModTime().After(lastRead[zoneName].time) {
+			modTime := file.ModTime()
 			if ok {
 				logPrintf("Reloading %s\n", fileName)
+				lastRead[zoneName].time = modTime
 			} else {
 				logPrintf("Reading new file %s\n", fileName)
+				lastRead[zoneName] = &ZoneReadRecord{time: modTime}
 			}
 
-			lastRead[zoneName] = file.ModTime()
+			filename := path.Join(dirName, fileName)
+
+			// Check the sha256 of the file has not changed. It's worth an explanation of
+			// why there isn't a TOCTOU race here. Conceivably after checking whether the
+			// SHA has changed, the contents then change again before we actually load
+			// the JSON. This can occur in two situations:
+			//
+			// 1. The SHA has not changed when we read the file for the SHA, but then
+			//    changes before we process the JSON
+			//
+			// 2. The SHA has changed when we read the file for the SHA, but then changes
+			//    again before we process the JSON
+			//
+			// In circumstance (1) we won't reread the file the first time, but the subsequent
+			// change should alter the mtime again, causing us to reread it. This reflects
+			// the fact there were actually two changes.
+			//
+			// In circumstance (2) we have already reread the file once, and then when the
+			// contents are changed the mtime changes again
+			//
+			// Provided files are replaced atomically, this should be OK. If files are not
+			// replaced atomically we have other problems (e.g. partial reads).
+
+			sha256 := sha256File(filename)
+			if lastRead[zoneName].hash == sha256 {
+				logPrintf("Skipping new file %s as hash is unchanged\n", filename)
+				continue
+			}
 
-			//log.Println("FILE:", i, file, zoneName)
-			config, err := readZoneFile(zoneName, path.Join(dirName, fileName))
+			config, err := readZoneFile(zoneName, filename)
 			if config == nil || err != nil {
 				parseErr = fmt.Errorf("Error reading zone '%s': %s", zoneName, err)
 				log.Println(parseErr.Error())
 				continue
 			}
 
+			(lastRead[zoneName]).hash = sha256
+
 			addHandler(zones, zoneName, config)
 		}
 	}
@@ -650,3 +688,13 @@ func valueToInt(v interface{}) (rv int) {
 func zoneNameFromFile(fileName string) string {
 	return fileName[0:strings.LastIndex(fileName, ".")]
 }
+
+func sha256File(fn string) string {
+	if data, err := ioutil.ReadFile(fn); err != nil {
+		return ""
+	} else {
+		hasher := sha256.New()
+		hasher.Write(data)
+		return hex.EncodeToString(hasher.Sum(nil))
+	}
+}

zones_test.go

@@ -5,7 +5,6 @@ import (
 	"io/ioutil"
 	"os"
 	"testing"
-	"time"
 
 	"github.com/abh/geodns/Godeps/_workspace/src/github.com/miekg/dns"
 	. "github.com/abh/geodns/Godeps/_workspace/src/gopkg.in/check.v1"
@@ -22,7 +21,7 @@ var _ = Suite(&ConfigSuite{})
 
 func (s *ConfigSuite) SetUpSuite(c *C) {
 	s.zones = make(Zones)
-	lastRead = map[string]time.Time{}
+	lastRead = map[string]*ZoneReadRecord{}
 	zonesReadDir("dns", s.zones)
 }