cache per-provider config

internal/engine/inputloader.go

@@ -30,7 +30,7 @@ type InputLoaderSession interface {
 	CheckIn(ctx context.Context,
 		config *model.OOAPICheckInConfig) (*model.OOAPICheckInResultNettests, error)
 	FetchOpenVPNConfig(ctx context.Context,
-		cc string) (map[string]model.OOAPIVPNProviderConfig, error)
+		provider, cc string) (*model.OOAPIVPNProviderConfig, error)
 }
 
 // InputLoaderLogger is the logger according to an InputLoader.
@@ -330,29 +330,33 @@ func (il *InputLoader) loadRemoteWebConnectivity(ctx context.Context) ([]model.O
 	return reply.WebConnectivity.URLs, nil
 }
 
+// These are the providers that are enabled in the API.
+var openvpnDefaultProviders = []string{
+	"riseup",
+}
+
 // loadRemoteOpenVPN loads openvpn inputs from a remote source.
 func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.OOAPIURLInfo, error) {
-	reply, err := il.vpnConfig(ctx)
-	if err != nil {
-		return nil, err
-	}
-
 	// VPN Inputs do not match exactly the semantics expected from [model.OOAPIURLInfo],
 	// since OOAPIURLInfo is oriented twards webconnectivity,
 	// but we force VPN targets in the URL and ignore all the other fields.
 	urls := make([]model.OOAPIURLInfo, 0)
 
-	// here we're just collecting all the inputs. we also cache the configs so that
-	// each experiment run can access the credentials for a given provider.
-	for _, config := range reply {
-		for _, input := range config.Inputs {
+	for _, provider := range openvpnDefaultProviders {
+		reply, err := il.vpnConfig(ctx, provider)
+		if err != nil {
+			return nil, err
+		}
+		// here we're just collecting all the inputs. we also cache the configs so that
+		// each experiment run can access the credentials for a given provider.
+		for _, input := range reply.Inputs {
 			urls = append(urls, model.OOAPIURLInfo{URL: input})
 		}
 	}
+
 	if len(urls) == 0 {
 		return nil, ErrNoURLsReturned
 	}
-	// TODO(ainghazal): persist config for all providers
 	return urls, nil
 }
 
@@ -375,8 +379,8 @@ func (il *InputLoader) checkIn(
 }
 
 // vpnConfig fetches vpn information for the configured providers
-func (il *InputLoader) vpnConfig(ctx context.Context) (map[string]model.OOAPIVPNProviderConfig, error) {
-	reply, err := il.Session.FetchOpenVPNConfig(ctx, "XX")
+func (il *InputLoader) vpnConfig(ctx context.Context, provider string) (*model.OOAPIVPNProviderConfig, error) {
+	reply, err := il.Session.FetchOpenVPNConfig(ctx, provider, "XX")
 	if err != nil {
 		return nil, err
 	}

internal/engine/session.go

@@ -376,24 +376,23 @@ func (s *Session) FetchTorTargets(
 	return clnt.FetchTorTargets(ctx, cc)
 }
 
-// FetchOpenVPNConfig fetches openvpn config from the API.
+// FetchOpenVPNConfig fetches openvpn config from the API if it's not found in the
+// internal cache. We do this to avoid hitting the API for every input.
 func (s *Session) FetchOpenVPNConfig(
-	ctx context.Context, cc string) (map[string]model.OOAPIVPNProviderConfig, error) {
-	// TODO: need to cache only the requested provider, since it's different calls to the API.
-	if len(s.vpnConfig) > 0 {
-		return s.vpnConfig, nil
+	ctx context.Context, provider, cc string) (*model.OOAPIVPNProviderConfig, error) {
+	if config, ok := s.vpnConfig[provider]; ok {
+		return &config, nil
 	}
-
 	clnt, err := s.NewOrchestraClient(ctx)
 	if err != nil {
-		return nil, err
+		return &model.OOAPIVPNProviderConfig{}, err
 	}
-	config, err := clnt.FetchOpenVPNConfig(ctx, cc)
+	config, err := clnt.FetchOpenVPNConfig(ctx, provider, cc)
 	if err != nil {
-		return nil, err
+		return &model.OOAPIVPNProviderConfig{}, err
 	}
-	s.vpnConfig = config
-	return config, nil
+	s.vpnConfig[provider] = config
+	return &config, nil
 }
 
 // KeyValueStore returns the configured key-value store.

internal/experiment/openvpn/openvpn.go

@@ -223,9 +223,7 @@ func (m *Measurer) getCredentialsFromOptionsOrAPI(
 	}
 
 	// No options passed, let's hit OONI API for credential distribution.
-	// TODO(ainghazal): cache credentials fetch?
-
-	apiCreds, err := m.fetchProviderCredentials(ctx, sess)
+	apiCreds, err := m.fetchProviderCredentials(ctx, sess, provider)
 	// TODO(ainghazal): need to validate
 
 	if err != nil {
@@ -331,11 +329,14 @@ func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, zeroTim
 }
 
 // TODO: get cached from session instead of fetching every time
-func (m *Measurer) fetchProviderCredentials(ctx context.Context, sess model.ExperimentSession) (model.OOAPIVPNProviderConfig, error) {
-	// TODO do pass country code, can be useful to orchestrate campaigns specific to areas
-	config, err := sess.FetchOpenVPNConfig(ctx, "XX")
+func (m *Measurer) fetchProviderCredentials(
+	ctx context.Context,
+	sess model.ExperimentSession,
+	provider string) (*model.OOAPIVPNProviderConfig, error) {
+	// TODO(ainghazal): do pass country code, can be useful to orchestrate campaigns specific to areas
+	config, err := sess.FetchOpenVPNConfig(ctx, provider, "XX")
 	if err != nil {
-		return model.OOAPIVPNProviderConfig{}, err
+		return &model.OOAPIVPNProviderConfig{}, err
 	}
-	return config["riseup"], nil
+	return config, nil
 }

internal/model/experiment.go

@@ -18,7 +18,7 @@ type ExperimentSession interface {
 	DefaultHTTPClient() HTTPClient
 
 	// FetchOpenVPNConfig returns vpn config as a serialized JSON or an error.
-	FetchOpenVPNConfig(ctx context.Context, cc string) (map[string]OOAPIVPNProviderConfig, error)
+	FetchOpenVPNConfig(ctx context.Context, provider, cc string) (*OOAPIVPNProviderConfig, error)
 
 	// FetchPsiphonConfig returns psiphon's config as a serialized JSON or an error.
 	FetchPsiphonConfig(ctx context.Context) ([]byte, error)

internal/probeservices/openvpn.go

@@ -9,26 +9,23 @@ import (
 )
 
 // FetchOpenVPNConfig returns valid configuration for the openvpn experiment.
-func (c Client) FetchOpenVPNConfig(ctx context.Context, cc string) (map[string]model.OOAPIVPNProviderConfig, error) {
-	fmt.Println("FETCHING OPENVPN CONFIG>>>>")
+// It accepts the provider label, and the country code for the probe, in case the API wants to
+// return different targets to us depending on where we are located.
+func (c Client) FetchOpenVPNConfig(ctx context.Context, provider, cc string) (result model.OOAPIVPNProviderConfig, err error) {
 	_, auth, err := c.GetCredsAndAuth()
 	if err != nil {
-		return nil, err
+		return model.OOAPIVPNProviderConfig{}, err
 	}
 	s := fmt.Sprintf("Bearer %s", auth.Token)
 	client := c.APIClientTemplate.BuildWithAuthorization(s)
 	query := url.Values{}
 	query.Add("country_code", cc)
 
-	result := model.OOAPIVPNProviderConfig{}
-
 	err = client.GetJSONWithQuery(
-		ctx, "/api/v2/ooniprobe/vpn-config/riseup/", query, &result,
+		ctx,
+		fmt.Sprintf("/api/v2/ooniprobe/vpn-config/%s/", provider),
+		query,
+		&result,
 	)
-
-	allProviders := map[string]model.OOAPIVPNProviderConfig{
-		"riseup": result,
-	}
-
-	return allProviders, err
+	return
 }