lingsamuel commented on code in PR #1440:
URL:
https://github.com/apache/apisix-ingress-controller/pull/1440#discussion_r1015237155
##########
pkg/providers/gateway/provider.go:
##########
@@ -274,6 +286,30 @@ func (p *Provider) RemoveListeners(ns, name string) error {
}
func (p *Provider) FindListener(ns, name, sectionName string)
(*types.ListenerConf, error) {
+ p.listenersLock.Lock()
+ defer p.listenersLock.Unlock()
+ key := ns + "/" + name
+ listeners, exist := p.listeners[key]
+ if !exist {
+ return nil, ErrListenerNotExist
+ }
+ for _, listener := range listeners {
+ if listener.SectionName == sectionName {
+ return listener, nil
+ }
+ }
return nil, nil
}
+
+func (p *Provider) QueryListeners(ns, name string)
(map[string]*types.ListenerConf, error) {
+ p.listenersLock.Lock()
Review Comment:
RLock is enough here
##########
pkg/providers/gateway/provider.go:
##########
@@ -274,6 +286,30 @@ func (p *Provider) RemoveListeners(ns, name string) error {
}
func (p *Provider) FindListener(ns, name, sectionName string)
(*types.ListenerConf, error) {
+ p.listenersLock.Lock()
Review Comment:
RLock is enough here
##########
pkg/providers/utils/string.go:
##########
@@ -45,3 +45,11 @@ func Equal(a, b []string) bool {
}
return len(Difference(a, b)) == 0 && len(Difference(b, a)) == 0
}
+
+func ReverseString(s string) string {
+ var reversed string
+ for _, v := range s {
+ reversed = string(v) + reversed
Review Comment:
use rune array and convert it to string in return statement would be better
than convert rune to string every iteration.
##########
pkg/providers/gateway/validator.go:
##########
@@ -0,0 +1,249 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package gateway
+
+import (
+ "context"
+ "fmt"
+
+ "github.com/apache/apisix-ingress-controller/pkg/log"
+
"github.com/apache/apisix-ingress-controller/pkg/providers/gateway/types"
+ "go.uber.org/zap"
+ corev1 "k8s.io/api/core/v1"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
+ gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
+)
+
+type Validator struct {
+ provider *Provider
+}
+
+func newValidator(p *Provider) *Validator {
+ return &Validator{
+ provider: p,
+ }
+}
+
+type commonRoute struct {
+ routeNamespace string
+ parentRefs []gatewayv1alpha2.ParentRef
+ routeProtocol gatewayv1alpha2.ProtocolType
+ routeHostnames []gatewayv1alpha2.Hostname
+ routeGroupKind gatewayv1alpha2.RouteGroupKind
+}
+
+func (r *commonRoute) hasParentRefs() bool {
+ return len(r.parentRefs) != 0
+}
+
+func (r *commonRoute) isHTTPProtocol() bool {
+ return r.routeProtocol == gatewayv1alpha2.HTTPProtocolType
+}
+
+func (r *commonRoute) isHTTPSProtocol() bool {
+ return r.routeProtocol == gatewayv1alpha2.HTTPSProtocolType
+}
+
+func parseToCommentRoute(route any) (*commonRoute, error) {
+ r := new(commonRoute)
+ group := gatewayv1alpha2.Group(gatewayv1alpha2.GroupName)
+ switch route := route.(type) {
+ case *gatewayv1alpha2.HTTPRoute:
+ r.routeNamespace = route.Namespace
+ r.parentRefs = route.Spec.ParentRefs
+ r.routeProtocol = gatewayv1alpha2.HTTPProtocolType
+ r.routeHostnames = route.Spec.Hostnames
+ r.routeGroupKind = gatewayv1alpha2.RouteGroupKind{
+ Group: &group,
+ Kind: types.KindHTTPRoute,
+ }
+ case *gatewayv1alpha2.TLSRoute:
+ r.routeNamespace = route.Namespace
+ r.parentRefs = route.Spec.ParentRefs
+ r.routeProtocol = gatewayv1alpha2.HTTPSProtocolType
+ r.routeHostnames = route.Spec.Hostnames
+ r.routeGroupKind = gatewayv1alpha2.RouteGroupKind{
+ Group: &group,
+ Kind: types.KindTLSRoute,
+ }
+ case *gatewayv1alpha2.TCPRoute:
+ r.routeNamespace = route.Namespace
+ r.parentRefs = route.Spec.ParentRefs
+ r.routeProtocol = gatewayv1alpha2.TCPProtocolType
+ r.routeGroupKind = gatewayv1alpha2.RouteGroupKind{
+ Group: &group,
+ Kind: types.KindTCPRoute,
+ }
+ case *gatewayv1alpha2.UDPRoute:
+ r.routeNamespace = route.Namespace
+ r.parentRefs = route.Spec.ParentRefs
+ r.routeProtocol = gatewayv1alpha2.UDPProtocolType
+ r.routeGroupKind = gatewayv1alpha2.RouteGroupKind{
+ Group: &group,
+ Kind: types.KindUDPRoute,
+ }
+ default:
+ return nil, fmt.Errorf("validator unsupported Route")
Review Comment:
Would be better to include the exact type in the error message
##########
test/e2e/scaffold/ingress.go:
##########
@@ -200,6 +200,14 @@ rules:
- get
- list
- watch
+ - apiGroups:
+ - gateway.networking.k8s.io
+ resources:
+ - gateways/status
+ - gatewayclasses/status
+ verbs:
+ - get
+ - update
Review Comment:
We can also add `list` verb here
##########
test/e2e/suite-gateway/route_attchment.go:
##########
@@ -0,0 +1,288 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package gateway
+
+import (
+ "fmt"
+ "time"
+
+ "github.com/onsi/ginkgo/v2"
+ "github.com/stretchr/testify/assert"
+
+ "github.com/apache/apisix-ingress-controller/test/e2e/scaffold"
+)
+
+var _ = ginkgo.Describe("suite-gateway: Route Attachment", func() {
+ s := scaffold.NewDefaultScaffold()
+
+ gatewayClassName := "test-gateway-class"
+ gatewayName := "test-gateway"
+ // create Gateway resource with AllowedRoute
+ ginkgo.JustBeforeEach(func() {
+ gatewayClass := fmt.Sprintf(`
+apiVersion: gateway.networking.k8s.io/v1alpha2
+kind: GatewayClass
+metadata:
+ name: %s
+spec:
+ controllerName: apisix.apache.org/gateway-controller
+`, gatewayClassName)
+ gateway := fmt.Sprintf(`
+apiVersion: gateway.networking.k8s.io/v1alpha2
+kind: Gateway
+metadata:
+ namespace: %s
+ name: %s
+spec:
+ gatewayClassName: %s
+ listeners:
+ - protocol: HTTP
+ port: 80
+ name: same-namespace-route
+ allowedRoutes:
+ namespaces:
+ from: Same
+ - protocol: HTTP
+ port: 80
+ name: crocss-namespace-route
Review Comment:
same typos below
##########
pkg/providers/utils/domain.go:
##########
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package utils
+
+import (
+ "strings"
+)
+
+// IsHostnameMatch follow GatewayAPI specification to match listenr and route
hostname.
+// FYI:
https://github.com/kubernetes-sigs/gateway-api/blob/a596211672a5aed54881862dc87c8c1cad9c7bd8/apis/v1beta1/gateway_types.go#L154
+func IsHostnameMatch(listener, route string) bool {
+ if listener == "" || route == "" {
+ return true
+ }
+
+ // reverse hostname from "ingress.apisix.com" to "moc.xisipa.ssergni"
for matching
+ l := ReverseString(listener)
+ r := ReverseString(route)
+
+ lLabels := strings.Split(l, ".")
+ rLabels := strings.Split(r, ".")
+ lLength := len(lLabels)
+ rLength := len(rLabels)
+
+ // helper function
+ isLastLabel := func(labels *[]string, idx int) bool {
+ if labels == &lLabels {
+ return idx == lLength-1
+ }
+ if labels == &rLabels {
+ return idx == rLength-1
+ }
+ return false
+ }
+
+ // reverse matching, hostname seq like ["moc", "xisipa", "ssergni"]
+ for i := 0; i < lLength || i < rLength; i++ {
Review Comment:
Using two variables (`i,j`) here can make the logic clearer and simpler.
##########
test/e2e/suite-gateway/route_attchment.go:
##########
@@ -0,0 +1,288 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package gateway
+
+import (
+ "fmt"
+ "time"
+
+ "github.com/onsi/ginkgo/v2"
+ "github.com/stretchr/testify/assert"
+
+ "github.com/apache/apisix-ingress-controller/test/e2e/scaffold"
+)
+
+var _ = ginkgo.Describe("suite-gateway: Route Attachment", func() {
+ s := scaffold.NewDefaultScaffold()
+
+ gatewayClassName := "test-gateway-class"
+ gatewayName := "test-gateway"
+ // create Gateway resource with AllowedRoute
+ ginkgo.JustBeforeEach(func() {
+ gatewayClass := fmt.Sprintf(`
+apiVersion: gateway.networking.k8s.io/v1alpha2
+kind: GatewayClass
+metadata:
+ name: %s
+spec:
+ controllerName: apisix.apache.org/gateway-controller
+`, gatewayClassName)
+ gateway := fmt.Sprintf(`
+apiVersion: gateway.networking.k8s.io/v1alpha2
+kind: Gateway
+metadata:
+ namespace: %s
+ name: %s
+spec:
+ gatewayClassName: %s
+ listeners:
+ - protocol: HTTP
+ port: 80
+ name: same-namespace-route
+ allowedRoutes:
+ namespaces:
+ from: Same
+ - protocol: HTTP
+ port: 80
+ name: crocss-namespace-route
Review Comment:
```suggestion
name: cross-namespace-route
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
