From 946e3e9b41c8d73eda53456618e5cc1f26582cdb Mon Sep 17 00:00:00 2001 From: Idan Novogroder <43949240+idanovo@users.noreply.github.com> Date: Sun, 19 Jun 2022 11:35:55 +0300 Subject: [PATCH] Adding a configuration for setting audit logs level (#3512) * Adding a configuration for setting audit logs level --- cmd/lakefs/cmd/run.go | 1 + docs/reference/configuration.md | 7 ++++--- pkg/api/serve.go | 1 + pkg/config/config.go | 6 ++++++ pkg/config/logger.go | 1 + pkg/config/template.go | 1 + pkg/gateway/handler.go | 3 ++- pkg/gateway/testutil/gateway_setup.go | 2 +- pkg/httputil/logging.go | 24 ++++++++++++++++++------ pkg/logging/dummy.go | 12 +++++++++++- pkg/logging/logger.go | 12 ++++++++++++ pkg/version/audit_test.go | 16 +++++++++++++--- 12 files changed, 71 insertions(+), 15 deletions(-) diff --git a/cmd/lakefs/cmd/run.go b/cmd/lakefs/cmd/run.go index 0862209f01a..98898605583 100644 --- a/cmd/lakefs/cmd/run.go +++ b/cmd/lakefs/cmd/run.go @@ -300,6 +300,7 @@ var runCmd = &cobra.Command{ cfg.GetS3GatewayDomainNames(), bufferedCollector, s3FallbackURL, + cfg.GetAuditLogLevel(), cfg.GetLoggingTraceRequestHeaders(), ) ctx, cancelFn := context.WithCancel(cmd.Context()) diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 326c0a87897..d9f44acfa16 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -13,7 +13,7 @@ has_children: false {% include toc.html %} Configuring lakeFS is done using a yaml configuration file and/or environment variable. -The configuration file location can be set with the '--config' flag. If not specified, the the first file found in the following order will be used: +The configuration file location can be set with the '--config' flag. If not specified, the first file found in the following order will be used: 1. ./config.yaml 1. `$HOME`/lakefs/config.yaml 1. /etc/lakefs/config.yaml @@ -28,9 +28,10 @@ This reference uses `.` to denote the nesting of values. * `logging.format` `(one of ["json", "text"] : "text")` - Format to output log message in * `logging.level` `(one of ["TRACE", "DEBUG", "INFO", "WARN", "ERROR", "NONE"] : "DEBUG")` - Logging level to output +* `logging.audit_log_level` `(one of ["TRACE", "DEBUG", "INFO", "WARN", "ERROR", "NONE"] : "DEBUG")` - Audit logs level to output. **Please notice that in case you configure this field to be lower than the main logger level, you won't be able to get the audit logs** * `logging.output` `(string : "-")` - A path or paths to write logs to. A `-` means the standard output, `=` means the standard error. * `logging.file_max_size_mb` `(int : 100)` - Output file maximum size in megabytes. -* `logging.files_keep` `(int : 0)` - Numbe of log files to keep, default is all. +* `logging.files_keep` `(int : 0)` - Number of log files to keep, default is all. * `actions.enabled` `(bool : true)` - Setting this to false will block hooks from being executed * `database.connection_string` `(string : "postgres://localhost:5432/postgres?sslmode=disable")` - PostgreSQL connection string to use * `database.max_open_connections` `(int : 25)` - Maximum number of open connections to the database @@ -121,7 +122,7 @@ This reference uses `.` to denote the nesting of values. local development, if using [virtual-host addressing](https://docs.aws.amazon.com/AmazonS3/latest/userguide/VirtualHosting.html). * `gateways.s3.region` `(string : "us-east-1")` - AWS region we're pretending to be. Should match the region configuration used in AWS SDK clients * `gateways.s3.fallback_url` `(string)` - If specified, requests with a non-existing repository will be forwarded to this url. This can be useful for using lakeFS side-by-side with S3, with the URL pointing at an [S3Proxy](https://github.com/gaul/s3proxy) instance. -* `stats.enabled` `(boolean : true)` - Whether or not to periodically collect anonymous usage statistics +* `stats.enabled` `(boolean : true)` - Whether to periodically collect anonymous usage statistics * `security.audit_check_interval` `(duration : 12h)` - Duration in which we check for security audit {: .ref-list } diff --git a/pkg/api/serve.go b/pkg/api/serve.go index 263ababd9d3..9a4c94fbdc2 100644 --- a/pkg/api/serve.go +++ b/pkg/api/serve.go @@ -70,6 +70,7 @@ func Serve( httputil.LoggingMiddleware( RequestIDHeaderName, logging.Fields{logging.ServiceNameFieldKey: LoggerServiceName}, + cfg.GetAuditLogLevel(), cfg.GetLoggingTraceRequestHeaders()), AuthMiddleware(logger, swagger, middlewareAuthenticator, authService), MetricsMiddleware(swagger), diff --git a/pkg/config/config.go b/pkg/config/config.go index df7cfeda0e2..9a45475559f 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -119,6 +119,7 @@ const ( LoggingOutputKey = "logging.output" LoggingFileMaxSizeMBKey = "logging.file_max_size_mb" LoggingFilesKeepKey = "logging.files_keep" + LoggingAuditLogLevel = "logging.audit_log_level" ActionsEnabledKey = "actions.enabled" @@ -179,6 +180,7 @@ func setDefaults() { viper.SetDefault(LoggingLevelKey, DefaultLoggingLevel) viper.SetDefault(LoggingOutputKey, DefaultLoggingOutput) viper.SetDefault(LoggingFilesKeepKey, DefaultLoggingFilesKeepKey) + viper.SetDefault(LoggingAuditLogLevel, DefaultAuditLogLevel) viper.SetDefault(ActionsEnabledKey, DefaultActionsEnabled) @@ -488,6 +490,10 @@ func (c *Config) GetLoggingTraceRequestHeaders() bool { return c.values.Logging.TraceRequestHeaders } +func (c *Config) GetAuditLogLevel() string { + return c.values.Logging.AuditLogLevel +} + func (c *Config) GetSecurityAuditCheckInterval() time.Duration { return c.values.Security.AuditCheckInterval } diff --git a/pkg/config/logger.go b/pkg/config/logger.go index 207ff7d0d35..003d4906948 100644 --- a/pkg/config/logger.go +++ b/pkg/config/logger.go @@ -10,6 +10,7 @@ const ( DefaultLoggingLevel = "INFO" DefaultLoggingOutput = "-" DefaultLoggingFilesKeepKey = 100 + DefaultAuditLogLevel = "DEBUG" ) func setupLogger() { diff --git a/pkg/config/template.go b/pkg/config/template.go index 36706073fc4..1e967e8c641 100644 --- a/pkg/config/template.go +++ b/pkg/config/template.go @@ -47,6 +47,7 @@ type configuration struct { Output []string `mapstructure:"output"` FileMaxSizeMB int `mapstructure:"file_max_size_mb"` FilesKeep int `mapstructure:"files_keep"` + AuditLogLevel string `mapstructure:"audit_log_level"` // TraceRequestHeaders work only on 'trace' level, default is false as it may log sensitive data to the log TraceRequestHeaders bool `mapstructure:"trace_request_headers"` } diff --git a/pkg/gateway/handler.go b/pkg/gateway/handler.go index f58db054686..f89e73a7e45 100644 --- a/pkg/gateway/handler.go +++ b/pkg/gateway/handler.go @@ -58,7 +58,7 @@ type ServerContext struct { stats stats.Collector } -func NewHandler(region string, catalog catalog.Interface, multipartsTracker multiparts.Tracker, blockStore block.Adapter, authService auth.GatewayService, bareDomains []string, stats stats.Collector, fallbackURL *url.URL, traceRequestHeaders bool) http.Handler { +func NewHandler(region string, catalog catalog.Interface, multipartsTracker multiparts.Tracker, blockStore block.Adapter, authService auth.GatewayService, bareDomains []string, stats stats.Collector, fallbackURL *url.URL, auditLogLevel string, traceRequestHeaders bool) http.Handler { var fallbackHandler http.Handler if fallbackURL != nil { fallbackProxy := gohttputil.NewSingleHostReverseProxy(fallbackURL) @@ -105,6 +105,7 @@ func NewHandler(region string, catalog catalog.Interface, multipartsTracker mult loggingMiddleware := httputil.LoggingMiddleware( "X-Amz-Request-Id", logging.Fields{"service_name": "s3_gateway"}, + auditLogLevel, traceRequestHeaders) h = loggingMiddleware(h) diff --git a/pkg/gateway/testutil/gateway_setup.go b/pkg/gateway/testutil/gateway_setup.go index 309f6d2e127..920e21ca115 100644 --- a/pkg/gateway/testutil/gateway_setup.go +++ b/pkg/gateway/testutil/gateway_setup.go @@ -73,7 +73,7 @@ func GetBasicHandler(t *testing.T, authService *FakeAuthService, databaseURI str _, err = c.CreateRepository(ctx, repoName, storageNamespace, "main") testutil.Must(t, err) - handler := gateway.NewHandler(authService.Region, c, multipartsTracker, blockAdapter, authService, []string{authService.BareDomain}, &mockCollector{}, nil, true) + handler := gateway.NewHandler(authService.Region, c, multipartsTracker, blockAdapter, authService, []string{authService.BareDomain}, &mockCollector{}, nil, config.DefaultAuditLogLevel, true) return handler, &Dependencies{ blocks: blockAdapter, diff --git a/pkg/httputil/logging.go b/pkg/httputil/logging.go index 38489eee3fe..c211e20b4fe 100644 --- a/pkg/httputil/logging.go +++ b/pkg/httputil/logging.go @@ -3,9 +3,11 @@ package httputil import ( "context" "net/http" + "strings" "time" "github.com/google/uuid" + "github.com/sirupsen/logrus" "github.com/treeverse/lakefs/pkg/logging" ) @@ -13,6 +15,7 @@ type contextKey string const ( RequestIDContextKey contextKey = "request_id" + AuditLogEndMessage string = "HTTP call ended" ) type ResponseRecordingWriter struct { @@ -50,7 +53,7 @@ func RequestID(r *http.Request) (*http.Request, string) { return r, reqID } -func DebugLoggingMiddleware(requestIDHeaderName string, fields logging.Fields) func(next http.Handler) http.Handler { +func DefaultLoggingMiddleware(requestIDHeaderName string, fields logging.Fields, middlewareLogLevel string) func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { startTime := time.Now() @@ -63,6 +66,7 @@ func DebugLoggingMiddleware(requestIDHeaderName string, fields logging.Fields) f logging.MethodFieldKey: r.Method, logging.HostFieldKey: r.Host, logging.RequestIDFieldKey: reqID, + logging.LogAudit: "API", } for k, v := range fields { requestFields[k] = v @@ -71,18 +75,26 @@ func DebugLoggingMiddleware(requestIDHeaderName string, fields logging.Fields) f writer.Header().Set(requestIDHeaderName, reqID) next.ServeHTTP(writer, r) // handle the request - logging.FromContext(r.Context()).WithFields(logging.Fields{ + loggingFields := logging.Fields{ "took": time.Since(startTime), "status_code": writer.StatusCode, "sent_bytes": writer.ResponseSize, - }).Debug("HTTP call ended") + } + + logLevel := strings.ToLower(middlewareLogLevel) + if logLevel == "null" || logLevel == "none" { + logging.FromContext(r.Context()).WithFields(loggingFields).Debug(AuditLogEndMessage) + } else { + level, _ := logrus.ParseLevel(logLevel) + logging.FromContext(r.Context()).WithFields(loggingFields).Log(level, AuditLogEndMessage) + } }) } } -func LoggingMiddleware(requestIDHeaderName string, fields logging.Fields, traceRequestHeaders bool) func(next http.Handler) http.Handler { - if logging.Level() == "trace" { +func LoggingMiddleware(requestIDHeaderName string, fields logging.Fields, loggingMiddlewareLevel string, traceRequestHeaders bool) func(next http.Handler) http.Handler { + if strings.ToLower(loggingMiddlewareLevel) == "trace" { return TracingMiddleware(requestIDHeaderName, fields, traceRequestHeaders) } - return DebugLoggingMiddleware(requestIDHeaderName, fields) + return DefaultLoggingMiddleware(requestIDHeaderName, fields, loggingMiddlewareLevel) } diff --git a/pkg/logging/dummy.go b/pkg/logging/dummy.go index 0a6b257609d..9ccb18ca133 100644 --- a/pkg/logging/dummy.go +++ b/pkg/logging/dummy.go @@ -1,6 +1,10 @@ package logging -import "context" +import ( + "context" + + "github.com/sirupsen/logrus" +) type DummyLogger struct{} @@ -52,6 +56,10 @@ func (d DummyLogger) Panic(args ...interface{}) { } +func (d DummyLogger) Log(level logrus.Level, args ...interface{}) { + +} + func (d DummyLogger) Tracef(format string, args ...interface{}) { } @@ -83,7 +91,9 @@ func (d DummyLogger) Fatalf(format string, args ...interface{}) { func (d DummyLogger) Panicf(format string, args ...interface{}) { } +func (d DummyLogger) Logf(level logrus.Level, format string, args ...interface{}) { +} func (d DummyLogger) IsTracing() bool { return true } diff --git a/pkg/logging/logger.go b/pkg/logging/logger.go index fcbc6d3121b..d9a30fd6663 100644 --- a/pkg/logging/logger.go +++ b/pkg/logging/logger.go @@ -50,6 +50,8 @@ const ( UserFieldKey = "user" // ServiceNameFieldKey service name (string, ex: rest_api) ServiceNameFieldKey = "service_name" + // LogAudit kind of information to audit (string, ex: API) + LogAudit = "log_audit" ) var ( @@ -160,6 +162,7 @@ type Logger interface { Error(args ...interface{}) Fatal(args ...interface{}) Panic(args ...interface{}) + Log(level logrus.Level, args ...interface{}) Tracef(format string, args ...interface{}) Debugf(format string, args ...interface{}) Infof(format string, args ...interface{}) @@ -168,6 +171,7 @@ type Logger interface { Errorf(format string, args ...interface{}) Fatalf(format string, args ...interface{}) Panicf(format string, args ...interface{}) + Logf(level logrus.Level, format string, args ...interface{}) IsTracing() bool } @@ -226,6 +230,10 @@ func (l logrusEntryWrapper) Panic(args ...interface{}) { l.e.Panic(args...) } +func (l logrusEntryWrapper) Log(level logrus.Level, args ...interface{}) { + l.e.Log(level, args...) +} + func (l *logrusEntryWrapper) Tracef(format string, args ...interface{}) { l.e.Tracef(format, args...) } @@ -258,6 +266,10 @@ func (l *logrusEntryWrapper) Panicf(format string, args ...interface{}) { l.e.Panicf(format, args...) } +func (l logrusEntryWrapper) Logf(level logrus.Level, format string, args ...interface{}) { + l.e.Logf(level, format, args...) +} + func (*logrusEntryWrapper) IsTracing() bool { return logrus.IsLevelEnabled(logrus.TraceLevel) } diff --git a/pkg/version/audit_test.go b/pkg/version/audit_test.go index b9ce6ab96ad..275cf45e9a6 100644 --- a/pkg/version/audit_test.go +++ b/pkg/version/audit_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/go-test/deep" + "github.com/sirupsen/logrus" "github.com/treeverse/lakefs/pkg/logging" ) @@ -19,7 +20,7 @@ type LogLine struct { Msg string } type MemLogger struct { - Log []*LogLine + log []*LogLine fields logging.Fields } @@ -81,6 +82,11 @@ func (m *MemLogger) Panic(args ...interface{}) { m.logLine("PANIC", args...) } +func (m *MemLogger) Log(level logrus.Level, args ...interface{}) { + m.logLine(level.String(), args...) + +} + func (m *MemLogger) Tracef(format string, args ...interface{}) { m.logLine("TRACE", fmt.Sprintf(format, args...)) } @@ -113,12 +119,16 @@ func (m *MemLogger) Panicf(format string, args ...interface{}) { m.logLine("PANIC", fmt.Sprintf(format, args...)) } +func (m *MemLogger) Logf(level logrus.Level, format string, args ...interface{}) { + m.logLine(level.String(), fmt.Sprintf(format, args...)) +} + func (m *MemLogger) IsTracing() bool { return true } func (m *MemLogger) logLine(level string, args ...interface{}) { - m.Log = append(m.Log, &LogLine{ + m.log = append(m.log, &LogLine{ Fields: m.fields, Level: level, Msg: fmt.Sprint(args...), @@ -236,7 +246,7 @@ func TestAuditChecker_CheckAndLog(t *testing.T) { checker.CheckAndLog(ctx, memLog) // verify we logged the right information - if diff := deep.Equal(memLog.Log[0], &LogLine{ + if diff := deep.Equal(memLog.log[0], &LogLine{ Fields: logging.Fields{ "id": responseAlert.ID, "affected_versions": responseAlert.AffectedVersions,