diff options
-rw-r--r-- | acceptance_test.go | 3 | ||||
-rw-r--r-- | app.go | 13 | ||||
-rw-r--r-- | daemon.go | 2 | ||||
-rw-r--r-- | helpers.go | 8 | ||||
-rw-r--r-- | internal/source/domains.go | 19 | ||||
-rw-r--r-- | internal/source/domains_test.go | 50 | ||||
-rw-r--r-- | internal/source/gitlab/client/client.go | 14 | ||||
-rw-r--r-- | internal/source/gitlab/client/client_test.go | 34 | ||||
-rw-r--r-- | internal/source/gitlab/gitlab.go | 9 | ||||
-rw-r--r-- | main.go | 12 |
10 files changed, 128 insertions, 36 deletions
diff --git a/acceptance_test.go b/acceptance_test.go index 6f2e5fb4..dfa334b9 100644 --- a/acceptance_test.go +++ b/acceptance_test.go @@ -1536,7 +1536,8 @@ func TestGitlabDomainsSource(t *testing.T) { defer source.Close() newSourceDomains := "GITLAB_NEW_SOURCE_DOMAINS=new-source-test.gitlab.io,other-test.gitlab.io" - teardown := RunPagesProcessWithEnvs(t, true, *pagesBinary, listeners, "", []string{newSourceDomains}, "-gitlab-server", source.URL) + pagesArgs := []string{"-gitlab-server", source.URL, "-api-secret-key", "README.md"} + teardown := RunPagesProcessWithEnvs(t, true, *pagesBinary, listeners, "", []string{newSourceDomains}, pagesArgs...) defer teardown() response, err := GetPageFromListener(t, httpListener, "new-source-test.gitlab.io", "/my/pages/project/") @@ -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") } @@ -451,6 +456,6 @@ func runApp(config appConfig) { } // fatal will log a fatal error and exit. -func fatal(err error) { - log.WithError(err).Fatal() +func fatal(err error, message string) { + log.WithError(err).Fatal(message) } @@ -34,7 +34,7 @@ func daemonMain() { // read the configuration from the pipe "ExtraFiles" var config appConfig if err := json.NewDecoder(os.NewFile(3, "options")).Decode(&config); err != nil { - fatal(err) + fatal(err, "could not decode app config") } runApp(config) os.Exit(0) @@ -11,7 +11,7 @@ import ( func readFile(file string) (result []byte) { result, err := ioutil.ReadFile(file) if err != nil { - fatal(err) + fatal(err, "could not read file") } return } @@ -21,7 +21,7 @@ func readFile(file string) (result []byte) { func createSocket(addr string) (net.Listener, *os.File) { l, err := net.Listen("tcp", addr) if err != nil { - fatal(err) + fatal(err, "could not create socket") } return l, fileForListener(l) @@ -34,7 +34,7 @@ func fileForListener(l net.Listener) *os.File { f, err := l.(filer).File() if err != nil { - fatal(err) + fatal(err, "could not find file for listener") } return f @@ -42,5 +42,5 @@ func fileForListener(l net.Listener) *os.File { func capturingFatal(err error, fields ...errortracking.CaptureOption) { errortracking.Capture(err, fields...) - fatal(err) + fatal(err, "capturing fatal") } diff --git a/internal/source/domains.go b/internal/source/domains.go index f47884ef..85d06152 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 || len(config.GitlabAPISecret()) == 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/domains_test.go b/internal/source/domains_test.go index 6854c359..378f8c89 100644 --- a/internal/source/domains_test.go +++ b/internal/source/domains_test.go @@ -3,14 +3,44 @@ package source import ( "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitlab-pages/internal/domain" "gitlab.com/gitlab-org/gitlab-pages/internal/source/disk" ) -func TestHasDomain(t *testing.T) { +type sourceConfig struct { + api string + secret string +} + +func (c sourceConfig) GitlabServerURL() string { + return c.api +} + +func (c sourceConfig) GitlabAPISecret() []byte { + return []byte(c.secret) +} + +func TestDomainSources(t *testing.T) { + t.Run("when GitLab API URL has been provided", func(t *testing.T) { + domains, err := NewDomains(sourceConfig{api: "https://gitlab.com", secret: "abc"}) + require.NoError(t, err) + + require.NotNil(t, domains.gitlab) + require.NotNil(t, domains.disk) + }) + + t.Run("when GitLab API has not been provided", func(t *testing.T) { + domains, err := NewDomains(sourceConfig{}) + require.NoError(t, err) + + require.Nil(t, domains.gitlab) + require.NotNil(t, domains.disk) + }) +} + +func TestGetDomain(t *testing.T) { newSourceDomains = []string{"new-source-test.gitlab.io"} brokenSourceDomain = "pages-broken-poc.gitlab.io" @@ -43,7 +73,7 @@ func TestHasDomain(t *testing.T) { domain, err := domains.GetDomain("domain.test.io") require.NoError(t, err) - assert.Nil(t, domain) + require.Nil(t, domain) }) t.Run("when requesting a broken test domain", func(t *testing.T) { @@ -57,7 +87,17 @@ func TestHasDomain(t *testing.T) { domain, err := domains.GetDomain("pages-broken-poc.gitlab.io") - assert.Nil(t, domain) - assert.EqualError(t, err, "broken test domain used") + require.Nil(t, domain) + require.EqualError(t, err, "broken test domain used") + }) + + t.Run("when requesting a test domain in case of the source not being fully configured", func(t *testing.T) { + domains, err := NewDomains(sourceConfig{}) + require.NoError(t, err) + + domain, err := domains.GetDomain("new-source-test.gitlab.io") + + require.Nil(t, domain) + require.NoError(t, err) }) } diff --git a/internal/source/gitlab/client/client.go b/internal/source/gitlab/client/client.go index 6c9327dc..1c3024b7 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 || 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{ @@ -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..d056f2bd 100644 --- a/internal/source/gitlab/client/client_test.go +++ b/internal/source/gitlab/client/client_test.go @@ -16,6 +16,34 @@ 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()) + + 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 TestGetVirtualDomainForErrorResponses(t *testing.T) { tests := map[int]string{ http.StatusNoContent: "No Content", @@ -35,7 +63,8 @@ func TestGetVirtualDomainForErrorResponses(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) actual, err := client.GetVirtualDomain("group.gitlab.io") @@ -78,7 +107,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) 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 @@ -57,7 +57,7 @@ var ( secret = flag.String("auth-secret", "", "Cookie store hash key, should be at least 32 bytes long.") gitLabAuthServer = flag.String("auth-server", "", "DEPRECATED, use gitlab-server instead. GitLab server, for example https://www.gitlab.com") gitLabServer = flag.String("gitlab-server", "", "GitLab server, for example https://www.gitlab.com") - gitLabAPISecretKey = flag.String("api-secret-key", "", "File with secret key used to authenticate with the GitLab API (NOT YET IMPLEMENTED)") + gitLabAPISecretKey = flag.String("api-secret-key", "", "File with secret key used to authenticate with the GitLab API") clientID = flag.String("auth-client-id", "", "GitLab application Client ID") clientSecret = flag.String("auth-client-secret", "", "GitLab application Client Secret") redirectURI = flag.String("auth-redirect-uri", "", "GitLab application redirect URI") @@ -243,10 +243,6 @@ func loadConfig() appConfig { "auth-redirect-uri": config.RedirectURI, }).Debug("Start daemon with configuration") - if *gitLabAPISecretKey != "" { - log.Warn("api-secret-key parameter is a placeholder for future developments, this option will be ignored.") - } - return config } @@ -256,7 +252,7 @@ func appMain() { flag.String(flag.DefaultConfigFlagname, "", "path to config file") flag.Parse() if err := tlsconfig.ValidateTLSVersions(*tlsMinVersion, *tlsMaxVersion); err != nil { - fatal(err) + fatal(err, "invalid TLS version") } printVersion(*showVersion, VERSION) @@ -273,7 +269,7 @@ func appMain() { log.Printf("URL: https://gitlab.com/gitlab-org/gitlab-pages") if err := os.Chdir(*pagesRoot); err != nil { - fatal(err) + fatal(err, "could not change directory into pagesRoot") } config := loadConfig() @@ -288,7 +284,7 @@ func appMain() { if *daemonUID != 0 || *daemonGID != 0 { if err := daemonize(config, *daemonUID, *daemonGID, *daemonInplaceChroot); err != nil { errortracking.Capture(err) - fatal(err) + fatal(err, "could not create pages daemon") } return |