diff options
author | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2019-12-02 13:25:50 +0300 |
---|---|---|
committer | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2019-12-02 13:25:50 +0300 |
commit | 1d550ef24d760535d4469e19963e897fa63da43d (patch) | |
tree | b7715c4af37953d6c00d45ccd050665b242e8e78 | |
parent | b535811a888f52fc44925cc7abd3295b0e44903e (diff) |
Improve error handing when creating new domains config source
-rw-r--r-- | app.go | 9 | ||||
-rw-r--r-- | internal/source/domains.go | 19 | ||||
-rw-r--r-- | internal/source/gitlab/client/client.go | 14 | ||||
-rw-r--r-- | internal/source/gitlab/client/client_test.go | 26 | ||||
-rw-r--r-- | internal/source/gitlab/gitlab.go | 9 |
5 files changed, 62 insertions, 15 deletions
@@ -413,9 +413,14 @@ func (a *theApp) listenMetricsFD(wg *sync.WaitGroup, fd uintptr) { } func runApp(config appConfig) { - a := theApp{appConfig: config, domains: source.NewDomains(config)} + domains, err := source.NewDomains(config) + if err != nil { + log.WithError(err).Fatal("could not create domains config source") + } + + a := theApp{appConfig: config, domains: domains} - err := logging.ConfigureLogging(a.LogFormat, a.LogVerbose) + err = logging.ConfigureLogging(a.LogFormat, a.LogVerbose) if err != nil { log.WithError(err).Fatal("Failed to initialize logging") } diff --git a/internal/source/domains.go b/internal/source/domains.go index f47884ef..ed1c3de8 100644 --- a/internal/source/domains.go +++ b/internal/source/domains.go @@ -36,11 +36,20 @@ type Domains struct { // NewDomains is a factory method for domains initializing a mutex. It should // not initialize `dm` as we later check the readiness by comparing it with a // nil value. -func NewDomains(config Config) *Domains { +func NewDomains(config Config) (*Domains, error) { + if len(config.GitlabServerURL()) == 0 { + return &Domains{disk: disk.New()}, nil + } + + gitlab, err := gitlab.New(config) + if err != nil { + return nil, err + } + return &Domains{ + gitlab: gitlab, disk: disk.New(), - gitlab: gitlab.New(config), - } + }, nil } // GetDomain retrieves a domain information from a source. We are using two @@ -69,6 +78,10 @@ func (d *Domains) IsReady() bool { } func (d *Domains) source(domain string) Source { + if d.gitlab == nil { + return d.disk + } + for _, name := range newSourceDomains { if domain == name { return d.gitlab diff --git a/internal/source/gitlab/client/client.go b/internal/source/gitlab/client/client.go index 6c9327dc..a0781251 100644 --- a/internal/source/gitlab/client/client.go +++ b/internal/source/gitlab/client/client.go @@ -9,8 +9,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" ) @@ -35,10 +33,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 { + return nil, errors.New("GitLab API URL 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{ @@ -48,11 +50,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 f3ce4fa6..24ef31db 100644 --- a/internal/source/gitlab/client/client_test.go +++ b/internal/source/gitlab/client/client_test.go @@ -7,6 +7,7 @@ import ( "net/http/httptest" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" jwt "github.com/dgrijalva/jwt-go" @@ -16,6 +17,27 @@ var ( encodedSecret = "e41rcFh7XBA7sNABWVCe2AZvxMsy6QDtJ8S9Ql1UiN8=" // 32 bytes, base64 encoded ) +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()) + + assert.Error(t, err) + assert.Nil(t, client) + }) + + t.Run("when API URL empty", func(t *testing.T) { + client, err := NewClient("", secretKey()) + + assert.Nil(t, client) + assert.EqualError(t, err, "GitLab API URL has not been provided") + }) +} + func TestGetVirtualDomainForErrorResponses(t *testing.T) { tests := map[int]string{ http.StatusNoContent: "No Content", @@ -35,7 +57,7 @@ func TestGetVirtualDomainForErrorResponses(t *testing.T) { server := httptest.NewServer(mux) defer server.Close() - client := NewClient(server.URL, secretKey()) + client, _ := NewClient(server.URL, secretKey()) actual, err := client.GetVirtualDomain("group.gitlab.io") @@ -78,7 +100,7 @@ func TestGetVirtualDomainAuthenticatedRequest(t *testing.T) { server := httptest.NewServer(mux) defer server.Close() - client := NewClient(server.URL, secretKey()) + client, _ := NewClient(server.URL, secretKey()) actual, err := client.GetVirtualDomain("group.gitlab.io") require.NoError(t, err) diff --git a/internal/source/gitlab/gitlab.go b/internal/source/gitlab/gitlab.go index 7cb88e42..a2911e6b 100644 --- a/internal/source/gitlab/gitlab.go +++ b/internal/source/gitlab/gitlab.go @@ -20,8 +20,13 @@ type Gitlab struct { } // New returns a new instance of gitlab domain source. -func New(config client.Config) *Gitlab { - return &Gitlab{client: client.NewFromConfig(config), cache: cache.New()} +func New(config client.Config) (*Gitlab, error) { + client, err := client.NewFromConfig(config) + if err != nil { + return nil, err + } + + return &Gitlab{client: client, cache: cache.New()}, nil } // GetDomain return a representation of a domain that we have fetched from |