wip

internal/experiment/openvpn/endpoint.go

@@ -133,7 +133,7 @@ func (e *endpoint) AsInputURI() string {
 type endpointList []*endpoint
 
 // DefaultEndpoints contains a subset of known endpoints to be used if no input is passed to the experiment and
-// the backend query fails. We risk distributing endpoints that can go stale, so we should be careful about
+// the backend query fails for whatever reason. We risk distributing endpoints that can go stale, so we should be careful about
 // the stability of the endpoints selected here, but in restrictive environments it's useful to have something
 // to probe in absence of an useful OONI API. Valid credentials are still needed, though.
 var DefaultEndpoints = endpointList{

internal/experiment/openvpn/openvpn.go

@@ -4,7 +4,6 @@ package openvpn
 import (
 	"context"
 	"errors"
-	"fmt"
 	"strconv"
 	"strings"
 	"time"
@@ -28,6 +27,7 @@ const (
 // of this experiment. By tagging these variables with `ooni:"..."`, we allow
 // miniooni's -O flag to find them and set them.
 type Config struct {
+	// TODO(ainghazal): Provider is right now ignored. InputLoader should get the provider from options.
 	Provider string `ooni:"VPN provider"`
 	SafeKey  string `ooni:"key to connect to the OpenVPN endpoint"`
 	SafeCert string `ooni:"cert to connect to the OpenVPN endpoint"`
@@ -90,8 +90,6 @@ type Measurer struct {
 
 // NewExperimentMeasurer creates a new ExperimentMeasurer.
 func NewExperimentMeasurer(config Config, testName string) model.ExperimentMeasurer {
-	// TODO(ainghazal): allow ooniprobe to override this.
-	config.Provider = "riseup"
 	return Measurer{config: config, testName: testName}
 }
 
@@ -109,19 +107,14 @@ var (
 	ErrInvalidInput = errors.New("invalid input")
 )
 
-// parseListOfInputs return an endpointlist from a comma-separated list of inputs,
-// and any error if the endpoints could not be parsed properly.
-func parseListOfInputs(inputs string) (endpointList, error) {
-	endpoints := make(endpointList, 0)
-	inputList := strings.Split(inputs, ",")
-	for _, i := range inputList {
-		e, err := newEndpointFromInputString(i)
-		if err != nil {
-			return endpoints, err
-		}
-		endpoints = append(endpoints, e)
+func isValidProtocol(s string) bool {
+	if strings.HasPrefix(s, "openvpn://") {
+		return true
 	}
-	return endpoints, nil
+	if strings.HasPrefix(s, "openvpn+obfs4://") {
+		return true
+	}
+	return false
 }
 
 // Run implements model.ExperimentMeasurer.Run.
@@ -134,26 +127,26 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {
 
 	var endpoint *endpoint
 
-	if measurement.Input == "" {
+	if measurement.Input != "" {
+		if ok := isValidProtocol(string(measurement.Input)); !ok {
+			return ErrInvalidInput
+		}
+		var err error
+		endpoint, err = newEndpointFromInputString(string(measurement.Input))
+		if err != nil {
+			return err
+		}
+	} else {
 		// if input is null, we get one from the hardcoded list of inputs.
+		// TODO(ainghazal): can input be empty at this stage?
+		// InputPolicy should ensure we have a hardcoded input,
+		// so this is probably non-reachable code. Move the shuffling there.
 		sess.Logger().Info("No input given, picking one hardcoded endpoint at random")
 		endpoint = DefaultEndpoints.Shuffle()[0]
 		measurement.Input = model.MeasurementTarget(endpoint.AsInputURI())
-	} else {
-		// otherwise, we expect a comma-separated value of inputs in
-		// the URI scheme defined for openvpn experiments.
-		endpoints, err := parseListOfInputs(string(measurement.Input))
-		if err != nil {
-			return err
-		}
-		if len(endpoints) != 1 {
-			return fmt.Errorf("%w: only single input accepted", ErrInvalidInput)
-		}
-		endpoint = endpoints[0]
 	}
 
 	tk := NewTestKeys()
-
 	sess.Logger().Infof("Probing endpoint %s", endpoint.String())
 
 	// TODO:  separate pre-connection checks
@@ -221,10 +214,11 @@ func (m *Measurer) getCredentialsFromOptionsOrAPI(
 		return creds, nil
 	}
 
-	// No options passed, let's hit OONI API for credential distribution.
+	// No options passed, so let's get the credentials that inputbuilder should have cached
+	// for us after hitting the OONI API.
 	// We expect the credentials from the API response to be encoded as the direct PEM serialization.
 	apiCreds, err := m.fetchProviderCredentials(ctx, sess, provider)
-	// TODO(ainghazal): validate
+	// TODO(ainghazal): validate credentials have the info we expect, certs are not expired etc.
 
 	if err != nil {
 		sess.Logger().Warnf("Error fetching credentials from API: %s", err.Error())

internal/experiment/openvpn/openvpn_test.go

@@ -6,11 +6,9 @@ import (
 	"testing"
 	"time"
 
-	"github.com/apex/log"
 	"github.com/google/go-cmp/cmp"
 	"github.com/ooni/probe-cli/v3/internal/experiment/example"
 	"github.com/ooni/probe-cli/v3/internal/experiment/openvpn"
-	"github.com/ooni/probe-cli/v3/internal/legacy/mockable"
 	"github.com/ooni/probe-cli/v3/internal/mocks"
 	"github.com/ooni/probe-cli/v3/internal/model"
 
@@ -59,7 +57,6 @@ func TestAddConnectionTestKeys(t *testing.T) {
 				Provider:       "unknown",
 				OpenVPNOptions: openvpn.OpenVPNOptions{},
 				Status:         openvpn.ArchivalOpenVPNConnectStatus{},
-				StartTime:      time.Now(),
 				T0:             0,
 				T:              0,
 				Tags:           []string{},
@@ -122,31 +119,55 @@ func TestSuccess(t *testing.T) {
 	if m.ExperimentName() != "openvpn" {
 		t.Fatal("invalid ExperimentName")
 	}
-	if m.ExperimentVersion() != "0.1.0" {
+	if m.ExperimentVersion() != "0.1.1" {
 		t.Fatal("invalid ExperimentVersion")
 	}
 	ctx := context.Background()
-	sess := &mocks.Session{}
+	sess := &mocks.Session{
+		MockLogger: func() model.Logger {
+			return model.DiscardLogger
+		},
+		MockFetchOpenVPNConfig: func(context.Context, string, string) (*model.OOAPIVPNProviderConfig, error) {
+			return &model.OOAPIVPNProviderConfig{
+				Provider: "provider",
+				Config: &struct {
+					CA       string "json:\"ca\""
+					Cert     string "json:\"cert,omitempty\""
+					Key      string "json:\"key,omitempty\""
+					Username string "json:\"username,omitempty\""
+					Password string "json:\"password,omitempty\""
+				}{
+					CA:   "ca",
+					Cert: "cert",
+					Key:  "key",
+				},
+				Inputs:      []string{},
+				DateUpdated: time.Now(),
+			}, nil
+		},
+	}
 	callbacks := model.NewPrinterCallbacks(sess.Logger())
 	measurement := new(model.Measurement)
 	args := &model.ExperimentArgs{
 		Callbacks:   callbacks,
 		Measurement: measurement,
 		Session:     sess,
 	}
+	// TODO: mock runner
 	err := m.Run(ctx, args)
 	if err != nil {
 		t.Fatal(err)
 	}
 }
 
 func TestFailure(t *testing.T) {
-	m := example.NewExperimentMeasurer(example.Config{
-		SleepTime:   int64(2 * time.Millisecond),
-		ReturnError: true,
-	}, "example")
+	m := openvpn.NewExperimentMeasurer(openvpn.Config{}, "openvpn")
 	ctx := context.Background()
-	sess := &mockable.Session{MockableLogger: log.Log}
+	sess := &mocks.Session{
+		MockLogger: func() model.Logger {
+			return model.DiscardLogger
+		},
+	}
 	callbacks := model.NewPrinterCallbacks(sess.Logger())
 	args := &model.ExperimentArgs{
 		Callbacks:   callbacks,