From 196d23c880aad0df90907e5bdddb0863e7955233 Mon Sep 17 00:00:00 2001 From: Shawnh2 Date: Wed, 7 Jun 2023 21:06:08 +0800 Subject: [PATCH 1/5] dedup the context via reflect Signed-off-by: Shawnh2 --- internal/gatewayapi/contexts.go | 481 ++++++++------------------------ internal/gatewayapi/route.go | 44 +-- 2 files changed, 140 insertions(+), 385 deletions(-) diff --git a/internal/gatewayapi/contexts.go b/internal/gatewayapi/contexts.go index aad24bf2f67..41b2332c629 100644 --- a/internal/gatewayapi/contexts.go +++ b/internal/gatewayapi/contexts.go @@ -7,6 +7,7 @@ package gatewayapi import ( "reflect" + "strings" "time" v1 "k8s.io/api/core/v1" @@ -155,26 +156,14 @@ func (l *ListenerContext) SetTLSSecrets(tlsSecrets []*v1.Secret) { type RouteContext interface { client.Object - // GetRouteType returns the Kind of the Route object, HTTPRoute, - // TLSRoute, TCPRoute, UDPRoute etc. - GetRouteType() v1beta1.Kind - - // GetRouteStatus returns the RouteStatus object associated with the Route. - GetRouteStatus() *v1beta1.RouteStatus - - // TODO: [v1alpha2-v1beta1] This should not be required once all Route - // objects being implemented are of type v1beta1. - // GetParentReferences returns the ParentReference of the Route object. - GetParentReferences() []v1beta1.ParentReference - // GetRouteParentContext returns RouteParentContext by using the Route // objects' ParentReference. - GetRouteParentContext(forParentRef v1beta1.ParentReference) *RouteParentContext - + //GetRouteParentContext(forParentRef v1beta1.ParentReference) *RouteParentContext + // // TODO: [v1alpha2-v1beta1] This should not be required once all Route // objects being implemented are of type v1beta1. // GetHostnames returns the hosts targeted by the Route object. - GetHostnames() []string + //GetHostnames() []string } // HTTPRouteContext wraps an HTTPRoute and provides helper methods for @@ -185,73 +174,7 @@ type HTTPRouteContext struct { *v1beta1.HTTPRoute - parentRefs map[v1beta1.ParentReference]*RouteParentContext -} - -func (h *HTTPRouteContext) GetRouteType() v1beta1.Kind { - return KindHTTPRoute -} - -func (h *HTTPRouteContext) GetHostnames() []string { - hostnames := make([]string, len(h.Spec.Hostnames)) - for idx, s := range h.Spec.Hostnames { - hostnames[idx] = string(s) - } - return hostnames -} - -func (h *HTTPRouteContext) GetParentReferences() []v1beta1.ParentReference { - return h.Spec.ParentRefs -} - -func (h *HTTPRouteContext) GetRouteStatus() *v1beta1.RouteStatus { - return &h.Status.RouteStatus -} - -func (h *HTTPRouteContext) GetRouteParentContext(forParentRef v1beta1.ParentReference) *RouteParentContext { - if h.parentRefs == nil { - h.parentRefs = make(map[v1beta1.ParentReference]*RouteParentContext) - } - - if ctx := h.parentRefs[forParentRef]; ctx != nil { - return ctx - } - - var parentRef *v1beta1.ParentReference - for i, p := range h.Spec.ParentRefs { - if reflect.DeepEqual(p, forParentRef) { - parentRef = &h.Spec.ParentRefs[i] - break - } - } - if parentRef == nil { - panic("parentRef not found") - } - - routeParentStatusIdx := -1 - for i := range h.Status.Parents { - if reflect.DeepEqual(h.Status.Parents[i].ParentRef, forParentRef) { - routeParentStatusIdx = i - break - } - } - if routeParentStatusIdx == -1 { - rParentStatus := v1beta1.RouteParentStatus{ - ControllerName: v1beta1.GatewayController(h.GatewayControllerName), - ParentRef: forParentRef, - } - h.Status.Parents = append(h.Status.Parents, rParentStatus) - routeParentStatusIdx = len(h.Status.Parents) - 1 - } - - ctx := &RouteParentContext{ - ParentReference: parentRef, - - httpRoute: h.HTTPRoute, - routeParentStatusIdx: routeParentStatusIdx, - } - h.parentRefs[forParentRef] = ctx - return ctx + ParentRefs map[v1beta1.ParentReference]*RouteParentContext } // GRPCRouteContext wraps a GRPCRoute and provides helper methods for @@ -262,83 +185,7 @@ type GRPCRouteContext struct { *v1alpha2.GRPCRoute - parentRefs map[v1beta1.ParentReference]*RouteParentContext -} - -func (g *GRPCRouteContext) GetRouteType() v1beta1.Kind { - return KindGRPCRoute -} - -func (g *GRPCRouteContext) GetHostnames() []string { - hostnames := make([]string, len(g.Spec.Hostnames)) - for idx, s := range g.Spec.Hostnames { - hostnames[idx] = string(s) - } - return hostnames -} - -func (g *GRPCRouteContext) GetParentReferences() []v1beta1.ParentReference { - return g.Spec.ParentRefs -} - -func (g *GRPCRouteContext) GetRouteStatus() *v1beta1.RouteStatus { - return &g.Status.RouteStatus -} - -func (g *GRPCRouteContext) GetRouteParentContext(forParentRef v1beta1.ParentReference) *RouteParentContext { - if g.parentRefs == nil { - g.parentRefs = make(map[v1beta1.ParentReference]*RouteParentContext) - } - - if ctx := g.parentRefs[forParentRef]; ctx != nil { - return ctx - } - - var parentRef *v1beta1.ParentReference - for i, p := range g.Spec.ParentRefs { - p := UpgradeParentReference(p) - if reflect.DeepEqual(p, forParentRef) { - upgraded := UpgradeParentReference(g.Spec.ParentRefs[i]) - parentRef = &upgraded - break - } - } - if parentRef == nil { - panic("parentRef not found") - } - - routeParentStatusIdx := -1 - for i := range g.Status.Parents { - p := UpgradeParentReference(g.Status.Parents[i].ParentRef) - defaultNamespace := v1beta1.Namespace(metav1.NamespaceDefault) - if forParentRef.Namespace == nil { - forParentRef.Namespace = &defaultNamespace - } - if p.Namespace == nil { - p.Namespace = &defaultNamespace - } - if reflect.DeepEqual(p, forParentRef) { - routeParentStatusIdx = i - break - } - } - if routeParentStatusIdx == -1 { - rParentStatus := v1alpha2.RouteParentStatus{ - ControllerName: v1alpha2.GatewayController(g.GatewayControllerName), - ParentRef: DowngradeParentReference(forParentRef), - } - g.Status.Parents = append(g.Status.Parents, rParentStatus) - routeParentStatusIdx = len(g.Status.Parents) - 1 - } - - ctx := &RouteParentContext{ - ParentReference: parentRef, - - grpcRoute: g.GRPCRoute, - routeParentStatusIdx: routeParentStatusIdx, - } - g.parentRefs[forParentRef] = ctx - return ctx + ParentRefs map[v1beta1.ParentReference]*RouteParentContext } // TLSRouteContext wraps a TLSRoute and provides helper methods for @@ -349,87 +196,7 @@ type TLSRouteContext struct { *v1alpha2.TLSRoute - parentRefs map[v1beta1.ParentReference]*RouteParentContext -} - -func (t *TLSRouteContext) GetRouteType() v1beta1.Kind { - return KindTLSRoute -} - -func (t *TLSRouteContext) GetHostnames() []string { - hostnames := make([]string, len(t.Spec.Hostnames)) - for idx, s := range t.Spec.Hostnames { - hostnames[idx] = string(s) - } - return hostnames -} - -func (t *TLSRouteContext) GetRouteStatus() *v1beta1.RouteStatus { - return &t.Status.RouteStatus -} - -func (t *TLSRouteContext) GetParentReferences() []v1beta1.ParentReference { - parentReferences := make([]v1beta1.ParentReference, len(t.Spec.ParentRefs)) - for idx, p := range t.Spec.ParentRefs { - parentReferences[idx] = UpgradeParentReference(p) - } - return parentReferences -} - -func (t *TLSRouteContext) GetRouteParentContext(forParentRef v1beta1.ParentReference) *RouteParentContext { - if t.parentRefs == nil { - t.parentRefs = make(map[v1beta1.ParentReference]*RouteParentContext) - } - - if ctx := t.parentRefs[forParentRef]; ctx != nil { - return ctx - } - - var parentRef *v1beta1.ParentReference - for i, p := range t.Spec.ParentRefs { - p := UpgradeParentReference(p) - if reflect.DeepEqual(p, forParentRef) { - upgraded := UpgradeParentReference(t.Spec.ParentRefs[i]) - parentRef = &upgraded - break - } - } - if parentRef == nil { - panic("parentRef not found") - } - - routeParentStatusIdx := -1 - for i := range t.Status.Parents { - p := UpgradeParentReference(t.Status.Parents[i].ParentRef) - defaultNamespace := v1beta1.Namespace(metav1.NamespaceDefault) - if forParentRef.Namespace == nil { - forParentRef.Namespace = &defaultNamespace - } - if p.Namespace == nil { - p.Namespace = &defaultNamespace - } - if reflect.DeepEqual(p, forParentRef) { - routeParentStatusIdx = i - break - } - } - if routeParentStatusIdx == -1 { - rParentStatus := v1alpha2.RouteParentStatus{ - ControllerName: v1alpha2.GatewayController(t.GatewayControllerName), - ParentRef: DowngradeParentReference(forParentRef), - } - t.Status.Parents = append(t.Status.Parents, rParentStatus) - routeParentStatusIdx = len(t.Status.Parents) - 1 - } - - ctx := &RouteParentContext{ - ParentReference: parentRef, - - tlsRoute: t.TLSRoute, - routeParentStatusIdx: routeParentStatusIdx, - } - t.parentRefs[forParentRef] = ctx - return ctx + ParentRefs map[v1beta1.ParentReference]*RouteParentContext } // UDPRouteContext wraps a UDPRoute and provides helper methods for @@ -440,83 +207,7 @@ type UDPRouteContext struct { *v1alpha2.UDPRoute - parentRefs map[v1beta1.ParentReference]*RouteParentContext -} - -func (u *UDPRouteContext) GetRouteType() v1beta1.Kind { - return KindUDPRoute -} - -func (u *UDPRouteContext) GetParentReferences() []v1beta1.ParentReference { - parentReferences := make([]v1beta1.ParentReference, len(u.Spec.ParentRefs)) - for idx, p := range u.Spec.ParentRefs { - parentReferences[idx] = UpgradeParentReference(p) - } - return parentReferences -} - -func (u *UDPRouteContext) GetRouteStatus() *v1beta1.RouteStatus { - return &u.Status.RouteStatus -} - -func (u *UDPRouteContext) GetRouteParentContext(forParentRef v1beta1.ParentReference) *RouteParentContext { - if u.parentRefs == nil { - u.parentRefs = make(map[v1beta1.ParentReference]*RouteParentContext) - } - - if ctx := u.parentRefs[forParentRef]; ctx != nil { - return ctx - } - - var parentRef *v1beta1.ParentReference - for i, p := range u.Spec.ParentRefs { - p := UpgradeParentReference(p) - if reflect.DeepEqual(p, forParentRef) { - upgraded := UpgradeParentReference(u.Spec.ParentRefs[i]) - parentRef = &upgraded - break - } - } - if parentRef == nil { - panic("parentRef not found") - } - - routeParentStatusIdx := -1 - for i := range u.Status.Parents { - p := UpgradeParentReference(u.Status.Parents[i].ParentRef) - defaultNamespace := v1beta1.Namespace(metav1.NamespaceDefault) - if forParentRef.Namespace == nil { - forParentRef.Namespace = &defaultNamespace - } - if p.Namespace == nil { - p.Namespace = &defaultNamespace - } - if reflect.DeepEqual(p, forParentRef) { - routeParentStatusIdx = i - break - } - } - if routeParentStatusIdx == -1 { - rParentStatus := v1alpha2.RouteParentStatus{ - ControllerName: v1alpha2.GatewayController(u.GatewayControllerName), - ParentRef: DowngradeParentReference(forParentRef), - } - u.Status.Parents = append(u.Status.Parents, rParentStatus) - routeParentStatusIdx = len(u.Status.Parents) - 1 - } - - ctx := &RouteParentContext{ - ParentReference: parentRef, - - udpRoute: u.UDPRoute, - routeParentStatusIdx: routeParentStatusIdx, - } - u.parentRefs[forParentRef] = ctx - return ctx -} - -func (u *UDPRouteContext) GetHostnames() []string { - return nil + ParentRefs map[v1beta1.ParentReference]*RouteParentContext } // TCPRouteContext wraps a TCPRoute and provides helper methods for @@ -527,40 +218,100 @@ type TCPRouteContext struct { *v1alpha2.TCPRoute - parentRefs map[v1beta1.ParentReference]*RouteParentContext + ParentRefs map[v1beta1.ParentReference]*RouteParentContext } -func (t *TCPRouteContext) GetRouteType() v1beta1.Kind { - return KindTCPRoute +// GetRouteType returns the Kind of the Route object, HTTPRoute, +// TLSRoute, TCPRoute, UDPRoute etc. +// +// This function use the typename of RouteContext and its return is +// corresponding to the const defined in translator, like KindHTTPRoute, +// KindGRPCRoute, KindTLSRoute, KindTCPRoute, KindUDPRoute +func GetRouteType(route RouteContext) v1beta1.Kind { + rv := reflect.ValueOf(route) + rt := rv.Type().String() + return v1beta1.Kind(strings.TrimSuffix(rt, "Context")[strings.LastIndex(rt, ".")+1:]) } -func (t *TCPRouteContext) GetParentReferences() []v1beta1.ParentReference { - parentReferences := make([]v1beta1.ParentReference, len(t.Spec.ParentRefs)) - for idx, p := range t.Spec.ParentRefs { - parentReferences[idx] = UpgradeParentReference(p) +// TODO: [v1alpha2-v1beta1] This should not be required once all Route +// objects being implemented are of type v1beta1. +// GetHostnames returns the hosts targeted by the Route object. +func GetHostnames(route RouteContext) []string { + rv := reflect.ValueOf(route) + rt := rv.Type().String() + if strings.Contains(rt, "TCP") || strings.Contains(rt, "UDP") { + return nil + } + + h := rv.Elem().FieldByName("Spec").FieldByName("Hostnames") + hostnames := make([]string, h.Len()) + for i := 0; i < len(hostnames); i++ { + hostnames[i] = h.Index(i).String() + } + return hostnames +} + +// TODO: [v1alpha2-v1beta1] This should not be required once all Route +// objects being implemented are of type v1beta1. +// GetParentReferences returns the ParentReference of the Route object. +func GetParentReferences(route RouteContext) []v1beta1.ParentReference { + rv := reflect.ValueOf(route) + rt := rv.Type().String() + pr := rv.Elem().FieldByName("Spec").FieldByName("ParentRefs") + if strings.Contains(rt, "HTTP") || strings.Contains(rt, "GRPC") { + return pr.Interface().([]v1beta1.ParentReference) + } + + parentReferences := make([]v1beta1.ParentReference, pr.Len()) + for i := 0; i < len(parentReferences); i++ { + p := pr.Index(i).Interface().(v1beta1.ParentReference) + parentReferences[i] = UpgradeParentReference(p) } return parentReferences } -func (t *TCPRouteContext) GetRouteStatus() *v1beta1.RouteStatus { - return &t.Status.RouteStatus +// GetRouteStatus returns the RouteStatus object associated with the Route. +func GetRouteStatus(route RouteContext) *v1beta1.RouteStatus { + rv := reflect.ValueOf(route).Elem() + rs := rv.FieldByName("Status").FieldByName("RouteStatus").Interface().(v1beta1.RouteStatus) + return &rs } -func (t *TCPRouteContext) GetRouteParentContext(forParentRef v1beta1.ParentReference) *RouteParentContext { - if t.parentRefs == nil { - t.parentRefs = make(map[v1beta1.ParentReference]*RouteParentContext) +// GetRouteParentContext returns RouteParentContext by using the Route +// objects' ParentReference. +func GetRouteParentContext(route RouteContext, forParentRef v1beta1.ParentReference) *RouteParentContext { + rv := reflect.ValueOf(route).Elem() + pr := rv.FieldByName("ParentRefs") + if pr.IsNil() { + mm := reflect.MakeMap(reflect.TypeOf(map[v1beta1.ParentReference]*RouteParentContext{})) + pr.Set(mm) } - if ctx := t.parentRefs[forParentRef]; ctx != nil { + if p := pr.MapIndex(reflect.ValueOf(forParentRef)); p.IsValid() && !p.IsZero() { + ctx := p.Interface().(*RouteParentContext) return ctx } + isHTTPRoute := false + if strings.Contains(rv.Type().String(), "HTTP") { + isHTTPRoute = true + } + var parentRef *v1beta1.ParentReference - for i, p := range t.Spec.ParentRefs { - p := UpgradeParentReference(p) - if reflect.DeepEqual(p, forParentRef) { - upgraded := UpgradeParentReference(t.Spec.ParentRefs[i]) - parentRef = &upgraded + specParentRefs := rv.FieldByName("Spec").FieldByName("ParentRefs") + for i := 0; i < specParentRefs.Len(); i++ { + p := specParentRefs.Index(i).Interface().(v1beta1.ParentReference) + up := p + if !isHTTPRoute { + up = UpgradeParentReference(p) + } + if reflect.DeepEqual(up, forParentRef) { + if isHTTPRoute { + parentRef = &p + } else { + upgraded := UpgradeParentReference(p) + parentRef = &upgraded + } break } } @@ -569,14 +320,18 @@ func (t *TCPRouteContext) GetRouteParentContext(forParentRef v1beta1.ParentRefer } routeParentStatusIdx := -1 - for i := range t.Status.Parents { - p := UpgradeParentReference(t.Status.Parents[i].ParentRef) - defaultNamespace := v1beta1.Namespace(metav1.NamespaceDefault) - if forParentRef.Namespace == nil { - forParentRef.Namespace = &defaultNamespace - } - if p.Namespace == nil { - p.Namespace = &defaultNamespace + statusParents := rv.FieldByName("Status").FieldByName("Parents") + for i := 0; i < statusParents.Len(); i++ { + p := statusParents.Index(i).FieldByName("ParentRef").Interface().(v1beta1.ParentReference) + if !isHTTPRoute { + p = UpgradeParentReference(p) + defaultNamespace := v1beta1.Namespace(metav1.NamespaceDefault) + if forParentRef.Namespace == nil { + forParentRef.Namespace = &defaultNamespace + } + if p.Namespace == nil { + p.Namespace = &defaultNamespace + } } if reflect.DeepEqual(p, forParentRef) { routeParentStatusIdx = i @@ -584,28 +339,28 @@ func (t *TCPRouteContext) GetRouteParentContext(forParentRef v1beta1.ParentRefer } } if routeParentStatusIdx == -1 { + fpr := forParentRef + if !isHTTPRoute { + fpr = DowngradeParentReference(fpr) + } rParentStatus := v1alpha2.RouteParentStatus{ - ControllerName: v1alpha2.GatewayController(t.GatewayControllerName), - ParentRef: DowngradeParentReference(forParentRef), + ControllerName: v1alpha2.GatewayController(rv.FieldByName("GatewayControllerName").String()), + ParentRef: fpr, } - t.Status.Parents = append(t.Status.Parents, rParentStatus) - routeParentStatusIdx = len(t.Status.Parents) - 1 + statusParents.Set(reflect.Append(statusParents, reflect.ValueOf(rParentStatus))) + routeParentStatusIdx = statusParents.Len() - 1 } ctx := &RouteParentContext{ - ParentReference: parentRef, - - tcpRoute: t.TCPRoute, + ParentReference: parentRef, routeParentStatusIdx: routeParentStatusIdx, } - t.parentRefs[forParentRef] = ctx + rctx := reflect.ValueOf(ctx) + rctx.Elem().FieldByName(string(GetRouteType(route))).Set(rv.Field(1)) + pr.SetMapIndex(reflect.ValueOf(forParentRef), rctx) return ctx } -func (t *TCPRouteContext) GetHostnames() []string { - return nil -} - // RouteParentContext wraps a ParentReference and provides helper methods for // setting conditions and other status information on the associated // HTTPRoute, TLSRoute etc. @@ -614,11 +369,11 @@ type RouteParentContext struct { // TODO: [v1alpha2-v1beta1] This can probably be replaced with // a single field pointing to *v1beta1.RouteStatus. - httpRoute *v1beta1.HTTPRoute - grpcRoute *v1alpha2.GRPCRoute - tlsRoute *v1alpha2.TLSRoute - tcpRoute *v1alpha2.TCPRoute - udpRoute *v1alpha2.UDPRoute + HTTPRoute *v1beta1.HTTPRoute + GRPCRoute *v1alpha2.GRPCRoute + TLSRoute *v1alpha2.TLSRoute + TCPRoute *v1alpha2.TCPRoute + UDPRoute *v1alpha2.UDPRoute routeParentStatusIdx int listeners []*ListenerContext @@ -639,7 +394,7 @@ func (r *RouteParentContext) SetCondition(route RouteContext, conditionType v1be } idx := -1 - routeStatus := route.GetRouteStatus() + routeStatus := GetRouteStatus(route) for i, existing := range routeStatus.Parents[r.routeParentStatusIdx].Conditions { if existing.Type == cond.Type { // return early if the condition is unchanged @@ -662,13 +417,13 @@ func (r *RouteParentContext) SetCondition(route RouteContext, conditionType v1be } func (r *RouteParentContext) ResetConditions(route RouteContext) { - routeStatus := route.GetRouteStatus() + routeStatus := GetRouteStatus(route) routeStatus.Parents[r.routeParentStatusIdx].Conditions = make([]metav1.Condition, 0) } func (r *RouteParentContext) HasCondition(route RouteContext, condType v1beta1.RouteConditionType, status metav1.ConditionStatus) bool { var conditions []metav1.Condition - routeStatus := route.GetRouteStatus() + routeStatus := GetRouteStatus(route) conditions = routeStatus.Parents[r.routeParentStatusIdx].Conditions for _, c := range conditions { if c.Type == string(condType) && c.Status == status { diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go index 8050503569f..bb211189bee 100644 --- a/internal/gatewayapi/route.go +++ b/internal/gatewayapi/route.go @@ -87,7 +87,7 @@ func (t *Translator) ProcessGRPCRoutes(grpcRoutes []*v1alpha2.GRPCRoute, gateway } func (t *Translator) processHTTPRouteParentRefs(httpRoute *HTTPRouteContext, resources *Resources, xdsIR XdsIRMap) { - for _, parentRef := range httpRoute.parentRefs { + for _, parentRef := range httpRoute.ParentRefs { // Need to compute Route rules within the parentRef loop because // any conditions that come out of it have to go on each RouteParentStatus, // not on the Route as a whole. @@ -119,8 +119,8 @@ func (t *Translator) processHTTPRouteParentRefs(httpRoute *HTTPRouteContext, res } // If no negative conditions have been set, the route is considered "Accepted=True". - if parentRef.httpRoute != nil && - len(parentRef.httpRoute.Status.Parents[parentRef.routeParentStatusIdx].Conditions) == 0 { + if parentRef.HTTPRoute != nil && + len(parentRef.HTTPRoute.Status.Parents[parentRef.routeParentStatusIdx].Conditions) == 0 { parentRef.SetCondition(httpRoute, v1beta1.RouteConditionAccepted, metav1.ConditionTrue, @@ -297,7 +297,7 @@ func applyHTTPFiltersContextToIRRoute(httpFiltersContext *HTTPFiltersContext, ir } func (t *Translator) processGRPCRouteParentRefs(grpcRoute *GRPCRouteContext, resources *Resources, xdsIR XdsIRMap) { - for _, parentRef := range grpcRoute.parentRefs { + for _, parentRef := range grpcRoute.ParentRefs { // Need to compute Route rules within the parentRef loop because // any conditions that come out of it have to go on each RouteParentStatus, @@ -328,8 +328,8 @@ func (t *Translator) processGRPCRouteParentRefs(grpcRoute *GRPCRouteContext, res } // If no negative conditions have been set, the route is considered "Accepted=True". - if parentRef.grpcRoute != nil && - len(parentRef.grpcRoute.Status.Parents[parentRef.routeParentStatusIdx].Conditions) == 0 { + if parentRef.GRPCRoute != nil && + len(parentRef.GRPCRoute.Status.Parents[parentRef.routeParentStatusIdx].Conditions) == 0 { parentRef.SetCondition(grpcRoute, v1beta1.RouteConditionAccepted, metav1.ConditionTrue, @@ -480,7 +480,7 @@ func (t *Translator) processHTTPRouteParentRefListener(route RouteContext, route var hasHostnameIntersection bool for _, listener := range parentRef.listeners { - routeHostnames := route.GetHostnames() + routeHostnames := GetHostnames(route) hosts := computeHosts(routeHostnames, listener.Hostname) if len(hosts) == 0 { continue @@ -497,7 +497,7 @@ func (t *Translator) processHTTPRouteParentRefListener(route RouteContext, route irListener.StripAnyHostPort = true } - if route.GetRouteType() == KindGRPCRoute { + if GetRouteType(route) == KindGRPCRoute { irListener.IsHTTP2 = true } } @@ -593,7 +593,7 @@ func (t *Translator) ProcessTLSRoutes(tlsRoutes []*v1alpha2.TLSRoute, gateways [ } func (t *Translator) processTLSRouteParentRefs(tlsRoute *TLSRouteContext, resources *Resources, xdsIR XdsIRMap) { - for _, parentRef := range tlsRoute.parentRefs { + for _, parentRef := range tlsRoute.ParentRefs { // Need to compute Route rules within the parentRef loop because // any conditions that come out of it have to go on each RouteParentStatus, @@ -632,7 +632,7 @@ func (t *Translator) processTLSRouteParentRefs(tlsRoute *TLSRouteContext, resour var hasHostnameIntersection bool for _, listener := range parentRef.listeners { - hosts := computeHosts(tlsRoute.GetHostnames(), listener.Hostname) + hosts := computeHosts(GetHostnames(tlsRoute), listener.Hostname) if len(hosts) == 0 { continue } @@ -673,8 +673,8 @@ func (t *Translator) processTLSRouteParentRefs(tlsRoute *TLSRouteContext, resour } // If no negative conditions have been set, the route is considered "Accepted=True". - if parentRef.tlsRoute != nil && - len(parentRef.tlsRoute.Status.Parents[parentRef.routeParentStatusIdx].Conditions) == 0 { + if parentRef.TLSRoute != nil && + len(parentRef.TLSRoute.Status.Parents[parentRef.routeParentStatusIdx].Conditions) == 0 { parentRef.SetCondition(tlsRoute, v1beta1.RouteConditionAccepted, metav1.ConditionTrue, @@ -715,7 +715,7 @@ func (t *Translator) ProcessUDPRoutes(udpRoutes []*v1alpha2.UDPRoute, gateways [ } func (t *Translator) processUDPRouteParentRefs(udpRoute *UDPRouteContext, resources *Resources, xdsIR XdsIRMap) { - for _, parentRef := range udpRoute.parentRefs { + for _, parentRef := range udpRoute.ParentRefs { // Need to compute Route rules within the parentRef loop because // any conditions that come out of it have to go on each RouteParentStatus, // not on the Route as a whole. @@ -796,8 +796,8 @@ func (t *Translator) processUDPRouteParentRefs(udpRoute *UDPRouteContext, resour } // If no negative conditions have been set, the route is considered "Accepted=True". - if accepted && parentRef.udpRoute != nil && - len(parentRef.udpRoute.Status.Parents[parentRef.routeParentStatusIdx].Conditions) == 0 { + if accepted && parentRef.UDPRoute != nil && + len(parentRef.UDPRoute.Status.Parents[parentRef.routeParentStatusIdx].Conditions) == 0 { parentRef.SetCondition(udpRoute, v1beta1.RouteConditionAccepted, metav1.ConditionTrue, @@ -847,7 +847,7 @@ func (t *Translator) ProcessTCPRoutes(tcpRoutes []*v1alpha2.TCPRoute, gateways [ } func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resources *Resources, xdsIR XdsIRMap) { - for _, parentRef := range tcpRoute.parentRefs { + for _, parentRef := range tcpRoute.ParentRefs { // Need to compute Route rules within the parentRef loop because // any conditions that come out of it have to go on each RouteParentStatus, @@ -929,8 +929,8 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour } // If no negative conditions have been set, the route is considered "Accepted=True". - if accepted && parentRef.tcpRoute != nil && - len(parentRef.tcpRoute.Status.Parents[parentRef.routeParentStatusIdx].Conditions) == 0 { + if accepted && parentRef.TCPRoute != nil && + len(parentRef.TCPRoute.Status.Parents[parentRef.routeParentStatusIdx].Conditions) == 0 { parentRef.SetCondition(tcpRoute, v1beta1.RouteConditionAccepted, metav1.ConditionTrue, @@ -966,7 +966,7 @@ func (t *Translator) processRouteDestinations(backendRef v1beta1.BackendRef, serviceNamespace := NamespaceDerefOr(backendRef.Namespace, route.GetNamespace()) service := resources.GetService(serviceNamespace, string(backendRef.Name)) - routeType := route.GetRouteType() + routeType := GetRouteType(route) if !t.validateBackendRef(&backendRef, parentRef, route, resources, serviceNamespace, routeType) { return nil, weight } @@ -993,7 +993,7 @@ func (t *Translator) processRouteDestinations(backendRef v1beta1.BackendRef, func (t *Translator) processAllowedListenersForParentRefs(routeContext RouteContext, gateways []*GatewayContext, resources *Resources) bool { var relevantRoute bool - for _, parentRef := range routeContext.GetParentReferences() { + for _, parentRef := range GetParentReferences(routeContext) { isRelevantParentRef, selectedListeners := GetReferencedListeners(parentRef, gateways) // Parent ref is not to a Gateway that we control: skip it @@ -1002,7 +1002,7 @@ func (t *Translator) processAllowedListenersForParentRefs(routeContext RouteCont } relevantRoute = true - parentRefCtx := routeContext.GetRouteParentContext(parentRef) + parentRefCtx := GetRouteParentContext(routeContext, parentRef) // Reset conditions since they will be recomputed during translation parentRefCtx.ResetConditions(routeContext) @@ -1028,7 +1028,7 @@ func (t *Translator) processAllowedListenersForParentRefs(routeContext RouteCont var allowedListeners []*ListenerContext for _, listener := range selectedListeners { - acceptedKind := routeContext.GetRouteType() + acceptedKind := GetRouteType(routeContext) if listener.AllowsKind(v1beta1.RouteGroupKind{Group: GroupPtr(v1beta1.GroupName), Kind: acceptedKind}) && listener.AllowsNamespace(resources.GetNamespace(routeContext.GetNamespace())) { allowedListeners = append(allowedListeners, listener) From 9db75cc12d36697c4dc8abe644691fe40dc0871d Mon Sep 17 00:00:00 2001 From: Shawnh2 Date: Wed, 7 Jun 2023 21:15:21 +0800 Subject: [PATCH 2/5] clean up code Signed-off-by: Shawnh2 --- internal/gatewayapi/contexts.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/internal/gatewayapi/contexts.go b/internal/gatewayapi/contexts.go index 41b2332c629..bcbc24cda76 100644 --- a/internal/gatewayapi/contexts.go +++ b/internal/gatewayapi/contexts.go @@ -155,15 +155,6 @@ func (l *ListenerContext) SetTLSSecrets(tlsSecrets []*v1.Secret) { // that can reference Gateway objects. type RouteContext interface { client.Object - - // GetRouteParentContext returns RouteParentContext by using the Route - // objects' ParentReference. - //GetRouteParentContext(forParentRef v1beta1.ParentReference) *RouteParentContext - // - // TODO: [v1alpha2-v1beta1] This should not be required once all Route - // objects being implemented are of type v1beta1. - // GetHostnames returns the hosts targeted by the Route object. - //GetHostnames() []string } // HTTPRouteContext wraps an HTTPRoute and provides helper methods for From 00a1e80a6c825385cfba3c0808657a9e642850df Mon Sep 17 00:00:00 2001 From: Shawnh2 Date: Wed, 7 Jun 2023 21:27:54 +0800 Subject: [PATCH 3/5] fix lint Signed-off-by: Shawnh2 --- internal/gatewayapi/contexts.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/gatewayapi/contexts.go b/internal/gatewayapi/contexts.go index bcbc24cda76..421810dfef5 100644 --- a/internal/gatewayapi/contexts.go +++ b/internal/gatewayapi/contexts.go @@ -330,13 +330,13 @@ func GetRouteParentContext(route RouteContext, forParentRef v1beta1.ParentRefere } } if routeParentStatusIdx == -1 { - fpr := forParentRef + tmpPR := forParentRef if !isHTTPRoute { - fpr = DowngradeParentReference(fpr) + tmpPR = DowngradeParentReference(tmpPR) } rParentStatus := v1alpha2.RouteParentStatus{ ControllerName: v1alpha2.GatewayController(rv.FieldByName("GatewayControllerName").String()), - ParentRef: fpr, + ParentRef: tmpPR, } statusParents.Set(reflect.Append(statusParents, reflect.ValueOf(rParentStatus))) routeParentStatusIdx = statusParents.Len() - 1 From 8b1aff9d0f1542c06f0dc12205f749bdd06198b9 Mon Sep 17 00:00:00 2001 From: Shawnh2 Date: Sat, 17 Jun 2023 00:05:14 +0800 Subject: [PATCH 4/5] get route type info from Kind by FieldByName Signed-off-by: Shawnh2 --- .../translate/out/default-resources.all.yaml | 15 ++++++++----- .../out/echo-gateway-api.cluster.yaml | 3 ++- .../translate/out/echo-gateway-api.route.json | 1 + .../translate/out/invalid-envoyproxy.all.yaml | 15 ++++++++----- .../out/rejected-http-route.route.yaml | 3 ++- .../translate/out/valid-envoyproxy.all.yaml | 15 ++++++++----- internal/cmd/egctl/translate.go | 15 +++++++++++++ internal/gatewayapi/contexts.go | 21 +++++++------------ 8 files changed, 58 insertions(+), 30 deletions(-) diff --git a/internal/cmd/egctl/testdata/translate/out/default-resources.all.yaml b/internal/cmd/egctl/testdata/translate/out/default-resources.all.yaml index 7c579ae9e3f..0dfe6f83775 100644 --- a/internal/cmd/egctl/testdata/translate/out/default-resources.all.yaml +++ b/internal/cmd/egctl/testdata/translate/out/default-resources.all.yaml @@ -210,7 +210,8 @@ gateways: - group: gateway.networking.k8s.io kind: GRPCRoute grpcRoutes: -- metadata: +- kind: GRPCRoute + metadata: creationTimestamp: null name: backend namespace: default @@ -249,7 +250,8 @@ grpcRoutes: name: eg sectionName: grpc httpRoutes: -- metadata: +- kind: HTTPRoute + metadata: creationTimestamp: null name: backend namespace: default @@ -286,7 +288,8 @@ httpRoutes: parentRef: name: eg tcpRoutes: -- metadata: +- kind: TCPRoute + metadata: creationTimestamp: null name: backend namespace: default @@ -319,7 +322,8 @@ tcpRoutes: name: eg sectionName: tcp tlsRoutes: -- metadata: +- kind: TLSRoute + metadata: creationTimestamp: null name: backend namespace: default @@ -352,7 +356,8 @@ tlsRoutes: name: eg sectionName: tls-passthrough udpRoutes: -- metadata: +- kind: UDPRoute + metadata: creationTimestamp: null name: backend namespace: default diff --git a/internal/cmd/egctl/testdata/translate/out/echo-gateway-api.cluster.yaml b/internal/cmd/egctl/testdata/translate/out/echo-gateway-api.cluster.yaml index e82c3acf116..c5dbc42ccf4 100644 --- a/internal/cmd/egctl/testdata/translate/out/echo-gateway-api.cluster.yaml +++ b/internal/cmd/egctl/testdata/translate/out/echo-gateway-api.cluster.yaml @@ -44,7 +44,8 @@ gateways: - group: gateway.networking.k8s.io kind: GRPCRoute httpRoutes: -- metadata: +- kind: HTTPRoute + metadata: creationTimestamp: null name: backend namespace: envoy-gateway-system diff --git a/internal/cmd/egctl/testdata/translate/out/echo-gateway-api.route.json b/internal/cmd/egctl/testdata/translate/out/echo-gateway-api.route.json index 80b65f6cb1a..90cc143a04f 100644 --- a/internal/cmd/egctl/testdata/translate/out/echo-gateway-api.route.json +++ b/internal/cmd/egctl/testdata/translate/out/echo-gateway-api.route.json @@ -75,6 +75,7 @@ ], "httpRoutes": [ { + "kind": "HTTPRoute", "metadata": { "name": "backend", "namespace": "envoy-gateway-system", diff --git a/internal/cmd/egctl/testdata/translate/out/invalid-envoyproxy.all.yaml b/internal/cmd/egctl/testdata/translate/out/invalid-envoyproxy.all.yaml index 1f155bff6f3..5c6ef54cbab 100644 --- a/internal/cmd/egctl/testdata/translate/out/invalid-envoyproxy.all.yaml +++ b/internal/cmd/egctl/testdata/translate/out/invalid-envoyproxy.all.yaml @@ -153,7 +153,8 @@ gateways: - group: gateway.networking.k8s.io kind: GRPCRoute grpcRoutes: -- metadata: +- kind: GRPCRoute + metadata: creationTimestamp: null name: backend namespace: default @@ -192,7 +193,8 @@ grpcRoutes: name: eg sectionName: grpc httpRoutes: -- metadata: +- kind: HTTPRoute + metadata: creationTimestamp: null name: backend namespace: default @@ -229,7 +231,8 @@ httpRoutes: parentRef: name: eg tcpRoutes: -- metadata: +- kind: TCPRoute + metadata: creationTimestamp: null name: backend namespace: default @@ -262,7 +265,8 @@ tcpRoutes: name: eg sectionName: tcp tlsRoutes: -- metadata: +- kind: TLSRoute + metadata: creationTimestamp: null name: backend namespace: default @@ -295,7 +299,8 @@ tlsRoutes: name: eg sectionName: tls-passthrough udpRoutes: -- metadata: +- kind: UDPRoute + metadata: creationTimestamp: null name: backend namespace: default diff --git a/internal/cmd/egctl/testdata/translate/out/rejected-http-route.route.yaml b/internal/cmd/egctl/testdata/translate/out/rejected-http-route.route.yaml index 252a27f7873..9ab3b49be96 100644 --- a/internal/cmd/egctl/testdata/translate/out/rejected-http-route.route.yaml +++ b/internal/cmd/egctl/testdata/translate/out/rejected-http-route.route.yaml @@ -39,7 +39,8 @@ gateways: - group: gateway.networking.k8s.io kind: TLSRoute httpRoutes: -- metadata: +- kind: "HTTPRoute" + metadata: creationTimestamp: null name: backend namespace: envoy-gateway-system diff --git a/internal/cmd/egctl/testdata/translate/out/valid-envoyproxy.all.yaml b/internal/cmd/egctl/testdata/translate/out/valid-envoyproxy.all.yaml index 419b3a21f3c..c02ac7f6cb6 100644 --- a/internal/cmd/egctl/testdata/translate/out/valid-envoyproxy.all.yaml +++ b/internal/cmd/egctl/testdata/translate/out/valid-envoyproxy.all.yaml @@ -148,7 +148,8 @@ gateways: - group: gateway.networking.k8s.io kind: GRPCRoute grpcRoutes: -- metadata: +- kind: GRPCRoute + metadata: creationTimestamp: null name: backend namespace: default @@ -187,7 +188,8 @@ grpcRoutes: name: eg sectionName: grpc httpRoutes: -- metadata: +- kind: HTTPRoute + metadata: creationTimestamp: null name: backend namespace: default @@ -224,7 +226,8 @@ httpRoutes: parentRef: name: eg tcpRoutes: -- metadata: +- kind: TCPRoute + metadata: creationTimestamp: null name: backend namespace: default @@ -257,7 +260,8 @@ tcpRoutes: name: eg sectionName: tcp tlsRoutes: -- metadata: +- kind: TLSRoute + metadata: creationTimestamp: null name: backend namespace: default @@ -290,7 +294,8 @@ tlsRoutes: name: eg sectionName: tls-passthrough udpRoutes: -- metadata: +- kind: UDPRoute + metadata: creationTimestamp: null name: backend namespace: default diff --git a/internal/cmd/egctl/translate.go b/internal/cmd/egctl/translate.go index 102cd18b936..3285f9c0f70 100644 --- a/internal/cmd/egctl/translate.go +++ b/internal/cmd/egctl/translate.go @@ -641,6 +641,9 @@ func kubernetesYAMLToResources(str string, addMissingResources bool) (*gatewayap case gatewayapi.KindTCPRoute: typedSpec := spec.Interface() tcpRoute := &v1alpha2.TCPRoute{ + TypeMeta: metav1.TypeMeta{ + Kind: gatewayapi.KindTCPRoute, + }, ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, @@ -651,6 +654,9 @@ func kubernetesYAMLToResources(str string, addMissingResources bool) (*gatewayap case gatewayapi.KindUDPRoute: typedSpec := spec.Interface() udpRoute := &v1alpha2.UDPRoute{ + TypeMeta: metav1.TypeMeta{ + Kind: gatewayapi.KindUDPRoute, + }, ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, @@ -661,6 +667,9 @@ func kubernetesYAMLToResources(str string, addMissingResources bool) (*gatewayap case gatewayapi.KindTLSRoute: typedSpec := spec.Interface() tlsRoute := &v1alpha2.TLSRoute{ + TypeMeta: metav1.TypeMeta{ + Kind: gatewayapi.KindTLSRoute, + }, ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, @@ -671,6 +680,9 @@ func kubernetesYAMLToResources(str string, addMissingResources bool) (*gatewayap case gatewayapi.KindHTTPRoute: typedSpec := spec.Interface() httpRoute := &v1beta1.HTTPRoute{ + TypeMeta: metav1.TypeMeta{ + Kind: gatewayapi.KindHTTPRoute, + }, ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, @@ -681,6 +693,9 @@ func kubernetesYAMLToResources(str string, addMissingResources bool) (*gatewayap case gatewayapi.KindGRPCRoute: typedSpec := spec.Interface() grpcRoute := &v1alpha2.GRPCRoute{ + TypeMeta: metav1.TypeMeta{ + Kind: gatewayapi.KindGRPCRoute, + }, ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, diff --git a/internal/gatewayapi/contexts.go b/internal/gatewayapi/contexts.go index 421810dfef5..065ae4ea24f 100644 --- a/internal/gatewayapi/contexts.go +++ b/internal/gatewayapi/contexts.go @@ -214,27 +214,22 @@ type TCPRouteContext struct { // GetRouteType returns the Kind of the Route object, HTTPRoute, // TLSRoute, TCPRoute, UDPRoute etc. -// -// This function use the typename of RouteContext and its return is -// corresponding to the const defined in translator, like KindHTTPRoute, -// KindGRPCRoute, KindTLSRoute, KindTCPRoute, KindUDPRoute func GetRouteType(route RouteContext) v1beta1.Kind { - rv := reflect.ValueOf(route) - rt := rv.Type().String() - return v1beta1.Kind(strings.TrimSuffix(rt, "Context")[strings.LastIndex(rt, ".")+1:]) + rv := reflect.ValueOf(route).Elem() + return v1beta1.Kind(rv.FieldByName("Kind").String()) } // TODO: [v1alpha2-v1beta1] This should not be required once all Route // objects being implemented are of type v1beta1. // GetHostnames returns the hosts targeted by the Route object. func GetHostnames(route RouteContext) []string { - rv := reflect.ValueOf(route) - rt := rv.Type().String() + rv := reflect.ValueOf(route).Elem() + rt := rv.FieldByName("Kind").String() if strings.Contains(rt, "TCP") || strings.Contains(rt, "UDP") { return nil } - h := rv.Elem().FieldByName("Spec").FieldByName("Hostnames") + h := rv.FieldByName("Spec").FieldByName("Hostnames") hostnames := make([]string, h.Len()) for i := 0; i < len(hostnames); i++ { hostnames[i] = h.Index(i).String() @@ -246,9 +241,9 @@ func GetHostnames(route RouteContext) []string { // objects being implemented are of type v1beta1. // GetParentReferences returns the ParentReference of the Route object. func GetParentReferences(route RouteContext) []v1beta1.ParentReference { - rv := reflect.ValueOf(route) - rt := rv.Type().String() - pr := rv.Elem().FieldByName("Spec").FieldByName("ParentRefs") + rv := reflect.ValueOf(route).Elem() + rt := rv.FieldByName("Kind").String() + pr := rv.FieldByName("Spec").FieldByName("ParentRefs") if strings.Contains(rt, "HTTP") || strings.Contains(rt, "GRPC") { return pr.Interface().([]v1beta1.ParentReference) } From efd5395355ab55d94bcca6d44f4f499acaad38c7 Mon Sep 17 00:00:00 2001 From: Shawnh2 Date: Sat, 17 Jun 2023 11:30:54 +0800 Subject: [PATCH 5/5] replacing string contains with the constants that already defined Signed-off-by: Shawnh2 --- internal/gatewayapi/contexts.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/internal/gatewayapi/contexts.go b/internal/gatewayapi/contexts.go index 065ae4ea24f..03823dc6642 100644 --- a/internal/gatewayapi/contexts.go +++ b/internal/gatewayapi/contexts.go @@ -7,7 +7,6 @@ package gatewayapi import ( "reflect" - "strings" "time" v1 "k8s.io/api/core/v1" @@ -224,15 +223,15 @@ func GetRouteType(route RouteContext) v1beta1.Kind { // GetHostnames returns the hosts targeted by the Route object. func GetHostnames(route RouteContext) []string { rv := reflect.ValueOf(route).Elem() - rt := rv.FieldByName("Kind").String() - if strings.Contains(rt, "TCP") || strings.Contains(rt, "UDP") { + kind := rv.FieldByName("Kind").String() + if kind == KindTCPRoute || kind == KindUDPRoute { return nil } - h := rv.FieldByName("Spec").FieldByName("Hostnames") - hostnames := make([]string, h.Len()) + hs := rv.FieldByName("Spec").FieldByName("Hostnames") + hostnames := make([]string, hs.Len()) for i := 0; i < len(hostnames); i++ { - hostnames[i] = h.Index(i).String() + hostnames[i] = hs.Index(i).String() } return hostnames } @@ -242,9 +241,9 @@ func GetHostnames(route RouteContext) []string { // GetParentReferences returns the ParentReference of the Route object. func GetParentReferences(route RouteContext) []v1beta1.ParentReference { rv := reflect.ValueOf(route).Elem() - rt := rv.FieldByName("Kind").String() + kind := rv.FieldByName("Kind").String() pr := rv.FieldByName("Spec").FieldByName("ParentRefs") - if strings.Contains(rt, "HTTP") || strings.Contains(rt, "GRPC") { + if kind == KindHTTPRoute || kind == KindGRPCRoute { return pr.Interface().([]v1beta1.ParentReference) } @@ -279,7 +278,7 @@ func GetRouteParentContext(route RouteContext, forParentRef v1beta1.ParentRefere } isHTTPRoute := false - if strings.Contains(rv.Type().String(), "HTTP") { + if rv.FieldByName("Kind").String() == KindHTTPRoute { isHTTPRoute = true }