Pull request 1897: nextapi-write-conf

Squashed commit of the following:

commit 72f25ffe73d6b8216b01e590fba66fb5f6944113
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Wed Jun 28 21:29:04 2023 +0300

    next: add conf writing, validation
This commit is contained in:
Ainar Garipov 2023-06-29 14:34:06 +03:00
parent 2069eddf98
commit d4a4bda645
10 changed files with 232 additions and 37 deletions

View file

@ -51,6 +51,9 @@ func migrateDB(conf *ServerConfig) (err error) {
oldLeasesPath := filepath.Join(conf.WorkDir, dbFilename)
dataDirPath := filepath.Join(conf.DataDir, dataFilename)
// #nosec G304 -- Trust this path, since it's taken from the old file name
// relative to the working directory and should generally be considered
// safe.
file, err := os.Open(oldLeasesPath)
if errors.Is(err, os.ErrNotExist) {
// Nothing to migrate.

View file

@ -540,6 +540,7 @@ func (s *Server) prepareIpsetListSettings() (err error) {
return s.ipset.init(s.conf.IpsetList)
}
// #nosec G304 -- Trust the path explicitly given by the user.
data, err := os.ReadFile(fn)
if err != nil {
return err

View file

@ -24,6 +24,7 @@ enough.
### Fixed
- `--check-config` breaking the configuration file ([#4067]).
- Inconsistent application of `--work-dir/-w` ([#2598], [#2902]).
- The order of `-v/--verbose` and `--version` being significant ([#2893]).
@ -36,4 +37,5 @@ enough.
[#2598]: https://github.com/AdguardTeam/AdGuardHome/issues/2598
[#2893]: https://github.com/AdguardTeam/AdGuardHome/issues/2893
[#2902]: https://github.com/AdguardTeam/AdGuardHome/issues/2902
[#4067]: https://github.com/AdguardTeam/AdGuardHome/issues/4067
[#5676]: https://github.com/AdguardTeam/AdGuardHome/issues/5676

View file

@ -19,8 +19,6 @@ import (
func Main(frontend fs.FS) {
start := time.Now()
// Initial Configuration
cmdName := os.Args[0]
opts, err := parseOptions(cmdName, os.Args[1:])
exitCode, needExit := processOptions(opts, cmdName, err)
@ -39,9 +37,7 @@ func Main(frontend fs.FS) {
check(err)
}
// Web Service
confMgr, err := configmgr.New(opts.confFile, frontend, start)
confMgr, err := newConfigMgr(opts.confFile, frontend, start)
check(err)
web := confMgr.Web()
@ -73,6 +69,15 @@ func ctxWithDefaultTimeout() (ctx context.Context, cancel context.CancelFunc) {
return context.WithTimeout(context.Background(), defaultTimeout)
}
// newConfigMgr returns a new configuration manager using defaultTimeout as the
// context timeout.
func newConfigMgr(confFile string, frontend fs.FS, start time.Time) (m *configmgr.Manager, err error) {
ctx, cancel := ctxWithDefaultTimeout()
defer cancel()
return configmgr.New(ctx, confFile, frontend, start)
}
// check is a simple error-checking helper. It must only be used within Main.
func check(err error) {
if err != nil {

View file

@ -8,6 +8,7 @@ import (
"os"
"strings"
"github.com/AdguardTeam/AdGuardHome/internal/next/configmgr"
"github.com/AdguardTeam/AdGuardHome/internal/version"
"golang.org/x/exp/slices"
)
@ -55,9 +56,8 @@ type options struct {
webAddrs []netip.AddrPort
// checkConfig, if true, instructs AdGuard Home to check the configuration
// file and exit with a corresponding exit code.
//
// TODO(a.garipov): Use.
// file, optionally print an error message to stdout, and exit with a
// corresponding exit code.
checkConfig bool
// disableUpdate, if true, prevents AdGuard Home from automatically checking
@ -183,7 +183,7 @@ var commandLineOptions = []*commandLineOption{
checkConfigIdx: {
defaultValue: false,
description: "Check configuration and quit.",
description: "Check configuration, print errors to stdout, and quit.",
long: "check-config",
short: "",
valueType: "",
@ -399,5 +399,16 @@ func processOptions(
return 0, true
}
if opts.checkConfig {
err := configmgr.Validate(opts.confFile)
if err != nil {
_, _ = io.WriteString(os.Stdout, err.Error()+"\n")
return 1, true
}
return 0, true
}
return 0, false
}

View file

@ -7,7 +7,6 @@ import (
"github.com/AdguardTeam/AdGuardHome/internal/aghos"
"github.com/AdguardTeam/AdGuardHome/internal/next/agh"
"github.com/AdguardTeam/AdGuardHome/internal/next/configmgr"
"github.com/AdguardTeam/golibs/log"
)
@ -63,7 +62,7 @@ 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.frontend, h.start)
confMgr, err := newConfigMgr(h.confFile, h.frontend, h.start)
check(err)
web := confMgr.Web()

View file

@ -1,8 +1,10 @@
package configmgr
import (
"fmt"
"net/netip"
"github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/timeutil"
)
@ -19,6 +21,38 @@ type config struct {
Verbose bool `yaml:"verbose"`
}
const errNoConf errors.Error = "configuration not found"
// validate returns an error if the configuration structure is invalid.
func (c *config) validate() (err error) {
if c == nil {
return errNoConf
}
// TODO(a.garipov): Add more validations.
// Keep this in the same order as the fields in the config.
validators := []struct {
validate func() (err error)
name string
}{{
validate: c.DNS.validate,
name: "dns",
}, {
validate: c.HTTP.validate,
name: "http",
}}
for _, v := range validators {
err = v.validate()
if err != nil {
return fmt.Errorf("%s: %w", v.name, err)
}
}
return nil
}
// dnsConfig is the on-disk DNS configuration.
//
// TODO(a.garipov): Validate.
@ -32,6 +66,21 @@ type dnsConfig struct {
UseDNS64 bool `yaml:"use_dns64"`
}
// validate returns an error if the DNS configuration structure is invalid.
//
// TODO(a.garipov): Add more validations.
func (c *dnsConfig) validate() (err error) {
// TODO(a.garipov): Add more validations.
switch {
case c == nil:
return errNoConf
case c.UpstreamTimeout.Duration <= 0:
return newMustBePositiveError("upstream_timeout", c.UpstreamTimeout)
default:
return nil
}
}
// httpConfig is the on-disk web API configuration.
//
// TODO(a.garipov): Validate.
@ -41,3 +90,17 @@ type httpConfig struct {
Timeout timeutil.Duration `yaml:"timeout"`
ForceHTTPS bool `yaml:"force_https"`
}
// validate returns an error if the HTTP configuration structure is invalid.
//
// TODO(a.garipov): Add more validations.
func (c *httpConfig) validate() (err error) {
switch {
case c == nil:
return errNoConf
case c.Timeout.Duration <= 0:
return newMustBePositiveError("timeout", c.Timeout)
default:
return nil
}
}

View file

@ -1,5 +1,7 @@
// Package configmgr defines the AdGuard Home on-disk configuration entities and
// configuration manager.
//
// TODO(a.garipov): Add tests.
package configmgr
import (
@ -14,6 +16,10 @@ import (
"github.com/AdguardTeam/AdGuardHome/internal/next/dnssvc"
"github.com/AdguardTeam/AdGuardHome/internal/next/websvc"
"github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/log"
"github.com/AdguardTeam/golibs/timeutil"
"github.com/google/renameio/maybe"
"golang.org/x/exp/slices"
"gopkg.in/yaml.v3"
)
@ -39,17 +45,58 @@ type Manager struct {
fileName string
}
// Validate returns an error if the configuration file with the given name does
// not exist or is invalid.
func Validate(fileName string) (err error) {
conf, err := read(fileName)
if err != nil {
// Don't wrap the error, because it's informative enough as is.
return err
}
// Don't wrap the error, because it's informative enough as is.
return conf.validate()
}
// 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(
ctx context.Context,
fileName string,
frontend fs.FS,
start time.Time,
) (m *Manager, err error) {
conf, err := read(fileName)
if err != nil {
// Don't wrap the error, because it's informative enough as is.
return nil, err
}
err = conf.validate()
if err != nil {
return nil, fmt.Errorf("validating config: %w", err)
}
m = &Manager{
updMu: &sync.RWMutex{},
current: conf,
fileName: fileName,
}
err = m.assemble(ctx, conf, frontend, start)
if err != nil {
return nil, fmt.Errorf("creating config manager: %w", err)
}
return m, nil
}
// read reads and decodes configuration from the provided filename.
func read(fileName string) (conf *config, err error) {
defer func() { err = errors.Annotate(err, "reading config: %w") }()
conf := &config{}
conf = &config{}
f, err := os.Open(fileName)
if err != nil {
// Don't wrap the error, because it's informative enough as is.
@ -63,27 +110,7 @@ func New(
return nil, err
}
// TODO(a.garipov): Validate the configuration structure. Return an error
// if it's incorrect.
m = &Manager{
updMu: &sync.RWMutex{},
current: conf,
fileName: fileName,
}
// TODO(a.garipov): Get the context with the timeout from the arguments?
const assemblyTimeout = 5 * time.Second
ctx, cancel := context.WithTimeout(context.Background(), assemblyTimeout)
defer cancel()
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
}
return m, nil
return conf, nil
}
// assemble creates all services and puts them into the corresponding fields.
@ -128,6 +155,23 @@ func (m *Manager) assemble(
return nil
}
// write writes the current configuration to disk.
func (m *Manager) write() (err error) {
b, err := yaml.Marshal(m.current)
if err != nil {
return fmt.Errorf("encoding: %w", err)
}
err = maybe.WriteFile(m.fileName, b, 0o755)
if err != nil {
return fmt.Errorf("writing: %w", err)
}
log.Info("configmgr: written to %q", m.fileName)
return nil
}
// DNS returns the current DNS service. It is safe for concurrent use.
func (m *Manager) DNS() (dns agh.ServiceWithConfig[*dnssvc.Config]) {
m.updMu.RLock()
@ -150,7 +194,9 @@ func (m *Manager) UpdateDNS(ctx context.Context, c *dnssvc.Config) (err error) {
return fmt.Errorf("reassembling dnssvc: %w", err)
}
return nil
m.updateCurrentDNS(c)
return m.write()
}
// updateDNS recreates the DNS service. m.updMu is expected to be locked.
@ -172,6 +218,17 @@ func (m *Manager) updateDNS(ctx context.Context, c *dnssvc.Config) (err error) {
return nil
}
// updateCurrentDNS updates the DNS configuration in the current config.
func (m *Manager) updateCurrentDNS(c *dnssvc.Config) {
m.current.DNS.Addresses = slices.Clone(c.Addresses)
m.current.DNS.BootstrapDNS = slices.Clone(c.BootstrapServers)
m.current.DNS.UpstreamDNS = slices.Clone(c.UpstreamServers)
m.current.DNS.DNS64Prefixes = slices.Clone(c.DNS64Prefixes)
m.current.DNS.UpstreamTimeout = timeutil.Duration{Duration: c.UpstreamTimeout}
m.current.DNS.BootstrapPreferIPv6 = c.BootstrapPreferIPv6
m.current.DNS.UseDNS64 = c.UseDNS64
}
// Web returns the current web service. It is safe for concurrent use.
func (m *Manager) Web() (web agh.ServiceWithConfig[*websvc.Config]) {
m.updMu.RLock()
@ -194,7 +251,9 @@ func (m *Manager) UpdateWeb(ctx context.Context, c *websvc.Config) (err error) {
return fmt.Errorf("reassembling websvc: %w", err)
}
return nil
m.updateCurrentWeb(c)
return m.write()
}
// updateWeb recreates the web service. m.upd is expected to be locked.
@ -213,3 +272,11 @@ func (m *Manager) updateWeb(ctx context.Context, c *websvc.Config) (err error) {
return nil
}
// updateCurrentWeb updates the web configuration in the current config.
func (m *Manager) updateCurrentWeb(c *websvc.Config) {
m.current.HTTP.Addresses = slices.Clone(c.Addresses)
m.current.HTTP.SecureAddresses = slices.Clone(c.SecureAddresses)
m.current.HTTP.Timeout = timeutil.Duration{Duration: c.Timeout}
m.current.HTTP.ForceHTTPS = c.ForceHTTPS
}

View file

@ -0,0 +1,27 @@
package configmgr
import (
"fmt"
"github.com/AdguardTeam/golibs/timeutil"
"golang.org/x/exp/constraints"
)
// numberOrDuration is the constraint for integer types along with
// timeutil.Duration.
type numberOrDuration interface {
constraints.Integer | timeutil.Duration
}
// newMustBePositiveError returns an error about the value that must be positive
// but isn't. prop is the name of the property to mention in the error message.
//
// TODO(a.garipov): Consider moving such helpers to golibs and use in AdGuardDNS
// as well.
func newMustBePositiveError[T numberOrDuration](prop string, v T) (err error) {
if s, ok := any(v).(fmt.Stringer); ok {
return fmt.Errorf("%s must be positive, got %s", prop, s)
}
return fmt.Errorf("%s must be positive, got %d", prop, v)
}

View file

@ -197,8 +197,25 @@ run_linter nilness ./...
run_linter -e shadow --strict ./...
# TODO(a.garipov): Enable in v0.108.0.
# run_linter gosec --quiet ./...
# TODO(a.garipov): Enable for all.
run_linter gosec --quiet\
./internal/aghalg\
./internal/aghchan\
./internal/aghhttp\
./internal/aghio\
./internal/aghnet\
./internal/aghos\
./internal/aghtest\
./internal/dhcpd\
./internal/dhcpsvc\
./internal/dnsforward\
./internal/next\
./internal/schedule\
./internal/stats\
./internal/tools\
./internal/version\
./internal/whois\
;
# TODO(a.garipov): Enable --blank?
run_linter errcheck --asserts ./...