Pull request 1870: nextapi-frontend-fs

Merge in DNS/adguard-home from nextapi-frontend-fs to master

Squashed commit of the following:

commit 3ed959f21939cf5590c27426af46906cbffed502
Merge: e60bbdd04 9fda7bfd3
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Jun 13 13:37:00 2023 +0300

    Merge branch 'master' into nextapi-frontend-fs

commit e60bbdd04ce841c1aaaa198cc9dc85ae14799ffa
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Fri Jun 9 14:53:09 2023 +0300

    next: support frontend fs

internal/aghnet/interfaces_windows.go

@@ -1,17 +0,0 @@
-//go:build windows
-
-package aghnet
-
-import (
-	"net"
-
-	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
-)
-
-// listenPacketReusable announces on the local network address additionally
-// configuring the socket to have a reusable binding.
-func listenPacketReusable(_, _, _ string) (c net.PacketConn, err error) {
-	// TODO(e.burkov):  Check if we are able to control sockets on Windows
-	// in the same way as on Unix.
-	return nil, aghos.Unsupported("listening packet reusable")
-}

internal/dhcpd/os_windows.go

@@ -1,15 +0,0 @@
-//go:build windows
-
-package dhcpd
-
-import (
-	"net"
-
-	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
-	"golang.org/x/net/ipv4"
-)
-
-// Create a socket for receiving broadcast packets
-func newBroadcastPacketConn(_ net.IP, _ int, _ string) (*ipv4.PacketConn, error) {
-	return nil, aghos.Unsupported("newBroadcastPacketConn")
-}

internal/next/cmd/cmd.go

@@ -1,5 +1,5 @@
-// Package cmd is the AdGuard Home entry point.  It contains the on-disk
-// configuration file utilities, signal processing logic, and so on.
+// Package cmd is the AdGuard Home entry point.  It assembles the configuration
+// file manager, sets up signal processing logic, and so on.
 //
 // TODO(a.garipov): Move to the upper-level internal/.
 package cmd
@@ -7,7 +7,6 @@ package cmd
 import (
 	"context"
 	"io/fs"
-	"math/rand"
 	"os"
 	"time"
 
@@ -16,12 +15,11 @@ import (
 	"github.com/AdguardTeam/golibs/log"
 )
 
-// Main is the entry point of application.
-func Main(clientBuildFS fs.FS) {
+// Main is the entry point of AdGuard Home.
+func Main(frontend fs.FS) {
 	// Initial Configuration
 
 	start := time.Now()
-	rand.Seed(start.UnixNano())
 
 	// TODO(a.garipov): Set up logging.
 
@@ -29,38 +27,34 @@ func Main(clientBuildFS fs.FS) {
 
 	// Web Service
 
-	// TODO(a.garipov): Use in the Web service.
-	_ = clientBuildFS
-
 	// TODO(a.garipov): Set up configuration file name.
 	const confFile = "AdGuardHome.1.yaml"
 
-	confMgr, err := configmgr.New(confFile, start)
-	fatalOnError(err)
+	confMgr, err := configmgr.New(confFile, frontend, start)
+	check(err)
 
 	web := confMgr.Web()
 	err = web.Start()
-	fatalOnError(err)
+	check(err)
 
 	dns := confMgr.DNS()
 	err = dns.Start()
-	fatalOnError(err)
+	check(err)
 
 	sigHdlr := newSignalHandler(
 		confFile,
+		frontend,
 		start,
 		web,
 		dns,
 	)
 
-	go sigHdlr.handle()
-
-	select {}
+	sigHdlr.handle()
 }
 
 // defaultTimeout is the timeout used for some operations where another timeout
 // hasn't been defined yet.
-const defaultTimeout = 15 * time.Second
+const defaultTimeout = 5 * time.Second
 
 // ctxWithDefaultTimeout is a helper function that returns a context with
 // timeout set to defaultTimeout.
@@ -68,10 +62,9 @@ func ctxWithDefaultTimeout() (ctx context.Context, cancel context.CancelFunc) {
 	return context.WithTimeout(context.Background(), defaultTimeout)
 }
 
-// fatalOnError is a helper that exits the program with an error code if err is
-// not nil.  It must only be used within Main.
-func fatalOnError(err error) {
+// check is a simple error-checking helper.  It must only be used within Main.
+func check(err error) {
 	if err != nil {
-		log.Fatal(err)
+		panic(err)
 	}
 }

internal/next/cmd/signal.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"io/fs"
 	"os"
 	"time"
 
@@ -18,6 +19,10 @@ type signalHandler struct {
 	// confFile is the path to the configuration file.
 	confFile string
 
+	// frontend is the filesystem with the frontend and other statically
+	// compiled files.
+	frontend fs.FS
+
 	// start is the time at which AdGuard Home has been started.
 	start time.Time
 
@@ -58,16 +63,16 @@ func (h *signalHandler) reconfigure() {
 	// reconfigured without the full shutdown, and the error handling is
 	// currently not the best.
 
-	confMgr, err := configmgr.New(h.confFile, h.start)
-	fatalOnError(err)
+	confMgr, err := configmgr.New(h.confFile, h.frontend, h.start)
+	check(err)
 
 	web := confMgr.Web()
 	err = web.Start()
-	fatalOnError(err)
+	check(err)
 
 	dns := confMgr.DNS()
 	err = dns.Start()
-	fatalOnError(err)
+	check(err)
 
 	h.services = []agh.Service{
 		dns,
@@ -103,10 +108,16 @@ func (h *signalHandler) shutdown() (status int) {
 }
 
 // newSignalHandler returns a new signalHandler that shuts down svcs.
-func newSignalHandler(confFile string, start time.Time, svcs ...agh.Service) (h *signalHandler) {
+func newSignalHandler(
+	confFile string,
+	frontend fs.FS,
+	start time.Time,
+	svcs ...agh.Service,
+) (h *signalHandler) {
 	h = &signalHandler{
 		signal:   make(chan os.Signal, 1),
 		confFile: confFile,
+		frontend: frontend,
 		start:    start,
 		services: svcs,
 	}

internal/next/configmgr/configmgr.go

@@ -5,6 +5,7 @@ package configmgr
 import (
 	"context"
 	"fmt"
+	"io/fs"
 	"os"
 	"sync"
 	"time"
@@ -42,7 +43,11 @@ type Manager struct {
 // New creates a new *Manager that persists changes to the file pointed to by
 // fileName.  It reads the configuration file and populates the service fields.
 // start is the startup time of AdGuard Home.
-func New(fileName string, start time.Time) (m *Manager, err error) {
+func New(
+	fileName string,
+	frontend fs.FS,
+	start time.Time,
+) (m *Manager, err error) {
 	defer func() { err = errors.Annotate(err, "reading config") }()
 
 	conf := &config{}
@@ -79,7 +84,7 @@ func New(fileName string, start time.Time) (m *Manager, err error) {
 	ctx, cancel := context.WithTimeout(context.Background(), assemblyTimeout)
 	defer cancel()
 
-	err = m.assemble(ctx, conf, start)
+	err = m.assemble(ctx, conf, frontend, start)
 	if err != nil {
 		// Don't wrap the error, because it's informative enough as is.
 		return nil, err
@@ -90,7 +95,12 @@ func New(fileName string, start time.Time) (m *Manager, err error) {
 
 // assemble creates all services and puts them into the corresponding fields.
 // The fields of conf must not be modified after calling assemble.
-func (m *Manager) assemble(ctx context.Context, conf *config, start time.Time) (err error) {
+func (m *Manager) assemble(
+	ctx context.Context,
+	conf *config,
+	frontend fs.FS,
+	start time.Time,
+) (err error) {
 	dnsConf := &dnssvc.Config{
 		Addresses:        conf.DNS.Addresses,
 		BootstrapServers: conf.DNS.BootstrapDNS,
@@ -104,6 +114,7 @@ func (m *Manager) assemble(ctx context.Context, conf *config, start time.Time) (
 
 	webSvcConf := &websvc.Config{
 		ConfigManager: m,
+		Frontend:      frontend,
 		// TODO(a.garipov): Fill from config file.
 		TLS:             nil,
 		Start:           start,
@@ -199,7 +210,10 @@ func (m *Manager) updateWeb(ctx context.Context, c *websvc.Config) (err error) {
 		}
 	}
 
-	m.web = websvc.New(c)
+	m.web, err = websvc.New(c)
+	if err != nil {
+		return fmt.Errorf("creating web svc: %w", err)
+	}
 
 	return nil
 }

internal/next/websvc/http.go

@@ -53,6 +53,7 @@ func (svc *Service) handlePatchSettingsHTTP(w http.ResponseWriter, r *http.Reque
 
 	newConf := &Config{
 		ConfigManager:   svc.confMgr,
+		Frontend:        svc.frontend,
 		TLS:             svc.tls,
 		Addresses:       req.Addresses,
 		SecureAddresses: req.SecureAddresses,

internal/next/websvc/http_test.go

@@ -24,21 +24,20 @@ func TestService_HandlePatchSettingsHTTP(t *testing.T) {
 		ForceHTTPS:      false,
 	}
 
+	svc, err := websvc.New(&websvc.Config{
+		TLS: &tls.Config{
+			Certificates: []tls.Certificate{{}},
+		},
+		Addresses:       []netip.AddrPort{netip.MustParseAddrPort("127.0.0.1:80")},
+		SecureAddresses: []netip.AddrPort{netip.MustParseAddrPort("127.0.0.1:443")},
+		Timeout:         5 * time.Second,
+		ForceHTTPS:      true,
+	})
+	require.NoError(t, err)
+
 	confMgr := newConfigManager()
-	confMgr.onWeb = func() (s agh.ServiceWithConfig[*websvc.Config]) {
-		return websvc.New(&websvc.Config{
-			TLS: &tls.Config{
-				Certificates: []tls.Certificate{{}},
-			},
-			Addresses:       []netip.AddrPort{netip.MustParseAddrPort("127.0.0.1:80")},
-			SecureAddresses: []netip.AddrPort{netip.MustParseAddrPort("127.0.0.1:443")},
-			Timeout:         5 * time.Second,
-			ForceHTTPS:      true,
-		})
-	}
-	confMgr.onUpdateWeb = func(ctx context.Context, c *websvc.Config) (err error) {
-		return nil
-	}
+	confMgr.onWeb = func() (s agh.ServiceWithConfig[*websvc.Config]) { return svc }
+	confMgr.onUpdateWeb = func(ctx context.Context, c *websvc.Config) (err error) { return nil }
 
 	_, addr := newTestServer(t, confMgr)
 	u := &url.URL{
@@ -56,7 +55,7 @@ func TestService_HandlePatchSettingsHTTP(t *testing.T) {
 
 	respBody := httpPatch(t, u, req, http.StatusOK)
 	resp := &websvc.HTTPAPIHTTPSettings{}
-	err := json.Unmarshal(respBody, resp)
+	err = json.Unmarshal(respBody, resp)
 	require.NoError(t, err)
 
 	assert.Equal(t, wantWeb, resp)

internal/next/websvc/middleware.go

@@ -2,9 +2,11 @@ package websvc
 
 import (
 	"net/http"
+	"time"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
 	"github.com/AdguardTeam/golibs/httphdr"
+	"github.com/AdguardTeam/golibs/log"
 )
 
 // Middlewares
@@ -19,3 +21,18 @@ func jsonMw(h http.Handler) (wrapped http.HandlerFunc) {
 
 	return http.HandlerFunc(f)
 }
+
+// logMw logs the queries with level debug.
+func logMw(h http.Handler) (wrapped http.HandlerFunc) {
+	f := func(w http.ResponseWriter, r *http.Request) {
+		start := time.Now()
+		m, u := r.Method, r.RequestURI
+
+		log.Debug("websvc: %s %s started", m, u)
+		defer func() { log.Debug("websvc: %s %s finished in %s", m, u, time.Since(start)) }()
+
+		h.ServeHTTP(w, r)
+	}
+
+	return http.HandlerFunc(f)
+}

internal/next/websvc/path.go

@@ -2,6 +2,9 @@ package websvc
 
 // Path constants
 const (
+	PathRoot     = "/"
+	PathFrontend = "/*filepath"
+
 	PathHealthCheck = "/health-check"
 
 	PathV1SettingsAll  = "/api/v1/settings/all"

internal/next/websvc/settings_test.go

@@ -46,16 +46,19 @@ func TestService_HandleGetSettingsAll(t *testing.T) {
 		return c
 	}
 
+	svc, err := websvc.New(&websvc.Config{
+		TLS: &tls.Config{
+			Certificates: []tls.Certificate{{}},
+		},
+		Addresses:       wantWeb.Addresses,
+		SecureAddresses: wantWeb.SecureAddresses,
+		Timeout:         time.Duration(wantWeb.Timeout),
+		ForceHTTPS:      true,
+	})
+	require.NoError(t, err)
+
 	confMgr.onWeb = func() (s agh.ServiceWithConfig[*websvc.Config]) {
-		return websvc.New(&websvc.Config{
-			TLS: &tls.Config{
-				Certificates: []tls.Certificate{{}},
-			},
-			Addresses:       wantWeb.Addresses,
-			SecureAddresses: wantWeb.SecureAddresses,
-			Timeout:         time.Duration(wantWeb.Timeout),
-			ForceHTTPS:      true,
-		})
+		return svc
 	}
 
 	_, addr := newTestServer(t, confMgr)
@@ -67,7 +70,7 @@ func TestService_HandleGetSettingsAll(t *testing.T) {
 
 	body := httpGet(t, u, http.StatusOK)
 	resp := &websvc.RespGetV1SettingsAll{}
-	err := json.Unmarshal(body, resp)
+	err = json.Unmarshal(body, resp)
 	require.NoError(t, err)
 
 	assert.Equal(t, wantDNS, resp.DNS)

internal/next/websvc/websvc.go

@@ -11,6 +11,7 @@ import (
 	"crypto/tls"
 	"fmt"
 	"io"
+	"io/fs"
 	"net"
 	"net/http"
 	"net/netip"
@@ -39,6 +40,10 @@ type Config struct {
 	// dynamically reconfigure them.
 	ConfigManager ConfigManager
 
+	// Frontend is the filesystem with the frontend and other statically
+	// compiled files.
+	Frontend fs.FS
+
 	// TLS is the optional TLS configuration.  If TLS is not nil,
 	// SecureAddresses must not be empty.
 	TLS *tls.Config
@@ -67,6 +72,7 @@ type Config struct {
 // [agh.Service] that does nothing.
 type Service struct {
 	confMgr    ConfigManager
+	frontend   fs.FS
 	tls        *tls.Config
 	start      time.Time
 	servers    []*http.Server
@@ -77,13 +83,22 @@ type Service struct {
 // New returns a new properly initialized *Service.  If c is nil, svc is a nil
 // *Service that does nothing.  The fields of c must not be modified after
 // calling New.
-func New(c *Config) (svc *Service) {
+//
+// TODO(a.garipov): Get rid of this special handling of nil or explain it
+// better.
+func New(c *Config) (svc *Service, err error) {
 	if c == nil {
-		return nil
+		return nil, nil
+	}
+
+	frontend, err := fs.Sub(c.Frontend, "build/static")
+	if err != nil {
+		return nil, fmt.Errorf("frontend fs: %w", err)
 	}
 
 	svc = &Service{
 		confMgr:    c.ConfigManager,
+		frontend:   frontend,
 		tls:        c.TLS,
 		start:      c.Start,
 		timeout:    c.Timeout,
@@ -121,7 +136,7 @@ func New(c *Config) (svc *Service) {
 		})
 	}
 
-	return svc
+	return svc, nil
 }
 
 // newMux returns a new HTTP request multiplexor for the AdGuard Home web
@@ -132,41 +147,54 @@ func newMux(svc *Service) (mux *httptreemux.ContextMux) {
 	routes := []struct {
 		handler http.HandlerFunc
 		method  string
-		path    string
+		pattern string
 		isJSON  bool
 	}{{
 		handler: svc.handleGetHealthCheck,
 		method:  http.MethodGet,
-		path:    PathHealthCheck,
+		pattern: PathHealthCheck,
+		isJSON:  false,
+	}, {
+		handler: http.FileServer(http.FS(svc.frontend)).ServeHTTP,
+		method:  http.MethodGet,
+		pattern: PathFrontend,
+		isJSON:  false,
+	}, {
+		handler: http.FileServer(http.FS(svc.frontend)).ServeHTTP,
+		method:  http.MethodGet,
+		pattern: PathRoot,
 		isJSON:  false,
 	}, {
 		handler: svc.handleGetSettingsAll,
 		method:  http.MethodGet,
-		path:    PathV1SettingsAll,
+		pattern: PathV1SettingsAll,
 		isJSON:  true,
 	}, {
 		handler: svc.handlePatchSettingsDNS,
 		method:  http.MethodPatch,
-		path:    PathV1SettingsDNS,
+		pattern: PathV1SettingsDNS,
 		isJSON:  true,
 	}, {
 		handler: svc.handlePatchSettingsHTTP,
 		method:  http.MethodPatch,
-		path:    PathV1SettingsHTTP,
+		pattern: PathV1SettingsHTTP,
 		isJSON:  true,
 	}, {
 		handler: svc.handleGetV1SystemInfo,
 		method:  http.MethodGet,
-		path:    PathV1SystemInfo,
+		pattern: PathV1SystemInfo,
 		isJSON:  true,
 	}}
 
 	for _, r := range routes {
+		var hdlr http.Handler
 		if r.isJSON {
-			mux.Handle(r.method, r.path, jsonMw(r.handler))
+			hdlr = jsonMw(r.handler)
 		} else {
-			mux.Handle(r.method, r.path, r.handler)
+			hdlr = r.handler
 		}
+
+		mux.Handle(r.method, r.pattern, logMw(hdlr))
 	}
 
 	return mux

internal/next/websvc/websvc_test.go

@@ -5,12 +5,14 @@ import (
 	"context"
 	"encoding/json"
 	"io"
+	"io/fs"
 	"net/http"
 	"net/netip"
 	"net/url"
 	"testing"
 	"time"
 
+	"github.com/AdguardTeam/AdGuardHome/internal/aghtest"
 	"github.com/AdguardTeam/AdGuardHome/internal/next/agh"
 	"github.com/AdguardTeam/AdGuardHome/internal/next/dnssvc"
 	"github.com/AdguardTeam/AdGuardHome/internal/next/websvc"
@@ -87,7 +89,10 @@ func newTestServer(
 	t.Helper()
 
 	c := &websvc.Config{
-		ConfigManager:   confMgr,
+		ConfigManager: confMgr,
+		Frontend: &aghtest.FS{
+			OnOpen: func(_ string) (_ fs.File, _ error) { return nil, fs.ErrNotExist },
+		},
 		TLS:             nil,
 		Addresses:       []netip.AddrPort{netip.MustParseAddrPort("127.0.0.1:0")},
 		SecureAddresses: nil,
@@ -96,9 +101,10 @@ func newTestServer(
 		ForceHTTPS:      false,
 	}
 
-	svc = websvc.New(c)
+	svc, err := websvc.New(c)
+	require.NoError(t, err)
 
-	err := svc.Start()
+	err = svc.Start()
 	require.NoError(t, err)
 	t.Cleanup(func() {
 		ctx, cancel := context.WithTimeout(context.Background(), testTimeout)

main_next.go

@@ -13,8 +13,8 @@ import (
 // outside of the same or underlying directory.
 
 //go:embed build
-var clientBuildFS embed.FS
+var frontend embed.FS
 
 func main() {
-	cmd.Main(clientBuildFS)
+	cmd.Main(frontend)
 }

scripts/make/go-lint.sh

@@ -3,7 +3,7 @@
 # This comment is used to simplify checking local copies of the script.  Bump
 # this number every time a significant change is made to this script.
 #
-# AdGuard-Project-Version: 3
+# AdGuard-Project-Version: 4
 
 verbose="${VERBOSE:-0}"
 readonly verbose
@@ -80,6 +80,12 @@ esac
 #
 #   *  Package golang.org/x/net/context has been moved into stdlib.
 #
+# Currently, the only standard exception are files generated from protobuf
+# schemas, which use package reflect.  If your project needs more exceptions,
+# add and document them.
+#
+# TODO(a.garipov): Add deprecated packages golang.org/x/exp/maps and
+# golang.org/x/exp/slices once all projects switch to Go 1.21.
 blocklist_imports() {
 	git grep\
 		-e '[[:space:]]"errors"$'\
@@ -91,6 +97,7 @@ blocklist_imports() {
 		-e '[[:space:]]"golang.org/x/net/context"$'\
 		-n\
 		-- '*.go'\
+		':!*.pb.go'\
 		| sed -e 's/^\([^[:space:]]\+\)\(.*\)$/\1 blocked import:\2/'\
 		|| exit 0
 }
@@ -101,6 +108,7 @@ method_const() {
 	git grep -F\
 		-e '"DELETE"'\
 		-e '"GET"'\
+		-e '"PATCH"'\
 		-e '"POST"'\
 		-e '"PUT"'\
 		-n\
@@ -127,7 +135,7 @@ underscores() {
 			-e '_others.go'\
 			-e '_test.go'\
 			-e '_unix.go'\
-			-e '_windows.go' \
+			-e '_windows.go'\
 			-v\
 			| sed -e 's/./\t\0/'
 	)"
@@ -166,8 +174,9 @@ run_linter ineffassign ./...
 
 run_linter unparam ./...
 
-git ls-files -- 'Makefile' '*.go' '*.mod' '*.sh' '*.yaml' '*.yml'\
-	| xargs misspell --error
+git ls-files -- 'Makefile' '*.conf' '*.go' '*.mod' '*.sh' '*.yaml' '*.yml'\
+	| xargs misspell --error\
+	| sed -e 's/^/misspell: /'
 
 run_linter looppointer ./...
 
@@ -183,4 +192,13 @@ run_linter -e shadow --strict ./...
 # TODO(a.garipov): Enable --blank?
 run_linter errcheck --asserts ./...
 
-run_linter staticcheck ./...
+staticcheck_matrix='
+darwin:  GOOS=darwin
+freebsd: GOOS=freebsd
+linux:   GOOS=linux
+openbsd: GOOS=openbsd
+windows: GOOS=windows
+'
+readonly staticcheck_matrix
+
+echo "$staticcheck_matrix" | run_linter staticcheck --matrix ./...