Skip to content

Commit

Permalink
remove handshake start
Browse files Browse the repository at this point in the history
  • Loading branch information
ainghazal committed Mar 29, 2024
1 parent 06a15c8 commit a040a0b
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 19 deletions.
9 changes: 9 additions & 0 deletions internal/engine/inputloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net/url"

"github.com/apex/log"
"github.com/ooni/probe-cli/v3/internal/experiment/openvpn"
"github.com/ooni/probe-cli/v3/internal/fsx"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/registry"
Expand Down Expand Up @@ -355,6 +356,14 @@ func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.OOAPIURLI
}

if len(urls) == 0 {
// loadRemote returns ErrNoURLsReturned at this point for webconnectivity,
// but for OpenVPN we want to return a sensible default to be
// able to probe some endpoints even in very restrictive environments.
// Do note this means that you have to provide valid credentials
// by some other means.
for _, endpoint := range openvpn.DefaultEndpoints {
urls = append(urls, model.OOAPIURLInfo{URL: endpoint.AsInputURI()})
}
return nil, ErrNoURLsReturned
}
return urls, nil
Expand Down
3 changes: 0 additions & 3 deletions internal/experiment/openvpn/archival.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package openvpn

import "time"

// TODO(ainghazal): move to ooni internal archival package when consolidated.

// OpenVPNOptions is a subset of [vpnconfig.OpenVPNOptions] that we want to include
Expand All @@ -22,7 +20,6 @@ type ArchivalOpenVPNHandshakeResult struct {
Provider string `json:"provider"`
OpenVPNOptions OpenVPNOptions `json:"openvpn_options"`
Status ArchivalOpenVPNConnectStatus `json:"status"`
StartTime time.Time `json:"handshake_start_time"`

// T0 can differ from StartTime because for TCP we take T0 *after* dialing has successfully completed.
// This might be counterintuitive, review.
Expand Down
11 changes: 5 additions & 6 deletions internal/experiment/openvpn/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,11 @@ func (e *endpoint) AsInputURI() string {
// endpointList is a list of endpoints.
type endpointList []*endpoint

// allEndpoints contains a subset of known endpoints to be used if no input is passed to the experiment.
// This is a hardcoded list for now, but the idea is that we can receive this from the check-in api in the future.
// In any case, having hardcoded endpoints is good as a fallback for the cases in which we cannot contact
// OONI's backend.
// TODO: hardcoded, setup as backup if we cannot contact API.
var allEndpoints = endpointList{
// 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 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{
{
Provider: "riseup",
IPAddr: "51.15.187.53",
Expand Down
16 changes: 6 additions & 10 deletions internal/experiment/openvpn/openvpn.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
)

const (
testVersion = "0.1.0"
testVersion = "0.1.1"
openVPNProcol = "openvpn"
)

Expand Down Expand Up @@ -124,12 +124,9 @@ func parseListOfInputs(inputs string) (endpointList, error) {
return endpoints, nil
}

// ErrFailure is the error returned when you set the
// config.ReturnError field to true.
var ErrFailure = errors.New("mocked error")

// Run implements model.ExperimentMeasurer.Run.
// A single run expects exactly ONE input (endpoint).
// A single run expects exactly ONE input (endpoint), but we can modify whether
// to test different transports by settings options.
func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {
callbacks := args.Callbacks
measurement := args.Measurement
Expand All @@ -139,8 +136,8 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {

if measurement.Input == "" {
// if input is null, we get one from the hardcoded list of inputs.
sess.Logger().Info("No input given, picking one endpoint at random")
endpoint = allEndpoints.Shuffle()[0]
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
Expand Down Expand Up @@ -286,7 +283,7 @@ func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, zeroTim
if len(handshakeEvents) != 0 {
tFirst = handshakeEvents[0].AtTime
tLast = handshakeEvents[len(handshakeEvents)-1].AtTime
bootstrapTime = time.Since(zeroTime).Seconds()
bootstrapTime = tLast - tFirst
}

return &SingleConnection{
Expand All @@ -307,7 +304,6 @@ func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, zeroTim
Failure: &failure,
Success: err == nil,
},
StartTime: zeroTime,
T0: tFirst,
T: tLast,
Tags: []string{},
Expand Down

0 comments on commit a040a0b

Please sign in to comment.