add docs, simplify function, some marginal perf gain

ooni · Oct 24, 2024 · 78ac661 · 78ac661
1 parent bcab63d
commit 78ac661
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 19 deletions.
diff --git a/go.mod b/go.mod
@@ -26,7 +26,7 @@ require (
 	github.com/miekg/dns v1.1.59
 	github.com/mitchellh/go-wordwrap v1.0.1
 	github.com/montanaflynn/stats v0.7.1
-	github.com/ooni/minivpn v0.0.6
+	github.com/ooni/minivpn v0.0.7
 	github.com/ooni/netem v0.0.0-20240208095707-608dcbcd82b8
 	github.com/ooni/oocrypto v0.6.2
 	github.com/ooni/oohttp v0.7.3

diff --git a/internal/experiment/openvpn/richerinput.go b/internal/experiment/openvpn/richerinput.go
@@ -77,6 +77,9 @@ type targetLoader struct {
 }
 
 // Load implements model.ExperimentTargetLoader.
+// Returning an empty ExperimentTarget slice here is equivalent to not
+// passing any input to the experiment; in this case the `openvpn` experiment
+// just does not probe any endpoint (no-op).
 func (tl *targetLoader) Load(ctx context.Context) ([]model.ExperimentTarget, error) {
 	// First, attempt to load the static inputs from CLI and files
 	inputs, err := targetloading.LoadStatic(tl.loader)
@@ -97,8 +100,17 @@ func (tl *targetLoader) Load(ctx context.Context) ([]model.ExperimentTarget, err
 		return targets, nil
 	}
 
-	// Return the hardcoded endpoints.
-	return tl.loadFromDefaultEndpoints()
+	// As a fallback (no backend, no files, no input from cli)
+	// return the hardcoded endpoints.
+	targets, err = tl.loadFromDefaultEndpoints()
+	if err != nil {
+		tl.loader.Logger.Warnf("Error loading default endpoints: %v", err)
+		return targets, nil
+	}
+	if len(targets) == 0 {
+		tl.loader.Logger.Warnf("No targets loaded from default endpoints")
+	}
+	return targets, nil
 }
 
 func (tl *targetLoader) loadFromDefaultEndpoints() ([]model.ExperimentTarget, error) {

diff --git a/internal/experiment/openvpn/targets.go b/internal/experiment/openvpn/targets.go
@@ -31,27 +31,22 @@ func sampleN(a []string, n int) []string {
 	if n > len(a) {
 		n = len(a)
 	}
-
-	sampled := make([]string, 0)
-
-	// Use a map to track indices we've already selected to avoid duplicates
-	picked := make(map[int]struct{})
-
-	for len(sampled) < n {
-		idx := rand.Intn(len(a)) // Random index
-		if _, exists := picked[idx]; !exists {
-			sampled = append(sampled, a[idx])
-			picked[idx] = struct{}{} // Mark index as used
-		}
-	}
-
-	return sampled
+	rand.Shuffle(len(a), func(i, j int) {
+		a[i], a[j] = a[j], a[i]
+	})
+	return a[:n]
 }
 
 // resolveOONIAddresses returns a max of maxDefaultOONIAddresses after
 // performing DNS resolution. The returned IP addreses exclude possible
 // bogons.
 func resolveOONIAddresses(logger model.Logger) ([]string, error) {
+
+	// We explicitely resolve with BogonIsError set to false, and
+	// later remove bogons from the list. The reason is that in this way
+	// we are able to control the rate at which we run tests by adding bogon addresses to the
+	// domain records for the test.
+
 	resolver := netx.NewResolver(netx.Config{
 		BogonIsError: false,
 		Logger:       logger,
@@ -62,7 +57,8 @@ func resolveOONIAddresses(logger model.Logger) ([]string, error) {
 
 	var lastErr error
 
-	// get the set of all IPs for all the hostnames we have.
+	// Get the set of all IPs for all the hostnames we have.
+
 	for _, hostname := range defaultOONIEndpoints {
 		resolved, err := lookupHost(context.Background(), hostname, resolver)
 		if err != nil {
@@ -107,6 +103,9 @@ func lookupHost(ctx context.Context, hostname string, r model.Resolver) ([]strin
 
 // pickOONIOpenVPNTargets crafts targets from the passed array of IP addresses.
 func pickOONIOpenVPNTargets(ipaddrList []string) ([]string, error) {
+	if len(ipaddrList) == 0 {
+		return []string{}, nil
+	}
 
 	// Step 1. Create endpoint list.
 

diff --git a/internal/experiment/openvpn/targets_test.go b/internal/experiment/openvpn/targets_test.go
@@ -75,3 +75,33 @@ func Test_resolveOONIAddresses(t *testing.T) {
 		}
 	})
 }
+
+func Test_pickOONIOpenVPNTargets(t *testing.T) {
+	t.Run("empty ip list produces empty targets", func(t *testing.T) {
+		endpoints, err := pickOONIOpenVPNTargets([]string{})
+		if err != nil {
+			t.Fatal("expected nil error")
+		}
+		if len(endpoints) != 0 {
+			t.Fatal("expected empty endpoints")
+		}
+	})
+	t.Run("single-item ip list produces valid targets", func(t *testing.T) {
+		endpoints, err := pickOONIOpenVPNTargets([]string{"1.1.1.1"})
+		if err != nil {
+			t.Fatal("expected nil error")
+		}
+		if len(endpoints) != 4 {
+			t.Fatalf("expected 4 endpoints, got %d", len(endpoints))
+		}
+	})
+	t.Run("2-item ip list produces 6 targets", func(t *testing.T) {
+		endpoints, err := pickOONIOpenVPNTargets([]string{"1.1.1.1", "2.2.2.2"})
+		if err != nil {
+			t.Fatal("expected nil error")
+		}
+		if len(endpoints) != 6 {
+			t.Fatalf("expected 6 endpoints, got %d", len(endpoints))
+		}
+	})
+}