diff --git a/internal/ingress/controller/store/endpointslice.go b/internal/ingress/controller/store/endpointslice.go index fdd7374e9e5..78d08869514 100644 --- a/internal/ingress/controller/store/endpointslice.go +++ b/internal/ingress/controller/store/endpointslice.go @@ -21,6 +21,7 @@ import ( "strings" discoveryv1 "k8s.io/api/discovery/v1" + apiNames "k8s.io/apiserver/pkg/storage/names" "k8s.io/client-go/tools/cache" ) @@ -32,9 +33,21 @@ type EndpointSliceLister struct { // MatchByKey returns the EndpointsSlices of the Service matching key in the local Endpoint Store. func (s *EndpointSliceLister) MatchByKey(key string) ([]*discoveryv1.EndpointSlice, error) { var eps []*discoveryv1.EndpointSlice + keyNsLen := strings.Index(key, "/") + if keyNsLen < -1 { + keyNsLen = 0 + } else { + // count '/' char + keyNsLen += 1 + } // filter endpointSlices owned by svc for _, listKey := range s.ListKeys() { - if !strings.HasPrefix(listKey, key) { + if len(key) < (apiNames.MaxGeneratedNameLength+keyNsLen) && !strings.HasPrefix(listKey, key) { + continue + } + // generated endpointslices names has truncated svc name as prefix when svc name is too long, we compare only non truncated part + // https://github.com/kubernetes/ingress-nginx/issues/9240 + if len(key) >= (apiNames.MaxGeneratedNameLength+keyNsLen) && !strings.HasPrefix(listKey, key[:apiNames.MaxGeneratedNameLength+keyNsLen-1]) { continue } epss, exists, err := s.GetByKey(listKey) diff --git a/internal/ingress/controller/store/endpointslice_test.go b/internal/ingress/controller/store/endpointslice_test.go index fdc51c0e486..e12a98c2f4f 100644 --- a/internal/ingress/controller/store/endpointslice_test.go +++ b/internal/ingress/controller/store/endpointslice_test.go @@ -17,6 +17,7 @@ limitations under the License. package store import ( + "fmt" "testing" discoveryv1 "k8s.io/api/discovery/v1" @@ -91,4 +92,42 @@ func TestEndpointSliceLister(t *testing.T) { t.Errorf("expected %v, error, got %v", endpointSlice.GetName(), eps[0].GetName()) } }) + t.Run("svc long name", func(t *testing.T) { + el := newEndpointSliceLister(t) + ns := "namespace" + ns2 := "another-ns" + svcName := "test-backend-http-test-http-test-http-test-http-test-http-truncated" + svcName2 := "another-long-svc-name-for-test-inhttp-test-http-test-http-truncated" + key := fmt.Sprintf("%s/%s", ns, svcName) + endpointSlice := &discoveryv1.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: "test-backend-http-test-http-test-http-test-http-test-http-bar88", + Labels: map[string]string{ + discoveryv1.LabelServiceName: svcName, + }, + }, + } + el.Add(endpointSlice) + endpointSlice2 := &discoveryv1.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns2, + Name: "another-long-svc-name-for-test-inhttp-test-http-test-http-bar88", + Labels: map[string]string{ + discoveryv1.LabelServiceName: svcName2, + }, + }, + } + el.Add(endpointSlice2) + eps, err := el.MatchByKey(key) + if err != nil { + t.Errorf("unexpeted error %v", err) + } + if len(eps) != 1 { + t.Errorf("expected one slice %v, error, got %d slices", endpointSlice, len(eps)) + } + if len(eps) == 1 && eps[0].Labels[discoveryv1.LabelServiceName] != svcName { + t.Errorf("expected slice %v, error, got %v slices", endpointSlice, eps[0]) + } + }) } diff --git a/test/e2e/e2e.go b/test/e2e/e2e.go index c7010b977e9..57b04723082 100644 --- a/test/e2e/e2e.go +++ b/test/e2e/e2e.go @@ -34,6 +34,7 @@ import ( _ "k8s.io/ingress-nginx/test/e2e/annotations/modsecurity" _ "k8s.io/ingress-nginx/test/e2e/dbg" _ "k8s.io/ingress-nginx/test/e2e/defaultbackend" + _ "k8s.io/ingress-nginx/test/e2e/endpointslices" _ "k8s.io/ingress-nginx/test/e2e/gracefulshutdown" _ "k8s.io/ingress-nginx/test/e2e/ingress" _ "k8s.io/ingress-nginx/test/e2e/leaks" diff --git a/test/e2e/endpointslices/longname.go b/test/e2e/endpointslices/longname.go new file mode 100644 index 00000000000..0adb6676713 --- /dev/null +++ b/test/e2e/endpointslices/longname.go @@ -0,0 +1,55 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed 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 endpointslices + +import ( + "fmt" + "net/http" + "strings" + + "github.com/onsi/ginkgo/v2" + + "k8s.io/ingress-nginx/test/e2e/framework" +) + +var _ = framework.IngressNginxDescribe("[Endpointslices] long service name", func() { + f := framework.NewDefaultFramework("endpointslices") + host := "longsvcname.foo.com" + name := "long-name-foobar-foobar-foobar-foobar-foobar-bar-foo-bar-foobuz" + + ginkgo.BeforeEach(func() { + f.NewEchoDeployment(framework.WithName(name)) + }) + + ginkgo.It("should return 200 when service name has max allowed number of characters 63", func() { + + annotations := make(map[string]string) + ing := framework.NewSingleIngress(host, "/", host, f.Namespace, name, 80, annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, func(server string) bool { + return strings.Contains(server, fmt.Sprintf("server_name %s", host)) + }) + + ginkgo.By("checking if the service is reached") + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + Expect(). + Status(http.StatusOK) + }) +}) diff --git a/test/e2e/framework/deployment.go b/test/e2e/framework/deployment.go index b37060af1db..cb6ef9accbc 100644 --- a/test/e2e/framework/deployment.go +++ b/test/e2e/framework/deployment.go @@ -67,6 +67,12 @@ func WithDeploymentReplicas(r int) func(*deploymentOptions) { } } +func WithName(n string) func(*deploymentOptions) { + return func(o *deploymentOptions) { + o.name = n + } +} + // NewEchoDeployment creates a new single replica deployment of the echo server image in a particular namespace func (f *Framework) NewEchoDeployment(opts ...func(*deploymentOptions)) { options := &deploymentOptions{