diff --git a/app.go b/app.go index 5a1953962666155b508cbb66a9e0b61d27b501bb..9871328c957b633321bedd4f4dbf8475da2a4070 100644 --- a/app.go +++ b/app.go @@ -41,7 +41,7 @@ var ( type theApp struct { appConfig - domains *source.Domains + domains source.Source Artifact *artifact.Artifact Auth *auth.Auth Handlers *handlers.Handlers diff --git a/internal/source/domains.go b/internal/source/domains.go index e4a582ea9378a9e13b489d390b29c7d6f1b2902b..40ac078b7e84ead0043d4b56dd5b33a7bca79787 100644 --- a/internal/source/domains.go +++ b/internal/source/domains.go @@ -1,6 +1,7 @@ package source import ( + "errors" "fmt" "regexp" @@ -18,128 +19,117 @@ var ( serverlessDomainRegex = regexp.MustCompile(`^[^.]+-[[:xdigit:]]{2}a1[[:xdigit:]]{10}f2[[:xdigit:]]{2}[[:xdigit:]]+-?.*`) ) -type configSource int +// 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) (Source, error) { + switch config.DomainConfigSource() { + case "gitlab": + return gitlab.New(config) + case "auto": + return newAutoSource(config), nil + case "disk": + return newDiskServerlessSource(config), nil + } -const ( - sourceGitlab configSource = iota - sourceDisk - sourceAuto -) + return nil, fmt.Errorf("invalid option for -domain-config-source: %q", config.DomainConfigSource()) +} + +type notAvailableSource struct{} -// Domains struct represents a map of all domains supported by pages. It is -// currently using two sources during the transition to the new GitLab domains -// source. -type Domains struct { - configSource configSource - gitlab Source - disk *disk.Disk // legacy disk source +func (s notAvailableSource) GetDomain(domain string) (*domain.Domain, error) { + return nil, errors.New("Source is not available") } -// 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, error) { - domains := &Domains{ - // TODO: disable domains.disk https://gitlab.com/gitlab-org/gitlab-pages/-/issues/382 - disk: disk.New(), +func (s notAvailableSource) IsReady() bool { + return false +} + +func (s notAvailableSource) Read(string) { +} + +type autoSource struct { + gitlab Source + disk Source +} + +func (s *autoSource) source(domain string) Source { + if IsServerlessDomain(domain) { + return s.gitlab } - if err := domains.setConfigSource(config); err != nil { - return nil, err + if s.gitlab.IsReady() { + return s.gitlab } - return domains, nil + return s.disk } -// setConfigSource and initialize gitlab source -// returns error if -domain-config-source is not valid -// returns error if -domain-config-source=gitlab and init fails -func (d *Domains) setConfigSource(config Config) error { - // TODO: Handle domain-config-source=auto https://gitlab.com/gitlab-org/gitlab/-/issues/218358 - // attach gitlab by default when source is not disk (auto, gitlab) - switch config.DomainConfigSource() { - case "gitlab": - // TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/218357 - d.configSource = sourceGitlab - return d.setGitLabClient(config) - case "auto": - // TODO: handle DomainConfigSource == "auto" https://gitlab.com/gitlab-org/gitlab/-/issues/218358 - d.configSource = sourceAuto - return d.setGitLabClient(config) - case "disk": - d.configSource = sourceDisk - default: - return fmt.Errorf("invalid option for -domain-config-source: %q", config.DomainConfigSource()) - } +func (s *autoSource) GetDomain(domain string) (*domain.Domain, error) { + return s.source(domain).GetDomain(domain) +} - return nil +func (s *autoSource) IsReady() bool { + return s.gitlab.IsReady() || s.disk.IsReady() } -// setGitLabClient when domain-config-source is `gitlab` or `auto`, only return error for `gitlab` source -func (d *Domains) setGitLabClient(config Config) error { - // We want to notify users about any API issues - // Creating a glClient will start polling connectivity in the background - // and spam errors in log +func (s *autoSource) Read(rootDomain string) { + s.disk.Read(rootDomain) +} + +func newAutoSource(config Config) *autoSource { + source := &autoSource{disk: disk.New()} + glClient, err := gitlab.New(config) if err != nil { - if d.configSource == sourceGitlab { - return err - } - log.WithError(err).Warn("failed to initialize GitLab client for `-domain-config-source=auto`") - return nil + source.gitlab = notAvailableSource{} + } else { + source.gitlab = glClient } - d.gitlab = glClient + return source +} - return nil +type diskServerlessSource struct { + serverless Source + disk Source } -// GetDomain retrieves a domain information from a source. We are using two -// sources here because it allows us to switch behavior and the domain source -// for some subset of domains, to test / PoC the new GitLab Domains Source that -// we plan to use to replace the disk source. -func (d *Domains) GetDomain(name string) (*domain.Domain, error) { - return d.source(name).GetDomain(name) +func (s *diskServerlessSource) source(domain string) Source { + if IsServerlessDomain(domain) { + return s.serverless + } + + return s.disk } -// Read starts the disk domain source. It is DEPRECATED, because we want to -// remove it entirely when disk source gets removed. -func (d *Domains) Read(rootDomain string) { - d.disk.Read(rootDomain) +func (s *diskServerlessSource) GetDomain(domain string) (*domain.Domain, error) { + return s.source(domain).GetDomain(domain) } -// IsReady checks if the disk domain source managed to traverse entire pages -// filesystem and is ready for use. It is DEPRECATED, because we want to remove -// it entirely when disk source gets removed. -func (d *Domains) IsReady() bool { - return d.disk.IsReady() +func (s *diskServerlessSource) IsReady() bool { + return s.disk.IsReady() } -func (d *Domains) source(domain string) Source { - if d.gitlab == nil { - return d.disk - } +func (s *diskServerlessSource) Read(rootDomain string) { + s.disk.Read(rootDomain) +} - // This check is only needed until we enable `d.gitlab` source in all - // environments (including on-premises installations) followed by removal of - // `d.disk` source. This can be safely removed afterwards. - if IsServerlessDomain(domain) { - return d.gitlab - } +func newDiskServerlessSource(config Config) *diskServerlessSource { + source := &diskServerlessSource{disk: disk.New()} - if d.configSource == sourceDisk { - return d.disk - } + glClient, err := gitlab.New(config) + if err != nil { + log.WithError(err).Warn("failed to initialize GitLab client for serverless domains") - // TODO: handle sourceAuto https://gitlab.com/gitlab-org/gitlab/-/issues/218358 - // check IsReady for sourceAuto for now - if d.configSource == sourceGitlab || d.gitlab.IsReady() { - return d.gitlab + source.serverless = notAvailableSource{} + } else { + source.serverless = glClient } - return d.disk + return source } // IsServerlessDomain checks if a domain requested is a serverless domain we diff --git a/internal/source/domains_test.go b/internal/source/domains_test.go index 173fc1d3df459d064d4aa6980ae328fb8ac5f8ff..5c4559d2f1f168f7a1bfd904405e0fa0c1e034c1 100644 --- a/internal/source/domains_test.go +++ b/internal/source/domains_test.go @@ -8,6 +8,7 @@ import ( "gitlab.com/gitlab-org/gitlab-pages/internal/domain" "gitlab.com/gitlab-org/gitlab-pages/internal/source/disk" + "gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab" ) type sourceConfig struct { @@ -37,10 +38,10 @@ func (c sourceConfig) DomainConfigSource() string { func TestNewDomains(t *testing.T) { tests := []struct { - name string - sourceConfig sourceConfig - expectedErr string - expectGitlabNil bool + name string + sourceConfig sourceConfig + expectedErr string + expectedType interface{} }{ { name: "no_source_config", @@ -53,23 +54,24 @@ func TestNewDomains(t *testing.T) { expectedErr: "invalid option for -domain-config-source: \"invalid\"", }, { - name: "disk_source", - sourceConfig: sourceConfig{domainSource: "disk"}, - expectGitlabNil: true, + name: "disk_source", + sourceConfig: sourceConfig{domainSource: "disk"}, + expectedType: &diskServerlessSource{}, }, { - name: "auto_without_api_config", - sourceConfig: sourceConfig{domainSource: "auto"}, - expectGitlabNil: true, + name: "auto_without_api_config", + sourceConfig: sourceConfig{domainSource: "auto"}, + expectedType: &autoSource{}, }, { - name: "auto_with_api_config", - sourceConfig: sourceConfig{api: "https://gitlab.com", secret: "abc", domainSource: "auto"}, - expectGitlabNil: false, + name: "auto_with_api_config", + sourceConfig: sourceConfig{api: "https://gitlab.com", secret: "abc", domainSource: "auto"}, + expectedType: &autoSource{}, }, { name: "gitlab_source_success", sourceConfig: sourceConfig{api: "https://gitlab.com", secret: "abc", domainSource: "gitlab"}, + expectedType: &gitlab.Gitlab{}, }, { name: "gitlab_source_no_url", @@ -91,78 +93,260 @@ func TestNewDomains(t *testing.T) { return } require.NoError(t, err) - - require.Equal(t, tt.expectGitlabNil, domains.gitlab == nil) - require.NotNil(t, domains.disk) + require.IsType(t, tt.expectedType, domains) }) } } -func TestGetDomain(t *testing.T) { - t.Run("when requesting an existing domain for gitlab source", func(t *testing.T) { - testDomain := "new-source-test.gitlab.io" +func TestNotAvailableSource(t *testing.T) { + source := ¬AvailableSource{} + + domain, err := source.GetDomain("example.com") + require.Nil(t, domain) + require.Error(t, err) + + require.False(t, source.IsReady()) +} + +func TestNewAutoSource(t *testing.T) { + t.Run("with valid API config sets up both gitlab and disk source", func(t *testing.T) { + source := newAutoSource(sourceConfig{api: "https://gitlab.com", secret: "abc"}) + + require.IsType(t, &gitlab.Gitlab{}, source.gitlab) + require.IsType(t, &disk.Disk{}, source.disk) + }) + + t.Run("with empty API config sets up only disk source", func(t *testing.T) { + source := newAutoSource(sourceConfig{}) + + require.IsType(t, notAvailableSource{}, source.gitlab) + require.IsType(t, &disk.Disk{}, source.disk) + }) + + t.Run("with invalid API config sets up only disk source", func(t *testing.T) { + source := newAutoSource(sourceConfig{api: "https://gitlab.com"}) - newSource := NewMockSource() - newSource.On("GetDomain", testDomain). + require.IsType(t, notAvailableSource{}, source.gitlab) + require.IsType(t, &disk.Disk{}, source.disk) + }) +} + +func TestAutoSourceGetDomain(t *testing.T) { + t.Run("uses gitlab source for serverless domains regardless of gitlab source being ready", func(t *testing.T) { + testDomain := "func-aba1aabbccddeef2abaabbcc.serverless.gitlab.io" + + gitlabSource := NewMockSource() + gitlabSource.On("GetDomain", testDomain). Return(&domain.Domain{Name: testDomain}, nil). Once() - defer newSource.AssertExpectations(t) - domains := newTestDomains(t, newSource, sourceGitlab) + defer gitlabSource.AssertExpectations(t) + + source := &autoSource{gitlab: gitlabSource} - domain, err := domains.GetDomain(testDomain) + domain, err := source.GetDomain(testDomain) require.NoError(t, err) require.NotNil(t, domain) }) - t.Run("when requesting an existing domain for auto source", func(t *testing.T) { - testDomain := "new-source-test.gitlab.io" + t.Run("uses gitlab source if it's ready", func(t *testing.T) { + testDomain := "example.com" - newSource := NewMockSource() - newSource.On("GetDomain", testDomain). + gitlabSource := NewMockSource() + gitlabSource.On("GetDomain", testDomain). Return(&domain.Domain{Name: testDomain}, nil). Once() - newSource.On("IsReady").Return(true).Once() - defer newSource.AssertExpectations(t) + gitlabSource.On("IsReady").Return(true).Once() + + defer gitlabSource.AssertExpectations(t) - domains := newTestDomains(t, newSource, sourceAuto) + source := &autoSource{gitlab: gitlabSource} - domain, err := domains.GetDomain(testDomain) + domain, err := source.GetDomain(testDomain) require.NoError(t, err) require.NotNil(t, domain) }) - t.Run("when requesting a domain that doesn't exist for gitlab source", func(t *testing.T) { - newSource := NewMockSource() - newSource.On("GetDomain", "does-not-exist.test.io"). + t.Run("returns nil if gitlab source couldn't find it", func(t *testing.T) { + testDomain := "example.com" + + gitlabSource := NewMockSource() + gitlabSource.On("GetDomain", testDomain). Return(nil, nil). Once() + gitlabSource.On("IsReady").Return(true).Once() - defer newSource.AssertExpectations(t) + defer gitlabSource.AssertExpectations(t) - domains := newTestDomains(t, newSource, sourceGitlab) + source := &autoSource{gitlab: gitlabSource} - domain, err := domains.GetDomain("does-not-exist.test.io") + domain, err := source.GetDomain(testDomain) require.NoError(t, err) require.Nil(t, domain) }) - t.Run("when requesting a serverless domain", func(t *testing.T) { + t.Run("uses disk source if gitlab source isn't ready", func(t *testing.T) { + testDomain := "example.com" + + gitlabSource := NewMockSource() + gitlabSource.On("IsReady").Return(false).Once() + + diskSource := NewMockSource() + diskSource.On("GetDomain", testDomain). + Return(&domain.Domain{Name: testDomain}, nil). + Once() + + defer gitlabSource.AssertExpectations(t) + defer diskSource.AssertExpectations(t) + + source := &autoSource{gitlab: gitlabSource, disk: diskSource} + + domain, err := source.GetDomain(testDomain) + require.NoError(t, err) + require.NotNil(t, domain) + }) +} + +func TestAutoSourceIsReady(t *testing.T) { + tests := []struct { + name string + gitlabSourceReady bool + diskSourceReady bool + expectedResult bool + }{ + { + name: "when both sources aren't ready", + gitlabSourceReady: false, + diskSourceReady: false, + expectedResult: false, + }, + { + name: "when gitlab source is ready", + gitlabSourceReady: true, + diskSourceReady: false, + expectedResult: true, + }, + { + name: "when disk source is ready", + gitlabSourceReady: false, + diskSourceReady: true, + expectedResult: true, + }, + { + name: "when both sources are ready", + gitlabSourceReady: false, + diskSourceReady: true, + expectedResult: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gitlabSource := NewMockSource() + gitlabSource.On("IsReady").Return(tt.gitlabSourceReady).Once() + + diskSource := NewMockSource() + diskSource.On("IsReady").Return(tt.diskSourceReady).Once() + + source := &autoSource{gitlab: gitlabSource, disk: diskSource} + + require.Equal(t, tt.expectedResult, source.IsReady()) + }) + } +} + +func TestAutoSourceRead(t *testing.T) { + diskSource := NewMockSource() + diskSource.On("Read", "gitlab.io").Once() + + defer diskSource.AssertExpectations(t) + + source := &autoSource{disk: diskSource} + + source.Read("gitlab.io") +} + +func TestNewDiskServerlessSource(t *testing.T) { + t.Run("with valid API config sets up both gitlab and disk source", func(t *testing.T) { + source := newDiskServerlessSource(sourceConfig{api: "https://gitlab.com", secret: "abc"}) + + require.IsType(t, &gitlab.Gitlab{}, source.serverless) + require.IsType(t, &disk.Disk{}, source.disk) + }) + + t.Run("with empty API config sets up only disk source", func(t *testing.T) { + source := newDiskServerlessSource(sourceConfig{}) + + require.IsType(t, notAvailableSource{}, source.serverless) + require.IsType(t, &disk.Disk{}, source.disk) + }) + + t.Run("with invalid API config sets up only disk source", func(t *testing.T) { + source := newDiskServerlessSource(sourceConfig{api: "https://gitlab.com"}) + + require.IsType(t, notAvailableSource{}, source.serverless) + require.IsType(t, &disk.Disk{}, source.disk) + }) +} + +func TestDiskServerlessSourceGetDomain(t *testing.T) { + t.Run("uses gitlab source for serverless domains regardless of gitlab source being ready", func(t *testing.T) { testDomain := "func-aba1aabbccddeef2abaabbcc.serverless.gitlab.io" - newSource := NewMockSource() - newSource.On("GetDomain", testDomain). + gitlabSource := NewMockSource() + gitlabSource.On("GetDomain", testDomain). Return(&domain.Domain{Name: testDomain}, nil). Once() - defer newSource.AssertExpectations(t) + defer gitlabSource.AssertExpectations(t) - domains := newTestDomains(t, newSource, sourceGitlab) + source := &diskServerlessSource{serverless: gitlabSource} - domain, err := domains.GetDomain(testDomain) + domain, err := source.GetDomain(testDomain) require.NoError(t, err) require.NotNil(t, domain) }) + + t.Run("uses disk source for other domains", func(t *testing.T) { + testDomain := "example.com" + + diskSource := NewMockSource() + diskSource.On("GetDomain", testDomain). + Return(&domain.Domain{Name: testDomain}, nil). + Once() + + defer diskSource.AssertExpectations(t) + + source := &diskServerlessSource{disk: diskSource} + + domain, err := source.GetDomain(testDomain) + require.NoError(t, err) + require.NotNil(t, domain) + }) +} + +func TestDiskServerlessSourceIsReady(t *testing.T) { + gitlabSource := NewMockSource() + gitlabSource.On("IsReady").Return(false).Once() + + diskSource := NewMockSource() + diskSource.On("IsReady").Return(true).Once() + + defer diskSource.AssertExpectations(t) + + source := &autoSource{gitlab: gitlabSource, disk: diskSource} + + require.Equal(t, true, source.IsReady()) +} + +func TestDiskServerlessSourceRead(t *testing.T) { + diskSource := NewMockSource() + diskSource.On("Read", "gitlab.io").Once() + + defer diskSource.AssertExpectations(t) + + source := &diskServerlessSource{disk: diskSource} + + source.Read("gitlab.io") } func TestIsServerlessDomain(t *testing.T) { @@ -178,13 +362,3 @@ func TestIsServerlessDomain(t *testing.T) { require.False(t, IsServerlessDomain("somedomain.gitlab.io")) }) } - -func newTestDomains(t *testing.T, gitlabSource *MockSource, config configSource) *Domains { - t.Helper() - - return &Domains{ - configSource: config, - gitlab: gitlabSource, - disk: disk.New(), - } -} diff --git a/internal/source/gitlab/gitlab.go b/internal/source/gitlab/gitlab.go index 67c4eb6ba5d02d471a9ee2e3dbdd5a582ca5ee42..c4278a1982f26db92f35d8dcca3e1f064741165e 100644 --- a/internal/source/gitlab/gitlab.go +++ b/internal/source/gitlab/gitlab.go @@ -114,3 +114,7 @@ func (g *Gitlab) IsReady() bool { return g.isReady } + +// Read is a dummy method to match disk interface +func (g *Gitlab) Read(string) { +} diff --git a/internal/source/source.go b/internal/source/source.go index 5540066c7bad34398b33b795a946fc1077045554..1efc7d4fdbd7cc5eef32570ee030a902fdcafed2 100644 --- a/internal/source/source.go +++ b/internal/source/source.go @@ -6,4 +6,5 @@ import "gitlab.com/gitlab-org/gitlab-pages/internal/domain" type Source interface { GetDomain(string) (*domain.Domain, error) IsReady() bool + Read(string) } diff --git a/internal/source/source_mock.go b/internal/source/source_mock.go index d7cd15333f65d625bcb7a3531e0a1fd2656bb78c..0987f2f1df70b03301265bcc71cdb8d97cb49f44 100644 --- a/internal/source/source_mock.go +++ b/internal/source/source_mock.go @@ -30,6 +30,10 @@ func (m *MockSource) IsReady() bool { return args.Get(0).(bool) } +func (m *MockSource) Read(rootDomain string) { + m.Called(rootDomain) +} + // NewMockSource returns a new Source mock for testing func NewMockSource() *MockSource { return &MockSource{}