refactoring: introduce GetService() and GetProtocol() methods to be u…

…sed by client code instead of Tags[ServiceTag] and Tags[ProtocolTag]

api/mesh/v1alpha1/dataplane_helpers.go

@@ -201,6 +201,28 @@ func (d *Dataplane_Networking_Gateway) MatchTags(selector TagSelector) bool {
 	return selector.Matches(d.Tags)
 }
 
+// GetService returns a service represented by this inbound interface.
+//
+// The purpose of this method is to encapsulate implementation detail
+// that service is modeled as a tag rather than a separate field.
+func (d *Dataplane_Networking_Inbound) GetService() string {
+	if d == nil {
+		return ""
+	}
+	return d.Tags[ServiceTag]
+}
+
+// GetProtocol returns a protocol supported by this inbound interface.
+//
+// The purpose of this method is to encapsulate implementation detail
+// that protocol is modeled as a tag rather than a separate field.
+func (d *Dataplane_Networking_Inbound) GetProtocol() string {
+	if d == nil {
+		return ""
+	}
+	return d.Tags[ProtocolTag]
+}
+
 func (d *Dataplane_Networking_Inbound) MatchTags(selector TagSelector) bool {
 	return selector.Matches(d.Tags)
 }

api/mesh/v1alpha1/dataplane_helpers_test.go

@@ -432,6 +432,77 @@ var _ = Describe("Dataplane_Networking_Outbound", func() {
 	)
 })
 
+var _ = Describe("Dataplane_Networking_Inbound", func() {
+
+	DescribeTable("GetService()", func() {
+
+		type testCase struct {
+			inbound  *Dataplane_Networking_Inbound
+			expected string
+		}
+
+		DescribeTable("should infer service name from `service` tag",
+			func(given testCase) {
+				Expect(given.inbound.GetService()).To(Equal(given.expected))
+			},
+			Entry("inbound is `nil`", testCase{
+				inbound:  nil,
+				expected: "",
+			}),
+			Entry("inbound has no `service` tag", testCase{
+				inbound:  &Dataplane_Networking_Inbound{},
+				expected: "",
+			}),
+			Entry("inbound has `service` tag", testCase{
+				inbound: &Dataplane_Networking_Inbound{
+					Tags: map[string]string{
+						"service": "backend",
+					},
+				},
+				expected: "backend",
+			}),
+		)
+	})
+
+	DescribeTable("GetProtocol()", func() {
+
+		type testCase struct {
+			inbound  *Dataplane_Networking_Inbound
+			expected string
+		}
+
+		DescribeTable("should infer protocol from `protocol` tag",
+			func(given testCase) {
+				Expect(given.inbound.GetProtocol()).To(Equal(given.expected))
+			},
+			Entry("inbound is `nil`", testCase{
+				inbound:  nil,
+				expected: "",
+			}),
+			Entry("inbound has no `protocol` tag", testCase{
+				inbound:  &Dataplane_Networking_Inbound{},
+				expected: "",
+			}),
+			Entry("inbound has `protocol` tag with a known value", testCase{
+				inbound: &Dataplane_Networking_Inbound{
+					Tags: map[string]string{
+						"protocol": "http",
+					},
+				},
+				expected: "http",
+			}),
+			Entry("inbound has `protocol` tag with an unknown value", testCase{
+				inbound: &Dataplane_Networking_Inbound{
+					Tags: map[string]string{
+						"protocol": "not-yet-supported-protocol",
+					},
+				},
+				expected: "not-yet-supported-protocol",
+			}),
+		)
+	})
+})
+
 var _ = Describe("Dataplane with inbound", func() {
 	d := Dataplane{
 		Networking: &Dataplane_Networking{

pkg/core/resources/apis/mesh/dataplane_helpers.go

@@ -122,15 +122,3 @@ func (d *DataplaneResource) GetIP() string {
 	}
 	return ifaces[0].DataplaneIP
 }
-
-// GetProtocol returns a protocol of an inbound interface with a given index.
-func (d *DataplaneResource) GetProtocol(inboundIdx int) Protocol {
-	if d == nil {
-		return ProtocolUnknown
-	}
-	if inboundIdx < 0 || inboundIdx > len(d.Spec.Networking.GetInbound())-1 {
-		return ProtocolUnknown
-	}
-	iface := d.Spec.Networking.Inbound[inboundIdx]
-	return ParseProtocol(iface.Tags[mesh_proto.ProtocolTag])
-}

pkg/core/resources/apis/mesh/dataplane_helpers_test.go

@@ -411,93 +411,6 @@ var _ = Describe("Dataplane", func() {
 			}),
 		)
 	})
-
-	Describe("GetProtocol()", func() {
-
-		type testCase struct {
-			dataplane string
-			index     int
-			expected  Protocol
-		}
-
-		DescribeTable("should return protocol of a given inbound interface",
-			func(given testCase) {
-				// setup
-				dataplane := &DataplaneResource{}
-				// when
-				err := util_proto.FromYAML([]byte(given.dataplane), &dataplane.Spec)
-				// then
-				Expect(err).ToNot(HaveOccurred())
-
-				// expect
-				Expect(dataplane.GetProtocol(given.index)).To(Equal(given.expected))
-			},
-			Entry("negative index", testCase{
-				index:    -1,
-				expected: ProtocolUnknown,
-			}),
-			Entry("too big index", testCase{
-				index:    2,
-				expected: ProtocolUnknown,
-			}),
-			Entry("no `protocol` tag", testCase{
-				dataplane: `
-                networking:
-                  inbound:
-                  - interface: 192.168.0.1:80:8080
-                    tags:
-                      service: backend
-`,
-				index:    0,
-				expected: ProtocolUnknown,
-			}),
-			Entry("`protocol: unsupported`", testCase{
-				dataplane: `
-                networking:
-                  inbound:
-                  - interface: 192.168.0.1:80:8080
-                    tags:
-                      service: backend
-                      protocol: unsupported
-                  - interface: 192.168.0.1:443:8443
-                    tags:
-                      service: backend-https
-`,
-				index:    0,
-				expected: ProtocolUnknown,
-			}),
-			Entry("`protocol: http`", testCase{
-				dataplane: `
-                networking:
-                  inbound:
-                  - interface: 192.168.0.1:443:8443
-                    tags:
-                      service: backend-https
-                  - interface: 192.168.0.1:80:8080
-                    tags:
-                      service: backend
-                      protocol: http
-`,
-				index:    1,
-				expected: ProtocolHTTP,
-			}),
-			Entry("`protocol: tcp`", testCase{
-				dataplane: `
-                networking:
-                  inbound:
-                  - interface: 192.168.0.1:80:8080
-                    tags:
-                      service: backend
-                      protocol: tcp
-                  - interface: 192.168.0.1:443:8443
-                    tags:
-                      service: backend-https
-`,
-				index:    0,
-				expected: ProtocolTCP,
-			}),
-		)
-	})
 })
 
 var _ = Describe("ParseProtocol()", func() {

pkg/xds/envoy/listeners/http_inbound_route_configurer.go

@@ -1,6 +1,8 @@
 package listeners
 
 import (
+	"fmt"
+
 	"github.com/golang/protobuf/proto"
 	wrappers "github.com/golang/protobuf/ptypes/wrappers"
 
@@ -10,24 +12,41 @@ import (
 	envoy_wellknown "github.com/envoyproxy/go-control-plane/pkg/wellknown"
 )
 
-func HttpInboundRoute(cluster ClusterInfo) ListenerBuilderOpt {
+func HttpInboundRoute(service string, cluster ClusterInfo) ListenerBuilderOpt {
 	return ListenerBuilderOptFunc(func(config *ListenerBuilderConfig) {
 		config.Add(&HttpInboundRouteConfigurer{
+			service: service,
 			cluster: cluster,
 		})
 	})
 }
 
 type HttpInboundRouteConfigurer struct {
+	service string
 	// Cluster to forward traffic to.
 	cluster ClusterInfo
 }
 
 func (c *HttpInboundRouteConfigurer) Configure(l *v2.Listener) error {
-	config := &v2.RouteConfiguration{
-		Name: "inbound",
+	routeConfig := c.routeConfiguration()
+
+	return UpdateFilterConfig(l, envoy_wellknown.HTTPConnectionManager, func(filterConfig proto.Message) error {
+		hcm, ok := filterConfig.(*envoy_hcm.HttpConnectionManager)
+		if !ok {
+			return NewUnexpectedFilterConfigTypeError(filterConfig, &envoy_hcm.HttpConnectionManager{})
+		}
+		hcm.RouteSpecifier = &envoy_hcm.HttpConnectionManager_RouteConfig{
+			RouteConfig: routeConfig,
+		}
+		return nil
+	})
+}
+
+func (c *HttpInboundRouteConfigurer) routeConfiguration() *v2.RouteConfiguration {
+	return &v2.RouteConfiguration{
+		Name: fmt.Sprintf("inbound:%s", c.service),
 		VirtualHosts: []*envoy_route.VirtualHost{{
-			Name:    "local_service",
+			Name:    c.service,
 			Domains: []string{"*"},
 			Routes: []*envoy_route.Route{{
 				Match: &envoy_route.RouteMatch{
@@ -48,15 +67,4 @@ func (c *HttpInboundRouteConfigurer) Configure(l *v2.Listener) error {
 			Value: true,
 		},
 	}
-
-	return UpdateFilterConfig(l, envoy_wellknown.HTTPConnectionManager, func(filterConfig proto.Message) error {
-		hcm, ok := filterConfig.(*envoy_hcm.HttpConnectionManager)
-		if !ok {
-			return NewUnexpectedFilterConfigTypeError(filterConfig, &envoy_hcm.HttpConnectionManager{})
-		}
-		hcm.RouteSpecifier = &envoy_hcm.HttpConnectionManager_RouteConfig{
-			RouteConfig: config,
-		}
-		return nil
-	})
 }

pkg/xds/envoy/listeners/http_inbound_route_configurer_test.go

@@ -17,6 +17,7 @@ var _ = Describe("HttpInboundRouteConfigurer", func() {
 		listenerAddress string
 		listenerPort    uint32
 		statsName       string
+		service         string
 		cluster         ClusterInfo
 		expected        string
 	}
@@ -27,7 +28,7 @@ var _ = Describe("HttpInboundRouteConfigurer", func() {
 			listener, err := NewListenerBuilder().
 				Configure(InboundListener(given.listenerName, given.listenerAddress, given.listenerPort)).
 				Configure(HttpConnectionManager(given.statsName)).
-				Configure(HttpInboundRoute(given.cluster)).
+				Configure(HttpInboundRoute(given.service, given.cluster)).
 				Build()
 			// then
 			Expect(err).ToNot(HaveOccurred())
@@ -43,6 +44,7 @@ var _ = Describe("HttpInboundRouteConfigurer", func() {
 			listenerAddress: "192.168.0.1",
 			listenerPort:    8080,
 			statsName:       "localhost:8080",
+			service:         "backend",
 			cluster:         ClusterInfo{Name: "localhost:8080", Weight: 200},
 			expected: `
             name: inbound:192.168.0.1:8080
@@ -58,12 +60,12 @@ var _ = Describe("HttpInboundRouteConfigurer", func() {
                   httpFilters:
                   - name: envoy.router
                   routeConfig:
-                    name: inbound
+                    name: inbound:backend
                     validateClusters: true
                     virtualHosts:
                     - domains:
                       - '*'
-                      name: local_service
+                      name: backend
                       routes:
                       - match:
                           prefix: /

pkg/xds/envoy/listeners/tcp_proxy_configurer.go

@@ -33,11 +33,31 @@ type TcpProxyConfigurer struct {
 }
 
 func (c *TcpProxyConfigurer) Configure(l *v2.Listener) error {
-	config := &envoy_tcp.TcpProxy{
+	tcpProxy := c.tcpProxy()
+
+	pbst, err := ptypes.MarshalAny(tcpProxy)
+	if err != nil {
+		return err
+	}
+
+	for i := range l.FilterChains {
+		l.FilterChains[i].Filters = append(l.FilterChains[i].Filters, &envoy_listener.Filter{
+			Name: envoy_wellknown.TCPProxy,
+			ConfigType: &envoy_listener.Filter_TypedConfig{
+				TypedConfig: pbst,
+			},
+		})
+	}
+
+	return nil
+}
+
+func (c *TcpProxyConfigurer) tcpProxy() *envoy_tcp.TcpProxy {
+	proxy := envoy_tcp.TcpProxy{
 		StatPrefix: util_xds.SanitizeMetric(c.statsName),
 	}
 	if len(c.clusters) == 1 {
-		config.ClusterSpecifier = &envoy_tcp.TcpProxy_Cluster{
+		proxy.ClusterSpecifier = &envoy_tcp.TcpProxy_Cluster{
 			Cluster: c.clusters[0].Name,
 		}
 	} else {
@@ -48,25 +68,11 @@ func (c *TcpProxyConfigurer) Configure(l *v2.Listener) error {
 				Weight: cluster.Weight,
 			})
 		}
-		config.ClusterSpecifier = &envoy_tcp.TcpProxy_WeightedClusters{
+		proxy.ClusterSpecifier = &envoy_tcp.TcpProxy_WeightedClusters{
 			WeightedClusters: &envoy_tcp.TcpProxy_WeightedCluster{
 				Clusters: weightedClusters,
 			},
 		}
 	}
-	pbst, err := ptypes.MarshalAny(config)
-	if err != nil {
-		return err
-	}
-
-	for i := range l.FilterChains {
-		l.FilterChains[i].Filters = append(l.FilterChains[i].Filters, &envoy_listener.Filter{
-			Name: envoy_wellknown.TCPProxy,
-			ConfigType: &envoy_listener.Filter_TypedConfig{
-				TypedConfig: pbst,
-			},
-		})
-	}
-
-	return nil
+	return &proxy
 }