refactor(enginenetx): make static-policy API private

internal/enginenetx/beacons.go

@@ -1,5 +1,10 @@
 package enginenetx
 
+//
+// beacons policy - a policy where we treat some IP addresses as special for
+// some domains, bypassing DNS lookups and using custom SNIs
+//
+
 import (
 	"context"
 	"math/rand"

internal/enginenetx/network.go

@@ -1,5 +1,10 @@
 package enginenetx
 
+//
+// Network - the top-level object of this package, used by the
+// OONI engine to communicate with several backends
+//
+
 import (
 	"net/http"
 	"net/http/cookiejar"
@@ -141,7 +146,7 @@ func newHTTPSDialerPolicy(kvStore model.KeyValueStore, logger model.Logger, reso
 	}
 
 	// make sure we honor a user-provided policy
-	policy, err := NewHTTPSDialerStaticPolicy(kvStore, fallback)
+	policy, err := newStaticPolicy(kvStore, fallback)
 	if err != nil {
 		return fallback
 	}

internal/enginenetx/network_internal_test.go

@@ -1,12 +1,20 @@
 package enginenetx
 
 import (
+	"context"
+	"encoding/json"
 	"sync"
 	"testing"
 
+	"github.com/apex/log"
+	"github.com/google/go-cmp/cmp"
+	"github.com/ooni/probe-cli/v3/internal/bytecounter"
 	"github.com/ooni/probe-cli/v3/internal/kvstore"
 	"github.com/ooni/probe-cli/v3/internal/mocks"
 	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/netemx"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
+	"github.com/ooni/probe-cli/v3/internal/runtimex"
 )
 
 func TestNetworkUnit(t *testing.T) {
@@ -75,4 +83,88 @@ func TestNetworkUnit(t *testing.T) {
 			t.Fatal("did not call the transport's CloseIdleConnections")
 		}
 	})
+
+	t.Run("NewNetwork uses the correct HTTPSDialerPolicy", func(t *testing.T) {
+		// testcase is a test case run by this func
+		type testcase struct {
+			name         string
+			kvStore      func() model.KeyValueStore
+			expectStatus int
+			expectBody   []byte
+		}
+
+		cases := []testcase{
+			// Without a policy accessing www.example.com should lead to 200 as status
+			// code and the expected web page when we're using netem
+			{
+				name: "when there is no user-provided policy",
+				kvStore: func() model.KeyValueStore {
+					return &kvstore.Memory{}
+				},
+				expectStatus: 200,
+				expectBody:   []byte(netemx.ExampleWebPage),
+			},
+
+			// But we can create a policy that can land us on a different website (not the
+			// typical use case of the policy, but definitely demonstrating it works)
+			{
+				name: "when there's a user-provided policy",
+				kvStore: func() model.KeyValueStore {
+					policy := &staticPolicyRoot{
+						DomainEndpoints: map[string][]*HTTPSDialerTactic{
+							"www.example.com:443": {{
+								Address:        netemx.AddressApiOONIIo,
+								InitialDelay:   0,
+								Port:           "443",
+								SNI:            "www.example.com",
+								VerifyHostname: "api.ooni.io",
+							}},
+						},
+						Version: staticPolicyVersion,
+					}
+					rawPolicy := runtimex.Try1(json.Marshal(policy))
+					kvStore := &kvstore.Memory{}
+					runtimex.Try0(kvStore.Set(staticPolicyKey, rawPolicy))
+					return kvStore
+				},
+				expectStatus: 404,
+				expectBody:   []byte{},
+			},
+		}
+
+		for _, tc := range cases {
+			t.Run(tc.name, func(t *testing.T) {
+				env := netemx.MustNewScenario(netemx.InternetScenario)
+				defer env.Close()
+
+				env.Do(func() {
+					netx := NewNetwork(
+						bytecounter.New(),
+						tc.kvStore(),
+						log.Log,
+						nil, // proxy URL
+						netxlite.NewStdlibResolver(log.Log),
+					)
+					defer netx.Close()
+
+					client := netx.NewHTTPClient()
+					resp, err := client.Get("https://www.example.com/")
+					if err != nil {
+						t.Fatal(err)
+					}
+					defer resp.Body.Close()
+					if resp.StatusCode != tc.expectStatus {
+						t.Fatal("StatusCode: expected", tc.expectStatus, "got", resp.StatusCode)
+					}
+					data, err := netxlite.ReadAllContext(context.Background(), resp.Body)
+					if err != nil {
+						t.Fatal(err)
+					}
+					if diff := cmp.Diff(tc.expectBody, data); diff != "" {
+						t.Fatal(diff)
+					}
+				})
+			})
+		}
+	})
 }

internal/enginenetx/network_test.go

@@ -2,23 +2,19 @@ package enginenetx_test
 
 import (
 	"context"
-	"encoding/json"
 	"net"
 	"net/http"
 	"net/url"
 	"testing"
 	"time"
 
 	"github.com/apex/log"
-	"github.com/google/go-cmp/cmp"
 	"github.com/ooni/probe-cli/v3/internal/bytecounter"
 	"github.com/ooni/probe-cli/v3/internal/enginenetx"
 	"github.com/ooni/probe-cli/v3/internal/kvstore"
 	"github.com/ooni/probe-cli/v3/internal/measurexlite"
-	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/netemx"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
-	"github.com/ooni/probe-cli/v3/internal/runtimex"
 	"github.com/ooni/probe-cli/v3/internal/testingsocks5"
 	"github.com/ooni/probe-cli/v3/internal/testingx"
 )
@@ -273,88 +269,4 @@ func TestNetworkQA(t *testing.T) {
 			t.Fatal("expected non-nil cookie jar")
 		}
 	})
-
-	t.Run("NewNetwork uses the correct HTTPSDialerPolicy", func(t *testing.T) {
-		// testcase is a test case run by this func
-		type testcase struct {
-			name         string
-			kvStore      func() model.KeyValueStore
-			expectStatus int
-			expectBody   []byte
-		}
-
-		cases := []testcase{
-			// Without a policy accessing www.example.com should lead to 200 as status
-			// code and the expected web page when we're using netem
-			{
-				name: "when there is no user-provided policy",
-				kvStore: func() model.KeyValueStore {
-					return &kvstore.Memory{}
-				},
-				expectStatus: 200,
-				expectBody:   []byte(netemx.ExampleWebPage),
-			},
-
-			// But we can create a policy that can land us on a different website (not the
-			// typical use case of the policy, but definitely demonstrating it works)
-			{
-				name: "when there's a user-provided policy",
-				kvStore: func() model.KeyValueStore {
-					policy := &enginenetx.HTTPSDialerStaticPolicyRoot{
-						DomainEndpoints: map[string][]*enginenetx.HTTPSDialerTactic{
-							"www.example.com:443": {{
-								Address:        netemx.AddressApiOONIIo,
-								InitialDelay:   0,
-								Port:           "443",
-								SNI:            "www.example.com",
-								VerifyHostname: "api.ooni.io",
-							}},
-						},
-						Version: enginenetx.HTTPSDialerStaticPolicyVersion,
-					}
-					rawPolicy := runtimex.Try1(json.Marshal(policy))
-					kvStore := &kvstore.Memory{}
-					runtimex.Try0(kvStore.Set(enginenetx.HTTPSDialerStaticPolicyKey, rawPolicy))
-					return kvStore
-				},
-				expectStatus: 404,
-				expectBody:   []byte{},
-			},
-		}
-
-		for _, tc := range cases {
-			t.Run(tc.name, func(t *testing.T) {
-				env := netemx.MustNewScenario(netemx.InternetScenario)
-				defer env.Close()
-
-				env.Do(func() {
-					netx := enginenetx.NewNetwork(
-						bytecounter.New(),
-						tc.kvStore(),
-						log.Log,
-						nil, // proxy URL
-						netxlite.NewStdlibResolver(log.Log),
-					)
-					defer netx.Close()
-
-					client := netx.NewHTTPClient()
-					resp, err := client.Get("https://www.example.com/")
-					if err != nil {
-						t.Fatal(err)
-					}
-					defer resp.Body.Close()
-					if resp.StatusCode != tc.expectStatus {
-						t.Fatal("StatusCode: expected", tc.expectStatus, "got", resp.StatusCode)
-					}
-					data, err := netxlite.ReadAllContext(context.Background(), resp.Body)
-					if err != nil {
-						t.Fatal(err)
-					}
-					if diff := cmp.Diff(tc.expectBody, data); diff != "" {
-						t.Fatal(diff)
-					}
-				})
-			})
-		}
-	})
 }

internal/enginenetx/httpsdialerstatic.go → internal/enginenetx/static.go

@@ -1,5 +1,13 @@
 package enginenetx
 
+//
+// static policy - the possibility of loading a static policy from a JSON
+// document named `httpsdialerstatic.conf` in $OONI_HOME/engine that contains
+// a specific policy for TLS dialing for specific endpoints.
+//
+// This policy helps a lot with exploration and experimentation.
+//
+
 import (
 	"context"
 	"errors"
@@ -10,77 +18,77 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/model"
 )
 
-// HTTPSDialerStaticPolicy is an [HTTPSDialerPolicy] incorporating verbatim
+// staticPolicy is an [HTTPSDialerPolicy] incorporating verbatim
 // a static policy loaded from the engine's key-value store.
 //
 // This policy is very useful for exploration and experimentation.
-type HTTPSDialerStaticPolicy struct {
+type staticPolicy struct {
 	// Fallback is the fallback policy in case the static one does not
 	// contain a rule for a specific domain.
 	Fallback HTTPSDialerPolicy
 
 	// Root is the root of the statically loaded policy.
-	Root *HTTPSDialerStaticPolicyRoot
+	Root *staticPolicyRoot
 }
 
-// HTTPSDialerStaticPolicyKey is the kvstore key used to retrieve the static policy.
-const HTTPSDialerStaticPolicyKey = "httpsdialerstatic.conf"
+// staticPolicyKey is the kvstore key used to retrieve the static policy.
+const staticPolicyKey = "httpsdialerstatic.conf"
 
-// errDialerStaticPolicyWrongVersion means that the static policy document has the wrong version number.
-var errDialerStaticPolicyWrongVersion = errors.New("wrong static policy version")
+// errStaticPolicyWrongVersion means that the static policy document has the wrong version number.
+var errStaticPolicyWrongVersion = errors.New("wrong static policy version")
 
-// NewHTTPSDialerStaticPolicy attempts to constructs a static policy using a given fallback
+// newStaticPolicy attempts to constructs a static policy using a given fallback
 // policy and either returns a good policy or an error. The typical error case is the one
 // in which there's no httpsDialerStaticPolicyKey in the key-value store.
-func NewHTTPSDialerStaticPolicy(
-	kvStore model.KeyValueStore, fallback HTTPSDialerPolicy) (*HTTPSDialerStaticPolicy, error) {
+func newStaticPolicy(
+	kvStore model.KeyValueStore, fallback HTTPSDialerPolicy) (*staticPolicy, error) {
 	// attempt to read the static policy bytes from the kvstore
-	data, err := kvStore.Get(HTTPSDialerStaticPolicyKey)
+	data, err := kvStore.Get(staticPolicyKey)
 	if err != nil {
 		return nil, err
 	}
 
 	// attempt to parse the static policy using human-readable JSON
-	var root HTTPSDialerStaticPolicyRoot
+	var root staticPolicyRoot
 	if err := hujsonx.Unmarshal(data, &root); err != nil {
 		return nil, err
 	}
 
 	// make sure the version is OK
-	if root.Version != HTTPSDialerStaticPolicyVersion {
+	if root.Version != staticPolicyVersion {
 		err := fmt.Errorf(
 			"%s: %w: expected=%d got=%d",
-			HTTPSDialerStaticPolicyKey,
-			errDialerStaticPolicyWrongVersion,
-			HTTPSDialerStaticPolicyVersion,
+			staticPolicyKey,
+			errStaticPolicyWrongVersion,
+			staticPolicyVersion,
 			root.Version,
 		)
 		return nil, err
 	}
 
-	out := &HTTPSDialerStaticPolicy{
+	out := &staticPolicy{
 		Fallback: fallback,
 		Root:     &root,
 	}
 	return out, nil
 }
 
-// HTTPSDialerStaticPolicyVersion is the current version of the static policy file.
-const HTTPSDialerStaticPolicyVersion = 3
+// staticPolicyVersion is the current version of the static policy file.
+const staticPolicyVersion = 3
 
-// HTTPSDialerStaticPolicyRoot is the root of a statically loaded policy.
-type HTTPSDialerStaticPolicyRoot struct {
+// staticPolicyRoot is the root of a statically loaded policy.
+type staticPolicyRoot struct {
 	// DomainEndpoints maps each domain endpoint to its policies.
 	DomainEndpoints map[string][]*HTTPSDialerTactic
 
 	// Version is the data structure version.
 	Version int
 }
 
-var _ HTTPSDialerPolicy = &HTTPSDialerStaticPolicy{}
+var _ HTTPSDialerPolicy = &staticPolicy{}
 
 // LookupTactics implements HTTPSDialerPolicy.
-func (ldp *HTTPSDialerStaticPolicy) LookupTactics(
+func (ldp *staticPolicy) LookupTactics(
 	ctx context.Context, domain string, port string) <-chan *HTTPSDialerTactic {
 	tactics, found := ldp.Root.DomainEndpoints[net.JoinHostPort(domain, port)]
 	if !found {