[feat] gateway pool

5 files changed, 386 insertions, 165 deletions

diff --git a/pkg/vpn/bonafide/bonafide.go b/pkg/vpn/bonafide/bonafide.go
index 4426da6..22e3051 100644
--- a/pkg/vpn/bonafide/bonafide.go
+++ b/pkg/vpn/bonafide/bonafide.go
@@ -1,4 +1,4 @@
-// Copyright (C) 2018 LEAP
+// Copyright (C) 2018-2020 LEAP
 //
 // This program is free software: you can redistribute it and/or modify
 // it under the terms of the GNU General Public License as published by
@@ -46,19 +46,12 @@ type Bonafide struct {
 	client        httpClient
 	eip           *eipService
 	tzOffsetHours int
+	gateways      *gatewayPool
+	maxGateways   int
 	auth          authentication
 	token         []byte
 }
 
-type Gateway struct {
-	Host      string
-	IPAddress string
-	Location  string
-	Ports     []string
-	Protocols []string
-	Options   map[string]string
-}
-
 type openvpnConfig map[string]interface{}
 
 type httpClient interface {
@@ -192,24 +185,42 @@ func (b *Bonafide) getURL(object string) string {
 	return ""
 }
 
-func (b *Bonafide) GetGateways(transport string) ([]Gateway, error) {
+func (b *Bonafide) maybeInitializeEIP() error {
 	if b.eip == nil {
 		err := b.fetchEipJSON()
 		if err != nil {
-			return nil, err
+			return err
 		}
+		b.gateways = newGatewayPool(b.eip)
+		b.fetchGatewayRanking()
+	}
+	return nil
+}
+
+func (b *Bonafide) GetGateways(transport string) ([]Gateway, error) {
+	err := b.maybeInitializeEIP()
+	if err != nil {
+		return nil, err
+	}
+	max := maxGateways
+	if b.maxGateways != 0 {
+		max = b.maxGateways
 	}
 
-	return b.eip.getGateways(transport), nil
+	gws, err := b.gateways.getBest(transport, b.tzOffsetHours, max)
+	return gws, err
 }
 
-func (b *Bonafide) SetManualGateway(name string) {
-	/* TODO use gateway-id instead - a location-id is probably more useful than
-	* the gateway hostname */
-	b.eip.setManualGateway(name)
+func (b *Bonafide) SetManualGateway(label string) {
+	b.gateways.setUserChoice(label)
 }
 
-func (b *Bonafide) requestBestGatewaysFromService() ([]string, error) {
+func (b *Bonafide) SetAutomaticGateway() {
+	b.gateways.setAutomaticChoice()
+}
+
+/* TODO this still needs to be called periodically */
+func (b *Bonafide) fetchGatewayRanking() error {
 	/* FIXME in float deployments, geolocation is served on gemyip.domain/json, with a LE certificate, but in riseup is served behind the api certificate.
 	So this is a workaround until we streamline that behavior */
 	resp, err := b.client.Post(config.GeolocationAPI, "", nil)
@@ -218,7 +229,7 @@ func (b *Bonafide) requestBestGatewaysFromService() ([]string, error) {
 		_resp, err := client.Post(config.GeolocationAPI, "", nil)
 		if err != nil {
 			log.Printf("ERROR: could not fetch geolocation: %s\n", err)
-			return nil, err
+			return err
 		}
 		resp = _resp
 	}
@@ -226,7 +237,7 @@ func (b *Bonafide) requestBestGatewaysFromService() ([]string, error) {
 	defer resp.Body.Close()
 	if resp.StatusCode != 200 {
 		log.Println("ERROR: bad status code while fetching geolocation:", resp.StatusCode)
-		return nil, fmt.Errorf("Get geolocation failed with status: %d", resp.StatusCode)
+		return fmt.Errorf("Get geolocation failed with status: %d", resp.StatusCode)
 	}
 
 	geo := &geoLocation{}
@@ -236,31 +247,18 @@ func (b *Bonafide) requestBestGatewaysFromService() ([]string, error) {
 		log.Printf("ERROR: cannot parse geolocation json: %s\n", err)
 		log.Println(string(dataJSON))
 		_ = fmt.Errorf("bad json")
-		return nil, err
+		return err
 	}
 
 	log.Println("Got sorted gateways:", geo.SortedGateways)
-	return geo.SortedGateways, nil
-
-}
-
-func (b *Bonafide) sortGateways() {
-	serviceSelection, _ := b.requestBestGatewaysFromService()
-
-	if len(serviceSelection) > 0 {
-		b.eip.autoSortGateways(serviceSelection)
-	} else {
-		log.Printf("Falling back to timezone heuristic for gateway selection")
-		b.eip.sortGatewaysByTimezone(b.tzOffsetHours)
-	}
+	b.gateways.setRanking(geo.SortedGateways)
+	return nil
 }
 
 func (b *Bonafide) GetOpenvpnArgs() ([]string, error) {
-	if b.eip == nil {
-		err := b.fetchEipJSON()
-		if err != nil {
-			return nil, err
-		}
+	err := b.maybeInitializeEIP()
+	if err != nil {
+		return nil, err
 	}
 	return b.eip.getOpenvpnArgs(), nil
 }
diff --git a/pkg/vpn/bonafide/bonafide_test.go b/pkg/vpn/bonafide/bonafide_test.go
index 481c079..5a6a220 100644
--- a/pkg/vpn/bonafide/bonafide_test.go
+++ b/pkg/vpn/bonafide/bonafide_test.go
@@ -1,4 +1,4 @@
-// Copyright (C) 2018 LEAP
+// Copyright (C) 2018-2020 LEAP
 //
 // This program is free software: you can redistribute it and/or modify
 // it under the terms of the GNU General Public License as published by
@@ -18,6 +18,7 @@ package bonafide
 import (
 	"io"
 	"io/ioutil"
+	"log"
 	"net/http"
 	"os"
 	"reflect"
@@ -40,12 +41,13 @@ type nopCloser struct {
 
 func (nopCloser) Close() error { return nil }
 
-type client struct {
+type mockClient struct {
 	path string
 	geo  string
 }
 
-func (c client) Post(url, contentType string, body io.Reader) (resp *http.Response, err error) {
+func (c mockClient) Post(url, contentType string, body io.Reader) (resp *http.Response, err error) {
+	/* FIXME - should get the mocked geolocation configured too */
 	if strings.Contains(url, "api.black.riseup.net:9001/json") {
 		f, err := os.Open(c.geo)
 		return &http.Response{
@@ -61,7 +63,7 @@ func (c client) Post(url, contentType string, body io.Reader) (resp *http.Respon
 	}
 }
 
-func (c client) Do(req *http.Request) (*http.Response, error) {
+func (c mockClient) Do(req *http.Request) (*http.Response, error) {
 	f, err := os.Open(c.path)
 	return &http.Response{
 		Body:       f,
@@ -70,7 +72,7 @@ func (c client) Do(req *http.Request) (*http.Response, error) {
 }
 
 func TestAnonGetCert(t *testing.T) {
-	b := Bonafide{client: client{certPath, geoPath}}
+	b := Bonafide{client: mockClient{certPath, geoPath}}
 	b.auth = &anonymousAuthentication{}
 	cert, err := b.GetPemCertificate()
 	if err != nil {
@@ -109,9 +111,10 @@ func TestGatewayTzLocation(t *testing.T) {
 
 	for tzOffset, location := range values {
 		b := Bonafide{
-			client:        client{eipPath, geoPath},
+			client:        mockClient{eipPath, geoPath},
 			tzOffsetHours: tzOffset,
 		}
+		b.maxGateways = 99
 		gateways, err := b.GetGateways("openvpn")
 
 		if err != nil {
@@ -119,9 +122,8 @@ func TestGatewayTzLocation(t *testing.T) {
 			continue
 		}
 		if len(gateways) < 4 {
-			t.Errorf("Wrong number of gateways: %d", len(gateways))
-			continue
-
+			t.Errorf("Wrong number of gateways for tz %d: %d", tzOffset, len(gateways))
+			t.Fatal("aborting")
 		}
 		if gateways[0].Location != location {
 			t.Errorf("Wrong location for tz %d: %s, expected: %s", tzOffset, gateways[0].Location, location)
@@ -131,21 +133,22 @@ func TestGatewayTzLocation(t *testing.T) {
 
 func TestOpenvpnGateways(t *testing.T) {
 	b := Bonafide{
-		client: client{eipPath, geoPath},
+		client: mockClient{eipPath, geoPath},
 	}
+	b.maxGateways = 10
 	gateways, err := b.GetGateways("openvpn")
 	if err != nil {
 		t.Fatalf("getGateways returned an error: %v", err)
 	}
 	if len(gateways) == 0 {
-		t.Fatalf("No obfs4 gateways found")
+		t.Fatalf("No openvpn gateways found")
 	}
 
 	present := make([]bool, 6)
 	for _, g := range gateways {
 		i, err := strconv.Atoi(g.Host[0:1])
 		if err != nil {
-			t.Fatalf("unkonwn host %s: %v", g.Host, err)
+			t.Fatalf("unkown host %s: %v", g.Host, err)
 		}
 		present[i] = true
 	}
@@ -155,11 +158,12 @@ func TestOpenvpnGateways(t *testing.T) {
 			continue
 		case 5:
 			if p {
-				t.Errorf("Host %d should not have obfs4 transport", i)
+				t.Errorf("Host %d should not have openvpn transport", i)
 			}
 		default:
 			if !p {
-				t.Errorf("Host %d should have obfs4 transport", i)
+				log.Println(">> present", present)
+				t.Errorf("Host %d should have openvpn transport", i)
 			}
 		}
 	}
@@ -167,8 +171,9 @@ func TestOpenvpnGateways(t *testing.T) {
 
 func TestObfs4Gateways(t *testing.T) {
 	b := Bonafide{
-		client: client{eipPath, geoPath},
+		client: mockClient{eipPath, geoPath},
 	}
+	b.maxGateways = 10
 	gateways, err := b.GetGateways("obfs4")
 	if err != nil {
 		t.Fatalf("getGateways returned an error: %v", err)
@@ -237,6 +242,7 @@ func TestEipServiceV1Fallback(t *testing.T) {
 	b := Bonafide{
 		client: failingClient{eip1Path},
 	}
+	b.maxGateways = 10
 	gateways, err := b.GetGateways("obfs4")
 	if err != nil {
 		t.Fatalf("getGateways obfs4 returned an error: %v", err)
@@ -250,6 +256,6 @@ func TestEipServiceV1Fallback(t *testing.T) {
 		t.Fatalf("getGateways openvpn returned an error: %v", err)
 	}
 	if len(gateways) != 4 {
-		t.Fatalf("It not right number of gateways: %v", gateways)
+		t.Fatalf("Got wrong number of gateways: %v", gateways)
 	}
 }
diff --git a/pkg/vpn/bonafide/eip_service.go b/pkg/vpn/bonafide/eip_service.go
index 9c8dc66..26a8f3c 100644
--- a/pkg/vpn/bonafide/eip_service.go
+++ b/pkg/vpn/bonafide/eip_service.go
@@ -5,9 +5,6 @@ import (
 	"fmt"
 	"io"
 	"log"
-	"math/rand"
-	"sort"
-	"strconv"
 	"strings"
 	"time"
 
@@ -16,27 +13,19 @@ import (
 
 type eipService struct {
 	Gateways             []gatewayV3
-	SelectedGateways     []gatewayV3
-	Locations            map[string]location
 	defaultGateway       string
+	Locations            map[string]location
 	OpenvpnConfiguration openvpnConfig `json:"openvpn_configuration"`
 	auth                 string
 }
 
 type eipServiceV1 struct {
 	Gateways             []gatewayV1
-	SelectedGateways     []gatewayV1
+	defaultGateway       string
 	Locations            map[string]location
 	OpenvpnConfiguration openvpnConfig `json:"openvpn_configuration"`
 }
 
-type location struct {
-	CountryCode string
-	Hemisphere  string
-	Name        string
-	Timezone    string
-}
-
 type gatewayV1 struct {
 	Capabilities struct {
 		Ports     []string
@@ -56,6 +45,13 @@ type gatewayV3 struct {
 	Location  string
 }
 
+type location struct {
+	CountryCode string
+	Hemisphere  string
+	Name        string
+	Timezone    string
+}
+
 type transportV3 struct {
 	Type      string
 	Protocols []string
@@ -84,6 +80,7 @@ func (b *Bonafide) fetchEipJSON() error {
 	resp, err := b.client.Post(eip3API, "", nil)
 	for err != nil {
 		log.Printf("Error fetching eip v3 json: %v", err)
+		// TODO why exactly 1 retry? Make it configurable, for tests
 		time.Sleep(retryFetchJSONSeconds * time.Second)
 		resp, err = b.client.Post(eip3API, "", nil)
 	}
@@ -115,10 +112,6 @@ func (b *Bonafide) fetchEipJSON() error {
 	}
 
 	b.setupAuthentication(b.eip)
-	/* TODO we could launch the looping call from here.
-	but smells: calls a bonafide method that in turn calls methods in this file
-	*/
-	b.sortGateways()
 	return nil
 }
 
@@ -161,15 +154,10 @@ func decodeEIP1(body io.Reader) (*eipService, error) {
 	return &eip3, nil
 }
 
-func (eip eipService) getGateways(transport string) []Gateway {
+func (eip eipService) getGateways() []Gateway {
 	gws := []Gateway{}
-	// TODO check that len(selected) != 0
-	for _, g := range eip.SelectedGateways {
+	for _, g := range eip.Gateways {
 		for _, t := range g.Capabilities.Transport {
-			if t.Type != transport {
-				continue
-			}
-
 			gateway := Gateway{
 				Host:      g.Host,
 				IPAddress: g.IPAddress,
@@ -177,80 +165,14 @@ func (eip eipService) getGateways(transport string) []Gateway {
 				Ports:     t.Ports,
 				Protocols: t.Protocols,
 				Options:   t.Options,
+				Transport: t.Type,
 			}
 			gws = append(gws, gateway)
 		}
 	}
-	// TODO return only top 3, at least for openvpn
 	return gws
 }
 
-func (eip *eipService) setManualGateway(name string) {
-	eip.defaultGateway = name
-
-	gws := make([]gatewayV3, 0)
-	for _, gw := range eip.Gateways {
-		if gw.Location == eip.defaultGateway {
-			gws = append(gws, gw)
-			break
-		}
-	}
-	eip.SelectedGateways = gws
-}
-
-func (eip *eipService) autoSortGateways(serviceSelection []string) {
-	gws := make([]gatewayV3, 0)
-
-	for _, host := range serviceSelection {
-		for _, gw := range eip.Gateways {
-			if gw.Host == host {
-				gws = append(gws, gw)
-			}
-		}
-	}
-
-	if len(gws) == 0 {
-		// this can happen if a misconfigured geoip service does not match the
-		// providers list we got.
-		log.Println("ERROR: did not get any useful selection. Is the geolocation service properly configured?")
-		eip.SelectedGateways = eip.Gateways
-	} else {
-		eip.SelectedGateways = gws
-	}
-}
-
-func (eip *eipService) sortGatewaysByTimezone(tzOffsetHours int) {
-	gws := []gatewayDistance{}
-
-	for _, gw := range eip.Gateways {
-		distance := 13
-		if gw.Location == eip.defaultGateway {
-			distance = -1
-		} else {
-			gwOffset, err := strconv.Atoi(eip.Locations[gw.Location].Timezone)
-			if err != nil {
-				log.Printf("Error sorting gateways: %v", err)
-			} else {
-				distance = tzDistance(tzOffsetHours, gwOffset)
-			}
-		}
-		gws = append(gws, gatewayDistance{gw, distance})
-	}
-	rand.Seed(time.Now().UnixNano())
-	cmp := func(i, j int) bool {
-		if gws[i].distance == gws[j].distance {
-			return rand.Intn(2) == 1
-		}
-		return gws[i].distance < gws[j].distance
-	}
-	sort.Slice(gws, cmp)
-
-	eip.SelectedGateways = make([]gatewayV3, len(eip.Gateways))
-	for i, gw := range gws {
-		eip.SelectedGateways[i] = gw.gateway
-	}
-}
-
 func (eip eipService) getOpenvpnArgs() []string {
 	args := []string{}
 	for arg, value := range eip.OpenvpnConfiguration {
@@ -268,22 +190,3 @@ func (eip eipService) getOpenvpnArgs() []string {
 	}
 	return args
 }
-
-type gatewayDistance struct {
-	gateway  gatewayV3
-	distance int
-}
-
-func tzDistance(offset1, offset2 int) int {
-	abs := func(x int) int {
-		if x < 0 {
-			return -x
-		}
-		return x
-	}
-	distance := abs(offset1 - offset2)
-	if distance > 12 {
-		distance = 24 - distance
-	}
-	return distance
-}
diff --git a/pkg/vpn/bonafide/gateways.go b/pkg/vpn/bonafide/gateways.go
new file mode 100644
index 0000000..6084985
--- /dev/null
+++ b/pkg/vpn/bonafide/gateways.go
@@ -0,0 +1,229 @@
+package bonafide
+
+import (
+	"errors"
+	"log"
+	"math/rand"
+	"sort"
+	"strconv"
+	"time"
+)
+
+const (
+	maxGateways = 3
+)
+
+// A Gateway is a representation of gateways that is independent of the api version.
+// If a given physical location offers different transports, they will appear as separate gateways.
+type Gateway struct {
+	Host      string
+	IPAddress string
+	Location  string
+	Ports     []string
+	Protocols []string
+	Options   map[string]string
+	Transport string
+	Label     string
+}
+
+/* TODO add a String method with a human representation: Label (cc) */
+/* For that, we should pass the locations to genLabels, and generate a string repr */
+
+type gatewayDistance struct {
+	gateway  Gateway
+	distance int
+}
+
+type gatewayPool struct {
+	available []Gateway
+	/* ranked is, for now, just an array of hostnames (fetched from the
+	geoip service). it should be a map in the future, to keep track of
+	quantitative metrics */
+	ranked     []string
+	userChoice string
+	locations  map[string]location
+}
+
+/* genLabels generates unique, human-readable labels for a gateway. It gives a serial
+   number to each gateway in the same location (paris-1, paris-2,...). The
+   current implementation will give a different label to each transport.
+*/
+func (p *gatewayPool) genLabels() {
+	acc := make(map[string]int)
+	for i, gw := range p.available {
+		if _, count := acc[gw.Location]; !count {
+			acc[gw.Location] = 1
+		} else {
+			acc[gw.Location] += 1
+		}
+		gw.Label = gw.Location + "-" + strconv.Itoa(acc[gw.Location])
+		p.available[i] = gw
+	}
+	/* skip suffix if only one occurence */
+	for i, gw := range p.available {
+		if acc[gw.Location] == 1 {
+			gw.Label = gw.Location
+			p.available[i] = gw
+		}
+	}
+}
+
+func (p *gatewayPool) getLabels() []string {
+	labels := make([]string, 0)
+	for _, gw := range p.available {
+		labels = append(labels, gw.Label)
+	}
+	/* TODO return error if called when no labels have been generated */
+	return labels
+}
+
+func (p *gatewayPool) isValidLabel(label string) bool {
+	labels := p.getLabels()
+	valid := stringInSlice(label, labels)
+	return valid
+}
+
+func (p *gatewayPool) getGatewayByLabel(label string) (Gateway, error) {
+	for _, gw := range p.available {
+		if gw.Label == label {
+			return gw, nil
+		}
+	}
+	return Gateway{}, errors.New("bonafide: not a valid label")
+}
+
+func (p *gatewayPool) getGatewayByIP(ip string) (Gateway, error) {
+	for _, gw := range p.available {
+		if gw.IPAddress == ip {
+			return gw, nil
+		}
+	}
+	return Gateway{}, errors.New("bonafide: not a valid ip address")
+}
+
+func (p *gatewayPool) setAutomaticChoice() {
+	p.userChoice = ""
+}
+
+func (p *gatewayPool) setUserChoice(label string) error {
+	if !p.isValidLabel(label) {
+		return errors.New("bonafide: not a valid label for gateway choice")
+	}
+	p.userChoice = label
+	return nil
+}
+
+func (p *gatewayPool) setRanking(hostnames []string) {
+	hosts := make([]string, 0)
+	for _, gw := range p.available {
+		hosts = append(hosts, gw.Host)
+	}
+
+	for _, host := range hostnames {
+		if !stringInSlice(host, hosts) {
+			log.Println("ERROR: invalid host in ranked hostnames", host)
+			return
+		}
+	}
+
+	p.ranked = hostnames
+}
+
+func (p *gatewayPool) getBest(transport string, tz, max int) ([]Gateway, error) {
+	gws := make([]Gateway, 0)
+	if len(p.userChoice) != 0 {
+		gw, err := p.getGatewayByLabel(p.userChoice)
+		gws = append(gws, gw)
+		return gws, err
+	} else if len(p.ranked) != 0 {
+		return p.getGatewaysByServiceRank(transport, max)
+	} else {
+		return p.getGatewaysByTimezone(transport, tz, max)
+	}
+}
+
+func (p *gatewayPool) getGatewaysByServiceRank(transport string, max int) ([]Gateway, error) {
+	gws := make([]Gateway, 0)
+	for _, host := range p.ranked {
+		for _, gw := range p.available {
+			if gw.Transport != transport {
+				continue
+			}
+			if gw.Host == host {
+				gws = append(gws, gw)
+			}
+			if len(gws) == max {
+				goto end
+			}
+		}
+	}
+end:
+	return gws, nil
+}
+
+func (p *gatewayPool) getGatewaysByTimezone(transport string, tzOffsetHours, max int) ([]Gateway, error) {
+	gws := make([]Gateway, 0)
+	gwVector := []gatewayDistance{}
+
+	for _, gw := range p.available {
+		if gw.Transport != transport {
+			continue
+		}
+		distance := 13
+		gwOffset, err := strconv.Atoi(p.locations[gw.Location].Timezone)
+		if err != nil {
+			log.Printf("Error sorting gateways: %v", err)
+			return gws, err
+		} else {
+			distance = tzDistance(tzOffsetHours, gwOffset)
+		}
+		gwVector = append(gwVector, gatewayDistance{gw, distance})
+	}
+	rand.Seed(time.Now().UnixNano())
+	cmp := func(i, j int) bool {
+		if gwVector[i].distance == gwVector[j].distance {
+			return rand.Intn(2) == 1
+		}
+		return gwVector[i].distance < gwVector[j].distance
+	}
+	sort.Slice(gwVector, cmp)
+
+	for _, gw := range gwVector {
+		gws = append(gws, gw.gateway)
+		if len(gws) == max {
+			break
+		}
+	}
+	return gws, nil
+}
+
+func newGatewayPool(eip *eipService) *gatewayPool {
+	p := gatewayPool{}
+	p.available = eip.getGateways()
+	p.locations = eip.Locations
+	p.genLabels()
+	return &p
+}
+
+func tzDistance(offset1, offset2 int) int {
+	abs := func(x int) int {
+		if x < 0 {
+			return -x
+		}
+		return x
+	}
+	distance := abs(offset1 - offset2)
+	if distance > 12 {
+		distance = 24 - distance
+	}
+	return distance
+}
+
+func stringInSlice(a string, list []string) bool {
+	for _, b := range list {
+		if b == a {
+			return true
+		}
+	}
+	return false
+}
diff --git a/pkg/vpn/bonafide/gateways_test.go b/pkg/vpn/bonafide/gateways_test.go
new file mode 100644
index 0000000..b88423f
--- /dev/null
+++ b/pkg/vpn/bonafide/gateways_test.go
@@ -0,0 +1,85 @@
+package bonafide
+
+import (
+	"reflect"
+	"sort"
+	"testing"
+)
+
+const (
+	eipGwTestPath = "testdata/eip-service3.json"
+)
+
+func TestGatewayPool(t *testing.T) {
+	b := Bonafide{client: mockClient{eipGwTestPath, geoPath}}
+	err := b.fetchEipJSON()
+	if err != nil {
+		t.Fatal("fetchEipJSON returned an error: ", err)
+	}
+
+	g := gatewayPool{available: b.eip.getGateways()}
+	if len(g.available) != 7 {
+		/* just to check that the dataset has not changed */
+		t.Fatal("Expected 7 initial gateways, got", len(g.available))
+	}
+
+	/* now we initialize a pool the proper way */
+	pool := newGatewayPool(b.eip)
+	if len(pool.available) != 7 {
+		t.Fatal("Expected 7 initial gateways, got", len(g.available))
+	}
+	expectedLabels := []string{"a-1", "a-2", "b-1", "b-2", "b-3", "c-1", "c-2"}
+	sort.Strings(expectedLabels)
+
+	labels := pool.getLabels()
+	sort.Strings(labels)
+	if !reflect.DeepEqual(expectedLabels, labels) {
+		t.Fatal("gatewayPool labels not what expected. Got:", labels)
+	}
+
+	if pool.userChoice != "" {
+		t.Fatal("userChoice should be empty by default")
+	}
+
+	err = pool.setUserChoice("foo")
+	if err == nil {
+		t.Fatal("gatewayPool should not let you set a foo gateway")
+	}
+	err = pool.setUserChoice("a-1")
+	if err != nil {
+		t.Fatal("location 'a-1' should be a valid label")
+	}
+	err = pool.setUserChoice("c-2")
+	if err != nil {
+		t.Fatal("location 'c-2' should be a valid label")
+	}
+	if pool.userChoice != "c-2" {
+		t.Fatal("userChoice should be c-2")
+	}
+
+	pool.setAutomaticChoice()
+	if pool.userChoice != "" {
+		t.Fatal("userChoice should be empty after auto selection")
+	}
+
+	gw, err := pool.getGatewayByLabel("foo")
+	if err == nil {
+		t.Fatal("should get an error with invalid label")
+	}
+
+	gw, err = pool.getGatewayByLabel("a-1")
+	if gw.IPAddress != "1.1.1.1" {
+		t.Fatal("expected to get gw 1.1.1.1 with label a-1")
+	}
+
+	gw, err = pool.getGatewayByIP("1.1.1.1")
+	if err != nil {
+		t.Fatal("expected to get gw a with ip 1.1.1.1")
+	}
+	if gw.Host != "1.example.com" {
+		t.Fatal("expected to get gw 1.example.com with ip 1.1.1.1")
+	}
+
+	// TODO test getBest
+
+}