Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CBG-3975: Configuration wiring for audit logging and /db/_config APIs #6918

Merged
merged 3 commits into from
Jul 8, 2024
Merged

CBG-3975: Configuration wiring for audit logging and /db/_config APIs #6918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7058571
Wire up config for audit logging. Per-database audit log settings can…
bbrks Jun 25, 2024
0054a9e
Show correct 'enabled' state for event in /db/_config/audit
bbrks Jun 25, 2024
f7c0070
Allow rutime setting of bootstrap logging.audit.enabled from root /_c…
bbrks Jun 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions base/audit_events.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,16 @@ var AuditEvents = events{
EventType: eventTypeUser,
},
}

// DefaultAuditEventIDs is a list of audit event IDs that are enabled by default.
var DefaultAuditEventIDs = buildDefaultAuditIDList(AuditEvents)

func buildDefaultAuditIDList(e events) []uint {
var ids []uint
for id, event := range e {
if event.EnabledByDefault {
ids = append(ids, uint(id))
}
}
return ids
}
22 changes: 21 additions & 1 deletion base/logger_audit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import (
)

const (
auditLogName = "audit"
auditLogName = "audit"
defaultAuditEnabled = false
)

// commonly used fields for audit events
Expand Down Expand Up @@ -131,12 +132,28 @@ type AuditLogger struct {
config AuditLoggerConfig
}

func (l *AuditLogger) getAuditLoggerConfig() *AuditLoggerConfig {
c := AuditLoggerConfig{}
if l != nil {
// Copy config struct to avoid mutating running config
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this help a lot given that all the member of AuditLoggerConfig are pointers and this is a shallow copy?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem nessesary since there's no runtime configuration beyond the embedded FileLoggerConfig (yet), but I think I'd rather just keep this here in case we do add runtime-configurable options to auditLogger. Follows the same pattern as console and file loggers.

c = l.config
}

c.FileLoggerConfig = *l.getFileLoggerConfig()

return &c
}

// NewAuditLogger returns a new AuditLogger from a config.
func NewAuditLogger(ctx context.Context, config *AuditLoggerConfig, logFilePath string, minAge int, buffer *strings.Builder) (*AuditLogger, error) {
if config == nil {
config = &AuditLoggerConfig{}
}

if config.FileLoggerConfig.Enabled == nil {
config.FileLoggerConfig.Enabled = BoolPtr(defaultAuditEnabled)
}

fl, err := NewFileLogger(ctx, &config.FileLoggerConfig, LevelNone, auditLogName, logFilePath, minAge, buffer)
if err != nil {
return nil, err
Expand All @@ -156,6 +173,9 @@ func (al *AuditLogger) shouldLog(id AuditID, ctx context.Context) bool {
}
logCtx := getLogCtx(ctx)
if logCtx.DbLogConfig != nil && logCtx.DbLogConfig.Audit != nil {
if !logCtx.DbLogConfig.Audit.Enabled {
return false
}
if _, ok := logCtx.DbLogConfig.Audit.EnabledEvents[id]; !ok {
return false
}
Expand Down
15 changes: 10 additions & 5 deletions base/logging_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,12 @@ func EnableStatsLogger(enabled bool) {
}
}

func EnableAuditLogger(enabled bool) {
if auditLogger != nil {
auditLogger.Enabled.Set(enabled)
}
}

// === Used by tests only ===
func ErrorLoggerIsEnabled() bool {
return errorLogger.Enabled.IsTrue()
Expand Down Expand Up @@ -267,10 +273,10 @@ type AuditLoggerConfig struct {
AuditLogFilePath *string `json:"audit_log_file_path,omitempty"` // If set, overrides the output path for the audit log files
}

func BuildLoggingConfigFromLoggers(redactionLevel RedactionLevel, LogFilePath string) *LoggingConfig {
func BuildLoggingConfigFromLoggers(originalConfig LoggingConfig) *LoggingConfig {
config := LoggingConfig{
RedactionLevel: redactionLevel,
LogFilePath: LogFilePath,
RedactionLevel: originalConfig.RedactionLevel,
LogFilePath: originalConfig.LogFilePath,
}

config.Console = consoleLogger.getConsoleLoggerConfig()
Expand All @@ -280,8 +286,7 @@ func BuildLoggingConfigFromLoggers(redactionLevel RedactionLevel, LogFilePath st
config.Debug = debugLogger.getFileLoggerConfig()
config.Trace = traceLogger.getFileLoggerConfig()
config.Stats = statsLogger.getFileLoggerConfig()
// FIXME(bbrks): Once AuditLogger is implemented
config.Audit = &AuditLoggerConfig{}
config.Audit = auditLogger.getAuditLoggerConfig()

return &config
}
Expand Down
1 change: 1 addition & 0 deletions base/logging_context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ type DbConsoleLogConfig struct {

// DbAuditLogConfig can be used to customise the audit logging for events associated with this database.
type DbAuditLogConfig struct {
Enabled bool
EnabledEvents map[AuditID]struct{}
}

Expand Down
17 changes: 14 additions & 3 deletions rest/admin_api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,8 @@ func (h *handler) handleGetConfig() error {
}
}

cfg.Logging = *base.BuildLoggingConfigFromLoggers(h.server.Config.Logging.RedactionLevel, h.server.Config.Logging.LogFilePath)
// because loggers can be changed at runtime, we need to work backwards to get the config that would've created the actually running instances
cfg.Logging = *base.BuildLoggingConfigFromLoggers(h.server.Config.Logging)
cfg.Databases = databaseMap

h.writeJSON(cfg)
Expand Down Expand Up @@ -430,6 +431,7 @@ func (h *handler) handlePutConfig() error {
Debug FileLoggerPutConfig `json:"debug,omitempty"`
Trace FileLoggerPutConfig `json:"trace,omitempty"`
Stats FileLoggerPutConfig `json:"stats,omitempty"`
Audit FileLoggerPutConfig `json:"audit,omitempty"`
} `json:"logging"`
ReplicationLimit *int `json:"max_concurrent_replications,omitempty"`
}
Expand Down Expand Up @@ -483,6 +485,10 @@ func (h *handler) handlePutConfig() error {
base.EnableStatsLogger(*config.Logging.Stats.Enabled)
}

if config.Logging.Audit.Enabled != nil {
base.EnableAuditLogger(*config.Logging.Audit.Enabled)
}

if config.ReplicationLimit != nil {
if *config.ReplicationLimit < 0 {
return base.HTTPErrorf(http.StatusBadRequest, "replication limit cannot be less than 0")
Expand Down Expand Up @@ -672,6 +678,11 @@ func (h *handler) handleGetDbAuditConfig() error {
showOnlyFilterable := h.getBoolQuery("filterable")
verbose := h.getBoolQuery("verbose")

isEnabledFn := func(id base.AuditID) bool {
_, ok := h.db.Options.LoggingConfig.Audit.EnabledEvents[id]
return ok
}

// TODO: Move to structs
events := make(map[string]interface{}, len(base.AuditEvents))
for id, descriptor := range base.AuditEvents {
Expand All @@ -683,11 +694,11 @@ func (h *handler) handleGetDbAuditConfig() error {
events[idStr] = map[string]interface{}{
"name": descriptor.Name,
"description": descriptor.Description,
"enabled": descriptor.EnabledByDefault, // TODO: Switch to actual configuration
"enabled": isEnabledFn(id),
"filterable": descriptor.FilteringPermitted,
}
} else {
events[idStr] = descriptor.EnabledByDefault // TODO: Switch to actual configuration
events[idStr] = isEnabledFn(id)
}
}

Expand Down
2 changes: 1 addition & 1 deletion rest/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2174,7 +2174,7 @@ func RegisterSignalHandler(ctx context.Context) {
}()
}

// toDbLogConfig converts the console logging from a DbConfig to a DbLogConfig
// toDbLogConfig converts the logging from a DbConfig to a DbLogConfig
func (c *DbConfig) toDbLogConfig(ctx context.Context) *base.DbLogConfig {
l := c.Logging
if l == nil || (l.Console == nil && l.Audit == nil) {
Expand Down
17 changes: 11 additions & 6 deletions rest/config_database.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,22 @@ func GenerateDatabaseConfigVersionID(ctx context.Context, previousRevID string,
}

func DefaultPerDBLogging(bootstrapLoggingCnf base.LoggingConfig) *DbLoggingConfig {
dblc := &DbLoggingConfig{}
if bootstrapLoggingCnf.Console != nil {
if *bootstrapLoggingCnf.Console.Enabled {
return &DbLoggingConfig{
Console: &DbConsoleLoggingConfig{
LogLevel: bootstrapLoggingCnf.Console.LogLevel,
LogKeys: bootstrapLoggingCnf.Console.LogKeys,
},
dblc.Console = &DbConsoleLoggingConfig{
LogLevel: bootstrapLoggingCnf.Console.LogLevel,
LogKeys: bootstrapLoggingCnf.Console.LogKeys,
}
}
}
return &DbLoggingConfig{}
if bootstrapLoggingCnf.Audit != nil {
dblc.Audit = &DbAuditLoggingConfig{
Enabled: base.BoolPtr(false),
EnabledEvents: base.DefaultAuditEventIDs,
}
Comment on lines +103 to +106
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have expected these to inherit the value from the bootstrap and override if appropriate?

Is this change specific to capella, since audit logging should be enabled globally, but potentially turned off per database?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's anticipated that users will enable audit logging (globally), and then opt-into databases one at a time where required. Enabling globally then disabling subsets of databases creates unnessesary audit work/noise for the sake of one extra API call. It's expected that users will want to customise their enabled audit events anyway.

}
return dblc
}

// MergeDatabaseConfigWithDefaults merges the passed in config onto a DefaultDbConfig which results in returned value
Expand Down
Loading