Repository: osbuild/osbuild-composer
Pull request #110 (2019-11-17 12:34:17)

Get repositories from distro
Please see individual commits. I included some cleanup commits before so that my changes don't make the API handlers more complex.


Patch:
diff --git a/cmd/osbuild-composer/main.go b/cmd/osbuild-composer/main.go
index 3322c497e..db884337b 100644
--- a/cmd/osbuild-composer/main.go
+++ b/cmd/osbuild-composer/main.go
@@ -5,6 +5,7 @@ import (
 	"log"
 	"os"
 
+	"github.com/osbuild/osbuild-composer/internal/distro"
 	"github.com/osbuild/osbuild-composer/internal/jobqueue"
 	"github.com/osbuild/osbuild-composer/internal/rpmmd"
 	"github.com/osbuild/osbuild-composer/internal/store"
@@ -32,13 +33,8 @@ func main() {
 	weldrListener := listeners[0]
 	jobListener := listeners[1]
 
-	repo := rpmmd.RepoConfig{
-		Id:       "fedora",
-		Name:     "Fedora 30",
-		Metalink: "https://mirrors.fedoraproject.org/metalink?repo=fedora-30&arch=x86_64",
-	}
-
 	rpm := rpmmd.NewRPMMD()
+	distribution := distro.New("")
 
 	var logger *log.Logger
 	if verbose {
@@ -53,7 +49,7 @@ func main() {
 	store := store.New(&stateFile)
 
 	jobAPI := jobqueue.New(logger, store)
-	weldrAPI := weldr.New(rpm, repo, logger, store)
+	weldrAPI := weldr.New(rpm, distribution, logger, store)
 
 	go jobAPI.Serve(jobListener)
 	weldrAPI.Serve(weldrListener)
diff --git a/internal/distro/distro.go b/internal/distro/distro.go
index 42bbd6dcf..2317c79a6 100644
--- a/internal/distro/distro.go
+++ b/internal/distro/distro.go
@@ -10,9 +10,14 @@ import (
 
 	"github.com/osbuild/osbuild-composer/internal/blueprint"
 	"github.com/osbuild/osbuild-composer/internal/pipeline"
+	"github.com/osbuild/osbuild-composer/internal/rpmmd"
 )
 
 type Distro interface {
+	// Returns a list of repositories from which this distribution gets its
+	// content.
+	Repositories() []rpmmd.RepoConfig
+
 	// Returns a sorted list of the output formats this distro supports.
 	ListOutputFormats() []string
 
diff --git a/internal/distro/fedora30/os.go b/internal/distro/fedora30/os.go
index e834035b3..0664057a5 100644
--- a/internal/distro/fedora30/os.go
+++ b/internal/distro/fedora30/os.go
@@ -6,6 +6,7 @@ import (
 	"github.com/osbuild/osbuild-composer/internal/blueprint"
 	"github.com/osbuild/osbuild-composer/internal/distro"
 	"github.com/osbuild/osbuild-composer/internal/pipeline"
+	"github.com/osbuild/osbuild-composer/internal/rpmmd"
 )
 
 type Fedora30 struct {
@@ -34,6 +35,16 @@ func init() {
 	})
 }
 
+func (f *Fedora30) Repositories() []rpmmd.RepoConfig {
+	return []rpmmd.RepoConfig{
+		{
+			Id:       "fedora",
+			Name:     "Fedora 30",
+			Metalink: "https://mirrors.fedoraproject.org/metalink?repo=fedora-30&arch=x86_64",
+		},
+	}
+}
+
 // ListOutputFormats returns a sorted list of the supported output formats
 func (f *Fedora30) ListOutputFormats() []string {
 	formats := make([]string, 0, len(f.outputs))
diff --git a/internal/rpmmd/repository.go b/internal/rpmmd/repository.go
index 06fa7a12c..41f00b2c0 100644
--- a/internal/rpmmd/repository.go
+++ b/internal/rpmmd/repository.go
@@ -10,8 +10,6 @@ import (
 	"time"
 
 	"github.com/gobwas/glob"
-
-	"github.com/osbuild/osbuild-composer/internal/store"
 )
 
 type RepoConfig struct {
@@ -250,20 +248,3 @@ func (pkg *PackageInfo) FillDependencies(rpmmd RPMMD, repos []RepoConfig) (err e
 	pkg.Dependencies, err = rpmmd.Depsolve([]string{pkg.Name}, repos)
 	return
 }
-
-func SourceToRepo(source store.SourceConfig) RepoConfig {
-	var repo RepoConfig
-
-	repo.Name = source.Name
-	repo.Id = source.Name
-
-	if source.Type == "yum-baseurl" {
-		repo.BaseURL = source.URL
-	} else if source.Type == "yum-metalink" {
-		repo.Metalink = source.URL
-	} else if source.Type == "yum-mirrorlist" {
-		repo.MirrorList = source.URL
-	}
-
-	return repo
-}
diff --git a/internal/store/store.go b/internal/store/store.go
index 732118a78..1b4c01c82 100644
--- a/internal/store/store.go
+++ b/internal/store/store.go
@@ -16,6 +16,7 @@ import (
 
 	"github.com/osbuild/osbuild-composer/internal/blueprint"
 	"github.com/osbuild/osbuild-composer/internal/pipeline"
+	"github.com/osbuild/osbuild-composer/internal/rpmmd"
 	"github.com/osbuild/osbuild-composer/internal/target"
 
 	"github.com/osbuild/osbuild-composer/internal/distro"
@@ -604,3 +605,42 @@ func (s *Store) GetAllSources() map[string]SourceConfig {
 
 	return sources
 }
+
+func NewSourceConfig(repo rpmmd.RepoConfig, system bool) SourceConfig {
+	sc := SourceConfig{
+		Name:     repo.Id,
+		CheckGPG: true,
+		CheckSSL: true,
+		System:   system,
+	}
+
+	if repo.BaseURL != "" {
+		sc.URL = repo.BaseURL
+		sc.Type = "yum-baseurl"
+	} else if repo.Metalink != "" {
+		sc.URL = repo.Metalink
+		sc.Type = "yum-metalink"
+	} else if repo.MirrorList != "" {
+		sc.URL = repo.MirrorList
+		sc.Type = "yum-mirrorlist"
+	}
+
+	return sc
+}
+
+func (s *SourceConfig) RepoConfig() rpmmd.RepoConfig {
+	var repo rpmmd.RepoConfig
+
+	repo.Name = s.Name
+	repo.Id = s.Name
+
+	if s.Type == "yum-baseurl" {
+		repo.BaseURL = s.URL
+	} else if s.Type == "yum-metalink" {
+		repo.Metalink = s.URL
+	} else if s.Type == "yum-mirrorlist" {
+		repo.MirrorList = s.URL
+	}
+
+	return repo
+}
diff --git a/internal/weldr/api.go b/internal/weldr/api.go
index 2a941dcb9..a684897e6 100644
--- a/internal/weldr/api.go
+++ b/internal/weldr/api.go
@@ -25,19 +25,19 @@ type API struct {
 	store *store.Store
 
 	rpmmd rpmmd.RPMMD
-	repo  rpmmd.RepoConfig
+	distro distro.Distro
 
 	logger *log.Logger
 	router *httprouter.Router
 }
 
-func New(rpmmd rpmmd.RPMMD, repo rpmmd.RepoConfig, logger *log.Logger, store *store.Store) *API {
+func New(rpmmd rpmmd.RPMMD, distro distro.Distro, logger *log.Logger, store *store.Store) *API {
 	// This needs to be shared with the worker API so that they can communicate with each other
 	// builds := make(chan queue.Build, 200)
 	api := &API{
 		store:  store,
 		rpmmd:  rpmmd,
-		repo:   repo,
+		distro: distro,
 		logger: logger,
 	}
 
@@ -49,7 +49,8 @@ func New(rpmmd rpmmd.RPMMD, repo rpmmd.RepoConfig, logger *log.Logger, store *st
 
 	api.router.GET("/api/status", api.statusHandler)
 	api.router.GET("/api/v0/projects/source/list", api.sourceListHandler)
-	api.router.GET("/api/v0/projects/source/info/*sources", api.sourceInfoHandler)
+	api.router.GET("/api/v0/projects/source/info/", api.sourceEmptyInfoHandler)
+	api.router.GET("/api/v0/projects/source/info/:sources", api.sourceInfoHandler)
 	api.router.POST("/api/v0/projects/source/new", api.sourceNewHandler)
 	api.router.DELETE("/api/v0/projects/source/delete/*source", api.sourceDeleteHandler)
 
@@ -170,13 +171,25 @@ func (api *API) sourceListHandler(writer http.ResponseWriter, request *http.Requ
 	}
 
 	names := api.store.ListSources()
-	names = append(names, api.repo.Id)
+
+	for _, repo := range api.distro.Repositories() {
+		names = append(names, repo.Id)
+	}
 
 	json.NewEncoder(writer).Encode(reply{
 		Sources: names,
 	})
 }
 
+func (api *API) sourceEmptyInfoHandler(writer http.ResponseWriter, request *http.Request, params httprouter.Params) {
+	errors := responseError{
+		Code: http.StatusNotFound,
+		ID:   "HTTPError",
+		Msg:  "Not Found",
+	}
+	statusResponseError(writer, http.StatusNotFound, errors)
+}
+
 func (api *API) sourceInfoHandler(writer http.ResponseWriter, request *http.Request, params httprouter.Params) {
 	// weldr uses a slightly different format than dnf to store repository
 	// configuration
@@ -185,58 +198,41 @@ func (api *API) sourceInfoHandler(writer http.ResponseWriter, request *http.Requ
 		Errors  []responseError               `json:"errors"`
 	}
 
-	names := strings.Split(params.ByName("sources"), ",")
-
-	if names[0] == "/" {
-		errors := responseError{
-			Code: http.StatusNotFound,
-			ID:   "HTTPError",
-			Msg:  "Not Found",
-		}
-		statusResponseError(writer, http.StatusNotFound, errors)
-		return
-	}
+	names := params.ByName("sources")
 
 	sources := map[string]store.SourceConfig{}
 	errors := []responseError{}
-	for i, name := range names {
-		// remove leading / from first name
-		if i == 0 {
-			name = name[1:]
-		}
-		// if name is "*" we want all sources from the store and the base repo
-		if name == "*" {
-			sources = api.store.GetAllSources()
+
+	// if names is "*" we want all sources
+	if names == "*" {
+		sources = api.store.GetAllSources()
+		for _, repo := range api.distro.Repositories() {
+			sources[repo.Id] = store.NewSourceConfig(repo, true)
 		}
-		// check if the source is in the base repo
-		if name == api.repo.Id || name == "*" {
-			cfg := store.SourceConfig{
-				Name:     api.repo.Name,
-				CheckGPG: true,
-				CheckSSL: true,
-				System:   true,
+	} else {
+		for _, name := range strings.Split(names, ",") {
+			// check if the source is one of the base repos
+			found := false
+			for _, repo := range api.distro.Repositories() {
+				if name == repo.Id {
+					sources[repo.Id] = store.NewSourceConfig(repo, true)
+					found = true
+					break
+				}
 			}
-
-			if api.repo.BaseURL != "" {
-				cfg.URL = api.repo.BaseURL
-				cfg.Type = "yum-baseurl"
-			} else if api.repo.Metalink != "" {
-				cfg.URL = api.repo.Metalink
-				cfg.Type = "yum-metalink"
-			} else if api.repo.MirrorList != "" {
-				cfg.URL = api.repo.MirrorList
-				cfg.Type = "yum-mirrorlist"
+			if found {
+				continue
 			}
-			sources[cfg.Name] = cfg
 			// check if the source is in the store
-		} else if source := api.store.GetSource(name); source != nil {
-			sources[source.Name] = *source
-		} else {
-			error := responseError{
-				ID:  "UnknownSource",
-				Msg: fmt.Sprintf("%s is not a valid source", name),
+			if source := api.store.GetSource(name); source != nil {
+				sources[source.Name] = *source
+			} else {
+				error := responseError{
+					ID:  "UnknownSource",
+					Msg: fmt.Sprintf("%s is not a valid source", name),
+				}
+				errors = append(errors, error)
 			}
-			errors = append(errors, error)
 		}
 	}
 
@@ -501,7 +497,7 @@ func (api *API) modulesInfoHandler(writer http.ResponseWriter, request *http.Req
 
 	if modulesRequested {
 		for i, _ := range packageInfos {
-			err := packageInfos[i].FillDependencies(api.rpmmd, []rpmmd.RepoConfig{api.repo})
+			err := packageInfos[i].FillDependencies(api.rpmmd, api.distro.Repositories())
 			if err != nil {
 				errors := responseError{
 					ID:  errorId,
@@ -527,7 +523,7 @@ func (api *API) projectsDepsolveHandler(writer http.ResponseWriter, request *htt
 
 	names := strings.Split(params.ByName("projects"), ",")
 
-	packages, err := api.rpmmd.Depsolve(names, []rpmmd.RepoConfig{api.repo})
+	packages, err := api.rpmmd.Depsolve(names, api.distro.Repositories())
 
 	if err != nil {
 		errors := responseError{
@@ -675,9 +671,11 @@ func (api *API) blueprintsDepsolveHandler(writer http.ResponseWriter, request *h
 		}
 
 		var repos []rpmmd.RepoConfig
-		repos = append(repos, api.repo)
+		for _, repo := range api.distro.Repositories() {
+			repos = append(repos, repo)
+		}
 		for _, source := range api.store.GetAllSources() {
-			repos = append(repos, rpmmd.SourceToRepo(source))
+			repos = append(repos, source.RepoConfig())
 		}
 
 		dependencies, _ := api.rpmmd.Depsolve(specs, repos)
@@ -743,9 +741,11 @@ func (api *API) blueprintsFreezeHandler(writer http.ResponseWriter, request *htt
 		}
 
 		var repos []rpmmd.RepoConfig
-		repos = append(repos, api.repo)
+		for _, repo := range api.distro.Repositories() {
+			repos = append(repos, repo)
+		}
 		for _, source := range api.store.GetAllSources() {
-			repos = append(repos, rpmmd.SourceToRepo(source))
+			repos = append(repos, source.RepoConfig())
 		}
 
 		dependencies, _ := api.rpmmd.Depsolve(specs, repos)
@@ -1181,9 +1181,11 @@ func (api *API) composeFailedHandler(writer http.ResponseWriter, request *http.R
 
 func (api *API) fetchPackageList() (rpmmd.PackageList, error) {
 	var repos []rpmmd.RepoConfig
-	repos = append(repos, api.repo)
+	for _, repo := range api.distro.Repositories() {
+		repos = append(repos, repo)
+	}
 	for _, source := range api.store.GetAllSources() {
-		repos = append(repos, rpmmd.SourceToRepo(source))
+		repos = append(repos, source.RepoConfig())
 	}
 
 	return api.rpmmd.FetchPackageList(repos)


Test patch:
diff --git a/internal/distro/test/distro.go b/internal/distro/test/distro.go
new file mode 100644
index 000000000..bc2c9b2b3
--- /dev/null
+++ b/internal/distro/test/distro.go
@@ -0,0 +1,36 @@
+package test
+
+import (
+	"github.com/osbuild/osbuild-composer/internal/blueprint"
+	"github.com/osbuild/osbuild-composer/internal/distro"
+	"github.com/osbuild/osbuild-composer/internal/pipeline"
+	"github.com/osbuild/osbuild-composer/internal/rpmmd"
+)
+
+type TestDistro struct{}
+
+func init() {
+	distro.Register("test", &TestDistro{})
+}
+
+func (d *TestDistro) Repositories() []rpmmd.RepoConfig {
+	return []rpmmd.RepoConfig{
+		{
+			Id:      "test",
+			Name:    "Test",
+			BaseURL: "http://example.com/test/os",
+		},
+	}
+}
+
+func (d *TestDistro) ListOutputFormats() []string {
+	return []string{}
+}
+
+func (d *TestDistro) FilenameFromType(outputFormat string) (string, string, error) {
+	return "", "", &distro.InvalidOutputFormatError{outputFormat}
+}
+
+func (d *TestDistro) Pipeline(b *blueprint.Blueprint, outputFormat string) (*pipeline.Pipeline, error) {
+	return nil, &distro.InvalidOutputFormatError{outputFormat}
+}
diff --git a/internal/weldr/api_test.go b/internal/weldr/api_test.go
index 2fb4a53b6..8a3087529 100644
--- a/internal/weldr/api_test.go
+++ b/internal/weldr/api_test.go
@@ -17,16 +17,12 @@ import (
 	"time"
 
 	rpmmd_mock "github.com/osbuild/osbuild-composer/internal/mocks/rpmmd"
-	"github.com/osbuild/osbuild-composer/internal/rpmmd"
 	"github.com/osbuild/osbuild-composer/internal/store"
 	"github.com/osbuild/osbuild-composer/internal/weldr"
-)
 
-var repo = rpmmd.RepoConfig{
-	Id:      "test",
-	Name:    "Test",
-	BaseURL: "http://example.com/test/os",
-}
+	"github.com/osbuild/osbuild-composer/internal/distro"
+	_ "github.com/osbuild/osbuild-composer/internal/distro/test"
+)
 
 func externalRequest(method, path, body string) *http.Response {
 	client := http.Client{
@@ -159,8 +155,9 @@ func testRoute(t *testing.T, api *weldr.API, external bool, method, path, body s
 func createWeldrAPI(fixture rpmmd_mock.Fixture) (*weldr.API, *store.Store) {
 	s := store.New(nil)
 	rpm := rpmmd_mock.NewRPMMDMock(fixture)
+	d := distro.New("test")
 
-	return weldr.New(rpm, repo, nil, s), s
+	return weldr.New(rpm, d, nil, s), s
 }
 
 func TestBasic(t *testing.T) {
@@ -176,8 +173,8 @@ func TestBasic(t *testing.T) {
 		{"/api/v0/projects/source/info", http.StatusNotFound, ``},
 		{"/api/v0/projects/source/info/", http.StatusNotFound, `{"errors":[{"code":404,"id":"HTTPError","msg":"Not Found"}],"status":false}`},
 		{"/api/v0/projects/source/info/foo", http.StatusOK, `{"errors":[{"id":"UnknownSource","msg":"foo is not a valid source"}],"sources":{}}`},
-		{"/api/v0/projects/source/info/test", http.StatusOK, `{"sources":{"Test":{"name":"Test","type":"yum-baseurl","url":"http://example.com/test/os","check_gpg":true,"check_ssl":true,"system":true}},"errors":[]}`},
-		{"/api/v0/projects/source/info/*", http.StatusOK, `{"sources":{"Test":{"name":"Test","type":"yum-baseurl","url":"http://example.com/test/os","check_gpg":true,"check_ssl":true,"system":true}},"errors":[]}`},
+		{"/api/v0/projects/source/info/test", http.StatusOK, `{"sources":{"test":{"name":"test","type":"yum-baseurl","url":"http://example.com/test/os","check_gpg":true,"check_ssl":true,"system":true}},"errors":[]}`},
+		{"/api/v0/projects/source/info/*", http.StatusOK, `{"sources":{"test":{"name":"test","type":"yum-baseurl","url":"http://example.com/test/os","check_gpg":true,"check_ssl":true,"system":true}},"errors":[]}`},
 
 		{"/api/v0/blueprints/list", http.StatusOK, `{"total":0,"offset":0,"limit":0,"blueprints":[]}`},
 		{"/api/v0/blueprints/info/", http.StatusNotFound, `{"errors":[{"code":404,"id":"HTTPError","msg":"Not Found"}],"status":false}`},