wip: add --gateway-discovery-dns-strategy flag

Kong · May 25, 2023 · 13a4376 · 13a4376
1 parent e237dc0
commit 13a4376
Show file tree

Hide file tree

Showing 19 changed files with 385 additions and 279 deletions.
diff --git a/config/variants/multi-gw/base/manager_multi_gateway_patch.yaml b/config/variants/multi-gw/base/manager_multi_gateway_patch.yaml
@@ -13,5 +13,7 @@ spec:
         env:
         - name: CONTROLLER_KONG_ADMIN_SVC
           value: kong/kong-admin
+        - name: CONTROLLER_GATEWAY_DISCOVERY_DNS_STRATEGY
+          value: ip
         - name: CONTROLLER_KONG_ADMIN_URL
           $patch: delete
diff --git a/deploy/single/all-in-one-dbless-enterprise.yaml b/deploy/single/all-in-one-dbless-enterprise.yaml
@@ -1654,6 +1654,8 @@ spec:
       - env:
         - name: CONTROLLER_KONG_ADMIN_SVC
           value: kong/kong-admin
+        - name: CONTROLLER_GATEWAY_DISCOVERY_DNS_STRATEGY
+          value: ip
         - name: CONTROLLER_KONG_ADMIN_TLS_SKIP_VERIFY
           value: "true"
         - name: CONTROLLER_PUBLISH_SERVICE

diff --git a/deploy/single/all-in-one-dbless-k4k8s-enterprise.yaml b/deploy/single/all-in-one-dbless-k4k8s-enterprise.yaml
@@ -1654,6 +1654,8 @@ spec:
       - env:
         - name: CONTROLLER_KONG_ADMIN_SVC
           value: kong/kong-admin
+        - name: CONTROLLER_GATEWAY_DISCOVERY_DNS_STRATEGY
+          value: ip
         - name: CONTROLLER_KONG_ADMIN_TLS_SKIP_VERIFY
           value: "true"
         - name: CONTROLLER_PUBLISH_SERVICE

diff --git a/deploy/single/all-in-one-dbless-konnect-enterprise.yaml b/deploy/single/all-in-one-dbless-konnect-enterprise.yaml
@@ -1666,6 +1666,8 @@ spec:
               name: konnect-client-tls
         - name: CONTROLLER_KONG_ADMIN_SVC
           value: kong/kong-admin
+        - name: CONTROLLER_GATEWAY_DISCOVERY_DNS_STRATEGY
+          value: ip
         - name: CONTROLLER_KONG_ADMIN_TLS_SKIP_VERIFY
           value: "true"
         - name: CONTROLLER_PUBLISH_SERVICE

diff --git a/deploy/single/all-in-one-dbless-konnect.yaml b/deploy/single/all-in-one-dbless-konnect.yaml
@@ -1666,6 +1666,8 @@ spec:
               name: konnect-client-tls
         - name: CONTROLLER_KONG_ADMIN_SVC
           value: kong/kong-admin
+        - name: CONTROLLER_GATEWAY_DISCOVERY_DNS_STRATEGY
+          value: ip
         - name: CONTROLLER_KONG_ADMIN_TLS_SKIP_VERIFY
           value: "true"
         - name: CONTROLLER_PUBLISH_SERVICE

diff --git a/deploy/single/all-in-one-dbless.yaml b/deploy/single/all-in-one-dbless.yaml
@@ -1654,6 +1654,8 @@ spec:
       - env:
         - name: CONTROLLER_KONG_ADMIN_SVC
           value: kong/kong-admin
+        - name: CONTROLLER_GATEWAY_DISCOVERY_DNS_STRATEGY
+          value: ip
         - name: CONTROLLER_KONG_ADMIN_TLS_SKIP_VERIFY
           value: "true"
         - name: CONTROLLER_PUBLISH_SERVICE

diff --git a/internal/adminapi/endpoints.go b/internal/adminapi/endpoints.go
@@ -11,6 +11,8 @@ import (
 	k8stypes "k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	cfgtypes "github.com/kong/kubernetes-ingress-controller/v2/internal/manager/config/types"
 )
 
 // DiscoveredAdminAPI represents an Admin API discovered from a Kubernetes Service.
@@ -27,6 +29,7 @@ func GetAdminAPIsForService(
 	kubeClient client.Client,
 	service k8stypes.NamespacedName,
 	portNames sets.Set[string],
+	dnsStrategy cfgtypes.DNSStrategy,
 ) (sets.Set[DiscoveredAdminAPI], error) {
 	const (
 		defaultEndpointSliceListPagingLimit = 100
@@ -55,7 +58,11 @@ func GetAdminAPIsForService(
 		}
 
 		for _, es := range endpointsList.Items {
-			addresses = addresses.Union(AdminAPIsFromEndpointSlice(es, portNames))
+			adminAPI, err := AdminAPIsFromEndpointSlice(es, portNames, dnsStrategy)
+			if err != nil {
+				return nil, err
+			}
+			addresses = addresses.Union(adminAPI)
 		}
 
 		if endpointsList.Continue == "" {
@@ -68,7 +75,11 @@ func GetAdminAPIsForService(
 
 // AdminAPIsFromEndpointSlice returns a list of Admin APIs when given
 // an EndpointSlice.
-func AdminAPIsFromEndpointSlice(endpoints discoveryv1.EndpointSlice, portNames sets.Set[string]) sets.Set[DiscoveredAdminAPI] {
+func AdminAPIsFromEndpointSlice(
+	endpoints discoveryv1.EndpointSlice,
+	portNames sets.Set[string],
+	dnsStrategy cfgtypes.DNSStrategy,
+) (sets.Set[DiscoveredAdminAPI], error) {
 	discoveredAdminAPIs := sets.New[DiscoveredAdminAPI]()
 	for _, p := range endpoints.Ports {
 		if p.Name == nil {
@@ -96,42 +107,79 @@ func AdminAPIsFromEndpointSlice(endpoints discoveryv1.EndpointSlice, portNames s
 			if e.TargetRef == nil || e.TargetRef.Kind != "Pod" {
 				continue
 			}
-			podNN := k8stypes.NamespacedName{
-				Name:      e.TargetRef.Name,
-				Namespace: e.TargetRef.Namespace,
-			}
 
 			if len(e.Addresses) < 1 {
 				continue
 			}
 
-			// Endpoint's addresses are assumed to be fungible, therefore we pick only the first one.
-			// For the context please see the `Endpoint.Addresses` godoc.
-			addr := strings.ReplaceAll(e.Addresses[0], ".", "-")
-
-			var adminAPI DiscoveredAdminAPI
-			// NOTE: We assume https here because the referenced Admin API
-			// server will live in another Pod/elsewhere so allowing http would
-			// not be considered best practice.
-			if serviceName == "" {
-				// If we couldn't find a service that's the owner of provided EndpointSlice
-				// then fallback to providing a DNS name for the Pod only.
-				adminAPI = DiscoveredAdminAPI{
-					Address: fmt.Sprintf("https://%s.%s.pod:%d",
-						addr, endpoints.Namespace, *p.Port,
-					),
-					PodRef: podNN,
-				}
-			} else {
-				adminAPI = DiscoveredAdminAPI{
-					Address: fmt.Sprintf("https://%s.%s.%s.svc:%d",
-						addr, serviceName, endpoints.Namespace, *p.Port,
-					),
-					PodRef: podNN,
-				}
+			svc := k8stypes.NamespacedName{
+				Name:      serviceName,
+				Namespace: endpoints.Namespace,
+			}
+
+			adminAPI, err := adminAPIFromEndpoint(e, p, svc, dnsStrategy)
+			if err != nil {
+				return nil, err
 			}
 			discoveredAdminAPIs = discoveredAdminAPIs.Insert(adminAPI)
 		}
 	}
-	return discoveredAdminAPIs
+	return discoveredAdminAPIs, nil
+}
+
+func adminAPIFromEndpoint(
+	endpoint discoveryv1.Endpoint,
+	port discoveryv1.EndpointPort,
+	service k8stypes.NamespacedName,
+	dnsStrategy cfgtypes.DNSStrategy,
+) (DiscoveredAdminAPI, error) {
+	podNN := k8stypes.NamespacedName{
+		Name:      endpoint.TargetRef.Name,
+		Namespace: endpoint.TargetRef.Namespace,
+	}
+
+	// NOTE: Endpoint's addresses are assumed to be fungible, therefore we pick
+	// only the first one.
+	// For the context please see the `Endpoint.Addresses` godoc.
+	eAddress := endpoint.Addresses[0]
+
+	// NOTE: We assume https below because the referenced Admin API
+	// server will live in another Pod/elsewhere so allowing http would
+	// not be considered best practice.
+
+	switch dnsStrategy {
+	case cfgtypes.ServiceScopedPodDNSStrategy:
+		if service.Name == "" {
+			return DiscoveredAdminAPI{}, fmt.Errorf(
+				"service name is empty for an endpoint with TargetRef %s/%s",
+				endpoint.TargetRef.Namespace, endpoint.TargetRef.Name,
+			)
+		}
+
+		ipAddr := strings.ReplaceAll(eAddress, ".", "-")
+		address := fmt.Sprintf("%s.%s.%s.svc", ipAddr, service.Name, service.Namespace)
+
+		return DiscoveredAdminAPI{
+			Address: fmt.Sprintf("https://%s:%d", address, *port.Port),
+			PodRef:  podNN,
+		}, nil
+
+	case cfgtypes.NamespaceScopedPodDNSStrategy:
+		ipAddr := strings.ReplaceAll(eAddress, ".", "-")
+		address := fmt.Sprintf("%s.%s.pod", ipAddr, service.Namespace)
+
+		return DiscoveredAdminAPI{
+			Address: fmt.Sprintf("https://%s:%d", address, *port.Port),
+			PodRef:  podNN,
+		}, nil
+
+	case cfgtypes.IPDNSStrategy:
+		return DiscoveredAdminAPI{
+			Address: fmt.Sprintf("https://%s:%d", eAddress, *port.Port),
+			PodRef:  podNN,
+		}, nil
+
+	default:
+		return DiscoveredAdminAPI{}, fmt.Errorf("unknown dns strategy: %s", dnsStrategy)
+	}
 }
diff --git a/internal/adminapi/endpoints_envtest_test.go b/internal/adminapi/endpoints_envtest_test.go
@@ -20,6 +20,8 @@ import (
 	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/adminapi"
+	cfgtypes "github.com/kong/kubernetes-ingress-controller/v2/internal/manager/config/types"
+	"github.com/kong/kubernetes-ingress-controller/v2/internal/util/builder"
 	"github.com/kong/kubernetes-ingress-controller/v2/test/envtest"
 )
 
@@ -88,18 +90,13 @@ func TestGetAdminAPIsForServiceReturnsAllAddressesCorrectlyPagingThroughResults(
 								TargetRef: testPodReference("pod-1", ns.Name),
 							},
 						},
-						Ports: []discoveryv1.EndpointPort{
-							{
-								Name: lo.ToPtr("admin"),
-								Port: lo.ToPtr(int32(8444)),
-							},
-						},
+						Ports: builder.NewEndpointPort(8444).WithName("admin").IntoSlice(),
 					}
 					require.NoError(t, client.Create(ctx, &es))
 				}
 			}
 
-			got, err := adminapi.GetAdminAPIsForService(ctx, client, service, sets.New("admin"))
+			got, err := adminapi.GetAdminAPIsForService(ctx, client, service, sets.New("admin"), cfgtypes.IPDNSStrategy)
 			require.NoError(t, err)
 			require.Len(t, got, tc.subnetD*tc.subnetC, "GetAdminAPIsForService should return all valid addresses")
 		})