From 4ea0f1e38236599e082e8906d20db4c76f0c92ab Mon Sep 17 00:00:00 2001 From: Owen Cabalceta Date: Thu, 28 Mar 2024 19:06:18 -0400 Subject: [PATCH] chore: fix tests and add new ws validators for nil funcs --- cmd/xmidt-agent/ws.go | 3 ++ cmd/xmidt-agent/xmidt_agent.yaml | 6 ++- internal/websocket/e2e_test.go | 48 +++++++++++++--------- internal/websocket/internal_options.go | 44 ++++++++++++++++++++- internal/websocket/options.go | 2 +- internal/websocket/ws.go | 4 ++ internal/websocket/ws_test.go | 55 ++++++++++++++++++++++---- 7 files changed, 132 insertions(+), 30 deletions(-) diff --git a/cmd/xmidt-agent/ws.go b/cmd/xmidt-agent/ws.go index d096517..5abd4d4 100644 --- a/cmd/xmidt-agent/ws.go +++ b/cmd/xmidt-agent/ws.go @@ -1,3 +1,6 @@ +// SPDX-FileCopyrightText: 2023 Comcast Cable Communications Management, LLC +// SPDX-License-Identifier: Apache-2.0 + package main import ( diff --git a/cmd/xmidt-agent/xmidt_agent.yaml b/cmd/xmidt-agent/xmidt_agent.yaml index 4fdbb0b..cc019a9 100644 --- a/cmd/xmidt-agent/xmidt_agent.yaml +++ b/cmd/xmidt-agent/xmidt_agent.yaml @@ -1,6 +1,8 @@ +# SPDX-FileCopyrightText: 2023 Comcast Cable Communications Management, LLC +# SPDX-License-Identifier: Apache-2.0 + websocket: - enable_defaults: true - registration_api: api/v2/device + url_path: api/v2/device xmidt_credentials: url: https://localhost:8080/issue file_name: crt.pem diff --git a/internal/websocket/e2e_test.go b/internal/websocket/e2e_test.go index f5a1723..2467cc7 100644 --- a/internal/websocket/e2e_test.go +++ b/internal/websocket/e2e_test.go @@ -51,7 +51,7 @@ func TestEndToEnd(t *testing.T) { err = wrp.NewDecoderBytes(got, wrp.Msgpack).Decode(&msg) require.NoError(err) require.Equal(wrp.SimpleEventMessageType, msg.Type) - require.Equal("client", msg.Source) + require.Equal("server", msg.Source) c.Close(websocket.StatusNormalClosure, "") })) @@ -79,14 +79,21 @@ func TestEndToEnd(t *testing.T) { func(event.Disconnect) { disconnectCnt.Add(1) })), - ws.WithIPv6(), ws.RetryPolicy(&retry.Config{ Interval: time.Second, Multiplier: 2.0, Jitter: 1.0 / 3.0, MaxInterval: 341*time.Second + 333*time.Millisecond, }), + ws.WithIPv6(), + ws.WithIPv4(), ws.NowFunc(time.Now), + ws.ConnectTimeout(30*time.Second), + ws.FetchURLTimeout(30*time.Second), + ws.MaxMessageBytes(256*1024), + ws.CredentialsDecorator(func(h http.Header) error { + return nil + }), ) require.NoError(err) require.NotNil(got) @@ -196,14 +203,19 @@ func TestEndToEndBadData(t *testing.T) { func(event.Disconnect) { disconnectCnt.Add(1) })), - ws.WithIPv6(), ws.RetryPolicy(&retry.Config{ - Interval: time.Second, - Multiplier: 2.0, - Jitter: 1.0 / 3.0, - MaxInterval: 341*time.Second + 333*time.Millisecond, + Interval: 50 * time.Millisecond, + Multiplier: 2.0, + MaxElapsedTime: 300 * time.Millisecond, }), + ws.WithIPv4(), ws.NowFunc(time.Now), + ws.ConnectTimeout(30*time.Second), + ws.FetchURLTimeout(30*time.Second), + ws.MaxMessageBytes(256*1024), + ws.CredentialsDecorator(func(h http.Header) error { + return nil + }), ) require.NoError(err) require.NotNil(got) @@ -244,7 +256,7 @@ func TestEndToEndConnectionIssues(t *testing.T) { require.NoError(err) defer c.CloseNow() - ctx, cancel := context.WithTimeout(r.Context(), 200*time.Millisecond) + ctx, cancel := context.WithTimeout(r.Context(), 2000000*time.Millisecond) defer cancel() msg := wrp.Message{ @@ -269,11 +281,6 @@ func TestEndToEndConnectionIssues(t *testing.T) { return s.URL, nil }), ws.DeviceID("mac:112233445566"), - ws.RetryPolicy(&retry.Config{ - Interval: 50 * time.Millisecond, - Multiplier: 2.0, - MaxElapsedTime: 300 * time.Millisecond, - }), ws.AddMessageListener( event.MsgListenerFunc( func(m wrp.Message) { @@ -289,14 +296,19 @@ func TestEndToEndConnectionIssues(t *testing.T) { func(event.Disconnect) { disconnectCnt.Add(1) })), - ws.WithIPv6(), ws.RetryPolicy(&retry.Config{ - Interval: time.Second, - Multiplier: 2.0, - Jitter: 1.0 / 3.0, - MaxInterval: 341*time.Second + 333*time.Millisecond, + Interval: 50 * time.Millisecond, + Multiplier: 2.0, + MaxElapsedTime: 300 * time.Millisecond, }), + ws.WithIPv4(), ws.NowFunc(time.Now), + ws.ConnectTimeout(30*time.Second), + ws.FetchURLTimeout(30*time.Second), + ws.MaxMessageBytes(256*1024), + ws.CredentialsDecorator(func(h http.Header) error { + return nil + }), ) require.NoError(err) require.NotNil(got) diff --git a/internal/websocket/internal_options.go b/internal/websocket/internal_options.go index 819fa03..58bb665 100644 --- a/internal/websocket/internal_options.go +++ b/internal/websocket/internal_options.go @@ -3,7 +3,9 @@ package websocket -import "fmt" +import ( + "fmt" +) func validateDeviceID() Option { return optionFunc( @@ -34,3 +36,43 @@ func validateIPMode() Option { return nil }) } + +func validateFetchURL() Option { + return optionFunc( + func(ws *Websocket) error { + if ws.urlFetcher == nil { + return fmt.Errorf("%w: nil FetchURL", ErrMisconfiguredWS) + } + return nil + }) +} + +func validateCredentialsDecorator() Option { + return optionFunc( + func(ws *Websocket) error { + if ws.credDecorator == nil { + return fmt.Errorf("%w: nil CredentialsDecorator", ErrMisconfiguredWS) + } + return nil + }) +} + +func validateNowFunc() Option { + return optionFunc( + func(ws *Websocket) error { + if ws.nowFunc == nil { + return fmt.Errorf("%w: nil NowFunc", ErrMisconfiguredWS) + } + return nil + }) +} + +func validRetryPolicy() Option { + return optionFunc( + func(ws *Websocket) error { + if ws.retryPolicyFactory == nil { + return fmt.Errorf("%w: nil RetryPolicy", ErrMisconfiguredWS) + } + return nil + }) +} diff --git a/internal/websocket/options.go b/internal/websocket/options.go index 82ce4c8..82cb830 100644 --- a/internal/websocket/options.go +++ b/internal/websocket/options.go @@ -79,7 +79,7 @@ func CredentialsDecorator(f func(http.Header) error) Option { return optionFunc( func(ws *Websocket) error { if f == nil { - return fmt.Errorf("%w: negative FetchURLTimeout", ErrMisconfiguredWS) + return fmt.Errorf("%w: nil CredentialsDecorator", ErrMisconfiguredWS) } ws.credDecorator = f diff --git a/internal/websocket/ws.go b/internal/websocket/ws.go index e25e27f..f2418e9 100644 --- a/internal/websocket/ws.go +++ b/internal/websocket/ws.go @@ -123,6 +123,10 @@ func New(opts ...Option) (*Websocket, error) { validateDeviceID(), validateURL(), validateIPMode(), + validateFetchURL(), + validateCredentialsDecorator(), + validateNowFunc(), + validRetryPolicy(), ) for _, opt := range opts { diff --git a/internal/websocket/ws_test.go b/internal/websocket/ws_test.go index dd7011e..b1d3024 100644 --- a/internal/websocket/ws_test.go +++ b/internal/websocket/ws_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/assert" mock "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "github.com/xmidt-org/retry" "github.com/xmidt-org/wrp-go/v3" "github.com/xmidt-org/xmidt-agent/internal/websocket/event" ) @@ -53,6 +54,8 @@ func TestNew(t *testing.T) { h.Add("Credentials-Decorator", "some value") return nil }), + NowFunc(time.Now), + RetryPolicy(retry.Config{}), ), check: func(assert *assert.Assertions, c *Websocket) { // URL Related @@ -84,6 +87,11 @@ func TestNew(t *testing.T) { wsDefaults, DeviceID("mac:112233445566"), FetchURL(fetcher), + CredentialsDecorator(func(h http.Header) error { + return nil + }), + NowFunc(time.Now), + RetryPolicy(retry.Config{}), ), check: func(assert *assert.Assertions, c *Websocket) { u, err := c.urlFetcher(context.Background()) @@ -141,6 +149,10 @@ func TestNew(t *testing.T) { NowFunc(func() time.Time { return time.Unix(1234, 0) }), + CredentialsDecorator(func(h http.Header) error { + return nil + }), + RetryPolicy(retry.Config{}), ), check: func(assert *assert.Assertions, c *Websocket) { if assert.NotNil(c.nowFunc) { @@ -196,6 +208,11 @@ func TestMessageListener(t *testing.T) { DeviceID("mac:112233445566"), AddMessageListener(&m), WithIPv6(), + CredentialsDecorator(func(h http.Header) error { + return nil + }), + NowFunc(time.Now), + RetryPolicy(retry.Config{}), ) assert.NoError(err) @@ -219,6 +236,11 @@ func TestConnectListener(t *testing.T) { DeviceID("mac:112233445566"), AddConnectListener(&m), WithIPv6(), + CredentialsDecorator(func(h http.Header) error { + return nil + }), + NowFunc(time.Now), + RetryPolicy(retry.Config{}), ) assert.NoError(err) @@ -242,6 +264,11 @@ func TestDisconnectListener(t *testing.T) { DeviceID("mac:112233445566"), AddDisconnectListener(&m), WithIPv6(), + CredentialsDecorator(func(h http.Header) error { + return nil + }), + NowFunc(time.Now), + RetryPolicy(retry.Config{}), ) assert.NoError(err) @@ -265,6 +292,11 @@ func TestHeartbeatListener(t *testing.T) { DeviceID("mac:112233445566"), AddHeartbeatListener(&m), WithIPv6(), + CredentialsDecorator(func(h http.Header) error { + return nil + }), + NowFunc(time.Now), + RetryPolicy(retry.Config{}), ) assert.NoError(err) @@ -277,6 +309,13 @@ func TestHeartbeatListener(t *testing.T) { } func TestNextMode(t *testing.T) { + defaults := []Option{ + CredentialsDecorator(func(h http.Header) error { + return nil + }), + NowFunc(time.Now), + RetryPolicy(retry.Config{}), + } tests := []struct { description string opts []Option @@ -287,32 +326,32 @@ func TestNextMode(t *testing.T) { description: "IPv4 to IPv6", mode: ipv4, expected: ipv6, - opts: []Option{ + opts: append(defaults, WithIPv6(true), WithIPv4(true), - }, + ), }, { description: "IPv6 to IPv4", mode: ipv6, expected: ipv4, - opts: []Option{ + opts: append(defaults, WithIPv6(true), WithIPv4(true), - }, + ), }, { description: "IPv4 to IPv4", - opts: []Option{ + opts: append(defaults, WithIPv4(true), WithIPv6(false), - }, + ), mode: ipv4, expected: ipv4, }, { description: "IPv6 to IPv6", - opts: []Option{ + opts: append(defaults, WithIPv4(false), WithIPv6(true), - }, + ), mode: ipv6, expected: ipv6, },