Feat: HEALTH_ADDRESS_TO_PING variable

- Defaults to `1.1.1.1`
- Add more Ping integration tests with different addresses
- Add unit test pinging 127.0.0.1
- Add comment explaining why we need to use ICMP instead of UDP

Dockerfile

@@ -125,6 +125,7 @@ ENV VPNSP=pia \
     LOG_LEVEL=info \
     # Health
     HEALTH_SERVER_ADDRESS=127.0.0.1:9999 \
+    HEALTH_ADDRESS_TO_PING=1.1.1.1 \
     HEALTH_OPENVPN_DURATION_INITIAL=6s \
     HEALTH_OPENVPN_DURATION_ADDITION=5s \
     # DNS over TLS

internal/configuration/health.go

@@ -12,6 +12,7 @@ import (
 // Health contains settings for the healthcheck and health server.
 type Health struct {
 	ServerAddress string
+	AddressToPing string
 	VPN           HealthyWait
 }
 
@@ -24,6 +25,8 @@ func (settings *Health) lines() (lines []string) {
 
 	lines = append(lines, indent+lastIndent+"Server address: "+settings.ServerAddress)
 
+	lines = append(lines, indent+lastIndent+"Address to ping: "+settings.AddressToPing)
+
 	lines = append(lines, indent+lastIndent+"VPN:")
 	for _, line := range settings.VPN.lines() {
 		lines = append(lines, indent+indent+line)
@@ -49,6 +52,11 @@ func (settings *Health) read(r reader) (err error) {
 		return fmt.Errorf("environment variable HEALTH_SERVER_ADDRESS: %w", err)
 	}
 
+	settings.AddressToPing, err = r.env.Get("HEALTH_ADDRESS_TO_PING", params.Default("1.1.1.1"))
+	if err != nil {
+		return fmt.Errorf("environment variable HEALTH_ADDRESS_TO_PING: %w", err)
+	}
+
 	retroKeyOption := params.RetroKeys([]string{"HEALTH_OPENVPN_DURATION_INITIAL"}, r.onRetroActive)
 	settings.VPN.Initial, err = r.env.Duration("HEALTH_VPN_DURATION_INITIAL", params.Default("6s"), retroKeyOption)
 	if err != nil {

internal/configuration/health_test.go

@@ -15,8 +15,15 @@ import (
 func Test_Health_String(t *testing.T) {
 	t.Parallel()
 
-	var health Health
+	health := Health{
-	const expected = "|--Health:\n   |--Server address: \n   |--VPN:\n      |--Initial duration: 0s"
+		ServerAddress: "a",
+		AddressToPing: "b",
+	}
+	const expected = `|--Health:
+   |--Server address: a
+   |--Address to ping: b
+   |--VPN:
+      |--Initial duration: 0s`
 
 	s := health.String()
 
@@ -34,6 +41,7 @@ func Test_Health_lines(t *testing.T) {
 			lines: []string{
 				"|--Health:",
 				"   |--Server address: ",
+				"   |--Address to ping: ",
 				"   |--VPN:",
 				"      |--Initial duration: 0s",
 			},
@@ -41,6 +49,7 @@ func Test_Health_lines(t *testing.T) {
 		"filled settings": {
 			settings: Health{
 				ServerAddress: "address:9999",
+				AddressToPing: "1.1.1.1",
 				VPN: HealthyWait{
 					Initial:  time.Second,
 					Addition: time.Minute,
@@ -49,6 +58,7 @@ func Test_Health_lines(t *testing.T) {
 			lines: []string{
 				"|--Health:",
 				"   |--Server address: address:9999",
+				"   |--Address to ping: 1.1.1.1",
 				"   |--VPN:",
 				"      |--Initial duration: 1s",
 				"      |--Addition duration: 1m0s",
@@ -73,6 +83,12 @@ func Test_Health_read(t *testing.T) {
 
 	errDummy := errors.New("dummy")
 
+	type stringCall struct {
+		call bool
+		s    string
+		err  error
+	}
+
 	type stringCallWithWarning struct {
 		call    bool
 		s       string
@@ -88,6 +104,7 @@ func Test_Health_read(t *testing.T) {
 
 	testCases := map[string]struct {
 		serverAddress stringCallWithWarning
+		addressToPing stringCall
 		vpnInitial    durationCall
 		vpnAddition   durationCall
 		expected      Health
@@ -98,6 +115,10 @@ func Test_Health_read(t *testing.T) {
 				call: true,
 				s:    "127.0.0.1:9999",
 			},
+			addressToPing: stringCall{
+				call: true,
+				s:    "1.2.3.4",
+			},
 			vpnInitial: durationCall{
 				call:     true,
 				duration: time.Second,
@@ -108,6 +129,7 @@ func Test_Health_read(t *testing.T) {
 			},
 			expected: Health{
 				ServerAddress: "127.0.0.1:9999",
+				AddressToPing: "1.2.3.4",
 				VPN: HealthyWait{
 					Initial:  time.Second,
 					Addition: time.Minute,
@@ -126,10 +148,27 @@ func Test_Health_read(t *testing.T) {
 			},
 			err: errors.New("environment variable HEALTH_SERVER_ADDRESS: dummy"),
 		},
+		"address to ping error": {
+			serverAddress: stringCallWithWarning{
+				call: true,
+			},
+			addressToPing: stringCall{
+				call: true,
+				s:    "address",
+				err:  errDummy,
+			},
+			expected: Health{
+				AddressToPing: "address",
+			},
+			err: errors.New("environment variable HEALTH_ADDRESS_TO_PING: dummy"),
+		},
 		"initial error": {
 			serverAddress: stringCallWithWarning{
 				call: true,
 			},
+			addressToPing: stringCall{
+				call: true,
+			},
 			vpnInitial: durationCall{
 				call:     true,
 				duration: time.Second,
@@ -146,6 +185,9 @@ func Test_Health_read(t *testing.T) {
 			serverAddress: stringCallWithWarning{
 				call: true,
 			},
+			addressToPing: stringCall{
+				call: true,
+			},
 			vpnInitial: durationCall{
 				call:     true,
 				duration: time.Second,
@@ -186,6 +228,13 @@ func Test_Health_read(t *testing.T) {
 				}
 			}
 
+			if testCase.addressToPing.call {
+				value := testCase.addressToPing.s
+				err := testCase.addressToPing.err
+				env.EXPECT().Get("HEALTH_ADDRESS_TO_PING", gomock.Any()).
+					Return(value, err)
+			}
+
 			if testCase.vpnInitial.call {
 				value := testCase.vpnInitial.duration
 				err := testCase.vpnInitial.err

internal/configuration/settings_test.go

@@ -51,6 +51,7 @@ func Test_Settings_lines(t *testing.T) {
 				"   |--Timezone: NOT SET ⚠️ - it can cause time related issues",
 				"|--Health:",
 				"   |--Server address: ",
+				"   |--Address to ping: ",
 				"   |--VPN:",
 				"      |--Initial duration: 0s",
 				"|--HTTP control server:",

internal/healthcheck/health_ping_test.go

@@ -5,6 +5,7 @@ package healthcheck
 import (
 	"context"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -12,9 +13,34 @@ import (
 func Test_healthCheck_ping(t *testing.T) {
 	t.Parallel()
 
-	pinger := newPinger()
+	const timeout = time.Second
 
-	err := healthCheck(context.Background(), pinger)
+	testCases := map[string]struct {
+		address string
+		err     error
+	}{
+		"1.1.1.1": {
+			address: "1.1.1.1",
+		},
+		"99.99.99.99": {
+			address: "99.99.99.99",
+			err:     context.DeadlineExceeded,
+		},
+	}
 
-	assert.NoError(t, err)
+	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)
+		})
+	}
 }

internal/healthcheck/health_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"testing"
+	"time"
 
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
@@ -69,7 +70,7 @@ func Test_healthCheck(t *testing.T) {
 	t.Run("canceled real pinger", func(t *testing.T) {
 		t.Parallel()
 
-		pinger := newPinger()
+		pinger := newPinger("1.1.1.1")
 
 		canceledCtx, cancel := context.WithCancel(context.Background())
 		cancel()
@@ -78,4 +79,18 @@ func Test_healthCheck(t *testing.T) {
 
 		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)
+	})
 }

internal/healthcheck/ping.go

@@ -9,11 +9,10 @@ type Pinger interface {
 	Stop()
 }
 
-func newPinger() (pinger *ping.Pinger) {
+func newPinger(addrToPing string) (pinger *ping.Pinger) {
-	const addrToPing = "1.1.1.1"
 	const count = 1
 	pinger = ping.New(addrToPing)
 	pinger.Count = count
-	pinger.SetPrivileged(true)
+	pinger.SetPrivileged(true) // see https://github.com/go-ping/ping#linux
 	return pinger
 }

internal/healthcheck/server.go

@@ -27,7 +27,7 @@ func NewServer(config configuration.Health,
 	return &Server{
 		logger:  logger,
 		handler: newHandler(logger),
-		pinger:  newPinger(),
+		pinger:  newPinger(config.AddressToPing),
 		config:  config,
 		vpn: vpnHealth{
 			looper:      vpnLooper,