From 163ad8484a0cc5e9c0b287df64e539c225a5bd63 Mon Sep 17 00:00:00 2001 From: ersonp Date: Wed, 22 Jun 2022 17:56:54 +0530 Subject: [PATCH 01/17] Fix apps logs --- cmd/apps/skychat/skychat.go | 2 +- cmd/apps/skysocks-client/skysocks-client.go | 8 +++---- cmd/apps/skysocks/skysocks.go | 13 +++++++---- cmd/apps/vpn-client/vpn-client.go | 6 ++--- cmd/apps/vpn-server/vpn-server.go | 14 +++++------ internal/skysocks/client.go | 6 ++--- internal/skysocks/server.go | 2 +- internal/vpn/client.go | 2 +- internal/vpn/server.go | 26 ++++++++++----------- 9 files changed, 41 insertions(+), 38 deletions(-) diff --git a/cmd/apps/skychat/skychat.go b/cmd/apps/skychat/skychat.go index ad6854f100..afb3e10b1e 100644 --- a/cmd/apps/skychat/skychat.go +++ b/cmd/apps/skychat/skychat.go @@ -106,7 +106,7 @@ func listenLoop() { fmt.Println("Accepting skychat conn...") conn, err := l.Accept() if err != nil { - print(fmt.Sprintf("Failed to accept conn: %v", err)) + print(fmt.Sprintf("Failed to accept conn: %v\n", err)) return } fmt.Println("Accepted skychat conn") diff --git a/cmd/apps/skysocks-client/skysocks-client.go b/cmd/apps/skysocks-client/skysocks-client.go index a5ec435a86..f829516d8c 100644 --- a/cmd/apps/skysocks-client/skysocks-client.go +++ b/cmd/apps/skysocks-client/skysocks-client.go @@ -66,9 +66,9 @@ func main() { if *serverPK == "" { err := errors.New("Empty server PubKey. Exiting") - print(err) + print(fmt.Sprintf("%v\n", err)) setAppErr(appCl, err) - return + os.Exit(1) } pk := cipher.PubKey{} @@ -113,12 +113,12 @@ func main() { func setAppErr(appCl *app.Client, err error) { if appErr := appCl.SetError(err.Error()); appErr != nil { - fmt.Printf("Failed to set error %v: %v\n", err, appErr) + print(fmt.Sprintf("Failed to set error %v: %v\n", err, appErr)) } } func setAppStatus(appCl *app.Client, status launcher.AppDetailedStatus) { if err := appCl.SetDetailedStatus(string(status)); err != nil { - fmt.Printf("Failed to set status %v: %v\n", status, err) + print(fmt.Sprintf("Failed to set status %v: %v\n", status, err)) } } diff --git a/cmd/apps/skysocks/skysocks.go b/cmd/apps/skysocks/skysocks.go index 76a524da84..bae0ba3e72 100644 --- a/cmd/apps/skysocks/skysocks.go +++ b/cmd/apps/skysocks/skysocks.go @@ -31,7 +31,7 @@ func main() { defer appCl.Close() if _, err := buildinfo.Get().WriteTo(os.Stdout); err != nil { - fmt.Printf("Failed to output build info: %v", err) + print(fmt.Sprintf("Failed to output build info: %v", err)) } var passcode = flag.String("passcode", "", "Authorize user against this passcode") @@ -39,12 +39,15 @@ func main() { srv, err := skysocks.NewServer(*passcode, appCl) if err != nil { - log.Fatal("Failed to create a new server: ", err) + print(fmt.Sprintf("Failed to create a new server: %v\n", err)) + os.Exit(1) } l, err := appCl.Listen(netType, port) if err != nil { + print(fmt.Sprintf("Error listening network %v on port %d: %v\n", netType, port, err)) log.Fatalf("Error listening network %v on port %d: %v\n", netType, port, err) + os.Exit(1) } fmt.Println("Starting serving proxy server") @@ -52,7 +55,7 @@ func main() { if runtime.GOOS == "windows" { ipcClient, err := ipc.StartClient(skyenv.VPNClientName, nil) if err != nil { - fmt.Printf("Error creating ipc server for VPN client: %v\n", err) + print(fmt.Sprintf("Error creating ipc server for VPN client: %v\n", err)) os.Exit(1) } go srv.ListenIPC(ipcClient) @@ -64,7 +67,7 @@ func main() { <-termCh if err := srv.Close(); err != nil { - fmt.Println(err) + print(fmt.Sprintf("%v\n", err)) os.Exit(1) } }() @@ -72,7 +75,7 @@ func main() { } if err := srv.Serve(l); err != nil { - fmt.Println(err) + print(fmt.Sprintf("%v\n", err)) os.Exit(1) } } diff --git a/cmd/apps/vpn-client/vpn-client.go b/cmd/apps/vpn-client/vpn-client.go index 0bc19cf875..2f8da92287 100644 --- a/cmd/apps/vpn-client/vpn-client.go +++ b/cmd/apps/vpn-client/vpn-client.go @@ -74,7 +74,7 @@ func main() { // TODO(darkrengarius): fix args passage for Windows //*serverPKStr = "03e9019b3caa021dbee1c23e6295c6034ab4623aec50802fcfdd19764568e2958d" err := errors.New("VPN server pub key is missing") - print(err) + print(fmt.Sprintf("%v\n", err)) setAppErr(appCl, err) os.Exit(1) } @@ -171,12 +171,12 @@ func main() { } if err := vpnClient.Serve(); err != nil { - print(fmt.Sprintf("Failed to serve VPN: %v", err)) + print(fmt.Sprintf("Failed to serve VPN: %v\n", err)) } } func setAppErr(appCl *app.Client, err error) { if appErr := appCl.SetError(err.Error()); appErr != nil { - fmt.Printf("Failed to set error %v: %v\n", err, appErr) + print(fmt.Sprintf("Failed to set error %v: %v\n", err, appErr)) } } diff --git a/cmd/apps/vpn-server/vpn-server.go b/cmd/apps/vpn-server/vpn-server.go index dcbdf96d57..43616df989 100644 --- a/cmd/apps/vpn-server/vpn-server.go +++ b/cmd/apps/vpn-server/vpn-server.go @@ -52,7 +52,7 @@ func main() { localPK := cipher.PubKey{} if *localPKStr != "" { if err := localPK.UnmarshalText([]byte(*localPKStr)); err != nil { - print(fmt.Sprintf("Invalid local PK: %v", err)) + print(fmt.Sprintf("Invalid local PK: %v\n", err)) setAppErr(appCl, err) os.Exit(1) } @@ -61,7 +61,7 @@ func main() { localSK := cipher.SecKey{} if *localSKStr != "" { if err := localSK.UnmarshalText([]byte(*localSKStr)); err != nil { - print(fmt.Sprintf("Invalid local SK: %v", err)) + print(fmt.Sprintf("Invalid local SK: %v\n", err)) setAppErr(appCl, err) os.Exit(1) } @@ -76,7 +76,7 @@ func main() { l, err := appCl.Listen(netType, vpnPort) if err != nil { - print(fmt.Sprintf("Error listening network %v on port %d: %v", netType, vpnPort, err)) + print(fmt.Sprintf("Error listening network %v on port %d: %v\n", netType, vpnPort, err)) setAppErr(appCl, err) os.Exit(1) } @@ -90,13 +90,13 @@ func main() { } srv, err := vpn.NewServer(srvCfg, appCl) if err != nil { - print(fmt.Sprintf("Error creating VPN server: %v", err)) + print(fmt.Sprintf("Error creating VPN server: %v\n", err)) setAppErr(appCl, err) os.Exit(1) } defer func() { if err := srv.Close(); err != nil { - print(fmt.Sprintf("Error closing server: %v", err)) + print(fmt.Sprintf("Error closing server: %v\n", err)) } }() @@ -112,12 +112,12 @@ func main() { select { case <-osSigs: case err := <-errCh: - print(fmt.Sprintf("Error serving: %v", err)) + print(fmt.Sprintf("Error serving: %v\n", err)) } } func setAppErr(appCl *app.Client, err error) { if appErr := appCl.SetError(err.Error()); appErr != nil { - fmt.Printf("Failed to set error %v: %v\n", err, appErr) + print(fmt.Sprintf("Failed to set error %v: %v\n", err, appErr)) } } diff --git a/internal/skysocks/client.go b/internal/skysocks/client.go index 3de64be714..04b8097096 100644 --- a/internal/skysocks/client.go +++ b/internal/skysocks/client.go @@ -145,7 +145,7 @@ func (c *Client) handleStream(conn, stream net.Conn) { } if err != nil { - print(fmt.Sprintf("Copy error: %v", err)) + print(fmt.Sprintf("Copy error: %v\n", err)) } } @@ -159,7 +159,7 @@ func (c *Client) handleStream(conn, stream net.Conn) { func (c *Client) close() { print("Session failed, closing skysocks client") if err := c.Close(); err != nil { - print(fmt.Sprintf("Error closing skysocks client: %v", err)) + print(fmt.Sprintf("Error closing skysocks client: %v\n", err)) } } @@ -168,7 +168,7 @@ func (c *Client) ListenIPC(client *ipc.Client) { listenIPC(client, skyenv.SkychatName+"-client", func() { client.Close() if err := c.Close(); err != nil { - print(fmt.Sprintf("Error closing skysocks-client: %v", err)) + print(fmt.Sprintf("Error closing skysocks-client: %v\n", err)) } }) } diff --git a/internal/skysocks/server.go b/internal/skysocks/server.go index 4d9e09ac28..76fee6de3c 100644 --- a/internal/skysocks/server.go +++ b/internal/skysocks/server.go @@ -84,7 +84,7 @@ func (s *Server) Serve(l net.Listener) error { go func() { if err := s.socks.Serve(session); err != nil { - print(fmt.Sprintf("Failed to start SOCKS5 server: %v", err)) + print(fmt.Sprintf("Failed to start SOCKS5 server: %v\n", err)) } }() } diff --git a/internal/vpn/client.go b/internal/vpn/client.go index 05f0d78d02..cbdf8d2e49 100644 --- a/internal/vpn/client.go +++ b/internal/vpn/client.go @@ -233,7 +233,7 @@ func (c *Client) ListenIPC(client *ipc.Client) { if m != nil { if m.MsgType == skyenv.IPCShutdownMessageType { - print(fmt.Sprintln("Stopping " + skyenv.VPNClientName + " via IPC")) + fmt.Println("Stopping " + skyenv.VPNClientName + " via IPC") break } } diff --git a/internal/vpn/server.go b/internal/vpn/server.go index 4d9ab7decb..47de6fee99 100644 --- a/internal/vpn/server.go +++ b/internal/vpn/server.go @@ -175,7 +175,7 @@ func (s *Server) Close() error { func (s *Server) revertIPv4ForwardingValue() { if err := SetIPv4ForwardingValue(s.ipv4ForwardingVal); err != nil { - print(fmt.Sprintf("Error reverting IPv4 forwarding: %v", err)) + print(fmt.Sprintf("Error reverting IPv4 forwarding: %v\n", err)) } else { fmt.Printf("Set IPv4 forwarding = %s\n", s.ipv4ForwardingVal) } @@ -183,7 +183,7 @@ func (s *Server) revertIPv4ForwardingValue() { func (s *Server) revertIPv6ForwardingValue() { if err := SetIPv6ForwardingValue(s.ipv6ForwardingVal); err != nil { - print(fmt.Sprintf("Error reverting IPv6 forwarding: %v", err)) + print(fmt.Sprintf("Error reverting IPv6 forwarding: %v\n", err)) } else { fmt.Printf("Set IPv6 forwarding = %s\n", s.ipv6ForwardingVal) } @@ -191,7 +191,7 @@ func (s *Server) revertIPv6ForwardingValue() { func (s *Server) disableIPMasquerading() { if err := DisableIPMasquerading(s.defaultNetworkInterface); err != nil { - print(fmt.Sprintf("Error disabling IP masquerading for %s: %v", s.defaultNetworkInterface, err)) + print(fmt.Sprintf("Error disabling IP masquerading for %s: %v\n", s.defaultNetworkInterface, err)) } else { fmt.Printf("Disabled IP masquerading for %s\n", s.defaultNetworkInterface) } @@ -199,7 +199,7 @@ func (s *Server) disableIPMasquerading() { func (s *Server) restoreIPTablesForwardPolicy() { if err := SetIPTablesForwardPolicy(s.iptablesForwardPolicy); err != nil { - print(fmt.Sprintf("Error restoring iptables forward policy to %s: %v", s.iptablesForwardPolicy, err)) + print(fmt.Sprintf("Error restoring iptables forward policy to %s: %v\n", s.iptablesForwardPolicy, err)) } else { fmt.Printf("Restored iptables forward policy to %s\n", s.iptablesForwardPolicy) } @@ -207,7 +207,7 @@ func (s *Server) restoreIPTablesForwardPolicy() { func (s *Server) closeConn(conn net.Conn) { if err := conn.Close(); err != nil { - print(fmt.Sprintf("Error closing client %s connection: %v", conn.RemoteAddr(), err)) + print(fmt.Sprintf("Error closing client %s connection: %v\n", conn.RemoteAddr(), err)) } } @@ -216,26 +216,26 @@ func (s *Server) serveConn(conn net.Conn) { tunIP, tunGateway, allowTrafficToLocalNet, err := s.shakeHands(conn) if err != nil { - print(fmt.Sprintf("Error negotiating with client %s: %v", conn.RemoteAddr(), err)) + print(fmt.Sprintf("Error negotiating with client %s: %v\n", conn.RemoteAddr(), err)) return } defer allowTrafficToLocalNet() tun, err := newTUNDevice() if err != nil { - print(fmt.Sprintf("Error allocating TUN interface: %v", err)) + print(fmt.Sprintf("Error allocating TUN interface: %v\n", err)) return } defer func() { if err := tun.Close(); err != nil { - print(fmt.Sprintf("Error closing TUN %s: %v", tun.Name(), err)) + print(fmt.Sprintf("Error closing TUN %s: %v\n", tun.Name(), err)) } }() fmt.Printf("Allocated TUN %s", tun.Name()) if err := SetupTUN(tun.Name(), tunIP.String()+TUNNetmaskCIDR, tunGateway.String(), TUNMTU); err != nil { - print(fmt.Sprintf("Error setting up TUN %s: %v", tun.Name(), err)) + print(fmt.Sprintf("Error setting up TUN %s: %v\n", tun.Name(), err)) return } @@ -245,14 +245,14 @@ func (s *Server) serveConn(conn net.Conn) { defer close(connToTunDoneCh) if _, err := io.Copy(tun, conn); err != nil { - print(fmt.Sprintf("Error resending traffic from VPN client to TUN %s: %v", tun.Name(), err)) + print(fmt.Sprintf("Error resending traffic from VPN client to TUN %s: %v\n", tun.Name(), err)) } }() go func() { defer close(tunToConnCh) if _, err := io.Copy(conn, tun); err != nil { - print(fmt.Sprintf("Error resending traffic from TUN %s to VPN client: %v", tun.Name(), err)) + print(fmt.Sprintf("Error resending traffic from TUN %s to VPN client: %v\n", tun.Name(), err)) } }() @@ -322,7 +322,7 @@ func (s *Server) shakeHands(conn net.Conn) (tunIP, tunGateway net.IP, unsecureVP unsecureVPN = func() { if err := AllowIPToLocalNetwork(cTUNIP, sTUNIP); err != nil { - print(fmt.Sprintf("Error allowing traffic to local network: %v", err)) + print(fmt.Sprintf("Error allowing traffic to local network: %v\n", err)) } } } @@ -359,7 +359,7 @@ func (s *Server) sendServerErrHello(conn net.Conn, status HandshakeStatus) { } if err := WriteJSON(conn, &sHello); err != nil { - print(fmt.Sprintf("Error sending server hello: %v", err)) + print(fmt.Sprintf("Error sending server hello: %v\n", err)) } } From a0fa5219f57ab1e26bec23903b6ea952f22ce365 Mon Sep 17 00:00:00 2001 From: ersonp Date: Wed, 22 Jun 2022 18:29:13 +0530 Subject: [PATCH 02/17] Filter false positive app errors This commit filters the false positive app errors by getting the StderrPipe of the cmd and using a *bufio.Scanner to read, filter and print the logs of the apps. --- pkg/app/appserver/proc.go | 31 +++++++++++++++++++---------- pkg/app/appserver/stderr.go | 24 ++++++++++++++++++++++ pkg/app/appserver/stderr_unix.go | 10 ++++++++++ pkg/app/appserver/stderr_windows.go | 13 ++++++++++++ 4 files changed, 68 insertions(+), 10 deletions(-) create mode 100644 pkg/app/appserver/stderr.go create mode 100644 pkg/app/appserver/stderr_unix.go create mode 100644 pkg/app/appserver/stderr_windows.go diff --git a/pkg/app/appserver/proc.go b/pkg/app/appserver/proc.go index 75ff5a4dec..ea4ee6b202 100644 --- a/pkg/app/appserver/proc.go +++ b/pkg/app/appserver/proc.go @@ -3,6 +3,7 @@ package appserver import ( "errors" "fmt" + "io" "net" "net/rpc" "os" @@ -66,6 +67,8 @@ type Proc struct { errMx sync.RWMutex err string + cmdStderr io.ReadCloser + startWg sync.WaitGroup } @@ -86,18 +89,23 @@ func NewProc(mLog *logging.MasterLogger, conf appcommon.ProcConfig, disc appdisc appLog, appLogDB := appcommon.NewProcLogger(conf, mLog) cmd.Stdout = appLog.WithField("_module", moduleName).WithField("func", "(STDOUT)").WriterLevel(logrus.DebugLevel) - cmd.Stderr = appLog.WithField("_module", moduleName).WithField("func", "(STDERR)").WriterLevel(logrus.ErrorLevel) + + // we read the Stderr pipe in order to filter some false positive app errors + errorLog := appLog.WithField("_module", moduleName).WithField("func", "(STDERR)") + stderr, _ := cmd.StderrPipe() //nolint:errcheck + printStdErr(stderr, errorLog) p := &Proc{ - disc: disc, - conf: conf, - log: mLog.PackageLogger(moduleName), - logDB: appLogDB, - cmd: cmd, - connCh: make(chan struct{}, 1), - m: m, - appName: appName, - startWg: sync.WaitGroup{}, + disc: disc, + conf: conf, + log: mLog.PackageLogger(moduleName), + logDB: appLogDB, + cmd: cmd, + connCh: make(chan struct{}, 1), + m: m, + appName: appName, + startWg: sync.WaitGroup{}, + cmdStderr: stderr, } if runtime.GOOS == "windows" { @@ -293,6 +301,9 @@ func (p *Proc) Stop() error { if p.ipcServer != nil { p.ipcServer.Close() } + if p.cmdStderr != nil { + _ = p.cmdStderr.Close() //nolint:errcheck + } p.waitMx.Unlock() p.connOnce.Do(func() { close(p.connCh) }) }() diff --git a/pkg/app/appserver/stderr.go b/pkg/app/appserver/stderr.go new file mode 100644 index 0000000000..0453f2bd9b --- /dev/null +++ b/pkg/app/appserver/stderr.go @@ -0,0 +1,24 @@ +package appserver + +import ( + "bufio" + "io" + "strings" + + "github.com/sirupsen/logrus" +) + +func printStdErr(stderr io.ReadCloser, errorLog *logrus.Entry) { + cmdStderr := bufio.NewScanner(stderr) + ignoreErrs := getIgnoreErrs() + go func() { + for cmdStderr.Scan() { + err := cmdStderr.Text() + if _, ok := ignoreErrs[err]; !ok { + if !strings.Contains(err, "rpc.Serve: accept:accept") { + errorLog.Error(cmdStderr.Text()) + } + } + } + }() +} diff --git a/pkg/app/appserver/stderr_unix.go b/pkg/app/appserver/stderr_unix.go new file mode 100644 index 0000000000..a0486f828e --- /dev/null +++ b/pkg/app/appserver/stderr_unix.go @@ -0,0 +1,10 @@ +package appserver + +func getIgnoreErrs() map[string]struct{} { + ignoreErrs := map[string]struct{}{ + "RTNETLINK answers: File exists": {}, + "RTNETLINK answers: Operation not permitted": {}, + "Fatal: can't open lock file /run/xtables.lock: Permission denied": {}, + } + return ignoreErrs +} diff --git a/pkg/app/appserver/stderr_windows.go b/pkg/app/appserver/stderr_windows.go new file mode 100644 index 0000000000..0c95e2efe7 --- /dev/null +++ b/pkg/app/appserver/stderr_windows.go @@ -0,0 +1,13 @@ +//go:build windows +// +build windows + +package appserver + +func getIgnoreErrs() map[string]struct{} { + ignoreErrs := map[string]struct{}{ + "RTNETLINK answers: File exists": {}, + "RTNETLINK answers: Operation not permitted": {}, + "Fatal: can't open lock file /run/xtables.lock: Permission denied": {}, + } + return ignoreErrs +} From c650a70f26f30d27ad55b0821dc8c79135d2b3f3 Mon Sep 17 00:00:00 2001 From: ersonp Date: Wed, 22 Jun 2022 20:15:14 +0530 Subject: [PATCH 03/17] Fix error linting --- internal/vpn/errors.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/vpn/errors.go b/internal/vpn/errors.go index beeedf9de2..eb4b01a179 100644 --- a/internal/vpn/errors.go +++ b/internal/vpn/errors.go @@ -10,15 +10,15 @@ import ( ) var ( - errCouldFindDefaultNetworkGateway = errors.New("Could not find default network gateway") - errHandshakeStatusForbidden = errors.New("Password didn't match") - errHandshakeStatusInternalError = errors.New("Internal server error") - errHandshakeNoFreeIPs = errors.New("No free IPs left to serve") - errHandshakeStatusBadRequest = errors.New("Request was malformed") - errTimeout = errors.New("Internal error: Timeout") + errCouldFindDefaultNetworkGateway = errors.New("could not find default network gateway") + errHandshakeStatusForbidden = errors.New("password didn't match") + errHandshakeStatusInternalError = errors.New("internal server error") + errHandshakeNoFreeIPs = errors.New("no free IPs left to serve") + errHandshakeStatusBadRequest = errors.New("request was malformed") + errTimeout = errors.New("internal error: Timeout") errNotPermited = errors.New("ioctl: operation not permitted") - errVPNServerClosed = errors.New("Vpn-server closed") - errPermissionDenied = errors.New("Permission denied") //nolint + errVPNServerClosed = errors.New("vpn-server closed") + errPermissionDenied = errors.New("permission denied") errNoTransportFound = appserver.RPCErr{ Err: router.ErrNoTransportFound.Error(), From a4c99a6a217e60d52a8eaf335f4bbb5d848e31f4 Mon Sep 17 00:00:00 2001 From: ersonp Date: Wed, 22 Jun 2022 20:15:53 +0530 Subject: [PATCH 04/17] Add permission denied error --- internal/vpn/os_windows.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/vpn/os_windows.go b/internal/vpn/os_windows.go index af4e819eff..109dee6849 100644 --- a/internal/vpn/os_windows.go +++ b/internal/vpn/os_windows.go @@ -58,5 +58,9 @@ func modifyRoutingTable(action, ipCIDR, gateway string) error { } cmd := fmt.Sprintf(modifyRouteCMDFmt, action, ip, netmask, gateway) - return osutil.Run("cmd", "/C", cmd) + err = osutil.Run("cmd", "/C", cmd) + if err != nil { + return errPermissionDenied + } + return nil } From 0ea0ac568ebac2e6a1dd9bf7be9856f2b390aec9 Mon Sep 17 00:00:00 2001 From: ersonp Date: Wed, 22 Jun 2022 20:24:49 +0530 Subject: [PATCH 05/17] Fix stderr for windows --- pkg/app/appserver/stderr.go | 17 ++++++++++++----- pkg/app/appserver/stderr_unix.go | 14 +++++++++----- pkg/app/appserver/stderr_windows.go | 11 ++++++----- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/pkg/app/appserver/stderr.go b/pkg/app/appserver/stderr.go index 0453f2bd9b..684f598153 100644 --- a/pkg/app/appserver/stderr.go +++ b/pkg/app/appserver/stderr.go @@ -10,15 +10,22 @@ import ( func printStdErr(stderr io.ReadCloser, errorLog *logrus.Entry) { cmdStderr := bufio.NewScanner(stderr) - ignoreErrs := getIgnoreErrs() + iErrs := getIgnoreErrs() go func() { for cmdStderr.Scan() { err := cmdStderr.Text() - if _, ok := ignoreErrs[err]; !ok { - if !strings.Contains(err, "rpc.Serve: accept:accept") { - errorLog.Error(cmdStderr.Text()) - } + if !contains(iErrs, err) { + errorLog.Error(cmdStderr.Text()) } } }() } + +func contains(iErrs []string, err string) bool { + for _, iErr := range iErrs { + if strings.Contains(err, iErr) { + return true + } + } + return false +} diff --git a/pkg/app/appserver/stderr_unix.go b/pkg/app/appserver/stderr_unix.go index a0486f828e..731948db4f 100644 --- a/pkg/app/appserver/stderr_unix.go +++ b/pkg/app/appserver/stderr_unix.go @@ -1,10 +1,14 @@ +//go:build !windows +// +build !windows + package appserver -func getIgnoreErrs() map[string]struct{} { - ignoreErrs := map[string]struct{}{ - "RTNETLINK answers: File exists": {}, - "RTNETLINK answers: Operation not permitted": {}, - "Fatal: can't open lock file /run/xtables.lock: Permission denied": {}, +func getIgnoreErrs() []string { + ignoreErrs := []string{ + "RTNETLINK answers: File exists", + "RTNETLINK answers: Operation not permitted", + "Fatal: can't open lock file /run/xtables.lock: Permission denied", + "rpc.Serve: accept:accept", } return ignoreErrs } diff --git a/pkg/app/appserver/stderr_windows.go b/pkg/app/appserver/stderr_windows.go index 0c95e2efe7..87dd86c753 100644 --- a/pkg/app/appserver/stderr_windows.go +++ b/pkg/app/appserver/stderr_windows.go @@ -3,11 +3,12 @@ package appserver -func getIgnoreErrs() map[string]struct{} { - ignoreErrs := map[string]struct{}{ - "RTNETLINK answers: File exists": {}, - "RTNETLINK answers: Operation not permitted": {}, - "Fatal: can't open lock file /run/xtables.lock: Permission denied": {}, +func getIgnoreErrs() []string { + ignoreErrs := []string{ + "Creating adapter", + "Using existing driver 0.14", + "rpc.Serve: accept:accept", + "The route addition failed: The object already exists.", } return ignoreErrs } From e756f51a1aa44e88daf00e9e9d6f63fd3c53ec6d Mon Sep 17 00:00:00 2001 From: ersonp Date: Thu, 23 Jun 2022 09:18:30 +0530 Subject: [PATCH 06/17] Darwin errPermissionDenied --- internal/vpn/os_darwin.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/vpn/os_darwin.go b/internal/vpn/os_darwin.go index c22c7b2502..d8e70a6765 100644 --- a/internal/vpn/os_darwin.go +++ b/internal/vpn/os_darwin.go @@ -55,5 +55,10 @@ func modifyRoutingTable(action, ipCIDR, gateway string) error { return fmt.Errorf("error parsing IP CIDR: %w", err) } - return osutil.Run("route", action, "-net", ip, gateway, netmask) + err = osutil.Run("route", action, "-net", ip, gateway, netmask) + if err != nil { + print(fmt.Sprintf("%v\n", err)) + return errPermissionDenied + } + return nil } From e7f4c2d247fb0fc79c65d29165bcb5a4f5010e33 Mon Sep 17 00:00:00 2001 From: ersonp Date: Fri, 24 Jun 2022 14:02:11 +0530 Subject: [PATCH 07/17] Add todo --- pkg/app/appserver/stderr.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/app/appserver/stderr.go b/pkg/app/appserver/stderr.go index 684f598153..37218413c2 100644 --- a/pkg/app/appserver/stderr.go +++ b/pkg/app/appserver/stderr.go @@ -8,6 +8,8 @@ import ( "github.com/sirupsen/logrus" ) +// TODO(ersonp): check if we can get rid of the errors altogether instead of ignoring/suppressing them. + func printStdErr(stderr io.ReadCloser, errorLog *logrus.Entry) { cmdStderr := bufio.NewScanner(stderr) iErrs := getIgnoreErrs() From 679a274acf16d1f11d58d77fe57b75df1b052ee1 Mon Sep 17 00:00:00 2001 From: ersonp Date: Tue, 28 Jun 2022 13:00:16 +0530 Subject: [PATCH 08/17] Add debug log to routing Table --- pkg/router/route_group_test.go | 2 +- pkg/router/router.go | 2 +- pkg/routing/table.go | 8 ++++++-- pkg/routing/table_test.go | 2 +- pkg/visor/rpc_client.go | 2 +- 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/pkg/router/route_group_test.go b/pkg/router/route_group_test.go index c3eba2c0f0..cbfa7d1cf4 100644 --- a/pkg/router/route_group_test.go +++ b/pkg/router/route_group_test.go @@ -32,7 +32,7 @@ func TestRouteGroup_RemoteAddr(t *testing.T) { func createRouteGroup(cfg *RouteGroupConfig) *RouteGroup { l := logging.NewMasterLogger() - rt := routing.NewTable() + rt := routing.NewTable(l.PackageLogger("rgt")) pk1, _ := cipher.GenerateKeyPair() pk2, _ := cipher.GenerateKeyPair() diff --git a/pkg/router/router.go b/pkg/router/router.go index a78422e2a1..59fc2f576f 100644 --- a/pkg/router/router.go +++ b/pkg/router/router.go @@ -193,7 +193,7 @@ func New(dmsgC *dmsg.Client, config *Config, routeSetupHooks []RouteSetupHook) ( logger: config.Logger, mLogger: config.MasterLogger, tm: config.TransportManager, - rt: routing.NewTable(), + rt: routing.NewTable(config.Logger), sl: sl, dmsgC: dmsgC, rgsNs: make(map[routing.RouteDescriptor]*NoiseRouteGroup), diff --git a/pkg/routing/table.go b/pkg/routing/table.go index ad2201ee70..02d9e3b973 100644 --- a/pkg/routing/table.go +++ b/pkg/routing/table.go @@ -6,6 +6,8 @@ import ( "math" "sync" "time" + + "github.com/skycoin/skywire-utilities/pkg/logging" ) var ( @@ -52,13 +54,15 @@ type memTable struct { nextID RouteID rules map[RouteID]Rule activity map[RouteID]time.Time + log *logging.Logger } // NewTable returns an in-memory routing table implementation with a specified configuration. -func NewTable() Table { +func NewTable(log *logging.Logger) Table { mt := &memTable{ rules: map[RouteID]Rule{}, activity: make(map[RouteID]time.Time), + log: log, } return mt @@ -101,7 +105,7 @@ func (mt *memTable) SaveRule(rule Rule) error { defer mt.Unlock() mt.rules[key] = rule - fmt.Printf("ROUTING TABLE CONTENTS: %v\n", mt.rules) + mt.log.Debugf("ROUTING TABLE CONTENTS: %v", mt.rules) mt.activity[key] = now return nil diff --git a/pkg/routing/table_test.go b/pkg/routing/table_test.go index 5704486959..cae6a35f13 100644 --- a/pkg/routing/table_test.go +++ b/pkg/routing/table_test.go @@ -71,5 +71,5 @@ func RoutingTableSuite(t *testing.T, tbl Table) { } func TestRoutingTable(t *testing.T) { - RoutingTableSuite(t, NewTable()) + RoutingTableSuite(t, NewTable(logging.MustGetLogger("tt"))) } diff --git a/pkg/visor/rpc_client.go b/pkg/visor/rpc_client.go index c9e4bba162..1922d19d1e 100644 --- a/pkg/visor/rpc_client.go +++ b/pkg/visor/rpc_client.go @@ -432,7 +432,7 @@ func NewMockRPCClient(r *rand.Rand, maxTps int, maxRules int) (cipher.PubKey, AP log.Infof("tp[%2d]: %v", i, tps[i]) } - rt := routing.NewTable() + rt := routing.NewTable(log) ruleKeepAlive := router.DefaultRouteKeepAlive for i := 0; i < r.Intn(maxRules+1); i++ { From 35d7853995f75e3565429a2744b0619af9584e4a Mon Sep 17 00:00:00 2001 From: ersonp Date: Tue, 28 Jun 2022 13:51:44 +0530 Subject: [PATCH 09/17] Add app start and stop info logs This commit adds info logs to app start and stopped detailed state. We add the new detailed state AppDetailedStatusStopped to every app on shutdown. The app_state is also moved from package launcher to package appserver to get around the cyclic dependency so that we can use actual Detailed status instead of strings in Proc in the package appserver. --- cmd/apps/skychat/skychat.go | 17 +++++- cmd/apps/skysocks-client/skysocks-client.go | 8 +-- cmd/apps/skysocks/skysocks.go | 20 ++++++- cmd/apps/vpn-client/vpn-client.go | 9 +++ cmd/apps/vpn-server/vpn-server.go | 9 +++ cmd/skywire-cli/commands/config/gen.go | 4 +- cmd/skywire-cli/commands/visor/vapps/app.go | 6 +- internal/skysocks/client.go | 6 +- internal/skysocks/server.go | 6 +- internal/vpn/client.go | 14 ++--- internal/vpn/server.go | 8 +-- pkg/app/{launcher => appserver}/app_state.go | 15 ++++- pkg/app/appserver/proc.go | 6 +- pkg/app/launcher/launcher.go | 43 ++++++--------- pkg/visor/api.go | 32 +++++------ pkg/visor/hypervisor.go | 8 +-- pkg/visor/rpc.go | 3 +- pkg/visor/rpc_client.go | 23 ++++---- pkg/visor/visorconfig/config.go | 6 +- pkg/visor/visorconfig/v1.go | 19 ++++--- pkg/visor/visorconfig/v1_test.go | 58 ++++++++++---------- 21 files changed, 183 insertions(+), 137 deletions(-) rename pkg/app/{launcher => appserver}/app_state.go (76%) diff --git a/cmd/apps/skychat/skychat.go b/cmd/apps/skychat/skychat.go index afb3e10b1e..e52a6b09a6 100644 --- a/cmd/apps/skychat/skychat.go +++ b/cmd/apps/skychat/skychat.go @@ -13,6 +13,7 @@ import ( "net" "net/http" "os" + "os/signal" "runtime" "sync" "time" @@ -24,7 +25,7 @@ import ( "github.com/skycoin/skywire-utilities/pkg/netutil" "github.com/skycoin/skywire/pkg/app" "github.com/skycoin/skywire/pkg/app/appnet" - "github.com/skycoin/skywire/pkg/app/launcher" + "github.com/skycoin/skywire/pkg/app/appserver" "github.com/skycoin/skywire/pkg/routing" "github.com/skycoin/skywire/pkg/skyenv" ) @@ -83,8 +84,17 @@ func main() { http.HandleFunc("/sse", sseHandler) fmt.Println("Serving HTTP on", *addr) + if runtime.GOOS != "windows" { + termCh := make(chan os.Signal, 1) + signal.Notify(termCh, os.Interrupt) - setAppStatus(appCl, launcher.AppDetailedStatusRunning) + go func() { + <-termCh + setAppStatus(appCl, appserver.AppDetailedStatusStopped) + os.Exit(1) + }() + } + setAppStatus(appCl, appserver.AppDetailedStatusRunning) err := http.ListenAndServe(*addr, nil) if err != nil { @@ -92,6 +102,7 @@ func main() { setAppError(appCl, err) os.Exit(1) } + } func listenLoop() { @@ -253,7 +264,7 @@ func handleIPCSignal(client *ipc.Client) { os.Exit(0) } -func setAppStatus(appCl *app.Client, status launcher.AppDetailedStatus) { +func setAppStatus(appCl *app.Client, status appserver.AppDetailedStatus) { if err := appCl.SetDetailedStatus(string(status)); err != nil { print(fmt.Sprintf("Failed to set status %v: %v\n", status, err)) } diff --git a/cmd/apps/skysocks-client/skysocks-client.go b/cmd/apps/skysocks-client/skysocks-client.go index f829516d8c..55a7f048e0 100644 --- a/cmd/apps/skysocks-client/skysocks-client.go +++ b/cmd/apps/skysocks-client/skysocks-client.go @@ -19,7 +19,7 @@ import ( "github.com/skycoin/skywire/internal/skysocks" "github.com/skycoin/skywire/pkg/app" "github.com/skycoin/skywire/pkg/app/appnet" - "github.com/skycoin/skywire/pkg/app/launcher" + "github.com/skycoin/skywire/pkg/app/appserver" "github.com/skycoin/skywire/pkg/routing" "github.com/skycoin/skywire/pkg/skyenv" ) @@ -77,7 +77,7 @@ func main() { setAppErr(appCl, err) os.Exit(1) } - + defer setAppStatus(appCl, appserver.AppDetailedStatusStopped) for { conn, err := dialServer(ctx, appCl, pk) if err != nil { @@ -107,7 +107,7 @@ func main() { } fmt.Println("Reconnecting to skysocks server") - setAppStatus(appCl, launcher.AppDetailedStatusReconnecting) + setAppStatus(appCl, appserver.AppDetailedStatusReconnecting) } } @@ -117,7 +117,7 @@ func setAppErr(appCl *app.Client, err error) { } } -func setAppStatus(appCl *app.Client, status launcher.AppDetailedStatus) { +func setAppStatus(appCl *app.Client, status appserver.AppDetailedStatus) { if err := appCl.SetDetailedStatus(string(status)); err != nil { print(fmt.Sprintf("Failed to set status %v: %v\n", status, err)) } diff --git a/cmd/apps/skysocks/skysocks.go b/cmd/apps/skysocks/skysocks.go index bae0ba3e72..824215ab8e 100644 --- a/cmd/apps/skysocks/skysocks.go +++ b/cmd/apps/skysocks/skysocks.go @@ -6,7 +6,6 @@ package main import ( "flag" "fmt" - "log" "os" "os/signal" "runtime" @@ -17,6 +16,7 @@ import ( "github.com/skycoin/skywire/internal/skysocks" "github.com/skycoin/skywire/pkg/app" "github.com/skycoin/skywire/pkg/app/appnet" + "github.com/skycoin/skywire/pkg/app/appserver" "github.com/skycoin/skywire/pkg/routing" "github.com/skycoin/skywire/pkg/skyenv" ) @@ -39,14 +39,15 @@ func main() { srv, err := skysocks.NewServer(*passcode, appCl) if err != nil { + setAppError(appCl, err) print(fmt.Sprintf("Failed to create a new server: %v\n", err)) os.Exit(1) } l, err := appCl.Listen(netType, port) if err != nil { + setAppError(appCl, err) print(fmt.Sprintf("Error listening network %v on port %d: %v\n", netType, port, err)) - log.Fatalf("Error listening network %v on port %d: %v\n", netType, port, err) os.Exit(1) } @@ -55,6 +56,7 @@ func main() { if runtime.GOOS == "windows" { ipcClient, err := ipc.StartClient(skyenv.VPNClientName, nil) if err != nil { + setAppError(appCl, err) print(fmt.Sprintf("Error creating ipc server for VPN client: %v\n", err)) os.Exit(1) } @@ -71,11 +73,23 @@ func main() { os.Exit(1) } }() - } + defer setAppStatus(appCl, appserver.AppDetailedStatusStopped) if err := srv.Serve(l); err != nil { print(fmt.Sprintf("%v\n", err)) os.Exit(1) } } + +func setAppStatus(appCl *app.Client, status appserver.AppDetailedStatus) { + if err := appCl.SetDetailedStatus(string(status)); err != nil { + print(fmt.Sprintf("Failed to set status %v: %v\n", status, err)) + } +} + +func setAppError(appCl *app.Client, appErr error) { + if err := appCl.SetError(appErr.Error()); err != nil { + print(fmt.Sprintf("Failed to set error %v: %v\n", appErr, err)) + } +} diff --git a/cmd/apps/vpn-client/vpn-client.go b/cmd/apps/vpn-client/vpn-client.go index 2f8da92287..a71908c4a5 100644 --- a/cmd/apps/vpn-client/vpn-client.go +++ b/cmd/apps/vpn-client/vpn-client.go @@ -17,6 +17,7 @@ import ( "github.com/skycoin/skywire/internal/vpn" "github.com/skycoin/skywire/pkg/app" "github.com/skycoin/skywire/pkg/app/appevent" + "github.com/skycoin/skywire/pkg/app/appserver" "github.com/skycoin/skywire/pkg/skyenv" ) @@ -170,6 +171,8 @@ func main() { go vpnClient.ListenIPC(ipcClient) } + defer setAppStatus(appCl, appserver.AppDetailedStatusStopped) + if err := vpnClient.Serve(); err != nil { print(fmt.Sprintf("Failed to serve VPN: %v\n", err)) } @@ -180,3 +183,9 @@ func setAppErr(appCl *app.Client, err error) { print(fmt.Sprintf("Failed to set error %v: %v\n", err, appErr)) } } + +func setAppStatus(appCl *app.Client, status appserver.AppDetailedStatus) { + if err := appCl.SetDetailedStatus(string(status)); err != nil { + print(fmt.Sprintf("Failed to set status %v: %v\n", status, err)) + } +} diff --git a/cmd/apps/vpn-server/vpn-server.go b/cmd/apps/vpn-server/vpn-server.go index 43616df989..24a871ec30 100644 --- a/cmd/apps/vpn-server/vpn-server.go +++ b/cmd/apps/vpn-server/vpn-server.go @@ -14,6 +14,7 @@ import ( "github.com/skycoin/skywire/internal/vpn" "github.com/skycoin/skywire/pkg/app" "github.com/skycoin/skywire/pkg/app/appnet" + "github.com/skycoin/skywire/pkg/app/appserver" "github.com/skycoin/skywire/pkg/routing" "github.com/skycoin/skywire/pkg/skyenv" ) @@ -109,6 +110,8 @@ func main() { close(errCh) }() + defer setAppStatus(appCl, appserver.AppDetailedStatusStopped) + select { case <-osSigs: case err := <-errCh: @@ -121,3 +124,9 @@ func setAppErr(appCl *app.Client, err error) { print(fmt.Sprintf("Failed to set error %v: %v\n", err, appErr)) } } + +func setAppStatus(appCl *app.Client, status appserver.AppDetailedStatus) { + if err := appCl.SetDetailedStatus(string(status)); err != nil { + print(fmt.Sprintf("Failed to set status %v: %v\n", status, err)) + } +} diff --git a/cmd/skywire-cli/commands/config/gen.go b/cmd/skywire-cli/commands/config/gen.go index 2dd3c2e698..d8fe1445e3 100644 --- a/cmd/skywire-cli/commands/config/gen.go +++ b/cmd/skywire-cli/commands/config/gen.go @@ -17,7 +17,7 @@ import ( "github.com/skycoin/skywire-utilities/pkg/logging" "github.com/skycoin/skywire-utilities/pkg/netutil" utilenv "github.com/skycoin/skywire-utilities/pkg/skyenv" - "github.com/skycoin/skywire/pkg/app/launcher" + "github.com/skycoin/skywire/pkg/app/appserver" "github.com/skycoin/skywire/pkg/skyenv" "github.com/skycoin/skywire/pkg/visor/visorconfig" ) @@ -353,7 +353,7 @@ var genConfigCmd = &cobra.Command{ for _, app := range apps { appsSlice[app] = true } - var newConfLauncherApps []launcher.AppConfig + var newConfLauncherApps []appserver.AppConfig for _, app := range conf.Launcher.Apps { if _, ok := appsSlice[app.Name]; !ok { newConfLauncherApps = append(newConfLauncherApps, app) diff --git a/cmd/skywire-cli/commands/visor/vapps/app.go b/cmd/skywire-cli/commands/visor/vapps/app.go index c8bad0080c..889c5fc1af 100644 --- a/cmd/skywire-cli/commands/visor/vapps/app.go +++ b/cmd/skywire-cli/commands/visor/vapps/app.go @@ -10,7 +10,7 @@ import ( "github.com/spf13/cobra" "github.com/skycoin/skywire/cmd/skywire-cli/internal" - "github.com/skycoin/skywire/pkg/app/launcher" + "github.com/skycoin/skywire/pkg/app/appserver" ) func init() { @@ -37,10 +37,10 @@ var lsAppsCmd = &cobra.Command{ for _, state := range states { status := "stopped" - if state.Status == launcher.AppStatusRunning { + if state.Status == appserver.AppStatusRunning { status = "running" } - if state.Status == launcher.AppStatusErrored { + if state.Status == appserver.AppStatusErrored { status = "errored" } _, err = fmt.Fprintf(w, "%s\t%s\t%t\t%s\n", state.Name, strconv.Itoa(int(state.Port)), state.AutoStart, status) diff --git a/internal/skysocks/client.go b/internal/skysocks/client.go index 04b8097096..753d77b5fa 100644 --- a/internal/skysocks/client.go +++ b/internal/skysocks/client.go @@ -11,7 +11,7 @@ import ( "github.com/skycoin/yamux" "github.com/skycoin/skywire/pkg/app" - "github.com/skycoin/skywire/pkg/app/launcher" + "github.com/skycoin/skywire/pkg/app/appserver" "github.com/skycoin/skywire/pkg/router" "github.com/skycoin/skywire/pkg/skyenv" ) @@ -61,7 +61,7 @@ func (c *Client) ListenAndServe(addr string) error { c.listener = l if c.appCl != nil { - c.setAppStatus(launcher.AppDetailedStatusRunning) + c.setAppStatus(appserver.AppDetailedStatusRunning) } for { @@ -173,7 +173,7 @@ func (c *Client) ListenIPC(client *ipc.Client) { }) } -func (c *Client) setAppStatus(status launcher.AppDetailedStatus) { +func (c *Client) setAppStatus(status appserver.AppDetailedStatus) { if err := c.appCl.SetDetailedStatus(string(status)); err != nil { print(fmt.Sprintf("Failed to set status %v: %v\n", status, err)) } diff --git a/internal/skysocks/server.go b/internal/skysocks/server.go index 76fee6de3c..723e324798 100644 --- a/internal/skysocks/server.go +++ b/internal/skysocks/server.go @@ -12,7 +12,7 @@ import ( "github.com/skycoin/yamux" "github.com/skycoin/skywire/pkg/app" - "github.com/skycoin/skywire/pkg/app/launcher" + "github.com/skycoin/skywire/pkg/app/appserver" "github.com/skycoin/skywire/pkg/skyenv" ) @@ -53,7 +53,7 @@ func (s *Server) Serve(l net.Listener) error { s.sMu.Unlock() if s.appCl != nil { - s.setAppStatus(launcher.AppDetailedStatusRunning) + s.setAppStatus(appserver.AppDetailedStatusRunning) } for { @@ -101,7 +101,7 @@ func (s *Server) ListenIPC(client *ipc.Client) { }) } -func (s *Server) setAppStatus(status launcher.AppDetailedStatus) { +func (s *Server) setAppStatus(status appserver.AppDetailedStatus) { if err := s.appCl.SetDetailedStatus(string(status)); err != nil { print(fmt.Sprintf("Failed to set status %v: %v\n", status, err)) } diff --git a/internal/vpn/client.go b/internal/vpn/client.go index cbdf8d2e49..98bf6bd088 100644 --- a/internal/vpn/client.go +++ b/internal/vpn/client.go @@ -19,7 +19,7 @@ import ( "github.com/skycoin/skywire-utilities/pkg/netutil" "github.com/skycoin/skywire/pkg/app" "github.com/skycoin/skywire/pkg/app/appnet" - "github.com/skycoin/skywire/pkg/app/launcher" + "github.com/skycoin/skywire/pkg/app/appserver" "github.com/skycoin/skywire/pkg/routing" "github.com/skycoin/skywire/pkg/skyenv" ) @@ -128,7 +128,7 @@ func NewClient(cfg ClientConfig, appCl *app.Client) (*Client, error) { // Serve dials VPN server, sets up TUN and establishes VPN session. func (c *Client) Serve() error { - c.setAppStatus(launcher.AppDetailedStatusStarting) + c.setAppStatus(appserver.AppDetailedStatusStarting) if err := c.setSysPrivileges(); err != nil { c.setAppError(err) return fmt.Errorf("failed to setup system privileges: %w", err) @@ -179,11 +179,11 @@ func (c *Client) Serve() error { }() defer func() { - c.setAppStatus(launcher.AppDetailedStatusShuttingDown) + c.setAppStatus(appserver.AppDetailedStatusShuttingDown) c.resetConnDuration() }() - c.setAppStatus(launcher.AppDetailedStatusVPNConnecting) + c.setAppStatus(appserver.AppDetailedStatusVPNConnecting) r := netutil.NewRetrier(nil, netutil.DefaultInitBackoff, netutil.DefaultMaxBackoff, 3, netutil.DefaultFactor). WithErrWhitelist(errHandshakeStatusForbidden, errHandshakeStatusInternalError, errHandshakeNoFreeIPs, @@ -203,7 +203,7 @@ func (c *Client) Serve() error { return err default: c.resetConnDuration() - c.setAppStatus(launcher.AppDetailedStatusReconnecting) + c.setAppStatus(appserver.AppDetailedStatusReconnecting) c.setAppError(errTimeout) fmt.Println("\nConnection broke, reconnecting...") return fmt.Errorf("dialServeConn: %w", err) @@ -427,7 +427,7 @@ func (c *Client) serveConn(conn net.Conn) error { return fmt.Errorf("error routing traffic through TUN %s: %w", tun.Name(), err) } - c.setAppStatus(launcher.AppDetailedStatusRunning) + c.setAppStatus(appserver.AppDetailedStatusRunning) c.resetConnDuration() t := time.NewTicker(time.Second) @@ -763,7 +763,7 @@ func (c *Client) dialServer(appCl *app.Client, pk cipher.PubKey) (net.Conn, erro return conn, nil } -func (c *Client) setAppStatus(status launcher.AppDetailedStatus) { +func (c *Client) setAppStatus(status appserver.AppDetailedStatus) { if err := c.appCl.SetDetailedStatus(string(status)); err != nil { print(fmt.Sprintf("Failed to set status %v: %v\n", status, err)) } diff --git a/internal/vpn/server.go b/internal/vpn/server.go index 47de6fee99..5808e42e97 100644 --- a/internal/vpn/server.go +++ b/internal/vpn/server.go @@ -10,7 +10,7 @@ import ( "github.com/skycoin/skywire-utilities/pkg/netutil" "github.com/skycoin/skywire/pkg/app" - "github.com/skycoin/skywire/pkg/app/launcher" + "github.com/skycoin/skywire/pkg/app/appserver" ) // Server is a VPN server. @@ -94,7 +94,7 @@ func NewServer(cfg ServerConfig, appCl *app.Client) (*Server, error) { func (s *Server) Serve(l net.Listener) error { serveErr := errors.New("already serving") s.serveOnce.Do(func() { - s.setAppStatus(launcher.AppDetailedStatusStarting) + s.setAppStatus(appserver.AppDetailedStatusStarting) if err := EnableIPv4Forwarding(); err != nil { serveErr = fmt.Errorf("error enabling IPv4 forwarding: %w", err) return @@ -137,7 +137,7 @@ func (s *Server) Serve(l net.Listener) error { s.lisMx.Lock() s.lis = l s.lisMx.Unlock() - s.setAppStatus(launcher.AppDetailedStatusRunning) + s.setAppStatus(appserver.AppDetailedStatusRunning) for { conn, err := s.lis.Accept() if err != nil { @@ -341,7 +341,7 @@ func (s *Server) shakeHands(conn net.Conn) (tunIP, tunGateway net.IP, unsecureVP return sTUNIP, sTUNGateway, unsecureVPN, nil } -func (s *Server) setAppStatus(status launcher.AppDetailedStatus) { +func (s *Server) setAppStatus(status appserver.AppDetailedStatus) { if err := s.appCl.SetDetailedStatus(string(status)); err != nil { fmt.Printf("Failed to set status %v: %v\n", status, err) } diff --git a/pkg/app/launcher/app_state.go b/pkg/app/appserver/app_state.go similarity index 76% rename from pkg/app/launcher/app_state.go rename to pkg/app/appserver/app_state.go index ea457fc7fd..e5aea25f39 100644 --- a/pkg/app/launcher/app_state.go +++ b/pkg/app/appserver/app_state.go @@ -1,4 +1,6 @@ -package launcher +package appserver + +import "github.com/skycoin/skywire/pkg/routing" // AppStatus defines running status of an App. type AppStatus int @@ -17,6 +19,14 @@ const ( AppStatusStarting ) +// AppConfig defines app startup parameters. +type AppConfig struct { + Name string `json:"name"` + Args []string `json:"args,omitempty"` + AutoStart bool `json:"auto_start"` + Port routing.Port `json:"port"` +} + // AppState defines state parameters for a registered App. type AppState struct { AppConfig @@ -42,4 +52,7 @@ const ( // AppDetailedStatusShuttingDown is set during shutdown. AppDetailedStatusShuttingDown = "Shutting down" + + // AppDetailedStatusStopped is set after shutdown. + AppDetailedStatusStopped = "Stopped" ) diff --git a/pkg/app/appserver/proc.go b/pkg/app/appserver/proc.go index ea4ee6b202..5fb8c531b3 100644 --- a/pkg/app/appserver/proc.go +++ b/pkg/app/appserver/proc.go @@ -333,10 +333,14 @@ func (p *Proc) IsRunning() bool { func (p *Proc) SetDetailedStatus(status string) { p.statusMx.Lock() defer p.statusMx.Unlock() - if status == "Running" { + if status == AppDetailedStatusRunning { p.startWg.Done() } + if status == AppDetailedStatusRunning || status == AppDetailedStatusStopped { + p.log.Infof("App %v is %v", p.appName, status) + } + p.status = status } diff --git a/pkg/app/launcher/launcher.go b/pkg/app/launcher/launcher.go index ee6ddcf2b2..c2fa0426d8 100644 --- a/pkg/app/launcher/launcher.go +++ b/pkg/app/launcher/launcher.go @@ -20,7 +20,6 @@ import ( "github.com/skycoin/skywire/pkg/app/appnet" "github.com/skycoin/skywire/pkg/app/appserver" "github.com/skycoin/skywire/pkg/router" - "github.com/skycoin/skywire/pkg/routing" "github.com/skycoin/skywire/pkg/util/pathutil" ) @@ -34,18 +33,10 @@ var ( ErrAppNotRunning = errors.New("app not running") ) -// AppConfig defines app startup parameters. -type AppConfig struct { - Name string `json:"name"` - Args []string `json:"args,omitempty"` - AutoStart bool `json:"auto_start"` - Port routing.Port `json:"port"` -} - // Config configures the launcher. type Config struct { VisorPK cipher.PubKey - Apps []AppConfig + Apps []appserver.AppConfig ServerAddr string BinPath string LocalPath string @@ -57,7 +48,7 @@ type Launcher struct { log logrus.FieldLogger r router.Router procM appserver.ProcManager - apps map[string]AppConfig + apps map[string]appserver.AppConfig mx sync.Mutex } @@ -94,7 +85,7 @@ func NewLauncher(log logrus.FieldLogger, conf Config, dmsgC *dmsg.Client, r rout } // Prepare apps (autostart if necessary). - apps := make(map[string]AppConfig, len(conf.Apps)) + apps := make(map[string]appserver.AppConfig, len(conf.Apps)) for _, ac := range conf.Apps { apps[ac.Name] = ac } @@ -108,7 +99,7 @@ func (l *Launcher) ResetConfig(conf Config) { l.mx.Lock() defer l.mx.Unlock() - apps := make(map[string]AppConfig, len(conf.Apps)) + apps := make(map[string]appserver.AppConfig, len(conf.Apps)) for _, ac := range conf.Apps { apps[ac.Name] = ac } @@ -164,7 +155,7 @@ func (l *Launcher) AutoStart(envMap EnvMap) error { } // AppState returns a single app state of given name. -func (l *Launcher) AppState(name string) (*AppState, bool) { +func (l *Launcher) AppState(name string) (*appserver.AppState, bool) { l.mx.Lock() defer l.mx.Unlock() @@ -172,35 +163,35 @@ func (l *Launcher) AppState(name string) (*AppState, bool) { if !ok { return nil, false } - state := &AppState{AppConfig: ac, Status: AppStatusStopped} + state := &appserver.AppState{AppConfig: ac, Status: appserver.AppStatusStopped} if err, ok := l.procM.ErrorByName(ac.Name); ok { //nolint:errcheck if err != "" { state.DetailedStatus = err - state.Status = AppStatusErrored + state.Status = appserver.AppStatusErrored } } if proc, ok := l.procM.ProcByName(ac.Name); ok { //nolint:errcheck state.DetailedStatus = proc.DetailedStatus() connSummary := proc.ConnectionsSummary() if connSummary != nil { - state.Status = AppStatusRunning + state.Status = appserver.AppStatusRunning } // for a edge case where app has given the start status but we are unable to retrieve the conn info - if connSummary == nil && state.DetailedStatus == AppDetailedStatusRunning { - state.DetailedStatus = AppDetailedStatusStarting - state.Status = AppStatusStarting + if connSummary == nil && state.DetailedStatus == appserver.AppDetailedStatusRunning { + state.DetailedStatus = appserver.AppDetailedStatusStarting + state.Status = appserver.AppStatusStarting } switch state.DetailedStatus { - case AppDetailedStatusVPNConnecting, AppDetailedStatusStarting, AppDetailedStatusReconnecting: - state.Status = AppStatusStarting + case appserver.AppDetailedStatusVPNConnecting, appserver.AppDetailedStatusStarting, appserver.AppDetailedStatusReconnecting: + state.Status = appserver.AppStatusStarting } } return state, true } // AppStates returns list of AppStates for all registered apps. -func (l *Launcher) AppStates() []*AppState { - var states []*AppState +func (l *Launcher) AppStates() []*appserver.AppState { + var states []*appserver.AppState for _, app := range l.apps { state, _ := l.AppState(app.Name) states = append(states, state) @@ -256,8 +247,6 @@ func (l *Launcher) StopApp(name string) (*appserver.Proc, error) { return nil, ErrAppNotRunning } - l.log.Info("Stopping app...") - if err := l.procM.Stop(name); err != nil { log.WithError(err).Warn("Failed to stop app.") return proc, err @@ -284,7 +273,7 @@ func (l *Launcher) RestartApp(name string) error { return nil } -func makeProcConfig(lc Config, ac AppConfig, envs []string) (appcommon.ProcConfig, error) { +func makeProcConfig(lc Config, ac appserver.AppConfig, envs []string) (appcommon.ProcConfig, error) { procConf := appcommon.ProcConfig{ AppName: ac.Name, AppSrvAddr: lc.ServerAddr, diff --git a/pkg/visor/api.go b/pkg/visor/api.go index 3a65546409..12341c0df9 100644 --- a/pkg/visor/api.go +++ b/pkg/visor/api.go @@ -18,7 +18,6 @@ import ( "github.com/skycoin/skywire-utilities/pkg/cipher" "github.com/skycoin/skywire-utilities/pkg/netutil" "github.com/skycoin/skywire/pkg/app/appserver" - "github.com/skycoin/skywire/pkg/app/launcher" "github.com/skycoin/skywire/pkg/routing" "github.com/skycoin/skywire/pkg/skyenv" "github.com/skycoin/skywire/pkg/transport" @@ -34,8 +33,8 @@ type API interface { Health() (*HealthInfo, error) Uptime() (float64, error) - App(appName string) (*launcher.AppState, error) - Apps() ([]*launcher.AppState, error) + App(appName string) (*appserver.AppState, error) + Apps() ([]*appserver.AppState, error) StartApp(appName string) error StopApp(appName string) error SetAppDetailedStatus(appName, state string) error @@ -89,16 +88,16 @@ type HealthCheckable interface { // Overview provides a range of basic information about a Visor. type Overview struct { - PubKey cipher.PubKey `json:"local_pk"` - BuildInfo *buildinfo.Info `json:"build_info"` - AppProtoVersion string `json:"app_protocol_version"` - Apps []*launcher.AppState `json:"apps"` - Transports []*TransportSummary `json:"transports"` - RoutesCount int `json:"routes_count"` - LocalIP string `json:"local_ip"` - PublicIP string `json:"public_ip"` - IsSymmetricNAT bool `json:"is_symmetic_nat"` - Hypervisors []cipher.PubKey `json:"hypervisors"` + PubKey cipher.PubKey `json:"local_pk"` + BuildInfo *buildinfo.Info `json:"build_info"` + AppProtoVersion string `json:"app_protocol_version"` + Apps []*appserver.AppState `json:"apps"` + Transports []*TransportSummary `json:"transports"` + RoutesCount int `json:"routes_count"` + LocalIP string `json:"local_ip"` + PublicIP string `json:"public_ip"` + IsSymmetricNAT bool `json:"is_symmetic_nat"` + Hypervisors []cipher.PubKey `json:"hypervisors"` } // Overview implements API. @@ -293,15 +292,15 @@ func (v *Visor) Uptime() (float64, error) { } // Apps implements API. -func (v *Visor) Apps() ([]*launcher.AppState, error) { +func (v *Visor) Apps() ([]*appserver.AppState, error) { return v.appL.AppStates(), nil } // App implements API. -func (v *Visor) App(appName string) (*launcher.AppState, error) { +func (v *Visor) App(appName string) (*appserver.AppState, error) { appState, ok := v.appL.AppState(appName) if !ok { - return &launcher.AppState{}, ErrAppProcNotRunning + return &appserver.AppState{}, ErrAppProcNotRunning } return appState, nil } @@ -357,7 +356,6 @@ func (v *Visor) SetAppDetailedStatus(appName, status string) error { return ErrAppProcNotRunning } - v.log.Infof("Setting app detailed status %v for app %v", status, appName) proc.SetDetailedStatus(status) return nil diff --git a/pkg/visor/hypervisor.go b/pkg/visor/hypervisor.go index d5eb8f8235..ccfd311cf7 100644 --- a/pkg/visor/hypervisor.go +++ b/pkg/visor/hypervisor.go @@ -26,7 +26,7 @@ import ( "github.com/skycoin/skywire-utilities/pkg/httputil" "github.com/skycoin/skywire-utilities/pkg/logging" "github.com/skycoin/skywire/pkg/app/appcommon" - "github.com/skycoin/skywire/pkg/app/launcher" + "github.com/skycoin/skywire/pkg/app/appserver" "github.com/skycoin/skywire/pkg/routing" "github.com/skycoin/skywire/pkg/skyenv" "github.com/skycoin/skywire/pkg/transport" @@ -687,9 +687,9 @@ func (hv *Hypervisor) putApp() http.HandlerFunc { httputil.WriteJSON(w, r, http.StatusInternalServerError, err) return } - appStatus := launcher.AppDetailedStatusStarting + appStatus := appserver.AppDetailedStatusStarting if ctx.App.Name == skyenv.VPNClientName { - appStatus = launcher.AppDetailedStatusVPNConnecting + appStatus = appserver.AppDetailedStatusVPNConnecting } if err := ctx.API.SetAppDetailedStatus(ctx.App.Name, appStatus); err != nil { httputil.WriteJSON(w, r, http.StatusInternalServerError, err) @@ -1254,7 +1254,7 @@ type httpCtx struct { Conn // App - App *launcher.AppState + App *appserver.AppState // Transport Tp *TransportSummary diff --git a/pkg/visor/rpc.go b/pkg/visor/rpc.go index b76b6b40f4..cbe7c88134 100644 --- a/pkg/visor/rpc.go +++ b/pkg/visor/rpc.go @@ -11,7 +11,6 @@ import ( "github.com/skycoin/skywire-utilities/pkg/cipher" "github.com/skycoin/skywire/pkg/app/appserver" - "github.com/skycoin/skywire/pkg/app/launcher" "github.com/skycoin/skywire/pkg/routing" "github.com/skycoin/skywire/pkg/transport" "github.com/skycoin/skywire/pkg/transport/network" @@ -196,7 +195,7 @@ func (r *RPC) SetAppError(in *SetAppErrorIn, _ *struct{}) (err error) { } // Apps returns list of Apps registered on the Visor. -func (r *RPC) Apps(_ *struct{}, reply *[]*launcher.AppState) (err error) { +func (r *RPC) Apps(_ *struct{}, reply *[]*appserver.AppState) (err error) { defer rpcutil.LogCall(r.log, "Apps", nil)(reply, &err) apps, err := r.visor.Apps() diff --git a/pkg/visor/rpc_client.go b/pkg/visor/rpc_client.go index 1922d19d1e..d670fe53ef 100644 --- a/pkg/visor/rpc_client.go +++ b/pkg/visor/rpc_client.go @@ -20,7 +20,6 @@ import ( "github.com/skycoin/skywire-utilities/pkg/logging" "github.com/skycoin/skywire/pkg/app/appcommon" "github.com/skycoin/skywire/pkg/app/appserver" - "github.com/skycoin/skywire/pkg/app/launcher" "github.com/skycoin/skywire/pkg/router" "github.com/skycoin/skywire/pkg/routing" "github.com/skycoin/skywire/pkg/skyenv" @@ -121,15 +120,15 @@ func (rc *rpcClient) Uptime() (float64, error) { } // Apps calls Apps. -func (rc *rpcClient) Apps() ([]*launcher.AppState, error) { - states := make([]*launcher.AppState, 0) +func (rc *rpcClient) Apps() ([]*appserver.AppState, error) { + states := make([]*appserver.AppState, 0) err := rc.Call("Apps", &struct{}{}, &states) return states, err } // App calls App. -func (rc *rpcClient) App(appName string) (*launcher.AppState, error) { - var state *launcher.AppState +func (rc *rpcClient) App(appName string) (*appserver.AppState, error) { + var state *appserver.AppState err := rc.Call("App", appName, &state) return state, err } @@ -483,9 +482,9 @@ func NewMockRPCClient(r *rand.Rand, maxTps int, maxRules int) (cipher.PubKey, AP PubKey: localPK, BuildInfo: buildinfo.Get(), AppProtoVersion: supportedProtocolVersion, - Apps: []*launcher.AppState{ - {AppConfig: launcher.AppConfig{Name: "foo.v1.0", AutoStart: false, Port: 10}}, - {AppConfig: launcher.AppConfig{Name: "bar.v2.0", AutoStart: false, Port: 20}}, + Apps: []*appserver.AppState{ + {AppConfig: appserver.AppConfig{Name: "foo.v1.0", AutoStart: false, Port: 10}}, + {AppConfig: appserver.AppConfig{Name: "bar.v2.0", AutoStart: false, Port: 20}}, }, Transports: tps, RoutesCount: rt.Count(), @@ -584,8 +583,8 @@ func (mc *mockRPCClient) Uptime() (float64, error) { } // Apps implements API. -func (mc *mockRPCClient) Apps() ([]*launcher.AppState, error) { - var apps []*launcher.AppState +func (mc *mockRPCClient) Apps() ([]*appserver.AppState, error) { + var apps []*appserver.AppState err := mc.do(false, func() error { for _, a := range mc.o.Apps { a := a @@ -597,8 +596,8 @@ func (mc *mockRPCClient) Apps() ([]*launcher.AppState, error) { } // App implements API. -func (mc *mockRPCClient) App(appName string) (*launcher.AppState, error) { - var app *launcher.AppState +func (mc *mockRPCClient) App(appName string) (*appserver.AppState, error) { + var app *appserver.AppState err := mc.do(false, func() error { for _, a := range mc.o.Apps { if a.Name == appName { diff --git a/pkg/visor/visorconfig/config.go b/pkg/visor/visorconfig/config.go index 555124ec38..28ea882308 100644 --- a/pkg/visor/visorconfig/config.go +++ b/pkg/visor/visorconfig/config.go @@ -12,7 +12,7 @@ import ( "github.com/skycoin/skywire-utilities/pkg/cipher" "github.com/skycoin/skywire-utilities/pkg/logging" utilenv "github.com/skycoin/skywire-utilities/pkg/skyenv" - "github.com/skycoin/skywire/pkg/app/launcher" + "github.com/skycoin/skywire/pkg/app/appserver" "github.com/skycoin/skywire/pkg/dmsgc" "github.com/skycoin/skywire/pkg/restart" "github.com/skycoin/skywire/pkg/routing" @@ -189,8 +189,8 @@ func MakeDefaultConfig(log *logging.MasterLogger, sk *cipher.SecKey, usrEnv bool // makeDefaultLauncherAppsConfig creates default launcher config for apps, // for package based installation in other platform (Darwin, Windows) it only includes // the shipped apps for that platforms -func makeDefaultLauncherAppsConfig() []launcher.AppConfig { - defaultConfig := []launcher.AppConfig{ +func makeDefaultLauncherAppsConfig() []appserver.AppConfig { + defaultConfig := []appserver.AppConfig{ { Name: skyenv.VPNClientName, AutoStart: false, diff --git a/pkg/visor/visorconfig/v1.go b/pkg/visor/visorconfig/v1.go index 60a97778e7..6059aae398 100644 --- a/pkg/visor/visorconfig/v1.go +++ b/pkg/visor/visorconfig/v1.go @@ -6,6 +6,7 @@ import ( "sync" "github.com/skycoin/skywire-utilities/pkg/cipher" + "github.com/skycoin/skywire/pkg/app/appserver" "github.com/skycoin/skywire/pkg/app/launcher" "github.com/skycoin/skywire/pkg/dmsgc" "github.com/skycoin/skywire/pkg/transport" @@ -76,12 +77,12 @@ type UptimeTracker struct { Addr string `json:"addr"` } -// Launcher configures the app launcher. +// Launcher configures the app appserver. type Launcher struct { - ServiceDisc string `json:"service_discovery"` - Apps []launcher.AppConfig `json:"apps"` - ServerAddr string `json:"server_addr"` - BinPath string `json:"bin_path"` + ServiceDisc string `json:"service_discovery"` + Apps []appserver.AppConfig `json:"apps"` + ServerAddr string `json:"server_addr"` + BinPath string `json:"bin_path"` } // Flush flushes the config to file (if specified). @@ -92,7 +93,7 @@ func (v1 *V1) Flush() error { return v1.Common.flush(v1) } -// UpdateAppAutostart modifies a single app's autostart value within the config and also the given launcher. +// UpdateAppAutostart modifies a single app's autostart value within the config and also the given appserver. // The updated config gets flushed to file if there are any changes. func (v1 *V1) UpdateAppAutostart(launch *launcher.Launcher, appName string, autoStart bool) error { v1.mu.Lock() @@ -121,7 +122,7 @@ func (v1 *V1) UpdateAppAutostart(launch *launcher.Launcher, appName string, auto return v1.flush(v1) } -// UpdateAppArg updates the cli flag of the specified app config and also within the launcher. +// UpdateAppArg updates the cli flag of the specified app config and also within the appserver. // The updated config gets flushed to file if there are any changes. func (v1 *V1) UpdateAppArg(launch *launcher.Launcher, appName, argName string, value interface{}) error { v1.mu.Lock() @@ -186,7 +187,7 @@ func (v1 *V1) UpdatePublicAutoconnect(pAc bool) error { return v1.flush(v1) } -// updateStringArg updates the cli non-boolean flag of the specified app config and also within the launcher. +// updateStringArg updates the cli non-boolean flag of the specified app config and also within the appserver. // It removes argName from app args if value is an empty string. // The updated config gets flushed to file if there are any changes. func updateStringArg(conf *Launcher, appName, argName, value string) bool { @@ -228,7 +229,7 @@ func updateStringArg(conf *Launcher, appName, argName, value string) bool { return configChanged } -// updateBoolArg updates the cli boolean flag of the specified app config and also within the launcher. +// updateBoolArg updates the cli boolean flag of the specified app config and also within the appserver. // All flag names and values are formatted as "-name=value" to allow arbitrary values with respect to different // possible default values. // The updated config gets flushed to file if there are any changes. diff --git a/pkg/visor/visorconfig/v1_test.go b/pkg/visor/visorconfig/v1_test.go index 65ef6b07f7..5a9d6721c2 100644 --- a/pkg/visor/visorconfig/v1_test.go +++ b/pkg/visor/visorconfig/v1_test.go @@ -5,7 +5,7 @@ import ( "github.com/stretchr/testify/assert" - "github.com/skycoin/skywire/pkg/app/launcher" + "github.com/skycoin/skywire/pkg/app/appserver" "github.com/skycoin/skywire/pkg/skyenv" ) @@ -26,7 +26,7 @@ func Test_updateStringArg(t *testing.T) { name: "Case 1", args: args{ conf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: "skysocks-client", Args: []string{"-passcode", "1234"}, @@ -39,7 +39,7 @@ func Test_updateStringArg(t *testing.T) { }, wantResult: true, wantConf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: "skysocks-client", Args: []string{"-passcode", "4321"}, @@ -51,7 +51,7 @@ func Test_updateStringArg(t *testing.T) { name: "Case 2", args: args{ conf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: "skysocks-client", Args: []string{"-passcode", "1234"}, @@ -64,7 +64,7 @@ func Test_updateStringArg(t *testing.T) { }, wantResult: true, wantConf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: "skysocks-client", Args: []string{}, @@ -76,7 +76,7 @@ func Test_updateStringArg(t *testing.T) { name: "Case 3", args: args{ conf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: "skysocks-client", Args: []string{"-t", "-passcode", "1234", "-test", "abc"}, @@ -89,7 +89,7 @@ func Test_updateStringArg(t *testing.T) { }, wantResult: true, wantConf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: "skysocks-client", Args: []string{"-t", "-test", "abc"}, @@ -101,7 +101,7 @@ func Test_updateStringArg(t *testing.T) { name: "Case 4", args: args{ conf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: "skysocks-client", Args: []string{"-t", "-passcode", "1234", "-test", "abc"}, @@ -114,7 +114,7 @@ func Test_updateStringArg(t *testing.T) { }, wantResult: true, wantConf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: "skysocks-client", Args: []string{"-t", "-passcode", "1234", "-test", "abc", "-arg1", "678"}, @@ -126,7 +126,7 @@ func Test_updateStringArg(t *testing.T) { name: "Case 5", args: args{ conf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: "skysocks-client", Args: []string{"-t", "-passcode", "1234", "-test", "abc"}, @@ -139,7 +139,7 @@ func Test_updateStringArg(t *testing.T) { }, wantResult: false, wantConf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: "skysocks-client", Args: []string{"-t", "-passcode", "1234", "-test", "abc"}, @@ -151,7 +151,7 @@ func Test_updateStringArg(t *testing.T) { name: "Case 6", args: args{ conf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: "skysocks-client", }, @@ -163,7 +163,7 @@ func Test_updateStringArg(t *testing.T) { }, wantResult: true, wantConf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: "skysocks-client", }, @@ -198,7 +198,7 @@ func Test_updateBoolArg(t *testing.T) { name: "Single dash flag, absent value", args: args{ conf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: skyenv.VPNClientName, Args: []string{"-passcode", "1234"}, @@ -211,7 +211,7 @@ func Test_updateBoolArg(t *testing.T) { }, wantResult: true, wantConf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: skyenv.VPNClientName, Args: []string{"-passcode", "1234", "-killswitch=true"}, @@ -223,7 +223,7 @@ func Test_updateBoolArg(t *testing.T) { name: "Double dash flag, absent value", args: args{ conf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: skyenv.VPNClientName, Args: []string{"-passcode", "1234"}, @@ -236,7 +236,7 @@ func Test_updateBoolArg(t *testing.T) { }, wantResult: true, wantConf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: skyenv.VPNClientName, Args: []string{"-passcode", "1234", "-killswitch=false"}, @@ -248,7 +248,7 @@ func Test_updateBoolArg(t *testing.T) { name: "Present valid double-dash-named value", args: args{ conf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: skyenv.VPNClientName, Args: []string{"-passcode", "1234", "--killswitch=true"}, @@ -261,7 +261,7 @@ func Test_updateBoolArg(t *testing.T) { }, wantResult: true, wantConf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: skyenv.VPNClientName, Args: []string{"-passcode", "1234", "-killswitch=false"}, @@ -273,7 +273,7 @@ func Test_updateBoolArg(t *testing.T) { name: "Present valid single-dash-named value", args: args{ conf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: skyenv.VPNClientName, Args: []string{"-passcode", "1234", "-killswitch=false"}, @@ -286,7 +286,7 @@ func Test_updateBoolArg(t *testing.T) { }, wantResult: true, wantConf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: skyenv.VPNClientName, Args: []string{"-passcode", "1234", "-killswitch=true"}, @@ -298,7 +298,7 @@ func Test_updateBoolArg(t *testing.T) { name: "Present invalid single-dash-named value", args: args{ conf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: skyenv.VPNClientName, Args: []string{"-passcode", "1234", "-killswitch", "false"}, @@ -311,7 +311,7 @@ func Test_updateBoolArg(t *testing.T) { }, wantResult: true, wantConf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: skyenv.VPNClientName, Args: []string{"-passcode", "1234", "-killswitch=true"}, @@ -323,7 +323,7 @@ func Test_updateBoolArg(t *testing.T) { name: "Present invalid double-dash-named value", args: args{ conf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: skyenv.VPNClientName, Args: []string{"--killswitch", "true"}, @@ -336,7 +336,7 @@ func Test_updateBoolArg(t *testing.T) { }, wantResult: true, wantConf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: skyenv.VPNClientName, Args: []string{"-killswitch=false"}, @@ -348,7 +348,7 @@ func Test_updateBoolArg(t *testing.T) { name: "Empty args list", args: args{ conf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: skyenv.VPNClientName, }, @@ -360,7 +360,7 @@ func Test_updateBoolArg(t *testing.T) { }, wantResult: true, wantConf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: skyenv.VPNClientName, Args: []string{"-killswitch=false"}, @@ -372,7 +372,7 @@ func Test_updateBoolArg(t *testing.T) { name: "List with a single arg and empty value", args: args{ conf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: skyenv.VPNClientName, Args: []string{"-passcode", ""}, @@ -385,7 +385,7 @@ func Test_updateBoolArg(t *testing.T) { }, wantResult: true, wantConf: &Launcher{ - Apps: []launcher.AppConfig{ + Apps: []appserver.AppConfig{ { Name: skyenv.VPNClientName, Args: []string{"-passcode", "", "-killswitch=false"}, From 7091527034736b2d0d8da593874fa04ca47bb571 Mon Sep 17 00:00:00 2001 From: ersonp Date: Tue, 28 Jun 2022 15:21:56 +0530 Subject: [PATCH 10/17] Fix comment --- pkg/app/appserver/rpc_ingress_gateway.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/app/appserver/rpc_ingress_gateway.go b/pkg/app/appserver/rpc_ingress_gateway.go index b1e6a32af4..515705af77 100644 --- a/pkg/app/appserver/rpc_ingress_gateway.go +++ b/pkg/app/appserver/rpc_ingress_gateway.go @@ -276,7 +276,7 @@ func (r *RPCIngressGateway) Read(req *ReadReq, resp *ReadResp) error { copy(resp.B, buf[:resp.N]) } if err != nil { - // we don't print warning if the conn is ready closed + // we don't print warning if the conn is already closed _, ok := r.cm.Get(req.ConnID) if ok { r.log.WithError(err).Warn("Received unexpected error when reading from server.") From 021e260a9e7c70a443e9132ceb02012120614c38 Mon Sep 17 00:00:00 2001 From: ersonp Date: Tue, 28 Jun 2022 17:28:55 +0530 Subject: [PATCH 11/17] Fix windows error logs --- pkg/app/appserver/stderr.go | 4 +++- pkg/app/appserver/stderr_windows.go | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/app/appserver/stderr.go b/pkg/app/appserver/stderr.go index 37218413c2..be12a59a90 100644 --- a/pkg/app/appserver/stderr.go +++ b/pkg/app/appserver/stderr.go @@ -17,7 +17,9 @@ func printStdErr(stderr io.ReadCloser, errorLog *logrus.Entry) { for cmdStderr.Scan() { err := cmdStderr.Text() if !contains(iErrs, err) { - errorLog.Error(cmdStderr.Text()) + if err != "" { + errorLog.Error(err) + } } } }() diff --git a/pkg/app/appserver/stderr_windows.go b/pkg/app/appserver/stderr_windows.go index 87dd86c753..e175d4cb90 100644 --- a/pkg/app/appserver/stderr_windows.go +++ b/pkg/app/appserver/stderr_windows.go @@ -6,7 +6,7 @@ package appserver func getIgnoreErrs() []string { ignoreErrs := []string{ "Creating adapter", - "Using existing driver 0.14", + "Using existing driver", "rpc.Serve: accept:accept", "The route addition failed: The object already exists.", } From 4e56ae4c6b786a1d883a3ed2d50041e08bb5abed Mon Sep 17 00:00:00 2001 From: ersonp Date: Tue, 28 Jun 2022 17:56:23 +0530 Subject: [PATCH 12/17] Fix permission denied error on mac --- internal/vpn/os_client_darwin.go | 6 +++++- internal/vpn/os_darwin.go | 10 +--------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/internal/vpn/os_client_darwin.go b/internal/vpn/os_client_darwin.go index 7405e530c7..30f64a5bfe 100644 --- a/internal/vpn/os_client_darwin.go +++ b/internal/vpn/os_client_darwin.go @@ -39,7 +39,11 @@ func DefaultNetworkGateway() (net.IP, error) { } func setupClientSysPrivileges() (int, error) { - return osutil.GainRoot() + err := osutil.GainRoot() + if strings.Contains(err.Error(), "operation not permitted") { + return errPermissionDenied + } + return } func releaseClientSysPrivileges(oldUID int) error { diff --git a/internal/vpn/os_darwin.go b/internal/vpn/os_darwin.go index d8e70a6765..b7062281a7 100644 --- a/internal/vpn/os_darwin.go +++ b/internal/vpn/os_darwin.go @@ -37,10 +37,8 @@ func AddRoute(ipCIDR, gateway string) error { return nil } } - return err } - return nil } @@ -54,11 +52,5 @@ func modifyRoutingTable(action, ipCIDR, gateway string) error { if err != nil { return fmt.Errorf("error parsing IP CIDR: %w", err) } - - err = osutil.Run("route", action, "-net", ip, gateway, netmask) - if err != nil { - print(fmt.Sprintf("%v\n", err)) - return errPermissionDenied - } - return nil + return osutil.Run("route", action, "-net", ip, gateway, netmask) } From 7cc172955015d56601a1b026868d77237e18a8da Mon Sep 17 00:00:00 2001 From: ersonp Date: Wed, 29 Jun 2022 08:39:39 +0530 Subject: [PATCH 13/17] Fix setupClientSysPrivileges for darwin --- internal/vpn/os_client_darwin.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/vpn/os_client_darwin.go b/internal/vpn/os_client_darwin.go index 30f64a5bfe..d102b1de89 100644 --- a/internal/vpn/os_client_darwin.go +++ b/internal/vpn/os_client_darwin.go @@ -39,11 +39,11 @@ func DefaultNetworkGateway() (net.IP, error) { } func setupClientSysPrivileges() (int, error) { - err := osutil.GainRoot() - if strings.Contains(err.Error(), "operation not permitted") { - return errPermissionDenied + value, err := osutil.GainRoot() + if err != nil && strings.Contains(err.Error(), "operation not permitted") { + return value, errPermissionDenied } - return + return value, err } func releaseClientSysPrivileges(oldUID int) error { From 55b094a2d1827ff40085df82956e0a7fdc201b4e Mon Sep 17 00:00:00 2001 From: ersonp Date: Wed, 29 Jun 2022 11:48:43 +0530 Subject: [PATCH 14/17] Fix spelling --- internal/vpn/client.go | 5 ++--- internal/vpn/errors.go | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/vpn/client.go b/internal/vpn/client.go index 98bf6bd088..0b164a4956 100644 --- a/internal/vpn/client.go +++ b/internal/vpn/client.go @@ -187,7 +187,7 @@ func (c *Client) Serve() error { r := netutil.NewRetrier(nil, netutil.DefaultInitBackoff, netutil.DefaultMaxBackoff, 3, netutil.DefaultFactor). WithErrWhitelist(errHandshakeStatusForbidden, errHandshakeStatusInternalError, errHandshakeNoFreeIPs, - errHandshakeStatusBadRequest, errNoTransportFound, errTransportNotFound, errErrSetupNode, errNotPermited) + errHandshakeStatusBadRequest, errNoTransportFound, errTransportNotFound, errErrSetupNode, errNotPermitted) err := r.Do(context.Background(), func() error { if c.isClosed() { @@ -197,7 +197,7 @@ func (c *Client) Serve() error { if err := c.dialServeConn(); err != nil { switch err { case errHandshakeStatusForbidden, errHandshakeStatusInternalError, errHandshakeNoFreeIPs, - errHandshakeStatusBadRequest, errNoTransportFound, errTransportNotFound, errErrSetupNode, errNotPermited: + errHandshakeStatusBadRequest, errNoTransportFound, errTransportNotFound, errErrSetupNode, errNotPermitted: c.setAppError(err) c.resetConnDuration() return err @@ -209,7 +209,6 @@ func (c *Client) Serve() error { return fmt.Errorf("dialServeConn: %w", err) } } - return nil }) if err != nil { diff --git a/internal/vpn/errors.go b/internal/vpn/errors.go index eb4b01a179..1f35f8c884 100644 --- a/internal/vpn/errors.go +++ b/internal/vpn/errors.go @@ -16,7 +16,7 @@ var ( errHandshakeNoFreeIPs = errors.New("no free IPs left to serve") errHandshakeStatusBadRequest = errors.New("request was malformed") errTimeout = errors.New("internal error: Timeout") - errNotPermited = errors.New("ioctl: operation not permitted") + errNotPermitted = errors.New("ioctl: operation not permitted") errVPNServerClosed = errors.New("vpn-server closed") errPermissionDenied = errors.New("permission denied") From a64ff68859d89429c3b056bb95c73699fecbf6b9 Mon Sep 17 00:00:00 2001 From: ersonp Date: Wed, 29 Jun 2022 12:44:16 +0530 Subject: [PATCH 15/17] Fix spelling --- pkg/router/route_group.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/router/route_group.go b/pkg/router/route_group.go index 74278a0d5a..b8b65b3c68 100644 --- a/pkg/router/route_group.go +++ b/pkg/router/route_group.go @@ -616,8 +616,8 @@ func (rg *RouteGroup) handleNetworkProbePacket(packet routing.Packet) error { sentAt := time.Unix(int64(sentAtMs/1000), int64(ms)*int64(time.Millisecond)).UTC() latency := time.Now().UTC().Sub(sentAt).Milliseconds() - // todo (ersonp): this is a dirty fix, we nned to implement new packets Ping and Pong to calculate the RTT. - // if larency is negative we set it to be the previous one + // todo (ersonp): this is a dirty fix, we need to implement new packets Ping and Pong to calculate the RTT. + // if latency is negative we set it to be the previous one if math.Signbit(float64(latency)) { latency = int64(rg.networkStats.Latency()) } From f4a9c58cf52b81cc721a83522bd5a142b3120642 Mon Sep 17 00:00:00 2001 From: ersonp Date: Wed, 29 Jun 2022 14:21:53 +0530 Subject: [PATCH 16/17] Fix spelling --- internal/vpn/os_client_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/vpn/os_client_linux.go b/internal/vpn/os_client_linux.go index a613dd555a..1261772414 100644 --- a/internal/vpn/os_client_linux.go +++ b/internal/vpn/os_client_linux.go @@ -68,7 +68,7 @@ func setupClientSysPrivileges() (int, error) { caps.Set(capability.CAPS|capability.BOUNDS|capability.AMBIENT, capability.CAP_NET_ADMIN) err = caps.Apply(capability.CAPS | capability.BOUNDS | capability.AMBIENT) if err != nil { - err = fmt.Errorf("failed to apply capabilties: %w", err) + err = fmt.Errorf("failed to apply capabilities: %w", err) return } From d3987eb91f3be6a98648efe297911a3a0e0b4df3 Mon Sep 17 00:00:00 2001 From: ersonp Date: Wed, 29 Jun 2022 14:22:23 +0530 Subject: [PATCH 17/17] Fix darwin import --- internal/vpn/os_client_darwin.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/vpn/os_client_darwin.go b/internal/vpn/os_client_darwin.go index d102b1de89..4ee044da7f 100644 --- a/internal/vpn/os_client_darwin.go +++ b/internal/vpn/os_client_darwin.go @@ -6,6 +6,7 @@ package vpn import ( "bytes" "net" + "strings" "github.com/skycoin/skywire/pkg/util/osutil" )