From 5a195b441c5669b5ad45595853f58e2e9b714c2c Mon Sep 17 00:00:00 2001 From: Dimitry Kolyshev Date: Thu, 6 Jul 2023 10:09:04 +0300 Subject: [PATCH] Pull request: log-yaml-conf Updates #4897. Squashed commit of the following: commit 8a961157c9930bf4859ce2209e5016ce94987e12 Author: Dimitry Kolyshev Date: Wed Jul 5 10:13:24 2023 +0400 home: imp code commit 509c07eed06311d773bc3e34b3ca28d9f14186fe Author: Dimitry Kolyshev Date: Tue Jul 4 17:33:31 2023 +0400 all: fix commit f032e28f98552f238721491c998fca2d7d4b9802 Merge: ee0113435 c46516475 Author: Dimitry Kolyshev Date: Tue Jul 4 17:31:29 2023 +0400 Merge remote-tracking branch 'origin/master' into log-yaml-conf # Conflicts: # CHANGELOG.md commit ee011343512e82d4e21cb402759d0284523ba02a Author: Dimitry Kolyshev Date: Tue Jul 4 12:31:42 2023 +0400 all: changelog commit 07f4c4a244b1b6200d3056cde5ebced6254084a7 Merge: 2042c0753 97af062f7 Author: Dimitry Kolyshev Date: Tue Jul 4 12:25:21 2023 +0400 Merge remote-tracking branch 'origin/master' into log-yaml-conf commit 2042c0753ec29de6045c3f1de6d075cb93d6ec27 Merge: a1d3a5130 8004b135b Author: Dimitry Kolyshev Date: Mon Jul 3 16:25:26 2023 +0400 Merge remote-tracking branch 'origin/master' into log-yaml-conf commit a1d3a51307e80f9e509bd6f3bee1a7b17bf1ffe6 Author: Dimitry Kolyshev Date: Fri Jun 30 11:11:32 2023 +0400 home: imp code commit 2392a3b02620b8c38e88afb4d75988be85fe1338 Merge: 4224fbed7 39f5c50ac Author: Dimitry Kolyshev Date: Thu Jun 29 16:46:47 2023 +0400 home: imp code commit 4224fbed7113e94bee44d0ab0272e8302a8086f3 Author: Dimitry Kolyshev Date: Thu Jun 29 16:39:35 2023 +0400 home: imp code commit 5ce708cc50bed83e64f062e599fc8b6143d0d44d Author: Dimitry Kolyshev Date: Thu Jun 29 12:48:42 2023 +0400 all: docs commit 4b6d898a888818410f59b843c3ca1a685aafec82 Author: Dimitry Kolyshev Date: Thu Jun 29 12:20:54 2023 +0400 home: imp code commit 431b44eda71f488c747c3efa4da0a6c222b1cf06 Author: Dimitry Kolyshev Date: Thu Jun 29 12:03:17 2023 +0400 home: imp tests commit 53a5c31060018c37953beb27d80c46f92bbe14af Author: Dimitry Kolyshev Date: Wed Jun 28 17:40:49 2023 +0400 home: imp docs commit bfa57d9f21e3326baafd3a52e91d54396d8e03fa Author: Dimitry Kolyshev Date: Wed Jun 28 15:49:40 2023 +0400 home: log conf commit 49e06dca9dc2ceb2647b7e36dac145ccf78a6c3f Author: Dimitry Kolyshev Date: Wed Jun 28 15:41:02 2023 +0400 home: log conf commit 9be432dea7cec8ae0c0d3ee1f73c58c76376d07e Author: Dimitry Kolyshev Date: Wed Jun 28 10:45:01 2023 +0400 home: log conf --- CHANGELOG.md | 34 +++++++++++ internal/home/config.go | 27 +++++---- internal/home/home.go | 15 ++--- internal/home/upgrade.go | 107 +++++++++++++++++++++++++++++++++- internal/home/upgrade_test.go | 73 +++++++++++++++++++++++ 5 files changed, 234 insertions(+), 22 deletions(-) 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) + }) + } +}