diff --git a/internal/engine/inputloader.go b/internal/engine/inputloader.go index c3f9a53f26..d10d851205 100644 --- a/internal/engine/inputloader.go +++ b/internal/engine/inputloader.go @@ -304,6 +304,11 @@ func (il *InputLoader) readfile(filepath string, open inputLoaderOpenFn) ([]mode func (il *InputLoader) loadRemote(ctx context.Context) ([]model.OOAPIURLInfo, error) { switch registry.CanonicalizeExperimentName(il.ExperimentName) { case "openvpn": + // TODO(ainghazal): given the semantics of the current API call, in an ideal world we'd need to pass + // the desired provider here, if known. We only have one provider for now, so I'm acting like YAGNI. Another + // option is perhaps to coalesce all the known providers per proto into a single API call and let client + // pick whatever they want. + // This will likely improve after Richer Input is available. return il.loadRemoteOpenVPN(ctx) default: return il.loadRemoteWebConnectivity(ctx) @@ -335,9 +340,13 @@ func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.OOAPIURLI // VPN Inputs do not match exactly the semantics expected from [model.OOAPIURLInfo], // since OOAPIURLInfo is oriented towards webconnectivity, // but we force VPN targets in the URL and ignore all the other fields. + // There's still some level of impedance mismatch here, since it's also possible that + // the user of the library wants to use remotes by unknown providers passed via cli options, + // oonirun etc; in that case we'll need to extract the provider annotation from the URLs. urls := make([]model.OOAPIURLInfo, 0) // The openvpn experiment contains an array of the providers that the API knows about. + // We try to get all the remotes from the API for the list of enabled providers. for _, provider := range openvpn.APIEnabledProviders { reply, err := il.vpnConfig(ctx, provider) if err != nil { diff --git a/internal/engine/inputloader_test.go b/internal/engine/inputloader_test.go index cb13aa3a5c..0a7a889bb2 100644 --- a/internal/engine/inputloader_test.go +++ b/internal/engine/inputloader_test.go @@ -584,9 +584,9 @@ func TestInputLoaderOpenVPNSuccessWithNoInputAndAPICall(t *testing.T) { Session: &InputLoaderMockableSession{ Error: nil, FetchOpenVPNConfigOutput: &model.OOAPIVPNProviderConfig{ - Provider: "riseup", + Provider: "riseupvpn", Inputs: []string{ - "openvpn://foo.corp/?address=1.1.1.1:1194&transport=tcp", + "openvpn://foo.corp/?address=1.2.3.4:1194&transport=tcp", }, DateUpdated: time.Now(), }, diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index 73bc251314..bd456dab6a 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -193,7 +193,6 @@ var defaultOptionsByProvider = map[string]*vpnconfig.OpenVPNOptions{ var APIEnabledProviders = []string{ // TODO: fix the backend so that we can remove the spurious "vpn" suffix here. "riseupvpn", - "riseup", } // isValidProvider returns true if the provider is found as key in the registry of defaultOptionsByProvider.