From 85e6182bdcbae7f3ddff59969171ec1ae99ea029 Mon Sep 17 00:00:00 2001 From: Lars Karlitski Date: Thu, 28 Nov 2019 19:49:45 +0100 Subject: [PATCH] distro: don't fall back to fedora-30 Make osbuild-composer use FromHost() directly. Everywhere else needs to specify the distro explicitly. Also don't panic when a distro doesn't exist. Instead, return nil. Make sure all callers check for that. --- cmd/osbuild-composer/main.go | 8 ++++++-- cmd/osbuild-pipeline/main.go | 4 ++++ internal/distro/distro.go | 21 +++++---------------- internal/jobqueue/api_test.go | 7 ++++--- internal/mocks/rpmmd/fixtures.go | 13 ++++++++----- internal/store/store.go | 11 ++++++----- internal/weldr/api.go | 3 +-- 7 files changed, 34 insertions(+), 33 deletions(-) diff --git a/cmd/osbuild-composer/main.go b/cmd/osbuild-composer/main.go index 49123aebd..37aae7909 100644 --- a/cmd/osbuild-composer/main.go +++ b/cmd/osbuild-composer/main.go @@ -34,14 +34,18 @@ func main() { jobListener := listeners[1] rpm := rpmmd.NewRPMMD() - distribution := distro.New("") + + distribution, err := distro.FromHost() + if err != nil { + panic("cannot detect distro from host: " + err.Error()) + } var logger *log.Logger if verbose { logger = log.New(os.Stdout, "", 0) } - store := store.New(&stateFile) + store := store.New(&stateFile, distribution) jobAPI := jobqueue.New(logger, store) weldrAPI := weldr.New(rpm, distribution, logger, store) diff --git a/cmd/osbuild-pipeline/main.go b/cmd/osbuild-pipeline/main.go index f7c893d80..ea108a859 100644 --- a/cmd/osbuild-pipeline/main.go +++ b/cmd/osbuild-pipeline/main.go @@ -32,6 +32,10 @@ func main() { } d := distro.New(distroArg) + if d == nil { + panic("unknown distro: " + distroArg) + } + pipeline, err := d.Pipeline(blueprint, format) if err != nil { panic(err.Error()) diff --git a/internal/distro/distro.go b/internal/distro/distro.go index f0834697a..6e10ebbea 100644 --- a/internal/distro/distro.go +++ b/internal/distro/distro.go @@ -4,7 +4,6 @@ import ( "bufio" "errors" "io" - "log" "os" "strings" @@ -44,20 +43,9 @@ func init() { } func New(name string) Distro { - if name == "" { - distro, err := FromHost() - if err == nil { - return distro - } else { - log.Println("cannot detect distro from host: " + err.Error()) - log.Println("falling back to 'fedora-30'") - return New("fedora-30") - } - } - distro, ok := registered[name] if !ok { - panic("unknown distro: " + name) + return nil } return distro @@ -77,11 +65,12 @@ func FromHost() (Distro, error) { name := osrelease["ID"] + "-" + osrelease["VERSION_ID"] - distro, ok := registered[name] - if !ok { + d := New(name) + if d == nil { return nil, errors.New("unknown distro: " + name) } - return distro, nil + + return d, nil } func Register(name string, distro Distro) { diff --git a/internal/jobqueue/api_test.go b/internal/jobqueue/api_test.go index edead4827..b74f767e0 100644 --- a/internal/jobqueue/api_test.go +++ b/internal/jobqueue/api_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/osbuild/osbuild-composer/internal/blueprint" + "github.com/osbuild/osbuild-composer/internal/distro" "github.com/osbuild/osbuild-composer/internal/jobqueue" "github.com/osbuild/osbuild-composer/internal/store" "github.com/osbuild/osbuild-composer/internal/test" @@ -31,7 +32,7 @@ func TestBasic(t *testing.T) { } for _, c := range cases { - api := jobqueue.New(nil, store.New(nil)) + api := jobqueue.New(nil, store.New(nil, distro.New("fedora-30"))) test.TestRoute(t, api, false, c.Method, c.Path, c.Body, c.ExpectedStatus, c.ExpectedJSON) } @@ -39,7 +40,7 @@ func TestBasic(t *testing.T) { func TestCreate(t *testing.T) { id, _ := uuid.Parse("ffffffff-ffff-ffff-ffff-ffffffffffff") - store := store.New(nil) + store := store.New(nil, distro.New("fedora-30")) api := jobqueue.New(nil, store) err := store.PushCompose(id, &blueprint.Blueprint{}, "tar") @@ -53,7 +54,7 @@ func TestCreate(t *testing.T) { func testUpdateTransition(t *testing.T, from, to string, expectedStatus int) { id, _ := uuid.Parse("ffffffff-ffff-ffff-ffff-ffffffffffff") - store := store.New(nil) + store := store.New(nil, distro.New("fedora-30")) api := jobqueue.New(nil, store) if from != "VOID" { diff --git a/internal/mocks/rpmmd/fixtures.go b/internal/mocks/rpmmd/fixtures.go index 8e6912558..65c4662ff 100644 --- a/internal/mocks/rpmmd/fixtures.go +++ b/internal/mocks/rpmmd/fixtures.go @@ -2,12 +2,14 @@ package rpmmd_mock import ( "fmt" - "github.com/google/uuid" - "github.com/osbuild/osbuild-composer/internal/blueprint" - "github.com/osbuild/osbuild-composer/internal/rpmmd" - "github.com/osbuild/osbuild-composer/internal/store" "sort" "time" + + "github.com/google/uuid" + "github.com/osbuild/osbuild-composer/internal/blueprint" + "github.com/osbuild/osbuild-composer/internal/distro" + "github.com/osbuild/osbuild-composer/internal/rpmmd" + "github.com/osbuild/osbuild-composer/internal/store" ) type FixtureGenerator func() Fixture @@ -56,7 +58,8 @@ func createBaseStoreFixture() *store.Store { var date = time.Date(2019, 11, 27, 13, 19, 0, 0, time.FixedZone("UTC+1", 60*60)) - s := store.New(nil) + d := distro.New("fedora-30") + s := store.New(nil, d) s.Blueprints[bName] = b s.Composes = map[uuid.UUID]store.Compose{ diff --git a/internal/store/store.go b/internal/store/store.go index 28f77d023..5950b835f 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -35,6 +35,7 @@ type Store struct { mu sync.RWMutex // protects all fields pendingJobs chan Job stateChannel chan []byte + distro distro.Distro } // A Compose represent the task of building one image. It contains all the information @@ -105,7 +106,7 @@ func (e *InvalidRequestError) Error() string { return e.message } -func New(stateFile *string) *Store { +func New(stateFile *string, distro distro.Distro) *Store { var s Store if stateFile != nil { @@ -149,6 +150,8 @@ func New(stateFile *string) *Store { } s.pendingJobs = make(chan Job, 200) + s.distro = distro + return &s } @@ -448,8 +451,7 @@ func (s *Store) PushCompose(composeID uuid.UUID, bp *blueprint.Blueprint, compos }, ), } - d := distro.New("") - pipeline, err := d.Pipeline(bp, composeType) + pipeline, err := s.distro.Pipeline(bp, composeType) if err != nil { return err } @@ -527,8 +529,7 @@ func (s *Store) GetImage(composeID uuid.UUID) (*Image, error) { if compose.QueueStatus != "FINISHED" { return nil, &InvalidRequestError{"compose was not finished"} } - d := distro.New("") - name, mime, err := d.FilenameFromType(compose.OutputType) + name, mime, err := s.distro.FilenameFromType(compose.OutputType) if err != nil { panic("invalid output type") } diff --git a/internal/weldr/api.go b/internal/weldr/api.go index 232ca780c..1a9b84865 100644 --- a/internal/weldr/api.go +++ b/internal/weldr/api.go @@ -1195,8 +1195,7 @@ func (api *API) composeTypesHandler(writer http.ResponseWriter, request *http.Re Types []composeType `json:"types"` } - d := distro.New("") - for _, format := range d.ListOutputFormats() { + for _, format := range api.distro.ListOutputFormats() { reply.Types = append(reply.Types, composeType{format, true}) }