Pull request: 2499 merge rewrites vol.1

Merge in DNS/adguard-home from 2499-merge-rewrites-vol.1 to master

Updates #2499.

Squashed commit of the following:

commit 6b308bc2b360cee8c22e506f31d62bacb4bf8fb3
Merge: f49e9186 2b635bf6
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Oct 14 19:23:07 2021 +0300

    Merge branch 'master' into 2499-merge-rewrites-vol.1

commit f49e9186ffc8b7074d03c6721ee56cdb09243684
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Oct 14 18:50:49 2021 +0300

    aghos: fix fs events filtering

commit 567dd646556606212af5dab60e3ecbb8fff22c25
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Oct 14 16:50:37 2021 +0300

    all: imp code, docs, fix windows

commit 140c8bf519345eb54d0e7500a996fcf465353d71
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Oct 13 19:41:53 2021 +0300

    aghnet: use const

commit bebf3f76bd394a498ccad812c57d4507c69529ba
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Oct 13 19:32:37 2021 +0300

    all: imp tests, docs

commit 9bfdbb6eb454833135d616e208e82699f98e2562
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Oct 13 18:42:20 2021 +0300

    all: imp path more, imp docs

commit ee9ea4c132a6b17787d150bf2bee703abaa57be3
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Oct 13 16:09:46 2021 +0300

    all: fix windows, imp paths

commit 6fac8338a81e9ecfebfc23a1adcb964e89f6aee6
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Oct 11 19:53:35 2021 +0300

    all: imp code, docs

commit da1ce1a2a3dd2be3fdff2412a6dbd596859dc249
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Oct 11 18:22:50 2021 +0300

    aghnet: fix windows tests

commit d29de359ed68118d71efb226a8433fac15ff5c66
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Fri Oct 8 21:02:14 2021 +0300

    all: repl & imp

commit 1356c08944cdbb85ce5532d90fe5b077219ce5ff
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Fri Oct 8 01:41:19 2021 +0300

    all: add tests, mv logic, added tmpfs

commit f4b11adf8998bc8d9d955c5ac9f386f671bd5213
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Oct 7 14:26:30 2021 +0300

    all: imp filewalker, refactor hosts container

internal/aghos/filewalker.go

@@ -3,8 +3,7 @@ package aghos
 import (
 	"fmt"
 	"io"
-	"os"
-	"path/filepath"
+	"io/fs"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghio"
 	"github.com/AdguardTeam/golibs/errors"
@@ -14,10 +13,10 @@ import (
 // FileWalker is the signature of a function called for files in the file tree.
 // As opposed to filepath.Walk it only walk the files (not directories) matching
 // the provided pattern and those returned by function itself.  All patterns
-// should be valid for filepath.Glob.  If cont is false, the walking terminates.
-// Each opened file is also limited for reading to MaxWalkedFileSize.
+// should be valid for fs.Glob.  If cont is false, the walking terminates.  Each
+// opened file is also limited for reading to MaxWalkedFileSize.
 //
-// TODO(e.burkov):  Consider moving to the separate package like pathutil.
+// TODO(e.burkov, a.garipov): Move into another package like aghfs.
 //
 // TODO(e.burkov):  Think about passing filename or any additional data.
 type FileWalker func(r io.Reader) (patterns []string, cont bool, err error)
@@ -26,15 +25,19 @@ type FileWalker func(r io.Reader) (patterns []string, cont bool, err error)
 // check.
 const MaxWalkedFileSize = 1024 * 1024
 
-// checkFile tries to open and process a single file located on sourcePath.
-func checkFile(c FileWalker, sourcePath string) (patterns []string, cont bool, err error) {
-	var f *os.File
-	f, err = os.Open(sourcePath)
+// checkFile tries to open and process a single file located on sourcePath in
+// the specified fsys.  The path is skipped if it's a directory.
+func checkFile(
+	fsys fs.FS,
+	c FileWalker,
+	sourcePath string,
+) (patterns []string, cont bool, err error) {
+	var f fs.File
+	f, err = fsys.Open(sourcePath)
 	if err != nil {
-		if errors.Is(err, os.ErrNotExist) {
-			// Ignore non-existing files since this may only happen
-			// when the file was removed after filepath.Glob matched
-			// it.
+		if errors.Is(err, fs.ErrNotExist) {
+			// Ignore non-existing files since this may only happen when the
+			// file was removed after filepath.Glob matched it.
 			return nil, true, nil
 		}
 
@@ -42,9 +45,18 @@ func checkFile(c FileWalker, sourcePath string) (patterns []string, cont bool, e
 	}
 	defer func() { err = errors.WithDeferred(err, f.Close()) }()
 
+	var fi fs.FileInfo
+	if fi, err = f.Stat(); err != nil {
+		return nil, true, err
+	}
+	if fi.IsDir() {
+		// Skip the directories.
+		return nil, true, nil
+	}
+
 	var r io.Reader
-	// Ignore the error since LimitReader function returns error only if
-	// passed limit value is less than zero, but the constant used.
+	// Ignore the error since LimitReader function returns error only if passed
+	// limit value is less than zero, but the constant used.
 	//
 	// TODO(e.burkov):  Make variable.
 	r, _ = aghio.LimitReader(f, MaxWalkedFileSize)
@@ -52,13 +64,17 @@ func checkFile(c FileWalker, sourcePath string) (patterns []string, cont bool, e
 	return c(r)
 }
 
-// handlePatterns parses the patterns and ignores duplicates using srcSet.
-// srcSet must be non-nil.
-func handlePatterns(srcSet *stringutil.Set, patterns ...string) (sub []string, err error) {
+// handlePatterns parses the patterns in fsys and ignores duplicates using
+// srcSet.  srcSet must be non-nil.
+func handlePatterns(
+	fsys fs.FS,
+	srcSet *stringutil.Set,
+	patterns ...string,
+) (sub []string, err error) {
 	sub = make([]string, 0, len(patterns))
 	for _, p := range patterns {
 		var matches []string
-		matches, err = filepath.Glob(p)
+		matches, err = fs.Glob(fsys, p)
 		if err != nil {
 			// Enrich error with the pattern because filepath.Glob
 			// doesn't do it.
@@ -78,14 +94,14 @@ func handlePatterns(srcSet *stringutil.Set, patterns ...string) (sub []string, e
 	return sub, nil
 }
 
-// Walk starts walking the files defined by initPattern.  It only returns true
-// if c signed to stop walking.
-func (c FileWalker) Walk(initPattern string) (ok bool, err error) {
-	// The slice of sources keeps the order in which the files are walked
-	// since srcSet.Values() returns strings in undefined order.
+// Walk starts walking the files in fsys defined by patterns from initial.
+// It only returns true if fw signed to stop walking.
+func (fw FileWalker) Walk(fsys fs.FS, initial ...string) (ok bool, err error) {
+	// The slice of sources keeps the order in which the files are walked since
+	// srcSet.Values() returns strings in undefined order.
 	srcSet := stringutil.NewSet()
 	var src []string
-	src, err = handlePatterns(srcSet, initPattern)
+	src, err = handlePatterns(fsys, srcSet, initial...)
 	if err != nil {
 		return false, err
 	}
@@ -97,7 +113,7 @@ func (c FileWalker) Walk(initPattern string) (ok bool, err error) {
 		var patterns []string
 		var cont bool
 		filename = src[i]
-		patterns, cont, err = checkFile(c, src[i])
+		patterns, cont, err = checkFile(fsys, fw, src[i])
 		if err != nil {
 			return false, err
 		}
@@ -107,7 +123,7 @@ func (c FileWalker) Walk(initPattern string) (ok bool, err error) {
 		}
 
 		var subsrc []string
-		subsrc, err = handlePatterns(srcSet, patterns...)
+		subsrc, err = handlePatterns(fsys, srcSet, patterns...)
 		if err != nil {
 			return false, err
 		}

internal/aghos/filewalker_test.go

@@ -4,56 +4,19 @@ import (
 	"bufio"
 	"io"
 	"io/fs"
-	"os"
-	"path/filepath"
+	"path"
 	"testing"
+	"testing/fstest"
 
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-// testFSDir maps entries' names to entries which should either be a testFSDir
-// or byte slice.
-type testFSDir map[string]interface{}
-
-// testFSGen is used to generate a temporary filesystem consisting of
-// directories and plain text files from itself.
-type testFSGen testFSDir
-
-// gen returns the name of top directory of the generated filesystem.
-func (g testFSGen) gen(t *testing.T) (dirName string) {
-	t.Helper()
-
-	dirName = t.TempDir()
-	g.rangeThrough(t, dirName)
-
-	return dirName
-}
-
-func (g testFSGen) rangeThrough(t *testing.T, dirName string) {
-	const perm fs.FileMode = 0o777
-
-	for k, e := range g {
-		switch e := e.(type) {
-		case []byte:
-			require.NoError(t, os.WriteFile(filepath.Join(dirName, k), e, perm))
-
-		case testFSDir:
-			newDir := filepath.Join(dirName, k)
-			require.NoError(t, os.Mkdir(newDir, perm))
-
-			testFSGen(e).rangeThrough(t, newDir)
-		default:
-			t.Fatalf("unexpected entry type %T", e)
-		}
-	}
-}
-
 func TestFileWalker_Walk(t *testing.T) {
 	const attribute = `000`
 
-	makeFileWalker := func(dirName string) (fw FileWalker) {
+	makeFileWalker := func(_ string) (fw FileWalker) {
 		return func(r io.Reader) (patterns []string, cont bool, err error) {
 			s := bufio.NewScanner(r)
 			for s.Scan() {
@@ -63,7 +26,7 @@ func TestFileWalker_Walk(t *testing.T) {
 				}
 
 				if len(line) != 0 {
-					patterns = append(patterns, filepath.Join(dirName, line))
+					patterns = append(patterns, path.Join(".", line))
 				}
 			}
 
@@ -74,136 +37,150 @@ func TestFileWalker_Walk(t *testing.T) {
 	const nl = "\n"
 
 	testCases := []struct {
-		name        string
-		testFS      testFSGen
+		testFS      fstest.MapFS
+		want        assert.BoolAssertionFunc
 		initPattern string
-		want        bool
+		name        string
 	}{{
 		name: "simple",
-		testFS: testFSGen{
-			"simple_0001.txt": []byte(attribute + nl),
+		testFS: fstest.MapFS{
+			"simple_0001.txt": &fstest.MapFile{Data: []byte(attribute + nl)},
 		},
 		initPattern: "simple_0001.txt",
-		want:        true,
+		want:        assert.True,
 	}, {
 		name: "chain",
-		testFS: testFSGen{
-			"chain_0001.txt": []byte(`chain_0002.txt` + nl),
-			"chain_0002.txt": []byte(`chain_0003.txt` + nl),
-			"chain_0003.txt": []byte(attribute + nl),
+		testFS: fstest.MapFS{
+			"chain_0001.txt": &fstest.MapFile{Data: []byte(`chain_0002.txt` + nl)},
+			"chain_0002.txt": &fstest.MapFile{Data: []byte(`chain_0003.txt` + nl)},
+			"chain_0003.txt": &fstest.MapFile{Data: []byte(attribute + nl)},
 		},
 		initPattern: "chain_0001.txt",
-		want:        true,
+		want:        assert.True,
 	}, {
 		name: "several",
-		testFS: testFSGen{
-			"several_0001.txt": []byte(`several_*` + nl),
-			"several_0002.txt": []byte(`several_0001.txt` + nl),
-			"several_0003.txt": []byte(attribute + nl),
+		testFS: fstest.MapFS{
+			"several_0001.txt": &fstest.MapFile{Data: []byte(`several_*` + nl)},
+			"several_0002.txt": &fstest.MapFile{Data: []byte(`several_0001.txt` + nl)},
+			"several_0003.txt": &fstest.MapFile{Data: []byte(attribute + nl)},
 		},
 		initPattern: "several_0001.txt",
-		want:        true,
+		want:        assert.True,
 	}, {
 		name: "no",
-		testFS: testFSGen{
-			"no_0001.txt": []byte(nl),
-			"no_0002.txt": []byte(nl),
-			"no_0003.txt": []byte(nl),
+		testFS: fstest.MapFS{
+			"no_0001.txt": &fstest.MapFile{Data: []byte(nl)},
+			"no_0002.txt": &fstest.MapFile{Data: []byte(nl)},
+			"no_0003.txt": &fstest.MapFile{Data: []byte(nl)},
 		},
 		initPattern: "no_*",
-		want:        false,
+		want:        assert.False,
 	}, {
 		name: "subdirectory",
-		testFS: testFSGen{
-			"dir": testFSDir{
-				"subdir_0002.txt": []byte(attribute + nl),
+		testFS: fstest.MapFS{
+			path.Join("dir", "subdir_0002.txt"): &fstest.MapFile{
+				Data: []byte(attribute + nl),
 			},
-			"subdir_0001.txt": []byte(`dir/*`),
+			"subdir_0001.txt": &fstest.MapFile{Data: []byte(`dir/*`)},
 		},
 		initPattern: "subdir_0001.txt",
-		want:        true,
+		want:        assert.True,
 	}}
 
 	for _, tc := range testCases {
-		testDir := tc.testFS.gen(t)
-		fw := makeFileWalker(testDir)
+		fw := makeFileWalker("")
 
 		t.Run(tc.name, func(t *testing.T) {
-			ok, err := fw.Walk(filepath.Join(testDir, tc.initPattern))
+			ok, err := fw.Walk(tc.testFS, tc.initPattern)
 			require.NoError(t, err)
 
-			assert.Equal(t, tc.want, ok)
+			tc.want(t, ok)
 		})
 	}
 
 	t.Run("pattern_malformed", func(t *testing.T) {
-		ok, err := makeFileWalker("").Walk("[]")
+		f := fstest.MapFS{}
+		ok, err := makeFileWalker("").Walk(f, "[]")
 		require.Error(t, err)
 
 		assert.False(t, ok)
-		assert.ErrorIs(t, err, filepath.ErrBadPattern)
+		assert.ErrorIs(t, err, path.ErrBadPattern)
 	})
 
 	t.Run("bad_filename", func(t *testing.T) {
-		dir := testFSGen{
-			"bad_filename.txt": []byte("[]"),
-		}.gen(t)
-		fw := FileWalker(func(r io.Reader) (patterns []string, cont bool, err error) {
+		const filename = "bad_filename.txt"
+
+		f := fstest.MapFS{
+			filename: &fstest.MapFile{Data: []byte("[]")},
+		}
+		ok, err := FileWalker(func(r io.Reader) (patterns []string, cont bool, err error) {
 			s := bufio.NewScanner(r)
 			for s.Scan() {
 				patterns = append(patterns, s.Text())
 			}
 
 			return patterns, true, s.Err()
-		})
-
-		ok, err := fw.Walk(filepath.Join(dir, "bad_filename.txt"))
+		}).Walk(f, filename)
 		require.Error(t, err)
 
 		assert.False(t, ok)
-		assert.ErrorIs(t, err, filepath.ErrBadPattern)
+		assert.ErrorIs(t, err, path.ErrBadPattern)
 	})
 
 	t.Run("itself_error", func(t *testing.T) {
 		const rerr errors.Error = "returned error"
 
-		dir := testFSGen{
-			"mockfile.txt": []byte(`mockdata`),
-		}.gen(t)
+		f := fstest.MapFS{
+			"mockfile.txt": &fstest.MapFile{Data: []byte(`mockdata`)},
+		}
 
 		ok, err := FileWalker(func(r io.Reader) (patterns []string, ok bool, err error) {
 			return nil, true, rerr
-		}).Walk(filepath.Join(dir, "*"))
-		require.Error(t, err)
-		require.False(t, ok)
+		}).Walk(f, "*")
+		require.ErrorIs(t, err, rerr)
 
-		assert.ErrorIs(t, err, rerr)
+		assert.False(t, ok)
 	})
 }
 
+type errFS struct {
+	fs.GlobFS
+}
+
+const errErrFSOpen errors.Error = "this error is always returned"
+
+func (efs *errFS) Open(name string) (fs.File, error) {
+	return nil, errErrFSOpen
+}
+
 func TestWalkerFunc_CheckFile(t *testing.T) {
+	emptyFS := fstest.MapFS{}
+
 	t.Run("non-existing", func(t *testing.T) {
-		_, ok, err := checkFile(nil, "lol")
+		_, ok, err := checkFile(emptyFS, nil, "lol")
 		require.NoError(t, err)
 
 		assert.True(t, ok)
 	})
 
 	t.Run("invalid_argument", func(t *testing.T) {
-		const badPath = "\x00"
-
-		_, ok, err := checkFile(nil, badPath)
-		require.Error(t, err)
+		_, ok, err := checkFile(&errFS{}, nil, "")
+		require.ErrorIs(t, err, errErrFSOpen)
 
 		assert.False(t, ok)
-		// TODO(e.burkov):  Use assert.ErrorsIs within the error from
-		// less platform-dependent package instead of syscall.EINVAL.
-		//
-		// See https://github.com/golang/go/issues/46849 and
-		// https://github.com/golang/go/issues/30322.
-		pathErr := &os.PathError{}
-		require.ErrorAs(t, err, &pathErr)
-		assert.Equal(t, "open", pathErr.Op)
-		assert.Equal(t, badPath, pathErr.Path)
+	})
+
+	t.Run("ignore_dirs", func(t *testing.T) {
+		const dirName = "dir"
+
+		testFS := fstest.MapFS{
+			path.Join(dirName, "file"): &fstest.MapFile{Data: []byte{}},
+		}
+
+		patterns, ok, err := checkFile(testFS, nil, dirName)
+		require.NoError(t, err)
+
+		assert.Empty(t, patterns)
+		assert.True(t, ok)
 	})
 }

internal/aghos/fswatcher.go

@@ -0,0 +1,131 @@
+package aghos
+
+import (
+	"fmt"
+	"io"
+	"io/fs"
+	"path/filepath"
+
+	"github.com/AdguardTeam/golibs/errors"
+	"github.com/AdguardTeam/golibs/log"
+	"github.com/fsnotify/fsnotify"
+)
+
+// event is a convenient alias for an empty struct to signal that watching
+// event happened.
+type event = struct{}
+
+// FSWatcher tracks all the fyle system events and notifies about those.
+//
+// TODO(e.burkov, a.garipov): Move into another package like aghfs.
+type FSWatcher interface {
+	io.Closer
+
+	// Events should return a read-only channel which notifies about events.
+	Events() (e <-chan event)
+
+	// Add should check if the file named name is accessible and starts tracking
+	// it.
+	Add(name string) (err error)
+}
+
+// osWatcher tracks the file system provided by the OS.
+type osWatcher struct {
+	// w is the actual notifier that is handled by osWatcher.
+	w *fsnotify.Watcher
+
+	// events is the channel to notify.
+	events chan event
+}
+
+const (
+	// osWatcherPref is a prefix for logging and wrapping errors in osWathcer's
+	// methods.
+	osWatcherPref = "os watcher"
+)
+
+// NewOSWritesWatcher creates FSWatcher that tracks the real file system of the
+// OS and notifies only about writing events.
+func NewOSWritesWatcher() (w FSWatcher, err error) {
+	defer func() { err = errors.Annotate(err, "%s: %w", osWatcherPref) }()
+
+	var watcher *fsnotify.Watcher
+	watcher, err = fsnotify.NewWatcher()
+	if err != nil {
+		return nil, fmt.Errorf("creating watcher: %w", err)
+	}
+
+	fsw := &osWatcher{
+		w:      watcher,
+		events: make(chan event, 1),
+	}
+
+	go fsw.handleErrors()
+	go fsw.handleEvents()
+
+	return fsw, nil
+}
+
+// handleErrors handles accompanying errors.  It used to be called in a separate
+// goroutine.
+func (w *osWatcher) handleErrors() {
+	defer log.OnPanic(fmt.Sprintf("%s: handling errors", osWatcherPref))
+
+	for err := range w.w.Errors {
+		log.Error("%s: %s", osWatcherPref, err)
+	}
+}
+
+// Events implements the FSWatcher interface for *osWatcher.
+func (w *osWatcher) Events() (e <-chan event) {
+	return w.events
+}
+
+// Add implements the FSWatcher interface for *osWatcher.
+func (w *osWatcher) Add(name string) (err error) {
+	defer func() { err = errors.Annotate(err, "%s: %w", osWatcherPref) }()
+
+	if _, err = fs.Stat(RootDirFS(), name); err != nil {
+		return fmt.Errorf("checking file %q: %w", name, err)
+	}
+
+	return w.w.Add(filepath.Join("/", name))
+}
+
+// Close implements the FSWatcher interface for *osWatcher.
+func (w *osWatcher) Close() (err error) {
+	return w.w.Close()
+}
+
+// handleEvents notifies about the received file system's event if needed.  It
+// used to be called in a separate goroutine.
+func (w *osWatcher) handleEvents() {
+	defer log.OnPanic(fmt.Sprintf("%s: handling events", osWatcherPref))
+
+	defer close(w.events)
+
+	ch := w.w.Events
+	for e := range ch {
+		if e.Op&fsnotify.Write == 0 {
+			continue
+		}
+
+		// Skip the following events assuming that sometimes the same event
+		// occurrs several times.
+		for ok := true; ok; {
+			select {
+			case _, ok = <-ch:
+				// Go on.
+			default:
+				ok = false
+			}
+		}
+
+		select {
+		case w.events <- event{}:
+			// Go on.
+		default:
+			log.Debug("%s: events buffer is full", osWatcherPref)
+		}
+	}
+}

internal/aghos/os.go

@@ -7,6 +7,8 @@ import (
 	"bufio"
 	"fmt"
 	"io"
+	"io/fs"
+	"os"
 	"os/exec"
 	"path"
 	"runtime"
@@ -159,3 +161,10 @@ ScanLoop:
 func IsOpenWrt() (ok bool) {
 	return isOpenWrt()
 }
+
+// RootDirFS returns the fs.FS rooted at the operating system's root.
+func RootDirFS() (fsys fs.FS) {
+	// Use empty string since os.DirFS implicitly prepends a slash to it.  This
+	// behavior is undocumented but it currently works.
+	return os.DirFS("")
+}

internal/aghos/os_linux.go

@@ -26,6 +26,8 @@ func haveAdminRights() (bool, error) {
 }
 
 func isOpenWrt() (ok bool) {
+	const etcReleasePattern = "etc/*release*"
+
 	var err error
 	ok, err = FileWalker(func(r io.Reader) (_ []string, cont bool, err error) {
 		const osNameData = "openwrt"
@@ -39,7 +41,7 @@ func isOpenWrt() (ok bool) {
 		}
 
 		return nil, !stringutil.ContainsFold(string(data), osNameData), nil
-	}).Walk("/etc/*release*")
+	}).Walk(RootDirFS(), etcReleasePattern)
 
 	return err == nil && ok
 }