From 3afcadae76cf8722b0b9c25bd8df1f3264dddd5d Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Tue, 12 May 2020 00:13:46 +0100 Subject: [PATCH 1/5] Move logging options to a struct --- pkg/apis/options/logging.go | 24 +++++++++++++++ pkg/apis/options/options.go | 60 ++++++++++++++++--------------------- pkg/validation/options.go | 40 ++++++++++++------------- 3 files changed, 69 insertions(+), 55 deletions(-) create mode 100644 pkg/apis/options/logging.go diff --git a/pkg/apis/options/logging.go b/pkg/apis/options/logging.go new file mode 100644 index 00000000..3eb6ddc5 --- /dev/null +++ b/pkg/apis/options/logging.go @@ -0,0 +1,24 @@ +package options + +// Logging contains all options required for configuring the logging +type Logging struct { + AuthEnabled bool `flag:"auth-logging" cfg:"auth_logging"` + AuthFormat string `flag:"auth-logging-format" cfg:"auth_logging_format"` + RequestEnabled bool `flag:"request-logging" cfg:"request_logging"` + RequestFormat string `flag:"request-logging-format" cfg:"request_logging_format"` + StandardEnabled bool `flag:"standard-logging" cfg:"standard_logging"` + StandardFormat string `flag:"standard-logging-format" cfg:"standard_logging_format"` + ExcludePaths string `flag:"exclude-logging-paths" cfg:"exclude_logging_paths"` + LocalTime bool `flag:"logging-local-time" cfg:"logging_local_time"` + SilencePing bool `flag:"silence-ping-logging" cfg:"silence_ping_logging"` + File LogFileOptions `cfg:",squash"` +} + +// LogFileOptions contains options for configuring logging to a file +type LogFileOptions struct { + Filename string `flag:"logging-filename" cfg:"logging_filename"` + MaxSize int `flag:"logging-max-size" cfg:"logging_max_size"` + MaxAge int `flag:"logging-max-age" cfg:"logging_max_age"` + MaxBackups int `flag:"logging-max-backups" cfg:"logging_max_backups"` + Compress bool `flag:"logging-compress" cfg:"logging_compress"` +} diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index 27b0f66c..e5e509a3 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -61,6 +61,7 @@ type Options struct { Cookie CookieOptions `cfg:",squash"` Session SessionOptions `cfg:",squash"` + Logging Logging `cfg:",squash"` Upstreams []string `flag:"upstream" cfg:"upstreams"` SkipAuthRegex []string `flag:"skip-auth-regex" cfg:"skip_auth_regex"` @@ -101,27 +102,12 @@ type Options struct { ApprovalPrompt string `flag:"approval-prompt" cfg:"approval_prompt"` // Deprecated by OIDC 1.0 UserIDClaim string `flag:"user-id-claim" cfg:"user_id_claim"` - // Configuration values for logging - LoggingFilename string `flag:"logging-filename" cfg:"logging_filename"` - LoggingMaxSize int `flag:"logging-max-size" cfg:"logging_max_size"` - LoggingMaxAge int `flag:"logging-max-age" cfg:"logging_max_age"` - LoggingMaxBackups int `flag:"logging-max-backups" cfg:"logging_max_backups"` - LoggingLocalTime bool `flag:"logging-local-time" cfg:"logging_local_time"` - LoggingCompress bool `flag:"logging-compress" cfg:"logging_compress"` - StandardLogging bool `flag:"standard-logging" cfg:"standard_logging"` - StandardLoggingFormat string `flag:"standard-logging-format" cfg:"standard_logging_format"` - RequestLogging bool `flag:"request-logging" cfg:"request_logging"` - RequestLoggingFormat string `flag:"request-logging-format" cfg:"request_logging_format"` - ExcludeLoggingPaths string `flag:"exclude-logging-paths" cfg:"exclude_logging_paths"` - SilencePingLogging bool `flag:"silence-ping-logging" cfg:"silence_ping_logging"` - AuthLogging bool `flag:"auth-logging" cfg:"auth_logging"` - AuthLoggingFormat string `flag:"auth-logging-format" cfg:"auth_logging_format"` - SignatureKey string `flag:"signature-key" cfg:"signature_key"` - AcrValues string `flag:"acr-values" cfg:"acr_values"` - JWTKey string `flag:"jwt-key" cfg:"jwt_key"` - JWTKeyFile string `flag:"jwt-key-file" cfg:"jwt_key_file"` - PubJWKURL string `flag:"pubjwk-url" cfg:"pubjwk_url"` - GCPHealthChecks bool `flag:"gcp-healthchecks" cfg:"gcp_healthchecks"` + SignatureKey string `flag:"signature-key" cfg:"signature_key"` + AcrValues string `flag:"acr-values" cfg:"acr_values"` + JWTKey string `flag:"jwt-key" cfg:"jwt_key"` + JWTKeyFile string `flag:"jwt-key-file" cfg:"jwt_key_file"` + PubJWKURL string `flag:"pubjwk-url" cfg:"pubjwk_url"` + GCPHealthChecks bool `flag:"gcp-healthchecks" cfg:"gcp_healthchecks"` // internal values that are set after config validation redirectURL *url.URL @@ -197,20 +183,24 @@ func NewOptions() *Options { UserIDClaim: "email", InsecureOIDCAllowUnverifiedEmail: false, SkipOIDCDiscovery: false, - LoggingFilename: "", - LoggingMaxSize: 100, - LoggingMaxAge: 7, - LoggingMaxBackups: 0, - LoggingLocalTime: true, - LoggingCompress: false, - ExcludeLoggingPaths: "", - SilencePingLogging: false, - StandardLogging: true, - StandardLoggingFormat: logger.DefaultStandardLoggingFormat, - RequestLogging: true, - RequestLoggingFormat: logger.DefaultRequestLoggingFormat, - AuthLogging: true, - AuthLoggingFormat: logger.DefaultAuthLoggingFormat, + Logging: Logging{ + ExcludePaths: "", + LocalTime: true, + SilencePing: false, + AuthEnabled: true, + AuthFormat: logger.DefaultAuthLoggingFormat, + RequestEnabled: true, + RequestFormat: logger.DefaultRequestLoggingFormat, + StandardEnabled: true, + StandardFormat: logger.DefaultStandardLoggingFormat, + File: LogFileOptions{ + Filename: "", + MaxSize: 100, + MaxAge: 7, + MaxBackups: 0, + Compress: false, + }, + }, } } diff --git a/pkg/validation/options.go b/pkg/validation/options.go index 761d216d..ecf6fcc7 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -455,55 +455,55 @@ func validateCookieName(o *options.Options, msgs []string) []string { func setupLogger(o *options.Options, msgs []string) []string { // Setup the log file - if len(o.LoggingFilename) > 0 { + if len(o.Logging.File.Filename) > 0 { // Validate that the file/dir can be written - file, err := os.OpenFile(o.LoggingFilename, os.O_WRONLY|os.O_CREATE, 0666) + file, err := os.OpenFile(o.Logging.File.Filename, os.O_WRONLY|os.O_CREATE, 0666) if err != nil { if os.IsPermission(err) { - return append(msgs, "unable to write to log file: "+o.LoggingFilename) + return append(msgs, "unable to write to log file: "+o.Logging.File.Filename) } } file.Close() - logger.Printf("Redirecting logging to file: %s", o.LoggingFilename) + logger.Printf("Redirecting logging to file: %s", o.Logging.File.Filename) logWriter := &lumberjack.Logger{ - Filename: o.LoggingFilename, - MaxSize: o.LoggingMaxSize, // megabytes - MaxAge: o.LoggingMaxAge, // days - MaxBackups: o.LoggingMaxBackups, - LocalTime: o.LoggingLocalTime, - Compress: o.LoggingCompress, + Filename: o.Logging.File.Filename, + MaxSize: o.Logging.File.MaxSize, // megabytes + MaxAge: o.Logging.File.MaxAge, // days + MaxBackups: o.Logging.File.MaxBackups, + LocalTime: o.Logging.LocalTime, + Compress: o.Logging.File.Compress, } logger.SetOutput(logWriter) } // Supply a sanity warning to the logger if all logging is disabled - if !o.StandardLogging && !o.AuthLogging && !o.RequestLogging { + if !o.Logging.StandardEnabled && !o.Logging.AuthEnabled && !o.Logging.RequestEnabled { logger.Print("Warning: Logging disabled. No further logs will be shown.") } // Pass configuration values to the standard logger - logger.SetStandardEnabled(o.StandardLogging) - logger.SetAuthEnabled(o.AuthLogging) - logger.SetReqEnabled(o.RequestLogging) - logger.SetStandardTemplate(o.StandardLoggingFormat) - logger.SetAuthTemplate(o.AuthLoggingFormat) - logger.SetReqTemplate(o.RequestLoggingFormat) + logger.SetStandardEnabled(o.Logging.StandardEnabled) + logger.SetAuthEnabled(o.Logging.AuthEnabled) + logger.SetReqEnabled(o.Logging.RequestEnabled) + logger.SetStandardTemplate(o.Logging.StandardFormat) + logger.SetAuthTemplate(o.Logging.AuthFormat) + logger.SetReqTemplate(o.Logging.RequestFormat) logger.SetGetClientFunc(func(r *http.Request) string { return ip.GetClientString(o.GetRealClientIPParser(), r, false) }) excludePaths := make([]string, 0) - excludePaths = append(excludePaths, strings.Split(o.ExcludeLoggingPaths, ",")...) - if o.SilencePingLogging { + excludePaths = append(excludePaths, strings.Split(o.Logging.ExcludePaths, ",")...) + if o.Logging.SilencePing { excludePaths = append(excludePaths, o.PingPath) } logger.SetExcludePaths(excludePaths) - if !o.LoggingLocalTime { + if !o.Logging.LocalTime { logger.SetFlags(logger.Flags() | logger.LUTC) } From 3cbac6122dbd88137b2ac578e898248ca4cdd5ab Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Tue, 12 May 2020 00:32:30 +0100 Subject: [PATCH 2/5] Move configuration of logger to separate file --- pkg/validation/logging.go | 65 +++++++++++++++++++++++++++++++++++++++ pkg/validation/options.go | 65 ++++----------------------------------- 2 files changed, 71 insertions(+), 59 deletions(-) create mode 100644 pkg/validation/logging.go diff --git a/pkg/validation/logging.go b/pkg/validation/logging.go new file mode 100644 index 00000000..e72b669f --- /dev/null +++ b/pkg/validation/logging.go @@ -0,0 +1,65 @@ +package validation + +import ( + "os" + "strings" + + "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options" + "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" + "gopkg.in/natefinch/lumberjack.v2" +) + +// configureLogger is responsible for configuring the logger based on the options given +func configureLogger(o options.Logging, pingPath string, msgs []string) []string { + // Setup the log file + if len(o.File.Filename) > 0 { + // Validate that the file/dir can be written + file, err := os.OpenFile(o.File.Filename, os.O_WRONLY|os.O_CREATE, 0666) + if err != nil { + if os.IsPermission(err) { + return append(msgs, "unable to write to log file: "+o.File.Filename) + } + } + file.Close() + + logger.Printf("Redirecting logging to file: %s", o.File.Filename) + + logWriter := &lumberjack.Logger{ + Filename: o.File.Filename, + MaxSize: o.File.MaxSize, // megabytes + MaxAge: o.File.MaxAge, // days + MaxBackups: o.File.MaxBackups, + LocalTime: o.LocalTime, + Compress: o.File.Compress, + } + + logger.SetOutput(logWriter) + } + + // Supply a sanity warning to the logger if all logging is disabled + if !o.StandardEnabled && !o.AuthEnabled && !o.RequestEnabled { + logger.Print("Warning: Logging disabled. No further logs will be shown.") + } + + // Pass configuration values to the standard logger + logger.SetStandardEnabled(o.StandardEnabled) + logger.SetAuthEnabled(o.AuthEnabled) + logger.SetReqEnabled(o.RequestEnabled) + logger.SetStandardTemplate(o.StandardFormat) + logger.SetAuthTemplate(o.AuthFormat) + logger.SetReqTemplate(o.RequestFormat) + + excludePaths := make([]string, 0) + excludePaths = append(excludePaths, strings.Split(o.ExcludePaths, ",")...) + if o.SilencePing { + excludePaths = append(excludePaths, pingPath) + } + + logger.SetExcludePaths(excludePaths) + + if !o.LocalTime { + logger.SetFlags(logger.Flags() | logger.LUTC) + } + + return msgs +} diff --git a/pkg/validation/options.go b/pkg/validation/options.go index ecf6fcc7..5a99e4ff 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -23,7 +23,6 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/pkg/requests" "github.com/oauth2-proxy/oauth2-proxy/pkg/sessions" "github.com/oauth2-proxy/oauth2-proxy/providers" - "gopkg.in/natefinch/lumberjack.v2" ) // Validate checks that required options are set and validates those that they @@ -265,7 +264,7 @@ func Validate(o *options.Options) error { msgs = parseSignatureKey(o, msgs) msgs = validateCookieName(o, msgs) - msgs = setupLogger(o, msgs) + msgs = configureLogger(o.Logging, o.PingPath, msgs) if o.ReverseProxy { parser, err := ip.GetRealClientIPParser(o.RealClientIPHeader) @@ -273,6 +272,11 @@ func Validate(o *options.Options) error { msgs = append(msgs, fmt.Sprintf("real_client_ip_header (%s) not accepted parameter value: %v", o.RealClientIPHeader, err)) } o.SetRealClientIPParser(parser) + + // Allow the logger to get client IPs + logger.SetGetClientFunc(func(r *http.Request) string { + return ip.GetClientString(o.GetRealClientIPParser(), r, false) + }) } if len(msgs) != 0 { @@ -453,63 +457,6 @@ func validateCookieName(o *options.Options, msgs []string) []string { return msgs } -func setupLogger(o *options.Options, msgs []string) []string { - // Setup the log file - if len(o.Logging.File.Filename) > 0 { - // Validate that the file/dir can be written - file, err := os.OpenFile(o.Logging.File.Filename, os.O_WRONLY|os.O_CREATE, 0666) - if err != nil { - if os.IsPermission(err) { - return append(msgs, "unable to write to log file: "+o.Logging.File.Filename) - } - } - file.Close() - - logger.Printf("Redirecting logging to file: %s", o.Logging.File.Filename) - - logWriter := &lumberjack.Logger{ - Filename: o.Logging.File.Filename, - MaxSize: o.Logging.File.MaxSize, // megabytes - MaxAge: o.Logging.File.MaxAge, // days - MaxBackups: o.Logging.File.MaxBackups, - LocalTime: o.Logging.LocalTime, - Compress: o.Logging.File.Compress, - } - - logger.SetOutput(logWriter) - } - - // Supply a sanity warning to the logger if all logging is disabled - if !o.Logging.StandardEnabled && !o.Logging.AuthEnabled && !o.Logging.RequestEnabled { - logger.Print("Warning: Logging disabled. No further logs will be shown.") - } - - // Pass configuration values to the standard logger - logger.SetStandardEnabled(o.Logging.StandardEnabled) - logger.SetAuthEnabled(o.Logging.AuthEnabled) - logger.SetReqEnabled(o.Logging.RequestEnabled) - logger.SetStandardTemplate(o.Logging.StandardFormat) - logger.SetAuthTemplate(o.Logging.AuthFormat) - logger.SetReqTemplate(o.Logging.RequestFormat) - logger.SetGetClientFunc(func(r *http.Request) string { - return ip.GetClientString(o.GetRealClientIPParser(), r, false) - }) - - excludePaths := make([]string, 0) - excludePaths = append(excludePaths, strings.Split(o.Logging.ExcludePaths, ",")...) - if o.Logging.SilencePing { - excludePaths = append(excludePaths, o.PingPath) - } - - logger.SetExcludePaths(excludePaths) - - if !o.Logging.LocalTime { - logger.SetFlags(logger.Flags() | logger.LUTC) - } - - return msgs -} - // jwtIssuer hold parsed JWT issuer info that's used to construct a verifier. type jwtIssuer struct { issuerURI string From bbc4eee17ef7a85b332a8cc83bad1195ddde740d Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Tue, 12 May 2020 00:51:23 +0100 Subject: [PATCH 3/5] Create Logging FlagSet and Default --- pkg/apis/options/logging.go | 49 +++++++++++++++++++++++++++++++++++++ pkg/apis/options/options.go | 40 +++--------------------------- 2 files changed, 52 insertions(+), 37 deletions(-) diff --git a/pkg/apis/options/logging.go b/pkg/apis/options/logging.go index 3eb6ddc5..b46a0a05 100644 --- a/pkg/apis/options/logging.go +++ b/pkg/apis/options/logging.go @@ -1,5 +1,10 @@ package options +import ( + "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" + "github.com/spf13/pflag" +) + // Logging contains all options required for configuring the logging type Logging struct { AuthEnabled bool `flag:"auth-logging" cfg:"auth_logging"` @@ -22,3 +27,47 @@ type LogFileOptions struct { MaxBackups int `flag:"logging-max-backups" cfg:"logging_max_backups"` Compress bool `flag:"logging-compress" cfg:"logging_compress"` } + +func loggingFlagSet() *pflag.FlagSet { + flagSet := pflag.NewFlagSet("logging", pflag.ExitOnError) + + flagSet.Bool("auth-logging", true, "Log authentication attempts") + flagSet.String("auth-logging-format", logger.DefaultAuthLoggingFormat, "Template for authentication log lines") + flagSet.Bool("standard-logging", true, "Log standard runtime information") + flagSet.String("standard-logging-format", logger.DefaultStandardLoggingFormat, "Template for standard log lines") + flagSet.Bool("request-logging", true, "Log HTTP requests") + flagSet.String("request-logging-format", logger.DefaultRequestLoggingFormat, "Template for HTTP request log lines") + + flagSet.String("exclude-logging-paths", "", "Exclude logging requests to paths (eg: '/path1,/path2,/path3')") + flagSet.Bool("logging-local-time", true, "If the time in log files and backup filenames are local or UTC time") + flagSet.Bool("silence-ping-logging", false, "Disable logging of requests to ping endpoint") + + flagSet.String("logging-filename", "", "File to log requests to, empty for stdout") + flagSet.Int("logging-max-size", 100, "Maximum size in megabytes of the log file before rotation") + flagSet.Int("logging-max-age", 7, "Maximum number of days to retain old log files") + flagSet.Int("logging-max-backups", 0, "Maximum number of old log files to retain; 0 to disable") + flagSet.Bool("logging-compress", false, "Should rotated log files be compressed using gzip") + + return flagSet +} + +// loggingDefaults creates a Logging structure, populating each field with its default value +func loggingDefaults() Logging { + return Logging{ + LocalTime: true, + SilencePing: false, + AuthEnabled: true, + AuthFormat: logger.DefaultAuthLoggingFormat, + RequestEnabled: true, + RequestFormat: logger.DefaultRequestLoggingFormat, + StandardEnabled: true, + StandardFormat: logger.DefaultStandardLoggingFormat, + File: LogFileOptions{ + Filename: "", + MaxSize: 100, + MaxAge: 7, + MaxBackups: 0, + Compress: false, + }, + } +} diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index e5e509a3..abd96495 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -9,7 +9,6 @@ import ( oidc "github.com/coreos/go-oidc" ipapi "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/ip" sessionsapi "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions" - "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" "github.com/oauth2-proxy/oauth2-proxy/providers" "github.com/spf13/pflag" ) @@ -183,24 +182,7 @@ func NewOptions() *Options { UserIDClaim: "email", InsecureOIDCAllowUnverifiedEmail: false, SkipOIDCDiscovery: false, - Logging: Logging{ - ExcludePaths: "", - LocalTime: true, - SilencePing: false, - AuthEnabled: true, - AuthFormat: logger.DefaultAuthLoggingFormat, - RequestEnabled: true, - RequestFormat: logger.DefaultRequestLoggingFormat, - StandardEnabled: true, - StandardFormat: logger.DefaultStandardLoggingFormat, - File: LogFileOptions{ - Filename: "", - MaxSize: 100, - MaxAge: 7, - MaxBackups: 0, - Compress: false, - }, - }, + Logging: loggingDefaults(), } } @@ -283,24 +265,6 @@ func NewFlagSet() *pflag.FlagSet { flagSet.Bool("redis-use-cluster", false, "Connect to redis cluster. Must set --redis-cluster-connection-urls to use this feature") flagSet.StringSlice("redis-cluster-connection-urls", []string{}, "List of Redis cluster connection URLs (eg redis://HOST[:PORT]). Used in conjunction with --redis-use-cluster") - flagSet.String("logging-filename", "", "File to log requests to, empty for stdout") - flagSet.Int("logging-max-size", 100, "Maximum size in megabytes of the log file before rotation") - flagSet.Int("logging-max-age", 7, "Maximum number of days to retain old log files") - flagSet.Int("logging-max-backups", 0, "Maximum number of old log files to retain; 0 to disable") - flagSet.Bool("logging-local-time", true, "If the time in log files and backup filenames are local or UTC time") - flagSet.Bool("logging-compress", false, "Should rotated log files be compressed using gzip") - - flagSet.Bool("standard-logging", true, "Log standard runtime information") - flagSet.String("standard-logging-format", logger.DefaultStandardLoggingFormat, "Template for standard log lines") - - flagSet.Bool("request-logging", true, "Log HTTP requests") - flagSet.String("request-logging-format", logger.DefaultRequestLoggingFormat, "Template for HTTP request log lines") - flagSet.String("exclude-logging-paths", "", "Exclude logging requests to paths (eg: '/path1,/path2,/path3')") - flagSet.Bool("silence-ping-logging", false, "Disable logging of requests to ping endpoint") - - flagSet.Bool("auth-logging", true, "Log authentication attempts") - flagSet.String("auth-logging-format", logger.DefaultAuthLoggingFormat, "Template for authentication log lines") - flagSet.String("provider", "google", "OAuth provider") flagSet.String("provider-display-name", "", "Provider display name") flagSet.String("oidc-issuer-url", "", "OpenID Connect issuer URL (ie: https://accounts.google.com)") @@ -326,5 +290,7 @@ func NewFlagSet() *pflag.FlagSet { flagSet.String("user-id-claim", "email", "which claim contains the user ID") + flagSet.AddFlagSet(loggingFlagSet()) + return flagSet } From f7c88f53d1a71d22cc1ec3ceb2b09ad18c92c23e Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Tue, 12 May 2020 00:59:49 +0100 Subject: [PATCH 4/5] Update changelog for logging options move --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9a7afeb..fd63a9f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,9 +48,14 @@ This lead to confusion and misconfiguration as it was not obvious when a session should be encrypted. - Cookie Secrets must now be 16, 24 or 32 bytes. - If you need to change your secret, this will force users to reauthenticate. +- [#548](https://github.com/oauth2-proxy/oauth2-proxy/pull/548) Separate logging options out of main options structure + - Fixes an inconsistency in the `--exclude-logging-paths` option by renaming it to `--exclude-logging-option`. + - This flag may now be given multiple times as with other list options + - This flag also accepts comma separated values ## Changes since v5.1.1 +- [#548](https://github.com/oauth2-proxy/oauth2-proxy/pull/548) Separate logging options out of main options structure (@JoelSpeed) - [#536](https://github.com/oauth2-proxy/oauth2-proxy/pull/536) Improvements to Session State code (@JoelSpeed) - [#573](https://github.com/oauth2-proxy/oauth2-proxy/pull/573) Properly parse redis urls for cluster and sentinel connections (@amnay-mo) - [#574](https://github.com/oauth2-proxy/oauth2-proxy/pull/574) render error page on 502 proxy status (@amnay-mo) From 94e31f8b65102f9ace88dfd47c06fe40ef468583 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Sun, 24 May 2020 21:59:18 +0100 Subject: [PATCH 5/5] Ensure exclude-logging-paths is consistent with other options --- pkg/apis/options/logging.go | 5 +++-- pkg/validation/logging.go | 5 +---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/apis/options/logging.go b/pkg/apis/options/logging.go index b46a0a05..d7407678 100644 --- a/pkg/apis/options/logging.go +++ b/pkg/apis/options/logging.go @@ -13,7 +13,7 @@ type Logging struct { RequestFormat string `flag:"request-logging-format" cfg:"request_logging_format"` StandardEnabled bool `flag:"standard-logging" cfg:"standard_logging"` StandardFormat string `flag:"standard-logging-format" cfg:"standard_logging_format"` - ExcludePaths string `flag:"exclude-logging-paths" cfg:"exclude_logging_paths"` + ExcludePaths []string `flag:"exclude-logging-path" cfg:"exclude_logging_paths"` LocalTime bool `flag:"logging-local-time" cfg:"logging_local_time"` SilencePing bool `flag:"silence-ping-logging" cfg:"silence_ping_logging"` File LogFileOptions `cfg:",squash"` @@ -38,7 +38,7 @@ func loggingFlagSet() *pflag.FlagSet { flagSet.Bool("request-logging", true, "Log HTTP requests") flagSet.String("request-logging-format", logger.DefaultRequestLoggingFormat, "Template for HTTP request log lines") - flagSet.String("exclude-logging-paths", "", "Exclude logging requests to paths (eg: '/path1,/path2,/path3')") + flagSet.StringSlice("exclude-logging-path", []string{}, "Exclude logging requests to paths (eg: '/path1,/path2,/path3')") flagSet.Bool("logging-local-time", true, "If the time in log files and backup filenames are local or UTC time") flagSet.Bool("silence-ping-logging", false, "Disable logging of requests to ping endpoint") @@ -54,6 +54,7 @@ func loggingFlagSet() *pflag.FlagSet { // loggingDefaults creates a Logging structure, populating each field with its default value func loggingDefaults() Logging { return Logging{ + ExcludePaths: nil, LocalTime: true, SilencePing: false, AuthEnabled: true, diff --git a/pkg/validation/logging.go b/pkg/validation/logging.go index e72b669f..1c8ab2a3 100644 --- a/pkg/validation/logging.go +++ b/pkg/validation/logging.go @@ -2,7 +2,6 @@ package validation import ( "os" - "strings" "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options" "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" @@ -49,12 +48,10 @@ func configureLogger(o options.Logging, pingPath string, msgs []string) []string logger.SetAuthTemplate(o.AuthFormat) logger.SetReqTemplate(o.RequestFormat) - excludePaths := make([]string, 0) - excludePaths = append(excludePaths, strings.Split(o.ExcludePaths, ",")...) + excludePaths := o.ExcludePaths if o.SilencePing { excludePaths = append(excludePaths, pingPath) } - logger.SetExcludePaths(excludePaths) if !o.LocalTime {