Code maintenance: rework ovpn host extraction

internal/updater/openvpn.go

@@ -1,26 +1,50 @@
 package updater
 
 import (
+	"errors"
+	"fmt"
+	"net"
 	"strings"
 )
 
-func extractRemoteLinesFromOpenvpn(content []byte) (remoteLines []string) {
-	lines := strings.Split(string(content), "\n")
-	for _, line := range lines {
-		if strings.HasPrefix(line, "remote ") {
-			remoteLines = append(remoteLines, line)
-		}
+var (
+	errRemoteHostNotFound = errors.New("remote host not found")
+)
+
+func extractHostFromOVPN(b []byte) (host, warning string, err error) {
+	const (
+		rejectIP     = true
+		rejectDomain = false
+	)
+	hosts := extractRemoteHostsFromOpenvpn(b, rejectIP, rejectDomain)
+	if len(hosts) == 0 {
+		return "", "", errRemoteHostNotFound
+	} else if len(hosts) > 1 {
+		warning = fmt.Sprintf(
+			"only using the first host %q and discarding %d other hosts",
+			hosts[0], len(hosts)-1)
 	}
-	return remoteLines
+	return hosts[0], warning, nil
 }
 
-func extractHostnamesFromRemoteLines(remoteLines []string) (hostnames []string) {
-	for _, remoteLine := range remoteLines {
-		fields := strings.Fields(remoteLine)
-		if len(fields[1]) == 0 {
+func extractRemoteHostsFromOpenvpn(content []byte,
+	rejectIP, rejectDomain bool) (hosts []string) {
+	lines := strings.Split(string(content), "\n")
+	for _, line := range lines {
+		if !strings.HasPrefix(line, "remote ") {
 			continue
 		}
-		hostnames = append(hostnames, fields[1])
+		fields := strings.Fields(line)
+		if len(fields) == 1 || len(fields[1]) == 0 {
+			continue
+		}
+		host := fields[1]
+		parsedIP := net.ParseIP(host)
+		if (rejectIP && parsedIP != nil) ||
+			(rejectDomain && parsedIP == nil) {
+			continue
+		}
+		hosts = append(hosts, host)
 	}
-	return hostnames
+	return hosts
 }

internal/updater/privado.go

@@ -3,7 +3,6 @@ package updater
 import (
 	"context"
 	"fmt"
-	"net"
 	"sort"
 
 	"github.com/qdm12/gluetun/internal/models"
@@ -39,22 +38,12 @@ func findPrivadoServersFromZip(ctx context.Context, client network.Client, looku
 		if err := ctx.Err(); err != nil {
 			return nil, warnings, err
 		}
-		remoteLines := extractRemoteLinesFromOpenvpn(content)
-		if len(remoteLines) == 0 {
-			return nil, warnings, fmt.Errorf("cannot find any remote lines in %s", fileName)
-		}
-		hostnames := extractHostnamesFromRemoteLines(remoteLines)
-		if len(hostnames) == 0 {
-			return nil, warnings, fmt.Errorf("cannot find any hosts in %s", fileName)
-		} else if len(hostnames) > 1 {
-			warning := fmt.Sprintf("more than one host in %q, only taking first one %q into account", fileName, hostnames[0])
+		hostname, warning, err := extractHostFromOVPN(content)
+		if len(warning) > 0 {
 			warnings = append(warnings, warning)
 		}
-		hostname := hostnames[0]
-		if net.ParseIP(hostname) != nil {
-			warning := fmt.Sprintf("ignoring IP address host %q in %s", hostname, fileName)
-			warnings = append(warnings, warning)
-			continue
+		if err != nil {
+			return nil, warnings, fmt.Errorf("%w in %q", err, fileName)
 		}
 		const repetition = 1
 		IPs, err := resolveRepeat(ctx, lookupIP, hostname, repetition)

internal/updater/surfshark.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"net"
 	"net/http"
 	"sort"
 	"strings"
@@ -84,22 +83,12 @@ func findSurfsharkServersFromZip(ctx context.Context, client network.Client, loo
 		if strings.HasSuffix(fileName, "_tcp.ovpn") {
 			continue // only parse UDP files
 		}
-		remoteLines := extractRemoteLinesFromOpenvpn(content)
-		if len(remoteLines) == 0 {
-			return nil, warnings, fmt.Errorf("cannot find any remote lines in %s", fileName)
-		}
-		hosts := extractHostnamesFromRemoteLines(remoteLines)
-		if len(hosts) == 0 {
-			return nil, warnings, fmt.Errorf("cannot find any hosts in %s", fileName)
-		} else if len(hosts) > 1 {
-			warning := fmt.Sprintf("more than one host in %q, only taking first one %q into account", fileName, hosts[0])
+		host, warning, err := extractHostFromOVPN(content)
+		if len(warning) > 0 {
 			warnings = append(warnings, warning)
 		}
-		host := hosts[0]
-		if net.ParseIP(host) != nil {
-			warning := fmt.Sprintf("ignoring IP address host %q in %q", host, fileName)
-			warnings = append(warnings, warning)
-			continue
+		if err != nil {
+			return nil, warnings, fmt.Errorf("%w in %s", err, fileName)
 		}
 		const repetition = 5
 		IPs, err := resolveRepeat(ctx, lookupIP, host, repetition)
@@ -115,7 +104,7 @@ func findSurfsharkServersFromZip(ctx context.Context, client network.Client, loo
 		if ok {
 			delete(mapping, subdomain)
 		} else {
-			region = strings.TrimSuffix(hosts[0], ".prod.surfshark.com")
+			region = strings.TrimSuffix(host, ".prod.surfshark.com")
 			warning := fmt.Sprintf("subdomain %q not found in Surfshark mapping", subdomain)
 			warnings = append(warnings, warning)
 		}

internal/updater/vyprvpn.go

@@ -3,7 +3,6 @@ package updater
 import (
 	"context"
 	"fmt"
-	"net"
 	"sort"
 	"strings"
 
@@ -12,7 +11,12 @@ import (
 )
 
 func (u *updater) updateVyprvpn(ctx context.Context) (err error) {
-	servers, err := findVyprvpnServers(ctx, u.client, u.lookupIP)
+	servers, warnings, err := findVyprvpnServers(ctx, u.client, u.lookupIP)
+	if u.options.CLI {
+		for _, warning := range warnings {
+			u.logger.Warn("Privado: %s", warning)
+		}
+	}
 	if err != nil {
 		return fmt.Errorf("cannot update Vyprvpn servers: %w", err)
 	}
@@ -25,32 +29,27 @@ func (u *updater) updateVyprvpn(ctx context.Context) (err error) {
 }
 
 func findVyprvpnServers(ctx context.Context, client network.Client, lookupIP lookupIPFunc) (
-	servers []models.VyprvpnServer, err error) {
+	servers []models.VyprvpnServer, warnings []string, err error) {
 	const zipURL = "https://support.vyprvpn.com/hc/article_attachments/360052617332/Vypr_OpenVPN_20200320.zip"
 	contents, err := fetchAndExtractFiles(ctx, client, zipURL)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	for fileName, content := range contents {
 		if err := ctx.Err(); err != nil {
-			return nil, err
+			return nil, warnings, err
 		}
-		remoteLines := extractRemoteLinesFromOpenvpn(content)
-		if len(remoteLines) == 0 {
-			return nil, fmt.Errorf("cannot find any remote lines in %s", fileName)
+		host, warning, err := extractHostFromOVPN(content)
+		if len(warning) > 0 {
+			warnings = append(warnings, warning)
 		}
-		hosts := extractHostnamesFromRemoteLines(remoteLines)
-		if len(hosts) == 0 {
-			return nil, fmt.Errorf("cannot find any hosts in %s", fileName)
+		if err != nil {
+			return nil, warnings, fmt.Errorf("%w in %s", err, fileName)
 		}
-		var IPs []net.IP
-		for _, host := range hosts {
-			const repetitions = 1
-			newIPs, err := resolveRepeat(ctx, lookupIP, host, repetitions)
-			if err != nil {
-				return nil, err
-			}
-			IPs = append(IPs, newIPs...)
+		const repetitions = 1
+		IPs, err := resolveRepeat(ctx, lookupIP, host, repetitions)
+		if err != nil {
+			return nil, warnings, err
 		}
 		region := strings.TrimSuffix(fileName, ".ovpn")
 		region = strings.ReplaceAll(region, " - ", " ")
@@ -63,7 +62,7 @@ func findVyprvpnServers(ctx context.Context, client network.Client, lookupIP loo
 	sort.Slice(servers, func(i, j int) bool {
 		return servers[i].Region < servers[j].Region
 	})
-	return servers, nil
+	return servers, warnings, nil
 }
 
 func stringifyVyprvpnServers(servers []models.VyprvpnServer) (s string) {