diff --git a/internal/source/domains.go b/internal/source/domains.go index ca2a8b6e201895e5b7ff79707b7422b55ba44252..77e1aa1d41704f57ca44c6c30ed33fcd2a8b28a4 100644 --- a/internal/source/domains.go +++ b/internal/source/domains.go @@ -44,22 +44,19 @@ func NewDomains(config Config) (*Domains, error) { // TODO: choose domain source config via config.DomainConfigSource() // https://gitlab.com/gitlab-org/gitlab/-/issues/217912 - domains := &Domains{ - disk: disk.New(), - } - if len(config.InternalGitLabServerURL()) == 0 || len(config.GitlabAPISecret()) == 0 { - return domains, nil + return &Domains{disk: disk.New()}, nil } - glClient, err := gitlab.New(config) + gitlab, err := gitlab.New(config) if err != nil { return nil, err } - domains.gitlab = glClient - - return domains, nil + return &Domains{ + gitlab: gitlab, + disk: disk.New(), + }, nil } // GetDomain retrieves a domain information from a source. We are using two @@ -88,7 +85,7 @@ func (d *Domains) IsReady() bool { } func (d *Domains) source(domain string) Source { - if d.gitlab == nil || !d.gitlab.IsReady() { + if d.gitlab == nil { return d.disk } diff --git a/internal/source/domains_test.go b/internal/source/domains_test.go index 2382e75666a51c9eab3f26be4be13da4540727ff..24008b08e28ac9f29e04674ce36388d8c2a3aa54 100644 --- a/internal/source/domains_test.go +++ b/internal/source/domains_test.go @@ -65,21 +65,24 @@ func TestGetDomain(t *testing.T) { newSource.On("GetDomain", testDomain). Return(&domain.Domain{Name: testDomain}, nil). Once() - newSource.On("IsReady").Return(true).Once() defer newSource.AssertExpectations(t) - domains := newTestDomains(t, newSource) + domains := &Domains{ + disk: disk.New(), + gitlab: newSource, + } domains.GetDomain(testDomain) }) t.Run("when requesting a non-test domain", func(t *testing.T) { newSource := NewMockSource() - newSource.On("IsReady").Return(true).Once() - defer newSource.AssertExpectations(t) - domains := newTestDomains(t, newSource) + domains := &Domains{ + disk: disk.New(), + gitlab: newSource, + } domain, err := domains.GetDomain("domain.test.io") @@ -91,7 +94,10 @@ func TestGetDomain(t *testing.T) { newSource := NewMockSource() defer newSource.AssertExpectations(t) - domains := newTestDomains(t, newSource) + domains := &Domains{ + disk: disk.New(), + gitlab: newSource, + } domain, err := domains.GetDomain("pages-broken-poc.gitlab.io") @@ -116,11 +122,12 @@ func TestGetDomain(t *testing.T) { newSource.On("GetDomain", testDomain). Return(&domain.Domain{Name: testDomain}, nil). Once() - newSource.On("IsReady").Return(true).Once() - defer newSource.AssertExpectations(t) - domains := newTestDomains(t, newSource) + domains := &Domains{ + disk: disk.New(), + gitlab: newSource, + } domains.GetDomain(testDomain) }) @@ -149,6 +156,8 @@ func TestGetDomainWithIncrementalrolloutOfGitLabSource(t *testing.T) { // Generates FNV 2643293380, 2643293380 % 100 = 80 domain80 := "test-domain-b.com" + diskSource := disk.New() + gitlabSourceConfig.Domains.Rollout.Percentage = 80 type testDomain struct { @@ -192,10 +201,12 @@ func TestGetDomainWithIncrementalrolloutOfGitLabSource(t *testing.T) { Once() } } - gitlabSource.On("IsReady").Return(true) defer gitlabSource.AssertExpectations(t) - domains := newTestDomains(t, gitlabSource) + domains := &Domains{ + disk: diskSource, + gitlab: gitlabSource, + } gitlabSourceConfig.Domains.Rollout.Stickiness = tc.stickiness @@ -206,12 +217,3 @@ func TestGetDomainWithIncrementalrolloutOfGitLabSource(t *testing.T) { }) } } - -func newTestDomains(t *testing.T, gitlabSource *MockSource) *Domains { - t.Helper() - - return &Domains{ - disk: disk.New(), - gitlab: gitlabSource, - } -} diff --git a/internal/source/gitlab/gitlab.go b/internal/source/gitlab/gitlab.go index 2635d864c4a1db38cdda8d01d8166195db566895..5bacb603c8b3992f699d76bad76cb4516a0cd257 100644 --- a/internal/source/gitlab/gitlab.go +++ b/internal/source/gitlab/gitlab.go @@ -104,11 +104,3 @@ func (g *Gitlab) Resolve(r *http.Request) (*serving.Request, error) { return &serving.Request{Serving: defaultServing()}, errors.New("could not match lookup path") } - -// IsReady returns the value of Gitlab `isReady` which is updated by `Poll`. -func (g *Gitlab) IsReady() bool { - g.mu.RLock() - defer g.mu.RUnlock() - - return g.isReady -} diff --git a/internal/source/gitlab/gitlab_poll.go b/internal/source/gitlab/gitlab_poll.go index 9fce7250c9f5cee8ac9a2b6e65404b79fc079309..7064482226d86a4989298e61efaf6f04226d2e69 100644 --- a/internal/source/gitlab/gitlab_poll.go +++ b/internal/source/gitlab/gitlab_poll.go @@ -15,7 +15,6 @@ const ( // poll tries to call the /internal/pages/status API endpoint once plus // `retries` every `interval`. -// It updates the `isReady` value when successful. // TODO: Remove in https://gitlab.com/gitlab-org/gitlab/-/issues/218357 func (g *Gitlab) poll(retries int, interval time.Duration) { var err error diff --git a/internal/source/source.go b/internal/source/source.go index 5540066c7bad34398b33b795a946fc1077045554..4b43b8f4b6f5f0073c5feb1688c261933bb3539d 100644 --- a/internal/source/source.go +++ b/internal/source/source.go @@ -5,5 +5,4 @@ import "gitlab.com/gitlab-org/gitlab-pages/internal/domain" // Source represents an abstract interface of a domains configuration source. type Source interface { GetDomain(string) (*domain.Domain, error) - IsReady() bool } diff --git a/internal/source/source_mock.go b/internal/source/source_mock.go index 7c693eb139cc6a724ccdba13197bff07d21a00f1..ee24d804e6702fd17f57c0901cfdbdde238fdfe4 100644 --- a/internal/source/source_mock.go +++ b/internal/source/source_mock.go @@ -18,12 +18,6 @@ func (m *MockSource) GetDomain(name string) (*domain.Domain, error) { return args.Get(0).(*domain.Domain), args.Error(1) } -func (m *MockSource) IsReady() bool { - args := m.Called() - - return args.Get(0).(bool) -} - // NewMockSource returns a new Source mock for testing func NewMockSource() *MockSource { return &MockSource{}