Allow for Access Log Config to be fine-grained.

Signed-off-by: Dipack Panjabi <[email protected]>

agent/config/config.go

@@ -33,9 +33,9 @@ type ListenerConfig struct {
 	// Defaults to "http".
 	Protocol ListenerProtocol `json:"protocol" yaml:"protocol"`
 
-	// AccessLog indicates whether to log all incoming connections and requests
-	// for the endpoint.
-	AccessLog bool `json:"access_log" yaml:"access_log"`
+	// AccessLogConfig allows us to control how the incoming requests to
+	// the proxy are logged.
+	AccessLog log.AccessLogConfig `json:"access_log" yaml:"access_log"`
 
 	// Timeout is the timeout to forward incoming requests to the upstream.
 	Timeout time.Duration `json:"timeout" yaml:"timeout"`
@@ -124,6 +124,11 @@ func (c *ListenerConfig) Validate() error {
 	if err := c.TLS.Validate(); err != nil {
 		return fmt.Errorf("tls: %w", err)
 	}
+
+	if err := c.AccessLog.Validate(); err != nil {
+		return fmt.Errorf("access_log: %w", err)
+	}
+
 	return nil
 }
 

agent/tcpproxy/server.go

@@ -114,15 +114,15 @@ func (s *Server) removeConn(c net.Conn) {
 }
 
 func (s *Server) logConnOpened() {
-	if s.conf.AccessLog {
+	if s.conf.AccessLog.Enabled {
 		s.accessLogger.Info("connection opened")
 	} else {
 		s.accessLogger.Debug("connection opened")
 	}
 }
 
 func (s *Server) logConnClosed() {
-	if s.conf.AccessLog {
+	if s.conf.AccessLog.Enabled {
 		s.accessLogger.Info("connection closed")
 	} else {
 		s.accessLogger.Debug("connection closed")

cli/agent/http.go

@@ -62,8 +62,10 @@ Timeout forwarding incoming HTTP requests to the upstream.`,
 			EndpointID: args[0],
 			Addr:       args[1],
 			Protocol:   config.ListenerProtocolHTTP,
-			AccessLog:  accessLog,
-			Timeout:    timeout,
+			AccessLog: log.AccessLogConfig{
+				Enabled: true,
+			},
+			Timeout: timeout,
 		}}
 
 		var err error

cli/agent/tcp.go

@@ -59,8 +59,11 @@ Timeout connecting to the upstream.`,
 			EndpointID: args[0],
 			Addr:       args[1],
 			Protocol:   config.ListenerProtocolTCP,
-			AccessLog:  accessLog,
-			Timeout:    timeout,
+			AccessLog: log.AccessLogConfig{
+				// FIXME: What should be done about the logging config here?
+				Enabled: accessLog,
+			},
+			Timeout: timeout,
 		}}
 
 		var err error

pkg/log/config.go

@@ -2,6 +2,7 @@ package log
 
 import (
 	"fmt"
+	"net/http"
 
 	"github.com/spf13/pflag"
 )
@@ -50,3 +51,99 @@ debug logs.
 Such as you can enable 'gossip' logs with '--log.subsystems gossip'.`,
 	)
 }
+
+// FIXME: Update docs
+// FIXME: Tests in agent/config_test.go
+type AccessLogHeaderConfig struct {
+	// Prevent these headers from being logged.
+	// You can only define one of Allowlist or Blocklist.
+	Blocklist []string `json:"blocklist" yaml:"blocklist"`
+
+	// Log only these headers.
+	// You can only define one of Allowlist or Blocklist.
+	Allowlist []string `json:"allowlist" yaml:"allowlist"`
+}
+
+func (c *AccessLogHeaderConfig) Validate() error {
+	if len(c.Allowlist) > 0 && len(c.Blocklist) > 0 {
+		return fmt.Errorf("cannot define both allowlist and blocklist")
+	}
+	return nil
+}
+
+func (c *AccessLogHeaderConfig) Filter(h http.Header) http.Header {
+	if len(c.Allowlist) > 0 {
+		// FIXME: Make this a one-time thing on boot.
+		allowList := make(map[string]string)
+		for _, el := range c.Allowlist {
+			allowList[el] = el
+		}
+		for name := range h {
+			if _, ok := allowList[name]; !ok {
+				h.Del(name)
+			}
+		}
+		return h
+	}
+
+	if len(c.Blocklist) > 0 {
+		for _, blocked := range c.Blocklist {
+			h.Del(blocked)
+		}
+		return h
+	}
+
+	return h
+}
+
+func (c *AccessLogHeaderConfig) RegisterFlags(fs *pflag.FlagSet, prefix string) {
+	fs.StringSliceVar(
+		&c.Allowlist,
+		prefix+"allowlist",
+		[]string{},
+		`Log only these headers`,
+	)
+	fs.StringSliceVar(
+		&c.Blocklist,
+		prefix+"blocklist",
+		[]string{},
+		`Block these headers from being logged`,
+	)
+}
+
+type AccessLogConfig struct {
+	// If Access logging is enabled, using the 'info' log level.
+	// If disabled, logs will be emitted with the 'debug' log level,
+	// while still respecting the header allow and block lists.
+	Enabled bool `json:"enabled" yaml:"enabled"`
+
+	// Control how Request Headers are logged.
+	RequestHeaders AccessLogHeaderConfig `json:"request_headers" yaml:"request_headers"`
+
+	// Control how Response Headers are logged.
+	ResponseHeaders AccessLogHeaderConfig `json:"response_headers" yaml:"response_headers"`
+}
+
+func (c *AccessLogConfig) Validate() error {
+	if err := c.RequestHeaders.Validate(); err != nil {
+		return fmt.Errorf("request_headers: %w", err)
+	}
+
+	if err := c.ResponseHeaders.Validate(); err != nil {
+		return fmt.Errorf("response_headers: %w", err)
+	}
+	return nil
+}
+
+func (c *AccessLogConfig) RegisterFlags(fs *pflag.FlagSet, prefix string) {
+	prefix = prefix + ".access-log."
+	fs.BoolVar(
+		&c.Enabled,
+		prefix+"enabled",
+		true,
+		`
+If Access logging is enabled`,
+	)
+	c.RequestHeaders.RegisterFlags(fs, prefix+"request-headers.")
+	c.ResponseHeaders.RegisterFlags(fs, prefix+"response-headers.")
+}

pkg/middleware/logger.go

@@ -23,7 +23,7 @@ type loggedRequest struct {
 }
 
 // NewLogger creates logging middleware that logs every request.
-func NewLogger(accessLog bool, logger log.Logger) gin.HandlerFunc {
+func NewLogger(accessLogConfig log.AccessLogConfig, logger log.Logger) gin.HandlerFunc {
 	logger = logger.WithSubsystem(logger.Subsystem() + ".access")
 	return func(c *gin.Context) {
 		s := time.Now()
@@ -35,19 +35,22 @@ func NewLogger(accessLog bool, logger log.Logger) gin.HandlerFunc {
 			return
 		}
 
+		requestHeaders := accessLogConfig.RequestHeaders.Filter(c.Request.Header)
+		responseHeaders := accessLogConfig.ResponseHeaders.Filter(c.Writer.Header())
+
 		req := &loggedRequest{
 			Proto:           c.Request.Proto,
 			Method:          c.Request.Method,
 			Host:            c.Request.Host,
 			Path:            c.Request.URL.Path,
-			RequestHeaders:  c.Request.Header,
-			ResponseHeaders: c.Writer.Header(),
+			RequestHeaders:  requestHeaders,
+			ResponseHeaders: responseHeaders,
 			Status:          c.Writer.Status(),
 			Duration:        time.Since(s).String(),
 		}
 		if c.Writer.Status() >= http.StatusInternalServerError {
 			logger.Warn("request", zap.Any("request", req))
-		} else if accessLog {
+		} else if accessLogConfig.Enabled {
 			logger.Info("request", zap.Any("request", req))
 		} else {
 			logger.Debug("request", zap.Any("request", req))

server/config/config.go

@@ -187,9 +187,9 @@ type ProxyConfig struct {
 	// Timeout is the timeout to forward incoming requests to the upstream.
 	Timeout time.Duration `json:"timeout" yaml:"timeout"`
 
-	// AccessLog indicates whether to log all incoming connections and
-	// requests.
-	AccessLog bool `json:"access_log" yaml:"access_log"`
+	// AccessLogConfig allows us to control how the incoming requests to
+	// the proxy are logged.
+	AccessLog log.AccessLogConfig `json:"access_log" yaml:"access_log"`
 
 	Auth auth.Config `json:"auth" yaml:"auth"`
 
@@ -205,6 +205,10 @@ func (c *ProxyConfig) Validate() error {
 	if err := c.TLS.Validate(); err != nil {
 		return fmt.Errorf("tls: %w", err)
 	}
+
+	if err := c.AccessLog.Validate(); err != nil {
+		return fmt.Errorf("access_log: %w", err)
+	}
 	return nil
 }
 
@@ -245,13 +249,7 @@ advertise address of '10.26.104.14:8000'.`,
 Timeout when forwarding incoming requests to the upstream.`,
 	)
 
-	fs.BoolVar(
-		&c.AccessLog,
-		"proxy.access-log",
-		c.AccessLog,
-		`
-Whether to log all incoming connections and requests.`,
-	)
+	c.AccessLog.RegisterFlags(fs, "proxy")
 
 	c.HTTP.RegisterFlags(fs, "proxy")
 
@@ -519,9 +517,11 @@ type Config struct {
 func Default() *Config {
 	return &Config{
 		Proxy: ProxyConfig{
-			BindAddr:  ":8000",
-			Timeout:   time.Second * 30,
-			AccessLog: true,
+			BindAddr: ":8000",
+			Timeout:  time.Second * 30,
+			AccessLog: log.AccessLogConfig{
+				Enabled: true,
+			},
 			HTTP: HTTPConfig{
 				ReadTimeout:       time.Second * 10,
 				ReadHeaderTimeout: time.Second * 10,

server/config/config_test.go

@@ -28,7 +28,16 @@ proxy:
   bind_addr: 10.15.104.25:8000
   advertise_addr: 1.2.3.4:8000
   timeout: 20s
-  access_log: true
+  access_log:
+    enabled: false
+    request_headers:
+      blocklist:
+        - abc
+        - xyz
+    response_headers:
+      allowlist:
+        - def
+        - ghi
 
   http:
     read_timeout: 5s
@@ -125,7 +134,15 @@ grace_period: 2m
 			BindAddr:      "10.15.104.25:8000",
 			AdvertiseAddr: "1.2.3.4:8000",
 			Timeout:       time.Second * 20,
-			AccessLog:     true,
+			AccessLog: log.AccessLogConfig{
+				Enabled: false,
+				RequestHeaders: log.AccessLogHeaderConfig{
+					Blocklist: []string{"abc", "xyz"},
+				},
+				ResponseHeaders: log.AccessLogHeaderConfig{
+					Allowlist: []string{"def", "ghi"},
+				},
+			},
 			HTTP: HTTPConfig{
 				ReadTimeout:       time.Second * 5,
 				ReadHeaderTimeout: time.Second * 5,
@@ -217,7 +234,9 @@ func TestConfig_LoadFlags(t *testing.T) {
 		"--proxy.bind-addr", "10.15.104.25:8000",
 		"--proxy.advertise-addr", "1.2.3.4:8000",
 		"--proxy.timeout", "20s",
-		"--proxy.access-log",
+		"--proxy.access-log.enabled",
+		"--proxy.access-log.request-headers.allowlist", "abc,def",
+		"--proxy.access-log.response-headers.blocklist", "xyz,ghi",
 		"--proxy.http.read-timeout", "5s",
 		"--proxy.http.read-header-timeout", "5s",
 		"--proxy.http.write-timeout", "5s",
@@ -277,7 +296,15 @@ func TestConfig_LoadFlags(t *testing.T) {
 			BindAddr:      "10.15.104.25:8000",
 			AdvertiseAddr: "1.2.3.4:8000",
 			Timeout:       time.Second * 20,
-			AccessLog:     true,
+			AccessLog: log.AccessLogConfig{
+				Enabled: true,
+				RequestHeaders: log.AccessLogHeaderConfig{
+					Allowlist: []string{"abc", "def"},
+				},
+				ResponseHeaders: log.AccessLogHeaderConfig{
+					Blocklist: []string{"xyz", "ghi"},
+				},
+			},
 			HTTP: HTTPConfig{
 				ReadTimeout:       time.Second * 5,
 				ReadHeaderTimeout: time.Second * 5,