Skip to content

Commit

Permalink
support sort httproute when use gateway api (#622)
Browse files Browse the repository at this point in the history
  • Loading branch information
johnlanni committed Nov 3, 2023
1 parent 7b1f538 commit 70176cd
Show file tree
Hide file tree
Showing 2 changed files with 380 additions and 3 deletions.
377 changes: 377 additions & 0 deletions istio/1.12/patches/istio/20231103-gatewayapi-sort.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,377 @@
diff -Naur istio/pilot/pkg/config/kube/gateway/conversion.go istio-new/pilot/pkg/config/kube/gateway/conversion.go
--- istio/pilot/pkg/config/kube/gateway/conversion.go 2023-11-03 17:18:56.000000000 +0800
+++ istio-new/pilot/pkg/config/kube/gateway/conversion.go 2023-11-03 17:14:50.000000000 +0800
@@ -151,15 +151,113 @@
}
}

+ // for gateway routes, build one VS per gateway+host
+ gatewayRoutes := make(map[string]map[string]*config.Config)
+
for _, obj := range r.HTTPRoute {
- if vsConfig := buildHTTPVirtualServices(obj, gatewayMap, r.Domain); vsConfig != nil {
+ buildHTTPVirtualServices(r, obj, gatewayMap, gatewayRoutes, r.Domain)
+ }
+ for _, vsByHost := range gatewayRoutes {
+ for _, vsConfig := range vsByHost {
result = append(result, *vsConfig)
}
}
return result
}

-func buildHTTPVirtualServices(obj config.Config, gateways map[parentKey]map[gatewayapiV1beta1.SectionName]*parentInfo, domain string) *config.Config {
+// getURIRank ranks a URI match type. Exact > Prefix > Regex
+func getURIRank(match *istio.HTTPMatchRequest) int {
+ if match.Uri == nil {
+ return -1
+ }
+ switch match.Uri.MatchType.(type) {
+ case *istio.StringMatch_Exact:
+ return 3
+ case *istio.StringMatch_Prefix:
+ return 2
+ case *istio.StringMatch_Regex:
+ // TODO optimize in new verison envoy
+ if strings.HasSuffix(match.Uri.GetRegex(), prefixMatchRegex) &&
+ !strings.ContainsAny(strings.TrimSuffix(match.Uri.GetRegex(), prefixMatchRegex), `\.+*?()|[]{}^$`) {
+ return 2
+ }
+ return 1
+ }
+ // should not happen
+ return -1
+}
+
+func getURILength(match *istio.HTTPMatchRequest) int {
+ if match.Uri == nil {
+ return 0
+ }
+ switch match.Uri.MatchType.(type) {
+ case *istio.StringMatch_Prefix:
+ return len(match.Uri.GetPrefix())
+ case *istio.StringMatch_Exact:
+ return len(match.Uri.GetExact())
+ case *istio.StringMatch_Regex:
+ return len(match.Uri.GetRegex())
+ }
+ // should not happen
+ return -1
+}
+
+// sortHTTPRoutes sorts generated vs routes to meet gateway-api requirements
+// see https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1alpha2.HTTPRouteRule
+func sortHTTPRoutes(routes []*istio.HTTPRoute) {
+ sort.SliceStable(routes, func(i, j int) bool {
+ if len(routes[i].Match) == 0 {
+ return false
+ } else if len(routes[j].Match) == 0 {
+ return true
+ }
+ // Only look at match[0], we always generate only one match
+ m1, m2 := routes[i].Match[0], routes[j].Match[0]
+ r1, r2 := getURIRank(m1), getURIRank(m2)
+ len1, len2 := getURILength(m1), getURILength(m2)
+ switch {
+ // 1: Exact/Prefix/Regex
+ case r1 != r2:
+ return r1 > r2
+ case len1 != len2:
+ return len1 > len2
+ // 2: method math
+ case (m1.Method == nil) != (m2.Method == nil):
+ return m1.Method != nil
+ // 3: number of header matches
+ case len(m1.Headers) != len(m2.Headers):
+ return len(m1.Headers) > len(m2.Headers)
+ // 4: number of query matches
+ default:
+ return len(m1.QueryParams) > len(m2.QueryParams)
+ }
+ })
+}
+
+func routeMeta(obj config.Config) map[string]string {
+ m := parentMeta(obj, nil)
+ m[constants.InternalRouteSemantics] = constants.RouteSemanticsGateway
+ return m
+}
+
+func filteredReferences(parents []routeParentReference) []routeParentReference {
+ ret := make([]routeParentReference, 0, len(parents))
+ for _, p := range parents {
+ if p.DeniedReason != nil {
+ // We should filter this out
+ continue
+ }
+ ret = append(ret, p)
+ }
+ // To ensure deterministic order, sort them
+ sort.Slice(ret, func(i, j int) bool {
+ return ret[i].InternalName < ret[j].InternalName
+ })
+ return ret
+}
+
+func buildHTTPVirtualServices(ctx *KubernetesResources, obj config.Config, gateways map[parentKey]map[gatewayapiV1beta1.SectionName]*parentInfo, gatewayRoutes map[string]map[string]*config.Config, domain string) {
route := obj.Spec.(*gatewayapiV1beta1.HTTPRouteSpec)

parentRefs := extractParentReferenceInfo(gateways, route.ParentRefs, route.Hostnames, gvk.HTTPRoute, obj.Namespace)
@@ -172,10 +270,7 @@
})
}

- name := fmt.Sprintf("%s-%s", obj.Name, constants.KubernetesGatewayName)
-
httproutes := []*istio.HTTPRoute{}
- hosts := hostnameToStringList(route.Hostnames)
for _, r := range route.Rules {
// TODO: implement rewrite, timeout, mirror, corspolicy, retries
vs := &istio.HTTPRoute{
@@ -185,22 +280,22 @@
uri, err := createURIMatch(match)
if err != nil {
reportError(err)
- return nil
+ return
}
headers, err := createHeadersMatch(match)
if err != nil {
reportError(err)
- return nil
+ return
}
qp, err := createQueryParamsMatch(match)
if err != nil {
reportError(err)
- return nil
+ return
}
method, err := createMethodMatch(match)
if err != nil {
reportError(err)
- return nil
+ return
}
vs.Match = append(vs.Match, &istio.HTTPMatchRequest{
Uri: uri,
@@ -219,7 +314,7 @@
mirror, err := createMirrorFilter(filter.RequestMirror, obj.Namespace, domain)
if err != nil {
reportError(err)
- return nil
+ return
}
vs.Mirror = mirror
default:
@@ -227,7 +322,7 @@
Reason: InvalidFilter,
Message: fmt.Sprintf("unsupported filter type %q", filter.Type),
})
- return nil
+ return
}
}

@@ -255,33 +350,65 @@
route, err := buildHTTPDestination(r.BackendRefs, obj.Namespace, domain, zero, fallbackCluster)
if err != nil {
reportError(err)
- return nil
+ return
}
vs.Route = route

httproutes = append(httproutes, vs)
}
reportError(nil)
- gatewayNames := referencesToInternalNames(parentRefs)
- if len(gatewayNames) == 0 {
- return nil
+
+ count := 0
+ for _, parent := range filteredReferences(parentRefs) {
+ // for gateway routes, build one VS per gateway+host
+ routeMap := gatewayRoutes
+ routeKey := parent.InternalName
+ vsHosts := hostnameToStringList(route.Hostnames)
+ routes := httproutes
+ if len(routes) == 0 {
+ continue
+ }
+ if _, f := routeMap[routeKey]; !f {
+ routeMap[routeKey] = make(map[string]*config.Config)
+ }
+
+ // Create one VS per hostname with a single hostname.
+ // This ensures we can treat each hostname independently, as the spec requires
+ for _, h := range vsHosts {
+ if cfg := routeMap[routeKey][h]; cfg != nil {
+ // merge http routes
+ vs := cfg.Spec.(*istio.VirtualService)
+ vs.Http = append(vs.Http, routes...)
+ // append parents
+ cfg.Annotations[constants.InternalParentNames] = fmt.Sprintf("%s,%s/%s.%s",
+ cfg.Annotations[constants.InternalParentNames], obj.GroupVersionKind.Kind, obj.Name, obj.Namespace)
+ } else {
+ name := fmt.Sprintf("%s-%d-%s", obj.Name, count, constants.KubernetesGatewayName)
+ routeMap[routeKey][h] = &config.Config{
+ Meta: config.Meta{
+ CreationTimestamp: obj.CreationTimestamp,
+ GroupVersionKind: gvk.VirtualService,
+ Name: name,
+ Annotations: routeMeta(obj),
+ Namespace: obj.Namespace,
+ Domain: ctx.Domain,
+ },
+ Spec: &istio.VirtualService{
+ Hosts: []string{h},
+ Gateways: []string{parent.InternalName},
+ Http: routes,
+ },
+ }
+ count++
+ }
+ }
}
- vsConfig := config.Config{
- Meta: config.Meta{
- CreationTimestamp: obj.CreationTimestamp,
- GroupVersionKind: gvk.VirtualService,
- Name: name,
- Annotations: parentMeta(obj, nil),
- Namespace: obj.Namespace,
- Domain: domain,
- },
- Spec: &istio.VirtualService{
- Hosts: hosts,
- Gateways: gatewayNames,
- Http: httproutes,
- },
+ for _, vsByHost := range gatewayRoutes {
+ for _, cfg := range vsByHost {
+ vs := cfg.Spec.(*istio.VirtualService)
+ sortHTTPRoutes(vs.Http)
+ }
}
- return &vsConfig
}

func parentMeta(obj config.Config, sectionName *gatewayapiV1beta1.SectionName) map[string]string {
@@ -1155,9 +1282,11 @@
}
gs.Addresses = make([]gatewayapiV1beta1.GatewayAddress, 0, len(addressesToReport))
for _, addr := range addressesToReport {
+ addrPairs := strings.Split(addr, ":")
gs.Addresses = append(gs.Addresses, gatewayapiV1beta1.GatewayAddress{
- Type: &addrType,
- Value: addr,
+ Type: &addrType,
+ // strip the port
+ Value: addrPairs[0],
})
}
return gs
diff -Naur istio/pilot/pkg/model/push_context.go istio-new/pilot/pkg/model/push_context.go
--- istio/pilot/pkg/model/push_context.go 2023-11-03 17:18:56.000000000 +0800
+++ istio-new/pilot/pkg/model/push_context.go 2023-11-03 17:05:47.000000000 +0800
@@ -841,7 +841,19 @@
func (ps *PushContext) VirtualServicesForGateway(proxy *Proxy, gateway string) []config.Config {
res := ps.virtualServiceIndex.privateByNamespaceAndGateway[proxy.ConfigNamespace][gateway]
res = append(res, ps.virtualServiceIndex.exportedToNamespaceByGateway[proxy.ConfigNamespace][gateway]...)
- res = append(res, ps.virtualServiceIndex.publicByGateway[gateway]...)
+
+ // Favor same-namespace Gateway routes, to give the "consumer override" preference.
+ // We do 2 iterations here to avoid extra allocations.
+ for _, vs := range ps.virtualServiceIndex.publicByGateway[gateway] {
+ if UseGatewaySemantics(vs) && vs.Namespace == proxy.ConfigNamespace {
+ res = append(res, vs)
+ }
+ }
+ for _, vs := range ps.virtualServiceIndex.publicByGateway[gateway] {
+ if !(UseGatewaySemantics(vs) && vs.Namespace == proxy.ConfigNamespace) {
+ res = append(res, vs)
+ }
+ }
return res
}

diff -Naur istio/pilot/pkg/model/virtualservice.go istio-new/pilot/pkg/model/virtualservice.go
--- istio/pilot/pkg/model/virtualservice.go 2023-11-03 17:18:55.000000000 +0800
+++ istio-new/pilot/pkg/model/virtualservice.go 2023-11-03 15:19:08.000000000 +0800
@@ -76,6 +76,11 @@
}

func resolveVirtualServiceShortnames(rule *networking.VirtualService, meta config.Meta) {
+ // Kubernetes Gateway API semantics support shortnames
+ // if UseGatewaySemantics(config.Config{Meta: meta}) {
+ // return
+ // }
+
// resolve top level hosts
for i, h := range rule.Hosts {
rule.Hosts[i] = string(ResolveShortnameToFQDN(h, meta))
@@ -524,3 +529,10 @@
}
return false
}
+
+// UseGatewaySemantics determines which logic we should use for VirtualService
+// This allows gateway-api and VS to both be represented by VirtualService, but have different
+// semantics.
+func UseGatewaySemantics(cfg config.Config) bool {
+ return cfg.Annotations[constants.InternalRouteSemantics] == constants.RouteSemanticsGateway
+}
diff -Naur istio/pilot/pkg/networking/core/v1alpha3/route/route.go istio-new/pilot/pkg/networking/core/v1alpha3/route/route.go
--- istio/pilot/pkg/networking/core/v1alpha3/route/route.go 2023-11-03 17:18:56.000000000 +0800
+++ istio-new/pilot/pkg/networking/core/v1alpha3/route/route.go 2023-11-03 17:05:55.000000000 +0800
@@ -408,7 +408,6 @@
break
}
}
-
if len(out) == 0 {
return nil, fmt.Errorf("no routes matched")
}
@@ -493,6 +492,14 @@
},
}

+ if model.UseGatewaySemantics(virtualService) {
+ if uri, isPrefixReplace := cutPrefix(redirect.Uri, "%PREFIX()%"); isPrefixReplace {
+ action.Redirect.PathRewriteSpecifier = &route.RedirectAction_PrefixRewrite{
+ PrefixRewrite: uri,
+ }
+ }
+ }
+
if redirect.Scheme != "" {
action.Redirect.SchemeRewriteSpecifier = &route.RedirectAction_SchemeRedirect{SchemeRedirect: redirect.Scheme}
}
@@ -1616,3 +1623,10 @@
isSupport = curVersion.GreaterThan(notSupportFallback)
return
}
+
+func cutPrefix(s, prefix string) (after string, found bool) {
+ if !strings.HasPrefix(s, prefix) {
+ return s, false
+ }
+ return s[len(prefix):], true
+}
diff -Naur istio/pkg/config/constants/constants.go istio-new/pkg/config/constants/constants.go
--- istio/pkg/config/constants/constants.go 2023-11-03 17:18:54.000000000 +0800
+++ istio-new/pkg/config/constants/constants.go 2023-11-03 14:29:27.000000000 +0800
@@ -15,6 +15,12 @@
package constants

const (
+ InternalParentNames = "internal.istio.io/parents"
+
+ InternalRouteSemantics = "internal.istio.io/route-semantics"
+
+ RouteSemanticsGateway = "gateway"
+
// UnspecifiedIP constant for empty IP address
UnspecifiedIP = "0.0.0.0"

Loading

0 comments on commit 70176cd

Please sign in to comment.