fix(service): omit nil endpoints and prefer endpointsForHostname()

Also add a test with an invalid hostname.
kubernetes-sigs · Mar 4, 2024 · 7cac442 · 7cac442
1 parent 1eec428
commit 7cac442
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 53 deletions.
diff --git a/source/service.go b/source/service.go
@@ -360,10 +360,15 @@ func (sc *serviceSource) extractHeadlessEndpoints(svc *v1.Service, hostname stri
 			targets = append(targets, target)
 		}
 
+		var ep *endpoint.Endpoint
 		if ttl.IsConfigured() {
-			endpoints = append(endpoints, endpoint.NewEndpointWithTTL(headlessKey.DNSName, headlessKey.RecordType, ttl, targets...))
+			ep = endpoint.NewEndpointWithTTL(headlessKey.DNSName, headlessKey.RecordType, ttl, targets...)
 		} else {
-			endpoints = append(endpoints, endpoint.NewEndpoint(headlessKey.DNSName, headlessKey.RecordType, targets...))
+			ep = endpoint.NewEndpoint(headlessKey.DNSName, headlessKey.RecordType, targets...)
+		}
+
+		if ep != nil {
+			endpoints = append(endpoints, ep)
 		}
 	}
 
@@ -458,38 +463,14 @@ func (sc *serviceSource) setResourceLabel(service *v1.Service, endpoints []*endp
 	}
 }
 
-func (sc *serviceSource) generateEndpoints(svc *v1.Service, hostname string, providerSpecific endpoint.ProviderSpecific, setIdentifier string, useClusterIP bool) []*endpoint.Endpoint {
+func (sc *serviceSource) generateEndpoints(svc *v1.Service, hostname string, providerSpecific endpoint.ProviderSpecific, setIdentifier string, useClusterIP bool) (endpoints []*endpoint.Endpoint) {
 	hostname = strings.TrimSuffix(hostname, ".")
-	ttl := getTTLFromAnnotations(svc.Annotations, fmt.Sprintf("service/%s/%s", svc.Namespace, svc.Name))
 
-	epA := &endpoint.Endpoint{
-		RecordTTL:  ttl,
-		RecordType: endpoint.RecordTypeA,
-		Labels:     endpoint.NewLabels(),
-		Targets:    make(endpoint.Targets, 0, defaultTargetsCapacity),
-		DNSName:    hostname,
-	}
+	resource := fmt.Sprintf("service/%s/%s", svc.Namespace, svc.Name)
 
-	epAAAA := &endpoint.Endpoint{
-		RecordTTL:  ttl,
-		RecordType: endpoint.RecordTypeAAAA,
-		Labels:     endpoint.NewLabels(),
-		Targets:    make(endpoint.Targets, 0, defaultTargetsCapacity),
-		DNSName:    hostname,
-	}
-
-	epCNAME := &endpoint.Endpoint{
-		RecordTTL:  ttl,
-		RecordType: endpoint.RecordTypeCNAME,
-		Labels:     endpoint.NewLabels(),
-		Targets:    make(endpoint.Targets, 0, defaultTargetsCapacity),
-		DNSName:    hostname,
-	}
-
-	var endpoints []*endpoint.Endpoint
-	var targets endpoint.Targets
+	ttl := getTTLFromAnnotations(svc.Annotations, resource)
 
-	targets = getTargetsFromTargetAnnotation(svc.Annotations)
+	targets := getTargetsFromTargetAnnotation(svc.Annotations)
 
 	if len(targets) == 0 {
 		switch svc.Spec.Type {
@@ -517,32 +498,15 @@ func (sc *serviceSource) generateEndpoints(svc *v1.Service, hostname string, pro
 		case v1.ServiceTypeExternalName:
 			targets = extractServiceExternalName(svc)
 		}
-	}
 
-	for _, t := range targets {
-		switch suitableType(t) {
-		case endpoint.RecordTypeA:
-			epA.Targets = append(epA.Targets, t)
-		case endpoint.RecordTypeAAAA:
-			epAAAA.Targets = append(epAAAA.Targets, t)
-		case endpoint.RecordTypeCNAME:
-			epCNAME.Targets = append(epCNAME.Targets, t)
+		for _, endpoint := range endpoints {
+			endpoint.ProviderSpecific = providerSpecific
+			endpoint.SetIdentifier = setIdentifier
 		}
 	}
 
-	if len(epA.Targets) > 0 {
-		endpoints = append(endpoints, epA)
-	}
-	if len(epAAAA.Targets) > 0 {
-		endpoints = append(endpoints, epAAAA)
-	}
-	if len(epCNAME.Targets) > 0 {
-		endpoints = append(endpoints, epCNAME)
-	}
-	for _, endpoint := range endpoints {
-		endpoint.ProviderSpecific = providerSpecific
-		endpoint.SetIdentifier = setIdentifier
-	}
+	endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier, resource)...)
+
 	return endpoints
 }
 
@@ -742,7 +706,9 @@ func (sc *serviceSource) extractNodePortEndpoints(svc *v1.Service, hostname stri
 				ep = endpoint.NewEndpoint(recordName, endpoint.RecordTypeSRV, target)
 			}
 
-			endpoints = append(endpoints, ep)
+			if ep != nil {
+				endpoints = append(endpoints, ep)
+			}
 		}
 	}
 

diff --git a/source/service_test.go b/source/service_test.go
@@ -1560,6 +1560,17 @@ func TestClusterIpServices(t *testing.T) {
 			expected:      []*endpoint.Endpoint{},
 			labelSelector: "app=web-external",
 		},
+		{
+			title:        "invalid hostname does not generate endpoints",
+			svcNamespace: "testing",
+			svcName:      "foo",
+			svcType:      v1.ServiceTypeClusterIP,
+			annotations: map[string]string{
+				hostnameAnnotationKey: "this-is-an-exceedingly-long-label-that-external-dns-should-reject.example.org.",
+			},
+			clusterIP: "1.2.3.4",
+			expected:  []*endpoint.Endpoint{},
+		},
 	} {
 		tc := tc
 		t.Run(tc.title, func(t *testing.T) {