Merge pull request #175 from launchdarkly/eb/ch87407/tests-1

(#2) more test coverage in core packages

Makefile

@@ -12,6 +12,9 @@ LINTER=./bin/golangci-lint
 
 TEST_COVERAGE_REPORT_FILE ?= coverage.txt
 
+build:
+	go build ./...
+
 test:
 	go test -race -v ./...
 
@@ -48,4 +51,4 @@ test-centos test-debian test-docker test-docker-standalone: release
 
 integration-test: test-centos test-debian test-docker test-docker-standalone
 
-.PHONY: docker lint publish release test test-centos test-debian test-docker test-all test-docker-standalone
+.PHONY: docker build lint publish release test test-centos test-debian test-docker test-all test-docker-standalone

core/application/commandline_test.go

@@ -4,10 +4,10 @@ import (
 	"io/ioutil"
 	"testing"
 
+	helpers "github.com/launchdarkly/go-test-helpers/v2"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-
-	helpers "github.com/launchdarkly/go-test-helpers/v2"
 )
 
 func TestReadOptions(t *testing.T) {
@@ -59,3 +59,8 @@ func TestReadOptions(t *testing.T) {
 		assert.Error(t, err)
 	})
 }
+
+func TestDescribeRelayVersion(t *testing.T) {
+	assert.Equal(t, "1.2.3", DescribeRelayVersion("1.2.3"))
+	assert.Equal(t, "1.2.3 (build 999)", DescribeRelayVersion("1.2.3+999"))
+}

core/application/server.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 
 	"github.com/launchdarkly/ld-relay/v6/core/config"
+
 	"gopkg.in/launchdarkly/go-sdk-common.v2/ldlog"
 )
 
@@ -18,7 +19,7 @@ func StartHTTPServer(
 	tlsCertFile, tlsKeyFile string,
 	tlsMinVersion uint16,
 	loggers ldlog.Loggers,
-) <-chan error {
+) (*http.Server, <-chan error) {
 	srv := &http.Server{
 		Addr:    fmt.Sprintf(":%d", port),
 		Handler: handler,
@@ -50,5 +51,5 @@ func StartHTTPServer(
 		}
 	}()
 
-	return errCh
+	return srv, errCh
 }

core/application/server_test.go

@@ -0,0 +1,133 @@
+package application
+
+import (
+	"crypto/tls"
+	"crypto/x509"
+	"fmt"
+	"io/ioutil"
+	"net"
+	"net/http"
+	"strconv"
+	"strings"
+	"testing"
+	"time"
+
+	helpers "github.com/launchdarkly/go-test-helpers/v2"
+	"github.com/launchdarkly/go-test-helpers/v2/httphelpers"
+	"gopkg.in/launchdarkly/go-sdk-common.v2/ldlog"
+	"gopkg.in/launchdarkly/go-sdk-common.v2/ldlogtest"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func startListenerForAnyAvailablePort(t *testing.T) (net.Listener, int) {
+	l, err := net.Listen("tcp", ":0")
+	require.NoError(t, err)
+	addr := l.Addr().String()
+	port, err := strconv.Atoi(addr[strings.LastIndex(addr, ":")+1:])
+	require.NoError(t, err)
+	return l, port
+}
+
+func withSelfSignedCert(t *testing.T, action func(certFilePath, keyFilePath string, certPool *x509.CertPool)) {
+	helpers.WithTempFile(func(certFilePath string) {
+		helpers.WithTempFile(func(keyFilePath string) {
+			err := httphelpers.MakeSelfSignedCert(certFilePath, keyFilePath)
+			require.NoError(t, err)
+			certData, err := ioutil.ReadFile(certFilePath)
+			require.NoError(t, err)
+			certPool, err := x509.SystemCertPool()
+			if err != nil {
+				certPool = x509.NewCertPool()
+			}
+			certPool.AppendCertsFromPEM(certData)
+
+			action(certFilePath, keyFilePath, certPool)
+		})
+	})
+}
+
+func TestStartHTTPServerInsecure(t *testing.T) {
+	l, port := startListenerForAnyAvailablePort(t)
+	l.Close()
+	mockLog := ldlogtest.NewMockLog()
+	server, errCh := StartHTTPServer(port, httphelpers.HandlerWithStatus(http.StatusOK), false, "", "", 0, mockLog.Loggers)
+	require.NotNil(t, server)
+	require.NotNil(t, errCh)
+	require.Eventually(t, func() bool {
+		resp, err := http.Get(fmt.Sprintf("http://localhost:%d", port))
+		require.NoError(t, err)
+		return resp.StatusCode == http.StatusOK
+	}, time.Second, time.Millisecond*10)
+	mockLog.AssertMessageMatch(t, true, ldlog.Info, fmt.Sprintf("listening on port %d", port))
+	mockLog.AssertMessageMatch(t, false, ldlog.Info, "TLS enabled")
+}
+
+func TestStartHTTPServerSecure(t *testing.T) {
+	l, port := startListenerForAnyAvailablePort(t)
+	l.Close()
+	mockLog := ldlogtest.NewMockLog()
+
+	withSelfSignedCert(t, func(certFilePath, keyFilePath string, certPool *x509.CertPool) {
+		server, errCh := StartHTTPServer(port, httphelpers.HandlerWithStatus(http.StatusOK),
+			true, certFilePath, keyFilePath, 0, mockLog.Loggers)
+		require.NotNil(t, server)
+		require.NotNil(t, errCh)
+
+		client := &http.Client{Transport: &http.Transport{
+			TLSClientConfig: &tls.Config{
+				RootCAs: certPool,
+			},
+		}}
+
+		require.Eventually(t, func() bool {
+			resp, err := client.Get(fmt.Sprintf("https://127.0.0.1:%d", port))
+			require.NoError(t, err)
+			return resp.StatusCode == http.StatusOK
+		}, time.Second, time.Millisecond*10)
+		mockLog.AssertMessageMatch(t, true, ldlog.Info, fmt.Sprintf("listening on port %d", port))
+		mockLog.AssertMessageMatch(t, true, ldlog.Info, "TLS enabled for server")
+	})
+}
+
+func TestStartHTTPServerSecureWithMinTLSVersion(t *testing.T) {
+	l, port := startListenerForAnyAvailablePort(t)
+	l.Close()
+	mockLog := ldlogtest.NewMockLog()
+
+	withSelfSignedCert(t, func(certFilePath, keyFilePath string, certPool *x509.CertPool) {
+		server, errCh := StartHTTPServer(port, httphelpers.HandlerWithStatus(http.StatusOK),
+			true, certFilePath, keyFilePath, tls.VersionTLS12, mockLog.Loggers)
+		require.NotNil(t, server)
+		require.NotNil(t, errCh)
+
+		client := &http.Client{Transport: &http.Transport{
+			TLSClientConfig: &tls.Config{
+				RootCAs:    certPool,
+				MaxVersion: tls.VersionTLS11,
+			},
+		}}
+
+		require.Eventually(t, func() bool {
+			_, err := client.Get(fmt.Sprintf("https://127.0.0.1:%d", port))
+			require.Error(t, err)
+			return strings.Contains(err.Error(), "protocol version not supported")
+		}, time.Second, time.Millisecond*10)
+		mockLog.AssertMessageMatch(t, true, ldlog.Info, fmt.Sprintf("listening on port %d", port))
+		mockLog.AssertMessageMatch(t, true, ldlog.Info, "TLS enabled for server \\(minimum TLS version: 1.2\\)")
+	})
+}
+
+func TestStartHTTPServerPortAlreadyUsed(t *testing.T) {
+	l, port := startListenerForAnyAvailablePort(t)
+	defer l.Close()
+	_, errCh := StartHTTPServer(port, httphelpers.HandlerWithStatus(200), false, "", "", 0, ldlog.NewDisabledLoggers())
+	require.NotNil(t, errCh)
+	select {
+	case err := <-errCh:
+		assert.NotNil(t, err)
+	case <-time.After(time.Second):
+		assert.Fail(t, "timed out waiting for error")
+	}
+}

core/client-side.go

@@ -22,7 +22,7 @@ func getEventsImage(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 	handler := clientCtx.Env.GetEventDispatcher().GetHandler(events.JavaScriptSDKEventsEndpoint)
-	if handler == nil {
+	if handler == nil { // COVERAGE: abnormal condition that can't be caused in unit tests
 		w.WriteHeader(http.StatusServiceUnavailable)
 		_, _ = w.Write(util.ErrorJSONMsg("Event proxy for browser clients is not enabled for this environment"))
 		return

core/httpconfig/httpconfig.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 
 	"github.com/launchdarkly/ld-relay/v6/core/config"
+
 	"gopkg.in/launchdarkly/go-sdk-common.v2/ldlog"
 	"gopkg.in/launchdarkly/go-server-sdk.v5/interfaces"
 	"gopkg.in/launchdarkly/go-server-sdk.v5/ldcomponents"

core/httpconfig/httpconfig_test.go

@@ -1,13 +1,22 @@
 package httpconfig
 
 import (
+	"crypto/x509"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-
 	"github.com/launchdarkly/ld-relay/v6/core/config"
+
+	"github.com/launchdarkly/go-configtypes"
+	helpers "github.com/launchdarkly/go-test-helpers/v2"
+	"github.com/launchdarkly/go-test-helpers/v2/httphelpers"
 	"gopkg.in/launchdarkly/go-sdk-common.v2/ldlog"
+	"gopkg.in/launchdarkly/go-sdk-common.v2/ldlogtest"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestUserAgentHeader(t *testing.T) {
@@ -33,3 +42,98 @@ func TestAuthorizationHeader(t *testing.T) {
 	headers := hc.SDKHTTPConfig.GetDefaultHeaders()
 	assert.Equal(t, "key", headers.Get("Authorization"))
 }
+
+func TestSimpleProxy(t *testing.T) {
+	fakeURL := "http://fake-url/"
+	handler, requestsCh := httphelpers.RecordingHandler(httphelpers.HandlerWithStatus(http.StatusOK))
+	mockLog := ldlogtest.NewMockLog()
+
+	httphelpers.WithServer(handler, func(server *httptest.Server) {
+		proxyConfig := config.ProxyConfig{}
+		proxyConfig.URL, _ = configtypes.NewOptURLAbsoluteFromString(server.URL)
+		hc, err := NewHTTPConfig(proxyConfig, nil, "", mockLog.Loggers)
+
+		mockLog.AssertMessageMatch(t, true, ldlog.Info, "Using proxy server at "+server.URL)
+
+		client := hc.Client()
+		resp, err := client.Get(fakeURL)
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusOK, resp.StatusCode)
+
+		req := <-requestsCh
+		assert.Equal(t, fakeURL, req.Request.URL.String())
+	})
+}
+
+func TestSimpleProxyWithCACert(t *testing.T) {
+	fakeURL := "http://fake-url/"
+	handler, requestsCh := httphelpers.RecordingHandler(httphelpers.HandlerWithStatus(http.StatusOK))
+	mockLog := ldlogtest.NewMockLog()
+
+	httphelpers.WithSelfSignedServer(handler, func(server *httptest.Server, certData []byte, certPool *x509.CertPool) {
+		helpers.WithTempFile(func(certFilePath string) {
+			require.NoError(t, ioutil.WriteFile(certFilePath, certData, 0))
+			proxyConfig := config.ProxyConfig{}
+			proxyConfig.URL, _ = configtypes.NewOptURLAbsoluteFromString(server.URL)
+			proxyConfig.CACertFiles = configtypes.NewOptStringList([]string{certFilePath})
+			hc, err := NewHTTPConfig(proxyConfig, nil, "", mockLog.Loggers)
+
+			mockLog.AssertMessageMatch(t, true, ldlog.Info, "Using proxy server at "+server.URL)
+
+			client := hc.Client()
+			resp, err := client.Get(fakeURL)
+			require.NoError(t, err)
+			assert.Equal(t, http.StatusOK, resp.StatusCode)
+
+			req := <-requestsCh
+			assert.Equal(t, fakeURL, req.Request.URL.String())
+		})
+	})
+}
+
+func TestSimpleProxyCACertError(t *testing.T) {
+	mockLog := ldlogtest.NewMockLog()
+
+	helpers.WithTempFile(func(certFilePath string) {
+		proxyConfig := config.ProxyConfig{}
+		proxyConfig.URL, _ = configtypes.NewOptURLAbsoluteFromString("http://fake-proxy")
+		proxyConfig.CACertFiles = configtypes.NewOptStringList([]string{certFilePath})
+		_, err := NewHTTPConfig(proxyConfig, nil, "", mockLog.Loggers)
+		if assert.Error(t, err) {
+			assert.Contains(t, err.Error(), "invalid CA certificate data")
+		}
+	})
+}
+
+func TestNTLMProxyInvalidConfigs(t *testing.T) {
+	// The actual functioning of the NTLM proxy transport is tested in the SDK package where it is defined,
+	// so here we're only testing that we validate the parameters correctly.
+
+	proxyConfig1 := config.ProxyConfig{NTLMAuth: true}
+	_, err := NewHTTPConfig(proxyConfig1, nil, "", ldlog.NewDisabledLoggers())
+	assert.Equal(t, errProxyAuthWithoutProxyURL, err)
+
+	proxyConfig2 := proxyConfig1
+	proxyConfig2.URL, _ = configtypes.NewOptURLAbsoluteFromString("http://fake-proxy")
+	_, err = NewHTTPConfig(proxyConfig2, nil, "", ldlog.NewDisabledLoggers())
+	assert.Equal(t, errNTLMProxyAuthWithoutCredentials, err)
+
+	proxyConfig3 := proxyConfig2
+	proxyConfig3.User = "user"
+	_, err = NewHTTPConfig(proxyConfig3, nil, "", ldlog.NewDisabledLoggers())
+	assert.Equal(t, errNTLMProxyAuthWithoutCredentials, err)
+
+	proxyConfig4 := proxyConfig3
+	proxyConfig4.Password = "pass"
+	_, err = NewHTTPConfig(proxyConfig4, nil, "", ldlog.NewDisabledLoggers())
+	assert.NoError(t, err)
+
+	proxyConfig5 := proxyConfig4
+	helpers.WithTempFile(func(certFileName string) {
+		proxyConfig5.CACertFiles = configtypes.NewOptStringList([]string{certFileName})
+		_, err = NewHTTPConfig(proxyConfig5, nil, "", ldlog.NewDisabledLoggers())
+		if assert.Error(t, err) {
+			assert.Contains(t, err.Error(), "invalid CA certificate data")
+		}
+	})
+}

core/internal/events/event-relay.go

@@ -12,12 +12,12 @@ import (
 	"sync"
 	"time"
 
-	"gopkg.in/launchdarkly/go-sdk-common.v2/ldlog"
-
-	c "github.com/launchdarkly/ld-relay/v6/core/config"
 	"github.com/launchdarkly/ld-relay/v6/core/httpconfig"
 	"github.com/launchdarkly/ld-relay/v6/core/internal/store"
 	"github.com/launchdarkly/ld-relay/v6/core/internal/util"
+
+	c "github.com/launchdarkly/ld-relay/v6/core/config"
+	"gopkg.in/launchdarkly/go-sdk-common.v2/ldlog"
 )
 
 // Endpoint describes one of the possible endpoints (on both events.launchdarkly.com and Relay) for posting events.

core/internal/events/event-summarizing-relay.go

@@ -8,6 +8,10 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/launchdarkly/ld-relay/v6/core/httpconfig"
+	"github.com/launchdarkly/ld-relay/v6/core/internal/store"
+
+	c "github.com/launchdarkly/ld-relay/v6/core/config"
 	"gopkg.in/launchdarkly/go-sdk-common.v2/ldlog"
 	"gopkg.in/launchdarkly/go-sdk-common.v2/ldreason"
 	"gopkg.in/launchdarkly/go-sdk-common.v2/ldtime"
@@ -16,10 +20,6 @@ import (
 	ldevents "gopkg.in/launchdarkly/go-sdk-events.v1"
 	"gopkg.in/launchdarkly/go-server-sdk-evaluation.v1/ldmodel"
 	"gopkg.in/launchdarkly/go-server-sdk.v5/ldcomponents/ldstoreimpl"
-
-	c "github.com/launchdarkly/ld-relay/v6/core/config"
-	"github.com/launchdarkly/ld-relay/v6/core/httpconfig"
-	"github.com/launchdarkly/ld-relay/v6/core/internal/store"
 )
 
 var (