Follow HTTP redirects after failed WS dials

This commit allows the opamp client to follow redirects after websocket
handshake failures. Redirect following is not implemented by
gorilla/websocket, but can be handled by inspecting the returned
response object for 3xx status and Location header.

client/wsclient.go

@@ -29,7 +29,7 @@ type wsClient struct {
 	requestHeader http.Header
 
 	// Websocket dialer and connection.
-	dialer    websocket.Dialer
+	dialer    *websocket.Dialer
 	conn      *websocket.Conn
 	connMutex sync.RWMutex
 
@@ -57,7 +57,7 @@ func (c *wsClient) Start(ctx context.Context, settings types.StartSettings) erro
 	}
 
 	// Prepare connection settings.
-	c.dialer = *websocket.DefaultDialer
+	c.dialer = websocket.DefaultDialer
 
 	var err error
 	c.url, err = url.Parse(settings.OpAMPServerURL)
@@ -131,8 +131,26 @@ func (c *wsClient) tryConnectOnce(ctx context.Context) (err error, retryAfter sh
 			c.common.Callbacks.OnConnectFailed(ctx, err)
 		}
 		if resp != nil {
-			c.common.Logger.Errorf(ctx, "Server responded with status=%v", resp.Status)
 			duration := sharedinternal.ExtractRetryAfterHeader(resp)
+			if resp.StatusCode >= 300 && resp.StatusCode < 400 {
+				// very liberal handling of 3xx that largely ignores HTTP semantics
+				redirect, err := resp.Location()
+				if err != nil {
+					c.common.Logger.Errorf(ctx, "3xx redirect, but no valid location: %s", err)
+					return err, duration
+				}
+				if redirect.Scheme == "http" || redirect.Scheme == "" {
+					redirect.Scheme = "ws"
+				} else if redirect.Scheme == "https" {
+					redirect.Scheme = "wss"
+				}
+				c.common.Logger.Debugf(ctx, "%d redirect to %s", resp.StatusCode, redirect)
+				// Set the URL to the redirect, so that it connects to it on the
+				// next cycle.
+				c.url = redirect
+			} else {
+				c.common.Logger.Errorf(ctx, "Server responded with status=%v", resp.Status)
+			}
 			return err, duration
 		}
 		return err, sharedinternal.OptionalDuration{Defined: false}

client/wsclient_test.go

@@ -3,6 +3,9 @@ package client
 import (
 	"context"
 	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
 	"strings"
 	"sync/atomic"
 	"testing"
@@ -177,3 +180,50 @@ func TestVerifyWSCompress(t *testing.T) {
 		})
 	}
 }
+
+func redirectServer(to string) *httptest.Server {
+	return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		http.Redirect(w, req, to, http.StatusSeeOther)
+	}))
+}
+
+func TestRedirectWS(t *testing.T) {
+	redirectee := internal.StartMockServer(t)
+	redirector := redirectServer("ws://" + redirectee.Endpoint)
+	defer redirector.Close()
+
+	var conn atomic.Value
+	redirectee.OnWSConnect = func(c *websocket.Conn) {
+		conn.Store(c)
+	}
+
+	// Start an OpAMP/WebSocket client.
+	var connected int64
+	var connectErr atomic.Value
+	settings := types.StartSettings{
+		Callbacks: types.CallbacksStruct{
+			OnConnectFunc: func(ctx context.Context) {
+				atomic.StoreInt64(&connected, 1)
+			},
+			OnConnectFailedFunc: func(ctx context.Context, err error) {
+				if err != websocket.ErrBadHandshake {
+					connectErr.Store(err)
+				}
+			},
+		},
+	}
+	reURL, err := url.Parse(redirector.URL)
+	assert.NoError(t, err)
+	reURL.Scheme = "ws"
+	settings.OpAMPServerURL = reURL.String()
+	client := NewWebSocket(nil)
+	startClient(t, settings, client)
+
+	// Wait for connection to be established.
+	eventually(t, func() bool { return conn.Load() != nil })
+	assert.True(t, connectErr.Load() == nil)
+
+	// Stop the client.
+	err = client.Stop(context.Background())
+	assert.NoError(t, err)
+}