Repository: jkroepke/openvpn-auth-oauth2
Pull request #436 (2025-03-15 19:57:21)

fix: tests
#### What this PR does / why we need it

#### Which issue this PR fixes

*(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

- fixes #

#### Special notes for your reviewer

#### Particularly user-facing changes

#### Checklist

Complete these before marking the PR as `ready to review`:

<!-- [Place an '[x]' (no spaces) in all applicable fields.] -->

- [ ] [DCO](https://github.com/prometheus-community/helm-charts/blob/main/CONTRIBUTING.md#sign-off-your-work) signed
- [ ] The PR title has a summary of the changes
- [ ] The PR body has a summary to reflect any significant (and particularly user-facing) changes introduced by this PR



Patch:
diff --git a/internal/openvpn/client.go b/internal/openvpn/client.go
index cc665ee..1705c2b 100644
--- a/internal/openvpn/client.go
+++ b/internal/openvpn/client.go
@@ -36,7 +36,7 @@ func (c *Client) processClient(ctx context.Context, client connection.Client) er
 	case "DISCONNECT":
 		c.clientDisconnect(ctx, logger, client)
 	default:
-		return fmt.Errorf("unknown client reason: %s", client.Reason)
+		return fmt.Errorf("%w: %s", ErrUnknownClientReason, client.Reason)
 	}
 
 	return nil
diff --git a/internal/openvpn/connection/client.go b/internal/openvpn/connection/client.go
index 37894d2..9e38c43 100644
--- a/internal/openvpn/connection/client.go
+++ b/internal/openvpn/connection/client.go
@@ -79,7 +79,7 @@ func NewClient(conf config.Config, message string) (Client, error) { //nolint:cy
 	}
 
 	if client.Reason == "" {
-		return Client{}, fmt.Errorf("unable to parse client reason from message: %s", message)
+		return Client{}, fmt.Errorf("%w: %s", ErrParseErrorClientReason, message)
 	}
 
 	return client, nil
diff --git a/internal/openvpn/connection/errors.go b/internal/openvpn/connection/errors.go
index 331345b..6a7be17 100644
--- a/internal/openvpn/connection/errors.go
+++ b/internal/openvpn/connection/errors.go
@@ -5,6 +5,7 @@ import (
 )
 
 var (
-	ErrEmptyClientReasons = errors.New("empty client reason")
-	ErrInvalidMessage     = errors.New("message invalid")
+	ErrEmptyClientReasons     = errors.New("empty client reason")
+	ErrInvalidMessage         = errors.New("message invalid")
+	ErrParseErrorClientReason = errors.New("unable to parse client reason from message")
 )
diff --git a/internal/openvpn/errors.go b/internal/openvpn/errors.go
index ee6228c..b585f10 100644
--- a/internal/openvpn/errors.go
+++ b/internal/openvpn/errors.go
@@ -5,13 +5,16 @@ import (
 )
 
 var (
-	ErrTimeout                            = errors.New("timeout")
-	ErrEmptyResponse                      = errors.New("empty response")
-	ErrUnknownProtocol                    = errors.New("unknown protocol")
-	ErrInvalidPassword                    = errors.New("invalid password")
-	ErrErrorResponse                      = errors.New("error response")
-	ErrConnectionTerminated               = errors.New("openvpn management interface connection terminated")
-	ErrClientSessionStateInvalidOrExpired = errors.New(ReasonStateExpiredOrInvalid)
+	ErrTimeout                              = errors.New("timeout")
+	ErrEmptyResponse                        = errors.New("empty response")
+	ErrUnknownProtocol                      = errors.New("unknown protocol")
+	ErrInvalidPassword                      = errors.New("invalid password")
+	ErrErrorResponse                        = errors.New("error response")
+	ErrConnectionTerminated                 = errors.New("openvpn management interface connection terminated")
+	ErrUnknownClientReason                  = errors.New("unknown client reason")
+	ErrUnexpectedResponseFromVersionCommand = errors.New("unexpected response from version command")
+	ErrRequireManagementInterfaceVersion5   = errors.New("openvpn-auth-oauth2 requires OpenVPN management interface version 5 or higher")
+	ErrClientSessionStateInvalidOrExpired   = errors.New(ReasonStateExpiredOrInvalid)
 )
 
 const (
diff --git a/internal/openvpn/handler.go b/internal/openvpn/handler.go
index 1c5b8de..b672a72 100644
--- a/internal/openvpn/handler.go
+++ b/internal/openvpn/handler.go
@@ -68,7 +68,7 @@ func (c *Client) sendPassword(ctx context.Context) error {
 	}
 
 	if !strings.Contains("SUCCESS: password is correct", string(buf)) { //nolint:gocritic
-		return fmt.Errorf("unable to connect to openvpn management interface: %w", ErrInvalidPassword)
+		return ErrInvalidPassword
 	}
 
 	// read the whole line
diff --git a/internal/openvpn/main.go b/internal/openvpn/main.go
index a0ce12d..854b836 100644
--- a/internal/openvpn/main.go
+++ b/internal/openvpn/main.go
@@ -65,7 +65,7 @@ func (c *Client) Connect(ctx context.Context) error {
 	c.scanner.Buffer(make([]byte, 0, bufio.MaxScanTokenSize), bufio.MaxScanTokenSize)
 
 	if err = c.handlePassword(ctx); err != nil {
-		return fmt.Errorf("openvpn management error: %w", err)
+		return fmt.Errorf("unable to authenticate with OpenVPN management interface: %w", err)
 	}
 
 	defer c.Shutdown()
@@ -90,14 +90,26 @@ func (c *Client) Connect(ctx context.Context) error {
 			return nil
 		}
 
-		return fmt.Errorf("openvpn management error: %w", err)
+		return fmt.Errorf("unable to check OpenVPN management interface version: %w", err)
 	}
 
 	select {
 	case err = <-errChMessages:
+		if err != nil {
+			err = fmt.Errorf("error handling messages: %w", err)
+		}
 	case err = <-errChClients:
+		if err != nil {
+			err = fmt.Errorf("error handling clients: %w", err)
+		}
 	case err = <-errChCommands:
+		if err != nil {
+			err = fmt.Errorf("error handling commands: %w", err)
+		}
 	case err = <-errChPassThrough:
+		if err != nil {
+			err = fmt.Errorf("error handling passthrough: %w", err)
+		}
 	}
 
 	if err != nil {
@@ -142,7 +154,7 @@ func (c *Client) checkManagementInterfaceVersion() error {
 	versionParts := strings.Split(resp, "\r\n")
 
 	if len(versionParts) != 4 {
-		return fmt.Errorf("unexpected response from version command: %s", resp)
+		return fmt.Errorf("%w: %s", ErrUnexpectedResponseFromVersionCommand, resp)
 	}
 
 	c.logger.Info(utils.StringConcat(versionParts[0], " - ", versionParts[1]))
@@ -155,7 +167,7 @@ func (c *Client) checkManagementInterfaceVersion() error {
 	// Management Interface Version 5 is required at minimum
 	// ref: https://github.com/OpenVPN/openvpn/commit/a261e173341f8e68505a6ab5a413d09b0797a459
 	if managementInterfaceVersion < minManagementInterfaceVersion {
-		return errors.New("openvpn-auth-oauth2 requires OpenVPN management interface version 5 or higher")
+		return ErrRequireManagementInterfaceVersion5
 	}
 
 	return nil


Test patch:
diff --git a/internal/openvpn/main_test.go b/internal/openvpn/main_test.go
index 8acd783..e828ad2 100644
--- a/internal/openvpn/main_test.go
+++ b/internal/openvpn/main_test.go
@@ -8,12 +8,15 @@ import (
 	"io"
 	"net/http"
 	"regexp"
+	"strconv"
 	"strings"
 	"sync"
 	"testing"
 	"time"
 
 	"github.com/jkroepke/openvpn-auth-oauth2/internal/config"
+	"github.com/jkroepke/openvpn-auth-oauth2/internal/openvpn"
+	"github.com/jkroepke/openvpn-auth-oauth2/internal/openvpn/connection"
 	"github.com/jkroepke/openvpn-auth-oauth2/internal/state"
 	"github.com/jkroepke/openvpn-auth-oauth2/internal/tokenstorage"
 	"github.com/jkroepke/openvpn-auth-oauth2/internal/utils/testutils"
@@ -276,8 +279,7 @@ func TestClientFull(t *testing.T) {
 			},
 			">CLIENT:FOO,0\r\n>CLIENT:ENV,common_name=bypass\r\n>CLIENT:ENV,END\r\n",
 			"",
-			//nolint:revive
-			errors.New("openvpn management error: error parsing client message: unable to parse client reason from message: >CLIENT:FOO,0\r\n>CLIENT:ENV,common_name=bypass\r\n>CLIENT:ENV,END\r\n"),
+			connection.ErrParseErrorClientReason,
 		},
 		{
 			"client invalid reason 2",
@@ -296,7 +298,7 @@ func TestClientFull(t *testing.T) {
 			},
 			">CLIENT:CONNECT1,0,1\r\n>CLIENT:ENV,common_name=bypass\r\n>CLIENT:ENV,END\r\n",
 			"",
-			errors.New("openvpn management error: unknown client reason: CONNECT1"),
+			openvpn.ErrUnknownClientReason,
 		},
 	}
 
@@ -382,8 +384,7 @@ func TestClientFull(t *testing.T) {
 
 			err = openVPNClient.Connect(t.Context())
 			if tt.err != nil {
-				require.Error(t, err)
-				assert.Equal(t, tt.err.Error(), err.Error())
+				require.ErrorIs(t, err, tt.err)
 			} else {
 				wg.Wait()
 
@@ -439,7 +440,7 @@ func TestClientInvalidPassword(t *testing.T) {
 
 	err = openVPNClient.Connect(t.Context())
 
-	require.EqualError(t, err, "openvpn management error: unable to connect to openvpn management interface: invalid password")
+	require.ErrorIs(t, err, openvpn.ErrInvalidPassword)
 }
 
 func TestClientInvalidVersion(t *testing.T) {
@@ -460,22 +461,22 @@ func TestClientInvalidVersion(t *testing.T) {
 	versions := []struct {
 		name    string
 		version string
-		err     string
+		err     error
 	}{
 		{
 			"invalid parts",
 			"OpenVPN Version: OpenVPN Mock\r\nEND\r\n",
-			"openvpn management error: unexpected response from version command: OpenVPN Version: OpenVPN Mock\r\nEND\r\n",
+			openvpn.ErrUnexpectedResponseFromVersionCommand,
 		},
 		{
 			"invalid version",
 			"OpenVPN Version: OpenVPN Mock\r\nManagement Interface Version:\r\nEND\r\n",
-			`openvpn management error: unable to parse openvpn management interface version: strconv.Atoi: parsing ":": invalid syntax`,
+			strconv.ErrSyntax,
 		},
 		{
 			"version to low",
 			"OpenVPN Version: OpenVPN Mock\r\nManagement Interface Version: 4\r\nEND\r\n",
-			`openvpn management error: openvpn-auth-oauth2 requires OpenVPN management interface version 5 or higher`,
+			openvpn.ErrRequireManagementInterfaceVersion5,
 		},
 	}
 
@@ -531,7 +532,7 @@ func TestClientInvalidVersion(t *testing.T) {
 
 			err = <-errCh
 
-			require.EqualError(t, err, tt.err, tt.err)
+			require.ErrorIs(t, err, tt.err)
 		})
 	}
 }