diff --git a/internal/cmd/grpc.go b/internal/cmd/grpc.go index 8ec17a83d2..cba42b706e 100644 --- a/internal/cmd/grpc.go +++ b/internal/cmd/grpc.go @@ -26,7 +26,7 @@ import ( "go.flipt.io/flipt/internal/server/analytics/clickhouse" "go.flipt.io/flipt/internal/server/audit" "go.flipt.io/flipt/internal/server/audit/cloud" - "go.flipt.io/flipt/internal/server/audit/logfile" + "go.flipt.io/flipt/internal/server/audit/log" "go.flipt.io/flipt/internal/server/audit/template" "go.flipt.io/flipt/internal/server/audit/webhook" authmiddlewaregrpc "go.flipt.io/flipt/internal/server/authn/middleware/grpc" @@ -350,10 +350,10 @@ func NewGRPCServer( // audit sinks configuration sinks := make([]audit.Sink, 0) - if cfg.Audit.Sinks.LogFile.Enabled { - logFileSink, err := logfile.NewSink(logger, cfg.Audit.Sinks.LogFile.File) + if cfg.Audit.Sinks.Log.Enabled { + logFileSink, err := log.NewSink(logger, cfg.Audit.Sinks.Log.File) if err != nil { - return nil, fmt.Errorf("opening file at path: %s", cfg.Audit.Sinks.LogFile.File) + return nil, fmt.Errorf("opening file at path: %s", cfg.Audit.Sinks.Log.File) } sinks = append(sinks, logFileSink) diff --git a/internal/config/audit.go b/internal/config/audit.go index 05e5aea4f1..3768897f18 100644 --- a/internal/config/audit.go +++ b/internal/config/audit.go @@ -20,7 +20,7 @@ type AuditConfig struct { // Enabled returns true if any nested sink is enabled func (c AuditConfig) Enabled() bool { - return c.Sinks.LogFile.Enabled || c.Sinks.Webhook.Enabled || c.Sinks.Cloud.Enabled + return c.Sinks.Log.Enabled || c.Sinks.Webhook.Enabled || c.Sinks.Cloud.Enabled } func (c AuditConfig) IsZero() bool { @@ -52,10 +52,6 @@ func (c *AuditConfig) setDefaults(v *viper.Viper) error { } func (c *AuditConfig) validate() error { - if c.Sinks.LogFile.Enabled && c.Sinks.LogFile.File == "" { - return errors.New("file not specified") - } - if c.Sinks.Webhook.Enabled { if c.Sinks.Webhook.URL == "" && len(c.Sinks.Webhook.Templates) == 0 { return errors.New("url or template(s) not provided") @@ -80,7 +76,7 @@ func (c *AuditConfig) validate() error { // SinksConfig contains configuration held in structures for the different sinks // that we will send audits to. type SinksConfig struct { - LogFile LogFileSinkConfig `json:"log,omitempty" mapstructure:"log" yaml:"log,omitempty"` + Log LogSinkConfig `json:"log,omitempty" mapstructure:"log" yaml:"log,omitempty"` Webhook WebhookSinkConfig `json:"webhook,omitempty" mapstructure:"webhook" yaml:"webhook,omitempty"` Cloud CloudSinkConfig `json:"cloud,omitempty" mapstructure:"cloud" yaml:"cloud,omitempty"` } @@ -99,9 +95,9 @@ type WebhookSinkConfig struct { Templates []WebhookTemplate `json:"templates,omitempty" mapstructure:"templates" yaml:"templates,omitempty"` } -// LogFileSinkConfig contains fields that hold configuration for sending audits -// to a log file. -type LogFileSinkConfig struct { +// LogSinkConfig contains fields that hold configuration for sending audits +// to a log. +type LogSinkConfig struct { Enabled bool `json:"enabled,omitempty" mapstructure:"enabled" yaml:"enabled,omitempty"` File string `json:"file,omitempty" mapstructure:"file" yaml:"file,omitempty"` } diff --git a/internal/config/config.go b/internal/config/config.go index ee129d0b4a..afa6f6fb72 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -607,7 +607,7 @@ func Default() *Config { Audit: AuditConfig{ Sinks: SinksConfig{ - LogFile: LogFileSinkConfig{ + Log: LogSinkConfig{ Enabled: false, File: "", }, diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c6b62a8742..a7d850bd79 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -643,7 +643,7 @@ func TestLoad(t *testing.T) { cfg.Audit = AuditConfig{ Sinks: SinksConfig{ - LogFile: LogFileSinkConfig{ + Log: LogSinkConfig{ Enabled: true, File: "/path/to/logs.txt", }, @@ -827,9 +827,13 @@ func TestLoad(t *testing.T) { wantErr: errors.New("flush period below 2 minutes or greater than 5 minutes"), }, { - name: "file not specified", - path: "./testdata/audit/invalid_enable_without_file.yml", - wantErr: errors.New("file not specified"), + name: "file not specified", + path: "./testdata/audit/enable_without_file.yml", + expected: func() *Config { + cfg := Default() + cfg.Audit.Sinks.Log.Enabled = true + return cfg + }, }, { name: "url or template not specified", diff --git a/internal/config/testdata/audit/invalid_enable_without_file.yml b/internal/config/testdata/audit/enable_without_file.yml similarity index 100% rename from internal/config/testdata/audit/invalid_enable_without_file.yml rename to internal/config/testdata/audit/enable_without_file.yml diff --git a/internal/server/audit/logfile/logfile.go b/internal/server/audit/log/log.go similarity index 69% rename from internal/server/audit/logfile/logfile.go rename to internal/server/audit/log/log.go index 8320997cbd..d78412c9fb 100644 --- a/internal/server/audit/logfile/logfile.go +++ b/internal/server/audit/log/log.go @@ -1,4 +1,4 @@ -package logfile +package log import ( "context" @@ -42,12 +42,12 @@ func (osFS) MkdirAll(path string, perm os.FileMode) error { return os.MkdirAll(path, perm) } -const sinkType = "logfile" +const sinkType = "log" -// Sink is the structure in charge of sending Audits to a specified file location. +// Sink is the structure in charge of sending audit events to a specified file location. type Sink struct { logger *zap.Logger - file file + f file mtx sync.Mutex enc *json.Encoder } @@ -59,26 +59,33 @@ func NewSink(logger *zap.Logger, path string) (audit.Sink, error) { // newSink is the constructor for a Sink visible for testing. func newSink(logger *zap.Logger, path string, fs filesystem) (audit.Sink, error) { - // check if path exists, if not create it - dir := filepath.Dir(path) - if _, err := fs.Stat(dir); err != nil { - if !os.IsNotExist(err) { - return nil, fmt.Errorf("checking log directory: %w", err) + var f file + + if path == "" { + f = os.Stdout + } else { + // check if path exists, if not create it + dir := filepath.Dir(path) + if _, err := fs.Stat(dir); err != nil { + if !os.IsNotExist(err) { + return nil, fmt.Errorf("checking log directory: %w", err) + } + + if err := fs.MkdirAll(dir, 0755); err != nil { + return nil, fmt.Errorf("creating log directory: %w", err) + } } - if err := fs.MkdirAll(dir, 0755); err != nil { - return nil, fmt.Errorf("creating log directory: %w", err) + var err error + f, err = fs.OpenFile(path, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0666) + if err != nil { + return nil, fmt.Errorf("opening log file: %w", err) } } - f, err := fs.OpenFile(path, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0666) - if err != nil { - return nil, fmt.Errorf("opening log file: %w", err) - } - return &Sink{ logger: logger, - file: f, + f: f, enc: json.NewEncoder(f), }, nil } @@ -91,7 +98,7 @@ func (l *Sink) SendAudits(ctx context.Context, events []audit.Event) error { for _, e := range events { err := l.enc.Encode(e) if err != nil { - l.logger.Error("failed to write audit event to file", zap.String("file", l.file.Name()), zap.Error(err)) + l.logger.Error("failed to write audit event", zap.String("file", l.f.Name()), zap.Error(err)) result = multierror.Append(result, err) } } @@ -102,7 +109,7 @@ func (l *Sink) SendAudits(ctx context.Context, events []audit.Event) error { func (l *Sink) Close() error { l.mtx.Lock() defer l.mtx.Unlock() - return l.file.Close() + return l.f.Close() } func (l *Sink) String() string { diff --git a/internal/server/audit/logfile/logfile_test.go b/internal/server/audit/log/log_test.go similarity index 96% rename from internal/server/audit/logfile/logfile_test.go rename to internal/server/audit/log/log_test.go index 515638dabe..641feb3bc6 100644 --- a/internal/server/audit/logfile/logfile_test.go +++ b/internal/server/audit/log/log_test.go @@ -1,4 +1,4 @@ -package logfile +package log import ( "bytes" @@ -29,7 +29,7 @@ func TestNewSink_NewFile(t *testing.T) { require.NoError(t, err) require.NotNil(t, sink) - assert.Equal(t, "logfile", sink.String()) + assert.Equal(t, "log", sink.String()) require.NoError(t, sink.Close()) } @@ -51,7 +51,7 @@ func TestNewSink_ExistingFile(t *testing.T) { require.NoError(t, err) require.NotNil(t, sink) - assert.Equal(t, "logfile", sink.String()) + assert.Equal(t, "log", sink.String()) require.NoError(t, sink.Close()) } @@ -86,7 +86,7 @@ func TestNewSink_DirNotExists(t *testing.T) { require.NoError(t, err) require.NotNil(t, sink) - assert.Equal(t, "logfile", sink.String()) + assert.Equal(t, "log", sink.String()) require.NoError(t, sink.Close()) }) diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go index 4d8516e5da..d40e96cb96 100644 --- a/internal/telemetry/telemetry.go +++ b/internal/telemetry/telemetry.go @@ -238,7 +238,7 @@ func (r *Reporter) ping(_ context.Context, f file) error { auditSinks := []string{} if r.cfg.Audit.Enabled() { - if r.cfg.Audit.Sinks.LogFile.Enabled { + if r.cfg.Audit.Sinks.Log.Enabled { auditSinks = append(auditSinks, "log") } if r.cfg.Audit.Sinks.Webhook.Enabled { diff --git a/internal/telemetry/telemetry_test.go b/internal/telemetry/telemetry_test.go index b84da4401b..800da1343d 100644 --- a/internal/telemetry/telemetry_test.go +++ b/internal/telemetry/telemetry_test.go @@ -253,7 +253,7 @@ func TestPing(t *testing.T) { }, Audit: config.AuditConfig{ Sinks: config.SinksConfig{ - LogFile: config.LogFileSinkConfig{ + Log: config.LogSinkConfig{ Enabled: false, }, }, @@ -277,7 +277,7 @@ func TestPing(t *testing.T) { }, Audit: config.AuditConfig{ Sinks: config.SinksConfig{ - LogFile: config.LogFileSinkConfig{ + Log: config.LogSinkConfig{ Enabled: true, }, }, @@ -359,7 +359,7 @@ func TestPing(t *testing.T) { }, Audit: config.AuditConfig{ Sinks: config.SinksConfig{ - LogFile: config.LogFileSinkConfig{ + Log: config.LogSinkConfig{ Enabled: true, }, Webhook: config.WebhookSinkConfig{