Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Commit

Permalink
Rename Route type to HTTPRoute where applicable (#1612)
Browse files Browse the repository at this point in the history
Currently the code refers to an HTTPRoute as a Route in a traffic target
policy. Rename this to HTTPRoute to be explicit. In the future, a route
for different traffic types will be supported, ex. TCPRoute.

This also helps disambiguate the HTTP route type in the traffic target from
the XDS Route type in Envoy.

Signed-off-by: Shashank Ram <shashank08@gmail.com>
  • Loading branch information
shashankram authored Aug 24, 2020
1 parent b4e3fce commit 930991c
Show file tree
Hide file tree
Showing 11 changed files with 65 additions and 65 deletions.
6 changes: 3 additions & 3 deletions DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ type MeshCataloger interface {
// ExpectProxy catalogs the fact that a certificate was issued for an Envoy proxy and this is expected to connect to XDS.
ExpectProxy(certificate.CommonName)
// GetServicesFromEnvoyCertificate returns a list of services the given Envoy is a member of based on the certificate provided,
// GetServicesFromEnvoyCertificate returns a list of services the given Envoy is a member of based on the certificate provided,
// which is a cert issued to an Envoy for XDS communication (not Envoy-to-Envoy).
GetServicesFromEnvoyCertificate(certificate.CommonName) ([]service.MeshService, error)
Expand All @@ -317,8 +317,8 @@ type MeshCataloger interface {
//GetWeightedClusterForService returns the weighted cluster for a service
GetWeightedClusterForService(service service.MeshService) (service.WeightedCluster, error)
// GetIngressRoutesPerHost returns the routes per host associated with an ingress service
GetIngressRoutesPerHost(service.MeshService) (map[string][]trafficpolicy.Route, error)
// GetIngressRoutesPerHost returns the HTTP routes per host associated with an ingress service
GetIngressRoutesPerHost(service.MeshService) (map[string][]trafficpolicy.HTTPRoute, error)
}
```

Expand Down
8 changes: 4 additions & 4 deletions pkg/catalog/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (
)

// GetIngressRoutesPerHost returns routes per host as defined in observed ingress k8s resources.
func (mc *MeshCatalog) GetIngressRoutesPerHost(service service.MeshService) (map[string][]trafficpolicy.Route, error) {
domainRoutesMap := make(map[string][]trafficpolicy.Route)
func (mc *MeshCatalog) GetIngressRoutesPerHost(service service.MeshService) (map[string][]trafficpolicy.HTTPRoute, error) {
domainRoutesMap := make(map[string][]trafficpolicy.HTTPRoute)
ingresses, err := mc.ingressMonitor.GetIngressResources(service)
if err != nil {
log.Error().Err(err).Msgf("Failed to get ingress resources with backend %s", service)
Expand All @@ -18,14 +18,14 @@ func (mc *MeshCatalog) GetIngressRoutesPerHost(service service.MeshService) (map
return domainRoutesMap, err
}

defaultRoute := trafficpolicy.Route{
defaultRoute := trafficpolicy.HTTPRoute{
PathRegex: constants.RegexMatchAll,
Methods: []string{constants.RegexMatchAll},
}

for _, ingress := range ingresses {
if ingress.Spec.Backend != nil && ingress.Spec.Backend.ServiceName == service.Name {
domainRoutesMap[constants.WildcardHTTPMethod] = []trafficpolicy.Route{defaultRoute}
domainRoutesMap[constants.WildcardHTTPMethod] = []trafficpolicy.HTTPRoute{defaultRoute}
}

for _, rule := range ingress.Spec.Rules {
Expand Down
18 changes: 9 additions & 9 deletions pkg/catalog/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,8 @@ func (mc *MeshCatalog) getServiceHostnames(meshService service.MeshService) ([]s
return hostnames, nil
}

func (mc *MeshCatalog) getHTTPPathsPerRoute() (map[trafficpolicy.TrafficSpecName]map[trafficpolicy.TrafficSpecMatchName]trafficpolicy.Route, error) {
routePolicies := make(map[trafficpolicy.TrafficSpecName]map[trafficpolicy.TrafficSpecMatchName]trafficpolicy.Route)
func (mc *MeshCatalog) getHTTPPathsPerRoute() (map[trafficpolicy.TrafficSpecName]map[trafficpolicy.TrafficSpecMatchName]trafficpolicy.HTTPRoute, error) {
routePolicies := make(map[trafficpolicy.TrafficSpecName]map[trafficpolicy.TrafficSpecMatchName]trafficpolicy.HTTPRoute)
for _, trafficSpecs := range mc.meshSpec.ListHTTPTrafficSpecs() {
log.Debug().Msgf("Discovered TrafficSpec resource: %s/%s", trafficSpecs.Namespace, trafficSpecs.Name)
if trafficSpecs.Spec.Matches == nil {
Expand All @@ -197,9 +197,9 @@ func (mc *MeshCatalog) getHTTPPathsPerRoute() (map[trafficpolicy.TrafficSpecName

// since this method gets only specs related to HTTPRouteGroups added HTTPTraffic to the specKey by default
specKey := mc.getTrafficSpecName(HTTPTraffic, trafficSpecs.Namespace, trafficSpecs.Name)
routePolicies[specKey] = make(map[trafficpolicy.TrafficSpecMatchName]trafficpolicy.Route)
routePolicies[specKey] = make(map[trafficpolicy.TrafficSpecMatchName]trafficpolicy.HTTPRoute)
for _, trafficSpecsMatches := range trafficSpecs.Spec.Matches {
serviceRoute := trafficpolicy.Route{}
serviceRoute := trafficpolicy.HTTPRoute{}
serviceRoute.PathRegex = trafficSpecsMatches.PathRegex
serviceRoute.Methods = trafficSpecsMatches.Methods
serviceRoute.Headers = trafficSpecsMatches.Headers
Expand All @@ -224,7 +224,7 @@ func (mc *MeshCatalog) getTrafficSpecName(trafficSpecKind string, trafficSpecNam
return trafficpolicy.TrafficSpecName(specKey)
}

func getTrafficPolicyPerRoute(mc *MeshCatalog, routePolicies map[trafficpolicy.TrafficSpecName]map[trafficpolicy.TrafficSpecMatchName]trafficpolicy.Route, meshService service.MeshService) ([]trafficpolicy.TrafficTarget, error) {
func getTrafficPolicyPerRoute(mc *MeshCatalog, routePolicies map[trafficpolicy.TrafficSpecName]map[trafficpolicy.TrafficSpecMatchName]trafficpolicy.HTTPRoute, meshService service.MeshService) ([]trafficpolicy.TrafficTarget, error) {
var trafficPolicies []trafficpolicy.TrafficTarget
for _, trafficTargets := range mc.meshSpec.ListTrafficTargets() {
log.Debug().Msgf("Discovered TrafficTarget resource: %s/%s", trafficTargets.Namespace, trafficTargets.Name)
Expand Down Expand Up @@ -273,7 +273,7 @@ func getTrafficPolicyPerRoute(mc *MeshCatalog, routePolicies map[trafficpolicy.T
if len(trafficTargetSpecs.Matches) == 0 {
// no match name provided, so routes are build for all matches in traffic spec
for _, routePolicy := range routePoliciesMatched {
trafficTarget.Route = routePolicy
trafficTarget.HTTPRoute = routePolicy
// append a traffic trafficTarget only if it corresponds to the service
if trafficTarget.Source.Equals(meshService) || trafficTarget.Destination.Equals(meshService) {
trafficPolicies = append(trafficPolicies, trafficTarget)
Expand All @@ -287,7 +287,7 @@ func getTrafficPolicyPerRoute(mc *MeshCatalog, routePolicies map[trafficpolicy.T
log.Error().Msgf("TrafficTarget %s/%s could not find a TrafficSpec %s with match name %s", trafficTargets.Namespace, trafficTargets.Name, specKey, specMatchesName)
return nil, errNoTrafficSpecFoundForTrafficPolicy
}
trafficTarget.Route = routePolicy
trafficTarget.HTTPRoute = routePolicy
// append a traffic trafficTarget only if it corresponds to the service
if trafficTarget.Source.Equals(meshService) || trafficTarget.Destination.Equals(meshService) {
trafficPolicies = append(trafficPolicies, trafficTarget)
Expand Down Expand Up @@ -321,7 +321,7 @@ func (mc *MeshCatalog) buildAllowAllTrafficPolicies(service service.MeshService)
}

func (mc *MeshCatalog) buildAllowPolicyForSourceToDest(source *corev1.Service, destination *corev1.Service) trafficpolicy.TrafficTarget {
allowAllRoute := trafficpolicy.Route{
allowAllRoute := trafficpolicy.HTTPRoute{
PathRegex: constants.RegexMatchAll,
Methods: []string{constants.WildcardHTTPMethod},
}
Expand All @@ -331,7 +331,7 @@ func (mc *MeshCatalog) buildAllowPolicyForSourceToDest(source *corev1.Service, d
Name: utils.GetTrafficTargetName("", srcMeshSvc, dstMeshSvc),
Destination: dstMeshSvc,
Source: srcMeshSvc,
Route: allowAllRoute,
HTTPRoute: allowAllRoute,
}
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/catalog/routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,15 @@ var _ = Describe("Catalog tests", func() {
Name: utils.GetTrafficTargetName(tests.TrafficTargetName, tests.BookbuyerService, tests.BookstoreService),
Destination: tests.BookstoreService,
Source: tests.BookbuyerService,
Route: trafficpolicy.Route{PathRegex: tests.BookstoreBuyPath, Methods: []string{"GET"}, Headers: map[string]string{
HTTPRoute: trafficpolicy.HTTPRoute{PathRegex: tests.BookstoreBuyPath, Methods: []string{"GET"}, Headers: map[string]string{
"user-agent": tests.HTTPUserAgent,
}},
},
{
Name: utils.GetTrafficTargetName(tests.TrafficTargetName, tests.BookbuyerService, tests.BookstoreApexService),
Destination: tests.BookstoreApexService,
Source: tests.BookbuyerService,
Route: trafficpolicy.Route{PathRegex: tests.BookstoreBuyPath, Methods: []string{"GET"}, Headers: map[string]string{
HTTPRoute: trafficpolicy.HTTPRoute{PathRegex: tests.BookstoreBuyPath, Methods: []string{"GET"}, Headers: map[string]string{
"user-agent": tests.HTTPUserAgent,
}},
},
Expand All @@ -65,7 +65,7 @@ var _ = Describe("Catalog tests", func() {
Expect(err).ToNot(HaveOccurred())

specKey := mc.getTrafficSpecName("HTTPRouteGroup", tests.Namespace, tests.RouteGroupName)
expected := map[trafficpolicy.TrafficSpecName]map[trafficpolicy.TrafficSpecMatchName]trafficpolicy.Route{
expected := map[trafficpolicy.TrafficSpecName]map[trafficpolicy.TrafficSpecMatchName]trafficpolicy.HTTPRoute{
specKey: {
trafficpolicy.TrafficSpecMatchName(tests.BuyBooksMatchName): {
PathRegex: tests.BookstoreBuyPath,
Expand Down Expand Up @@ -117,7 +117,7 @@ var _ = Describe("Catalog tests", func() {
expectedDestinationTrafficResource := utils.K8sSvcToMeshSvc(destination)

expectedHostHeaders := map[string]string{"user-agent": tests.HTTPUserAgent}
expectedRoute := trafficpolicy.Route{
expectedRoute := trafficpolicy.HTTPRoute{
PathRegex: constants.RegexMatchAll,
Methods: []string{constants.WildcardHTTPMethod},
Headers: expectedHostHeaders,
Expand All @@ -126,8 +126,8 @@ var _ = Describe("Catalog tests", func() {
trafficTarget := mc.buildAllowPolicyForSourceToDest(source, destination)
Expect(cmp.Equal(trafficTarget.Source, expectedSourceTrafficResource)).To(BeTrue())
Expect(cmp.Equal(trafficTarget.Destination, expectedDestinationTrafficResource)).To(BeTrue())
Expect(cmp.Equal(trafficTarget.Route.PathRegex, expectedRoute.PathRegex)).To(BeTrue())
Expect(cmp.Equal(trafficTarget.Route.Methods, expectedRoute.Methods)).To(BeTrue())
Expect(cmp.Equal(trafficTarget.HTTPRoute.PathRegex, expectedRoute.PathRegex)).To(BeTrue())
Expect(cmp.Equal(trafficTarget.HTTPRoute.Methods, expectedRoute.Methods)).To(BeTrue())
})
})

Expand Down
4 changes: 2 additions & 2 deletions pkg/catalog/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ type MeshCataloger interface {
//GetWeightedClusterForService returns the weighted cluster for a service
GetWeightedClusterForService(service service.MeshService) (service.WeightedCluster, error)

// GetIngressRoutesPerHost returns the routes per host associated with an ingress service
GetIngressRoutesPerHost(service.MeshService) (map[string][]trafficpolicy.Route, error)
// GetIngressRoutesPerHost returns the HTTP routes per host associated with an ingress service
GetIngressRoutesPerHost(service.MeshService) (map[string][]trafficpolicy.HTTPRoute, error)

// ListMonitoredNamespaces lists namespaces monitored by the control plane
ListMonitoredNamespaces() []string
Expand Down
18 changes: 9 additions & 9 deletions pkg/envoy/rds/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,11 @@ func NewResponse(catalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_disco
}

if isSourceService {
aggregateRoutesByHost(outboundAggregatedRoutesByHostnames, trafficPolicies.Route, weightedCluster, hostnames)
aggregateRoutesByHost(outboundAggregatedRoutesByHostnames, trafficPolicies.HTTPRoute, weightedCluster, hostnames)
}

if isDestinationService {
aggregateRoutesByHost(inboundAggregatedRoutesByHostnames, trafficPolicies.Route, weightedCluster, hostnames)
aggregateRoutesByHost(inboundAggregatedRoutesByHostnames, trafficPolicies.HTTPRoute, weightedCluster, hostnames)
}
}

Expand All @@ -85,7 +85,7 @@ func NewResponse(catalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_disco
return resp, nil
}

func aggregateRoutesByHost(routesPerHost map[string]map[string]trafficpolicy.RouteWeightedClusters, routePolicy trafficpolicy.Route, weightedCluster service.WeightedCluster, host string) {
func aggregateRoutesByHost(routesPerHost map[string]map[string]trafficpolicy.RouteWeightedClusters, routePolicy trafficpolicy.HTTPRoute, weightedCluster service.WeightedCluster, host string) {
_, exists := routesPerHost[host]
if !exists {
// no host found, create a new route map
Expand All @@ -95,12 +95,12 @@ func aggregateRoutesByHost(routesPerHost map[string]map[string]trafficpolicy.Rou
if routeFound {
// add the cluster to the existing route
routePolicyWeightedCluster.WeightedClusters.Add(weightedCluster)
routePolicyWeightedCluster.Route.Methods = append(routePolicyWeightedCluster.Route.Methods, routePolicy.Methods...)
if routePolicyWeightedCluster.Route.Headers == nil {
routePolicyWeightedCluster.Route.Headers = make(map[string]string)
routePolicyWeightedCluster.HTTPRoute.Methods = append(routePolicyWeightedCluster.HTTPRoute.Methods, routePolicy.Methods...)
if routePolicyWeightedCluster.HTTPRoute.Headers == nil {
routePolicyWeightedCluster.HTTPRoute.Headers = make(map[string]string)
}
for headerKey, headerValue := range routePolicy.Headers {
routePolicyWeightedCluster.Route.Headers[headerKey] = headerValue
routePolicyWeightedCluster.HTTPRoute.Headers[headerKey] = headerValue
}
routesPerHost[host][routePolicy.PathRegex] = routePolicyWeightedCluster
} else {
Expand All @@ -109,9 +109,9 @@ func aggregateRoutesByHost(routesPerHost map[string]map[string]trafficpolicy.Rou
}
}

func createRoutePolicyWeightedClusters(routePolicy trafficpolicy.Route, weightedCluster service.WeightedCluster) trafficpolicy.RouteWeightedClusters {
func createRoutePolicyWeightedClusters(routePolicy trafficpolicy.HTTPRoute, weightedCluster service.WeightedCluster) trafficpolicy.RouteWeightedClusters {
return trafficpolicy.RouteWeightedClusters{
Route: routePolicy,
HTTPRoute: routePolicy,
WeightedClusters: set.NewSet(weightedCluster),
}
}
18 changes: 9 additions & 9 deletions pkg/envoy/rds/response_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,15 @@ var _ = Describe("Construct RoutePolicyWeightedClusters object", func() {
ClusterName: service.ClusterName("osm/bookstore-1"),
Weight: constants.ClusterWeightAcceptAll,
}
routePolicy := trafficpolicy.Route{
routePolicy := trafficpolicy.HTTPRoute{
PathRegex: "/books-bought",
Methods: []string{"GET"},
}

routePolicyWeightedClusters := createRoutePolicyWeightedClusters(routePolicy, weightedCluster)
Expect(routePolicyWeightedClusters).NotTo(Equal(nil))
Expect(routePolicyWeightedClusters.Route.PathRegex).To(Equal("/books-bought"))
Expect(routePolicyWeightedClusters.Route.Methods).To(Equal([]string{"GET"}))
Expect(routePolicyWeightedClusters.HTTPRoute.PathRegex).To(Equal("/books-bought"))
Expect(routePolicyWeightedClusters.HTTPRoute.Methods).To(Equal([]string{"GET"}))
Expect(routePolicyWeightedClusters.WeightedClusters.Cardinality()).To(Equal(1))
routePolicyWeightedClustersSlice := routePolicyWeightedClusters.WeightedClusters.ToSlice()
Expect(string(routePolicyWeightedClustersSlice[0].(service.WeightedCluster).ClusterName)).To(Equal("osm/bookstore-1"))
Expand All @@ -64,7 +64,7 @@ var _ = Describe("AggregateRoutesByDomain", func() {
It("Returns a new aggregated map of domain and routes", func() {

weightedCluster := service.WeightedCluster{ClusterName: service.ClusterName("osm/bookstore-1"), Weight: 100}
routePolicies := []trafficpolicy.Route{
routePolicies := []trafficpolicy.HTTPRoute{
{PathRegex: "/books-bought", Methods: []string{"GET"}},
{PathRegex: "/buy-a-book", Methods: []string{"GET"}},
}
Expand Down Expand Up @@ -95,7 +95,7 @@ var _ = Describe("AggregateRoutesByDomain", func() {
ClusterName: service.ClusterName("osm/bookstore-1"),
Weight: constants.ClusterWeightAcceptAll,
}
routePolicy := trafficpolicy.Route{
routePolicy := trafficpolicy.HTTPRoute{
PathRegex: "/update-books-bought",
Methods: []string{"GET"},
Headers: map[string]string{
Expand All @@ -108,9 +108,9 @@ var _ = Describe("AggregateRoutesByDomain", func() {
Expect(domainRoutesMap).NotTo(Equal(nil))
Expect(len(domainRoutesMap)).To(Equal(1))
Expect(len(domainRoutesMap["bookstore.mesh"])).To(Equal(3))
Expect(domainRoutesMap["bookstore.mesh"][routePolicy.PathRegex].Route).To(Equal(routePolicy))
Expect(domainRoutesMap["bookstore.mesh"][routePolicy.PathRegex].HTTPRoute).To(Equal(routePolicy))
Expect(domainRoutesMap["bookstore.mesh"][routePolicy.PathRegex].WeightedClusters.Cardinality()).To(Equal(1))
Expect(domainRoutesMap["bookstore.mesh"][routePolicy.PathRegex].Route).To(Equal(trafficpolicy.Route{PathRegex: "/update-books-bought", Methods: []string{"GET"}, Headers: map[string]string{testHeaderKey1: "This is a test header 1"}}))
Expect(domainRoutesMap["bookstore.mesh"][routePolicy.PathRegex].HTTPRoute).To(Equal(trafficpolicy.HTTPRoute{PathRegex: "/update-books-bought", Methods: []string{"GET"}, Headers: map[string]string{testHeaderKey1: "This is a test header 1"}}))
Expect(domainRoutesMap["bookstore.mesh"][routePolicy.PathRegex].WeightedClusters.Equal(weightedClustersMap)).To(Equal(true))
})
})
Expand All @@ -122,7 +122,7 @@ var _ = Describe("AggregateRoutesByDomain", func() {
ClusterName: service.ClusterName("osm/bookstore-2"),
Weight: constants.ClusterWeightAcceptAll,
}
routePolicy := trafficpolicy.Route{
routePolicy := trafficpolicy.HTTPRoute{
PathRegex: "/update-books-bought",
Methods: []string{"GET"},
Headers: map[string]string{
Expand All @@ -136,7 +136,7 @@ var _ = Describe("AggregateRoutesByDomain", func() {
Expect(len(domainRoutesMap)).To(Equal(1))
Expect(len(domainRoutesMap["bookstore.mesh"])).To(Equal(3))
Expect(domainRoutesMap["bookstore.mesh"][routePolicy.PathRegex].WeightedClusters.Cardinality()).To(Equal(2))
Expect(domainRoutesMap["bookstore.mesh"][routePolicy.PathRegex].Route).To(Equal(trafficpolicy.Route{PathRegex: "/update-books-bought", Methods: []string{"GET", "GET"}, Headers: map[string]string{testHeaderKey1: "This is a test header 1", testHeaderKey2: "This is a test header 2"}}))
Expect(domainRoutesMap["bookstore.mesh"][routePolicy.PathRegex].HTTPRoute).To(Equal(trafficpolicy.HTTPRoute{PathRegex: "/update-books-bought", Methods: []string{"GET", "GET"}, Headers: map[string]string{testHeaderKey1: "This is a test header 1", testHeaderKey2: "This is a test header 2"}}))
Expect(domainRoutesMap["bookstore.mesh"][routePolicy.PathRegex].WeightedClusters.Equal(weightedClustersMap)).To(Equal(true))
})
})
Expand Down
4 changes: 2 additions & 2 deletions pkg/envoy/route/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,9 @@ func createRoutes(routePolicyWeightedClustersMap map[string]trafficpolicy.RouteW
for _, routePolicyWeightedClusters := range routePolicyWeightedClustersMap {
// For a given route path, sanitize the methods in case there
// is wildcard or if there are duplicates
allowedMethods := sanitizeHTTPMethods(routePolicyWeightedClusters.Route.Methods)
allowedMethods := sanitizeHTTPMethods(routePolicyWeightedClusters.HTTPRoute.Methods)
for _, method := range allowedMethods {
route := getRoute(routePolicyWeightedClusters.Route.PathRegex, method, routePolicyWeightedClusters.Route.Headers, routePolicyWeightedClusters.WeightedClusters, 100, direction)
route := getRoute(routePolicyWeightedClusters.HTTPRoute.PathRegex, method, routePolicyWeightedClusters.HTTPRoute.Headers, routePolicyWeightedClusters.WeightedClusters, 100, direction)
routes = append(routes, route)
}
}
Expand Down
Loading

0 comments on commit 930991c

Please sign in to comment.