-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
skywire-cli vpn
subcommands + separate-systray
#1317
Changes from 1 commit
8dde57a
405d731
0f83f14
b202365
70dfbb6
c7bbb0e
498aa61
a2b1a5c
8ddf64d
ab9bf63
37b22e1
f5d0e06
73d4dbf
d5e68d6
67bb78c
b42e900
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,33 +2,47 @@ package clivpn | |
|
||
import ( | ||
"encoding/json" | ||
"io/ioutil" | ||
"net/http" | ||
"strings" | ||
"time" | ||
|
||
"fmt" | ||
"log" | ||
"os" | ||
"strings" | ||
|
||
"github.com/spf13/cobra" | ||
"github.com/toqueteos/webbrowser" | ||
|
||
utilenv "github.com/skycoin/skywire-utilities/pkg/skyenv" | ||
skyenv "github.com/skycoin/skywire/pkg/skyenv" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. May be a good move to get this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the skyenv is here just getting the default name of the app binary "vpn-client" that could be redundantly defined in skywire-utilities to avoid the import in this instance, but considering that the vpn-client app itself s defined in skycoin/skywire I think it prudent to keep it here. |
||
"github.com/skycoin/skywire/pkg/visor/visorconfig" | ||
clirpc "github.com/skycoin/skywire/cmd/skywire-cli/commands/rpc" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason we are naming this import here? Is RPC not fine? I dont see us importing the stdlib rpc package here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, is the file name intentional? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it was an attempt to avoid confusion with any other package named rpc and specify that this is the one that is associated with or provided by the cli subcommands that same rpc client function was in a few different files |
||
"github.com/skycoin/skywire/pkg/servicedisc" | ||
"github.com/skycoin/skywire/cmd/skywire-cli/internal" | ||
"github.com/skycoin/skywire/pkg/transport/network" | ||
"github.com/skycoin/skywire/pkg/visor" | ||
"github.com/skycoin/skywire/cmd/skywire-cli/commands/visor" | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesnt appear to be your issue, but please remove the commented import. |
||
//"github.com/skycoin/skywire-utilities/pkg/cipher" | ||
|
||
|
||
) | ||
|
||
var timeout time.Duration | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Format your code please. |
||
|
||
func init() { | ||
RootCmd.AddCommand( | ||
vpnListCmd, | ||
vpnUICmd, | ||
vpnURLCmd, | ||
vpnStartCmd, | ||
) | ||
vpnListCmd.Flags().StringVarP(&ver, "ver", "v", "1.0.1", "filter results by version") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we get the default value for the version from the build tag instead maybe? |
||
vpnListCmd.Flags().StringVarP(&country, "country", "c", "", "filter results by country") | ||
vpnListCmd.Flags().BoolVarP(&stats, "stats", "s", false, "return only a count of the resuts") | ||
vpnListCmd.Flags().BoolVarP(&systray, "systray", "y", false, "format results for systray") | ||
|
||
} | ||
|
||
|
||
|
@@ -113,11 +127,65 @@ var vpnListCmd = &cobra.Command{ | |
} | ||
} | ||
} | ||
if len(a) > 0 { | ||
servers = a | ||
a = []servicedisc.Service{} | ||
servers = a | ||
a = []servicedisc.Service{} | ||
} | ||
var u uptime | ||
var s []string | ||
if len(servers) == 0 { | ||
fmt.Printf("No VPN Servers found\n") | ||
os.Exit(0) | ||
} | ||
for _, i := range servers { | ||
s = append(s, strings.Replace(i.Addr.String(), ":44", "", 1)+",") | ||
} | ||
//https://ut.skywire.skycoin.com/uptimes?visors= | ||
urlstr := []string{utilenv.UptimeTrackerAddr, "/uptimes?visors="} | ||
for _, i := range s { | ||
urlstr = append(urlstr, i) | ||
} | ||
serviceConf := strings.Join(urlstr, "") | ||
httpclient := http.Client{ | ||
Timeout: time.Second * 2, // Timeout after 2 seconds | ||
} | ||
//create the http request | ||
req, err := http.NewRequest(http.MethodGet, serviceConf, nil) | ||
if err != nil { | ||
logger.WithError(err).Fatal("Failed to create http request\n") | ||
} | ||
req.Header.Add("Cache-Control", "no-cache") | ||
//check for errors in the response | ||
res, err := httpclient.Do(req) | ||
if err != nil { | ||
logger.Error("Failed to fetch online status for visor") | ||
} else { | ||
// nil error from client.Do(req) | ||
if res.Body != nil { | ||
defer res.Body.Close() //nolint | ||
} | ||
body, err := ioutil.ReadAll(res.Body) | ||
if err != nil { | ||
logger.WithError(err).Fatal("Failed to read response\n") | ||
} | ||
//fill in services struct with the response | ||
err = json.Unmarshal(body, &u) | ||
if err == nil { | ||
for _, i := range u { | ||
if i.Online { | ||
for _, j := range servers { | ||
if j.Addr.String() == i.Key { | ||
a = append(a, j) | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
if len(a) > 0 { | ||
servers = a | ||
a = []servicedisc.Service{} | ||
} | ||
|
||
if stats { | ||
fmt.Printf("%d VPN Servers\n", len(servers)) | ||
os.Exit(0) | ||
|
@@ -141,7 +209,54 @@ var vpnListCmd = &cobra.Command{ | |
} | ||
|
||
fmt.Printf("%s", j) | ||
|
||
// fmt.Println(servers) | ||
}, | ||
} | ||
|
||
var vpnStartCmd = &cobra.Command{ | ||
Use: "start", | ||
Short: "start the vpn for <public-key>", | ||
Args: cobra.MinimumNArgs(1), | ||
Run: func(_ *cobra.Command, args []string) { | ||
pk := internal.ParsePK("remote-public-key", args[0]) | ||
var tp *visor.TransportSummary | ||
var err error | ||
transportTypes := []network.Type{ | ||
network.STCP, | ||
network.STCPR, | ||
network.SUDPH, | ||
network.DMSG, | ||
} | ||
for _, transportType := range transportTypes { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will almost necessarily fail for most transports here. Why are we trying to establish 4 transports? In my opinion, we should leave auto transport establishment to the router module as it has a substantially better view of current transports etc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From the currently available cli interface; the method I have used is the same as is used by Is what you described provided by some other cli command currently? if so, what? |
||
tp, err = clirpc.RpcClient().AddTransport(pk, string(transportType), timeout) | ||
if err == nil { | ||
logger.Infof("Established %v transport to %v", transportType, pk) | ||
} else { | ||
logger.WithError(err).Warnf("Failed to establish %v transport", transportType) | ||
} | ||
} | ||
clivisor.PrintTransports(tp) | ||
fmt.Println("%s", args[0]) | ||
internal.Catch(clirpc.RpcClient().StartVPNClient(args[0])) | ||
fmt.Println("OK") | ||
}, | ||
} | ||
|
||
var vpnStopCmd = &cobra.Command{ | ||
Use: "stop", | ||
Short: "stop the vpn", | ||
Args: cobra.MinimumNArgs(1), | ||
Run: func(_ *cobra.Command, args []string) { | ||
internal.Catch(clirpc.RpcClient().StopVPNClient(skyenv.VPNClientName)) | ||
fmt.Println("OK") | ||
}, | ||
} | ||
|
||
|
||
type uptime []struct { | ||
Key string `json:"key"` | ||
Uptime int `json:"uptime"` | ||
Downtime int `json:"downtime"` | ||
Percentage float64 `json:"percentage"` | ||
Online bool `json:"online"` | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,8 @@ type API interface { | |
Apps() ([]*appserver.AppState, error) | ||
StartApp(appName string) error | ||
StopApp(appName string) error | ||
StartVPNClient(pubkey string) error | ||
StopVPNClient(appName string) error | ||
SetAppDetailedStatus(appName, state string) error | ||
SetAppError(appName, stateErr string) error | ||
RestartApp(appName string) error | ||
|
@@ -355,6 +357,50 @@ func (v *Visor) StopApp(appName string) error { | |
return ErrProcNotAvailable | ||
} | ||
|
||
|
||
// StartVPNClient implements API. | ||
func (v *Visor) StartVPNClient(pubkey string) error { | ||
var envs []string | ||
var err error | ||
// todo: can we use some kind of app start hook that will be used for both autostart | ||
// and start? Reason: this is also called in init for autostart | ||
// check transport manager availability | ||
if v.tpM == nil { | ||
return ErrTrpMangerNotAvailable | ||
} | ||
v.conf.Launcher.Apps[0].Args = []string{"-srv", pubkey} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do we know the first app is the VPN? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this code equivalent to what we do when we call the VPN start cli command typically? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this was mostly adapted from the adjacent functions and the I know it's the first app for myself but there are instances where it may not even exist and then this would cause panic. Will solve that. |
||
for _, i := range v.conf.Launcher.Apps[0].Args { | ||
v.log.Infof(i) | ||
|
||
} | ||
maker := vpnEnvMaker(v.conf, v.dmsgC, v.dmsgDC, v.tpM.STCPRRemoteAddrs()) | ||
envs, err = maker() | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// if v.GetVPNClientAddress() == "" { | ||
// return errors.New("VPN server pub key is missing") | ||
// } | ||
|
||
// check process manager availability | ||
if v.procM != nil { | ||
return v.appL.StartApp(skyenv.VPNClientName, nil, envs) | ||
} | ||
return ErrProcNotAvailable | ||
} | ||
|
||
// StopVPNClient implements API. | ||
func (v *Visor) StopVPNClient(appName string) error { | ||
// check process manager availability | ||
if v.procM != nil { | ||
_, err := v.appL.StopApp(skyenv.VPNClientName) //nolint:errcheck | ||
return err | ||
} | ||
return ErrProcNotAvailable | ||
} | ||
|
||
|
||
// SetAppDetailedStatus implements API. | ||
func (v *Visor) SetAppDetailedStatus(appName, status string) error { | ||
proc, ok := v.procM.ProcByName(appName) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the file name intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my reasoning behind this naming scheme is to avoid making generic names for files or imports like "vpn.go" or "package vpn" as surely these exist somewhere else in the source already, and I would not want to pollute search results for such files and package names.
Is it your suggestion to change it?