From 29d92fd307eee291bc7da083c6fb9036a951d1a4 Mon Sep 17 00:00:00 2001 From: "Quentin McGaw (desktop)" Date: Sat, 28 Aug 2021 18:14:21 +0000 Subject: [PATCH] Fix: Surfshark REGION retro-compatibility --- internal/configuration/surfshark.go | 69 +++-- internal/configuration/surfshark_test.go | 305 +++++++++++++++++++++ internal/provider/surfshark/filter_test.go | 145 ++++++++++ 3 files changed, 497 insertions(+), 22 deletions(-) create mode 100644 internal/configuration/surfshark_test.go create mode 100644 internal/provider/surfshark/filter_test.go diff --git a/internal/configuration/surfshark.go b/internal/configuration/surfshark.go index d2752e23..6449b72f 100644 --- a/internal/configuration/surfshark.go +++ b/internal/configuration/surfshark.go @@ -2,6 +2,7 @@ package configuration import ( "fmt" + "strings" "github.com/qdm12/gluetun/internal/constants" "github.com/qdm12/gluetun/internal/models" @@ -35,27 +36,14 @@ func (settings *Provider) readSurfshark(r reader) (err error) { regionChoices := constants.SurfsharkRegionChoices(servers) regionChoices = append(regionChoices, constants.SurfsharkRetroLocChoices(servers)...) - regions, err := r.env.CSVInside("REGION", regionChoices) + settings.ServerSelection.Regions, err = r.env.CSVInside("REGION", regionChoices) if err != nil { return fmt.Errorf("environment variable REGION: %w", err) } // Retro compatibility // TODO remove in v4 - for i, region := range regions { - locationData, isRetro := - surfsharkConvertRetroLoc(region) - if !isRetro { - continue - } - - regions[i] = locationData.Region - settings.ServerSelection.Countries = append(settings.ServerSelection.Countries, locationData.Country) - if locationData.City != "" { // city is empty for some servers - settings.ServerSelection.Cities = append(settings.ServerSelection.Cities, locationData.City) - } - settings.ServerSelection.Hostnames = append(settings.ServerSelection.Hostnames, locationData.Hostname) - } + settings.ServerSelection = surfsharkRetroRegion(settings.ServerSelection) settings.ServerSelection.MultiHopOnly, err = r.env.YesNo("MULTIHOP_ONLY", params.Default("no")) if err != nil { @@ -65,13 +53,50 @@ func (settings *Provider) readSurfshark(r reader) (err error) { return settings.ServerSelection.OpenVPN.readProtocolOnly(r.env) } -// TODO remove in v4. -func surfsharkConvertRetroLoc(retroLoc string) ( - locationData models.SurfsharkLocationData, isRetro bool) { - for _, data := range constants.SurfsharkLocationData() { - if retroLoc == data.RetroLoc { - return data, true +func surfsharkRetroRegion(selection ServerSelection) ( + updatedSelection ServerSelection) { + locationData := constants.SurfsharkLocationData() + + retroToLocation := make(map[string]models.SurfsharkLocationData, len(locationData)) + for _, data := range locationData { + if data.RetroLoc == "" { + continue + } + retroToLocation[strings.ToLower(data.RetroLoc)] = data + } + + for i, region := range selection.Regions { + location, ok := retroToLocation[region] + if !ok { + continue + } + selection.Regions[i] = strings.ToLower(location.Region) + selection.Countries = append(selection.Countries, strings.ToLower(location.Country)) + selection.Cities = append(selection.Cities, strings.ToLower(location.City)) // even empty string + selection.Hostnames = append(selection.Hostnames, location.Hostname) + } + + selection.Regions = dedupSlice(selection.Regions) + selection.Countries = dedupSlice(selection.Countries) + selection.Cities = dedupSlice(selection.Cities) + selection.Hostnames = dedupSlice(selection.Hostnames) + + return selection +} + +func dedupSlice(slice []string) (deduped []string) { + if slice == nil { + return nil + } + + deduped = make([]string, 0, len(slice)) + seen := make(map[string]struct{}, len(slice)) + for _, s := range slice { + if _, ok := seen[s]; !ok { + seen[s] = struct{}{} + deduped = append(deduped, s) } } - return locationData, false + + return deduped } diff --git a/internal/configuration/surfshark_test.go b/internal/configuration/surfshark_test.go new file mode 100644 index 00000000..eaccd650 --- /dev/null +++ b/internal/configuration/surfshark_test.go @@ -0,0 +1,305 @@ +package configuration + +import ( + "errors" + "net" + "testing" + + "github.com/golang/mock/gomock" + "github.com/qdm12/gluetun/internal/constants" + "github.com/qdm12/gluetun/internal/models" + "github.com/qdm12/golibs/params/mock_params" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_Provider_readSurfshark(t *testing.T) { + t.Parallel() + + var errDummy = errors.New("dummy test error") + + type stringCall struct { + call bool + value string + err error + } + + type boolCall struct { + call bool + value bool + err error + } + + type sliceStringCall struct { + call bool + values []string + err error + } + + testCases := map[string]struct { + targetIP stringCall + countries sliceStringCall + cities sliceStringCall + hostnames sliceStringCall + regions sliceStringCall + multiHop boolCall + protocol stringCall + settings Provider + err error + }{ + "target IP error": { + targetIP: stringCall{call: true, value: "something", err: errDummy}, + settings: Provider{ + Name: constants.Surfshark, + }, + err: errors.New("environment variable OPENVPN_TARGET_IP: dummy test error"), + }, + "countries error": { + targetIP: stringCall{call: true}, + countries: sliceStringCall{call: true, err: errDummy}, + settings: Provider{ + Name: constants.Surfshark, + }, + err: errors.New("environment variable COUNTRY: dummy test error"), + }, + "cities error": { + targetIP: stringCall{call: true}, + countries: sliceStringCall{call: true}, + cities: sliceStringCall{call: true, err: errDummy}, + settings: Provider{ + Name: constants.Surfshark, + }, + err: errors.New("environment variable CITY: dummy test error"), + }, + "hostnames error": { + targetIP: stringCall{call: true}, + countries: sliceStringCall{call: true}, + cities: sliceStringCall{call: true}, + hostnames: sliceStringCall{call: true, err: errDummy}, + settings: Provider{ + Name: constants.Surfshark, + }, + err: errors.New("environment variable SERVER_HOSTNAME: dummy test error"), + }, + "regions error": { + targetIP: stringCall{call: true}, + countries: sliceStringCall{call: true}, + cities: sliceStringCall{call: true}, + hostnames: sliceStringCall{call: true}, + regions: sliceStringCall{call: true, err: errDummy}, + settings: Provider{ + Name: constants.Surfshark, + }, + err: errors.New("environment variable REGION: dummy test error"), + }, + "multi hop error": { + targetIP: stringCall{call: true}, + countries: sliceStringCall{call: true}, + cities: sliceStringCall{call: true}, + hostnames: sliceStringCall{call: true}, + regions: sliceStringCall{call: true}, + multiHop: boolCall{call: true, err: errDummy}, + settings: Provider{ + Name: constants.Surfshark, + }, + err: errors.New("environment variable MULTIHOP_ONLY: dummy test error"), + }, + "openvpn protocol error": { + targetIP: stringCall{call: true}, + countries: sliceStringCall{call: true}, + cities: sliceStringCall{call: true}, + hostnames: sliceStringCall{call: true}, + regions: sliceStringCall{call: true}, + multiHop: boolCall{call: true}, + protocol: stringCall{call: true, err: errDummy}, + settings: Provider{ + Name: constants.Surfshark, + }, + err: errors.New("environment variable PROTOCOL: dummy test error"), + }, + "default settings": { + targetIP: stringCall{call: true}, + countries: sliceStringCall{call: true}, + cities: sliceStringCall{call: true}, + hostnames: sliceStringCall{call: true}, + regions: sliceStringCall{call: true}, + multiHop: boolCall{call: true}, + protocol: stringCall{call: true}, + settings: Provider{ + Name: constants.Surfshark, + }, + }, + "set settings": { + targetIP: stringCall{call: true, value: "1.2.3.4"}, + countries: sliceStringCall{call: true, values: []string{"A", "B"}}, + cities: sliceStringCall{call: true, values: []string{"C", "D"}}, + regions: sliceStringCall{call: true, values: []string{ + "E", "F", "netherlands amsterdam", + }}, // Netherlands Amsterdam is for retro compatibility test + multiHop: boolCall{call: true}, + hostnames: sliceStringCall{call: true, values: []string{"E", "F"}}, + protocol: stringCall{call: true, value: constants.TCP}, + settings: Provider{ + Name: constants.Surfshark, + ServerSelection: ServerSelection{ + OpenVPN: OpenVPNSelection{ + TCP: true, + }, + TargetIP: net.IPv4(1, 2, 3, 4), + Regions: []string{"E", "F", "europe"}, + Countries: []string{"A", "B", "netherlands"}, + Cities: []string{"C", "D", "amsterdam"}, + Hostnames: []string{"E", "F", "nl-ams.prod.surfshark.com"}, + }, + }, + }, + "Netherlands Amsterdam": { + targetIP: stringCall{call: true}, + countries: sliceStringCall{call: true}, + cities: sliceStringCall{call: true}, + regions: sliceStringCall{call: true, values: []string{"netherlands amsterdam"}}, + multiHop: boolCall{call: true}, + hostnames: sliceStringCall{call: true}, + protocol: stringCall{call: true}, + settings: Provider{ + Name: constants.Surfshark, + ServerSelection: ServerSelection{ + Regions: []string{"europe"}, + Countries: []string{"netherlands"}, + Cities: []string{"amsterdam"}, + Hostnames: []string{"nl-ams.prod.surfshark.com"}, + }, + }, + }, + } + for name, testCase := range testCases { + testCase := testCase + t.Run(name, func(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + + env := mock_params.NewMockInterface(ctrl) + + servers := []models.SurfsharkServer{{Hostname: "a"}} + allServers := models.AllServers{ + Surfshark: models.SurfsharkServers{ + Servers: servers, + }, + } + + if testCase.targetIP.call { + env.EXPECT().Get("OPENVPN_TARGET_IP"). + Return(testCase.targetIP.value, testCase.targetIP.err) + } + if testCase.countries.call { + env.EXPECT().CSVInside("COUNTRY", constants.SurfsharkCountryChoices(servers)). + Return(testCase.countries.values, testCase.countries.err) + } + if testCase.cities.call { + env.EXPECT().CSVInside("CITY", constants.SurfsharkCityChoices(servers)). + Return(testCase.cities.values, testCase.cities.err) + } + if testCase.hostnames.call { + env.EXPECT().CSVInside("SERVER_HOSTNAME", constants.SurfsharkHostnameChoices(servers)). + Return(testCase.hostnames.values, testCase.hostnames.err) + } + if testCase.regions.call { + regionChoices := constants.SurfsharkRegionChoices(servers) + regionChoices = append(regionChoices, constants.SurfsharkRetroLocChoices(servers)...) + env.EXPECT().CSVInside("REGION", regionChoices). + Return(testCase.regions.values, testCase.regions.err) + } + if testCase.multiHop.call { + env.EXPECT().YesNo("MULTIHOP_ONLY", gomock.Any()). + Return(testCase.multiHop.value, testCase.multiHop.err) + } + if testCase.protocol.call { + env.EXPECT().Inside("PROTOCOL", []string{constants.TCP, constants.UDP}, gomock.Any()). + Return(testCase.protocol.value, testCase.protocol.err) + } + + r := reader{ + servers: allServers, + env: env, + } + + var settings Provider + err := settings.readSurfshark(r) + + if testCase.err != nil { + require.Error(t, err) + assert.Equal(t, testCase.err.Error(), err.Error()) + } else { + assert.NoError(t, err) + } + + assert.Equal(t, testCase.settings, settings) + }) + } +} + +func Test_surfsharkRetroRegion(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + original ServerSelection + modified ServerSelection + }{ + "empty": {}, + "1 retro region": { + original: ServerSelection{ + Regions: []string{"australia adelaide"}, + }, + modified: ServerSelection{ + Regions: []string{"asia pacific"}, + Countries: []string{"australia"}, + Cities: []string{"adelaide"}, + Hostnames: []string{"au-adl.prod.surfshark.com"}, + }, + }, + "2 overlapping retro regions": { + original: ServerSelection{ + Regions: []string{"australia adelaide", "australia melbourne"}, + }, + modified: ServerSelection{ + Regions: []string{"asia pacific"}, + Countries: []string{"australia"}, + Cities: []string{"adelaide", "melbourne"}, + Hostnames: []string{"au-adl.prod.surfshark.com", "au-mel.prod.surfshark.com"}, + }, + }, + "2 distinct retro regions": { + original: ServerSelection{ + Regions: []string{"australia adelaide", "netherlands amsterdam"}, + }, + modified: ServerSelection{ + Regions: []string{"asia pacific", "europe"}, + Countries: []string{"australia", "netherlands"}, + Cities: []string{"adelaide", "amsterdam"}, + Hostnames: []string{"au-adl.prod.surfshark.com", "nl-ams.prod.surfshark.com"}, + }, + }, + "retro region with existing region": { + // note TestRegion will be ignored in the filters downstream + original: ServerSelection{ + Regions: []string{"TestRegion", "australia adelaide"}, + }, + modified: ServerSelection{ + Regions: []string{"TestRegion", "asia pacific"}, + Countries: []string{"australia"}, + Cities: []string{"adelaide"}, + Hostnames: []string{"au-adl.prod.surfshark.com"}, + }, + }, + } + for name, testCase := range testCases { + testCase := testCase + t.Run(name, func(t *testing.T) { + t.Parallel() + + selection := surfsharkRetroRegion(testCase.original) + + assert.Equal(t, testCase.modified, selection) + }) + } +} diff --git a/internal/provider/surfshark/filter_test.go b/internal/provider/surfshark/filter_test.go new file mode 100644 index 00000000..11be62fc --- /dev/null +++ b/internal/provider/surfshark/filter_test.go @@ -0,0 +1,145 @@ +package surfshark + +import ( + "errors" + "math/rand" + "testing" + + "github.com/qdm12/gluetun/internal/configuration" + "github.com/qdm12/gluetun/internal/constants" + "github.com/qdm12/gluetun/internal/models" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_Surfshark_filterServers(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + servers []models.SurfsharkServer + selection configuration.ServerSelection + filtered []models.SurfsharkServer + err error + }{ + "no server available": { + selection: configuration.ServerSelection{ + VPN: constants.OpenVPN, + }, + err: errors.New("no server found: for VPN openvpn; protocol udp"), + }, + "no filter": { + servers: []models.SurfsharkServer{ + {Hostname: "a", UDP: true}, + {Hostname: "b", UDP: true}, + {Hostname: "c", UDP: true}, + }, + filtered: []models.SurfsharkServer{ + {Hostname: "a", UDP: true}, + {Hostname: "b", UDP: true}, + {Hostname: "c", UDP: true}, + }, + }, + "filter by region": { + selection: configuration.ServerSelection{ + Regions: []string{"b"}, + }, + servers: []models.SurfsharkServer{ + {Region: "a", UDP: true}, + {Region: "b", UDP: true}, + {Region: "c", UDP: true}, + }, + filtered: []models.SurfsharkServer{ + {Region: "b", UDP: true}, + }, + }, + "filter by country": { + selection: configuration.ServerSelection{ + Countries: []string{"b"}, + }, + servers: []models.SurfsharkServer{ + {Country: "a", UDP: true}, + {Country: "b", UDP: true}, + {Country: "c", UDP: true}, + }, + filtered: []models.SurfsharkServer{ + {Country: "b", UDP: true}, + }, + }, + "filter by city": { + selection: configuration.ServerSelection{ + Cities: []string{"b"}, + }, + servers: []models.SurfsharkServer{ + {City: "a", UDP: true}, + {City: "b", UDP: true}, + {City: "c", UDP: true}, + }, + filtered: []models.SurfsharkServer{ + {City: "b", UDP: true}, + }, + }, + "filter by hostname": { + selection: configuration.ServerSelection{ + Hostnames: []string{"b"}, + }, + servers: []models.SurfsharkServer{ + {Hostname: "a", UDP: true}, + {Hostname: "b", UDP: true}, + {Hostname: "c", UDP: true}, + }, + filtered: []models.SurfsharkServer{ + {Hostname: "b", UDP: true}, + }, + }, + "filter by protocol": { + selection: configuration.ServerSelection{ + OpenVPN: configuration.OpenVPNSelection{ + TCP: true, + }, + }, + servers: []models.SurfsharkServer{ + {Hostname: "a", UDP: true}, + {Hostname: "b", UDP: true, TCP: true}, + {Hostname: "c", UDP: true}, + }, + filtered: []models.SurfsharkServer{ + {Hostname: "b", UDP: true, TCP: true}, + }, + }, + "filter by multihop only": { + selection: configuration.ServerSelection{ + MultiHopOnly: true, + }, + servers: []models.SurfsharkServer{ + {Hostname: "a", UDP: true}, + {Hostname: "b", MultiHop: true, UDP: true}, + {Hostname: "c", UDP: true}, + }, + filtered: []models.SurfsharkServer{ + {Hostname: "b", MultiHop: true, UDP: true}, + }, + }, + } + + for name, testCase := range testCases { + testCase := testCase + t.Run(name, func(t *testing.T) { + t.Parallel() + + randSource := rand.NewSource(0) + + s := New(testCase.servers, randSource) + + servers, err := s.filterServers(testCase.selection) + + if testCase.err != nil { + require.Error(t, err) + assert.Equal(t, testCase.err.Error(), err.Error()) + } else { + assert.NoError(t, err) + } + + assert.Equal(t, testCase.filtered, servers) + }) + } +}