Repository: containous/maesh
Pull request #705 (2020-08-18 12:52:20)

Add support for header-based filtering on TrafficTargets
## What does this PR do?

This PR adds support for header-based filtering on TrafficTargets. As described in the specification, an HTTPRouteGroup can define a set of headers that must match to pass through.

https://github.com/servicemeshinterface/smi-spec/blob/master/apis/traffic-specs/v1alpha3/traffic-specs.md#http-header-filters

```
kind: HTTPRouteGroup
metadata:
  name: the-routes
  namespace: default
spec:
  matches:
  - name: android-insiders
    headers:
    - user-agent: ".*Android.*"
    - cookie: "^(.*?;)?(type=insider)(;.*)?$"
```

### How to test it

* `make test`
* `make test-integration`
* Install maesh and play with this feature. Instruction on how to use this new filter can be found in the specification


Patch:
diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go
index 7140b5a..23b0a59 100644
--- a/pkg/provider/provider.go
+++ b/pkg/provider/provider.go
@@ -353,7 +353,7 @@ func (p *Provider) buildHTTPServicesAndRoutersForTrafficTarget(t *topology.Topol
 }
 
 func (p *Provider) buildTCPServicesAndRoutersForTrafficTarget(t *topology.Topology, tt *topology.ServiceTrafficTarget, cfg *dynamic.Configuration, ttSvc *topology.Service, ttKey topology.ServiceTrafficTargetKey) {
-	if !hasTrafficTargetSpecTCPRoute(tt) {
+	if !hasTrafficTargetRuleTCPRoute(tt) {
 		return
 	}
 
@@ -914,9 +914,9 @@ func buildUDPRouter(entrypoint string, svcKey string) *dynamic.UDPRouter {
 	}
 }
 
-func hasTrafficTargetSpecTCPRoute(tt *topology.ServiceTrafficTarget) bool {
-	for _, spec := range tt.Specs {
-		if spec.TCPRoute != nil {
+func hasTrafficTargetRuleTCPRoute(tt *topology.ServiceTrafficTarget) bool {
+	for _, rule := range tt.Rules {
+		if rule.TCPRoute != nil {
 			return true
 		}
 	}
diff --git a/pkg/provider/rule.go b/pkg/provider/rule.go
index 3ea937b..2f0ef05 100644
--- a/pkg/provider/rule.go
+++ b/pkg/provider/rule.go
@@ -11,8 +11,8 @@ import (
 func buildTrafficTargetRule(tt *topology.ServiceTrafficTarget) string {
 	var orRules []string
 
-	for _, spec := range tt.Specs {
-		for _, match := range spec.HTTPMatches {
+	for _, rule := range tt.Rules {
+		for _, match := range rule.HTTPMatches {
 			var matchParts []string
 
 			// Handle Path filtering.
@@ -21,6 +21,9 @@ func buildTrafficTargetRule(tt *topology.ServiceTrafficTarget) string {
 			// Handle Method filtering.
 			matchParts = appendMethodFilter(matchParts, match)
 
+			// Handle Header filtering.
+			matchParts = appendHeaderFilter(matchParts, match)
+
 			// Conditions within a HTTPMatch must all be fulfilled to be considered valid.
 			if len(matchParts) > 0 {
 				matchCond := strings.Join(matchParts, " && ")
@@ -72,6 +75,20 @@ func appendMethodFilter(matchParts []string, match *specs.HTTPMatch) []string {
 	return matchParts
 }
 
+func appendHeaderFilter(matchParts []string, match *specs.HTTPMatch) []string {
+	rules := make([]string, 0, len(match.Headers))
+
+	for name, value := range match.Headers {
+		rules = append(rules, fmt.Sprintf("HeadersRegexp(`%s`, `%s`)", name, value))
+	}
+
+	if len(rules) > 0 {
+		matchParts = append(matchParts, strings.Join(rules, " && "))
+	}
+
+	return matchParts
+}
+
 func buildHTTPRuleFromService(svc *topology.Service) string {
 	return fmt.Sprintf("Host(`%s.%s.maesh`) || Host(`%s`)", svc.Name, svc.Namespace, svc.ClusterIP)
 }
diff --git a/pkg/topology/builder.go b/pkg/topology/builder.go
index 3c1c5dd..5b96cd8 100644
--- a/pkg/topology/builder.go
+++ b/pkg/topology/builder.go
@@ -146,7 +146,7 @@ func (b *Builder) evaluateTrafficTarget(res *resources, topology *Topology, tt *
 
 		var err error
 
-		stt.Specs, err = b.buildTrafficTargetSpecs(res, tt)
+		stt.Rules, err = b.buildTrafficTargetRules(res, tt)
 		if err != nil {
 			err = fmt.Errorf("unable to build spec: %v", err)
 			stt.AddError(err)
@@ -465,7 +465,7 @@ func (b *Builder) buildTrafficTargetSources(res *resources, t *Topology, tt *acc
 	return sources
 }
 
-func (b *Builder) buildTrafficTargetSpecs(res *resources, tt *access.TrafficTarget) ([]TrafficSpec, error) {
+func (b *Builder) buildTrafficTargetRules(res *resources, tt *access.TrafficTarget) ([]TrafficSpec, error) {
 	var trafficSpecs []TrafficSpec
 
 	for _, s := range tt.Spec.Rules {
diff --git a/pkg/topology/topology.go b/pkg/topology/topology.go
index 5ebde08..1d970f2 100644
--- a/pkg/topology/topology.go
+++ b/pkg/topology/topology.go
@@ -164,7 +164,7 @@ type ServiceTrafficTarget struct {
 
 	Sources     []ServiceTrafficTargetSource    `json:"sources,omitempty"`
 	Destination ServiceTrafficTargetDestination `json:"destination"`
-	Specs       []TrafficSpec                   `json:"specs,omitempty"`
+	Rules       []TrafficSpec                   `json:"rules,omitempty"`
 
 	Errors []string `json:"errors"`
 }
@@ -193,7 +193,7 @@ type ServiceTrafficTargetDestination struct {
 	Pods           []Key                `json:"pods,omitempty"`
 }
 
-// TrafficSpec represents a Spec which can be used for restricting access to a route in a TrafficTarget.
+// TrafficSpec represents a spec which can be used for restricting access to a route in a TrafficTarget.
 type TrafficSpec struct {
 	HTTPRouteGroup *specs.HTTPRouteGroup `json:"httpRouteGroup,omitempty"`
 	TCPRoute       *specs.TCPRoute       `json:"tcpRoute,omitempty"`


Test patch:
diff --git a/pkg/provider/provider_test.go b/pkg/provider/provider_test.go
index bb5b97b..4a1e96b 100644
--- a/pkg/provider/provider_test.go
+++ b/pkg/provider/provider_test.go
@@ -108,6 +108,13 @@ func TestProvider_BuildConfig(t *testing.T) {
 			topology:   "testdata/acl-enabled-tcp-basic-topology.json",
 			wantConfig: "testdata/acl-enabled-tcp-basic-config.json",
 		},
+		{
+			desc:               "ACL enabled: HTTP service with http-route-group",
+			acl:                true,
+			defaultTrafficType: "http",
+			topology:           "testdata/acl-enabled-http-route-group-topology.json",
+			wantConfig:         "testdata/acl-enabled-http-route-group-config.json",
+		},
 		{
 			desc:               "ACL enabled: HTTP service with traffic-split",
 			acl:                true,
diff --git a/pkg/provider/testdata/acl-enabled-http-route-group-config.json b/pkg/provider/testdata/acl-enabled-http-route-group-config.json
new file mode 100644
index 0000000..5f86c21
--- /dev/null
+++ b/pkg/provider/testdata/acl-enabled-http-route-group-config.json
@@ -0,0 +1,78 @@
+{
+  "http": {
+    "routers": {
+      "my-ns-svc-b-8080": {
+        "entryPoints": [
+          "http-10000"
+        ],
+        "middlewares": [
+          "block-all-middleware"
+        ],
+        "service": "block-all-service",
+        "rule": "Host(`svc-b.my-ns.maesh`) || Host(`10.10.14.1`)",
+        "priority": 1
+      },
+      "my-ns-svc-b-tt-8080-traffic-target-direct": {
+        "entryPoints": [
+          "http-10000"
+        ],
+        "middlewares": [
+          "my-ns-svc-b-tt-whitelist-traffic-target-direct"
+        ],
+        "service": "my-ns-svc-b-tt-8080-traffic-target",
+        "rule": "(Host(`svc-b.my-ns.maesh`) || Host(`10.10.14.1`)) && (PathPrefix(`/{path:app}`) || (PathPrefix(`/{path:api/notifications}`) && Method(`GET`)) || HeadersRegexp(`User-Agent`, `Mozilla/.*`))",
+        "priority": 2005
+      },
+      "readiness": {
+        "entryPoints": [
+          "readiness"
+        ],
+        "service": "readiness",
+        "rule": "Path(`/ping`)"
+      }
+    },
+    "services": {
+      "block-all-service": {
+        "loadBalancer": {
+          "passHostHeader": false
+        }
+      },
+      "my-ns-svc-b-tt-8080-traffic-target": {
+        "loadBalancer": {
+          "servers": [
+            {
+              "url": "http://10.10.3.1:8080"
+            }
+          ],
+          "passHostHeader": true
+        }
+      },
+      "readiness": {
+        "loadBalancer": {
+          "servers": [
+            {
+              "url": "http://127.0.0.1:8080"
+            }
+          ],
+          "passHostHeader": true
+        }
+      }
+    },
+    "middlewares": {
+      "block-all-middleware": {
+        "ipWhiteList": {
+          "sourceRange": [
+            "255.255.255.255"
+          ]
+        }
+      },
+      "my-ns-svc-b-tt-whitelist-traffic-target-direct": {
+        "ipWhiteList": {
+          "sourceRange": [
+            "10.10.2.1"
+          ]
+        }
+      }
+    }
+  }
+}
diff --git a/pkg/provider/testdata/acl-enabled-http-route-group-topology.json b/pkg/provider/testdata/acl-enabled-http-route-group-topology.json
new file mode 100644
index 0000000..4a40e5d
--- /dev/null
+++ b/pkg/provider/testdata/acl-enabled-http-route-group-topology.json
@@ -0,0 +1,153 @@
+{
+  "services": {
+    "svc-b@my-ns": {
+      "name": "svc-b",
+      "namespace": "my-ns",
+      "selector": {},
+      "annotations": {},
+      "ports": [
+        {
+          "name": "port-8080",
+          "protocol": "TCP",
+          "port": 8080,
+          "targetPort": 8080
+        }
+      ],
+      "clusterIp": "10.10.14.1",
+      "pods": [
+        "pod-b@my-ns"
+      ],
+      "trafficTargets": [
+        "svc-b@my-ns:tt@my-ns"
+      ]
+    }
+  },
+  "pods": {
+    "pod-a@my-ns": {
+      "name": "pod-a",
+      "namespace": "my-ns",
+      "serviceAccount": "client",
+      "ip": "10.10.2.1"
+    },
+    "pod-b@my-ns": {
+      "name": "pod-b",
+      "namespace": "my-ns",
+      "serviceAccount": "server",
+      "ip": "10.10.3.1",
+      "containerPorts": [
+        {
+          "name": "web",
+          "protocol": "TCP",
+          "containerPort": 8081
+        }
+      ]
+    }
+  },
+  "serviceTrafficTargets": {
+    "svc-b@my-ns:tt@my-ns": {
+      "service": "svc-b@my-ns",
+      "name": "tt",
+      "namespace": "my-ns",
+      "sources": [
+        {
+          "serviceAccount": "client",
+          "namespace": "my-ns",
+          "pods": [
+            "pod-a@my-ns"
+          ]
+        }
+      ],
+      "destination": {
+        "serviceAccount": "server",
+        "namespace": "my-ns",
+        "ports": [
+          {
+            "name": "port-8080",
+            "protocol": "TCP",
+            "port": 8080,
+            "targetPort": 8080
+          }
+        ],
+        "pods": [
+          "pod-b@my-ns"
+        ]
+      },
+      "rules": [
+        {
+          "httpRouteGroup": {
+            "kind": "HTTPRouteGroup",
+            "apiVersion": "specs.smi-spec.io/v1alpha3",
+            "metadata": {
+              "name": "app-route-group",
+              "namespace": "my-ns"
+            },
+            "spec": {
+              "matches": [
+                {
+                  "name": "app",
+                  "methods": ["*"],
+                  "pathRegex": "/app"
+                }
+              ]
+            }
+          },
+          "httpMatches": [
+            {
+              "name": "app",
+              "methods": ["*"],
+              "pathRegex": "/app"
+            }
+          ]
+        },
+        {
+          "httpRouteGroup": {
+            "kind": "HTTPRouteGroup",
+            "apiVersion": "specs.smi-spec.io/v1alpha3",
+            "metadata": {
+              "name": "app-route-group",
+              "namespace": "my-ns"
+            },
+            "spec": {
+              "matches": [
+                {
+                  "name": "users",
+                  "methods": ["GET", "POST", "PUT"],
+                  "pathRegex": "/api/users"
+                },
+                {
+                  "name": "notifications",
+                  "methods": ["GET"],
+                  "pathRegex": "/api/notifications"
+                },
+                {
+                  "name": "firefox-beta",
+                  "headers": [
+                    {
+                      "User-Agent": "Mozilla/.*"
+                    }
+                  ]
+                }
+              ]
+            }
+          },
+          "httpMatches": [
+            {
+              "name": "notifications",
+              "methods": ["GET"],
+              "pathRegex": "/api/notifications"
+            },
+            {
+              "name": "firefox-beta",
+              "headers": [
+                {
+                  "User-Agent": "Mozilla/.*"
+                }
+              ]
+            }
+          ]
+        }
+      ]
+    }
+  },
+  "trafficSplits": {}
+}
diff --git a/pkg/provider/testdata/acl-enabled-tcp-basic-topology.json b/pkg/provider/testdata/acl-enabled-tcp-basic-topology.json
index 83e8740..cfd7bc8 100644
--- a/pkg/provider/testdata/acl-enabled-tcp-basic-topology.json
+++ b/pkg/provider/testdata/acl-enabled-tcp-basic-topology.json
@@ -54,7 +54,7 @@
       "service": "svc-b@my-ns",
       "name": "tt",
       "namespace": "my-ns",
-      "specs": [
+      "rules": [
         {
           "tcpRoute": {
             "kind": "TCPRoute",
diff --git a/pkg/topology/builder_test.go b/pkg/topology/builder_test.go
index 09c2702..099bc5d 100644
--- a/pkg/topology/builder_test.go
+++ b/pkg/topology/builder_test.go
@@ -55,8 +55,8 @@ func TestTopologyBuilder_BuildIgnoresNamespaces(t *testing.T) {
 	svcC := createService("ignored-ns", "svc-c", annotations, svccPorts, selectorAppA, "10.10.1.17")
 	svcD := createService("ignored-ns", "svc-d", annotations, svcdPorts, selectorAppA, "10.10.1.18")
 
-	apiMatch := createHTTPMatch("api", []string{"GET", "POST"}, "/api")
-	metricMatch := createHTTPMatch("metric", []string{"GET"}, "/metric")
+	apiMatch := createHTTPMatch("api", []string{"GET", "POST"}, "/api", nil)
+	metricMatch := createHTTPMatch("metric", []string{"GET"}, "/metric", nil)
 	rtGrp := createHTTPRouteGroup("ignored-ns", "http-rt-grp-ignored", []specs.HTTPMatch{apiMatch, metricMatch})
 
 	tt := createTrafficTarget("ignored-ns", "tt", saB, intPtr(8080), []*corev1.ServiceAccount{saA}, rtGrp, []string{})
@@ -120,8 +120,8 @@ func TestTopologyBuilder_HandleCircularReferenceOnTrafficSplit(t *testing.T) {
 	epD := createEndpoints(svcD, createEndpointSubset(svcPorts, podD))
 	epE := createEndpoints(svcE, createEndpointSubset(svcPorts, podE))
 
-	apiMatch := createHTTPMatch("api", []string{"GET", "POST"}, "/api")
-	metricMatch := createHTTPMatch("metric", []string{"GET"}, "/metric")
+	apiMatch := createHTTPMatch("api", []string{"GET", "POST"}, "/api", nil)
+	metricMatch := createHTTPMatch("metric", []string{"GET"}, "/metric", nil)
 	rtGrp := createHTTPRouteGroup("my-ns", "http-rt-grp", []specs.HTTPMatch{apiMatch, metricMatch})
 
 	ttMatch := []string{apiMatch.Name}
@@ -184,8 +184,8 @@ func TestTopologyBuilder_TrafficTargetSourcesForbiddenTrafficSplit(t *testing.T)
 	epC := createEndpoints(svcC, createEndpointSubset(svcPorts, podC))
 	epD := createEndpoints(svcD, createEndpointSubset(svcPorts, podD))
 
-	apiMatch := createHTTPMatch("api", []string{"GET", "POST"}, "/api")
-	metricMatch := createHTTPMatch("metric", []string{"GET"}, "/metric")
+	apiMatch := createHTTPMatch("api", []string{"GET", "POST"}, "/api", nil)
+	metricMatch := createHTTPMatch("metric", []string{"GET"}, "/metric", nil)
 	rtGrp := createHTTPRouteGroup("my-ns", "http-rt-grp", []specs.HTTPMatch{apiMatch, metricMatch})
 
 	ttMatch := []string{apiMatch.Name}
@@ -249,8 +249,8 @@ func TestTopologyBuilder_EvaluatesIncomingTrafficSplit(t *testing.T) {
 	epD := createEndpoints(svcD, createEndpointSubset(svcPorts, podD))
 	epE := createEndpoints(svcE, createEndpointSubset(svcPorts, podE))
 
-	apiMatch := createHTTPMatch("api", []string{"GET", "POST"}, "/api")
-	metricMatch := createHTTPMatch("metric", []string{"GET"}, "/metric")
+	apiMatch := createHTTPMatch("api", []string{"GET", "POST"}, "/api", nil)
+	metricMatch := createHTTPMatch("metric", []string{"GET"}, "/metric", nil)
 	rtGrp := createHTTPRouteGroup("my-ns", "http-rt-grp", []specs.HTTPMatch{apiMatch, metricMatch})
 
 	ttMatch := []string{apiMatch.Name}
@@ -310,8 +310,10 @@ func TestTopologyBuilder_BuildWithTrafficTarget(t *testing.T) {
 
 	epB := createEndpoints(svcB, createEndpointSubset(svcPorts, podB))
 
-	apiMatch := createHTTPMatch("api", []string{"GET", "POST"}, "/api")
-	metricMatch := createHTTPMatch("metric", []string{"GET"}, "/metric")
+	metricMatch := createHTTPMatch("metric", []string{"GET"}, "/metric", nil)
+	apiMatch := createHTTPMatch("api", []string{"GET", "POST"}, "/api", map[string]string{
+		"User-Agent": "curl/.*",
+	})
 	rtGrp := createHTTPRouteGroup("my-ns", "http-rt-grp", []specs.HTTPMatch{apiMatch, metricMatch})
 
 	ttMatch := []string{apiMatch.Name}
@@ -369,8 +371,8 @@ func TestTopologyBuilder_BuildWithTrafficTargetAndTrafficSplitOnSameService(t *t
 	epC := createEndpoints(svcC, createEndpointSubset(svcPorts, podC))
 	epD := createEndpoints(svcD, createEndpointSubset(svcPorts, podD))
 
-	apiMatch := createHTTPMatch("api", []string{"GET", "POST"}, "/api")
-	metricMatch := createHTTPMatch("metric", []string{"GET"}, "/metric")
+	apiMatch := createHTTPMatch("api", []string{"GET", "POST"}, "/api", nil)
+	metricMatch := createHTTPMatch("metric", []string{"GET"}, "/metric", nil)
 	rtGrp := createHTTPRouteGroup("my-ns", "http-rt-grp", []specs.HTTPMatch{apiMatch, metricMatch})
 
 	ttMatch := []string{apiMatch.Name}
@@ -413,8 +415,8 @@ func TestTopologyBuilder_BuildWithTrafficTargetSpecEmptyMatch(t *testing.T) {
 
 	epB := createEndpoints(svcB, createEndpointSubset(svcbPorts, podB))
 
-	apiMatch := createHTTPMatch("api", []string{"GET", "POST"}, "/api")
-	metricMatch := createHTTPMatch("metric", []string{"GET"}, "/metric")
+	apiMatch := createHTTPMatch("api", []string{"GET", "POST"}, "/api", nil)
+	metricMatch := createHTTPMatch("metric", []string{"GET"}, "/metric", nil)
 	rtGrp := createHTTPRouteGroup("my-ns", "http-rt-grp", []specs.HTTPMatch{apiMatch, metricMatch})
 
 	tt := createTrafficTarget("my-ns", "tt", saB, intPtr(8080), []*corev1.ServiceAccount{saA}, rtGrp, []string{})
@@ -816,11 +818,12 @@ func createHTTPRouteGroup(namespace, name string, matches []specs.HTTPMatch) *sp
 	}
 }
 
-func createHTTPMatch(name string, methods []string, pathPrefix string) specs.HTTPMatch {
+func createHTTPMatch(name string, methods []string, pathPrefix string, headers map[string]string) specs.HTTPMatch {
 	return specs.HTTPMatch{
 		Name:      name,
 		Methods:   methods,
 		PathRegex: pathPrefix,
+		Headers:   headers,
 	}
 }
 
diff --git a/pkg/topology/testdata/topology-spec-with-empty-match.json b/pkg/topology/testdata/topology-spec-with-empty-match.json
index 87f5cd3..dad78c0 100644
--- a/pkg/topology/testdata/topology-spec-with-empty-match.json
+++ b/pkg/topology/testdata/topology-spec-with-empty-match.json
@@ -74,7 +74,7 @@
           "app-b@my-ns"
         ]
       },
-      "specs": [
+      "rules": [
         {
           "httpRouteGroup": {
             "kind": "HTTPRouteGroup",
diff --git a/pkg/topology/testdata/topology-traffic-split-traffic-target.json b/pkg/topology/testdata/topology-traffic-split-traffic-target.json
index fc476c5..9135bc2 100644
--- a/pkg/topology/testdata/topology-traffic-split-traffic-target.json
+++ b/pkg/topology/testdata/topology-traffic-split-traffic-target.json
@@ -176,7 +176,7 @@
           "app-b2@my-ns"
         ]
       },
-      "specs": [
+      "rules": [
         {
           "httpRouteGroup": {
             "kind": "HTTPRouteGroup",
diff --git a/pkg/topology/testdata/topology-traffic-target.json b/pkg/topology/testdata/topology-traffic-target.json
index 39551e1..5e051f5 100644
--- a/pkg/topology/testdata/topology-traffic-target.json
+++ b/pkg/topology/testdata/topology-traffic-target.json
@@ -73,7 +73,7 @@
           "app-b@my-ns"
         ]
       },
-      "specs": [
+      "rules": [
         {
           "httpRouteGroup": {
             "kind": "HTTPRouteGroup",
@@ -91,7 +91,12 @@
                     "GET",
                     "POST"
                   ],
-                  "pathRegex": "/api"
+                  "pathRegex": "/api",
+                  "headers": [
+                    {
+                      "User-Agent": "curl/.*"
+                    }
+                  ]
                 },
                 {
                   "name": "metric",
@@ -110,7 +115,12 @@
                 "GET",
                 "POST"
               ],
-              "pathRegex": "/api"
+              "pathRegex": "/api",
+              "headers": [
+                {
+                  "User-Agent": "curl/.*"
+                }
+              ]
             }
           ]
         }