Refactor getTTLFromAnnotations() to not return error (#3939)

johngmyers · web-flow · commit 17e9637f1182 · 2023-10-02T03:56:45.000-07:00
* Refactor getTTLFromAnnotations() to not return error

* Improve log messages
diff --git a/source/ambassador_host.go b/source/ambassador_host.go
@@ -173,10 +173,7 @@ func (sc *ambassadorHostSource) endpointsFromHost(ctx context.Context, host *amb
 	resource := fmt.Sprintf("host/%s/%s", host.Namespace, host.Name)
 
 	annotations := host.Annotations
-	ttl, err := getTTLFromAnnotations(annotations)
-	if err != nil {
-		return nil, err
-	}
+	ttl := getTTLFromAnnotations(annotations, resource)
 
 	if host.Spec != nil {
 		hostname := host.Spec.Hostname
diff --git a/source/contour_httpproxy.go b/source/contour_httpproxy.go
@@ -186,10 +186,9 @@ func (sc *httpProxySource) endpointsFromTemplate(httpProxy *projectcontour.HTTPP
 		return nil, err
 	}
 
-	ttl, err := getTTLFromAnnotations(httpProxy.Annotations)
-	if err != nil {
-		log.Warn(err)
-	}
+	resource := fmt.Sprintf("HTTPProxy/%s/%s", httpProxy.Namespace, httpProxy.Name)
+
+	ttl := getTTLFromAnnotations(httpProxy.Annotations, resource)
 
 	targets := getTargetsFromTargetAnnotation(httpProxy.Annotations)
 	if len(targets) == 0 {
@@ -205,8 +204,6 @@ func (sc *httpProxySource) endpointsFromTemplate(httpProxy *projectcontour.HTTPP
 
 	providerSpecific, setIdentifier := getProviderSpecificAnnotations(httpProxy.Annotations)
 
-	resource := fmt.Sprintf("HTTPProxy/%s/%s", httpProxy.Namespace, httpProxy.Name)
-
 	var endpoints []*endpoint.Endpoint
 	for _, hostname := range hostnames {
 		endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier, resource)...)
@@ -252,10 +249,9 @@ func (sc *httpProxySource) endpointsFromHTTPProxy(httpProxy *projectcontour.HTTP
 		return nil, nil
 	}
 
-	ttl, err := getTTLFromAnnotations(httpProxy.Annotations)
-	if err != nil {
-		log.Warn(err)
-	}
+	resource := fmt.Sprintf("HTTPProxy/%s/%s", httpProxy.Namespace, httpProxy.Name)
+
+	ttl := getTTLFromAnnotations(httpProxy.Annotations, resource)
 
 	targets := getTargetsFromTargetAnnotation(httpProxy.Annotations)
 
@@ -272,8 +268,6 @@ func (sc *httpProxySource) endpointsFromHTTPProxy(httpProxy *projectcontour.HTTP
 
 	providerSpecific, setIdentifier := getProviderSpecificAnnotations(httpProxy.Annotations)
 
-	resource := fmt.Sprintf("HTTPProxy/%s/%s", httpProxy.Namespace, httpProxy.Name)
-
 	var endpoints []*endpoint.Endpoint
 
 	if virtualHost := httpProxy.Spec.VirtualHost; virtualHost != nil {
diff --git a/source/f5_virtualserver.go b/source/f5_virtualserver.go
@@ -147,10 +147,9 @@ func (vs *f5VirtualServerSource) endpointsFromVirtualServers(virtualServers []*f
 	var endpoints []*endpoint.Endpoint
 
 	for _, virtualServer := range virtualServers {
-		ttl, err := getTTLFromAnnotations(virtualServer.Annotations)
-		if err != nil {
-			return nil, err
-		}
+		resource := fmt.Sprintf("f5-virtualserver/%s/%s", virtualServer.Namespace, virtualServer.Name)
+
+		ttl := getTTLFromAnnotations(virtualServer.Annotations, resource)
 
 		targets := getTargetsFromTargetAnnotation(virtualServer.Annotations)
 		if len(targets) == 0 && virtualServer.Spec.VirtualServerAddress != "" {
@@ -160,7 +159,6 @@ func (vs *f5VirtualServerSource) endpointsFromVirtualServers(virtualServers []*f
 			targets = append(targets, virtualServer.Status.VSAddress)
 		}
 
-		resource := fmt.Sprintf("f5-virtualserver/%s/%s", virtualServer.Namespace, virtualServer.Name)
 		endpoints = append(endpoints, endpointsForHostname(virtualServer.Spec.Host, targets, ttl, nil, "", resource)...)
 	}
 
diff --git a/source/gateway.go b/source/gateway.go
@@ -230,10 +230,7 @@ func (src *gatewayRouteSource) Endpoints(ctx context.Context) ([]*endpoint.Endpo
 		// Create endpoints from hostnames and targets.
 		resource := fmt.Sprintf("%s/%s/%s", kind, meta.Namespace, meta.Name)
 		providerSpecific, setIdentifier := getProviderSpecificAnnotations(annots)
-		ttl, err := getTTLFromAnnotations(annots)
-		if err != nil {
-			log.Warn(err)
-		}
+		ttl := getTTLFromAnnotations(annots, resource)
 		for host, targets := range hostTargets {
 			endpoints = append(endpoints, endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier, resource)...)
 		}
diff --git a/source/gloo_proxy.go b/source/gloo_proxy.go
@@ -19,6 +19,7 @@ package source
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"strings"
 
 	log "github.com/sirupsen/logrus"
@@ -155,16 +156,16 @@ func (gs *glooSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, erro
 
 func (gs *glooSource) generateEndpointsFromProxy(ctx context.Context, proxy *proxy, targets endpoint.Targets) ([]*endpoint.Endpoint, error) {
 	endpoints := []*endpoint.Endpoint{}
+
+	resource := fmt.Sprintf("proxy/%s/%s", proxy.Metadata.Namespace, proxy.Metadata.Name)
+
 	for _, listener := range proxy.Spec.Listeners {
 		for _, virtualHost := range listener.HTTPListener.VirtualHosts {
 			annotations, err := gs.annotationsFromProxySource(ctx, virtualHost)
 			if err != nil {
 				return nil, err
 			}
-			ttl, err := getTTLFromAnnotations(annotations)
-			if err != nil {
-				return nil, err
-			}
+			ttl := getTTLFromAnnotations(annotations, resource)
 			providerSpecific, setIdentifier := getProviderSpecificAnnotations(annotations)
 			for _, domain := range virtualHost.Domains {
 				endpoints = append(endpoints, endpointsForHostname(strings.TrimSuffix(domain, "."), targets, ttl, providerSpecific, setIdentifier, "")...)
diff --git a/source/ingress.go b/source/ingress.go
@@ -188,10 +188,9 @@ func (sc *ingressSource) endpointsFromTemplate(ing *networkv1.Ingress) ([]*endpo
 		return nil, err
 	}
 
-	ttl, err := getTTLFromAnnotations(ing.Annotations)
-	if err != nil {
-		log.Warn(err)
-	}
+	resource := fmt.Sprintf("ingress/%s/%s", ing.Namespace, ing.Name)
+
+	ttl := getTTLFromAnnotations(ing.Annotations, resource)
 
 	targets := getTargetsFromTargetAnnotation(ing.Annotations)
 	if len(targets) == 0 {
@@ -200,8 +199,6 @@ func (sc *ingressSource) endpointsFromTemplate(ing *networkv1.Ingress) ([]*endpo
 
 	providerSpecific, setIdentifier := getProviderSpecificAnnotations(ing.Annotations)
 
-	resource := fmt.Sprintf("ingress/%s/%s", ing.Namespace, ing.Name)
-
 	var endpoints []*endpoint.Endpoint
 	for _, hostname := range hostnames {
 		endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier, resource)...)
@@ -289,10 +286,9 @@ func (sc *ingressSource) setDualstackLabel(ingress *networkv1.Ingress, endpoints
 
 // endpointsFromIngress extracts the endpoints from ingress object
 func endpointsFromIngress(ing *networkv1.Ingress, ignoreHostnameAnnotation bool, ignoreIngressTLSSpec bool, ignoreIngressRulesSpec bool) []*endpoint.Endpoint {
-	ttl, err := getTTLFromAnnotations(ing.Annotations)
-	if err != nil {
-		log.Warn(err)
-	}
+	resource := fmt.Sprintf("ingress/%s/%s", ing.Namespace, ing.Name)
+
+	ttl := getTTLFromAnnotations(ing.Annotations, resource)
 
 	targets := getTargetsFromTargetAnnotation(ing.Annotations)
 
@@ -302,8 +298,6 @@ func endpointsFromIngress(ing *networkv1.Ingress, ignoreHostnameAnnotation bool,
 
 	providerSpecific, setIdentifier := getProviderSpecificAnnotations(ing.Annotations)
 
-	resource := fmt.Sprintf("ingress/%s/%s", ing.Namespace, ing.Name)
-
 	// Gather endpoints defined on hosts sections of the ingress
 	var definedHostsEndpoints []*endpoint.Endpoint
 	// Skip endpoints if we do not want entries from Rules section
diff --git a/source/istio_gateway.go b/source/istio_gateway.go
@@ -304,15 +304,14 @@ func (sc *gatewaySource) targetsFromGateway(ctx context.Context, gateway *networ
 // endpointsFromGatewayConfig extracts the endpoints from an Istio Gateway Config object
 func (sc *gatewaySource) endpointsFromGateway(ctx context.Context, hostnames []string, gateway *networkingv1alpha3.Gateway) ([]*endpoint.Endpoint, error) {
 	var endpoints []*endpoint.Endpoint
+	var err error
+
+	resource := fmt.Sprintf("gateway/%s/%s", gateway.Namespace, gateway.Name)
 
 	annotations := gateway.Annotations
-	ttl, err := getTTLFromAnnotations(annotations)
-	if err != nil {
-		log.Warn(err)
-	}
+	ttl := getTTLFromAnnotations(annotations, resource)
 
 	targets := getTargetsFromTargetAnnotation(annotations)
-
 	if len(targets) == 0 {
 		targets, err = sc.targetsFromGateway(ctx, gateway)
 		if err != nil {
@@ -322,8 +321,6 @@ func (sc *gatewaySource) endpointsFromGateway(ctx context.Context, hostnames []s
 
 	providerSpecific, setIdentifier := getProviderSpecificAnnotations(annotations)
 
-	resource := fmt.Sprintf("gateway/%s/%s", gateway.Namespace, gateway.Name)
-
 	for _, host := range hostnames {
 		endpoints = append(endpoints, endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier, resource)...)
 	}
diff --git a/source/istio_virtualservice.go b/source/istio_virtualservice.go
@@ -222,14 +222,11 @@ func (sc *virtualServiceSource) endpointsFromTemplate(ctx context.Context, virtu
 		return nil, err
 	}
 
-	ttl, err := getTTLFromAnnotations(virtualService.Annotations)
-	if err != nil {
-		log.Warn(err)
-	}
+	resource := fmt.Sprintf("virtualservice/%s/%s", virtualService.Namespace, virtualService.Name)
 
-	providerSpecific, setIdentifier := getProviderSpecificAnnotations(virtualService.Annotations)
+	ttl := getTTLFromAnnotations(virtualService.Annotations, resource)
 
-	resource := fmt.Sprintf("virtualservice/%s/%s", virtualService.Namespace, virtualService.Name)
+	providerSpecific, setIdentifier := getProviderSpecificAnnotations(virtualService.Annotations)
 
 	var endpoints []*endpoint.Endpoint
 	for _, hostname := range hostnames {
@@ -312,18 +309,16 @@ func (sc *virtualServiceSource) targetsFromVirtualService(ctx context.Context, v
 // endpointsFromVirtualService extracts the endpoints from an Istio VirtualService Config object
 func (sc *virtualServiceSource) endpointsFromVirtualService(ctx context.Context, virtualservice *networkingv1alpha3.VirtualService) ([]*endpoint.Endpoint, error) {
 	var endpoints []*endpoint.Endpoint
+	var err error
 
-	ttl, err := getTTLFromAnnotations(virtualservice.Annotations)
-	if err != nil {
-		log.Warn(err)
-	}
+	resource := fmt.Sprintf("virtualservice/%s/%s", virtualservice.Namespace, virtualservice.Name)
+
+	ttl := getTTLFromAnnotations(virtualservice.Annotations, resource)
 
 	targetsFromAnnotation := getTargetsFromTargetAnnotation(virtualservice.Annotations)
 
 	providerSpecific, setIdentifier := getProviderSpecificAnnotations(virtualservice.Annotations)
 
-	resource := fmt.Sprintf("virtualservice/%s/%s", virtualservice.Namespace, virtualservice.Name)
-
 	for _, host := range virtualservice.Spec.Hosts {
 		if host == "" || host == "*" {
 			continue
diff --git a/source/kong_tcpingress.go b/source/kong_tcpingress.go
@@ -204,14 +204,11 @@ func (sc *kongTCPIngressSource) setDualstackLabel(tcpIngress *TCPIngress, endpoi
 func (sc *kongTCPIngressSource) endpointsFromTCPIngress(tcpIngress *TCPIngress, targets endpoint.Targets) ([]*endpoint.Endpoint, error) {
 	var endpoints []*endpoint.Endpoint
 
-	ttl, err := getTTLFromAnnotations(tcpIngress.Annotations)
-	if err != nil {
-		return nil, err
-	}
+	resource := fmt.Sprintf("tcpingress/%s/%s", tcpIngress.Namespace, tcpIngress.Name)
 
-	providerSpecific, setIdentifier := getProviderSpecificAnnotations(tcpIngress.Annotations)
+	ttl := getTTLFromAnnotations(tcpIngress.Annotations, resource)
 
-	resource := fmt.Sprintf("tcpingress/%s/%s", tcpIngress.Namespace, tcpIngress.Name)
+	providerSpecific, setIdentifier := getProviderSpecificAnnotations(tcpIngress.Annotations)
 
 	hostnameList := getHostnamesFromAnnotations(tcpIngress.Annotations)
 	for _, hostname := range hostnameList {
diff --git a/source/node.go b/source/node.go
@@ -104,10 +104,7 @@ func (ns *nodeSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, erro
 
 		log.Debugf("creating endpoint for node %s", node.Name)
 
-		ttl, err := getTTLFromAnnotations(node.Annotations)
-		if err != nil {
-			log.Warn(err)
-		}
+		ttl := getTTLFromAnnotations(node.Annotations, fmt.Sprintf("node/%s", node.Name))
 
 		// create new endpoint with the information we already have
 		ep := &endpoint.Endpoint{
diff --git a/source/openshift_route.go b/source/openshift_route.go
@@ -174,10 +174,9 @@ func (ors *ocpRouteSource) endpointsFromTemplate(ocpRoute *routev1.Route) ([]*en
 		return nil, err
 	}
 
-	ttl, err := getTTLFromAnnotations(ocpRoute.Annotations)
-	if err != nil {
-		log.Warn(err)
-	}
+	resource := fmt.Sprintf("route/%s/%s", ocpRoute.Namespace, ocpRoute.Name)
+
+	ttl := getTTLFromAnnotations(ocpRoute.Annotations, resource)
 
 	targets := getTargetsFromTargetAnnotation(ocpRoute.Annotations)
 	if len(targets) == 0 {
@@ -187,8 +186,6 @@ func (ors *ocpRouteSource) endpointsFromTemplate(ocpRoute *routev1.Route) ([]*en
 
 	providerSpecific, setIdentifier := getProviderSpecificAnnotations(ocpRoute.Annotations)
 
-	resource := fmt.Sprintf("route/%s/%s", ocpRoute.Namespace, ocpRoute.Name)
-
 	var endpoints []*endpoint.Endpoint
 	for _, hostname := range hostnames {
 		endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier, resource)...)
@@ -230,10 +227,9 @@ func (ors *ocpRouteSource) filterByAnnotations(ocpRoutes []*routev1.Route) ([]*r
 func (ors *ocpRouteSource) endpointsFromOcpRoute(ocpRoute *routev1.Route, ignoreHostnameAnnotation bool) []*endpoint.Endpoint {
 	var endpoints []*endpoint.Endpoint
 
-	ttl, err := getTTLFromAnnotations(ocpRoute.Annotations)
-	if err != nil {
-		log.Warn(err)
-	}
+	resource := fmt.Sprintf("route/%s/%s", ocpRoute.Namespace, ocpRoute.Name)
+
+	ttl := getTTLFromAnnotations(ocpRoute.Annotations, resource)
 
 	targets := getTargetsFromTargetAnnotation(ocpRoute.Annotations)
 	targetsFromRoute, host := ors.getTargetsFromRouteStatus(ocpRoute.Status)
@@ -244,8 +240,6 @@ func (ors *ocpRouteSource) endpointsFromOcpRoute(ocpRoute *routev1.Route, ignore
 
 	providerSpecific, setIdentifier := getProviderSpecificAnnotations(ocpRoute.Annotations)
 
-	resource := fmt.Sprintf("route/%s/%s", ocpRoute.Namespace, ocpRoute.Name)
-
 	if host != "" {
 		endpoints = append(endpoints, endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier, resource)...)
 	}
diff --git a/source/service.go b/source/service.go
@@ -460,10 +460,7 @@ 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 {
 	hostname = strings.TrimSuffix(hostname, ".")
-	ttl, err := getTTLFromAnnotations(svc.Annotations)
-	if err != nil {
-		log.Warn(err)
-	}
+	ttl := getTTLFromAnnotations(svc.Annotations, fmt.Sprintf("service/%s/%s", svc.Namespace, svc.Name))
 
 	epA := &endpoint.Endpoint{
 		RecordTTL:  ttl,
@@ -510,6 +507,7 @@ func (sc *serviceSource) generateEndpoints(svc *v1.Service, hostname string, pro
 			}
 		case v1.ServiceTypeNodePort:
 			// add the nodeTargets and extract an SRV endpoint
+			var err error
 			targets, err = sc.extractNodePortTargets(svc)
 			if err != nil {
 				log.Errorf("Unable to extract targets from service %s/%s error: %v", svc.Namespace, svc.Name, err)
diff --git a/source/skipper_routegroup.go b/source/skipper_routegroup.go
@@ -300,8 +300,10 @@ func (sc *routeGroupSource) endpointsFromTemplate(rg *routeGroup) ([]*endpoint.E
 
 	hostnames := buf.String()
 
+	resource := fmt.Sprintf("routegroup/%s/%s", rg.Metadata.Namespace, rg.Metadata.Name)
+
 	// error handled in endpointsFromRouteGroup(), otherwise duplicate log
-	ttl, _ := getTTLFromAnnotations(rg.Metadata.Annotations)
+	ttl := getTTLFromAnnotations(rg.Metadata.Annotations, resource)
 
 	targets := getTargetsFromTargetAnnotation(rg.Metadata.Annotations)
 
@@ -311,8 +313,6 @@ func (sc *routeGroupSource) endpointsFromTemplate(rg *routeGroup) ([]*endpoint.E
 
 	providerSpecific, setIdentifier := getProviderSpecificAnnotations(rg.Metadata.Annotations)
 
-	resource := fmt.Sprintf("routegroup/%s/%s", rg.Metadata.Namespace, rg.Metadata.Name)
-
 	var endpoints []*endpoint.Endpoint
 	// splits the FQDN template and removes the trailing periods
 	hostnameList := strings.Split(strings.Replace(hostnames, " ", "", -1), ",")
@@ -336,10 +336,10 @@ func (sc *routeGroupSource) setRouteGroupDualstackLabel(rg *routeGroup, eps []*e
 // annotation logic ported from source/ingress.go without Spec.TLS part, because it'S not supported in RouteGroup
 func (sc *routeGroupSource) endpointsFromRouteGroup(rg *routeGroup) []*endpoint.Endpoint {
 	endpoints := []*endpoint.Endpoint{}
-	ttl, err := getTTLFromAnnotations(rg.Metadata.Annotations)
-	if err != nil {
-		log.Warnf("Failed to get TTL from annotation: %v", err)
-	}
+
+	resource := fmt.Sprintf("routegroup/%s/%s", rg.Metadata.Namespace, rg.Metadata.Name)
+
+	ttl := getTTLFromAnnotations(rg.Metadata.Annotations, resource)
 
 	targets := getTargetsFromTargetAnnotation(rg.Metadata.Annotations)
 	if len(targets) == 0 {
@@ -355,8 +355,6 @@ func (sc *routeGroupSource) endpointsFromRouteGroup(rg *routeGroup) []*endpoint.
 
 	providerSpecific, setIdentifier := getProviderSpecificAnnotations(rg.Metadata.Annotations)
 
-	resource := fmt.Sprintf("routegroup/%s/%s", rg.Metadata.Namespace, rg.Metadata.Name)
-
 	for _, src := range rg.Spec.Hosts {
 		if src == "" {
 			continue
diff --git a/source/source.go b/source/source.go
diff --git a/source/source_test.go b/source/source_test.go
diff --git a/source/traefik_proxy.go b/source/traefik_proxy.go
