remove some code duplication in gateway source

kubernetes-sigs · May 14, 2020 · f87bea7 · f87bea7
1 parent 2ef1503
commit f87bea7
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 64 deletions.
diff --git a/source/gateway.go b/source/gateway.go
@@ -59,7 +59,7 @@ func NewIstioGatewaySource(
 	namespace string,
 	annotationFilter string,
 	fqdnTemplate string,
-	combineFqdnAnnotation bool,
+	combineFQDNAnnotation bool,
 	ignoreHostnameAnnotation bool,
 ) (Source, error) {
 	var (
@@ -107,7 +107,7 @@ func NewIstioGatewaySource(
 		namespace:                namespace,
 		annotationFilter:         annotationFilter,
 		fqdnTemplate:             tmpl,
-		combineFQDNAnnotation:    combineFqdnAnnotation,
+		combineFQDNAnnotation:    combineFQDNAnnotation,
 		ignoreHostnameAnnotation: ignoreHostnameAnnotation,
 		serviceInformer:          serviceInformer,
 	}, nil
@@ -126,7 +126,7 @@ func (sc *gatewaySource) Endpoints() ([]*endpoint.Endpoint, error) {
 		return nil, err
 	}
 
-	endpoints := []*endpoint.Endpoint{}
+	var endpoints []*endpoint.Endpoint
 
 	for _, config := range configs {
 		// Check controller annotation to see if we are responsible.
@@ -137,30 +137,35 @@ func (sc *gatewaySource) Endpoints() ([]*endpoint.Endpoint, error) {
 			continue
 		}
 
-		gwEndpoints, err := sc.endpointsFromGatewayConfig(config)
+		gwHostnames, err := sc.hostNamesFromGateway(&config)
 		if err != nil {
 			return nil, err
 		}
 
 		// apply template if host is missing on gateway
-		if (sc.combineFQDNAnnotation || len(gwEndpoints) == 0) && sc.fqdnTemplate != nil {
-			iEndpoints, err := sc.endpointsFromTemplate(&config)
+		if (sc.combineFQDNAnnotation || len(gwHostnames) == 0) && sc.fqdnTemplate != nil {
+			iHostnames, err := sc.hostNamesFromTemplate(&config)
 			if err != nil {
 				return nil, err
 			}
 
 			if sc.combineFQDNAnnotation {
-				gwEndpoints = append(gwEndpoints, iEndpoints...)
+				gwHostnames = append(gwHostnames, iHostnames...)
 			} else {
-				gwEndpoints = iEndpoints
+				gwHostnames = iHostnames
 			}
 		}
 
-		if len(gwEndpoints) == 0 {
-			log.Debugf("No endpoints could be generated from gateway %s/%s", config.Namespace, config.Name)
+		if len(gwHostnames) == 0 {
+			log.Debugf("No hostnames could be generated from gateway %s/%s", config.Namespace, config.Name)
 			continue
 		}
 
+		gwEndpoints, err := sc.endpointsFromConfig(gwHostnames, &config)
+		if err != nil {
+			return nil, err
+		}
+
 		log.Debugf("Endpoints generated from gateway: %s/%s: %v", config.Namespace, config.Name, gwEndpoints)
 		sc.setResourceLabel(config, gwEndpoints)
 		endpoints = append(endpoints, gwEndpoints...)
@@ -176,42 +181,6 @@ func (sc *gatewaySource) Endpoints() ([]*endpoint.Endpoint, error) {
 func (sc *gatewaySource) AddEventHandler(handler func() error, stopChan <-chan struct{}, minInterval time.Duration) {
 }
 
-func (sc *gatewaySource) endpointsFromTemplate(config *istiomodel.Config) ([]*endpoint.Endpoint, error) {
-	// Process the whole template string
-	var buf bytes.Buffer
-	err := sc.fqdnTemplate.Execute(&buf, config)
-	if err != nil {
-		return nil, fmt.Errorf("failed to apply template on istio config %s: %v", config, err)
-	}
-
-	hostnames := buf.String()
-
-	ttl, err := getTTLFromAnnotations(config.Annotations)
-	if err != nil {
-		log.Warn(err)
-	}
-
-	targets := getTargetsFromTargetAnnotation(config.Annotations)
-
-	if len(targets) == 0 {
-		targets, err = sc.targetsFromGatewayConfig(config)
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	providerSpecific, setIdentifier := getProviderSpecificAnnotations(config.Annotations)
-
-	var endpoints []*endpoint.Endpoint
-	// splits the FQDN template and removes the trailing periods
-	hostnameList := strings.Split(strings.Replace(hostnames, " ", "", -1), ",")
-	for _, hostname := range hostnameList {
-		hostname = strings.TrimSuffix(hostname, ".")
-		endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier)...)
-	}
-	return endpoints, nil
-}
-
 // filterByAnnotations filters a list of configs by a given annotation selector.
 func (sc *gatewaySource) filterByAnnotations(configs []istiomodel.Config) ([]istiomodel.Config, error) {
 	labelSelector, err := metav1.ParseToLabelSelector(sc.annotationFilter)
@@ -228,7 +197,7 @@ func (sc *gatewaySource) filterByAnnotations(configs []istiomodel.Config) ([]ist
 		return configs, nil
 	}
 
-	filteredList := []istiomodel.Config{}
+	var filteredList []istiomodel.Config
 
 	for _, config := range configs {
 		// convert the annotations to an equivalent label selector
@@ -270,8 +239,7 @@ func (sc *gatewaySource) targetsFromGatewayConfig(config *istiomodel.Config) (ta
 		for _, lb := range service.Status.LoadBalancer.Ingress {
 			if lb.IP != "" {
 				targets = append(targets, lb.IP)
-			}
-			if lb.Hostname != "" {
+			} else if lb.Hostname != "" {
 				targets = append(targets, lb.Hostname)
 			}
 		}
@@ -281,27 +249,44 @@ func (sc *gatewaySource) targetsFromGatewayConfig(config *istiomodel.Config) (ta
 }
 
 // endpointsFromGatewayConfig extracts the endpoints from an Istio Gateway Config object
-func (sc *gatewaySource) endpointsFromGatewayConfig(config istiomodel.Config) ([]*endpoint.Endpoint, error) {
+func (sc *gatewaySource) endpointsFromConfig(hostnames []string, config *istiomodel.Config) ([]*endpoint.Endpoint, error) {
 	var endpoints []*endpoint.Endpoint
 
-	ttl, err := getTTLFromAnnotations(config.Annotations)
+	annotations := config.Annotations
+
+	ttl, err := getTTLFromAnnotations(annotations)
 	if err != nil {
 		log.Warn(err)
 	}
 
-	targets := getTargetsFromTargetAnnotation(config.Annotations)
+	targets := getTargetsFromTargetAnnotation(annotations)
 
 	if len(targets) == 0 {
-		targets, err = sc.targetsFromGatewayConfig(&config)
+		targets, err = sc.targetsFromGatewayConfig(config)
 		if err != nil {
 			return nil, err
 		}
 	}
 
-	gateway := config.Spec.(*istionetworking.Gateway)
+	providerSpecific, setIdentifier := getProviderSpecificAnnotations(annotations)
+
+	// Skip endpoints if we do not want entries from annotations
+	if !sc.ignoreHostnameAnnotation {
+		hostnames = append(hostnames, getHostnamesFromAnnotations(annotations)...)
+	}
+
+	for _, host := range hostnames {
+		endpoints = append(endpoints, endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier)...)
+	}
+
+	return endpoints, nil
+}
 
-	providerSpecific, setIdentifier := getProviderSpecificAnnotations(config.Annotations)
+func (sc *gatewaySource) hostNamesFromGateway(config *istiomodel.Config) ([]string, error) {
+	// Process the whole template string
+	gateway := config.Spec.(*istionetworking.Gateway)
 
+	var hostnames []string
 	for _, server := range gateway.Servers {
 		for _, host := range server.Hosts {
 			if host == "" {
@@ -316,17 +301,20 @@ func (sc *gatewaySource) endpointsFromGatewayConfig(config istiomodel.Config) ([
 				host = parts[1]
 			}
 
-			endpoints = append(endpoints, endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier)...)
+			hostnames = append(hostnames, host)
 		}
 	}
+	return hostnames, nil
+}
 
-	// Skip endpoints if we do not want entries from annotations
-	if !sc.ignoreHostnameAnnotation {
-		hostnameList := getHostnamesFromAnnotations(config.Annotations)
-		for _, hostname := range hostnameList {
-			endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier)...)
-		}
+func (sc *gatewaySource) hostNamesFromTemplate(config *istiomodel.Config) ([]string, error) {
+	// Process the whole template string
+	var buf bytes.Buffer
+	err := sc.fqdnTemplate.Execute(&buf, config)
+	if err != nil {
+		return nil, fmt.Errorf("failed to apply template on istio config %s: %v", config, err)
 	}
 
-	return endpoints, nil
+	hostnames := strings.Split(strings.Replace(buf.String(), " ", "", -1), ",")
+	return hostnames, nil
 }
diff --git a/source/gateway_test.go b/source/gateway_test.go
@@ -316,9 +316,12 @@ func testEndpointsFromGatewayConfig(t *testing.T) {
 		},
 	} {
 		t.Run(ti.title, func(t *testing.T) {
+			istioCfg := ti.config.Config()
 			if source, err := newTestGatewaySource(ti.lbServices); err != nil {
 				require.NoError(t, err)
-			} else if endpoints, err := source.endpointsFromGatewayConfig(ti.config.Config()); err != nil {
+			} else if hostnames, err := source.hostNamesFromGateway(&istioCfg); err != nil {
+				require.NoError(t, err)
+			} else if endpoints, err := source.endpointsFromConfig(hostnames, &istioCfg); err != nil {
 				require.NoError(t, err)
 			} else {
 				validateEndpoints(t, endpoints, ti.expected)