Skip to content

Commit

Permalink
Refactor getTTLFromAnnotations() to not return error
Browse files Browse the repository at this point in the history
  • Loading branch information
johngmyers committed Sep 20, 2023
1 parent ed78d02 commit 1924dca
Show file tree
Hide file tree
Showing 16 changed files with 33 additions and 89 deletions.
5 changes: 1 addition & 4 deletions source/ambassador_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,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)

if host.Spec != nil {
hostname := host.Spec.Hostname
Expand Down
10 changes: 2 additions & 8 deletions source/contour_httpproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,7 @@ func (sc *httpProxySource) endpointsFromTemplate(httpProxy *projectcontour.HTTPP
return nil, err
}

ttl, err := getTTLFromAnnotations(httpProxy.Annotations)
if err != nil {
log.Warn(err)
}
ttl := getTTLFromAnnotations(httpProxy.Annotations)

targets := getTargetsFromTargetAnnotation(httpProxy.Annotations)
if len(targets) == 0 {
Expand Down Expand Up @@ -252,10 +249,7 @@ func (sc *httpProxySource) endpointsFromHTTPProxy(httpProxy *projectcontour.HTTP
return nil, nil
}

ttl, err := getTTLFromAnnotations(httpProxy.Annotations)
if err != nil {
log.Warn(err)
}
ttl := getTTLFromAnnotations(httpProxy.Annotations)

targets := getTargetsFromTargetAnnotation(httpProxy.Annotations)

Expand Down
5 changes: 1 addition & 4 deletions source/f5_virtualserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,7 @@ 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
}
ttl := getTTLFromAnnotations(virtualServer.Annotations)

if virtualServer.Spec.VirtualServerAddress != "" {
ep := &endpoint.Endpoint{
Expand Down
5 changes: 1 addition & 4 deletions source/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
for host, targets := range hostTargets {
endpoints = append(endpoints, endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier, resource)...)
}
Expand Down
5 changes: 1 addition & 4 deletions source/gloo_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,7 @@ func (gs *glooSource) generateEndpointsFromProxy(ctx context.Context, proxy *pro
if err != nil {
return nil, err
}
ttl, err := getTTLFromAnnotations(annotations)
if err != nil {
return nil, err
}
ttl := getTTLFromAnnotations(annotations)
providerSpecific, setIdentifier := getProviderSpecificAnnotations(annotations)
for _, domain := range virtualHost.Domains {
endpoints = append(endpoints, endpointsForHostname(strings.TrimSuffix(domain, "."), targets, ttl, providerSpecific, setIdentifier, "")...)
Expand Down
10 changes: 2 additions & 8 deletions source/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,7 @@ func (sc *ingressSource) endpointsFromTemplate(ing *networkv1.Ingress) ([]*endpo
return nil, err
}

ttl, err := getTTLFromAnnotations(ing.Annotations)
if err != nil {
log.Warn(err)
}
ttl := getTTLFromAnnotations(ing.Annotations)

targets := getTargetsFromTargetAnnotation(ing.Annotations)
if len(targets) == 0 {
Expand Down Expand Up @@ -289,10 +286,7 @@ 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)
}
ttl := getTTLFromAnnotations(ing.Annotations)

targets := getTargetsFromTargetAnnotation(ing.Annotations)

Expand Down
7 changes: 2 additions & 5 deletions source/istio_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,15 +304,12 @@ 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

annotations := gateway.Annotations
ttl, err := getTTLFromAnnotations(annotations)
if err != nil {
log.Warn(err)
}
ttl := getTTLFromAnnotations(annotations)

targets := getTargetsFromTargetAnnotation(annotations)

if len(targets) == 0 {
targets, err = sc.targetsFromGateway(ctx, gateway)
if err != nil {
Expand Down
11 changes: 3 additions & 8 deletions source/istio_virtualservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,7 @@ func (sc *virtualServiceSource) endpointsFromTemplate(ctx context.Context, virtu
return nil, err
}

ttl, err := getTTLFromAnnotations(virtualService.Annotations)
if err != nil {
log.Warn(err)
}
ttl := getTTLFromAnnotations(virtualService.Annotations)

providerSpecific, setIdentifier := getProviderSpecificAnnotations(virtualService.Annotations)

Expand Down Expand Up @@ -312,11 +309,9 @@ 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)
}
ttl := getTTLFromAnnotations(virtualservice.Annotations)

targetsFromAnnotation := getTargetsFromTargetAnnotation(virtualservice.Annotations)

Expand Down
5 changes: 1 addition & 4 deletions source/kong_tcpingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,7 @@ 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
}
ttl := getTTLFromAnnotations(tcpIngress.Annotations)

providerSpecific, setIdentifier := getProviderSpecificAnnotations(tcpIngress.Annotations)

Expand Down
5 changes: 1 addition & 4 deletions source/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,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)

// create new endpoint with the information we already have
ep := &endpoint.Endpoint{
Expand Down
10 changes: 2 additions & 8 deletions source/openshift_route.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,7 @@ func (ors *ocpRouteSource) endpointsFromTemplate(ocpRoute *routev1.Route) ([]*en
return nil, err
}

ttl, err := getTTLFromAnnotations(ocpRoute.Annotations)
if err != nil {
log.Warn(err)
}
ttl := getTTLFromAnnotations(ocpRoute.Annotations)

targets := getTargetsFromTargetAnnotation(ocpRoute.Annotations)
if len(targets) == 0 {
Expand Down Expand Up @@ -230,10 +227,7 @@ 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)
}
ttl := getTTLFromAnnotations(ocpRoute.Annotations)

targets := getTargetsFromTargetAnnotation(ocpRoute.Annotations)
targetsFromRoute, host := ors.getTargetsFromRouteStatus(ocpRoute.Status)
Expand Down
6 changes: 2 additions & 4 deletions source/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

epA := &endpoint.Endpoint{
RecordTTL: ttl,
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 2 additions & 5 deletions source/skipper_routegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ func (sc *routeGroupSource) endpointsFromTemplate(rg *routeGroup) ([]*endpoint.E
hostnames := buf.String()

// error handled in endpointsFromRouteGroup(), otherwise duplicate log
ttl, _ := getTTLFromAnnotations(rg.Metadata.Annotations)
ttl := getTTLFromAnnotations(rg.Metadata.Annotations)

targets := getTargetsFromTargetAnnotation(rg.Metadata.Annotations)

Expand Down Expand Up @@ -336,10 +336,7 @@ 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)
}
ttl := getTTLFromAnnotations(rg.Metadata.Annotations)

targets := getTargetsFromTargetAnnotation(rg.Metadata.Annotations)
if len(targets) == 0 {
Expand Down
13 changes: 8 additions & 5 deletions source/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"time"
"unicode"

log "github.com/sirupsen/logrus"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -86,20 +87,22 @@ type Source interface {
AddEventHandler(context.Context, func())
}

func getTTLFromAnnotations(annotations map[string]string) (endpoint.TTL, error) {
func getTTLFromAnnotations(annotations map[string]string) endpoint.TTL {
ttlNotConfigured := endpoint.TTL(0)
ttlAnnotation, exists := annotations[ttlAnnotationKey]
if !exists {
return ttlNotConfigured, nil
return ttlNotConfigured
}
ttlValue, err := parseTTL(ttlAnnotation)
if err != nil {
return ttlNotConfigured, fmt.Errorf("\"%v\" is not a valid TTL value", ttlAnnotation)
log.Warnf("\"%v\" is not a valid TTL value", ttlAnnotation)
return ttlNotConfigured
}
if ttlValue < ttlMinimum || ttlValue > ttlMaximum {
return ttlNotConfigured, fmt.Errorf("TTL value must be between [%d, %d]", ttlMinimum, ttlMaximum)
log.Warnf("TTL value %q must be between [%d, %d]", ttlValue, ttlMinimum, ttlMaximum)
return ttlNotConfigured
}
return endpoint.TTL(ttlValue), nil
return endpoint.TTL(ttlValue)
}

// parseTTL parses TTL from string, returning duration in seconds.
Expand Down
3 changes: 1 addition & 2 deletions source/source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,8 @@ func TestGetTTLFromAnnotations(t *testing.T) {
},
} {
t.Run(tc.title, func(t *testing.T) {
ttl, err := getTTLFromAnnotations(tc.annotations)
ttl := getTTLFromAnnotations(tc.annotations)
assert.Equal(t, tc.expectedTTL, ttl)
assert.Equal(t, tc.expectedErr, err)
})
}
}
Expand Down
15 changes: 3 additions & 12 deletions source/traefik_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -647,10 +647,7 @@ func (ts *traefikSource) setDualstackLabelIngressRouteUDP(ingressRoute *IngressR
func (ts *traefikSource) endpointsFromIngressRoute(ingressRoute *IngressRoute, targets endpoint.Targets) ([]*endpoint.Endpoint, error) {
var endpoints []*endpoint.Endpoint

ttl, err := getTTLFromAnnotations(ingressRoute.Annotations)
if err != nil {
return nil, err
}
ttl := getTTLFromAnnotations(ingressRoute.Annotations)

providerSpecific, setIdentifier := getProviderSpecificAnnotations(ingressRoute.Annotations)

Expand Down Expand Up @@ -684,10 +681,7 @@ func (ts *traefikSource) endpointsFromIngressRoute(ingressRoute *IngressRoute, t
func (ts *traefikSource) endpointsFromIngressRouteTCP(ingressRoute *IngressRouteTCP, targets endpoint.Targets) ([]*endpoint.Endpoint, error) {
var endpoints []*endpoint.Endpoint

ttl, err := getTTLFromAnnotations(ingressRoute.Annotations)
if err != nil {
return nil, err
}
ttl := getTTLFromAnnotations(ingressRoute.Annotations)

providerSpecific, setIdentifier := getProviderSpecificAnnotations(ingressRoute.Annotations)

Expand Down Expand Up @@ -722,10 +716,7 @@ func (ts *traefikSource) endpointsFromIngressRouteTCP(ingressRoute *IngressRoute
func (ts *traefikSource) endpointsFromIngressRouteUDP(ingressRoute *IngressRouteUDP, targets endpoint.Targets) ([]*endpoint.Endpoint, error) {
var endpoints []*endpoint.Endpoint

ttl, err := getTTLFromAnnotations(ingressRoute.Annotations)
if err != nil {
return nil, err
}
ttl := getTTLFromAnnotations(ingressRoute.Annotations)

providerSpecific, setIdentifier := getProviderSpecificAnnotations(ingressRoute.Annotations)

Expand Down

0 comments on commit 1924dca

Please sign in to comment.