Merge branch 'master' into feature/gb/gitlab-domains-source

* master:
  Check presence of GitLab API secret when building a domains source
  Make GitLab API Secret a supported parameter
  Improve error reporting in the main package
  Check if GitLab API secret has been provided too
  Avoid using `testify/assert` in favor of `require`
  Test domains source not fully configured
  Improve error handing when creating new domains config source
  Fix formatting in internal/source/gitlab/client/client_test.go

Conflicts:
	acceptance_test.go
	internal/source/gitlab/client/client_test.go

2 files changed, 43 insertions, 11 deletions

diff --git a/internal/source/gitlab/client/client.go b/internal/source/gitlab/client/client.go
index 382cc412..a230e529 100644
--- a/internal/source/gitlab/client/client.go
+++ b/internal/source/gitlab/client/client.go
@@ -10,8 +10,6 @@ import (
 
 	jwt "github.com/dgrijalva/jwt-go"
 
-	"gitlab.com/gitlab-org/labkit/log"
-
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httptransport"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api"
 )
@@ -36,10 +34,14 @@ var connectionTimeout = 10 * time.Second
 
 // NewClient initializes and returns new Client baseUrl is
 // appConfig.GitLabServer secretKey is appConfig.GitLabAPISecretKey
-func NewClient(baseURL string, secretKey []byte) *Client {
+func NewClient(baseURL string, secretKey []byte) (*Client, error) {
+	if len(baseURL) == 0 || len(secretKey) == 0 {
+		return nil, errors.New("GitLab API URL or API secret has not been provided")
+	}
+
 	url, err := url.Parse(baseURL)
 	if err != nil {
-		log.WithError(err).Fatal("could not parse GitLab server URL")
+		return nil, err
 	}
 
 	return &Client{
@@ -49,11 +51,11 @@ func NewClient(baseURL string, secretKey []byte) *Client {
 			Timeout:   connectionTimeout,
 			Transport: httptransport.Transport,
 		},
-	}
+	}, nil
 }
 
 // NewFromConfig creates a new client from Config struct
-func NewFromConfig(config Config) *Client {
+func NewFromConfig(config Config) (*Client, error) {
 	return NewClient(config.GitlabServerURL(), config.GitlabAPISecret())
 }
 
diff --git a/internal/source/gitlab/client/client_test.go b/internal/source/gitlab/client/client_test.go
index f91f241a..df4d080f 100644
--- a/internal/source/gitlab/client/client_test.go
+++ b/internal/source/gitlab/client/client_test.go
@@ -18,7 +18,35 @@ var (
 	encodedSecret = "e41rcFh7XBA7sNABWVCe2AZvxMsy6QDtJ8S9Ql1UiN8=" // 32 bytes, base64 encoded
 )
 
-func TestGetLookupForErrorResponses(t *testing.T) {
+func TestNewValidBaseURL(t *testing.T) {
+	_, err := NewClient("https://gitlab.com", secretKey())
+	require.NoError(t, err)
+}
+
+func TestNewInvalidBaseURL(t *testing.T) {
+	t.Run("when API URL is not valid", func(t *testing.T) {
+		client, err := NewClient("%", secretKey())
+
+		require.Error(t, err)
+		require.Nil(t, client)
+	})
+
+	t.Run("when API URL is empty", func(t *testing.T) {
+		client, err := NewClient("", secretKey())
+
+		require.Nil(t, client)
+		require.EqualError(t, err, "GitLab API URL or API secret has not been provided")
+	})
+
+	t.Run("when API secret is empty", func(t *testing.T) {
+		client, err := NewClient("https://gitlab.com", []byte{})
+
+		require.Nil(t, client)
+		require.EqualError(t, err, "GitLab API URL or API secret has not been provided")
+	})
+}
+
+func TestLookupForErrorResponses(t *testing.T) {
 	tests := map[int]string{
 		http.StatusNoContent:    "No Content",
 		http.StatusUnauthorized: "Unauthorized",
@@ -37,7 +65,8 @@ func TestGetLookupForErrorResponses(t *testing.T) {
 			server := httptest.NewServer(mux)
 			defer server.Close()
 
-			client := NewClient(server.URL, secretKey())
+			client, err := NewClient(server.URL, secretKey())
+			require.NoError(t, err)
 
 			lookup := client.GetLookup(context.Background(), "group.gitlab.io")
 
@@ -65,8 +94,8 @@ func TestGetVirtualDomainAuthenticatedRequest(t *testing.T) {
 					"access_control": false,
 					"source": {
 						"type": "file",
-							"path": "mygroup/myproject/public/"
-						},
+						"path": "mygroup/myproject/public/"
+					},
 					"https_only": true,
 					"prefix": "/myproject/"
 				}
@@ -80,7 +109,8 @@ func TestGetVirtualDomainAuthenticatedRequest(t *testing.T) {
 	server := httptest.NewServer(mux)
 	defer server.Close()
 
-	client := NewClient(server.URL, secretKey())
+	client, err := NewClient(server.URL, secretKey())
+	require.NoError(t, err)
 
 	lookup := client.GetLookup(context.Background(), "group.gitlab.io")
 	require.NoError(t, lookup.Error)