Merge pull request #73 from kaleido-io/ffresty_struct

feat: Add configuration struct for ffresty

Author: peterbroadhurst

pkg/ffresty/config.go

@@ -17,6 +17,8 @@
 package ffresty
 
 import (
+	"context"
+
 	"github.com/hyperledger/firefly-common/pkg/config"
 	"github.com/hyperledger/firefly-common/pkg/fftls"
 )
@@ -95,3 +97,34 @@ func InitConfig(conf config.Section) {
 	tlsConfig := conf.SubSection("tls")
 	fftls.InitTLSConfig(tlsConfig)
 }
+
+func GenerateConfig(ctx context.Context, conf config.Section) (*Config, error) {
+	ffrestyConfig := &Config{
+		URL:                           conf.GetString(HTTPConfigURL),
+		ProxyURL:                      conf.GetString(HTTPConfigProxyURL),
+		HTTPHeaders:                   conf.GetObject(HTTPConfigHeaders),
+		AuthUsername:                  conf.GetString(HTTPConfigAuthUsername),
+		AuthPassword:                  conf.GetString(HTTPConfigAuthPassword),
+		Retry:                         conf.GetBool(HTTPConfigRetryEnabled),
+		RetryCount:                    conf.GetInt(HTTPConfigRetryCount),
+		RetryInitialDelay:             conf.GetDuration(HTTPConfigRetryInitDelay),
+		RetryMaximumDelay:             conf.GetDuration(HTTPConfigRetryMaxDelay),
+		HTTPRequestTimeout:            conf.GetDuration(HTTPConfigRequestTimeout),
+		HTTPIdleConnTimeout:           conf.GetDuration(HTTPIdleTimeout),
+		HTTPMaxIdleConns:              conf.GetInt(HTTPMaxIdleConns),
+		HTTPConnectionTimeout:         conf.GetDuration(HTTPConnectionTimeout),
+		HTTPTLSHandshakeTimeout:       conf.GetDuration(HTTPTLSHandshakeTimeout),
+		HTTPExpectContinueTimeout:     conf.GetDuration(HTTPExpectContinueTimeout),
+		HTTPPassthroughHeadersEnabled: conf.GetBool(HTTPPassthroughHeadersEnabled),
+		HTTPCustomClient:              conf.Get(HTTPCustomClient),
+	}
+	tlsSection := conf.SubSection("tls")
+	tlsClientConfig, err := fftls.ConstructTLSConfig(ctx, tlsSection, fftls.ClientType)
+	if err != nil {
+		return nil, err
+	}
+
+	ffrestyConfig.TLSClientConfig = tlsClientConfig
+
+	return ffrestyConfig, nil
+}

pkg/ffresty/config_test.go

@@ -0,0 +1,81 @@
+// Copyright © 2023 Kaleido, Inc.
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package ffresty
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/hyperledger/firefly-common/pkg/fftls"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestWSConfigGeneration(t *testing.T) {
+	resetConf()
+
+	utConf.Set(HTTPConfigURL, "http://test:12345")
+	utConf.Set(HTTPConfigProxyURL, "http://proxy:12345")
+	utConf.Set(HTTPConfigHeaders, map[string]interface{}{
+		"custom-header": "custom value",
+	})
+	utConf.Set(HTTPConfigAuthUsername, "user")
+	utConf.Set(HTTPConfigAuthPassword, "pass")
+	utConf.Set(HTTPConfigRetryEnabled, true)
+	utConf.Set(HTTPConfigRetryInitDelay, 1)
+	utConf.Set(HTTPConfigRetryMaxDelay, 1)
+	utConf.Set(HTTPConfigRetryCount, 1)
+	utConf.Set(HTTPConfigRequestTimeout, 1)
+	utConf.Set(HTTPIdleTimeout, 1)
+	utConf.Set(HTTPMaxIdleConns, 1)
+	utConf.Set(HTTPConnectionTimeout, 1)
+	utConf.Set(HTTPTLSHandshakeTimeout, 1)
+	utConf.Set(HTTPExpectContinueTimeout, 1)
+	utConf.Set(HTTPPassthroughHeadersEnabled, true)
+
+	ctx := context.Background()
+	config, err := GenerateConfig(ctx, utConf)
+	assert.NoError(t, err)
+
+	assert.Equal(t, "http://test:12345", config.URL)
+	assert.Equal(t, "http://proxy:12345", config.ProxyURL)
+	assert.Equal(t, "user", config.AuthUsername)
+	assert.Equal(t, "pass", config.AuthPassword)
+	assert.Equal(t, time.Duration(1000000), config.RetryInitialDelay)
+	assert.Equal(t, time.Duration(1000000), config.RetryMaximumDelay)
+	assert.Equal(t, 1, config.RetryCount)
+	assert.Equal(t, true, config.Retry)
+	assert.Equal(t, true, config.HTTPPassthroughHeadersEnabled)
+	assert.Equal(t, time.Duration(1000000), config.HTTPExpectContinueTimeout)
+	assert.Equal(t, time.Duration(1000000), config.HTTPIdleConnTimeout)
+	assert.Equal(t, time.Duration(1000000), config.HTTPTLSHandshakeTimeout)
+	assert.Equal(t, time.Duration(1000000), config.HTTPConnectionTimeout)
+	assert.Equal(t, 1, config.HTTPMaxIdleConns)
+	assert.Equal(t, "custom value", config.HTTPHeaders.GetString("custom-header"))
+}
+
+func TestWSConfigTLSGenerationFail(t *testing.T) {
+	resetConf()
+
+	tlsSection := utConf.SubSection("tls")
+	tlsSection.Set(fftls.HTTPConfTLSEnabled, true)
+	tlsSection.Set(fftls.HTTPConfTLSCAFile, "bad-ca")
+
+	ctx := context.Background()
+	_, err := GenerateConfig(ctx, utConf)
+	assert.Regexp(t, "FF00153", err)
+}

pkg/ffresty/ffresty.go

@@ -18,6 +18,7 @@ package ffresty
 
 import (
 	"context"
+	"crypto/tls"
 	"encoding/base64"
 	"fmt"
 	"io"
@@ -29,7 +30,6 @@ import (
 	"github.com/go-resty/resty/v2"
 	"github.com/hyperledger/firefly-common/pkg/config"
 	"github.com/hyperledger/firefly-common/pkg/ffapi"
-	"github.com/hyperledger/firefly-common/pkg/fftls"
 	"github.com/hyperledger/firefly-common/pkg/fftypes"
 	"github.com/hyperledger/firefly-common/pkg/i18n"
 	"github.com/hyperledger/firefly-common/pkg/log"
@@ -44,6 +44,28 @@ type retryCtx struct {
 	attempts uint
 }
 
+type Config struct {
+	URL                           string             `json:"httpURL,omitempty"`
+	ProxyURL                      string             `json:"proxyURL,omitempty"`
+	HTTPRequestTimeout            time.Duration      `json:"requestTimeout,omitempty"`
+	HTTPIdleConnTimeout           time.Duration      `json:"idleTimeout,omitempty"`
+	HTTPMaxIdleTimeout            time.Duration      `json:"maxIdleTimeout,omitempty"`
+	HTTPConnectionTimeout         time.Duration      `json:"connectionTimeout,omitempty"`
+	HTTPExpectContinueTimeout     time.Duration      `json:"expectContinueTimeout,omitempty"`
+	AuthUsername                  string             `json:"authUsername,omitempty"`
+	AuthPassword                  string             `json:"authPassword,omitempty"`
+	Retry                         bool               `json:"retry,omitempty"`
+	RetryCount                    int                `json:"retryCount,omitempty"`
+	RetryInitialDelay             time.Duration      `json:"retryInitialDelay,omitempty"`
+	RetryMaximumDelay             time.Duration      `json:"retryMaximumDelay,omitempty"`
+	HTTPMaxIdleConns              int                `json:"maxIdleConns,omitempty"`
+	HTTPPassthroughHeadersEnabled bool               `json:"httpPassthroughHeadersEnabled,omitempty"`
+	HTTPHeaders                   fftypes.JSONObject `json:"headers,omitempty"`
+	TLSClientConfig               *tls.Config        `json:"tlsClientConfig,omitempty"`
+	HTTPTLSHandshakeTimeout       time.Duration      `json:"tlsHandshakeTimeout,omitempty"`
+	HTTPCustomClient              interface{}        `json:"httpCustomClient,omitempty"`
+}
+
 // OnAfterResponse when using SetDoNotParseResponse(true) for streaming binary replies,
 // the caller should invoke ffresty.OnAfterResponse on the response manually.
 // The middleware is disabled on this path :-(
@@ -69,54 +91,62 @@ func OnAfterResponse(c *resty.Client, resp *resty.Response) {
 // You can use the normal Resty builder pattern, to set per-instance configuration
 // as required.
 func New(ctx context.Context, staticConfig config.Section) (client *resty.Client, err error) {
-	passthroughHeadersEnabled := staticConfig.GetBool(HTTPPassthroughHeadersEnabled)
+	ffrestyConfig, err := GenerateConfig(ctx, staticConfig)
+	if err != nil {
+		return nil, err
+	}
 
-	iHTTPClient := staticConfig.Get(HTTPCustomClient)
-	if iHTTPClient != nil {
-		if httpClient, ok := iHTTPClient.(*http.Client); ok {
+	return NewWithConfig(ctx, *ffrestyConfig), nil
+}
+
+// New creates a new Resty client, using static configuration (from the config file)
+// from a given section in the static configuration
+//
+// You can use the normal Resty builder pattern, to set per-instance configuration
+// as required.
+func NewWithConfig(ctx context.Context, ffrestyConfig Config) (client *resty.Client) {
+	if ffrestyConfig.HTTPCustomClient != nil {
+		if httpClient, ok := ffrestyConfig.HTTPCustomClient.(*http.Client); ok {
 			client = resty.NewWithClient(httpClient)
 		}
 	}
+
 	if client == nil {
 
 		httpTransport := &http.Transport{
 			Proxy: http.ProxyFromEnvironment,
 			DialContext: (&net.Dialer{
-				Timeout:   staticConfig.GetDuration(HTTPConnectionTimeout),
-				KeepAlive: staticConfig.GetDuration(HTTPConnectionTimeout),
+				Timeout:   ffrestyConfig.HTTPConnectionTimeout,
+				KeepAlive: ffrestyConfig.HTTPConnectionTimeout,
 			}).DialContext,
 			ForceAttemptHTTP2:     true,
-			MaxIdleConns:          staticConfig.GetInt(HTTPMaxIdleConns),
-			IdleConnTimeout:       staticConfig.GetDuration(HTTPIdleTimeout),
-			TLSHandshakeTimeout:   staticConfig.GetDuration(HTTPTLSHandshakeTimeout),
-			ExpectContinueTimeout: staticConfig.GetDuration(HTTPExpectContinueTimeout),
+			MaxIdleConns:          ffrestyConfig.HTTPMaxIdleConns,
+			IdleConnTimeout:       ffrestyConfig.HTTPIdleConnTimeout,
+			TLSHandshakeTimeout:   ffrestyConfig.HTTPTLSHandshakeTimeout,
+			ExpectContinueTimeout: ffrestyConfig.HTTPExpectContinueTimeout,
 		}
 
-		tlsConfig, err := fftls.ConstructTLSConfig(ctx, staticConfig.SubSection("tls"), "client")
-		if err != nil {
-			return nil, err
+		if ffrestyConfig.TLSClientConfig != nil {
+			httpTransport.TLSClientConfig = ffrestyConfig.TLSClientConfig
 		}
 
-		httpTransport.TLSClientConfig = tlsConfig
-
 		httpClient := &http.Client{
 			Transport: httpTransport,
 		}
 		client = resty.NewWithClient(httpClient)
 	}
 
-	url := strings.TrimSuffix(staticConfig.GetString(HTTPConfigURL), "/")
+	url := strings.TrimSuffix(ffrestyConfig.URL, "/")
 	if url != "" {
 		client.SetBaseURL(url)
 		log.L(ctx).Debugf("Created REST client to %s", url)
 	}
 
-	proxy := staticConfig.GetString(HTTPConfigProxyURL)
-	if proxy != "" {
-		client.SetProxy(proxy)
+	if ffrestyConfig.ProxyURL != "" {
+		client.SetProxy(ffrestyConfig.ProxyURL)
 	}
 
-	client.SetTimeout(staticConfig.GetDuration(HTTPConfigRequestTimeout))
+	client.SetTimeout(ffrestyConfig.HTTPRequestTimeout)
 
 	client.OnBeforeRequest(func(c *resty.Client, req *resty.Request) error {
 		rCtx := req.Context()
@@ -135,7 +165,7 @@ func New(ctx context.Context, staticConfig config.Section) (client *resty.Client
 		}
 
 		// If passthroughHeaders: true for this rest client, pass any of the allowed headers on the original req
-		if passthroughHeadersEnabled {
+		if ffrestyConfig.HTTPPassthroughHeadersEnabled {
 			ctxHeaders := rCtx.Value(ffapi.CtxHeadersKey{})
 			if ctxHeaders != nil {
 				passthroughHeaders := ctxHeaders.(http.Header)
@@ -159,22 +189,19 @@ func New(ctx context.Context, staticConfig config.Section) (client *resty.Client
 
 	client.OnAfterResponse(func(c *resty.Client, r *resty.Response) error { OnAfterResponse(c, r); return nil })
 
-	headers := staticConfig.GetObject(HTTPConfigHeaders)
-	for k, v := range headers {
+	for k, v := range ffrestyConfig.HTTPHeaders {
 		if vs, ok := v.(string); ok {
 			client.SetHeader(k, vs)
 		}
 	}
-	authUsername := staticConfig.GetString((HTTPConfigAuthUsername))
-	authPassword := staticConfig.GetString((HTTPConfigAuthPassword))
-	if authUsername != "" && authPassword != "" {
-		client.SetHeader("Authorization", fmt.Sprintf("Basic %s", base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", authUsername, authPassword)))))
+	if ffrestyConfig.AuthUsername != "" && ffrestyConfig.AuthPassword != "" {
+		client.SetHeader("Authorization", fmt.Sprintf("Basic %s", base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", ffrestyConfig.AuthUsername, ffrestyConfig.AuthPassword)))))
 	}
 
-	if staticConfig.GetBool(HTTPConfigRetryEnabled) {
-		retryCount := staticConfig.GetInt(HTTPConfigRetryCount)
-		minTimeout := staticConfig.GetDuration(HTTPConfigRetryInitDelay)
-		maxTimeout := staticConfig.GetDuration(HTTPConfigRetryMaxDelay)
+	if ffrestyConfig.Retry {
+		retryCount := ffrestyConfig.RetryCount
+		minTimeout := ffrestyConfig.RetryInitialDelay
+		maxTimeout := ffrestyConfig.RetryMaximumDelay
 		client.
 			SetRetryCount(retryCount).
 			SetRetryWaitTime(minTimeout).
@@ -191,7 +218,7 @@ func New(ctx context.Context, staticConfig config.Section) (client *resty.Client
 			})
 	}
 
-	return client, nil
+	return client
 }
 
 func WrapRestErr(ctx context.Context, res *resty.Response, err error, key i18n.ErrorMessageKey) error {

pkg/wsclient/wsclient_test.go

@@ -56,6 +56,7 @@ func TestWSClientE2ETLS(t *testing.T) {
 		}
 		return nil
 	}
+
 	afterConnect := func(ctx context.Context, w WSClient) error {
 		return w.Send(ctx, []byte(`after connect message`))
 	}
@@ -94,6 +95,7 @@ func TestWSClientE2ETLS(t *testing.T) {
 	wsc2.SetURL(wsc2.URL() + "/updated")
 	err = wsc2.Connect()
 	assert.Error(t, err)
+	wsc2.Close()
 
 	// Receive the message automatically sent in afterConnect
 	message1 := <-toServer

pkg/wsclient/wstestserver.go

@@ -29,6 +29,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"sync"
 	"testing"
 	"time"
 
@@ -77,14 +78,17 @@ func NewTestTLSWSServer(testReq func(req *http.Request), publicKeyFile *os.File,
 	sendDone := make(chan struct{})
 	receiveDone := make(chan struct{})
 	connected := false
+	mu := sync.Mutex{}
 
 	handlerFunc := http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
+		mu.Lock()
 		if testReq != nil {
 			testReq(req)
 		}
 		if connected {
 			// test server only handles one open connection, as it only has one set of channels
 			res.WriteHeader(409)
+			mu.Unlock()
 			return
 		}
 		ws, _ := upgrader.Upgrade(res, req, http.Header{})
@@ -106,6 +110,7 @@ func NewTestTLSWSServer(testReq func(req *http.Request), publicKeyFile *os.File,
 			}
 		}()
 		connected = true
+		mu.Unlock()
 	})
 
 	svr := httptest.NewUnstartedServer(handlerFunc)