Remove cross-provider sanitization for Kubernetes service loading

Co-authored-by: Gina A. <70909035+gndz07@users.noreply.github.com>

pkg/provider/kubernetes/crd/fixtures/with_error_page_traefik_service.yml

@@ -0,0 +1,29 @@
+apiVersion: traefik.io/v1alpha1
+kind: TraefikService
+metadata:
+  name: errorpage-wrr
+  namespace: default
+
+spec:
+  weighted:
+    services:
+      - name: whoami
+        port: 80
+        weight: 1
+
+---
+apiVersion: traefik.io/v1alpha1
+kind: Middleware
+metadata:
+  name: errorpage
+  namespace: default
+
+spec:
+  errors:
+    status:
+      - "404"
+      - "500"
+    query: query
+    service:
+      name: errorpage-wrr
+      kind: TraefikService

pkg/provider/kubernetes/crd/kubernetes.go

@@ -243,16 +243,20 @@ func (p *Provider) loadConfigurationFromCRD(ctx context.Context, client Client)
 			continue
 		}
 
-		errorPage, errorPageService, err := p.createErrorPageMiddleware(client, middleware.Namespace, middleware.Spec.Errors)
+		errorPageName, errorPage, errorPageService, err := p.createErrorPageMiddleware(ctxMid, client, middleware.Namespace, middleware.Spec.Errors)
 		if err != nil {
 			log.FromContext(ctxMid).Errorf("Error while reading error page middleware: %v", err)
 			continue
 		}
 
-		if errorPage != nil && errorPageService != nil {
-			serviceName := id + "-errorpage-service"
-			errorPage.Service = serviceName
-			conf.HTTP.Services[serviceName] = errorPageService
+		if errorPage != nil {
+			if errorPageService != nil {
+				serviceName := id + "-errorpage-service"
+				errorPage.Service = serviceName
+				conf.HTTP.Services[serviceName] = errorPageService
+			} else {
+				errorPage.Service = errorPageName
+			}
 		}
 
 		plugin, err := createPluginMiddleware(client, middleware.Namespace, middleware.Spec.Plugin)
@@ -617,14 +621,9 @@ func createRetryMiddleware(retry *traefikv1alpha1.Retry) (*dynamic.Retry, error)
 	return r, nil
 }
 
-func (p *Provider) createErrorPageMiddleware(client Client, namespace string, errorPage *traefikv1alpha1.ErrorPage) (*dynamic.ErrorPage, *dynamic.Service, error) {
+func (p *Provider) createErrorPageMiddleware(ctx context.Context, client Client, namespace string, errorPage *traefikv1alpha1.ErrorPage) (string, *dynamic.ErrorPage, *dynamic.Service, error) {
 	if errorPage == nil {
-		return nil, nil, nil
-	}
-
-	errorPageMiddleware := &dynamic.ErrorPage{
-		Status: errorPage.Status,
-		Query:  errorPage.Query,
+		return "", nil, nil, nil
 	}
 
 	cb := configBuilder{
@@ -634,12 +633,15 @@ func (p *Provider) createErrorPageMiddleware(client Client, namespace string, er
 		allowEmptyServices:        p.AllowEmptyServices,
 	}
 
-	balancerServerHTTP, err := cb.buildServersLB(namespace, errorPage.Service.LoadBalancerSpec)
+	balancerName, balancerServerHTTP, err := cb.nameAndService(ctx, namespace, errorPage.Service.LoadBalancerSpec)
 	if err != nil {
-		return nil, nil, err
+		return "", nil, nil, err
 	}
 
-	return errorPageMiddleware, balancerServerHTTP, nil
+	return balancerName, &dynamic.ErrorPage{
+		Status: errorPage.Status,
+		Query:  errorPage.Query,
+	}, balancerServerHTTP, nil
 }
 
 func createForwardAuthMiddleware(k8sClient Client, namespace string, auth *traefikv1alpha1.ForwardAuth) (*dynamic.ForwardAuth, error) {

pkg/provider/kubernetes/crd/kubernetes_http.go

@@ -293,8 +293,8 @@ func (c configBuilder) buildMirroring(ctx context.Context, tService *traefikv1al
 }
 
 // buildServersLB creates the configuration for the load-balancer of servers defined by svc.
-func (c configBuilder) buildServersLB(namespace string, svc traefikv1alpha1.LoadBalancerSpec) (*dynamic.Service, error) {
-	servers, err := c.loadServers(namespace, svc)
+func (c configBuilder) buildServersLB(svc traefikv1alpha1.LoadBalancerSpec) (*dynamic.Service, error) {
+	servers, err := c.loadServers(svc)
 	if err != nil {
 		return nil, err
 	}
@@ -313,7 +313,7 @@ func (c configBuilder) buildServersLB(namespace string, svc traefikv1alpha1.Load
 
 	lb.Sticky = svc.Sticky
 
-	lb.ServersTransport, err = c.makeServersTransportKey(namespace, svc.ServersTransport)
+	lb.ServersTransport, err = c.makeServersTransportKey(svc.Namespace, svc.ServersTransport)
 	if err != nil {
 		return nil, err
 	}
@@ -340,7 +340,7 @@ func (c configBuilder) makeServersTransportKey(parentNamespace string, serversTr
 	return provider.Normalize(makeID(parentNamespace, serversTransportName)), nil
 }
 
-func (c configBuilder) loadServers(parentNamespace string, svc traefikv1alpha1.LoadBalancerSpec) ([]dynamic.Server, error) {
+func (c configBuilder) loadServers(svc traefikv1alpha1.LoadBalancerSpec) ([]dynamic.Server, error) {
 	strategy := svc.Strategy
 	if strategy == "" {
 		strategy = roundRobinStrategy
@@ -349,20 +349,12 @@ func (c configBuilder) loadServers(parentNamespace string, svc traefikv1alpha1.L
 		return nil, fmt.Errorf("load balancing strategy %s is not supported", strategy)
 	}
 
-	namespace := namespaceOrFallback(svc, parentNamespace)
-
-	if !isNamespaceAllowed(c.allowCrossNamespace, parentNamespace, namespace) {
-		return nil, fmt.Errorf("load balancer service %s/%s is not in the parent resource namespace %s", svc.Namespace, svc.Name, parentNamespace)
-	}
-
-	// If the service uses explicitly the provider suffix
-	sanitizedName := strings.TrimSuffix(svc.Name, providerNamespaceSeparator+providerName)
-	service, exists, err := c.client.GetService(namespace, sanitizedName)
+	service, exists, err := c.client.GetService(svc.Namespace, svc.Name)
 	if err != nil {
 		return nil, err
 	}
 	if !exists {
-		return nil, fmt.Errorf("kubernetes service not found: %s/%s", namespace, sanitizedName)
+		return nil, fmt.Errorf("kubernetes service not found: %s/%s", svc.Namespace, svc.Name)
 	}
 
 	svcPort, err := getServicePort(service, svc.Port)
@@ -387,7 +379,7 @@ func (c configBuilder) loadServers(parentNamespace string, svc traefikv1alpha1.L
 	var servers []dynamic.Server
 	if service.Spec.Type == corev1.ServiceTypeExternalName {
 		if !c.allowExternalNameServices {
-			return nil, fmt.Errorf("externalName services not allowed: %s/%s", namespace, sanitizedName)
+			return nil, fmt.Errorf("externalName services not allowed: %s/%s", svc.Namespace, svc.Name)
 		}
 
 		protocol, err := parseServiceProtocol(svc.Scheme, svcPort.Name, svcPort.Port)
@@ -402,16 +394,16 @@ func (c configBuilder) loadServers(parentNamespace string, svc traefikv1alpha1.L
 		}), nil
 	}
 
-	endpoints, endpointsExists, endpointsErr := c.client.GetEndpoints(namespace, sanitizedName)
+	endpoints, endpointsExists, endpointsErr := c.client.GetEndpoints(svc.Namespace, svc.Name)
 	if endpointsErr != nil {
 		return nil, endpointsErr
 	}
 	if !endpointsExists {
-		return nil, fmt.Errorf("endpoints not found for %s/%s", namespace, sanitizedName)
+		return nil, fmt.Errorf("endpoints not found for %s/%s", svc.Namespace, svc.Name)
 	}
 
 	if len(endpoints.Subsets) == 0 && !c.allowEmptyServices {
-		return nil, fmt.Errorf("subset not found for %s/%s", namespace, sanitizedName)
+		return nil, fmt.Errorf("subset not found for %s/%s", svc.Namespace, svc.Name)
 	}
 
 	for _, subset := range endpoints.Subsets {
@@ -451,25 +443,26 @@ func (c configBuilder) loadServers(parentNamespace string, svc traefikv1alpha1.L
 func (c configBuilder) nameAndService(ctx context.Context, parentNamespace string, service traefikv1alpha1.LoadBalancerSpec) (string, *dynamic.Service, error) {
 	svcCtx := log.With(ctx, log.Str(log.ServiceName, service.Name))
 
-	namespace := namespaceOrFallback(service, parentNamespace)
+	service = *service.DeepCopy()
+	service.Namespace = namespaceOrFallback(service, parentNamespace)
 
-	if !isNamespaceAllowed(c.allowCrossNamespace, parentNamespace, namespace) {
+	if !isNamespaceAllowed(c.allowCrossNamespace, parentNamespace, service.Namespace) {
 		return "", nil, fmt.Errorf("service %s/%s not in the parent resource namespace %s", service.Namespace, service.Name, parentNamespace)
 	}
 
 	switch service.Kind {
 	case "", "Service":
-		serversLB, err := c.buildServersLB(namespace, service)
+		serversLB, err := c.buildServersLB(service)
 		if err != nil {
 			return "", nil, err
 		}
 
-		fullName := fullServiceName(svcCtx, namespace, service, service.Port)
+		fullName := fullServiceName(svcCtx, service, service.Port)
 
 		return fullName, serversLB, nil
 
 	case "TraefikService":
-		return fullServiceName(svcCtx, namespace, service, intstr.FromInt(0)), nil, nil
+		return fullServiceName(svcCtx, service, intstr.FromInt(0)), nil, nil
 
 	default:
 		return "", nil, fmt.Errorf("unsupported service kind %s", service.Kind)
@@ -485,18 +478,18 @@ func splitSvcNameProvider(name string) (string, string) {
 	return svc, pvd
 }
 
-func fullServiceName(ctx context.Context, namespace string, service traefikv1alpha1.LoadBalancerSpec, port intstr.IntOrString) string {
+func fullServiceName(ctx context.Context, service traefikv1alpha1.LoadBalancerSpec, port intstr.IntOrString) string {
 	if (port.Type == intstr.Int && port.IntVal != 0) || (port.Type == intstr.String && port.StrVal != "") {
-		return provider.Normalize(fmt.Sprintf("%s-%s-%s", namespace, service.Name, &port))
+		return provider.Normalize(fmt.Sprintf("%s-%s-%s", service.Namespace, service.Name, &port))
 	}
 
 	if !strings.Contains(service.Name, providerNamespaceSeparator) {
-		return provider.Normalize(fmt.Sprintf("%s-%s", namespace, service.Name))
+		return provider.Normalize(fmt.Sprintf("%s-%s", service.Namespace, service.Name))
 	}
 
 	name, pName := splitSvcNameProvider(service.Name)
 	if pName == providerName {
-		return provider.Normalize(fmt.Sprintf("%s-%s", namespace, name))
+		return provider.Normalize(fmt.Sprintf("%s-%s", service.Namespace, name))
 	}
 
 	if service.Namespace != "" {

pkg/provider/kubernetes/crd/kubernetes_test.go

@@ -3596,6 +3596,60 @@ func TestLoadIngressRoutes(t *testing.T) {
 				},
 			},
 		},
+		{
+			desc:  "Error page middleware referencing a TraefikService",
+			paths: []string{"services.yml", "with_error_page_traefik_service.yml"},
+			expected: &dynamic.Configuration{
+				UDP: &dynamic.UDPConfiguration{
+					Routers:  map[string]*dynamic.UDPRouter{},
+					Services: map[string]*dynamic.UDPService{},
+				},
+				TLS: &dynamic.TLSConfiguration{},
+				TCP: &dynamic.TCPConfiguration{
+					Routers:     map[string]*dynamic.TCPRouter{},
+					Middlewares: map[string]*dynamic.TCPMiddleware{},
+					Services:    map[string]*dynamic.TCPService{},
+				},
+				HTTP: &dynamic.HTTPConfiguration{
+					Routers: map[string]*dynamic.Router{},
+					Middlewares: map[string]*dynamic.Middleware{
+						"default-errorpage": {
+							Errors: &dynamic.ErrorPage{
+								Status:  []string{"404", "500"},
+								Service: "default-errorpage-wrr",
+								Query:   "query",
+							},
+						},
+					},
+					Services: map[string]*dynamic.Service{
+						"default-errorpage-wrr": {
+							Weighted: &dynamic.WeightedRoundRobin{
+								Services: []dynamic.WRRService{
+									{
+										Name:   "default-whoami-80",
+										Weight: func(i int) *int { return &i }(1),
+									},
+								},
+							},
+						},
+						"default-whoami-80": {
+							LoadBalancer: &dynamic.ServersLoadBalancer{
+								Servers: []dynamic.Server{
+									{
+										URL: "http://10.10.0.1:80",
+									},
+									{
+										URL: "http://10.10.0.2:80",
+									},
+								},
+								PassHostHeader: pointer(true),
+							},
+						},
+					},
+					ServersTransports: map[string]*dynamic.ServersTransport{},
+				},
+			},
+		},
 		{
 			desc:  "Simple Ingress Route, with options",
 			paths: []string{"services.yml", "with_options.yml"},