From 19a8fbbb65aa25efaf53258e20f28aea4c379f83 Mon Sep 17 00:00:00 2001
From: Mikael Manukyan <mmanukyan@vmware.com>
Date: Fri, 23 Oct 2020 23:07:33 +0000
Subject: [PATCH] internal/envoy: send custom JSON fields to Envoy

Fixes #3032, #1507

Signed-off-by: Clay Kauzlaric <ckauzlaric@vmware.com>
Co-authored-by: Clay Kauzlaric <ckauzlaric@vmware.com>
Co-authored-by: Alexander Standke <astandke@vmware.com>
---
 internal/envoy/v2/accesslog.go      | 12 +++++-------
 internal/envoy/v2/accesslog_test.go | 19 ++++++++++++-------
 internal/xdscache/v2/listener.go    |  6 +++---
 3 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/internal/envoy/v2/accesslog.go b/internal/envoy/v2/accesslog.go
index ae8958de410..c218a15d778 100644
--- a/internal/envoy/v2/accesslog.go
+++ b/internal/envoy/v2/accesslog.go
@@ -38,19 +38,17 @@ func FileAccessLogEnvoy(path string) []*accesslog.AccessLog {
 
 // FileAccessLogJSON returns a new file based access log filter
 // that will log in JSON format
-func FileAccessLogJSON(path string, keys []string) []*accesslog.AccessLog {
+func FileAccessLogJSON(path string, fields config.AccessLogFields) []*accesslog.AccessLog {
 
 	jsonformat := &_struct.Struct{
 		Fields: make(map[string]*_struct.Value),
 	}
 
-	for _, k := range keys {
-		// This will silently ignore invalid headers.
-		// TODO(youngnick): this should tell users if a header is not valid
-		// https://github.com/projectcontour/contour/issues/1507
-		if template, ok := config.JSONFields[k]; ok {
-			jsonformat.Fields[k] = sv(template)
+	for k, v := range fields {
+		if v == "" {
+			v = config.JSONFields[k]
 		}
+		jsonformat.Fields[k] = sv(v)
 	}
 
 	return []*accesslog.AccessLog{{
diff --git a/internal/envoy/v2/accesslog_test.go b/internal/envoy/v2/accesslog_test.go
index d72d6e29ff6..80db3e4f252 100644
--- a/internal/envoy/v2/accesslog_test.go
+++ b/internal/envoy/v2/accesslog_test.go
@@ -52,12 +52,12 @@ func TestFileAccessLog(t *testing.T) {
 func TestJSONFileAccessLog(t *testing.T) {
 	tests := map[string]struct {
 		path    string
-		headers []string
+		headers map[string]string
 		want    []*envoy_accesslog.AccessLog
 	}{
 		"only timestamp": {
 			path:    "/dev/stdout",
-			headers: []string{"@timestamp"},
+			headers: map[string]string{"@timestamp": "%START_TIME%"},
 			want: []*envoy_accesslog.AccessLog{{
 				Name: wellknown.FileAccessLog,
 				ConfigType: &envoy_accesslog.AccessLog_TypedConfig{
@@ -75,12 +75,14 @@ func TestJSONFileAccessLog(t *testing.T) {
 			},
 			},
 		},
-		"invalid header should disappear": {
+		"custom fields should appear": {
 			path: "/dev/stdout",
-			headers: []string{
-				"@timestamp",
-				"invalid",
-				"method",
+			headers: map[string]string{
+				"@timestamp": config.JSONFields["@timestamp"],
+				"method":     config.JSONFields["method"],
+				"custom1":    "%REQ(X-CUSTOM-HEADER)%",
+				"custom2":    "%DURATION%.0",
+				"custom3":    "ST=%START_TIME(%s.%6f)%",
 			},
 			want: []*envoy_accesslog.AccessLog{{
 				Name: wellknown.FileAccessLog,
@@ -92,6 +94,9 @@ func TestJSONFileAccessLog(t *testing.T) {
 								Fields: map[string]*_struct.Value{
 									"@timestamp": sv(config.JSONFields["@timestamp"]),
 									"method":     sv(config.JSONFields["method"]),
+									"custom1":    sv("%REQ(X-CUSTOM-HEADER)%"),
+									"custom2":    sv("%DURATION%.0"),
+									"custom3":    sv("ST=%START_TIME(%s.%6f)%"),
 								},
 							},
 						},
diff --git a/internal/xdscache/v2/listener.go b/internal/xdscache/v2/listener.go
index b08eb7b2b8d..b7008e515e4 100644
--- a/internal/xdscache/v2/listener.go
+++ b/internal/xdscache/v2/listener.go
@@ -96,7 +96,7 @@ type ListenerConfig struct {
 	// AccessLogFields sets the fields that should be shown in JSON logs.
 	// Valid entries are the keys from internal/envoy/accesslog.go:jsonheaders
 	// Defaults to a particular set of fields.
-	AccessLogFields []string
+	AccessLogFields config.AccessLogFields
 
 	// RequestTimeout configures the request_timeout for all Connection Managers.
 	RequestTimeout timeout.Setting
@@ -181,11 +181,11 @@ func (lvc *ListenerConfig) accesslogType() string {
 
 // accesslogFields returns the access log fields that should be configured
 // for Envoy, or a default set if not configured.
-func (lvc *ListenerConfig) accesslogFields() []string {
+func (lvc *ListenerConfig) accesslogFields() config.AccessLogFields {
 	if lvc.AccessLogFields != nil {
 		return lvc.AccessLogFields
 	}
-	return config.DefaultFields
+	return config.ParsedDefaultFields
 }
 
 func (lvc *ListenerConfig) newInsecureAccessLog() []*envoy_api_v2_accesslog.AccessLog {