You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/06/07 15:43:10 UTC

[GitHub] [apisix-ingress-controller] tao12345666333 commented on a diff in pull request #1060: e2e: gateway api httproute

tao12345666333 commented on code in PR #1060:
URL: https://github.com/apache/apisix-ingress-controller/pull/1060#discussion_r891388113


##########
pkg/kube/translation/gateway_httproute.go:
##########
@@ -52,25 +52,46 @@ func (t *translator) TranslateGatewayHTTPRouteV1Alpha2(httpRoute *gatewayv1alpha
 		for j, backend := range backends {
 			//TODO: Support filters
 			//filters := backend.Filters
-			kind := strings.ToLower(string(*backend.Kind))
+			var kind string
+			if backend.Kind == nil {
+				kind = "service"
+			} else {
+				kind = strings.ToLower(string(*backend.Kind))
+			}
 			if kind != "service" {
 				log.Warnw(fmt.Sprintf("ignore non-service kind at Rules[%v].BackendRefs[%v]", i, j),
 					zap.String("kind", kind),
 				)
 				continue
 			}
 
-			ns := string(*backend.Namespace)
+			var ns string
+			if backend.Namespace == nil {
+				ns = httpRoute.Namespace
+			} else {
+				ns = string(*backend.Namespace)
+			}
 			//if ns != httpRoute.Namespace {
 			// TODO: check gatewayv1alpha2.ReferencePolicy
 			//}
 
+			if backend.Port == nil {
+				log.Warnw(fmt.Sprintf("ignore nil port at Rules[%v].BackendRefs[%v]", i, j),
+					zap.String("kind", kind),
+				)
+				continue
+			}
+
 			ups, err := t.TranslateUpstream(ns, string(backend.Name), "", int32(*backend.Port))
 			if err != nil {
 				return nil, errors.Wrap(err, fmt.Sprintf("failed to translate Rules[%v].BackendRefs[%v]", i, j))
 			}
 			name := apisixv1.ComposeUpstreamName(ns, string(backend.Name), "", int32(*backend.Port))
-			ups.Labels["id-name"] = name
+
+			ups.Labels["meta_namespace"] = truncate(ns, 64)

Review Comment:
   We can add a comment explaining why 64 is used  https://github.com/apache/apisix/blob/5b95b85faea3094d5e466ee2d39a52f1f805abbb/apisix/schema_def.lua#L85



##########
pkg/kube/translation/util.go:
##########
@@ -261,3 +261,10 @@ func validateRemoteAddrs(remoteAddrs []string) error {
 	}
 	return nil
 }
+
+func truncate(s string, max int) string {

Review Comment:
   we should check max must be non-negative



##########
pkg/kube/translation/translator.go:
##########
@@ -230,7 +232,14 @@ func (t *translator) TranslateUpstream(namespace, name, subset string, port int3
 }
 
 func (t *translator) TranslateUpstreamNodes(endpoint kube.Endpoint, port int32, labels types.Labels) (apisixv1.UpstreamNodes, error) {
-	namespace := endpoint.Namespace()
+	namespace, err := endpoint.Namespace()
+	if err != nil {
+		log.Warnw("failed to get endpoint namespace",

Review Comment:
   ditto, about log level



##########
pkg/ingress/gateway_httproute.go:
##########
@@ -121,7 +123,7 @@ func (c *gatewayHTTPRouteController) sync(ctx context.Context, ev *types.Event)
 	tctx, err := c.controller.translator.TranslateGatewayHTTPRouteV1Alpha2(httpRoute)
 
 	if err != nil {
-		log.Errorw("failed to translate gateway HTTPRoute",
+		log.Warnw("failed to translate gateway HTTPRoute",

Review Comment:
   This is an error, we log it with a higher level, isn't it better?



##########
.gitattributes:
##########
@@ -0,0 +1,3 @@
+# Gateway API CRDs:
+# https://github.com/kubernetes-sigs/gateway-api/tree/66a7c8ffc942a90294612f63c6dedcc73a3de5ca/config/crd/stable
+samples/deploy/gateway-api/* linguist-generated=true

Review Comment:
   good job! I like this configuration item



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org