From 8e77e03284e9022f3a551b40d2b6416df105609c Mon Sep 17 00:00:00 2001
From: Martin Sehnoutka <msehnout@redhat.com>
Date: Wed, 19 Feb 2020 11:46:29 +0100
Subject: [PATCH] distro: make it impossible to initialize registry with nil
 values

The current `NewRegistry` implementation allows for nil values in the
map, but this leads to subtle bugs when using the registry. This patch
enforces non-nil values by introducing additional checks before we
insert the value into the map.

The change unfortunately breaks a lot of tests and therefore it is
necessary to create additional mock: distro.

The new mock is used instead of the previous "real" implementation,
which used to contain nil values.
---
 internal/distro/distro.go            | 27 +++++++++++++++++++++++----
 internal/jobqueue/api_test.go        |  8 ++++----
 internal/mocks/distro/distro_mock.go | 14 ++++++++++++++
 internal/mocks/rpmmd/fixtures.go     |  6 +++---
 4 files changed, 44 insertions(+), 11 deletions(-)
 create mode 100644 internal/mocks/distro/distro_mock.go

diff --git a/internal/distro/distro.go b/internal/distro/distro.go
index 3efacb92d..792dd9d4b 100644
--- a/internal/distro/distro.go
+++ b/internal/distro/distro.go
@@ -69,14 +69,33 @@ type Registry struct {
 	distros map[common.Distribution]Distro
 }
 
+func WithSingleDistro(dist Distro) *Registry {
+	reg := &Registry{
+		distros: make(map[common.Distribution]Distro),
+	}
+	reg.register(dist)
+	return reg
+}
+
 func NewRegistry(confPaths []string) *Registry {
 	distros := &Registry{
 		distros: make(map[common.Distribution]Distro),
 	}
-	distros.register(fedora30.New(confPaths))
-	distros.register(fedora31.New(confPaths))
-	distros.register(fedora32.New(confPaths))
-	distros.register(rhel82.New(confPaths))
+	f30 := fedora30.New(confPaths)
+	if f30 == nil {
+		panic("Attempt to register Fedora 30 failed")
+	}
+	distros.register(f30)
+	f31 := fedora31.New(confPaths)
+	if f31 == nil {
+		panic("Attempt to register Fedora 30 failed")
+	}
+	distros.register(f31)
+	el82 := rhel82.New(confPaths)
+	if el82 == nil {
+		panic("Attempt to register RHEL 8.2 failed")
+	}
+	distros.register(el82)
 	return distros
 }
 
diff --git a/internal/jobqueue/api_test.go b/internal/jobqueue/api_test.go
index 8fc3ed205..69a5e1bd1 100644
--- a/internal/jobqueue/api_test.go
+++ b/internal/jobqueue/api_test.go
@@ -1,11 +1,11 @@
 package jobqueue_test
 
 import (
+	distro_mock "github.com/osbuild/osbuild-composer/internal/mocks/distro"
 	"net/http"
 	"testing"
 
 	"github.com/osbuild/osbuild-composer/internal/blueprint"
-	"github.com/osbuild/osbuild-composer/internal/distro"
 	test_distro "github.com/osbuild/osbuild-composer/internal/distro/fedoratest"
 	"github.com/osbuild/osbuild-composer/internal/jobqueue"
 	"github.com/osbuild/osbuild-composer/internal/store"
@@ -34,7 +34,7 @@ func TestBasic(t *testing.T) {
 
 	for _, c := range cases {
 		distroStruct := test_distro.New()
-		registry := distro.NewRegistry([]string{"."})
+		registry := distro_mock.NewRegistry()
 		api := jobqueue.New(nil, store.New(nil, distroStruct, *registry))
 
 		test.TestNonJsonRoute(t, api, false, c.Method, c.Path, c.Body, c.ExpectedStatus, c.ExpectedResponse)
@@ -44,7 +44,7 @@ func TestBasic(t *testing.T) {
 func TestCreate(t *testing.T) {
 	id, _ := uuid.Parse("ffffffff-ffff-ffff-ffff-ffffffffffff")
 	distroStruct := test_distro.New()
-	registry := distro.NewRegistry([]string{"."})
+	registry := distro_mock.NewRegistry()
 	store := store.New(nil, distroStruct, *registry)
 	api := jobqueue.New(nil, store)
 
@@ -60,7 +60,7 @@ func TestCreate(t *testing.T) {
 func testUpdateTransition(t *testing.T, from, to string, expectedStatus int, expectedResponse string) {
 	id, _ := uuid.Parse("ffffffff-ffff-ffff-ffff-ffffffffffff")
 	distroStruct := test_distro.New()
-	registry := distro.NewRegistry([]string{"."})
+	registry := distro_mock.NewRegistry()
 	store := store.New(nil, distroStruct, *registry)
 	api := jobqueue.New(nil, store)
 
diff --git a/internal/mocks/distro/distro_mock.go b/internal/mocks/distro/distro_mock.go
new file mode 100644
index 000000000..178ee3e93
--- /dev/null
+++ b/internal/mocks/distro/distro_mock.go
@@ -0,0 +1,14 @@
+package distro_mock
+
+import (
+	"github.com/osbuild/osbuild-composer/internal/distro"
+	"github.com/osbuild/osbuild-composer/internal/distro/fedoratest"
+)
+
+func NewRegistry() *distro.Registry {
+	ftest := fedoratest.New()
+	if ftest == nil {
+		panic("Attempt to register Fedora test failed")
+	}
+	return distro.WithSingleDistro(ftest)
+}
diff --git a/internal/mocks/rpmmd/fixtures.go b/internal/mocks/rpmmd/fixtures.go
index 60f7b07cb..547b485d4 100644
--- a/internal/mocks/rpmmd/fixtures.go
+++ b/internal/mocks/rpmmd/fixtures.go
@@ -4,7 +4,7 @@ import (
 	"fmt"
 	"github.com/osbuild/osbuild-composer/internal/common"
 	"github.com/osbuild/osbuild-composer/internal/compose"
-	"github.com/osbuild/osbuild-composer/internal/distro"
+	distro_mock "github.com/osbuild/osbuild-composer/internal/mocks/distro"
 	"sort"
 	"time"
 
@@ -94,7 +94,7 @@ func createBaseStoreFixture() *store.Store {
 	}
 
 	d := test_distro.New()
-	r := distro.NewRegistry([]string{"."})
+	r := distro_mock.NewRegistry()
 	s := store.New(nil, d, *r)
 
 	s.Blueprints[bName] = b
@@ -184,7 +184,7 @@ func createStoreWithoutComposesFixture() *store.Store {
 	}
 
 	d := test_distro.New()
-	r := distro.NewRegistry([]string{"."})
+	r := distro_mock.NewRegistry()
 	s := store.New(nil, d, *r)
 
 	s.Blueprints[bName] = b