From 541a4a3271cf13e7fef25e7bee6ba8612426329a Mon Sep 17 00:00:00 2001 From: "Quentin McGaw (desktop)" Date: Sat, 11 Sep 2021 21:49:46 +0000 Subject: [PATCH] Feat: healthcheck uses ping instead of DNS --- go.mod | 3 + go.sum | 6 ++ internal/healthcheck/health.go | 34 +++++----- internal/healthcheck/health_ping_test.go | 20 ++++++ internal/healthcheck/health_test.go | 81 ++++++++++++++++++++++++ internal/healthcheck/ping.go | 19 ++++++ internal/healthcheck/pinger_mock_test.go | 60 ++++++++++++++++++ internal/healthcheck/server.go | 19 +++--- 8 files changed, 215 insertions(+), 27 deletions(-) create mode 100644 internal/healthcheck/health_ping_test.go create mode 100644 internal/healthcheck/health_test.go create mode 100644 internal/healthcheck/ping.go create mode 100644 internal/healthcheck/pinger_mock_test.go diff --git a/go.mod b/go.mod index a72cc9e2..3dac5107 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.17 require ( github.com/fatih/color v1.12.0 + github.com/go-ping/ping v0.0.0-20210911151512-381826476871 github.com/golang/mock v1.6.0 github.com/qdm12/dns v1.11.0 github.com/qdm12/golibs v0.0.0-20210822203818-5c568b0777b6 @@ -22,6 +23,7 @@ require ( require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/google/go-cmp v0.5.5 // indirect + github.com/google/uuid v1.2.0 // indirect github.com/josharian/native v0.0.0-20200817173448-b6b71def0850 // indirect github.com/mattn/go-colorable v0.1.8 // indirect github.com/mattn/go-isatty v0.0.12 // indirect @@ -36,5 +38,6 @@ require ( go4.org/unsafe/assume-no-moving-gc v0.0.0-20201222180813-1025295fd063 // indirect golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97 // indirect golang.org/x/net v0.0.0-20210504132125-bbd867fde50d // indirect + golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect ) diff --git a/go.sum b/go.sum index 31a011a0..2e9d64ae 100644 --- a/go.sum +++ b/go.sum @@ -29,6 +29,8 @@ github.com/go-openapi/spec v0.17.0/go.mod h1:XkF/MOi14NmjsfZ8VtAKf8pIlbZzyoTvZsd github.com/go-openapi/strfmt v0.17.0/go.mod h1:P82hnJI0CXkErkXi8IKjPbNBM6lV6+5pLP5l494TcyU= github.com/go-openapi/swag v0.17.0/go.mod h1:AByQ+nYG6gQg71GINrmuDXCPWdL640yX49/kXLo40Tg= github.com/go-openapi/validate v0.17.0/go.mod h1:Uh4HdOzKt19xGIGm1qHf/ofbX1YQ4Y+MYsct2VUrAJ4= +github.com/go-ping/ping v0.0.0-20210911151512-381826476871 h1:wtjTfjwAR/BYYMJ+QOLI/3J/qGEI0fgrkZvgsEWK2/Q= +github.com/go-ping/ping v0.0.0-20210911151512-381826476871/go.mod h1:xIFjORFzTxqIV/tDVGO4eDy/bLuSyawEeojSm3GfRGk= github.com/golang/mock v1.5.0/go.mod h1:CWnOUgYIOo4TcNZ0wHX3YZCqsaM1I1Jvs6v3mP3KVu8= github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc= github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs= @@ -42,6 +44,8 @@ github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/google/uuid v1.2.0 h1:qJYtXnJRWmpe7m/3XlyhrsLrEURqHRM2kxzoxXqyUDs= +github.com/google/uuid v1.2.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/gotify/go-api-client/v2 v2.0.4/go.mod h1:VKiah/UK20bXsr0JObE1eBVLW44zbBouzjuri9iwjFU= github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo= github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI= @@ -163,6 +167,7 @@ golang.org/x/net v0.0.0-20201216054612-986b41b23924/go.mod h1:m0MpNAwzfU5UDzcl9v golang.org/x/net v0.0.0-20201224014010-6772e930b67b/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20210119194325-5f4716e94777/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= +golang.org/x/net v0.0.0-20210316092652-d523dce5a7f4/go.mod h1:RBQZq4jEuRlivfhVLdyRGr576XBO4/greRjx4P4O3yc= golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/net v0.0.0-20210504132125-bbd867fde50d h1:nTDGCTeAu2LhcsHTRzjyIUbZHCJ4QePArsm27Hka0UM= golang.org/x/net v0.0.0-20210504132125-bbd867fde50d/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= @@ -195,6 +200,7 @@ golang.org/x/sys v0.0.0-20210123111255-9b0068b26619/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210216163648-f7da38b97c65/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210309040221-94ec62e08169/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210315160823-c6e025ad8005/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210403161142-5e06dd20ab57/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= diff --git a/internal/healthcheck/health.go b/internal/healthcheck/health.go index f7ddd5fc..11c440c1 100644 --- a/internal/healthcheck/health.go +++ b/internal/healthcheck/health.go @@ -3,9 +3,6 @@ package healthcheck import ( "context" - "errors" - "fmt" - "net" "time" ) @@ -17,7 +14,7 @@ func (s *Server) runHealthcheckLoop(ctx context.Context, done chan<- struct{}) { for { previousErr := s.handler.getErr() - err := healthCheck(ctx, s.resolver) + err := healthCheck(ctx, s.pinger) s.handler.setErr(err) if previousErr != nil && err == nil { @@ -59,20 +56,23 @@ func (s *Server) runHealthcheckLoop(ctx context.Context, done chan<- struct{}) { } } -var ( - errNoIPResolved = errors.New("no IP address resolved") -) - -func healthCheck(ctx context.Context, resolver *net.Resolver) (err error) { +func healthCheck(ctx context.Context, pinger Pinger) (err error) { // TODO use mullvad API if current provider is Mullvad - const domainToResolve = "github.com" - ips, err := resolver.LookupIP(ctx, "ip", domainToResolve) - switch { - case err != nil: + // If we run without root, you need to run this on the gluetun binary: + // setcap cap_net_raw=+ep /path/to/your/compiled/binary + // Alternatively, we could have a separate binary just for healthcheck to + // reduce attack surface. + errCh := make(chan error) + go func() { + errCh <- pinger.Run() + }() + + select { + case <-ctx.Done(): + pinger.Stop() + <-errCh + return ctx.Err() + case err = <-errCh: return err - case len(ips) == 0: - return fmt.Errorf("%w for %s", errNoIPResolved, domainToResolve) - default: - return nil } } diff --git a/internal/healthcheck/health_ping_test.go b/internal/healthcheck/health_ping_test.go new file mode 100644 index 00000000..2d3b7f5b --- /dev/null +++ b/internal/healthcheck/health_ping_test.go @@ -0,0 +1,20 @@ +//go:build integration + +package healthcheck + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_healthCheck_ping(t *testing.T) { + t.Parallel() + + pinger := newPinger() + + err := healthCheck(context.Background(), pinger) + + assert.NoError(t, err) +} diff --git a/internal/healthcheck/health_test.go b/internal/healthcheck/health_test.go new file mode 100644 index 00000000..8e46f99e --- /dev/null +++ b/internal/healthcheck/health_test.go @@ -0,0 +1,81 @@ +package healthcheck + +import ( + "context" + "errors" + "testing" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" +) + +func Test_healthCheck(t *testing.T) { + t.Parallel() + + canceledCtx, cancel := context.WithCancel(context.Background()) + cancel() + + someErr := errors.New("error") + + testCases := map[string]struct { + ctx context.Context + runErr error + stopCall bool + err error + }{ + "success": { + ctx: context.Background(), + }, + "error": { + ctx: context.Background(), + runErr: someErr, + err: someErr, + }, + "context canceled": { + ctx: canceledCtx, + stopCall: true, + err: context.Canceled, + }, + } + + for name, testCase := range testCases { + testCase := testCase + t.Run(name, func(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + + stopped := make(chan struct{}) + + pinger := NewMockPinger(ctrl) + pinger.EXPECT().Run().DoAndReturn(func() error { + if testCase.stopCall { + <-stopped + } + return testCase.runErr + }) + + if testCase.stopCall { + pinger.EXPECT().Stop().DoAndReturn(func() { + close(stopped) + }) + } + + err := healthCheck(testCase.ctx, pinger) + + assert.ErrorIs(t, testCase.err, err) + }) + } + + t.Run("canceled real pinger", func(t *testing.T) { + t.Parallel() + + pinger := newPinger() + + canceledCtx, cancel := context.WithCancel(context.Background()) + cancel() + + err := healthCheck(canceledCtx, pinger) + + assert.ErrorIs(t, context.Canceled, err) + }) +} diff --git a/internal/healthcheck/ping.go b/internal/healthcheck/ping.go new file mode 100644 index 00000000..ac5f4706 --- /dev/null +++ b/internal/healthcheck/ping.go @@ -0,0 +1,19 @@ +package healthcheck + +import "github.com/go-ping/ping" + +//go:generate mockgen -destination=pinger_mock_test.go -package healthcheck . Pinger + +type Pinger interface { + Run() error + Stop() +} + +func newPinger() (pinger *ping.Pinger) { + const addrToPing = "1.1.1.1" + const count = 1 + pinger = ping.New(addrToPing) + pinger.Count = count + pinger.SetPrivileged(true) + return pinger +} diff --git a/internal/healthcheck/pinger_mock_test.go b/internal/healthcheck/pinger_mock_test.go new file mode 100644 index 00000000..63375cd4 --- /dev/null +++ b/internal/healthcheck/pinger_mock_test.go @@ -0,0 +1,60 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/qdm12/gluetun/internal/healthcheck (interfaces: Pinger) + +// Package healthcheck is a generated GoMock package. +package healthcheck + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" +) + +// MockPinger is a mock of Pinger interface. +type MockPinger struct { + ctrl *gomock.Controller + recorder *MockPingerMockRecorder +} + +// MockPingerMockRecorder is the mock recorder for MockPinger. +type MockPingerMockRecorder struct { + mock *MockPinger +} + +// NewMockPinger creates a new mock instance. +func NewMockPinger(ctrl *gomock.Controller) *MockPinger { + mock := &MockPinger{ctrl: ctrl} + mock.recorder = &MockPingerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockPinger) EXPECT() *MockPingerMockRecorder { + return m.recorder +} + +// Run mocks base method. +func (m *MockPinger) Run() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Run") + ret0, _ := ret[0].(error) + return ret0 +} + +// Run indicates an expected call of Run. +func (mr *MockPingerMockRecorder) Run() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Run", reflect.TypeOf((*MockPinger)(nil).Run)) +} + +// Stop mocks base method. +func (m *MockPinger) Stop() { + m.ctrl.T.Helper() + m.ctrl.Call(m, "Stop") +} + +// Stop indicates an expected call of Stop. +func (mr *MockPingerMockRecorder) Stop() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Stop", reflect.TypeOf((*MockPinger)(nil).Stop)) +} diff --git a/internal/healthcheck/server.go b/internal/healthcheck/server.go index 0a37bddf..abc9586c 100644 --- a/internal/healthcheck/server.go +++ b/internal/healthcheck/server.go @@ -2,7 +2,6 @@ package healthcheck import ( "context" - "net" "github.com/qdm12/gluetun/internal/configuration" "github.com/qdm12/gluetun/internal/vpn" @@ -16,20 +15,20 @@ type ServerRunner interface { } type Server struct { - logger logging.Logger - handler *handler - resolver *net.Resolver - config configuration.Health - vpn vpnHealth + logger logging.Logger + handler *handler + pinger Pinger + config configuration.Health + vpn vpnHealth } func NewServer(config configuration.Health, logger logging.Logger, vpnLooper vpn.Looper) *Server { return &Server{ - logger: logger, - handler: newHandler(logger), - resolver: net.DefaultResolver, - config: config, + logger: logger, + handler: newHandler(logger), + pinger: newPinger(), + config: config, vpn: vpnHealth{ looper: vpnLooper, healthyWait: config.VPN.Initial,