From f3b0e4da724b2e09eaf28cc6eb49e3b1af52d497 Mon Sep 17 00:00:00 2001 From: Benjamin Edwards Date: Fri, 24 Feb 2023 07:44:56 -0500 Subject: [PATCH] add configuration parameters for filesystem logging file rotation (#10048) --- ...logging-destination-rotation-configuration | 1 + cmd/fleetctl/get_test.go | 24 ++++++++-- docs/Deploying/Configuration.md | 44 +++++++++++++++++++ server/config/config.go | 10 +++++ server/logging/filesystem.go | 15 +++---- server/logging/filesystem_test.go | 8 ++-- server/logging/logging.go | 6 +++ server/service/service_appconfig_test.go | 1 + server/service/testing_utils.go | 7 +-- 9 files changed, 95 insertions(+), 21 deletions(-) create mode 100644 changes/issue-9600-filesystem-logging-destination-rotation-configuration diff --git a/changes/issue-9600-filesystem-logging-destination-rotation-configuration b/changes/issue-9600-filesystem-logging-destination-rotation-configuration new file mode 100644 index 000000000..2d62e38fe --- /dev/null +++ b/changes/issue-9600-filesystem-logging-destination-rotation-configuration @@ -0,0 +1 @@ +* added configuration parameters for the filesystem logging destination -- max_size, max_age, and max_backups are now configurable rather than hardcoded values diff --git a/cmd/fleetctl/get_test.go b/cmd/fleetctl/get_test.go index 58a710d62..ec0fbda64 100644 --- a/cmd/fleetctl/get_test.go +++ b/cmd/fleetctl/get_test.go @@ -672,6 +672,9 @@ spec: result_log_file: /dev/null status_log_file: /dev/null audit_log_file: /dev/null + max_age: 0 + max_backups: 0 + max_size: 500 plugin: filesystem status: config: @@ -680,6 +683,9 @@ spec: result_log_file: /dev/null status_log_file: /dev/null audit_log_file: /dev/null + max_age: 0 + max_backups: 0 + max_size: 500 plugin: filesystem audit: config: @@ -688,6 +694,9 @@ spec: result_log_file: /dev/null status_log_file: /dev/null audit_log_file: /dev/null + max_age: 0 + max_backups: 0 + max_size: 500 plugin: filesystem org_info: org_logo_url: "" @@ -879,7 +888,10 @@ spec: "enable_log_rotation": false, "result_log_file": "/dev/null", "status_log_file": "/dev/null", - "audit_log_file": "/dev/null" + "audit_log_file": "/dev/null", + "max_size": 500, + "max_age": 0, + "max_backups": 0 } }, "status": { @@ -889,7 +901,10 @@ spec: "enable_log_rotation": false, "result_log_file": "/dev/null", "status_log_file": "/dev/null", - "audit_log_file": "/dev/null" + "audit_log_file": "/dev/null", + "max_size": 500, + "max_age": 0, + "max_backups": 0 } }, "audit": { @@ -899,7 +914,10 @@ spec: "enable_log_rotation": false, "result_log_file": "/dev/null", "status_log_file": "/dev/null", - "audit_log_file": "/dev/null" + "audit_log_file": "/dev/null", + "max_size": 500, + "max_age": 0, + "max_backups": 0 } } } diff --git a/docs/Deploying/Configuration.md b/docs/Deploying/Configuration.md index ced3d3fc6..2b74a56aa 100644 --- a/docs/Deploying/Configuration.md +++ b/docs/Deploying/Configuration.md @@ -1346,6 +1346,50 @@ This flag will cause the rotated logs to be compressed with gzip. enable_log_compression: true ``` +##### filesystem_max_size + +This flag only has effect if `filesystem_enable_log_rotation` is set to `true`. + +Sets the maximum size in megabytes of log files before it gets rotated. + +- Default value: `500` +- Environment variable: `FLEET_FILESYSTEM_MAX_SIZE` +- Config file format: + ``` + filesystem: + max_size: 100 + ``` + +##### filesystem_max_age + +This flag only has effect if `filesystem_enable_log_rotation` is set to `true`. + +Sets the maximum age in days to retain old log files before deletion. Setting this +to zero will retain all logs. + +- Default value: `28` +- Environment variable: `FLEET_FILESYSTEM_MAX_AGE` +- Config file format: + ``` + filesystem: + max_age: 0 + ``` + +##### filesystem_max_backups + +This flag only has effect if `filesystem_enable_log_rotation` is set to `true`. + +Sets the maximum number of old files to retain before deletion. Setting this +to zero will retain all logs. _Note_ max_age may still cause them to be deleted. + +- Default value: `3` +- Environment variable: `FLEET_FILESYSTEM_MAX_BACKUPS` +- Config file format: + ``` + filesystem: + max_backups: 0 + ``` + ##### Example YAML ```yaml diff --git a/server/config/config.go b/server/config/config.go index d37cfd120..15dda0e0d 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -298,6 +298,9 @@ type FilesystemConfig struct { AuditLogFile string `json:"audit_log_file" yaml:"audit_log_file"` EnableLogRotation bool `json:"enable_log_rotation" yaml:"enable_log_rotation"` EnableLogCompression bool `json:"enable_log_compression" yaml:"enable_log_compression"` + MaxSize int `json:"max_size" yaml:"max_size"` + MaxAge int `json:"max_age" yaml:"max_age"` + MaxBackups int `json:"max_backups" yaml:"max_backups"` } // KafkaRESTConfig defines configs for the Kafka REST Proxy logging plugin. @@ -943,6 +946,9 @@ func (man Manager) addConfigs() { "Enable automatic rotation for osquery log files") man.addConfigBool("filesystem.enable_log_compression", false, "Enable compression for the rotated osquery log files") + man.addConfigInt("filesystem.max_size", 500, "Maximum size in megabytes log files will grow until rotated (only valid if enable_log_rotation is true) default is 500MB") + man.addConfigInt("filesystem.max_age", 28, "Maximum number of days to retain old log files based on the timestamp encoded in their filename. Setting to zero wil retain old log files indefinitely (only valid if enable_log_rotation is true) default is 28 days") + man.addConfigInt("filesystem.max_backups", 3, "Maximum number of old log files to retain. Setting to zero will retain all old log files (only valid if enable_log_rotation is true) default is 3") // KafkaREST man.addConfigString("kafkarest.status_topic", "", "Kafka REST topic for status logs") @@ -1223,6 +1229,9 @@ func (man Manager) LoadConfig() FleetConfig { AuditLogFile: man.getConfigString("filesystem.audit_log_file"), EnableLogRotation: man.getConfigBool("filesystem.enable_log_rotation"), EnableLogCompression: man.getConfigBool("filesystem.enable_log_compression"), + MaxSize: man.getConfigInt("filesystem.max_size"), + MaxAge: man.getConfigInt("filesystem.max_age"), + MaxBackups: man.getConfigInt("filesystem.max_backups"), }, KafkaREST: KafkaRESTConfig{ StatusTopic: man.getConfigString("kafkarest.status_topic"), @@ -1619,6 +1628,7 @@ func TestConfig() FleetConfig { StatusLogFile: testLogFile, ResultLogFile: testLogFile, AuditLogFile: testLogFile, + MaxSize: 500, }, } } diff --git a/server/logging/filesystem.go b/server/logging/filesystem.go index 3d4016d34..2ce7c7de3 100644 --- a/server/logging/filesystem.go +++ b/server/logging/filesystem.go @@ -6,16 +6,15 @@ import ( "encoding/json" "errors" "fmt" + "github.com/fleetdm/fleet/v4/pkg/secure" + "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" + lumberjack "gopkg.in/natefinch/lumberjack.v2" "io" "os" "os/signal" "sync" "syscall" - "github.com/fleetdm/fleet/v4/pkg/secure" - "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" - lumberjack "gopkg.in/natefinch/lumberjack.v2" - "github.com/go-kit/kit/log" ) @@ -29,7 +28,7 @@ type filesystemLogWriter struct { // enableRotation is true // // The enableCompression argument is only used when enableRotation is true. -func NewFilesystemLogWriter(path string, appLogger log.Logger, enableRotation bool, enableCompression bool) (*filesystemLogWriter, error) { +func NewFilesystemLogWriter(path string, appLogger log.Logger, enableRotation, enableCompression bool, maxSize, maxAge, maxBackups int) (*filesystemLogWriter, error) { // Fail early if the process does not have the necessary // permissions to open the file at path. file, err := openFile(path) @@ -46,9 +45,9 @@ func NewFilesystemLogWriter(path string, appLogger log.Logger, enableRotation bo file.Close() fsLogger := &lumberjack.Logger{ Filename: path, - MaxSize: 500, // megabytes - MaxBackups: 3, - MaxAge: 28, // days + MaxSize: maxSize, // megabytes + MaxBackups: maxBackups, + MaxAge: maxAge, // days Compress: enableCompression, } appLogger = log.With(appLogger, "component", "filesystem-logger") diff --git a/server/logging/filesystem_test.go b/server/logging/filesystem_test.go index 43f2a3e93..9146d9af5 100644 --- a/server/logging/filesystem_test.go +++ b/server/logging/filesystem_test.go @@ -20,7 +20,7 @@ func TestFilesystemLogger(t *testing.T) { tempPath := t.TempDir() require.NoError(t, os.Chmod(tempPath, 0o755)) fileName := filepath.Join(tempPath, "filesystemLogWriter") - lgr, err := NewFilesystemLogWriter(fileName, log.NewNopLogger(), false, false) + lgr, err := NewFilesystemLogWriter(fileName, log.NewNopLogger(), false, false, 500, 28, 3) require.Nil(t, err) defer os.Remove(fileName) @@ -73,7 +73,7 @@ func TestFilesystemLoggerPermission(t *testing.T) { {name: "without-rotation", rotation: false}, } { t.Run(tc.name, func(t *testing.T) { - _, err := NewFilesystemLogWriter(fileName, log.NewNopLogger(), tc.rotation, false) + _, err := NewFilesystemLogWriter(fileName, log.NewNopLogger(), tc.rotation, false, 500, 28, 3) require.Error(t, err) require.True(t, errors.Is(err, fs.ErrPermission), err) }) @@ -83,7 +83,7 @@ func TestFilesystemLoggerPermission(t *testing.T) { func BenchmarkFilesystemLogger(b *testing.B) { ctx := context.Background() fileName := filepath.Join(b.TempDir(), "filesystemLogWriter") - lgr, err := NewFilesystemLogWriter(fileName, log.NewNopLogger(), false, false) + lgr, err := NewFilesystemLogWriter(fileName, log.NewNopLogger(), false, false, 500, 28, 3) if err != nil { b.Fatal("new failed ", err) } @@ -119,7 +119,7 @@ func BenchmarkLumberjackWithCompression(b *testing.B) { func benchLumberjack(b *testing.B, compression bool) { ctx := context.Background() fileName := filepath.Join(b.TempDir(), "lumberjack") - lgr, err := NewFilesystemLogWriter(fileName, log.NewNopLogger(), true, compression) + lgr, err := NewFilesystemLogWriter(fileName, log.NewNopLogger(), true, compression, 500, 28, 3) if err != nil { b.Fatal("new failed ", err) } diff --git a/server/logging/logging.go b/server/logging/logging.go index 19c72f504..ed087e208 100644 --- a/server/logging/logging.go +++ b/server/logging/logging.go @@ -14,6 +14,9 @@ type FilesystemConfig struct { EnableLogRotation bool EnableLogCompression bool + MaxSize int + MaxAge int + MaxBackups int } type FirehoseConfig struct { @@ -86,6 +89,9 @@ func NewJSONLogger(name string, config Config, logger log.Logger) (fleet.JSONLog logger, config.Filesystem.EnableLogRotation, config.Filesystem.EnableLogCompression, + config.Filesystem.MaxSize, + config.Filesystem.MaxAge, + config.Filesystem.MaxBackups, ) if err != nil { return nil, fmt.Errorf("create filesystem %s logger: %w", name, err) diff --git a/server/service/service_appconfig_test.go b/server/service/service_appconfig_test.go index 7645e23a7..ee2f6dbca 100644 --- a/server/service/service_appconfig_test.go +++ b/server/service/service_appconfig_test.go @@ -158,6 +158,7 @@ func TestService_LoggingConfig(t *testing.T) { AuditLogFile: logFile, EnableLogRotation: false, EnableLogCompression: false, + MaxSize: 500, }} firehoseConfig := fleet.FirehoseConfig{ diff --git a/server/service/testing_utils.go b/server/service/testing_utils.go index a1d32d489..5a800be01 100644 --- a/server/service/testing_utils.go +++ b/server/service/testing_utils.go @@ -45,12 +45,7 @@ func newTestService(t *testing.T, ds fleet.Datastore, rs fleet.QueryResultStore, func newTestServiceWithConfig(t *testing.T, ds fleet.Datastore, fleetConfig config.FleetConfig, rs fleet.QueryResultStore, lq fleet.LiveQueryStore, opts ...*TestServerOpts) (fleet.Service, context.Context) { mailer := &mockMailService{SendEmailFn: func(e fleet.Email) error { return nil }} lic := &fleet.LicenseInfo{Tier: fleet.TierFree} - writer, err := logging.NewFilesystemLogWriter( - fleetConfig.Filesystem.StatusLogFile, - kitlog.NewNopLogger(), - fleetConfig.Filesystem.EnableLogRotation, - fleetConfig.Filesystem.EnableLogCompression, - ) + writer, err := logging.NewFilesystemLogWriter(fleetConfig.Filesystem.StatusLogFile, kitlog.NewNopLogger(), fleetConfig.Filesystem.EnableLogRotation, fleetConfig.Filesystem.EnableLogCompression, 500, 28, 3) require.NoError(t, err) osqlogger := &OsqueryLogger{Status: writer, Result: writer}