Repository: osbuild/osbuild-composer
Pull request #233 (2020-02-14 09:21:36)

fix ineffectual assignments found by golangci-lint
I think splitting this into individual commits doesn't add much value as these are very simple fixes. However, I don't have a strong opinion about it.


Patch:
diff --git a/cmd/osbuild-worker/main.go b/cmd/osbuild-worker/main.go
index bfaf03c04..ebadd1514 100644
--- a/cmd/osbuild-worker/main.go
+++ b/cmd/osbuild-worker/main.go
@@ -6,7 +6,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"github.com/osbuild/osbuild-composer/internal/compose"
+	"io"
 	"log"
 	"net"
 	"net/http"
@@ -55,10 +55,11 @@ func (c *ComposerClient) AddJob() (*jobqueue.Job, error) {
 	return job, nil
 }
 
-func (c *ComposerClient) UpdateJob(job *jobqueue.Job, status common.ImageBuildState, image *compose.Image, result *common.ComposeResult) error {
+func (c *ComposerClient) UpdateJob(job *jobqueue.Job, status common.ImageBuildState, result *common.ComposeResult) error {
 	var b bytes.Buffer
-	json.NewEncoder(&b).Encode(&jobqueue.JobStatus{status, job.ImageBuildID, image, result})
-	req, err := http.NewRequest("PATCH", "http://localhost/job-queue/v1/jobs/"+job.ID.String(), &b)
+	json.NewEncoder(&b).Encode(&jobqueue.JobStatus{status, result})
+	url := fmt.Sprintf("http://localhost/job-queue/v1/jobs/%s/builds/%d", job.ID.String(), job.ImageBuildID)
+	req, err := http.NewRequest("PATCH", url, &b)
 	if err != nil {
 		return err
 	}
@@ -77,6 +78,14 @@ func (c *ComposerClient) UpdateJob(job *jobqueue.Job, status common.ImageBuildSt
 	return nil
 }
 
+func (c *ComposerClient) UploadImage(job *jobqueue.Job, reader io.Reader) error {
+	// content type doesn't really matter
+	url := fmt.Sprintf("http://localhost/job-queue/v1/jobs/%s/builds/%d/image", job.ID.String(), job.ImageBuildID)
+	_, err := c.client.Post(url, "application/octet-stream", reader)
+
+	return err
+}
+
 func handleJob(client *ComposerClient) error {
 	fmt.Println("Waiting for a new job...")
 	job, err := client.AddJob()
@@ -84,19 +93,19 @@ func handleJob(client *ComposerClient) error {
 		return err
 	}
 
-	err = client.UpdateJob(job, common.IBRunning, nil, nil)
+	err = client.UpdateJob(job, common.IBRunning, nil)
 	if err != nil {
 		return err
 	}
 
 	fmt.Printf("Running job %s\n", job.ID.String())
-	image, result, err := job.Run()
+	result, err := job.Run(client)
 	if err != nil {
 		log.Printf("  Job failed: %v", err)
-		return client.UpdateJob(job, common.IBFailed, nil, result)
+		return client.UpdateJob(job, common.IBFailed, result)
 	}
 
-	return client.UpdateJob(job, common.IBFinished, image, result)
+	return client.UpdateJob(job, common.IBFinished, result)
 }
 
 func main() {
diff --git a/internal/compose/compose.go b/internal/compose/compose.go
index 232f7fa92..d08c56b0a 100644
--- a/internal/compose/compose.go
+++ b/internal/compose/compose.go
@@ -71,6 +71,16 @@ func (ib *ImageBuild) DeepCopy() ImageBuild {
 	}
 }
 
+func (ib *ImageBuild) HasLocalTarget() bool {
+	for _, t := range ib.Targets {
+		if _, ok := t.Options.(*target.LocalTargetOptions); ok {
+			return true
+		}
+	}
+
+	return false
+}
+
 // A Compose represent the task of building a set of images from a single blueprint.
 // It contains all the information necessary to generate the inputs for the job, as
 // well as the job's state.
diff --git a/internal/distro/fedora30/distro.go b/internal/distro/fedora30/distro.go
index d72e26b61..062d4b878 100644
--- a/internal/distro/fedora30/distro.go
+++ b/internal/distro/fedora30/distro.go
@@ -549,7 +549,7 @@ func (r *Fedora30) firewallStageOptions(firewall *blueprint.FirewallCustomizatio
 func (r *Fedora30) systemdStageOptions(enabledServices, disabledServices []string, s *blueprint.ServicesCustomization) *pipeline.SystemdStageOptions {
 	if s != nil {
 		enabledServices = append(enabledServices, s.Enabled...)
-		enabledServices = append(disabledServices, s.Disabled...)
+		disabledServices = append(disabledServices, s.Disabled...)
 	}
 	return &pipeline.SystemdStageOptions{
 		EnabledServices:  enabledServices,
diff --git a/internal/distro/rhel82/distro.go b/internal/distro/rhel82/distro.go
index 031b05551..77662ba68 100644
--- a/internal/distro/rhel82/distro.go
+++ b/internal/distro/rhel82/distro.go
@@ -441,7 +441,6 @@ func (r *RHEL82) Name() string {
 	return name
 }
 
-
 func (r *RHEL82) Distribution() common.Distribution {
 	return Distro
 }
@@ -692,7 +691,7 @@ func (r *RHEL82) firewallStageOptions(firewall *blueprint.FirewallCustomization)
 func (r *RHEL82) systemdStageOptions(enabledServices, disabledServices []string, s *blueprint.ServicesCustomization, target string) *pipeline.SystemdStageOptions {
 	if s != nil {
 		enabledServices = append(enabledServices, s.Enabled...)
-		enabledServices = append(disabledServices, s.Disabled...)
+		disabledServices = append(disabledServices, s.Disabled...)
 	}
 	return &pipeline.SystemdStageOptions{
 		EnabledServices:  enabledServices,
diff --git a/internal/jobqueue/api.go b/internal/jobqueue/api.go
index faa386e31..de3b001fd 100644
--- a/internal/jobqueue/api.go
+++ b/internal/jobqueue/api.go
@@ -5,6 +5,7 @@ import (
 	"log"
 	"net"
 	"net/http"
+	"strconv"
 
 	"github.com/osbuild/osbuild-composer/internal/store"
 
@@ -31,7 +32,8 @@ func New(logger *log.Logger, store *store.Store) *API {
 	api.router.NotFound = http.HandlerFunc(notFoundHandler)
 
 	api.router.POST("/job-queue/v1/jobs", api.addJobHandler)
-	api.router.PATCH("/job-queue/v1/jobs/:id", api.updateJobHandler)
+	api.router.PATCH("/job-queue/v1/jobs/:job_id/builds/:build_id", api.updateJobHandler)
+	api.router.POST("/job-queue/v1/jobs/:job_id/builds/:build_id/image", api.addJobImageHandler)
 
 	return api
 }
@@ -70,6 +72,10 @@ func statusResponseOK(writer http.ResponseWriter) {
 
 func statusResponseError(writer http.ResponseWriter, code int, errors ...string) {
 	writer.WriteHeader(code)
+
+	for _, err := range errors {
+		writer.Write([]byte(err))
+	}
 }
 
 func (api *API) addJobHandler(writer http.ResponseWriter, request *http.Request, _ httprouter.Params) {
@@ -94,12 +100,12 @@ func (api *API) addJobHandler(writer http.ResponseWriter, request *http.Request,
 	writer.WriteHeader(http.StatusCreated)
 	// FIXME: handle or comment this possible error
 	_ = json.NewEncoder(writer).Encode(Job{
-		ID: nextJob.ComposeID,
+		ID:           nextJob.ComposeID,
 		ImageBuildID: nextJob.ImageBuildID,
-		Distro: nextJob.Distro,
-		Pipeline: nextJob.Pipeline,
-		Targets: nextJob.Targets,
-		OutputType: nextJob.ImageType,
+		Distro:       nextJob.Distro,
+		Pipeline:     nextJob.Pipeline,
+		Targets:      nextJob.Targets,
+		OutputType:   nextJob.ImageType,
 	})
 }
 
@@ -110,12 +116,19 @@ func (api *API) updateJobHandler(writer http.ResponseWriter, request *http.Reque
 		return
 	}
 
-	id, err := uuid.Parse(params.ByName("id"))
+	id, err := uuid.Parse(params.ByName("job_id"))
 	if err != nil {
 		statusResponseError(writer, http.StatusBadRequest, "invalid compose id: "+err.Error())
 		return
 	}
 
+	imageBuildId, err := strconv.Atoi(params.ByName("build_id"))
+
+	if err != nil {
+		statusResponseError(writer, http.StatusBadRequest, "invalid image build id: "+err.Error())
+		return
+	}
+
 	var body JobStatus
 	err = json.NewDecoder(request.Body).Decode(&body)
 	if err != nil {
@@ -123,7 +136,7 @@ func (api *API) updateJobHandler(writer http.ResponseWriter, request *http.Reque
 		return
 	}
 
-	err = api.store.UpdateImageBuildInCompose(id, body.ImageBuildID, body.Status, body.Image, body.Result)
+	err = api.store.UpdateImageBuildInCompose(id, imageBuildId, body.Status, body.Result)
 	if err != nil {
 		switch err.(type) {
 		case *store.NotFoundError:
@@ -141,3 +154,34 @@ func (api *API) updateJobHandler(writer http.ResponseWriter, request *http.Reque
 	}
 	statusResponseOK(writer)
 }
+
+func (api *API) addJobImageHandler(writer http.ResponseWriter, request *http.Request, params httprouter.Params) {
+	id, err := uuid.Parse(params.ByName("job_id"))
+	if err != nil {
+		statusResponseError(writer, http.StatusBadRequest, "invalid compose id: "+err.Error())
+		return
+	}
+
+	imageBuildId, err := strconv.Atoi(params.ByName("build_id"))
+
+	if err != nil {
+		statusResponseError(writer, http.StatusBadRequest, "invalid build id: "+err.Error())
+		return
+	}
+
+	err = api.store.AddImageToImageUpload(id, imageBuildId, request.Body)
+
+	if err != nil {
+		switch err.(type) {
+		case *store.NotFoundError:
+			statusResponseError(writer, http.StatusNotFound, err.Error())
+		case *store.NoLocalTargetError:
+			statusResponseError(writer, http.StatusBadRequest, err.Error())
+		default:
+			statusResponseError(writer, http.StatusInternalServerError, err.Error())
+		}
+		return
+	}
+
+	statusResponseOK(writer)
+}
diff --git a/internal/jobqueue/job.go b/internal/jobqueue/job.go
index 5d6911887..03fe78fe3 100644
--- a/internal/jobqueue/job.go
+++ b/internal/jobqueue/job.go
@@ -3,7 +3,6 @@ package jobqueue
 import (
 	"encoding/json"
 	"fmt"
-	"github.com/osbuild/osbuild-composer/internal/compose"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -27,10 +26,8 @@ type Job struct {
 }
 
 type JobStatus struct {
-	Status       common.ImageBuildState `json:"status"`
-	ImageBuildID int                    `json:"image_build_id"`
-	Image        *compose.Image         `json:"image"`
-	Result       *common.ComposeResult  `json:"result"`
+	Status common.ImageBuildState `json:"status"`
+	Result *common.ComposeResult  `json:"result"`
 }
 
 type TargetsError struct {
@@ -47,11 +44,11 @@ func (e *TargetsError) Error() string {
 	return errString
 }
 
-func (job *Job) Run() (*compose.Image, *common.ComposeResult, error) {
+func (job *Job) Run(uploader LocalTargetUploader) (*common.ComposeResult, error) {
 	distros := distro.NewRegistry([]string{"/etc/osbuild-composer", "/usr/share/osbuild-composer"})
 	d := distros.GetDistro(job.Distro)
 	if d == nil {
-		return nil, nil, fmt.Errorf("unknown distro: %s", job.Distro)
+		return nil, fmt.Errorf("unknown distro: %s", job.Distro)
 	}
 
 	build := pipeline.Build{
@@ -60,19 +57,19 @@ func (job *Job) Run() (*compose.Image, *common.ComposeResult, error) {
 
 	buildFile, err := ioutil.TempFile("", "osbuild-worker-build-env-*")
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 	// FIXME: how to handle errors in defer?
 	defer os.Remove(buildFile.Name())
 
 	err = json.NewEncoder(buildFile).Encode(build)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error encoding build environment: %v", err)
+		return nil, fmt.Errorf("error encoding build environment: %v", err)
 	}
 
 	tmpStore, err := ioutil.TempDir("/var/tmp", "osbuild-store")
 	if err != nil {
-		return nil, nil, fmt.Errorf("error setting up osbuild store: %v", err)
+		return nil, fmt.Errorf("error setting up osbuild store: %v", err)
 	}
 	// FIXME: how to handle errors in defer?
 	defer os.RemoveAll(tmpStore)
@@ -87,22 +84,22 @@ func (job *Job) Run() (*compose.Image, *common.ComposeResult, error) {
 
 	stdin, err := cmd.StdinPipe()
 	if err != nil {
-		return nil, nil, fmt.Errorf("error setting up stdin for osbuild: %v", err)
+		return nil, fmt.Errorf("error setting up stdin for osbuild: %v", err)
 	}
 
 	stdout, err := cmd.StdoutPipe()
 	if err != nil {
-		return nil, nil, fmt.Errorf("error setting up stdout for osbuild: %v", err)
+		return nil, fmt.Errorf("error setting up stdout for osbuild: %v", err)
 	}
 
 	err = cmd.Start()
 	if err != nil {
-		return nil, nil, fmt.Errorf("error starting osbuild: %v", err)
+		return nil, fmt.Errorf("error starting osbuild: %v", err)
 	}
 
 	err = json.NewEncoder(stdin).Encode(job.Pipeline)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error encoding osbuild pipeline: %v", err)
+		return nil, fmt.Errorf("error encoding osbuild pipeline: %v", err)
 	}
 	// FIXME: handle or comment this possible error
 	_ = stdin.Close()
@@ -110,57 +107,37 @@ func (job *Job) Run() (*compose.Image, *common.ComposeResult, error) {
 	var result common.ComposeResult
 	err = json.NewDecoder(stdout).Decode(&result)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error decoding osbuild output: %#v", err)
+		return nil, fmt.Errorf("error decoding osbuild output: %#v", err)
 	}
 
 	err = cmd.Wait()
 	if err != nil {
-		return nil, &result, err
+		return &result, err
 	}
 
-	filename, mimeType, err := d.FilenameFromType(job.OutputType)
-	if err != nil {
-		return nil, &result, fmt.Errorf("cannot fetch information about output type %s: %v", job.OutputType, err)
-	}
-
-	var image compose.Image
-
 	var r []error
 
 	for _, t := range job.Targets {
 		switch options := t.Options.(type) {
 		case *target.LocalTargetOptions:
-
-			err = runCommand("cp", "-a", "-L", tmpStore+"/refs/"+result.OutputID+"/.", options.Location)
+			filename, _, err := d.FilenameFromType(job.OutputType)
 			if err != nil {
 				r = append(r, err)
 				continue
 			}
 
-			err = runCommand("chown", "-R", "_osbuild-composer:_osbuild-composer", options.Location)
+			f, err := os.Open(tmpStore + "/refs/" + result.OutputID + "/" + filename)
 			if err != nil {
 				r = append(r, err)
 				continue
 			}
 
-			imagePath := options.Location + "/" + filename
-			file, err := os.Open(imagePath)
+			err = uploader.UploadImage(job, f)
 			if err != nil {
 				r = append(r, err)
 				continue
 			}
 
-			fileStat, err := file.Stat()
-			if err != nil {
-				return nil, &result, err
-			}
-
-			image = compose.Image{
-				Path: imagePath,
-				Mime: mimeType,
-				Size: uint64(fileStat.Size()),
-			}
-
 		case *target.AWSTargetOptions:
 
 			a, err := awsupload.New(options.Region, options.AccessKeyID, options.SecretAccessKey)
@@ -192,10 +169,10 @@ func (job *Job) Run() (*compose.Image, *common.ComposeResult, error) {
 	}
 
 	if len(r) > 0 {
-		return nil, &result, &TargetsError{r}
+		return &result, &TargetsError{r}
 	}
 
-	return &image, &result, nil
+	return &result, nil
 }
 
 func runCommand(command string, params ...string) error {
diff --git a/internal/jobqueue/local_target_uploader.go b/internal/jobqueue/local_target_uploader.go
new file mode 100644
index 000000000..292b34733
--- /dev/null
+++ b/internal/jobqueue/local_target_uploader.go
@@ -0,0 +1,7 @@
+package jobqueue
+
+import "io"
+
+type LocalTargetUploader interface {
+	UploadImage(job *Job, reader io.Reader) error
+}
diff --git a/internal/mocks/rpmmd/fixtures.go b/internal/mocks/rpmmd/fixtures.go
index 11976cccd..60f7b07cb 100644
--- a/internal/mocks/rpmmd/fixtures.go
+++ b/internal/mocks/rpmmd/fixtures.go
@@ -75,9 +75,7 @@ func createBaseStoreFixture() *store.Store {
 		ImageName: "localimage",
 		Created:   date,
 		Status:    common.IBWaiting,
-		Options: &target.LocalTargetOptions{
-			Location: "/tmp/localimage",
-		},
+		Options:   &target.LocalTargetOptions{},
 	}
 
 	var awsTarget = &target.Target{
diff --git a/internal/store/store.go b/internal/store/store.go
index 2408e22c5..4ef5b67fa 100644
--- a/internal/store/store.go
+++ b/internal/store/store.go
@@ -100,6 +100,14 @@ func (e *InvalidRequestError) Error() string {
 	return e.message
 }
 
+type NoLocalTargetError struct {
+	message string
+}
+
+func (e *NoLocalTargetError) Error() string {
+	return e.message
+}
+
 func New(stateDir *string, distroArg distro.Distro, distroRegistryArg distro.Registry) *Store {
 	var s Store
 
@@ -445,18 +453,56 @@ func (s *Store) GetAllComposes() map[uuid.UUID]compose.Compose {
 	return composes
 }
 
-func (s *Store) GetComposeResult(id uuid.UUID) (io.ReadCloser, error) {
+func (s *Store) GetImageBuildResult(composeId uuid.UUID, imageBuildId int) (io.ReadCloser, error) {
 	if s.stateDir == nil {
 		return ioutil.NopCloser(bytes.NewBuffer([]byte("{}"))), nil
 	}
-	return os.Open(*s.stateDir + "/outputs/" + id.String() + "/result.json")
+
+	return os.Open(s.getImageBuildDirectory(composeId, imageBuildId) + "/result.json")
+}
+
+func (s *Store) GetImageBuildImage(composeId uuid.UUID, imageBuildId int) (io.ReadCloser, int64, error) {
+	c, ok := s.Composes[composeId]
+
+	if !ok {
+		return nil, 0, &NotFoundError{"compose does not exist"}
+	}
+
+	imageBuild := c.ImageBuilds[imageBuildId]
+	if !imageBuild.HasLocalTarget() {
+		return nil, 0, &NoLocalTargetError{"compose does not have local target"}
+	}
+
+	compatString, _ := imageBuild.ImageType.ToCompatString()
+	filename, _, err := s.distro.FilenameFromType(compatString)
+	if err != nil {
+		panic(err)
+	}
+
+	path := fmt.Sprintf("%s/%s", s.getImageBuildDirectory(composeId, imageBuildId), filename)
+
+	f, err := os.Open(path)
+
+	if err != nil {
+		return nil, 0, err
+	}
+
+	fileInfo, err := f.Stat()
+
+	if err != nil {
+		return nil, 0, err
+	}
+
+	return f, fileInfo.Size(), err
+
+}
+
+func (s *Store) getComposeDirectory(composeID uuid.UUID) string {
+	return fmt.Sprintf("%s/outputs/%s", *s.stateDir, composeID.String())
 }
 
-func (s *Store) getImageLocationForLocalTarget(composeID uuid.UUID) string {
-	// This might result in conflicts because one compose can have multiple images, but (!) the Weldr API
-	// does not support multiple images per build and the RCM API does not support Local target, so it does
-	// not really matter any more.
-	return *s.stateDir + "/outputs/" + composeID.String()
+func (s *Store) getImageBuildDirectory(composeID uuid.UUID, imageBuildID int) string {
+	return fmt.Sprintf("%s/%d", s.getComposeDirectory(composeID), imageBuildID)
 }
 
 func (s *Store) PushCompose(composeID uuid.UUID, bp *blueprint.Blueprint, checksums map[string]string, arch, composeType string, size uint64, uploadTarget *target.Target) error {
@@ -469,7 +515,7 @@ func (s *Store) PushCompose(composeID uuid.UUID, bp *blueprint.Blueprint, checks
 	}
 
 	if s.stateDir != nil {
-		outputDir := s.getImageLocationForLocalTarget(composeID)
+		outputDir := s.getImageBuildDirectory(composeID, 0)
 
 		err := os.MkdirAll(outputDir, 0755)
 		if err != nil {
@@ -477,9 +523,7 @@ func (s *Store) PushCompose(composeID uuid.UUID, bp *blueprint.Blueprint, checks
 		}
 
 		targets = append(targets, target.NewLocalTarget(
-			&target.LocalTargetOptions{
-				Location: outputDir,
-			},
+			&target.LocalTargetOptions{},
 		))
 	}
 
@@ -554,7 +598,7 @@ func (s *Store) PushComposeRequest(request common.ComposeRequest) error {
 		panic("Multiple image requests are not yet properly implemented")
 	}
 
-	for n, imageRequest := range request.RequestedImages {
+	for imageBuildID, imageRequest := range request.RequestedImages {
 		// TODO: handle custom upload targets
 		// TODO: this requires changes in the Compose Request struct
 		// TODO: implment support for no checksums
@@ -568,10 +612,17 @@ func (s *Store) PushComposeRequest(request common.ComposeRequest) error {
 			return err
 		}
 
+		if s.stateDir != nil {
+			err = os.MkdirAll(s.getImageBuildDirectory(request.ComposeID, imageBuildID), 0755)
+			if err != nil {
+				return err
+			}
+		}
+
 		// This will make the job submission atomic: either all of them or none of them
 		newJobs = append(newJobs, Job{
 			ComposeID:    request.ComposeID,
-			ImageBuildID: n,
+			ImageBuildID: imageBuildID,
 			Distro:       distroString,
 			Pipeline:     pipelineStruct,
 			Targets:      []*target.Target{},
@@ -633,7 +684,7 @@ func (s *Store) DeleteCompose(id uuid.UUID) error {
 			// TODO: we need to rename the files as the compose will have multiple images
 			for _, imageBuild := range compose.ImageBuilds {
 				if imageBuild.QueueStatus == common.IBFinished || imageBuild.QueueStatus == common.IBFailed {
-					err = os.RemoveAll(s.getImageLocationForLocalTarget(id))
+					err = os.RemoveAll(s.getComposeDirectory(id))
 					if err != nil {
 						return err
 					}
@@ -683,7 +734,7 @@ func (s *Store) PopJob() Job {
 }
 
 // UpdateImageBuildInCompose sets the status and optionally also the final image.
-func (s *Store) UpdateImageBuildInCompose(composeID uuid.UUID, imageBuildID int, status common.ImageBuildState, image *compose.Image, result *common.ComposeResult) error {
+func (s *Store) UpdateImageBuildInCompose(composeID uuid.UUID, imageBuildID int, status common.ImageBuildState, result *common.ComposeResult) error {
 	return s.change(func() error {
 		// Check that the compose exists
 		currentCompose, exists := s.Composes[composeID]
@@ -697,7 +748,7 @@ func (s *Store) UpdateImageBuildInCompose(composeID uuid.UUID, imageBuildID int,
 
 		// write result into file
 		if s.stateDir != nil && result != nil {
-			f, err := os.Create(s.getImageLocationForLocalTarget(composeID) + "/result.json")
+			f, err := os.Create(s.getImageBuildDirectory(composeID, imageBuildID) + "/result.json")
 
 			if err != nil {
 				return fmt.Errorf("cannot open result.json for job %v: %#v", composeID, err)
@@ -717,9 +768,6 @@ func (s *Store) UpdateImageBuildInCompose(composeID uuid.UUID, imageBuildID int,
 		// In case the image build is done, store the time and possibly also the image
 		if status == common.IBFinished || status == common.IBFailed {
 			currentCompose.ImageBuilds[imageBuildID].JobFinished = time.Now()
-			if status == common.IBFinished {
-				currentCompose.ImageBuilds[imageBuildID].Image = image
-			}
 		}
 
 		s.Composes[composeID] = currentCompose
@@ -728,6 +776,40 @@ func (s *Store) UpdateImageBuildInCompose(composeID uuid.UUID, imageBuildID int,
 	})
 }
 
+func (s *Store) AddImageToImageUpload(composeID uuid.UUID, imageBuildID int, reader io.Reader) error {
+	currentCompose, exists := s.Composes[composeID]
+	if !exists {
+		return &NotFoundError{"compose does not exist"}
+	}
+
+	imageBuild := currentCompose.ImageBuilds[imageBuildID]
+	if !imageBuild.HasLocalTarget() {
+		return &NoLocalTargetError{fmt.Sprintf("image upload requested for compse %s and image build %d but it has no local target", composeID.String(), imageBuildID)}
+	}
+
+	imageType, _ := imageBuild.ImageType.ToCompatString()
+	filename, _, err := s.distro.FilenameFromType(imageType)
+
+	if err != nil {
+		return &InvalidRequestError{err.Error()}
+	}
+
+	path := fmt.Sprintf("%s/%s", s.getImageBuildDirectory(composeID, imageBuildID), filename)
+	f, err := os.Create(path)
+
+	if err != nil {
+		return err
+	}
+
+	_, err = io.Copy(f, reader)
+
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func (s *Store) PushSource(source SourceConfig) {
 	// FIXME: handle or comment this possible error
 	_ = s.change(func() error {
diff --git a/internal/target/local_target.go b/internal/target/local_target.go
index 35209d524..eb2fa9dbc 100644
--- a/internal/target/local_target.go
+++ b/internal/target/local_target.go
@@ -1,7 +1,6 @@
 package target
 
 type LocalTargetOptions struct {
-	Location string `json:"location"`
 }
 
 func (LocalTargetOptions) isTargetOptions() {}
diff --git a/internal/upload/awsupload/awsupload.go b/internal/upload/awsupload/awsupload.go
index 429ca886b..255307adf 100644
--- a/internal/upload/awsupload/awsupload.go
+++ b/internal/upload/awsupload/awsupload.go
@@ -132,6 +132,9 @@ func (a *AWS) Register(name, bucket, key string) (*string, error) {
 			},
 		},
 	)
+	if err != nil {
+		return nil, err
+	}
 
 	snapshotId := importOutput.ImportSnapshotTasks[0].SnapshotTaskDetail.SnapshotId
 
diff --git a/internal/weldr/api.go b/internal/weldr/api.go
index c7c8d9f84..80287a9f5 100644
--- a/internal/weldr/api.go
+++ b/internal/weldr/api.go
@@ -11,8 +11,6 @@ import (
 	"net"
 	"net/http"
 	"net/url"
-	"os"
-	"path/filepath"
 	"sort"
 	"strconv"
 	"strings"
@@ -1522,9 +1520,7 @@ func (api *API) composeInfoHandler(writer http.ResponseWriter, request *http.Req
 	// 1st build is considered
 	reply.ComposeType, _ = compose.ImageBuilds[0].ImageType.ToCompatString()
 	reply.QueueStatus = compose.GetState().ToString()
-	if compose.ImageBuilds[0].Image != nil {
-		reply.ImageSize = compose.ImageBuilds[0].Size
-	}
+	reply.ImageSize = compose.ImageBuilds[0].Size
 
 	if isRequestVersionAtLeast(params, 1) {
 		reply.Uploads = TargetsToUploadResponses(compose.ImageBuilds[0].Targets)
@@ -1568,18 +1564,22 @@ func (api *API) composeImageHandler(writer http.ResponseWriter, request *http.Re
 		return
 	}
 
-	if compose.ImageBuilds[0].Image == nil {
+	imageBuild := compose.ImageBuilds[0]
+	imageType, _ := imageBuild.ImageType.ToCompatString()
+	imageName, imageMime, err := api.distro.FilenameFromType(imageType)
+
+	if err != nil {
 		errors := responseError{
-			ID:  "BuildMissingFile",
-			Msg: fmt.Sprintf("Compose %s doesn't have an image assigned", uuidString),
+			ID:  "BadCompose",
+			Msg: fmt.Sprintf("Compose %s is ill-formed: output type %v is invalid for distro %s", uuidString, imageBuild.ImageType, api.distro.Name()),
 		}
-		statusResponseError(writer, http.StatusBadRequest, errors)
+		statusResponseError(writer, http.StatusInternalServerError, errors)
 		return
 	}
 
-	imageName := filepath.Base(compose.ImageBuilds[0].Image.Path)
+	reader, fileSize, err := api.store.GetImageBuildImage(uuid, 0)
 
-	file, err := os.Open(compose.ImageBuilds[0].Image.Path)
+	// TODO: this might return misleading error
 	if err != nil {
 		errors := responseError{
 			ID:  "BuildMissingFile",
@@ -1590,10 +1590,10 @@ func (api *API) composeImageHandler(writer http.ResponseWriter, request *http.Re
 	}
 
 	writer.Header().Set("Content-Disposition", "attachment; filename="+uuid.String()+"-"+imageName)
-	writer.Header().Set("Content-Type", compose.ImageBuilds[0].Image.Mime)
-	writer.Header().Set("Content-Length", fmt.Sprintf("%d", compose.ImageBuilds[0].Image.Size))
+	writer.Header().Set("Content-Type", imageMime)
+	writer.Header().Set("Content-Length", fmt.Sprintf("%d", fileSize))
 
-	io.Copy(writer, file)
+	io.Copy(writer, reader)
 }
 
 func (api *API) composeLogsHandler(writer http.ResponseWriter, request *http.Request, params httprouter.Params) {
@@ -1631,7 +1631,7 @@ func (api *API) composeLogsHandler(writer http.ResponseWriter, request *http.Req
 		return
 	}
 
-	resultReader, err := api.store.GetComposeResult(id)
+	resultReader, err := api.store.GetImageBuildResult(id, 0)
 
 	if err != nil {
 		errors := responseError{
@@ -1716,7 +1716,7 @@ func (api *API) composeLogHandler(writer http.ResponseWriter, request *http.Requ
 		return
 	}
 
-	resultReader, err := api.store.GetComposeResult(id)
+	resultReader, err := api.store.GetImageBuildResult(id, 0)
 
 	if err != nil {
 		errors := responseError{
diff --git a/internal/weldr/compose.go b/internal/weldr/compose.go
index 404b846d4..1ca058a4e 100644
--- a/internal/weldr/compose.go
+++ b/internal/weldr/compose.go
@@ -4,7 +4,6 @@ import (
 	"github.com/google/uuid"
 	"github.com/osbuild/osbuild-composer/internal/common"
 	"github.com/osbuild/osbuild-composer/internal/compose"
-	"log"
 	"sort"
 )
 
@@ -13,7 +12,7 @@ type ComposeEntry struct {
 	Blueprint   string                 `json:"blueprint"`
 	Version     string                 `json:"version"`
 	ComposeType common.ImageType       `json:"compose_type"`
-	ImageSize   uint64                 `json:"image_size"`
+	ImageSize   uint64                 `json:"image_size"` // This is user-provided image size, not actual file size
 	QueueStatus common.ImageBuildState `json:"queue_status"`
 	JobCreated  float64                `json:"job_created"`
 	JobStarted  float64                `json:"job_started,omitempty"`
@@ -43,13 +42,7 @@ func composeToComposeEntry(id uuid.UUID, compose compose.Compose, includeUploads
 		composeEntry.JobStarted = float64(compose.ImageBuilds[0].JobStarted.UnixNano()) / 1000000000
 
 	case common.IBFinished:
-		if compose.ImageBuilds[0].Image != nil {
-			composeEntry.ImageSize = compose.ImageBuilds[0].Size
-		} else {
-			log.Printf("finished compose with id %s has nil image\n", id.String())
-			composeEntry.ImageSize = 0
-		}
-
+		composeEntry.ImageSize = compose.ImageBuilds[0].Size
 		composeEntry.JobCreated = float64(compose.ImageBuilds[0].JobCreated.UnixNano()) / 1000000000
 		composeEntry.JobStarted = float64(compose.ImageBuilds[0].JobStarted.UnixNano()) / 1000000000
 		composeEntry.JobFinished = float64(compose.ImageBuilds[0].JobFinished.UnixNano()) / 1000000000


Test patch:
diff --git a/internal/jobqueue/api_test.go b/internal/jobqueue/api_test.go
index 5e11fefe5..39787be11 100644
--- a/internal/jobqueue/api_test.go
+++ b/internal/jobqueue/api_test.go
@@ -16,20 +16,20 @@ import (
 
 func TestBasic(t *testing.T) {
 	var cases = []struct {
-		Method         string
-		Path           string
-		Body           string
-		ExpectedStatus int
-		ExpectedJSON   string
+		Method           string
+		Path             string
+		Body             string
+		ExpectedStatus   int
+		ExpectedResponse string
 	}{
 		// Create job with invalid body
-		{"POST", "/job-queue/v1/jobs", ``, http.StatusBadRequest, ``},
+		{"POST", "/job-queue/v1/jobs", ``, http.StatusBadRequest, `invalid request: EOF`},
 		// Update job with invalid ID
-		{"PATCH", "/job-queue/v1/jobs/foo", `{"status":"RUNNING"}`, http.StatusBadRequest, ``},
+		{"PATCH", "/job-queue/v1/jobs/foo/builds/0", `{"status":"RUNNING"}`, http.StatusBadRequest, `invalid compose id: invalid UUID length: 3`},
 		// Update job that does not exist, with invalid body
-		{"PATCH", "/job-queue/v1/jobs/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", ``, http.StatusBadRequest, ``},
+		{"PATCH", "/job-queue/v1/jobs/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa/builds/0", ``, http.StatusBadRequest, `invalid status: EOF`},
 		// Update job that does not exist
-		{"PATCH", "/job-queue/v1/jobs/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", `{"image_build_id": 0, "status":"RUNNING"}`, http.StatusNotFound, ``},
+		{"PATCH", "/job-queue/v1/jobs/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa/builds/0", `{"status":"RUNNING"}`, http.StatusNotFound, `compose does not exist`},
 	}
 
 	for _, c := range cases {
@@ -37,7 +37,7 @@ func TestBasic(t *testing.T) {
 		registry := distro.NewRegistry([]string{"."})
 		api := jobqueue.New(nil, store.New(nil, distroStruct, *registry))
 
-		test.TestRoute(t, api, false, c.Method, c.Path, c.Body, c.ExpectedStatus, c.ExpectedJSON)
+		test.TestNonJsonRoute(t, api, false, c.Method, c.Path, c.Body, c.ExpectedStatus, c.ExpectedResponse)
 	}
 }
 
@@ -57,7 +57,7 @@ func TestCreate(t *testing.T) {
 		`{"distro":"fedora-30","id":"ffffffff-ffff-ffff-ffff-ffffffffffff","image_build_id":0,"output_type":"qcow2","pipeline":{},"targets":[]}`, "created", "uuid")
 }
 
-func testUpdateTransition(t *testing.T, from, to string, expectedStatus int) {
+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{"."})
@@ -72,43 +72,44 @@ func testUpdateTransition(t *testing.T, from, to string, expectedStatus int) {
 		if from != "WAITING" {
 			test.SendHTTP(api, false, "POST", "/job-queue/v1/jobs", `{}`)
 			if from != "RUNNING" {
-				test.SendHTTP(api, false, "PATCH", "/job-queue/v1/jobs/ffffffff-ffff-ffff-ffff-ffffffffffff", `{"status":"`+from+`"}`)
+				test.SendHTTP(api, false, "PATCH", "/job-queue/v1/jobs/ffffffff-ffff-ffff-ffff-ffffffffffff/builds/0", `{"status":"`+from+`"}`)
 			}
 		}
 	}
 
-	test.TestRoute(t, api, false, "PATCH", "/job-queue/v1/jobs/ffffffff-ffff-ffff-ffff-ffffffffffff", `{"status":"`+to+`"}`, expectedStatus, ``)
+	test.TestNonJsonRoute(t, api, false, "PATCH", "/job-queue/v1/jobs/ffffffff-ffff-ffff-ffff-ffffffffffff/builds/0", `{"status":"`+to+`"}`, expectedStatus, expectedResponse)
 }
 
 func TestUpdate(t *testing.T) {
 	var cases = []struct {
-		From           string
-		To             string
-		ExpectedStatus int
+		From             string
+		To               string
+		ExpectedStatus   int
+		ExpectedResponse string
 	}{
-		{"VOID", "WAITING", http.StatusNotFound},
-		{"VOID", "RUNNING", http.StatusNotFound},
-		{"VOID", "FINISHED", http.StatusNotFound},
-		{"VOID", "FAILED", http.StatusNotFound},
-		{"WAITING", "WAITING", http.StatusNotFound},
-		{"WAITING", "RUNNING", http.StatusNotFound},
-		{"WAITING", "FINISHED", http.StatusNotFound},
-		{"WAITING", "FAILED", http.StatusNotFound},
-		{"RUNNING", "WAITING", http.StatusBadRequest},
-		{"RUNNING", "RUNNING", http.StatusOK},
-		{"RUNNING", "FINISHED", http.StatusOK},
-		{"RUNNING", "FAILED", http.StatusOK},
-		{"FINISHED", "WAITING", http.StatusBadRequest},
-		{"FINISHED", "RUNNING", http.StatusBadRequest},
-		{"FINISHED", "FINISHED", http.StatusBadRequest},
-		{"FINISHED", "FAILED", http.StatusBadRequest},
-		{"FAILED", "WAITING", http.StatusBadRequest},
-		{"FAILED", "RUNNING", http.StatusBadRequest},
-		{"FAILED", "FINISHED", http.StatusBadRequest},
-		{"FAILED", "FAILED", http.StatusBadRequest},
+		{"VOID", "WAITING", http.StatusNotFound, "compose does not exist"},
+		{"VOID", "RUNNING", http.StatusNotFound, "compose does not exist"},
+		{"VOID", "FINISHED", http.StatusNotFound, "compose does not exist"},
+		{"VOID", "FAILED", http.StatusNotFound, "compose does not exist"},
+		{"WAITING", "WAITING", http.StatusNotFound, "compose has not been popped"},
+		{"WAITING", "RUNNING", http.StatusNotFound, "compose has not been popped"},
+		{"WAITING", "FINISHED", http.StatusNotFound, "compose has not been popped"},
+		{"WAITING", "FAILED", http.StatusNotFound, "compose has not been popped"},
+		{"RUNNING", "WAITING", http.StatusBadRequest, "invalid state transition: image build cannot be moved into waiting state"},
+		{"RUNNING", "RUNNING", http.StatusOK, ""},
+		{"RUNNING", "FINISHED", http.StatusOK, ""},
+		{"RUNNING", "FAILED", http.StatusOK, ""},
+		{"FINISHED", "WAITING", http.StatusBadRequest, "invalid state transition: image build cannot be moved into waiting state"},
+		{"FINISHED", "RUNNING", http.StatusBadRequest, "invalid state transition: only waiting image build can be transitioned into running state"},
+		{"FINISHED", "FINISHED", http.StatusBadRequest, "invalid state transition: only running image build can be transitioned into finished or failed state"},
+		{"FINISHED", "FAILED", http.StatusBadRequest, "invalid state transition: only running image build can be transitioned into finished or failed state"},
+		{"FAILED", "WAITING", http.StatusBadRequest, "invalid state transition: image build cannot be moved into waiting state"},
+		{"FAILED", "RUNNING", http.StatusBadRequest, "invalid state transition: only waiting image build can be transitioned into running state"},
+		{"FAILED", "FINISHED", http.StatusBadRequest, "invalid state transition: only running image build can be transitioned into finished or failed state"},
+		{"FAILED", "FAILED", http.StatusBadRequest, "invalid state transition: only running image build can be transitioned into finished or failed state"},
 	}
 
 	for _, c := range cases {
-		testUpdateTransition(t, c.From, c.To, c.ExpectedStatus)
+		testUpdateTransition(t, c.From, c.To, c.ExpectedStatus, c.ExpectedResponse)
 	}
 }
diff --git a/internal/upload/awsupload/awsupload_test.go b/internal/upload/awsupload/awsupload_test.go
index 111e9f5bb..548eed6e9 100644
--- a/internal/upload/awsupload/awsupload_test.go
+++ b/internal/upload/awsupload/awsupload_test.go
@@ -43,13 +43,17 @@ func generateRandomFile(pattern string, length int) (string, []byte, error) {
 	cmd := exec.Command("dd", "bs=1", countArg, "if=/dev/urandom", fmt.Sprintf("of=%s", fileName))
 	err = cmd.Run()
 
+	if err != nil {
+		return "", []byte{}, err
+	}
+
 	contents, err := ioutil.ReadAll(f)
 	if err != nil {
 		return "", []byte{}, err
 	}
 
 	if err := f.Close(); err != nil {
-		return ",", []byte{}, err
+		return "", []byte{}, err
 	}
 
 	return fileName, contents, err