mirror of
https://github.com/AdguardTeam/AdGuardHome.git
synced 2024-09-11 10:45:42 +03:00
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:
parent
2069eddf98
commit
d4a4bda645
@ -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.
|
||||
|
@ -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
|
||||
|
@ -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
|
||||
|
@ -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 {
|
||||
|
@ -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
|
||||
}
|
||||
|
@ -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()
|
||||
|
@ -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
|
||||
}
|
||||
}
|
||||
|
@ -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
|
||||
}
|
||||
|
27
internal/next/configmgr/error.go
Normal file
27
internal/next/configmgr/error.go
Normal 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)
|
||||
}
|
@ -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 ./...
|
||||
|
Loading…
Reference in New Issue
Block a user