diff --git a/CHANGELOG.md b/CHANGELOG.md index 517776da..63a2e1ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,40 @@ See also the [v0.107.34 GitHub milestone][ms-v0.107.34]. NOTE: Add new changes BELOW THIS COMMENT. --> +### Changed + +#### Configuration Changes + +In this release, the schema version has changed from 23 to 24. + +- Properties starting with `log_`, and `verbose` property, which used to set up + logging are now moved to the new object `log` containing new properties `file`, + `max_backups`, `max_size`, `max_age`, `compress`, `local_time`, and `verbose`: + + ```yaml + # BEFORE: + 'log_file': "" + 'log_max_backups': 0 + 'log_max_size': 100 + 'log_max_age': 3 + 'log_compress': false + 'log_localtime': false + 'verbose': false + + # AFTER: + 'log': + 'file': "" + 'max_backups': 0 + 'max_size': 100 + 'max_age': 3 + 'compress': false + 'local_time': false + 'verbose': false + ``` + + To rollback this change, remove the new object `log`, set back `log_` and + `verbose` properties and change the `schema_version` back to `23`. + ### Fixed - Excessive RAM and CPU consumption by Safe Browsing and Parental Control diff --git a/internal/home/config.go b/internal/home/config.go index b7ac5bcc..42ab1af9 100644 --- a/internal/home/config.go +++ b/internal/home/config.go @@ -30,32 +30,30 @@ import ( const dataDir = "data" // logSettings are the logging settings part of the configuration file. -// -// TODO(a.garipov): Put them into a separate object. type logSettings struct { // File is the path to the log file. If empty, logs are written to stdout. // If "syslog", logs are written to syslog. - File string `yaml:"log_file"` + File string `yaml:"file"` // MaxBackups is the maximum number of old log files to retain. // // NOTE: MaxAge may still cause them to get deleted. - MaxBackups int `yaml:"log_max_backups"` + MaxBackups int `yaml:"max_backups"` // MaxSize is the maximum size of the log file before it gets rotated, in // megabytes. The default value is 100 MB. - MaxSize int `yaml:"log_max_size"` + MaxSize int `yaml:"max_size"` // MaxAge is the maximum duration for retaining old log files, in days. - MaxAge int `yaml:"log_max_age"` + MaxAge int `yaml:"max_age"` // Compress determines, if the rotated log files should be compressed using // gzip. - Compress bool `yaml:"log_compress"` + Compress bool `yaml:"compress"` // LocalTime determines, if the time used for formatting the timestamps in // is the computer's local time. - LocalTime bool `yaml:"log_localtime"` + LocalTime bool `yaml:"local_time"` // Verbose determines, if verbose (aka debug) logging is enabled. Verbose bool `yaml:"verbose"` @@ -142,7 +140,8 @@ type configuration struct { // Keep this field sorted to ensure consistent ordering. Clients *clientsConfig `yaml:"clients"` - logSettings `yaml:",inline"` + // Log is a block with log configuration settings. + Log logSettings `yaml:"log"` OSConfig *osConfig `yaml:"os"` @@ -390,7 +389,7 @@ var config = &configuration{ HostsFile: true, }, }, - logSettings: logSettings{ + Log: logSettings{ Compress: false, LocalTime: false, MaxBackups: 0, @@ -421,19 +420,19 @@ func (c *configuration) getConfigFilename() string { // separate method in order to configure logger before the actual configuration // is parsed and applied. func readLogSettings() (ls *logSettings) { - ls = &logSettings{} + conf := &configuration{} yamlFile, err := readConfigFile() if err != nil { - return ls + return &logSettings{} } - err = yaml.Unmarshal(yamlFile, ls) + err = yaml.Unmarshal(yamlFile, conf) if err != nil { log.Error("Couldn't get logging settings from the configuration: %s", err) } - return ls + return &conf.Log } // validateBindHosts returns error if any of binding hosts from configuration is diff --git a/internal/home/home.go b/internal/home/home.go index 572168fd..fe820c3d 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -829,20 +829,21 @@ func configureLogger(opts options) (err error) { // getLogSettings returns a log settings object properly initialized from opts. func getLogSettings(opts options) (ls *logSettings) { ls = readLogSettings() + configLogSettings := config.Log // Command-line arguments can override config settings. - if opts.verbose || config.Verbose { + if opts.verbose || configLogSettings.Verbose { ls.Verbose = true } - ls.File = stringutil.Coalesce(opts.logFile, config.File, ls.File) + ls.File = stringutil.Coalesce(opts.logFile, configLogSettings.File, ls.File) // Handle default log settings overrides. - ls.Compress = config.Compress - ls.LocalTime = config.LocalTime - ls.MaxBackups = config.MaxBackups - ls.MaxSize = config.MaxSize - ls.MaxAge = config.MaxAge + ls.Compress = configLogSettings.Compress + ls.LocalTime = configLogSettings.LocalTime + ls.MaxBackups = configLogSettings.MaxBackups + ls.MaxSize = configLogSettings.MaxSize + ls.MaxAge = configLogSettings.MaxAge if opts.runningAsService && ls.File == "" && runtime.GOOS == "windows" { // When running as a Windows service, use eventlog by default if diff --git a/internal/home/upgrade.go b/internal/home/upgrade.go index b6df4cad..96b46b77 100644 --- a/internal/home/upgrade.go +++ b/internal/home/upgrade.go @@ -23,7 +23,7 @@ import ( ) // currentSchemaVersion is the current schema version. -const currentSchemaVersion = 23 +const currentSchemaVersion = 24 // These aliases are provided for convenience. type ( @@ -98,6 +98,7 @@ func upgradeConfigSchema(oldVersion int, diskConf yobj) (err error) { upgradeSchema20to21, upgradeSchema21to22, upgradeSchema22to23, + upgradeSchema23to24, } n := 0 @@ -1325,6 +1326,110 @@ func upgradeSchema22to23(diskConf yobj) (err error) { return nil } +// upgradeSchema23to24 performs the following changes: +// +// # BEFORE: +// 'log_file': "" +// 'log_max_backups': 0 +// 'log_max_size': 100 +// 'log_max_age': 3 +// 'log_compress': false +// 'log_localtime': false +// 'verbose': false +// +// # AFTER: +// 'log': +// 'file': "" +// 'max_backups': 0 +// 'max_size': 100 +// 'max_age': 3 +// 'compress': false +// 'local_time': false +// 'verbose': false +func upgradeSchema23to24(diskConf yobj) (err error) { + log.Printf("Upgrade yaml: 23 to 24") + diskConf["schema_version"] = 24 + + logObj := yobj{} + err = coalesceError( + moveField[string](diskConf, logObj, "log_file", "file"), + moveField[int](diskConf, logObj, "log_max_backups", "max_backups"), + moveField[int](diskConf, logObj, "log_max_size", "max_size"), + moveField[int](diskConf, logObj, "log_max_age", "max_age"), + moveField[bool](diskConf, logObj, "log_compress", "compress"), + moveField[bool](diskConf, logObj, "log_localtime", "local_time"), + moveField[bool](diskConf, logObj, "verbose", "verbose"), + ) + if err != nil { + // Don't wrap the error, because it's informative enough as is. + return err + } + + if len(logObj) != 0 { + diskConf["log"] = logObj + } + + delete(diskConf, "log_file") + delete(diskConf, "log_max_backups") + delete(diskConf, "log_max_size") + delete(diskConf, "log_max_age") + delete(diskConf, "log_compress") + delete(diskConf, "log_localtime") + delete(diskConf, "verbose") + + return nil +} + +// moveField gets field value for key from diskConf, and then set this value +// in newConf for newKey. +func moveField[T any](diskConf, newConf yobj, key, newKey string) (err error) { + ok, newVal, err := fieldValue[T](diskConf, key) + if !ok { + return err + } + + switch v := newVal.(type) { + case int, bool, string: + newConf[newKey] = v + default: + return fmt.Errorf("invalid type of %s: %T", key, newVal) + } + + return nil +} + +// fieldValue returns the value of type T for key in diskConf object. +func fieldValue[T any](diskConf yobj, key string) (ok bool, field any, err error) { + fieldVal, ok := diskConf[key] + if !ok { + return false, new(T), nil + } + + f, ok := fieldVal.(T) + if !ok { + return false, nil, fmt.Errorf("unexpected type of %s: %T", key, fieldVal) + } + + return true, f, nil +} + +// coalesceError returns the first non-nil error. It is named after function +// COALESCE in SQL. If all errors are nil, it returns nil. +// +// TODO(a.garipov): Consider a similar helper to group errors together to show +// as many errors as possible. +// +// TODO(a.garipov): Think of ways to merge with [aghalg.Coalesce]. +func coalesceError(errors ...error) (res error) { + for _, err := range errors { + if err != nil { + return err + } + } + + return nil +} + // TODO(a.garipov): Replace with log.Output when we port it to our logging // package. func funcName() string { diff --git a/internal/home/upgrade_test.go b/internal/home/upgrade_test.go index 9f3f54dd..a440ccfc 100644 --- a/internal/home/upgrade_test.go +++ b/internal/home/upgrade_test.go @@ -1306,3 +1306,76 @@ func TestUpgradeSchema22to23(t *testing.T) { }) } } + +func TestUpgradeSchema23to24(t *testing.T) { + const newSchemaVer = 24 + + testCases := []struct { + in yobj + want yobj + name string + wantErrMsg string + }{{ + name: "empty", + in: yobj{}, + want: yobj{ + "schema_version": newSchemaVer, + }, + wantErrMsg: "", + }, { + name: "ok", + in: yobj{ + "log_file": "/test/path.log", + "log_max_backups": 1, + "log_max_size": 2, + "log_max_age": 3, + "log_compress": true, + "log_localtime": true, + "verbose": true, + }, + want: yobj{ + "log": yobj{ + "file": "/test/path.log", + "max_backups": 1, + "max_size": 2, + "max_age": 3, + "compress": true, + "local_time": true, + "verbose": true, + }, + "schema_version": newSchemaVer, + }, + wantErrMsg: "", + }, { + name: "invalid", + in: yobj{ + "log_file": "/test/path.log", + "log_max_backups": 1, + "log_max_size": 2, + "log_max_age": 3, + "log_compress": "", + "log_localtime": true, + "verbose": true, + }, + want: yobj{ + "log_file": "/test/path.log", + "log_max_backups": 1, + "log_max_size": 2, + "log_max_age": 3, + "log_compress": "", + "log_localtime": true, + "verbose": true, + "schema_version": newSchemaVer, + }, + wantErrMsg: "unexpected type of log_compress: string", + }} + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := upgradeSchema23to24(tc.in) + testutil.AssertErrorMsg(t, tc.wantErrMsg, err) + + assert.Equal(t, tc.want, tc.in) + }) + } +}