Respond with 502 if a domain can not be retrieved from a source

5 files changed, 18 insertions, 45 deletions

diff --git a/internal/source/disk/disk.go b/internal/source/disk/disk.go
index a3ae8901..b79d222d 100644
--- a/internal/source/disk/disk.go
+++ b/internal/source/disk/disk.go
@@ -24,25 +24,16 @@ func New() *Disk {
 	}
 }
 
-// GetDomain returns a domain from the domains map
-func (d *Disk) GetDomain(host string) *domain.Domain {
+// GetDomain returns a domain from the domains map if it exists
+func (d *Disk) GetDomain(host string) (*domain.Domain, error) {
 	host = strings.ToLower(host)
-	d.lock.RLock()
-	defer d.lock.RUnlock()
-	domain, _ := d.dm[host]
 
-	return domain
-}
-
-// HasDomain checks for presence of a domain in the domains map
-func (d *Disk) HasDomain(host string) bool {
 	d.lock.RLock()
 	defer d.lock.RUnlock()
 
-	host = strings.ToLower(host)
-	_, isPresent := d.dm[host]
+	domain, _ := d.dm[host]
 
-	return isPresent
+	return domain, nil
 }
 
 // IsReady checks if the domains source is ready for work. The disk source is
diff --git a/internal/source/domains.go b/internal/source/domains.go
index 92bab663..f9dadef8 100644
--- a/internal/source/domains.go
+++ b/internal/source/domains.go
@@ -34,16 +34,10 @@ func NewDomains() *Domains {
 // 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 {
+func (d *Domains) GetDomain(name string) (*domain.Domain, error) {
 	return d.source(name).GetDomain(name)
 }
 
-// HasDomain checks if domain exists. It is using new and the legacy domains
-// source.
-func (d *Domains) HasDomain(name string) bool {
-	return d.source(name).HasDomain(name)
-}
-
 // Start 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) {
diff --git a/internal/source/domains_test.go b/internal/source/domains_test.go
index 1da50db8..64ec5f8d 100644
--- a/internal/source/domains_test.go
+++ b/internal/source/domains_test.go
@@ -5,6 +5,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/domain"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/source/disk"
@@ -14,27 +15,20 @@ type mockSource struct {
 	mock.Mock
 }
 
-func (m mockSource) GetDomain(name string) *domain.Domain {
+func (m mockSource) GetDomain(name string) (*domain.Domain, error) {
 	args := m.Called(name)
 
-	return args.Get(0).(*domain.Domain)
+	return args.Get(0).(*domain.Domain), args.Error(1)
 }
 
-func (m mockSource) HasDomain(name string) bool {
-	args := m.Called(name)
-
-	return args.Bool(0)
-}
-
-func TestSourceTransition(t *testing.T) {
+func TestHasDomain(t *testing.T) {
 	testDomain := newSourceDomains[0]
 
 	t.Run("when requesting a test domain", func(t *testing.T) {
 		newSource := new(mockSource)
 		newSource.On("GetDomain", testDomain).
-			Return(&domain.Domain{Name: testDomain}).
+			Return(&domain.Domain{Name: testDomain}, nil).
 			Once()
-		newSource.On("HasDomain", testDomain).Return(true).Once()
 		defer newSource.AssertExpectations(t)
 
 		domains := &Domains{
@@ -43,7 +37,6 @@ func TestSourceTransition(t *testing.T) {
 		}
 
 		domains.GetDomain(testDomain)
-		domains.HasDomain(testDomain)
 	})
 
 	t.Run("when requesting a non-test domain", func(t *testing.T) {
@@ -55,7 +48,9 @@ func TestSourceTransition(t *testing.T) {
 			gitlab: newSource,
 		}
 
-		assert.Nil(t, domains.GetDomain("some.test.io"))
-		assert.False(t, domains.HasDomain("some.test.io"))
+		domain, err := domains.GetDomain("domain.test.io")
+
+		require.NoError(t, err)
+		assert.Nil(t, domain)
 	})
 }
diff --git a/internal/source/gitlab/gitlab.go b/internal/source/gitlab/gitlab.go
index b0efb01d..09cd06b9 100644
--- a/internal/source/gitlab/gitlab.go
+++ b/internal/source/gitlab/gitlab.go
@@ -1,6 +1,7 @@
 package gitlab
 
 import (
+	"errors"
 	"net/http"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/domain"
@@ -21,15 +22,8 @@ func New() *Gitlab {
 
 // GetDomain return a representation of a domain that we have fetched from
 // GitLab
-// It should return source.Lookup TODO
-func (g *Gitlab) GetDomain(name string) *domain.Domain {
-	return nil
-}
-
-// HasDomain checks if a domain is known to GitLab
-// TODO lookup status code etc.
-func (g *Gitlab) HasDomain(name string) bool {
-	return g.GetDomain(name) != nil
+func (g *Gitlab) GetDomain(name string) (*domain.Domain, error) {
+	return nil, errors.New("not implemented")
 }
 
 // Resolve is supposed to get the serving lookup path based on the request from
diff --git a/internal/source/source.go b/internal/source/source.go
index da63cbcc..4b43b8f4 100644
--- a/internal/source/source.go
+++ b/internal/source/source.go
@@ -4,6 +4,5 @@ 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
-	HasDomain(string) bool
+	GetDomain(string) (*domain.Domain, error)
 }