store/compose/ImageType: use distro.ImageType objects

This reduces the amount of resolving and error checking we have to do.
This exposed a bug in weldr's ComposeEntry type, which will be fixed in
a follow-up commit.

Signed-off-by: Tom Gundersen <teg@jklm.no>

cmd/osbuild-composer/main.go

@@ -117,7 +117,7 @@ func main() {
 		logger = log.New(os.Stdout, "", 0)
 	}
 
-	store := store.New(&stateDir)
+	store := store.New(&stateDir, arch)
 
 	queueDir := path.Join(stateDir, "jobs")
 	err = os.Mkdir(queueDir, 0700)

internal/store/compose.go

@@ -6,6 +6,7 @@ import (
 	"github.com/google/uuid"
 	"github.com/osbuild/osbuild-composer/internal/blueprint"
 	"github.com/osbuild/osbuild-composer/internal/common"
+	"github.com/osbuild/osbuild-composer/internal/distro"
 	"github.com/osbuild/osbuild-composer/internal/osbuild"
 	"github.com/osbuild/osbuild-composer/internal/target"
 )
@@ -21,7 +22,7 @@ func (ste *StateTransitionError) Error() string {
 // ImageBuild represents a single image build inside a compose
 type ImageBuild struct {
 	ID          int
-	ImageType   common.ImageType
+	ImageType   distro.ImageType
 	Manifest    *osbuild.Manifest
 	Targets     []*target.Target
 	JobCreated  time.Time

internal/store/fixtures.go

@@ -6,6 +6,7 @@ import (
 	"github.com/google/uuid"
 	"github.com/osbuild/osbuild-composer/internal/blueprint"
 	"github.com/osbuild/osbuild-composer/internal/common"
+	test_distro "github.com/osbuild/osbuild-composer/internal/distro/fedoratest"
 	"github.com/osbuild/osbuild-composer/internal/target"
 )
 
@@ -46,7 +47,16 @@ func FixtureBase() *Store {
 		},
 	}
 
-	s := New(nil)
+	d := test_distro.New()
+	arch, err := d.GetArch("x86_64")
+	if err != nil {
+		panic("invalid architecture x86_64 for fedoratest")
+	}
+	imgType, err := arch.GetImageType("qcow2")
+	if err != nil {
+		panic("invalid image type qcow2 for x86_64 @ fedoratest")
+	}
+	s := New(nil, arch)
 
 	s.blueprints[bName] = b
 	s.composes = map[uuid.UUID]Compose{
@@ -55,7 +65,7 @@ func FixtureBase() *Store {
 			ImageBuilds: []ImageBuild{
 				{
 					QueueStatus: common.IBWaiting,
-					ImageType:   common.Qcow2Generic,
+					ImageType:   imgType,
 					Targets:     []*target.Target{localTarget, awsTarget},
 					JobCreated:  date,
 				},
@@ -66,7 +76,7 @@ func FixtureBase() *Store {
 			ImageBuilds: []ImageBuild{
 				{
 					QueueStatus: common.IBRunning,
-					ImageType:   common.Qcow2Generic,
+					ImageType:   imgType,
 					Targets:     []*target.Target{localTarget},
 					JobCreated:  date,
 					JobStarted:  date,
@@ -78,7 +88,7 @@ func FixtureBase() *Store {
 			ImageBuilds: []ImageBuild{
 				{
 					QueueStatus: common.IBFinished,
-					ImageType:   common.Qcow2Generic,
+					ImageType:   imgType,
 					Targets:     []*target.Target{localTarget, awsTarget},
 					JobCreated:  date,
 					JobStarted:  date,
@@ -91,7 +101,7 @@ func FixtureBase() *Store {
 			ImageBuilds: []ImageBuild{
 				{
 					QueueStatus: common.IBFailed,
-					ImageType:   common.Qcow2Generic,
+					ImageType:   imgType,
 					Targets:     []*target.Target{localTarget, awsTarget},
 					JobCreated:  date,
 					JobStarted:  date,
@@ -115,7 +125,12 @@ func FixtureEmpty() *Store {
 		Customizations: nil,
 	}
 
-	s := New(nil)
+	d := test_distro.New()
+	arch, err := d.GetArch("x86_64")
+	if err != nil {
+		panic("invalid architecture x86_64 for fedoratest")
+	}
+	s := New(nil, arch)
 
 	s.blueprints[bName] = b
 

internal/store/json.go

@@ -8,6 +8,7 @@ import (
 	"github.com/google/uuid"
 	"github.com/osbuild/osbuild-composer/internal/blueprint"
 	"github.com/osbuild/osbuild-composer/internal/common"
+	"github.com/osbuild/osbuild-composer/internal/distro"
 	"github.com/osbuild/osbuild-composer/internal/osbuild"
 	"github.com/osbuild/osbuild-composer/internal/target"
 )
@@ -90,20 +91,28 @@ func newWorkspaceFromV0(workspaceStruct workspaceV0) map[string]blueprint.Bluepr
 	return workspace
 }
 
-func newComposesFromV0(composesStruct composesV0) map[uuid.UUID]Compose {
+func newComposesFromV0(composesStruct composesV0, arch distro.Arch) map[uuid.UUID]Compose {
 	composes := make(map[uuid.UUID]Compose)
 
 	for composeID, composeStruct := range composesStruct {
 		c := Compose{
 			Blueprint: composeStruct.Blueprint,
 		}
-		if len(composeStruct.ImageBuilds) == 0 {
-			panic("the was a compose with zero image builds, that is forbidden")
-		}
 		for _, imgBuild := range composeStruct.ImageBuilds {
+			imgTypeString, valid := imgBuild.ImageType.ToCompatString()
+			if !valid {
+				// Invalid type strings in serialization format, this may happen
+				// on upgrades. Ignore the image build.
+				continue
+			}
+			imgType, err := arch.GetImageType(imgTypeString)
+			if err != nil {
+				// Ditto.
+				continue
+			}
 			ib := ImageBuild{
 				ID:          imgBuild.ID,
-				ImageType:   imgBuild.ImageType,
+				ImageType:   imgType,
 				Manifest:    imgBuild.Manifest,
 				Targets:     imgBuild.Targets,
 				JobCreated:  imgBuild.JobCreated,
@@ -115,6 +124,10 @@ func newComposesFromV0(composesStruct composesV0) map[uuid.UUID]Compose {
 			}
 			c.ImageBuilds = append(c.ImageBuilds, ib)
 		}
+		if len(c.ImageBuilds) == 0 {
+			// In case no valid image builds were found, ignore the compose.
+			continue
+		}
 		composes[composeID] = c
 	}
 
@@ -158,11 +171,11 @@ func newCommitsFromV0(commitsStruct commitsV0) map[string][]string {
 	return commits
 }
 
-func newStoreFromV0(storeStruct storeV0) *Store {
+func newStoreFromV0(storeStruct storeV0, arch distro.Arch) *Store {
 	store := Store{
 		blueprints:        newBlueprintsFromV0(storeStruct.Blueprints),
 		workspace:         newWorkspaceFromV0(storeStruct.Workspace),
-		composes:          newComposesFromV0(storeStruct.Composes),
+		composes:          newComposesFromV0(storeStruct.Composes, arch),
 		sources:           newSourceConfigsFromV0(storeStruct.Sources),
 		blueprintsChanges: newChangesFromV0(storeStruct.Changes),
 		blueprintsCommits: newCommitsFromV0(storeStruct.Commits),
@@ -238,9 +251,13 @@ func newComposesV0(composes map[uuid.UUID]Compose) composesV0 {
 			panic("the was a compose with zero image builds, that is forbidden")
 		}
 		for _, imgBuild := range compose.ImageBuilds {
+			imgType, valid := common.ImageTypeFromCompatString(imgBuild.ImageType.Name())
+			if !valid {
+				panic("invalid image type")
+			}
 			ib := imageBuildV0{
 				ID:          imgBuild.ID,
-				ImageType:   imgBuild.ImageType,
+				ImageType:   imgType,
 				Manifest:    imgBuild.Manifest,
 				Targets:     imgBuild.Targets,
 				JobCreated:  imgBuild.JobCreated,

internal/store/store.go

@@ -73,7 +73,7 @@ func (e *NoLocalTargetError) Error() string {
 	return e.message
 }
 
-func New(stateDir *string) *Store {
+func New(stateDir *string, arch distro.Arch) *Store {
 	var storeStruct storeV0
 	var db *jsondb.JSONDatabase
 
@@ -90,7 +90,7 @@ func New(stateDir *string) *Store {
 		}
 	}
 
-	store := newStoreFromV0(storeStruct)
+	store := newStoreFromV0(storeStruct, arch)
 
 	store.stateDir = stateDir
 	store.db = db
@@ -397,12 +397,6 @@ func (s *Store) PushCompose(composeID uuid.UUID, manifest *osbuild.Manifest, ima
 		targets = []*target.Target{}
 	}
 
-	// Compatibility layer for image types in Weldr API v0
-	imageTypeCommon, exists := common.ImageTypeFromCompatString(imageType.Name())
-	if !exists {
-		panic("fatal error, compose type does not exist")
-	}
-
 	if s.stateDir != nil {
 		outputDir := s.getImageBuildDirectory(composeID, 0)
 
@@ -419,7 +413,7 @@ func (s *Store) PushCompose(composeID uuid.UUID, manifest *osbuild.Manifest, ima
 			ImageBuilds: []ImageBuild{
 				{
 					Manifest:   manifest,
-					ImageType:  imageTypeCommon,
+					ImageType:  imageType,
 					Targets:    targets,
 					JobCreated: time.Now(),
 					Size:       size,
@@ -440,12 +434,6 @@ func (s *Store) PushTestCompose(composeID uuid.UUID, manifest *osbuild.Manifest,
 		targets = []*target.Target{}
 	}
 
-	// Compatibility layer for image types in Weldr API v0
-	imageTypeCommon, exists := common.ImageTypeFromCompatString(imageType.Name())
-	if !exists {
-		panic("fatal error, compose type does not exist")
-	}
-
 	if s.stateDir != nil {
 		outputDir := s.getImageBuildDirectory(composeID, 0)
 
@@ -479,7 +467,7 @@ func (s *Store) PushTestCompose(composeID uuid.UUID, manifest *osbuild.Manifest,
 				{
 					QueueStatus: status,
 					Manifest:    manifest,
-					ImageType:   imageTypeCommon,
+					ImageType:   imageType,
 					Targets:     targets,
 					JobCreated:  time.Now(),
 					JobStarted:  time.Now(),

internal/store/store_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/stretchr/testify/suite"
 
 	"github.com/osbuild/osbuild-composer/internal/blueprint"
+	"github.com/osbuild/osbuild-composer/internal/distro/test_distro"
 )
 
 //struct for sharing state between tests
@@ -50,8 +51,11 @@ func (suite *storeTest) SetupSuite() {
 func (suite *storeTest) SetupTest() {
 	tmpDir, err := ioutil.TempDir("/tmp", "osbuild-composer-test-")
 	suite.NoError(err)
+	distro := test_distro.New()
+	arch, err := distro.GetArch("test_arch")
+	suite.NoError(err)
 	suite.dir = tmpDir
-	suite.myStore = New(&suite.dir)
+	suite.myStore = New(&suite.dir, arch)
 }
 
 //teardown after each test

internal/weldr/api.go

@@ -1800,7 +1800,7 @@ func (api *API) composeStatusHandler(writer http.ResponseWriter, request *http.R
 	}
 
 	filterStatus := q.Get("status")
-	filterImageType, filterImageTypeExists := common.ImageTypeFromCompatString(q.Get("type"))
+	filterImageType := q.Get("type")
 
 	filteredUUIDs := []uuid.UUID{}
 	for _, id := range uuids {
@@ -1813,7 +1813,7 @@ func (api *API) composeStatusHandler(writer http.ResponseWriter, request *http.R
 			continue
 		} else if filterStatus != "" && state.ToString() != filterStatus {
 			continue
-		} else if filterImageTypeExists && compose.ImageBuilds[0].ImageType != filterImageType {
+		} else if filterImageType != "" && compose.ImageBuilds[0].ImageType.Name() != filterImageType {
 			continue
 		}
 		filteredUUIDs = append(filteredUUIDs, id)
@@ -1884,7 +1884,7 @@ func (api *API) composeInfoHandler(writer http.ResponseWriter, request *http.Req
 	// Weldr API assumes only one image build per compose, that's why only the
 	// 1st build is considered
 	state, _, _, _ := api.getComposeState(compose)
-	reply.ComposeType, _ = compose.ImageBuilds[0].ImageType.ToCompatString()
+	reply.ComposeType = compose.ImageBuilds[0].ImageType.Name()
 	reply.QueueStatus = state.ToString()
 	reply.ImageSize = compose.ImageBuilds[0].Size
 
@@ -1932,20 +1932,8 @@ func (api *API) composeImageHandler(writer http.ResponseWriter, request *http.Re
 		return
 	}
 
-	imageBuild := compose.ImageBuilds[0]
+	imageName := compose.ImageBuilds[0].ImageType.Filename()
-	imageType, _ := imageBuild.ImageType.ToCompatString()
+	imageMime := compose.ImageBuilds[0].ImageType.MIMEType()
-	imageTypeStruct, err := api.arch.GetImageType(imageType)
-	if err != nil {
-		errors := responseError{
-			ID: "BadCompose",
-			Msg: fmt.Sprintf("Compose %s is ill-formed: output type %v is invalid for distro %s on %s",
-				uuidString, imageBuild.ImageType, api.distro.Name(), api.arch.Name()),
-		}
-		statusResponseError(writer, http.StatusInternalServerError, errors)
-		return
-	}
-	imageName := imageTypeStruct.Filename()
-	imageMime := imageTypeStruct.MIMEType()
 
 	reader, fileSize, err := api.store.GetImageBuildImage(uuid, 0)
 

internal/weldr/api_test.go

@@ -433,6 +433,10 @@ func TestBlueprintsDepsolve(t *testing.T) {
 }
 
 func TestCompose(t *testing.T) {
+	arch, err := test_distro.New().GetArch("x86_64")
+	require.NoError(t, err)
+	imgType, err := arch.GetImageType("qcow2")
+	require.NoError(t, err)
 	expectedComposeLocal := &store.Compose{
 		Blueprint: &blueprint.Blueprint{
 			Name:           "test",
@@ -445,7 +449,7 @@ func TestCompose(t *testing.T) {
 		ImageBuilds: []store.ImageBuild{
 			{
 				QueueStatus: common.IBWaiting,
-				ImageType:   common.Qcow2Generic,
+				ImageType:   imgType,
 				Targets: []*target.Target{
 					{
 						// skip Uuid and Created fields - they are ignored
@@ -470,7 +474,7 @@ func TestCompose(t *testing.T) {
 		ImageBuilds: []store.ImageBuild{
 			{
 				QueueStatus: common.IBWaiting,
-				ImageType:   common.Qcow2Generic,
+				ImageType:   imgType,
 				Targets: []*target.Target{
 					{
 						Name:      "org.osbuild.aws",
@@ -535,7 +539,7 @@ func TestCompose(t *testing.T) {
 		// TODO: find some (reasonable) way to verify the contents of the pipeline
 		composeStruct.ImageBuilds[0].Manifest = nil
 
-		if diff := cmp.Diff(composeStruct, *c.ExpectedCompose, test.IgnoreDates(), test.IgnoreUuids(), test.Ignore("Targets.Options.Location")); diff != "" {
+		if diff := cmp.Diff(composeStruct, *c.ExpectedCompose, test.IgnoreDates(), test.IgnoreUuids(), test.Ignore("Targets.Options.Location"), test.CompareImageTypes()); diff != "" {
 			t.Errorf("%s: compose in store isn't the same as expected, diff:\n%s", c.Path, diff)
 		}
 

internal/weldr/compose.go

@@ -28,7 +28,13 @@ func composeToComposeEntry(id uuid.UUID, compose store.Compose, state common.Com
 	composeEntry.ID = id
 	composeEntry.Blueprint = compose.Blueprint.Name
 	composeEntry.Version = compose.Blueprint.Version
-	composeEntry.ComposeType = compose.ImageBuilds[0].ImageType
+
+	var valid bool
+	// BUG: this leads to the wrong image type string being returned, we want the compat strings here
+	composeEntry.ComposeType, valid = common.ImageTypeFromCompatString(compose.ImageBuilds[0].ImageType.Name())
+	if !valid {
+		panic("compose contains invalid image type: " + compose.ImageBuilds[0].ImageType.Name())
+	}
 
 	if includeUploads {
 		composeEntry.Uploads = targetsToUploadResponses(compose.ImageBuilds[0].Targets)