fix(health): use TCP dialing instead of ping

- `HEALTH_TARGET_ADDRESS` to replace `HEALTH_ADDRESS_TO_PING`
- Remove `github.com/go-ping/ping` dependency
- Dial TCP the target address, appending `:443` if port is not set
This commit is contained in:
Quentin McGaw
2022-03-21 19:57:35 +00:00
parent 5aaa122460
commit c6f68a64e6
12 changed files with 131 additions and 227 deletions

View File

@@ -15,10 +15,10 @@ type Health struct {
// for the health check server.
// It cannot be the empty string in the internal state.
ServerAddress string
// AddressToPing is the IP address or domain name to
// ping periodically for the health check.
// TargetAddress is the address (host or host:port)
// to TCP dial to periodically for the health check.
// It cannot be the empty string in the internal state.
AddressToPing string
TargetAddress string
VPN HealthyWait
}
@@ -41,7 +41,7 @@ func (h Health) Validate() (err error) {
func (h *Health) copy() (copied Health) {
return Health{
ServerAddress: h.ServerAddress,
AddressToPing: h.AddressToPing,
TargetAddress: h.TargetAddress,
VPN: h.VPN.copy(),
}
}
@@ -50,7 +50,7 @@ func (h *Health) copy() (copied Health) {
// unset field of the receiver settings object.
func (h *Health) MergeWith(other Health) {
h.ServerAddress = helpers.MergeWithString(h.ServerAddress, other.ServerAddress)
h.AddressToPing = helpers.MergeWithString(h.AddressToPing, other.AddressToPing)
h.TargetAddress = helpers.MergeWithString(h.TargetAddress, other.TargetAddress)
h.VPN.mergeWith(other.VPN)
}
@@ -59,13 +59,13 @@ func (h *Health) MergeWith(other Health) {
// settings.
func (h *Health) OverrideWith(other Health) {
h.ServerAddress = helpers.OverrideWithString(h.ServerAddress, other.ServerAddress)
h.AddressToPing = helpers.OverrideWithString(h.AddressToPing, other.AddressToPing)
h.TargetAddress = helpers.OverrideWithString(h.TargetAddress, other.TargetAddress)
h.VPN.overrideWith(other.VPN)
}
func (h *Health) SetDefaults() {
h.ServerAddress = helpers.DefaultString(h.ServerAddress, "127.0.0.1:9999")
h.AddressToPing = helpers.DefaultString(h.AddressToPing, "github.com")
h.TargetAddress = helpers.DefaultString(h.TargetAddress, "github.com:443")
h.VPN.setDefaults()
}
@@ -76,7 +76,7 @@ func (h Health) String() string {
func (h Health) toLinesNode() (node *gotree.Node) {
node = gotree.New("Health settings:")
node.Appendf("Server listening address: %s", h.ServerAddress)
node.Appendf("Address to ping: %s", h.AddressToPing)
node.Appendf("Target address: %s", h.TargetAddress)
node.AppendNode(h.VPN.toLinesNode("VPN"))
return node
}

View File

@@ -66,7 +66,7 @@ func Test_Settings_String(t *testing.T) {
| └── Log level: INFO
├── Health settings:
| ├── Server listening address: 127.0.0.1:9999
| ├── Address to ping: github.com
| ├── Target address: github.com:443
| └── VPN wait durations:
| ├── Initial duration: 6s
| └── Additional duration: 5s

View File

@@ -10,7 +10,7 @@ import (
func (r *Reader) ReadHealth() (health settings.Health, err error) {
health.ServerAddress = os.Getenv("HEALTH_SERVER_ADDRESS")
health.AddressToPing = os.Getenv("HEALTH_ADDRESS_TO_PING")
_, health.TargetAddress = r.getEnvWithRetro("HEALTH_TARGET_ADDRESS", "HEALTH_ADDRESS_TO_PING")
health.VPN.Initial, err = r.readDurationWithRetro(
"HEALTH_VPN_DURATION_INITIAL",

View File

@@ -3,6 +3,9 @@ package healthcheck
import (
"context"
"errors"
"fmt"
"net"
"time"
)
@@ -14,7 +17,12 @@ func (s *Server) runHealthcheckLoop(ctx context.Context, done chan<- struct{}) {
for {
previousErr := s.handler.getErr()
err := healthCheck(ctx, s.pinger)
const healthcheckTimeout = 3 * time.Second
healthcheckCtx, healthcheckCancel := context.WithTimeout(
ctx, healthcheckTimeout)
err := s.healthCheck(healthcheckCtx)
healthcheckCancel()
s.handler.setErr(err)
if previousErr != nil && err == nil {
@@ -56,23 +64,40 @@ func (s *Server) runHealthcheckLoop(ctx context.Context, done chan<- struct{}) {
}
}
func healthCheck(ctx context.Context, pinger Pinger) (err error) {
func (s *Server) healthCheck(ctx context.Context) (err error) {
// TODO use mullvad API if current provider is Mullvad
// 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:
address, err := makeAddressToDial(s.config.TargetAddress)
if err != nil {
return err
}
const dialNetwork = "tcp4"
connection, err := s.dialer.DialContext(ctx, dialNetwork, address)
if err != nil {
return fmt.Errorf("cannot dial: %w", err)
}
err = connection.Close()
if err != nil {
return fmt.Errorf("cannot close connection: %w", err)
}
return nil
}
func makeAddressToDial(address string) (addressToDial string, err error) {
host, port, err := net.SplitHostPort(address)
if err != nil {
addrErr := new(net.AddrError)
ok := errors.As(err, &addrErr)
if !ok || addrErr.Err != "missing port in address" {
return "", fmt.Errorf("cannot split host and port from address: %w", err)
}
host = address
const defaultPort = "443"
port = defaultPort
}
address = net.JoinHostPort(host, port)
return address, nil
}

View File

@@ -1,46 +0,0 @@
//go:build integration
package healthcheck
import (
"context"
"testing"
"time"
"github.com/stretchr/testify/assert"
)
func Test_healthCheck_ping(t *testing.T) {
t.Parallel()
const timeout = time.Second
testCases := map[string]struct {
address string
err error
}{
"github.com": {
address: "github.com",
},
"99.99.99.99": {
address: "99.99.99.99",
err: context.DeadlineExceeded,
},
}
for name, testCase := range testCases {
testCase := testCase
t.Run(name, func(t *testing.T) {
t.Parallel()
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()
pinger := newPinger(testCase.address)
err := healthCheck(ctx, pinger)
assert.ErrorIs(t, testCase.err, err)
})
}
}

View File

@@ -2,40 +2,90 @@ package healthcheck
import (
"context"
"errors"
"fmt"
"net"
"testing"
"time"
"github.com/golang/mock/gomock"
"github.com/qdm12/gluetun/internal/configuration/settings"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func Test_healthCheck(t *testing.T) {
func Test_Server_healthCheck(t *testing.T) {
t.Parallel()
canceledCtx, cancel := context.WithCancel(context.Background())
cancel()
t.Run("canceled real dialer", func(t *testing.T) {
t.Parallel()
someErr := errors.New("error")
dialer := &net.Dialer{}
const address = "github.com:443"
server := &Server{
dialer: dialer,
config: settings.Health{
TargetAddress: address,
},
}
canceledCtx, cancel := context.WithCancel(context.Background())
cancel()
err := server.healthCheck(canceledCtx)
require.Error(t, err)
assert.Contains(t, err.Error(), "operation was canceled")
})
t.Run("dial localhost:0", func(t *testing.T) {
t.Parallel()
listener, err := net.Listen("tcp4", "localhost:0")
require.NoError(t, err)
t.Cleanup(func() {
err = listener.Close()
assert.NoError(t, err)
})
listeningAddress := listener.Addr()
dialer := &net.Dialer{}
server := &Server{
dialer: dialer,
config: settings.Health{
TargetAddress: listeningAddress.String(),
},
}
const timeout = 100 * time.Millisecond
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()
err = server.healthCheck(ctx)
assert.NoError(t, err)
})
}
func Test_makeAddressToDial(t *testing.T) {
t.Parallel()
testCases := map[string]struct {
ctx context.Context
runErr error
stopCall bool
err error
address string
addressToDial string
err error
}{
"success": {
ctx: context.Background(),
"host without port": {
address: "test.com",
addressToDial: "test.com:443",
},
"error": {
ctx: context.Background(),
runErr: someErr,
err: someErr,
"host with port": {
address: "test.com:80",
addressToDial: "test.com:80",
},
"context canceled": {
ctx: canceledCtx,
stopCall: true,
err: context.Canceled,
"bad address": {
address: "test.com::",
err: fmt.Errorf("cannot split host and port from address: address test.com::: too many colons in address"), //nolint:lll
},
}
@@ -43,54 +93,15 @@ func Test_healthCheck(t *testing.T) {
testCase := testCase
t.Run(name, func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
stopped := make(chan struct{})
addressToDial, err := makeAddressToDial(testCase.address)
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)
})
assert.Equal(t, testCase.addressToDial, addressToDial)
if testCase.err != nil {
assert.EqualError(t, err, testCase.err.Error())
} else {
assert.NoError(t, err)
}
err := healthCheck(testCase.ctx, pinger)
assert.ErrorIs(t, testCase.err, err)
})
}
t.Run("canceled real pinger", func(t *testing.T) {
t.Parallel()
pinger := newPinger("github.com")
canceledCtx, cancel := context.WithCancel(context.Background())
cancel()
err := healthCheck(canceledCtx, pinger)
assert.ErrorIs(t, context.Canceled, err)
})
t.Run("ping 127.0.0.1", func(t *testing.T) {
t.Parallel()
pinger := newPinger("127.0.0.1")
const timeout = 100 * time.Millisecond
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()
err := healthCheck(ctx, pinger)
assert.NoError(t, err)
})
}

View File

@@ -1,18 +0,0 @@
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(addrToPing string) (pinger *ping.Pinger) {
const count = 1
pinger = ping.New(addrToPing)
pinger.Count = count
pinger.SetPrivileged(true) // see https://github.com/go-ping/ping#linux
return pinger
}

View File

@@ -1,60 +0,0 @@
// 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))
}

View File

@@ -2,6 +2,7 @@ package healthcheck
import (
"context"
"net"
"github.com/qdm12/gluetun/internal/configuration/settings"
"github.com/qdm12/gluetun/internal/vpn"
@@ -16,7 +17,7 @@ type ServerRunner interface {
type Server struct {
logger Logger
handler *handler
pinger Pinger
dialer *net.Dialer
config settings.Health
vpn vpnHealth
}
@@ -26,7 +27,7 @@ func NewServer(config settings.Health,
return &Server{
logger: logger,
handler: newHandler(),
pinger: newPinger(config.AddressToPing),
dialer: &net.Dialer{},
config: config,
vpn: vpnHealth{
looper: vpnLooper,