Add additional comments.

Author: tecarter94

pkg/domainproxy/client/client.go

@@ -1,11 +1,9 @@
 package client
 
 import (
-	"context"
 	"fmt"
 	. "github.com/redhat-appstudio/jvm-build-service/pkg/domainproxy/common"
 	"net"
-	"sync/atomic"
 	"time"
 )
 
@@ -20,45 +18,40 @@ var logger = NewLogger("Domain Proxy Client")
 var common = NewCommon(logger)
 
 type DomainProxyClient struct {
-	sharedParams          SharedParams
-	httpPort              int
-	httpConnectionCounter atomic.Uint64
-	listener              net.Listener
-	shutdownContext       context.Context
-	initiateShutdown      context.CancelFunc
+	sharedParams *SharedParams
+	httpPort     int
 }
 
 func NewDomainProxyClient() *DomainProxyClient {
-	shutdownContext, initiateShutdown := context.WithCancel(context.Background())
 	return &DomainProxyClient{
-		sharedParams:     common.NewSharedParams(),
-		httpPort:         getHttpPort(),
-		shutdownContext:  shutdownContext,
-		initiateShutdown: initiateShutdown,
+		sharedParams: common.NewSharedParams(),
+		httpPort:     getHttpPort(),
 	}
 }
 
 func (dpc *DomainProxyClient) Start(ready chan<- bool) {
+	sharedParams := dpc.sharedParams
 	logger.Println("Starting domain proxy client...")
 	var err error
-	dpc.listener, err = net.Listen(TCP, fmt.Sprintf("%s:%d", Localhost, dpc.httpPort))
+	sharedParams.Listener, err = net.Listen(TCP, fmt.Sprintf("%s:%d", Localhost, dpc.httpPort))
 	if err != nil {
 		logger.Fatalf("Failed to start HTTP server: %v", err)
 	}
 	go dpc.startClient(ready)
 }
 
 func (dpc *DomainProxyClient) startClient(ready chan<- bool) {
+	sharedParams := dpc.sharedParams
 	logger.Printf("HTTP server listening on port %d", dpc.httpPort)
 	ready <- true
 	for {
 		select {
-		case <-dpc.shutdownContext.Done():
+		case <-sharedParams.RunningContext.Done():
 			return
 		default:
-			if serverConnection, err := dpc.listener.Accept(); err != nil {
+			if serverConnection, err := sharedParams.Listener.Accept(); err != nil {
 				select {
-				case <-dpc.shutdownContext.Done():
+				case <-sharedParams.RunningContext.Done():
 					return
 				default:
 					logger.Printf("Failed to accept server connection: %v", err)
@@ -76,7 +69,7 @@ func (dpc *DomainProxyClient) handleConnectionRequest(serverConnection net.Conn)
 		common.HandleSetDeadlineError(serverConnection, err)
 		return
 	}
-	connectionNo := dpc.httpConnectionCounter.Add(1)
+	connectionNo := sharedParams.HttpConnectionCounter.Add(1)
 	logger.Printf("Handling %s Connection %d", HttpToDomainSocket, connectionNo)
 	startTime := time.Now()
 	domainConnection, err := net.DialTimeout(UNIX, sharedParams.DomainSocket, sharedParams.ConnectionTimeout)
@@ -94,17 +87,19 @@ func (dpc *DomainProxyClient) handleConnectionRequest(serverConnection net.Conn)
 		}
 		return
 	}
+	// Initiate transfer between server and domain
 	go func() {
-		common.BiDirectionalTransfer(dpc.shutdownContext, serverConnection, domainConnection, sharedParams.ByteBufferSize, HttpToDomainSocket, connectionNo)
+		common.BiDirectionalTransfer(sharedParams.RunningContext, serverConnection, domainConnection, sharedParams.ByteBufferSize, HttpToDomainSocket, connectionNo)
 		logger.Printf("%s Connection %d ended after %d ms", HttpToDomainSocket, connectionNo, time.Since(startTime).Milliseconds())
 	}()
 }
 
 func (dpc *DomainProxyClient) Stop() {
+	sharedParams := dpc.sharedParams
 	logger.Println("Shutting down domain proxy client...")
-	dpc.initiateShutdown()
-	if dpc.listener != nil {
-		if err := dpc.listener.Close(); err != nil {
+	sharedParams.InitiateShutdown()
+	if sharedParams.Listener != nil {
+		if err := sharedParams.Listener.Close(); err != nil {
 			common.HandleListenerCloseError(err)
 		}
 	}

pkg/domainproxy/common/common.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"strconv"
 	"strings"
+	"sync/atomic"
 	"time"
 )
 
@@ -30,10 +31,14 @@ type Common struct {
 }
 
 type SharedParams struct {
-	ByteBufferSize    int
-	DomainSocket      string
-	ConnectionTimeout time.Duration
-	IdleTimeout       time.Duration
+	ByteBufferSize        int
+	DomainSocket          string
+	ConnectionTimeout     time.Duration
+	IdleTimeout           time.Duration
+	HttpConnectionCounter atomic.Uint64
+	Listener              net.Listener
+	RunningContext        context.Context
+	InitiateShutdown      context.CancelFunc
 }
 
 func NewCommon(logger *log.Logger) *Common {
@@ -46,12 +51,15 @@ func NewLogger(appName string) *log.Logger {
 	return log.New(os.Stdout, appName+" ", log.LstdFlags|log.Lshortfile)
 }
 
-func (c *Common) NewSharedParams() SharedParams {
-	return SharedParams{
+func (c *Common) NewSharedParams() *SharedParams {
+	runningContext, initiateShutdown := context.WithCancel(context.Background())
+	return &SharedParams{
 		ByteBufferSize:    c.getByteBufferSize(),
 		DomainSocket:      c.getDomainSocket(),
 		ConnectionTimeout: c.getConnectionTimeout(),
 		IdleTimeout:       c.getIdleTimeout(),
+		RunningContext:    runningContext,
+		InitiateShutdown:  initiateShutdown,
 	}
 }
 
@@ -77,6 +85,7 @@ func (c *Common) Transfer(transferContext context.Context, terminateTransfer con
 			} else if n > 0 {
 				c.logger.Printf("%d bytes transferred for %s connection %d", n, connectionType, connectionNo)
 			} else {
+				// Nothing more to transfer
 				return
 			}
 		}
@@ -100,7 +109,7 @@ func (c *Common) HandleListenerCloseError(err error) {
 
 func (c *Common) handleConnectionError(err error, connectionType string, connectionNo uint64) {
 	var netErr net.Error
-	if !errors.Is(err, net.ErrClosed) {
+	if !errors.Is(err, net.ErrClosed) { // We don't care if connection has been closed, because this is expected
 		if errors.As(err, &netErr) && netErr.Timeout() {
 			c.logger.Printf("%s connection %d timed out", connectionType, connectionNo)
 		} else if err != io.EOF {

pkg/domainproxy/integration/domainproxy_test.go

@@ -247,9 +247,10 @@ func commonTestBehaviour(t *testing.T, qualifier string) {
 
 	t.Run(fmt.Sprintf("Test HTTPS non-existent host%s", qualifier), func(t *testing.T) {
 		_, err := httpClient.Get("https://" + NonExistentHost)
-		statusText := http.StatusText(http.StatusBadGateway)
-		if !strings.Contains(err.Error(), statusText) {
-			t.Fatalf("Actual error %s did not contain expected HTTP status text %s", err.Error(), statusText)
+		internalServerStatusText := http.StatusText(http.StatusInternalServerError)
+		badGatewayStatusText := http.StatusText(http.StatusBadGateway)
+		if !strings.Contains(err.Error(), internalServerStatusText) && !strings.Contains(err.Error(), badGatewayStatusText) { // Internal proxy may return 502 Bad Gateway
+			t.Fatalf("Actual error %s did not contain expected HTTP status text %s or %s", err.Error(), internalServerStatusText, badGatewayStatusText)
 		}
 	})