WIP: add support for canonical representation

kumahq · Feb 21, 2020 · b9cd952 · b9cd952
1 parent 2d7025f
commit b9cd952
Show file tree

Hide file tree

Showing 22 changed files with 735 additions and 54 deletions.
diff --git a/pkg/envoy/accesslog/commands.go b/pkg/envoy/accesslog/commands.go
@@ -63,14 +63,14 @@ const (
 	CMD_HOSTNAME                        = "HOSTNAME"
 )
 
-// CommandOperatorName represents a reference name of an Envoy access log command operator.
+// CommandOperatorDescriptor represents a descriptor of an Envoy access log command operator.
 //
 // See https://www.envoyproxy.io/docs/envoy/latest/configuration/observability/access_log#command-operators
-type CommandOperatorName string
+type CommandOperatorDescriptor string
 
 // String returns the reference name of an Envoy access log command operator
 // as it appears on https://www.envoyproxy.io/docs/envoy/latest/configuration/observability/access_log#command-operators
-func (o CommandOperatorName) String() string {
+func (o CommandOperatorDescriptor) String() string {
 	switch string(o) {
 	case CMD_REQ:
 		return "%REQ(X?Y):Z%"

diff --git a/pkg/envoy/accesslog/composite_formatter.go b/pkg/envoy/accesslog/composite_formatter.go
@@ -8,7 +8,7 @@ import (
 )
 
 const (
-	unspecifiedValue = "-" // to replicate Envoy behaviour
+	unspecifiedValue = "-" // to replicate Envoy's behaviour
 )
 
 // CompositeLogConfigureFormatter represents the entire access log format string.
@@ -46,7 +46,7 @@ func (c *CompositeLogConfigureFormatter) FormatHttpLogEntry(entry *accesslog_dat
 			return "", err
 		}
 		if value == "" {
-			value = unspecifiedValue // to replicate Envoy behaviour
+			value = unspecifiedValue // to replicate Envoy's behaviour
 		}
 		values[i] = value
 	}
@@ -61,9 +61,18 @@ func (c *CompositeLogConfigureFormatter) FormatTcpLogEntry(entry *accesslog_data
 			return "", err
 		}
 		if value == "" {
-			value = unspecifiedValue // to replicate Envoy behaviour
+			value = unspecifiedValue // to replicate Envoy's behaviour
 		}
 		values[i] = value
 	}
 	return strings.Join(values, ""), nil
 }
+
+// String returns the canonical representation of this format string.
+func (f *CompositeLogConfigureFormatter) String() string {
+	fragments := make([]string, len(f.Formatters))
+	for i, formatter := range f.Formatters {
+		fragments[i] = formatter.String()
+	}
+	return strings.Join(fragments, "")
+}
diff --git a/pkg/envoy/accesslog/dynamic_metadata_formatter.go b/pkg/envoy/accesslog/dynamic_metadata_formatter.go
@@ -1,6 +1,9 @@
 package accesslog
 
 import (
+	"strconv"
+	"strings"
+
 	accesslog_data "github.com/envoyproxy/go-control-plane/envoy/data/accesslog/v2"
 )
 
@@ -21,5 +24,25 @@ func (f *DynamicMetadataFormatter) FormatTcpLogEntry(entry *accesslog_data.TCPAc
 
 func (f *DynamicMetadataFormatter) format(entry *accesslog_data.AccessLogCommon) (string, error) {
 	// TODO(yskopets): implement
-	return "UNSUPPORTED(DYNAMIC_METADATA)", nil
+	return "UNSUPPORTED_COMMAND(%DYNAMIC_METADATA(NAMESPACE:KEY*):Z%)", nil
+}
+
+// String returns the canonical representation of this command operator.
+func (f *DynamicMetadataFormatter) String() string {
+	var builder []string
+	builder = append(builder, "%DYNAMIC_METADATA(")
+	builder = append(builder, f.FilterNamespace)
+	if len(f.Path) > 0 {
+		for _, segment := range f.Path {
+			builder = append(builder, ":")
+			builder = append(builder, segment)
+		}
+	}
+	builder = append(builder, ")")
+	if f.MaxLength != 0 {
+		builder = append(builder, ":")
+		builder = append(builder, strconv.FormatInt(int64(f.MaxLength), 10))
+	}
+	builder = append(builder, "%")
+	return strings.Join(builder, "")
 }
diff --git a/pkg/envoy/accesslog/dynamic_metadata_formatter_test.go b/pkg/envoy/accesslog/dynamic_metadata_formatter_test.go
@@ -0,0 +1,81 @@
+package accesslog_test
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/gomega"
+
+	. "github.com/Kong/kuma/pkg/envoy/accesslog"
+)
+
+var _ = Describe("DynamicMetadataFormatter", func() {
+
+	Describe("String()", func() {
+		type testCase struct {
+			filterNamespace string
+			path            []string
+			maxLength       int
+			expected        string
+		}
+
+		DescribeTable("should return correct canonical representation",
+			func(given testCase) {
+				// setup
+				formatter := &DynamicMetadataFormatter{FilterNamespace: given.filterNamespace, Path: given.path, MaxLength: given.maxLength}
+
+				// when
+				actual := formatter.String()
+				// then
+				Expect(actual).To(Equal(given.expected))
+
+			},
+			Entry("%DYNAMIC_METADATA()%", testCase{
+				expected: `%DYNAMIC_METADATA()%`,
+			}),
+			Entry("%DYNAMIC_METADATA():10%", testCase{
+				maxLength: 10,
+				expected:  `%DYNAMIC_METADATA():10%`,
+			}),
+			Entry("%DYNAMIC_METADATA(com.test.my_filter)%", testCase{
+				filterNamespace: "com.test.my_filter",
+				expected:        `%DYNAMIC_METADATA(com.test.my_filter)%`,
+			}),
+			Entry("%DYNAMIC_METADATA(com.test.my_filter):10%", testCase{
+				filterNamespace: "com.test.my_filter",
+				maxLength:       10,
+				expected:        `%DYNAMIC_METADATA(com.test.my_filter):10%`,
+			}),
+			Entry("%DYNAMIC_METADATA(com.test.my_filter:test_object)%", testCase{
+				filterNamespace: "com.test.my_filter",
+				path:            []string{"test_object"},
+				expected:        `%DYNAMIC_METADATA(com.test.my_filter:test_object)%`,
+			}),
+			Entry("%DYNAMIC_METADATA(com.test.my_filter:test_object):10%", testCase{
+				filterNamespace: "com.test.my_filter",
+				path:            []string{"test_object"},
+				maxLength:       10,
+				expected:        `%DYNAMIC_METADATA(com.test.my_filter:test_object):10%`,
+			}),
+			Entry("%DYNAMIC_METADATA(com.test.my_filter:test_object:inner_key)%", testCase{
+				filterNamespace: "com.test.my_filter",
+				path:            []string{"test_object", "inner_key"},
+				expected:        `%DYNAMIC_METADATA(com.test.my_filter:test_object:inner_key)%`,
+			}),
+			Entry("%DYNAMIC_METADATA(com.test.my_filter:test_object:inner_key):10%", testCase{
+				filterNamespace: "com.test.my_filter",
+				path:            []string{"test_object", "inner_key"},
+				maxLength:       10,
+				expected:        `%DYNAMIC_METADATA(com.test.my_filter:test_object:inner_key):10%`,
+			}),
+			Entry("%DYNAMIC_METADATA(:test_object:inner_key)%", testCase{
+				path:     []string{"test_object", "inner_key"},
+				expected: `%DYNAMIC_METADATA(:test_object:inner_key)%`,
+			}),
+			Entry("%DYNAMIC_METADATA(:test_object:inner_key):10%", testCase{
+				path:      []string{"test_object", "inner_key"},
+				maxLength: 10,
+				expected:  `%DYNAMIC_METADATA(:test_object:inner_key):10%`,
+			}),
+		)
+	})
+})
diff --git a/pkg/envoy/accesslog/field_formatter.go b/pkg/envoy/accesslog/field_formatter.go
@@ -25,6 +25,11 @@ const (
 // such as `%BYTES_RECEIVED%` or `%PROTOCOL%`.
 type FieldFormatter string
 
+// String returns the canonical representation of this command operator.
+func (f FieldFormatter) String() string {
+	return CommandOperatorDescriptor(f).String()
+}
+
 func (f FieldFormatter) FormatHttpLogEntry(entry *accesslog_data.HTTPAccessLogEntry) (string, error) {
 	switch f {
 	case CMD_BYTES_RECEIVED:
@@ -125,7 +130,7 @@ func (f FieldFormatter) formatAccessLogCommon(entry *accesslog_data.AccessLogCom
 		fallthrough // these fields have no equivalent data in GrpcAccessLog
 	default:
 		// make it clear to the user what is happening
-		return fmt.Sprintf("UNSUPPORTED_FIELD(%s)", f), nil
+		return fmt.Sprintf("UNSUPPORTED_COMMAND(%s)", f), nil
 	}
 }
 

diff --git a/pkg/envoy/accesslog/field_formatter_test.go b/pkg/envoy/accesslog/field_formatter_test.go
@@ -856,31 +856,55 @@ var _ = Describe("FieldFormatter", func() {
 			}),
 			Entry("DOWNSTREAM_PEER_FINGERPRINT_256", testCase{
 				field:    "DOWNSTREAM_PEER_FINGERPRINT_256",
-				expected: `UNSUPPORTED_FIELD(DOWNSTREAM_PEER_FINGERPRINT_256)`,
+				expected: `UNSUPPORTED_COMMAND(%DOWNSTREAM_PEER_FINGERPRINT_256%)`,
 			}),
 			Entry("DOWNSTREAM_PEER_SERIAL", testCase{
 				field:    "DOWNSTREAM_PEER_SERIAL",
-				expected: `UNSUPPORTED_FIELD(DOWNSTREAM_PEER_SERIAL)`,
+				expected: `UNSUPPORTED_COMMAND(%DOWNSTREAM_PEER_SERIAL%)`,
 			}),
 			Entry("DOWNSTREAM_PEER_ISSUER", testCase{
 				field:    "DOWNSTREAM_PEER_ISSUER",
-				expected: `UNSUPPORTED_FIELD(DOWNSTREAM_PEER_ISSUER)`,
+				expected: `UNSUPPORTED_COMMAND(%DOWNSTREAM_PEER_ISSUER%)`,
 			}),
 			Entry("DOWNSTREAM_PEER_CERT", testCase{
 				field:    "DOWNSTREAM_PEER_CERT",
-				expected: `UNSUPPORTED_FIELD(DOWNSTREAM_PEER_CERT)`,
+				expected: `UNSUPPORTED_COMMAND(%DOWNSTREAM_PEER_CERT%)`,
 			}),
 			Entry("DOWNSTREAM_PEER_CERT_V_START", testCase{
 				field:    "DOWNSTREAM_PEER_CERT_V_START",
-				expected: `UNSUPPORTED_FIELD(DOWNSTREAM_PEER_CERT_V_START)`,
+				expected: `UNSUPPORTED_COMMAND(%DOWNSTREAM_PEER_CERT_V_START%)`,
 			}),
 			Entry("DOWNSTREAM_PEER_CERT_V_END", testCase{
 				field:    "DOWNSTREAM_PEER_CERT_V_END",
-				expected: `UNSUPPORTED_FIELD(DOWNSTREAM_PEER_CERT_V_END)`,
+				expected: `UNSUPPORTED_COMMAND(%DOWNSTREAM_PEER_CERT_V_END%)`,
 			}),
 			Entry("HOSTNAME", testCase{
 				field:    "HOSTNAME",
-				expected: `UNSUPPORTED_FIELD(HOSTNAME)`,
+				expected: `UNSUPPORTED_COMMAND(%HOSTNAME%)`,
+			}),
+		)
+	})
+
+	Describe("String()", func() {
+		type testCase struct {
+			field    string
+			expected string
+		}
+
+		DescribeTable("should return correct canonical representation",
+			func(given testCase) {
+				// setup
+				formatter := FieldFormatter(given.field)
+
+				// when
+				actual := formatter.String()
+				// then
+				Expect(actual).To(Equal(given.expected))
+
+			},
+			Entry("%BYTES_RECEIVED%", testCase{
+				field:    "BYTES_RECEIVED",
+				expected: `%BYTES_RECEIVED%`,
 			}),
 		)
 	})

diff --git a/pkg/envoy/accesslog/filter_state_formatter.go b/pkg/envoy/accesslog/filter_state_formatter.go
@@ -1,6 +1,9 @@
 package accesslog
 
 import (
+	"strconv"
+	"strings"
+
 	accesslog_config "github.com/envoyproxy/go-control-plane/envoy/config/accesslog/v2"
 	accesslog_data "github.com/envoyproxy/go-control-plane/envoy/data/accesslog/v2"
 )
@@ -21,7 +24,7 @@ func (f *FilterStateFormatter) FormatTcpLogEntry(entry *accesslog_data.TCPAccess
 
 func (f *FilterStateFormatter) format(entry *accesslog_data.AccessLogCommon) (string, error) {
 	// TODO(yskopets): implement
-	return "UNSUPPORTED(FILTER_STATE)", nil
+	return "UNSUPPORTED_COMMAND(%FILTER_STATE(KEY):Z%)", nil
 }
 
 func (f *FilterStateFormatter) ConfigureHttpLog(config *accesslog_config.HttpGrpcAccessLogConfig) error {
@@ -50,3 +53,17 @@ func (f *FilterStateFormatter) appendTo(values []string) []string {
 	}
 	return values
 }
+
+// String returns the canonical representation of this command operator.
+func (f *FilterStateFormatter) String() string {
+	var builder []string
+	builder = append(builder, "%FILTER_STATE(")
+	builder = append(builder, f.Key)
+	builder = append(builder, ")")
+	if f.MaxLength != 0 {
+		builder = append(builder, ":")
+		builder = append(builder, strconv.FormatInt(int64(f.MaxLength), 10))
+	}
+	builder = append(builder, "%")
+	return strings.Join(builder, "")
+}
diff --git a/pkg/envoy/accesslog/filter_state_formatter_test.go b/pkg/envoy/accesslog/filter_state_formatter_test.go
@@ -0,0 +1,45 @@
+package accesslog_test
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/gomega"
+
+	. "github.com/Kong/kuma/pkg/envoy/accesslog"
+)
+
+var _ = Describe("FilterStateFormatter", func() {
+
+	Describe("String()", func() {
+		type testCase struct {
+			key       string
+			maxLength int
+			expected  string
+		}
+
+		DescribeTable("should return correct canonical representation",
+			func(given testCase) {
+				// setup
+				formatter := &FilterStateFormatter{Key: given.key, MaxLength: given.maxLength}
+
+				// when
+				actual := formatter.String()
+				// then
+				Expect(actual).To(Equal(given.expected))
+
+			},
+			Entry("%FILTER_STATE()%", testCase{
+				expected: `%FILTER_STATE()%`,
+			}),
+			Entry("%FILTER_STATE(filter.state.key)%", testCase{
+				key:      "filter.state.key",
+				expected: `%FILTER_STATE(filter.state.key)%`,
+			}),
+			Entry("%FILTER_STATE(filter.state.key):10%", testCase{
+				key:       "filter.state.key",
+				maxLength: 10,
+				expected:  `%FILTER_STATE(filter.state.key):10%`,
+			}),
+		)
+	})
+})
diff --git a/pkg/envoy/accesslog/format_parser.go b/pkg/envoy/accesslog/format_parser.go
@@ -133,7 +133,7 @@ func (p formatParser) parseCommandOperator(token, command, args, limit string) (
 
 func (p formatParser) parseHeaderOperator(token, command, args, limit string) (header string, altHeader string, maxLen int, err error) {
 	if p.hasNoArguments(token, command, args, limit) {
-		return "", "", 0, errors.Errorf(`command %q requires a header and optional alternative header names as its arguments, instead got %q`, CommandOperatorName(command), token)
+		return "", "", 0, errors.Errorf(`command %q requires a header and optional alternative header names as its arguments, instead got %q`, CommandOperatorDescriptor(command), token)
 	}
 	header, altHeaders, maxLen, err := p.parseOperator(token, args, limit, "?")
 	if err != nil {
@@ -159,7 +159,7 @@ func (p formatParser) parseDynamicMetadataOperator(token, command, args, limit s
 		return "", nil, 0, err
 	}
 	if p.hasNoArguments(token, command, args, limit) {
-		return "", nil, 0, errors.Errorf(`command %q requires a filter namespace and optional path as its arguments, instead got %q`, CommandOperatorName(command), token)
+		return "", nil, 0, errors.Errorf(`command %q requires a filter namespace and optional path as its arguments, instead got %q`, CommandOperatorDescriptor(command), token)
 	}
 	return namespace, path, maxLen, err
 }
@@ -170,7 +170,7 @@ func (p formatParser) parseFilterStateOperator(token, command, args, limit strin
 		return "", 0, err
 	}
 	if p.hasNoArguments(token, command, args, limit) || key == "" {
-		return "", 0, errors.Errorf(`command %q requires a key as its argument, instead got %q`, CommandOperatorName(command), token)
+		return "", 0, errors.Errorf(`command %q requires a key as its argument, instead got %q`, CommandOperatorDescriptor(command), token)
 	}
 	return key, maxLen, nil
 }
@@ -186,7 +186,7 @@ func (p formatParser) parseStartTimeOperator(token, command, args, limit string)
 
 func (p formatParser) parseFieldOperator(token, command, args, limit string) (field string, err error) {
 	if token[1:len(token)-1] != command {
-		return "", errors.Errorf(`command %q doesn't support arguments or max length constraint, instead got %q`, CommandOperatorName(command), token)
+		return "", errors.Errorf(`command %q doesn't support arguments or max length constraint, instead got %q`, CommandOperatorDescriptor(command), token)
 	}
 	return command, nil
 }