Disable ExternalName Services by default on Kubernetes providers

2021-07-13 04:54:09 -06:00 · 2021-07-13 04:54:09 -06:00 · 3c1ed0d9b2
commit 3c1ed0d9b2
parent 10ab39c33b
19 changed files with 637 additions and 102 deletions
--- a/pkg/provider/kubernetes/crd/fixtures/udp/services.yml
+++ b/pkg/provider/kubernetes/crd/fixtures/udp/services.yml
@ -160,3 +160,16 @@ subsets:
    ports:
      - name: myapp
        port: 8000
+---
+kind: Service
+apiVersion: v1
+metadata:
+  name: external.service.with.port
+  namespace: default
+spec:
+  externalName: external.domain
+  type: ExternalName
+  ports:
+    - name: http
+      port: 80
+
--- a/pkg/provider/kubernetes/crd/fixtures/udp/with_externalname_service.yml
+++ b/pkg/provider/kubernetes/crd/fixtures/udp/with_externalname_service.yml
@ -0,0 +1,14 @@
+apiVersion: traefik.containo.us/v1alpha1
+kind: IngressRouteUDP
+metadata:
+  name: test.route
+  namespace: default
+
+spec:
+  entryPoints:
+    - foo
+
+  routes:
+  - services:
+    - name: external.service.with.port
+      port: 80
--- a/pkg/provider/kubernetes/crd/kubernetes.go
+++ b/pkg/provider/kubernetes/crd/kubernetes.go
@ -38,15 +38,16 @@ const (

 // Provider holds configurations of the provider.
 type Provider struct {
-	Endpoint            string          `description:"Kubernetes server endpoint (required for external cluster client)." json:"endpoint,omitempty" toml:"endpoint,omitempty" yaml:"endpoint,omitempty"`
-	Token               string          `description:"Kubernetes bearer token (not needed for in-cluster client)." json:"token,omitempty" toml:"token,omitempty" yaml:"token,omitempty"`
-	CertAuthFilePath    string          `description:"Kubernetes certificate authority file path (not needed for in-cluster client)." json:"certAuthFilePath,omitempty" toml:"certAuthFilePath,omitempty" yaml:"certAuthFilePath,omitempty"`
-	Namespaces          []string        `description:"Kubernetes namespaces." json:"namespaces,omitempty" toml:"namespaces,omitempty" yaml:"namespaces,omitempty" export:"true"`
-	AllowCrossNamespace bool            `description:"Allow cross namespace resource reference." json:"allowCrossNamespace,omitempty" toml:"allowCrossNamespace,omitempty" yaml:"allowCrossNamespace,omitempty" export:"true"`
-	LabelSelector       string          `description:"Kubernetes label selector to use." json:"labelSelector,omitempty" toml:"labelSelector,omitempty" yaml:"labelSelector,omitempty" export:"true"`
-	IngressClass        string          `description:"Value of kubernetes.io/ingress.class annotation to watch for." json:"ingressClass,omitempty" toml:"ingressClass,omitempty" yaml:"ingressClass,omitempty" export:"true"`
-	ThrottleDuration    ptypes.Duration `description:"Ingress refresh throttle duration" json:"throttleDuration,omitempty" toml:"throttleDuration,omitempty" yaml:"throttleDuration,omitempty" export:"true"`
-	lastConfiguration   safe.Safe
+	Endpoint                  string          `description:"Kubernetes server endpoint (required for external cluster client)." json:"endpoint,omitempty" toml:"endpoint,omitempty" yaml:"endpoint,omitempty"`
+	Token                     string          `description:"Kubernetes bearer token (not needed for in-cluster client)." json:"token,omitempty" toml:"token,omitempty" yaml:"token,omitempty"`
+	CertAuthFilePath          string          `description:"Kubernetes certificate authority file path (not needed for in-cluster client)." json:"certAuthFilePath,omitempty" toml:"certAuthFilePath,omitempty" yaml:"certAuthFilePath,omitempty"`
+	Namespaces                []string        `description:"Kubernetes namespaces." json:"namespaces,omitempty" toml:"namespaces,omitempty" yaml:"namespaces,omitempty" export:"true"`
+	AllowCrossNamespace       bool            `description:"Allow cross namespace resource reference." json:"allowCrossNamespace,omitempty" toml:"allowCrossNamespace,omitempty" yaml:"allowCrossNamespace,omitempty" export:"true"`
+	AllowExternalNameServices bool            `description:"Allow ExternalName services." json:"allowExternalNameServices,omitempty" toml:"allowExternalNameServices,omitempty" yaml:"allowExternalNameServices,omitempty" export:"true"`
+	LabelSelector             string          `description:"Kubernetes label selector to use." json:"labelSelector,omitempty" toml:"labelSelector,omitempty" yaml:"labelSelector,omitempty" export:"true"`
+	IngressClass              string          `description:"Value of kubernetes.io/ingress.class annotation to watch for." json:"ingressClass,omitempty" toml:"ingressClass,omitempty" yaml:"ingressClass,omitempty" export:"true"`
+	ThrottleDuration          ptypes.Duration `description:"Ingress refresh throttle duration" json:"throttleDuration,omitempty" toml:"throttleDuration,omitempty" yaml:"throttleDuration,omitempty" export:"true"`
+	lastConfiguration         safe.Safe
 }

 func (p *Provider) newK8sClient(ctx context.Context) (*clientWrapper, error) {
@ -102,6 +103,10 @@ func (p *Provider) Provide(configurationChan chan<- dynamic.Message, pool *safe.
 		logger.Warn("Cross-namespace reference between IngressRoutes and resources is enabled, please ensure that this is expected (see AllowCrossNamespace option)")
 	}

+	if p.AllowExternalNameServices {
+		logger.Warn("ExternalName service loading is enabled, please ensure that this is expected (see AllowExternalNameServices option)")
+	}
+
 	pool.GoCtx(func(ctxPool context.Context) {
 		operation := func() error {
 			eventsChan, err := k8sClient.WatchAll(p.Namespaces, ctxPool.Done())
@ -240,7 +245,7 @@ func (p *Provider) loadConfigurationFromCRD(ctx context.Context, client Client)
 		}
 	}

-	cb := configBuilder{client, p.AllowCrossNamespace}
+	cb := configBuilder{client: client, allowCrossNamespace: p.AllowCrossNamespace, allowExternalNameServices: p.AllowExternalNameServices}

 	for _, service := range client.GetTraefikServices() {
 		err := cb.buildTraefikService(ctx, service, conf.HTTP.Services)
@ -360,7 +365,7 @@ func (p *Provider) createErrorPageMiddleware(client Client, namespace string, er
 		Query:  errorPage.Query,
 	}

-	balancerServerHTTP, err := configBuilder{client, p.AllowCrossNamespace}.buildServersLB(namespace, errorPage.Service.LoadBalancerSpec)
+	balancerServerHTTP, err := configBuilder{client: client, allowCrossNamespace: p.AllowCrossNamespace, allowExternalNameServices: p.AllowExternalNameServices}.buildServersLB(namespace, errorPage.Service.LoadBalancerSpec)
 	if err != nil {
 		return nil, nil, err
 	}
--- a/pkg/provider/kubernetes/crd/kubernetes_http.go
+++ b/pkg/provider/kubernetes/crd/kubernetes_http.go
@ -49,7 +49,7 @@ func (p *Provider) loadIngressRouteConfiguration(ctx context.Context, client Cli
 			ingressName = ingressRoute.GenerateName
 		}

-		cb := configBuilder{client, p.AllowCrossNamespace}
+		cb := configBuilder{client: client, allowCrossNamespace: p.AllowCrossNamespace, allowExternalNameServices: p.AllowExternalNameServices}

 		for _, route := range ingressRoute.Spec.Routes {
 			if route.Kind != "Rule" {
@ -172,8 +172,9 @@ func (p *Provider) makeMiddlewareKeys(ctx context.Context, ingRouteNamespace str
 }

 type configBuilder struct {
-	client              Client
-	allowCrossNamespace bool
+	client                    Client
+	allowCrossNamespace       bool
+	allowExternalNameServices bool
 }

 // buildTraefikService creates the configuration for the traefik service defined in tService,
@ -322,6 +323,10 @@ func (c configBuilder) loadServers(parentNamespace string, svc v1alpha1.LoadBala

 	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)
+		}
+
 		protocol, err := parseServiceProtocol(svc.Scheme, svcPort.Name, svcPort.Port)
 		if err != nil {
 			return nil, err
--- a/pkg/provider/kubernetes/crd/kubernetes_tcp.go
+++ b/pkg/provider/kubernetes/crd/kubernetes_tcp.go
@ -135,7 +135,7 @@ func (p *Provider) createLoadBalancerServerTCP(client Client, parentNamespace st
 		ns = service.Namespace
 	}

-	servers, err := loadTCPServers(client, ns, service)
+	servers, err := p.loadTCPServers(client, ns, service)
 	if err != nil {
 		return nil, err
 	}
@ -162,7 +162,7 @@ func (p *Provider) createLoadBalancerServerTCP(client Client, parentNamespace st
 	return tcpService, nil
 }

-func loadTCPServers(client Client, namespace string, svc v1alpha1.ServiceTCP) ([]dynamic.TCPServer, error) {
+func (p *Provider) loadTCPServers(client Client, namespace string, svc v1alpha1.ServiceTCP) ([]dynamic.TCPServer, error) {
 	service, exists, err := client.GetService(namespace, svc.Name)
 	if err != nil {
 		return nil, err
@ -172,6 +172,10 @@ func loadTCPServers(client Client, namespace string, svc v1alpha1.ServiceTCP) ([
 		return nil, errors.New("service not found")
 	}

+	if service.Spec.Type == corev1.ServiceTypeExternalName && !p.AllowExternalNameServices {
+		return nil, fmt.Errorf("externalName services not allowed: %s/%s", namespace, svc.Name)
+	}
+
 	svcPort, err := getServicePort(service, svc.Port)
 	if err != nil {
 		return nil, err
--- a/pkg/provider/kubernetes/crd/kubernetes_test.go
+++ b/pkg/provider/kubernetes/crd/kubernetes_test.go
@ -1153,7 +1153,7 @@ func TestLoadIngressRouteTCPs(t *testing.T) {
 				return
 			}

-			p := Provider{IngressClass: test.ingressClass, AllowCrossNamespace: true}
+			p := Provider{IngressClass: test.ingressClass, AllowCrossNamespace: true, AllowExternalNameServices: true}

 			clientMock := newClientMock(test.paths...)
 			conf := p.loadConfigurationFromCRD(context.Background(), clientMock)
@ -3337,7 +3337,7 @@ func TestLoadIngressRoutes(t *testing.T) {
 				return
 			}

-			p := Provider{IngressClass: test.ingressClass, AllowCrossNamespace: true}
+			p := Provider{IngressClass: test.ingressClass, AllowCrossNamespace: true, AllowExternalNameServices: true}

 			clientMock := newClientMock(test.paths...)
 			conf := p.loadConfigurationFromCRD(context.Background(), clientMock)
@ -4435,9 +4435,292 @@ func TestCrossNamespace(t *testing.T) {
 				<-eventCh
 			}

-			p := Provider{}
+			p := Provider{AllowCrossNamespace: test.allowCrossNamespace}
+
+			conf := p.loadConfigurationFromCRD(context.Background(), client)
+			assert.Equal(t, test.expected, conf)
+		})
+	}
+}
+
+func TestExternalNameService(t *testing.T) {
+	testCases := []struct {
+		desc                     string
+		allowExternalNameService bool
+		ingressClass             string
+		paths                    []string
+		expected                 *dynamic.Configuration
+	}{
+		{
+			desc: "Empty",
+			expected: &dynamic.Configuration{
+				UDP: &dynamic.UDPConfiguration{
+					Routers:  map[string]*dynamic.UDPRouter{},
+					Services: map[string]*dynamic.UDPService{},
+				},
+				TCP: &dynamic.TCPConfiguration{
+					Routers:  map[string]*dynamic.TCPRouter{},
+					Services: map[string]*dynamic.TCPService{},
+				},
+				HTTP: &dynamic.HTTPConfiguration{
+					ServersTransports: map[string]*dynamic.ServersTransport{},
+					Routers:           map[string]*dynamic.Router{},
+					Middlewares:       map[string]*dynamic.Middleware{},
+					Services:          map[string]*dynamic.Service{},
+				},
+				TLS: &dynamic.TLSConfiguration{},
+			},
+		},
+		{
+			desc:                     "HTTP ExternalName services allowed",
+			paths:                    []string{"services.yml", "with_externalname_with_http.yml"},
+			allowExternalNameService: true,
+			expected: &dynamic.Configuration{
+				UDP: &dynamic.UDPConfiguration{
+					Routers:  map[string]*dynamic.UDPRouter{},
+					Services: map[string]*dynamic.UDPService{},
+				},
+				TCP: &dynamic.TCPConfiguration{
+					Routers:  map[string]*dynamic.TCPRouter{},
+					Services: map[string]*dynamic.TCPService{},
+				},
+				HTTP: &dynamic.HTTPConfiguration{
+					ServersTransports: map[string]*dynamic.ServersTransport{},
+					Routers: map[string]*dynamic.Router{
+						"default-test-route-6f97418635c7e18853da": {
+							EntryPoints: []string{"foo"},
+							Service:     "default-test-route-6f97418635c7e18853da",
+							Rule:        "Host(`foo.com`)",
+							Priority:    0,
+						},
+					},
+					Middlewares: map[string]*dynamic.Middleware{},
+					Services: map[string]*dynamic.Service{
+						"default-test-route-6f97418635c7e18853da": {
+							LoadBalancer: &dynamic.ServersLoadBalancer{
+								Servers: []dynamic.Server{
+									{
+										URL: "http://external.domain:80",
+									},
+								},
+								PassHostHeader: Bool(true),
+							},
+						},
+					},
+				},
+				TLS: &dynamic.TLSConfiguration{},
+			},
+		},
+		{
+			desc:  "HTTP Externalname services disallowed",
+			paths: []string{"services.yml", "with_externalname_with_http.yml"},
+			expected: &dynamic.Configuration{
+				UDP: &dynamic.UDPConfiguration{
+					Routers:  map[string]*dynamic.UDPRouter{},
+					Services: map[string]*dynamic.UDPService{},
+				},
+				TCP: &dynamic.TCPConfiguration{
+					Routers:  map[string]*dynamic.TCPRouter{},
+					Services: map[string]*dynamic.TCPService{},
+				},
+				HTTP: &dynamic.HTTPConfiguration{
+					ServersTransports: map[string]*dynamic.ServersTransport{},
+					Routers:           map[string]*dynamic.Router{},
+					Middlewares:       map[string]*dynamic.Middleware{},
+					Services:          map[string]*dynamic.Service{},
+				},
+				TLS: &dynamic.TLSConfiguration{},
+			},
+		},
+		{
+			desc:                     "TCP ExternalName services allowed",
+			paths:                    []string{"tcp/services.yml", "tcp/with_externalname_with_port.yml"},
+			allowExternalNameService: true,
+			expected: &dynamic.Configuration{
+				UDP: &dynamic.UDPConfiguration{
+					Routers:  map[string]*dynamic.UDPRouter{},
+					Services: map[string]*dynamic.UDPService{},
+				},
+				HTTP: &dynamic.HTTPConfiguration{
+					ServersTransports: map[string]*dynamic.ServersTransport{},
+					Routers:           map[string]*dynamic.Router{},
+					Middlewares:       map[string]*dynamic.Middleware{},
+					Services:          map[string]*dynamic.Service{},
+				},
+				TCP: &dynamic.TCPConfiguration{
+					Routers: map[string]*dynamic.TCPRouter{
+						"default-test.route-fdd3e9338e47a45efefc": {
+							EntryPoints: []string{"foo"},
+							Service:     "default-test.route-fdd3e9338e47a45efefc",
+							Rule:        "HostSNI(`foo.com`)",
+						},
+					},
+					Services: map[string]*dynamic.TCPService{
+						"default-test.route-fdd3e9338e47a45efefc": {
+							LoadBalancer: &dynamic.TCPServersLoadBalancer{
+								Servers: []dynamic.TCPServer{
+									{
+										Address: "external.domain:80",
+										Port:    "",
+									},
+								},
+							},
+						},
+					},
+				},
+				TLS: &dynamic.TLSConfiguration{},
+			},
+		},
+		{
+			desc:  "TCP ExternalName services disallowed",
+			paths: []string{"tcp/services.yml", "tcp/with_externalname_with_port.yml"},
+			expected: &dynamic.Configuration{
+				UDP: &dynamic.UDPConfiguration{
+					Routers:  map[string]*dynamic.UDPRouter{},
+					Services: map[string]*dynamic.UDPService{},
+				},
+				TCP: &dynamic.TCPConfiguration{
+					// The router that references the invalid service will be discarded.
+					Routers: map[string]*dynamic.TCPRouter{
+						"default-test.route-fdd3e9338e47a45efefc": {
+							EntryPoints: []string{"foo"},
+							Service:     "default-test.route-fdd3e9338e47a45efefc",
+							Rule:        "HostSNI(`foo.com`)",
+						},
+					},
+					Services: map[string]*dynamic.TCPService{},
+				},
+				HTTP: &dynamic.HTTPConfiguration{
+					ServersTransports: map[string]*dynamic.ServersTransport{},
+					Routers:           map[string]*dynamic.Router{},
+					Middlewares:       map[string]*dynamic.Middleware{},
+					Services:          map[string]*dynamic.Service{},
+				},
+				TLS: &dynamic.TLSConfiguration{},
+			},
+		},
+		{
+			desc:                     "UDP ExternalName services allowed",
+			paths:                    []string{"udp/services.yml", "udp/with_externalname_service.yml"},
+			allowExternalNameService: true,
+			expected: &dynamic.Configuration{
+				UDP: &dynamic.UDPConfiguration{
+					Routers: map[string]*dynamic.UDPRouter{
+						"default-test.route-0": {
+							EntryPoints: []string{"foo"},
+							Service:     "default-test.route-0",
+						},
+					},
+					Services: map[string]*dynamic.UDPService{
+						"default-test.route-0": {
+							LoadBalancer: &dynamic.UDPServersLoadBalancer{
+								Servers: []dynamic.UDPServer{
+									{
+										Address: "external.domain:80",
+										Port:    "",
+									},
+								},
+							},
+						},
+					},
+				},
+				HTTP: &dynamic.HTTPConfiguration{
+					ServersTransports: map[string]*dynamic.ServersTransport{},
+					Routers:           map[string]*dynamic.Router{},
+					Middlewares:       map[string]*dynamic.Middleware{},
+					Services:          map[string]*dynamic.Service{},
+				},
+				TCP: &dynamic.TCPConfiguration{
+					Routers:  map[string]*dynamic.TCPRouter{},
+					Services: map[string]*dynamic.TCPService{},
+				},
+				TLS: &dynamic.TLSConfiguration{},
+			},
+		},
+		{
+			desc:  "UDP ExternalName service disallowed",
+			paths: []string{"udp/services.yml", "udp/with_externalname_service.yml"},
+			expected: &dynamic.Configuration{
+				// The router that references the invalid service will be discarded.
+				UDP: &dynamic.UDPConfiguration{
+					Routers: map[string]*dynamic.UDPRouter{
+						"default-test.route-0": {
+							EntryPoints: []string{"foo"},
+							Service:     "default-test.route-0",
+						},
+					},
+					Services: map[string]*dynamic.UDPService{},
+				},
+				TCP: &dynamic.TCPConfiguration{
+					Routers:  map[string]*dynamic.TCPRouter{},
+					Services: map[string]*dynamic.TCPService{},
+				},
+				HTTP: &dynamic.HTTPConfiguration{
+					ServersTransports: map[string]*dynamic.ServersTransport{},
+					Routers:           map[string]*dynamic.Router{},
+					Middlewares:       map[string]*dynamic.Middleware{},
+					Services:          map[string]*dynamic.Service{},
+				},
+				TLS: &dynamic.TLSConfiguration{},
+			},
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
+
+			var k8sObjects []runtime.Object
+			var crdObjects []runtime.Object
+			for _, path := range test.paths {
+				yamlContent, err := os.ReadFile(filepath.FromSlash("./fixtures/" + path))
+				if err != nil {
+					panic(err)
+				}
+
+				objects := k8s.MustParseYaml(yamlContent)
+				for _, obj := range objects {
+					switch o := obj.(type) {
+					case *corev1.Service, *corev1.Endpoints, *corev1.Secret:
+						k8sObjects = append(k8sObjects, o)
+					case *v1alpha1.IngressRoute:
+						crdObjects = append(crdObjects, o)
+					case *v1alpha1.IngressRouteTCP:
+						crdObjects = append(crdObjects, o)
+					case *v1alpha1.IngressRouteUDP:
+						crdObjects = append(crdObjects, o)
+					case *v1alpha1.Middleware:
+						crdObjects = append(crdObjects, o)
+					case *v1alpha1.TraefikService:
+						crdObjects = append(crdObjects, o)
+					case *v1alpha1.TLSOption:
+						crdObjects = append(crdObjects, o)
+					case *v1alpha1.TLSStore:
+						crdObjects = append(crdObjects, o)
+					default:
+					}
+				}
+			}
+
+			kubeClient := kubefake.NewSimpleClientset(k8sObjects...)
+			crdClient := crdfake.NewSimpleClientset(crdObjects...)
+
+			client := newClientImpl(kubeClient, crdClient)
+
+			stopCh := make(chan struct{})
+
+			eventCh, err := client.WatchAll([]string{"default", "cross-ns"}, stopCh)
+			require.NoError(t, err)
+
+			if k8sObjects != nil || crdObjects != nil {
+				// just wait for the first event
+				<-eventCh
+			}
+
+			p := Provider{AllowExternalNameServices: test.allowExternalNameService}

-			p.AllowCrossNamespace = test.allowCrossNamespace
 			conf := p.loadConfigurationFromCRD(context.Background(), client)
 			assert.Equal(t, test.expected, conf)
 		})
--- a/pkg/provider/kubernetes/crd/kubernetes_udp.go
+++ b/pkg/provider/kubernetes/crd/kubernetes_udp.go
@ -87,7 +87,7 @@ func (p *Provider) createLoadBalancerServerUDP(client Client, parentNamespace st
 		ns = service.Namespace
 	}

-	servers, err := loadUDPServers(client, ns, service)
+	servers, err := p.loadUDPServers(client, ns, service)
 	if err != nil {
 		return nil, err
 	}
@ -101,7 +101,7 @@ func (p *Provider) createLoadBalancerServerUDP(client Client, parentNamespace st
 	return udpService, nil
 }

-func loadUDPServers(client Client, namespace string, svc v1alpha1.ServiceUDP) ([]dynamic.UDPServer, error) {
+func (p *Provider) loadUDPServers(client Client, namespace string, svc v1alpha1.ServiceUDP) ([]dynamic.UDPServer, error) {
 	service, exists, err := client.GetService(namespace, svc.Name)
 	if err != nil {
 		return nil, err
@ -111,6 +111,10 @@ func loadUDPServers(client Client, namespace string, svc v1alpha1.ServiceUDP) ([
 		return nil, errors.New("service not found")
 	}

+	if service.Spec.Type == corev1.ServiceTypeExternalName && !p.AllowExternalNameServices {
+		return nil, fmt.Errorf("externalName services not allowed: %s/%s", namespace, svc.Name)
+	}
+
 	var portSpec *corev1.ServicePort
 	for _, p := range service.Spec.Ports {
 		p := p