cleanup(probeservices): remove residual httpx.Client dependency (#1578)

This diff removes the residual httpx.Client dependency of probeservices.
While there, recognize that we were not testing for cloudfront hard
enough and add more testing for this feature. Part of
ooni/probe#2723.

internal/cmd/apitool/main.go

@@ -17,7 +17,6 @@ import (
 	"sync/atomic"
 
 	"github.com/apex/log"
-	"github.com/ooni/probe-cli/v3/internal/httpx"
 	"github.com/ooni/probe-cli/v3/internal/kvstore"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
@@ -32,16 +31,14 @@ func newclient() probeservices.Client {
 	txp := netx.NewHTTPTransportStdlib(log.Log)
 	ua := fmt.Sprintf("apitool/%s ooniprobe-engine/%s", version.Version, version.Version)
 	return probeservices.Client{
-		APIClientTemplate: httpx.APIClientTemplate{
-			BaseURL:    *backend,
-			HTTPClient: &http.Client{Transport: txp},
-			Logger:     log.Log,
-			UserAgent:  ua,
-		},
+		BaseURL:       *backend,
+		HTTPClient:    &http.Client{Transport: txp},
 		KVStore:       &kvstore.Memory{},
+		Logger:        log.Log,
 		LoginCalls:    &atomic.Int64{},
 		RegisterCalls: &atomic.Int64{},
 		StateFile:     probeservices.NewStateFile(&kvstore.Memory{}),
+		UserAgent:     ua,
 	}
 }
 

internal/probeservices/bouncer.go

@@ -23,6 +23,7 @@ func (c *Client) GetTestHelpers(ctx context.Context) (map[string][]model.OOAPISe
 	// get the response
 	return httpclientx.GetJSON[map[string][]model.OOAPIService](ctx, URL, &httpclientx.Config{
 		Client:    c.HTTPClient,
+		Host:      c.Host,
 		Logger:    c.Logger,
 		UserAgent: c.UserAgent,
 	})

internal/probeservices/bouncer_test.go

@@ -91,6 +91,57 @@ func TestGetTestHelpers(t *testing.T) {
 		}
 	})
 
+	t.Run("we can use cloudfronting", func(t *testing.T) {
+		// this is what we expect to receive
+		expect := map[string][]model.OOAPIService{
+			"web-connectivity": {{
+				Address: "https://0.th.ooni.org/",
+				Type:    "https",
+			}},
+		}
+
+		// create quick and dirty server to serve the response
+		srv := testingx.MustNewHTTPServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			runtimex.Assert(r.Host == "www.cloudfront.com", "invalid r.Host")
+			runtimex.Assert(r.Method == http.MethodGet, "invalid method")
+			runtimex.Assert(r.URL.Path == "/api/v1/test-helpers", "invalid URL path")
+			w.Write(must.MarshalJSON(expect))
+		}))
+		defer srv.Close()
+
+		// create a probeservices client
+		client := newclient()
+
+		// make sure we're using cloudfronting
+		client.Host = "www.cloudfront.com"
+
+		// override the HTTP client
+		client.HTTPClient = &mocks.HTTPClient{
+			MockDo: func(req *http.Request) (*http.Response, error) {
+				URL := runtimex.Try1(url.Parse(srv.URL))
+				req.URL.Scheme = URL.Scheme
+				req.URL.Host = URL.Host
+				return http.DefaultClient.Do(req)
+			},
+			MockCloseIdleConnections: func() {
+				http.DefaultClient.CloseIdleConnections()
+			},
+		}
+
+		// issue the GET request
+		testhelpers, err := client.GetTestHelpers(context.Background())
+
+		// we do not expect an error
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// we expect to see exactly what the server sent
+		if diff := cmp.Diff(expect, testhelpers); diff != "" {
+			t.Fatal(diff)
+		}
+	})
+
 	t.Run("reports an error when the connection is reset", func(t *testing.T) {
 		// create quick and dirty server to serve the response
 		srv := testingx.MustNewHTTPServer(testingx.HTTPHandlerReset())

internal/probeservices/checkin_test.go

@@ -141,6 +141,81 @@ func TestCheckIn(t *testing.T) {
 		}
 	})
 
+	t.Run("we can use cloudfronting", func(t *testing.T) {
+		// define our expectations
+		expect := &model.OOAPICheckInResult{
+			Conf: model.OOAPICheckInResultConfig{
+				Features: map[string]bool{},
+				TestHelpers: map[string][]model.OOAPIService{
+					"web-connectivity": {{
+						Address: "https://0.th.ooni.org/",
+						Type:    "https",
+					}},
+				},
+			},
+			ProbeASN: "AS30722",
+			ProbeCC:  "US",
+			Tests: model.OOAPICheckInResultNettests{
+				WebConnectivity: &model.OOAPICheckInInfoWebConnectivity{
+					ReportID: "20240424T134700Z_webconnectivity_IT_30722_n1_q5N5YSTWEqHYDo9v",
+					URLs: []model.OOAPIURLInfo{{
+						CategoryCode: "NEWS",
+						CountryCode:  "IT",
+						URL:          "https://www.example.com/",
+					}},
+				},
+			},
+			UTCTime: time.Date(2022, 11, 22, 1, 2, 3, 0, time.UTC),
+			V:       1,
+		}
+
+		// create a local server that responds with the expectation
+		srv := testingx.MustNewHTTPServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			runtimex.Assert(r.Host == "www.cloudfront.com", "invalid r.Host")
+			runtimex.Assert(r.Method == http.MethodPost, "invalid method")
+			runtimex.Assert(r.URL.Path == "/api/v1/check-in", "invalid URL path")
+			rawreqbody := runtimex.Try1(netxlite.ReadAllContext(r.Context(), r.Body))
+			var gotrequest model.OOAPICheckInConfig
+			must.UnmarshalJSON(rawreqbody, &gotrequest)
+			diff := cmp.Diff(config, gotrequest)
+			runtimex.Assert(diff == "", "request mismatch:"+diff)
+			w.Write(must.MarshalJSON(expect))
+		}))
+		defer srv.Close()
+
+		// create a probeservices client
+		client := newclient()
+
+		// make sure we're using cloudfronting
+		client.Host = "www.cloudfront.com"
+
+		// override the HTTP client
+		client.HTTPClient = &mocks.HTTPClient{
+			MockDo: func(req *http.Request) (*http.Response, error) {
+				URL := runtimex.Try1(url.Parse(srv.URL))
+				req.URL.Scheme = URL.Scheme
+				req.URL.Host = URL.Host
+				return http.DefaultClient.Do(req)
+			},
+			MockCloseIdleConnections: func() {
+				http.DefaultClient.CloseIdleConnections()
+			},
+		}
+
+		// call the API
+		result, err := client.CheckIn(context.Background(), config)
+
+		// we do not expect to see an error
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// we expect to see exactly what the server sent
+		if diff := cmp.Diff(expect, result); diff != "" {
+			t.Fatal(diff)
+		}
+	})
+
 	t.Run("reports an error when the connection is reset", func(t *testing.T) {
 		// create quick and dirty server to serve the response
 		srv := testingx.MustNewHTTPServer(testingx.HTTPHandlerReset())

internal/probeservices/collector.go

@@ -69,6 +69,7 @@ func (c Client) OpenReport(ctx context.Context, rt model.OOAPIReportTemplate) (R
 	cor, err := httpclientx.PostJSON[model.OOAPIReportTemplate, *model.OOAPICollectorOpenResponse](
 		ctx, URL, rt, &httpclientx.Config{
 			Client:    c.HTTPClient,
+			Host:      c.Host,
 			Logger:    c.Logger,
 			UserAgent: c.UserAgent,
 		},
@@ -118,6 +119,7 @@ func (r reportChan) SubmitMeasurement(ctx context.Context, m *model.Measurement)
 		model.OOAPICollectorUpdateRequest, *model.OOAPICollectorUpdateResponse](
 		ctx, URL, apiReq, &httpclientx.Config{
 			Client:    r.client.HTTPClient,
+			Host:      r.client.Host,
 			Logger:    r.client.Logger,
 			UserAgent: r.client.UserAgent,
 		},

internal/probeservices/collector_test.go

@@ -179,6 +179,69 @@ func TestReportLifecycle(t *testing.T) {
 		}
 	})
 
+	t.Run("we can use cloudfronting", func(t *testing.T) {
+		// create state for emulating the OONI backend
+		state := &testingx.OONICollector{}
+
+		// expose the state via HTTP
+		srv := testingx.MustNewHTTPServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			runtimex.Assert(r.Host == "www.cloudfront.com", "invalid r.Host")
+			state.ServeHTTP(w, r)
+		}))
+		defer srv.Close()
+
+		// create a probeservices client
+		client := newclient()
+
+		// make sure we're using cloudfronting
+		client.Host = "www.cloudfront.com"
+
+		// override the HTTP client so we speak with our local server rather than the true backend
+		client.HTTPClient = &mocks.HTTPClient{
+			MockDo: func(req *http.Request) (*http.Response, error) {
+				URL := runtimex.Try1(url.Parse(srv.URL))
+				req.URL.Scheme = URL.Scheme
+				req.URL.Host = URL.Host
+				return http.DefaultClient.Do(req)
+			},
+			MockCloseIdleConnections: func() {
+				http.DefaultClient.CloseIdleConnections()
+			},
+		}
+
+		// create the report template used for testing
+		template := newReportTemplateForTesting()
+
+		// open the report
+		report, err := client.OpenReport(context.Background(), template)
+
+		// we expect to be able to open the report
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// make a measurement out of the report template
+		measurement := makeMeasurement(template, report.ReportID())
+
+		// make sure we can submit this measurement within the report, which we really
+		// expect to succeed since we created the measurement from the template
+		if report.CanSubmit(&measurement) != true {
+			t.Fatal("report should be able to submit this measurement")
+		}
+
+		// attempt to submit the measurement to the backend, which should succeed
+		// since we've just opened a report for it
+		if err = report.SubmitMeasurement(context.Background(), &measurement); err != nil {
+			t.Fatal(err)
+		}
+
+		// additionally make sure we edited the measurement report ID to
+		// contain the correct report ID used to submit
+		if measurement.ReportID != report.ReportID() {
+			t.Fatal("report ID mismatch")
+		}
+	})
+
 	t.Run("opening a report fails with an error when the connection is reset", func(t *testing.T) {
 		// create quick and dirty server to serve the response
 		srv := testingx.MustNewHTTPServer(testingx.HTTPHandlerReset())

internal/probeservices/login.go

@@ -28,6 +28,7 @@ func (c Client) MaybeLogin(ctx context.Context) error {
 	auth, err := httpclientx.PostJSON[*model.OOAPILoginCredentials, *model.OOAPILoginAuth](
 		ctx, URL, creds, &httpclientx.Config{
 			Client:    c.HTTPClient,
+			Host:      c.Host,
 			Logger:    model.DiscardLogger,
 			UserAgent: c.UserAgent,
 		},

internal/probeservices/login_test.go

@@ -97,6 +97,60 @@ func TestMaybeLogin(t *testing.T) {
 		}
 	})
 
+	t.Run("we can use cloudfronting", func(t *testing.T) {
+		// create state for emulating the OONI backend
+		state := &testingx.OONIBackendWithLoginFlow{}
+		mux := state.NewMux()
+
+		// expose the state via HTTP
+		srv := testingx.MustNewHTTPServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			runtimex.Assert(r.Host == "www.cloudfront.com", "invalid r.Host")
+			mux.ServeHTTP(w, r)
+		}))
+		defer srv.Close()
+
+		// create a probeservices client
+		client := newclient()
+
+		// make sure we're using cloudfronting
+		client.Host = "www.cloudfront.com"
+
+		// override the HTTP client so we speak with our local server rather than the true backend
+		client.HTTPClient = &mocks.HTTPClient{
+			MockDo: func(req *http.Request) (*http.Response, error) {
+				URL := runtimex.Try1(url.Parse(srv.URL))
+				req.URL.Scheme = URL.Scheme
+				req.URL.Host = URL.Host
+				return http.DefaultClient.Do(req)
+			},
+			MockCloseIdleConnections: func() {
+				http.DefaultClient.CloseIdleConnections()
+			},
+		}
+
+		// we need to register first because we don't have state yet
+		if err := client.MaybeRegister(context.Background(), MetadataFixture()); err != nil {
+			t.Fatal(err)
+		}
+
+		// now we try to login and get a token
+		if err := client.MaybeLogin(context.Background()); err != nil {
+			t.Fatal(err)
+		}
+
+		// do this again, and later on we'll verify that we
+		// did actually issue just a single login call
+		if err := client.MaybeLogin(context.Background()); err != nil {
+			t.Fatal(err)
+		}
+
+		// make sure we did call login just once: the second call
+		// should not invoke login because we have good state
+		if client.LoginCalls.Load() != 1 {
+			t.Fatal("called login API too many times")
+		}
+	})
+
 	t.Run("reports an error when the connection is reset", func(t *testing.T) {
 		// create quick and dirty server to serve the response
 		srv := testingx.MustNewHTTPServer(testingx.HTTPHandlerReset())

internal/probeservices/measurementmeta.go

@@ -35,6 +35,7 @@ func (c Client) GetMeasurementMeta(
 	// get the response
 	return httpclientx.GetJSON[*model.OOAPIMeasurementMeta](ctx, URL, &httpclientx.Config{
 		Client:    c.HTTPClient,
+		Host:      c.Host,
 		Logger:    c.Logger,
 		UserAgent: c.UserAgent,
 	})

internal/probeservices/measurementmeta_test.go

@@ -129,6 +129,49 @@ func TestGetMeasurementMeta(t *testing.T) {
 		}
 	})
 
+	t.Run("we can use cloudfronting", func(t *testing.T) {
+		// create quick and dirty server to serve the response
+		srv := testingx.MustNewHTTPServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			runtimex.Assert(r.Host == "www.cloudfront.com", "invalid r.Host")
+			runtimex.Assert(r.Method == http.MethodGet, "invalid method")
+			runtimex.Assert(r.URL.Path == "/api/v1/measurement_meta", "invalid URL path")
+			w.Write(must.MarshalJSON(expectMmeta))
+		}))
+		defer srv.Close()
+
+		// create a probeservices client
+		client := newclient()
+
+		// make sure we're using cloudfronting
+		client.Host = "www.cloudfront.com"
+
+		// override the HTTP client
+		client.HTTPClient = &mocks.HTTPClient{
+			MockDo: func(req *http.Request) (*http.Response, error) {
+				URL := runtimex.Try1(url.Parse(srv.URL))
+				req.URL.Scheme = URL.Scheme
+				req.URL.Host = URL.Host
+				return http.DefaultClient.Do(req)
+			},
+			MockCloseIdleConnections: func() {
+				http.DefaultClient.CloseIdleConnections()
+			},
+		}
+
+		// issue the API call proper
+		mmeta, err := client.GetMeasurementMeta(context.Background(), config)
+
+		// we do not expect to see errors obviously
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// compare with the expectation
+		if diff := cmp.Diff(expectMmeta, mmeta); diff != "" {
+			t.Fatal(diff)
+		}
+	})
+
 	t.Run("reports an error when the connection is reset", func(t *testing.T) {
 		// create quick and dirty server to serve the response
 		srv := testingx.MustNewHTTPServer(testingx.HTTPHandlerReset())